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/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.test.ts b/src/plugins/path-escape-plugin.test.ts index c27676824..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", () => { @@ -429,4 +447,136 @@ 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); + }); + + 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 55796cfd6..4b3070c1f 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( @@ -66,16 +72,17 @@ function escapeValue( cwd: string, rootsProvider: RootsProvider, allowOutside: boolean, - key?: string, + key: string | undefined, + 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,46 @@ 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 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 === 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)) { + 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; +} + // 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 +190,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. @@ -189,11 +238,14 @@ function blockReasonFor( value: unknown, cwd: string, rootsProvider: RootsProvider, - key?: string, + key: string | undefined, + 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 +253,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 +278,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;