MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-04-22 19:14:33 -07:00
Code Issues Packages Projects Releases Wiki Activity

feat(core): integrate SandboxManager to sandbox all process-spawning tools (#22231)

Browse Source
This commit is contained in:
Gal Zahavi
2026-03-13 14:11:51 -07:00
committed by GitHub
parent 24adacdbc2
commit fa024133e6
31 changed files with 558 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/core/src/services/environmentSanitization.ts
+2 -2
View File
@@ -125,7 +125,7 @@ export const NEVER_ALLOWED_VALUE_PATTERNS = [
/-----BEGIN (RSA|OPENSSH|EC|PGP) PRIVATE KEY-----/i,
/-----BEGIN CERTIFICATE-----/i,
// Credentials in URL
/(https?|ftp|smtp):\/\/[^:]+:[^@]+@/i,
/(https?|ftp|smtp):\/\/[^:\s]{1,1024}:[^@\s]{1,1024}@/i,
// GitHub tokens (classic, fine-grained, OAuth, etc.)
/(ghp|gho|ghu|ghs|ghr|github_pat)_[a-zA-Z0-9_]{36,}/i,
// Google API keys
@@ -133,7 +133,7 @@ export const NEVER_ALLOWED_VALUE_PATTERNS = [
// Amazon AWS Access Key ID
/AKIA[A-Z0-9]{16}/i,
// Generic OAuth/JWT tokens
/eyJ[a-zA-Z0-9_-]*\.[a-zA-Z0-9_-]*\.[a-zA-Z0-9_-]*/i,
/eyJ[a-zA-Z0-9_-]{0,10240}\.[a-zA-Z0-9_-]{0,10240}\.[a-zA-Z0-9_-]{0,10240}/i,
// Stripe API keys
/(s|r)k_(live|test)_[0-9a-zA-Z]{24}/i,
// Slack tokens (bot, user, etc.)
packages/core/src/services/sandboxManager.test.ts
+2 -2
View File
@@ -45,7 +45,7 @@ describe('NoopSandboxManager', () => {
expect(result.env['MY_SECRET']).toBeUndefined();
});
it('should force environment variable redaction even if not requested in config', async () => {
it('should allow disabling environment variable redaction if requested in config', async () => {
const req = {
command: 'echo',
args: ['hello'],
@@ -62,7 +62,7 @@ describe('NoopSandboxManager', () => {
const result = await sandboxManager.prepareCommand(req);
expect(result.env['API_KEY']).toBeUndefined();
expect(result.env['API_KEY']).toBe('sensitive-key');
});
it('should respect allowedEnvironmentVariables in config', async () => {
packages/core/src/services/sandboxManager.ts
+27 -2
View File
@@ -1,6 +1,6 @@
/**
* @license
* Copyright 2025 Google LLC
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
@@ -37,6 +37,8 @@ export interface SandboxedCommand {
args: string[];
/** Sanitized environment variables. */
env: NodeJS.ProcessEnv;
/** The working directory. */
cwd?: string;
}
/**
@@ -64,7 +66,9 @@ export class NoopSandboxManager implements SandboxManager {
req.config?.sanitizationConfig?.allowedEnvironmentVariables ?? [],
blockedEnvironmentVariables:
req.config?.sanitizationConfig?.blockedEnvironmentVariables ?? [],
enableEnvironmentVariableRedaction: true, // Forced for safety
enableEnvironmentVariableRedaction:
req.config?.sanitizationConfig?.enableEnvironmentVariableRedaction ??
true,
};
const sanitizedEnv = sanitizeEnvironment(req.env, sanitizationConfig);
@@ -76,3 +80,24 @@ export class NoopSandboxManager implements SandboxManager {
};
}
}
/**
* SandboxManager that implements actual sandboxing.
*/
export class LocalSandboxManager implements SandboxManager {
async prepareCommand(_req: SandboxRequest): Promise<SandboxedCommand> {
throw new Error('Tool sandboxing is not yet implemented.');
}
}
/**
* Creates a sandbox manager based on the provided settings.
*/
export function createSandboxManager(
sandboxingEnabled: boolean,
): SandboxManager {
if (sandboxingEnabled) {
return new LocalSandboxManager();
}
return new NoopSandboxManager();
}
packages/core/src/services/shellExecutionService.test.ts
+60 -1
View File
@@ -22,6 +22,7 @@ import {
type ShellOutputEvent,
type ShellExecutionConfig,
} from './shellExecutionService.js';
import { NoopSandboxManager } from './sandboxManager.js';
import { ExecutionLifecycleService } from './executionLifecycleService.js';
import type { AnsiOutput, AnsiToken } from '../utils/terminalSerializer.js';
@@ -137,6 +138,7 @@ const shellExecutionConfig: ShellExecutionConfig = {
allowedEnvironmentVariables: [],
blockedEnvironmentVariables: [],
},
sandboxManager: new NoopSandboxManager(),
};
const createMockSerializeTerminalToObjectReturnValue = (
@@ -625,6 +627,7 @@ describe('ShellExecutionService', () => {
new AbortController().signal,
true,
{
...shellExecutionConfig,
sanitizationConfig: {
enableEnvironmentVariableRedaction: true,
allowedEnvironmentVariables: [],
@@ -1396,7 +1399,7 @@ describe('ShellExecutionService child_process fallback', () => {
expect(mockCpSpawn).toHaveBeenCalledWith(
expectedCommand,
['/pid', String(mockChildProcess.pid), '/f', '/t'],
undefined,
expect.anything(),
);
}
});
@@ -1417,6 +1420,7 @@ describe('ShellExecutionService child_process fallback', () => {
abortController.signal,
true,
{
...shellExecutionConfig,
sanitizationConfig: {
enableEnvironmentVariableRedaction: true,
allowedEnvironmentVariables: [],
@@ -1631,6 +1635,7 @@ describe('ShellExecutionService execution method selection', () => {
abortController.signal,
false, // shouldUseNodePty
{
...shellExecutionConfig,
sanitizationConfig: {
enableEnvironmentVariableRedaction: true,
allowedEnvironmentVariables: [],
@@ -1778,6 +1783,7 @@ describe('ShellExecutionService environment variables', () => {
new AbortController().signal,
true,
{
...shellExecutionConfig,
sanitizationConfig: {
enableEnvironmentVariableRedaction: false,
allowedEnvironmentVariables: [],
@@ -1837,6 +1843,7 @@ describe('ShellExecutionService environment variables', () => {
new AbortController().signal,
true,
{
...shellExecutionConfig,
sanitizationConfig: {
enableEnvironmentVariableRedaction: false,
allowedEnvironmentVariables: [],
@@ -1904,6 +1911,58 @@ describe('ShellExecutionService environment variables', () => {
await new Promise(process.nextTick);
});
it('should call prepareCommand on sandboxManager when provided', async () => {
const mockSandboxManager = {
prepareCommand: vi.fn().mockResolvedValue({
program: 'sandboxed-bash',
args: ['-c', 'ls'],
env: { SANDBOXED: 'true' },
}),
};
const configWithSandbox: ShellExecutionConfig = {
...shellExecutionConfig,
sandboxManager: mockSandboxManager,
};
mockResolveExecutable.mockResolvedValue('/bin/bash/resolved');
const mockChild = new EventEmitter() as unknown as ChildProcess;
mockChild.stdout = new EventEmitter() as unknown as Readable;
mockChild.stderr = new EventEmitter() as unknown as Readable;
Object.assign(mockChild, { pid: 123 });
mockCpSpawn.mockReturnValue(mockChild);
const handle = await ShellExecutionService.execute(
'ls',
'/test/cwd',
() => {},
new AbortController().signal,
false, // child_process path
configWithSandbox,
);
expect(mockResolveExecutable).toHaveBeenCalledWith(expect.any(String));
expect(mockSandboxManager.prepareCommand).toHaveBeenCalledWith(
expect.objectContaining({
command: '/bin/bash/resolved',
args: expect.arrayContaining([expect.stringContaining('ls')]),
cwd: '/test/cwd',
}),
);
expect(mockCpSpawn).toHaveBeenCalledWith(
'sandboxed-bash',
['-c', 'ls'],
expect.objectContaining({
env: expect.objectContaining({ SANDBOXED: 'true' }),
}),
);
// Clean up
mockChild.emit('exit', 0, null);
mockChild.emit('close', 0, null);
await handle.result;
});
it('should include headless git and gh environment variables in non-interactive mode and append git config safely', async () => {
vi.resetModules();
vi.stubEnv('GIT_CONFIG_COUNT', '2');
packages/core/src/services/shellExecutionService.ts
+110 -48
View File
@@ -1,6 +1,6 @@
/**
* @license
* Copyright 2025 Google LLC
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
@@ -27,11 +27,8 @@ import {
serializeTerminalToObject,
type AnsiOutput,
} from '../utils/terminalSerializer.js';
import {
sanitizeEnvironment,
type EnvironmentSanitizationConfig,
} from './environmentSanitization.js';
import { NoopSandboxManager } from './sandboxManager.js';
import { type EnvironmentSanitizationConfig } from './environmentSanitization.js';
import { type SandboxManager } from './sandboxManager.js';
import { killProcessGroup } from '../utils/process-utils.js';
import {
ExecutionLifecycleService,
@@ -90,6 +87,7 @@ export interface ShellExecutionConfig {
defaultFg?: string;
defaultBg?: string;
sanitizationConfig: EnvironmentSanitizationConfig;
sandboxManager: SandboxManager;
// Used for testing
disableDynamicLineTrimming?: boolean;
scrollback?: number;
@@ -274,15 +272,6 @@ export class ShellExecutionService {
shouldUseNodePty: boolean,
shellExecutionConfig: ShellExecutionConfig,
): Promise<ShellExecutionHandle> {
const sandboxManager = new NoopSandboxManager();
const { env: sanitizedEnv } = await sandboxManager.prepareCommand({
command: commandToExecute,
args: [],
env: process.env,
cwd,
config: shellExecutionConfig,
});
if (shouldUseNodePty) {
const ptyInfo = await getPty();
if (ptyInfo) {
@@ -294,7 +283,6 @@ export class ShellExecutionService {
abortSignal,
shellExecutionConfig,
ptyInfo,
sanitizedEnv,
);
} catch (_e) {
// Fallback to child_process
@@ -307,7 +295,7 @@ export class ShellExecutionService {
cwd,
onOutputEvent,
abortSignal,
shellExecutionConfig.sanitizationConfig,
shellExecutionConfig,
shouldUseNodePty,
);
}
@@ -342,14 +330,49 @@ export class ShellExecutionService {
return { newBuffer: truncatedBuffer + chunk, truncated: true };
}
private static childProcessFallback(
private static async prepareExecution(
executable: string,
args: string[],
cwd: string,
env: NodeJS.ProcessEnv,
shellExecutionConfig: ShellExecutionConfig,
sanitizationConfigOverride?: EnvironmentSanitizationConfig,
): Promise<{
program: string;
args: string[];
env: NodeJS.ProcessEnv;
cwd: string;
}> {
const resolvedExecutable =
(await resolveExecutable(executable)) ?? executable;
const prepared = await shellExecutionConfig.sandboxManager.prepareCommand({
command: resolvedExecutable,
args,
cwd,
env,
config: {
sanitizationConfig:
sanitizationConfigOverride ?? shellExecutionConfig.sanitizationConfig,
},
});
return {
program: prepared.program,
args: prepared.args,
env: prepared.env,
cwd: prepared.cwd ?? cwd,
};
}
private static async childProcessFallback(
commandToExecute: string,
cwd: string,
onOutputEvent: (event: ShellOutputEvent) => void,
abortSignal: AbortSignal,
sanitizationConfig: EnvironmentSanitizationConfig,
shellExecutionConfig: ShellExecutionConfig,
isInteractive: boolean,
): ShellExecutionHandle {
): Promise<ShellExecutionHandle> {
try {
const isWindows = os.platform() === 'win32';
const { executable, argsPrefix, shell } = getShellConfiguration();
@@ -361,16 +384,17 @@ export class ShellExecutionService {
const gitConfigKeys = !isInteractive
? Object.keys(process.env).filter((k) => k.startsWith('GIT_CONFIG_'))
: [];
const sanitizedEnv = sanitizeEnvironment(process.env, {
...sanitizationConfig,
const localSanitizationConfig = {
...shellExecutionConfig.sanitizationConfig,
allowedEnvironmentVariables: [
...(sanitizationConfig.allowedEnvironmentVariables || []),
...(shellExecutionConfig.sanitizationConfig
.allowedEnvironmentVariables || []),
...gitConfigKeys,
],
});
};
const env: NodeJS.ProcessEnv = {
...sanitizedEnv,
const env = {
...process.env,
[GEMINI_CLI_IDENTIFICATION_ENV_VAR]:
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
TERM: 'xterm-256color',
@@ -378,12 +402,28 @@ export class ShellExecutionService {
GIT_PAGER: 'cat',
};
const {
program: finalExecutable,
args: finalArgs,
env: sanitizedEnv,
cwd: finalCwd,
} = await this.prepareExecution(
executable,
spawnArgs,
cwd,
env,
shellExecutionConfig,
localSanitizationConfig,
);
const finalEnv = { ...sanitizedEnv };
if (!isInteractive) {
const gitConfigCount = parseInt(
sanitizedEnv['GIT_CONFIG_COUNT'] || '0',
finalEnv['GIT_CONFIG_COUNT'] || '0',
10,
);
Object.assign(env, {
Object.assign(finalEnv, {
// Disable interactive prompts and session-linked credential helpers
// in non-interactive mode to prevent hangs in detached process groups.
GIT_TERMINAL_PROMPT: '0',
@@ -399,13 +439,13 @@ export class ShellExecutionService {
});
}
const child = cpSpawn(executable, spawnArgs, {
cwd,
const child = cpSpawn(finalExecutable, finalArgs, {
cwd: finalCwd,
stdio: ['ignore', 'pipe', 'pipe'],
windowsVerbatimArguments: isWindows ? false : undefined,
shell: false,
detached: !isWindows,
env,
env: finalEnv,
});
const state = {
@@ -682,7 +722,6 @@ export class ShellExecutionService {
abortSignal: AbortSignal,
shellExecutionConfig: ShellExecutionConfig,
ptyInfo: PtyImplementation,
sanitizedEnv: Record<string, string | undefined>,
): Promise<ShellExecutionHandle> {
if (!ptyInfo) {
// This should not happen, but as a safeguard...
@@ -695,29 +734,52 @@ export class ShellExecutionService {
const rows = shellExecutionConfig.terminalHeight ?? 30;
const { executable, argsPrefix, shell } = getShellConfiguration();
const resolvedExecutable = await resolveExecutable(executable);
if (!resolvedExecutable) {
throw new Error(
`Shell executable "${executable}" not found in PATH or at absolute location. Please ensure the shell is installed and available in your environment.`,
);
}
const guardedCommand = ensurePromptvarsDisabled(commandToExecute, shell);
const args = [...argsPrefix, guardedCommand];
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const ptyProcess = ptyInfo.module.spawn(executable, args, {
const env = {
...process.env,
GEMINI_CLI: '1',
TERM: 'xterm-256color',
PAGER: shellExecutionConfig.pager ?? 'cat',
GIT_PAGER: shellExecutionConfig.pager ?? 'cat',
};
// Specifically allow GIT_CONFIG_* variables to pass through sanitization
// so we can safely append our overrides if needed.
const gitConfigKeys = Object.keys(process.env).filter((k) =>
k.startsWith('GIT_CONFIG_'),
);
const localSanitizationConfig = {
...shellExecutionConfig.sanitizationConfig,
allowedEnvironmentVariables: [
...(shellExecutionConfig.sanitizationConfig
?.allowedEnvironmentVariables ?? []),
...gitConfigKeys,
],
};
const {
program: finalExecutable,
args: finalArgs,
env: finalEnv,
cwd: finalCwd,
} = await this.prepareExecution(
executable,
args,
cwd,
env,
shellExecutionConfig,
localSanitizationConfig,
);
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const ptyProcess = ptyInfo.module.spawn(finalExecutable, finalArgs, {
cwd: finalCwd,
name: 'xterm-256color',
cols,
rows,
env: {
...sanitizedEnv,
GEMINI_CLI: '1',
TERM: 'xterm-256color',
PAGER: shellExecutionConfig.pager ?? 'cat',
GIT_PAGER: shellExecutionConfig.pager ?? 'cat',
},
env: finalEnv,
handleFlowControl: true,
});
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion