diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index b93496262c..c8b9dcfb87 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -57,7 +57,9 @@ vi.mock('fs', async (importOriginal) => { return { ...actualFs, - mkdirSync: vi.fn(), + mkdirSync: vi.fn((p) => { + mockPaths.add(p.toString()); + }), writeFileSync: vi.fn(), existsSync: vi.fn((p) => mockPaths.has(p.toString())), statSync: vi.fn((p) => { diff --git a/packages/cli/src/config/policy-engine.integration.test.ts b/packages/cli/src/config/policy-engine.integration.test.ts index 422ca92aad..f4cc35dd8a 100644 --- a/packages/cli/src/config/policy-engine.integration.test.ts +++ b/packages/cli/src/config/policy-engine.integration.test.ts @@ -324,6 +324,117 @@ describe('Policy Engine Integration Tests', () => { ).toBe(PolicyDecision.DENY); }); + it('should allow write_file to plans directory in Plan mode', async () => { + const settings: Settings = {}; + + const config = await createPolicyEngineConfig( + settings, + ApprovalMode.PLAN, + ); + const engine = new PolicyEngine(config); + + // Valid plan file path (64-char hex hash, .md extension, safe filename) + const validPlanPath = + '/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/my-plan.md'; + expect( + ( + await engine.check( + { name: 'write_file', args: { file_path: validPlanPath } }, + undefined, + ) + ).decision, + ).toBe(PolicyDecision.ALLOW); + + // Valid plan with underscore in filename + const validPlanPath2 = + '/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/feature_auth.md'; + expect( + ( + await engine.check( + { name: 'write_file', args: { file_path: validPlanPath2 } }, + undefined, + ) + ).decision, + ).toBe(PolicyDecision.ALLOW); + }); + + it('should deny write_file outside plans directory in Plan mode', async () => { + const settings: Settings = {}; + + const config = await createPolicyEngineConfig( + settings, + ApprovalMode.PLAN, + ); + const engine = new PolicyEngine(config); + + // Write to workspace (not plans dir) should be denied + expect( + ( + await engine.check( + { name: 'write_file', args: { file_path: '/project/src/file.ts' } }, + undefined, + ) + ).decision, + ).toBe(PolicyDecision.DENY); + + // Write to plans dir but wrong extension should be denied + const wrongExtPath = + '/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/script.js'; + expect( + ( + await engine.check( + { name: 'write_file', args: { file_path: wrongExtPath } }, + undefined, + ) + ).decision, + ).toBe(PolicyDecision.DENY); + + // Path traversal attempt should be denied (filename contains /) + const traversalPath = + '/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/../../../etc/passwd.md'; + expect( + ( + await engine.check( + { name: 'write_file', args: { file_path: traversalPath } }, + undefined, + ) + ).decision, + ).toBe(PolicyDecision.DENY); + + // Invalid hash length should be denied + const shortHashPath = '/home/user/.gemini/tmp/abc123/plans/plan.md'; + expect( + ( + await engine.check( + { name: 'write_file', args: { file_path: shortHashPath } }, + undefined, + ) + ).decision, + ).toBe(PolicyDecision.DENY); + }); + + it('should deny write_file to subdirectories in Plan mode', async () => { + const settings: Settings = {}; + + const config = await createPolicyEngineConfig( + settings, + ApprovalMode.PLAN, + ); + const engine = new PolicyEngine(config); + + // Write to subdirectory should be denied + const subdirPath = + '/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/subdir/plan.md'; + expect( + ( + await engine.check( + { name: 'write_file', args: { file_path: subdirPath } }, + undefined, + ) + ).decision, + ).toBe(PolicyDecision.DENY); + }); + it('should verify priority ordering works correctly in practice', async () => { const settings: Settings = { tools: { diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 2ee826c466..97b2ab67bb 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -14,6 +14,7 @@ import { ApprovalMode } from '../policy/types.js'; import type { HookDefinition } from '../hooks/types.js'; import { HookType, HookEventName } from '../hooks/types.js'; import * as path from 'node:path'; +import * as fs from 'node:fs'; import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryTool.js'; import { DEFAULT_TELEMETRY_TARGET, @@ -2232,3 +2233,55 @@ describe('Config JIT Initialization', () => { }); }); }); + +describe('Plans Directory Initialization', () => { + const baseParams: ConfigParameters = { + sessionId: 'test-session', + targetDir: '/tmp/test', + debugMode: false, + model: 'test-model', + cwd: '/tmp/test', + }; + + beforeEach(() => { + vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined); + }); + + afterEach(() => { + vi.mocked(fs.promises.mkdir).mockRestore(); + }); + + it('should create plans directory and add it to workspace context when plan is enabled', async () => { + const config = new Config({ + ...baseParams, + plan: true, + }); + + await config.initialize(); + + const plansDir = config.storage.getProjectTempPlansDir(); + expect(fs.promises.mkdir).toHaveBeenCalledWith(plansDir, { + recursive: true, + }); + + const context = config.getWorkspaceContext(); + expect(context.getDirectories()).toContain(plansDir); + }); + + it('should NOT create plans directory or add it to workspace context when plan is disabled', async () => { + const config = new Config({ + ...baseParams, + plan: false, + }); + + await config.initialize(); + + const plansDir = config.storage.getProjectTempPlansDir(); + expect(fs.promises.mkdir).not.toHaveBeenCalledWith(plansDir, { + recursive: true, + }); + + const context = config.getWorkspaceContext(); + expect(context.getDirectories()).not.toContain(plansDir); + }); +}); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 2dd235becf..e65abff562 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as fs from 'node:fs'; import * as path from 'node:path'; import { inspect } from 'node:util'; import process from 'node:process'; @@ -696,6 +697,7 @@ export class Config { this.extensionManagement = params.extensionManagement ?? true; this.enableExtensionReloading = params.enableExtensionReloading ?? false; this.storage = new Storage(this.targetDir); + this.fakeResponses = params.fakeResponses; this.recordResponses = params.recordResponses; this.enablePromptCompletion = params.enablePromptCompletion ?? false; @@ -794,6 +796,13 @@ export class Config { this.workspaceContext.addDirectory(dir); } + // Add plans directory to workspace context for plan file storage + if (this.planEnabled) { + const plansDir = this.storage.getProjectTempPlansDir(); + await fs.promises.mkdir(plansDir, { recursive: true }); + this.workspaceContext.addDirectory(plansDir); + } + // Initialize centralized FileDiscoveryService const discoverToolsHandle = startupProfiler.start('discover_tools'); this.getFileService(); diff --git a/packages/core/src/config/storage.test.ts b/packages/core/src/config/storage.test.ts index 342ae3866e..b0b4fa8791 100644 --- a/packages/core/src/config/storage.test.ts +++ b/packages/core/src/config/storage.test.ts @@ -78,4 +78,10 @@ describe('Storage – additional helpers', () => { const expected = path.join(os.homedir(), GEMINI_DIR, 'tmp', 'bin'); expect(Storage.getGlobalBinDir()).toBe(expected); }); + + it('getProjectTempPlansDir returns ~/.gemini/tmp//plans', () => { + const tempDir = storage.getProjectTempDir(); + const expected = path.join(tempDir, 'plans'); + expect(storage.getProjectTempPlansDir()).toBe(expected); + }); }); diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index ac7efb8103..1f317d4ddf 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -155,6 +155,10 @@ export class Storage { return path.join(this.getProjectTempDir(), 'logs'); } + getProjectTempPlansDir(): string { + return path.join(this.getProjectTempDir(), 'plans'); + } + getExtensionsDir(): string { return path.join(this.getGeminiDir(), 'extensions'); } diff --git a/packages/core/src/core/__snapshots__/prompts.test.ts.snap b/packages/core/src/core/__snapshots__/prompts.test.ts.snap index 779c7bb48d..59a7f25d7f 100644 --- a/packages/core/src/core/__snapshots__/prompts.test.ts.snap +++ b/packages/core/src/core/__snapshots__/prompts.test.ts.snap @@ -182,6 +182,11 @@ You are operating in **Plan Mode** - a structured planning workflow for designin ## Available Tools The following read-only tools are available in Plan Mode: +- \`write_file\` - Save plans to the plans directory (see Plan Storage below) + +## Plan Storage +- Save your plans as Markdown (.md) files directly to: \`/tmp/project-temp/plans/\` +- Use descriptive filenames: \`feature-name.md\` or \`bugfix-description.md\` ## Workflow Phases @@ -201,7 +206,7 @@ The following read-only tools are available in Plan Mode: - Only begin this phase after exploration is complete - Create a detailed implementation plan with clear steps - Include file paths, function signatures, and code snippets where helpful -- Present the plan for review +- After saving the plan, present the full content of the markdown file to the user for review ### Phase 4: Review & Approval - Ask the user if they approve the plan, want revisions, or want to reject it diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index 149f46dc00..7805702986 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -65,6 +65,9 @@ describe('Core System Prompt (prompts.ts)', () => { getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true), storage: { getProjectTempDir: vi.fn().mockReturnValue('/tmp/project-temp'), + getProjectTempPlansDir: vi + .fn() + .mockReturnValue('/tmp/project-temp/plans'), }, isInteractive: vi.fn().mockReturnValue(true), isInteractiveShellEnabled: vi.fn().mockReturnValue(true), diff --git a/packages/core/src/core/prompts.ts b/packages/core/src/core/prompts.ts index fb5f14cf9b..83e346f368 100644 --- a/packages/core/src/core/prompts.ts +++ b/packages/core/src/core/prompts.ts @@ -146,6 +146,8 @@ export function getCoreSystemPrompt( .map((toolName) => `- \`${toolName}\``) .join('\n'); + const plansDir = config.storage.getProjectTempPlansDir(); + approvalModePrompt = ` # Active Approval Mode: Plan @@ -154,6 +156,11 @@ You are operating in **Plan Mode** - a structured planning workflow for designin ## Available Tools The following read-only tools are available in Plan Mode: ${planModeToolsList} +- \`${WRITE_FILE_TOOL_NAME}\` - Save plans to the plans directory (see Plan Storage below) + +## Plan Storage +- Save your plans as Markdown (.md) files directly to: \`${plansDir}/\` +- Use descriptive filenames: \`feature-name.md\` or \`bugfix-description.md\` ## Workflow Phases @@ -173,7 +180,7 @@ ${planModeToolsList} - Only begin this phase after exploration is complete - Create a detailed implementation plan with clear steps - Include file paths, function signatures, and code snippets where helpful -- Present the plan for review +- After saving the plan, present the full content of the markdown file to the user for review ### Phase 4: Review & Approval - Ask the user if they approve the plan, want revisions, or want to reject it diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index c69493e7e3..8487f34965 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -68,3 +68,11 @@ modes = ["plan"] toolName = "SubagentInvocation" decision = "allow" priority = 50 + +# Allow write_file for .md files in plans directory +[[rule]] +toolName = "write_file" +decision = "allow" +priority = 50 +modes = ["plan"] +argsPattern = "\"file_path\":\"[^\"]+/\\.gemini/tmp/[a-f0-9]{64}/plans/[a-zA-Z0-9_-]+\\.md\"" diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 965656e4f8..6bdbab64ed 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -47,6 +47,7 @@ import { } from '../test-utils/mock-message-bus.js'; const rootDir = path.resolve(os.tmpdir(), 'gemini-cli-test-root'); +const plansDir = path.resolve(os.tmpdir(), 'gemini-cli-test-plans'); // --- MOCKS --- vi.mock('../core/client.js'); @@ -84,7 +85,7 @@ const mockConfigInternal = { getBaseLlmClient: vi.fn(), // Initialize as a plain mock function getFileSystemService: () => fsService, getIdeMode: vi.fn(() => false), - getWorkspaceContext: () => new WorkspaceContext(rootDir), + getWorkspaceContext: () => new WorkspaceContext(rootDir, [plansDir]), getApiKey: () => 'test-key', getModel: () => 'test-model', getSandbox: () => false, @@ -126,10 +127,13 @@ describe('WriteFileTool', () => { tempDir = fs.mkdtempSync( path.join(os.tmpdir(), 'write-file-test-external-'), ); - // Ensure the rootDir for the tool exists + // Ensure the rootDir and plansDir for the tool exists if (!fs.existsSync(rootDir)) { fs.mkdirSync(rootDir, { recursive: true }); } + if (!fs.existsSync(plansDir)) { + fs.mkdirSync(plansDir, { recursive: true }); + } // Setup GeminiClient mock mockGeminiClientInstance = new (vi.mocked(GeminiClient))( @@ -206,6 +210,9 @@ describe('WriteFileTool', () => { if (fs.existsSync(rootDir)) { fs.rmSync(rootDir, { recursive: true, force: true }); } + if (fs.existsSync(plansDir)) { + fs.rmSync(plansDir, { recursive: true, force: true }); + } vi.clearAllMocks(); }); @@ -813,6 +820,24 @@ describe('WriteFileTool', () => { /File path must be within one of the workspace directories/, ); }); + + it('should allow paths within the plans directory', () => { + const params = { + file_path: path.join(plansDir, 'my-plan.md'), + content: '# My Plan', + }; + expect(() => tool.build(params)).not.toThrow(); + }); + + it('should reject paths that try to escape the plans directory', () => { + const params = { + file_path: path.join(plansDir, '..', 'escaped.txt'), + content: 'malicious', + }; + expect(() => tool.build(params)).toThrow( + /File path must be within one of the workspace directories/, + ); + }); }); describe('specific error types for write failures', () => {