From 863a0aa01e6d2021fe03f29dc476dc188da376df Mon Sep 17 00:00:00 2001 From: galz10 Date: Mon, 9 Mar 2026 11:20:13 -0700 Subject: [PATCH] feat(core): implement SandboxManager interface and config schema - Add `sandbox` block to `ConfigSchema` with `enabled`, `allowedPaths`, and `networkAccess` properties. - Define the `SandboxManager` interface and request/response types. - Implement `NoopSandboxManager` fallback that silently passes commands through but rigorously enforces environment variable sanitization via `sanitizeEnvironment`. - Update config and sandbox tests to use the new `SandboxConfig` schema. - Add `createMockSandboxConfig` utility to `test-utils` for cleaner test mocking across the monorepo. --- package-lock.json | 26 +++- packages/cli/src/config/sandboxConfig.ts | 8 +- packages/cli/src/gemini.test.tsx | 104 ++++++++++++---- packages/cli/src/utils/sandbox.test.ts | 53 +++++---- packages/cli/src/utils/sandbox.ts | 2 + packages/core/src/config/config.test.ts | 108 +++++++++++++++-- packages/core/src/config/config.ts | 50 +++++++- .../core/src/services/sandboxManager.test.ts | 111 ++++++++++++++++++ packages/core/src/services/sandboxManager.ts | 78 ++++++++++++ packages/test-utils/src/index.ts | 1 + packages/test-utils/src/mock-utils.ts | 18 +++ 11 files changed, 494 insertions(+), 65 deletions(-) create mode 100644 packages/core/src/services/sandboxManager.test.ts create mode 100644 packages/core/src/services/sandboxManager.ts create mode 100644 packages/test-utils/src/mock-utils.ts diff --git a/package-lock.json b/package-lock.json index 85448711c7..a5437ac5c5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2292,6 +2292,7 @@ "integrity": "sha512-t54CUOsFMappY1Jbzb7fetWeO0n6K0k/4+/ZpkS+3Joz8I4VcvY9OiEBFRYISqaI2fq5sCiPtAjRDOzVYG8m+Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.2", @@ -2472,6 +2473,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", + "peer": true, "engines": { "node": ">=8.0.0" } @@ -2521,6 +2523,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.5.0.tgz", "integrity": "sha512-ka4H8OM6+DlUhSAZpONu0cPBtPPTQKxbxVzC4CzVx5+K4JnroJVBtDzLAMx4/3CDTJXRvVFhpFjtl4SaiTNoyQ==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/semantic-conventions": "^1.29.0" }, @@ -2895,6 +2898,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-2.5.0.tgz", "integrity": "sha512-F8W52ApePshpoSrfsSk1H2yJn9aKjCrbpQF1M9Qii0GHzbfVeFUB+rc3X4aggyZD8x9Gu3Slua+s6krmq6Dt8g==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/semantic-conventions": "^1.29.0" @@ -2928,6 +2932,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.5.0.tgz", "integrity": "sha512-BeJLtU+f5Gf905cJX9vXFQorAr6TAfK3SPvTFqP+scfIpDQEJfRaGJWta7sJgP+m4dNtBf9y3yvBKVAZZtJQVA==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/resources": "2.5.0" @@ -2982,6 +2987,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.5.0.tgz", "integrity": "sha512-VzRf8LzotASEyNDUxTdaJ9IRJ1/h692WyArDBInf5puLCjxbICD6XkHgpuudis56EndyS7LYFmtTMny6UABNdQ==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/resources": "2.5.0", @@ -4178,6 +4184,7 @@ "integrity": "sha512-6mDvHUFSjyT2B2yeNx2nUgMxh9LtOWvkhIU3uePn2I2oyNymUAX1NIsdgviM4CH+JSrp2D2hsMvJOkxY+0wNRA==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -4451,6 +4458,7 @@ "integrity": "sha512-klQbnPAAiGYFyI02+znpBRLyjL4/BrBd0nyWkdC0s/6xFLkXYQ8OoRrSkqacS1ddVxf/LDyODIKbQ5TgKAf/Fg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.56.1", "@typescript-eslint/types": "8.56.1", @@ -5298,6 +5306,7 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.15.0.tgz", "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -7901,6 +7910,7 @@ "integrity": "sha512-VmQ+sifHUbI/IcSopBCF/HO3YiHQx/AVd3UVyYL6weuwW+HvON9VYn5l6Zl1WZzPWXPNZrSQpxwkkZ/VuvJZzg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -8533,6 +8543,7 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -9847,6 +9858,7 @@ "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.2.tgz", "integrity": "sha512-gJnaDHXKDayjt8ue0n8Gs0A007yKXj4Xzb8+cNjZeYsSzzwKc0Lr+OZgYwVfB0pHfUs17EPoLvrOsEaJ9mj+Tg==", "license": "MIT", + "peer": true, "engines": { "node": ">=16.9.0" } @@ -10126,6 +10138,7 @@ "resolved": "https://registry.npmjs.org/@jrichman/ink/-/ink-6.4.11.tgz", "integrity": "sha512-93LQlzT7vvZ1XJcmOMwN4s+6W334QegendeHOMnEJBlhnpIzr8bws6/aOEHG8ZCuVD/vNeeea5m1msHIdAY6ig==", "license": "MIT", + "peer": true, "dependencies": { "@alcalzone/ansi-tokenize": "^0.2.1", "ansi-escapes": "^7.0.0", @@ -13808,6 +13821,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz", "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -13818,6 +13832,7 @@ "integrity": "sha512-ePrwPfxAnB+7hgnEr8vpKxL9cmnp7F322t8oqcPshbIQQhDKgFDW4tjhF2wjVbdXF9O/nyuy3sQWd9JGpiLPvA==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "shell-quote": "^1.6.1", "ws": "^7" @@ -15906,6 +15921,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -16129,7 +16145,8 @@ "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", "dev": true, - "license": "0BSD" + "license": "0BSD", + "peer": true }, "node_modules/tsx": { "version": "4.20.3", @@ -16137,6 +16154,7 @@ "integrity": "sha512-qjbnuR9Tr+FJOMBqJCW5ehvIo/buZq7vH7qD7JziU98h6l3qGy0a/yPFjwO+y0/T7GFpNgNAvEcPPVfyT8rrPQ==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -16296,6 +16314,7 @@ "integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==", "devOptional": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -16519,6 +16538,7 @@ "resolved": "https://registry.npmjs.org/vite/-/vite-7.2.2.tgz", "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -16632,6 +16652,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -16644,6 +16665,7 @@ "resolved": "https://registry.npmjs.org/vitest/-/vitest-3.2.4.tgz", "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "license": "MIT", + "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -17288,6 +17310,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -17687,6 +17710,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, diff --git a/packages/cli/src/config/sandboxConfig.ts b/packages/cli/src/config/sandboxConfig.ts index 968d3e427a..281e5c0a9f 100644 --- a/packages/cli/src/config/sandboxConfig.ts +++ b/packages/cli/src/config/sandboxConfig.ts @@ -31,7 +31,9 @@ const VALID_SANDBOX_COMMANDS: ReadonlyArray = [ 'lxc', ]; -function isSandboxCommand(value: string): value is SandboxConfig['command'] { +function isSandboxCommand( + value: string, +): value is Exclude { return (VALID_SANDBOX_COMMANDS as readonly string[]).includes(value); } @@ -124,5 +126,7 @@ export async function loadSandboxConfig( process.env['GEMINI_SANDBOX_IMAGE_DEFAULT'] ?? packageJson?.config?.sandboxImageUri; - return command && image ? { command, image } : undefined; + return command && image + ? { enabled: true, allowedPaths: [], networkAccess: false, command, image } + : undefined; } diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 90c63651e7..47008c9991 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -23,6 +23,7 @@ import { } from './gemini.js'; import { loadCliConfig, parseArguments } from './config/config.js'; import { loadSandboxConfig } from './config/sandboxConfig.js'; +import { createMockSandboxConfig } from '@google/gemini-cli-test-utils'; import { terminalCapabilityManager } from './ui/utils/terminalCapabilityManager.js'; import { start_sandbox } from './utils/sandbox.js'; import { validateNonInteractiveAuth } from './validateNonInterActiveAuth.js'; @@ -189,15 +190,26 @@ vi.mock('./ui/utils/terminalCapabilityManager.js', () => ({ vi.mock('./config/config.js', () => ({ loadCliConfig: vi.fn().mockImplementation(async () => createMockConfig()), - parseArguments: vi.fn().mockResolvedValue({}), + parseArguments: vi + .fn() + .mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, + }), isDebugMode: vi.fn(() => false), })); vi.mock('read-package-up', () => ({ - readPackageUp: vi.fn().mockResolvedValue({ - packageJson: { name: 'test-pkg', version: 'test-version' }, - path: '/fake/path/package.json', - }), + readPackageUp: vi + .fn() + .mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, + packageJson: { name: 'test-pkg', version: 'test-version' }, + path: '/fake/path/package.json', + }), })); vi.mock('update-notifier', () => ({ @@ -231,10 +243,15 @@ vi.mock('./utils/relaunch.js', () => ({ })); vi.mock('./config/sandboxConfig.js', () => ({ - loadSandboxConfig: vi.fn().mockResolvedValue({ - command: 'docker', - image: 'test-image', - }), + loadSandboxConfig: vi + .fn() + .mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, + command: 'docker', + image: 'test-image', + }), })); vi.mock('./deferred.js', () => ({ @@ -536,6 +553,9 @@ describe('gemini.tsx main function kitty protocol', () => { ); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any @@ -599,6 +619,9 @@ describe('gemini.tsx main function kitty protocol', () => { }); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any @@ -618,14 +641,17 @@ describe('gemini.tsx main function kitty protocol', () => { const mockConfig = createMockConfig({ isInteractive: () => false, getQuestion: () => '', - getSandbox: () => ({ command: 'docker', image: 'test-image' }), + getSandbox: () => + createMockSandboxConfig({ command: 'docker', image: 'test-image' }), }); vi.mocked(loadCliConfig).mockResolvedValue(mockConfig); - vi.mocked(loadSandboxConfig).mockResolvedValue({ - command: 'docker', - image: 'test-image', - }); + vi.mocked(loadSandboxConfig).mockResolvedValue( + createMockSandboxConfig({ + command: 'docker', + image: 'test-image', + }), + ); process.env['GEMINI_API_KEY'] = 'test-key'; try { @@ -666,6 +692,9 @@ describe('gemini.tsx main function kitty protocol', () => { ); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any vi.mocked(loadCliConfig).mockResolvedValue( @@ -721,6 +750,9 @@ describe('gemini.tsx main function kitty protocol', () => { ); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, promptInteractive: false, resume: 'session-id', } as any); // eslint-disable-line @typescript-eslint/no-explicit-any @@ -777,6 +809,9 @@ describe('gemini.tsx main function kitty protocol', () => { ); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, promptInteractive: false, resume: 'latest', } as unknown as CliArgs); @@ -827,6 +862,9 @@ describe('gemini.tsx main function kitty protocol', () => { ); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any vi.mocked(loadCliConfig).mockResolvedValue( @@ -877,6 +915,9 @@ describe('gemini.tsx main function kitty protocol', () => { ); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, promptInteractive: false, } as any); // eslint-disable-line @typescript-eslint/no-explicit-any vi.mocked(loadCliConfig).mockResolvedValue( @@ -951,6 +992,9 @@ describe('gemini.tsx main function exit codes', () => { }), ); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, promptInteractive: true, } as unknown as CliArgs); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -967,10 +1011,12 @@ describe('gemini.tsx main function exit codes', () => { it('should exit with 41 for auth failure during sandbox setup', async () => { vi.stubEnv('SANDBOX', ''); - vi.mocked(loadSandboxConfig).mockResolvedValue({ - command: 'docker', - image: 'test-image', - }); + vi.mocked(loadSandboxConfig).mockResolvedValue( + createMockSandboxConfig({ + command: 'docker', + image: 'test-image', + }), + ); vi.mocked(loadCliConfig).mockResolvedValue( createMockConfig({ refreshAuth: vi.fn().mockRejectedValue(new Error('Auth failed')), @@ -1010,6 +1056,9 @@ describe('gemini.tsx main function exit codes', () => { }), ); vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, resume: 'invalid-session', } as unknown as CliArgs); @@ -1051,7 +1100,11 @@ describe('gemini.tsx main function exit codes', () => { merged: { security: { auth: {} }, ui: {} }, }), ); - vi.mocked(parseArguments).mockResolvedValue({} as unknown as CliArgs); + vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, + } as unknown as CliArgs); // eslint-disable-next-line @typescript-eslint/no-explicit-any (process.stdin as any).isTTY = true; @@ -1086,7 +1139,11 @@ describe('gemini.tsx main function exit codes', () => { merged: { security: { auth: { selectedType: undefined } }, ui: {} }, }), ); - vi.mocked(parseArguments).mockResolvedValue({} as unknown as CliArgs); + vi.mocked(parseArguments).mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, + } as unknown as CliArgs); runNonInteractiveSpy.mockImplementation(() => Promise.resolve()); @@ -1156,7 +1213,12 @@ describe('project hooks loading based on trust', () => { const configModule = await import('./config/config.js'); loadCliConfig = vi.mocked(configModule.loadCliConfig); parseArguments = vi.mocked(configModule.parseArguments); - parseArguments.mockResolvedValue({ startupMessages: [] }); + parseArguments.mockResolvedValue({ + enabled: true, + allowedPaths: [], + networkAccess: false, + startupMessages: [], + }); const settingsModule = await import('./config/settings.js'); loadSettings = vi.mocked(settingsModule.loadSettings); diff --git a/packages/cli/src/utils/sandbox.test.ts b/packages/cli/src/utils/sandbox.test.ts index fa562f7ad6..f6a245cc67 100644 --- a/packages/cli/src/utils/sandbox.test.ts +++ b/packages/cli/src/utils/sandbox.test.ts @@ -10,6 +10,7 @@ import os from 'node:os'; import fs from 'node:fs'; import { start_sandbox } from './sandbox.js'; import { FatalSandboxError, type SandboxConfig } from '@google/gemini-cli-core'; +import { createMockSandboxConfig } from '@google/gemini-cli-test-utils'; import { EventEmitter } from 'node:events'; const { mockedHomedir, mockedGetContainerPath } = vi.hoisted(() => ({ @@ -137,10 +138,10 @@ describe('sandbox', () => { describe('start_sandbox', () => { it('should handle macOS seatbelt (sandbox-exec)', async () => { vi.mocked(os.platform).mockReturnValue('darwin'); - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'sandbox-exec', image: 'some-image', - }; + }); interface MockProcess extends EventEmitter { stdout: EventEmitter; @@ -173,19 +174,19 @@ describe('sandbox', () => { it('should throw FatalSandboxError if seatbelt profile is missing', async () => { vi.mocked(os.platform).mockReturnValue('darwin'); vi.mocked(fs.existsSync).mockReturnValue(false); - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'sandbox-exec', image: 'some-image', - }; + }); await expect(start_sandbox(config)).rejects.toThrow(FatalSandboxError); }); it('should handle Docker execution', async () => { - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); // Mock image check to return true (image exists) interface MockProcessWithStdout extends EventEmitter { @@ -231,10 +232,10 @@ describe('sandbox', () => { }); it('should pull image if missing', async () => { - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'missing-image', - }; + }); // 1. Image check fails interface MockProcessWithStdout extends EventEmitter { @@ -300,10 +301,10 @@ describe('sandbox', () => { }); it('should throw if image pull fails', async () => { - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'missing-image', - }; + }); // 1. Image check fails interface MockProcessWithStdout extends EventEmitter { @@ -338,10 +339,10 @@ describe('sandbox', () => { }); it('should mount volumes correctly', async () => { - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); process.env['SANDBOX_MOUNTS'] = '/host/path:/container/path:ro'; vi.mocked(fs.existsSync).mockReturnValue(true); // For mount path check @@ -395,10 +396,10 @@ describe('sandbox', () => { }); it('should pass through GOOGLE_GEMINI_BASE_URL and GOOGLE_VERTEX_BASE_URL', async () => { - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); process.env['GOOGLE_GEMINI_BASE_URL'] = 'http://gemini.proxy'; process.env['GOOGLE_VERTEX_BASE_URL'] = 'http://vertex.proxy'; @@ -442,10 +443,10 @@ describe('sandbox', () => { }); it('should handle user creation on Linux if needed', async () => { - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); process.env['SANDBOX_SET_UID_GID'] = 'true'; vi.mocked(os.platform).mockReturnValue('linux'); vi.mocked(execSync).mockImplementation((cmd) => { @@ -508,10 +509,10 @@ describe('sandbox', () => { it('should run lxc exec with correct args for a running container', async () => { process.env['TEST_LXC_LIST_OUTPUT'] = LXC_RUNNING; - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'lxc', image: 'gemini-sandbox', - }; + }); const mockSpawnProcess = new EventEmitter() as unknown as ReturnType< typeof spawn @@ -542,10 +543,10 @@ describe('sandbox', () => { it('should throw FatalSandboxError if lxc list fails', async () => { process.env['TEST_LXC_LIST_OUTPUT'] = 'throw'; - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'lxc', image: 'gemini-sandbox', - }; + }); await expect(start_sandbox(config)).rejects.toThrow( /Failed to query LXC container/, @@ -554,20 +555,20 @@ describe('sandbox', () => { it('should throw FatalSandboxError if container is not running', async () => { process.env['TEST_LXC_LIST_OUTPUT'] = LXC_STOPPED; - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'lxc', image: 'gemini-sandbox', - }; + }); await expect(start_sandbox(config)).rejects.toThrow(/is not running/); }); it('should throw FatalSandboxError if container is not found in list', async () => { process.env['TEST_LXC_LIST_OUTPUT'] = '[]'; - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'lxc', image: 'gemini-sandbox', - }; + }); await expect(start_sandbox(config)).rejects.toThrow(/not found/); }); @@ -577,10 +578,10 @@ describe('sandbox', () => { describe('gVisor (runsc)', () => { it('should use docker with --runtime=runsc on Linux', async () => { vi.mocked(os.platform).mockReturnValue('linux'); - const config: SandboxConfig = { + const config: SandboxConfig = createMockSandboxConfig({ command: 'runsc', image: 'gemini-cli-sandbox', - }; + }); // Mock image check interface MockProcessWithStdout extends EventEmitter { diff --git a/packages/cli/src/utils/sandbox.ts b/packages/cli/src/utils/sandbox.ts index df9a88cc4c..1b138cf4b5 100644 --- a/packages/cli/src/utils/sandbox.ts +++ b/packages/cli/src/utils/sandbox.ts @@ -217,6 +217,7 @@ export async function start_sandbox( // runsc uses docker with --runtime=runsc const command = config.command === 'runsc' ? 'docker' : config.command; + if (!command) throw new FatalSandboxError('Sandbox command is required'); debugLogger.log(`hopping into sandbox (command: ${command}) ...`); @@ -230,6 +231,7 @@ export async function start_sandbox( const isCustomProjectSandbox = fs.existsSync(projectSandboxDockerfile); const image = config.image; + if (!image) throw new FatalSandboxError('Sandbox image is required'); const workdir = path.resolve(process.cwd()); const containerWorkdir = getContainerPath(workdir); diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index fc262e2b13..e4ce17ec78 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -19,6 +19,7 @@ import { type ConfigParameters, type SandboxConfig, } from './config.js'; +import { createMockSandboxConfig } from '@google/gemini-cli-test-utils'; import { DEFAULT_MAX_ATTEMPTS } from '../utils/retry.js'; import { ExperimentFlags } from '../code_assist/experiments/flagNames.js'; import { debugLogger } from '../utils/debugLogger.js'; @@ -247,10 +248,10 @@ vi.mock('../code_assist/experiments/experiments.js'); describe('Server Config (config.ts)', () => { const MODEL = DEFAULT_GEMINI_MODEL; - const SANDBOX: SandboxConfig = { + const SANDBOX: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); const TARGET_DIR = '/path/to/target'; const DEBUG_MODE = false; const QUESTION = 'test question'; @@ -1477,14 +1478,62 @@ describe('Server Config (config.ts)', () => { expect(browserConfig.customConfig.sessionMode).toBe('persistent'); }); }); + + describe('Sandbox Configuration', () => { + it('should default sandbox settings when not provided', () => { + const config = new Config({ + ...baseParams, + sandbox: undefined, + }); + + expect(config.getSandboxEnabled()).toBe(false); + expect(config.getSandboxAllowedPaths()).toEqual([]); + expect(config.getSandboxNetworkAccess()).toBe(false); + }); + + it('should store provided sandbox settings', () => { + const sandbox: SandboxConfig = { + enabled: true, + allowedPaths: ['/tmp/foo', '/var/bar'], + networkAccess: true, + command: 'docker', + image: 'my-image', + }; + const config = new Config({ + ...baseParams, + sandbox, + }); + + expect(config.getSandboxEnabled()).toBe(true); + expect(config.getSandboxAllowedPaths()).toEqual(['/tmp/foo', '/var/bar']); + expect(config.getSandboxNetworkAccess()).toBe(true); + expect(config.getSandbox()?.command).toBe('docker'); + expect(config.getSandbox()?.image).toBe('my-image'); + }); + + it('should partially override default sandbox settings', () => { + const config = new Config({ + ...baseParams, + sandbox: { + enabled: true, + allowedPaths: ['/only/this'], + networkAccess: false, + } as SandboxConfig, + }); + + expect(config.getSandboxEnabled()).toBe(true); + expect(config.getSandboxAllowedPaths()).toEqual(['/only/this']); + expect(config.getSandboxNetworkAccess()).toBe(false); + }); + }); }); describe('GemmaModelRouterSettings', () => { const MODEL = DEFAULT_GEMINI_MODEL; - const SANDBOX: SandboxConfig = { + const SANDBOX: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); const TARGET_DIR = '/path/to/target'; const DEBUG_MODE = false; const QUESTION = 'test question'; @@ -1861,10 +1910,10 @@ describe('isYoloModeDisabled', () => { describe('BaseLlmClient Lifecycle', () => { const MODEL = 'gemini-pro'; - const SANDBOX: SandboxConfig = { + const SANDBOX: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); const TARGET_DIR = '/path/to/target'; const DEBUG_MODE = false; const QUESTION = 'test question'; @@ -1916,10 +1965,10 @@ describe('BaseLlmClient Lifecycle', () => { describe('Generation Config Merging (HACK)', () => { const MODEL = 'gemini-pro'; - const SANDBOX: SandboxConfig = { + const SANDBOX: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); const TARGET_DIR = '/path/to/target'; const DEBUG_MODE = false; const QUESTION = 'test question'; @@ -2222,10 +2271,10 @@ describe('Config getHooks', () => { describe('LocalLiteRtLmClient Lifecycle', () => { const MODEL = 'gemini-pro'; - const SANDBOX: SandboxConfig = { + const SANDBOX: SandboxConfig = createMockSandboxConfig({ command: 'docker', image: 'gemini-cli-sandbox', - }; + }); const TARGET_DIR = '/path/to/target'; const DEBUG_MODE = false; const QUESTION = 'test question'; @@ -2540,6 +2589,9 @@ describe('Config Quota & Preview Model Access', () => { usageStatisticsEnabled: false, embeddingModel: 'gemini-embedding', sandbox: { + enabled: true, + allowedPaths: [], + networkAccess: false, command: 'docker', image: 'gemini-cli-sandbox', }, @@ -3175,3 +3227,39 @@ describe('Model Persistence Bug Fix (#19864)', () => { expect(config.getModel()).toBe(PREVIEW_GEMINI_3_1_MODEL); }); }); + +describe('ConfigSchema validation', () => { + it('should validate a valid sandbox config', async () => { + const validConfig = { + sandbox: { + enabled: true, + allowedPaths: ['/tmp'], + networkAccess: false, + command: 'docker', + image: 'node:20', + }, + }; + + const { ConfigSchema } = await import('./config.js'); + const result = ConfigSchema.safeParse(validConfig); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.sandbox?.enabled).toBe(true); + } + }); + + it('should apply defaults in ConfigSchema', async () => { + const minimalConfig = { + sandbox: {}, + }; + + const { ConfigSchema } = await import('./config.js'); + const result = ConfigSchema.safeParse(minimalConfig); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.sandbox?.enabled).toBe(false); + expect(result.data.sandbox?.allowedPaths).toEqual([]); + expect(result.data.sandbox?.networkAccess).toBe(false); + } + }); +}); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index f615564533..53d93a2759 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -8,6 +8,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { inspect } from 'node:util'; import process from 'node:process'; +import { z } from 'zod'; import { AuthType, createContentGenerator, @@ -96,7 +97,6 @@ import type { import { ModelAvailabilityService } from '../availability/modelAvailabilityService.js'; import { ModelRouterService } from '../routing/modelRouterService.js'; import { OutputFormat } from '../output/types.js'; -//import { type AgentLoopContext } from './agent-loop-context.js'; import { ModelConfigService, type ModelConfig, @@ -447,10 +447,27 @@ export enum AuthProviderType { } export interface SandboxConfig { - command: 'docker' | 'podman' | 'sandbox-exec' | 'runsc' | 'lxc'; - image: string; + enabled: boolean; + allowedPaths: string[]; + networkAccess: boolean; + command?: 'docker' | 'podman' | 'sandbox-exec' | 'runsc' | 'lxc'; + image?: string; } +export const ConfigSchema = z.object({ + sandbox: z + .object({ + enabled: z.boolean().default(false), + allowedPaths: z.array(z.string()).default([]), + networkAccess: z.boolean().default(false), + command: z + .enum(['docker', 'podman', 'sandbox-exec', 'runsc', 'lxc']) + .optional(), + image: z.string().optional(), + }) + .optional(), +}); + /** * Callbacks for checking MCP server enablement status. * These callbacks are provided by the CLI package to bridge @@ -814,7 +831,19 @@ export class Config implements McpContext, AgentLoopContext { this.embeddingModel = params.embeddingModel ?? DEFAULT_GEMINI_EMBEDDING_MODEL; this.fileSystemService = new StandardFileSystemService(); - this.sandbox = params.sandbox; + this.sandbox = params.sandbox + ? { + enabled: params.sandbox.enabled ?? false, + allowedPaths: params.sandbox.allowedPaths ?? [], + networkAccess: params.sandbox.networkAccess ?? false, + command: params.sandbox.command, + image: params.sandbox.image, + } + : { + enabled: false, + allowedPaths: [], + networkAccess: false, + }; this.targetDir = path.resolve(params.targetDir); this.folderTrust = params.folderTrust ?? false; this.workspaceContext = new WorkspaceContext(this.targetDir, []); @@ -948,7 +977,6 @@ export class Config implements McpContext, AgentLoopContext { this.truncateToolOutputThreshold = params.truncateToolOutputThreshold ?? DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD; - // // TODO(joshualitt): Re-evaluate the todo tool for 3 family. this.useWriteTodos = isPreviewModel(this.model) ? false : (params.useWriteTodos ?? true); @@ -1599,6 +1627,18 @@ export class Config implements McpContext, AgentLoopContext { return this.sandbox; } + getSandboxEnabled(): boolean { + return this.sandbox?.enabled ?? false; + } + + getSandboxAllowedPaths(): string[] { + return this.sandbox?.allowedPaths ?? []; + } + + getSandboxNetworkAccess(): boolean { + return this.sandbox?.networkAccess ?? false; + } + isRestrictiveSandbox(): boolean { const sandboxConfig = this.getSandbox(); const seatbeltProfile = process.env['SEATBELT_PROFILE']; diff --git a/packages/core/src/services/sandboxManager.test.ts b/packages/core/src/services/sandboxManager.test.ts new file mode 100644 index 0000000000..bac8a8a55c --- /dev/null +++ b/packages/core/src/services/sandboxManager.test.ts @@ -0,0 +1,111 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { NoopSandboxManager } from './sandboxManager.js'; + +describe('NoopSandboxManager', () => { + const sandboxManager = new NoopSandboxManager(); + + it('should pass through the command and arguments unchanged', async () => { + const req = { + command: 'ls', + args: ['-la'], + cwd: '/tmp', + env: { PATH: '/usr/bin' }, + }; + + const result = await sandboxManager.prepareCommand(req); + + expect(result.program).toBe('ls'); + expect(result.args).toEqual(['-la']); + }); + + it('should sanitize the environment variables', async () => { + const req = { + command: 'echo', + args: ['hello'], + cwd: '/tmp', + env: { + PATH: '/usr/bin', + GITHUB_TOKEN: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', + MY_SECRET: 'super-secret', + SAFE_VAR: 'is-safe', + }, + }; + + const result = await sandboxManager.prepareCommand(req); + + expect(result.env['PATH']).toBe('/usr/bin'); + expect(result.env['SAFE_VAR']).toBe('is-safe'); + expect(result.env['GITHUB_TOKEN']).toBeUndefined(); + expect(result.env['MY_SECRET']).toBeUndefined(); + }); + + it('should force environment variable redaction even if not requested in config', async () => { + const req = { + command: 'echo', + args: ['hello'], + cwd: '/tmp', + env: { + API_KEY: 'sensitive-key', + }, + config: { + sanitizationConfig: { + enableEnvironmentVariableRedaction: false, + }, + }, + }; + + const result = await sandboxManager.prepareCommand(req); + + expect(result.env['API_KEY']).toBeUndefined(); + }); + + it('should respect allowedEnvironmentVariables in config', async () => { + const req = { + command: 'echo', + args: ['hello'], + cwd: '/tmp', + env: { + MY_TOKEN: 'secret-token', + OTHER_SECRET: 'another-secret', + }, + config: { + sanitizationConfig: { + allowedEnvironmentVariables: ['MY_TOKEN'], + }, + }, + }; + + const result = await sandboxManager.prepareCommand(req); + + expect(result.env['MY_TOKEN']).toBe('secret-token'); + expect(result.env['OTHER_SECRET']).toBeUndefined(); + }); + + it('should respect blockedEnvironmentVariables in config', async () => { + const req = { + command: 'echo', + args: ['hello'], + cwd: '/tmp', + env: { + SAFE_VAR: 'safe-value', + BLOCKED_VAR: 'blocked-value', + }, + config: { + sanitizationConfig: { + blockedEnvironmentVariables: ['BLOCKED_VAR'], + }, + }, + }; + + const result = await sandboxManager.prepareCommand(req); + + expect(result.env['SAFE_VAR']).toBe('safe-value'); + expect(result.env['BLOCKED_VAR']).toBeUndefined(); + }); +}); diff --git a/packages/core/src/services/sandboxManager.ts b/packages/core/src/services/sandboxManager.ts new file mode 100644 index 0000000000..458e15260e --- /dev/null +++ b/packages/core/src/services/sandboxManager.ts @@ -0,0 +1,78 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + sanitizeEnvironment, + type EnvironmentSanitizationConfig, +} from './environmentSanitization.js'; + +/** + * Request for preparing a command to run in a sandbox. + */ +export interface SandboxRequest { + /** The program to execute. */ + command: string; + /** Arguments for the program. */ + args: string[]; + /** The working directory. */ + cwd: string; + /** Environment variables to be passed to the program. */ + env: NodeJS.ProcessEnv; + /** Optional sandbox-specific configuration. */ + config?: { + sanitizationConfig?: Partial; + }; +} + +/** + * A command that has been prepared for sandboxed execution. + */ +export interface SandboxedCommand { + /** The program or wrapper to execute. */ + program: string; + /** Final arguments for the program. */ + args: string[]; + /** Sanitized environment variables. */ + env: NodeJS.ProcessEnv; +} + +/** + * Interface for a service that prepares commands for sandboxed execution. + */ +export interface SandboxManager { + /** + * Prepares a command to run in a sandbox, including environment sanitization. + */ + prepareCommand(req: SandboxRequest): Promise; +} + +/** + * A no-op implementation of SandboxManager that silently passes commands + * through while applying environment sanitization. + */ +export class NoopSandboxManager implements SandboxManager { + /** + * Prepares a command by sanitizing the environment and passing through + * the original program and arguments. + */ + async prepareCommand(req: SandboxRequest): Promise { + const sanitizationConfig: EnvironmentSanitizationConfig = { + allowedEnvironmentVariables: + req.config?.sanitizationConfig?.allowedEnvironmentVariables ?? [], + blockedEnvironmentVariables: + req.config?.sanitizationConfig?.blockedEnvironmentVariables ?? [], + enableEnvironmentVariableRedaction: true, // Forced for safety + }; + + const sanitizedEnv = sanitizeEnvironment(req.env, sanitizationConfig); + + return { + program: req.command, + args: req.args, + env: sanitizedEnv, + }; + } +} diff --git a/packages/test-utils/src/index.ts b/packages/test-utils/src/index.ts index c1f2f09d3e..583cbc8a8b 100644 --- a/packages/test-utils/src/index.ts +++ b/packages/test-utils/src/index.ts @@ -6,3 +6,4 @@ export * from './file-system-test-helpers.js'; export * from './test-rig.js'; +export * from './mock-utils.js'; diff --git a/packages/test-utils/src/mock-utils.ts b/packages/test-utils/src/mock-utils.ts new file mode 100644 index 0000000000..6815eb8a32 --- /dev/null +++ b/packages/test-utils/src/mock-utils.ts @@ -0,0 +1,18 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { SandboxConfig } from '@google/gemini-cli-core'; + +export function createMockSandboxConfig( + overrides?: Partial, +): SandboxConfig { + return { + enabled: true, + allowedPaths: [], + networkAccess: false, + ...overrides, + }; +}