diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 841861a035..f13768a63a 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -3266,6 +3266,9 @@ describe('Plans Directory Initialization', () => { debugMode: false, model: 'test-model', cwd: '/tmp/test', + planSettings: { + directory: 'plans', + }, }; beforeEach(() => { @@ -3377,7 +3380,8 @@ describe('Plans Directory Initialization', () => { await config.initialize(); - const plansDir = config.storage.getPlansDir(); + // Even if getPlansDir is called manually, it should NOT create the directory + const plansDir = config.getPlansDir(); expect(fs.mkdirSync).not.toHaveBeenCalled(); expect(config.getWorkspaceContext().getDirectories()).not.toContain( plansDir, diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index b81c0555f8..fec78c9d81 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -2228,22 +2228,22 @@ export class Config implements McpContext, AgentLoopContext { getPlansDir(): string { const plansDir = this.storage.getPlansDir(this.getActiveExtensionPlanDir()); - if (this.initializedPlanDirs.has(plansDir)) { + if (!this.planEnabled || this.initializedPlanDirs.has(plansDir)) { return plansDir; } try { fs.mkdirSync(plansDir, { recursive: true }); - let realPlansDir = plansDir; - try { - const resolved = resolveToRealPath(plansDir); - if (resolved) { - realPlansDir = resolved; - } - } catch { - // Ignore failures in mock environments + const realPlansDir = resolveToRealPath(plansDir); + const realProjectRoot = resolveToRealPath(this.getTargetDir()); + + if (!isSubpath(realProjectRoot, realPlansDir)) { + throw new Error( + `Security violation: Resolved plan directory '${realPlansDir}' is outside the project root '${realProjectRoot}'.`, + ); } + this.workspaceContext.addDirectory(realPlansDir); this.initializedPlanDirs.add(plansDir); } catch (e: unknown) { diff --git a/packages/core/src/config/storage.test.ts b/packages/core/src/config/storage.test.ts index e695ae5d9b..50cfbe1a65 100644 --- a/packages/core/src/config/storage.test.ts +++ b/packages/core/src/config/storage.test.ts @@ -378,6 +378,28 @@ describe('Storage – additional helpers', () => { }, expected: path.resolve(projectRoot, 'new-plans'), }, + { + name: 'security escape via symbolic link with non-existent dir throws', + customDir: 'link-to-outside/new-dir', + setup: () => { + vi.mocked(fs.realpathSync).mockImplementation((p: fs.PathLike) => { + const pStr = p.toString(); + if (pStr.includes('link-to-outside/new-dir')) { + const err = new Error('ENOENT') as NodeJS.ErrnoException; + err.code = 'ENOENT'; + throw err; + } + if (pStr.includes('link-to-outside')) { + return '/outside/project/root'; + } + return pStr; + }); + return () => vi.mocked(fs.realpathSync).mockRestore(); + }, + expected: '', + expectedError: + "Custom plans directory 'link-to-outside/new-dir' resolves to '/outside/project/root/new-dir', which is outside the project root '/tmp/project'.", + }, ]; testCases.forEach(({ name, customDir, expected, expectedError, setup }) => { diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index 63d3921c58..804bea0ea7 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -318,24 +318,7 @@ export class Storage { if (customDir) { const resolvedPath = path.resolve(this.getProjectRoot(), customDir); const realProjectRoot = resolveToRealPath(this.getProjectRoot()); - let realResolvedPath = resolvedPath; - - try { - realResolvedPath = resolveToRealPath(resolvedPath); - } catch (e: unknown) { - if ( - !( - e && - typeof e === 'object' && - 'code' in e && - (e.code === 'ENOENT' || e.code === 'EISDIR') - ) - ) { - throw e; - } - // Construct the fallback path safely against the real project root - realResolvedPath = path.resolve(realProjectRoot, customDir); - } + const realResolvedPath = resolveToRealPath(resolvedPath); if (!isSubpath(realProjectRoot, realResolvedPath)) { throw new Error(