mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-08 04:10:35 -07:00
fix(core): ensure global temp directory is always in sandbox allowed paths (#24638)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit is contained in:
@@ -1579,7 +1579,9 @@ describe('Server Config (config.ts)', () => {
|
||||
});
|
||||
|
||||
expect(config.getSandboxEnabled()).toBe(false);
|
||||
expect(config.getSandboxAllowedPaths()).toEqual([]);
|
||||
expect(config.getSandboxAllowedPaths()).toEqual([
|
||||
Storage.getGlobalTempDir(),
|
||||
]);
|
||||
expect(config.getSandboxNetworkAccess()).toBe(false);
|
||||
});
|
||||
|
||||
@@ -1597,7 +1599,11 @@ describe('Server Config (config.ts)', () => {
|
||||
});
|
||||
|
||||
expect(config.getSandboxEnabled()).toBe(true);
|
||||
expect(config.getSandboxAllowedPaths()).toEqual(['/tmp/foo', '/var/bar']);
|
||||
expect(config.getSandboxAllowedPaths()).toEqual([
|
||||
'/tmp/foo',
|
||||
'/var/bar',
|
||||
Storage.getGlobalTempDir(),
|
||||
]);
|
||||
expect(config.getSandboxNetworkAccess()).toBe(true);
|
||||
expect(config.getSandbox()?.command).toBe('docker');
|
||||
expect(config.getSandbox()?.image).toBe('my-image');
|
||||
@@ -1614,7 +1620,10 @@ describe('Server Config (config.ts)', () => {
|
||||
});
|
||||
|
||||
expect(config.getSandboxEnabled()).toBe(true);
|
||||
expect(config.getSandboxAllowedPaths()).toEqual(['/only/this']);
|
||||
expect(config.getSandboxAllowedPaths()).toEqual([
|
||||
'/only/this',
|
||||
Storage.getGlobalTempDir(),
|
||||
]);
|
||||
expect(config.getSandboxNetworkAccess()).toBe(false);
|
||||
});
|
||||
|
||||
|
||||
@@ -508,6 +508,7 @@ export enum AuthProviderType {
|
||||
export interface SandboxConfig {
|
||||
enabled: boolean;
|
||||
allowedPaths?: string[];
|
||||
includeDirectories?: string[];
|
||||
networkAccess?: boolean;
|
||||
command?:
|
||||
| 'docker'
|
||||
@@ -524,6 +525,7 @@ export const ConfigSchema = z.object({
|
||||
.object({
|
||||
enabled: z.boolean().default(false),
|
||||
allowedPaths: z.array(z.string()).default([]),
|
||||
includeDirectories: z.array(z.string()).default([]),
|
||||
networkAccess: z.boolean().default(false),
|
||||
command: z
|
||||
.enum([
|
||||
@@ -965,6 +967,11 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
? {
|
||||
enabled: params.sandbox.enabled || params.toolSandboxing || false,
|
||||
allowedPaths: params.sandbox.allowedPaths ?? [],
|
||||
includeDirectories: [
|
||||
...(params.sandbox.includeDirectories ?? []),
|
||||
...(params.sandbox.allowedPaths ?? []),
|
||||
Storage.getGlobalTempDir(),
|
||||
],
|
||||
networkAccess: params.sandbox.networkAccess ?? false,
|
||||
command: params.sandbox.command,
|
||||
image: params.sandbox.image,
|
||||
@@ -972,6 +979,7 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
: {
|
||||
enabled: params.toolSandboxing || false,
|
||||
allowedPaths: [],
|
||||
includeDirectories: [Storage.getGlobalTempDir()],
|
||||
networkAccess: false,
|
||||
};
|
||||
|
||||
@@ -994,7 +1002,10 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
{
|
||||
workspace: this.targetDir,
|
||||
forbiddenPaths: this.getSandboxForbiddenPaths.bind(this),
|
||||
includeDirectories: this.pendingIncludeDirectories,
|
||||
includeDirectories: [
|
||||
...this.pendingIncludeDirectories,
|
||||
Storage.getGlobalTempDir(),
|
||||
],
|
||||
policyManager: this._sandboxPolicyManager,
|
||||
},
|
||||
initialApprovalMode,
|
||||
@@ -1002,7 +1013,7 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
|
||||
if (
|
||||
!(this._sandboxManager instanceof NoopSandboxManager) &&
|
||||
this.sandbox.enabled
|
||||
this.sandbox?.enabled
|
||||
) {
|
||||
this.fileSystemService = new SandboxedFileSystemService(
|
||||
this._sandboxManager,
|
||||
@@ -1702,7 +1713,10 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
{
|
||||
workspace: this.targetDir,
|
||||
forbiddenPaths: this.getSandboxForbiddenPaths.bind(this),
|
||||
includeDirectories: this.pendingIncludeDirectories,
|
||||
includeDirectories: [
|
||||
...this.pendingIncludeDirectories,
|
||||
Storage.getGlobalTempDir(),
|
||||
],
|
||||
policyManager: this._sandboxPolicyManager,
|
||||
},
|
||||
this.getApprovalMode(),
|
||||
@@ -1981,7 +1995,12 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
}
|
||||
|
||||
getSandboxAllowedPaths(): string[] {
|
||||
return this.sandbox?.allowedPaths ?? [];
|
||||
const paths = [...(this.sandbox?.allowedPaths ?? [])];
|
||||
const globalTempDir = Storage.getGlobalTempDir();
|
||||
if (!paths.includes(globalTempDir)) {
|
||||
paths.push(globalTempDir);
|
||||
}
|
||||
return paths;
|
||||
}
|
||||
|
||||
getSandboxNetworkAccess(): boolean {
|
||||
|
||||
@@ -148,6 +148,10 @@ export class LinuxSandboxManager implements SandboxManager {
|
||||
return this.options.workspace;
|
||||
}
|
||||
|
||||
getOptions(): GlobalSandboxOptions {
|
||||
return this.options;
|
||||
}
|
||||
|
||||
private getMaskFilePath(): string {
|
||||
if (
|
||||
LinuxSandboxManager.maskFilePath &&
|
||||
|
||||
@@ -59,6 +59,10 @@ export class MacOsSandboxManager implements SandboxManager {
|
||||
return this.options.workspace;
|
||||
}
|
||||
|
||||
getOptions(): GlobalSandboxOptions {
|
||||
return this.options;
|
||||
}
|
||||
|
||||
async prepareCommand(req: SandboxRequest): Promise<SandboxedCommand> {
|
||||
await initializeShellParsers();
|
||||
const sanitizationConfig = getSecureSanitizationConfig(
|
||||
|
||||
@@ -80,6 +80,10 @@ export class WindowsSandboxManager implements SandboxManager {
|
||||
return this.options.workspace;
|
||||
}
|
||||
|
||||
getOptions(): GlobalSandboxOptions {
|
||||
return this.options;
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensures a file or directory exists.
|
||||
*/
|
||||
|
||||
@@ -146,6 +146,11 @@ export interface SandboxManager {
|
||||
* Returns the primary workspace directory for this sandbox.
|
||||
*/
|
||||
getWorkspace(): string;
|
||||
|
||||
/**
|
||||
* Returns the global sandbox options for this sandbox.
|
||||
*/
|
||||
getOptions(): GlobalSandboxOptions | undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -283,6 +288,10 @@ export class NoopSandboxManager implements SandboxManager {
|
||||
getWorkspace(): string {
|
||||
return this.options?.workspace ?? process.cwd();
|
||||
}
|
||||
|
||||
getOptions(): GlobalSandboxOptions | undefined {
|
||||
return this.options;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -310,6 +319,10 @@ export class LocalSandboxManager implements SandboxManager {
|
||||
getWorkspace(): string {
|
||||
return this.options?.workspace ?? process.cwd();
|
||||
}
|
||||
|
||||
getOptions(): GlobalSandboxOptions | undefined {
|
||||
return this.options;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -18,6 +18,7 @@ import type {
|
||||
SandboxManager,
|
||||
SandboxRequest,
|
||||
SandboxedCommand,
|
||||
GlobalSandboxOptions,
|
||||
} from './sandboxManager.js';
|
||||
import { spawn, type ChildProcess } from 'node:child_process';
|
||||
import { EventEmitter } from 'node:events';
|
||||
@@ -52,6 +53,13 @@ class MockSandboxManager implements SandboxManager {
|
||||
getWorkspace(): string {
|
||||
return path.resolve('/workspace');
|
||||
}
|
||||
|
||||
getOptions(): GlobalSandboxOptions | undefined {
|
||||
return {
|
||||
workspace: path.resolve('/workspace'),
|
||||
includeDirectories: [path.resolve('/test/cwd')],
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
describe('SandboxedFileSystemService', () => {
|
||||
|
||||
@@ -22,12 +22,29 @@ export class SandboxedFileSystemService implements FileSystemService {
|
||||
|
||||
private sanitizeAndValidatePath(filePath: string): string {
|
||||
const resolvedPath = resolveToRealPath(filePath);
|
||||
if (!isSubpath(this.cwd, resolvedPath) && this.cwd !== resolvedPath) {
|
||||
throw new Error(
|
||||
`Access denied: Path '${filePath}' is outside the workspace.`,
|
||||
);
|
||||
const workspace = resolveToRealPath(this.sandboxManager.getWorkspace());
|
||||
|
||||
if (isSubpath(workspace, resolvedPath) || workspace === resolvedPath) {
|
||||
return resolvedPath;
|
||||
}
|
||||
return resolvedPath;
|
||||
|
||||
// Check if the path is explicitly allowed by the sandbox manager
|
||||
const options = this.sandboxManager.getOptions();
|
||||
const allowedPaths = options?.includeDirectories ?? [];
|
||||
|
||||
for (const allowed of allowedPaths) {
|
||||
const resolvedAllowed = resolveToRealPath(allowed);
|
||||
if (
|
||||
isSubpath(resolvedAllowed, resolvedPath) ||
|
||||
resolvedAllowed === resolvedPath
|
||||
) {
|
||||
return resolvedPath;
|
||||
}
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Access denied: Path '${filePath}' is outside the workspace and not in allowed paths.`,
|
||||
);
|
||||
}
|
||||
|
||||
async readTextFile(filePath: string): Promise<string> {
|
||||
|
||||
@@ -2015,6 +2015,7 @@ describe('ShellExecutionService environment variables', () => {
|
||||
isDangerousCommand: vi.fn().mockReturnValue(false),
|
||||
parseDenials: vi.fn().mockReturnValue(undefined),
|
||||
getWorkspace: vi.fn().mockReturnValue('/workspace'),
|
||||
getOptions: vi.fn().mockReturnValue(undefined),
|
||||
};
|
||||
|
||||
const configWithSandbox: ShellExecutionConfig = {
|
||||
|
||||
Reference in New Issue
Block a user