Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions src/permission/path-restriction.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { realpathSync } from "node:fs";
import { lstatSync, realpathSync } from "node:fs";
import { dirname, join, resolve, sep } from "node:path";
import { homedir } from "node:os";
import type { RootsProvider } from "./worktree-roots.js";
Expand Down Expand Up @@ -39,22 +39,46 @@ function realpathOr(path: string): string {
}
}

// Sentinel returned by realpathNearestOr for a path that exists but couldn't
// be resolved (a dangling symlink, or a symlink loop) rather than one that's
// simply missing. Contains a NUL byte, which can never appear in a real
// filesystem path, so it can't collide with (or be mistaken for a prefix of)
// any genuine result, and every containment compare against it fails.
export const UNRESOLVABLE = "\0unresolvable\0";

// A write/edit target usually doesn't exist yet, so realpath the nearest
// existing ancestor and rejoin the missing tail rather than falling back to
// the raw (possibly symlink-relative) path, which would defeat containment
// checks whenever the workspace root itself is reached through a symlink
// (e.g. macOS's /tmp -> /private/tmp).
//
// realpath failure is ambiguous: it's either "this component doesn't exist
// yet" (safe — the missing tail gets rejoined onto the nearest real ancestor)
// or "this component exists but is a dangling symlink / symlink loop" (unsafe
// — the link's name must not stand in for a normal under-cwd path segment,
// since the walk otherwise reattaches it verbatim and containment sees a
// plain child path with no idea a symlink is involved). lstat distinguishes
// the two: it succeeds for an existing-but-broken symlink and fails only when
// the component is genuinely absent.
export function realpathNearestOr(path: string): string {
try {
return realpathSync(path);
} catch {
try {
lstatSync(path);
return UNRESOLVABLE;
} catch {
// Doesn't exist at all — fall through to the nearest-ancestor walk.
}
const parent = dirname(path);
if (parent === path) return path;
// Root (e.g. "/") already ends in the separator, so slicing past
// parent.length alone lands on the tail; anywhere else the separator
// between parent and tail must be skipped too.
const tailStart = parent.endsWith(sep) ? parent.length : parent.length + 1;
return join(realpathNearestOr(parent), path.slice(tailStart));
const parentReal = realpathNearestOr(parent);
if (parentReal === UNRESOLVABLE) return UNRESOLVABLE;
return join(parentReal, path.slice(tailStart));
}
}

Expand Down Expand Up @@ -93,6 +117,7 @@ export function resolveWorkspacePath(
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;
Expand Down Expand Up @@ -139,6 +164,7 @@ function underRoot(abs: string, root: string): boolean {
// unresolved while the abs path is rebuilt through an existing ancestor).
const realRoot = realpathNearestOr(root);
const realAbs = realpathNearestOr(abs);
if (realRoot === UNRESOLVABLE || realAbs === UNRESOLVABLE) return false;
return realAbs === realRoot || realAbs.startsWith(realRoot + sep);
}

Expand Down
35 changes: 35 additions & 0 deletions src/permission/workspace-containment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,41 @@ test("a symlink pointing outside the workspace is refused, even for a not-yet-ex
expect(restricted).toBe(true);
});

test("a dangling symlink under cwd pointing outside denies a child path, and stays denied after the outside target is created (CL-6715)", async () => {
// The symlink target does not exist yet, so realpath fails on the link
// itself (not just on the not-yet-existing child) — the walk must not
// treat the dangling link's name as an ordinary missing tail segment.
const rootsProvider = () => [];
const link = join(cwd, "dangling-link");
const outsideTarget = join(outside, "not-created-yet");
await symlink(outsideTarget, link);
const target = join(link, "child.txt");

expect(resolveWorkspacePath(cwd, join("dangling-link", "child.txt"), rootsProvider)).toBeUndefined();

const restriction = createPathRestriction(cwd, rootsProvider, home);
expect(restriction.isRestricted(target, false)).toBe(true);
expect(restriction.isRestricted(target, true)).toBe(true);

// Creating the outside target after the initial check must not retroactively
// legitimize it: this is ordinary outside-symlink denial (the link still
// ultimately points outside the workspace), re-checked once the target exists.
await mkdir(outsideTarget, { recursive: true });
await writeFile(join(outsideTarget, "child.txt"), "s");
expect(resolveWorkspacePath(cwd, join("dangling-link", "child.txt"), rootsProvider)).toBeUndefined();
});

test("a symlink loop under cwd is denied by resolveWorkspacePath (CL-6715)", async () => {
const rootsProvider = () => [];
const linkA = join(cwd, "loop-a");
const linkB = join(cwd, "loop-b");
await symlink(linkB, linkA);
await symlink(linkA, linkB);

expect(resolveWorkspacePath(cwd, join("loop-a", "child.txt"), rootsProvider)).toBeUndefined();
expect(resolveWorkspacePath(cwd, "loop-a", rootsProvider)).toBeUndefined();
});

test("resolveWorkspacePath returns the canonical target so a later symlink retarget cannot redirect a write (CL-6712 TOCTOU)", async () => {
// A symlink that is in-bounds at check time (target -> inside cwd) must
// resolve to the canonical real path, not the lexical path through the
Expand Down
20 changes: 20 additions & 0 deletions src/permission/write-path-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,26 @@ describe("matchesWritePathAllowlist with a symlinked cwd", () => {
});
});

describe("matchesWritePathAllowlist with an unresolvable cwd", () => {
test("a cwd whose final component is a dangling symlink is hard-denied, not spuriously allowed (CL-6715)", () => {
// If cwd itself is unresolvable, both absCwd and abs collapse to the same
// UNRESOLVABLE sentinel, `abs === absCwd` goes true, rel becomes ".", and
// a root-matching pattern (e.g. "**") would otherwise spuriously allow —
// turning a hard authz deny into an ask-prompt.
const parent = mkdtempSync(join(realpathSync(tmpdir()), "write-path-dangling-parent-"));
const danglingCwd = join(parent, "dangling-cwd");
try {
symlinkSync(join(parent, "does-not-exist"), danglingCwd);

expect(matchesWritePathAllowlist("anything.md", ["**"], danglingCwd)).toBe(false);
expect(matchesWritePathAllowlist("PRODUCT.md", ["PRODUCT.md"], danglingCwd)).toBe(false);
} finally {
rmSync(danglingCwd, { force: true });
rmSync(parent, { recursive: true, force: true });
}
});
});

describe("writePathDeniedReason", () => {
test("names allowlist and subject", () => {
const reason = writePathDeniedReason("src/x.ts", ["PRODUCT.md", "docs/*"]);
Expand Down
7 changes: 6 additions & 1 deletion src/permission/write-path-policy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { resolve, sep } from "node:path";
import { matchesPattern } from "./matcher.js";
import { realpathNearestOr } from "./path-restriction.js";
import { realpathNearestOr, UNRESOLVABLE } from "./path-restriction.js";

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