Only skip the spill URI sandbox for reader tools

[TheGreatAxios]

Summary

• The path-escape sandbox now only skips containment for tool-output URIs on read_file and archive refs on read_file, grep, and search_files; every other tool is denied at the sandbox layer instead of relying on plugin order
• The permission gate passes the tool name into the same check so authorize-time and execution-time verdicts match
• The tool-output URI rejector plugin stays in place as defense-in-depth
• Parallel stack, lands on main independently: shares path-escape wiring with CL-6730/CL-6729, expect a rebase

Verification

• bun run check passes (lint, typecheck, build, projects-dir guard, 7311 tests across 501 files)
• New order-independent tests fail without the fix and pass with it

Fixes CL-6727

[linear-code]

CL-6727

[TheGreatAxios]

Critic · Comment

Scoped sandbox skip for virtual refs; gate and middleware verdicts match. Verified on-worktree: 17/17 pass in src/plugins/path-escape-plugin.test.ts, including the order-independence test.

Findings

• src/plugins/path-escape-plugin.ts — virtualRefVerdict blocks non-readers at both layers (pathEscapeBlockReason for authorize-time, sanitizePath for execution-time), so the deny no longer depends on toolOutputUriPlugin ordering. The no-rejector middleware test proves it.
• src/plugins/path-escape-plugin.ts — archive scope (read_file, grep, search_files) matches evidenceArchiveSearchPlugin middleware exactly (src/plugins/evidence-archive-search-plugin.ts:152-158), which ignores all other tools. Justified, not over-broad.
• src/plugins/path-escape-plugin.ts — omitted toolName keeps legacy skip, but the only production callers (middleware, gate) always pass a name; grep shows no other prod callers of pathEscapeBlockReason. Back-compat risk is negligible.
• src/permission/gate.ts:674 — call.name threading keeps authorize-time and execution-time verdicts in sync; no sync/async drift, toolName is optional.
• src/plugins/path-escape-plugin.test.ts — tests are real: fail-without-fix shape (red dfb297f5 → green 4a0304af), no mocks of the unit under test, reader-pass and non-reader-block both covered.

Notes

• Parallel stack with Harden path-escape sanitization for nested and alternate path keys #1005 (CL-6730) touches the same two files; expect a mechanical rebase conflict. This PR already discloses it.
• toolOutputUriPlugin rejector stays in place; sandbox message mirrors it. Defense-in-depth, not duplication worth removing.

No blocking issues.

[TheGreatAxios]

[warden trust review] Approve (trust lane — permission path only)

Reviewed the CL-6727 spill-URI sandbox diff (src/plugins/path-escape-plugin.ts, src/permission/gate.ts consult, src/plugins/path-escape-plugin.test.ts). No trust blocker; no merge performed.

What I verified, with evidence:

• Order-independent deny: pathEscapePlugin runs before toolOutputUriPlugin in src/agent/posix-tool-plugins.ts:115-118, so previously only the later plugin rejected non-reader spill URIs. The new virtualRefVerdict deny in escapeArgs/sanitizePath plus the gate consult means a non-reader (grep + path: tool-output:///abc123) is denied at authorize time and at execution even if plugin order changes or toolOutputUriPlugin is removed. Deny sits before grant matching in gate.ts decide(), so no grant can cover it.
• gate.ts consult passes the real call.name (the dispatched tool identity, not a user-controlled argument), matching what the middleware passes. No spoof vector: choosing a different call.name just dispatches a different tool, which is exactly what the verdict keys on.
• Exemptions tight: TOOL_OUTPUT_URI_TOOL = read_file matches the only consumer in tool-output-uri-plugin.ts:21; ARCHIVE_URI_TOOLS = {read_file, grep, search_files} matches the exact triple served by evidenceArchiveSearchPlugin (evidence-archive-search-plugin.ts:152-158) and advertised by advertiseArchiveSurface. write_file + archive:/// is denied (covered by new test).
• allowOutside/yolo does not peel this: sanitizePath throws on a block verdict before the allowOutside resolve branch, so yolo still denies non-reader virtual refs at execution.
• No secret-guard/shell-policy/plugin-trust change: error strings echo only the opaque URI ref, not blob content; no new capability granted to untrusted plugins.

Should-fix (non-blocking, route to builder): toolName?: string defaults to legacy skip when omitted (virtualRefVerdict: toolName === undefined → "skip" for both schemes). Both production callers (gate consult, middleware via escapeArgs) pass a name today so this is not reachable in production, but the default is fail-open — any future/out-of-tree caller that forgets the argument silently skips the new deny. Make the parameter required, or default an omitted name to deny rather than skip, with a test pinning it.

Suggested permanent tests (for testsmith/builder, not implemented here): gate-level deny test (grep + tool-output:/// denied at authorize, read_file + same URI not denied); allowOutside=true still denies non-reader virtual ref at execution; omitted-toolName behavior pinned fail-closed once the signature is tightened.

[TheGreatAxios]

Warden should-fix addressed in 946bd93: omitted toolName now fails closed. virtualRefVerdict denies (with a no-identity message) when no tool name reaches it instead of legacy skip, and toolName is now a required parameter on pathEscapeBlockReason, escapeArgs, and sanitizePath so future callers get a compile error rather than a silent skip. Pinned by an omitted-toolName test covering both tool-output:/// and archive:/// refs. Exemptions unchanged (read_file-only tool-output skip; read_file/grep/search_files archive triple) and the rejector stays in place. Not merging per brief.

[TheGreatAxios]

Critic follow-up: the suggested permanent tests are now landed in 946bd93 alongside the fail-closed fix — gate-level authorize deny (grep + tool-output:/// denied, read_file + same URI not denied, in src/permission/gate.test.ts), allowOutside-still-denies at execution for a non-reader virtual ref (both schemes), and the omitted-toolName fail-closed pin. Verification: 527 pass / 0 fail across src/permission + path-escape plugin suite; lint, typecheck, and build each exit 0; full test suite 7315 pass / 0 fail. Not merging per brief.

[TheGreatAxios]

Confirmed: 946bd93 closes the omitted-toolName fail-open. toolName is now required on escapeArgs/sanitizePath/pathEscapeBlockReason; both production callers (plugin middleware, permission gate) pass call.name, and virtualRefVerdict denies on undefined for both URI schemes instead of skipping. New tests pin it: omitted-name fail-closed, gate-level authorize deny, allowOutside execution deny. Verified in the worktree: plugin suite 19 pass, gate suite 34 pass, tsc --noEmit clean. No new issues found.