refactor: enhance security and streamline file handling

This commit introduces several improvements to the security and file handling mechanisms across the application. Key changes include:

- Updated the Dockerfile to pin the GitHub CLI version for reproducible builds.
- Refactored the secure file system operations to ensure consistent path validation and type handling.
- Removed legacy path management functions and streamlined the allowed paths logic in the security module.
- Enhanced route handlers to validate path parameters against the ALLOWED_ROOT_DIRECTORY, improving security against unauthorized access.
- Updated the settings service to focus solely on the Anthropic API key, removing references to Google and OpenAI keys.

These changes aim to enhance security, maintainability, and clarity in the codebase.

Tests: All unit tests passing.

apps/server/Dockerfile

@@ -26,13 +26,13 @@ RUN npm run build --workspace=apps/server
 # Production stage
 FROM node:20-alpine
 
-# Install git, curl, and GitHub CLI
+# Install git, curl, and GitHub CLI (pinned version for reproducible builds)
 RUN apk add --no-cache git curl && \
-    GH_VERSION=$(curl -s https://api.github.com/repos/cli/cli/releases/latest | grep '"tag_name"' | cut -d '"' -f 4 | sed 's/v//') && \
+    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_*_linux_amd64/bin/gh /usr/local/bin/gh && \
-    rm -rf gh.tar.gz gh_*_linux_amd64
+    mv "gh_${GH_VERSION}_linux_amd64/bin/gh" /usr/local/bin/gh && \
+    rm -rf gh.tar.gz "gh_${GH_VERSION}_linux_amd64"
 
 WORKDIR /app
 

apps/server/src/lib/secure-fs.ts

@@ -7,6 +7,7 @@
  */
 
 import fs from "fs/promises";
+import type { Dirent } from "fs";
 import path from "path";
 import { validatePath } from "./security.js";
 
@@ -41,7 +42,7 @@ export async function writeFile(
   encoding?: BufferEncoding
 ): Promise<void> {
   const validatedPath = validatePath(filePath);
-  return fs.writeFile(validatedPath, data, encoding as any);
+  return fs.writeFile(validatedPath, data, encoding);
 }
 
 /**
@@ -58,12 +59,23 @@ export async function mkdir(
 /**
  * Wrapper around fs.readdir that validates path first
  */
+export async function readdir(
+  dirPath: string,
+  options?: { withFileTypes?: false; encoding?: BufferEncoding }
+): Promise<string[]>;
+export async function readdir(
+  dirPath: string,
+  options: { withFileTypes: true; encoding?: BufferEncoding }
+): Promise<Dirent[]>;
 export async function readdir(
   dirPath: string,
   options?: { withFileTypes?: boolean; encoding?: BufferEncoding }
-): Promise<string[] | any[]> {
+): Promise<string[] | Dirent[]> {
   const validatedPath = validatePath(dirPath);
-  return fs.readdir(validatedPath, options as any);
+  if (options?.withFileTypes === true) {
+    return fs.readdir(validatedPath, { withFileTypes: true });
+  }
+  return fs.readdir(validatedPath);
 }
 
 /**
@@ -115,7 +127,7 @@ export async function appendFile(
   encoding?: BufferEncoding
 ): Promise<void> {
   const validatedPath = validatePath(filePath);
-  return fs.appendFile(validatedPath, data, encoding as any);
+  return fs.appendFile(validatedPath, data, encoding);
 }
 
 /**

apps/server/src/lib/security.ts

@@ -23,9 +23,6 @@ let allowedRootDirectory: string | null = null;
 // Data directory - always allowed for settings/credentials
 let dataDirectory: string | null = null;
 
-// Allowed paths set - stores ALLOWED_ROOT_DIRECTORY and DATA_DIR
-const allowedPaths = new Set<string>();
-
 /**
  * Initialize security settings from environment variables
  * - ALLOWED_ROOT_DIRECTORY: main security boundary
@@ -36,7 +33,6 @@ export function initAllowedPaths(): void {
   const rootDir = process.env.ALLOWED_ROOT_DIRECTORY;
   if (rootDir) {
     allowedRootDirectory = path.resolve(rootDir);
-    allowedPaths.add(allowedRootDirectory);
     console.log(
       `[Security] ✓ ALLOWED_ROOT_DIRECTORY configured: ${allowedRootDirectory}`
     );
@@ -50,19 +46,10 @@ export function initAllowedPaths(): void {
   const dataDir = process.env.DATA_DIR;
   if (dataDir) {
     dataDirectory = path.resolve(dataDir);
-    allowedPaths.add(dataDirectory);
     console.log(`[Security] ✓ DATA_DIR configured: ${dataDirectory}`);
   }
 }
 
-/**
- * Add a path to the allowed list
- * Used when dynamically creating new directories within the allowed root
- */
-export function addAllowedPath(filePath: string): void {
-  allowedPaths.add(path.resolve(filePath));
-}
-
 /**
  * Check if a path is allowed based on ALLOWED_ROOT_DIRECTORY
  * Returns true if:
@@ -145,5 +132,12 @@ export function getDataDirectory(): string | null {
  * 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;
 }

apps/server/src/routes/auto-mode/index.ts

@@ -22,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("/status", validatePathParams("projectPath?"), createStatusHandler(autoModeService));
   router.post("/run-feature", validatePathParams("projectPath"), createRunFeatureHandler(autoModeService));
-  router.post("/verify-feature", createVerifyFeatureHandler(autoModeService));
-  router.post("/resume-feature", createResumeFeatureHandler(autoModeService));
-  router.post("/context-exists", createContextExistsHandler(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", validatePathParams("projectPath", "worktreePath?"), createCommitFeatureHandler(autoModeService));
-  router.post("/approve-plan", createApprovePlanHandler(autoModeService));
+  router.post("/approve-plan", validatePathParams("projectPath"), createApprovePlanHandler(autoModeService));
 
   return router;
 }

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

@@ -6,7 +6,11 @@ import type { Request, Response } from "express";
 import fs from "fs/promises";
 import os from "os";
 import path from "path";
-import { isPathAllowed, PathNotAllowedError } from "../../../lib/security.js";
+import {
+  getAllowedRootDirectory,
+  isPathAllowed,
+  PathNotAllowedError,
+} from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createBrowseHandler() {
@@ -14,8 +18,9 @@ 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)) {

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, isPathAllowed, PathNotAllowedError } from "../../../lib/security.js";
+import { isPathAllowed, PathNotAllowedError } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createMkdirHandler() {
@@ -31,7 +31,6 @@ export function createMkdirHandler() {
         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;
         }
@@ -52,9 +51,6 @@ 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

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,

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) {

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) {

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,

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";

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<Credentials>` (usually just apiKeys)
- * Response: `{ "success": true, "credentials": { anthropic, google, openai } }`
+ * Response: `{ "success": true, "credentials": { anthropic } }`
  */
 
 import type { Request, Response } from "express";

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, isPathAllowed, PathNotAllowedError } from "../../../lib/security.js";
+import { isPathAllowed, PathNotAllowedError } from "../../../lib/security.js";
 import { logger, getErrorMessage, logError } from "../common.js";
 
 export function createCloneHandler() {
@@ -204,9 +204,6 @@ export function createCloneHandler() {
         });
       });
 
-      // Add to allowed paths
-      addAllowedPath(projectPath);
-
       logger.info(`[Templates] Successfully cloned template to ${projectPath}`);
 
       res.json({

apps/server/src/routes/workspace/routes/config.ts

@@ -6,7 +6,6 @@ import type { Request, Response } from "express";
 import fs from "fs/promises";
 import path from "path";
 import {
-  addAllowedPath,
   getAllowedRootDirectory,
   getDataDirectory,
 } from "../../../lib/security.js";
@@ -41,9 +40,6 @@ export function createConfigHandler() {
           return;
         }
 
-        // Add workspace dir to allowed paths
-        addAllowedPath(resolvedWorkspaceDir);
-
         res.json({
           success: true,
           configured: true,

apps/server/src/routes/workspace/routes/directories.ts

@@ -5,7 +5,7 @@
 import type { Request, Response } from "express";
 import fs from "fs/promises";
 import path from "path";
-import { addAllowedPath, getAllowedRootDirectory } from "../../../lib/security.js";
+import { getAllowedRootDirectory } from "../../../lib/security.js";
 import { getErrorMessage, logError } from "../common.js";
 
 export function createDirectoriesHandler() {
@@ -34,9 +34,6 @@ export function createDirectoriesHandler() {
         return;
       }
 
-      // Add workspace dir to allowed paths
-      addAllowedPath(resolvedWorkspaceDir);
-
       // Read directory contents
       const entries = await fs.readdir(resolvedWorkspaceDir, { withFileTypes: true });
 
@@ -49,9 +46,6 @@ export function createDirectoriesHandler() {
         }))
         .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,

apps/server/src/services/auto-mode-service.ts

@@ -1117,7 +1117,7 @@ Address the follow-up instructions above. Review the previous work and make the
       // Check if directory exists first
       await secureFs.access(contextDir);
 
-      const files = await secureFs.readdir(contextDir) as string[];
+      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();
@@ -1582,7 +1582,7 @@ Format your response as a structured markdown document.`;
     const featuresDir = getFeaturesDir(projectPath);
 
     try {
-      const entries = await secureFs.readdir(featuresDir, { withFileTypes: true }) as any[];
+      const entries = await secureFs.readdir(featuresDir, { withFileTypes: true });
       const allFeatures: Feature[] = [];
       const pendingFeatures: Feature[] = [];
 

apps/server/src/services/settings-service.ts

@@ -269,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();
 
@@ -284,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),
-      },
     };
   }
 
@@ -505,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;

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: "",
   },
 };
 

apps/server/tests/unit/lib/security.test.ts

@@ -51,37 +51,6 @@ describe("security.ts", () => {
     });
   });
 
-  describe("addAllowedPath", () => {
-    it("should add path to allowed list", async () => {
-      delete process.env.ALLOWED_ROOT_DIRECTORY;
-      process.env.DATA_DIR = "";
-
-      const { initAllowedPaths, addAllowedPath, 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 () => {
-      delete process.env.ALLOWED_ROOT_DIRECTORY;
-      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"));
-    });
-  });
-
   describe("isPathAllowed", () => {
     it("should allow paths within ALLOWED_ROOT_DIRECTORY", async () => {
       process.env.ALLOWED_ROOT_DIRECTORY = "/allowed/project";

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<Credentials> = {
         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<Credentials> = {
         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 () => {