Skip to content

Commit 7f2c8f6

Browse files
CL-9341: fingerprint hidden-shell declines through the canonical coercion (#1204)
* 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. * 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.
1 parent 1a7fa51 commit 7f2c8f6

2 files changed

Lines changed: 239 additions & 9 deletions

File tree

‎src/permission/gate.ts‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -741,9 +741,16 @@ export function createPermissionGate(
741741
// provider-identity switches). Timeouts and aborts are never recorded.
742742
const denialMemory = new DenialMemory();
743743

744-
const denialFingerprint = (call: ToolCall, cwd: string): string =>
744+
// Single canonical denial key: the raw call coerced onto its engine id
745+
// (hidden shell argv/workdir, default./doubled aliases) before the stable
746+
// request id, so a same-turn retry in any shape — raw, coerced, or fresh
747+
// tool_call.id — fingerprints identically. Coercion is idempotent on
748+
// already-coerced run_shell (looksLikeCodexShellArgs ignores native
749+
// cwd/timeout), so decide() passing the raw call and evaluate() passing
750+
// the raw call share one derivation with resolveSuspended()'s rebuild.
751+
const denialFingerprint = (rawCall: ToolCall, cwd: string): string =>
745752
stableRequestId(
746-
withCanonicalToolName(call),
753+
callForIdentity(rawCall),
747754
cwd,
748755
rootsProvider,
749756
trustedPluginRoots,
@@ -817,7 +824,11 @@ export function createPermissionGate(
817824
// deny below: the decision is the same deny either way, but the reason
818825
// text is the originally recorded one, not a freshly computed escape
819826
// reason.
820-
const stableId = denialFingerprint(call, effectiveCwd);
827+
// Fingerprint the raw call, not the coerced `call` above: the coercion
828+
// inside denialFingerprint is idempotent, so both forms share one key,
829+
// and the raw form is what evaluate() and resolveSuspended() also derive
830+
// from — keeping the three decline paths on the identical reason.
831+
const stableId = denialFingerprint(rawCall, effectiveCwd);
821832
const cachedDenial = denialMemory.isDenied(stableId);
822833
if (cachedDenial !== undefined)
823834
return { kind: "deny", reason: cachedDenial };
@@ -1121,8 +1132,8 @@ export function createPermissionGate(
11211132
// calls never pass through the reactor (sub-agents). When the gate is
11221133
// reactor-gated, gateToolCall uses executionVerdict instead of evaluate() so
11231134
// deny still blocks and ask never re-prompts (see gateToolCall).
1124-
const evaluate = async (call: ToolCall): Promise<GateVerdict> => {
1125-
const decision = await decide(call);
1135+
const evaluate = async (rawCall: ToolCall): Promise<GateVerdict> => {
1136+
const decision = await decide(rawCall);
11261137
if (decision.kind === "allow") return { allowed: true };
11271138
if (decision.kind === "deny")
11281139
return { allowed: false, reason: decision.reason };
@@ -1133,14 +1144,15 @@ export function createPermissionGate(
11331144
// same stable fingerprint) denies with the identical reason instead of
11341145
// re-asking. Timeouts, aborts, and missing outcomes are never cached —
11351146
// the operator made no decision, so the retry must ask again (mirrors
1136-
// resolveSuspended).
1147+
// resolveSuspended). The key derives from the raw call through the one
1148+
// canonical coercion, so a coerced-shape retry hits the same entry.
11371149
if (
11381150
outcome !== undefined &&
11391151
!outcome.allow &&
11401152
classifyOutcome(outcome) === "deny"
11411153
) {
11421154
denialMemory.record(
1143-
denialFingerprint(call, getSubAgentIdentity()?.cwd ?? resolvedCwd),
1155+
denialFingerprint(rawCall, getSubAgentIdentity()?.cwd ?? resolvedCwd),
11441156
reason,
11451157
);
11461158
}
@@ -1241,14 +1253,28 @@ export function createPermissionGate(
12411253
// reason instead of re-prompting. Timeouts, aborts, and missing outcomes
12421254
// are never cached — the operator made no decision, so the retry must
12431255
// ask again. Invalidation (grant-mint/mode/identity clears) is unchanged.
1256+
// Rebuild the denied ToolCall from the suspended request through the
1257+
// one canonical helper decide()/evaluate() use. The suspended request
1258+
// carries post-coercion run_shell {command} only — buildRequests drops
1259+
// the workdir-scoped cwd — so the recorded key is {command}-keyed while
1260+
// a same-workdir retry's decide() key is {command, cwd}-keyed: the retry
1261+
// misses and re-asks (fail-safe, never a bypass). Conversely a cwd-less
1262+
// retry of the same command hits the workdir record and denies
1263+
// (fail-closed over-deny). The synthesized id never participates
1264+
// (stableRequestId ignores call.id and correlationId).
12441265
if (
12451266
outcome !== undefined &&
12461267
!outcome.allow &&
12471268
classifyOutcome(outcome) === "deny"
12481269
) {
1270+
const suspendedCall = callForIdentity({
1271+
id: "",
1272+
name: request.tool,
1273+
arguments: request.arguments ?? {},
1274+
});
12491275
denialMemory.record(
12501276
denialFingerprint(
1251-
{ id: "", name: request.tool, arguments: request.arguments ?? {} },
1277+
suspendedCall,
12521278
request.cwd ?? getSubAgentIdentity()?.cwd ?? resolvedCwd,
12531279
),
12541280
declineReason(request, outcome),

‎src/permission/shell-argv-authorize.test.ts‎

Lines changed: 205 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { describe, expect, test } from "bun:test";
2+
import { join } from "node:path";
23
import type { ToolCall } from "@intx/types/runtime";
34

4-
import { prepareDispatchedToolCall } from "../agent/tool-aliases.js";
5+
import {
6+
looksLikeCodexShellArgs,
7+
prepareDispatchedToolCall,
8+
} from "../agent/tool-aliases.js";
59
import { gateToolCall } from "../plugins/permission-plugin.js";
610
import { BLOCKED_BY_POLICY_PREFIX } from "./decline-markers.js";
711
import { createPermissionGate } from "./gate.js";
@@ -17,6 +21,33 @@ function hiddenShellArgv(id: string): ToolCall {
1721
};
1822
}
1923

24+
function hiddenShellArgvWorkdir(id: string, workdir: string): ToolCall {
25+
return {
26+
id,
27+
name: "shell",
28+
arguments: { command: ["bash", "-lc", DESTRUCTIVE], workdir },
29+
};
30+
}
31+
32+
function countingDenyGate(reactorGated: boolean): {
33+
gate: ReturnType<typeof createPermissionGate>;
34+
asked: () => number;
35+
} {
36+
let asked = 0;
37+
const gate = createPermissionGate({
38+
approvals: [],
39+
cwd: process.cwd(),
40+
requestApproval: async () => {
41+
asked++;
42+
return { allow: false };
43+
},
44+
interactive: true,
45+
skipPermissions: false,
46+
reactorGated,
47+
});
48+
return { gate, asked: () => asked };
49+
}
50+
2051
describe("reactor-gated hidden shell argv coerce-before-authorize", () => {
2152
test("command string[] does not auto-allow; unwrapped script is the ask subject; ask does not execute", async () => {
2253
const gate = createPermissionGate({
@@ -60,3 +91,176 @@ describe("reactor-gated hidden shell argv coerce-before-authorize", () => {
6091
expect(nextCalled).toBe(false);
6192
});
6293
});
94+
95+
describe("hidden-shell decline fingerprint (CL-9341)", () => {
96+
test("prepareDispatchedToolCall is idempotent on coerced run_shell", () => {
97+
const coerced = prepareDispatchedToolCall(
98+
hiddenShellArgv("idem-0"),
99+
"run_shell",
100+
);
101+
expect(coerced.name).toBe("run_shell");
102+
expect(coerced.arguments.command).toBe(DESTRUCTIVE);
103+
expect(prepareDispatchedToolCall(coerced, "run_shell")).toBe(coerced);
104+
const native: ToolCall = {
105+
id: "idem-native",
106+
name: "run_shell",
107+
arguments: { command: DESTRUCTIVE, cwd: "/tmp", timeout: 5 },
108+
};
109+
expect(prepareDispatchedToolCall(native, "run_shell")).toBe(native);
110+
expect(
111+
looksLikeCodexShellArgs({
112+
command: DESTRUCTIVE,
113+
cwd: "/tmp",
114+
timeout: 5,
115+
}),
116+
).toBe(false);
117+
expect(
118+
looksLikeCodexShellArgs({ command: DESTRUCTIVE, workdir: "/tmp" }),
119+
).toBe(true);
120+
});
121+
122+
test("middleware hidden-shell-array decline suppresses the coerced retry with the identical reason", async () => {
123+
const { gate, asked } = countingDenyGate(false);
124+
const first = await gate.evaluate(hiddenShellArgv("shell-decline-1"));
125+
if (first.allowed) throw new Error("expected the hidden shell declined");
126+
expect(asked()).toBe(1);
127+
const coerced = prepareDispatchedToolCall(
128+
hiddenShellArgv("shell-decline-2"),
129+
"run_shell",
130+
);
131+
expect(coerced.arguments.command).toBe(DESTRUCTIVE);
132+
const retry = await gate.evaluate(coerced);
133+
if (retry.allowed)
134+
throw new Error("expected the coerced retry denied from denial memory");
135+
expect(asked()).toBe(1);
136+
expect(retry.reason).toBe(first.reason);
137+
const rawRetry = await gate.evaluate(hiddenShellArgv("shell-decline-3"));
138+
if (rawRetry.allowed)
139+
throw new Error("expected the raw retry denied from denial memory");
140+
expect(asked()).toBe(1);
141+
expect(rawRetry.reason).toBe(first.reason);
142+
});
143+
144+
test("middleware hidden-shell workdir decline suppresses the cwd-coerced retry", async () => {
145+
// In-workspace so the coerced cwd clears path-escape and reaches the
146+
// operator (an outside-workspace workdir is denied outright, never
147+
// asked). Uncreated on disk: the gate never executes here.
148+
const workdir = join(process.cwd(), "tmp-corbits-decline-wd");
149+
const { gate, asked } = countingDenyGate(false);
150+
const first = await gate.evaluate(
151+
hiddenShellArgvWorkdir("shell-wd-1", workdir),
152+
);
153+
if (first.allowed)
154+
throw new Error("expected the hidden workdir shell declined");
155+
expect(asked()).toBe(1);
156+
const coerced = prepareDispatchedToolCall(
157+
hiddenShellArgvWorkdir("shell-wd-2", workdir),
158+
"run_shell",
159+
);
160+
expect(coerced.arguments).toEqual({ command: DESTRUCTIVE, cwd: workdir });
161+
const retry = await gate.evaluate(coerced);
162+
if (retry.allowed)
163+
throw new Error(
164+
"expected the cwd-coerced retry denied from denial memory",
165+
);
166+
expect(asked()).toBe(1);
167+
expect(retry.reason).toBe(first.reason);
168+
});
169+
170+
test("reactor hidden-shell-array decline suppresses raw and coerced retries with the middleware-identical reason", async () => {
171+
const { gate, asked } = countingDenyGate(true);
172+
const suspended = await gate.authorizeCall(hiddenShellArgv("shell-r-1"));
173+
if (suspended.effect !== "ask")
174+
throw new Error("expected the hidden shell to suspend for approval");
175+
const outcome = await gate.resolveSuspended(suspended.request);
176+
expect(outcome?.allow).toBe(false);
177+
expect(asked()).toBe(1);
178+
const rawRetry = await gate.authorizeCall(hiddenShellArgv("shell-r-2"));
179+
if (rawRetry.effect !== "deny")
180+
throw new Error("expected the raw retry denied from denial memory");
181+
expect(asked()).toBe(1);
182+
const coercedRetry = await gate.authorizeCall(
183+
prepareDispatchedToolCall(hiddenShellArgv("shell-r-3"), "run_shell"),
184+
);
185+
if (coercedRetry.effect !== "deny")
186+
throw new Error("expected the coerced retry denied from denial memory");
187+
expect(asked()).toBe(1);
188+
expect(coercedRetry.reason).toBe(rawRetry.reason);
189+
const { gate: middleware } = countingDenyGate(false);
190+
const verdict = await middleware.evaluate(hiddenShellArgv("shell-r-4"));
191+
if (verdict.allowed)
192+
throw new Error("expected the middleware hidden shell declined");
193+
expect(rawRetry.reason).toBe(verdict.reason);
194+
});
195+
196+
test("reactor workdir decline does not suppress the same-workdir raw retry (fail-safe re-ask)", async () => {
197+
// In-workspace so the workdir call reaches the operator (an
198+
// outside-workspace workdir is denied outright, never asked). The
199+
// suspended request carries {command} only (buildRequests drops the
200+
// workdir cwd), so the recorded denial key misses the same-workdir
201+
// retry's {command, cwd} decide() key and the retry suspends again.
202+
const workdir = join(process.cwd(), "tmp-corbits-decline-wd");
203+
const { gate, asked } = countingDenyGate(true);
204+
const suspended = await gate.authorizeCall(
205+
hiddenShellArgvWorkdir("shell-rwd-1", workdir),
206+
);
207+
if (suspended.effect !== "ask")
208+
throw new Error("expected the workdir shell to suspend for approval");
209+
const outcome = await gate.resolveSuspended(suspended.request);
210+
expect(outcome?.allow).toBe(false);
211+
expect(asked()).toBe(1);
212+
const rawRetry = await gate.authorizeCall(
213+
hiddenShellArgvWorkdir("shell-rwd-2", workdir),
214+
);
215+
if (rawRetry.effect !== "ask")
216+
throw new Error("expected the same-workdir retry to re-ask fail-safe");
217+
expect(asked()).toBe(1);
218+
const retryOutcome = await gate.resolveSuspended(rawRetry.request);
219+
expect(retryOutcome?.allow).toBe(false);
220+
expect(asked()).toBe(2);
221+
});
222+
223+
test("reactor workdir decline suppresses the cwd-less same-command retry (fail-closed over-deny)", async () => {
224+
// Companion direction: the {command}-keyed workdir record matches a
225+
// cwd-less retry of the same command, so it denies without re-asking.
226+
const workdir = join(process.cwd(), "tmp-corbits-decline-wd");
227+
const { gate, asked } = countingDenyGate(true);
228+
const suspended = await gate.authorizeCall(
229+
hiddenShellArgvWorkdir("shell-rwdc-1", workdir),
230+
);
231+
if (suspended.effect !== "ask")
232+
throw new Error("expected the workdir shell to suspend for approval");
233+
const outcome = await gate.resolveSuspended(suspended.request);
234+
expect(outcome?.allow).toBe(false);
235+
expect(asked()).toBe(1);
236+
const cwdLessRetry = await gate.authorizeCall({
237+
id: "shell-rwdc-2",
238+
name: "run_shell",
239+
arguments: { command: DESTRUCTIVE },
240+
});
241+
if (cwdLessRetry.effect !== "deny")
242+
throw new Error("expected the cwd-less retry denied from denial memory");
243+
expect(asked()).toBe(1);
244+
});
245+
246+
test("plain run_shell decline still suppresses the fresh-id retry", async () => {
247+
const { gate, asked } = countingDenyGate(false);
248+
const args = { command: DESTRUCTIVE };
249+
const first = await gate.evaluate({
250+
id: "shell-plain-1",
251+
name: "run_shell",
252+
arguments: args,
253+
});
254+
if (first.allowed) throw new Error("expected the plain shell declined");
255+
expect(asked()).toBe(1);
256+
const retry = await gate.evaluate({
257+
id: "shell-plain-2",
258+
name: "run_shell",
259+
arguments: args,
260+
});
261+
if (retry.allowed)
262+
throw new Error("expected the plain retry denied from denial memory");
263+
expect(asked()).toBe(1);
264+
expect(retry.reason).toBe(first.reason);
265+
});
266+
});

0 commit comments

Comments
 (0)