Skip to content

Commit ecf1242

Browse files
committed
fix(permission): drop the cross-call realpath cache
A symlink target can move without changing the link text or the leaf inode, and stamping only lexical ancestors misses the directory behind the link. Resolved paths are no longer reused across tool calls. lstat results live only for the current synchronous check.
1 parent 91506da commit ecf1242

6 files changed

Lines changed: 69 additions & 299 deletions

File tree

‎src/permission/path-restriction.ts‎

Lines changed: 10 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,7 @@ import { dirname, join, resolve, sep } from "node:path";
33
import { homedir } from "node:os";
44
import { realpathOrOriginal, type RootsProvider } from "./worktree-roots.js";
55
import { projectSessionsRoot } from "../session/project-key.js";
6-
import {
7-
cachedLinkReal,
8-
cachedNodeReal,
9-
enterPathResolution,
10-
lstatMemoized,
11-
readlinkMemoized,
12-
rememberLink,
13-
rememberNode,
14-
} from "./realpath-cache.js";
6+
import { enterPathResolution, lstatMemoized } from "./realpath-cache.js";
157

168
// Paths the agent must not touch without explicit operator approval, even
179
// though read tools are otherwise allow-tier and write/edit auto-allow in
@@ -41,7 +33,7 @@ const LEGACY_STATE_DIR = ".agent-state";
4133
export const UNRESOLVABLE = "\0unresolvable\0";
4234

4335
// A single normal path segment. `.`, `..`, a trailing slash, or an empty tail
44-
// stay on realpathSync so the cached join cannot diverge from the kernel.
36+
// stay on realpathSync so the join cannot diverge from the kernel.
4537
function existingTail(path: string, parent: string): string | undefined {
4638
const tailStart = parent.endsWith(sep) ? parent.length : parent.length + 1;
4739
const tail = path.slice(tailStart);
@@ -56,14 +48,9 @@ function existingTail(path: string, parent: string): string | undefined {
5648
return tail;
5749
}
5850

59-
function rememberRealpath(
60-
path: string,
61-
st: NonNullable<ReturnType<typeof lstatMemoized>>,
62-
): string {
51+
function realpathOrUnresolvable(path: string): string {
6352
try {
64-
const real = realpathSync(path);
65-
rememberNode(path, st, real);
66-
return real;
53+
return realpathSync(path);
6754
} catch {
6855
return UNRESOLVABLE;
6956
}
@@ -72,32 +59,17 @@ function rememberRealpath(
7259
function resolveNearest(path: string): string {
7360
const st = lstatMemoized(path);
7461
if (st !== undefined && !st.isSymbolicLink()) {
75-
const hit = cachedNodeReal(path, st);
76-
if (hit !== undefined) return hit;
7762
const parent = dirname(path);
78-
if (parent === path) return rememberRealpath(path, st);
63+
if (parent === path) return realpathOrUnresolvable(path);
7964
const tail = existingTail(path, parent);
80-
if (tail === undefined) return rememberRealpath(path, st);
65+
if (tail === undefined) return realpathOrUnresolvable(path);
8166
const parentReal = resolveNearest(parent);
8267
if (parentReal === UNRESOLVABLE) return UNRESOLVABLE;
83-
const real = join(parentReal, tail);
84-
rememberNode(path, st, real);
85-
return real;
68+
return join(parentReal, tail);
8669
}
8770

8871
if (st !== undefined && st.isSymbolicLink()) {
89-
const linkTarget = readlinkMemoized(path);
90-
if (linkTarget !== undefined) {
91-
const hit = cachedLinkReal(path, st, linkTarget);
92-
if (hit !== undefined) return hit;
93-
}
94-
try {
95-
const real = realpathSync(path);
96-
if (linkTarget !== undefined) rememberLink(path, st, linkTarget, real);
97-
return real;
98-
} catch {
99-
return UNRESOLVABLE;
100-
}
72+
return realpathOrUnresolvable(path);
10173
}
10274

10375
const parent = dirname(path);
@@ -122,9 +94,8 @@ function resolveNearest(path: string): string {
12294
// distinguishes the two: it succeeds for an existing-but-broken symlink and
12395
// fails only when the component is genuinely absent.
12496
//
125-
// Already-resolved ancestors are memoized across calls only while each
126-
// ancestor keeps the dev/ino (and symlink text) stored with the entry. The
127-
// check stays synchronous.
97+
// lstat results are shared only for the duration of this synchronous check.
98+
// A resolved path is not reused on a later tool call.
12899
export function realpathNearestOr(path: string): string {
129100
return enterPathResolution(() => resolveNearest(path));
130101
}

‎src/permission/realpath-cache.test.ts‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ afterEach(async () => {
2424
}
2525
});
2626

27-
test("an unchanged path skips realpath but rechecks every ancestor", async () => {
27+
test("a later check of the same path lstats and realpaths again", async () => {
2828
scratch = await mkdtemp(join(tmpdir(), "corbits-realpath-once-"));
2929
const ancestorA = join(scratch, "a");
3030
const ancestorB = join(ancestorA, "b");
@@ -54,8 +54,9 @@ test("an unchanged path skips realpath but rechecks every ancestor", async () =>
5454
expect(realpathNearestOr(file)).toBe(canonical);
5555
const lstatAfterFirst = new Map(lstatCounts);
5656
const realpathAfterFirst = realpathCalls;
57+
expect(realpathAfterFirst).toBeGreaterThan(0);
5758
expect(realpathNearestOr(file)).toBe(canonical);
58-
expect(realpathCalls).toBe(realpathAfterFirst);
59+
expect(realpathCalls).toBeGreaterThan(realpathAfterFirst);
5960
for (const ancestor of [scratch, ancestorA, ancestorB]) {
6061
expect(lstatAfterFirst.get(ancestor)).toBe(1);
6162
expect(lstatCounts.get(ancestor)).toBe(2);
@@ -170,3 +171,39 @@ test("renaming an ancestor and replacing it with a symlink does not keep the old
170171
}),
171172
).toBe(outsideDir);
172173
});
174+
175+
test("a symlink target tree moved out from behind an unchanged link is not served as the old real path", async () => {
176+
// jail/link → inner/sub. Lexical stamps of the link do not include jail/inner.
177+
// Moving that directory and symlinking it back keeps the link text and the
178+
// leaf inode, so a cached link real path must not be reused or joined onto
179+
// a child that was never resolved.
180+
scratch = await mkdtemp(join(tmpdir(), "corbits-realpath-behind-"));
181+
const jail = join(scratch, "jail");
182+
const outside = join(scratch, "outside");
183+
const inner = join(jail, "inner");
184+
const sub = join(inner, "sub");
185+
const file = join(sub, "file.txt");
186+
await mkdir(sub, { recursive: true });
187+
await mkdir(outside);
188+
await writeFile(file, "x");
189+
const link = join(jail, "link");
190+
await symlink(join("inner", "sub"), link);
191+
192+
expect(realpathNearestOr(link)).toBe(realpathSync(sub));
193+
194+
const moved = join(outside, "inner");
195+
await rename(inner, moved);
196+
await symlink(moved, inner);
197+
198+
const outsideSub = realpathSync(link);
199+
const outsideFile = realpathSync(join(link, "file.txt"));
200+
expect(outsideSub).toBe(realpathSync(join(moved, "sub")));
201+
expect(realpathNearestOr(link)).toBe(outsideSub);
202+
expect(realpathNearestOr(join(link, "file.txt"))).toBe(outsideFile);
203+
expect(() => resolvePerCallShellCwd(jail, "link")).toThrow(
204+
/outside the session workspace/,
205+
);
206+
expect(
207+
resolvePerCallShellCwd(jail, "link", { allowOutsideSession: true }),
208+
).toBe(outsideSub);
209+
});

0 commit comments

Comments
 (0)