From c35e3882ce33357bb12034f5189560d2d2f0ce6f Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 8 Sep 2026 20:55:59 -0700 Subject: [PATCH 1/6] Block policy denials in reactor-gated tool middleware --- CHANGELOG.md | 9 ++ docs/ARCHITECTURE.md | 2 +- src/permission/gate.ts | 21 ++-- src/plugins/permission-plugin.test.ts | 168 ++++++++++++++++++++++++++ src/plugins/permission-plugin.ts | 17 ++- 5 files changed, 201 insertions(+), 16 deletions(-) create mode 100644 src/plugins/permission-plugin.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ab36cd39c..ca342255e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,15 @@ matching `## [X.Y.Z]` section (plus install instructions). Do not maintain parallel copies under `docs/` or `scripts/notes/`. At cut time: rename `## [Unreleased]` to `## [X.Y.Z] - YYYY-MM-DD`, then run the release script. +## [Unreleased] + +### Security + +- Reactor-gated tool middleware still blocks policy denials. A `decide()` deny + (authorization hard-deny, auto-shell deny, or headless deny) returns a + blocked tool error and does not run the call. Ask and allow still skip the + middleware prompt so an approved re-dispatch never re-asks. + ## [0.3.18] - 2026-09-08 ### Added diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8b476ef4d..e0a224d0c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -377,7 +377,7 @@ tool call - **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). - **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`. - **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. - - **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` bypasses the gate so an approved re-dispatch never re-asks. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`. + - **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` still blocks a `decide()` deny (authz hard-deny, auto-shell deny, headless deny) and does not call `next`; an `ask` or `allow` skips the middleware prompt so an approved re-dispatch never re-asks. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`. - **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). - **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. diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 03e90f42d..3d888350c 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -295,10 +295,11 @@ export interface PermissionGateOptions { // silent by construction. telemetry?: Telemetry | undefined; // This gate's decisions are consumed by the reactor's before-tool authz - // seam (env.authorize) instead of the tool-runner middleware. Set for the - // main session so approved re-dispatches skip the middleware gate; kept - // false for sub-agents, which still gate in the middleware. Required so a - // caller cannot silently fall back to middleware gating by omitting it. + // seam (env.authorize) instead of evaluate() in the tool-runner middleware. + // Set for the main session so approved re-dispatches skip the middleware + // prompt; kept false for sub-agents, which still gate via evaluate(). + // Required so a caller cannot silently fall back to middleware gating by + // omitting it. reactorGated: boolean; // Ask/settle event log (see approval-log.ts): one record per consequential // decision, auto or interactive. Defaults to a no-op so nothing depends on @@ -321,9 +322,9 @@ export interface PermissionGate { // outcome's grant). Returns undefined when no outcome arrived. resolveSuspended: (request: PermissionRequest) => Promise; // True when this gate's decisions are consumed by the reactor's authz seam - // (env.authorize) rather than by the tool-runner middleware. Tool-runner - // gating (gateToolCall) is bypassed under reactor gating so an approved - // re-dispatch runs without a second prompt. + // (env.authorize) rather than by evaluate() in the tool-runner middleware. + // Under reactor gating, gateToolCall still blocks decide() deny; ask/allow + // skip the middleware prompt so an approved re-dispatch never re-asks. isReactorGated: () => boolean; // The gate's current in-memory approvals, including any granted this session. getApprovals: () => readonly Approval[]; @@ -718,9 +719,9 @@ export function createPermissionGate(options: PermissionGateOptions): Permission }; // Middleware path: blocking evaluation used by tool-runner consumers whose - // calls never pass through the reactor (sub-agents, late MCP wrappers). - // When the gate is reactor-gated this is bypassed entirely — the reactor's - // before-tool authz hook owns the decision (see authorizeCall / gateToolCall). + // calls never pass through the reactor (sub-agents). When the gate is + // reactor-gated, gateToolCall uses authorizeCall 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); if (decision.kind === "allow") return { allowed: true }; diff --git a/src/plugins/permission-plugin.test.ts b/src/plugins/permission-plugin.test.ts new file mode 100644 index 000000000..4215ddaa0 --- /dev/null +++ b/src/plugins/permission-plugin.test.ts @@ -0,0 +1,168 @@ +import { describe, expect, test } from "bun:test"; +import type { ToolCall, ToolResult } from "@intx/types/runtime"; + +import { BLOCKED_BY_POLICY_PREFIX } from "../permission/decline-markers.js"; +import { createPermissionGate } from "../permission/gate.js"; +import { gateToolCall, permissionPlugin } from "./permission-plugin.js"; + +function shellCall(command: string): ToolCall { + return { + id: "test-call", + name: "run_shell", + arguments: { command }, + }; +} + +function trackingNext() { + let called = false; + const next = async (call: ToolCall, _signal: AbortSignal): Promise => { + called = true; + return { callId: call.id, content: "ok" }; + }; + return { + next, + wasCalled: () => called, + }; +} + +describe("gateToolCall", () => { + test("reactor-gated authz hard-deny blocks and skips next", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: false, + reactorGated: true, + }); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall( + gate, + shellCall("rm -rf /"), + new AbortController().signal, + next, + ); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + }); + + test("reactor-gated auto-shell deny blocks without asking", async () => { + let asked = 0; + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + requestApproval: async () => { + asked++; + return { allow: true }; + }, + }); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall( + gate, + shellCall("echo x | tee src/a.ts"), + new AbortController().signal, + next, + ); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + expect(asked).toBe(0); + }); + + test("reactor-gated headless deny blocks and skips next", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: false, + reactorGated: true, + }); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall( + gate, + shellCall("curl https://example.com"), + new AbortController().signal, + next, + ); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + }); + + test("reactor-gated ask skips the prompt and still calls next", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: true, + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall( + gate, + shellCall("curl https://example.com"), + new AbortController().signal, + next, + ); + expect(result.isError).not.toBe(true); + expect(wasCalled()).toBe(true); + }); + + test("reactor-gated allow-tier still calls next", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: false, + reactorGated: true, + }); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall( + gate, + { id: "test-call", name: "read_file", arguments: { path: "src/a.ts" } }, + new AbortController().signal, + next, + ); + expect(result.isError).not.toBe(true); + expect(wasCalled()).toBe(true); + }); + + test("sub-agent path still evaluates and denies authz hard-deny", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: false, + reactorGated: false, + }); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall( + gate, + shellCall("rm -rf /"), + new AbortController().signal, + next, + ); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + }); +}); + +describe("permissionPlugin", () => { + test("middleware blocks reactor-gated authz hard-deny", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: false, + reactorGated: true, + }); + const { next, wasCalled } = trackingNext(); + const plugin = permissionPlugin(gate); + const handler = plugin.middleware ? plugin.middleware(next) : next; + const result = await handler(shellCall("rm -rf /"), new AbortController().signal); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + }); +}); diff --git a/src/plugins/permission-plugin.ts b/src/plugins/permission-plugin.ts index b93607afa..e67b4272d 100644 --- a/src/plugins/permission-plugin.ts +++ b/src/plugins/permission-plugin.ts @@ -7,11 +7,10 @@ import type { PermissionGate } from "../permission/gate.js"; // the posix middleware and the late-connected MCP tools (which are not part of // the posix runner the middleware wraps) so both produce the same denial result. // -// Under reactor gating the bypass is unconditional: the reactor's before-tool -// authz hook already ran this gate's policy (evaluate and authorizeCall share -// one decide), and an approved re-dispatch arrives here with the one-shot -// bypass consumed — a second gate would re-ask the operator for a call the -// reactor already approved. +// Under reactor gating, authorizeCall still enforces decide() deny (authz, +// auto-shell, headless). Ask/allow skip the middleware prompt so an approved +// re-dispatch never re-asks — evaluate() is not used here because it would +// prompt again. export async function gateToolCall( gate: PermissionGate, call: ToolCall, @@ -19,6 +18,14 @@ export async function gateToolCall( next: (call: ToolCall, signal: AbortSignal) => Promise, ): Promise { if (gate.isReactorGated()) { + const verdict = await gate.authorizeCall(call); + if (verdict.effect === "deny") { + return { + callId: call.id, + content: `${BLOCKED_BY_POLICY_PREFIX}${verdict.reason}`, + isError: true, + }; + } return next(call, signal); } const verdict = await gate.evaluate(call); From bf3a399a426bcc9d9ae7dae8a97614a7ccb7c846 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 8 Sep 2026 21:15:35 -0700 Subject: [PATCH 2/6] Record each reactor-gated decision once --- src/permission/gate.ts | 19 ++++++++--- src/plugins/permission-plugin.test.ts | 49 +++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 3d888350c..57e0031d7 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -471,11 +471,20 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // settled all collapse to now. Interactive prompts use approvalLog.ask // directly (see below) so their real queued/displayed/settled timestamps // are captured. + // + // Recording owns uniqueness: reactor-gated calls run decide() twice (env.authorize + // then gateToolCall) and decide() is not otherwise idempotent. A second pass + // for the same call.id must still return deny, but must not append a second + // JSONL record. + const recordedCallIds = new Set(); const recordAutoDecision = ( + callId: string, tool: string, rule: string | undefined, outcome: ApprovalOutcomeKind, ): void => { + if (recordedCallIds.has(callId)) return; + recordedCallIds.add(callId); approvalLog .ask({ tool, @@ -546,15 +555,15 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // a secret path is ask so an explicit one-time approval can pass it. const shellRule = autoShellRuleForCall(call, isRestrictedHere, effectiveCwd, rootsProvider); if (shellRule?.effect === "deny") { - recordAutoDecision(call.name, shellRule.name, "auto-deny"); + recordAutoDecision(call.id, call.name, shellRule.name, "auto-deny"); return { kind: "deny", reason: shellRule.reason }; } if (shellRule === undefined) { - recordAutoDecision(call.name, undefined, "auto-allow"); + recordAutoDecision(call.id, call.name, undefined, "auto-allow"); return { kind: "allow" }; } } else if (!restricted && AUTO_ALLOWED_TOOLS.has(call.name)) { - recordAutoDecision(call.name, "auto-allowed-tool", "auto-allow"); + recordAutoDecision(call.id, call.name, "auto-allowed-tool", "auto-allow"); return { kind: "allow" }; } // Any other tool in auto mode (MCP or unknown built-in) is not @@ -633,7 +642,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission const askRule = anySecret ? "sensitive-path" : undefined; if (!interactive || requestApproval === undefined) { - recordAutoDecision(request.tool, askRule ?? "non-interactive", "deny"); + recordAutoDecision(call.id, request.tool, askRule ?? "non-interactive", "deny"); return { kind: "deny", reason: anySecret @@ -668,7 +677,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission } if (!interactive || requestApproval === undefined) { - recordAutoDecision(request.tool, "non-interactive", "deny"); + recordAutoDecision(call.id, request.tool, "non-interactive", "deny"); return { kind: "deny", reason: `${request.action} requires operator approval, which is unavailable in a non-interactive run. Re-run with --dangerously-skip-permissions to bypass, or narrow the action.`, diff --git a/src/plugins/permission-plugin.test.ts b/src/plugins/permission-plugin.test.ts index 4215ddaa0..98a2793ee 100644 --- a/src/plugins/permission-plugin.test.ts +++ b/src/plugins/permission-plugin.test.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; +import { APPROVAL_LOG_FILE, createApprovalLog } from "../permission/approval-log.js"; import { BLOCKED_BY_POLICY_PREFIX } from "../permission/decline-markers.js"; import { createPermissionGate } from "../permission/gate.js"; import { gateToolCall, permissionPlugin } from "./permission-plugin.js"; @@ -25,6 +29,19 @@ function trackingNext() { }; } +function readApprovalRecords(dir: string): Record[] { + let raw: string; + try { + raw = readFileSync(join(dir, APPROVAL_LOG_FILE), "utf8"); + } catch { + return []; + } + return raw + .split("\n") + .filter((l) => l.trim().length > 0) + .map((l) => JSON.parse(l) as Record); +} + describe("gateToolCall", () => { test("reactor-gated authz hard-deny blocks and skips next", async () => { const gate = createPermissionGate({ @@ -129,6 +146,38 @@ describe("gateToolCall", () => { expect(wasCalled()).toBe(true); }); + test("reactor-gated auto-allow records once across authorizeCall then gateToolCall", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + cwd, + approvalLog: createApprovalLog(dir), + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const call: ToolCall = { + id: "write-1", + name: "write_file", + arguments: { path: "src/a.ts", content: "x" }, + }; + const first = await gate.authorizeCall(call); + expect(first.effect).toBe("allow"); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall(gate, call, new AbortController().signal, next); + expect(result.isError).not.toBe(true); + expect(wasCalled()).toBe(true); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(1); + expect(records[0]?.outcome).toBe("auto-allow"); + }); + test("sub-agent path still evaluates and denies authz hard-deny", async () => { const gate = createPermissionGate({ approvals: [], From 81a619b76258327a1a738eceee67ac936bfa696b Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 8 Sep 2026 21:32:19 -0700 Subject: [PATCH 3/6] Assert two-pass reactor-gated denials record once --- src/plugins/permission-plugin.test.ts | 54 +++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/plugins/permission-plugin.test.ts b/src/plugins/permission-plugin.test.ts index 98a2793ee..aff999b18 100644 --- a/src/plugins/permission-plugin.test.ts +++ b/src/plugins/permission-plugin.test.ts @@ -178,6 +178,60 @@ describe("gateToolCall", () => { expect(records[0]?.outcome).toBe("auto-allow"); }); + test("reactor-gated auto-shell deny records once across authorizeCall then gateToolCall", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + cwd, + approvalLog: createApprovalLog(dir), + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const call = shellCall("echo x | tee src/a.ts"); + const first = await gate.authorizeCall(call); + expect(first.effect).toBe("deny"); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall(gate, call, new AbortController().signal, next); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(1); + expect(records[0]?.outcome).toBe("auto-deny"); + }); + + test("reactor-gated headless deny records once across authorizeCall then gateToolCall", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: false, + reactorGated: true, + cwd, + approvalLog: createApprovalLog(dir), + }); + const call = shellCall("curl https://example.com"); + const first = await gate.authorizeCall(call); + expect(first.effect).toBe("deny"); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall(gate, call, new AbortController().signal, next); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(1); + expect(records[0]?.outcome).toBe("deny"); + }); + test("sub-agent path still evaluates and denies authz hard-deny", async () => { const gate = createPermissionGate({ approvals: [], From 606ded2195dfcda0a5e1222c0d018b429e4628a0 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 10:21:29 -0700 Subject: [PATCH 4/6] Consume reactor-gated verdicts once at execution call.id is reused for every Codex proxy inner posix op, so a session-lifetime uniqueness set muted later JSONL records and reset() could not recover. Middleware now consumes the prior verdict and decides only on a cache miss. --- CHANGELOG.md | 5 +- docs/ARCHITECTURE.md | 2 +- src/permission/gate.ts | 72 +++++++++------- src/plugins/permission-plugin.test.ts | 116 ++++++++++++++++++++++++++ src/plugins/permission-plugin.ts | 31 +++---- tests/unit/tui/agent-tools.test.ts | 1 + 6 files changed, 180 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca342255e..e1ceb5f6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,10 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename - Reactor-gated tool middleware still blocks policy denials. A `decide()` deny (authorization hard-deny, auto-shell deny, or headless deny) returns a blocked tool error and does not run the call. Ask and allow still skip the - middleware prompt so an approved re-dispatch never re-asks. + middleware prompt so an approved re-dispatch never re-asks. Middleware is + not a second copy of `env.authorize`: it consumes the prior verdict when one + exists, and decides only for inner posix runs whose outer tool is not + `run_shell` (Codex apply_patch proxy). ## [0.3.18] - 2026-09-08 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e0a224d0c..9da7dee72 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -377,7 +377,7 @@ tool call - **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). - **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`. - **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. - - **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` still blocks a `decide()` deny (authz hard-deny, auto-shell deny, headless deny) and does not call `next`; an `ask` or `allow` skips the middleware prompt so an approved re-dispatch never re-asks. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`. + - **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()`. - **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). - **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. diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 57e0031d7..d15fc2381 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -307,17 +307,20 @@ export interface PermissionGateOptions { approvalLog?: ApprovalLog; } +export type AuthorizeVerdict = + | { effect: "allow" } + | { effect: "deny"; reason: string } + | { effect: "ask"; request: PermissionRequest }; + export interface PermissionGate { evaluate: (call: ToolCall) => Promise; // Reactor-path policy: the same decision evaluate() makes, as the effect the // vendored before-tool authz hook consumes (see authorizeCall above). - authorizeCall: ( - call: ToolCall, - ) => Promise< - | { effect: "allow" } - | { effect: "deny"; reason: string } - | { effect: "ask"; request: PermissionRequest } - >; + authorizeCall: (call: ToolCall) => Promise; + // Execution-time backstop for reactor-gated posix/MCP middleware: consume the + // authorizeCall verdict when one exists; decide only when there is no prior + // verdict (nested posix whose outer tool is not run_shell, and tests). + executionVerdict: (call: ToolCall) => Promise; // Resolve a suspended reactor approval against the operator (and mint the // outcome's grant). Returns undefined when no outcome arrived. resolveSuspended: (request: PermissionRequest) => Promise; @@ -471,20 +474,11 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // settled all collapse to now. Interactive prompts use approvalLog.ask // directly (see below) so their real queued/displayed/settled timestamps // are captured. - // - // Recording owns uniqueness: reactor-gated calls run decide() twice (env.authorize - // then gateToolCall) and decide() is not otherwise idempotent. A second pass - // for the same call.id must still return deny, but must not append a second - // JSONL record. - const recordedCallIds = new Set(); const recordAutoDecision = ( - callId: string, tool: string, rule: string | undefined, outcome: ApprovalOutcomeKind, ): void => { - if (recordedCallIds.has(callId)) return; - recordedCallIds.add(callId); approvalLog .ask({ tool, @@ -494,6 +488,12 @@ export function createPermissionGate(options: PermissionGateOptions): Permission .settle(outcome); }; + // Consume-once handoff from env.authorize to execution-time middleware. + // Not session-lifetime uniqueness: call.id is reused for every Codex proxy + // inner posix op, so a lasting set would mute later JSONL records. reset() + // clears leftovers (outer tools that never hit posix middleware). + const authorizedByCallId = new Map(); + // Non-blocking policy decision for one tool call: everything the gate owns — // tier pre-filter, auto rules, pre-grant guards, grants, headless denial — // resolved WITHOUT waiting on an operator. `ask` carries the fully-built @@ -555,15 +555,15 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // a secret path is ask so an explicit one-time approval can pass it. const shellRule = autoShellRuleForCall(call, isRestrictedHere, effectiveCwd, rootsProvider); if (shellRule?.effect === "deny") { - recordAutoDecision(call.id, call.name, shellRule.name, "auto-deny"); + recordAutoDecision(call.name, shellRule.name, "auto-deny"); return { kind: "deny", reason: shellRule.reason }; } if (shellRule === undefined) { - recordAutoDecision(call.id, call.name, undefined, "auto-allow"); + recordAutoDecision(call.name, undefined, "auto-allow"); return { kind: "allow" }; } } else if (!restricted && AUTO_ALLOWED_TOOLS.has(call.name)) { - recordAutoDecision(call.id, call.name, "auto-allowed-tool", "auto-allow"); + recordAutoDecision(call.name, "auto-allowed-tool", "auto-allow"); return { kind: "allow" }; } // Any other tool in auto mode (MCP or unknown built-in) is not @@ -642,7 +642,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission const askRule = anySecret ? "sensitive-path" : undefined; if (!interactive || requestApproval === undefined) { - recordAutoDecision(call.id, request.tool, askRule ?? "non-interactive", "deny"); + recordAutoDecision(request.tool, askRule ?? "non-interactive", "deny"); return { kind: "deny", reason: anySecret @@ -677,7 +677,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission } if (!interactive || requestApproval === undefined) { - recordAutoDecision(call.id, request.tool, "non-interactive", "deny"); + recordAutoDecision(request.tool, "non-interactive", "deny"); return { kind: "deny", reason: `${request.action} requires operator approval, which is unavailable in a non-interactive run. Re-run with --dangerously-skip-permissions to bypass, or narrow the action.`, @@ -729,7 +729,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // Middleware path: blocking evaluation used by tool-runner consumers whose // calls never pass through the reactor (sub-agents). When the gate is - // reactor-gated, gateToolCall uses authorizeCall instead of evaluate() so + // 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); @@ -751,14 +751,9 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // vendored before-tool authz hook consumes. `allow` proceeds, `deny` becomes // an upstream `block`, and `ask` suspends the call as a PendingOperation // keyed by the hook-minted correlationId — no resolve closure is held here. - const authorizeCall = async ( - call: ToolCall, - ): Promise< - | { effect: "allow" } - | { effect: "deny"; reason: string } - | { effect: "ask"; request: PermissionRequest } - > => { - const decision = await decide(call); + // Stashes the verdict for gateToolCall to consume so middleware is not a + // second copy of env.authorize. + const mapAuthorizeVerdict = (decision: GateDecision): AuthorizeVerdict => { switch (decision.kind) { case "allow": return { effect: "allow" }; @@ -769,6 +764,21 @@ export function createPermissionGate(options: PermissionGateOptions): Permission } }; + const authorizeCall = async (call: ToolCall): Promise => { + const verdict = mapAuthorizeVerdict(await decide(call)); + authorizedByCallId.set(call.id, verdict); + return verdict; + }; + + const executionVerdict = async (call: ToolCall): Promise => { + const cached = authorizedByCallId.get(call.id); + if (cached !== undefined) { + authorizedByCallId.delete(call.id); + return cached; + } + return mapAuthorizeVerdict(await decide(call)); + }; + // Resolve a suspended reactor approval once the operator answers. The // request is the one authorizeCall built at decision time, so the ask log, // wait span, and grant minting are identical to the middleware path. @@ -792,6 +802,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission if (index !== -1) approvals.splice(index, 1); } sessionGrants.length = 0; + authorizedByCallId.clear(); }; const sameApproval = (a: Approval, b: Approval): boolean => @@ -824,6 +835,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission return { evaluate, authorizeCall, + executionVerdict, resolveSuspended, isReactorGated: () => reactorGated, getApprovals: () => approvals, diff --git a/src/plugins/permission-plugin.test.ts b/src/plugins/permission-plugin.test.ts index aff999b18..4c0420e17 100644 --- a/src/plugins/permission-plugin.test.ts +++ b/src/plugins/permission-plugin.test.ts @@ -232,6 +232,122 @@ describe("gateToolCall", () => { expect(records[0]?.outcome).toBe("deny"); }); + test("nested posix with reused call.id records each auto-allow", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + cwd, + approvalLog: createApprovalLog(dir), + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const first: ToolCall = { + id: "codex-proxy", + name: "write_file", + arguments: { path: "src/a.ts", content: "x" }, + }; + const second: ToolCall = { + id: "codex-proxy", + name: "write_file", + arguments: { path: "src/b.ts", content: "y" }, + }; + const { next, wasCalled } = trackingNext(); + expect((await gateToolCall(gate, first, new AbortController().signal, next)).isError).not.toBe( + true, + ); + expect((await gateToolCall(gate, second, new AbortController().signal, next)).isError).not.toBe( + true, + ); + expect(wasCalled()).toBe(true); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(2); + expect(records[0]?.outcome).toBe("auto-allow"); + expect(records[1]?.outcome).toBe("auto-allow"); + }); + + test("nested posix with reused call.id records each auto-deny and blocks", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + cwd, + approvalLog: createApprovalLog(dir), + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const first: ToolCall = { + id: "codex-proxy", + name: "run_shell", + arguments: { command: "echo x | tee src/a.ts" }, + }; + const second: ToolCall = { + id: "codex-proxy", + name: "run_shell", + arguments: { command: "echo y | tee src/b.ts" }, + }; + const run = trackingNext(); + const firstResult = await gateToolCall(gate, first, new AbortController().signal, run.next); + const secondResult = await gateToolCall(gate, second, new AbortController().signal, run.next); + expect(firstResult.isError).toBe(true); + expect(secondResult.isError).toBe(true); + expect(firstResult.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(secondResult.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(run.wasCalled()).toBe(false); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(2); + expect(records[0]?.outcome).toBe("auto-deny"); + expect(records[1]?.outcome).toBe("auto-deny"); + }); + + test("reset does not mute a later auto-decision with a reused call.id", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + cwd, + approvalLog: createApprovalLog(dir), + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const call: ToolCall = { + id: "write-1", + name: "write_file", + arguments: { path: "src/a.ts", content: "x" }, + }; + expect((await gate.authorizeCall(call)).effect).toBe("allow"); + const firstRun = trackingNext(); + await gateToolCall(gate, call, new AbortController().signal, firstRun.next); + expect(firstRun.wasCalled()).toBe(true); + gate.reset(); + expect((await gate.authorizeCall(call)).effect).toBe("allow"); + const secondRun = trackingNext(); + await gateToolCall(gate, call, new AbortController().signal, secondRun.next); + expect(secondRun.wasCalled()).toBe(true); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(2); + expect(records[0]?.outcome).toBe("auto-allow"); + expect(records[1]?.outcome).toBe("auto-allow"); + }); + test("sub-agent path still evaluates and denies authz hard-deny", async () => { const gate = createPermissionGate({ approvals: [], diff --git a/src/plugins/permission-plugin.ts b/src/plugins/permission-plugin.ts index e67b4272d..e9a852850 100644 --- a/src/plugins/permission-plugin.ts +++ b/src/plugins/permission-plugin.ts @@ -3,14 +3,23 @@ import type { ToolCall, ToolResult } from "@intx/types/runtime"; import { BLOCKED_BY_POLICY_PREFIX } from "../permission/decline-markers.js"; import type { PermissionGate } from "../permission/gate.js"; +function blockedByPolicy(call: ToolCall, reason: string): ToolResult { + return { + callId: call.id, + content: `${BLOCKED_BY_POLICY_PREFIX}${reason}`, + isError: true, + }; +} + // Run a tool call past the gate, invoking `next` only if it is allowed. Shared by // the posix middleware and the late-connected MCP tools (which are not part of // the posix runner the middleware wraps) so both produce the same denial result. // -// Under reactor gating, authorizeCall still enforces decide() deny (authz, -// auto-shell, headless). Ask/allow skip the middleware prompt so an approved -// re-dispatch never re-asks — evaluate() is not used here because it would -// prompt again. +// Under reactor gating this is an execution backstop, not a second env.authorize. +// Consume the prior authorizeCall verdict when one exists; decide only when there +// is no prior verdict — nested posix whose outer tool is not run_shell (Codex +// apply_patch proxy) and tests. Deny blocks next; ask/allow skip the middleware +// prompt so an approved re-dispatch never re-asks. export async function gateToolCall( gate: PermissionGate, call: ToolCall, @@ -18,23 +27,15 @@ export async function gateToolCall( next: (call: ToolCall, signal: AbortSignal) => Promise, ): Promise { if (gate.isReactorGated()) { - const verdict = await gate.authorizeCall(call); + const verdict = await gate.executionVerdict(call); if (verdict.effect === "deny") { - return { - callId: call.id, - content: `${BLOCKED_BY_POLICY_PREFIX}${verdict.reason}`, - isError: true, - }; + return blockedByPolicy(call, verdict.reason); } return next(call, signal); } const verdict = await gate.evaluate(call); if (!verdict.allowed) { - return { - callId: call.id, - content: `${BLOCKED_BY_POLICY_PREFIX}${verdict.reason}`, - isError: true, - }; + return blockedByPolicy(call, verdict.reason); } return next(call, signal); } diff --git a/tests/unit/tui/agent-tools.test.ts b/tests/unit/tui/agent-tools.test.ts index 59ebc0d31..ce5a79a48 100644 --- a/tests/unit/tui/agent-tools.test.ts +++ b/tests/unit/tui/agent-tools.test.ts @@ -143,6 +143,7 @@ const { createAgentToolset, ASK_OPERATOR_OPTION_MAX_CHARS, ASK_OPERATOR_QUESTION const fakePermissionGate: PermissionGate = { evaluate: mock(async () => ({ allowed: true as const })), authorizeCall: mock(async () => ({ effect: "allow" as const })), + executionVerdict: mock(async () => ({ effect: "allow" as const })), resolveSuspended: mock(async () => undefined), isReactorGated: () => false, getApprovals: () => [], From 4b9a822fed402a6ac62833970bd862981ba64c08 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 11:56:30 -0700 Subject: [PATCH 5/6] 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. --- CHANGELOG.md | 5 +- docs/ARCHITECTURE.md | 2 +- src/permission/gate.ts | 40 +++++++---- src/plugins/permission-plugin.test.ts | 95 ++++++++++++++++++++++++--- src/plugins/permission-plugin.ts | 9 +-- 5 files changed, 123 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1ceb5f6b..b27dd4cdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,8 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename (authorization hard-deny, auto-shell deny, or headless deny) returns a blocked tool error and does not run the call. Ask and allow still skip the middleware prompt so an approved re-dispatch never re-asks. Middleware is - not a second copy of `env.authorize`: it consumes the prior verdict when one - exists, and decides only for inner posix runs whose outer tool is not - `run_shell` (Codex apply_patch proxy). + not a second copy of `env.authorize`: it consumes the prior verdict when the + same call (id, name, and arguments) is cached, and decides on a cache miss. ## [0.3.18] - 2026-09-08 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9da7dee72..3681d2f36 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -377,7 +377,7 @@ tool call - **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). - **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`. - **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. - - **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()`. + - **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()`. - **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). - **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. diff --git a/src/permission/gate.ts b/src/permission/gate.ts index d15fc2381..e104f5dc8 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -318,16 +318,18 @@ export interface PermissionGate { // vendored before-tool authz hook consumes (see authorizeCall above). authorizeCall: (call: ToolCall) => Promise; // Execution-time backstop for reactor-gated posix/MCP middleware: consume the - // authorizeCall verdict when one exists; decide only when there is no prior - // verdict (nested posix whose outer tool is not run_shell, and tests). + // authorizeCall verdict when the same call identity (id, name, arguments) is + // cached; decide only on a miss (nested posix whose outer tool is not + // run_shell, colliding reused ids, and tests). executionVerdict: (call: ToolCall) => Promise; // Resolve a suspended reactor approval against the operator (and mint the // outcome's grant). Returns undefined when no outcome arrived. resolveSuspended: (request: PermissionRequest) => Promise; - // True when this gate's decisions are consumed by the reactor's authz seam - // (env.authorize) rather than by evaluate() in the tool-runner middleware. - // Under reactor gating, gateToolCall still blocks decide() deny; ask/allow - // skip the middleware prompt so an approved re-dispatch never re-asks. + // True when this gate's decisions go through env.authorize (authorizeCall) + // rather than evaluate() in the tool-runner middleware. Under reactor gating, + // gateToolCall is an execution backstop: it consumes a matching cached + // verdict and decides on a miss. Deny still blocks; ask/allow skip the + // middleware prompt so an approved re-dispatch never re-asks. isReactorGated: () => boolean; // The gate's current in-memory approvals, including any granted this session. getApprovals: () => readonly Approval[]; @@ -490,9 +492,15 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // Consume-once handoff from env.authorize to execution-time middleware. // Not session-lifetime uniqueness: call.id is reused for every Codex proxy - // inner posix op, so a lasting set would mute later JSONL records. reset() - // clears leftovers (outer tools that never hit posix middleware). - const authorizedByCallId = new Map(); + // inner posix op, so a lasting set would mute later JSONL records. A hit + // still requires matching name and arguments so a reused id cannot apply an + // outer allow to a different inner tool. Nested posix with the same id still + // consume-once when identity matches. reset() clears leftovers (outer tools + // that never hit posix middleware). + const authorizedByCallId = new Map< + string, + { name: string; arguments: ToolCall["arguments"]; verdict: AuthorizeVerdict } + >(); // Non-blocking policy decision for one tool call: everything the gate owns — // tier pre-filter, auto rules, pre-grant guards, grants, headless denial — @@ -766,15 +774,23 @@ export function createPermissionGate(options: PermissionGateOptions): Permission const authorizeCall = async (call: ToolCall): Promise => { const verdict = mapAuthorizeVerdict(await decide(call)); - authorizedByCallId.set(call.id, verdict); + authorizedByCallId.set(call.id, { + name: call.name, + arguments: call.arguments, + verdict, + }); return verdict; }; const executionVerdict = async (call: ToolCall): Promise => { const cached = authorizedByCallId.get(call.id); - if (cached !== undefined) { + if ( + cached !== undefined && + cached.name === call.name && + JSON.stringify(cached.arguments) === JSON.stringify(call.arguments) + ) { authorizedByCallId.delete(call.id); - return cached; + return cached.verdict; } return mapAuthorizeVerdict(await decide(call)); }; diff --git a/src/plugins/permission-plugin.test.ts b/src/plugins/permission-plugin.test.ts index 4c0420e17..e9e4528d9 100644 --- a/src/plugins/permission-plugin.test.ts +++ b/src/plugins/permission-plugin.test.ts @@ -312,7 +312,89 @@ describe("gateToolCall", () => { expect(records[1]?.outcome).toBe("auto-deny"); }); - test("reset does not mute a later auto-decision with a reused call.id", async () => { + test("colliding reused call.id does not inherit allow onto a different tool", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [{ tool: "shell", pattern: "shell" }], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + cwd, + approvalLog: createApprovalLog(dir), + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const outer: ToolCall = { + id: "codex-proxy", + name: "shell", + arguments: { command: "echo x | tee src/a.ts" }, + }; + const inner: ToolCall = { + id: "codex-proxy", + name: "run_shell", + arguments: { command: "echo x | tee src/a.ts" }, + }; + expect((await gate.authorizeCall(outer)).effect).toBe("allow"); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall(gate, inner, new AbortController().signal, next); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(1); + expect(records[0]?.outcome).toBe("auto-deny"); + }); + + test("authorizeCall apply_patch then nested posix with reused id each record", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + cwd, + approvalLog: createApprovalLog(dir), + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const outer: ToolCall = { + id: "apply-1", + name: "apply_patch", + arguments: { input: "*** Begin Patch\n*** Add File: src/a.ts\n+x\n*** End Patch\n" }, + }; + const first: ToolCall = { + id: "codex-proxy", + name: "write_file", + arguments: { path: "src/a.ts", content: "x" }, + }; + const second: ToolCall = { + id: "codex-proxy", + name: "write_file", + arguments: { path: "src/b.ts", content: "y" }, + }; + expect((await gate.authorizeCall(outer)).effect).toBe("allow"); + const { next, wasCalled } = trackingNext(); + expect((await gateToolCall(gate, first, new AbortController().signal, next)).isError).not.toBe( + true, + ); + expect((await gateToolCall(gate, second, new AbortController().signal, next)).isError).not.toBe( + true, + ); + expect(wasCalled()).toBe(true); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(3); + expect(records.map((r) => r.outcome)).toEqual(["auto-allow", "auto-allow", "auto-allow"]); + }); + + test("leftover authorizeCall is cleared by reset so a later gateToolCall records", async () => { const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); const gate = createPermissionGate({ @@ -333,14 +415,11 @@ describe("gateToolCall", () => { arguments: { path: "src/a.ts", content: "x" }, }; expect((await gate.authorizeCall(call)).effect).toBe("allow"); - const firstRun = trackingNext(); - await gateToolCall(gate, call, new AbortController().signal, firstRun.next); - expect(firstRun.wasCalled()).toBe(true); gate.reset(); - expect((await gate.authorizeCall(call)).effect).toBe("allow"); - const secondRun = trackingNext(); - await gateToolCall(gate, call, new AbortController().signal, secondRun.next); - expect(secondRun.wasCalled()).toBe(true); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall(gate, call, new AbortController().signal, next); + expect(result.isError).not.toBe(true); + expect(wasCalled()).toBe(true); await new Promise((r) => setTimeout(r, 10)); const records = readApprovalRecords(dir); expect(records).toHaveLength(2); diff --git a/src/plugins/permission-plugin.ts b/src/plugins/permission-plugin.ts index e9a852850..6d8c186dd 100644 --- a/src/plugins/permission-plugin.ts +++ b/src/plugins/permission-plugin.ts @@ -16,10 +16,11 @@ function blockedByPolicy(call: ToolCall, reason: string): ToolResult { // the posix runner the middleware wraps) so both produce the same denial result. // // Under reactor gating this is an execution backstop, not a second env.authorize. -// Consume the prior authorizeCall verdict when one exists; decide only when there -// is no prior verdict — nested posix whose outer tool is not run_shell (Codex -// apply_patch proxy) and tests. Deny blocks next; ask/allow skip the middleware -// prompt so an approved re-dispatch never re-asks. +// Consume the prior authorizeCall verdict when the same call identity (id, name, +// arguments) is cached; decide only on a miss — nested posix whose outer tool is +// not run_shell (Codex apply_patch proxy), colliding reused ids, and tests. Deny +// blocks next; ask/allow skip the middleware prompt so an approved re-dispatch +// never re-asks. export async function gateToolCall( gate: PermissionGate, call: ToolCall, From 8246612195eea70a1c32b03e861d3228eee03aea Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 14:50:56 -0700 Subject: [PATCH 6/6] Assert reused call ids do not inherit allow across args --- src/plugins/permission-plugin.test.ts | 37 +++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/plugins/permission-plugin.test.ts b/src/plugins/permission-plugin.test.ts index e9e4528d9..ce2753d50 100644 --- a/src/plugins/permission-plugin.test.ts +++ b/src/plugins/permission-plugin.test.ts @@ -349,6 +349,43 @@ describe("gateToolCall", () => { expect(records[0]?.outcome).toBe("auto-deny"); }); + test("colliding reused call.id does not inherit allow onto different args", async () => { + const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); + const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-")); + const gate = createPermissionGate({ + approvals: [{ tool: "run_shell", pattern: "echo hello" }], + interactive: true, + skipPermissions: false, + reactorGated: true, + auto: true, + cwd, + approvalLog: createApprovalLog(dir), + requestApproval: async () => { + throw new Error("requestApproval must not be invoked under reactor gating"); + }, + }); + const granted: ToolCall = { + id: "codex-proxy", + name: "run_shell", + arguments: { command: "echo hello" }, + }; + const inner: ToolCall = { + id: "codex-proxy", + name: "run_shell", + arguments: { command: "echo x | tee src/a.ts" }, + }; + expect((await gate.authorizeCall(granted)).effect).toBe("allow"); + const { next, wasCalled } = trackingNext(); + const result = await gateToolCall(gate, inner, new AbortController().signal, next); + expect(result.isError).toBe(true); + expect(result.content).toContain(BLOCKED_BY_POLICY_PREFIX); + expect(wasCalled()).toBe(false); + await new Promise((r) => setTimeout(r, 10)); + const records = readApprovalRecords(dir); + expect(records).toHaveLength(1); + expect(records[0]?.outcome).toBe("auto-deny"); + }); + test("authorizeCall apply_patch then nested posix with reused id each record", async () => { const dir = mkdtempSync(join(tmpdir(), "approval-log-reactor-")); const cwd = mkdtempSync(join(tmpdir(), "gate-cwd-"));