From 73f9085cada7e03c2928b31c3e806e2b665cbdd1 Mon Sep 17 00:00:00 2001 From: galz10 Date: Fri, 3 Apr 2026 12:53:21 -0700 Subject: [PATCH] fix(sandbox): proactively register plans directory for write validation - Enhance WorkspaceContext with addWritablePath and isPathWritable to support proactive registration of paths that do not yet exist. - Update Config and enter_plan_mode to proactively register the plans directory in WorkspaceContext. - Refactor SandboxedFileSystemService to use isPathWritable for granular write validation. - Add comprehensive unit tests for the new WorkspaceContext methods. --- packages/core/src/config/config.ts | 7 ++-- .../services/sandboxedFileSystemService.ts | 2 +- packages/core/src/tools/enter-plan-mode.ts | 1 + .../core/src/utils/workspaceContext.test.ts | 41 +++++++++++++++++++ packages/core/src/utils/workspaceContext.ts | 41 +++++++++++++++++++ 5 files changed, 87 insertions(+), 5 deletions(-) diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 7b0c0b6159..ed201054bb 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1398,14 +1398,13 @@ export class Config implements McpContext, AgentLoopContext { // Add plans directory to workspace context for plan file storage if (this.planEnabled) { const plansDir = this.storage.getPlansDir(); + this.workspaceContext.addWritablePath(plansDir); try { await fs.promises.access(plansDir); this.workspaceContext.addDirectory(plansDir); } catch { - // Directory does not exist yet, so we don't add it to the workspace context. - // It will be created when the first plan is written. Since custom plan - // directories must be within the project root, they are automatically - // covered by the project-wide file discovery once created. + // Directory does not exist yet, so we only add it as a writable path + // for validation, but not as a workspace directory for discovery. } } diff --git a/packages/core/src/services/sandboxedFileSystemService.ts b/packages/core/src/services/sandboxedFileSystemService.ts index e24634f5cc..6f77cc01b4 100644 --- a/packages/core/src/services/sandboxedFileSystemService.ts +++ b/packages/core/src/services/sandboxedFileSystemService.ts @@ -40,7 +40,7 @@ export class SandboxedFileSystemService implements FileSystemService { const isAllowed = checkType === 'read' ? this.workspaceContext.isPathReadable(resolvedPath) - : this.workspaceContext.isPathWithinWorkspace(resolvedPath); + : this.workspaceContext.isPathWritable(resolvedPath); if (!isAllowed) { throw new Error( diff --git a/packages/core/src/tools/enter-plan-mode.ts b/packages/core/src/tools/enter-plan-mode.ts index 7e4ce658ba..bc237b589f 100644 --- a/packages/core/src/tools/enter-plan-mode.ts +++ b/packages/core/src/tools/enter-plan-mode.ts @@ -128,6 +128,7 @@ export class EnterPlanModeInvocation extends BaseToolInvocation< // In sandboxed environments, the plans directory must exist on the host // before it can be bound/allowed in the sandbox. const plansDir = this.config.storage.getPlansDir(); + this.config.getWorkspaceContext().addWritablePath(plansDir); if (!fs.existsSync(plansDir)) { try { fs.mkdirSync(plansDir, { recursive: true }); diff --git a/packages/core/src/utils/workspaceContext.test.ts b/packages/core/src/utils/workspaceContext.test.ts index 8c29819a79..913e184a5c 100644 --- a/packages/core/src/utils/workspaceContext.test.ts +++ b/packages/core/src/utils/workspaceContext.test.ts @@ -162,6 +162,47 @@ describe('WorkspaceContext with real filesystem', () => { ); }); + it('should support granular writable paths that do not exist yet', () => { + const workspaceContext = new WorkspaceContext(cwd); + const plansDir = path.join(tempDir, 'plans'); + const planFile = path.join(plansDir, 'my-plan.md'); + + // Initially rejected + expect(workspaceContext.isPathWritable(planFile)).toBe(false); + + // Register as writable + workspaceContext.addWritablePath(plansDir); + + // Now accepted even though plansDir does not exist on disk yet + expect(workspaceContext.isPathWritable(planFile)).toBe(true); + expect(workspaceContext.isPathWritable(plansDir)).toBe(true); + + // Other paths still rejected + const otherFile = path.join(tempDir, 'other', 'file.txt'); + expect(workspaceContext.isPathWritable(otherFile)).toBe(false); + }); + + it('should support workspace paths in isPathWritable', () => { + const workspaceContext = new WorkspaceContext(cwd); + const projectFile = path.join(cwd, 'src', 'index.ts'); + + expect(workspaceContext.isPathWritable(projectFile)).toBe(true); + }); + + it('should support read-only paths in isPathReadable', () => { + const workspaceContext = new WorkspaceContext(cwd); + const readOnlyDir = path.join(tempDir, 'readonly'); + const readOnlyFile = path.join(readOnlyDir, 'data.json'); + + fs.mkdirSync(readOnlyDir); + fs.writeFileSync(readOnlyFile, '{}'); + + workspaceContext.addReadOnlyPath(readOnlyDir); + + expect(workspaceContext.isPathReadable(readOnlyFile)).toBe(true); + expect(workspaceContext.isPathWritable(readOnlyFile)).toBe(false); + }); + describe.skipIf(os.platform() === 'win32')('with symbolic link', () => { describe('in the workspace', () => { let realDir: string; diff --git a/packages/core/src/utils/workspaceContext.ts b/packages/core/src/utils/workspaceContext.ts index 48c2fa2107..1a92221ce8 100755 --- a/packages/core/src/utils/workspaceContext.ts +++ b/packages/core/src/utils/workspaceContext.ts @@ -25,6 +25,7 @@ export class WorkspaceContext { private directories = new Set(); private initialDirectories: Set; private readOnlyPaths = new Set(); + private writablePaths = new Set(); private onDirectoriesChangedListeners = new Set<() => void>(); /** @@ -132,6 +133,18 @@ export class WorkspaceContext { } } + /** + * Adds a path that is allowed to be written to, even if it's not within the workspace. + * This is useful for temporary directories or specific files that need to be + * writable even in restricted modes. Unlike addDirectory, this does not + * require the path to exist. + */ + addWritablePath(pathToAdd: string): void { + // Resolve to absolute path, but don't use realpath as it might not exist + const resolved = path.resolve(this.targetDir, pathToAdd); + this.writablePaths.add(resolved); + } + private resolveAndValidateDir(directory: string): string { const absolutePath = path.resolve(this.targetDir, directory); @@ -221,6 +234,34 @@ export class WorkspaceContext { } } + /** + * Checks if a path is allowed to be written to. + * This includes workspace paths and explicitly added writable paths. + * @param pathToCheck The path to validate + * @returns True if the path is writable, false otherwise + */ + isPathWritable(pathToCheck: string): boolean { + if (this.isPathWithinWorkspace(pathToCheck)) { + return true; + } + try { + const fullyResolvedPath = this.fullyResolvedPath(pathToCheck); + + for (const allowedPath of this.writablePaths) { + // Allow exact matches or subpaths (if allowedPath is a directory) + if ( + fullyResolvedPath === allowedPath || + this.isPathWithinRoot(fullyResolvedPath, allowedPath) + ) { + return true; + } + } + return false; + } catch { + return false; + } + } + /** * Fully resolves a path, including symbolic links. * If the path does not exist, it returns the fully resolved path as it would be