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/automaker-paths.ts b/apps/server/src/lib/automaker-paths.ts index 988d7bbc..556650f8 100644 --- a/apps/server/src/lib/automaker-paths.ts +++ b/apps/server/src/lib/automaker-paths.ts @@ -9,7 +9,7 @@ * Directory creation is handled separately by ensure* functions. */ -import fs from "fs/promises"; +import * as secureFs from "./secure-fs.js"; import path from "path"; /** @@ -149,7 +149,7 @@ export function getBranchTrackingPath(projectPath: string): string { */ export async function ensureAutomakerDir(projectPath: string): Promise { const automakerDir = getAutomakerDir(projectPath); - await fs.mkdir(automakerDir, { recursive: true }); + await secureFs.mkdir(automakerDir, { recursive: true }); return automakerDir; } @@ -211,6 +211,6 @@ export function getProjectSettingsPath(projectPath: string): string { * @returns Promise resolving to the created data directory path */ export async function ensureDataDir(dataDir: string): Promise { - await fs.mkdir(dataDir, { recursive: true }); + await secureFs.mkdir(dataDir, { recursive: true }); return dataDir; } diff --git a/apps/server/src/lib/fs-utils.ts b/apps/server/src/lib/fs-utils.ts index 5b67124a..ac60dda6 100644 --- a/apps/server/src/lib/fs-utils.ts +++ b/apps/server/src/lib/fs-utils.ts @@ -2,7 +2,7 @@ * File system utilities that handle symlinks safely */ -import fs from "fs/promises"; +import * as secureFs from "./secure-fs.js"; import path from "path"; /** @@ -14,7 +14,7 @@ export async function mkdirSafe(dirPath: string): Promise { // Check if path already exists using lstat (doesn't follow symlinks) try { - const stats = await fs.lstat(resolvedPath); + const stats = await secureFs.lstat(resolvedPath); // Path exists - if it's a directory or symlink, consider it success if (stats.isDirectory() || stats.isSymbolicLink()) { return; @@ -36,7 +36,7 @@ export async function mkdirSafe(dirPath: string): Promise { // Path doesn't exist, create it try { - await fs.mkdir(resolvedPath, { recursive: true }); + await secureFs.mkdir(resolvedPath, { recursive: true }); } catch (error: any) { // Handle race conditions and symlink issues if (error.code === "EEXIST" || error.code === "ELOOP") { @@ -52,7 +52,7 @@ export async function mkdirSafe(dirPath: string): Promise { */ export async function existsSafe(filePath: string): Promise { try { - await fs.lstat(filePath); + await secureFs.lstat(filePath); return true; } catch (error: any) { if (error.code === "ENOENT") { diff --git a/apps/server/src/lib/image-handler.ts b/apps/server/src/lib/image-handler.ts index 167f948f..e0c92688 100644 --- a/apps/server/src/lib/image-handler.ts +++ b/apps/server/src/lib/image-handler.ts @@ -8,7 +8,7 @@ * - Path resolution (relative/absolute) */ -import fs from "fs/promises"; +import * as secureFs from "./secure-fs.js"; import path from "path"; /** @@ -63,7 +63,7 @@ export function getMimeTypeForImage(imagePath: string): string { * @throws Error if file cannot be read */ export async function readImageAsBase64(imagePath: string): Promise { - const imageBuffer = await fs.readFile(imagePath); + const imageBuffer = await secureFs.readFile(imagePath) as Buffer; const base64Data = imageBuffer.toString("base64"); const mimeType = getMimeTypeForImage(imagePath); diff --git a/apps/server/src/lib/secure-fs.ts b/apps/server/src/lib/secure-fs.ts new file mode 100644 index 00000000..805457c9 --- /dev/null +++ b/apps/server/src/lib/secure-fs.ts @@ -0,0 +1,168 @@ +/** + * Secure File System Adapter + * + * All file I/O operations must go through this adapter to enforce + * ALLOWED_ROOT_DIRECTORY restrictions at the actual access point, + * not just at the API layer. This provides defense-in-depth security. + */ + +import fs from "fs/promises"; +import type { Dirent } from "fs"; +import path from "path"; +import { validatePath } from "./security.js"; + +/** + * Wrapper around fs.access that validates path first + */ +export async function access(filePath: string, mode?: number): Promise { + const validatedPath = validatePath(filePath); + return fs.access(validatedPath, mode); +} + +/** + * Wrapper around fs.readFile that validates path first + */ +export async function readFile( + filePath: string, + encoding?: BufferEncoding +): Promise { + const validatedPath = validatePath(filePath); + if (encoding) { + return fs.readFile(validatedPath, encoding); + } + return fs.readFile(validatedPath); +} + +/** + * Wrapper around fs.writeFile that validates path first + */ +export async function writeFile( + filePath: string, + data: string | Buffer, + encoding?: BufferEncoding +): Promise { + const validatedPath = validatePath(filePath); + return fs.writeFile(validatedPath, data, encoding); +} + +/** + * Wrapper around fs.mkdir that validates path first + */ +export async function mkdir( + dirPath: string, + options?: { recursive?: boolean; mode?: number } +): Promise { + const validatedPath = validatePath(dirPath); + return fs.mkdir(validatedPath, options); +} + +/** + * Wrapper around fs.readdir that validates path first + */ +export async function readdir( + dirPath: string, + options?: { withFileTypes?: false; encoding?: BufferEncoding } +): Promise; +export async function readdir( + dirPath: string, + options: { withFileTypes: true; encoding?: BufferEncoding } +): Promise; +export async function readdir( + dirPath: string, + options?: { withFileTypes?: boolean; encoding?: BufferEncoding } +): Promise { + const validatedPath = validatePath(dirPath); + if (options?.withFileTypes === true) { + return fs.readdir(validatedPath, { withFileTypes: true }); + } + return fs.readdir(validatedPath); +} + +/** + * Wrapper around fs.stat that validates path first + */ +export async function stat(filePath: string): Promise { + const validatedPath = validatePath(filePath); + return fs.stat(validatedPath); +} + +/** + * Wrapper around fs.rm that validates path first + */ +export async function rm( + filePath: string, + options?: { recursive?: boolean; force?: boolean } +): Promise { + const validatedPath = validatePath(filePath); + return fs.rm(validatedPath, options); +} + +/** + * Wrapper around fs.unlink that validates path first + */ +export async function unlink(filePath: string): Promise { + const validatedPath = validatePath(filePath); + return fs.unlink(validatedPath); +} + +/** + * Wrapper around fs.copyFile that validates both paths first + */ +export async function copyFile( + src: string, + dest: string, + mode?: number +): Promise { + const validatedSrc = validatePath(src); + const validatedDest = validatePath(dest); + return fs.copyFile(validatedSrc, validatedDest, mode); +} + +/** + * Wrapper around fs.appendFile that validates path first + */ +export async function appendFile( + filePath: string, + data: string | Buffer, + encoding?: BufferEncoding +): Promise { + const validatedPath = validatePath(filePath); + return fs.appendFile(validatedPath, data, encoding); +} + +/** + * Wrapper around fs.rename that validates both paths first + */ +export async function rename( + oldPath: string, + newPath: string +): Promise { + const validatedOldPath = validatePath(oldPath); + const validatedNewPath = validatePath(newPath); + return fs.rename(validatedOldPath, validatedNewPath); +} + +/** + * Wrapper around fs.lstat that validates path first + * Returns file stats without following symbolic links + */ +export async function lstat(filePath: string): Promise { + const validatedPath = validatePath(filePath); + return fs.lstat(validatedPath); +} + +/** + * Wrapper around path.join that returns resolved path + * Does NOT validate - use this for path construction, then pass to other operations + */ +export function joinPath(...pathSegments: string[]): string { + return path.join(...pathSegments); +} + +/** + * Wrapper around path.resolve that returns resolved path + * Does NOT validate - use this for path construction, then pass to other operations + */ +export function resolvePath(...pathSegments: string[]): string { + return path.resolve(...pathSegments); +} diff --git a/apps/server/src/lib/security.ts b/apps/server/src/lib/security.ts index 7525d82f..8ff065f6 100644 --- a/apps/server/src/lib/security.ts +++ b/apps/server/src/lib/security.ts @@ -1,63 +1,143 @@ /** * Security utilities for path validation - * Note: All permission checks have been disabled to allow unrestricted access + * Enforces ALLOWED_ROOT_DIRECTORY constraint with appData exception */ import path from "path"; -// Allowed project directories - kept for API compatibility -const allowedPaths = new Set(); +/** + * Error thrown when a path is not allowed by security policy + */ +export class PathNotAllowedError extends Error { + constructor(filePath: string) { + super( + `Path not allowed: ${filePath}. Must be within ALLOWED_ROOT_DIRECTORY or DATA_DIR.` + ); + this.name = "PathNotAllowedError"; + } +} + +// Allowed root directory - main security boundary +let allowedRootDirectory: string | null = null; + +// Data directory - always allowed for settings/credentials +let dataDirectory: string | null = null; /** - * Initialize allowed paths from environment variable - * Note: All paths are now allowed regardless of this setting + * Initialize security settings from environment variables + * - ALLOWED_ROOT_DIRECTORY: main security boundary + * - DATA_DIR: appData exception, always allowed */ export function initAllowedPaths(): void { - const dirs = process.env.ALLOWED_PROJECT_DIRS; - if (dirs) { - for (const dir of dirs.split(",")) { - const trimmed = dir.trim(); - if (trimmed) { - allowedPaths.add(path.resolve(trimmed)); - } - } + // Load ALLOWED_ROOT_DIRECTORY + const rootDir = process.env.ALLOWED_ROOT_DIRECTORY; + if (rootDir) { + allowedRootDirectory = path.resolve(rootDir); + console.log( + `[Security] ✓ ALLOWED_ROOT_DIRECTORY configured: ${allowedRootDirectory}` + ); + } else { + console.log( + "[Security] ⚠️ ALLOWED_ROOT_DIRECTORY not set - allowing access to all paths" + ); } + // Load DATA_DIR (appData exception - always allowed) const dataDir = process.env.DATA_DIR; if (dataDir) { - allowedPaths.add(path.resolve(dataDir)); - } - - const workspaceDir = process.env.WORKSPACE_DIR; - if (workspaceDir) { - allowedPaths.add(path.resolve(workspaceDir)); + dataDirectory = path.resolve(dataDir); + console.log(`[Security] ✓ DATA_DIR configured: ${dataDirectory}`); } } /** - * Add a path to the allowed list (no-op, all paths allowed) + * Check if a path is allowed based on ALLOWED_ROOT_DIRECTORY + * Returns true if: + * - Path is within ALLOWED_ROOT_DIRECTORY, OR + * - Path is within DATA_DIR (appData exception), OR + * - No restrictions are configured (backward compatibility) */ -export function addAllowedPath(filePath: string): void { - allowedPaths.add(path.resolve(filePath)); +export function isPathAllowed(filePath: string): boolean { + const resolvedPath = path.resolve(filePath); + + // Always allow appData directory (settings, credentials) + if (dataDirectory && isPathWithinDirectory(resolvedPath, dataDirectory)) { + return true; + } + + // If no ALLOWED_ROOT_DIRECTORY restriction is configured, allow all paths + // Note: DATA_DIR is checked above as an exception, but doesn't restrict other paths + if (!allowedRootDirectory) { + return true; + } + + // Allow if within ALLOWED_ROOT_DIRECTORY + if ( + allowedRootDirectory && + isPathWithinDirectory(resolvedPath, allowedRootDirectory) + ) { + return true; + } + + // If restrictions are configured but path doesn't match, deny + return false; } /** - * Check if a path is allowed - always returns true - */ -export function isPathAllowed(_filePath: string): boolean { - return true; -} - -/** - * Validate a path - just resolves the path without checking permissions + * Validate a path - resolves it and checks permissions + * Throws PathNotAllowedError if path is not allowed */ export function validatePath(filePath: string): string { - return path.resolve(filePath); + const resolvedPath = path.resolve(filePath); + + if (!isPathAllowed(resolvedPath)) { + throw new PathNotAllowedError(filePath); + } + + return resolvedPath; +} + +/** + * Check if a path is within a directory, with protection against path traversal + * Returns true only if resolvedPath is within directoryPath + */ +export function isPathWithinDirectory( + resolvedPath: string, + directoryPath: string +): boolean { + // Get the relative path from directory to the target + const relativePath = path.relative(directoryPath, resolvedPath); + + // If relative path starts with "..", it's outside the directory + // If relative path is absolute, it's outside the directory + // If relative path is empty or ".", it's the directory itself + return !relativePath.startsWith("..") && !path.isAbsolute(relativePath); +} + +/** + * Get the configured allowed root directory + */ +export function getAllowedRootDirectory(): string | null { + return allowedRootDirectory; +} + +/** + * Get the configured data directory + */ +export function getDataDirectory(): string | null { + return dataDirectory; } /** * Get list of allowed paths (for debugging) */ export function getAllowedPaths(): string[] { - return Array.from(allowedPaths); + const paths: string[] = []; + if (allowedRootDirectory) { + paths.push(allowedRootDirectory); + } + if (dataDirectory) { + paths.push(dataDirectory); + } + return paths; } diff --git a/apps/server/src/middleware/validate-paths.ts b/apps/server/src/middleware/validate-paths.ts new file mode 100644 index 00000000..e4052ab8 --- /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 "../lib/security.js"; + +/** + * 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 fa012e89..f9e71cfb 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 "../../../lib/logger.js"; 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 3686bad5..8cd111f6 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 "../../../lib/logger.js"; 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 fda12589..8d228608 100644 --- a/apps/server/src/routes/features/routes/create.ts +++ b/apps/server/src/routes/features/routes/create.ts @@ -7,7 +7,6 @@ import { FeatureLoader, type Feature, } from "../../../services/feature-loader.js"; -import { addAllowedPath } from "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; export function createCreateHandler(featureLoader: FeatureLoader) { @@ -28,9 +27,6 @@ export function createCreateHandler(featureLoader: FeatureLoader) { 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 33dc68b6..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 "../../../lib/security.js"; 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..c8757ff6 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 "../../../lib/security.js"; 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 0f0604f1..20820c30 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 "../../../lib/security.js"; +import { validatePath, PathNotAllowedError } from "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; export function createDeleteHandler() { @@ -22,6 +22,12 @@ export function createDeleteHandler() { res.json({ success: true }); } catch (error) { + // Path not allowed - return 403 Forbidden + if (error instanceof PathNotAllowedError) { + res.status(403).json({ success: false, error: getErrorMessage(error) }); + return; + } + logError(error, "Delete file failed"); res.status(500).json({ success: false, error: getErrorMessage(error) }); } diff --git a/apps/server/src/routes/fs/routes/exists.ts b/apps/server/src/routes/fs/routes/exists.ts index 2ca33bee..b63266b4 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 "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; export function createExistsHandler() { @@ -17,10 +18,13 @@ export function createExistsHandler() { return; } - // For exists, we check but don't require the path to be pre-allowed - // This allows the UI to validate user-entered paths const resolvedPath = path.resolve(filePath); + // Validate that the path is allowed + if (!isPathAllowed(resolvedPath)) { + throw new PathNotAllowedError(filePath); + } + try { await fs.access(resolvedPath); res.json({ success: true, exists: true }); @@ -28,6 +32,12 @@ export function createExistsHandler() { res.json({ success: true, exists: false }); } } catch (error) { + // Path not allowed - return 403 Forbidden + if (error instanceof PathNotAllowedError) { + res.status(403).json({ success: false, error: getErrorMessage(error) }); + return; + } + logError(error, "Check exists failed"); res.status(500).json({ success: false, error: getErrorMessage(error) }); } diff --git a/apps/server/src/routes/fs/routes/mkdir.ts b/apps/server/src/routes/fs/routes/mkdir.ts index 8cf41033..9b1ee322 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 "../../../lib/security.js"; +import { isPathAllowed, PathNotAllowedError } from "../../../lib/security.js"; 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 a1833d5c..cceb1669 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 "../../../lib/security.js"; +import { validatePath, PathNotAllowedError } from "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; // Optional files that are expected to not exist in new projects @@ -39,6 +39,12 @@ export function createReadHandler() { res.json({ success: true, content }); } catch (error) { + // Path not allowed - return 403 Forbidden + if (error instanceof PathNotAllowedError) { + res.status(403).json({ success: false, error: getErrorMessage(error) }); + return; + } + // Don't log ENOENT errors for optional files (expected to be missing in new projects) const shouldLog = !(isENOENT(error) && isOptionalFile(req.body?.filePath || "")); if (shouldLog) { diff --git a/apps/server/src/routes/fs/routes/readdir.ts b/apps/server/src/routes/fs/routes/readdir.ts index c30fa6b2..093fac07 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 "../../../lib/security.js"; +import { validatePath, PathNotAllowedError } from "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; export function createReaddirHandler() { @@ -28,6 +28,12 @@ export function createReaddirHandler() { res.json({ success: true, entries: result }); } catch (error) { + // Path not allowed - return 403 Forbidden + if (error instanceof PathNotAllowedError) { + res.status(403).json({ success: false, error: getErrorMessage(error) }); + return; + } + logError(error, "Read directory failed"); res.status(500).json({ success: false, error: getErrorMessage(error) }); } diff --git a/apps/server/src/routes/fs/routes/resolve-directory.ts b/apps/server/src/routes/fs/routes/resolve-directory.ts index 9b165c42..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 "../../../lib/security.js"; 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 9a496c7c..6c68eebd 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 "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; import { getBoardDir } from "../../../lib/automaker-paths.js"; @@ -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 b56b5a12..983da34b 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 "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; import { getImagesDir } from "../../../lib/automaker-paths.js"; @@ -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 b92ed00c..886510a3 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 "../../../lib/security.js"; +import { validatePath, PathNotAllowedError } from "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; export function createStatHandler() { @@ -30,6 +30,12 @@ export function createStatHandler() { }, }); } catch (error) { + // Path not allowed - return 403 Forbidden + if (error instanceof PathNotAllowedError) { + res.status(403).json({ success: false, error: getErrorMessage(error) }); + return; + } + logError(error, "Get file stats failed"); res.status(500).json({ success: false, error: getErrorMessage(error) }); } diff --git a/apps/server/src/routes/fs/routes/validate-path.ts b/apps/server/src/routes/fs/routes/validate-path.ts index 69bb3eaa..e9942a9c 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 "../../../lib/security.js"; +import { isPathAllowed } from "../../../lib/security.js"; 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 b984b25d..fad43175 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 "../../../lib/security.js"; +import { validatePath, PathNotAllowedError } from "../../../lib/security.js"; import { getErrorMessage, logError } from "../common.js"; import { mkdirSafe } from "../../../lib/fs-utils.js"; @@ -30,6 +30,12 @@ export function createWriteHandler() { res.json({ success: true }); } catch (error) { + // Path not allowed - return 403 Forbidden + if (error instanceof PathNotAllowedError) { + res.status(403).json({ success: false, error: getErrorMessage(error) }); + return; + } + logError(error, "Write file failed"); res.status(500).json({ success: false, error: getErrorMessage(error) }); } 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 b6168282..1bedce6e 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 3a62401e..7d6c435d 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 11e9bf45..96316629 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 "../../../lib/security.js"; +import { isPathAllowed, PathNotAllowedError } from "../../../lib/security.js"; 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 19f3c661..753addda 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 "../../../lib/security.js"; +import path from "path"; +import { + getAllowedRootDirectory, + getDataDirectory, +} from "../../../lib/security.js"; 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 6c780fb6..86405c4b 100644 --- a/apps/server/src/routes/workspace/routes/directories.ts +++ b/apps/server/src/routes/workspace/routes/directories.ts @@ -5,51 +5,47 @@ import type { Request, Response } from "express"; import fs from "fs/promises"; import path from "path"; -import { addAllowedPath } from "../../../lib/security.js"; +import { getAllowedRootDirectory } from "../../../lib/security.js"; 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 9e2e4b36..460d784b 100644 --- a/apps/server/src/services/agent-service.ts +++ b/apps/server/src/services/agent-service.ts @@ -5,7 +5,7 @@ import { AbortError } from "@anthropic-ai/claude-agent-sdk"; 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 { ProviderFactory } from "../providers/provider-factory.js"; import type { ExecuteOptions } from "../providers/types.js"; @@ -13,6 +13,7 @@ import { readImageAsBase64 } from "../lib/image-handler.js"; import { buildPromptWithImages } from "../lib/prompt-builder.js"; import { createChatOptions } from "../lib/sdk-options.js"; import { isAbortError } from "../lib/error-handler.js"; +import { isPathAllowed, PathNotAllowedError } from "../lib/security.js"; interface Message { id: string; @@ -62,7 +63,7 @@ export class AgentService { } async initialize(): Promise { - await fs.mkdir(this.stateDir, { recursive: true }); + await secureFs.mkdir(this.stateDir, { recursive: true }); } /** @@ -80,11 +81,20 @@ 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 PathNotAllowedError(effectiveWorkingDirectory); + } + this.sessions.set(sessionId, { messages, isRunning: false, abortController: null, - workingDirectory: workingDirectory || process.cwd(), + workingDirectory: resolvedWorkingDirectory, sdkSessionId: sessionMetadata?.sdkSessionId, // Load persisted SDK session ID }); } @@ -391,7 +401,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 []; @@ -402,7 +412,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" @@ -415,7 +425,7 @@ 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 {}; @@ -423,7 +433,7 @@ export class AgentService { } async saveMetadata(metadata: Record): Promise { - await fs.writeFile( + await secureFs.writeFile( this.metadataFile, JSON.stringify(metadata, null, 2), "utf-8" @@ -461,11 +471,28 @@ 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, @@ -524,7 +551,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 14fdf724..c19d1c15 100644 --- a/apps/server/src/services/auto-mode-service.ts +++ b/apps/server/src/services/auto-mode-service.ts @@ -14,7 +14,7 @@ import type { ExecuteOptions } from "../providers/types.js"; 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 { buildPromptWithImages } from "../lib/prompt-builder.js"; import { resolveModelString, DEFAULT_MODELS } from "../lib/model-resolver.js"; @@ -24,6 +24,7 @@ import { resolveDependencies, areDependenciesSatisfied } from "../lib/dependency import type { Feature } from "./feature-loader.js"; import { FeatureLoader } from "./feature-loader.js"; import { getFeatureDir, getAutomakerDir, getFeaturesDir, getContextDir } from "../lib/automaker-paths.js"; +import { isPathAllowed, PathNotAllowedError } from "../lib/security.js"; const execAsync = promisify(exec); @@ -486,6 +487,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) { @@ -549,6 +555,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; @@ -687,7 +698,7 @@ export class AutoModeService { let hasContext = false; try { - await fs.access(contextPath); + await secureFs.access(contextPath); hasContext = true; } catch { // No context @@ -695,7 +706,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, @@ -755,7 +766,7 @@ 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 } @@ -821,7 +832,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 { @@ -830,7 +841,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); @@ -872,7 +883,7 @@ 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); } @@ -938,7 +949,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 @@ -1007,7 +1018,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 { @@ -1023,7 +1034,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 { @@ -1086,7 +1097,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; @@ -1104,9 +1115,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(); @@ -1119,7 +1130,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}`); } @@ -1218,8 +1229,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, @@ -1487,7 +1498,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; @@ -1504,7 +1515,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(); @@ -1516,7 +1527,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 } @@ -1539,7 +1550,7 @@ 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 @@ -1560,7 +1571,7 @@ 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); } @@ -1571,7 +1582,7 @@ 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[] = []; @@ -1584,7 +1595,7 @@ 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); @@ -1788,7 +1799,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, @@ -1813,8 +1824,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}` @@ -1890,8 +1901,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( @@ -1934,7 +1945,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." ); } diff --git a/apps/server/src/services/feature-loader.ts b/apps/server/src/services/feature-loader.ts index 9b812642..888a81f0 100644 --- a/apps/server/src/services/feature-loader.ts +++ b/apps/server/src/services/feature-loader.ts @@ -4,7 +4,7 @@ */ import path from "path"; -import fs from "fs/promises"; +import * as secureFs from "../lib/secure-fs.js"; import { getFeaturesDir, getFeatureDir, @@ -88,7 +88,7 @@ export class FeatureLoader { if (!newPathSet.has(oldPath)) { try { // Paths are now absolute - await fs.unlink(oldPath); + await secureFs.unlink(oldPath); console.log(`[FeatureLoader] Deleted orphaned image: ${oldPath}`); } catch (error) { // Ignore errors when deleting (file may already be gone) @@ -116,7 +116,7 @@ export class FeatureLoader { } const featureImagesDir = this.getFeatureImagesDir(projectPath, featureId); - await fs.mkdir(featureImagesDir, { recursive: true }); + await secureFs.mkdir(featureImagesDir, { recursive: true }); const updatedPaths: Array = []; @@ -139,7 +139,7 @@ export class FeatureLoader { // Check if file exists try { - await fs.access(fullOriginalPath); + await secureFs.access(fullOriginalPath); } catch { console.warn( `[FeatureLoader] Image not found, skipping: ${fullOriginalPath}` @@ -152,14 +152,14 @@ export class FeatureLoader { const newPath = path.join(featureImagesDir, filename); // Copy the file - await fs.copyFile(fullOriginalPath, newPath); + 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 } @@ -217,13 +217,13 @@ 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 @@ -233,7 +233,7 @@ 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) { @@ -280,7 +280,7 @@ 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") { @@ -309,7 +309,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( @@ -328,7 +328,7 @@ export class FeatureLoader { }; // Write feature.json - await fs.writeFile( + await secureFs.writeFile( featureJsonPath, JSON.stringify(feature, null, 2), "utf-8" @@ -380,7 +380,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" @@ -396,7 +396,7 @@ 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 }); + await secureFs.rm(featureDir, { recursive: true, force: true }); console.log(`[FeatureLoader] Deleted feature ${featureId}`); return true; } catch (error) { @@ -417,7 +417,7 @@ 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") { @@ -440,10 +440,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"); } /** @@ -455,7 +455,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 d733bbd1..7c8acb6d 100644 --- a/apps/server/src/services/settings-service.ts +++ b/apps/server/src/services/settings-service.ts @@ -7,8 +7,7 @@ * - Per-project settings ({projectPath}/.automaker/settings.json) */ -import fs from "fs/promises"; -import path from "path"; +import * as secureFs from "../lib/secure-fs.js"; import { createLogger } from "../lib/logger.js"; import { getGlobalSettingsPath, @@ -47,12 +46,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 +64,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 +80,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; @@ -270,8 +269,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 +282,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), - }, }; } @@ -506,14 +495,10 @@ export class SettingsService { if (appState.apiKeys) { const apiKeys = appState.apiKeys as { anthropic?: string; - google?: string; - openai?: string; }; await this.updateCredentials({ apiKeys: { anthropic: apiKeys.anthropic || "", - google: apiKeys.google || "", - openai: apiKeys.openai || "", }, }); migratedCredentials = true; diff --git a/apps/server/src/types/settings.ts b/apps/server/src/types/settings.ts index 31034e3e..d2f2b0df 100644 --- a/apps/server/src/types/settings.ts +++ b/apps/server/src/types/settings.ts @@ -266,10 +266,6 @@ export interface Credentials { apiKeys: { /** Anthropic Claude API key */ anthropic: string; - /** Google API key (for embeddings or other services) */ - google: string; - /** OpenAI API key (for compatibility or alternative providers) */ - openai: string; }; } @@ -410,8 +406,6 @@ export const DEFAULT_CREDENTIALS: Credentials = { version: 1, apiKeys: { anthropic: "", - google: "", - openai: "", }, }; 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 c4d63add..914c7656 100644 --- a/apps/server/tests/unit/lib/security.test.ts +++ b/apps/server/tests/unit/lib/security.test.ts @@ -11,134 +11,92 @@ describe("security.ts", () => { }); describe("initAllowedPaths", () => { - it("should parse comma-separated directories from environment", async () => { - process.env.ALLOWED_PROJECT_DIRS = "/path1,/path2,/path3"; + it("should load ALLOWED_ROOT_DIRECTORY if set", async () => { + process.env.ALLOWED_ROOT_DIRECTORY = "/projects"; process.env.DATA_DIR = ""; - const { initAllowedPaths, getAllowedPaths } = await import( - "@/lib/security.js" - ); + const { initAllowedPaths, getAllowedPaths, getAllowedRootDirectory } = + await import("@/lib/security.js"); initAllowedPaths(); const allowed = getAllowedPaths(); - expect(allowed).toContain(path.resolve("/path1")); - expect(allowed).toContain(path.resolve("/path2")); - expect(allowed).toContain(path.resolve("/path3")); - }); - - it("should trim whitespace from paths", async () => { - process.env.ALLOWED_PROJECT_DIRS = " /path1 , /path2 , /path3 "; - process.env.DATA_DIR = ""; - - const { initAllowedPaths, getAllowedPaths } = await import( - "@/lib/security.js" - ); - initAllowedPaths(); - - const allowed = getAllowedPaths(); - expect(allowed).toContain(path.resolve("/path1")); - expect(allowed).toContain(path.resolve("/path2")); + expect(allowed).toContain(path.resolve("/projects")); + expect(getAllowedRootDirectory()).toBe(path.resolve("/projects")); }); it("should always include DATA_DIR if set", async () => { - process.env.ALLOWED_PROJECT_DIRS = ""; + delete process.env.ALLOWED_ROOT_DIRECTORY; process.env.DATA_DIR = "/data/dir"; - const { initAllowedPaths, getAllowedPaths } = await import( - "@/lib/security.js" - ); + const { initAllowedPaths, getAllowedPaths } = + await import("@/lib/security.js"); 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( - "@/lib/security.js" - ); - 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 handle both ALLOWED_ROOT_DIRECTORY and DATA_DIR", async () => { + process.env.ALLOWED_ROOT_DIRECTORY = "/projects"; process.env.DATA_DIR = "/data"; - delete process.env.WORKSPACE_DIR; - const { initAllowedPaths, getAllowedPaths } = await import( - "@/lib/security.js" - ); - 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( - "@/lib/security.js" - ); - 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("@/lib/security.js"); initAllowedPaths(); - addAllowedPath("/new/path"); - const allowed = getAllowedPaths(); - expect(allowed).toContain(path.resolve("/new/path")); - }); - - it("should resolve relative paths before adding", async () => { - process.env.ALLOWED_PROJECT_DIRS = ""; - process.env.DATA_DIR = ""; - - const { initAllowedPaths, addAllowedPath, getAllowedPaths } = - await import("@/lib/security.js"); - initAllowedPaths(); - - addAllowedPath("./relative/path"); - - const allowed = getAllowedPaths(); - const cwd = process.cwd(); - expect(allowed).toContain(path.resolve(cwd, "./relative/path")); + expect(allowed).toContain(path.resolve("/projects")); + expect(allowed).toContain(path.resolve("/data")); + expect(allowed).toHaveLength(2); }); }); 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( - "@/lib/security.js" - ); + const { initAllowedPaths, isPathAllowed } = + await import("@/lib/security.js"); 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("@/lib/security.js"); + 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("@/lib/security.js"); + 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 +106,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( - "@/lib/security.js" - ); + const { initAllowedPaths, validatePath } = + await import("@/lib/security.js"); 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( - "@/lib/security.js" - ); + const { initAllowedPaths, validatePath } = + await import("@/lib/security.js"); 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("@/lib/security.js"); + 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( - "@/lib/security.js" - ); + const { initAllowedPaths, validatePath } = + await import("@/lib/security.js"); initAllowedPaths(); const result = validatePath("./file.txt"); @@ -194,26 +161,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( - "@/lib/security.js" - ); + const { initAllowedPaths, getAllowedPaths } = + await import("@/lib/security.js"); 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( - "@/lib/security.js" - ); + const { initAllowedPaths, getAllowedPaths } = + await import("@/lib/security.js"); 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}