diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index cbf151bfc7..c00ea4c0da 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -28,7 +28,6 @@ import { buildFilePathArgsPattern } from '../policy/utils.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { ToolErrorType } from './tool-error.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; -import { resolvePlanPath } from '../utils/planUtils.js'; import { isNodeError } from '../utils/errors.js'; import { correctPath } from '../utils/pathCorrector.js'; import type { Config } from '../config/config.js'; diff --git a/packages/core/src/tools/exit-plan-mode.ts b/packages/core/src/tools/exit-plan-mode.ts index 5318e56889..86d3cccd6f 100644 --- a/packages/core/src/tools/exit-plan-mode.ts +++ b/packages/core/src/tools/exit-plan-mode.ts @@ -226,28 +226,19 @@ export class ExitPlanModeInvocation extends BaseToolInvocation< const exitMessage = getPlanModeExitMessage(newMode); return { - llmContent: `${exitMessage} - -The approved implementation plan is stored at: ${resolvedPlanPath} -Read and follow the plan strictly during implementation.`, + llmContent: `${exitMessage}\n\nThe approved implementation plan is stored at: ${resolvedPlanPath}\nRead and follow the plan strictly during implementation.`, returnDisplay: `Plan approved: ${resolvedPlanPath}`, }; } else { const feedback = payload?.feedback?.trim(); if (feedback) { return { - llmContent: `Plan rejected. User feedback: ${feedback} - -The plan is stored at: ${resolvedPlanPath} -Revise the plan based on the feedback.`, + llmContent: `Plan rejected. User feedback: ${feedback}\n\nThe plan is stored at: ${resolvedPlanPath}\nRevise the plan based on the feedback.`, returnDisplay: `Feedback: ${feedback}`, }; } else { return { - llmContent: `Plan rejected. No feedback provided. - -The plan is stored at: ${resolvedPlanPath} -Ask the user for specific feedback on how to improve the plan.`, + llmContent: `Plan rejected. No feedback provided.\n\nThe plan is stored at: ${resolvedPlanPath}\nAsk the user for specific feedback on how to improve the plan.`, returnDisplay: 'Rejected (no feedback)', }; } diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index e276f1f792..0e889ddd90 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -1176,6 +1176,10 @@ describe('WriteFileTool', () => { const planFilePath = `${plansDirName}/tracks/fibsqrt_20260519/spec.md`; const params = { file_path: planFilePath, content: '# Spec' }; const invocation = tool.build(params); + + expect( + (invocation as unknown as { resolvedPath: string }).resolvedPath, + ).toBe(path.resolve(plansDir, 'tracks/fibsqrt_20260519/spec.md')); }); }); }); diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 080b3f1bf8..9ec19b879f 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -29,7 +29,6 @@ import { import { buildFilePathArgsPattern } from '../policy/utils.js'; import { ToolErrorType } from './tool-error.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; -import { resolvePlanPath } from '../utils/planUtils.js'; import { getErrorMessage, isNodeError } from '../utils/errors.js'; import { ensureCorrectFileContent } from '../utils/editCorrector.js'; import { detectLineEnding } from '../utils/textUtils.js'; diff --git a/packages/core/src/utils/planUtils.test.ts b/packages/core/src/utils/planUtils.test.ts index 5cf6ce0bc8..56719c6fd3 100644 --- a/packages/core/src/utils/planUtils.test.ts +++ b/packages/core/src/utils/planUtils.test.ts @@ -11,7 +11,7 @@ import os from 'node:os'; import { validatePlanPath, validatePlanContent, - resolvePlanPath, + resolveAndValidatePlanPath, } from './planUtils.js'; describe('planUtils', () => { @@ -80,9 +80,9 @@ describe('planUtils', () => { }); }); - describe('resolvePlanPath', () => { + describe('resolveAndValidatePlanPath', () => { it('should resolve simple filenames relative to plansDir', () => { - const result = resolvePlanPath( + const result = resolveAndValidatePlanPath( 'implementation_plan.md', plansDir, tempRootDir, @@ -97,7 +97,7 @@ describe('planUtils', () => { 'fibsqrt_20260519', 'spec.md', ); - const result = resolvePlanPath(planPath, plansDir, tempRootDir); + const result = resolveAndValidatePlanPath(planPath, plansDir, tempRootDir); expect(result).toBe( path.join(plansDir, 'tracks', 'fibsqrt_20260519', 'spec.md'), ); @@ -105,16 +105,17 @@ describe('planUtils', () => { it('should resolve paths relative to plansDir if they contain subdirectories', () => { const planPath = path.join('tracks', 'fibsqrt_20260519', 'spec.md'); - const result = resolvePlanPath(planPath, plansDir, tempRootDir); + const result = resolveAndValidatePlanPath(planPath, plansDir, tempRootDir); expect(result).toBe( path.join(plansDir, 'tracks', 'fibsqrt_20260519', 'spec.md'), ); }); - it('should fallback to safe basename when escaping', () => { + it('should throw access denied when escaping', () => { const planPath = '../../escaped.md'; - const result = resolvePlanPath(planPath, plansDir, tempRootDir); - expect(result).toBe(path.join(plansDir, 'escaped.md')); + expect(() => + resolveAndValidatePlanPath(planPath, plansDir, tempRootDir), + ).toThrow(/Access denied/); }); }); diff --git a/packages/core/src/utils/planUtils.ts b/packages/core/src/utils/planUtils.ts index a1a869edd0..39750ea954 100644 --- a/packages/core/src/utils/planUtils.ts +++ b/packages/core/src/utils/planUtils.ts @@ -22,6 +22,14 @@ export const PlanErrorMessages = { READ_FAILURE: (detail: string) => `Failed to read plan file: ${detail}`, } as const; +/** + * Resolves a plan file path and strictly validates it against the plans directory boundary. + * Useful for tools that need to write or read plans. + * @param planPath The untrusted file path provided by the model. + * @param plansDir The authorized project plans directory. + * @returns The safely resolved path string. + * @throws Error if the path is empty, malicious, or escapes boundaries. + */ export function resolveAndValidatePlanPath( planPath: string, plansDir: string,