fix: enforce ALLOWED_ROOT_DIRECTORY path validation across all routes

This fixes a critical security issue where path parameters from client requests
were not validated against ALLOWED_ROOT_DIRECTORY, allowing attackers to access
files and directories outside the configured root directory.

Changes:
- Add validatePath() checks to 29 route handlers that accept path parameters
- Validate paths in agent routes (workingDirectory, imagePaths)
- Validate paths in feature routes (projectPath)
- Validate paths in worktree routes (projectPath, worktreePath)
- Validate paths in git routes (projectPath, filePath)
- Validate paths in auto-mode routes (projectPath, worktreePath)
- Validate paths in settings/suggestions routes (projectPath)
- Return 403 Forbidden for paths outside ALLOWED_ROOT_DIRECTORY
- Maintain backward compatibility (unrestricted when env var not set)

Security Impact:
- Prevents directory traversal attacks
- Prevents unauthorized file access
- Prevents arbitrary code execution via unvalidated paths

All validation follows the existing pattern in fs routes and session creation,
using the validatePath() function from lib/security.ts which checks against
both ALLOWED_ROOT_DIRECTORY and DATA_DIR (appData).

Tests: All 653 unit tests passing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
Test User
2025-12-20 18:13:34 -05:00
parent 873429db19
commit ade80484bb
30 changed files with 449 additions and 10 deletions

View File

@@ -6,6 +6,7 @@ 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";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const logger = createLogger("Agent");
@@ -29,6 +30,27 @@ export function createSendHandler(agentService: AgentService) {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
if (workingDirectory) {
validatePath(workingDirectory);
}
if (imagePaths && imagePaths.length > 0) {
for (const imagePath of imagePaths) {
validatePath(imagePath);
}
}
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
// Start the message processing (don't await - it streams via WebSocket)
agentService
.sendMessage({

View File

@@ -6,6 +6,7 @@ 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";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const logger = createLogger("Agent");
@@ -24,6 +25,22 @@ export function createStartHandler(agentService: AgentService) {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
if (workingDirectory) {
try {
validatePath(workingDirectory);
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
}
const result = await agentService.startConversation({
sessionId,
workingDirectory,

View File

@@ -6,6 +6,7 @@ import type { Request, Response } from "express";
import type { AutoModeService } from "../../../services/auto-mode-service.js";
import { createLogger } from "../../../lib/logger.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const logger = createLogger("AutoMode");
@@ -21,6 +22,20 @@ export function createAnalyzeProjectHandler(autoModeService: AutoModeService) {
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;
}
// Start analysis in background
autoModeService.analyzeProject(projectPath).catch((error) => {
logger.error(`[AutoMode] Project analysis error:`, error);

View File

@@ -5,6 +5,7 @@
import type { Request, Response } from "express";
import type { AutoModeService } from "../../../services/auto-mode-service.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
export function createCommitFeatureHandler(autoModeService: AutoModeService) {
return async (req: Request, res: Response): Promise<void> => {
@@ -25,6 +26,23 @@ export function createCommitFeatureHandler(autoModeService: AutoModeService) {
return;
}
// Validate paths are within ALLOWED_ROOT_DIRECTORY
try {
validatePath(projectPath);
if (worktreePath) {
validatePath(worktreePath);
}
} catch (error) {
if (error instanceof PathNotAllowedError) {
res.status(403).json({
success: false,
error: error.message,
});
return;
}
throw error;
}
const commitHash = await autoModeService.commitFeature(
projectPath,
featureId,

View File

@@ -6,6 +6,7 @@ import type { Request, Response } from "express";
import type { AutoModeService } from "../../../services/auto-mode-service.js";
import { createLogger } from "../../../lib/logger.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const logger = createLogger("AutoMode");
@@ -26,6 +27,20 @@ export function createRunFeatureHandler(autoModeService: AutoModeService) {
return;
}
// Validate path is 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;
}
// Start execution in background
// executeFeature derives workDir from feature.branchName
autoModeService

View File

@@ -7,7 +7,7 @@ import {
FeatureLoader,
type Feature,
} from "../../../services/feature-loader.js";
import { addAllowedPath } from "../../../lib/security.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
import { getErrorMessage, logError } from "../common.js";
export function createCreateHandler(featureLoader: FeatureLoader) {
@@ -28,8 +28,19 @@ export function createCreateHandler(featureLoader: FeatureLoader) {
return;
}
// Add project path to allowed paths
addAllowedPath(projectPath);
// Validate path is 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 created = await featureLoader.create(projectPath, feature);
res.json({ success: true, feature: created });

View File

@@ -5,6 +5,7 @@
import type { Request, Response } from "express";
import { FeatureLoader } from "../../../services/feature-loader.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
export function createDeleteHandler(featureLoader: FeatureLoader) {
return async (req: Request, res: Response): Promise<void> => {
@@ -24,6 +25,20 @@ export function createDeleteHandler(featureLoader: FeatureLoader) {
return;
}
// Validate path is 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 success = await featureLoader.delete(projectPath, featureId);
res.json({ success });
} catch (error) {

View File

@@ -5,6 +5,7 @@
import type { Request, Response } from "express";
import { FeatureLoader } from "../../../services/feature-loader.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
export function createGetHandler(featureLoader: FeatureLoader) {
return async (req: Request, res: Response): Promise<void> => {
@@ -24,6 +25,20 @@ export function createGetHandler(featureLoader: FeatureLoader) {
return;
}
// Validate path is 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 feature = await featureLoader.get(projectPath, featureId);
if (!feature) {
res.status(404).json({ success: false, error: "Feature not found" });

View File

@@ -4,7 +4,7 @@
import type { Request, Response } from "express";
import { FeatureLoader } from "../../../services/feature-loader.js";
import { addAllowedPath } from "../../../lib/security.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
import { getErrorMessage, logError } from "../common.js";
export function createListHandler(featureLoader: FeatureLoader) {
@@ -19,8 +19,19 @@ export function createListHandler(featureLoader: FeatureLoader) {
return;
}
// Add project path to allowed paths
addAllowedPath(projectPath);
// Validate path is 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 features = await featureLoader.getAll(projectPath);
res.json({ success: true, features });

View File

@@ -8,6 +8,7 @@ import {
type Feature,
} from "../../../services/feature-loader.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
export function createUpdateHandler(featureLoader: FeatureLoader) {
return async (req: Request, res: Response): Promise<void> => {
@@ -26,6 +27,20 @@ export function createUpdateHandler(featureLoader: FeatureLoader) {
return;
}
// Validate path is 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 updated = await featureLoader.update(
projectPath,
featureId,

View File

@@ -5,6 +5,7 @@
import type { Request, Response } from "express";
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> => {
@@ -16,6 +17,20 @@ 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;
}
try {
const result = await getGitRepositoryDiffs(projectPath);
res.json({

View File

@@ -7,6 +7,7 @@ import { exec } from "child_process";
import { promisify } from "util";
import { getErrorMessage, logError } from "../common.js";
import { generateSyntheticDiffForNewFile } from "../../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -25,6 +26,21 @@ 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;
}
try {
// First check if the file is untracked
const { stdout: status } = await execAsync(

View File

@@ -11,6 +11,7 @@
import type { Request, Response } from "express";
import type { SettingsService } from "../../../services/settings-service.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
/**
* Create handler factory for POST /api/settings/project
@@ -31,6 +32,20 @@ export function createGetProjectHandler(settingsService: SettingsService) {
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 settings = await settingsService.getProjectSettings(projectPath);
res.json({

View File

@@ -12,6 +12,7 @@ import type { Request, Response } from "express";
import type { SettingsService } from "../../../services/settings-service.js";
import type { ProjectSettings } from "../../../types/settings.js";
import { getErrorMessage, logError } from "../common.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
/**
* Create handler factory for PUT /api/settings/project
@@ -43,6 +44,20 @@ export function createUpdateProjectHandler(settingsService: SettingsService) {
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 settings = await settingsService.updateProjectSettings(
projectPath,
updates

View File

@@ -12,6 +12,7 @@ import {
logError,
} from "../common.js";
import { generateSuggestions } from "../generate-suggestions.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const logger = createLogger("Suggestions");
@@ -28,6 +29,20 @@ export function createGenerateHandler(events: EventEmitter) {
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 { isRunning } = getSuggestionsStatus();
if (isRunning) {
res.json({

View File

@@ -6,6 +6,7 @@ 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,6 +26,20 @@ 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,6 +20,7 @@ import {
ensureInitialCommit,
} from "../common.js";
import { trackBranch } from "./branch-tracking.js";
import { validatePath, PathNotAllowedError } from "../../../lib/security.js";
const execAsync = promisify(exec);
@@ -91,6 +92,20 @@ 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,6 +6,7 @@ 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);
@@ -26,6 +27,21 @@ 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,6 +7,7 @@ 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> => {
@@ -26,6 +27,20 @@ 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,6 +9,7 @@ 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);
@@ -29,6 +30,21 @@ 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,6 +8,7 @@ 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);
@@ -29,6 +30,20 @@ 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,6 +8,7 @@ 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);
@@ -26,6 +27,20 @@ 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,6 +6,7 @@ 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);
@@ -30,6 +31,20 @@ 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,6 +7,7 @@ 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);
@@ -29,6 +30,20 @@ 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,6 +7,7 @@ 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);
@@ -108,6 +109,20 @@ 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,6 +6,7 @@ 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);
@@ -24,6 +25,20 @@ 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,6 +6,7 @@ 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,6 +26,20 @@ 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,6 +9,7 @@
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> => {
@@ -34,6 +35,21 @@ 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,6 +8,7 @@ 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);
@@ -29,6 +30,20 @@ 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);

View File

@@ -8,7 +8,3 @@ services:
# Set root directory for all projects and file operations
# Users can only create/open projects within this directory
- ALLOWED_ROOT_DIRECTORY=/projects
# Optional: Set workspace directory for UI project discovery
# Falls back to ALLOWED_ROOT_DIRECTORY if not set
# - WORKSPACE_DIR=/projects