Skip to content

feat(code-execution): expose read-only MCP tools in execute_code sandbox (#97044) - #103604

Open
0xalydev wants to merge 1 commit into
NousResearch:mainfrom
0xalydev:feat/execute-code-readonly-mcp-tools-97044
Open

feat(code-execution): expose read-only MCP tools in execute_code sandbox (#97044)#103604
0xalydev wants to merge 1 commit into
NousResearch:mainfrom
0xalydev:feat/execute-code-readonly-mcp-tools-97044

Conversation

@0xalydev

@0xalydev 0xalydev commented Sep 5, 2026

Copy link
Copy Markdown

Summary

Implements the maintainer-directed design by @teknium1 in #97044 ("feat(code-execution): expose MCP tools inside execute_code (hermes_tools stubs) — design + gates") to bring Anthropic's Programmatic Tool Calling (PTC) capability to user MCPs while strictly respecting all 5 design gates.

Gate Implementation

  1. Gate 1 — Security / Read-Only Gating:

    • Evaluated at stub-generation and RPC validation time from discovery metadata.
    • Only MCP tools with readOnlyHint: true annotation (and read-only utilities list_resources, read_resource, list_prompts, get_prompt) are exposed in execute_code.
    • All write-capable MCP tools (missing annotation or readOnlyHint: false) are excluded.
  2. Gate 2 — Schema Budget & Diet (refactor(execute_code): schema diet — persistence woven in, not bolted on (712 → 654 tok/call) #96997):

    • No per-tool multi-line docstring bloat.
    • Emits a compact name-list note in the execute_code tool description:
      "Also callable via from hermes_tools import ...: linear_search(...), notion_query(...) (same arguments as the model-visible tools)."
    • Preserves prompt cache byte stability and context token budget.
  3. Gate 3 — Config Gate:

    • code_execution.expose_mcp_tools: false by default (0 tokens added, 100% backward compatible).
    • Opt-in via true (exposes all active read-only MCP tools) or list of strings ["linear", "notion_query"] (scoped per server or tool).
  4. Gate 4 — Call Budget Attribution:

    • Teaching error explicitly attributes the blocked tool when the per-cell 50-call budget is reached ("Tool call limit reached (50). Call to '<tool>' blocked — no more tool calls allowed in this execution.").
  5. Gate 5 — Server-side RPC Allow-List Enforcement & Remote Parity:

    • Enforced in the host RPC server (_handle_rpc_request in tools/code_execution_rpc.py) for both local socket transport and remote file-based polling (_rpc_poll_loop / code_kernel_remote.py).
    • Raw RPC calls attempting to invoke unexposed or write-capable tools fail closed with "Tool '<tool>' is not available in execute_code".

Testing

  • Added tests/tools/test_code_execution_mcp.py (14 unit tests covering read-only detection, config gating, schema budget, stub generation, RPC allow-list enforcement, and failure hints).
  • All 14 tests pass (pytest tests/tools/test_code_execution_mcp.py).
  • Existing test suites pass (pytest tests/tools/test_code_execution_modes.py -> 28 passed, 9 skipped).

Closes #97044
cc @teknium1

…box (NousResearch#97044)

Implements maintainer design from @teknium1 for programmatic MCP tool calling:
- Gate 1 (read-only): only MCP tools with readOnlyHint=True and read-only utilities are exposed; write-tools excluded.
- Gate 2 (schema budget): compact name-list note in execute_code schema description; no per-tool doc bloat (NousResearch#96997).
- Gate 3 (config gate): code_execution.expose_mcp_tools defaults to false; supports boolean or server/tool filter list.
- Gate 4 (call budget): error message names blocked tool on exhaustion.
- Gate 5 (RPC enforcement): allow-list enforced server-side in local socket & remote file RPC loops.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have tool/code-exec execute_code sandbox tool/mcp MCP client and OAuth comp/tools Tool registry, model_tools, toolsets area/config Config system, migrations, profiles duplicate This issue or pull request already exists labels Sep 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #98615 — that open PR already implements the #97044 gated design (same code_execution.expose_mcp_tools config gate, readOnlyHint filtering, host-side RPC allowlist) and additionally covers the remote kernel path and docs. Consider consolidating any deltas (e.g. the call-budget attribution message) into #98615 as review feedback.

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed exact head 8f1768b32e07943382c176471c6cde328be08fa0 against current main/base b499ab11fe8b081470e269f2fb27abae03000da5, the full 7-file diff, #97044's five gates, current MCP trust/annotation registration, the local/remote RPC paths, the existing trust-gating tests, and the overlapping implementation history. The default-off config, compact schema surface, raw-RPC allow-listing, and explicit exhausted-budget teaching error are all the right shape. I do not think this head is safe to land yet.

1. Blocker — readOnlyHint is being promoted from untrusted metadata into programmatic authority

tools/mcp_tool_registration.py already records two distinct facts: _server_trust_levels[server] and _tool_read_only_hints[server][tool]. This PR's new is_mcp_tool_read_only() / get_read_only_mcp_tools() consult the hint but never the server trust level. That means a server explicitly configured as trust: untrusted can advertise readOnlyHint: true, enter the execute_code stub/allow-list, and then be invoked programmatically in a loop.

That is a materially stronger authority grant than the existing single model-visible call path. It is also directly contrary to MCP's annotation contract: ToolAnnotations are hints, are not guaranteed faithful, and clients should not make tool-use decisions from annotations received from untrusted servers. A malicious/untrusted server can simply label a mutating tool read-only.

Required repair: programmatic eligibility needs a trustworthy authority fact, not the hint alone. At minimum, combine exact readOnlyHint is True with the repository's server trust policy (or introduce an explicit, separately named operator grant for programmatic invocation). An explicitly untrusted server must not become approval-less execute_code authority merely by self-reporting readOnlyHint: true. Please add regressions for trust: untrusted + readOnlyHint=True and for the corresponding trusted/explicitly-authorized case.

Reference: MCP schema contract: https://modelcontextprotocol.io/specification/2025-11-25/schema

2. Blocker — the claimed RPC-time revalidation is not present; eligibility is snapshotted before execution

The PR body says the read-only gate is evaluated "at stub-generation and RPC validation time." The actual tools/code_execution_rpc.py change only alters the budget error string. _handle_rpc_request() still authorizes solely by membership in the previously computed allowed_tools set; it never re-reads the current MCP classification/config before dispatch.

That leaves a capability-drift/TOCTOU hole for persistent execution: a tool can be admitted as read-only, then a live MCP list_changed refresh can downgrade/re-register the same registry name as write-capable while the cell still holds the stale allowed name. The host consumes the stale name and dispatches the current handler. This is exactly the consequential boundary where the current authority has to be revalidated, not reconstructed from the earlier stub set.

Required repair: on every MCP RPC, re-resolve the current tool provenance + current read-only/trust/config eligibility immediately before dispatch and fail closed if any coordinate changed. Add a regression that admits a read-only tool, downgrades/re-registers the same name, then proves a raw RPC is refused without reaching the handler.

There is already earlier work on this exact defect class in #98615: its RPC path explicitly revalidates live MCP exposure on every local UDS and remote file-RPC call and its registration changes track read-only downgrades. Preserve that contributor's work rather than independently re-solving the same boundary.

3. Merge-order / ownership blocker — #98615 is a direct earlier implementation of the same #97044 contract

#98615 was opened August 30 for the same issue and currently implements the same direct read-only-MCP-in-execute_code feature across the same core owners (model_tools.py, tools/code_execution_tool.py, MCP discovery/registration, RPC, kernels, config, tests). It additionally carries live downgrade revalidation and a separate default MCP sub-budget. This PR was opened September 5 and is therefore not an independent new defect class.

Before either lands, please consolidate to one semantic owner. If #103604 becomes the landing vehicle, carry forward the unique security/tests/behavior from #98615 and preserve ayushnangia's attribution for that work; if #98615 remains canonical, this PR should contribute only unique delta rather than establish a competing config/runtime contract. #94647, #96991, and #96997 are complementary foundations, not duplicates. #82243 is adjacent broader Tool Search/deferred-tool composition and should remain a distinct contract; it does, however, touch the same execute_code ownership surface and will need explicit conflict/merge-order preservation if it advances.

4. Verification blocker — remote parity is asserted but the required topology proof is absent, and the only surviving commit is not green

#97044 explicitly called out #96991's remote harness as the acceptance proof before claiming remote parity. The new tests here call _handle_rpc_request() directly; they do not exercise the remote file-RPC poller/persistent-remote topology or the #96991 live harness. So the current "Remote Parity" claim is ahead of the evidence.

Hosted exact-head evidence is also not green: CI, Docker Build, Test, and Publish, and Nix flake check are all action_required; the inspected runs have zero jobs. With one surviving commit, that means the entire surviving series is currently unverified rather than green.

Required repair: exercise the actual remote transport/harness with an allowed read-only MCP call plus a denied write/untrusted/downgraded call, then obtain exact-head hosted green receipts for every surviving commit.

The core feature direction is useful, and the default-off/schema-diet pieces are clean. The landing bar here is mostly about making the authority proof survive the projection from MCP metadata → sandbox stub → persistent kernel → actual RPC consume boundary, and consolidating the already-open carrier instead of splitting policy/provenance across two implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have tool/code-exec execute_code sandbox tool/mcp MCP client and OAuth type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(code-execution): expose MCP tools inside execute_code (hermes_tools stubs) — design + gates

3 participants