From 63aa657e4ebca93c7191f78bf66a1b382db836f3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 22 Aug 2026 13:57:25 -0700 Subject: [PATCH 1/2] Return canonical realpath from workspace containment allow resolveWorkspacePath already computes each candidate's realpath to decide containment but returned the lexical path, leaving a TOCTOU window: a symlink in-bounds at allow time could be retargeted before the actual write, escaping the workspace. Return the resolved real path instead so pathEscapePlugin substitutes it into the tool call args, and writers act on the already-resolved location rather than re-traversing the symlink. Fixes CL-6712 https://linear.app/abklabs/issue/CL-6712 --- src/permission/path-restriction.ts | 21 +++++--- src/permission/permission.test.ts | 5 +- src/permission/workspace-containment.test.ts | 33 ++++++++++++- src/plugins/path-escape-plugin.test.ts | 50 +++++++++++++++++++- 4 files changed, 100 insertions(+), 9 deletions(-) diff --git a/src/permission/path-restriction.ts b/src/permission/path-restriction.ts index 9f24bd2b4..e864b953e 100644 --- a/src/permission/path-restriction.ts +++ b/src/permission/path-restriction.ts @@ -68,9 +68,18 @@ const inKnownRoots = (real: string, roots: readonly string[]): boolean => // Resolves `path` (relative or absolute, possibly traversing `..`) against // `cwd` and checks it against the workspace boundary: `cwd` itself plus every // root `rootsProvider` reports (the session's registered git worktrees, or -// any other allowlisted sibling). Returns the resolved absolute path when the -// target is in bounds, `undefined` otherwise — callers that need a hard -// allow/deny (rather than an allow/ask distinction) can key off that. +// any other allowlisted sibling). Returns the CANONICAL real path (symlink +// segments resolved; for a not-yet-created target, the nearest existing +// ancestor's real path rejoined with the missing tail) when the target is in +// bounds, `undefined` otherwise — callers that need a hard allow/deny (rather +// than an allow/ask distinction) can key off that. +// +// Returning the canonical path rather than the lexical `abs` closes a +// TOCTOU: a symlink segment that is in-bounds at check time can be +// retargeted before a write actually happens. Callers (e.g. pathEscapePlugin) +// substitute this return value into the tool call's path argument, so the +// writer that ultimately opens the file never re-traverses the original +// symlink — it uses the already-resolved location. // // A relative `../` is deliberately resolved and realpath-checked against the // allowlist rather than rejected outright: the raw path alone can't tell a @@ -84,9 +93,9 @@ export function resolveWorkspacePath( const abs = resolve(cwd, path); const realCwd = realpathOr(resolve(cwd)); const real = realpathNearestOr(abs); - if (real === realCwd || real.startsWith(realCwd + sep)) return abs; - if (inKnownRoots(real, rootsProvider())) return abs; - if (inKnownRoots(real, rootsProvider(true))) return abs; + if (real === realCwd || real.startsWith(realCwd + sep)) return real; + if (inKnownRoots(real, rootsProvider())) return real; + if (inKnownRoots(real, rootsProvider(true))) return real; return undefined; } diff --git a/src/permission/permission.test.ts b/src/permission/permission.test.ts index 622cea535..566156e44 100644 --- a/src/permission/permission.test.ts +++ b/src/permission/permission.test.ts @@ -2813,7 +2813,10 @@ describe("listWorktreeRoots", () => { const { repo, worktree } = createRepoWithWorktree(); const roots = await listWorktreeRoots(repo); const relativeTarget = join("..", "secondary", "notes.md"); - expect(resolveWorkspacePath(repo, relativeTarget, () => roots)).toBe(join(repo, "..", "secondary", "notes.md")); + // Canonical (realpath-resolved), not the lexical join — see CL-6712. + expect(resolveWorkspacePath(repo, relativeTarget, () => roots)).toBe( + join(realpathSync(join(repo, "..")), "secondary", "notes.md"), + ); }); test("resolveWorkspacePath still rejects a genuinely unrelated outside path", async () => { diff --git a/src/permission/workspace-containment.test.ts b/src/permission/workspace-containment.test.ts index 23354e2be..1d19b2a01 100644 --- a/src/permission/workspace-containment.test.ts +++ b/src/permission/workspace-containment.test.ts @@ -6,7 +6,7 @@ import { tmpdir } from "node:os"; import type { ToolCall } from "@intx/types/runtime"; import { isAutoAllowedShellCall } from "./classify.js"; -import { createPathRestriction } from "./path-restriction.js"; +import { createPathRestriction, resolveWorkspacePath } from "./path-restriction.js"; let cwd = ""; let worktree = ""; @@ -91,3 +91,34 @@ test("a symlink pointing outside the workspace is refused, even for a not-yet-ex expect(autoAllowed).toBe(false); expect(restricted).toBe(true); }); + +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 + // symlink. If the caller only remembered the lexical path and re-opened it + // after the symlink is retargeted, the write would follow the new target + // instead of the one that was actually approved. + const rootsProvider = () => []; + const realTarget = join(cwd, "real-target"); + await mkdir(realTarget, { recursive: true }); + const link = join(cwd, "link"); + await symlink(realTarget, link); + + const resolved = resolveWorkspacePath(cwd, join("link", "note.txt"), rootsProvider); + expect(resolved).toBe(join(realpathSync(realTarget), "note.txt")); + + // Retarget the symlink to point outside the workspace, as an attacker + // would do between the allow check and the actual write. + await rm(link); + await symlink(outside, link); + + // The canonical path captured before the retarget still points at the + // originally-approved location inside the workspace — it never traverses + // "link" again, so it is unaffected by the retarget. + expect(resolved).not.toContain(outside); + + // A fresh check against the now-retargeted symlink correctly sees the + // escape and denies it. + const restriction = createPathRestriction(cwd, rootsProvider, home); + expect(restriction.isRestricted(join(link, "note.txt"), true)).toBe(true); +}); diff --git a/src/plugins/path-escape-plugin.test.ts b/src/plugins/path-escape-plugin.test.ts index c792c4e67..94027fe8d 100644 --- a/src/plugins/path-escape-plugin.test.ts +++ b/src/plugins/path-escape-plugin.test.ts @@ -1,4 +1,8 @@ -import { describe, test, expect } from "bun:test"; +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import { mkdir, mkdtemp, rm, symlink } from "node:fs/promises"; +import { realpathSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { pathEscapePlugin } from "./path-escape-plugin.js"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; @@ -175,4 +179,48 @@ describe("pathEscapePlugin", () => { const args = JSON.parse(String(allowed.content)) as { path: string }; expect(args.path).toBe("/other-repo/README.md"); }); + + describe("symlink TOCTOU (CL-6712)", () => { + let cwd = ""; + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "corbits-path-escape-")); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + test("write_file receives the canonical path, unaffected by a later symlink retarget", async () => { + const realTarget = join(cwd, "real-target"); + await mkdir(realTarget, { recursive: true }); + const link = join(cwd, "link"); + await symlink(realTarget, link); + + const plugin = pathEscapePlugin(cwd); + const next = async (call: ToolCall): Promise => ({ + callId: call.id, + content: JSON.stringify(call.arguments), + }); + const handler = plugin.middleware ? plugin.middleware(next) : next; + + const result = await handler( + makeCall("write_file", { path: join("link", "note.txt"), content: "hi" }), + new AbortController().signal, + ); + const args = JSON.parse(String(result.content)) as { path: string }; + // The path handed to write_file is already the resolved real-target + // location, not the symlink-relative path. + expect(args.path).toBe(join(realpathSync(realTarget), "note.txt")); + + // An attacker retargets the symlink after the allow check. A writer + // that (correctly) uses the path it was given above is unaffected — + // it never re-traverses "link". + const outside = await mkdtemp(join(tmpdir(), "corbits-path-escape-outside-")); + await rm(link); + await symlink(outside, link); + expect(args.path).not.toContain(outside); + await rm(outside, { recursive: true, force: true }); + }); + }); }); From f3f8bb46ee84b41aa00f78651dd2fc205660791b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 22 Aug 2026 14:06:22 -0700 Subject: [PATCH 2/2] Canonicalize cwd in write-path allowlist compare --- src/permission/path-restriction.ts | 2 +- src/permission/write-path-policy.test.ts | 37 +++++++++++++++++++++++- src/permission/write-path-policy.ts | 9 ++++-- src/plugins/path-escape-plugin.test.ts | 11 +++++-- 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/permission/path-restriction.ts b/src/permission/path-restriction.ts index e864b953e..43a02db76 100644 --- a/src/permission/path-restriction.ts +++ b/src/permission/path-restriction.ts @@ -44,7 +44,7 @@ function realpathOr(path: string): string { // 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). -function realpathNearestOr(path: string): string { +export function realpathNearestOr(path: string): string { try { return realpathSync(path); } catch { diff --git a/src/permission/write-path-policy.test.ts b/src/permission/write-path-policy.test.ts index ca0f14064..40df9090d 100644 --- a/src/permission/write-path-policy.test.ts +++ b/src/permission/write-path-policy.test.ts @@ -1,5 +1,7 @@ import { describe, expect, test } from "bun:test"; -import { resolve } from "node:path"; +import { mkdtempSync, realpathSync, rmSync, symlinkSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; import { matchesWritePathAllowlist, writePathDeniedReason, @@ -58,6 +60,39 @@ describe("matchesWritePathAllowlist", () => { }); }); +describe("matchesWritePathAllowlist with a symlinked cwd", () => { + test("allows a write under the canonical target of a symlinked cwd", () => { + // Mirrors macOS's /tmp -> /private/tmp: cwd is spelled via the symlink, + // but resolveWorkspacePath (and any tool arg it rewrites) hands the + // subject in already realpathed. Both sides of the compare must + // canonicalize the same way or a legitimate write is hard-denied. + const real = mkdtempSync(join(realpathSync(tmpdir()), "write-path-real-")); + const linkDir = join(realpathSync(tmpdir()), `write-path-link-${process.pid}`); + try { + symlinkSync(real, linkDir); + const symlinkedCwd = linkDir; // lexically distinct from `real` + const canonicalSubject = join(real, "docs", "a.md"); // already realpathed + + expect( + matchesWritePathAllowlist(canonicalSubject, ["docs/*"], symlinkedCwd), + ).toBe(true); + + // A genuinely outside path is still denied. + const outsideReal = mkdtempSync(join(realpathSync(tmpdir()), "write-path-outside-")); + try { + expect( + matchesWritePathAllowlist(join(outsideReal, "docs", "a.md"), ["docs/*"], symlinkedCwd), + ).toBe(false); + } finally { + rmSync(outsideReal, { recursive: true, force: true }); + } + } finally { + rmSync(linkDir, { force: true }); + rmSync(real, { recursive: true, force: true }); + } + }); +}); + describe("writePathDeniedReason", () => { test("names allowlist and subject", () => { const reason = writePathDeniedReason("src/x.ts", ["PRODUCT.md", "docs/*"]); diff --git a/src/permission/write-path-policy.ts b/src/permission/write-path-policy.ts index 6c6edee01..dd7a30547 100644 --- a/src/permission/write-path-policy.ts +++ b/src/permission/write-path-policy.ts @@ -1,5 +1,6 @@ import { resolve, sep } from "node:path"; import { matchesPattern } from "./matcher.js"; +import { realpathNearestOr } from "./path-restriction.js"; /** * Director write-path allowlist (authz, not prompt policy). @@ -29,8 +30,12 @@ export function matchesWritePathAllowlist( if (allowlist.length === 0) return false; if (subject.length === 0) return false; - const absCwd = resolve(cwd); - const abs = resolve(cwd, subject); + // Canonicalize both sides the same way path-restriction does: a symlinked + // cwd (e.g. macOS /tmp -> /private/tmp) must not desync from a subject + // already resolved to its realpath by resolveWorkspacePath, which would + // otherwise hard-deny a legitimate allowlisted write. + const absCwd = realpathNearestOr(resolve(cwd)); + const abs = realpathNearestOr(resolve(cwd, subject)); let rel: string; if (abs === absCwd) { rel = "."; diff --git a/src/plugins/path-escape-plugin.test.ts b/src/plugins/path-escape-plugin.test.ts index 94027fe8d..fdf6b9017 100644 --- a/src/plugins/path-escape-plugin.test.ts +++ b/src/plugins/path-escape-plugin.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect, beforeEach, afterEach } from "bun:test"; -import { mkdir, mkdtemp, rm, symlink } from "node:fs/promises"; -import { realpathSync } from "node:fs"; +import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { existsSync, realpathSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -220,6 +220,13 @@ describe("pathEscapePlugin", () => { await rm(link); await symlink(outside, link); expect(args.path).not.toContain(outside); + + // A real writer using the resolved path lands the bytes at the + // canonical (safe) location, never under the retargeted symlink. + await writeFile(args.path, "hi"); + expect(await readFile(args.path, "utf8")).toBe("hi"); + expect(existsSync(join(outside, "note.txt"))).toBe(false); + await rm(outside, { recursive: true, force: true }); }); });