Skip to content

Commit 13add2b

Browse files
committed
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.
1 parent 62c6f46 commit 13add2b

8 files changed

Lines changed: 120 additions & 41 deletions

File tree

docs/ARCHITECTURE.md

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

383383
- **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).
384384
- **authz-grants** — Maps stored approvals into `@intx/authz` `GrantRule`s and evaluates them with `evaluateGrants` (allow-only; Corbits cwd/provider-model filters applied first). Exact-escaped grants bypass the package path and use equality.

src/agent/tools.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ import {
3232
EXA_MCP_SERVER_NAME,
3333
isBuiltinExaMCPServer,
3434
} from "../mcp/exa.js";
35-
import { mcpClientToAgentTools } from "../mcp/plugin.js";
35+
import { mcpClientTools } from "../mcp/plugin.js";
3636
import { parseMcpToolName } from "../mcp/tool-name.js";
37+
import { gateAgentTools } from "../plugins/permission-plugin.js";
3738
import { createDynamicToolRunner, type DynamicToolRunner } from "../tui/dynamic-tool-runner.js";
3839
import type { MCPServerConfig, Settings } from "../config/settings.js";
3940
import {
@@ -402,7 +403,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
402403
fleetSessionsForDispose = fleetSessions;
403404
const fleetDeps = {
404405
permissionGate,
405-
inheritMcpTools: () => inheritedMcpTools,
406+
inheritMcpTools: (gate: PermissionGate) => gateAgentTools(inheritedMcpTools, gate),
406407
...(shellTimeout !== undefined ? { shellTimeout } : {}),
407408
...(shellEnv !== undefined ? { shellEnv } : {}),
408409
...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}),
@@ -798,12 +799,12 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
798799
return;
799800
}
800801
permissionGate.registerMcpClient(result.client);
801-
const mcpTools = mcpClientToAgentTools(result.client, permissionGate, {
802+
const mcpTools = mcpClientTools(result.client, {
802803
...(getBlobWriter !== undefined ? { getBlobWriter } : {}),
803804
...(getContextDir !== undefined ? { getContextDir } : {}),
804805
...(isBuiltinExaMCPServer(config) ? { excludeToolNames: ["web_fetch_exa"] } : {}),
805806
});
806-
dynamicRunner.addTools(mcpTools);
807+
dynamicRunner.addTools(gateAgentTools(mcpTools, permissionGate));
807808
inheritedMcpTools.push(...mcpTools);
808809
connectedClients.set(config.name, result.client);
809810
} catch (err) {

src/mcp/plugin.ts

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { AgentTool } from "@intx/agent";
22
import type { ToolCall, ToolResult } from "@intx/types/runtime";
33
import type { PermissionGate } from "../permission/gate.js";
4-
import { gateToolCall } from "../plugins/permission-plugin.js";
4+
import { gateAgentTools } from "../plugins/permission-plugin.js";
55
import { scrubSecretShapedContent } from "../plugins/tool-result-secret-scrub.js";
66
import {
77
truncateToolResultContent,
@@ -27,14 +27,7 @@ function sanitizeMcpResultContent(
2727
return truncateToolResultContent(scrubSecretShapedContent(content), undefined, spill);
2828
}
2929

30-
// Convert a connected client's tools into AgentTools for the dynamic runner used
31-
// by the TUI. These tools live in a separate runner from the posix tool plugin
32-
// chain, so each handler is wrapped with the permission gate directly.
33-
export function mcpClientToAgentTools(
34-
client: MCPClient,
35-
gate: PermissionGate,
36-
spillOptions: McpSpillOptions = {},
37-
): AgentTool[] {
30+
export function mcpClientTools(client: MCPClient, spillOptions: McpSpillOptions = {}): AgentTool[] {
3831
const { getBlobWriter, getContextDir, excludeToolNames = [] } = spillOptions;
3932
const excluded = new Set(excludeToolNames);
4033

@@ -47,28 +40,38 @@ export function mcpClientToAgentTools(
4740
description: `[${client.serverName}] ${tool.description}`,
4841
inputSchema: tool.inputSchema,
4942
},
50-
handler: (call: ToolCall, signal: AbortSignal): Promise<ToolResult> =>
51-
gateToolCall(gate, call, signal, async () => {
52-
try {
53-
const content = await client.call(tool.name, call.arguments, signal);
54-
const writeBlob = getBlobWriter?.();
55-
const contextDir = getContextDir?.();
56-
const spill =
57-
writeBlob !== undefined
58-
? {
59-
callId: call.id,
60-
writeBlob,
61-
...(contextDir !== undefined ? { contextDir } : {}),
62-
}
63-
: undefined;
64-
return { callId: call.id, content: await sanitizeMcpResultContent(content, spill) };
65-
} catch (err) {
66-
return {
67-
callId: call.id,
68-
content: err instanceof Error ? err.message : String(err),
69-
isError: true,
70-
};
71-
}
72-
}),
43+
handler: async (call: ToolCall, signal: AbortSignal): Promise<ToolResult> => {
44+
try {
45+
const content = await client.call(tool.name, call.arguments, signal);
46+
const writeBlob = getBlobWriter?.();
47+
const contextDir = getContextDir?.();
48+
const spill =
49+
writeBlob !== undefined
50+
? {
51+
callId: call.id,
52+
writeBlob,
53+
...(contextDir !== undefined ? { contextDir } : {}),
54+
}
55+
: undefined;
56+
return { callId: call.id, content: await sanitizeMcpResultContent(content, spill) };
57+
} catch (err) {
58+
return {
59+
callId: call.id,
60+
content: err instanceof Error ? err.message : String(err),
61+
isError: true,
62+
};
63+
}
64+
},
7365
}));
7466
}
67+
68+
// Convert a connected client's tools into AgentTools for the dynamic runner used
69+
// by the TUI. These tools live in a separate runner from the posix tool plugin
70+
// chain, so each handler is wrapped with the permission gate directly.
71+
export function mcpClientToAgentTools(
72+
client: MCPClient,
73+
gate: PermissionGate,
74+
spillOptions: McpSpillOptions = {},
75+
): AgentTool[] {
76+
return gateAgentTools(mcpClientTools(client, spillOptions), gate);
77+
}

src/plugins/permission-plugin.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { AgentTool } from "@intx/agent";
12
import type { ToolPlugin } from "@intx/tools-posix";
23
import type { ToolCall, ToolResult } from "@intx/types/runtime";
34
import { BLOCKED_BY_POLICY_PREFIX } from "../permission/decline-markers.js";
@@ -32,6 +33,19 @@ export async function gateToolCall(
3233
return next(call, signal);
3334
}
3435

36+
// Bind full AgentTools to a gate. Workers pass workerPermissionGate so inherited
37+
// MCP handlers skip middleware the same way posix plugins do.
38+
export function gateAgentTools(tools: readonly AgentTool[], gate: PermissionGate): AgentTool[] {
39+
return tools.map((tool) => {
40+
if (tool.kind !== "full") return tool;
41+
const inner = tool.handler;
42+
return {
43+
...tool,
44+
handler: (call: ToolCall, signal: AbortSignal) => gateToolCall(gate, call, signal, inner),
45+
};
46+
});
47+
}
48+
3549
// Gate consequential tool calls on operator approval. Runs after the
3650
// authorization plugin (which hard-denies catastrophic commands), so by the time
3751
// a call reaches here it is at worst "consequential but legitimate" — the gate

src/subagent/run.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ async function runSubAgentInner(
523523
),
524524
}));
525525

526-
const inherited = params.inheritMcpTools?.() ?? [];
526+
const inherited = params.inheritMcpTools?.(permissionGate) ?? [];
527527
tools = [...tools, ...coreSubAgentWebTools(inherited)];
528528

529529
if (inherited.length > 0) {

src/subagent/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export interface SubAgentProvider {
4242
// recursion bottoms out at one hop of orchestration.
4343
export interface SubAgentSandboxDeps {
4444
permissionGate: PermissionGate;
45-
inheritMcpTools?: () => readonly AgentTool[];
45+
inheritMcpTools?: (gate: PermissionGate) => readonly AgentTool[];
4646
shellTimeout?: ShellTimeoutConfig;
4747
extraToolPlugins?: ToolPlugin[];
4848
/** Parent session blob store for bounded tool-output:// reads in workers. */

tests/integration/subagent-permission.test.ts

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,7 @@ test.serial(
264264
close: async () => undefined,
265265
};
266266
params.permissionGate.registerMcpClient(client);
267-
const tools = mcpClientToAgentTools(client, params.permissionGate);
268-
params.inheritMcpTools = () => tools;
267+
params.inheritMcpTools = (gate) => mcpClientToAgentTools(client, gate);
269268
harness.scenario.replyOnce("openai", {
270269
toolCalls: [{ name: "mcp__probe__mutate", args: {} }],
271270
});
@@ -293,6 +292,66 @@ test.serial(
293292
20000,
294293
);
295294

295+
test.serial(
296+
"allowed inherited MCP call with middleware-gated parent does not requestApproval",
297+
async () => {
298+
let asks = 0;
299+
await withWorker(
300+
async ({ harness, params, audit }) => {
301+
let calls = 0;
302+
const client = {
303+
serverName: "probe",
304+
tools: [
305+
{
306+
name: "mutate",
307+
description: "mutates",
308+
inputSchema: { type: "object", properties: {} },
309+
},
310+
],
311+
call: async () => {
312+
calls++;
313+
return "changed";
314+
},
315+
close: async () => undefined,
316+
};
317+
params.permissionGate.registerMcpClient(client);
318+
params.permissionGate.setSeededApprovals([
319+
{ tool: "mcp__probe__mutate", pattern: "mcp__probe__mutate" },
320+
]);
321+
const authorize = params.permissionGate.authorizeCall;
322+
params.permissionGate.authorizeCall = async (call) => {
323+
const result = await authorize(call);
324+
params.permissionGate.setSeededApprovals([]);
325+
return result;
326+
};
327+
params.inheritMcpTools = (gate) => mcpClientToAgentTools(client, gate);
328+
harness.scenario.replyOnce("openai", {
329+
toolCalls: [{ name: "mcp__probe__mutate", args: {} }],
330+
});
331+
harness.scenario.replyOnce("openai", { text: report });
332+
await Promise.all([runSubAgent(params), harness.run({ wallClockBudgetMs: 15000 })]);
333+
expect(calls).toBe(1);
334+
expect(asks).toBe(0);
335+
expect((await audit())[0]?.authz?.effect).toBe("allow");
336+
},
337+
(cwd) =>
338+
createPermissionGate({
339+
cwd,
340+
approvals: [],
341+
interactive: true,
342+
auto: false,
343+
skipPermissions: false,
344+
reactorGated: false,
345+
requestApproval: async () => {
346+
asks++;
347+
return { allow: true };
348+
},
349+
}),
350+
);
351+
},
352+
20000,
353+
);
354+
296355
test.serial(
297356
"live worker authorization is not evaluated again after policy revocation before runner",
298357
async () => {

tests/unit/tui/agent-tools.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ await withMockedModule(
7373
);
7474

7575
await withMockedModule(import.meta.resolve("../../../src/mcp/plugin.js"), () => ({
76+
mcpClientTools: () => [],
7677
mcpClientToAgentTools: () => [],
7778
}));
7879

@@ -90,6 +91,7 @@ await withMockedModule(import.meta.resolve("../../../src/plugins/verify-plugin.j
9091

9192
await withMockedModule(import.meta.resolve("../../../src/plugins/permission-plugin.js"), () => ({
9293
permissionPlugin: () => ({}),
94+
gateAgentTools: (tools: unknown) => tools,
9395
gateToolCall: async (
9496
_gate: unknown,
9597
call: ToolCall,

0 commit comments

Comments
 (0)