diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 6431d8add4..42288500e4 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -90,10 +90,10 @@ const AUTH_ENV_VAR_WHITELIST = [ /** * Sanitizes an environment variable value to prevent shell injection. - * Restricts values to a safe character set: alphanumeric, -, _, ., / + * Restricts values to a safe character set: alphanumeric, -, _, ., /, =, , */ export function sanitizeEnvVar(value: string): string { - return value.replace(/[^a-zA-Z0-9\-_./]/g, ''); + return value.replace(/[^a-zA-Z0-9\-_./=,]/g, ''); } export function getSystemSettingsPath(): string { diff --git a/packages/cli/src/utils/sandbox.test.ts b/packages/cli/src/utils/sandbox.test.ts index cc1f31421e..b7994b6436 100644 --- a/packages/cli/src/utils/sandbox.test.ts +++ b/packages/cli/src/utils/sandbox.test.ts @@ -549,6 +549,41 @@ describe('sandbox', () => { ); }); + it('should handle SANDBOX_ENV in macOS seatbelt with commas in values', async () => { + vi.mocked(os.platform).mockReturnValue('darwin'); + process.env['SANDBOX_ENV'] = 'MY_VAR=hello,world,ANOTHER_VAR=baz'; + const config: SandboxConfig = createMockSandboxConfig({ + command: 'sandbox-exec', + image: 'some-image', + }); + + interface MockProcess extends EventEmitter { + stdout: EventEmitter; + stderr: EventEmitter; + } + const mockSpawnProcess = new EventEmitter() as MockProcess; + mockSpawnProcess.stdout = new EventEmitter(); + mockSpawnProcess.stderr = new EventEmitter(); + vi.mocked(spawn).mockReturnValue( + mockSpawnProcess as unknown as ReturnType, + ); + + const promise = start_sandbox(config); + setTimeout(() => mockSpawnProcess.emit('close', 0), 10); + await promise; + + // Check that SANDBOX_ENV variables are parsed correctly despite commas + expect(spawn).toHaveBeenCalledWith( + 'sandbox-exec', + expect.arrayContaining([ + 'sh', + '-c', + expect.stringContaining('MY_VAR=hello\\,world ANOTHER_VAR=baz'), + ]), + expect.any(Object), + ); + }); + it('should pass through GOOGLE_GEMINI_BASE_URL and GOOGLE_VERTEX_BASE_URL', async () => { const config: SandboxConfig = createMockSandboxConfig({ command: 'docker', diff --git a/packages/cli/src/utils/sandbox.ts b/packages/cli/src/utils/sandbox.ts index 4f027e1ba0..a6696f3023 100644 --- a/packages/cli/src/utils/sandbox.ts +++ b/packages/cli/src/utils/sandbox.ts @@ -25,6 +25,7 @@ import { FatalSandboxError, GEMINI_DIR, homedir, + parseSandboxEnv, } from '@google/gemini-cli-core'; import { ConsolePatcher } from '../ui/utils/ConsolePatcher.js'; import { randomBytes } from 'node:crypto'; @@ -140,47 +141,19 @@ export async function start_sandbox( args.push('-D', `INCLUDE_DIR_${i}=${dirPath}`); } - const finalArgv = cliArgs; const shCommandParts = [ - `SANDBOX=sandbox-exec`, - `NODE_OPTIONS="${nodeOptions}"`, + 'SANDBOX=sandbox-exec', + `NODE_OPTIONS=${quote([nodeOptions])}`, ]; // copy additional environment variables from SANDBOX_ENV - if (process.env['SANDBOX_ENV']) { - let currentEnv = ''; - for (let part of process.env['SANDBOX_ENV'].split(',')) { - part = part.trim(); - if (!part) continue; - if (part.includes('=')) { - if (currentEnv) { - debugLogger.log(`SANDBOX_ENV: ${currentEnv}`); - const [k, ...vParts] = currentEnv.split('='); - const v = vParts.join('='); - shCommandParts.push(`${k}=${quote([v])}`); - } - currentEnv = part; - } else { - if (currentEnv) { - currentEnv += ',' + part; - } else { - debugLogger.log(`SANDBOX_ENV: ${part} (forwarded)`); - const val = process.env[part]; - if (val !== undefined) { - shCommandParts.push(`${part}=${quote([val])}`); - } - } - } - } - if (currentEnv) { - debugLogger.log(`SANDBOX_ENV: ${currentEnv}`); - const [k, ...vParts] = currentEnv.split('='); - const v = vParts.join('='); - shCommandParts.push(`${k}=${quote([v])}`); - } + const parsedSandboxEnv = parseSandboxEnv(process.env['SANDBOX_ENV']); + for (const [key, value] of Object.entries(parsedSandboxEnv)) { + debugLogger.log(`SANDBOX_ENV: ${key}=${value}`); + shCommandParts.push(`${key}=${quote([value])}`); } - shCommandParts.push(...finalArgv.map((arg) => quote([arg]))); + shCommandParts.push(...cliArgs.map((arg) => quote([arg]))); args.push('-f', profileFile, 'sh', '-c', shCommandParts.join(' ')); // start and set up proxy if GEMINI_SANDBOX_PROXY_COMMAND is set @@ -645,30 +618,10 @@ export async function start_sandbox( } // copy additional environment variables from SANDBOX_ENV - if (process.env['SANDBOX_ENV']) { - let currentEnv = ''; - for (let part of process.env['SANDBOX_ENV'].split(',')) { - part = part.trim(); - if (!part) continue; - if (part.includes('=')) { - if (currentEnv) { - debugLogger.log(`SANDBOX_ENV: ${currentEnv}`); - args.push('--env', currentEnv); - } - currentEnv = part; - } else { - if (currentEnv) { - currentEnv += ',' + part; - } else { - debugLogger.log(`SANDBOX_ENV: ${part} (forwarded)`); - args.push('--env', part); - } - } - } - if (currentEnv) { - debugLogger.log(`SANDBOX_ENV: ${currentEnv}`); - args.push('--env', currentEnv); - } + const parsedSandboxEnv = parseSandboxEnv(process.env['SANDBOX_ENV']); + for (const [key, value] of Object.entries(parsedSandboxEnv)) { + debugLogger.log(`SANDBOX_ENV: ${key}=${value}`); + args.push('--env', `${key}=${value}`); } // copy NODE_OPTIONS @@ -1019,31 +972,10 @@ async function start_lxc_sandbox( } // Forward SANDBOX_ENV key=value pairs - if (process.env['SANDBOX_ENV']) { - let currentEnv = ''; - for (let part of process.env['SANDBOX_ENV'].split(',')) { - part = part.trim(); - if (!part) continue; - if (part.includes('=')) { - if (currentEnv) { - envArgs.push('--env', currentEnv); - } - currentEnv = part; - } else { - if (currentEnv) { - currentEnv += ',' + part; - } else { - // LXC doesn't automatically forward from host, so we look it up - const val = process.env[part]; - if (val !== undefined) { - envArgs.push('--env', `${part}=${val}`); - } - } - } - } - if (currentEnv) { - envArgs.push('--env', currentEnv); - } + const parsedSandboxEnv = parseSandboxEnv(process.env['SANDBOX_ENV']); + for (const [key, value] of Object.entries(parsedSandboxEnv)) { + debugLogger.log(`SANDBOX_ENV: ${key}=${value}`); + envArgs.push('--env', `${key}=${value}`); } // Forward NODE_OPTIONS (e.g. from --inspect flags) const existingNodeOptions = process.env['NODE_OPTIONS'] || ''; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 87feca53e7..b9d9b6b02f 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -105,6 +105,8 @@ export * from './utils/filesearch/fileSearch.js'; export * from './utils/errorParsing.js'; export * from './utils/fastAckHelper.js'; export * from './utils/workspaceContext.js'; +export * from './utils/envUtils.js'; +export * from './utils/envExpansion.js'; export * from './utils/environmentContext.js'; export * from './utils/ignorePatterns.js'; export * from './utils/partUtils.js'; diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index a9d9b97f18..10d8ffcd15 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -22,6 +22,7 @@ import { import { isBinary, truncateString } from '../utils/textUtils.js'; import pkg from '@xterm/headless'; import { debugLogger } from '../utils/debugLogger.js'; +import { parseSandboxEnv } from '../utils/envUtils.js'; import { Storage } from '../config/storage.js'; import { serializeTerminalToObject, @@ -426,17 +427,9 @@ export class ShellExecutionService { }; // Forward SANDBOX_ENV key=value pairs - if (process.env['SANDBOX_ENV']) { - for (let env of process.env['SANDBOX_ENV'].split(',')) { - if ((env = env.trim())) { - const index = env.indexOf('='); - if (index > 0) { - const key = env.substring(0, index); - const value = env.substring(index + 1); - baseEnv[key] = value; - } - } - } + const sandboxEnv = parseSandboxEnv(process.env['SANDBOX_ENV']); + for (const [key, value] of Object.entries(sandboxEnv)) { + baseEnv[key] = value; } if (!isInteractive) { diff --git a/packages/core/src/utils/envUtils.test.ts b/packages/core/src/utils/envUtils.test.ts new file mode 100644 index 0000000000..9d95efe27a --- /dev/null +++ b/packages/core/src/utils/envUtils.test.ts @@ -0,0 +1,91 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { parseSandboxEnv } from './envUtils.js'; + +describe('parseSandboxEnv', () => { + it('should return an empty object for empty input', () => { + expect(parseSandboxEnv(undefined)).toEqual({}); + expect(parseSandboxEnv('')).toEqual({}); + }); + + it('should parse simple key-value pairs', () => { + const input = 'KEY1=VALUE1,KEY2=VALUE2'; + const expected = { + KEY1: 'VALUE1', + KEY2: 'VALUE2', + }; + expect(parseSandboxEnv(input)).toEqual(expected); + }); + + it('should handle values with commas using heuristic', () => { + const input = 'KEY1=VALUE1,WITH,COMMAS,KEY2=VALUE2'; + const expected = { + KEY1: 'VALUE1,WITH,COMMAS', + KEY2: 'VALUE2', + }; + expect(parseSandboxEnv(input)).toEqual(expected); + }); + + it('should forward host environment variables (limitations noted)', () => { + const hostEnv = { + HOST_VAR1: 'host_value1', + HOST_VAR2: 'host_value2', + }; + // Put forwarded ones first to avoid ambiguity with values containing commas + const input = 'HOST_VAR1,KEY2=VALUE2'; + const expected = { + HOST_VAR1: 'host_value1', + KEY2: 'VALUE2', + }; + expect(parseSandboxEnv(input, hostEnv)).toEqual(expected); + + // Note: If a forwarded var follows a KEY=VALUE, it's treated as part of the value + const input2 = 'KEY1=VALUE1,HOST_VAR2'; + const expected2 = { + KEY1: 'VALUE1,HOST_VAR2', + }; + expect(parseSandboxEnv(input2, hostEnv)).toEqual(expected2); + }); + + it('should handle keys and values with spaces (trimmed)', () => { + const input = ' KEY1 = VALUE1 , KEY2 = VALUE2 '; + const expected = { + KEY1: 'VALUE1', + KEY2: 'VALUE2', + }; + expect(parseSandboxEnv(input)).toEqual(expected); + }); + + it('should handle multiple equals signs in a value', () => { + const input = 'KEY1=VALUE1=WITH=EQUALS,KEY2=VALUE2'; + const expected = { + KEY1: 'VALUE1=WITH=EQUALS', + KEY2: 'VALUE2', + }; + expect(parseSandboxEnv(input)).toEqual(expected); + }); + + it('should handle commas and equals in the middle (heuristic limitation)', () => { + const input = 'KEY1=VALUE1,KEY2=VALUE2'; + const expected = { + KEY1: 'VALUE1', + KEY2: 'VALUE2', + }; + expect(parseSandboxEnv(input)).toEqual(expected); + }); + + it('should demonstrate the heuristic limitation: equals after comma', () => { + // If a value contains '=' after a comma, it will be misparsed as a new key. + const input = 'KEY1=foo,bar=baz'; + const expected = { + KEY1: 'foo', + bar: 'baz', + }; + expect(parseSandboxEnv(input)).toEqual(expected); + }); +}); diff --git a/packages/core/src/utils/envUtils.ts b/packages/core/src/utils/envUtils.ts new file mode 100644 index 0000000000..7238213710 --- /dev/null +++ b/packages/core/src/utils/envUtils.ts @@ -0,0 +1,68 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Parses a SANDBOX_ENV string into a map of environment variables. + * + * The input string is a comma-separated list of either: + * 1. `KEY=VALUE` pairs. + * 2. `HOST_VAR_NAME` which forwards the variable from the host environment. + * + * Heuristic: A comma is treated as a separator ONLY if the next part contains an `=`. + * This allows values to contain commas (e.g., `VAR1=a,b,VAR2=c`). + * + * Note: If a value contains an `=` after a comma, it will be misparsed as a new key. + * This is a known limitation of the heuristic. + * + * @param input - The SANDBOX_ENV string to parse. + * @param hostEnv - The host environment variables (defaults to process.env). + * @returns A record of parsed environment variables. + */ +export function parseSandboxEnv( + input: string | undefined, + hostEnv: Record = process.env, +): Record { + const result: Record = {}; + if (!input) { + return result; + } + + let currentPair = ''; + const parts = input.split(','); + + for (let part of parts) { + part = part.trim(); + if (!part) continue; + + if (part.includes('=')) { + // If we have a pending pair, process it first + if (currentPair) { + const [key, ...valParts] = currentPair.split('='); + result[key.trim()] = valParts.join('=').trim(); + } + currentPair = part; + } else { + if (currentPair) { + // This part is a continuation of the previous value (it has a comma) + currentPair += ',' + part; + } else { + // This is a forwarded host variable + const val = hostEnv[part]; + if (val !== undefined) { + result[part] = val; + } + } + } + } + + // Process the last pair + if (currentPair) { + const [key, ...valParts] = currentPair.split('='); + result[key.trim()] = valParts.join('=').trim(); + } + + return result; +}