Skip to content

Commit 606ded2

Browse files
committed
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.
1 parent 81a619b commit 606ded2

6 files changed

Lines changed: 180 additions & 47 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1818
- Reactor-gated tool middleware still blocks policy denials. A `decide()` deny
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
21-
middleware prompt so an approved re-dispatch never re-asks.
21+
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).
2225

2326
## [0.3.18] - 2026-09-08
2427

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` 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()`.
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()`.
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: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -307,17 +307,20 @@ export interface PermissionGateOptions {
307307
approvalLog?: ApprovalLog;
308308
}
309309

310+
export type AuthorizeVerdict =
311+
| { effect: "allow" }
312+
| { effect: "deny"; reason: string }
313+
| { effect: "ask"; request: PermissionRequest };
314+
310315
export interface PermissionGate {
311316
evaluate: (call: ToolCall) => Promise<GateVerdict>;
312317
// Reactor-path policy: the same decision evaluate() makes, as the effect the
313318
// vendored before-tool authz hook consumes (see authorizeCall above).
314-
authorizeCall: (
315-
call: ToolCall,
316-
) => Promise<
317-
| { effect: "allow" }
318-
| { effect: "deny"; reason: string }
319-
| { effect: "ask"; request: PermissionRequest }
320-
>;
319+
authorizeCall: (call: ToolCall) => Promise<AuthorizeVerdict>;
320+
// 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).
323+
executionVerdict: (call: ToolCall) => Promise<AuthorizeVerdict>;
321324
// Resolve a suspended reactor approval against the operator (and mint the
322325
// outcome's grant). Returns undefined when no outcome arrived.
323326
resolveSuspended: (request: PermissionRequest) => Promise<ApprovalOutcome | undefined>;
@@ -471,20 +474,11 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
471474
// settled all collapse to now. Interactive prompts use approvalLog.ask
472475
// directly (see below) so their real queued/displayed/settled timestamps
473476
// are captured.
474-
//
475-
// Recording owns uniqueness: reactor-gated calls run decide() twice (env.authorize
476-
// then gateToolCall) and decide() is not otherwise idempotent. A second pass
477-
// for the same call.id must still return deny, but must not append a second
478-
// JSONL record.
479-
const recordedCallIds = new Set<string>();
480477
const recordAutoDecision = (
481-
callId: string,
482478
tool: string,
483479
rule: string | undefined,
484480
outcome: ApprovalOutcomeKind,
485481
): void => {
486-
if (recordedCallIds.has(callId)) return;
487-
recordedCallIds.add(callId);
488482
approvalLog
489483
.ask({
490484
tool,
@@ -494,6 +488,12 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
494488
.settle(outcome);
495489
};
496490

491+
// Consume-once handoff from env.authorize to execution-time middleware.
492+
// 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>();
496+
497497
// Non-blocking policy decision for one tool call: everything the gate owns —
498498
// tier pre-filter, auto rules, pre-grant guards, grants, headless denial —
499499
// resolved WITHOUT waiting on an operator. `ask` carries the fully-built
@@ -555,15 +555,15 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
555555
// a secret path is ask so an explicit one-time approval can pass it.
556556
const shellRule = autoShellRuleForCall(call, isRestrictedHere, effectiveCwd, rootsProvider);
557557
if (shellRule?.effect === "deny") {
558-
recordAutoDecision(call.id, call.name, shellRule.name, "auto-deny");
558+
recordAutoDecision(call.name, shellRule.name, "auto-deny");
559559
return { kind: "deny", reason: shellRule.reason };
560560
}
561561
if (shellRule === undefined) {
562-
recordAutoDecision(call.id, call.name, undefined, "auto-allow");
562+
recordAutoDecision(call.name, undefined, "auto-allow");
563563
return { kind: "allow" };
564564
}
565565
} else if (!restricted && AUTO_ALLOWED_TOOLS.has(call.name)) {
566-
recordAutoDecision(call.id, call.name, "auto-allowed-tool", "auto-allow");
566+
recordAutoDecision(call.name, "auto-allowed-tool", "auto-allow");
567567
return { kind: "allow" };
568568
}
569569
// Any other tool in auto mode (MCP or unknown built-in) is not
@@ -642,7 +642,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
642642
const askRule = anySecret ? "sensitive-path" : undefined;
643643

644644
if (!interactive || requestApproval === undefined) {
645-
recordAutoDecision(call.id, request.tool, askRule ?? "non-interactive", "deny");
645+
recordAutoDecision(request.tool, askRule ?? "non-interactive", "deny");
646646
return {
647647
kind: "deny",
648648
reason: anySecret
@@ -677,7 +677,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
677677
}
678678

679679
if (!interactive || requestApproval === undefined) {
680-
recordAutoDecision(call.id, request.tool, "non-interactive", "deny");
680+
recordAutoDecision(request.tool, "non-interactive", "deny");
681681
return {
682682
kind: "deny",
683683
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
729729

730730
// Middleware path: blocking evaluation used by tool-runner consumers whose
731731
// calls never pass through the reactor (sub-agents). When the gate is
732-
// reactor-gated, gateToolCall uses authorizeCall instead of evaluate() so
732+
// reactor-gated, gateToolCall uses executionVerdict instead of evaluate() so
733733
// deny still blocks and ask never re-prompts (see gateToolCall).
734734
const evaluate = async (call: ToolCall): Promise<GateVerdict> => {
735735
const decision = await decide(call);
@@ -751,14 +751,9 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
751751
// vendored before-tool authz hook consumes. `allow` proceeds, `deny` becomes
752752
// an upstream `block`, and `ask` suspends the call as a PendingOperation
753753
// keyed by the hook-minted correlationId — no resolve closure is held here.
754-
const authorizeCall = async (
755-
call: ToolCall,
756-
): Promise<
757-
| { effect: "allow" }
758-
| { effect: "deny"; reason: string }
759-
| { effect: "ask"; request: PermissionRequest }
760-
> => {
761-
const decision = await decide(call);
754+
// Stashes the verdict for gateToolCall to consume so middleware is not a
755+
// second copy of env.authorize.
756+
const mapAuthorizeVerdict = (decision: GateDecision): AuthorizeVerdict => {
762757
switch (decision.kind) {
763758
case "allow":
764759
return { effect: "allow" };
@@ -769,6 +764,21 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
769764
}
770765
};
771766

767+
const authorizeCall = async (call: ToolCall): Promise<AuthorizeVerdict> => {
768+
const verdict = mapAuthorizeVerdict(await decide(call));
769+
authorizedByCallId.set(call.id, verdict);
770+
return verdict;
771+
};
772+
773+
const executionVerdict = async (call: ToolCall): Promise<AuthorizeVerdict> => {
774+
const cached = authorizedByCallId.get(call.id);
775+
if (cached !== undefined) {
776+
authorizedByCallId.delete(call.id);
777+
return cached;
778+
}
779+
return mapAuthorizeVerdict(await decide(call));
780+
};
781+
772782
// Resolve a suspended reactor approval once the operator answers. The
773783
// request is the one authorizeCall built at decision time, so the ask log,
774784
// wait span, and grant minting are identical to the middleware path.
@@ -792,6 +802,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
792802
if (index !== -1) approvals.splice(index, 1);
793803
}
794804
sessionGrants.length = 0;
805+
authorizedByCallId.clear();
795806
};
796807

797808
const sameApproval = (a: Approval, b: Approval): boolean =>
@@ -824,6 +835,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
824835
return {
825836
evaluate,
826837
authorizeCall,
838+
executionVerdict,
827839
resolveSuspended,
828840
isReactorGated: () => reactorGated,
829841
getApprovals: () => approvals,

0 commit comments

Comments
 (0)