From 65024d453856677d99c89497e8c6adcdf8a1a030 Mon Sep 17 00:00:00 2001 From: Gal Zahavi <38544478+galz10@users.noreply.github.com> Date: Fri, 3 Apr 2026 17:23:27 -0700 Subject: [PATCH] fix(core): ensure global temp directory is always in sandbox allowed paths (#24638) Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- packages/core/src/config/config.test.ts | 15 ++++++++--- packages/core/src/config/config.ts | 27 ++++++++++++++++--- .../src/sandbox/linux/LinuxSandboxManager.ts | 4 +++ .../src/sandbox/macos/MacOsSandboxManager.ts | 4 +++ .../sandbox/windows/WindowsSandboxManager.ts | 4 +++ packages/core/src/services/sandboxManager.ts | 13 +++++++++ .../sandboxedFileSystemService.test.ts | 8 ++++++ .../services/sandboxedFileSystemService.ts | 27 +++++++++++++++---- .../services/shellExecutionService.test.ts | 1 + 9 files changed, 91 insertions(+), 12 deletions(-) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 386f42754f..002d4da50e 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -1579,7 +1579,9 @@ describe('Server Config (config.ts)', () => { }); expect(config.getSandboxEnabled()).toBe(false); - expect(config.getSandboxAllowedPaths()).toEqual([]); + expect(config.getSandboxAllowedPaths()).toEqual([ + Storage.getGlobalTempDir(), + ]); expect(config.getSandboxNetworkAccess()).toBe(false); }); @@ -1597,7 +1599,11 @@ describe('Server Config (config.ts)', () => { }); expect(config.getSandboxEnabled()).toBe(true); - expect(config.getSandboxAllowedPaths()).toEqual(['/tmp/foo', '/var/bar']); + expect(config.getSandboxAllowedPaths()).toEqual([ + '/tmp/foo', + '/var/bar', + Storage.getGlobalTempDir(), + ]); expect(config.getSandboxNetworkAccess()).toBe(true); expect(config.getSandbox()?.command).toBe('docker'); expect(config.getSandbox()?.image).toBe('my-image'); @@ -1614,7 +1620,10 @@ describe('Server Config (config.ts)', () => { }); expect(config.getSandboxEnabled()).toBe(true); - expect(config.getSandboxAllowedPaths()).toEqual(['/only/this']); + expect(config.getSandboxAllowedPaths()).toEqual([ + '/only/this', + Storage.getGlobalTempDir(), + ]); expect(config.getSandboxNetworkAccess()).toBe(false); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 9e9133bb82..efb3e296df 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -508,6 +508,7 @@ export enum AuthProviderType { export interface SandboxConfig { enabled: boolean; allowedPaths?: string[]; + includeDirectories?: string[]; networkAccess?: boolean; command?: | 'docker' @@ -524,6 +525,7 @@ export const ConfigSchema = z.object({ .object({ enabled: z.boolean().default(false), allowedPaths: z.array(z.string()).default([]), + includeDirectories: z.array(z.string()).default([]), networkAccess: z.boolean().default(false), command: z .enum([ @@ -965,6 +967,11 @@ export class Config implements McpContext, AgentLoopContext { ? { enabled: params.sandbox.enabled || params.toolSandboxing || false, allowedPaths: params.sandbox.allowedPaths ?? [], + includeDirectories: [ + ...(params.sandbox.includeDirectories ?? []), + ...(params.sandbox.allowedPaths ?? []), + Storage.getGlobalTempDir(), + ], networkAccess: params.sandbox.networkAccess ?? false, command: params.sandbox.command, image: params.sandbox.image, @@ -972,6 +979,7 @@ export class Config implements McpContext, AgentLoopContext { : { enabled: params.toolSandboxing || false, allowedPaths: [], + includeDirectories: [Storage.getGlobalTempDir()], networkAccess: false, }; @@ -994,7 +1002,10 @@ export class Config implements McpContext, AgentLoopContext { { workspace: this.targetDir, forbiddenPaths: this.getSandboxForbiddenPaths.bind(this), - includeDirectories: this.pendingIncludeDirectories, + includeDirectories: [ + ...this.pendingIncludeDirectories, + Storage.getGlobalTempDir(), + ], policyManager: this._sandboxPolicyManager, }, initialApprovalMode, @@ -1002,7 +1013,7 @@ export class Config implements McpContext, AgentLoopContext { if ( !(this._sandboxManager instanceof NoopSandboxManager) && - this.sandbox.enabled + this.sandbox?.enabled ) { this.fileSystemService = new SandboxedFileSystemService( this._sandboxManager, @@ -1702,7 +1713,10 @@ export class Config implements McpContext, AgentLoopContext { { workspace: this.targetDir, forbiddenPaths: this.getSandboxForbiddenPaths.bind(this), - includeDirectories: this.pendingIncludeDirectories, + includeDirectories: [ + ...this.pendingIncludeDirectories, + Storage.getGlobalTempDir(), + ], policyManager: this._sandboxPolicyManager, }, this.getApprovalMode(), @@ -1981,7 +1995,12 @@ export class Config implements McpContext, AgentLoopContext { } getSandboxAllowedPaths(): string[] { - return this.sandbox?.allowedPaths ?? []; + const paths = [...(this.sandbox?.allowedPaths ?? [])]; + const globalTempDir = Storage.getGlobalTempDir(); + if (!paths.includes(globalTempDir)) { + paths.push(globalTempDir); + } + return paths; } getSandboxNetworkAccess(): boolean { diff --git a/packages/core/src/sandbox/linux/LinuxSandboxManager.ts b/packages/core/src/sandbox/linux/LinuxSandboxManager.ts index d91ab1a836..000fea510f 100644 --- a/packages/core/src/sandbox/linux/LinuxSandboxManager.ts +++ b/packages/core/src/sandbox/linux/LinuxSandboxManager.ts @@ -148,6 +148,10 @@ export class LinuxSandboxManager implements SandboxManager { return this.options.workspace; } + getOptions(): GlobalSandboxOptions { + return this.options; + } + private getMaskFilePath(): string { if ( LinuxSandboxManager.maskFilePath && diff --git a/packages/core/src/sandbox/macos/MacOsSandboxManager.ts b/packages/core/src/sandbox/macos/MacOsSandboxManager.ts index 497bf30c31..0fee35110a 100644 --- a/packages/core/src/sandbox/macos/MacOsSandboxManager.ts +++ b/packages/core/src/sandbox/macos/MacOsSandboxManager.ts @@ -59,6 +59,10 @@ export class MacOsSandboxManager implements SandboxManager { return this.options.workspace; } + getOptions(): GlobalSandboxOptions { + return this.options; + } + async prepareCommand(req: SandboxRequest): Promise { await initializeShellParsers(); const sanitizationConfig = getSecureSanitizationConfig( diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts index 3cfb85b36a..943a339960 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts @@ -80,6 +80,10 @@ export class WindowsSandboxManager implements SandboxManager { return this.options.workspace; } + getOptions(): GlobalSandboxOptions { + return this.options; + } + /** * Ensures a file or directory exists. */ diff --git a/packages/core/src/services/sandboxManager.ts b/packages/core/src/services/sandboxManager.ts index 7260551d35..673c13b9af 100644 --- a/packages/core/src/services/sandboxManager.ts +++ b/packages/core/src/services/sandboxManager.ts @@ -146,6 +146,11 @@ export interface SandboxManager { * Returns the primary workspace directory for this sandbox. */ getWorkspace(): string; + + /** + * Returns the global sandbox options for this sandbox. + */ + getOptions(): GlobalSandboxOptions | undefined; } /** @@ -283,6 +288,10 @@ export class NoopSandboxManager implements SandboxManager { getWorkspace(): string { return this.options?.workspace ?? process.cwd(); } + + getOptions(): GlobalSandboxOptions | undefined { + return this.options; + } } /** @@ -310,6 +319,10 @@ export class LocalSandboxManager implements SandboxManager { getWorkspace(): string { return this.options?.workspace ?? process.cwd(); } + + getOptions(): GlobalSandboxOptions | undefined { + return this.options; + } } /** diff --git a/packages/core/src/services/sandboxedFileSystemService.test.ts b/packages/core/src/services/sandboxedFileSystemService.test.ts index 83b7247d70..83d5e9896a 100644 --- a/packages/core/src/services/sandboxedFileSystemService.test.ts +++ b/packages/core/src/services/sandboxedFileSystemService.test.ts @@ -18,6 +18,7 @@ import type { SandboxManager, SandboxRequest, SandboxedCommand, + GlobalSandboxOptions, } from './sandboxManager.js'; import { spawn, type ChildProcess } from 'node:child_process'; import { EventEmitter } from 'node:events'; @@ -52,6 +53,13 @@ class MockSandboxManager implements SandboxManager { getWorkspace(): string { return path.resolve('/workspace'); } + + getOptions(): GlobalSandboxOptions | undefined { + return { + workspace: path.resolve('/workspace'), + includeDirectories: [path.resolve('/test/cwd')], + }; + } } describe('SandboxedFileSystemService', () => { diff --git a/packages/core/src/services/sandboxedFileSystemService.ts b/packages/core/src/services/sandboxedFileSystemService.ts index 2a5d3d08ac..03907657f3 100644 --- a/packages/core/src/services/sandboxedFileSystemService.ts +++ b/packages/core/src/services/sandboxedFileSystemService.ts @@ -22,12 +22,29 @@ export class SandboxedFileSystemService implements FileSystemService { private sanitizeAndValidatePath(filePath: string): string { const resolvedPath = resolveToRealPath(filePath); - if (!isSubpath(this.cwd, resolvedPath) && this.cwd !== resolvedPath) { - throw new Error( - `Access denied: Path '${filePath}' is outside the workspace.`, - ); + const workspace = resolveToRealPath(this.sandboxManager.getWorkspace()); + + if (isSubpath(workspace, resolvedPath) || workspace === resolvedPath) { + return resolvedPath; } - return resolvedPath; + + // Check if the path is explicitly allowed by the sandbox manager + const options = this.sandboxManager.getOptions(); + const allowedPaths = options?.includeDirectories ?? []; + + for (const allowed of allowedPaths) { + const resolvedAllowed = resolveToRealPath(allowed); + if ( + isSubpath(resolvedAllowed, resolvedPath) || + resolvedAllowed === resolvedPath + ) { + return resolvedPath; + } + } + + throw new Error( + `Access denied: Path '${filePath}' is outside the workspace and not in allowed paths.`, + ); } async readTextFile(filePath: string): Promise { diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 0fc20225ac..0f41c55671 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -2015,6 +2015,7 @@ describe('ShellExecutionService environment variables', () => { isDangerousCommand: vi.fn().mockReturnValue(false), parseDenials: vi.fn().mockReturnValue(undefined), getWorkspace: vi.fn().mockReturnValue('/workspace'), + getOptions: vi.fn().mockReturnValue(undefined), }; const configWithSandbox: ShellExecutionConfig = {