fix(executor): enforce MCP tool_permissions at the tool-call boundary

[kgabryje]

Problem

mcp_servers.data.tool_permissions (Record<string, 'ask' | 'allow' | 'deny'>) was persisted, settable via agor_mcp_servers_update, and typed in packages/core/src/types/mcp.ts — but never read by any handler. Write-only config: set write_file: deny and the executor called write_file anyway.

What this does

Enforces it at the point each SDK decides whether a tool call proceeds. The mechanism is necessarily per-handler — the tool list a model sees is discovered live from the MCP server by each SDK, not read from the cached server.tools snapshot, so the three SDKs expose three non-unifiable filter primitives.

Handler	deny	ask
Claude	disallowedTools + PreToolUse hook + canUseTool	routes to the existing permission prompt
Gemini	excludeTools (dropped at discovery)	fails closed — no prompt channel
Codex	disabled_tools	fails closed — no prompt channel
Copilot	per-server tools include-list	fails closed (see caveat below)
Cursor / OpenCode	server withheld entirely	server withheld entirely

allow and unlisted tools are unchanged throughout.

Namespacing. tool_permissions is keyed on the bare tool name; SDKs qualify differently — Claude mcp__<server>__<tool>, Gemini bare (or <server>__<tool> on collision), Codex per-server config. base/mcp-tool-permissions.ts bridges this with longest-server-prefix matching and sanitized/lowercased name aliases.

Admission gate. getMcpServersForSession now takes a required HandlerPermissionCapabilities and withholds any server whose permissions the calling handler cannot honour, reusing the existing template-resolution drop pattern. Six call sites resolve MCP servers; three originally enforced. Making the argument required means a new handler can't repeat that omission silently.

Security fixes found during architectural review

Both predate this branch and are independent of the original bug:

• allowedTools was a permission-rule injection sink. It was fed server.tools[].name — whatever a remote MCP server returns from tools/list, stored verbatim. The SDK defines allowedTools as "auto-allowed without prompting" and it matches built-in tool names, so an attached server advertising a tool called Bash silently disabled the permission prompt for the built-in Bash. It was also inert for its stated purpose, since bare names never match mcp__server__tool. Removed; the false MCPTool.name comment that made it look correct is fixed.
• A DB server named agor could hijack the built-in Agor MCP namespace. User servers were spread over the built-in config, so a server named agor inherited the session's daemon bearer token and permission-hooks' unconditional auto-approve. Codex already guarded this; Claude had the spread backwards. The key is now reserved.

Also closed: a stale settings.json allow rule (written by Agor itself on "remember") could skip an ask gate, since the SDK matches rules before canUseTool — a PreToolUse hook now decides first. And ask no longer degrades to allow under bypassPermissions.

Testing

44 new tests. The deny path is asserted at the dispatch boundary — calls are routed through a stand-in for the SDK's forward step that only reaches the server on allow, so a deny that still dispatched would fail. Coverage: deny / ask / allow / unlisted-default, namespaced-vs-bare mapping, the admission gate across all three capability shapes, bypassPermissions, and both security fixes above.

pnpm check passes (typecheck 11/11, lint, shortid, multitenancy-boundaries, builds 7/7). Executor 660/660, core 2908 passed / 15 skipped.

Known gaps — not fixed here

• caps.interactiveApproval is declared at all six call sites and read nowhere. The admission gate switches only on toolFiltering. Each handler still computes its own prompt-channel decision independently, so behaviour is correct, but the field is currently dead config — the same category as the bug this PR fixes. Should either be consumed by the shared resolver or deleted.
• Copilot declares interactiveApproval: true but behaves as false. Its permission handler never consults tool_permissions, so its isAttachedMcpServer auto-approve fast path — the hole closed on Claude — is still open at runtime. Config-level enforcement works; the runtime per-tool check does not exist.
• Subagent propagation unverified. Whether disallowedTools and hooks reach Task-tool sidechains could not be confirmed from the minified CLI bundle. Wants one integration test.
• Out of scope per the brief: no UI for tool_permissions (only agents can set it today), no validation of keys against the discovered tool list (a typo'd key is a silently ineffective deny), and no server-name validation at the REST boundary.

🤖 Generated with Claude Code