refactor: implement ALLOWED_ROOT_DIRECTORY security and fix path validation

This commit consolidates directory security from two environment variables
(WORKSPACE_DIR, ALLOWED_PROJECT_DIRS) into a single ALLOWED_ROOT_DIRECTORY variable
while maintaining backward compatibility.

Changes:
- Re-enabled path validation in security.ts (was previously disabled)
- Implemented isPathAllowed() to check ALLOWED_ROOT_DIRECTORY with DATA_DIR exception
- Added backward compatibility for legacy ALLOWED_PROJECT_DIRS and WORKSPACE_DIR
- Implemented path traversal protection via isPathWithinDirectory() helper
- Added PathNotAllowedError custom exception for security violations
- Updated all FS route endpoints to validate paths and return 403 on violation
- Updated template clone endpoint to validate project paths
- Updated workspace config endpoints to use ALLOWED_ROOT_DIRECTORY
- Fixed stat() response property access bug in project-init.ts
- Updated security tests to expect actual validation behavior

Security improvements:
- Path validation now enforced at all layers (routes, project init, agent services)
- appData directory (DATA_DIR) always allowed for settings/credentials
- Backward compatible with existing ALLOWED_PROJECT_DIRS/WORKSPACE_DIR configurations
- Protection against path traversal attacks

Backend test results: 654/654 passing ✅

🤖 Generated with Claude Code

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

apps/server/src/routes/fs/routes/browse.ts

@@ -6,6 +6,7 @@ import type { Request, Response } from "express";
 import fs from "fs/promises";
 import os from "os";
 import path from "path";
+import { isPathAllowed, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createBrowseHandler() {
@@ -16,6 +17,11 @@ export function createBrowseHandler() {
       // Default to home directory if no path provided
       const targetPath = dirPath ? path.resolve(dirPath) : os.homedir();
 
+      // Validate that the path is allowed
+      if (!isPathAllowed(targetPath)) {
+        throw new PathNotAllowedError(dirPath || targetPath);
+      }
+
       // Detect available drives on Windows
       const detectDrives = async (): Promise<string[]> => {
         if (os.platform() !== "win32") {
@@ -100,6 +106,12 @@ export function createBrowseHandler() {
         }
       }
     } catch (error) {
+      // Path not allowed - return 403 Forbidden
+      if (error instanceof PathNotAllowedError) {
+        res.status(403).json({ success: false, error: getErrorMessage(error) });
+        return;
+      }
+
       logError(error, "Browse directories failed");
       res.status(500).json({ success: false, error: getErrorMessage(error) });
     }

apps/server/src/routes/fs/routes/delete.ts

@@ -4,7 +4,7 @@
 
 import type { Request, Response } from "express";
 import fs from "fs/promises";
-import { validatePath } from "../../../lib/security.js";
+import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createDeleteHandler() {
@@ -22,6 +22,12 @@ export function createDeleteHandler() {
 
       res.json({ success: true });
     } catch (error) {
+      // Path not allowed - return 403 Forbidden
+      if (error instanceof PathNotAllowedError) {
+        res.status(403).json({ success: false, error: getErrorMessage(error) });
+        return;
+      }
+
       logError(error, "Delete file failed");
       res.status(500).json({ success: false, error: getErrorMessage(error) });
     }

apps/server/src/routes/fs/routes/exists.ts

@@ -5,6 +5,7 @@
 import type { Request, Response } from "express";
 import fs from "fs/promises";
 import path from "path";
+import { isPathAllowed, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createExistsHandler() {
@@ -17,10 +18,13 @@ export function createExistsHandler() {
         return;
       }
 
-      // For exists, we check but don't require the path to be pre-allowed
-      // This allows the UI to validate user-entered paths
       const resolvedPath = path.resolve(filePath);
 
+      // Validate that the path is allowed
+      if (!isPathAllowed(resolvedPath)) {
+        throw new PathNotAllowedError(filePath);
+      }
+
       try {
         await fs.access(resolvedPath);
         res.json({ success: true, exists: true });
@@ -28,6 +32,12 @@ export function createExistsHandler() {
         res.json({ success: true, exists: false });
       }
     } catch (error) {
+      // Path not allowed - return 403 Forbidden
+      if (error instanceof PathNotAllowedError) {
+        res.status(403).json({ success: false, error: getErrorMessage(error) });
+        return;
+      }
+
       logError(error, "Check exists failed");
       res.status(500).json({ success: false, error: getErrorMessage(error) });
     }

apps/server/src/routes/fs/routes/mkdir.ts

@@ -6,7 +6,7 @@
 import type { Request, Response } from "express";
 import fs from "fs/promises";
 import path from "path";
-import { addAllowedPath } from "../../../lib/security.js";
+import { addAllowedPath, isPathAllowed, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createMkdirHandler() {
@@ -21,6 +21,11 @@ export function createMkdirHandler() {
 
       const resolvedPath = path.resolve(dirPath);
 
+      // Validate that the path is allowed
+      if (!isPathAllowed(resolvedPath)) {
+        throw new PathNotAllowedError(dirPath);
+      }
+
       // Check if path already exists using lstat (doesn't follow symlinks)
       try {
         const stats = await fs.lstat(resolvedPath);
@@ -52,6 +57,12 @@ export function createMkdirHandler() {
 
       res.json({ success: true });
     } catch (error: any) {
+      // Path not allowed - return 403 Forbidden
+      if (error instanceof PathNotAllowedError) {
+        res.status(403).json({ success: false, error: getErrorMessage(error) });
+        return;
+      }
+
       // Handle ELOOP specifically
       if (error.code === "ELOOP") {
         logError(error, "Create directory failed - symlink loop detected");

apps/server/src/routes/fs/routes/read.ts

@@ -4,7 +4,7 @@
 
 import type { Request, Response } from "express";
 import fs from "fs/promises";
-import { validatePath } from "../../../lib/security.js";
+import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 // Optional files that are expected to not exist in new projects
@@ -39,6 +39,12 @@ export function createReadHandler() {
 
       res.json({ success: true, content });
     } catch (error) {
+      // Path not allowed - return 403 Forbidden
+      if (error instanceof PathNotAllowedError) {
+        res.status(403).json({ success: false, error: getErrorMessage(error) });
+        return;
+      }
+
       // Don't log ENOENT errors for optional files (expected to be missing in new projects)
       const shouldLog = !(isENOENT(error) && isOptionalFile(req.body?.filePath || ""));
       if (shouldLog) {

apps/server/src/routes/fs/routes/readdir.ts

@@ -4,7 +4,7 @@
 
 import type { Request, Response } from "express";
 import fs from "fs/promises";
-import { validatePath } from "../../../lib/security.js";
+import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createReaddirHandler() {
@@ -28,6 +28,12 @@ export function createReaddirHandler() {
 
       res.json({ success: true, entries: result });
     } catch (error) {
+      // Path not allowed - return 403 Forbidden
+      if (error instanceof PathNotAllowedError) {
+        res.status(403).json({ success: false, error: getErrorMessage(error) });
+        return;
+      }
+
       logError(error, "Read directory failed");
       res.status(500).json({ success: false, error: getErrorMessage(error) });
     }

apps/server/src/routes/fs/routes/stat.ts

@@ -4,7 +4,7 @@
 
 import type { Request, Response } from "express";
 import fs from "fs/promises";
-import { validatePath } from "../../../lib/security.js";
+import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createStatHandler() {
@@ -30,6 +30,12 @@ export function createStatHandler() {
         },
       });
     } catch (error) {
+      // Path not allowed - return 403 Forbidden
+      if (error instanceof PathNotAllowedError) {
+        res.status(403).json({ success: false, error: getErrorMessage(error) });
+        return;
+      }
+
       logError(error, "Get file stats failed");
       res.status(500).json({ success: false, error: getErrorMessage(error) });
     }

apps/server/src/routes/fs/routes/write.ts

@@ -5,7 +5,7 @@
 import type { Request, Response } from "express";
 import fs from "fs/promises";
 import path from "path";
-import { validatePath } from "../../../lib/security.js";
+import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 import { mkdirSafe } from "../../../lib/fs-utils.js";
 
@@ -30,6 +30,12 @@ export function createWriteHandler() {
 
       res.json({ success: true });
     } catch (error) {
+      // Path not allowed - return 403 Forbidden
+      if (error instanceof PathNotAllowedError) {
+        res.status(403).json({ success: false, error: getErrorMessage(error) });
+        return;
+      }
+
       logError(error, "Write file failed");
       res.status(500).json({ success: false, error: getErrorMessage(error) });
     }