diff --git a/apps/server/.env.example b/apps/server/.env.example index 5d9b7118..9fbb4cbd 100644 --- a/apps/server/.env.example +++ b/apps/server/.env.example @@ -16,9 +16,11 @@ ANTHROPIC_API_KEY=sk-ant-... # If set, all API requests must include X-API-Key header AUTOMAKER_API_KEY= -# Restrict file operations to these directories (comma-separated) -# Important for security in multi-tenant environments -ALLOWED_PROJECT_DIRS=/home/user/projects,/var/www +# Root directory for projects and file operations +# If set, users can only create/open projects and files within this directory +# Recommended for sandboxed deployments (Docker, restricted environments) +# Example: ALLOWED_ROOT_DIRECTORY=/projects +ALLOWED_ROOT_DIRECTORY= # CORS origin - which domains can access the API # Use "*" for development, set specific origin for production @@ -34,13 +36,6 @@ PORT=3008 # Data directory for sessions and metadata DATA_DIR=./data -# ============================================ -# OPTIONAL - Additional AI Providers -# ============================================ - -# Google API key (for future Gemini support) -GOOGLE_API_KEY= - # ============================================ # OPTIONAL - Terminal Access # ============================================ diff --git a/apps/server/Dockerfile b/apps/server/Dockerfile index 8c019a2f..67ecedf0 100644 --- a/apps/server/Dockerfile +++ b/apps/server/Dockerfile @@ -26,6 +26,14 @@ RUN npm run build --workspace=apps/server # Production stage FROM node:20-alpine +# Install git, curl, and GitHub CLI (pinned version for reproducible builds) +RUN apk add --no-cache git curl && \ + GH_VERSION="2.63.2" && \ + curl -L "https://github.com/cli/cli/releases/download/v${GH_VERSION}/gh_${GH_VERSION}_linux_amd64.tar.gz" -o gh.tar.gz && \ + tar -xzf gh.tar.gz && \ + mv "gh_${GH_VERSION}_linux_amd64/bin/gh" /usr/local/bin/gh && \ + rm -rf gh.tar.gz "gh_${GH_VERSION}_linux_amd64" + WORKDIR /app # Create non-root user diff --git a/apps/server/src/lib/secure-fs.ts b/apps/server/src/lib/secure-fs.ts new file mode 100644 index 00000000..eab1be0a --- /dev/null +++ b/apps/server/src/lib/secure-fs.ts @@ -0,0 +1,23 @@ +/** + * Re-export secure file system utilities from @automaker/platform + * This file exists for backward compatibility with existing imports + */ + +import { secureFs } from "@automaker/platform"; + +export const { + access, + readFile, + writeFile, + mkdir, + readdir, + stat, + rm, + unlink, + copyFile, + appendFile, + rename, + lstat, + joinPath, + resolvePath, +} = secureFs; diff --git a/apps/server/src/middleware/validate-paths.ts b/apps/server/src/middleware/validate-paths.ts new file mode 100644 index 00000000..5973451f --- /dev/null +++ b/apps/server/src/middleware/validate-paths.ts @@ -0,0 +1,69 @@ +/** + * Middleware for validating path parameters against ALLOWED_ROOT_DIRECTORY + * Provides a clean, reusable way to validate paths without repeating the same + * try-catch block in every route handler + */ + +import type { Request, Response, NextFunction } from "express"; +import { validatePath, PathNotAllowedError } from "@automaker/platform"; + +/** + * Creates a middleware that validates specified path parameters in req.body + * @param paramNames - Names of parameters to validate (e.g., 'projectPath', 'worktreePath') + * @example + * router.post('/create', validatePathParams('projectPath'), handler); + * router.post('/delete', validatePathParams('projectPath', 'worktreePath'), handler); + * router.post('/send', validatePathParams('workingDirectory?', 'imagePaths[]'), handler); + * + * Special syntax: + * - 'paramName?' - Optional parameter (only validated if present) + * - 'paramName[]' - Array parameter (validates each element) + */ +export function validatePathParams(...paramNames: string[]) { + return (req: Request, res: Response, next: NextFunction): void => { + try { + for (const paramName of paramNames) { + // Handle optional parameters (paramName?) + if (paramName.endsWith("?")) { + const actualName = paramName.slice(0, -1); + const value = req.body[actualName]; + if (value) { + validatePath(value); + } + continue; + } + + // Handle array parameters (paramName[]) + if (paramName.endsWith("[]")) { + const actualName = paramName.slice(0, -2); + const values = req.body[actualName]; + if (Array.isArray(values) && values.length > 0) { + for (const value of values) { + validatePath(value); + } + } + continue; + } + + // Handle regular parameters + const value = req.body[paramName]; + if (value) { + validatePath(value); + } + } + + next(); + } catch (error) { + if (error instanceof PathNotAllowedError) { + res.status(403).json({ + success: false, + error: error.message, + }); + return; + } + + // Re-throw unexpected errors + throw error; + } + }; +} diff --git a/apps/server/src/routes/agent/index.ts b/apps/server/src/routes/agent/index.ts index ed12e296..61f34656 100644 --- a/apps/server/src/routes/agent/index.ts +++ b/apps/server/src/routes/agent/index.ts @@ -5,6 +5,7 @@ import { Router } from "express"; import { AgentService } from "../../services/agent-service.js"; import type { EventEmitter } from "../../lib/events.js"; +import { validatePathParams } from "../../middleware/validate-paths.js"; import { createStartHandler } from "./routes/start.js"; import { createSendHandler } from "./routes/send.js"; import { createHistoryHandler } from "./routes/history.js"; @@ -18,8 +19,8 @@ export function createAgentRoutes( ): Router { const router = Router(); - router.post("/start", createStartHandler(agentService)); - router.post("/send", createSendHandler(agentService)); + router.post("/start", validatePathParams("workingDirectory?"), createStartHandler(agentService)); + router.post("/send", validatePathParams("workingDirectory?", "imagePaths[]"), createSendHandler(agentService)); router.post("/history", createHistoryHandler(agentService)); router.post("/stop", createStopHandler(agentService)); router.post("/clear", createClearHandler(agentService)); diff --git a/apps/server/src/routes/agent/routes/send.ts b/apps/server/src/routes/agent/routes/send.ts index 6206bca9..b39ede76 100644 --- a/apps/server/src/routes/agent/routes/send.ts +++ b/apps/server/src/routes/agent/routes/send.ts @@ -6,7 +6,6 @@ import type { Request, Response } from "express"; import { AgentService } from "../../../services/agent-service.js"; import { createLogger } from "@automaker/utils"; import { getErrorMessage, logError } from "../common.js"; - const logger = createLogger("Agent"); export function createSendHandler(agentService: AgentService) { diff --git a/apps/server/src/routes/agent/routes/start.ts b/apps/server/src/routes/agent/routes/start.ts index 9088e1c9..9f7d8da5 100644 --- a/apps/server/src/routes/agent/routes/start.ts +++ b/apps/server/src/routes/agent/routes/start.ts @@ -6,7 +6,6 @@ import type { Request, Response } from "express"; import { AgentService } from "../../../services/agent-service.js"; import { createLogger } from "@automaker/utils"; import { getErrorMessage, logError } from "../common.js"; - const logger = createLogger("Agent"); export function createStartHandler(agentService: AgentService) { diff --git a/apps/server/src/routes/auto-mode/index.ts b/apps/server/src/routes/auto-mode/index.ts index 8ad4510c..6bdd7dbb 100644 --- a/apps/server/src/routes/auto-mode/index.ts +++ b/apps/server/src/routes/auto-mode/index.ts @@ -6,6 +6,7 @@ import { Router } from "express"; import type { AutoModeService } from "../../services/auto-mode-service.js"; +import { validatePathParams } from "../../middleware/validate-paths.js"; import { createStopFeatureHandler } from "./routes/stop-feature.js"; import { createStatusHandler } from "./routes/status.js"; import { createRunFeatureHandler } from "./routes/run-feature.js"; @@ -21,18 +22,19 @@ export function createAutoModeRoutes(autoModeService: AutoModeService): Router { const router = Router(); router.post("/stop-feature", createStopFeatureHandler(autoModeService)); - router.post("/status", createStatusHandler(autoModeService)); - router.post("/run-feature", createRunFeatureHandler(autoModeService)); - router.post("/verify-feature", createVerifyFeatureHandler(autoModeService)); - router.post("/resume-feature", createResumeFeatureHandler(autoModeService)); - router.post("/context-exists", createContextExistsHandler(autoModeService)); - router.post("/analyze-project", createAnalyzeProjectHandler(autoModeService)); + router.post("/status", validatePathParams("projectPath?"), createStatusHandler(autoModeService)); + router.post("/run-feature", validatePathParams("projectPath"), createRunFeatureHandler(autoModeService)); + router.post("/verify-feature", validatePathParams("projectPath"), createVerifyFeatureHandler(autoModeService)); + router.post("/resume-feature", validatePathParams("projectPath"), createResumeFeatureHandler(autoModeService)); + router.post("/context-exists", validatePathParams("projectPath"), createContextExistsHandler(autoModeService)); + router.post("/analyze-project", validatePathParams("projectPath"), createAnalyzeProjectHandler(autoModeService)); router.post( "/follow-up-feature", + validatePathParams("projectPath", "imagePaths[]"), createFollowUpFeatureHandler(autoModeService) ); - router.post("/commit-feature", createCommitFeatureHandler(autoModeService)); - router.post("/approve-plan", createApprovePlanHandler(autoModeService)); + router.post("/commit-feature", validatePathParams("projectPath", "worktreePath?"), createCommitFeatureHandler(autoModeService)); + router.post("/approve-plan", validatePathParams("projectPath"), createApprovePlanHandler(autoModeService)); return router; } diff --git a/apps/server/src/routes/features/index.ts b/apps/server/src/routes/features/index.ts index d4406766..dcd98f56 100644 --- a/apps/server/src/routes/features/index.ts +++ b/apps/server/src/routes/features/index.ts @@ -4,6 +4,7 @@ import { Router } from "express"; import { FeatureLoader } from "../../services/feature-loader.js"; +import { validatePathParams } from "../../middleware/validate-paths.js"; import { createListHandler } from "./routes/list.js"; import { createGetHandler } from "./routes/get.js"; import { createCreateHandler } from "./routes/create.js"; @@ -15,11 +16,11 @@ import { createGenerateTitleHandler } from "./routes/generate-title.js"; export function createFeaturesRoutes(featureLoader: FeatureLoader): Router { const router = Router(); - router.post("/list", createListHandler(featureLoader)); - router.post("/get", createGetHandler(featureLoader)); - router.post("/create", createCreateHandler(featureLoader)); - router.post("/update", createUpdateHandler(featureLoader)); - router.post("/delete", createDeleteHandler(featureLoader)); + router.post("/list", validatePathParams("projectPath"), createListHandler(featureLoader)); + router.post("/get", validatePathParams("projectPath"), createGetHandler(featureLoader)); + router.post("/create", validatePathParams("projectPath"), createCreateHandler(featureLoader)); + router.post("/update", validatePathParams("projectPath"), createUpdateHandler(featureLoader)); + router.post("/delete", validatePathParams("projectPath"), createDeleteHandler(featureLoader)); router.post("/agent-output", createAgentOutputHandler(featureLoader)); router.post("/generate-title", createGenerateTitleHandler()); diff --git a/apps/server/src/routes/features/routes/create.ts b/apps/server/src/routes/features/routes/create.ts index 62993190..cd95b487 100644 --- a/apps/server/src/routes/features/routes/create.ts +++ b/apps/server/src/routes/features/routes/create.ts @@ -5,7 +5,6 @@ import type { Request, Response } from "express"; import { FeatureLoader } from "../../../services/feature-loader.js"; import type { Feature } from "@automaker/types"; -import { addAllowedPath } from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; export function createCreateHandler(featureLoader: FeatureLoader) { @@ -17,18 +16,13 @@ export function createCreateHandler(featureLoader: FeatureLoader) { }; if (!projectPath || !feature) { - res - .status(400) - .json({ - success: false, - error: "projectPath and feature are required", - }); + res.status(400).json({ + success: false, + error: "projectPath and feature are required", + }); return; } - // Add project path to allowed paths - addAllowedPath(projectPath); - const created = await featureLoader.create(projectPath, feature); res.json({ success: true, feature: created }); } catch (error) { diff --git a/apps/server/src/routes/features/routes/list.ts b/apps/server/src/routes/features/routes/list.ts index 261335ac..cc20b1a1 100644 --- a/apps/server/src/routes/features/routes/list.ts +++ b/apps/server/src/routes/features/routes/list.ts @@ -4,7 +4,6 @@ import type { Request, Response } from "express"; import { FeatureLoader } from "../../../services/feature-loader.js"; -import { addAllowedPath } from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; export function createListHandler(featureLoader: FeatureLoader) { @@ -19,9 +18,6 @@ export function createListHandler(featureLoader: FeatureLoader) { return; } - // Add project path to allowed paths - addAllowedPath(projectPath); - const features = await featureLoader.getAll(projectPath); res.json({ success: true, features }); } catch (error) { diff --git a/apps/server/src/routes/fs/routes/browse.ts b/apps/server/src/routes/fs/routes/browse.ts index 7579fb34..c003fafc 100644 --- a/apps/server/src/routes/fs/routes/browse.ts +++ b/apps/server/src/routes/fs/routes/browse.ts @@ -6,6 +6,11 @@ import type { Request, Response } from "express"; import fs from "fs/promises"; import os from "os"; import path from "path"; +import { + getAllowedRootDirectory, + isPathAllowed, + PathNotAllowedError, +} from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; export function createBrowseHandler() { @@ -13,8 +18,14 @@ export function createBrowseHandler() { try { const { dirPath } = req.body as { dirPath?: string }; - // Default to home directory if no path provided - const targetPath = dirPath ? path.resolve(dirPath) : os.homedir(); + // Default to ALLOWED_ROOT_DIRECTORY if set, otherwise home directory + const defaultPath = getAllowedRootDirectory() || os.homedir(); + const targetPath = dirPath ? path.resolve(dirPath) : defaultPath; + + // Validate that the path is allowed + if (!isPathAllowed(targetPath)) { + throw new PathNotAllowedError(dirPath || targetPath); + } // Detect available drives on Windows const detectDrives = async (): Promise => { @@ -100,6 +111,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) }); } diff --git a/apps/server/src/routes/fs/routes/delete.ts b/apps/server/src/routes/fs/routes/delete.ts index 5a879539..93d2f027 100644 --- a/apps/server/src/routes/fs/routes/delete.ts +++ b/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 "@automaker/platform"; +import { validatePath, PathNotAllowedError } from "@automaker/platform"; 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) }); } diff --git a/apps/server/src/routes/fs/routes/exists.ts b/apps/server/src/routes/fs/routes/exists.ts index 2ca33bee..86b27ea3 100644 --- a/apps/server/src/routes/fs/routes/exists.ts +++ b/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 "@automaker/platform"; 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) }); } diff --git a/apps/server/src/routes/fs/routes/mkdir.ts b/apps/server/src/routes/fs/routes/mkdir.ts index fd89bc99..aee9d281 100644 --- a/apps/server/src/routes/fs/routes/mkdir.ts +++ b/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 "@automaker/platform"; +import { isPathAllowed, PathNotAllowedError } from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; export function createMkdirHandler() { @@ -21,12 +21,16 @@ 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); // Path exists - if it's a directory or symlink, consider it success if (stats.isDirectory() || stats.isSymbolicLink()) { - addAllowedPath(resolvedPath); res.json({ success: true }); return; } @@ -47,11 +51,14 @@ export function createMkdirHandler() { // Path doesn't exist, create it await fs.mkdir(resolvedPath, { recursive: true }); - // Add the new directory to allowed paths for tracking - addAllowedPath(resolvedPath); - 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"); diff --git a/apps/server/src/routes/fs/routes/read.ts b/apps/server/src/routes/fs/routes/read.ts index fbcbaeb3..f485b7d9 100644 --- a/apps/server/src/routes/fs/routes/read.ts +++ b/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 "@automaker/platform"; +import { validatePath, PathNotAllowedError } from "@automaker/platform"; 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) { diff --git a/apps/server/src/routes/fs/routes/readdir.ts b/apps/server/src/routes/fs/routes/readdir.ts index f266372b..1b686610 100644 --- a/apps/server/src/routes/fs/routes/readdir.ts +++ b/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 "@automaker/platform"; +import { validatePath, PathNotAllowedError } from "@automaker/platform"; 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) }); } diff --git a/apps/server/src/routes/fs/routes/resolve-directory.ts b/apps/server/src/routes/fs/routes/resolve-directory.ts index 1bbc4b3c..ca882aba 100644 --- a/apps/server/src/routes/fs/routes/resolve-directory.ts +++ b/apps/server/src/routes/fs/routes/resolve-directory.ts @@ -5,7 +5,6 @@ import type { Request, Response } from "express"; import fs from "fs/promises"; import path from "path"; -import { addAllowedPath } from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; export function createResolveDirectoryHandler() { @@ -30,7 +29,6 @@ export function createResolveDirectoryHandler() { const resolvedPath = path.resolve(directoryName); const stats = await fs.stat(resolvedPath); if (stats.isDirectory()) { - addAllowedPath(resolvedPath); res.json({ success: true, path: resolvedPath, @@ -102,7 +100,6 @@ export function createResolveDirectoryHandler() { } // Found matching directory - addAllowedPath(candidatePath); res.json({ success: true, path: candidatePath, diff --git a/apps/server/src/routes/fs/routes/save-board-background.ts b/apps/server/src/routes/fs/routes/save-board-background.ts index f7c29b95..d3ac0fb1 100644 --- a/apps/server/src/routes/fs/routes/save-board-background.ts +++ b/apps/server/src/routes/fs/routes/save-board-background.ts @@ -5,7 +5,6 @@ import type { Request, Response } from "express"; import fs from "fs/promises"; import path from "path"; -import { addAllowedPath } from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; import { getBoardDir } from "@automaker/platform"; @@ -43,9 +42,6 @@ export function createSaveBoardBackgroundHandler() { // Write file await fs.writeFile(filePath, buffer); - // Add board directory to allowed paths - addAllowedPath(boardDir); - // Return the absolute path res.json({ success: true, path: filePath }); } catch (error) { diff --git a/apps/server/src/routes/fs/routes/save-image.ts b/apps/server/src/routes/fs/routes/save-image.ts index 5f80d189..af87e014 100644 --- a/apps/server/src/routes/fs/routes/save-image.ts +++ b/apps/server/src/routes/fs/routes/save-image.ts @@ -5,7 +5,6 @@ import type { Request, Response } from "express"; import fs from "fs/promises"; import path from "path"; -import { addAllowedPath } from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; import { getImagesDir } from "@automaker/platform"; @@ -45,9 +44,6 @@ export function createSaveImageHandler() { // Write file await fs.writeFile(filePath, buffer); - // Add automaker directory to allowed paths - addAllowedPath(imagesDir); - // Return the absolute path res.json({ success: true, path: filePath }); } catch (error) { diff --git a/apps/server/src/routes/fs/routes/stat.ts b/apps/server/src/routes/fs/routes/stat.ts index a7c9b975..cd81cc74 100644 --- a/apps/server/src/routes/fs/routes/stat.ts +++ b/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 "@automaker/platform"; +import { validatePath, PathNotAllowedError } from "@automaker/platform"; 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) }); } diff --git a/apps/server/src/routes/fs/routes/validate-path.ts b/apps/server/src/routes/fs/routes/validate-path.ts index f027526b..24dcaf85 100644 --- a/apps/server/src/routes/fs/routes/validate-path.ts +++ b/apps/server/src/routes/fs/routes/validate-path.ts @@ -5,7 +5,7 @@ import type { Request, Response } from "express"; import fs from "fs/promises"; import path from "path"; -import { addAllowedPath, isPathAllowed } from "@automaker/platform"; +import { isPathAllowed } from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; export function createValidatePathHandler() { @@ -31,9 +31,6 @@ export function createValidatePathHandler() { return; } - // Add to allowed paths - addAllowedPath(resolvedPath); - res.json({ success: true, path: resolvedPath, diff --git a/apps/server/src/routes/fs/routes/write.ts b/apps/server/src/routes/fs/routes/write.ts index 1fe14735..c31eb63b 100644 --- a/apps/server/src/routes/fs/routes/write.ts +++ b/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 "@automaker/platform"; +import { validatePath, PathNotAllowedError } from "@automaker/platform"; import { mkdirSafe } from "@automaker/utils"; import { getErrorMessage, logError } from "../common.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) }); } diff --git a/apps/server/src/routes/git/index.ts b/apps/server/src/routes/git/index.ts index eb8ce590..25dc333d 100644 --- a/apps/server/src/routes/git/index.ts +++ b/apps/server/src/routes/git/index.ts @@ -3,14 +3,15 @@ */ import { Router } from "express"; +import { validatePathParams } from "../../middleware/validate-paths.js"; import { createDiffsHandler } from "./routes/diffs.js"; import { createFileDiffHandler } from "./routes/file-diff.js"; export function createGitRoutes(): Router { const router = Router(); - router.post("/diffs", createDiffsHandler()); - router.post("/file-diff", createFileDiffHandler()); + router.post("/diffs", validatePathParams("projectPath"), createDiffsHandler()); + router.post("/file-diff", validatePathParams("projectPath", "filePath"), createFileDiffHandler()); return router; } diff --git a/apps/server/src/routes/models/routes/providers.ts b/apps/server/src/routes/models/routes/providers.ts index 9740b94f..3f140f37 100644 --- a/apps/server/src/routes/models/routes/providers.ts +++ b/apps/server/src/routes/models/routes/providers.ts @@ -17,10 +17,6 @@ export function createProvidersHandler() { available: statuses.claude?.installed || false, hasApiKey: !!process.env.ANTHROPIC_API_KEY, }, - google: { - available: !!process.env.GOOGLE_API_KEY, - hasApiKey: !!process.env.GOOGLE_API_KEY, - }, }; res.json({ success: true, providers }); diff --git a/apps/server/src/routes/settings/index.ts b/apps/server/src/routes/settings/index.ts index 73944f84..53c4556e 100644 --- a/apps/server/src/routes/settings/index.ts +++ b/apps/server/src/routes/settings/index.ts @@ -14,6 +14,7 @@ import { Router } from "express"; import type { SettingsService } from "../../services/settings-service.js"; +import { validatePathParams } from "../../middleware/validate-paths.js"; import { createGetGlobalHandler } from "./routes/get-global.js"; import { createUpdateGlobalHandler } from "./routes/update-global.js"; import { createGetCredentialsHandler } from "./routes/get-credentials.js"; @@ -57,8 +58,8 @@ export function createSettingsRoutes(settingsService: SettingsService): Router { router.put("/credentials", createUpdateCredentialsHandler(settingsService)); // Project settings - router.post("/project", createGetProjectHandler(settingsService)); - router.put("/project", createUpdateProjectHandler(settingsService)); + router.post("/project", validatePathParams("projectPath"), createGetProjectHandler(settingsService)); + router.put("/project", validatePathParams("projectPath"), createUpdateProjectHandler(settingsService)); // Migration from localStorage router.post("/migrate", createMigrateHandler(settingsService)); diff --git a/apps/server/src/routes/settings/routes/get-credentials.ts b/apps/server/src/routes/settings/routes/get-credentials.ts index 41057b41..2294e9c4 100644 --- a/apps/server/src/routes/settings/routes/get-credentials.ts +++ b/apps/server/src/routes/settings/routes/get-credentials.ts @@ -5,7 +5,7 @@ * Each provider shows: `{ configured: boolean, masked: string }` * Masked shows first 4 and last 4 characters for verification. * - * Response: `{ "success": true, "credentials": { anthropic, google, openai } }` + * Response: `{ "success": true, "credentials": { anthropic } }` */ import type { Request, Response } from "express"; diff --git a/apps/server/src/routes/settings/routes/update-credentials.ts b/apps/server/src/routes/settings/routes/update-credentials.ts index b358164d..16367879 100644 --- a/apps/server/src/routes/settings/routes/update-credentials.ts +++ b/apps/server/src/routes/settings/routes/update-credentials.ts @@ -1,11 +1,11 @@ /** * PUT /api/settings/credentials - Update API credentials * - * Updates API keys for Anthropic, Google, or OpenAI. Partial updates supported. + * Updates API keys for Anthropic. Partial updates supported. * Returns masked credentials for verification without exposing full keys. * * Request body: `Partial` (usually just apiKeys) - * Response: `{ "success": true, "credentials": { anthropic, google, openai } }` + * Response: `{ "success": true, "credentials": { anthropic } }` */ import type { Request, Response } from "express"; diff --git a/apps/server/src/routes/setup/routes/api-keys.ts b/apps/server/src/routes/setup/routes/api-keys.ts index e292503a..201e4eba 100644 --- a/apps/server/src/routes/setup/routes/api-keys.ts +++ b/apps/server/src/routes/setup/routes/api-keys.ts @@ -12,7 +12,6 @@ export function createApiKeysHandler() { success: true, hasAnthropicKey: !!getApiKey("anthropic") || !!process.env.ANTHROPIC_API_KEY, - hasGoogleKey: !!getApiKey("google") || !!process.env.GOOGLE_API_KEY, }); } catch (error) { logError(error, "Get API keys failed"); diff --git a/apps/server/src/routes/setup/routes/delete-api-key.ts b/apps/server/src/routes/setup/routes/delete-api-key.ts index 554c9f2b..4bb3d4e5 100644 --- a/apps/server/src/routes/setup/routes/delete-api-key.ts +++ b/apps/server/src/routes/setup/routes/delete-api-key.ts @@ -64,15 +64,13 @@ export function createDeleteApiKeyHandler() { // Map provider to env key name const envKeyMap: Record = { anthropic: "ANTHROPIC_API_KEY", - google: "GOOGLE_GENERATIVE_AI_API_KEY", - openai: "OPENAI_API_KEY", }; const envKey = envKeyMap[provider]; if (!envKey) { res.status(400).json({ success: false, - error: `Unknown provider: ${provider}`, + error: `Unknown provider: ${provider}. Only anthropic is supported.`, }); return; } diff --git a/apps/server/src/routes/setup/routes/store-api-key.ts b/apps/server/src/routes/setup/routes/store-api-key.ts index df6f87e2..71011e01 100644 --- a/apps/server/src/routes/setup/routes/store-api-key.ts +++ b/apps/server/src/routes/setup/routes/store-api-key.ts @@ -36,9 +36,12 @@ export function createStoreApiKeyHandler() { process.env.ANTHROPIC_API_KEY = apiKey; await persistApiKeyToEnv("ANTHROPIC_API_KEY", apiKey); logger.info("[Setup] Stored API key as ANTHROPIC_API_KEY"); - } else if (provider === "google") { - process.env.GOOGLE_API_KEY = apiKey; - await persistApiKeyToEnv("GOOGLE_API_KEY", apiKey); + } else { + res.status(400).json({ + success: false, + error: `Unsupported provider: ${provider}. Only anthropic is supported.`, + }); + return; } res.json({ success: true }); diff --git a/apps/server/src/routes/suggestions/index.ts b/apps/server/src/routes/suggestions/index.ts index 176ac5c2..a4b2ec20 100644 --- a/apps/server/src/routes/suggestions/index.ts +++ b/apps/server/src/routes/suggestions/index.ts @@ -4,6 +4,7 @@ import { Router } from "express"; import type { EventEmitter } from "../../lib/events.js"; +import { validatePathParams } from "../../middleware/validate-paths.js"; import { createGenerateHandler } from "./routes/generate.js"; import { createStopHandler } from "./routes/stop.js"; import { createStatusHandler } from "./routes/status.js"; @@ -11,7 +12,7 @@ import { createStatusHandler } from "./routes/status.js"; export function createSuggestionsRoutes(events: EventEmitter): Router { const router = Router(); - router.post("/generate", createGenerateHandler(events)); + router.post("/generate", validatePathParams("projectPath"), createGenerateHandler(events)); router.post("/stop", createStopHandler()); router.get("/status", createStatusHandler()); diff --git a/apps/server/src/routes/templates/routes/clone.ts b/apps/server/src/routes/templates/routes/clone.ts index da52117e..d8c7b6bd 100644 --- a/apps/server/src/routes/templates/routes/clone.ts +++ b/apps/server/src/routes/templates/routes/clone.ts @@ -6,7 +6,7 @@ import type { Request, Response } from "express"; import { spawn } from "child_process"; import path from "path"; import fs from "fs/promises"; -import { addAllowedPath } from "@automaker/platform"; +import { isPathAllowed } from "@automaker/platform"; import { logger, getErrorMessage, logError } from "../common.js"; export function createCloneHandler() { @@ -63,6 +63,24 @@ export function createCloneHandler() { return; } + // Validate that parent directory is within allowed root directory + if (!isPathAllowed(resolvedParent)) { + res.status(403).json({ + success: false, + error: `Parent directory not allowed: ${parentDir}. Must be within ALLOWED_ROOT_DIRECTORY.`, + }); + return; + } + + // Validate that project path will be within allowed root directory + if (!isPathAllowed(resolvedProject)) { + res.status(403).json({ + success: false, + error: `Project path not allowed: ${projectPath}. Must be within ALLOWED_ROOT_DIRECTORY.`, + }); + return; + } + // Check if directory already exists try { await fs.access(projectPath); @@ -186,9 +204,6 @@ export function createCloneHandler() { }); }); - // Add to allowed paths - addAllowedPath(projectPath); - logger.info(`[Templates] Successfully cloned template to ${projectPath}`); res.json({ diff --git a/apps/server/src/routes/workspace/routes/config.ts b/apps/server/src/routes/workspace/routes/config.ts index 5c7b007f..82063e08 100644 --- a/apps/server/src/routes/workspace/routes/config.ts +++ b/apps/server/src/routes/workspace/routes/config.ts @@ -4,47 +4,53 @@ import type { Request, Response } from "express"; import fs from "fs/promises"; -import { addAllowedPath } from "@automaker/platform"; +import path from "path"; +import { + getAllowedRootDirectory, + getDataDirectory, +} from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; export function createConfigHandler() { return async (_req: Request, res: Response): Promise => { try { - const workspaceDir = process.env.WORKSPACE_DIR; + const allowedRootDirectory = getAllowedRootDirectory(); + const dataDirectory = getDataDirectory(); - if (!workspaceDir) { + if (!allowedRootDirectory) { + // When ALLOWED_ROOT_DIRECTORY is not set, return DATA_DIR as default directory res.json({ success: true, configured: false, + defaultDir: dataDirectory || null, }); return; } // Check if the directory exists try { - const stats = await fs.stat(workspaceDir); + const resolvedWorkspaceDir = path.resolve(allowedRootDirectory); + const stats = await fs.stat(resolvedWorkspaceDir); if (!stats.isDirectory()) { res.json({ success: true, configured: false, - error: "WORKSPACE_DIR is not a valid directory", + error: "ALLOWED_ROOT_DIRECTORY is not a valid directory", }); return; } - // Add workspace dir to allowed paths - addAllowedPath(workspaceDir); - res.json({ success: true, configured: true, - workspaceDir, + workspaceDir: resolvedWorkspaceDir, + defaultDir: resolvedWorkspaceDir, }); } catch { res.json({ success: true, configured: false, - error: "WORKSPACE_DIR path does not exist", + error: "ALLOWED_ROOT_DIRECTORY path does not exist", }); } } catch (error) { diff --git a/apps/server/src/routes/workspace/routes/directories.ts b/apps/server/src/routes/workspace/routes/directories.ts index 5d9cf97b..c4f26fec 100644 --- a/apps/server/src/routes/workspace/routes/directories.ts +++ b/apps/server/src/routes/workspace/routes/directories.ts @@ -5,51 +5,49 @@ import type { Request, Response } from "express"; import fs from "fs/promises"; import path from "path"; -import { addAllowedPath } from "@automaker/platform"; +import { getAllowedRootDirectory } from "@automaker/platform"; import { getErrorMessage, logError } from "../common.js"; export function createDirectoriesHandler() { return async (_req: Request, res: Response): Promise => { try { - const workspaceDir = process.env.WORKSPACE_DIR; + const allowedRootDirectory = getAllowedRootDirectory(); - if (!workspaceDir) { + if (!allowedRootDirectory) { res.status(400).json({ success: false, - error: "WORKSPACE_DIR is not configured", + error: "ALLOWED_ROOT_DIRECTORY is not configured", }); return; } + const resolvedWorkspaceDir = path.resolve(allowedRootDirectory); + // Check if directory exists try { - await fs.stat(workspaceDir); + await fs.stat(resolvedWorkspaceDir); } catch { res.status(400).json({ success: false, - error: "WORKSPACE_DIR path does not exist", + error: "Workspace directory path does not exist", }); return; } - // Add workspace dir to allowed paths - addAllowedPath(workspaceDir); - // Read directory contents - const entries = await fs.readdir(workspaceDir, { withFileTypes: true }); + const entries = await fs.readdir(resolvedWorkspaceDir, { + withFileTypes: true, + }); // Filter to directories only and map to result format const directories = entries .filter((entry) => entry.isDirectory() && !entry.name.startsWith(".")) .map((entry) => ({ name: entry.name, - path: path.join(workspaceDir, entry.name), + path: path.join(resolvedWorkspaceDir, entry.name), })) .sort((a, b) => a.name.localeCompare(b.name)); - // Add each directory to allowed paths - directories.forEach((dir) => addAllowedPath(dir.path)); - res.json({ success: true, directories, diff --git a/apps/server/src/routes/worktree/index.ts b/apps/server/src/routes/worktree/index.ts index 304d0678..6d81c854 100644 --- a/apps/server/src/routes/worktree/index.ts +++ b/apps/server/src/routes/worktree/index.ts @@ -3,6 +3,7 @@ */ import { Router } from "express"; +import { validatePathParams } from "../../middleware/validate-paths.js"; import { createInfoHandler } from "./routes/info.js"; import { createStatusHandler } from "./routes/status.js"; import { createListHandler } from "./routes/list.js"; @@ -32,27 +33,27 @@ import { createListDevServersHandler } from "./routes/list-dev-servers.js"; export function createWorktreeRoutes(): Router { const router = Router(); - router.post("/info", createInfoHandler()); - router.post("/status", createStatusHandler()); + router.post("/info", validatePathParams("projectPath"), createInfoHandler()); + router.post("/status", validatePathParams("projectPath"), createStatusHandler()); router.post("/list", createListHandler()); - router.post("/diffs", createDiffsHandler()); - router.post("/file-diff", createFileDiffHandler()); - router.post("/merge", createMergeHandler()); - router.post("/create", createCreateHandler()); - router.post("/delete", createDeleteHandler()); + router.post("/diffs", validatePathParams("projectPath"), createDiffsHandler()); + router.post("/file-diff", validatePathParams("projectPath", "filePath"), createFileDiffHandler()); + router.post("/merge", validatePathParams("projectPath"), createMergeHandler()); + router.post("/create", validatePathParams("projectPath"), createCreateHandler()); + router.post("/delete", validatePathParams("projectPath", "worktreePath"), createDeleteHandler()); router.post("/create-pr", createCreatePRHandler()); router.post("/pr-info", createPRInfoHandler()); - router.post("/commit", createCommitHandler()); - router.post("/push", createPushHandler()); - router.post("/pull", createPullHandler()); + router.post("/commit", validatePathParams("worktreePath"), createCommitHandler()); + router.post("/push", validatePathParams("worktreePath"), createPushHandler()); + router.post("/pull", validatePathParams("worktreePath"), createPullHandler()); router.post("/checkout-branch", createCheckoutBranchHandler()); - router.post("/list-branches", createListBranchesHandler()); + router.post("/list-branches", validatePathParams("worktreePath"), createListBranchesHandler()); router.post("/switch-branch", createSwitchBranchHandler()); - router.post("/open-in-editor", createOpenInEditorHandler()); + router.post("/open-in-editor", validatePathParams("worktreePath"), createOpenInEditorHandler()); router.get("/default-editor", createGetDefaultEditorHandler()); - router.post("/init-git", createInitGitHandler()); + router.post("/init-git", validatePathParams("projectPath"), createInitGitHandler()); router.post("/migrate", createMigrateHandler()); - router.post("/start-dev", createStartDevHandler()); + router.post("/start-dev", validatePathParams("projectPath", "worktreePath"), createStartDevHandler()); router.post("/stop-dev", createStopDevHandler()); router.post("/list-dev-servers", createListDevServersHandler()); diff --git a/apps/server/src/services/agent-service.ts b/apps/server/src/services/agent-service.ts index 5a5d351b..b175074f 100644 --- a/apps/server/src/services/agent-service.ts +++ b/apps/server/src/services/agent-service.ts @@ -4,12 +4,17 @@ */ import path from "path"; -import fs from "fs/promises"; +import * as secureFs from "../lib/secure-fs.js"; import type { EventEmitter } from "../lib/events.js"; import type { ExecuteOptions } from "@automaker/types"; -import { readImageAsBase64, buildPromptWithImages, isAbortError } from "@automaker/utils"; +import { + readImageAsBase64, + buildPromptWithImages, + isAbortError, +} from "@automaker/utils"; import { ProviderFactory } from "../providers/provider-factory.js"; import { createChatOptions } from "../lib/sdk-options.js"; +import { isPathAllowed, PathNotAllowedError } from "@automaker/platform"; interface Message { id: string; @@ -59,7 +64,7 @@ export class AgentService { } async initialize(): Promise { - await fs.mkdir(this.stateDir, { recursive: true }); + await secureFs.mkdir(this.stateDir, { recursive: true }); } /** @@ -77,11 +82,22 @@ export class AgentService { const metadata = await this.loadMetadata(); const sessionMetadata = metadata[sessionId]; + // Determine the effective working directory + const effectiveWorkingDirectory = workingDirectory || process.cwd(); + const resolvedWorkingDirectory = path.resolve(effectiveWorkingDirectory); + + // Validate that the working directory is allowed + if (!isPathAllowed(resolvedWorkingDirectory)) { + throw new Error( + `Working directory ${effectiveWorkingDirectory} is not allowed` + ); + } + this.sessions.set(sessionId, { messages, isRunning: false, abortController: null, - workingDirectory: workingDirectory || process.cwd(), + workingDirectory: resolvedWorkingDirectory, sdkSessionId: sessionMetadata?.sdkSessionId, // Load persisted SDK session ID }); } @@ -388,7 +404,7 @@ export class AgentService { const sessionFile = path.join(this.stateDir, `${sessionId}.json`); try { - const data = await fs.readFile(sessionFile, "utf-8"); + const data = (await secureFs.readFile(sessionFile, "utf-8")) as string; return JSON.parse(data); } catch { return []; @@ -399,7 +415,7 @@ export class AgentService { const sessionFile = path.join(this.stateDir, `${sessionId}.json`); try { - await fs.writeFile( + await secureFs.writeFile( sessionFile, JSON.stringify(messages, null, 2), "utf-8" @@ -412,7 +428,10 @@ export class AgentService { async loadMetadata(): Promise> { try { - const data = await fs.readFile(this.metadataFile, "utf-8"); + const data = (await secureFs.readFile( + this.metadataFile, + "utf-8" + )) as string; return JSON.parse(data); } catch { return {}; @@ -420,7 +439,7 @@ export class AgentService { } async saveMetadata(metadata: Record): Promise { - await fs.writeFile( + await secureFs.writeFile( this.metadataFile, JSON.stringify(metadata, null, 2), "utf-8" @@ -458,11 +477,29 @@ export class AgentService { const sessionId = this.generateId(); const metadata = await this.loadMetadata(); + // Determine the effective working directory + const effectiveWorkingDirectory = + workingDirectory || projectPath || process.cwd(); + const resolvedWorkingDirectory = path.resolve(effectiveWorkingDirectory); + + // Validate that the working directory is allowed + if (!isPathAllowed(resolvedWorkingDirectory)) { + throw new PathNotAllowedError(effectiveWorkingDirectory); + } + + // Validate that projectPath is allowed if provided + if (projectPath) { + const resolvedProjectPath = path.resolve(projectPath); + if (!isPathAllowed(resolvedProjectPath)) { + throw new PathNotAllowedError(projectPath); + } + } + const session: SessionMetadata = { id: sessionId, name, projectPath, - workingDirectory: workingDirectory || projectPath || process.cwd(), + workingDirectory: resolvedWorkingDirectory, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), model, @@ -521,7 +558,7 @@ export class AgentService { // Delete session file try { const sessionFile = path.join(this.stateDir, `${sessionId}.json`); - await fs.unlink(sessionFile); + await secureFs.unlink(sessionFile); } catch { // File may not exist } diff --git a/apps/server/src/services/auto-mode-service.ts b/apps/server/src/services/auto-mode-service.ts index d108bd65..5a5b7f58 100644 --- a/apps/server/src/services/auto-mode-service.ts +++ b/apps/server/src/services/auto-mode-service.ts @@ -11,33 +11,46 @@ import { ProviderFactory } from "../providers/provider-factory.js"; import type { ExecuteOptions, Feature } from "@automaker/types"; -import { buildPromptWithImages, isAbortError, classifyError } from "@automaker/utils"; +import { + buildPromptWithImages, + isAbortError, + classifyError, +} from "@automaker/utils"; import { resolveModelString, DEFAULT_MODELS } from "@automaker/model-resolver"; -import { resolveDependencies, areDependenciesSatisfied } from "@automaker/dependency-resolver"; -import { getFeatureDir, getAutomakerDir, getFeaturesDir, getContextDir } from "@automaker/platform"; +import { + resolveDependencies, + areDependenciesSatisfied, +} from "@automaker/dependency-resolver"; +import { + getFeatureDir, + getAutomakerDir, + getFeaturesDir, + getContextDir, +} from "@automaker/platform"; import { exec } from "child_process"; import { promisify } from "util"; import path from "path"; -import fs from "fs/promises"; +import * as secureFs from "../lib/secure-fs.js"; import type { EventEmitter } from "../lib/events.js"; import { createAutoModeOptions } from "../lib/sdk-options.js"; import { FeatureLoader } from "./feature-loader.js"; +import { isPathAllowed, PathNotAllowedError } from "@automaker/platform"; const execAsync = promisify(exec); // Planning mode types for spec-driven development -type PlanningMode = 'skip' | 'lite' | 'spec' | 'full'; +type PlanningMode = "skip" | "lite" | "spec" | "full"; interface ParsedTask { - id: string; // e.g., "T001" + id: string; // e.g., "T001" description: string; // e.g., "Create user model" - filePath?: string; // e.g., "src/models/user.ts" - phase?: string; // e.g., "Phase 1: Foundation" (for full mode) - status: 'pending' | 'in_progress' | 'completed' | 'failed'; + filePath?: string; // e.g., "src/models/user.ts" + phase?: string; // e.g., "Phase 1: Foundation" (for full mode) + status: "pending" | "in_progress" | "completed" | "failed"; } interface PlanSpec { - status: 'pending' | 'generating' | 'generated' | 'approved' | 'rejected'; + status: "pending" | "generating" | "generated" | "approved" | "rejected"; content?: string; version: number; generatedAt?: string; @@ -202,7 +215,7 @@ When approved, execute tasks SEQUENTIALLY by phase. For each task: After completing all tasks in a phase, output: "[PHASE_COMPLETE] Phase N complete" -This allows real-time progress tracking during implementation.` +This allows real-time progress tracking during implementation.`, }; /** @@ -233,7 +246,7 @@ function parseTasksFromSpec(specContent: string): ParsedTask[] { } const tasksContent = tasksBlockMatch[1]; - const lines = tasksContent.split('\n'); + const lines = tasksContent.split("\n"); let currentPhase: string | undefined; @@ -248,7 +261,7 @@ function parseTasksFromSpec(specContent: string): ParsedTask[] { } // Check for task line - if (trimmedLine.startsWith('- [ ]')) { + if (trimmedLine.startsWith("- [ ]")) { const parsed = parseTaskLine(trimmedLine, currentPhase); if (parsed) { tasks.push(parsed); @@ -265,7 +278,9 @@ function parseTasksFromSpec(specContent: string): ParsedTask[] { */ function parseTaskLine(line: string, currentPhase?: string): ParsedTask | null { // Match pattern: - [ ] T###: Description | File: path - const taskMatch = line.match(/- \[ \] (T\d{3}):\s*([^|]+)(?:\|\s*File:\s*(.+))?$/); + const taskMatch = line.match( + /- \[ \] (T\d{3}):\s*([^|]+)(?:\|\s*File:\s*(.+))?$/ + ); if (!taskMatch) { // Try simpler pattern without file const simpleMatch = line.match(/- \[ \] (T\d{3}):\s*(.+)$/); @@ -274,7 +289,7 @@ function parseTaskLine(line: string, currentPhase?: string): ParsedTask | null { id: simpleMatch[1], description: simpleMatch[2].trim(), phase: currentPhase, - status: 'pending', + status: "pending", }; } return null; @@ -285,7 +300,7 @@ function parseTaskLine(line: string, currentPhase?: string): ParsedTask | null { description: taskMatch[2].trim(), filePath: taskMatch[3]?.trim(), phase: currentPhase, - status: 'pending', + status: "pending", }; } @@ -315,7 +330,11 @@ interface AutoLoopState { } interface PendingApproval { - resolve: (result: { approved: boolean; editedPlan?: string; feedback?: string }) => void; + resolve: (result: { + approved: boolean; + editedPlan?: string; + feedback?: string; + }) => void; reject: (error: Error) => void; featureId: string; projectPath: string; @@ -484,6 +503,11 @@ export class AutoModeService { this.runningFeatures.set(featureId, tempRunningFeature); try { + // Validate that project path is allowed + if (!isPathAllowed(projectPath)) { + throw new PathNotAllowedError(projectPath); + } + // Check if feature has existing context - if so, resume instead of starting fresh // Skip this check if we're already being called with a continuation prompt (from resumeFeature) if (!options?.continuationPrompt) { @@ -547,6 +571,11 @@ export class AutoModeService { ? path.resolve(worktreePath) : path.resolve(projectPath); + // Validate that working directory is allowed + if (!isPathAllowed(workDir)) { + throw new PathNotAllowedError(workDir); + } + // Update running feature with actual worktree info tempRunningFeature.worktreePath = worktreePath; tempRunningFeature.branchName = branchName ?? null; @@ -563,7 +592,9 @@ export class AutoModeService { // Continuation prompt is used when recovering from a plan approval // The plan was already approved, so skip the planning phase prompt = options.continuationPrompt; - console.log(`[AutoMode] Using continuation prompt for feature ${featureId}`); + console.log( + `[AutoMode] Using continuation prompt for feature ${featureId}` + ); } else { // Normal flow: build prompt with planning phase const featurePrompt = this.buildFeaturePrompt(feature); @@ -571,11 +602,11 @@ export class AutoModeService { prompt = planningPrefix + featurePrompt; // Emit planning mode info - if (feature.planningMode && feature.planningMode !== 'skip') { - this.emitAutoModeEvent('planning_started', { + if (feature.planningMode && feature.planningMode !== "skip") { + this.emitAutoModeEvent("planning_started", { featureId: feature.id, mode: feature.planningMode, - message: `Starting ${feature.planningMode} planning phase` + message: `Starting ${feature.planningMode} planning phase`, }); } } @@ -645,8 +676,12 @@ export class AutoModeService { }); } } finally { - console.log(`[AutoMode] Feature ${featureId} execution ended, cleaning up runningFeatures`); - console.log(`[AutoMode] Pending approvals at cleanup: ${Array.from(this.pendingApprovals.keys()).join(', ') || 'none'}`); + console.log( + `[AutoMode] Feature ${featureId} execution ended, cleaning up runningFeatures` + ); + console.log( + `[AutoMode] Pending approvals at cleanup: ${Array.from(this.pendingApprovals.keys()).join(", ") || "none"}` + ); this.runningFeatures.delete(featureId); } } @@ -685,7 +720,7 @@ export class AutoModeService { let hasContext = false; try { - await fs.access(contextPath); + await secureFs.access(contextPath); hasContext = true; } catch { // No context @@ -693,7 +728,7 @@ export class AutoModeService { if (hasContext) { // Load previous context and continue - const context = await fs.readFile(contextPath, "utf-8"); + const context = (await secureFs.readFile(contextPath, "utf-8")) as string; return this.executeFeatureWithContext( projectPath, featureId, @@ -753,7 +788,10 @@ export class AutoModeService { const contextPath = path.join(featureDir, "agent-output.md"); let previousContext = ""; try { - previousContext = await fs.readFile(contextPath, "utf-8"); + previousContext = (await secureFs.readFile( + contextPath, + "utf-8" + )) as string; } catch { // No previous context } @@ -819,7 +857,7 @@ Address the follow-up instructions above. Review the previous work and make the const featureDirForImages = getFeatureDir(projectPath, featureId); const featureImagesDir = path.join(featureDirForImages, "images"); - await fs.mkdir(featureImagesDir, { recursive: true }); + await secureFs.mkdir(featureImagesDir, { recursive: true }); for (const imagePath of imagePaths) { try { @@ -828,7 +866,7 @@ Address the follow-up instructions above. Review the previous work and make the const destPath = path.join(featureImagesDir, filename); // Copy the image - await fs.copyFile(imagePath, destPath); + await secureFs.copyFile(imagePath, destPath); // Store the absolute path (external storage uses absolute paths) copiedImagePaths.push(destPath); @@ -870,7 +908,10 @@ Address the follow-up instructions above. Review the previous work and make the const featurePath = path.join(featureDirForSave, "feature.json"); try { - await fs.writeFile(featurePath, JSON.stringify(feature, null, 2)); + await secureFs.writeFile( + featurePath, + JSON.stringify(feature, null, 2) + ); } catch (error) { console.error(`[AutoMode] Failed to save feature.json:`, error); } @@ -890,7 +931,7 @@ Address the follow-up instructions above. Review the previous work and make the model, { projectPath, - planningMode: 'skip', // Follow-ups don't require approval + planningMode: "skip", // Follow-ups don't require approval previousContent: previousContext || undefined, systemPrompt: contextFiles || undefined, } @@ -936,7 +977,7 @@ Address the follow-up instructions above. Review the previous work and make the let workDir = projectPath; try { - await fs.access(worktreePath); + await secureFs.access(worktreePath); workDir = worktreePath; } catch { // No worktree @@ -1005,7 +1046,7 @@ Address the follow-up instructions above. Review the previous work and make the // Use the provided worktree path if given if (providedWorktreePath) { try { - await fs.access(providedWorktreePath); + await secureFs.access(providedWorktreePath); workDir = providedWorktreePath; console.log(`[AutoMode] Committing in provided worktree: ${workDir}`); } catch { @@ -1021,7 +1062,7 @@ Address the follow-up instructions above. Review the previous work and make the featureId ); try { - await fs.access(legacyWorktreePath); + await secureFs.access(legacyWorktreePath); workDir = legacyWorktreePath; console.log(`[AutoMode] Committing in legacy worktree: ${workDir}`); } catch { @@ -1084,7 +1125,7 @@ Address the follow-up instructions above. Review the previous work and make the const contextPath = path.join(featureDir, "agent-output.md"); try { - await fs.access(contextPath); + await secureFs.access(contextPath); return true; } catch { return false; @@ -1102,9 +1143,9 @@ Address the follow-up instructions above. Review the previous work and make the try { // Check if directory exists first - await fs.access(contextDir); + await secureFs.access(contextDir); - const files = await fs.readdir(contextDir); + const files = await secureFs.readdir(contextDir); // Filter for text-based context files (case-insensitive for Windows) const textFiles = files.filter((f) => { const lower = f.toLowerCase(); @@ -1117,7 +1158,7 @@ Address the follow-up instructions above. Review the previous work and make the for (const file of textFiles) { // Use path.join for cross-platform path construction const filePath = path.join(contextDir, file); - const content = await fs.readFile(filePath, "utf-8"); + const content = (await secureFs.readFile(filePath, "utf-8")) as string; contents.push(`## ${file}\n\n${content}`); } @@ -1216,8 +1257,8 @@ Format your response as a structured markdown document.`; // Save analysis to .automaker directory const automakerDir = getAutomakerDir(projectPath); const analysisPath = path.join(automakerDir, "project-analysis.md"); - await fs.mkdir(automakerDir, { recursive: true }); - await fs.writeFile(analysisPath, analysisResult); + await secureFs.mkdir(automakerDir, { recursive: true }); + await secureFs.writeFile(analysisPath, analysisResult); this.emitAutoModeEvent("auto_mode_feature_complete", { featureId: analysisFeatureId, @@ -1236,7 +1277,6 @@ Format your response as a structured markdown document.`; } } - /** * Get current status */ @@ -1277,8 +1317,12 @@ Format your response as a structured markdown document.`; featureId: string, projectPath: string ): Promise<{ approved: boolean; editedPlan?: string; feedback?: string }> { - console.log(`[AutoMode] Registering pending approval for feature ${featureId}`); - console.log(`[AutoMode] Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(', ') || 'none'}`); + console.log( + `[AutoMode] Registering pending approval for feature ${featureId}` + ); + console.log( + `[AutoMode] Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(", ") || "none"}` + ); return new Promise((resolve, reject) => { this.pendingApprovals.set(featureId, { resolve, @@ -1286,7 +1330,9 @@ Format your response as a structured markdown document.`; featureId, projectPath, }); - console.log(`[AutoMode] Pending approval registered for feature ${featureId}`); + console.log( + `[AutoMode] Pending approval registered for feature ${featureId}` + ); }); } @@ -1301,61 +1347,89 @@ Format your response as a structured markdown document.`; feedback?: string, projectPathFromClient?: string ): Promise<{ success: boolean; error?: string }> { - console.log(`[AutoMode] resolvePlanApproval called for feature ${featureId}, approved=${approved}`); - console.log(`[AutoMode] Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(', ') || 'none'}`); + console.log( + `[AutoMode] resolvePlanApproval called for feature ${featureId}, approved=${approved}` + ); + console.log( + `[AutoMode] Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(", ") || "none"}` + ); const pending = this.pendingApprovals.get(featureId); if (!pending) { - console.log(`[AutoMode] No pending approval in Map for feature ${featureId}`); + console.log( + `[AutoMode] No pending approval in Map for feature ${featureId}` + ); // RECOVERY: If no pending approval but we have projectPath from client, // check if feature's planSpec.status is 'generated' and handle recovery if (projectPathFromClient) { - console.log(`[AutoMode] Attempting recovery with projectPath: ${projectPathFromClient}`); - const feature = await this.loadFeature(projectPathFromClient, featureId); + console.log( + `[AutoMode] Attempting recovery with projectPath: ${projectPathFromClient}` + ); + const feature = await this.loadFeature( + projectPathFromClient, + featureId + ); - if (feature?.planSpec?.status === 'generated') { - console.log(`[AutoMode] Feature ${featureId} has planSpec.status='generated', performing recovery`); + if (feature?.planSpec?.status === "generated") { + console.log( + `[AutoMode] Feature ${featureId} has planSpec.status='generated', performing recovery` + ); if (approved) { // Update planSpec to approved await this.updateFeaturePlanSpec(projectPathFromClient, featureId, { - status: 'approved', + status: "approved", approvedAt: new Date().toISOString(), reviewedByUser: true, content: editedPlan || feature.planSpec.content, }); // Build continuation prompt and re-run the feature - const planContent = editedPlan || feature.planSpec.content || ''; + const planContent = editedPlan || feature.planSpec.content || ""; let continuationPrompt = `The plan/specification has been approved. `; if (feedback) { continuationPrompt += `\n\nUser feedback: ${feedback}\n\n`; } continuationPrompt += `Now proceed with the implementation as specified in the plan:\n\n${planContent}\n\nImplement the feature now.`; - console.log(`[AutoMode] Starting recovery execution for feature ${featureId}`); + console.log( + `[AutoMode] Starting recovery execution for feature ${featureId}` + ); // Start feature execution with the continuation prompt (async, don't await) // Pass undefined for providedWorktreePath, use options for continuation prompt - this.executeFeature(projectPathFromClient, featureId, true, false, undefined, { - continuationPrompt, - }) - .catch((error) => { - console.error(`[AutoMode] Recovery execution failed for feature ${featureId}:`, error); - }); + this.executeFeature( + projectPathFromClient, + featureId, + true, + false, + undefined, + { + continuationPrompt, + } + ).catch((error) => { + console.error( + `[AutoMode] Recovery execution failed for feature ${featureId}:`, + error + ); + }); return { success: true }; } else { // Rejected - update status and emit event await this.updateFeaturePlanSpec(projectPathFromClient, featureId, { - status: 'rejected', + status: "rejected", reviewedByUser: true, }); - await this.updateFeatureStatus(projectPathFromClient, featureId, 'backlog'); + await this.updateFeatureStatus( + projectPathFromClient, + featureId, + "backlog" + ); - this.emitAutoModeEvent('plan_rejected', { + this.emitAutoModeEvent("plan_rejected", { featureId, projectPath: projectPathFromClient, feedback, @@ -1366,16 +1440,23 @@ Format your response as a structured markdown document.`; } } - console.log(`[AutoMode] ERROR: No pending approval found for feature ${featureId} and recovery not possible`); - return { success: false, error: `No pending approval for feature ${featureId}` }; + console.log( + `[AutoMode] ERROR: No pending approval found for feature ${featureId} and recovery not possible` + ); + return { + success: false, + error: `No pending approval for feature ${featureId}`, + }; } - console.log(`[AutoMode] Found pending approval for feature ${featureId}, proceeding...`); + console.log( + `[AutoMode] Found pending approval for feature ${featureId}, proceeding...` + ); const { projectPath } = pending; // Update feature's planSpec status await this.updateFeaturePlanSpec(projectPath, featureId, { - status: approved ? 'approved' : 'rejected', + status: approved ? "approved" : "rejected", approvedAt: approved ? new Date().toISOString() : undefined, reviewedByUser: true, content: editedPlan, // Update content if user provided an edited version @@ -1384,7 +1465,7 @@ Format your response as a structured markdown document.`; // If rejected with feedback, we can store it for the user to see if (!approved && feedback) { // Emit event so client knows the rejection reason - this.emitAutoModeEvent('plan_rejected', { + this.emitAutoModeEvent("plan_rejected", { featureId, projectPath, feedback, @@ -1402,15 +1483,25 @@ Format your response as a structured markdown document.`; * Cancel a pending plan approval (e.g., when feature is stopped). */ cancelPlanApproval(featureId: string): void { - console.log(`[AutoMode] cancelPlanApproval called for feature ${featureId}`); - console.log(`[AutoMode] Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(', ') || 'none'}`); + console.log( + `[AutoMode] cancelPlanApproval called for feature ${featureId}` + ); + console.log( + `[AutoMode] Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(", ") || "none"}` + ); const pending = this.pendingApprovals.get(featureId); if (pending) { - console.log(`[AutoMode] Found and cancelling pending approval for feature ${featureId}`); - pending.reject(new Error('Plan approval cancelled - feature was stopped')); + console.log( + `[AutoMode] Found and cancelling pending approval for feature ${featureId}` + ); + pending.reject( + new Error("Plan approval cancelled - feature was stopped") + ); this.pendingApprovals.delete(featureId); } else { - console.log(`[AutoMode] No pending approval to cancel for feature ${featureId}`); + console.log( + `[AutoMode] No pending approval to cancel for feature ${featureId}` + ); } } @@ -1423,7 +1514,6 @@ Format your response as a structured markdown document.`; // Private helpers - /** * Find an existing worktree for a given branch by checking git worktree list */ @@ -1485,7 +1575,7 @@ Format your response as a structured markdown document.`; const featurePath = path.join(featureDir, "feature.json"); try { - const data = await fs.readFile(featurePath, "utf-8"); + const data = (await secureFs.readFile(featurePath, "utf-8")) as string; return JSON.parse(data); } catch { return null; @@ -1502,7 +1592,7 @@ Format your response as a structured markdown document.`; const featurePath = path.join(featureDir, "feature.json"); try { - const data = await fs.readFile(featurePath, "utf-8"); + const data = (await secureFs.readFile(featurePath, "utf-8")) as string; const feature = JSON.parse(data); feature.status = status; feature.updatedAt = new Date().toISOString(); @@ -1514,7 +1604,7 @@ Format your response as a structured markdown document.`; // Clear the timestamp when moving to other statuses feature.justFinishedAt = undefined; } - await fs.writeFile(featurePath, JSON.stringify(feature, null, 2)); + await secureFs.writeFile(featurePath, JSON.stringify(feature, null, 2)); } catch { // Feature file may not exist } @@ -1537,13 +1627,13 @@ Format your response as a structured markdown document.`; ); try { - const data = await fs.readFile(featurePath, "utf-8"); + const data = (await secureFs.readFile(featurePath, "utf-8")) as string; const feature = JSON.parse(data); // Initialize planSpec if it doesn't exist if (!feature.planSpec) { feature.planSpec = { - status: 'pending', + status: "pending", version: 1, reviewedByUser: false, }; @@ -1558,9 +1648,12 @@ Format your response as a structured markdown document.`; } feature.updatedAt = new Date().toISOString(); - await fs.writeFile(featurePath, JSON.stringify(feature, null, 2)); + await secureFs.writeFile(featurePath, JSON.stringify(feature, null, 2)); } catch (error) { - console.error(`[AutoMode] Failed to update planSpec for ${featureId}:`, error); + console.error( + `[AutoMode] Failed to update planSpec for ${featureId}:`, + error + ); } } @@ -1569,7 +1662,9 @@ Format your response as a structured markdown document.`; const featuresDir = getFeaturesDir(projectPath); try { - const entries = await fs.readdir(featuresDir, { withFileTypes: true }); + const entries = await secureFs.readdir(featuresDir, { + withFileTypes: true, + }); const allFeatures: Feature[] = []; const pendingFeatures: Feature[] = []; @@ -1582,7 +1677,10 @@ Format your response as a structured markdown document.`; "feature.json" ); try { - const data = await fs.readFile(featurePath, "utf-8"); + const data = (await secureFs.readFile( + featurePath, + "utf-8" + )) as string; const feature = JSON.parse(data); allFeatures.push(feature); @@ -1636,24 +1734,25 @@ Format your response as a structured markdown document.`; * Get the planning prompt prefix based on feature's planning mode */ private getPlanningPromptPrefix(feature: Feature): string { - const mode = feature.planningMode || 'skip'; + const mode = feature.planningMode || "skip"; - if (mode === 'skip') { - return ''; // No planning phase + if (mode === "skip") { + return ""; // No planning phase } // For lite mode, use the approval variant if requirePlanApproval is true let promptKey: string = mode; - if (mode === 'lite' && feature.requirePlanApproval === true) { - promptKey = 'lite_with_approval'; + if (mode === "lite" && feature.requirePlanApproval === true) { + promptKey = "lite_with_approval"; } - const planningPrompt = PLANNING_PROMPTS[promptKey as keyof typeof PLANNING_PROMPTS]; + const planningPrompt = + PLANNING_PROMPTS[promptKey as keyof typeof PLANNING_PROMPTS]; if (!planningPrompt) { - return ''; + return ""; } - return planningPrompt + '\n\n---\n\n## Feature Request\n\n'; + return planningPrompt + "\n\n---\n\n## Feature Request\n\n"; } private buildFeaturePrompt(feature: Feature): string { @@ -1747,17 +1846,18 @@ This helps parse your summary correctly in the output logs.`; } ): Promise { const finalProjectPath = options?.projectPath || projectPath; - const planningMode = options?.planningMode || 'skip'; + const planningMode = options?.planningMode || "skip"; const previousContent = options?.previousContent; // Check if this planning mode can generate a spec/plan that needs approval // - spec and full always generate specs // - lite only generates approval-ready content when requirePlanApproval is true const planningModeRequiresApproval = - planningMode === 'spec' || - planningMode === 'full' || - (planningMode === 'lite' && options?.requirePlanApproval === true); - const requiresApproval = planningModeRequiresApproval && options?.requirePlanApproval === true; + planningMode === "spec" || + planningMode === "full" || + (planningMode === "lite" && options?.requirePlanApproval === true); + const requiresApproval = + planningModeRequiresApproval && options?.requirePlanApproval === true; // CI/CD Mock Mode: Return early with mock response when AUTOMAKER_MOCK_AGENT is set // This prevents actual API calls during automated testing @@ -1786,7 +1886,7 @@ This helps parse your summary correctly in the output logs.`; // Create a mock file with "yellow" content as requested in the test const mockFilePath = path.join(workDir, "yellow.txt"); - await fs.writeFile(mockFilePath, "yellow"); + await secureFs.writeFile(mockFilePath, "yellow"); this.emitAutoModeEvent("auto_mode_progress", { featureId, @@ -1811,8 +1911,8 @@ This is a mock agent response for CI/CD testing. This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. `; - await fs.mkdir(path.dirname(outputPath), { recursive: true }); - await fs.writeFile(outputPath, mockOutput); + await secureFs.mkdir(path.dirname(outputPath), { recursive: true }); + await secureFs.writeFile(outputPath, mockOutput); console.log( `[AutoMode] MOCK MODE: Completed mock execution for feature ${featureId}` @@ -1888,8 +1988,8 @@ This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. // Helper to write current responseText to file const writeToFile = async (): Promise => { try { - await fs.mkdir(path.dirname(outputPath), { recursive: true }); - await fs.writeFile(outputPath, responseText); + await secureFs.mkdir(path.dirname(outputPath), { recursive: true }); + await secureFs.writeFile(outputPath, responseText); } catch (error) { // Log but don't crash - file write errors shouldn't stop execution console.error( @@ -1932,7 +2032,7 @@ This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. ) { throw new Error( "Authentication failed: Invalid or expired API key. " + - "Please check your ANTHROPIC_API_KEY or GOOGLE_API_KEY, or run 'claude login' to re-authenticate." + "Please check your ANTHROPIC_API_KEY, or run 'claude login' to re-authenticate." ); } @@ -1940,11 +2040,15 @@ This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. scheduleWrite(); // Check for [SPEC_GENERATED] marker in planning modes (spec or full) - if (planningModeRequiresApproval && !specDetected && responseText.includes('[SPEC_GENERATED]')) { + if ( + planningModeRequiresApproval && + !specDetected && + responseText.includes("[SPEC_GENERATED]") + ) { specDetected = true; // Extract plan content (everything before the marker) - const markerIndex = responseText.indexOf('[SPEC_GENERATED]'); + const markerIndex = responseText.indexOf("[SPEC_GENERATED]"); const planContent = responseText.substring(0, markerIndex).trim(); // Parse tasks from the generated spec (for spec and full modes) @@ -1952,14 +2056,18 @@ This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. let parsedTasks = parseTasksFromSpec(planContent); const tasksTotal = parsedTasks.length; - console.log(`[AutoMode] Parsed ${tasksTotal} tasks from spec for feature ${featureId}`); + console.log( + `[AutoMode] Parsed ${tasksTotal} tasks from spec for feature ${featureId}` + ); if (parsedTasks.length > 0) { - console.log(`[AutoMode] Tasks: ${parsedTasks.map(t => t.id).join(', ')}`); + console.log( + `[AutoMode] Tasks: ${parsedTasks.map((t) => t.id).join(", ")}` + ); } // Update planSpec status to 'generated' and save content with parsed tasks await this.updateFeaturePlanSpec(projectPath, featureId, { - status: 'generated', + status: "generated", content: planContent, version: 1, generatedAt: new Date().toISOString(), @@ -1983,13 +2091,18 @@ This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. let planApproved = false; while (!planApproved) { - console.log(`[AutoMode] Spec v${planVersion} generated for feature ${featureId}, waiting for approval`); + console.log( + `[AutoMode] Spec v${planVersion} generated for feature ${featureId}, waiting for approval` + ); // CRITICAL: Register pending approval BEFORE emitting event - const approvalPromise = this.waitForPlanApproval(featureId, projectPath); + const approvalPromise = this.waitForPlanApproval( + featureId, + projectPath + ); // Emit plan_approval_required event - this.emitAutoModeEvent('plan_approval_required', { + this.emitAutoModeEvent("plan_approval_required", { featureId, projectPath, planContent: currentPlanContent, @@ -2003,15 +2116,21 @@ This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. if (approvalResult.approved) { // User approved the plan - console.log(`[AutoMode] Plan v${planVersion} approved for feature ${featureId}`); + console.log( + `[AutoMode] Plan v${planVersion} approved for feature ${featureId}` + ); planApproved = true; // If user provided edits, use the edited version if (approvalResult.editedPlan) { approvedPlanContent = approvalResult.editedPlan; - await this.updateFeaturePlanSpec(projectPath, featureId, { - content: approvalResult.editedPlan, - }); + await this.updateFeaturePlanSpec( + projectPath, + featureId, + { + content: approvalResult.editedPlan, + } + ); } else { approvedPlanContent = currentPlanContent; } @@ -2020,30 +2139,37 @@ This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. userFeedback = approvalResult.feedback; // Emit approval event - this.emitAutoModeEvent('plan_approved', { + this.emitAutoModeEvent("plan_approved", { featureId, projectPath, hasEdits: !!approvalResult.editedPlan, planVersion, }); - } else { // User rejected - check if they provided feedback for revision - const hasFeedback = approvalResult.feedback && approvalResult.feedback.trim().length > 0; - const hasEdits = approvalResult.editedPlan && approvalResult.editedPlan.trim().length > 0; + const hasFeedback = + approvalResult.feedback && + approvalResult.feedback.trim().length > 0; + const hasEdits = + approvalResult.editedPlan && + approvalResult.editedPlan.trim().length > 0; if (!hasFeedback && !hasEdits) { // No feedback or edits = explicit cancel - console.log(`[AutoMode] Plan rejected without feedback for feature ${featureId}, cancelling`); - throw new Error('Plan cancelled by user'); + console.log( + `[AutoMode] Plan rejected without feedback for feature ${featureId}, cancelling` + ); + throw new Error("Plan cancelled by user"); } // User wants revisions - regenerate the plan - console.log(`[AutoMode] Plan v${planVersion} rejected with feedback for feature ${featureId}, regenerating...`); + console.log( + `[AutoMode] Plan v${planVersion} rejected with feedback for feature ${featureId}, regenerating...` + ); planVersion++; // Emit revision event - this.emitAutoModeEvent('plan_revision_requested', { + this.emitAutoModeEvent("plan_revision_requested", { featureId, projectPath, feedback: approvalResult.feedback, @@ -2058,7 +2184,7 @@ This mock response was generated because AUTOMAKER_MOCK_AGENT=true was set. ${hasEdits ? approvalResult.editedPlan : currentPlanContent} ## User Feedback -${approvalResult.feedback || 'Please revise the plan based on the edits above.'} +${approvalResult.feedback || "Please revise the plan based on the edits above."} ## Instructions Please regenerate the specification incorporating the user's feedback. @@ -2069,7 +2195,7 @@ After generating the revised spec, output: // Update status to regenerating await this.updateFeaturePlanSpec(projectPath, featureId, { - status: 'generating', + status: "generating", version: planVersion, }); @@ -2096,27 +2222,38 @@ After generating the revised spec, output: } } } else if (msg.type === "error") { - throw new Error(msg.error || "Error during plan revision"); - } else if (msg.type === "result" && msg.subtype === "success") { + throw new Error( + msg.error || "Error during plan revision" + ); + } else if ( + msg.type === "result" && + msg.subtype === "success" + ) { revisionText += msg.result || ""; } } // Extract new plan content - const markerIndex = revisionText.indexOf('[SPEC_GENERATED]'); + const markerIndex = + revisionText.indexOf("[SPEC_GENERATED]"); if (markerIndex > 0) { - currentPlanContent = revisionText.substring(0, markerIndex).trim(); + currentPlanContent = revisionText + .substring(0, markerIndex) + .trim(); } else { currentPlanContent = revisionText.trim(); } // Re-parse tasks from revised plan - const revisedTasks = parseTasksFromSpec(currentPlanContent); - console.log(`[AutoMode] Revised plan has ${revisedTasks.length} tasks`); + const revisedTasks = + parseTasksFromSpec(currentPlanContent); + console.log( + `[AutoMode] Revised plan has ${revisedTasks.length} tasks` + ); // Update planSpec with revised content await this.updateFeaturePlanSpec(projectPath, featureId, { - status: 'generated', + status: "generated", content: currentPlanContent, version: planVersion, tasks: revisedTasks, @@ -2129,21 +2266,23 @@ After generating the revised spec, output: responseText += revisionText; } - } catch (error) { - if ((error as Error).message.includes('cancelled')) { + if ((error as Error).message.includes("cancelled")) { throw error; } - throw new Error(`Plan approval failed: ${(error as Error).message}`); + throw new Error( + `Plan approval failed: ${(error as Error).message}` + ); } } - } else { // Auto-approve: requirePlanApproval is false, just continue without pausing - console.log(`[AutoMode] Spec generated for feature ${featureId}, auto-approving (requirePlanApproval=false)`); + console.log( + `[AutoMode] Spec generated for feature ${featureId}, auto-approving (requirePlanApproval=false)` + ); // Emit info event for frontend - this.emitAutoModeEvent('plan_auto_approved', { + this.emitAutoModeEvent("plan_auto_approved", { featureId, projectPath, planContent, @@ -2155,11 +2294,13 @@ After generating the revised spec, output: // CRITICAL: After approval, we need to make a second call to continue implementation // The agent is waiting for "approved" - we need to send it and continue - console.log(`[AutoMode] Making continuation call after plan approval for feature ${featureId}`); + console.log( + `[AutoMode] Making continuation call after plan approval for feature ${featureId}` + ); // Update planSpec status to approved (handles both manual and auto-approval paths) await this.updateFeaturePlanSpec(projectPath, featureId, { - status: 'approved', + status: "approved", approvedAt: new Date().toISOString(), reviewedByUser: requiresApproval, }); @@ -2170,19 +2311,27 @@ After generating the revised spec, output: // ======================================== if (parsedTasks.length > 0) { - console.log(`[AutoMode] Starting multi-agent execution: ${parsedTasks.length} tasks for feature ${featureId}`); + console.log( + `[AutoMode] Starting multi-agent execution: ${parsedTasks.length} tasks for feature ${featureId}` + ); // Execute each task with a separate agent - for (let taskIndex = 0; taskIndex < parsedTasks.length; taskIndex++) { + for ( + let taskIndex = 0; + taskIndex < parsedTasks.length; + taskIndex++ + ) { const task = parsedTasks[taskIndex]; // Check for abort if (abortController.signal.aborted) { - throw new Error('Feature execution aborted'); + throw new Error("Feature execution aborted"); } // Emit task started - console.log(`[AutoMode] Starting task ${task.id}: ${task.description}`); + console.log( + `[AutoMode] Starting task ${task.id}: ${task.description}` + ); this.emitAutoModeEvent("auto_mode_task_started", { featureId, projectPath, @@ -2198,7 +2347,13 @@ After generating the revised spec, output: }); // Build focused prompt for this specific task - const taskPrompt = this.buildTaskPrompt(task, parsedTasks, taskIndex, approvedPlanContent, userFeedback); + const taskPrompt = this.buildTaskPrompt( + task, + parsedTasks, + taskIndex, + approvedPlanContent, + userFeedback + ); // Execute task with dedicated agent const taskStream = provider.executeQuery({ @@ -2232,15 +2387,22 @@ After generating the revised spec, output: } } } else if (msg.type === "error") { - throw new Error(msg.error || `Error during task ${task.id}`); - } else if (msg.type === "result" && msg.subtype === "success") { + throw new Error( + msg.error || `Error during task ${task.id}` + ); + } else if ( + msg.type === "result" && + msg.subtype === "success" + ) { taskOutput += msg.result || ""; responseText += msg.result || ""; } } // Emit task completed - console.log(`[AutoMode] Task ${task.id} completed for feature ${featureId}`); + console.log( + `[AutoMode] Task ${task.id} completed for feature ${featureId}` + ); this.emitAutoModeEvent("auto_mode_task_complete", { featureId, projectPath, @@ -2271,13 +2433,17 @@ After generating the revised spec, output: } } - console.log(`[AutoMode] All ${parsedTasks.length} tasks completed for feature ${featureId}`); + console.log( + `[AutoMode] All ${parsedTasks.length} tasks completed for feature ${featureId}` + ); } else { // No parsed tasks - fall back to single-agent execution - console.log(`[AutoMode] No parsed tasks, using single-agent execution for feature ${featureId}`); + console.log( + `[AutoMode] No parsed tasks, using single-agent execution for feature ${featureId}` + ); const continuationPrompt = `The plan/specification has been approved. Now implement it. -${userFeedback ? `\n## User Feedback\n${userFeedback}\n` : ''} +${userFeedback ? `\n## User Feedback\n${userFeedback}\n` : ""} ## Approved Plan ${approvedPlanContent} @@ -2313,14 +2479,21 @@ Implement all the changes described in the plan above.`; } } } else if (msg.type === "error") { - throw new Error(msg.error || "Unknown error during implementation"); - } else if (msg.type === "result" && msg.subtype === "success") { + throw new Error( + msg.error || "Unknown error during implementation" + ); + } else if ( + msg.type === "result" && + msg.subtype === "success" + ) { responseText += msg.result || ""; } } } - console.log(`[AutoMode] Implementation completed for feature ${featureId}`); + console.log( + `[AutoMode] Implementation completed for feature ${featureId}` + ); // Exit the original stream loop since continuation is done break streamLoop; } @@ -2397,9 +2570,16 @@ ${context} ## Instructions Review the previous work and continue the implementation. If the feature appears complete, verify it works correctly.`; - return this.executeFeature(projectPath, featureId, useWorktrees, false, undefined, { - continuationPrompt: prompt, - }); + return this.executeFeature( + projectPath, + featureId, + useWorktrees, + false, + undefined, + { + continuationPrompt: prompt, + } + ); } /** @@ -2424,8 +2604,8 @@ You are executing a specific task as part of a larger feature implementation. **Task ID:** ${task.id} **Description:** ${task.description} -${task.filePath ? `**Primary File:** ${task.filePath}` : ''} -${task.phase ? `**Phase:** ${task.phase}` : ''} +${task.filePath ? `**Primary File:** ${task.filePath}` : ""} +${task.phase ? `**Phase:** ${task.phase}` : ""} ## Context @@ -2434,7 +2614,7 @@ ${task.phase ? `**Phase:** ${task.phase}` : ''} // Show what's already done if (completedTasks.length > 0) { prompt += `### Already Completed (${completedTasks.length} tasks) -${completedTasks.map(t => `- [x] ${t.id}: ${t.description}`).join('\n')} +${completedTasks.map((t) => `- [x] ${t.id}: ${t.description}`).join("\n")} `; } @@ -2442,8 +2622,11 @@ ${completedTasks.map(t => `- [x] ${t.id}: ${t.description}`).join('\n')} // Show remaining tasks if (remainingTasks.length > 0) { prompt += `### Coming Up Next (${remainingTasks.length} tasks remaining) -${remainingTasks.slice(0, 3).map(t => `- [ ] ${t.id}: ${t.description}`).join('\n')} -${remainingTasks.length > 3 ? `... and ${remainingTasks.length - 3} more tasks` : ''} +${remainingTasks + .slice(0, 3) + .map((t) => `- [ ] ${t.id}: ${t.description}`) + .join("\n")} +${remainingTasks.length > 3 ? `... and ${remainingTasks.length - 3} more tasks` : ""} `; } diff --git a/apps/server/src/services/feature-loader.ts b/apps/server/src/services/feature-loader.ts index fe77abe6..41585103 100644 --- a/apps/server/src/services/feature-loader.ts +++ b/apps/server/src/services/feature-loader.ts @@ -4,9 +4,9 @@ */ import path from "path"; -import fs from "fs/promises"; import type { Feature } from "@automaker/types"; import { createLogger } from "@automaker/utils"; +import * as secureFs from "../lib/secure-fs.js"; import { getFeaturesDir, getFeatureDir, @@ -39,8 +39,12 @@ export class FeatureLoader { */ private async deleteOrphanedImages( projectPath: string, - oldPaths: Array | undefined, - newPaths: Array | undefined + oldPaths: + | Array + | undefined, + newPaths: + | Array + | undefined ): Promise { if (!oldPaths || oldPaths.length === 0) { return; @@ -59,8 +63,8 @@ export class FeatureLoader { if (!newPathSet.has(oldPath)) { try { // Paths are now absolute - await fs.unlink(oldPath); - logger.info(`Deleted orphaned image: ${oldPath}`); + await secureFs.unlink(oldPath); + console.log(`[FeatureLoader] Deleted orphaned image: ${oldPath}`); } catch (error) { // Ignore errors when deleting (file may already be gone) logger.warn( @@ -87,10 +91,11 @@ export class FeatureLoader { } const featureImagesDir = this.getFeatureImagesDir(projectPath, featureId); - await fs.mkdir(featureImagesDir, { recursive: true }); + await secureFs.mkdir(featureImagesDir, { recursive: true }); - const updatedPaths: Array = - []; + const updatedPaths: Array< + string | { path: string; [key: string]: unknown } + > = []; for (const imagePath of imagePaths) { try { @@ -110,7 +115,7 @@ export class FeatureLoader { // Check if file exists try { - await fs.access(fullOriginalPath); + await secureFs.access(fullOriginalPath); } catch { logger.warn( `[FeatureLoader] Image not found, skipping: ${fullOriginalPath}` @@ -123,14 +128,14 @@ export class FeatureLoader { const newPath = path.join(featureImagesDir, filename); // Copy the file - await fs.copyFile(fullOriginalPath, newPath); - logger.info( + await secureFs.copyFile(fullOriginalPath, newPath); + console.log( `[FeatureLoader] Copied image: ${originalPath} -> ${newPath}` ); // Try to delete the original temp file try { - await fs.unlink(fullOriginalPath); + await secureFs.unlink(fullOriginalPath); } catch { // Ignore errors when deleting temp file } @@ -163,14 +168,20 @@ export class FeatureLoader { * Get the path to a feature's feature.json file */ getFeatureJsonPath(projectPath: string, featureId: string): string { - return path.join(this.getFeatureDir(projectPath, featureId), "feature.json"); + return path.join( + this.getFeatureDir(projectPath, featureId), + "feature.json" + ); } /** * Get the path to a feature's agent-output.md file */ getAgentOutputPath(projectPath: string, featureId: string): string { - return path.join(this.getFeatureDir(projectPath, featureId), "agent-output.md"); + return path.join( + this.getFeatureDir(projectPath, featureId), + "agent-output.md" + ); } /** @@ -189,13 +200,15 @@ export class FeatureLoader { // Check if features directory exists try { - await fs.access(featuresDir); + await secureFs.access(featuresDir); } catch { return []; } // Read all feature directories - const entries = await fs.readdir(featuresDir, { withFileTypes: true }); + const entries = (await secureFs.readdir(featuresDir, { + withFileTypes: true, + })) as any[]; const featureDirs = entries.filter((entry) => entry.isDirectory()); // Load each feature @@ -205,7 +218,10 @@ export class FeatureLoader { const featureJsonPath = this.getFeatureJsonPath(projectPath, featureId); try { - const content = await fs.readFile(featureJsonPath, "utf-8"); + const content = (await secureFs.readFile( + featureJsonPath, + "utf-8" + )) as string; const feature = JSON.parse(content); if (!feature.id) { @@ -252,7 +268,10 @@ export class FeatureLoader { async get(projectPath: string, featureId: string): Promise { try { const featureJsonPath = this.getFeatureJsonPath(projectPath, featureId); - const content = await fs.readFile(featureJsonPath, "utf-8"); + const content = (await secureFs.readFile( + featureJsonPath, + "utf-8" + )) as string; return JSON.parse(content); } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") { @@ -281,7 +300,7 @@ export class FeatureLoader { await ensureAutomakerDir(projectPath); // Create feature directory - await fs.mkdir(featureDir, { recursive: true }); + await secureFs.mkdir(featureDir, { recursive: true }); // Migrate images from temp directory to feature directory const migratedImagePaths = await this.migrateImages( @@ -300,7 +319,7 @@ export class FeatureLoader { }; // Write feature.json - await fs.writeFile( + await secureFs.writeFile( featureJsonPath, JSON.stringify(feature, null, 2), "utf-8" @@ -352,7 +371,7 @@ export class FeatureLoader { // Write back to file const featureJsonPath = this.getFeatureJsonPath(projectPath, featureId); - await fs.writeFile( + await secureFs.writeFile( featureJsonPath, JSON.stringify(updatedFeature, null, 2), "utf-8" @@ -368,8 +387,8 @@ export class FeatureLoader { async delete(projectPath: string, featureId: string): Promise { try { const featureDir = this.getFeatureDir(projectPath, featureId); - await fs.rm(featureDir, { recursive: true, force: true }); - logger.info(`Deleted feature ${featureId}`); + await secureFs.rm(featureDir, { recursive: true, force: true }); + console.log(`[FeatureLoader] Deleted feature ${featureId}`); return true; } catch (error) { logger.error( @@ -389,7 +408,10 @@ export class FeatureLoader { ): Promise { try { const agentOutputPath = this.getAgentOutputPath(projectPath, featureId); - const content = await fs.readFile(agentOutputPath, "utf-8"); + const content = (await secureFs.readFile( + agentOutputPath, + "utf-8" + )) as string; return content; } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") { @@ -412,10 +434,10 @@ export class FeatureLoader { content: string ): Promise { const featureDir = this.getFeatureDir(projectPath, featureId); - await fs.mkdir(featureDir, { recursive: true }); + await secureFs.mkdir(featureDir, { recursive: true }); const agentOutputPath = this.getAgentOutputPath(projectPath, featureId); - await fs.writeFile(agentOutputPath, content, "utf-8"); + await secureFs.writeFile(agentOutputPath, content, "utf-8"); } /** @@ -427,7 +449,7 @@ export class FeatureLoader { ): Promise { try { const agentOutputPath = this.getAgentOutputPath(projectPath, featureId); - await fs.unlink(agentOutputPath); + await secureFs.unlink(agentOutputPath); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") { throw error; diff --git a/apps/server/src/services/settings-service.ts b/apps/server/src/services/settings-service.ts index 0e48c62d..a935d93a 100644 --- a/apps/server/src/services/settings-service.ts +++ b/apps/server/src/services/settings-service.ts @@ -7,9 +7,9 @@ * - Per-project settings ({projectPath}/.automaker/settings.json) */ -import fs from "fs/promises"; -import path from "path"; import { createLogger } from "@automaker/utils"; +import * as secureFs from "../lib/secure-fs.js"; + import { getGlobalSettingsPath, getCredentialsPath, @@ -47,12 +47,12 @@ async function atomicWriteJson(filePath: string, data: unknown): Promise { const content = JSON.stringify(data, null, 2); try { - await fs.writeFile(tempPath, content, "utf-8"); - await fs.rename(tempPath, filePath); + await secureFs.writeFile(tempPath, content, "utf-8"); + await secureFs.rename(tempPath, filePath); } catch (error) { // Clean up temp file if it exists try { - await fs.unlink(tempPath); + await secureFs.unlink(tempPath); } catch { // Ignore cleanup errors } @@ -65,7 +65,7 @@ async function atomicWriteJson(filePath: string, data: unknown): Promise { */ async function readJsonFile(filePath: string, defaultValue: T): Promise { try { - const content = await fs.readFile(filePath, "utf-8"); + const content = (await secureFs.readFile(filePath, "utf-8")) as string; return JSON.parse(content) as T; } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") { @@ -81,7 +81,7 @@ async function readJsonFile(filePath: string, defaultValue: T): Promise { */ async function fileExists(filePath: string): Promise { try { - await fs.access(filePath); + await secureFs.access(filePath); return true; } catch { return false; @@ -232,9 +232,7 @@ export class SettingsService { * @param updates - Partial Credentials (usually just apiKeys) * @returns Promise resolving to complete updated Credentials object */ - async updateCredentials( - updates: Partial - ): Promise { + async updateCredentials(updates: Partial): Promise { await ensureDataDir(this.dataDir); const credentialsPath = getCredentialsPath(this.dataDir); @@ -270,8 +268,6 @@ export class SettingsService { */ async getMaskedCredentials(): Promise<{ anthropic: { configured: boolean; masked: string }; - google: { configured: boolean; masked: string }; - openai: { configured: boolean; masked: string }; }> { const credentials = await this.getCredentials(); @@ -285,14 +281,6 @@ export class SettingsService { configured: !!credentials.apiKeys.anthropic, masked: maskKey(credentials.apiKeys.anthropic), }, - google: { - configured: !!credentials.apiKeys.google, - masked: maskKey(credentials.apiKeys.google), - }, - openai: { - configured: !!credentials.apiKeys.openai, - masked: maskKey(credentials.apiKeys.openai), - }, }; } @@ -563,8 +551,7 @@ export class SettingsService { // Get theme from project object const project = projects.find((p) => p.path === projectPath); if (project?.theme) { - projectSettings.theme = - project.theme as ProjectSettings["theme"]; + projectSettings.theme = project.theme as ProjectSettings["theme"]; } if (boardBackgroundByProject?.[projectPath]) { @@ -586,7 +573,9 @@ export class SettingsService { migratedProjectCount++; } } catch (e) { - errors.push(`Failed to migrate project settings for ${projectPath}: ${e}`); + errors.push( + `Failed to migrate project settings for ${projectPath}: ${e}` + ); } } diff --git a/apps/server/src/types/settings.ts b/apps/server/src/types/settings.ts index 4b4fa3ac..ef9f32d6 100644 --- a/apps/server/src/types/settings.ts +++ b/apps/server/src/types/settings.ts @@ -22,7 +22,7 @@ export type { BoardBackgroundSettings, WorktreeInfo, ProjectSettings, -} from '@automaker/types'; +} from "@automaker/types"; export { DEFAULT_KEYBOARD_SHORTCUTS, @@ -32,4 +32,4 @@ export { SETTINGS_VERSION, CREDENTIALS_VERSION, PROJECT_SETTINGS_VERSION, -} from '@automaker/types'; +} from "@automaker/types"; diff --git a/apps/server/tests/setup.ts b/apps/server/tests/setup.ts index 3ac88134..2b00c614 100644 --- a/apps/server/tests/setup.ts +++ b/apps/server/tests/setup.ts @@ -8,7 +8,6 @@ import { vi, beforeEach } from "vitest"; // Set test environment variables process.env.NODE_ENV = "test"; process.env.DATA_DIR = "/tmp/test-data"; -process.env.ALLOWED_PROJECT_DIRS = "/tmp/test-projects"; // Reset all mocks before each test beforeEach(() => { diff --git a/apps/server/tests/unit/lib/security.test.ts b/apps/server/tests/unit/lib/security.test.ts index 629171cf..18c378da 100644 --- a/apps/server/tests/unit/lib/security.test.ts +++ b/apps/server/tests/unit/lib/security.test.ts @@ -11,134 +11,103 @@ describe("security.ts", () => { }); describe("initAllowedPaths", () => { - it("should parse comma-separated directories from environment", async () => { - process.env.ALLOWED_PROJECT_DIRS = "/path1,/path2,/path3"; - process.env.DATA_DIR = ""; + it("should load ALLOWED_ROOT_DIRECTORY if set", async () => { + process.env.ALLOWED_ROOT_DIRECTORY = "/projects"; + delete process.env.DATA_DIR; - const { initAllowedPaths, getAllowedPaths } = await import( - "@automaker/platform" - ); + const { initAllowedPaths, getAllowedPaths } = + await import("@automaker/platform"); initAllowedPaths(); const allowed = getAllowedPaths(); - expect(allowed).toContain(path.resolve("/path1")); - expect(allowed).toContain(path.resolve("/path2")); - expect(allowed).toContain(path.resolve("/path3")); + expect(allowed).toContain(path.resolve("/projects")); }); - it("should trim whitespace from paths", async () => { - process.env.ALLOWED_PROJECT_DIRS = " /path1 , /path2 , /path3 "; - process.env.DATA_DIR = ""; - - const { initAllowedPaths, getAllowedPaths } = await import( - "@automaker/platform" - ); - initAllowedPaths(); - - const allowed = getAllowedPaths(); - expect(allowed).toContain(path.resolve("/path1")); - expect(allowed).toContain(path.resolve("/path2")); - }); - - it("should always include DATA_DIR if set", async () => { - process.env.ALLOWED_PROJECT_DIRS = ""; + it("should include DATA_DIR if set", async () => { + delete process.env.ALLOWED_ROOT_DIRECTORY; process.env.DATA_DIR = "/data/dir"; - const { initAllowedPaths, getAllowedPaths } = await import( - "@automaker/platform" - ); + const { initAllowedPaths, getAllowedPaths } = + await import("@automaker/platform"); initAllowedPaths(); const allowed = getAllowedPaths(); expect(allowed).toContain(path.resolve("/data/dir")); }); - it("should include WORKSPACE_DIR if set", async () => { - process.env.ALLOWED_PROJECT_DIRS = ""; - process.env.DATA_DIR = ""; - process.env.WORKSPACE_DIR = "/workspace/dir"; - - const { initAllowedPaths, getAllowedPaths } = await import( - "@automaker/platform" - ); - initAllowedPaths(); - - const allowed = getAllowedPaths(); - expect(allowed).toContain(path.resolve("/workspace/dir")); - }); - - it("should handle empty ALLOWED_PROJECT_DIRS", async () => { - process.env.ALLOWED_PROJECT_DIRS = ""; + it("should include both ALLOWED_ROOT_DIRECTORY and DATA_DIR if both set", async () => { + process.env.ALLOWED_ROOT_DIRECTORY = "/projects"; process.env.DATA_DIR = "/data"; - delete process.env.WORKSPACE_DIR; - const { initAllowedPaths, getAllowedPaths } = await import( - "@automaker/platform" - ); - initAllowedPaths(); - - const allowed = getAllowedPaths(); - expect(allowed).toHaveLength(1); - expect(allowed[0]).toBe(path.resolve("/data")); - }); - - it("should skip empty entries in comma list", async () => { - process.env.ALLOWED_PROJECT_DIRS = "/path1,,/path2, ,/path3"; - process.env.DATA_DIR = ""; - delete process.env.WORKSPACE_DIR; - - const { initAllowedPaths, getAllowedPaths } = await import( - "@automaker/platform" - ); - initAllowedPaths(); - - const allowed = getAllowedPaths(); - expect(allowed).toHaveLength(3); - }); - }); - - describe("addAllowedPath", () => { - it("should add path to allowed list", async () => { - process.env.ALLOWED_PROJECT_DIRS = ""; - process.env.DATA_DIR = ""; - - const { initAllowedPaths, addAllowedPath, getAllowedPaths } = + const { initAllowedPaths, getAllowedPaths } = await import("@automaker/platform"); initAllowedPaths(); - addAllowedPath("/new/path"); - const allowed = getAllowedPaths(); - expect(allowed).toContain(path.resolve("/new/path")); + expect(allowed).toContain(path.resolve("/projects")); + expect(allowed).toContain(path.resolve("/data")); + expect(allowed).toHaveLength(2); }); - it("should resolve relative paths before adding", async () => { - process.env.ALLOWED_PROJECT_DIRS = ""; - process.env.DATA_DIR = ""; + it("should return empty array when no paths configured", async () => { + delete process.env.ALLOWED_ROOT_DIRECTORY; + delete process.env.DATA_DIR; - const { initAllowedPaths, addAllowedPath, getAllowedPaths } = + const { initAllowedPaths, getAllowedPaths } = await import("@automaker/platform"); initAllowedPaths(); - addAllowedPath("./relative/path"); - const allowed = getAllowedPaths(); - const cwd = process.cwd(); - expect(allowed).toContain(path.resolve(cwd, "./relative/path")); + expect(allowed).toHaveLength(0); }); }); describe("isPathAllowed", () => { - it("should allow all paths (permissions disabled)", async () => { - process.env.ALLOWED_PROJECT_DIRS = "/allowed/project"; + it("should allow paths within ALLOWED_ROOT_DIRECTORY", async () => { + process.env.ALLOWED_ROOT_DIRECTORY = "/allowed/project"; process.env.DATA_DIR = ""; - const { initAllowedPaths, isPathAllowed } = await import( - "@automaker/platform" - ); + const { initAllowedPaths, isPathAllowed } = + await import("@automaker/platform"); initAllowedPaths(); - // All paths are now allowed regardless of configuration + // Paths within allowed directory should be allowed + expect(isPathAllowed("/allowed/project/file.txt")).toBe(true); + expect(isPathAllowed("/allowed/project/subdir/file.txt")).toBe(true); + + // Paths outside allowed directory should be denied + expect(isPathAllowed("/not/allowed/file.txt")).toBe(false); + expect(isPathAllowed("/tmp/file.txt")).toBe(false); + expect(isPathAllowed("/etc/passwd")).toBe(false); + }); + + it("should allow all paths when no restrictions are configured", async () => { + delete process.env.DATA_DIR; + delete process.env.ALLOWED_ROOT_DIRECTORY; + + const { initAllowedPaths, isPathAllowed } = + await import("@automaker/platform"); + initAllowedPaths(); + + // All paths should be allowed when no restrictions are configured + expect(isPathAllowed("/allowed/project/file.txt")).toBe(true); + expect(isPathAllowed("/not/allowed/file.txt")).toBe(true); + expect(isPathAllowed("/tmp/file.txt")).toBe(true); + expect(isPathAllowed("/etc/passwd")).toBe(true); + expect(isPathAllowed("/any/path")).toBe(true); + }); + + it("should allow all paths when DATA_DIR is set but ALLOWED_ROOT_DIRECTORY is not", async () => { + process.env.DATA_DIR = "/data"; + delete process.env.ALLOWED_ROOT_DIRECTORY; + + const { initAllowedPaths, isPathAllowed } = + await import("@automaker/platform"); + initAllowedPaths(); + + // DATA_DIR should be allowed + expect(isPathAllowed("/data/settings.json")).toBe(true); + // But all other paths should also be allowed when ALLOWED_ROOT_DIRECTORY is not set expect(isPathAllowed("/allowed/project/file.txt")).toBe(true); expect(isPathAllowed("/not/allowed/file.txt")).toBe(true); expect(isPathAllowed("/tmp/file.txt")).toBe(true); @@ -148,43 +117,52 @@ describe("security.ts", () => { }); describe("validatePath", () => { - it("should return resolved path for any path (permissions disabled)", async () => { - process.env.ALLOWED_PROJECT_DIRS = "/allowed"; + it("should return resolved path for allowed paths", async () => { + process.env.ALLOWED_ROOT_DIRECTORY = "/allowed"; process.env.DATA_DIR = ""; - const { initAllowedPaths, validatePath } = await import( - "@automaker/platform" - ); + const { initAllowedPaths, validatePath } = + await import("@automaker/platform"); initAllowedPaths(); const result = validatePath("/allowed/file.txt"); expect(result).toBe(path.resolve("/allowed/file.txt")); }); - it("should not throw error for any path (permissions disabled)", async () => { - process.env.ALLOWED_PROJECT_DIRS = "/allowed"; + it("should throw error for paths outside allowed directories", async () => { + process.env.ALLOWED_ROOT_DIRECTORY = "/allowed"; process.env.DATA_DIR = ""; - const { initAllowedPaths, validatePath } = await import( - "@automaker/platform" - ); + const { initAllowedPaths, validatePath } = + await import("@automaker/platform"); initAllowedPaths(); - // All paths are now allowed, no errors thrown + // Disallowed paths should throw PathNotAllowedError + expect(() => validatePath("/disallowed/file.txt")).toThrow(); + }); + + it("should not throw error for any path when no restrictions are configured", async () => { + delete process.env.DATA_DIR; + delete process.env.ALLOWED_ROOT_DIRECTORY; + + const { initAllowedPaths, validatePath } = + await import("@automaker/platform"); + initAllowedPaths(); + + // All paths are allowed when no restrictions configured expect(() => validatePath("/disallowed/file.txt")).not.toThrow(); expect(validatePath("/disallowed/file.txt")).toBe( path.resolve("/disallowed/file.txt") ); }); - it("should resolve relative paths", async () => { + it("should resolve relative paths within allowed directory", async () => { const cwd = process.cwd(); - process.env.ALLOWED_PROJECT_DIRS = cwd; + process.env.ALLOWED_ROOT_DIRECTORY = cwd; process.env.DATA_DIR = ""; - const { initAllowedPaths, validatePath } = await import( - "@automaker/platform" - ); + const { initAllowedPaths, validatePath } = + await import("@automaker/platform"); initAllowedPaths(); const result = validatePath("./file.txt"); @@ -194,26 +172,26 @@ describe("security.ts", () => { describe("getAllowedPaths", () => { it("should return array of allowed paths", async () => { - process.env.ALLOWED_PROJECT_DIRS = "/path1,/path2"; + process.env.ALLOWED_ROOT_DIRECTORY = "/projects"; process.env.DATA_DIR = "/data"; - const { initAllowedPaths, getAllowedPaths } = await import( - "@automaker/platform" - ); + const { initAllowedPaths, getAllowedPaths } = + await import("@automaker/platform"); initAllowedPaths(); const result = getAllowedPaths(); expect(Array.isArray(result)).toBe(true); - expect(result.length).toBeGreaterThan(0); + expect(result.length).toBe(2); + expect(result).toContain(path.resolve("/projects")); + expect(result).toContain(path.resolve("/data")); }); it("should return resolved paths", async () => { - process.env.ALLOWED_PROJECT_DIRS = "/test"; + process.env.ALLOWED_ROOT_DIRECTORY = "/test"; process.env.DATA_DIR = ""; - const { initAllowedPaths, getAllowedPaths } = await import( - "@automaker/platform" - ); + const { initAllowedPaths, getAllowedPaths } = + await import("@automaker/platform"); initAllowedPaths(); const result = getAllowedPaths(); diff --git a/apps/server/tests/unit/services/settings-service.test.ts b/apps/server/tests/unit/services/settings-service.test.ts index bed7d3e6..ecde0fb9 100644 --- a/apps/server/tests/unit/services/settings-service.test.ts +++ b/apps/server/tests/unit/services/settings-service.test.ts @@ -183,8 +183,6 @@ describe("settings-service.ts", () => { ...DEFAULT_CREDENTIALS, apiKeys: { anthropic: "sk-test-key", - google: "", - openai: "", }, }; const credentialsPath = path.join(testDataDir, "credentials.json"); @@ -206,8 +204,6 @@ describe("settings-service.ts", () => { const credentials = await settingsService.getCredentials(); expect(credentials.apiKeys.anthropic).toBe("sk-test"); - expect(credentials.apiKeys.google).toBe(""); - expect(credentials.apiKeys.openai).toBe(""); }); }); @@ -216,8 +212,6 @@ describe("settings-service.ts", () => { const updates: Partial = { apiKeys: { anthropic: "sk-test-key", - google: "", - openai: "", }, }; @@ -237,8 +231,6 @@ describe("settings-service.ts", () => { ...DEFAULT_CREDENTIALS, apiKeys: { anthropic: "sk-initial", - google: "google-key", - openai: "", }, }; const credentialsPath = path.join(testDataDir, "credentials.json"); @@ -253,7 +245,6 @@ describe("settings-service.ts", () => { const updated = await settingsService.updateCredentials(updates); expect(updated.apiKeys.anthropic).toBe("sk-updated"); - expect(updated.apiKeys.google).toBe("google-key"); // Preserved }); it("should deep merge api keys", async () => { @@ -261,8 +252,6 @@ describe("settings-service.ts", () => { ...DEFAULT_CREDENTIALS, apiKeys: { anthropic: "sk-anthropic", - google: "google-key", - openai: "openai-key", }, }; const credentialsPath = path.join(testDataDir, "credentials.json"); @@ -270,15 +259,13 @@ describe("settings-service.ts", () => { const updates: Partial = { apiKeys: { - openai: "new-openai-key", + anthropic: "sk-updated-anthropic", }, }; const updated = await settingsService.updateCredentials(updates); - expect(updated.apiKeys.anthropic).toBe("sk-anthropic"); - expect(updated.apiKeys.google).toBe("google-key"); - expect(updated.apiKeys.openai).toBe("new-openai-key"); + expect(updated.apiKeys.anthropic).toBe("sk-updated-anthropic"); }); }); @@ -287,34 +274,24 @@ describe("settings-service.ts", () => { const masked = await settingsService.getMaskedCredentials(); expect(masked.anthropic.configured).toBe(false); expect(masked.anthropic.masked).toBe(""); - expect(masked.google.configured).toBe(false); - expect(masked.openai.configured).toBe(false); }); it("should mask keys correctly", async () => { await settingsService.updateCredentials({ apiKeys: { anthropic: "sk-ant-api03-1234567890abcdef", - google: "AIzaSy1234567890abcdef", - openai: "sk-1234567890abcdef", }, }); const masked = await settingsService.getMaskedCredentials(); expect(masked.anthropic.configured).toBe(true); expect(masked.anthropic.masked).toBe("sk-a...cdef"); - expect(masked.google.configured).toBe(true); - expect(masked.google.masked).toBe("AIza...cdef"); - expect(masked.openai.configured).toBe(true); - expect(masked.openai.masked).toBe("sk-1...cdef"); }); it("should handle short keys", async () => { await settingsService.updateCredentials({ apiKeys: { anthropic: "short", - google: "", - openai: "", }, }); @@ -332,7 +309,7 @@ describe("settings-service.ts", () => { it("should return true when credentials file exists", async () => { await settingsService.updateCredentials({ - apiKeys: { anthropic: "test", google: "", openai: "" }, + apiKeys: { anthropic: "test" }, }); const exists = await settingsService.hasCredentials(); expect(exists).toBe(true); @@ -508,8 +485,6 @@ describe("settings-service.ts", () => { state: { apiKeys: { anthropic: "sk-test-key", - google: "google-key", - openai: "openai-key", }, }, }), @@ -522,8 +497,6 @@ describe("settings-service.ts", () => { const credentials = await settingsService.getCredentials(); expect(credentials.apiKeys.anthropic).toBe("sk-test-key"); - expect(credentials.apiKeys.google).toBe("google-key"); - expect(credentials.apiKeys.openai).toBe("openai-key"); }); it("should migrate project settings from localStorage data", async () => { diff --git a/apps/ui/playwright.config.ts b/apps/ui/playwright.config.ts index 65ab32cb..c2a4ac46 100644 --- a/apps/ui/playwright.config.ts +++ b/apps/ui/playwright.config.ts @@ -40,8 +40,7 @@ export default defineConfig({ PORT: String(serverPort), // Enable mock agent in CI to avoid real API calls AUTOMAKER_MOCK_AGENT: mockAgent ? "true" : "false", - // Allow access to test directories and common project paths - ALLOWED_PROJECT_DIRS: "/Users,/home,/tmp,/var/folders", + // No ALLOWED_ROOT_DIRECTORY restriction - allow all paths for testing }, }, // Frontend Vite dev server @@ -54,7 +53,8 @@ export default defineConfig({ ...process.env, VITE_SKIP_SETUP: "true", // Skip electron plugin in CI - no display available for Electron - VITE_SKIP_ELECTRON: process.env.CI === "true" ? "true" : undefined, + VITE_SKIP_ELECTRON: + process.env.CI === "true" ? "true" : undefined, }, }, ], diff --git a/apps/ui/src/components/dialogs/file-browser-dialog.tsx b/apps/ui/src/components/dialogs/file-browser-dialog.tsx index 1687218a..b6a05ab0 100644 --- a/apps/ui/src/components/dialogs/file-browser-dialog.tsx +++ b/apps/ui/src/components/dialogs/file-browser-dialog.tsx @@ -1,4 +1,3 @@ - import { useState, useEffect, useRef, useCallback } from "react"; import { FolderOpen, @@ -21,6 +20,11 @@ import { } from "@/components/ui/dialog"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; +import { getJSON, setJSON } from "@/lib/storage"; +import { + getDefaultWorkspaceDirectory, + saveLastProjectDirectory, +} from "@/lib/workspace-config"; interface DirectoryEntry { name: string; @@ -50,38 +54,22 @@ const RECENT_FOLDERS_KEY = "file-browser-recent-folders"; const MAX_RECENT_FOLDERS = 5; function getRecentFolders(): string[] { - if (typeof window === "undefined") return []; - try { - const stored = localStorage.getItem(RECENT_FOLDERS_KEY); - return stored ? JSON.parse(stored) : []; - } catch { - return []; - } + return getJSON(RECENT_FOLDERS_KEY) ?? []; } function addRecentFolder(path: string): void { - if (typeof window === "undefined") return; - try { - const recent = getRecentFolders(); - // Remove if already exists, then add to front - const filtered = recent.filter((p) => p !== path); - const updated = [path, ...filtered].slice(0, MAX_RECENT_FOLDERS); - localStorage.setItem(RECENT_FOLDERS_KEY, JSON.stringify(updated)); - } catch { - // Ignore localStorage errors - } + const recent = getRecentFolders(); + // Remove if already exists, then add to front + const filtered = recent.filter((p) => p !== path); + const updated = [path, ...filtered].slice(0, MAX_RECENT_FOLDERS); + setJSON(RECENT_FOLDERS_KEY, updated); } function removeRecentFolder(path: string): string[] { - if (typeof window === "undefined") return []; - try { - const recent = getRecentFolders(); - const updated = recent.filter((p) => p !== path); - localStorage.setItem(RECENT_FOLDERS_KEY, JSON.stringify(updated)); - return updated; - } catch { - return []; - } + const recent = getRecentFolders(); + const updated = recent.filter((p) => p !== path); + setJSON(RECENT_FOLDERS_KEY, updated); + return updated; } export function FileBrowserDialog({ @@ -110,17 +98,16 @@ export function FileBrowserDialog({ } }, [open]); - const handleRemoveRecent = useCallback((e: React.MouseEvent, path: string) => { - e.stopPropagation(); - const updated = removeRecentFolder(path); - setRecentFolders(updated); - }, []); + const handleRemoveRecent = useCallback( + (e: React.MouseEvent, path: string) => { + e.stopPropagation(); + const updated = removeRecentFolder(path); + setRecentFolders(updated); + }, + [] + ); - const handleSelectRecent = useCallback((path: string) => { - browseDirectory(path); - }, []); - - const browseDirectory = async (dirPath?: string) => { + const browseDirectory = useCallback(async (dirPath?: string) => { setLoading(true); setError(""); setWarning(""); @@ -155,7 +142,14 @@ export function FileBrowserDialog({ } finally { setLoading(false); } - }; + }, []); + + const handleSelectRecent = useCallback( + (path: string) => { + browseDirectory(path); + }, + [browseDirectory] + ); // Reset current path when dialog closes useEffect(() => { @@ -169,12 +163,46 @@ export function FileBrowserDialog({ } }, [open]); - // Load initial path or home directory when dialog opens + // Load initial path or workspace directory when dialog opens useEffect(() => { if (open && !currentPath) { - browseDirectory(initialPath); + // Priority order: + // 1. Last selected directory from this file browser (from localStorage) + // 2. initialPath prop (from parent component) + // 3. Default workspace directory + // 4. Home directory + const loadInitialPath = async () => { + try { + // First, check for last selected directory from getDefaultWorkspaceDirectory + // which already implements the priority: last used > Documents/Automaker > DATA_DIR + const defaultDir = await getDefaultWorkspaceDirectory(); + + // If we have a default directory, use it (unless initialPath is explicitly provided and different) + const pathToUse = initialPath || defaultDir; + + if (pathToUse) { + // Pre-fill the path input immediately + setPathInput(pathToUse); + // Then browse to that directory + browseDirectory(pathToUse); + } else { + // No default directory, browse home directory + browseDirectory(); + } + } catch (err) { + // If config fetch fails, try initialPath or fall back to home directory + if (initialPath) { + setPathInput(initialPath); + browseDirectory(initialPath); + } else { + browseDirectory(); + } + } + }; + + loadInitialPath(); } - }, [open, initialPath]); + }, [open, initialPath, currentPath, browseDirectory]); const handleSelectDirectory = (dir: DirectoryEntry) => { browseDirectory(dir.path); @@ -211,6 +239,8 @@ export function FileBrowserDialog({ const handleSelect = useCallback(() => { if (currentPath) { addRecentFolder(currentPath); + // Save to last project directory so it's used as default next time + saveLastProjectDirectory(currentPath); onSelect(currentPath); onOpenChange(false); } @@ -296,7 +326,9 @@ export function FileBrowserDialog({ title={folder} > - {getFolderName(folder)} + + {getFolderName(folder)} + - diff --git a/apps/ui/src/components/new-project-modal.tsx b/apps/ui/src/components/new-project-modal.tsx index 0af03a5d..93eef763 100644 --- a/apps/ui/src/components/new-project-modal.tsx +++ b/apps/ui/src/components/new-project-modal.tsx @@ -1,4 +1,3 @@ - import { useState, useEffect } from "react"; import { Dialog, @@ -26,11 +25,12 @@ import { } from "lucide-react"; import { starterTemplates, type StarterTemplate } from "@/lib/templates"; import { getElectronAPI } from "@/lib/electron"; -import { getHttpApiClient } from "@/lib/http-api-client"; import { cn } from "@/lib/utils"; import { useFileBrowser } from "@/contexts/file-browser-context"; - -const LAST_PROJECT_DIR_KEY = "automaker:lastProjectDir"; +import { + getDefaultWorkspaceDirectory, + saveLastProjectDirectory, +} from "@/lib/workspace-config"; interface ValidationErrors { projectName?: boolean; @@ -81,25 +81,15 @@ export function NewProjectModal({ // Fetch workspace directory when modal opens useEffect(() => { if (open) { - // First, check localStorage for last used directory - const lastUsedDir = localStorage.getItem(LAST_PROJECT_DIR_KEY); - if (lastUsedDir) { - setWorkspaceDir(lastUsedDir); - return; - } - - // Fall back to server config if no saved directory setIsLoadingWorkspace(true); - const httpClient = getHttpApiClient(); - httpClient.workspace - .getConfig() - .then((result) => { - if (result.success && result.workspaceDir) { - setWorkspaceDir(result.workspaceDir); + getDefaultWorkspaceDirectory() + .then((defaultDir) => { + if (defaultDir) { + setWorkspaceDir(defaultDir); } }) .catch((error) => { - console.error("Failed to get workspace config:", error); + console.error("Failed to get default workspace directory:", error); }) .finally(() => { setIsLoadingWorkspace(false); @@ -211,7 +201,7 @@ export function NewProjectModal({ if (selectedPath) { setWorkspaceDir(selectedPath); // Save to localStorage for next time - localStorage.setItem(LAST_PROJECT_DIR_KEY, selectedPath); + saveLastProjectDirectory(selectedPath); // Clear any workspace error when a valid directory is selected if (errors.workspaceDir) { setErrors((prev) => ({ ...prev, workspaceDir: false })); @@ -296,9 +286,7 @@ export function NewProjectModal({ {projectPath || workspaceDir} - ) : ( - No workspace configured - )} + ) : null}