refactor: replace fs with secureFs for improved file handling

This commit updates various modules to utilize the secure file system operations from the secureFs module instead of the native fs module. Key changes include:

- Replaced fs imports with secureFs in multiple route handlers and services to enhance security and consistency in file operations.
- Added centralized validation for working directories in the sdk-options module to ensure all AI model invocations are secure.

These changes aim to improve the security and maintainability of file handling across the application.
This commit is contained in:
Test User
2025-12-21 01:32:26 -05:00
parent 2b5479ae0d
commit 077a63b03b
62 changed files with 4866 additions and 3350 deletions

View File

@@ -9,45 +9,59 @@
* - Chat: Full tool access for interactive coding
*
* Uses model-resolver for consistent model handling across the application.
*
* SECURITY: All factory functions validate the working directory (cwd) against
* ALLOWED_ROOT_DIRECTORY before returning options. This provides a centralized
* security check that applies to ALL AI model invocations, regardless of provider.
*/
import type { Options } from "@anthropic-ai/claude-agent-sdk";
import { resolveModelString } from "@automaker/model-resolver";
import { DEFAULT_MODELS, CLAUDE_MODEL_MAP } from "@automaker/types";
import type { Options } from '@anthropic-ai/claude-agent-sdk';
import path from 'path';
import { resolveModelString } from '@automaker/model-resolver';
import { DEFAULT_MODELS, CLAUDE_MODEL_MAP } from '@automaker/types';
import { isPathAllowed, PathNotAllowedError, getAllowedRootDirectory } from '@automaker/platform';
/**
* Validate that a working directory is allowed by ALLOWED_ROOT_DIRECTORY.
* This is the centralized security check for ALL AI model invocations.
*
* @param cwd - The working directory to validate
* @throws PathNotAllowedError if the directory is not within ALLOWED_ROOT_DIRECTORY
*
* This function is called by all create*Options() factory functions to ensure
* that AI models can only operate within allowed directories. This applies to:
* - All current models (Claude, future models)
* - All invocation types (chat, auto-mode, spec generation, etc.)
*/
export function validateWorkingDirectory(cwd: string): void {
const resolvedCwd = path.resolve(cwd);
if (!isPathAllowed(resolvedCwd)) {
const allowedRoot = getAllowedRootDirectory();
throw new PathNotAllowedError(
`Working directory "${cwd}" (resolved: ${resolvedCwd}) is not allowed. ` +
(allowedRoot
? `Must be within ALLOWED_ROOT_DIRECTORY: ${allowedRoot}`
: 'ALLOWED_ROOT_DIRECTORY is configured but path is not within allowed directories.')
);
}
}
/**
* Tool presets for different use cases
*/
export const TOOL_PRESETS = {
/** Read-only tools for analysis */
readOnly: ["Read", "Glob", "Grep"] as const,
readOnly: ['Read', 'Glob', 'Grep'] as const,
/** Tools for spec generation that needs to read the codebase */
specGeneration: ["Read", "Glob", "Grep"] as const,
specGeneration: ['Read', 'Glob', 'Grep'] as const,
/** Full tool access for feature implementation */
fullAccess: [
"Read",
"Write",
"Edit",
"Glob",
"Grep",
"Bash",
"WebSearch",
"WebFetch",
] as const,
fullAccess: ['Read', 'Write', 'Edit', 'Glob', 'Grep', 'Bash', 'WebSearch', 'WebFetch'] as const,
/** Tools for chat/interactive mode */
chat: [
"Read",
"Write",
"Edit",
"Glob",
"Grep",
"Bash",
"WebSearch",
"WebFetch",
] as const,
chat: ['Read', 'Write', 'Edit', 'Glob', 'Grep', 'Bash', 'WebSearch', 'WebFetch'] as const,
} as const;
/**
@@ -78,7 +92,7 @@ export const MAX_TURNS = {
* - AUTOMAKER_MODEL_DEFAULT: Fallback model for all operations
*/
export function getModelForUseCase(
useCase: "spec" | "features" | "suggestions" | "chat" | "auto" | "default",
useCase: 'spec' | 'features' | 'suggestions' | 'chat' | 'auto' | 'default',
explicitModel?: string
): string {
// Explicit model takes precedence
@@ -102,12 +116,12 @@ export function getModelForUseCase(
}
const defaultModels: Record<string, string> = {
spec: CLAUDE_MODEL_MAP["haiku"], // used to generate app specs
features: CLAUDE_MODEL_MAP["haiku"], // used to generate features from app specs
suggestions: CLAUDE_MODEL_MAP["haiku"], // used for suggestions
chat: CLAUDE_MODEL_MAP["haiku"], // used for chat
auto: CLAUDE_MODEL_MAP["opus"], // used to implement kanban cards
default: CLAUDE_MODEL_MAP["opus"],
spec: CLAUDE_MODEL_MAP['haiku'], // used to generate app specs
features: CLAUDE_MODEL_MAP['haiku'], // used to generate features from app specs
suggestions: CLAUDE_MODEL_MAP['haiku'], // used for suggestions
chat: CLAUDE_MODEL_MAP['haiku'], // used for chat
auto: CLAUDE_MODEL_MAP['opus'], // used to implement kanban cards
default: CLAUDE_MODEL_MAP['opus'],
};
return resolveModelString(defaultModels[useCase] || DEFAULT_MODELS.claude);
@@ -118,7 +132,7 @@ export function getModelForUseCase(
*/
function getBaseOptions(): Partial<Options> {
return {
permissionMode: "acceptEdits",
permissionMode: 'acceptEdits',
};
}
@@ -143,7 +157,7 @@ export interface CreateSdkOptionsConfig {
/** Optional output format for structured outputs */
outputFormat?: {
type: "json_schema";
type: 'json_schema';
schema: Record<string, unknown>;
};
}
@@ -156,16 +170,17 @@ export interface CreateSdkOptionsConfig {
* - Extended turns for thorough exploration
* - Opus model by default (can be overridden)
*/
export function createSpecGenerationOptions(
config: CreateSdkOptionsConfig
): Options {
export function createSpecGenerationOptions(config: CreateSdkOptionsConfig): Options {
// Validate working directory before creating options
validateWorkingDirectory(config.cwd);
return {
...getBaseOptions(),
// Override permissionMode - spec generation only needs read-only tools
// Using "acceptEdits" can cause Claude to write files to unexpected locations
// See: https://github.com/AutoMaker-Org/automaker/issues/149
permissionMode: "default",
model: getModelForUseCase("spec", config.model),
permissionMode: 'default',
model: getModelForUseCase('spec', config.model),
maxTurns: MAX_TURNS.maximum,
cwd: config.cwd,
allowedTools: [...TOOL_PRESETS.specGeneration],
@@ -183,14 +198,15 @@ export function createSpecGenerationOptions(
* - Quick turns since it's mostly JSON generation
* - Sonnet model by default for speed
*/
export function createFeatureGenerationOptions(
config: CreateSdkOptionsConfig
): Options {
export function createFeatureGenerationOptions(config: CreateSdkOptionsConfig): Options {
// Validate working directory before creating options
validateWorkingDirectory(config.cwd);
return {
...getBaseOptions(),
// Override permissionMode - feature generation only needs read-only tools
permissionMode: "default",
model: getModelForUseCase("features", config.model),
permissionMode: 'default',
model: getModelForUseCase('features', config.model),
maxTurns: MAX_TURNS.quick,
cwd: config.cwd,
allowedTools: [...TOOL_PRESETS.readOnly],
@@ -207,12 +223,13 @@ export function createFeatureGenerationOptions(
* - Standard turns to allow thorough codebase exploration and structured output generation
* - Opus model by default for thorough analysis
*/
export function createSuggestionsOptions(
config: CreateSdkOptionsConfig
): Options {
export function createSuggestionsOptions(config: CreateSdkOptionsConfig): Options {
// Validate working directory before creating options
validateWorkingDirectory(config.cwd);
return {
...getBaseOptions(),
model: getModelForUseCase("suggestions", config.model),
model: getModelForUseCase('suggestions', config.model),
maxTurns: MAX_TURNS.extended,
cwd: config.cwd,
allowedTools: [...TOOL_PRESETS.readOnly],
@@ -232,12 +249,15 @@ export function createSuggestionsOptions(
* - Sandbox enabled for bash safety
*/
export function createChatOptions(config: CreateSdkOptionsConfig): Options {
// Validate working directory before creating options
validateWorkingDirectory(config.cwd);
// Model priority: explicit model > session model > chat default
const effectiveModel = config.model || config.sessionModel;
return {
...getBaseOptions(),
model: getModelForUseCase("chat", effectiveModel),
model: getModelForUseCase('chat', effectiveModel),
maxTurns: MAX_TURNS.standard,
cwd: config.cwd,
allowedTools: [...TOOL_PRESETS.chat],
@@ -260,9 +280,12 @@ export function createChatOptions(config: CreateSdkOptionsConfig): Options {
* - Sandbox enabled for bash safety
*/
export function createAutoModeOptions(config: CreateSdkOptionsConfig): Options {
// Validate working directory before creating options
validateWorkingDirectory(config.cwd);
return {
...getBaseOptions(),
model: getModelForUseCase("auto", config.model),
model: getModelForUseCase('auto', config.model),
maxTurns: MAX_TURNS.maximum,
cwd: config.cwd,
allowedTools: [...TOOL_PRESETS.fullAccess],
@@ -287,14 +310,15 @@ export function createCustomOptions(
sandbox?: { enabled: boolean; autoAllowBashIfSandboxed?: boolean };
}
): Options {
// Validate working directory before creating options
validateWorkingDirectory(config.cwd);
return {
...getBaseOptions(),
model: getModelForUseCase("default", config.model),
model: getModelForUseCase('default', config.model),
maxTurns: config.maxTurns ?? MAX_TURNS.maximum,
cwd: config.cwd,
allowedTools: config.allowedTools
? [...config.allowedTools]
: [...TOOL_PRESETS.readOnly],
allowedTools: config.allowedTools ? [...config.allowedTools] : [...TOOL_PRESETS.readOnly],
...(config.sandbox && { sandbox: config.sandbox }),
...(config.systemPrompt && { systemPrompt: config.systemPrompt }),
...(config.abortController && { abortController: config.abortController }),

View File

@@ -3,8 +3,8 @@
* Stores worktree-specific data in .automaker/worktrees/:branch/worktree.json
*/
import * as fs from "fs/promises";
import * as path from "path";
import * as secureFs from './secure-fs.js';
import * as path from 'path';
/** Maximum length for sanitized branch names in filesystem paths */
const MAX_SANITIZED_BRANCH_PATH_LENGTH = 200;
@@ -32,11 +32,11 @@ function sanitizeBranchName(branch: string): string {
// - Windows invalid chars: : * ? " < > |
// - Other potentially problematic chars
let safeBranch = branch
.replace(/[/\\:*?"<>|]/g, "-") // Replace invalid chars with dash
.replace(/\s+/g, "_") // Replace spaces with underscores
.replace(/\.+$/g, "") // Remove trailing dots (Windows issue)
.replace(/-+/g, "-") // Collapse multiple dashes
.replace(/^-|-$/g, ""); // Remove leading/trailing dashes
.replace(/[/\\:*?"<>|]/g, '-') // Replace invalid chars with dash
.replace(/\s+/g, '_') // Replace spaces with underscores
.replace(/\.+$/g, '') // Remove trailing dots (Windows issue)
.replace(/-+/g, '-') // Collapse multiple dashes
.replace(/^-|-$/g, ''); // Remove leading/trailing dashes
// Truncate to safe length (leave room for path components)
safeBranch = safeBranch.substring(0, MAX_SANITIZED_BRANCH_PATH_LENGTH);
@@ -44,7 +44,7 @@ function sanitizeBranchName(branch: string): string {
// Handle Windows reserved names (CON, PRN, AUX, NUL, COM1-9, LPT1-9)
const windowsReserved = /^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])$/i;
if (windowsReserved.test(safeBranch) || safeBranch.length === 0) {
safeBranch = `_${safeBranch || "branch"}`;
safeBranch = `_${safeBranch || 'branch'}`;
}
return safeBranch;
@@ -55,14 +55,14 @@ function sanitizeBranchName(branch: string): string {
*/
function getWorktreeMetadataDir(projectPath: string, branch: string): string {
const safeBranch = sanitizeBranchName(branch);
return path.join(projectPath, ".automaker", "worktrees", safeBranch);
return path.join(projectPath, '.automaker', 'worktrees', safeBranch);
}
/**
* Get the path to the worktree metadata file
*/
function getWorktreeMetadataPath(projectPath: string, branch: string): string {
return path.join(getWorktreeMetadataDir(projectPath, branch), "worktree.json");
return path.join(getWorktreeMetadataDir(projectPath, branch), 'worktree.json');
}
/**
@@ -74,7 +74,7 @@ export async function readWorktreeMetadata(
): Promise<WorktreeMetadata | null> {
try {
const metadataPath = getWorktreeMetadataPath(projectPath, branch);
const content = await fs.readFile(metadataPath, "utf-8");
const content = (await secureFs.readFile(metadataPath, 'utf-8')) as string;
return JSON.parse(content) as WorktreeMetadata;
} catch (error) {
// File doesn't exist or can't be read
@@ -94,10 +94,10 @@ export async function writeWorktreeMetadata(
const metadataPath = getWorktreeMetadataPath(projectPath, branch);
// Ensure directory exists
await fs.mkdir(metadataDir, { recursive: true });
await secureFs.mkdir(metadataDir, { recursive: true });
// Write metadata
await fs.writeFile(metadataPath, JSON.stringify(metadata, null, 2), "utf-8");
await secureFs.writeFile(metadataPath, JSON.stringify(metadata, null, 2), 'utf-8');
}
/**
@@ -143,16 +143,16 @@ export async function readAllWorktreeMetadata(
projectPath: string
): Promise<Map<string, WorktreeMetadata>> {
const result = new Map<string, WorktreeMetadata>();
const worktreesDir = path.join(projectPath, ".automaker", "worktrees");
const worktreesDir = path.join(projectPath, '.automaker', 'worktrees');
try {
const dirs = await fs.readdir(worktreesDir, { withFileTypes: true });
const dirs = await secureFs.readdir(worktreesDir, { withFileTypes: true });
for (const dir of dirs) {
if (dir.isDirectory()) {
const metadataPath = path.join(worktreesDir, dir.name, "worktree.json");
const metadataPath = path.join(worktreesDir, dir.name, 'worktree.json');
try {
const content = await fs.readFile(metadataPath, "utf-8");
const content = (await secureFs.readFile(metadataPath, 'utf-8')) as string;
const metadata = JSON.parse(content) as WorktreeMetadata;
result.set(metadata.branch, metadata);
} catch {
@@ -170,13 +170,10 @@ export async function readAllWorktreeMetadata(
/**
* Delete worktree metadata for a branch
*/
export async function deleteWorktreeMetadata(
projectPath: string,
branch: string
): Promise<void> {
export async function deleteWorktreeMetadata(projectPath: string, branch: string): Promise<void> {
const metadataDir = getWorktreeMetadataDir(projectPath, branch);
try {
await fs.rm(metadataDir, { recursive: true, force: true });
await secureFs.rm(metadataDir, { recursive: true, force: true });
} catch {
// Ignore errors if directory doesn't exist
}