From 8a044246c262383f9b2b26eaadb244743ea78dbc Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 8 Sep 2026 20:33:18 -0700 Subject: [PATCH 1/6] Enforce inherited permissions for spawned workers --- docs/ARCHITECTURE.md | 1 + docs/IMPLEMENTATION.md | 29 +- src/permission/gate.ts | 5 +- src/permission/reactor-authorize.test.ts | 98 ++++ src/permission/reactor-authorize.ts | 11 + src/plugins/permission-plugin.ts | 3 +- src/subagent/identity-context.ts | 10 +- src/subagent/run.ts | 15 +- tests/integration/subagent-permission.test.ts | 521 ++++++++++++++++++ 9 files changed, 671 insertions(+), 22 deletions(-) create mode 100644 src/permission/reactor-authorize.test.ts create mode 100644 tests/integration/subagent-permission.test.ts diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2ad65ebbe..a6fbaf75d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -383,6 +383,7 @@ tool call - **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 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()`. + - **Worker reactor ownership.** `createWorkerAuthorize` reuses the validated reactor authorization bridge to the parent's live permission gate: grants and policy are shared, not copied or toggled. Authorization and tool execution run under the same async-local worker identity and cwd. A worker-local reactor ownership marker suppresses duplicate middleware evaluation even when the shared parent gate has `reactorGated: false`; `gateToolCall` still consumes cached `authorizeCall` verdicts and blocks deny. Unresolved `ask` decisions become denials without invoking an approval callback or suspending, even with an interactive parent; the parent can obtain a grant and retry. Fleet authority remains an independent restriction, not an alternative permission grant. - **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/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index 77e65f37b..e250b933f 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -398,6 +398,19 @@ Session runtime state lives under the global projects tree (not in the repo): - Migration: if a session exists only under in-repo `.agent-state//`, it is moved into the global tree on open/list - Atomic JSON writes with schema validation on load +**Worker audit persistence.** Workers initialize a real `@intx/storage-isogit` +`AuditStore` at `/audit-store` (`src/subagent/run.ts`), separate +from the native context store's Git index. Initialization failure prevents worker +execution. The existing agent-owned audit and error collectors persist at +checkpoint and shutdown; retained worker sessions flush at checkpoint/resume and +close. The parent still supplies `noopAuditStore()`: collectors exist there too, +but the parent does not durably store their records. + +Audit storage is not transactional with tool execution. A runtime `commitAudit` +failure after an authorized side effect can lose the drained audit record while +allowing the worker to complete. It emits a reactor error persisted by the existing +error collector; there is no added retry subsystem or side-effect rollback. + `createOptimizedContextStore` (`src/session/optimized-context-store.ts`) wraps the Interchange git store to keep per-checkpoint cost independent of session length. Checkpoint commits go through system git and use the operator's global @@ -470,15 +483,15 @@ Corbits Code v0.3 memory and stall hardening is implemented under `src/`, `tests ### Bounded audit collector retention between checkpoints -| Field | Detail | -| ----------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Status** | Not applicable on the default path; deferred until real audit persistence is enabled | -| **Risk** | A live audit collector that buffers full tool results in memory until `flush()` on checkpoint/shutdown can grow without bound on long, checkpoint-sparse runs. | -| **Why Corbits Code-only scope cannot close it** | Production agent setup wires `noopAuditStore()` from `@intx/agent/testing` in `src/tui/runner.ts` and `src/subagent/index.ts`. No `AuditCollector` from `@intx/inference` is instantiated, so bounding `completed` retention in `audit-collector` does not change shipped behavior today. | -| **Upstream owner** | `@intx/inference` audit collector (`audit-collector` module): opportunistic flush or capped result bodies while preserving metadata. | -| **Future Corbits Code work** | If settings later select a persistent audit store, add a bounded wrapper or configuration in `src/` and re-run hardening tests; until then, document the noop path only. | +Agent-owned audit collectors buffer completed tool results until checkpoint or +shutdown flush, including when a noop store is supplied. Workers use the durable +store described under State Persistence; the parent's noop store does not make +collector retention inapplicable. Long, checkpoint-sparse runs can retain +unbounded results. Bounded retention remains owned by the `@intx/inference` +audit collector: opportunistic flushing or capped result bodies must preserve +metadata. -Other wave items (read bounds, shell truncation, process-group kill, grep caps, plugin spawn mitigation, per-tool watchdog, inference retry UX) are implemented or partially mitigated in `src/` with co-located tests; only the two rows above remain upstream or product-gated. +Other wave items (read bounds, shell truncation, process-group kill, grep caps, plugin spawn mitigation, per-tool watchdog, inference retry UX) are implemented or partially mitigated in `src/` with co-located tests; the two items above remain upstream-owned. ## Build and Validation diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 181aef23b..490478ce7 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -298,8 +298,9 @@ export interface PermissionGateOptions { // 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. + // Workers own reactor enforcement via their execution identity regardless + // of this flag. 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 diff --git a/src/permission/reactor-authorize.test.ts b/src/permission/reactor-authorize.test.ts new file mode 100644 index 000000000..d3a789a7f --- /dev/null +++ b/src/permission/reactor-authorize.test.ts @@ -0,0 +1,98 @@ +import { expect, test } from "bun:test"; +import { createPermissionGate } from "./gate.js"; +import { createReactorAuthorize, createWorkerAuthorize } from "./reactor-authorize.js"; +import { runWithSubAgentIdentity, getSubAgentIdentity } from "../subagent/identity-context.js"; +import { gateToolCall } from "../plugins/permission-plugin.js"; +import type { ToolCall } from "@intx/types/runtime"; + +const call: ToolCall = { + id: "write-1", + name: "write_file", + arguments: { path: "probe.txt", content: "data" }, +}; +const gate = () => + createPermissionGate({ + cwd: process.cwd(), + approvals: [], + interactive: true, + auto: false, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => { + throw new Error("worker must never ask"); + }, + }); + +test("worker maps unresolved ask to deny while main reactor suspends", async () => { + const policy = gate(); + expect((await createReactorAuthorize(policy)("tool:write_file", "invoke", call)).effect).toBe( + "ask", + ); + expect((await createWorkerAuthorize(policy)("tool:write_file", "invoke", call)).effect).toBe( + "deny", + ); + policy.setAuto(true); + expect((await createWorkerAuthorize(policy)("tool:write_file", "invoke", call)).effect).toBe( + "allow", + ); + policy.setAuto(false); + expect((await createWorkerAuthorize(policy)("tool:write_file", "invoke", call)).effect).toBe( + "deny", + ); +}); + +test("worker bridge rejects malformed context, resource, and action", async () => { + const authorize = createWorkerAuthorize(gate()); + await expect(authorize("tool:write_file", "invoke", {})).rejects.toThrow("ToolCall"); + await expect(authorize("tool:read_file", "invoke", call)).rejects.toThrow("does not match"); + await expect(authorize("tool:write_file", "read", call)).rejects.toThrow("unexpected action"); +}); + +test("worker reactor is sole owner even if parent middleware mode changes policy before runner", async () => { + const policy = gate(); + policy.setAuto(true); + const identity = { description: "worker", cwd: process.cwd(), reactorOwnsPermissions: true }; + const authorize = createWorkerAuthorize(policy); + expect( + (await runWithSubAgentIdentity(identity, () => authorize("tool:write_file", "invoke", call))) + .effect, + ).toBe("allow"); + policy.setAuto(false); + const result = await runWithSubAgentIdentity(identity, () => + gateToolCall(policy, call, new AbortController().signal, async () => ({ + callId: call.id, + content: "executed", + isError: false, + })), + ); + expect(result.isError).toBe(false); + expect(policy.isReactorGated()).toBe(false); + expect( + (await runWithSubAgentIdentity(identity, () => authorize("tool:write_file", "invoke", call))) + .effect, + ).toBe("deny"); + expect(getSubAgentIdentity()).toBeUndefined(); +}); + +test("concurrent authorization preserves each worker cwd across awaited policy evaluation", async () => { + const policy = gate(); + const seen: string[] = []; + const realAuthorize = policy.authorizeCall; + policy.authorizeCall = async (toolCall) => { + await new Promise((resolve) => setTimeout(resolve, 2)); + const identity = getSubAgentIdentity(); + if (identity === undefined) throw new Error("missing worker identity"); + seen.push(identity.cwd); + return realAuthorize(toolCall); + }; + const authorize = createWorkerAuthorize(policy); + await Promise.all( + ["/worker-a", "/worker-b"].map((cwd) => + runWithSubAgentIdentity({ description: cwd, cwd, reactorOwnsPermissions: true }, () => + authorize("tool:write_file", "invoke", call), + ), + ), + ); + expect(seen.sort()).toEqual(["/worker-a", "/worker-b"]); + expect(getSubAgentIdentity()).toBeUndefined(); +}); diff --git a/src/permission/reactor-authorize.ts b/src/permission/reactor-authorize.ts index 72dc35f58..8af5b91b5 100644 --- a/src/permission/reactor-authorize.ts +++ b/src/permission/reactor-authorize.ts @@ -18,6 +18,7 @@ import { type } from "arktype"; import type { AuthzCallResult } from "@intx/inference"; import type { PermissionGate } from "./gate.js"; +import { getSubAgentIdentity } from "../subagent/identity-context.js"; const logger = getLogger([LOG_NAMESPACE_ROOT, "authz"]); @@ -55,3 +56,13 @@ export function createReactorAuthorize( } }; } + +export function createWorkerAuthorize(gate: PermissionGate) { + const authorize = createReactorAuthorize(gate); + return async (resource: string, action: string, context: unknown): Promise => { + const verdict = await authorize(resource, action, context); + if (verdict.effect !== "ask") return verdict; + logger.warn`worker authz denied unresolved approval worker=${getSubAgentIdentity()} resource=${resource}; parent must grant permission and retry`; + return { ...verdict, effect: "deny" }; + }; +} diff --git a/src/plugins/permission-plugin.ts b/src/plugins/permission-plugin.ts index 6d8c186dd..cefa021f2 100644 --- a/src/plugins/permission-plugin.ts +++ b/src/plugins/permission-plugin.ts @@ -2,6 +2,7 @@ import type { ToolPlugin } from "@intx/tools-posix"; 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"; +import { getSubAgentIdentity } from "../subagent/identity-context.js"; function blockedByPolicy(call: ToolCall, reason: string): ToolResult { return { @@ -27,7 +28,7 @@ export async function gateToolCall( signal: AbortSignal, next: (call: ToolCall, signal: AbortSignal) => Promise, ): Promise { - if (gate.isReactorGated()) { + if (gate.isReactorGated() || getSubAgentIdentity()?.reactorOwnsPermissions === true) { const verdict = await gate.executionVerdict(call); if (verdict.effect === "deny") { return blockedByPolicy(call, verdict.reason); diff --git a/src/subagent/identity-context.ts b/src/subagent/identity-context.ts index d47fd9de3..d669058fb 100644 --- a/src/subagent/identity-context.ts +++ b/src/subagent/identity-context.ts @@ -1,14 +1,12 @@ import { AsyncLocalStorage } from "node:async_hooks"; -// Identifies which sub-agent a tool call belongs to, so the permission gate -// can attribute an approval prompt to the agent that raised it (its dispatch -// description) and the working directory it is operating in. Set once per -// sub-agent around its own tool-call dispatch (see run.ts's toolsFactory) so -// every awaited call within that sub-agent's turn — including the permission -// gate and its operator prompt — can read it back via getSubAgentIdentity(). +// Authorization and tool dispatch share this async-local identity so concurrent +// workers resolve relative permission subjects against their own cwd. export interface SubAgentIdentity { description: string; cwd: string; + // Worker reactor authorization must not be repeated by parent middleware. + reactorOwnsPermissions?: boolean; } const subAgentIdentityAls = new AsyncLocalStorage(); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 35915c04e..5bc290337 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -17,7 +17,8 @@ import { type SendResult, } from "@intx/agent"; import type { AgentTool } from "@intx/agent"; -import { noopAuditStore, permissiveAuthorize } from "@intx/agent/testing"; +import { createIsogitStore } from "@intx/storage-isogit/node"; +import { createWorkerAuthorize } from "../permission/reactor-authorize.js"; import { createOptimizedContextStore } from "../session/optimized-context-store.js"; import { createAgentWithLiveToolDispatch } from "../agent/live-tool-dispatch.js"; import { type } from "arktype"; @@ -155,8 +156,7 @@ import type { TaskIntent } from "./report.js"; import { runWithSubAgentIdentity } from "./identity-context.js"; /** - * The sub-agent toolset resolves permission approvals inside the tool - * handler (see createDynamicToolRunner's waitForApproval wiring), so the + * Worker authorization denies unresolved approvals without suspending, so the * reactor should never park a gate and Agent.send should always settle on * "reply". This guard is the sound fallback if that ever drifts: instead of * flattening the suspension into an opaque message, the thrown error carries @@ -850,6 +850,7 @@ async function runSubAgentInner( const subAgentIdentity = { description: params.description, cwd: params.cwd, + reactorOwnsPermissions: true, }; const toolsFactory = defineTool({ id: `${ID_PREFIX}/subagent-tools`, @@ -897,6 +898,9 @@ async function runSubAgentInner( }); const storage = await createOptimizedContextStore(workdir); + // Audit commits must not race the native context store's git index. + const audit = await createIsogitStore(join(workdir, "audit-store")); + const authorize = createWorkerAuthorize(permissionGate); const head = { provider: params.provider.providerName, model: params.provider.model }; const bundle = @@ -924,8 +928,9 @@ async function runSubAgentInner( ...inferenceDeps, contextTransforms: [createAttachmentRehydrateTransform((key) => storage.readBlob(key))], }, - audit: noopAuditStore(), - authorize: permissiveAuthorize(), + audit, + authorize: (resource, action, context) => + runWithSubAgentIdentity(subAgentIdentity, () => authorize(resource, action, context)), directors: createDirectorRegistry({ factories: [directorDef.factory], defaultId: `${ID_PREFIX}/subagent`, diff --git a/tests/integration/subagent-permission.test.ts b/tests/integration/subagent-permission.test.ts new file mode 100644 index 000000000..c8e7ee940 --- /dev/null +++ b/tests/integration/subagent-permission.test.ts @@ -0,0 +1,521 @@ +import { expect, test } from "bun:test"; +import { mkdtemp, readdir, rm, writeFile, mkdir } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { setupHarness, type RequestPredicate } from "@intx/inference-testing"; +import { type } from "arktype"; +import { createIsogitStore } from "@intx/storage-isogit/node"; +import { ErrorRecord, type AuditRecord } from "@intx/types/audit"; +import { createPermissionGate, type PermissionGate } from "../../src/permission/gate.js"; +import { runSubAgent, type RunSubAgentParams } from "../../src/subagent/run.js"; +import { withMockedModuleDuring } from "../helpers/mock-module.js"; +import { mcpClientToAgentTools } from "../../src/mcp/plugin.js"; +import { getSubAgentIdentity } from "../../src/subagent/identity-context.js"; +import { createSubAgentSessionStore } from "../../src/subagent/session-store.js"; + +const report = + "## Summary\nFinished.\n## Findings\nAttempted write.\n## Blockers\nNone.\n## Paths\nprobe.txt"; +const RequestURL = type({ url: "string" }); +const fromHost = + (host: string): RequestPredicate => + (request) => + RequestURL.assert(request).url.includes(host); + +async function withWorker( + run: (ctx: { + cwd: string; + auditPath: string; + harness: ReturnType; + params: RunSubAgentParams; + write: () => void; + audit: () => Promise; + }) => Promise, + gate?: (cwd: string) => PermissionGate, +) { + const cwd = await mkdtemp(join(tmpdir(), "worker-permission-")); + const harness = setupHarness(); + const workdirBase = join(cwd, "state"); + const auditPath = join(workdirBase, "subagents", "worker", "audit-store"); + const params: RunSubAgentParams = { + id: "worker", + cwd, + workdirBase, + description: "permission probe", + prompt: "Write probe.txt then report.", + provider: { providerName: "openai", baseURL: "https://api.openai.com/v1", model: "test-model" }, + permissionGate: + gate?.(cwd) ?? + createPermissionGate({ + cwd, + approvals: [], + interactive: false, + auto: false, + skipPermissions: false, + reactorGated: true, + }), + }; + try { + await withMockedModuleDuring( + import.meta.resolve("../../src/session/assemble-runtime.js"), + (real: typeof import("../../src/session/assemble-runtime.js")) => ({ + ...real, + assembleInferenceBase: async () => harness.deps, + }), + () => + run({ + cwd, + auditPath, + harness, + params, + write: () => { + harness.scenario.replyOnce("openai", { + toolCalls: [ + { + name: "write_file", + args: { path: join(cwd, "probe.txt"), content: "unauthorized" }, + }, + ], + }); + harness.scenario.replyOnce("openai", { text: report }); + }, + audit: async () => { + const store = await createIsogitStore(auditPath); + const sessions = await readdir(join(auditPath, "state", "audit")); + expect(sessions).toHaveLength(1); + const session = sessions[0]; + if (session === undefined) throw new Error("missing runtime audit session"); + return store.loadAudit(session); + }, + }), + ); + } finally { + harness.dispose(); + await rm(cwd, { recursive: true, force: true }); + } +} + +for (const interactive of [false, true]) { + test.serial( + `worker denies unapproved write and persists audit (interactive=${interactive})`, + async () => { + let asks = 0; + await withWorker( + async ({ cwd, harness, params, write, audit }) => { + write(); + await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]); + expect(await Bun.file(join(cwd, "probe.txt")).exists()).toBe(false); + expect(asks).toBe(0); + const records = await audit(); + expect(records).toHaveLength(1); + expect(records[0]).toMatchObject({ + tool: "write_file", + arguments: {}, + authz: { effect: "deny", blocked: true }, + result: { isError: true }, + }); + expect(records[0]?.callId.length).toBeGreaterThan(0); + expect(records[0]?.sessionId.length).toBeGreaterThan(0); + }, + (cwd) => + createPermissionGate({ + cwd, + approvals: [], + interactive, + auto: false, + skipPermissions: false, + reactorGated: true, + requestApproval: async () => { + asks++; + return { allow: true }; + }, + }), + ); + }, + 20000, + ); +} + +test.serial( + "retained worker checkpoints audit before close and observes live grants on resume", + async () => { + await withWorker(async ({ cwd, harness, params, write, audit }) => { + let handles: Parameters>[0] | undefined; + write(); + const [result] = await Promise.all([ + runSubAgent({ + ...params, + persist: true, + onAgentReady: (value) => { + handles = value; + }, + }), + harness.run({ wallClockBudgetMs: 15000 }), + ]); + try { + expect(result.agentRetained).toBe(true); + expect((await audit())[0]?.authz?.effect).toBe("deny"); + if (handles === undefined) throw new Error("missing retained worker handles"); + params.permissionGate.setSeededApprovals([ + { tool: "write_file", pattern: join(cwd, "probe.txt") }, + ]); + write(); + await Promise.all([ + handles.followup("Retry the write."), + harness.run({ wallClockBudgetMs: 15000 }), + ]); + expect(await Bun.file(join(cwd, "probe.txt")).text()).toBe("unauthorized"); + const records = await audit(); + expect(records).toHaveLength(2); + expect(records[1]).toMatchObject({ + tool: "write_file", + authz: { effect: "allow", blocked: false }, + result: { isError: false }, + }); + params.permissionGate.setSeededApprovals([]); + write(); + await Promise.all([ + handles.followup("Retry after revocation."), + harness.run({ wallClockBudgetMs: 15000 }), + ]); + expect((await audit())[2]?.authz?.effect).toBe("deny"); + } finally { + await handles?.close(); + } + expect(await audit()).toHaveLength(3); + }); + }, + 20000, +); + +async function loadErrors(auditPath: string) { + // The vendored AuditStore exposes loadAudit but no error reader. + const root = join(auditPath, "state", "errors"); + const sessions = await readdir(root); + return ( + await Promise.all( + sessions.map(async (session) => { + const dir = join(root, session); + return Promise.all( + (await readdir(dir)).map(async (file) => + ErrorRecord.assert(await Bun.file(join(dir, file)).json()), + ), + ); + }), + ) + ).flat(); +} + +test.serial( + "nonmatching grant denies writes while read-only tools still execute", + async () => { + await withWorker(async ({ cwd, harness, params, audit }) => { + await writeFile(join(cwd, "read.txt"), "read evidence"); + params.permissionGate.setSeededApprovals([ + { tool: "write_file", pattern: join(cwd, "other.txt") }, + ]); + harness.scenario.replyOnce("openai", { + toolCalls: [ + { name: "write_file", args: { path: join(cwd, "probe.txt"), content: "blocked" } }, + { name: "read_file", args: { path: join(cwd, "read.txt") } }, + ], + }); + harness.scenario.replyOnce("openai", { text: report }); + await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]); + expect(await Bun.file(join(cwd, "probe.txt")).exists()).toBe(false); + const records = await audit(); + expect(records.find((record) => record.tool === "write_file")?.authz?.effect).toBe("deny"); + expect(records.find((record) => record.tool === "read_file")?.result.content).toContain( + "read evidence", + ); + }); + }, + 20000, +); + +test.serial( + "worker owns MCP authorization with a middleware-gated parent", + async () => { + let asks = 0; + await withWorker( + async ({ harness, params, audit }) => { + let calls = 0; + const client = { + serverName: "probe", + tools: [ + { + name: "mutate", + description: "mutates", + inputSchema: { type: "object", properties: {} }, + }, + ], + call: async () => { + calls++; + return "changed"; + }, + close: async () => undefined, + }; + params.permissionGate.registerMcpClient(client); + const tools = mcpClientToAgentTools(client, params.permissionGate); + params.inheritMcpTools = () => tools; + harness.scenario.replyOnce("openai", { + toolCalls: [{ name: "mcp__probe__mutate", args: {} }], + }); + harness.scenario.replyOnce("openai", { text: report }); + await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]); + expect(calls).toBe(0); + expect(asks).toBe(0); + expect((await audit())[0]?.authz?.effect).toBe("deny"); + }, + (cwd) => + createPermissionGate({ + cwd, + approvals: [], + interactive: true, + auto: false, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => { + asks++; + return { allow: true }; + }, + }), + ); + }, + 20000, +); + +test.serial( + "live worker authorization is not evaluated again after policy revocation before runner", + async () => { + let asks = 0; + await withWorker( + async ({ cwd, harness, params, write, audit }) => { + params.permissionGate.setAuto(true); + const authorize = params.permissionGate.authorizeCall; + let decisions = 0; + params.permissionGate.authorizeCall = async (call) => { + decisions++; + const result = await authorize(call); + params.permissionGate.setAuto(false); + return result; + }; + write(); + await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]); + expect(await Bun.file(join(cwd, "probe.txt")).exists()).toBe(true); + expect(decisions).toBe(1); + expect(asks).toBe(0); + expect((await audit())[0]?.authz?.effect).toBe("allow"); + }, + (cwd) => + createPermissionGate({ + cwd, + approvals: [], + interactive: true, + auto: false, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => { + asks++; + return { allow: false }; + }, + }), + ); + }, + 20000, +); + +test.serial( + "concurrent real workers authorize under their own cwd on the shared gate", + async () => { + await withWorker(async ({ cwd, harness, params }) => { + const cwds = [join(cwd, "first"), join(cwd, "second")]; + await Promise.all(cwds.map((dir) => mkdir(dir))); + const seen: string[] = []; + const authorize = params.permissionGate.authorizeCall; + params.permissionGate.authorizeCall = async (call) => { + await new Promise((resolve) => setTimeout(resolve, 5)); + const identity = getSubAgentIdentity(); + if (identity === undefined) throw new Error("missing authorization identity"); + seen.push(identity.cwd); + return authorize(call); + }; + for (const dir of cwds) { + const predicate = fromHost(dir.endsWith("first") ? "first.invalid" : "second.invalid"); + harness.scenario.replyOnce("openai", { + predicate, + toolCalls: [{ name: "write_file", args: { path: "probe.txt", content: "blocked" } }], + }); + harness.scenario.replyOnce("openai", { predicate, text: report }); + } + await Promise.all([ + ...cwds.map((dir, index) => + runSubAgent({ + ...params, + id: `worker-${index}`, + cwd: dir, + provider: { + ...params.provider, + baseURL: `https://${index === 0 ? "first" : "second"}.invalid/v1`, + }, + }), + ), + harness.run({ wallClockBudgetMs: 15000 }), + ]); + expect(seen.sort()).toEqual(cwds.sort()); + for (const dir of cwds) expect(await Bun.file(join(dir, "probe.txt")).exists()).toBe(false); + }); + }, + 20000, +); + +test.serial( + "worker persists inference failures through the agent error collector", + async () => { + await withWorker(async ({ harness, auditPath, params }) => { + harness.scenario.replyOnce("openai", { text: "unauthorized", responseOpts: { status: 401 } }); + const results = await Promise.allSettled([ + runSubAgent(params), + harness.run({ wallClockBudgetMs: 15000 }), + ]); + expect(results[0]?.status).toBe("rejected"); + const errors = await loadErrors(auditPath); + expect(errors.some((error) => error.source === "inference" && error.statusCode === 401)).toBe( + true, + ); + expect(errors.every((error) => error.sessionId.length > 0)).toBe(true); + }); + }, + 20000, +); + +test.serial( + "nested dispatch inherits the same live permission gate", + async () => { + await withWorker(async ({ cwd, harness, params, audit }) => { + const sessions = createSubAgentSessionStore(); + let handles: Parameters>[0] | undefined; + params.permissionGate.setSeededApprovals([{ tool: "spawn_agent", pattern: "*" }]); + const parent = fromHost("api.openai.com"); + const child = fromHost("nested.invalid"); + harness.scenario.replyOnce("openai", { + predicate: parent, + toolCalls: [ + { + name: "spawn_agent", + args: { + description: "nested probe", + prompt: "Write probe.txt then report.", + intent: "implement", + success_criteria: ["Report write result"], + }, + }, + ], + }); + harness.scenario.replyOnce("openai", { predicate: parent, text: report }); + harness.scenario.replyOnce("openai", { + predicate: child, + toolCalls: [ + { name: "write_file", args: { path: join(cwd, "probe.txt"), content: "unauthorized" } }, + ], + }); + harness.scenario.replyOnce("openai", { predicate: child, text: report }); + try { + await Promise.all([ + runSubAgent({ + ...params, + persist: true, + orchestrator: true, + orchestratorTier: "nested-orchestrator", + onAgentReady: (value) => { + handles = value; + }, + nestedDispatch: { + permissionGate: params.permissionGate, + sessions, + useWorktree: false, + getWorkdirBase: () => params.workdirBase, + provider: { ...params.provider, baseURL: "https://nested.invalid/v1" }, + }, + }), + harness.run({ wallClockBudgetMs: 15000 }), + ]); + for ( + let i = 0; + i < 200 && sessions.list().some((session) => session.finishedAt === undefined); + i++ + ) + await new Promise((resolve) => setTimeout(resolve, 10)); + expect((await audit()).map((record) => record.result)).toEqual([ + { content: expect.anything(), isError: false }, + ]); + expect(sessions.list()).toHaveLength(1); + const nested = sessions.list()[0]; + if (nested === undefined) throw new Error("missing nested worker"); + expect(nested.finishedAt).toBeDefined(); + expect(await Bun.file(join(cwd, "probe.txt")).exists()).toBe(false); + const auditPath = join(params.workdirBase, "subagents", nested.id, "audit-store"); + const store = await createIsogitStore(auditPath); + const [sessionId] = await readdir(join(auditPath, "state", "audit")); + if (sessionId === undefined) throw new Error("missing nested audit"); + expect((await store.loadAudit(sessionId))[0]?.authz?.effect).toBe("deny"); + } finally { + sessions.cancelAll("test cleanup"); + await handles?.close(); + } + }); + }, + 20000, +); + +test.serial( + "runtime audit commit failure is observable after the tool side effect", + async () => { + await withWorker(async ({ cwd, harness, auditPath, params, write }) => { + params.permissionGate.setAuto(true); + write(); + const result = await Promise.allSettled([ + runSubAgent({ + ...params, + extraToolPlugins: [ + { + middleware: (next) => async (call, signal) => { + await mkdir(join(auditPath, "state"), { recursive: true }); + await writeFile(join(auditPath, "state", "audit"), "block audit commit"); + return next(call, signal); + }, + }, + ], + }), + harness.run({ wallClockBudgetMs: 15000 }), + ]); + expect(await Bun.file(join(cwd, "probe.txt")).text()).toBe("unauthorized"); + expect(result[0]?.status).toBe("fulfilled"); + expect( + (await loadErrors(auditPath)).some( + (error) => error.source === "reactor" && error.message.includes("afterCheckpoint failed"), + ), + ).toBe(true); + await rm(join(auditPath, "state", "audit")); + const store = await createIsogitStore(auditPath); + const errors = await loadErrors(auditPath); + const session = errors[0]?.sessionId; + if (session === undefined) throw new Error("missing storage failure session"); + expect(await store.loadAudit(session)).toEqual([]); + }); + }, + 20000, +); + +test.serial( + "audit initialization failure prevents any worker tool side effect", + async () => { + await withWorker(async ({ cwd, auditPath, params, write }) => { + await mkdir(join(auditPath, ".."), { recursive: true }); + await writeFile(auditPath, "not a directory"); + write(); + await expect(runSubAgent(params)).rejects.toThrow(); + expect(await Bun.file(join(cwd, "probe.txt")).exists()).toBe(false); + }); + }, + 20000, +); From eae8b2a776916936bfc0484b37ab2fcbebbff90e Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 11:56:36 -0700 Subject: [PATCH 2/6] Gate spawned workers through a reactor-gated permission view An identity-store ownership flag made the permission plugin worker-aware. A view over the parent gate reports isReactorGated and maps unresolved asks to denials that name the subject. --- CHANGELOG.md | 3 + docs/ARCHITECTURE.md | 2 +- src/permission/decline-markers.ts | 3 + src/permission/gate.ts | 6 +- src/permission/reactor-authorize.test.ts | 95 +++++++++- src/permission/reactor-authorize.ts | 133 ++++++++++--- src/plugins/permission-plugin.ts | 3 +- src/subagent/identity-context.ts | 7 +- src/subagent/run.ts | 18 +- tests/integration/subagent-permission.test.ts | 177 ++++++++++++++++++ vendor/intx-inference/PATCHES.md | 15 ++ .../src/authz-extension.test.ts | 10 + vendor/intx-inference/src/authz-extension.ts | 14 +- 13 files changed, 430 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67ef15144..7642a367f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,9 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename sentinel. Spacer-only replies are incomplete and stay on open-task and workflow rails. Frozen-prefix matching ignores model-emitted copies of the marker. +- Spawned workers enforce the parent permission gate. Unresolved worker + approvals deny with a reason that names the permission subject so the parent + can grant and retry, without hanging on operator approval. ### Changed diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a6fbaf75d..56c436533 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -383,7 +383,7 @@ tool call - **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 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()`. - - **Worker reactor ownership.** `createWorkerAuthorize` reuses the validated reactor authorization bridge to the parent's live permission gate: grants and policy are shared, not copied or toggled. Authorization and tool execution run under the same async-local worker identity and cwd. A worker-local reactor ownership marker suppresses duplicate middleware evaluation even when the shared parent gate has `reactorGated: false`; `gateToolCall` still consumes cached `authorizeCall` verdicts and blocks deny. Unresolved `ask` decisions become denials without invoking an approval callback or suspending, even with an interactive parent; the parent can obtain a grant and retry. Fleet authority remains an independent restriction, not an alternative permission grant. + - **Worker reactor ownership.** `workerPermissionGate` is a reactor-gated view over the parent's live permission gate: grants and policy are shared, not copied or toggled. Worker posix/MCP plugins take the reactor-gated `gateToolCall` path because that view reports `isReactorGated()`; deny still blocks, ask/allow skip the middleware prompt. `authorizeCall` on the view never emits `ask` — unresolved approvals become denials that name the permission subject, without invoking an approval callback or suspending, even with an interactive parent; the parent can obtain a grant and retry. Worker control-plane tools (`submit_result`, `ask_director`, and nested fleet verbs other than `spawn_agent`) allow without a parent grant. Authorization and tool execution run under the same async-local worker identity and cwd. Fleet authority remains an independent restriction, not an alternative permission grant. - **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/decline-markers.ts b/src/permission/decline-markers.ts index d671c09b8..048850105 100644 --- a/src/permission/decline-markers.ts +++ b/src/permission/decline-markers.ts @@ -31,3 +31,6 @@ export const APPROVER_REJECTION_MARKER = "denied by approver"; /** vendor/intx-inference reactor.ts — timed-out approval suspension result. */ export const APPROVAL_TIMEOUT_RESULT_TEXT = "approval timed out"; + +/** Worker unresolved-ask deny — parent grants the named subject and retries. */ +export const WORKER_CANNOT_COMPLETE_APPROVAL = "workers cannot complete operator approval."; diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 490478ce7..629711055 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -298,9 +298,9 @@ export interface PermissionGateOptions { // 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(). - // Workers own reactor enforcement via their execution identity regardless - // of this flag. Required so a caller cannot silently fall back to - // middleware gating by omitting it. + // Workers receive a reactor-gated view over this same policy (see + // workerPermissionGate). 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 diff --git a/src/permission/reactor-authorize.test.ts b/src/permission/reactor-authorize.test.ts index d3a789a7f..b088886d4 100644 --- a/src/permission/reactor-authorize.test.ts +++ b/src/permission/reactor-authorize.test.ts @@ -1,8 +1,13 @@ import { expect, test } from "bun:test"; import { createPermissionGate } from "./gate.js"; -import { createReactorAuthorize, createWorkerAuthorize } from "./reactor-authorize.js"; +import { + createReactorAuthorize, + createWorkerAuthorize, + workerPermissionGate, +} from "./reactor-authorize.js"; import { runWithSubAgentIdentity, getSubAgentIdentity } from "../subagent/identity-context.js"; import { gateToolCall } from "../plugins/permission-plugin.js"; +import { WORKER_CANNOT_COMPLETE_APPROVAL } from "./decline-markers.js"; import type { ToolCall } from "@intx/types/runtime"; const call: ToolCall = { @@ -10,12 +15,17 @@ const call: ToolCall = { name: "write_file", arguments: { path: "probe.txt", content: "data" }, }; -const gate = () => +const namedCall = (name: string, args: Record = {}): ToolCall => ({ + id: `${name}-1`, + name, + arguments: args, +}); +const gate = (opts?: { interactive?: boolean; auto?: boolean }) => createPermissionGate({ cwd: process.cwd(), approvals: [], - interactive: true, - auto: false, + interactive: opts?.interactive ?? true, + auto: opts?.auto ?? false, skipPermissions: false, reactorGated: false, requestApproval: async () => { @@ -31,6 +41,14 @@ test("worker maps unresolved ask to deny while main reactor suspends", async () expect((await createWorkerAuthorize(policy)("tool:write_file", "invoke", call)).effect).toBe( "deny", ); + const denied = await workerPermissionGate(policy).authorizeCall(call); + expect(denied.effect).toBe("deny"); + if (denied.effect !== "deny") throw new Error("expected deny"); + expect(denied.reason).toContain("probe.txt"); + expect(denied.reason).toContain(WORKER_CANNOT_COMPLETE_APPROVAL); + const workerDenied = await createWorkerAuthorize(policy)("tool:write_file", "invoke", call); + expect(workerDenied.effect).toBe("deny"); + expect(workerDenied.reason).toBe(denied.reason); policy.setAuto(true); expect((await createWorkerAuthorize(policy)("tool:write_file", "invoke", call)).effect).toBe( "allow", @@ -41,6 +59,54 @@ test("worker maps unresolved ask to deny while main reactor suspends", async () ); }); +test("leaf worker control-plane tools allow with empty parent approvals", async () => { + for (const mode of [ + { interactive: true, auto: false }, + { interactive: false, auto: true }, + ] as const) { + const policy = gate(mode); + const authorize = createWorkerAuthorize(policy); + expect( + (await authorize("tool:submit_result", "invoke", namedCall("submit_result"))).effect, + ).toBe("allow"); + expect((await authorize("tool:ask_director", "invoke", namedCall("ask_director"))).effect).toBe( + "allow", + ); + expect((await authorize("tool:wait_agents", "invoke", namedCall("wait_agents"))).effect).toBe( + "allow", + ); + } +}); + +test("nested orchestrator wait_agents allows with only a spawn_agent grant", async () => { + const policy = gate({ interactive: true, auto: false }); + policy.setSeededApprovals([{ tool: "spawn_agent", pattern: "*" }]); + const authorize = createWorkerAuthorize(policy); + expect((await authorize("tool:wait_agents", "invoke", namedCall("wait_agents"))).effect).toBe( + "allow", + ); + expect((await authorize("tool:list_agents", "invoke", namedCall("list_agents"))).effect).toBe( + "allow", + ); + expect((await authorize("tool:spawn_agent", "invoke", namedCall("spawn_agent"))).effect).toBe( + "allow", + ); +}); + +test("worker spawn_agent still needs a parent grant", async () => { + const policy = gate({ interactive: true, auto: false }); + expect( + (await createWorkerAuthorize(policy)("tool:spawn_agent", "invoke", namedCall("spawn_agent"))) + .effect, + ).toBe("deny"); +}); + +test("worker authorizeCall never emits ask", async () => { + const policy = gate({ interactive: true, auto: false }); + expect((await workerPermissionGate(policy).authorizeCall(call)).effect).not.toBe("ask"); + expect((await policy.authorizeCall(call)).effect).toBe("ask"); +}); + test("worker bridge rejects malformed context, resource, and action", async () => { const authorize = createWorkerAuthorize(gate()); await expect(authorize("tool:write_file", "invoke", {})).rejects.toThrow("ToolCall"); @@ -48,10 +114,24 @@ test("worker bridge rejects malformed context, resource, and action", async () = await expect(authorize("tool:write_file", "read", call)).rejects.toThrow("unexpected action"); }); +test("worker gateToolCall blocks unresolved ask on cache miss", async () => { + const policy = gate(); + const workerGate = workerPermissionGate(policy); + let called = false; + const result = await gateToolCall(workerGate, call, new AbortController().signal, async () => { + called = true; + return { callId: call.id, content: "executed", isError: false }; + }); + expect(result.isError).toBe(true); + expect(called).toBe(false); + expect(workerGate.isReactorGated()).toBe(true); +}); + test("worker reactor is sole owner even if parent middleware mode changes policy before runner", async () => { const policy = gate(); policy.setAuto(true); - const identity = { description: "worker", cwd: process.cwd(), reactorOwnsPermissions: true }; + const identity = { description: "worker", cwd: process.cwd() }; + const workerGate = workerPermissionGate(policy); const authorize = createWorkerAuthorize(policy); expect( (await runWithSubAgentIdentity(identity, () => authorize("tool:write_file", "invoke", call))) @@ -59,7 +139,7 @@ test("worker reactor is sole owner even if parent middleware mode changes policy ).toBe("allow"); policy.setAuto(false); const result = await runWithSubAgentIdentity(identity, () => - gateToolCall(policy, call, new AbortController().signal, async () => ({ + gateToolCall(workerGate, call, new AbortController().signal, async () => ({ callId: call.id, content: "executed", isError: false, @@ -67,6 +147,7 @@ test("worker reactor is sole owner even if parent middleware mode changes policy ); expect(result.isError).toBe(false); expect(policy.isReactorGated()).toBe(false); + expect(workerGate.isReactorGated()).toBe(true); expect( (await runWithSubAgentIdentity(identity, () => authorize("tool:write_file", "invoke", call))) .effect, @@ -88,7 +169,7 @@ test("concurrent authorization preserves each worker cwd across awaited policy e const authorize = createWorkerAuthorize(policy); await Promise.all( ["/worker-a", "/worker-b"].map((cwd) => - runWithSubAgentIdentity({ description: cwd, cwd, reactorOwnsPermissions: true }, () => + runWithSubAgentIdentity({ description: cwd, cwd }, () => authorize("tool:write_file", "invoke", call), ), ), diff --git a/src/permission/reactor-authorize.ts b/src/permission/reactor-authorize.ts index 8af5b91b5..7372e853f 100644 --- a/src/permission/reactor-authorize.ts +++ b/src/permission/reactor-authorize.ts @@ -6,7 +6,8 @@ // module validates that boundary and maps the gate's decision onto the // effect vocabulary the hook consumes: allow proceeds, deny blocks (with the // hook's generic reason text — the gate's specific reasons are policy-internal -// and are preserved in the ask/audit surfaces, not in the model-facing block), +// and are preserved in the ask/audit surfaces, not in the model-facing block, +// except worker unresolved-ask denials which pass the subject-named reason), // and ask suspends the call as a PendingOperation keyed by the correlationId // the hook mints. @@ -17,34 +18,110 @@ import { ToolCall, type ToolCall as ToolCallType } from "@intx/types/runtime"; import { type } from "arktype"; import type { AuthzCallResult } from "@intx/inference"; -import type { PermissionGate } from "./gate.js"; -import { getSubAgentIdentity } from "../subagent/identity-context.js"; +import { WORKER_CANNOT_COMPLETE_APPROVAL } from "./decline-markers.js"; +import type { AuthorizeVerdict, GateVerdict, PermissionGate } from "./gate.js"; +import type { PermissionRequest } from "./types.js"; +import { FLEET_VERBS, ORCHESTRATOR_ONLY_FLEET_VERBS } from "../subagent/authority.js"; const logger = getLogger([LOG_NAMESPACE_ROOT, "authz"]); const AuthorizeContext = ToolCall; +// Worker-only control plane: the parent never mounts these (leaf) or cannot +// grant them independently of spawn (nested fleet verbs other than spawn). +// spawn_agent stays grant-gated. search_agents is Tier 1 only. +const WORKER_CONTROL_PLANE_TOOLS = new Set([ + "submit_result", + "ask_director", + ...[...FLEET_VERBS].filter( + (name) => name !== "spawn_agent" && !ORCHESTRATOR_ONLY_FLEET_VERBS.has(name), + ), +]); + +export function workerUnresolvedAskReason(request: PermissionRequest): string { + return `${request.action} (${request.subject}) requires a parent permission grant; ${WORKER_CANNOT_COMPLETE_APPROVAL}`; +} + +function readAuthorizeToolCall(resource: string, action: string, context: unknown): ToolCallType { + const call = AuthorizeContext(context); + if (call instanceof type.errors) { + throw new Error(`authz seam context is not a ToolCall: ${call.summary}`); + } + if (resource !== `tool:${call.name}`) { + throw new Error( + `authz seam resource ${resource} does not match call context tool ${call.name}`, + ); + } + if (action !== "invoke") { + throw new Error(`authz seam saw unexpected action ${action}`); + } + return call satisfies ToolCallType; +} + +async function authorizeWorkerCall( + gate: PermissionGate, + call: ToolCallType, +): Promise<{ effect: "allow" } | { effect: "deny"; reason: string }> { + if (WORKER_CONTROL_PLANE_TOOLS.has(call.name)) return { effect: "allow" }; + const verdict = await gate.authorizeCall(call); + if (verdict.effect !== "ask") return verdict; + return { effect: "deny", reason: workerUnresolvedAskReason(verdict.request) }; +} + +async function evaluateWorkerCall(gate: PermissionGate, call: ToolCallType): Promise { + const verdict = await authorizeWorkerCall(gate, call); + if (verdict.effect === "allow") return { allowed: true }; + return { allowed: false, reason: verdict.reason }; +} + +async function executionVerdictWorkerCall( + gate: PermissionGate, + call: ToolCallType, +): Promise { + if (WORKER_CONTROL_PLANE_TOOLS.has(call.name)) return { effect: "allow" }; + const verdict = await gate.executionVerdict(call); + if (verdict.effect !== "ask") return verdict; + return { effect: "deny", reason: workerUnresolvedAskReason(verdict.request) }; +} + +// Shared-policy view for worker posix/MCP plugins and reactor authz: live +// grants stay on the parent, reactor-gated middleware is `isReactorGated()`, +// and authorizeCall never emits ask. +export function workerPermissionGate(gate: PermissionGate): PermissionGate { + return { + evaluate: (call) => evaluateWorkerCall(gate, call), + authorizeCall: (call) => authorizeWorkerCall(gate, call), + executionVerdict: (call) => executionVerdictWorkerCall(gate, call), + resolveSuspended: (request) => gate.resolveSuspended(request), + isReactorGated: () => true, + getApprovals: () => gate.getApprovals(), + reset: () => gate.reset(), + getSessionApprovals: () => gate.getSessionApprovals(), + removeSessionApproval: (target) => gate.removeSessionApproval(target), + setSeededApprovals: (seeded) => gate.setSeededApprovals(seeded), + getAuto: () => gate.getAuto(), + setAuto: (value) => gate.setAuto(value), + getSkipPermissions: () => gate.getSkipPermissions(), + setSkipPermissions: (value) => gate.setSkipPermissions(value), + setProviderIdentity: (providerName, model) => gate.setProviderIdentity(providerName, model), + registerMcpClient: (client) => gate.registerMcpClient(client), + unregisterMcpServer: (serverName) => gate.unregisterMcpServer(serverName), + }; +} + +function allowAuthz(): AuthzCallResult { + return { effect: "allow", matchingGrants: [], resolvedBy: null }; +} + export function createReactorAuthorize( gate: PermissionGate, ): (resource: string, action: string, context: unknown) => Promise { return async (resource, action, context) => { - const call = AuthorizeContext(context); - if (call instanceof type.errors) { - throw new Error(`authz seam context is not a ToolCall: ${call.summary}`); - } - if (resource !== `tool:${call.name}`) { - throw new Error( - `authz seam resource ${resource} does not match call context tool ${call.name}`, - ); - } - if (action !== "invoke") { - throw new Error(`authz seam saw unexpected action ${action}`); - } - - const verdict = await gate.authorizeCall(call satisfies ToolCallType); + const call = readAuthorizeToolCall(resource, action, context); + const verdict = await gate.authorizeCall(call); switch (verdict.effect) { case "allow": - return { effect: "allow", matchingGrants: [], resolvedBy: null }; + return allowAuthz(); case "deny": // The model-facing block stays generic (upstream's formatBlockReason); // the gate's specific reason is preserved here for the audit trail — @@ -57,12 +134,18 @@ export function createReactorAuthorize( }; } -export function createWorkerAuthorize(gate: PermissionGate) { - const authorize = createReactorAuthorize(gate); - return async (resource: string, action: string, context: unknown): Promise => { - const verdict = await authorize(resource, action, context); - if (verdict.effect !== "ask") return verdict; - logger.warn`worker authz denied unresolved approval worker=${getSubAgentIdentity()} resource=${resource}; parent must grant permission and retry`; - return { ...verdict, effect: "deny" }; +export function createWorkerAuthorize( + gate: PermissionGate, +): (resource: string, action: string, context: unknown) => Promise { + const workerGate = workerPermissionGate(gate); + return async (resource, action, context) => { + const call = readAuthorizeToolCall(resource, action, context); + const verdict = await workerGate.authorizeCall(call); + if (verdict.effect === "allow") return allowAuthz(); + if (verdict.effect === "ask") { + throw new Error("worker authorizeCall emitted ask"); + } + logger.warn`authz deny resource=${resource} reason=${verdict.reason}`; + return { effect: "deny", matchingGrants: [], resolvedBy: null, reason: verdict.reason }; }; } diff --git a/src/plugins/permission-plugin.ts b/src/plugins/permission-plugin.ts index cefa021f2..6d8c186dd 100644 --- a/src/plugins/permission-plugin.ts +++ b/src/plugins/permission-plugin.ts @@ -2,7 +2,6 @@ import type { ToolPlugin } from "@intx/tools-posix"; 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"; -import { getSubAgentIdentity } from "../subagent/identity-context.js"; function blockedByPolicy(call: ToolCall, reason: string): ToolResult { return { @@ -28,7 +27,7 @@ export async function gateToolCall( signal: AbortSignal, next: (call: ToolCall, signal: AbortSignal) => Promise, ): Promise { - if (gate.isReactorGated() || getSubAgentIdentity()?.reactorOwnsPermissions === true) { + if (gate.isReactorGated()) { const verdict = await gate.executionVerdict(call); if (verdict.effect === "deny") { return blockedByPolicy(call, verdict.reason); diff --git a/src/subagent/identity-context.ts b/src/subagent/identity-context.ts index d669058fb..8791ab9a7 100644 --- a/src/subagent/identity-context.ts +++ b/src/subagent/identity-context.ts @@ -5,16 +5,11 @@ import { AsyncLocalStorage } from "node:async_hooks"; export interface SubAgentIdentity { description: string; cwd: string; - // Worker reactor authorization must not be repeated by parent middleware. - reactorOwnsPermissions?: boolean; } const subAgentIdentityAls = new AsyncLocalStorage(); -export function runWithSubAgentIdentity( - identity: SubAgentIdentity, - fn: () => Promise, -): Promise { +export function runWithSubAgentIdentity(identity: SubAgentIdentity, fn: () => T): T { return subAgentIdentityAls.run(identity, fn); } diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 5bc290337..a439b1ba9 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -18,7 +18,7 @@ import { } from "@intx/agent"; import type { AgentTool } from "@intx/agent"; import { createIsogitStore } from "@intx/storage-isogit/node"; -import { createWorkerAuthorize } from "../permission/reactor-authorize.js"; +import { createWorkerAuthorize, workerPermissionGate } from "../permission/reactor-authorize.js"; import { createOptimizedContextStore } from "../session/optimized-context-store.js"; import { createAgentWithLiveToolDispatch } from "../agent/live-tool-dispatch.js"; import { type } from "arktype"; @@ -454,7 +454,7 @@ async function runSubAgentInner( ): Promise { const inferenceDeps = await assembleInferenceBase(); - const permissionGate = params.permissionGate; + const permissionGate = workerPermissionGate(params.permissionGate); // Identifies this dispatch to submit_result so a submission survives // only for the turn it was spawned under — a stale call from a redirected // orchestrator (echoing an old token) is rejected. @@ -844,14 +844,13 @@ async function runSubAgentInner( ); if (typeof stallWatchdog.unref === "function") stallWatchdog.unref(); - // Every tool call this sub-agent makes runs under its own identity in ALS - // (description + cwd), so the permission gate can attribute approvals to - // the agent that raised them (see identity-context.ts). + // Concurrent workers resolve relative permission subjects against this + // identity's cwd (see identity-context.ts). const subAgentIdentity = { description: params.description, cwd: params.cwd, - reactorOwnsPermissions: true, }; + const withWorkerIdentity = (fn: () => T): T => runWithSubAgentIdentity(subAgentIdentity, fn); const toolsFactory = defineTool({ id: `${ID_PREFIX}/subagent-tools`, definitions: [], @@ -861,8 +860,7 @@ async function runSubAgentInner( const runner = createDynamicToolRunner(tools, toolWatchdogFromSettings(params.settings)); return { ...runner, - run: (call, signal) => - runWithSubAgentIdentity(subAgentIdentity, () => runner.run(call, signal)), + run: (call, signal) => withWorkerIdentity(() => runner.run(call, signal)), }; }, }); @@ -900,7 +898,7 @@ async function runSubAgentInner( const storage = await createOptimizedContextStore(workdir); // Audit commits must not race the native context store's git index. const audit = await createIsogitStore(join(workdir, "audit-store")); - const authorize = createWorkerAuthorize(permissionGate); + const authorize = createWorkerAuthorize(params.permissionGate); const head = { provider: params.provider.providerName, model: params.provider.model }; const bundle = @@ -930,7 +928,7 @@ async function runSubAgentInner( }, audit, authorize: (resource, action, context) => - runWithSubAgentIdentity(subAgentIdentity, () => authorize(resource, action, context)), + withWorkerIdentity(() => authorize(resource, action, context)), directors: createDirectorRegistry({ factories: [directorDef.factory], defaultId: `${ID_PREFIX}/subagent`, diff --git a/tests/integration/subagent-permission.test.ts b/tests/integration/subagent-permission.test.ts index c8e7ee940..3f82c7f83 100644 --- a/tests/integration/subagent-permission.test.ts +++ b/tests/integration/subagent-permission.test.ts @@ -7,6 +7,10 @@ import { type } from "arktype"; import { createIsogitStore } from "@intx/storage-isogit/node"; import { ErrorRecord, type AuditRecord } from "@intx/types/audit"; import { createPermissionGate, type PermissionGate } from "../../src/permission/gate.js"; +import { + DENIED_BY_POLICY_MARKER, + WORKER_CANNOT_COMPLETE_APPROVAL, +} from "../../src/permission/decline-markers.js"; import { runSubAgent, type RunSubAgentParams } from "../../src/subagent/run.js"; import { withMockedModuleDuring } from "../helpers/mock-module.js"; import { mcpClientToAgentTools } from "../../src/mcp/plugin.js"; @@ -113,6 +117,11 @@ for (const interactive of [false, true]) { authz: { effect: "deny", blocked: true }, result: { isError: true }, }); + expect(String(records[0]?.result.content)).toContain(DENIED_BY_POLICY_MARKER); + if (interactive) { + expect(String(records[0]?.result.content)).toContain("probe.txt"); + expect(String(records[0]?.result.content)).toContain(WORKER_CANNOT_COMPLETE_APPROVAL); + } expect(records[0]?.callId.length).toBeGreaterThan(0); expect(records[0]?.sessionId.length).toBeGreaterThan(0); }, @@ -519,3 +528,171 @@ test.serial( }, 20000, ); + +for (const mode of [ + { interactive: true, auto: false }, + { interactive: false, auto: true }, +] as const) { + test.serial( + `leaf worker submit_result succeeds with empty parent approvals (interactive=${mode.interactive} auto=${mode.auto})`, + async () => { + let asks = 0; + await withWorker( + async ({ harness, params, audit }) => { + params.tier = "leaf"; + harness.scenario.replyOnce("openai", { + toolCalls: [{ name: "submit_result", args: { turn_token: "stale", result: {} } }], + }); + harness.scenario.replyOnce("openai", { text: report }); + await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]); + expect(asks).toBe(0); + const record = (await audit())[0]; + expect(record?.tool).toBe("submit_result"); + expect(record?.authz?.effect).toBe("allow"); + expect(record?.authz?.blocked).toBe(false); + expect(String(record?.result.content)).toContain("turn_token"); + }, + (cwd) => + createPermissionGate({ + cwd, + approvals: [], + interactive: mode.interactive, + auto: mode.auto, + skipPermissions: false, + reactorGated: mode.interactive, + requestApproval: async () => { + asks++; + return { allow: true }; + }, + }), + ); + }, + 20000, + ); +} + +test.serial( + "leaf worker ask_director reaches the parent mailbox with empty parent approvals", + async () => { + let asks = 0; + await withWorker( + async ({ harness, params, audit }) => { + params.tier = "leaf"; + let registered: { question: string; questionId: string } | undefined; + params.askDirectorPort = { + register: async (input) => { + registered = input; + return "src/foo.ts"; + }, + cancel: () => undefined, + }; + harness.scenario.replyOnce("openai", { + toolCalls: [{ name: "ask_director", args: { question: "which file?" } }], + }); + harness.scenario.replyOnce("openai", { text: report }); + await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]); + expect(asks).toBe(0); + expect(registered?.question).toBe("which file?"); + expect((await audit())[0]).toMatchObject({ + tool: "ask_director", + authz: { effect: "allow", blocked: false }, + }); + }, + (cwd) => + createPermissionGate({ + cwd, + approvals: [], + interactive: true, + auto: false, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => { + asks++; + return { allow: true }; + }, + }), + ); + }, + 20000, +); + +test.serial( + "nested orchestrator wait_agents allows with only a spawn_agent grant", + async () => { + let asks = 0; + await withWorker( + async ({ cwd, harness, params, audit }) => { + const sessions = createSubAgentSessionStore(); + let handles: Parameters>[0] | undefined; + params.permissionGate.setSeededApprovals([{ tool: "spawn_agent", pattern: "*" }]); + const parent = fromHost("api.openai.com"); + const child = fromHost("nested.invalid"); + harness.scenario.replyOnce("openai", { + predicate: parent, + toolCalls: [ + { + name: "spawn_agent", + args: { + description: "nested probe", + prompt: "Report only.", + intent: "implement", + success_criteria: ["Report"], + }, + }, + ], + }); + harness.scenario.replyOnce("openai", { + predicate: parent, + toolCalls: [{ name: "wait_agents", args: { timeout_ms: 8000 } }], + }); + harness.scenario.replyOnce("openai", { predicate: parent, text: report }); + harness.scenario.replyOnce("openai", { predicate: child, text: report }); + try { + await Promise.all([ + runSubAgent({ + ...params, + persist: true, + orchestrator: true, + orchestratorTier: "nested-orchestrator", + onAgentReady: (value) => { + handles = value; + }, + nestedDispatch: { + permissionGate: params.permissionGate, + sessions, + useWorktree: false, + getWorkdirBase: () => params.workdirBase, + provider: { ...params.provider, baseURL: "https://nested.invalid/v1" }, + }, + }), + harness.run({ wallClockBudgetMs: 15000 }), + ]); + expect(asks).toBe(0); + expect(sessions.list()).toHaveLength(1); + const records = await audit(); + expect(records.find((record) => record.tool === "wait_agents")?.authz?.effect).toBe( + "allow", + ); + expect(await Bun.file(join(cwd, "probe.txt")).exists()).toBe(false); + } finally { + sessions.cancelAll("test cleanup"); + await handles?.close(); + } + }, + (cwd) => + createPermissionGate({ + cwd, + approvals: [], + interactive: true, + auto: false, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => { + asks++; + return { allow: true }; + }, + }), + ); + }, + 20000, +); diff --git a/vendor/intx-inference/PATCHES.md b/vendor/intx-inference/PATCHES.md index 2ac352c82..447733607 100644 --- a/vendor/intx-inference/PATCHES.md +++ b/vendor/intx-inference/PATCHES.md @@ -84,6 +84,20 @@ upstream adopts; downstream users whose `authorize` ignores the context are unaffected. **Removal path:** Upstream PR to `@intx/inference` documenting/populating the per-call context at the `authorize` call site. +## authz-ts-deny-reason + +`authz-extension.ts` — `AuthzCallResult` may carry an optional `reason`. A +`deny` effect that includes a non-empty reason uses it as the model-facing +block text (`Denied by policy: ${reason}`) instead of the generic +`resource/action` form. Callers that omit `reason` keep the upstream wording. +Consumed by worker permission authorization so an unresolved ask names the +permission subject. + +**Disposition:** Promotion candidate. Requires upstream to accept a deny-reason +passthrough on `AuthzCallResult`. **Removal path:** Upstream PR adding +`reason?: string` and using it in `formatBlockReason`. +**Re-carry:** new after `0205b07b`. Low risk — additive optional field. + ## assembly-ts-deps-context-transforms `assembly.ts` — Resolves `contextTransforms` from either the direct assembly @@ -348,6 +362,7 @@ revisit point is the next vendored sync (see `docs/VENDORING.md`). | Patch | Upstream ask | Upstream contact | Tracking | Revisit | | --- | --- | --- | --- | --- | | adapter-ts-stream-terminal-detector (+ harness-ts-is-stream-terminal) | Add an `isStreamTerminal`/`StreamTerminalDetector` hook to `ProviderAdapter` for semantic end-of-stream protocols | Alexander Guy | This ledger (vendor/intx-inference/PATCHES.md#adapter-ts-stream-terminal-detector) | Next vendored sync | +| authz-ts-deny-reason | Optional `AuthzCallResult.reason` used as the deny block text | Alexander Guy | This ledger (#authz-ts-deny-reason) | Next vendored sync | | errors-ts-classify-abort-reason | Add optional `reason` param to `classifyAbortError`, carried as `raw: { origin }` | Alexander Guy | This ledger (#errors-ts-classify-abort-reason) | Next vendored sync | | harness-ts-inactivity-on-semantic-progress | Gate the inactivity watchdog's re-arm on parsed events, not raw SSE bytes | Alexander Guy | This ledger (#harness-ts-inactivity-on-semantic-progress) | Next vendored sync | | reactor-ts-correlating-ids-leak | Wrap `tryCorrelate` in try/finally so `correlatingIds` clears on success dispatch paths | Alexander Guy | This ledger (#reactor-ts-correlating-ids-leak) | Next vendored sync | diff --git a/vendor/intx-inference/src/authz-extension.test.ts b/vendor/intx-inference/src/authz-extension.test.ts index 5495304df..b4e7260a9 100644 --- a/vendor/intx-inference/src/authz-extension.test.ts +++ b/vendor/intx-inference/src/authz-extension.test.ts @@ -134,6 +134,16 @@ describe("createAuthzExtension", () => { expect(d.resolvedBy.id).toBe("grant-2"); }); + test("deny effect uses authorize reason when provided", async () => { + const ext = createAuthzExtension({ + authorize: async () => ({ ...denyResult(), reason: "write_file (probe.txt) blocked" }), + }); + const result = await ext.beforeTool(makeCall(), makeState(), signal); + expect(result.type).toBe("block"); + if (result.type !== "block") throw new Error("expected block"); + expect(result.reason).toBe("Denied by policy: write_file (probe.txt) blocked"); + }); + test("ask effect suspends with a minted correlation and pending operation", async () => { const decisions: AuthzDecision[] = []; diff --git a/vendor/intx-inference/src/authz-extension.ts b/vendor/intx-inference/src/authz-extension.ts index 0326159e2..d7dfdf3b0 100644 --- a/vendor/intx-inference/src/authz-extension.ts +++ b/vendor/intx-inference/src/authz-extension.ts @@ -49,6 +49,8 @@ export type AuthzCallResult = { effect: Effect | null; matchingGrants: AuthzMatchedGrant[]; resolvedBy: AuthzMatchedGrant | null; + // Locally patched — see vendor/intx-inference/PATCHES.md#authz-ts-deny-reason + reason?: string; }; export type AuthzDecision = { @@ -93,10 +95,13 @@ function formatBlockReason( effect: BlockEffect, resource: string, action: string, + detail?: string, ): string { switch (effect) { case "deny": - return `Denied by policy: ${resource}/${action}`; + return detail !== undefined && detail.length > 0 + ? `Denied by policy: ${detail}` + : `Denied by policy: ${resource}/${action}`; case null: return `No matching grants for ${resource}/${action}`; } @@ -178,7 +183,12 @@ export function createAuthzExtension( // are blocks. const blockReason = result.effect === "deny" || result.effect === null - ? formatBlockReason(result.effect, resource, action) + ? formatBlockReason( + result.effect, + resource, + action, + result.effect === "deny" ? result.reason : undefined, + ) : undefined; const decision: AuthzDecision = { From 84cd386b30294147aaddcf02896a85f2344e37ae Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 12:35:42 -0700 Subject: [PATCH 3/6] Bind inherited MCP tools to the worker permission gate Parent MCP handlers closed over a middleware-gated isReactorGated of false, so an allowed worker call re-entered evaluate and hung on requestApproval. Workers now wrap inherited MCP tools with the same reactor-gated view as posix. --- docs/ARCHITECTURE.md | 2 +- src/agent/tools.ts | 9 +-- src/mcp/plugin.ts | 67 ++++++++++--------- src/plugins/permission-plugin.ts | 14 ++++ src/subagent/run.ts | 2 +- src/subagent/types.ts | 2 +- tests/integration/subagent-permission.test.ts | 63 ++++++++++++++++- tests/unit/tui/agent-tools.test.ts | 2 + 8 files changed, 120 insertions(+), 41 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 56c436533..2f777eeb2 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -383,7 +383,7 @@ tool call - **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 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()`. - - **Worker reactor ownership.** `workerPermissionGate` is a reactor-gated view over the parent's live permission gate: grants and policy are shared, not copied or toggled. Worker posix/MCP plugins take the reactor-gated `gateToolCall` path because that view reports `isReactorGated()`; deny still blocks, ask/allow skip the middleware prompt. `authorizeCall` on the view never emits `ask` — unresolved approvals become denials that name the permission subject, without invoking an approval callback or suspending, even with an interactive parent; the parent can obtain a grant and retry. Worker control-plane tools (`submit_result`, `ask_director`, and nested fleet verbs other than `spawn_agent`) allow without a parent grant. Authorization and tool execution run under the same async-local worker identity and cwd. Fleet authority remains an independent restriction, not an alternative permission grant. + - **Worker reactor ownership.** `workerPermissionGate` is a reactor-gated view over the parent's live permission gate: grants and policy are shared, not copied or toggled. Worker posix plugins and inherited MCP tools are bound to that view at worker start, so they take the reactor-gated `gateToolCall` path because the view reports `isReactorGated()` — they do not close over the parent's middleware-gated `isReactorGated()`. Deny still blocks; ask/allow skip the middleware prompt. `authorizeCall` on the view never emits `ask` — unresolved approvals become denials that name the permission subject, without invoking an approval callback or suspending, even with an interactive parent; the parent can obtain a grant and retry. Worker control-plane tools (`submit_result`, `ask_director`, and nested fleet verbs other than `spawn_agent`) allow without a parent grant. Authorization and tool execution run under the same async-local worker identity and cwd. Fleet authority remains an independent restriction, not an alternative permission grant. - **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/agent/tools.ts b/src/agent/tools.ts index 631bfff4d..de2c0787e 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -32,8 +32,9 @@ import { EXA_MCP_SERVER_NAME, isBuiltinExaMCPServer, } from "../mcp/exa.js"; -import { mcpClientToAgentTools } from "../mcp/plugin.js"; +import { mcpClientTools } from "../mcp/plugin.js"; import { parseMcpToolName } from "../mcp/tool-name.js"; +import { gateAgentTools } from "../plugins/permission-plugin.js"; import { createDynamicToolRunner, type DynamicToolRunner } from "../tui/dynamic-tool-runner.js"; import type { MCPServerConfig, Settings } from "../config/settings.js"; import { @@ -436,7 +437,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise inheritedMcpTools, + inheritMcpTools: (gate: PermissionGate) => gateAgentTools(inheritedMcpTools, gate), ...(shellTimeout !== undefined ? { shellTimeout } : {}), ...(shellEnv !== undefined ? { shellEnv } : {}), ...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}), @@ -836,12 +837,12 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise => - gateToolCall(gate, call, signal, async () => { - try { - const content = await client.call(tool.name, call.arguments, signal); - const writeBlob = getBlobWriter?.(); - const contextDir = getContextDir?.(); - const spill = - writeBlob !== undefined - ? { - callId: call.id, - writeBlob, - ...(contextDir !== undefined ? { contextDir } : {}), - } - : undefined; - return { callId: call.id, content: await sanitizeMcpResultContent(content, spill) }; - } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; - } - }), + handler: async (call: ToolCall, signal: AbortSignal): Promise => { + try { + const content = await client.call(tool.name, call.arguments, signal); + const writeBlob = getBlobWriter?.(); + const contextDir = getContextDir?.(); + const spill = + writeBlob !== undefined + ? { + callId: call.id, + writeBlob, + ...(contextDir !== undefined ? { contextDir } : {}), + } + : undefined; + return { callId: call.id, content: await sanitizeMcpResultContent(content, spill) }; + } catch (err) { + return { + callId: call.id, + content: err instanceof Error ? err.message : String(err), + isError: true, + }; + } + }, })); } + +// Convert a connected client's tools into AgentTools for the dynamic runner used +// by the TUI. These tools live in a separate runner from the posix tool plugin +// chain, so each handler is wrapped with the permission gate directly. +export function mcpClientToAgentTools( + client: MCPClient, + gate: PermissionGate, + spillOptions: McpSpillOptions = {}, +): AgentTool[] { + return gateAgentTools(mcpClientTools(client, spillOptions), gate); +} diff --git a/src/plugins/permission-plugin.ts b/src/plugins/permission-plugin.ts index 6d8c186dd..289e21960 100644 --- a/src/plugins/permission-plugin.ts +++ b/src/plugins/permission-plugin.ts @@ -1,3 +1,4 @@ +import type { AgentTool } from "@intx/agent"; import type { ToolPlugin } from "@intx/tools-posix"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; import { BLOCKED_BY_POLICY_PREFIX } from "../permission/decline-markers.js"; @@ -41,6 +42,19 @@ export async function gateToolCall( return next(call, signal); } +// Bind full AgentTools to a gate. Workers pass workerPermissionGate so inherited +// MCP handlers skip middleware the same way posix plugins do. +export function gateAgentTools(tools: readonly AgentTool[], gate: PermissionGate): AgentTool[] { + return tools.map((tool) => { + if (tool.kind !== "full") return tool; + const inner = tool.handler; + return { + ...tool, + handler: (call: ToolCall, signal: AbortSignal) => gateToolCall(gate, call, signal, inner), + }; + }); +} + // Gate consequential tool calls on operator approval. Runs after the // authorization plugin (which hard-denies catastrophic commands), so by the time // a call reaches here it is at worst "consequential but legitimate" — the gate diff --git a/src/subagent/run.ts b/src/subagent/run.ts index a439b1ba9..2794b0ba6 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -537,7 +537,7 @@ async function runSubAgentInner( ), })); - const inherited = params.inheritMcpTools?.() ?? []; + const inherited = params.inheritMcpTools?.(permissionGate) ?? []; tools = [ ...tools, ...coreSubAgentWebTools(inherited), diff --git a/src/subagent/types.ts b/src/subagent/types.ts index c64d5bc72..11769ef18 100644 --- a/src/subagent/types.ts +++ b/src/subagent/types.ts @@ -42,7 +42,7 @@ export interface SubAgentProvider { // recursion bottoms out at one hop of orchestration. export interface SubAgentSandboxDeps { permissionGate: PermissionGate; - inheritMcpTools?: () => readonly AgentTool[]; + inheritMcpTools?: (gate: PermissionGate) => readonly AgentTool[]; shellTimeout?: ShellTimeoutConfig; extraToolPlugins?: ToolPlugin[]; /** Parent session blob store for bounded tool-output:// reads in workers. */ diff --git a/tests/integration/subagent-permission.test.ts b/tests/integration/subagent-permission.test.ts index 3f82c7f83..70e845f65 100644 --- a/tests/integration/subagent-permission.test.ts +++ b/tests/integration/subagent-permission.test.ts @@ -264,8 +264,7 @@ test.serial( close: async () => undefined, }; params.permissionGate.registerMcpClient(client); - const tools = mcpClientToAgentTools(client, params.permissionGate); - params.inheritMcpTools = () => tools; + params.inheritMcpTools = (gate) => mcpClientToAgentTools(client, gate); harness.scenario.replyOnce("openai", { toolCalls: [{ name: "mcp__probe__mutate", args: {} }], }); @@ -293,6 +292,66 @@ test.serial( 20000, ); +test.serial( + "allowed inherited MCP call with middleware-gated parent does not requestApproval", + async () => { + let asks = 0; + await withWorker( + async ({ harness, params, audit }) => { + let calls = 0; + const client = { + serverName: "probe", + tools: [ + { + name: "mutate", + description: "mutates", + inputSchema: { type: "object", properties: {} }, + }, + ], + call: async () => { + calls++; + return "changed"; + }, + close: async () => undefined, + }; + params.permissionGate.registerMcpClient(client); + params.permissionGate.setSeededApprovals([ + { tool: "mcp__probe__mutate", pattern: "mcp__probe__mutate" }, + ]); + const authorize = params.permissionGate.authorizeCall; + params.permissionGate.authorizeCall = async (call) => { + const result = await authorize(call); + params.permissionGate.setSeededApprovals([]); + return result; + }; + params.inheritMcpTools = (gate) => mcpClientToAgentTools(client, gate); + harness.scenario.replyOnce("openai", { + toolCalls: [{ name: "mcp__probe__mutate", args: {} }], + }); + harness.scenario.replyOnce("openai", { text: report }); + await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]); + expect(calls).toBe(1); + expect(asks).toBe(0); + expect((await audit())[0]?.authz?.effect).toBe("allow"); + }, + (cwd) => + createPermissionGate({ + cwd, + approvals: [], + interactive: true, + auto: false, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => { + asks++; + return { allow: true }; + }, + }), + ); + }, + 20000, +); + test.serial( "live worker authorization is not evaluated again after policy revocation before runner", async () => { diff --git a/tests/unit/tui/agent-tools.test.ts b/tests/unit/tui/agent-tools.test.ts index ce5a79a48..e09f910f6 100644 --- a/tests/unit/tui/agent-tools.test.ts +++ b/tests/unit/tui/agent-tools.test.ts @@ -73,6 +73,7 @@ await withMockedModule( ); await withMockedModule(import.meta.resolve("../../../src/mcp/plugin.js"), () => ({ + mcpClientTools: () => [], mcpClientToAgentTools: () => [], })); @@ -90,6 +91,7 @@ await withMockedModule(import.meta.resolve("../../../src/plugins/verify-plugin.j await withMockedModule(import.meta.resolve("../../../src/plugins/permission-plugin.js"), () => ({ permissionPlugin: () => ({}), + gateAgentTools: (tools: unknown) => tools, gateToolCall: async ( _gate: unknown, call: ToolCall, From 88c881dff93e030d951da339e5a0b771f5138f20 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 14:47:36 -0700 Subject: [PATCH 4/6] Cover production inherit wrapping ungated MCP tools Workers wrap inherited MCP handlers with the reactor-gated view. If the factory stored parent-gated tools, that outer wrap would still call the parent's requestApproval. --- tests/integration/subagent-permission.test.ts | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/tests/integration/subagent-permission.test.ts b/tests/integration/subagent-permission.test.ts index 70e845f65..040159aae 100644 --- a/tests/integration/subagent-permission.test.ts +++ b/tests/integration/subagent-permission.test.ts @@ -14,8 +14,11 @@ import { import { runSubAgent, type RunSubAgentParams } from "../../src/subagent/run.js"; import { withMockedModuleDuring } from "../helpers/mock-module.js"; import { mcpClientToAgentTools } from "../../src/mcp/plugin.js"; +import type { MCPClient } from "../../src/mcp/client.js"; import { getSubAgentIdentity } from "../../src/subagent/identity-context.js"; import { createSubAgentSessionStore } from "../../src/subagent/session-store.js"; +import { workerPermissionGate } from "../../src/permission/reactor-authorize.js"; +import { gateAgentTools } from "../../src/plugins/permission-plugin.js"; const report = "## Summary\nFinished.\n## Findings\nAttempted write.\n## Blockers\nNone.\n## Paths\nprobe.txt"; @@ -352,6 +355,184 @@ test.serial( 20000, ); +async function bindCreateAgentToolsetInherit(args: { + cwd: string; + permissionGate: PermissionGate; + client: MCPClient; +}): Promise<{ + inheritMcpTools: NonNullable; + dispose: () => Promise; +}> { + let inheritMcpTools: RunSubAgentParams["inheritMcpTools"]; + let dispose: () => Promise = async () => undefined; + await withMockedModuleDuring( + import.meta.resolve("../../src/subagent/agent-fleet.js"), + (real: typeof import("../../src/subagent/agent-fleet.js")) => ({ + ...real, + createSpawnAgentTool: (deps: Parameters[0]) => { + inheritMcpTools = deps.inheritMcpTools; + return real.createSpawnAgentTool(deps); + }, + }), + async () => + withMockedModuleDuring( + import.meta.resolve("../../src/mcp/client.js"), + (real: typeof import("../../src/mcp/client.js")) => ({ + ...real, + connectMCPServer: async () => ({ ok: true as const, client: args.client }), + }), + async () => { + const { createAgentToolset } = await import("../../src/agent/tools.js"); + const toolset = await createAgentToolset({ + cwd: args.cwd, + permissionGate: args.permissionGate, + onOperatorGate: async () => ({ kind: "cancel" }), + mcpServers: [], + subAgent: { + provider: { + providerName: "openai", + baseURL: "https://api.openai.com/v1", + model: "test-model", + }, + getWorkdirBase: () => join(args.cwd, "state"), + sessions: createSubAgentSessionStore(), + }, + }); + dispose = () => toolset.dispose(); + await toolset.connectMCPServer( + { name: "probe", type: "http", url: "https://mcp.probe.test/mcp" }, + { + interactiveAuth: false, + onStatus: () => undefined, + onToolsChanged: () => undefined, + }, + ); + }, + ), + ); + if (inheritMcpTools === undefined) { + await dispose(); + throw new Error("createAgentToolset did not wire inheritMcpTools"); + } + return { inheritMcpTools, dispose }; +} + +test.serial( + "createAgentToolset inherit wraps ungated MCP tools with the passed worker gate", + async () => { + let asks = 0; + await withWorker( + async ({ cwd, harness, params, audit }) => { + let calls = 0; + const client = { + serverName: "probe", + tools: [ + { + name: "mutate", + description: "mutates", + inputSchema: { type: "object", properties: {} }, + }, + ], + call: async () => { + calls++; + return "changed"; + }, + close: async () => undefined, + }; + const bound = await bindCreateAgentToolsetInherit({ + cwd, + permissionGate: params.permissionGate, + client, + }); + try { + params.permissionGate.setSeededApprovals([ + { tool: "mcp__probe__mutate", pattern: "mcp__probe__mutate" }, + ]); + const authorize = params.permissionGate.authorizeCall; + params.permissionGate.authorizeCall = async (call) => { + const result = await authorize(call); + params.permissionGate.setSeededApprovals([]); + return result; + }; + params.inheritMcpTools = bound.inheritMcpTools; + harness.scenario.replyOnce("openai", { + toolCalls: [{ name: "mcp__probe__mutate", args: {} }], + }); + harness.scenario.replyOnce("openai", { text: report }); + await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]); + expect(calls).toBe(1); + expect(asks).toBe(0); + expect((await audit())[0]?.authz?.effect).toBe("allow"); + } finally { + await bound.dispose(); + } + }, + (cwd) => + createPermissionGate({ + cwd, + approvals: [], + interactive: true, + auto: false, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => { + asks++; + return { allow: true }; + }, + }), + ); + }, + 20000, +); + +test("storing parent-gated MCP tools then wrapping again still calls requestApproval", async () => { + const cwd = await mkdtemp(join(tmpdir(), "worker-permission-")); + let asks = 0; + let calls = 0; + try { + const parent = createPermissionGate({ + cwd, + approvals: [], + interactive: true, + auto: false, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => { + asks++; + return { allow: true }; + }, + }); + const client = { + serverName: "probe", + tools: [ + { + name: "mutate", + description: "mutates", + inputSchema: { type: "object", properties: {} }, + }, + ], + call: async () => { + calls++; + return "changed"; + }, + close: async () => undefined, + }; + parent.registerMcpClient(client); + const parentGated = mcpClientToAgentTools(client, parent); + const doubleWrapped = gateAgentTools(parentGated, workerPermissionGate(parent)); + const tool = doubleWrapped[0]; + if (tool?.kind !== "full") throw new Error("expected full inherited MCP tool"); + await tool.handler( + { id: "c1", name: "mcp__probe__mutate", arguments: {} }, + new AbortController().signal, + ); + expect(asks).toBe(1); + expect(calls).toBe(1); + } finally { + await rm(cwd, { recursive: true, force: true }); + } +}); + test.serial( "live worker authorization is not evaluated again after policy revocation before runner", async () => { From 2bc44f14f7daf0bf8df94c70050ace0a02e72df1 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 18:09:14 -0700 Subject: [PATCH 5/6] Align inherit tests with middleware deny and grant retry Workers deny without asking when parent-gated MCP tools are wrapped again. A later call id after a parent grant must still pass gateToolCall instead of reusing the prior deny. --- src/permission/reactor-authorize.test.ts | 40 +++++++++++++++++++ tests/integration/subagent-permission.test.ts | 9 +++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/permission/reactor-authorize.test.ts b/src/permission/reactor-authorize.test.ts index b088886d4..bb353e207 100644 --- a/src/permission/reactor-authorize.test.ts +++ b/src/permission/reactor-authorize.test.ts @@ -1,4 +1,7 @@ import { expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { createPermissionGate } from "./gate.js"; import { createReactorAuthorize, @@ -33,6 +36,43 @@ const gate = (opts?: { interactive?: boolean; auto?: boolean }) => }, }); +test("worker grant after a denied write allows a later call id through gateToolCall", async () => { + const cwd = mkdtempSync(join(tmpdir(), "worker-grant-")); + const path = join(cwd, "probe.txt"); + const policy = createPermissionGate({ + cwd, + approvals: [], + interactive: false, + auto: false, + skipPermissions: false, + reactorGated: true, + requestApproval: async () => { + throw new Error("worker must never ask"); + }, + }); + const workerGate = workerPermissionGate(policy); + const first: ToolCall = { + id: "call_auto_0", + name: "write_file", + arguments: { path, content: "unauthorized" }, + }; + const second: ToolCall = { + id: "call_auto_1", + name: "write_file", + arguments: { path, content: "unauthorized" }, + }; + expect((await workerGate.authorizeCall(first)).effect).toBe("deny"); + policy.setSeededApprovals([{ tool: "write_file", pattern: path }]); + expect((await workerGate.authorizeCall(second)).effect).toBe("allow"); + let called = false; + const result = await gateToolCall(workerGate, second, new AbortController().signal, async () => { + called = true; + return { callId: second.id, content: "executed", isError: false }; + }); + expect(result.isError).toBe(false); + expect(called).toBe(true); +}); + test("worker maps unresolved ask to deny while main reactor suspends", async () => { const policy = gate(); expect((await createReactorAuthorize(policy)("tool:write_file", "invoke", call)).effect).toBe( diff --git a/tests/integration/subagent-permission.test.ts b/tests/integration/subagent-permission.test.ts index 040159aae..269d0e43c 100644 --- a/tests/integration/subagent-permission.test.ts +++ b/tests/integration/subagent-permission.test.ts @@ -485,7 +485,7 @@ test.serial( 20000, ); -test("storing parent-gated MCP tools then wrapping again still calls requestApproval", async () => { +test("storing parent-gated MCP tools then wrapping again denies without requestApproval", async () => { const cwd = await mkdtemp(join(tmpdir(), "worker-permission-")); let asks = 0; let calls = 0; @@ -522,12 +522,13 @@ test("storing parent-gated MCP tools then wrapping again still calls requestAppr const doubleWrapped = gateAgentTools(parentGated, workerPermissionGate(parent)); const tool = doubleWrapped[0]; if (tool?.kind !== "full") throw new Error("expected full inherited MCP tool"); - await tool.handler( + const result = await tool.handler( { id: "c1", name: "mcp__probe__mutate", arguments: {} }, new AbortController().signal, ); - expect(asks).toBe(1); - expect(calls).toBe(1); + expect(asks).toBe(0); + expect(calls).toBe(0); + expect(result.isError).toBe(true); } finally { await rm(cwd, { recursive: true, force: true }); } From 384a601acb2adef51b1a24ee2de3ef0d4fbec38d Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 18:17:33 -0700 Subject: [PATCH 6/6] Match reactor verdict cache to path-escaped arguments posix path escape rewrites workspace paths to their realpath before gateToolCall. Comparing cache identity after the same resolve keeps a worker allow from being re-decided as a deny on Darwin tmpdir symlinks, without letting a reused call id inherit allow onto a different tool or command. --- src/permission/gate.ts | 36 ++++++++++++++++---- src/permission/reactor-authorize.test.ts | 43 +++++++++++++++++++++++- 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 629711055..a472eee8d 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -17,11 +17,12 @@ import { } from "./classify.js"; import { autoShellRuleForCall, safeWorktreeCommand } from "./auto-shell-policy.js"; import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js"; +import { looksLikePath } from "../plugins/path-escape-plugin.js"; import { runShellAuthzBlockReason } from "../shell/run-shell-authz.js"; import { matchesPattern, escapeGlobLiteral } from "./matcher.js"; import { evaluateApprovals, grantScopeMatches, type GrantWorkspace } from "./authz-grants.js"; import { splitChainedCommand, isShellCommentOnly, stripCommentLines } from "./command.js"; -import { createPathRestriction } from "./path-restriction.js"; +import { createPathRestriction, resolveWorkspacePath } from "./path-restriction.js"; import { createWorktreeRootsProvider, type RootsProvider } from "./worktree-roots.js"; import { OPERATOR_DECLINED_PREFIX } from "./decline-markers.js"; import { getSubAgentIdentity } from "../subagent/identity-context.js"; @@ -378,6 +379,25 @@ function canSafelyMintPerSegment(pattern: string): boolean { return true; } +// posix pathEscapePlugin rewrites path-like args to resolveWorkspacePath before +// gateToolCall. Cache identity must use that same resolution so an authorizeCall +// allow is not treated as a different call (and re-decided) at execution. +function identityArguments( + args: ToolCall["arguments"], + cwd: string, + rootsProvider: RootsProvider, +): string { + const normalized: Record = {}; + for (const [key, value] of Object.entries(args)) { + if (typeof value === "string" && looksLikePath(key)) { + normalized[key] = resolveWorkspacePath(cwd, value, rootsProvider) ?? value; + } else { + normalized[key] = value; + } + } + return JSON.stringify(normalized); +} + export function createPermissionGate(options: PermissionGateOptions): PermissionGate { const { requestApproval, persist, interactive, providerName, model, cwd } = options; const reactorGated = options.reactorGated; @@ -496,11 +516,13 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // 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). + // consume-once when identity matches. Path-like arguments are compared after + // the same workspace resolve pathEscapePlugin applies, so a Darwin + // /var/folders vs /private/var/folders rewrite is still the same call. + // reset() clears leftovers (outer tools that never hit posix middleware). const authorizedByCallId = new Map< string, - { name: string; arguments: ToolCall["arguments"]; verdict: AuthorizeVerdict } + { name: string; arguments: string; verdict: AuthorizeVerdict } >(); // Non-blocking policy decision for one tool call: everything the gate owns — @@ -775,9 +797,10 @@ export function createPermissionGate(options: PermissionGateOptions): Permission const authorizeCall = async (call: ToolCall): Promise => { const verdict = mapAuthorizeVerdict(await decide(call)); + const identityCwd = getSubAgentIdentity()?.cwd ?? resolvedCwd; authorizedByCallId.set(call.id, { name: call.name, - arguments: call.arguments, + arguments: identityArguments(call.arguments, identityCwd, rootsProvider), verdict, }); return verdict; @@ -785,10 +808,11 @@ export function createPermissionGate(options: PermissionGateOptions): Permission const executionVerdict = async (call: ToolCall): Promise => { const cached = authorizedByCallId.get(call.id); + const identityCwd = getSubAgentIdentity()?.cwd ?? resolvedCwd; if ( cached !== undefined && cached.name === call.name && - JSON.stringify(cached.arguments) === JSON.stringify(call.arguments) + cached.arguments === identityArguments(call.arguments, identityCwd, rootsProvider) ) { authorizedByCallId.delete(call.id); return cached.verdict; diff --git a/src/permission/reactor-authorize.test.ts b/src/permission/reactor-authorize.test.ts index bb353e207..25f6465bf 100644 --- a/src/permission/reactor-authorize.test.ts +++ b/src/permission/reactor-authorize.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { mkdtempSync } from "node:fs"; +import { mkdtempSync, realpathSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createPermissionGate } from "./gate.js"; @@ -73,6 +73,47 @@ test("worker grant after a denied write allows a later call id through gateToolC expect(called).toBe(true); }); +test("worker grant survives pathEscape realpath rewrite through gateToolCall", async () => { + const cwd = mkdtempSync(join(tmpdir(), "worker-grant-realpath-")); + const lexical = join(cwd, "probe.txt"); + const escaped = join(realpathSync(cwd), "probe.txt"); + const policy = createPermissionGate({ + cwd, + approvals: [{ tool: "write_file", pattern: lexical }], + interactive: false, + auto: false, + skipPermissions: false, + reactorGated: true, + requestApproval: async () => { + throw new Error("worker must never ask"); + }, + }); + const workerGate = workerPermissionGate(policy); + const authorized: ToolCall = { + id: "call_auto_0", + name: "write_file", + arguments: { path: lexical, content: "unauthorized" }, + }; + const executed: ToolCall = { + id: "call_auto_0", + name: "write_file", + arguments: { path: escaped, content: "unauthorized" }, + }; + expect((await workerGate.authorizeCall(authorized)).effect).toBe("allow"); + let called = false; + const result = await gateToolCall( + workerGate, + executed, + new AbortController().signal, + async () => { + called = true; + return { callId: executed.id, content: "executed", isError: false }; + }, + ); + expect(result.isError).toBe(false); + expect(called).toBe(true); +}); + test("worker maps unresolved ask to deny while main reactor suspends", async () => { const policy = gate(); expect((await createReactorAuthorize(policy)("tool:write_file", "invoke", call)).effect).toBe(