Skip to content

Commit f06d513

Browse files
committed
Hard-deny unresolvable paths in write-path allowlist compare
1 parent e355175 commit f06d513

4 files changed

Lines changed: 41 additions & 5 deletions

File tree

src/permission/path-restriction.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ function realpathOr(path: string): string {
4444
// simply missing. Contains a NUL byte, which can never appear in a real
4545
// filesystem path, so it can't collide with (or be mistaken for a prefix of)
4646
// any genuine result, and every containment compare against it fails.
47-
const UNRESOLVABLE = "\0unresolvable\0";
47+
export const UNRESOLVABLE = "\0unresolvable\0";
4848

4949
// A write/edit target usually doesn't exist yet, so realpath the nearest
5050
// existing ancestor and rejoin the missing tail rather than falling back to

src/permission/workspace-containment.test.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,14 +108,25 @@ test("a dangling symlink under cwd pointing outside denies a child path, and sta
108108
expect(restriction.isRestricted(target, false)).toBe(true);
109109
expect(restriction.isRestricted(target, true)).toBe(true);
110110

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).
111+
// Creating the outside target after the initial check must not retroactively
112+
// legitimize it: this is ordinary outside-symlink denial (the link still
113+
// ultimately points outside the workspace), re-checked once the target exists.
114114
await mkdir(outsideTarget, { recursive: true });
115115
await writeFile(join(outsideTarget, "child.txt"), "s");
116116
expect(resolveWorkspacePath(cwd, join("dangling-link", "child.txt"), rootsProvider)).toBeUndefined();
117117
});
118118

119+
test("a symlink loop under cwd is denied by resolveWorkspacePath (CL-6715)", async () => {
120+
const rootsProvider = () => [];
121+
const linkA = join(cwd, "loop-a");
122+
const linkB = join(cwd, "loop-b");
123+
await symlink(linkB, linkA);
124+
await symlink(linkA, linkB);
125+
126+
expect(resolveWorkspacePath(cwd, join("loop-a", "child.txt"), rootsProvider)).toBeUndefined();
127+
expect(resolveWorkspacePath(cwd, "loop-a", rootsProvider)).toBeUndefined();
128+
});
129+
119130
test("resolveWorkspacePath returns the canonical target so a later symlink retarget cannot redirect a write (CL-6712 TOCTOU)", async () => {
120131
// A symlink that is in-bounds at check time (target -> inside cwd) must
121132
// resolve to the canonical real path, not the lexical path through the

src/permission/write-path-policy.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,26 @@ describe("matchesWritePathAllowlist with a symlinked cwd", () => {
9393
});
9494
});
9595

96+
describe("matchesWritePathAllowlist with an unresolvable cwd", () => {
97+
test("a cwd whose final component is a dangling symlink is hard-denied, not spuriously allowed (CL-6715)", () => {
98+
// If cwd itself is unresolvable, both absCwd and abs collapse to the same
99+
// UNRESOLVABLE sentinel, `abs === absCwd` goes true, rel becomes ".", and
100+
// a root-matching pattern (e.g. "**") would otherwise spuriously allow —
101+
// turning a hard authz deny into an ask-prompt.
102+
const parent = mkdtempSync(join(realpathSync(tmpdir()), "write-path-dangling-parent-"));
103+
const danglingCwd = join(parent, "dangling-cwd");
104+
try {
105+
symlinkSync(join(parent, "does-not-exist"), danglingCwd);
106+
107+
expect(matchesWritePathAllowlist("anything.md", ["**"], danglingCwd)).toBe(false);
108+
expect(matchesWritePathAllowlist("PRODUCT.md", ["PRODUCT.md"], danglingCwd)).toBe(false);
109+
} finally {
110+
rmSync(danglingCwd, { force: true });
111+
rmSync(parent, { recursive: true, force: true });
112+
}
113+
});
114+
});
115+
96116
describe("writePathDeniedReason", () => {
97117
test("names allowlist and subject", () => {
98118
const reason = writePathDeniedReason("src/x.ts", ["PRODUCT.md", "docs/*"]);

src/permission/write-path-policy.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { resolve, sep } from "node:path";
22
import { matchesPattern } from "./matcher.js";
3-
import { realpathNearestOr } from "./path-restriction.js";
3+
import { realpathNearestOr, UNRESOLVABLE } from "./path-restriction.js";
44

55
/**
66
* Director write-path allowlist (authz, not prompt policy).
@@ -36,6 +36,11 @@ export function matchesWritePathAllowlist(
3636
// otherwise hard-deny a legitimate allowlisted write.
3737
const absCwd = realpathNearestOr(resolve(cwd));
3838
const abs = realpathNearestOr(resolve(cwd, subject));
39+
// Either side unresolvable (dangling symlink/loop component) must hard-deny.
40+
// Otherwise an unresolvable cwd and an unresolvable subject both collapse to
41+
// the same sentinel, `abs === absCwd` goes true, rel becomes ".", and a
42+
// root-matching allowlist pattern spuriously allows.
43+
if (absCwd === UNRESOLVABLE || abs === UNRESOLVABLE) return false;
3944
let rel: string;
4045
if (abs === absCwd) {
4146
rel = ".";

0 commit comments

Comments
 (0)