Skip to content

Commit e355175

Browse files
committed
Deny dangling symlink components in nearest-realpath walks
realpathNearestOr treated a realpath failure as an always-missing tail and rejoined the raw component name, so a dangling symlink under cwd whose target didn't exist yet was reattached as a normal path segment and read as in-bounds. Now lstat distinguishes "doesn't exist at all" (safe, missing tail) from "exists but unresolvable" (dangling link or symlink loop), and the latter denies via resolveWorkspacePath/underRoot.
1 parent 026a0f6 commit e355175

2 files changed

Lines changed: 52 additions & 2 deletions

File tree

‎src/permission/path-restriction.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { realpathSync } from "node:fs";
1+
import { lstatSync, realpathSync } from "node:fs";
22
import { dirname, join, resolve, sep } from "node:path";
33
import { homedir } from "node:os";
44
import type { RootsProvider } from "./worktree-roots.js";
@@ -39,22 +39,46 @@ function realpathOr(path: string): string {
3939
}
4040
}
4141

42+
// Sentinel returned by realpathNearestOr for a path that exists but couldn't
43+
// be resolved (a dangling symlink, or a symlink loop) rather than one that's
44+
// simply missing. Contains a NUL byte, which can never appear in a real
45+
// filesystem path, so it can't collide with (or be mistaken for a prefix of)
46+
// any genuine result, and every containment compare against it fails.
47+
const UNRESOLVABLE = "\0unresolvable\0";
48+
4249
// A write/edit target usually doesn't exist yet, so realpath the nearest
4350
// existing ancestor and rejoin the missing tail rather than falling back to
4451
// the raw (possibly symlink-relative) path, which would defeat containment
4552
// checks whenever the workspace root itself is reached through a symlink
4653
// (e.g. macOS's /tmp -> /private/tmp).
54+
//
55+
// realpath failure is ambiguous: it's either "this component doesn't exist
56+
// yet" (safe — the missing tail gets rejoined onto the nearest real ancestor)
57+
// or "this component exists but is a dangling symlink / symlink loop" (unsafe
58+
// — the link's name must not stand in for a normal under-cwd path segment,
59+
// since the walk otherwise reattaches it verbatim and containment sees a
60+
// plain child path with no idea a symlink is involved). lstat distinguishes
61+
// the two: it succeeds for an existing-but-broken symlink and fails only when
62+
// the component is genuinely absent.
4763
export function realpathNearestOr(path: string): string {
4864
try {
4965
return realpathSync(path);
5066
} catch {
67+
try {
68+
lstatSync(path);
69+
return UNRESOLVABLE;
70+
} catch {
71+
// Doesn't exist at all — fall through to the nearest-ancestor walk.
72+
}
5173
const parent = dirname(path);
5274
if (parent === path) return path;
5375
// Root (e.g. "/") already ends in the separator, so slicing past
5476
// parent.length alone lands on the tail; anywhere else the separator
5577
// between parent and tail must be skipped too.
5678
const tailStart = parent.endsWith(sep) ? parent.length : parent.length + 1;
57-
return join(realpathNearestOr(parent), path.slice(tailStart));
79+
const parentReal = realpathNearestOr(parent);
80+
if (parentReal === UNRESOLVABLE) return UNRESOLVABLE;
81+
return join(parentReal, path.slice(tailStart));
5882
}
5983
}
6084

@@ -93,6 +117,7 @@ export function resolveWorkspacePath(
93117
const abs = resolve(cwd, path);
94118
const realCwd = realpathOr(resolve(cwd));
95119
const real = realpathNearestOr(abs);
120+
if (real === UNRESOLVABLE) return undefined;
96121
if (real === realCwd || real.startsWith(realCwd + sep)) return real;
97122
if (inKnownRoots(real, rootsProvider())) return real;
98123
if (inKnownRoots(real, rootsProvider(true))) return real;
@@ -139,6 +164,7 @@ function underRoot(abs: string, root: string): boolean {
139164
// unresolved while the abs path is rebuilt through an existing ancestor).
140165
const realRoot = realpathNearestOr(root);
141166
const realAbs = realpathNearestOr(abs);
167+
if (realRoot === UNRESOLVABLE || realAbs === UNRESOLVABLE) return false;
142168
return realAbs === realRoot || realAbs.startsWith(realRoot + sep);
143169
}
144170

‎src/permission/workspace-containment.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,30 @@ test("a symlink pointing outside the workspace is refused, even for a not-yet-ex
9292
expect(restricted).toBe(true);
9393
});
9494

95+
test("a dangling symlink under cwd pointing outside denies a child path, and stays denied after the outside target is created (CL-6715)", async () => {
96+
// The symlink target does not exist yet, so realpath fails on the link
97+
// itself (not just on the not-yet-existing child) — the walk must not
98+
// treat the dangling link's name as an ordinary missing tail segment.
99+
const rootsProvider = () => [];
100+
const link = join(cwd, "dangling-link");
101+
const outsideTarget = join(outside, "not-created-yet");
102+
await symlink(outsideTarget, link);
103+
const target = join(link, "child.txt");
104+
105+
expect(resolveWorkspacePath(cwd, join("dangling-link", "child.txt"), rootsProvider)).toBeUndefined();
106+
107+
const restriction = createPathRestriction(cwd, rootsProvider, home);
108+
expect(restriction.isRestricted(target, false)).toBe(true);
109+
expect(restriction.isRestricted(target, true)).toBe(true);
110+
111+
// A race that creates the outside target between check and open must not
112+
// retroactively legitimize the earlier check, and a fresh check afterward
113+
// must still deny (the link still ultimately points outside the workspace).
114+
await mkdir(outsideTarget, { recursive: true });
115+
await writeFile(join(outsideTarget, "child.txt"), "s");
116+
expect(resolveWorkspacePath(cwd, join("dangling-link", "child.txt"), rootsProvider)).toBeUndefined();
117+
});
118+
95119
test("resolveWorkspacePath returns the canonical target so a later symlink retarget cannot redirect a write (CL-6712 TOCTOU)", async () => {
96120
// A symlink that is in-bounds at check time (target -> inside cwd) must
97121
// resolve to the canonical real path, not the lexical path through the

0 commit comments

Comments
 (0)