From c57123cd815f2809618adb28af6b0c266b15d38a Mon Sep 17 00:00:00 2001 From: Mahima Shanware Date: Fri, 10 Apr 2026 18:24:46 +0000 Subject: [PATCH] fix(core): refactor exit-plan-mode to use resolveAndValidatePlanPath consistently This fixes unit test failures by centralizing path resolution logic. --- .../core/src/tools/exit-plan-mode.test.ts | 4 ++- packages/core/src/tools/exit-plan-mode.ts | 32 ++++++++++++------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/packages/core/src/tools/exit-plan-mode.test.ts b/packages/core/src/tools/exit-plan-mode.test.ts index 8f73162a51..d72f4242a3 100644 --- a/packages/core/src/tools/exit-plan-mode.test.ts +++ b/packages/core/src/tools/exit-plan-mode.test.ts @@ -73,8 +73,10 @@ describe('ExitPlanModeTool', () => { const createPlanFile = (name: string, content: string) => { const filePath = path.join(mockPlansDir, name); + // Ensure parent directory exists for nested tests + fs.mkdirSync(path.dirname(filePath), { recursive: true }); fs.writeFileSync(filePath, content); - return path.join('plans', name); + return name; }; describe('shouldConfirmExecute', () => { diff --git a/packages/core/src/tools/exit-plan-mode.ts b/packages/core/src/tools/exit-plan-mode.ts index 54ff8fb349..6975b36533 100644 --- a/packages/core/src/tools/exit-plan-mode.ts +++ b/packages/core/src/tools/exit-plan-mode.ts @@ -19,9 +19,13 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js'; import path from 'node:path'; import type { Config } from '../config/config.js'; import { EXIT_PLAN_MODE_TOOL_NAME } from './tool-names.js'; -import { validatePlanPath, validatePlanContent } from '../utils/planUtils.js'; +import { + validatePlanPath, + validatePlanContent, + resolveAndValidatePlanPath, +} from '../utils/planUtils.js'; import { ApprovalMode } from '../policy/types.js'; -import { resolveToRealPath, isSubpath } from '../utils/paths.js'; +// Remove unused imports import { logPlanExecution } from '../telemetry/loggers.js'; import { PlanExecutionEvent } from '../telemetry/types.js'; import { getExitPlanModeDefinition } from './definitions/coreTools.js'; @@ -59,16 +63,20 @@ export class ExitPlanModeTool extends BaseDeclarativeTool< if (!params.plan_filename || params.plan_filename.trim() === '') { return 'plan_filename is required.'; } - - const safeFilename = path.basename(params.plan_filename); - const plansDir = resolveToRealPath(this.config.getPlansDir()); - const resolvedPath = path.join(this.config.getPlansDir(), safeFilename); - - const realPath = resolveToRealPath(resolvedPath); - - if (!isSubpath(plansDir, realPath)) { - return `Access denied: plan path (${resolvedPath}) must be within the designated plans directory (${plansDir}).`; - } +try { + resolveAndValidatePlanPath( + params.plan_filename, + this.config.getPlansDir(), + ); +} catch (e) { + if (e instanceof Error && e.message.startsWith('Security violation')) { + return `Access denied: plan path (${path.join( + this.config.getPlansDir(), + params.plan_filename, + )}) must be within the designated plans directory (${this.config.getPlansDir()}).`; + } + return e instanceof Error ? e.message : String(e); +} return null; }