refactor: integrate secure file system operations across services

This commit replaces direct file system operations with a secure file system adapter to enhance security by enforcing path validation. The changes include:

- Replaced `fs` imports with `secureFs` in various services and utilities.
- Updated file operations in `agent-service`, `auto-mode-service`, `feature-loader`, and `settings-service` to use the secure file system methods.
- Ensured that all file I/O operations are validated against the ALLOWED_ROOT_DIRECTORY.

This refactor aims to prevent unauthorized file access and improve overall security posture.

Tests: All unit tests passing.

🤖 Generated with Claude Code
This commit is contained in:
Test User
2025-12-20 18:45:39 -05:00
parent ade80484bb
commit f3c9e828e2
45 changed files with 329 additions and 551 deletions

View File

@@ -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());

View File

@@ -6,7 +6,6 @@ import type { Request, Response } from "express";
import { exec } from "child_process";
import { promisify } from "util";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -26,20 +25,6 @@ export function createCommitHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(worktreePath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Check for uncommitted changes
const { stdout: status } = await execAsync("git status --porcelain", {
cwd: worktreePath,

View File

@@ -20,7 +20,6 @@ import {
ensureInitialCommit,
} from "../common.js";
import { trackBranch } from "./branch-tracking.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -92,20 +91,6 @@ export function createCreateHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
if (!(await isGitRepo(projectPath))) {
res.status(400).json({
success: false,

View File

@@ -6,7 +6,6 @@ import type { Request, Response } from "express";
import { exec } from "child_process";
import { promisify } from "util";
import { isGitRepo, getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -27,21 +26,6 @@ export function createDeleteHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
validatePath(worktreePath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
if (!(await isGitRepo(projectPath))) {
res.status(400).json({
success: false,

View File

@@ -7,7 +7,6 @@ import path from "path";
import fs from "fs/promises";
import { getErrorMessage, logError } from "../common.js";
import { getGitRepositoryDiffs } from "../../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
export function createDiffsHandler() {
return async (req: Request, res: Response): Promise<void> => {
@@ -27,20 +26,6 @@ export function createDiffsHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Git worktrees are stored in project directory
const worktreePath = path.join(projectPath, ".worktrees", featureId);

View File

@@ -9,7 +9,6 @@ import path from "path";
import fs from "fs/promises";
import { getErrorMessage, logError } from "../common.js";
import { generateSyntheticDiffForNewFile } from "../../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -30,21 +29,6 @@ export function createFileDiffHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
validatePath(filePath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Git worktrees are stored in project directory
const worktreePath = path.join(projectPath, ".worktrees", featureId);

View File

@@ -8,7 +8,6 @@ import { promisify } from "util";
import path from "path";
import fs from "fs/promises";
import { getErrorMessage, logError, normalizePath } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -30,20 +29,6 @@ export function createInfoHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Check if worktree exists (git worktrees are stored in project directory)
const worktreePath = path.join(projectPath, ".worktrees", featureId);
try {

View File

@@ -8,7 +8,6 @@ import { promisify } from "util";
import { existsSync } from "fs";
import { join } from "path";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -27,20 +26,6 @@ export function createInitGitHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Check if .git already exists
const gitDirPath = join(projectPath, ".git");
if (existsSync(gitDirPath)) {

View File

@@ -6,7 +6,6 @@ import type { Request, Response } from "express";
import { exec } from "child_process";
import { promisify } from "util";
import { getErrorMessage, logWorktreeError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -31,20 +30,6 @@ export function createListBranchesHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(worktreePath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Get current branch
const { stdout: currentBranchOutput } = await execAsync(
"git rev-parse --abbrev-ref HEAD",

View File

@@ -7,7 +7,6 @@ import { exec } from "child_process";
import { promisify } from "util";
import path from "path";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -30,20 +29,6 @@ export function createMergeHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
const branchName = `feature/${featureId}`;
// Git worktrees are stored in project directory
const worktreePath = path.join(projectPath, ".worktrees", featureId);

View File

@@ -7,7 +7,6 @@ import type { Request, Response } from "express";
import { exec } from "child_process";
import { promisify } from "util";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -109,20 +108,6 @@ export function createOpenInEditorHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(worktreePath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
const editor = await detectDefaultEditor();
try {

View File

@@ -6,7 +6,6 @@ import type { Request, Response } from "express";
import { exec } from "child_process";
import { promisify } from "util";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -25,20 +24,6 @@ export function createPullHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(worktreePath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Get current branch name
const { stdout: branchOutput } = await execAsync(
"git rev-parse --abbrev-ref HEAD",

View File

@@ -6,7 +6,6 @@ import type { Request, Response } from "express";
import { exec } from "child_process";
import { promisify } from "util";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -26,20 +25,6 @@ export function createPushHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(worktreePath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Get branch name
const { stdout: branchOutput } = await execAsync(
"git rev-parse --abbrev-ref HEAD",

View File

@@ -9,7 +9,6 @@
import type { Request, Response } from "express";
import { getDevServerService } from "../../../services/dev-server-service.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
export function createStartDevHandler() {
return async (req: Request, res: Response): Promise<void> => {
@@ -35,21 +34,6 @@ export function createStartDevHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
validatePath(worktreePath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
const devServerService = getDevServerService();
const result = await devServerService.startDevServer(projectPath, worktreePath);

View File

@@ -8,7 +8,6 @@ import { promisify } from "util";
import path from "path";
import fs from "fs/promises";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -30,20 +29,6 @@ export function createStatusHandler() {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Git worktrees are stored in project directory
const worktreePath = path.join(projectPath, ".worktrees", featureId);