Address comment

This commit is contained in:
Christine Betts
2026-04-02 12:38:04 -04:00
parent 60d40f3d4e
commit e62c566d39
7 changed files with 218 additions and 97 deletions
+2 -2
View File
@@ -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 {
+35
View File
@@ -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<typeof spawn>,
);
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',
+16 -84
View File
@@ -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'] || '';