From 3256b16039f3ffdec34e5c97761cc4f5d31f96a7 Mon Sep 17 00:00:00 2001 From: Mahima Shanware Date: Mon, 6 Apr 2026 21:53:10 +0000 Subject: [PATCH] fix(core): mitigate TOCTOU vulnerability in plan directory creation This change addresses a critical security review finding regarding a Time-of-Check to Time-of-Use (TOCTOU) vulnerability. Previously, plan directory paths were validated using `isSubpath` before creation. However, an attacker could potentially replace a path component with a symlink pointing outside the project root exactly between validation and creation. By resolving the physical path *after* `fs.mkdirSync` using `resolveToRealPath` and then verifying it with `isSubpath`, we ensure that the actual directory created on disk resides safely within the workspace. Any violation results in a warning, and the malicious path is prevented from being added to the agent's `workspaceContext`. --- packages/core/src/config/config.test.ts | 40 +++++++++++++++++++++++++ packages/core/src/config/config.ts | 8 +++++ 2 files changed, 48 insertions(+) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 4505b0bd2e..ba3a936e7e 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -3367,6 +3367,46 @@ describe('Plans Directory Initialization', () => { warnSpy.mockRestore(); }); + it('should log a warning if the resolved plan directory is outside the project root (TOCTOU security violation)', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + let isMalicious = false; + vi.spyOn(fs, 'mkdirSync').mockImplementation(() => { + // Simulate an attacker creating a symlink right after the initial check but before/during creation + isMalicious = true; + return undefined; + }); + + // When Config.getPlansDir calls resolveToRealPath AFTER creation, it resolves to an outside path. + const realpathSpy = vi + .spyOn(fs, 'realpathSync') + .mockImplementation((p: fs.PathLike) => { + const pStr = p.toString(); + if (isMalicious && pStr.includes('plans')) + return '/outside/the/project/root/plans'; + return pStr; + }); + + const config = new Config({ + ...baseParams, + plan: true, + }); + + await config.initialize(); + try { + config.getPlansDir(); + } finally { + realpathSpy.mockRestore(); + } + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching( + /Security violation: Resolved plan directory.*is outside the project root/, + ), + ); + warnSpy.mockRestore(); + }); + it('should log a warning if mkdirSync fails during getPlansDir (e.g. EACCES)', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); vi.spyOn(fs, 'mkdirSync').mockImplementation(() => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index b67d9588b3..684f9a93ef 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -2269,6 +2269,14 @@ export class Config implements McpContext, AgentLoopContext { fs.mkdirSync(plansDir, { recursive: true }); 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.set(plansDir, true); } catch (e: unknown) {