From 80177b2586d9bef8af82e0d2a850b400a480aa98 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 28 Sep 2026 07:36:04 -0700 Subject: [PATCH 1/2] CL-9341: fingerprint hidden-shell declines through the canonical coercion A hidden shell call declined by the operator must suppress its coerced run_shell retry with the identical reason. denialFingerprint now derives the key from callForIdentity(rawCall) instead of a name-only canonicalization, so raw, coerced, and fresh-id retries share one denial entry on the middleware, decide, and reactor paths. resolveSuspended rebuilds its key through the same helper; coercion is idempotent on coerced run_shell because looksLikeCodexShellArgs ignores native cwd/timeout, so tool-aliases needs no change. The workdir case rides inside arguments while the fingerprint cwd stays the session cwd; a suspended request cannot round-trip a coerced cwd (buildRequests keeps only command), so reactor-path workdir declines re-ask fail-safe. No grant/deny policy or wording changes. --- src/permission/gate.ts | 39 ++++- src/permission/shell-argv-authorize.test.ts | 156 +++++++++++++++++++- 2 files changed, 186 insertions(+), 9 deletions(-) diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 028598739..0c21d1230 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -741,9 +741,16 @@ export function createPermissionGate( // provider-identity switches). Timeouts and aborts are never recorded. const denialMemory = new DenialMemory(); - const denialFingerprint = (call: ToolCall, cwd: string): string => + // Single canonical denial key: the raw call coerced onto its engine id + // (hidden shell argv/workdir, default./doubled aliases) before the stable + // request id, so a same-turn retry in any shape — raw, coerced, or fresh + // tool_call.id — fingerprints identically. Coercion is idempotent on + // already-coerced run_shell (looksLikeCodexShellArgs ignores native + // cwd/timeout), so decide() passing the raw call and evaluate() passing + // the raw call share one derivation with resolveSuspended()'s rebuild. + const denialFingerprint = (rawCall: ToolCall, cwd: string): string => stableRequestId( - withCanonicalToolName(call), + callForIdentity(rawCall), cwd, rootsProvider, trustedPluginRoots, @@ -817,7 +824,11 @@ export function createPermissionGate( // deny below: the decision is the same deny either way, but the reason // text is the originally recorded one, not a freshly computed escape // reason. - const stableId = denialFingerprint(call, effectiveCwd); + // Fingerprint the raw call, not the coerced `call` above: the coercion + // inside denialFingerprint is idempotent, so both forms share one key, + // and the raw form is what evaluate() and resolveSuspended() also derive + // from — keeping the three decline paths on the identical reason. + const stableId = denialFingerprint(rawCall, effectiveCwd); const cachedDenial = denialMemory.isDenied(stableId); if (cachedDenial !== undefined) return { kind: "deny", reason: cachedDenial }; @@ -1121,8 +1132,8 @@ export function createPermissionGate( // calls never pass through the reactor (sub-agents). When the gate is // reactor-gated, gateToolCall uses executionVerdict instead of evaluate() so // deny still blocks and ask never re-prompts (see gateToolCall). - const evaluate = async (call: ToolCall): Promise => { - const decision = await decide(call); + const evaluate = async (rawCall: ToolCall): Promise => { + const decision = await decide(rawCall); if (decision.kind === "allow") return { allowed: true }; if (decision.kind === "deny") return { allowed: false, reason: decision.reason }; @@ -1133,14 +1144,15 @@ export function createPermissionGate( // same stable fingerprint) denies with the identical reason instead of // re-asking. Timeouts, aborts, and missing outcomes are never cached — // the operator made no decision, so the retry must ask again (mirrors - // resolveSuspended). + // resolveSuspended). The key derives from the raw call through the one + // canonical coercion, so a coerced-shape retry hits the same entry. if ( outcome !== undefined && !outcome.allow && classifyOutcome(outcome) === "deny" ) { denialMemory.record( - denialFingerprint(call, getSubAgentIdentity()?.cwd ?? resolvedCwd), + denialFingerprint(rawCall, getSubAgentIdentity()?.cwd ?? resolvedCwd), reason, ); } @@ -1241,14 +1253,25 @@ export function createPermissionGate( // reason instead of re-prompting. Timeouts, aborts, and missing outcomes // are never cached — the operator made no decision, so the retry must // ask again. Invalidation (grant-mint/mode/identity clears) is unchanged. + // Rebuild the denied ToolCall from the suspended request through the + // one canonical helper decide()/evaluate() use, so the recorded key + // matches the retry's decide() key: request.tool/arguments are already + // post-coercion (run_shell {command}), and callForIdentity re-applies + // the same canonicalization idempotently. The synthesized id never + // participates (stableRequestId ignores call.id and correlationId). if ( outcome !== undefined && !outcome.allow && classifyOutcome(outcome) === "deny" ) { + const suspendedCall = callForIdentity({ + id: "", + name: request.tool, + arguments: request.arguments ?? {}, + }); denialMemory.record( denialFingerprint( - { id: "", name: request.tool, arguments: request.arguments ?? {} }, + suspendedCall, request.cwd ?? getSubAgentIdentity()?.cwd ?? resolvedCwd, ), declineReason(request, outcome), diff --git a/src/permission/shell-argv-authorize.test.ts b/src/permission/shell-argv-authorize.test.ts index 7935e94f2..c82318395 100644 --- a/src/permission/shell-argv-authorize.test.ts +++ b/src/permission/shell-argv-authorize.test.ts @@ -1,7 +1,11 @@ import { describe, expect, test } from "bun:test"; +import { join } from "node:path"; import type { ToolCall } from "@intx/types/runtime"; -import { prepareDispatchedToolCall } from "../agent/tool-aliases.js"; +import { + looksLikeCodexShellArgs, + prepareDispatchedToolCall, +} from "../agent/tool-aliases.js"; import { gateToolCall } from "../plugins/permission-plugin.js"; import { BLOCKED_BY_POLICY_PREFIX } from "./decline-markers.js"; import { createPermissionGate } from "./gate.js"; @@ -17,6 +21,33 @@ function hiddenShellArgv(id: string): ToolCall { }; } +function hiddenShellArgvWorkdir(id: string, workdir: string): ToolCall { + return { + id, + name: "shell", + arguments: { command: ["bash", "-lc", DESTRUCTIVE], workdir }, + }; +} + +function countingDenyGate(reactorGated: boolean): { + gate: ReturnType; + asked: () => number; +} { + let asked = 0; + const gate = createPermissionGate({ + approvals: [], + cwd: process.cwd(), + requestApproval: async () => { + asked++; + return { allow: false }; + }, + interactive: true, + skipPermissions: false, + reactorGated, + }); + return { gate, asked: () => asked }; +} + describe("reactor-gated hidden shell argv coerce-before-authorize", () => { test("command string[] does not auto-allow; unwrapped script is the ask subject; ask does not execute", async () => { const gate = createPermissionGate({ @@ -60,3 +91,126 @@ describe("reactor-gated hidden shell argv coerce-before-authorize", () => { expect(nextCalled).toBe(false); }); }); + +describe("hidden-shell decline fingerprint (CL-9341)", () => { + test("prepareDispatchedToolCall is idempotent on coerced run_shell", () => { + const coerced = prepareDispatchedToolCall( + hiddenShellArgv("idem-0"), + "run_shell", + ); + expect(coerced.name).toBe("run_shell"); + expect(coerced.arguments.command).toBe(DESTRUCTIVE); + expect(prepareDispatchedToolCall(coerced, "run_shell")).toBe(coerced); + const native: ToolCall = { + id: "idem-native", + name: "run_shell", + arguments: { command: DESTRUCTIVE, cwd: "/tmp", timeout: 5 }, + }; + expect(prepareDispatchedToolCall(native, "run_shell")).toBe(native); + expect( + looksLikeCodexShellArgs({ + command: DESTRUCTIVE, + cwd: "/tmp", + timeout: 5, + }), + ).toBe(false); + expect( + looksLikeCodexShellArgs({ command: DESTRUCTIVE, workdir: "/tmp" }), + ).toBe(true); + }); + + test("middleware hidden-shell-array decline suppresses the coerced retry with the identical reason", async () => { + const { gate, asked } = countingDenyGate(false); + const first = await gate.evaluate(hiddenShellArgv("shell-decline-1")); + if (first.allowed) throw new Error("expected the hidden shell declined"); + expect(asked()).toBe(1); + const coerced = prepareDispatchedToolCall( + hiddenShellArgv("shell-decline-2"), + "run_shell", + ); + expect(coerced.arguments.command).toBe(DESTRUCTIVE); + const retry = await gate.evaluate(coerced); + if (retry.allowed) + throw new Error("expected the coerced retry denied from denial memory"); + expect(asked()).toBe(1); + expect(retry.reason).toBe(first.reason); + const rawRetry = await gate.evaluate(hiddenShellArgv("shell-decline-3")); + if (rawRetry.allowed) + throw new Error("expected the raw retry denied from denial memory"); + expect(asked()).toBe(1); + expect(rawRetry.reason).toBe(first.reason); + }); + + test("middleware hidden-shell workdir decline suppresses the cwd-coerced retry", async () => { + // In-workspace so the coerced cwd clears path-escape and reaches the + // operator (an outside-workspace workdir is denied outright, never + // asked). Uncreated on disk: the gate never executes here. + const workdir = join(process.cwd(), "tmp-corbits-decline-wd"); + const { gate, asked } = countingDenyGate(false); + const first = await gate.evaluate( + hiddenShellArgvWorkdir("shell-wd-1", workdir), + ); + if (first.allowed) + throw new Error("expected the hidden workdir shell declined"); + expect(asked()).toBe(1); + const coerced = prepareDispatchedToolCall( + hiddenShellArgvWorkdir("shell-wd-2", workdir), + "run_shell", + ); + expect(coerced.arguments).toEqual({ command: DESTRUCTIVE, cwd: workdir }); + const retry = await gate.evaluate(coerced); + if (retry.allowed) + throw new Error( + "expected the cwd-coerced retry denied from denial memory", + ); + expect(asked()).toBe(1); + expect(retry.reason).toBe(first.reason); + }); + + test("reactor hidden-shell-array decline suppresses raw and coerced retries with the middleware-identical reason", async () => { + const { gate, asked } = countingDenyGate(true); + const suspended = await gate.authorizeCall(hiddenShellArgv("shell-r-1")); + if (suspended.effect !== "ask") + throw new Error("expected the hidden shell to suspend for approval"); + const outcome = await gate.resolveSuspended(suspended.request); + expect(outcome?.allow).toBe(false); + expect(asked()).toBe(1); + const rawRetry = await gate.authorizeCall(hiddenShellArgv("shell-r-2")); + if (rawRetry.effect !== "deny") + throw new Error("expected the raw retry denied from denial memory"); + expect(asked()).toBe(1); + const coercedRetry = await gate.authorizeCall( + prepareDispatchedToolCall(hiddenShellArgv("shell-r-3"), "run_shell"), + ); + if (coercedRetry.effect !== "deny") + throw new Error("expected the coerced retry denied from denial memory"); + expect(asked()).toBe(1); + expect(coercedRetry.reason).toBe(rawRetry.reason); + const { gate: middleware } = countingDenyGate(false); + const verdict = await middleware.evaluate(hiddenShellArgv("shell-r-4")); + if (verdict.allowed) + throw new Error("expected the middleware hidden shell declined"); + expect(rawRetry.reason).toBe(verdict.reason); + }); + + test("plain run_shell decline still suppresses the fresh-id retry", async () => { + const { gate, asked } = countingDenyGate(false); + const args = { command: DESTRUCTIVE }; + const first = await gate.evaluate({ + id: "shell-plain-1", + name: "run_shell", + arguments: args, + }); + if (first.allowed) throw new Error("expected the plain shell declined"); + expect(asked()).toBe(1); + const retry = await gate.evaluate({ + id: "shell-plain-2", + name: "run_shell", + arguments: args, + }); + if (retry.allowed) + throw new Error("expected the plain retry denied from denial memory"); + expect(asked()).toBe(1); + expect(retry.reason).toBe(first.reason); + }); +}); From 068a6c9e4856fb7f3d98bad5c39e9cd9fb154096 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 28 Sep 2026 08:07:58 -0700 Subject: [PATCH 2/2] fix(permissions): state true reactor workdir decline semantics in comment Reactor-path declines of workdir-scoped hidden shell record a {command}-keyed denial while same-workdir retries look up a {command, cwd} key, so the retry re-asks fail-safe and only cwd-less retries over-deny fail-closed. Locks both limits with keeper tests; no behavior change. --- src/permission/gate.ts | 13 +++--- src/permission/shell-argv-authorize.test.ts | 50 +++++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 0c21d1230..49d82c87b 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -1254,11 +1254,14 @@ export function createPermissionGate( // are never cached — the operator made no decision, so the retry must // ask again. Invalidation (grant-mint/mode/identity clears) is unchanged. // Rebuild the denied ToolCall from the suspended request through the - // one canonical helper decide()/evaluate() use, so the recorded key - // matches the retry's decide() key: request.tool/arguments are already - // post-coercion (run_shell {command}), and callForIdentity re-applies - // the same canonicalization idempotently. The synthesized id never - // participates (stableRequestId ignores call.id and correlationId). + // one canonical helper decide()/evaluate() use. The suspended request + // carries post-coercion run_shell {command} only — buildRequests drops + // the workdir-scoped cwd — so the recorded key is {command}-keyed while + // a same-workdir retry's decide() key is {command, cwd}-keyed: the retry + // misses and re-asks (fail-safe, never a bypass). Conversely a cwd-less + // retry of the same command hits the workdir record and denies + // (fail-closed over-deny). The synthesized id never participates + // (stableRequestId ignores call.id and correlationId). if ( outcome !== undefined && !outcome.allow && diff --git a/src/permission/shell-argv-authorize.test.ts b/src/permission/shell-argv-authorize.test.ts index c82318395..30b613ca9 100644 --- a/src/permission/shell-argv-authorize.test.ts +++ b/src/permission/shell-argv-authorize.test.ts @@ -193,6 +193,56 @@ describe("hidden-shell decline fingerprint (CL-9341)", () => { expect(rawRetry.reason).toBe(verdict.reason); }); + test("reactor workdir decline does not suppress the same-workdir raw retry (fail-safe re-ask)", async () => { + // In-workspace so the workdir call reaches the operator (an + // outside-workspace workdir is denied outright, never asked). The + // suspended request carries {command} only (buildRequests drops the + // workdir cwd), so the recorded denial key misses the same-workdir + // retry's {command, cwd} decide() key and the retry suspends again. + const workdir = join(process.cwd(), "tmp-corbits-decline-wd"); + const { gate, asked } = countingDenyGate(true); + const suspended = await gate.authorizeCall( + hiddenShellArgvWorkdir("shell-rwd-1", workdir), + ); + if (suspended.effect !== "ask") + throw new Error("expected the workdir shell to suspend for approval"); + const outcome = await gate.resolveSuspended(suspended.request); + expect(outcome?.allow).toBe(false); + expect(asked()).toBe(1); + const rawRetry = await gate.authorizeCall( + hiddenShellArgvWorkdir("shell-rwd-2", workdir), + ); + if (rawRetry.effect !== "ask") + throw new Error("expected the same-workdir retry to re-ask fail-safe"); + expect(asked()).toBe(1); + const retryOutcome = await gate.resolveSuspended(rawRetry.request); + expect(retryOutcome?.allow).toBe(false); + expect(asked()).toBe(2); + }); + + test("reactor workdir decline suppresses the cwd-less same-command retry (fail-closed over-deny)", async () => { + // Companion direction: the {command}-keyed workdir record matches a + // cwd-less retry of the same command, so it denies without re-asking. + const workdir = join(process.cwd(), "tmp-corbits-decline-wd"); + const { gate, asked } = countingDenyGate(true); + const suspended = await gate.authorizeCall( + hiddenShellArgvWorkdir("shell-rwdc-1", workdir), + ); + if (suspended.effect !== "ask") + throw new Error("expected the workdir shell to suspend for approval"); + const outcome = await gate.resolveSuspended(suspended.request); + expect(outcome?.allow).toBe(false); + expect(asked()).toBe(1); + const cwdLessRetry = await gate.authorizeCall({ + id: "shell-rwdc-2", + name: "run_shell", + arguments: { command: DESTRUCTIVE }, + }); + if (cwdLessRetry.effect !== "deny") + throw new Error("expected the cwd-less retry denied from denial memory"); + expect(asked()).toBe(1); + }); + test("plain run_shell decline still suppresses the fresh-id retry", async () => { const { gate, asked } = countingDenyGate(false); const args = { command: DESTRUCTIVE };