diff --git a/CHANGELOG.md b/CHANGELOG.md index ab36cd39c..b27dd4cdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,17 @@ 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. Middleware is + 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 ### Added diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8b476ef4d..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` 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` 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 03e90f42d..e104f5dc8 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 @@ -306,24 +307,29 @@ 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 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 the tool-runner middleware. Tool-runner - // gating (gateToolCall) is bypassed under reactor gating so an approved - // re-dispatch runs without a second prompt. + // 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[]; @@ -484,6 +490,18 @@ 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. 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 — // resolved WITHOUT waiting on an operator. `ask` carries the fully-built @@ -718,9 +736,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 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); if (decision.kind === "allow") return { allowed: true }; @@ -741,14 +759,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" }; @@ -759,6 +772,29 @@ export function createPermissionGate(options: PermissionGateOptions): Permission } }; + const authorizeCall = async (call: ToolCall): Promise => { + const verdict = mapAuthorizeVerdict(await decide(call)); + 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 && + cached.name === call.name && + JSON.stringify(cached.arguments) === JSON.stringify(call.arguments) + ) { + authorizedByCallId.delete(call.id); + return cached.verdict; + } + 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. @@ -782,6 +818,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 => @@ -814,6 +851,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 new file mode 100644 index 000000000..ce2753d50 --- /dev/null +++ b/src/plugins/permission-plugin.test.ts @@ -0,0 +1,503 @@ +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"; + +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, + }; +} + +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({ + 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("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("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("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("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("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-")); + 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({ + 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"); + gate.reset(); + 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); + 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: [], + 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..6d8c186dd 100644 --- a/src/plugins/permission-plugin.ts +++ b/src/plugins/permission-plugin.ts @@ -3,15 +3,24 @@ 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 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 this is an execution backstop, not a second env.authorize. +// 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, @@ -19,15 +28,15 @@ export async function gateToolCall( next: (call: ToolCall, signal: AbortSignal) => Promise, ): Promise { if (gate.isReactorGated()) { + const verdict = await gate.executionVerdict(call); + if (verdict.effect === "deny") { + 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: () => [],