mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-15 06:12:50 -07:00
fix(sandbox): enforce read-only PLAN mode in tool sandboxing
- Refactor SandboxedFileSystemService to use granular read/write permissions. - Implement updateSandboxManager to propagate mode transitions to the file system service. - Update platform sandbox managers (macOS, Linux, Windows) to treat allowedPaths and includeDirectories as read-only when workspaceWrite is false. - Improve path validation in SandboxedFileSystemService using WorkspaceContext to correctly handle ~/.gemini/tmp.
This commit is contained in:
@@ -1007,6 +1007,7 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
this.fileSystemService = new SandboxedFileSystemService(
|
||||
this._sandboxManager,
|
||||
params.targetDir,
|
||||
this.workspaceContext,
|
||||
);
|
||||
} else {
|
||||
this.fileSystemService = new StandardFileSystemService();
|
||||
@@ -1708,6 +1709,11 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
this.getApprovalMode(),
|
||||
);
|
||||
this.shellExecutionConfig.sandboxManager = this._sandboxManager;
|
||||
|
||||
// Update FileSystemService if using SandboxedFileSystemService
|
||||
if (this.fileSystemService instanceof SandboxedFileSystemService) {
|
||||
this.fileSystemService.updateSandboxManager(this._sandboxManager);
|
||||
}
|
||||
}
|
||||
|
||||
get sandboxPolicyManager() {
|
||||
|
||||
@@ -101,7 +101,7 @@ export async function buildBwrapArgs(
|
||||
for (const includeDir of includeDirs) {
|
||||
try {
|
||||
const resolved = tryRealpath(includeDir);
|
||||
bwrapArgs.push('--ro-bind-try', resolved, resolved);
|
||||
bwrapArgs.push(bindFlag, resolved, resolved);
|
||||
} catch {
|
||||
// Ignore
|
||||
}
|
||||
@@ -127,7 +127,7 @@ export async function buildBwrapArgs(
|
||||
}
|
||||
const normalizedAllowedPath = normalize(resolved).replace(/\/$/, '');
|
||||
if (normalizedAllowedPath !== normalizedWorkspace) {
|
||||
bwrapArgs.push('--bind-try', resolved, resolved);
|
||||
bwrapArgs.push(bindFlag, resolved, resolved);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -169,7 +169,11 @@ export function buildSeatbeltProfile(options: SeatbeltArgsOptions): string {
|
||||
const allowedPaths = options.allowedPaths;
|
||||
for (let i = 0; i < allowedPaths.length; i++) {
|
||||
const allowedPath = tryRealpath(allowedPaths[i]);
|
||||
profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(allowedPath)}"))\n`;
|
||||
if (options.workspaceWrite) {
|
||||
profile += `(allow file-read* file-write* (subpath "${escapeSchemeString(allowedPath)}"))\n`;
|
||||
} else {
|
||||
profile += `(allow file-read* (subpath "${escapeSchemeString(allowedPath)}"))\n`;
|
||||
}
|
||||
}
|
||||
|
||||
// Handle granular additional permissions
|
||||
|
||||
@@ -280,13 +280,16 @@ export class WindowsSandboxManager implements SandboxManager {
|
||||
const { allowed: allowedPaths, forbidden: forbiddenPaths } =
|
||||
await resolveSandboxPaths(this.options, req);
|
||||
|
||||
// Grant "Low Mandatory Level" access to includeDirectories.
|
||||
// Grant "Low Mandatory Level" access to includeDirectories only if workspaceWrite is enabled.
|
||||
const includeDirs = sanitizePaths(this.options.includeDirectories);
|
||||
for (const includeDir of includeDirs) {
|
||||
await this.grantLowIntegrityAccess(includeDir);
|
||||
if (workspaceWrite) {
|
||||
await this.grantLowIntegrityAccess(includeDir);
|
||||
}
|
||||
}
|
||||
|
||||
// Grant "Low Mandatory Level" read/write access to allowedPaths.
|
||||
// Grant "Low Mandatory Level" read/write access to allowedPaths only if workspaceWrite is enabled.
|
||||
// If not enabled, they remain at Medium integrity (default), which means a Low integrity process cannot write to them.
|
||||
for (const allowedPath of allowedPaths) {
|
||||
const resolved = await tryRealpath(allowedPath);
|
||||
try {
|
||||
@@ -297,10 +300,12 @@ export class WindowsSandboxManager implements SandboxManager {
|
||||
'On Windows, granular sandbox access can only be granted to existing paths to avoid broad parent directory permissions.',
|
||||
);
|
||||
}
|
||||
await this.grantLowIntegrityAccess(resolved);
|
||||
if (workspaceWrite) {
|
||||
await this.grantLowIntegrityAccess(resolved);
|
||||
}
|
||||
}
|
||||
|
||||
// Grant "Low Mandatory Level" write access to additional permissions write paths.
|
||||
// Granular permissions: Grant write access to specifically requested paths
|
||||
const additionalWritePaths = sanitizePaths(
|
||||
mergedAdditional.fileSystem?.write,
|
||||
);
|
||||
|
||||
@@ -94,7 +94,11 @@ describe('SandboxedFileSystemService', () => {
|
||||
command: '__read',
|
||||
args: [testFile],
|
||||
policy: {
|
||||
allowedPaths: [testFile],
|
||||
additionalPermissions: {
|
||||
fileSystem: {
|
||||
read: [testFile],
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
@@ -136,7 +140,6 @@ describe('SandboxedFileSystemService', () => {
|
||||
command: '__write',
|
||||
args: [testFile],
|
||||
policy: {
|
||||
allowedPaths: [testFile],
|
||||
additionalPermissions: {
|
||||
fileSystem: {
|
||||
write: [testFile],
|
||||
|
||||
@@ -10,6 +10,7 @@ import { type SandboxManager } from './sandboxManager.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
import { isNodeError } from '../utils/errors.js';
|
||||
import { resolveToRealPath, isSubpath } from '../utils/paths.js';
|
||||
import type { WorkspaceContext } from '../utils/workspaceContext.js';
|
||||
|
||||
/**
|
||||
* A FileSystemService implementation that performs operations through a sandbox.
|
||||
@@ -18,27 +19,61 @@ export class SandboxedFileSystemService implements FileSystemService {
|
||||
constructor(
|
||||
private sandboxManager: SandboxManager,
|
||||
private cwd: string,
|
||||
private workspaceContext?: WorkspaceContext,
|
||||
) {}
|
||||
|
||||
private sanitizeAndValidatePath(filePath: string): string {
|
||||
/**
|
||||
* Updates the sandbox manager used by the service.
|
||||
* This is called when the global approval mode changes.
|
||||
*/
|
||||
updateSandboxManager(sandboxManager: SandboxManager): void {
|
||||
this.sandboxManager = sandboxManager;
|
||||
}
|
||||
|
||||
private sanitizeAndValidatePath(
|
||||
filePath: string,
|
||||
checkType: 'read' | 'write' = 'write',
|
||||
): string {
|
||||
const resolvedPath = resolveToRealPath(filePath);
|
||||
if (!isSubpath(this.cwd, resolvedPath) && this.cwd !== resolvedPath) {
|
||||
throw new Error(
|
||||
`Access denied: Path '${filePath}' is outside the workspace.`,
|
||||
);
|
||||
|
||||
if (this.workspaceContext) {
|
||||
const isAllowed =
|
||||
checkType === 'read'
|
||||
? this.workspaceContext.isPathReadable(resolvedPath)
|
||||
: this.workspaceContext.isPathWithinWorkspace(resolvedPath);
|
||||
|
||||
if (!isAllowed) {
|
||||
throw new Error(
|
||||
`Access denied: Path '${filePath}' is not ${
|
||||
checkType === 'read' ? 'readable' : 'writable'
|
||||
} in the current workspace context.`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// Fallback to legacy CWD check if workspaceContext is not provided
|
||||
if (!isSubpath(this.cwd, resolvedPath) && this.cwd !== resolvedPath) {
|
||||
throw new Error(
|
||||
`Access denied: Path '${filePath}' is outside the workspace.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return resolvedPath;
|
||||
}
|
||||
|
||||
async readTextFile(filePath: string): Promise<string> {
|
||||
const safePath = this.sanitizeAndValidatePath(filePath);
|
||||
const safePath = this.sanitizeAndValidatePath(filePath, 'read');
|
||||
const prepared = await this.sandboxManager.prepareCommand({
|
||||
command: '__read',
|
||||
args: [safePath],
|
||||
cwd: this.cwd,
|
||||
env: process.env,
|
||||
policy: {
|
||||
allowedPaths: [safePath],
|
||||
additionalPermissions: {
|
||||
fileSystem: {
|
||||
read: [safePath],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
@@ -91,14 +126,13 @@ export class SandboxedFileSystemService implements FileSystemService {
|
||||
}
|
||||
|
||||
async writeTextFile(filePath: string, content: string): Promise<void> {
|
||||
const safePath = this.sanitizeAndValidatePath(filePath);
|
||||
const safePath = this.sanitizeAndValidatePath(filePath, 'write');
|
||||
const prepared = await this.sandboxManager.prepareCommand({
|
||||
command: '__write',
|
||||
args: [safePath],
|
||||
cwd: this.cwd,
|
||||
env: process.env,
|
||||
policy: {
|
||||
allowedPaths: [safePath],
|
||||
additionalPermissions: {
|
||||
fileSystem: {
|
||||
write: [safePath],
|
||||
|
||||
Reference in New Issue
Block a user