Skip to content

Commit f3f8bb4

Browse files
committed
Canonicalize cwd in write-path allowlist compare
1 parent 63aa657 commit f3f8bb4

4 files changed

Lines changed: 53 additions & 6 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
// the raw (possibly symlink-relative) path, which would defeat containment
4545
// checks whenever the workspace root itself is reached through a symlink
4646
// (e.g. macOS's /tmp -> /private/tmp).
47-
function realpathNearestOr(path: string): string {
47+
export function realpathNearestOr(path: string): string {
4848
try {
4949
return realpathSync(path);
5050
} catch {

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { describe, expect, test } from "bun:test";
2-
import { resolve } from "node:path";
2+
import { mkdtempSync, realpathSync, rmSync, symlinkSync } from "node:fs";
3+
import { tmpdir } from "node:os";
4+
import { join, resolve } from "node:path";
35
import {
46
matchesWritePathAllowlist,
57
writePathDeniedReason,
@@ -58,6 +60,39 @@ describe("matchesWritePathAllowlist", () => {
5860
});
5961
});
6062

63+
describe("matchesWritePathAllowlist with a symlinked cwd", () => {
64+
test("allows a write under the canonical target of a symlinked cwd", () => {
65+
// Mirrors macOS's /tmp -> /private/tmp: cwd is spelled via the symlink,
66+
// but resolveWorkspacePath (and any tool arg it rewrites) hands the
67+
// subject in already realpathed. Both sides of the compare must
68+
// canonicalize the same way or a legitimate write is hard-denied.
69+
const real = mkdtempSync(join(realpathSync(tmpdir()), "write-path-real-"));
70+
const linkDir = join(realpathSync(tmpdir()), `write-path-link-${process.pid}`);
71+
try {
72+
symlinkSync(real, linkDir);
73+
const symlinkedCwd = linkDir; // lexically distinct from `real`
74+
const canonicalSubject = join(real, "docs", "a.md"); // already realpathed
75+
76+
expect(
77+
matchesWritePathAllowlist(canonicalSubject, ["docs/*"], symlinkedCwd),
78+
).toBe(true);
79+
80+
// A genuinely outside path is still denied.
81+
const outsideReal = mkdtempSync(join(realpathSync(tmpdir()), "write-path-outside-"));
82+
try {
83+
expect(
84+
matchesWritePathAllowlist(join(outsideReal, "docs", "a.md"), ["docs/*"], symlinkedCwd),
85+
).toBe(false);
86+
} finally {
87+
rmSync(outsideReal, { recursive: true, force: true });
88+
}
89+
} finally {
90+
rmSync(linkDir, { force: true });
91+
rmSync(real, { recursive: true, force: true });
92+
}
93+
});
94+
});
95+
6196
describe("writePathDeniedReason", () => {
6297
test("names allowlist and subject", () => {
6398
const reason = writePathDeniedReason("src/x.ts", ["PRODUCT.md", "docs/*"]);

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

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

45
/**
56
* Director write-path allowlist (authz, not prompt policy).
@@ -29,8 +30,12 @@ export function matchesWritePathAllowlist(
2930
if (allowlist.length === 0) return false;
3031
if (subject.length === 0) return false;
3132

32-
const absCwd = resolve(cwd);
33-
const abs = resolve(cwd, subject);
33+
// Canonicalize both sides the same way path-restriction does: a symlinked
34+
// cwd (e.g. macOS /tmp -> /private/tmp) must not desync from a subject
35+
// already resolved to its realpath by resolveWorkspacePath, which would
36+
// otherwise hard-deny a legitimate allowlisted write.
37+
const absCwd = realpathNearestOr(resolve(cwd));
38+
const abs = realpathNearestOr(resolve(cwd, subject));
3439
let rel: string;
3540
if (abs === absCwd) {
3641
rel = ".";

‎src/plugins/path-escape-plugin.test.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, test, expect, beforeEach, afterEach } from "bun:test";
2-
import { mkdir, mkdtemp, rm, symlink } from "node:fs/promises";
3-
import { realpathSync } from "node:fs";
2+
import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises";
3+
import { existsSync, realpathSync } from "node:fs";
44
import { tmpdir } from "node:os";
55
import { join } from "node:path";
66

@@ -220,6 +220,13 @@ describe("pathEscapePlugin", () => {
220220
await rm(link);
221221
await symlink(outside, link);
222222
expect(args.path).not.toContain(outside);
223+
224+
// A real writer using the resolved path lands the bytes at the
225+
// canonical (safe) location, never under the retargeted symlink.
226+
await writeFile(args.path, "hi");
227+
expect(await readFile(args.path, "utf8")).toBe("hi");
228+
expect(existsSync(join(outside, "note.txt"))).toBe(false);
229+
223230
await rm(outside, { recursive: true, force: true });
224231
});
225232
});

0 commit comments

Comments
 (0)