diff --git a/src/permission/path-restriction.test.ts b/src/permission/path-restriction.test.ts index 743221b69..9e85bb123 100644 --- a/src/permission/path-restriction.test.ts +++ b/src/permission/path-restriction.test.ts @@ -1,10 +1,11 @@ import { afterEach, beforeEach, expect, test } from "bun:test"; -import { mkdir, rm } from "node:fs/promises"; +import { mkdir, rm, symlink } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { createPathRestriction } from "./path-restriction.js"; import { projectSessionsRoot } from "../session/project-key.js"; +import { withMockedModuleDuring } from "../../tests/helpers/mock-module.js"; let cwd = ""; let home = ""; @@ -75,3 +76,61 @@ test("a directory sharing a string prefix with the workspace root is still restr expect(r.isRestricted(prefixSibling, false)).toBe(true); expect(r.isRestricted(join(prefixSibling, "file.txt"), false)).toBe(true); }); + +test("a cache entry cannot combine an outside realpath with an inside verdict", async () => { + const insideDir = join(cwd, "inside-race"); + const outsideDir = join(home, "outside-race"); + await mkdir(insideDir, { recursive: true }); + await mkdir(outsideDir, { recursive: true }); + const link = join(cwd, "race-link"); + await symlink(outsideDir, link); + const target = join(link, "file.txt"); + + const r = createPathRestriction(cwd, () => [], home); + let retargeted = false; + await withMockedModuleDuring( + "node:fs", + (realFS: typeof import("node:fs")) => ({ + ...realFS, + realpathSync: (path: Parameters[0]) => { + const realpath = realFS.realpathSync(path); + if (!retargeted && path === link) { + realFS.unlinkSync(link); + realFS.symlinkSync(insideDir, link); + retargeted = true; + } + return realpath; + }, + }), + async () => { + expect(r.isRestricted(target, false)).toBe(true); + }, + ); + + await rm(link); + await symlink(outsideDir, link); + expect(r.isRestricted(target, false)).toBe(true); +}); + +test("symlink retarget invalidates cache: inside-allowed → outside-restricted (CL-6708)", async () => { + // Create a symlink pointing inside the workspace + const insideDir = join(cwd, "inside"); + await mkdir(insideDir, { recursive: true }); + const link = join(cwd, "link"); + await symlink(insideDir, link); + + const r = createPathRestriction(cwd, () => [], home); + const target = join(link, "file.txt"); + + // First check: symlink points inside, so path is unrestricted + expect(r.isRestricted(target, false)).toBe(false); + + // Retarget symlink to point outside the workspace + await rm(link); + const outsideDir = join(home, "outside"); + await mkdir(outsideDir, { recursive: true }); + await symlink(outsideDir, link); + + // Second check: same lexical path, but now restricted due to retarget + expect(r.isRestricted(target, false)).toBe(true); +}); diff --git a/src/permission/path-restriction.ts b/src/permission/path-restriction.ts index 06bd1206e..925a19962 100644 --- a/src/permission/path-restriction.ts +++ b/src/permission/path-restriction.ts @@ -115,13 +115,21 @@ export function resolveWorkspacePath( rootsProvider: RootsProvider = () => [], ): string | undefined { const abs = resolve(cwd, path); - const realCwd = realpathOr(resolve(cwd)); const real = realpathNearestOr(abs); - if (real === UNRESOLVABLE) return undefined; - if (real === realCwd || real.startsWith(realCwd + sep)) return real; - if (inKnownRoots(real, rootsProvider())) return real; - if (inKnownRoots(real, rootsProvider(true))) return real; - return undefined; + return isResolvedPathInWorkspace(cwd, real, rootsProvider) ? real : undefined; +} + +function isResolvedPathInWorkspace( + cwd: string, + real: string, + rootsProvider: RootsProvider, +): boolean { + if (real === UNRESOLVABLE) return false; + const realCwd = realpathOr(resolve(cwd)); + if (real === realCwd || real.startsWith(realCwd + sep)) return true; + if (inKnownRoots(real, rootsProvider())) return true; + if (inKnownRoots(real, rootsProvider(true))) return true; + return false; } // Whether `path` (relative to `cwd`) names a not-yet-created sibling worktree @@ -158,14 +166,12 @@ export function isPermittedSiblingWorktreePath( return trustedParents.has(realParent); } -function underRoot(abs: string, root: string): boolean { - // realpathNearestOr on both sides so a not-yet-created state root still - // compares equal to paths under it (realpathOr alone leaves the root - // unresolved while the abs path is rebuilt through an existing ancestor). +function underResolvedRoot(real: string, root: string): boolean { + // Resolve a not-yet-created state root through its nearest existing ancestor + // so it can still compare equal to paths under it. const realRoot = realpathNearestOr(root); - const realAbs = realpathNearestOr(abs); - if (realRoot === UNRESOLVABLE || realAbs === UNRESOLVABLE) return false; - return realAbs === realRoot || realAbs.startsWith(realRoot + sep); + if (realRoot === UNRESOLVABLE || real === UNRESOLVABLE) return false; + return real === realRoot || real.startsWith(realRoot + sep); } // `rootsProvider` supplies the additional workspace roots (the session's @@ -184,27 +190,40 @@ export function createPathRestriction( ): PathRestriction { const legacyStateDir = resolve(cwd, LEGACY_STATE_DIR); const globalStateDir = projectSessionsRoot(cwd, home); - const cache = new Map(); + // Cache keyed by both absolute path and realpath to invalidate when symlinks + // change. If only keyed by absolute path, a cached "unrestricted" verdict + // persists after a symlink retargets outside the workspace. + const cache = new Map(); - const underStateDir = (abs: string): boolean => - underRoot(abs, legacyStateDir) || underRoot(abs, globalStateDir); + const underStateDir = (real: string): boolean => + underResolvedRoot(real, legacyStateDir) || underResolvedRoot(real, globalStateDir); return { isRestricted: (path: string, isWrite: boolean): boolean => { const abs = resolve(cwd, path); const cacheKey = `${isWrite ? "w" : "r"}:${abs}`; + // Use realpathNearestOr rather than realpathOr: the target file may not + // exist yet, in which case realpathOr returns the raw path unchanged — + // making the cache key identical before and after a symlink retarget. + // realpathNearestOr resolves up to the nearest existing ancestor, which + // does change when a symlink flips, invalidating the stale verdict. + const currentRealpath = realpathNearestOr(abs); const cached = cache.get(cacheKey); - if (cached !== undefined) return cached; + + // Cache hit only if realpath hasn't changed (symlink not retargeted) + if (cached !== undefined && cached.realpath === currentRealpath) { + return cached.verdict; + } // State root: read allow, write ask — even when the root lives outside // the workspace (global ~/.corbits/projects/...). - if (underStateDir(abs)) { - cache.set(cacheKey, isWrite); + if (underStateDir(currentRealpath)) { + cache.set(cacheKey, { realpath: currentRealpath, verdict: isWrite }); return isWrite; } - const outsideWorkspace = resolveWorkspacePath(cwd, path, rootsProvider) === undefined; - cache.set(cacheKey, outsideWorkspace); + const outsideWorkspace = !isResolvedPathInWorkspace(cwd, currentRealpath, rootsProvider); + cache.set(cacheKey, { realpath: currentRealpath, verdict: outsideWorkspace }); return outsideWorkspace; }, };