From eac78e6cadd17b2c02dccc1c3a54b9d00407461e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 13 Sep 2026 13:57:05 -0700 Subject: [PATCH 1/3] Reproduce spill URI sandbox bypass for non-reader tools --- src/plugins/path-escape-plugin.test.ts | 85 ++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/src/plugins/path-escape-plugin.test.ts b/src/plugins/path-escape-plugin.test.ts index c27676824..12f1394ec 100644 --- a/src/plugins/path-escape-plugin.test.ts +++ b/src/plugins/path-escape-plugin.test.ts @@ -429,4 +429,89 @@ describe("pathEscapePlugin", () => { ).toEqual({ xpath: "src/index.ts" }); }); }); + + describe("spill URI sandbox (CL-6727)", () => { + test("pathEscapeBlockReason blocks a tool-output URI for a non-reader", () => { + const reason = pathEscapeBlockReason( + { path: "tool-output:///abc123" }, + "/project", + () => [], + "grep", + ); + expect(reason).toMatch(/tool-output/); + }); + + test("middleware blocks a non-reader tool-output call with no rejector plugin", async () => { + const plugin = pathEscapePlugin("/project"); + const handler = plugin.middleware + ? plugin.middleware(nextHandler) + : nextHandler; + const result = await handler( + makeCall("grep", { + pattern: "foo", + path: "tool-output:///abc123", + }), + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/tool-output/); + }); + + test("read_file still passes a tool-output URI through", async () => { + expect( + pathEscapeBlockReason( + { path: "tool-output:///abc123" }, + "/project", + () => [], + "read_file", + ), + ).toBeUndefined(); + const plugin = pathEscapePlugin("/project"); + 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("read_file", { path: "tool-output:///abc123" }), + new AbortController().signal, + ); + expect(result.isError).not.toBe(true); + const args = JSON.parse(String(result.content)) as { path: string }; + expect(args.path).toBe("tool-output:///abc123"); + }); + + test("archive refs pass for archive readers but not for other tools", async () => { + for (const name of ["read_file", "grep", "search_files"]) { + expect( + pathEscapeBlockReason( + { path: "archive:///occ-abc" }, + "/project", + () => [], + name, + ), + ).toBeUndefined(); + } + expect( + pathEscapeBlockReason( + { path: "archive:///occ-abc" }, + "/project", + () => [], + "write_file", + ), + ).toMatch(/archive/); + const plugin = pathEscapePlugin("/project"); + const handler = plugin.middleware + ? plugin.middleware(nextHandler) + : nextHandler; + const blocked = await handler( + makeCall("write_file", { + path: "archive:///occ-abc", + content: "hi", + }), + new AbortController().signal, + ); + expect(blocked.isError).toBe(true); + }); + }); }); From ac779221bc9803b691cc4e9cb86cdbf6ba0b9f55 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 13 Sep 2026 13:58:43 -0700 Subject: [PATCH 2/3] Scope spill URI sandbox bypass to reader tools only --- src/permission/gate.ts | 1 + src/plugins/path-escape-plugin.ts | 81 ++++++++++++++++++++++++++----- 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/permission/gate.ts b/src/permission/gate.ts index f09bb2053..11936188d 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -666,6 +666,7 @@ export function createPermissionGate( call.arguments, effectiveCwd, escapeRoots, + call.name, ); if (escapeReason !== undefined) { return { kind: "deny", reason: escapeReason }; diff --git a/src/plugins/path-escape-plugin.ts b/src/plugins/path-escape-plugin.ts index 55796cfd6..fb080ac95 100644 --- a/src/plugins/path-escape-plugin.ts +++ b/src/plugins/path-escape-plugin.ts @@ -35,6 +35,7 @@ export function pathEscapePlugin( cwd, rootsProvider, resolveAllowOutside(options.allowOutside), + call.name, ); } catch (err) { const message = err instanceof Error ? err.message : String(err); @@ -50,15 +51,20 @@ function escapeArgs( cwd: string, rootsProvider: RootsProvider, allowOutside: boolean, + toolName?: string, ): Record { if (!allowOutside) { - const reason = pathEscapeBlockReason(args, cwd, rootsProvider); + const reason = pathEscapeBlockReason(args, cwd, rootsProvider, toolName); if (reason !== undefined) throw new Error(reason); } - return escapeValue(args, cwd, rootsProvider, allowOutside) as Record< - string, - unknown - >; + return escapeValue( + args, + cwd, + rootsProvider, + allowOutside, + undefined, + toolName, + ) as Record; } function escapeValue( @@ -67,15 +73,16 @@ function escapeValue( rootsProvider: RootsProvider, allowOutside: boolean, key?: string, + toolName?: string, ): unknown { if (typeof value === "string") { return key !== undefined && looksLikePath(key) - ? sanitizePath(value, cwd, rootsProvider, allowOutside) + ? sanitizePath(value, cwd, rootsProvider, allowOutside, toolName) : value; } if (Array.isArray(value)) { return value.map((entry) => - escapeValue(entry, cwd, rootsProvider, allowOutside, key), + escapeValue(entry, cwd, rootsProvider, allowOutside, key, toolName), ); } if (typeof value === "object" && value !== null) { @@ -87,6 +94,7 @@ function escapeValue( rootsProvider, allowOutside, entryKey, + toolName, ); } return out; @@ -135,6 +143,40 @@ export function looksLikePath(key: string): boolean { ); } +// Only read_file can consume a spilled tool-output blob; every other tool +// rejects the scheme in toolOutputUriPlugin. The sandbox skips containment +// for the same tool so a non-reader is denied here too instead of only by +// plugin order. +// archive:/// refs are served to read_file, grep, and search_files by +// evidenceArchiveSearchPlugin (see advertiseArchiveSurface); other tools have +// no archive reader, so the sandbox only skips containment for those three. +const TOOL_OUTPUT_URI_TOOL = "read_file"; +const ARCHIVE_URI_TOOLS = new Set(["read_file", "grep", "search_files"]); + +// "skip" when this tool may receive the virtual ref, a block message when it +// may not, undefined when the value is an ordinary filesystem path. An +// omitted toolName keeps the legacy skip so direct callers that predate the +// parameter see no behavior change; the middleware and the permission gate +// always pass a name. +function virtualRefVerdict( + value: string, + toolName: string | undefined, +): "skip" | string | undefined { + if (isToolOutputLike(value)) { + if (toolName === undefined || toolName === TOOL_OUTPUT_URI_TOOL) { + return "skip"; + } + return `cannot ${toolName} a tool-output:// URI: ${value}. Use read_file with that URI to read the spilled output instead.`; + } + if (isArchiveLike(value)) { + if (toolName === undefined || ARCHIVE_URI_TOOLS.has(toolName)) { + return "skip"; + } + return `cannot ${toolName} an archive:/// ref: ${value}. Only read_file, grep, and search_files accept archive:/// refs.`; + } + return undefined; +} + // Same sandbox pathEscapePlugin enforces at execution. The permission gate // consults this at authorize time so it can deny instead of asking for a call // the plugin will reject after Accept. @@ -142,8 +184,9 @@ export function pathEscapeBlockReason( args: Record, cwd: string, rootsProvider: RootsProvider = () => [], + toolName?: string, ): string | undefined { - return blockReasonFor(args, cwd, rootsProvider); + return blockReasonFor(args, cwd, rootsProvider, undefined, toolName); } // Deep-walk identity for the permission gate's authorize/execution cache. @@ -190,10 +233,13 @@ function blockReasonFor( cwd: string, rootsProvider: RootsProvider, key?: string, + toolName?: string, ): string | undefined { if (typeof value === "string") { if (key === undefined || !looksLikePath(key)) return undefined; - if (isToolOutputLike(value) || isArchiveLike(value)) return undefined; + const verdict = virtualRefVerdict(value, toolName); + if (verdict === "skip") return undefined; + if (typeof verdict === "string") return verdict; if (resolveWorkspacePath(cwd, value, rootsProvider) === undefined) { return `Path escapes working directory: ${value}`; } @@ -201,14 +247,20 @@ function blockReasonFor( } if (Array.isArray(value)) { for (const entry of value) { - const reason = blockReasonFor(entry, cwd, rootsProvider, key); + const reason = blockReasonFor(entry, cwd, rootsProvider, key, toolName); if (reason !== undefined) return reason; } return undefined; } if (typeof value === "object" && value !== null) { for (const [entryKey, entryValue] of Object.entries(value)) { - const reason = blockReasonFor(entryValue, cwd, rootsProvider, entryKey); + const reason = blockReasonFor( + entryValue, + cwd, + rootsProvider, + entryKey, + toolName, + ); if (reason !== undefined) return reason; } } @@ -220,10 +272,15 @@ function sanitizePath( cwd: string, rootsProvider: RootsProvider, allowOutside: boolean, + toolName?: string, ): string { - if (isToolOutputLike(value) || isArchiveLike(value)) { + const verdict = virtualRefVerdict(value, toolName); + if (verdict === "skip") { return value; } + if (typeof verdict === "string") { + throw new Error(verdict); + } const resolved = resolveWorkspacePath(cwd, value, rootsProvider); if (resolved !== undefined) { return resolved; From c66895644b0826a49c696ed35092bcf537dab67d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 13 Sep 2026 14:11:46 -0700 Subject: [PATCH 3/3] Close the omitted-toolName fail-open in the spill URI sandbox An omitted toolName used to keep the legacy skip, so a future or out-of-tree caller that forgets the argument would silently skip the deny. The parameter is now required and the verdict denies when no tool identity reaches it. Pin the behavior with an omitted-name test, a gate-level authorize test, and an allowOutside execution test. --- src/permission/gate.test.ts | 35 +++++++++++++ src/plugins/path-escape-plugin.test.ts | 71 ++++++++++++++++++++++++-- src/plugins/path-escape-plugin.ts | 32 +++++++----- 3 files changed, 122 insertions(+), 16 deletions(-) diff --git a/src/permission/gate.test.ts b/src/permission/gate.test.ts index f04d11652..d91aa3e96 100644 --- a/src/permission/gate.test.ts +++ b/src/permission/gate.test.ts @@ -475,3 +475,38 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", () expect(seen).toHaveLength(0); }); }); + +// Spill URI sandbox (CL-6727): the permission gate denies a non-reader +// virtual ref at authorize time, mirroring the execution-time middleware +// deny, while the exempted reader is not denied. +describe("spill URI sandbox at authorize time (CL-6727)", () => { + const cwd = mkdtempSync(join(tmpdir(), "gate-spill-uri-")); + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: false, + reactorGated: false, + cwd, + }); + + test("grep + tool-output:/// is denied at authorize", async () => { + const verdict = await gate.authorizeCall({ + id: "spill-grep", + name: "grep", + arguments: { pattern: "foo", path: "tool-output:///abc123" }, + }); + expect(verdict.effect).toBe("deny"); + expect(verdict.effect === "deny" ? verdict.reason : "").toMatch( + /tool-output/, + ); + }); + + test("read_file + the same tool-output:/// URI is not denied", async () => { + const verdict = await gate.authorizeCall({ + id: "spill-read", + name: "read_file", + arguments: { path: "tool-output:///abc123" }, + }); + expect(verdict.effect).not.toBe("deny"); + }); +}); diff --git a/src/plugins/path-escape-plugin.test.ts b/src/plugins/path-escape-plugin.test.ts index 12f1394ec..754eaf134 100644 --- a/src/plugins/path-escape-plugin.test.ts +++ b/src/plugins/path-escape-plugin.test.ts @@ -342,15 +342,24 @@ describe("pathEscapePlugin", () => { pathEscapeBlockReason( { options: { path: "../secret.txt" } }, "/project", + () => [], + "read_file", ), ).toMatch(/escapes working directory/); expect( - pathEscapeBlockReason({ filepath: "../secret.txt" }, "/project"), + pathEscapeBlockReason( + { filepath: "../secret.txt" }, + "/project", + () => [], + "read_file", + ), ).toMatch(/escapes working directory/); expect( pathEscapeBlockReason( { paths: ["src/index.ts", "../secret.txt"] }, "/project", + () => [], + "read_file", ), ).toMatch(/escapes working directory/); }); @@ -369,6 +378,8 @@ describe("pathEscapePlugin", () => { pathEscapeBlockReason( { options: { command: "../secret.txt" } }, "/project", + () => [], + "custom_tool", ), ).toBeUndefined(); }); @@ -386,7 +397,12 @@ describe("pathEscapePlugin", () => { expect(result.isError).toBe(true); expect(result.content).toMatch(/escapes working directory/); expect( - pathEscapeBlockReason({ [key]: "../secret.txt" }, "/project"), + pathEscapeBlockReason( + { [key]: "../secret.txt" }, + "/project", + () => [], + "read_file", + ), ).toMatch(/escapes working directory/); } }); @@ -406,7 +422,9 @@ describe("pathEscapePlugin", () => { ); expect(result.isError).not.toBe(true); expect(seen()).toEqual(args); - expect(pathEscapeBlockReason(args, "/project")).toBeUndefined(); + expect( + pathEscapeBlockReason(args, "/project", () => [], "custom_tool"), + ).toBeUndefined(); }); test("normalizePathArguments shares the plugin rewrite identity", () => { @@ -513,5 +531,52 @@ describe("pathEscapePlugin", () => { ); expect(blocked.isError).toBe(true); }); + + test("omitted toolName fails closed on virtual refs", () => { + const omitted = undefined as unknown as string; + expect( + pathEscapeBlockReason( + { path: "tool-output:///abc123" }, + "/project", + () => [], + omitted, + ), + ).toMatch(/tool-output/); + expect( + pathEscapeBlockReason( + { path: "archive:///occ-abc" }, + "/project", + () => [], + omitted, + ), + ).toMatch(/archive/); + }); + + test("allowOutside still denies a non-reader virtual ref at execution", async () => { + const plugin = pathEscapePlugin("/project", () => [], { + allowOutside: true, + }); + const handler = plugin.middleware + ? plugin.middleware(nextHandler) + : nextHandler; + const spill = await handler( + makeCall("grep", { + pattern: "foo", + path: "tool-output:///abc123", + }), + new AbortController().signal, + ); + expect(spill.isError).toBe(true); + expect(spill.content).toMatch(/tool-output/); + const archive = await handler( + makeCall("write_file", { + path: "archive:///occ-abc", + content: "hi", + }), + new AbortController().signal, + ); + expect(archive.isError).toBe(true); + expect(archive.content).toMatch(/archive/); + }); }); }); diff --git a/src/plugins/path-escape-plugin.ts b/src/plugins/path-escape-plugin.ts index fb080ac95..4b3070c1f 100644 --- a/src/plugins/path-escape-plugin.ts +++ b/src/plugins/path-escape-plugin.ts @@ -51,7 +51,7 @@ function escapeArgs( cwd: string, rootsProvider: RootsProvider, allowOutside: boolean, - toolName?: string, + toolName: string, ): Record { if (!allowOutside) { const reason = pathEscapeBlockReason(args, cwd, rootsProvider, toolName); @@ -72,8 +72,8 @@ function escapeValue( cwd: string, rootsProvider: RootsProvider, allowOutside: boolean, - key?: string, - toolName?: string, + key: string | undefined, + toolName: string, ): unknown { if (typeof value === "string") { return key !== undefined && looksLikePath(key) @@ -154,24 +154,30 @@ const TOOL_OUTPUT_URI_TOOL = "read_file"; const ARCHIVE_URI_TOOLS = new Set(["read_file", "grep", "search_files"]); // "skip" when this tool may receive the virtual ref, a block message when it -// may not, undefined when the value is an ordinary filesystem path. An -// omitted toolName keeps the legacy skip so direct callers that predate the -// parameter see no behavior change; the middleware and the permission gate -// always pass a name. +// may not, undefined when the value is an ordinary filesystem path. An omitted +// toolName denies rather than skips: both production callers (the middleware +// and the permission gate) always pass a name, so an omission is a caller bug +// and must fail closed instead of silently skipping the deny. function virtualRefVerdict( value: string, toolName: string | undefined, ): "skip" | string | undefined { if (isToolOutputLike(value)) { - if (toolName === undefined || toolName === TOOL_OUTPUT_URI_TOOL) { + if (toolName === TOOL_OUTPUT_URI_TOOL) { return "skip"; } + if (toolName === undefined) { + return `cannot use a tool-output:// URI without a tool identity: ${value}. Use read_file with that URI to read the spilled output instead.`; + } return `cannot ${toolName} a tool-output:// URI: ${value}. Use read_file with that URI to read the spilled output instead.`; } if (isArchiveLike(value)) { - if (toolName === undefined || ARCHIVE_URI_TOOLS.has(toolName)) { + if (toolName !== undefined && ARCHIVE_URI_TOOLS.has(toolName)) { return "skip"; } + if (toolName === undefined) { + return `cannot use an archive:/// ref without a tool identity: ${value}. Only read_file, grep, and search_files accept archive:/// refs.`; + } return `cannot ${toolName} an archive:/// ref: ${value}. Only read_file, grep, and search_files accept archive:/// refs.`; } return undefined; @@ -184,7 +190,7 @@ export function pathEscapeBlockReason( args: Record, cwd: string, rootsProvider: RootsProvider = () => [], - toolName?: string, + toolName: string, ): string | undefined { return blockReasonFor(args, cwd, rootsProvider, undefined, toolName); } @@ -232,8 +238,8 @@ function blockReasonFor( value: unknown, cwd: string, rootsProvider: RootsProvider, - key?: string, - toolName?: string, + key: string | undefined, + toolName: string, ): string | undefined { if (typeof value === "string") { if (key === undefined || !looksLikePath(key)) return undefined; @@ -272,7 +278,7 @@ function sanitizePath( cwd: string, rootsProvider: RootsProvider, allowOutside: boolean, - toolName?: string, + toolName: string, ): string { const verdict = virtualRefVerdict(value, toolName); if (verdict === "skip") {