feat(security): add first-invoke approval for MCP server tools

[tgmerritt]

What does this PR do?

Implements first-invoke approval for MCP server tools, as proposed in #16462.

Today, when a user adds an MCP server, its tools are registered in the tool registry and become immediately callable by the LLM with no human confirmation — unlike the terminal tool, which is gated by DANGEROUS_PATTERNS and the approval system. SECURITY.md assigns MCP servers lower trust than installed skills, but the dispatch path didn't reflect that.

With this change, the first call to each (server, tool) pair per session triggers the same approval flow already used for dangerous commands. The prompt shows the server id, tool name, and a truncated JSON dump of the arguments, so the user sees exactly what a newly added server is about to do before anything runs. Approval scopes map onto the existing once/session/always machinery:

• once — this single dispatch only (no persistence; addresses @kayalopez's "approve once is consumed by one dispatch only")
• session — this (server, tool) pair until the session is cleared (/new resets it via clear_session, matching the issue's "reset on /new")
• always — persisted to command_allowlist in config.yaml

Per-tool keys (mcp:<server>:<tool>) are deliberately narrower than per-server, following the review comments on the issue ("approve for this server should not silently approve newly added tools" — @kayalopez, @Ram9199). Users who want server-wide permanent trust can add mcp:<server>:* to command_allowlist explicitly.

Design decisions worth flagging for review:

1. Default-enabled (approvals.mcp_first_invoke: true). Friction is one prompt per (server, tool) pair per session, on interactive surfaces only. Happy to flip the default to opt-in if preferred — it's one line.
2. Headless/cron contexts auto-approve with a log line rather than blocking. First-use visibility has no value when nobody is watching, and a default-deny would break every existing cron/batch profile that uses MCP tools. Unattended profiles should restrict their tool surface via mcp_servers.<name>.tools.include/exclude instead (docs updated to say so).
3. smart mode does not auto-approve first invokes. First-use consent is a visibility decision, not a risk assessment the aux LLM can make; only --yolo/mode: off/the dedicated toggle bypass it.
4. A denial does not bump the MCP circuit breaker — it's user intent, not a server fault.
5. Scope is tools/call dispatch only; the resources/prompts utility tools are unchanged (can follow up if desired).

Related Issue

Fixes #16462

Type of Change

• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix

Changes Made

• tools/approval.py — new check_mcp_tool_guard() (modeled on check_execute_code_guard), plus mcp_approval_key() and the approvals.mcp_first_invoke config read. Reuses the existing session/permanent approval stores, gateway approval queue (_await_gateway_decision), CLI prompt path, and pre/post_approval_request plugin hooks.
• tools/mcp_tool.py — _make_tool_handler() calls the guard before dispatching tools/call; a block returns the standard {"error": ...} JSON to the model without reaching the MCP loop.
• website/docs/user-guide/security.md — documents the new key in the approvals table and example YAML.
• tests/tools/test_mcp_tool_approval.py — 21 new tests covering the decision matrix (toggle, yolo/off, headless/cron auto-approve, once/session/always persistence, per-tool vs wildcard keys, /new reset, gateway deny/timeout/missing-notify, CLI callback path, args preview + truncation) and handler integration (deny blocks before dispatch; breaker not bumped).

How to Test

1. scripts/run_tests.sh tests/tools/test_mcp_tool_approval.py — 21/21 pass.
2. Regression: scripts/run_tests.sh tests/tools/test_approval.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_approval_plugin_hooks.py tests/tools/test_cron_approval_mode.py tests/tools/test_yolo_mode.py tests/tools/test_hardline_blocklist.py tests/gateway/test_approve_deny_commands.py — 395/395 pass.
3. Regression: all MCP tool test files (tests/tools/test_mcp_*.py handler/breaker/discovery/stability suites) — 283/283 pass (hermetic test env is headless, so existing handler tests hit the auto-approve path unchanged).
4. Manual: add any MCP server, ask the agent to call one of its tools in a gateway session → approval request with server/tool/args appears; /approve once allows that call only, asking again re-prompts; approve session silences that pair until /new.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run the test suites above via scripts/run_tests.sh and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Ubuntu 24.04 (aarch64)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — N/A (the approvals: block is not present in the example file; documented in website/docs/user-guide/security.md alongside the other approvals keys)
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — pure-Python control flow, no platform-specific APIs
• I've updated tool descriptions/schemas if I changed tool behavior — N/A (no schema changes; blocked calls return the standard error JSON contract)

🤖 Generated with Claude Code

[liuhao1024]

Code Review: Verification

Reviewed by: automated PR review

This PR is clean — no substantive issues found.

What it does: Adds a first-invoke approval guard for MCP server tools. The first call to each (server, tool) pair per session triggers the same approval flow used for dangerous terminal commands. Subsequent calls reuse the session approval.

Security posture is sound:

• Follows the existing approval architecture exactly (session approval, permanent allowlist, gateway notify callback, CLI prompt)
• Server wildcard (mcp:server:*) support allows blanket-trusting a server via command_allowlist
• Configurable via approvals.mcp_first_invoke (default: enabled)
• YOLO/off bypass, headless/cron auto-approve (no user present)
• Denial does NOT bump the MCP server circuit breaker — correct, since denial is user intent not server fault
• Deny message explicitly instructs the LLM not to retry or rephrase

Integration in mcp_tool.py: The guard check runs in the tool-executor thread (which holds session context and the per-thread approval callback). The _get_approval_callback import is wrapped in try/except for robustness.

Test coverage: 350 lines covering the full decision matrix — config toggle, yolo/off bypass, headless auto-approve, session and wildcard approval, gateway approve/deny/timeout, CLI callback, persistence scopes.

Minor note: The approval check is synchronous and blocks the tool-executor thread waiting for user response. In gateway contexts with a long approval timeout, this could delay other tool calls in the same agent turn. This matches the existing terminal-command approval behavior, so it's consistent — just worth being aware of.

[egilewski]

Approve. I reviewed current GitHub main d1383a6 against PR head 88fc209da5d3b2d1719f45ceff7022b07a34bfe2. The MCP first-invoke guard is wired before tools/call dispatch, uses per-session/per-tool approval keys with explicit wildcard support, and the deny path returns an error without bumping the MCP server circuit breaker.

Validation:

• git merge-tree --write-tree upstream/main refs/remotes/upstream/pr/43045 => f27fcd6041bba5f4bbf28992d194720819655e21
• git diff --check upstream/main...refs/remotes/upstream/pr/43045 passed
• pytest -q tests/tools/test_mcp_tool_approval.py -p no:cacheprovider => 21 passed
• pytest -q tests/tools/test_approval.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_approval_plugin_hooks.py tests/tools/test_cron_approval_mode.py tests/tools/test_yolo_mode.py tests/tools/test_hardline_blocklist.py tests/gateway/test_approve_deny_commands.py -p no:cacheprovider => 395 passed
• pytest -q tests/tools/test_mcp_*.py -p no:cacheprovider => 488 passed, 1 existing cli.py SyntaxWarning
• direct probes confirmed gateway-without-notifier blocks as pending approval and mcp:<server>:* wildcard approval covers other tools on that server.

Signed: GPT-5.5-xhigh in Codex

[bedpan]

Following this PR with interest. We run a dual-agent Hermes deployment (two Docker containers) with GitHub and banking MCP servers connected. An agent posted an unapproved GitHub comment despite prompt-level rules prohibiting it — exactly the gap this PR addresses.

The first-invoke approval model would solve our use case well. One thing we'd find useful: a config option to keep certain tools permanently gated (require approval every session, not just first invoke) — e.g. banking writes should never get "approve for this server," while GitHub reads could be pre-approved.

Happy to test on our setup when this is ready.

[armorer-labs]

The permanent-gating point from @bedpan seems worth making explicit before this lands. First-invoke approval and per-action approval are solving different problems:

• first-invoke approval answers “is this server/tool pair allowed to become visible to the agent in this session?”;
• per-action approval answers “is this exact side-effecting call, with these args and this destination, authorized right now?”

A config shape that tends to avoid footguns is to classify tools by approval durability, not only by allow/deny:

• first_invoke: prompt once per (server, tool, schema digest) and then allow for the session;
• always: require approval for every call, even after first invoke;
• never: deny or hide the tool;
• optional allow: explicit permanent allow for low-risk reads/diagnostics.

For the always class, I would also make “Approve always” unavailable in the UI. Banking writes, outbound messages, repo writes, deploys, and credential/admin actions should not be able to escape back into session trust by one button click.

The regression test I would add is: approve first invoke for mcp_github_add_issue_comment, then ask the agent to post to a different issue or with materially different body text, and verify a fresh approval is required if the tool is classified always. That catches the case where first-use visibility accidentally becomes blanket write authority.

Disclosure: I work on Armorer Labs.

[teknium1]

Thanks for implementing the first-use MCP approval flow. The underlying gap still exists on current main: registered MCP handlers reach server.session.call_tool(...) directly at tools/mcp_tool.py:3943.

Problems

• Blocking: tools/approval.py:1835 reads HERMES_INTERACTIVE directly. Current main explicitly replaced process-global interactive detection with _is_interactive_cli() because concurrent ACP sessions can race the environment and auto-approve an action without the callback (tools/approval.py:54-93; acp_adapter/server.py:1504). The MCP guard must use that ContextVar-aware helper.

Suggested changes

• Replace the direct environment check with _is_interactive_cli() and add an ACP-context regression where set_hermes_interactive_context(True) is set while the environment flag is absent.
• Apply the guard against the current handler lifecycle, after the reconnect/live-session checks at tools/mcp_tool.py:3893-3932 and before call_tool at line 3943.

Automated hermes-sweeper review.

[teknium1]

Blocking: use _is_interactive_cli() here rather than the process-global environment flag. Current main moved interactive state to a ContextVar because concurrent ACP sessions can race HERMES_INTERACTIVE and incorrectly take an auto-approve path (tools/approval.py:54-93).

[tgmerritt]

Rebased onto current main — conflicts resolved. Ready for review.