diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 9e9133bb82..7b0c0b6159 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1007,6 +1007,7 @@ export class Config implements McpContext, AgentLoopContext { this.fileSystemService = new SandboxedFileSystemService( this._sandboxManager, params.targetDir, + this.workspaceContext, ); } else { this.fileSystemService = new StandardFileSystemService(); @@ -1708,6 +1709,11 @@ export class Config implements McpContext, AgentLoopContext { this.getApprovalMode(), ); this.shellExecutionConfig.sandboxManager = this._sandboxManager; + + // Update FileSystemService if using SandboxedFileSystemService + if (this.fileSystemService instanceof SandboxedFileSystemService) { + this.fileSystemService.updateSandboxManager(this._sandboxManager); + } } get sandboxPolicyManager() { diff --git a/packages/core/src/sandbox/linux/bwrapArgsBuilder.ts b/packages/core/src/sandbox/linux/bwrapArgsBuilder.ts index 96053d23ff..f346363bd5 100644 --- a/packages/core/src/sandbox/linux/bwrapArgsBuilder.ts +++ b/packages/core/src/sandbox/linux/bwrapArgsBuilder.ts @@ -101,7 +101,7 @@ export async function buildBwrapArgs( for (const includeDir of includeDirs) { try { const resolved = tryRealpath(includeDir); - bwrapArgs.push('--ro-bind-try', resolved, resolved); + bwrapArgs.push(bindFlag, resolved, resolved); } catch { // Ignore } @@ -127,7 +127,7 @@ export async function buildBwrapArgs( } const normalizedAllowedPath = normalize(resolved).replace(/\/$/, ''); if (normalizedAllowedPath !== normalizedWorkspace) { - bwrapArgs.push('--bind-try', resolved, resolved); + bwrapArgs.push(bindFlag, resolved, resolved); } } diff --git a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts index 2a301637c8..a688b53985 100644 --- a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts +++ b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts @@ -169,7 +169,11 @@ export function buildSeatbeltProfile(options: SeatbeltArgsOptions): string { const allowedPaths = options.allowedPaths; for (let i = 0; i < allowedPaths.length; i++) { const allowedPath = tryRealpath(allowedPaths[i]); - profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(allowedPath)}"))\n`; + if (options.workspaceWrite) { + profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(allowedPath)}"))\n`; + } else { + profile += `(allow file-read* (subpath "${escapeSchemeString(allowedPath)}"))\n`; + } } // Handle granular additional permissions diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts index 62ee8f4c17..b0c352e343 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts @@ -280,13 +280,16 @@ export class WindowsSandboxManager implements SandboxManager { const { allowed: allowedPaths, forbidden: forbiddenPaths } = await resolveSandboxPaths(this.options, req); - // Grant "Low Mandatory Level" access to includeDirectories. + // Grant "Low Mandatory Level" access to includeDirectories only if workspaceWrite is enabled. const includeDirs = sanitizePaths(this.options.includeDirectories); for (const includeDir of includeDirs) { - await this.grantLowIntegrityAccess(includeDir); + if (workspaceWrite) { + await this.grantLowIntegrityAccess(includeDir); + } } - // Grant "Low Mandatory Level" read/write access to allowedPaths. + // Grant "Low Mandatory Level" read/write access to allowedPaths only if workspaceWrite is enabled. + // If not enabled, they remain at Medium integrity (default), which means a Low integrity process cannot write to them. for (const allowedPath of allowedPaths) { const resolved = await tryRealpath(allowedPath); try { @@ -297,10 +300,12 @@ export class WindowsSandboxManager implements SandboxManager { 'On Windows, granular sandbox access can only be granted to existing paths to avoid broad parent directory permissions.', ); } - await this.grantLowIntegrityAccess(resolved); + if (workspaceWrite) { + await this.grantLowIntegrityAccess(resolved); + } } - // Grant "Low Mandatory Level" write access to additional permissions write paths. + // Granular permissions: Grant write access to specifically requested paths const additionalWritePaths = sanitizePaths( mergedAdditional.fileSystem?.write, ); diff --git a/packages/core/src/services/sandboxedFileSystemService.test.ts b/packages/core/src/services/sandboxedFileSystemService.test.ts index 83b7247d70..9e4d986a98 100644 --- a/packages/core/src/services/sandboxedFileSystemService.test.ts +++ b/packages/core/src/services/sandboxedFileSystemService.test.ts @@ -94,7 +94,11 @@ describe('SandboxedFileSystemService', () => { command: '__read', args: [testFile], policy: { - allowedPaths: [testFile], + additionalPermissions: { + fileSystem: { + read: [testFile], + }, + }, }, }), ); @@ -136,7 +140,6 @@ describe('SandboxedFileSystemService', () => { command: '__write', args: [testFile], policy: { - allowedPaths: [testFile], additionalPermissions: { fileSystem: { write: [testFile], diff --git a/packages/core/src/services/sandboxedFileSystemService.ts b/packages/core/src/services/sandboxedFileSystemService.ts index 2a5d3d08ac..e24634f5cc 100644 --- a/packages/core/src/services/sandboxedFileSystemService.ts +++ b/packages/core/src/services/sandboxedFileSystemService.ts @@ -10,6 +10,7 @@ import { type SandboxManager } from './sandboxManager.js'; import { debugLogger } from '../utils/debugLogger.js'; import { isNodeError } from '../utils/errors.js'; import { resolveToRealPath, isSubpath } from '../utils/paths.js'; +import type { WorkspaceContext } from '../utils/workspaceContext.js'; /** * A FileSystemService implementation that performs operations through a sandbox. @@ -18,27 +19,61 @@ export class SandboxedFileSystemService implements FileSystemService { constructor( private sandboxManager: SandboxManager, private cwd: string, + private workspaceContext?: WorkspaceContext, ) {} - private sanitizeAndValidatePath(filePath: string): string { + /** + * Updates the sandbox manager used by the service. + * This is called when the global approval mode changes. + */ + updateSandboxManager(sandboxManager: SandboxManager): void { + this.sandboxManager = sandboxManager; + } + + private sanitizeAndValidatePath( + filePath: string, + checkType: 'read' | 'write' = 'write', + ): string { const resolvedPath = resolveToRealPath(filePath); - if (!isSubpath(this.cwd, resolvedPath) && this.cwd !== resolvedPath) { - throw new Error( - `Access denied: Path '${filePath}' is outside the workspace.`, - ); + + if (this.workspaceContext) { + const isAllowed = + checkType === 'read' + ? this.workspaceContext.isPathReadable(resolvedPath) + : this.workspaceContext.isPathWithinWorkspace(resolvedPath); + + if (!isAllowed) { + throw new Error( + `Access denied: Path '${filePath}' is not ${ + checkType === 'read' ? 'readable' : 'writable' + } in the current workspace context.`, + ); + } + } else { + // Fallback to legacy CWD check if workspaceContext is not provided + if (!isSubpath(this.cwd, resolvedPath) && this.cwd !== resolvedPath) { + throw new Error( + `Access denied: Path '${filePath}' is outside the workspace.`, + ); + } } + return resolvedPath; } async readTextFile(filePath: string): Promise { - const safePath = this.sanitizeAndValidatePath(filePath); + const safePath = this.sanitizeAndValidatePath(filePath, 'read'); const prepared = await this.sandboxManager.prepareCommand({ command: '__read', args: [safePath], cwd: this.cwd, env: process.env, policy: { - allowedPaths: [safePath], + additionalPermissions: { + fileSystem: { + read: [safePath], + }, + }, }, }); @@ -91,14 +126,13 @@ export class SandboxedFileSystemService implements FileSystemService { } async writeTextFile(filePath: string, content: string): Promise { - const safePath = this.sanitizeAndValidatePath(filePath); + const safePath = this.sanitizeAndValidatePath(filePath, 'write'); const prepared = await this.sandboxManager.prepareCommand({ command: '__write', args: [safePath], cwd: this.cwd, env: process.env, policy: { - allowedPaths: [safePath], additionalPermissions: { fileSystem: { write: [safePath],