mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-21 18:44:30 -07:00
feat(core): integrate SandboxManager to sandbox all process-spawning tools
- Integrate `SandboxManager` into `Config` and `AgentLoopContext`.
- Refactor `ShellExecutionService` to use sandboxing for PTY and child process spawns.
- Update `GrepTool`, `ShellTool`, and `ToolRegistry` to execute commands via `SandboxManager`.
- Ensure consistent environment sanitization in `spawnAsync` and `execStreaming` utilities.
- Address PR review feedback and fix compilation/lint errors:
- Respect user redaction settings in `NoopSandboxManager`.
- Use idiomatic `async/await` in `GrepTool` availability checks.
- Update license headers to 2026.
- Fix cross-package build errors and exports.
- Resolve all TypeScript and ESLint warnings/errors.
- Update `sandboxConfig.test.ts` to match new `SandboxConfig` schema.
This commit is contained in:
@@ -105,6 +105,7 @@ export class AddMemoryCommand implements Command {
|
||||
|
||||
await tool.buildAndExecute(result.toolArgs, signal, undefined, {
|
||||
sanitizationConfig: DEFAULT_SANITIZATION_CONFIG,
|
||||
sandboxManager: context.config.sandboxManager,
|
||||
});
|
||||
await refreshMemory(context.config);
|
||||
return {
|
||||
|
||||
@@ -20,7 +20,12 @@ import {
|
||||
import { createExtension } from '../test-utils/createExtension.js';
|
||||
import { ExtensionManager } from './extension-manager.js';
|
||||
import { themeManager, DEFAULT_THEME } from '../ui/themes/theme-manager.js';
|
||||
import { GEMINI_DIR, type Config, tmpdir } from '@google/gemini-cli-core';
|
||||
import {
|
||||
GEMINI_DIR,
|
||||
type Config,
|
||||
tmpdir,
|
||||
NoopSandboxManager,
|
||||
} from '@google/gemini-cli-core';
|
||||
import { createTestMergedSettings, SettingScope } from './settings.js';
|
||||
|
||||
describe('ExtensionManager theme loading', () => {
|
||||
@@ -117,6 +122,7 @@ describe('ExtensionManager theme loading', () => {
|
||||
terminalHeight: 24,
|
||||
showColor: false,
|
||||
pager: 'cat',
|
||||
sandboxManager: new NoopSandboxManager(),
|
||||
sanitizationConfig: {
|
||||
allowedEnvironmentVariables: [],
|
||||
blockedEnvironmentVariables: [],
|
||||
|
||||
@@ -90,7 +90,13 @@ describe('loadSandboxConfig', () => {
|
||||
process.env['GEMINI_SANDBOX'] = 'docker';
|
||||
mockedCommandExistsSync.mockReturnValue(true);
|
||||
const config = await loadSandboxConfig({}, {});
|
||||
expect(config).toEqual({ command: 'docker', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'docker',
|
||||
image: 'default/image',
|
||||
});
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('docker');
|
||||
});
|
||||
|
||||
@@ -113,7 +119,13 @@ describe('loadSandboxConfig', () => {
|
||||
process.env['GEMINI_SANDBOX'] = 'lxc';
|
||||
mockedCommandExistsSync.mockReturnValue(true);
|
||||
const config = await loadSandboxConfig({}, {});
|
||||
expect(config).toEqual({ command: 'lxc', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'lxc',
|
||||
image: 'default/image',
|
||||
});
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('lxc');
|
||||
});
|
||||
|
||||
@@ -134,6 +146,9 @@ describe('loadSandboxConfig', () => {
|
||||
);
|
||||
const config = await loadSandboxConfig({}, { sandbox: true });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'sandbox-exec',
|
||||
image: 'default/image',
|
||||
});
|
||||
@@ -144,6 +159,9 @@ describe('loadSandboxConfig', () => {
|
||||
mockedCommandExistsSync.mockReturnValue(true); // all commands exist
|
||||
const config = await loadSandboxConfig({}, { sandbox: true });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'sandbox-exec',
|
||||
image: 'default/image',
|
||||
});
|
||||
@@ -153,14 +171,26 @@ describe('loadSandboxConfig', () => {
|
||||
mockedOsPlatform.mockReturnValue('linux');
|
||||
mockedCommandExistsSync.mockImplementation((cmd) => cmd === 'docker');
|
||||
const config = await loadSandboxConfig({ tools: { sandbox: true } }, {});
|
||||
expect(config).toEqual({ command: 'docker', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'docker',
|
||||
image: 'default/image',
|
||||
});
|
||||
});
|
||||
|
||||
it('should use podman if available and docker is not', async () => {
|
||||
mockedOsPlatform.mockReturnValue('linux');
|
||||
mockedCommandExistsSync.mockImplementation((cmd) => cmd === 'podman');
|
||||
const config = await loadSandboxConfig({}, { sandbox: true });
|
||||
expect(config).toEqual({ command: 'podman', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'podman',
|
||||
image: 'default/image',
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw if sandbox: true but no command is found', async () => {
|
||||
@@ -177,7 +207,13 @@ describe('loadSandboxConfig', () => {
|
||||
it('should use the specified command if it exists', async () => {
|
||||
mockedCommandExistsSync.mockReturnValue(true);
|
||||
const config = await loadSandboxConfig({}, { sandbox: 'podman' });
|
||||
expect(config).toEqual({ command: 'podman', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'podman',
|
||||
image: 'default/image',
|
||||
});
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('podman');
|
||||
});
|
||||
|
||||
@@ -205,14 +241,26 @@ describe('loadSandboxConfig', () => {
|
||||
process.env['GEMINI_SANDBOX'] = 'docker';
|
||||
mockedCommandExistsSync.mockReturnValue(true);
|
||||
const config = await loadSandboxConfig({}, {});
|
||||
expect(config).toEqual({ command: 'docker', image: 'env/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'docker',
|
||||
image: 'env/image',
|
||||
});
|
||||
});
|
||||
|
||||
it('should use image from package.json if env var is not set', async () => {
|
||||
process.env['GEMINI_SANDBOX'] = 'docker';
|
||||
mockedCommandExistsSync.mockReturnValue(true);
|
||||
const config = await loadSandboxConfig({}, {});
|
||||
expect(config).toEqual({ command: 'docker', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'docker',
|
||||
image: 'default/image',
|
||||
});
|
||||
});
|
||||
|
||||
it('should return undefined if command is found but no image is configured', async () => {
|
||||
@@ -234,7 +282,13 @@ describe('loadSandboxConfig', () => {
|
||||
'should enable sandbox for value: %s',
|
||||
async (value) => {
|
||||
const config = await loadSandboxConfig({}, { sandbox: value });
|
||||
expect(config).toEqual({ command: 'docker', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'docker',
|
||||
image: 'default/image',
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
@@ -257,7 +311,13 @@ describe('loadSandboxConfig', () => {
|
||||
it('should use runsc via CLI argument on Linux', async () => {
|
||||
const config = await loadSandboxConfig({}, { sandbox: 'runsc' });
|
||||
|
||||
expect(config).toEqual({ command: 'runsc', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'runsc',
|
||||
image: 'default/image',
|
||||
});
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('runsc');
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('docker');
|
||||
});
|
||||
@@ -266,7 +326,13 @@ describe('loadSandboxConfig', () => {
|
||||
process.env['GEMINI_SANDBOX'] = 'runsc';
|
||||
const config = await loadSandboxConfig({}, {});
|
||||
|
||||
expect(config).toEqual({ command: 'runsc', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'runsc',
|
||||
image: 'default/image',
|
||||
});
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('runsc');
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('docker');
|
||||
});
|
||||
@@ -277,7 +343,13 @@ describe('loadSandboxConfig', () => {
|
||||
{},
|
||||
);
|
||||
|
||||
expect(config).toEqual({ command: 'runsc', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'runsc',
|
||||
image: 'default/image',
|
||||
});
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('runsc');
|
||||
expect(mockedCommandExistsSync).toHaveBeenCalledWith('docker');
|
||||
});
|
||||
@@ -289,7 +361,13 @@ describe('loadSandboxConfig', () => {
|
||||
{ sandbox: 'podman' },
|
||||
);
|
||||
|
||||
expect(config).toEqual({ command: 'runsc', image: 'default/image' });
|
||||
expect(config).toEqual({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'runsc',
|
||||
image: 'default/image',
|
||||
});
|
||||
});
|
||||
|
||||
it('should reject runsc on macOS (Linux-only)', async () => {
|
||||
|
||||
@@ -34,7 +34,9 @@ const VALID_SANDBOX_COMMANDS: ReadonlyArray<SandboxConfig['command']> = [
|
||||
function isSandboxCommand(
|
||||
value: string,
|
||||
): value is Exclude<SandboxConfig['command'], undefined> {
|
||||
return (VALID_SANDBOX_COMMANDS as readonly string[]).includes(value);
|
||||
return (VALID_SANDBOX_COMMANDS as ReadonlyArray<string | undefined>).includes(
|
||||
value,
|
||||
);
|
||||
}
|
||||
|
||||
function getSandboxCommand(
|
||||
|
||||
@@ -190,26 +190,22 @@ vi.mock('./ui/utils/terminalCapabilityManager.js', () => ({
|
||||
|
||||
vi.mock('./config/config.js', () => ({
|
||||
loadCliConfig: vi.fn().mockImplementation(async () => createMockConfig()),
|
||||
parseArguments: vi
|
||||
.fn()
|
||||
.mockResolvedValue({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
}),
|
||||
parseArguments: vi.fn().mockResolvedValue({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
}),
|
||||
isDebugMode: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
vi.mock('read-package-up', () => ({
|
||||
readPackageUp: vi
|
||||
.fn()
|
||||
.mockResolvedValue({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
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', () => ({
|
||||
@@ -243,15 +239,13 @@ vi.mock('./utils/relaunch.js', () => ({
|
||||
}));
|
||||
|
||||
vi.mock('./config/sandboxConfig.js', () => ({
|
||||
loadSandboxConfig: vi
|
||||
.fn()
|
||||
.mockResolvedValue({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'docker',
|
||||
image: 'test-image',
|
||||
}),
|
||||
loadSandboxConfig: vi.fn().mockResolvedValue({
|
||||
enabled: true,
|
||||
allowedPaths: [],
|
||||
networkAccess: false,
|
||||
command: 'docker',
|
||||
image: 'test-image',
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('./deferred.js', () => ({
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
ApprovalMode,
|
||||
getShellConfiguration,
|
||||
PolicyDecision,
|
||||
NoopSandboxManager,
|
||||
} from '@google/gemini-cli-core';
|
||||
import { quote } from 'shell-quote';
|
||||
import { createPartFromText } from '@google/genai';
|
||||
@@ -77,7 +78,14 @@ describe('ShellProcessor', () => {
|
||||
getTargetDir: vi.fn().mockReturnValue('/test/dir'),
|
||||
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
|
||||
getEnableInteractiveShell: vi.fn().mockReturnValue(false),
|
||||
getShellExecutionConfig: vi.fn().mockReturnValue({}),
|
||||
getShellExecutionConfig: vi.fn().mockReturnValue({
|
||||
sandboxManager: new NoopSandboxManager(),
|
||||
sanitizationConfig: {
|
||||
allowedEnvironmentVariables: [],
|
||||
blockedEnvironmentVariables: [],
|
||||
enableEnvironmentVariableRedaction: false,
|
||||
},
|
||||
}),
|
||||
getPolicyEngine: vi.fn().mockReturnValue({
|
||||
check: mockPolicyEngineCheck,
|
||||
}),
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
|
||||
import { vi } from 'vitest';
|
||||
import type { Config } from '@google/gemini-cli-core';
|
||||
import { type Config, NoopSandboxManager } from '@google/gemini-cli-core';
|
||||
import type { LoadedSettings, Settings } from '../config/settings.js';
|
||||
import { createTestMergedSettings } from '../config/settings.js';
|
||||
|
||||
@@ -128,7 +128,14 @@ export const createMockConfig = (overrides: Partial<Config> = {}): Config =>
|
||||
getRetryFetchErrors: vi.fn().mockReturnValue(false),
|
||||
getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true),
|
||||
getShellToolInactivityTimeout: vi.fn().mockReturnValue(300000),
|
||||
getShellExecutionConfig: vi.fn().mockReturnValue({}),
|
||||
getShellExecutionConfig: vi.fn().mockReturnValue({
|
||||
sandboxManager: new NoopSandboxManager(),
|
||||
sanitizationConfig: {
|
||||
allowedEnvironmentVariables: [],
|
||||
blockedEnvironmentVariables: [],
|
||||
enableEnvironmentVariableRedaction: false,
|
||||
},
|
||||
}),
|
||||
setShellExecutionConfig: vi.fn(),
|
||||
getEnableToolOutputTruncation: vi.fn().mockReturnValue(true),
|
||||
getTruncateToolOutputThreshold: vi.fn().mockReturnValue(1000),
|
||||
|
||||
@@ -1402,6 +1402,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
|
||||
pager: settings.merged.tools.shell.pager,
|
||||
showColor: settings.merged.tools.shell.showColor,
|
||||
sanitizationConfig: config.sanitizationConfig,
|
||||
sandboxManager: config.sandboxManager,
|
||||
});
|
||||
|
||||
const { isFocused, hasReceivedFocusEvent } = useFocus();
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
afterEach,
|
||||
type Mock,
|
||||
} from 'vitest';
|
||||
import { NoopSandboxManager } from '@google/gemini-cli-core';
|
||||
|
||||
const mockIsBinary = vi.hoisted(() => vi.fn());
|
||||
const mockShellExecutionService = vi.hoisted(() => vi.fn());
|
||||
@@ -109,8 +110,14 @@ describe('useShellCommandProcessor', () => {
|
||||
getShellExecutionConfig: () => ({
|
||||
terminalHeight: 20,
|
||||
terminalWidth: 80,
|
||||
sandboxManager: new NoopSandboxManager(),
|
||||
sanitizationConfig: {
|
||||
allowedEnvironmentVariables: [],
|
||||
blockedEnvironmentVariables: [],
|
||||
enableEnvironmentVariableRedaction: false,
|
||||
},
|
||||
}),
|
||||
} as Config;
|
||||
} as unknown as Config;
|
||||
mockGeminiClient = { addHistory: vi.fn() } as unknown as GeminiClient;
|
||||
|
||||
vi.mocked(os.platform).mockReturnValue('linux');
|
||||
|
||||
Reference in New Issue
Block a user