fix(core): enable write to default plans dir in plan mode with sandbox

This commit is contained in:
A.K.M. Adib
2026-04-03 13:51:11 -04:00
parent 7a70ab9a5d
commit b6453a8abe
5 changed files with 145 additions and 19 deletions
+66
View File
@@ -112,6 +112,72 @@ describe('Plan Mode', () => {
).toBe(true);
});
it('should allow write_file to the default plans directory in plan mode with sandboxing enabled', async () => {
const testName =
'should allow write_file to the default plans directory in plan mode with sandboxing enabled';
await rig.setup(testName, {
settings: {
security: { toolSandboxing: true },
tools: {
core: [
'write_file',
'read_file',
'list_directory',
'exit_plan_mode',
],
},
general: {
plan: { enabled: true },
defaultApprovalMode: 'plan',
},
},
});
await rig.run({
approvalMode: 'plan',
args: `Create a file called default-plan.md in the plans directory with some content, don't ask me about the content. After that, exit plan mode with exit_plan_mode.`,
});
const toolLogs = rig.readToolLogs();
const planWrite = toolLogs.find(
(l) =>
l.toolRequest.name === 'write_file' &&
l.toolRequest.args.includes('default-plan.md'),
);
const exitPlanMode = toolLogs.find(
(l) => l.toolRequest.name === 'exit_plan_mode',
);
if (!planWrite || !exitPlanMode) {
console.error(
'All tool calls found:',
toolLogs.map((l) => ({
name: l.toolRequest.name,
args: l.toolRequest.args,
})),
);
}
expect(
planWrite,
'Expected write_file to be called for default-plan.md',
).toBeDefined();
expect(
planWrite?.toolRequest.success,
`Expected write_file to succeed, but it failed with error: ${planWrite?.toolRequest.error}`,
).toBe(true);
expect(
exitPlanMode,
'Expected exit_plan_mode to be called',
).toBeDefined();
expect(
exitPlanMode?.toolRequest.success,
`Expected exit_plan_mode to succeed, but it failed with error: ${exitPlanMode?.toolRequest.error}`,
).toBe(true);
});
it('should deny write_file to non-plans directory in plan mode', async () => {
const plansDir = '.gemini/tmp/foo/123/plans';
const testName =
+31 -3
View File
@@ -446,7 +446,8 @@
"version": "2.11.0",
"resolved": "https://registry.npmjs.org/@bufbuild/protobuf/-/protobuf-2.11.0.tgz",
"integrity": "sha512-sBXGT13cpmPR5BMgHE6UEEfEaShh5Ror6rfN3yEK5si7QVrtZg8LEPQb0VVhiLRUslD2yLnXtnRzG035J/mZXQ==",
"license": "(Apache-2.0 AND BSD-3-Clause)"
"license": "(Apache-2.0 AND BSD-3-Clause)",
"peer": true
},
"node_modules/@bundled-es-modules/cookie": {
"version": "2.0.1",
@@ -1449,6 +1450,7 @@
"resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.13.4.tgz",
"integrity": "sha512-GsFaMXCkMqkKIvwCQjCrwH+GHbPKBjhwo/8ZuUkWHqbI73Kky9I+pQltrlT0+MWpedCoosda53lgjYfyEPgxBg==",
"license": "Apache-2.0",
"peer": true,
"dependencies": {
"@grpc/proto-loader": "^0.7.13",
"@js-sdsl/ordered-map": "^4.4.2"
@@ -2155,6 +2157,7 @@
"integrity": "sha512-t54CUOsFMappY1Jbzb7fetWeO0n6K0k/4+/ZpkS+3Joz8I4VcvY9OiEBFRYISqaI2fq5sCiPtAjRDOzVYG8m+Q==",
"dev": true,
"license": "MIT",
"peer": true,
"dependencies": {
"@octokit/auth-token": "^6.0.0",
"@octokit/graphql": "^9.0.2",
@@ -2335,6 +2338,7 @@
"resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz",
"integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==",
"license": "Apache-2.0",
"peer": true,
"engines": {
"node": ">=8.0.0"
}
@@ -2384,6 +2388,7 @@
"resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.5.0.tgz",
"integrity": "sha512-ka4H8OM6+DlUhSAZpONu0cPBtPPTQKxbxVzC4CzVx5+K4JnroJVBtDzLAMx4/3CDTJXRvVFhpFjtl4SaiTNoyQ==",
"license": "Apache-2.0",
"peer": true,
"dependencies": {
"@opentelemetry/semantic-conventions": "^1.29.0"
},
@@ -2758,6 +2763,7 @@
"resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-2.5.0.tgz",
"integrity": "sha512-F8W52ApePshpoSrfsSk1H2yJn9aKjCrbpQF1M9Qii0GHzbfVeFUB+rc3X4aggyZD8x9Gu3Slua+s6krmq6Dt8g==",
"license": "Apache-2.0",
"peer": true,
"dependencies": {
"@opentelemetry/core": "2.5.0",
"@opentelemetry/semantic-conventions": "^1.29.0"
@@ -2791,6 +2797,7 @@
"resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.5.0.tgz",
"integrity": "sha512-BeJLtU+f5Gf905cJX9vXFQorAr6TAfK3SPvTFqP+scfIpDQEJfRaGJWta7sJgP+m4dNtBf9y3yvBKVAZZtJQVA==",
"license": "Apache-2.0",
"peer": true,
"dependencies": {
"@opentelemetry/core": "2.5.0",
"@opentelemetry/resources": "2.5.0"
@@ -2845,6 +2852,7 @@
"resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.5.0.tgz",
"integrity": "sha512-VzRf8LzotASEyNDUxTdaJ9IRJ1/h692WyArDBInf5puLCjxbICD6XkHgpuudis56EndyS7LYFmtTMny6UABNdQ==",
"license": "Apache-2.0",
"peer": true,
"dependencies": {
"@opentelemetry/core": "2.5.0",
"@opentelemetry/resources": "2.5.0",
@@ -4081,6 +4089,7 @@
"integrity": "sha512-6mDvHUFSjyT2B2yeNx2nUgMxh9LtOWvkhIU3uePn2I2oyNymUAX1NIsdgviM4CH+JSrp2D2hsMvJOkxY+0wNRA==",
"devOptional": true,
"license": "MIT",
"peer": true,
"dependencies": {
"csstype": "^3.0.2"
}
@@ -4355,6 +4364,7 @@
"integrity": "sha512-6sMvZePQrnZH2/cJkwRpkT7DxoAWh+g6+GFRK6bV3YQo7ogi3SX5rgF6099r5Q53Ma5qeT7LGmOmuIutF4t3lA==",
"dev": true,
"license": "MIT",
"peer": true,
"dependencies": {
"@typescript-eslint/scope-manager": "8.35.0",
"@typescript-eslint/types": "8.35.0",
@@ -5228,6 +5238,7 @@
"resolved": "https://registry.npmjs.org/acorn/-/acorn-8.15.0.tgz",
"integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==",
"license": "MIT",
"peer": true,
"bin": {
"acorn": "bin/acorn"
},
@@ -7362,7 +7373,8 @@
"version": "0.0.1581282",
"resolved": "https://registry.npmjs.org/devtools-protocol/-/devtools-protocol-0.0.1581282.tgz",
"integrity": "sha512-nv7iKtNZQshSW2hKzYNr46nM/Cfh5SEvE2oV0/SEGgc9XupIY5ggf84Cz8eJIkBce7S3bmTAauFD6aysMpnqsQ==",
"license": "BSD-3-Clause"
"license": "BSD-3-Clause",
"peer": true
},
"node_modules/dezalgo": {
"version": "1.0.4",
@@ -7946,6 +7958,7 @@
"integrity": "sha512-GsGizj2Y1rCWDu6XoEekL3RLilp0voSePurjZIkxL3wlm5o5EC9VpgaP7lrCvjnkuLvzFBQWB3vWB3K5KQTveQ==",
"dev": true,
"license": "MIT",
"peer": true,
"dependencies": {
"@eslint-community/eslint-utils": "^4.2.0",
"@eslint-community/regexpp": "^4.12.1",
@@ -8463,6 +8476,7 @@
"resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz",
"integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==",
"license": "MIT",
"peer": true,
"dependencies": {
"accepts": "^2.0.0",
"body-parser": "^2.2.1",
@@ -9775,6 +9789,7 @@
"resolved": "https://registry.npmjs.org/hono/-/hono-4.12.7.tgz",
"integrity": "sha512-jq9l1DM0zVIvsm3lv9Nw9nlJnMNPOcAtsbsgiUhWcFzPE99Gvo6yRTlszSLLYacMeQ6quHD6hMfId8crVHvexw==",
"license": "MIT",
"peer": true,
"engines": {
"node": ">=16.9.0"
}
@@ -10053,6 +10068,7 @@
"resolved": "https://registry.npmjs.org/@jrichman/ink/-/ink-6.6.7.tgz",
"integrity": "sha512-bDzQLpLzK/dn9Ur/Ku88ZZR9totVcMGrGYAgPHidsAAbe9NKztU1fggj/iu0wRp5g1kBeALb3cfagFGdDxAU1w==",
"license": "MIT",
"peer": true,
"dependencies": {
"ansi-escapes": "^7.0.0",
"ansi-styles": "^6.2.3",
@@ -13826,6 +13842,7 @@
"resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz",
"integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==",
"license": "MIT",
"peer": true,
"engines": {
"node": ">=0.10.0"
}
@@ -13836,6 +13853,7 @@
"integrity": "sha512-ePrwPfxAnB+7hgnEr8vpKxL9cmnp7F322t8oqcPshbIQQhDKgFDW4tjhF2wjVbdXF9O/nyuy3sQWd9JGpiLPvA==",
"devOptional": true,
"license": "MIT",
"peer": true,
"dependencies": {
"shell-quote": "^1.6.1",
"ws": "^7"
@@ -15985,6 +16003,7 @@
"resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz",
"integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==",
"license": "MIT",
"peer": true,
"engines": {
"node": ">=12"
},
@@ -16207,7 +16226,8 @@
"version": "2.8.1",
"resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz",
"integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==",
"license": "0BSD"
"license": "0BSD",
"peer": true
},
"node_modules/tsx": {
"version": "4.20.3",
@@ -16215,6 +16235,7 @@
"integrity": "sha512-qjbnuR9Tr+FJOMBqJCW5ehvIo/buZq7vH7qD7JziU98h6l3qGy0a/yPFjwO+y0/T7GFpNgNAvEcPPVfyT8rrPQ==",
"devOptional": true,
"license": "MIT",
"peer": true,
"dependencies": {
"esbuild": "~0.25.0",
"get-tsconfig": "^4.7.5"
@@ -16380,6 +16401,7 @@
"integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==",
"devOptional": true,
"license": "Apache-2.0",
"peer": true,
"bin": {
"tsc": "bin/tsc",
"tsserver": "bin/tsserver"
@@ -16602,6 +16624,7 @@
"resolved": "https://registry.npmjs.org/vite/-/vite-7.2.2.tgz",
"integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==",
"license": "MIT",
"peer": true,
"dependencies": {
"esbuild": "^0.25.0",
"fdir": "^6.5.0",
@@ -16715,6 +16738,7 @@
"resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz",
"integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==",
"license": "MIT",
"peer": true,
"engines": {
"node": ">=12"
},
@@ -16727,6 +16751,7 @@
"resolved": "https://registry.npmjs.org/vitest/-/vitest-3.2.4.tgz",
"integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==",
"license": "MIT",
"peer": true,
"dependencies": {
"@types/chai": "^5.2.2",
"@vitest/expect": "3.2.4",
@@ -17374,6 +17399,7 @@
"resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz",
"integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==",
"license": "MIT",
"peer": true,
"funding": {
"url": "https://github.com/sponsors/colinhacks"
}
@@ -17817,6 +17843,7 @@
"resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.14.3.tgz",
"integrity": "sha512-Iq8QQQ/7X3Sac15oB6p0FmUg/klxQvXLeileoqrTRGJYLV+/9tubbr9ipz0GKHjmXVsgFPo/+W+2cA8eNcR+XA==",
"license": "Apache-2.0",
"peer": true,
"dependencies": {
"@grpc/proto-loader": "^0.8.0",
"@js-sdsl/ordered-map": "^4.4.2"
@@ -17920,6 +17947,7 @@
"resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz",
"integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==",
"license": "MIT",
"peer": true,
"engines": {
"node": ">=12"
},
+12 -5
View File
@@ -1006,7 +1006,7 @@ export class Config implements McpContext, AgentLoopContext {
) {
this.fileSystemService = new SandboxedFileSystemService(
this._sandboxManager,
params.targetDir,
this.workspaceContext,
);
} else {
this.fileSystemService = new StandardFileSystemService();
@@ -1401,10 +1401,17 @@ export class Config implements McpContext, AgentLoopContext {
await fs.promises.access(plansDir);
this.workspaceContext.addDirectory(plansDir);
} catch {
// Directory does not exist yet, so we don't add it to the workspace context.
// It will be created when the first plan is written. Since custom plan
// directories must be within the project root, they are automatically
// covered by the project-wide file discovery once created.
// Directory does not exist yet.
// If sandboxing is enabled, we must create it now so it can be added to the workspace context,
// otherwise SandboxedFileSystemService will reject writes to it.
if (this.sandbox?.enabled) {
try {
await fs.promises.mkdir(plansDir, { recursive: true });
this.workspaceContext.addDirectory(plansDir);
} catch (e) {
debugLogger.warn(`Failed to create plans directory for sandboxing: ${e}`);
}
}
}
}
@@ -14,6 +14,7 @@ import {
type Mock,
} from 'vitest';
import { SandboxedFileSystemService } from './sandboxedFileSystemService.js';
import { WorkspaceContext } from '../utils/workspaceContext.js';
import type {
SandboxManager,
SandboxRequest,
@@ -27,6 +28,16 @@ vi.mock('node:child_process', () => ({
spawn: vi.fn(),
}));
// Mock resolveAndValidateDir to avoid filesystem checks in tests
vi.spyOn(WorkspaceContext.prototype as any, 'resolveAndValidateDir').mockImplementation((dir: unknown) => dir as string);
// Mock fs.existsSync and realpathSync for addDirectory called in constructor
vi.mock('node:fs', async (importOriginal) => ({
...(await importOriginal<typeof import('node:fs')>()),
existsSync: vi.fn(() => true),
realpathSync: vi.fn((p) => p),
statSync: vi.fn(() => ({ isDirectory: () => true })),
}));
class MockSandboxManager implements SandboxManager {
prepareCommand = vi.fn(
async (req: SandboxRequest): Promise<SandboxedCommand> => ({
@@ -56,11 +67,13 @@ class MockSandboxManager implements SandboxManager {
describe('SandboxedFileSystemService', () => {
let sandboxManager: MockSandboxManager;
let service: SandboxedFileSystemService;
let workspaceContext: WorkspaceContext;
const cwd = '/test/cwd';
beforeEach(() => {
sandboxManager = new MockSandboxManager();
service = new SandboxedFileSystemService(sandboxManager, cwd);
workspaceContext = new WorkspaceContext(cwd);
service = new SandboxedFileSystemService(sandboxManager, workspaceContext);
vi.clearAllMocks();
});
@@ -9,7 +9,8 @@ import { type FileSystemService } from './fileSystemService.js';
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 { resolveToRealPath } from '../utils/paths.js';
import { type WorkspaceContext } from '../utils/workspaceContext.js';
/**
* A FileSystemService implementation that performs operations through a sandbox.
@@ -17,12 +18,20 @@ import { resolveToRealPath, isSubpath } from '../utils/paths.js';
export class SandboxedFileSystemService implements FileSystemService {
constructor(
private sandboxManager: SandboxManager,
private cwd: string,
private workspaceContext: WorkspaceContext,
) {}
private sanitizeAndValidatePath(filePath: string): string {
private sanitizeAndValidatePath(
filePath: string,
operation: 'read' | 'write',
): string {
const resolvedPath = resolveToRealPath(filePath);
if (!isSubpath(this.cwd, resolvedPath) && this.cwd !== resolvedPath) {
const isAllowed =
operation === 'read'
? this.workspaceContext.isPathReadable(resolvedPath)
: this.workspaceContext.isPathWithinWorkspace(resolvedPath);
if (!isAllowed) {
throw new Error(
`Access denied: Path '${filePath}' is outside the workspace.`,
);
@@ -31,11 +40,11 @@ export class SandboxedFileSystemService implements FileSystemService {
}
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,
cwd: this.workspaceContext.targetDir,
env: process.env,
policy: {
allowedPaths: [safePath],
@@ -46,7 +55,7 @@ export class SandboxedFileSystemService implements FileSystemService {
// Direct spawn is necessary here for streaming large file contents.
const child = spawn(prepared.program, prepared.args, {
cwd: this.cwd,
cwd: this.workspaceContext.targetDir,
env: prepared.env,
});
@@ -91,11 +100,11 @@ 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,
cwd: this.workspaceContext.targetDir,
env: process.env,
policy: {
allowedPaths: [safePath],
@@ -111,7 +120,7 @@ export class SandboxedFileSystemService implements FileSystemService {
// Direct spawn is necessary here for streaming large file contents.
const child = spawn(prepared.program, prepared.args, {
cwd: this.cwd,
cwd: this.workspaceContext.targetDir,
env: prepared.env,
});
@@ -157,3 +166,6 @@ export class SandboxedFileSystemService implements FileSystemService {
});
}
}
);
}
}