Skip to content

Commit 4b9a822

Browse files
committed
Key reactor-gated verdict cache by call identity
A reused Codex proxy id could apply an outer shell allow to an inner run_shell deny. Consume a cached verdict only when name and arguments match as well as id.
1 parent 606ded2 commit 4b9a822

5 files changed

Lines changed: 123 additions & 28 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,8 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1919
(authorization hard-deny, auto-shell deny, or headless deny) returns a
2020
blocked tool error and does not run the call. Ask and allow still skip the
2121
middleware prompt so an approved re-dispatch never re-asks. Middleware is
22-
not a second copy of `env.authorize`: it consumes the prior verdict when one
23-
exists, and decides only for inner posix runs whose outer tool is not
24-
`run_shell` (Codex apply_patch proxy).
22+
not a second copy of `env.authorize`: it consumes the prior verdict when the
23+
same call (id, name, and arguments) is cached, and decides on a cache miss.
2524

2625
## [0.3.18] - 2026-09-08
2726

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ tool call
377377
- **command** — Splits chained commands for security classification and derives command-shape approval scopes. Multi-segment chains only offer an exact-command persist pattern (a prefix like `npm *` must not cover `npm i && rm -rf /` later).
378378
- **auto-shell-policy** — Constrains `run_shell` even when auto mode would otherwise rubber-stamp it. Before matching, `expandShellSubjects` peels `bash`/`sh`/`zsh -c`, `xargs` utility tails, and transparent prefixes (`env`, `nice`, `timeout`, …) so rules see the real payload; an unparseable wrapper (variable expansion or command substitution) sets an opaque flag that forces `ask`. Effects: `deny` blocks outright (file mutations through ad-hoc tooling — output redirection, `tee`, `sed -i`/`perl -i`, interpreter inline programs or heredocs — which must instead go through `write_file`/`edit_file`); `ask` declines to auto-allow and falls through to the operator prompt (recursive `rm`, dependency installs and remote runners: npm/yarn/pnpm/bun, pip, cargo, go, brew, npx/bunx, …, force or uncontained `git worktree` ops, shell that references a sensitive path such as `.env` or a private key, and opaque wrappers). Contained non-force `git worktree add`/`remove`/`prune` and read-only `list` auto-allow (sibling destinations like `../corbits-dispatch-wts/…` included; absolute outside, `~`, globs, and credential basenames still ask). Deny beats ask when multiple subjects match. Quoted spans are stripped before pattern matching so a quoted `>` or install word in an argument is not flagged, and program names are matched only in command position. Adding a table category is a one-line rule append in `AUTO_SHELL_RULES`.
379379
- **gate** — Evaluates a call: `skipPermissions` allows everything; `allow`-tier passes; for `ask`-tier, checks persisted approvals, otherwise requests operator approval. Shell security classifies each chain segment (`||` / `&&` / `|` / `;` / newlines), but the operator is prompted once for the full command block — any unapproved segment fails the whole block, and execution always runs the unsplit original. Safe pipeline tails and pure shell no-ops (`true` / `false` / `:` and bare control-flow keywords stranded by chain-splitting) skip without a prompt. In a non-interactive run an unresolved `ask` becomes a denial. In auto mode: non-shell built-ins in `AUTO_ALLOWED_TOOLS` (writes/edits/deletes, `manage_tasks`, `spawn_agent`, `wait_agents`, …) auto-allow when not path-restricted; for `run_shell` the gate consults the auto-shell policy — a `deny` rule fails the call, an `ask` rule skips the auto-allow shortcut and proceeds to the normal approval flow, and anything unmatched is auto-allowed. Paths outside the workspace and writes under the session state root (`~/.corbits/projects/...` and legacy `.agent-state`) still ask under auto mode. Under `--dangerously-skip-permissions` (forces this process) or `/yolo` (persists as the user-global default via `setSkipPermissions`), the gate auto-allows those same cases, and pre-gate sandboxes (path-escape, shell session cwd retention, `list_dir` / `delete_file` workspace bounds) honor `getSkipPermissions()` live so outside-workspace access is not hard-denied after the gate already allowed it — without rebuilding the plugin stack. Secret-guard path denies and authorization hard blocks still apply. Mutating MCP and unknown built-ins are not blanket-allowed outside skip. Newly granted scopes are appended in memory and persisted.
380-
- **Reactor-gated sessions (main session; `reactorGated: true`).** The gate's decision logic lives in one `decide()` used by both consumers: `evaluate()` (the middleware path below, still used by sub-agents) and `authorizeCall()`, which expresses the decision as the vendored reactor's before-tool authz effect (`src/permission/reactor-authorize.ts` bridges it into `env.authorize`). An `ask` there suspends the call as a reactor `PendingOperation` keyed by a correlationId (persisted through the context store's existing `pendingOperations`); `send()` settles as `suspended` and `src/session/approval-resume.ts` rebuilds the operator request from the approval snapshot, resolves it through the same `requestApproval` seam the TUI overlay uses, and delivers the decision to the reactor on the correlationId signal channel — an approved decision grants a one-shot bypass and the exact parked call re-dispatches; a rejected one answers it with an error result. Under reactor gating the middleware/MCP `gateToolCall` is an execution backstop, not a second copy of `env.authorize`: it consumes the `authorizeCall` verdict when one exists and does not re-decide. Deny still blocks and does not call `next`; an `ask` or `allow` skips the middleware prompt so an approved re-dispatch never re-asks. The remaining hole is inner posix runs whose outer tool is not `run_shell` (Codex `apply_patch` proxy uses a reused `codex-proxy` call id): those never pass `env.authorize`, so `gateToolCall` decides only on that cache miss and still blocks a deny. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`.
380+
- **Reactor-gated sessions (main session; `reactorGated: true`).** The gate's decision logic lives in one `decide()` used by both consumers: `evaluate()` (the middleware path below, still used by sub-agents) and `authorizeCall()`, which expresses the decision as the vendored reactor's before-tool authz effect (`src/permission/reactor-authorize.ts` bridges it into `env.authorize`). An `ask` there suspends the call as a reactor `PendingOperation` keyed by a correlationId (persisted through the context store's existing `pendingOperations`); `send()` settles as `suspended` and `src/session/approval-resume.ts` rebuilds the operator request from the approval snapshot, resolves it through the same `requestApproval` seam the TUI overlay uses, and delivers the decision to the reactor on the correlationId signal channel — an approved decision grants a one-shot bypass and the exact parked call re-dispatches; a rejected one answers it with an error result. Under reactor gating the middleware/MCP `gateToolCall` is an execution backstop, not a second copy of `env.authorize`: it consumes the `authorizeCall` verdict only when id, name, and arguments match, and does not re-decide. Deny still blocks and does not call `next`; an `ask` or `allow` skips the middleware prompt so an approved re-dispatch never re-asks. A reused `codex-proxy` id cannot apply an outer `shell` allow to an inner `run_shell` deny. Inner posix runs whose outer tool is not `run_shell` (Codex `apply_patch` proxy) never pass `env.authorize`, so `gateToolCall` decides on that cache miss and still blocks a deny. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`.
381381

382382
- **matcher** — Approval pattern matching via `@intx/authz` `matchPattern` (`*` wildcards). Exact-command grants store a backslash before each metacharacter; those patterns match by equality after unescape (the package has no escape syntax).
383383
- **authz-grants** — Maps stored approvals into `@intx/authz` `GrantRule`s and evaluates them with `evaluateGrants` (allow-only; Corbits cwd/provider-model filters applied first). Exact-escaped grants bypass the package path and use equality.

src/permission/gate.ts

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -318,16 +318,18 @@ export interface PermissionGate {
318318
// vendored before-tool authz hook consumes (see authorizeCall above).
319319
authorizeCall: (call: ToolCall) => Promise<AuthorizeVerdict>;
320320
// Execution-time backstop for reactor-gated posix/MCP middleware: consume the
321-
// authorizeCall verdict when one exists; decide only when there is no prior
322-
// verdict (nested posix whose outer tool is not run_shell, and tests).
321+
// authorizeCall verdict when the same call identity (id, name, arguments) is
322+
// cached; decide only on a miss (nested posix whose outer tool is not
323+
// run_shell, colliding reused ids, and tests).
323324
executionVerdict: (call: ToolCall) => Promise<AuthorizeVerdict>;
324325
// Resolve a suspended reactor approval against the operator (and mint the
325326
// outcome's grant). Returns undefined when no outcome arrived.
326327
resolveSuspended: (request: PermissionRequest) => Promise<ApprovalOutcome | undefined>;
327-
// True when this gate's decisions are consumed by the reactor's authz seam
328-
// (env.authorize) rather than by evaluate() in the tool-runner middleware.
329-
// Under reactor gating, gateToolCall still blocks decide() deny; ask/allow
330-
// skip the middleware prompt so an approved re-dispatch never re-asks.
328+
// True when this gate's decisions go through env.authorize (authorizeCall)
329+
// rather than evaluate() in the tool-runner middleware. Under reactor gating,
330+
// gateToolCall is an execution backstop: it consumes a matching cached
331+
// verdict and decides on a miss. Deny still blocks; ask/allow skip the
332+
// middleware prompt so an approved re-dispatch never re-asks.
331333
isReactorGated: () => boolean;
332334
// The gate's current in-memory approvals, including any granted this session.
333335
getApprovals: () => readonly Approval[];
@@ -490,9 +492,15 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
490492

491493
// Consume-once handoff from env.authorize to execution-time middleware.
492494
// Not session-lifetime uniqueness: call.id is reused for every Codex proxy
493-
// inner posix op, so a lasting set would mute later JSONL records. reset()
494-
// clears leftovers (outer tools that never hit posix middleware).
495-
const authorizedByCallId = new Map<string, AuthorizeVerdict>();
495+
// inner posix op, so a lasting set would mute later JSONL records. A hit
496+
// still requires matching name and arguments so a reused id cannot apply an
497+
// outer allow to a different inner tool. Nested posix with the same id still
498+
// consume-once when identity matches. reset() clears leftovers (outer tools
499+
// that never hit posix middleware).
500+
const authorizedByCallId = new Map<
501+
string,
502+
{ name: string; arguments: ToolCall["arguments"]; verdict: AuthorizeVerdict }
503+
>();
496504

497505
// Non-blocking policy decision for one tool call: everything the gate owns —
498506
// tier pre-filter, auto rules, pre-grant guards, grants, headless denial —
@@ -766,15 +774,23 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
766774

767775
const authorizeCall = async (call: ToolCall): Promise<AuthorizeVerdict> => {
768776
const verdict = mapAuthorizeVerdict(await decide(call));
769-
authorizedByCallId.set(call.id, verdict);
777+
authorizedByCallId.set(call.id, {
778+
name: call.name,
779+
arguments: call.arguments,
780+
verdict,
781+
});
770782
return verdict;
771783
};
772784

773785
const executionVerdict = async (call: ToolCall): Promise<AuthorizeVerdict> => {
774786
const cached = authorizedByCallId.get(call.id);
775-
if (cached !== undefined) {
787+
if (
788+
cached !== undefined &&
789+
cached.name === call.name &&
790+
JSON.stringify(cached.arguments) === JSON.stringify(call.arguments)
791+
) {
776792
authorizedByCallId.delete(call.id);
777-
return cached;
793+
return cached.verdict;
778794
}
779795
return mapAuthorizeVerdict(await decide(call));
780796
};

src/plugins/permission-plugin.test.ts

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,89 @@ describe("gateToolCall", () => {
312312
expect(records[1]?.outcome).toBe("auto-deny");
313313
});
314314

315-
test("reset does not mute a later auto-decision with a reused call.id", async () => {
315+
test("colliding reused call.id does not inherit allow onto a different tool", async () => {
316+
const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-"));
317+
const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-"));
318+
const gate = createPermissionGate({
319+
approvals: [{ tool: "shell", pattern: "shell" }],
320+
interactive: true,
321+
skipPermissions: false,
322+
reactorGated: true,
323+
auto: true,
324+
cwd,
325+
approvalLog: createApprovalLog(dir),
326+
requestApproval: async () => {
327+
throw new Error("requestApproval must not be invoked under reactor gating");
328+
},
329+
});
330+
const outer: ToolCall = {
331+
id: "codex-proxy",
332+
name: "shell",
333+
arguments: { command: "echo x | tee src/a.ts" },
334+
};
335+
const inner: ToolCall = {
336+
id: "codex-proxy",
337+
name: "run_shell",
338+
arguments: { command: "echo x | tee src/a.ts" },
339+
};
340+
expect((await gate.authorizeCall(outer)).effect).toBe("allow");
341+
const { next, wasCalled } = trackingNext();
342+
const result = await gateToolCall(gate, inner, new AbortController().signal, next);
343+
expect(result.isError).toBe(true);
344+
expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX);
345+
expect(wasCalled()).toBe(false);
346+
await new Promise((r) => setTimeout(r, 10));
347+
const records = readApprovalRecords(dir);
348+
expect(records).toHaveLength(1);
349+
expect(records[0]?.outcome).toBe("auto-deny");
350+
});
351+
352+
test("authorizeCall apply_patch then nested posix with reused id each record", async () => {
353+
const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-"));
354+
const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-"));
355+
const gate = createPermissionGate({
356+
approvals: [],
357+
interactive: true,
358+
skipPermissions: false,
359+
reactorGated: true,
360+
auto: true,
361+
cwd,
362+
approvalLog: createApprovalLog(dir),
363+
requestApproval: async () => {
364+
throw new Error("requestApproval must not be invoked under reactor gating");
365+
},
366+
});
367+
const outer: ToolCall = {
368+
id: "apply-1",
369+
name: "apply_patch",
370+
arguments: { input: "*** Begin Patch\n*** Add File: src/a.ts\n+x\n*** End Patch\n" },
371+
};
372+
const first: ToolCall = {
373+
id: "codex-proxy",
374+
name: "write_file",
375+
arguments: { path: "src/a.ts", content: "x" },
376+
};
377+
const second: ToolCall = {
378+
id: "codex-proxy",
379+
name: "write_file",
380+
arguments: { path: "src/b.ts", content: "y" },
381+
};
382+
expect((await gate.authorizeCall(outer)).effect).toBe("allow");
383+
const { next, wasCalled } = trackingNext();
384+
expect((await gateToolCall(gate, first, new AbortController().signal, next)).isError).not.toBe(
385+
true,
386+
);
387+
expect((await gateToolCall(gate, second, new AbortController().signal, next)).isError).not.toBe(
388+
true,
389+
);
390+
expect(wasCalled()).toBe(true);
391+
await new Promise((r) => setTimeout(r, 10));
392+
const records = readApprovalRecords(dir);
393+
expect(records).toHaveLength(3);
394+
expect(records.map((r) => r.outcome)).toEqual(["auto-allow", "auto-allow", "auto-allow"]);
395+
});
396+
397+
test("leftover authorizeCall is cleared by reset so a later gateToolCall records", async () => {
316398
const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-"));
317399
const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-"));
318400
const gate = createPermissionGate({
@@ -333,14 +415,11 @@ describe("gateToolCall", () => {
333415
arguments: { path: "src/a.ts", content: "x" },
334416
};
335417
expect((await gate.authorizeCall(call)).effect).toBe("allow");
336-
const firstRun = trackingNext();
337-
await gateToolCall(gate, call, new AbortController().signal, firstRun.next);
338-
expect(firstRun.wasCalled()).toBe(true);
339418
gate.reset();
340-
expect((await gate.authorizeCall(call)).effect).toBe("allow");
341-
const secondRun = trackingNext();
342-
await gateToolCall(gate, call, new AbortController().signal, secondRun.next);
343-
expect(secondRun.wasCalled()).toBe(true);
419+
const { next, wasCalled } = trackingNext();
420+
const result = await gateToolCall(gate, call, new AbortController().signal, next);
421+
expect(result.isError).not.toBe(true);
422+
expect(wasCalled()).toBe(true);
344423
await new Promise((r) => setTimeout(r, 10));
345424
const records = readApprovalRecords(dir);
346425
expect(records).toHaveLength(2);

src/plugins/permission-plugin.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@ function blockedByPolicy(call: ToolCall, reason: string): ToolResult {
1616
// the posix runner the middleware wraps) so both produce the same denial result.
1717
//
1818
// Under reactor gating this is an execution backstop, not a second env.authorize.
19-
// Consume the prior authorizeCall verdict when one exists; decide only when there
20-
// is no prior verdict — nested posix whose outer tool is not run_shell (Codex
21-
// apply_patch proxy) and tests. Deny blocks next; ask/allow skip the middleware
22-
// prompt so an approved re-dispatch never re-asks.
19+
// Consume the prior authorizeCall verdict when the same call identity (id, name,
20+
// arguments) is cached; decide only on a miss — nested posix whose outer tool is
21+
// not run_shell (Codex apply_patch proxy), colliding reused ids, and tests. Deny
22+
// blocks next; ask/allow skip the middleware prompt so an approved re-dispatch
23+
// never re-asks.
2324
export async function gateToolCall(
2425
gate: PermissionGate,
2526
call: ToolCall,

0 commit comments

Comments
 (0)