Repository navigation
agent: fence MCP 401 messages, keep challenges; one notice per dropped over-cap message - #149
Merged
Merged
Conversation
…rvives a body 1. The message a 401's JSON-RPC body carries went into the tool error raw and up to 64 KiB. It is untrusted text: now cut to 1 KiB (with an ellipsis), control characters made spaces, and fenced the way event payloads are (<mcp_server_message untrusted>…), with </> and quotes replaced so it cannot close its fence or a challenge parameter's quoted-string. 2. A 401 with both a WWW-Authenticate challenge and a JSON-RPC body lost the challenge (it became UnexpectedServerResponse). It now stays AuthRequired carrying the challenge, with the reason added as RFC 6750's error_description; a failed tool call prints its error's causes, so the challenge and the reason in it reach the model instead of a bare "Auth required". The post-refresh mcp-login error keeps the challenge in its text too. Tests (each fails with its fix reverted): mcp_unauthorized's a_servers_401_message_is_cut_short_and_fenced_as_untrusted and a_401_with_a_challenge_and_a_json_rpc_body_keeps_both; mcp_wire units a_401_with_a_challenge_and_a_reason_keeps_the_challenge and a_server_message_is_cut_short_and_cannot_close_its_fence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
…notice to the model 3. An over-cap message taken for an event without its method seen (a request whose id and method lie past the window looks the same) now reports its gap as possibly not an event, to the client (possibly_not_an_event) and to the model. With no push stream open on a stdio connection there was nothing to report it to, and it is now logged as dropped. 4. The stdio broadcast of a stand-in with no routing gave a session one gap notice per subscription. The router stamps the broadcast with one id per dropped message; the hub keeps each subscription's state and status but tells the model once per id. Tests (each fails with its fix reverted): mcp_message_cap's params-first cases now assert the label, and one_over_cap_message_reaching_two_subscriptions_is_one_notice_to_the_model (two gap frames, one model notice); units for the router's single id, the stand-in's ambiguity mark and the rendered label. ARCHITECTURE.md: items 1-4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
…`-keys only from the host Audit of #149 at 99919fd: 1. tool_call_err's cause walk printed rmcp's AuthRequiredError/InsufficientScopeError Display, which is the raw WWW-Authenticate header — unfenced and unbounded; so did the post-refresh mcp-login error. Both now pass the header through fenced_server_message, as do a non-JSON-RPC success's body and a legacy server/discover rejection's body. 2. fenced_server_message also replaces fullwidth, small-form and angle-bracket lookalikes of < > / " and drops bidi embeddings/overrides/isolates, zero-width and joiner characters and the BOM. 3. A server's own `$oversized`/`$dropped_id`/`$ambiguous` keys are stripped from every events notification at ingress (the router and direct-HTTP streams) unless it carries the per-process secret the host stamps into its own stand-ins (`$host`); the dropped-id sequence starts from that secret. Tests (each fails with its fix reverted): mcp_unauthorized's a_hostile_oversized_challenge_header_is_cut_short_and_fenced and the bare-challenge login test (the header shown fenced); units a_server_message_cannot_close_its_fence_with_lookalikes_or_hide_with_invisibles and a_servers_host_reserved_keys_are_stripped_at_ingress; the non-JSON-RPC body test now expects it fenced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
jaredLunde
force-pushed
the
jared/mcp-wire-polish
branch
from
October 7, 2026 10:23
99919fd to
eb2acf2
Compare
jaredLunde
added a commit
that referenced
this pull request
Oct 7, 2026
… model Re-audit of #149: 1. fenced_server_message drops by rule, not list: every character in General_Category Cc (tab, newline, CR become spaces), Cf, Co, Cn, Zl, Zp or Cs, the TAG block (ASCII smuggling), U+180E and the variation selectors. It maps by rule: any character whose NFKC form or UTS #39 confusable skeleton is a fence character, or whose Unicode name makes it a bare angle (a generated table, tools/fence_tables.rs from scripts/gen_angle_lookalikes.py: PRECEDES, the angle-bracket ornaments and presentation forms, arrowheads, ...), becomes ‹ › / or '. New deps: unicode-general-category (already in the graph), unicode-security. 2. fenced_server_message is the one place server text crosses into model-visible errors: a JSON-RPC error's message and data in tool_call_err; MCP Events' RpcError (from_json, from_service), fenced where it comes in with its code and structured data kept for the decisions made on them; rmcp's refresh errors carrying the authorization server's own text. 3. Pinned what #149 left untested: the insufficient-scope (and auth-required) challenge among a failed call's causes (describe_cause), the legacy server/discover rejection's body, the direct-HTTP stream's `$`-stripping (an e2e forge, over HTTP and stdio), and the dropped-id sequence seeded from the host's secret. 4. params_members no longer copies a server's own `$`-scalars into the host's genuine stand-in. Tests, each failing with its fix reverted (12 mutations): no_lookalike_or_invisible_spelling_ survives_the_fence (the auditor's full hostile set, plus private-use and unassigned), a_json_rpc_errors_message_and_data_are_fenced_in_the_tool_error, a_challenge_header_is_fenced_among_a_failed_calls_causes, a_servers_rpc_error_message_is_fenced_where_it_comes_in, a_refresh_failure_fences_the_authorization_servers_text, a_legacy_discover_rejections_body_is_fenced, a_servers_host_keys_forge_nothing_over_{http,stdio}, a_dropped_message_id_is_not_a_guessable_sequence, a_servers_dollar_keys_are_not_copied_into_the_stand_in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
jaredLunde
added a commit
that referenced
this pull request
Oct 7, 2026
…#157) * agent: the fence is a rule, and the one place server text reaches the model Re-audit of #149: 1. fenced_server_message drops by rule, not list: every character in General_Category Cc (tab, newline, CR become spaces), Cf, Co, Cn, Zl, Zp or Cs, the TAG block (ASCII smuggling), U+180E and the variation selectors. It maps by rule: any character whose NFKC form or UTS #39 confusable skeleton is a fence character, or whose Unicode name makes it a bare angle (a generated table, tools/fence_tables.rs from scripts/gen_angle_lookalikes.py: PRECEDES, the angle-bracket ornaments and presentation forms, arrowheads, ...), becomes ‹ › / or '. New deps: unicode-general-category (already in the graph), unicode-security. 2. fenced_server_message is the one place server text crosses into model-visible errors: a JSON-RPC error's message and data in tool_call_err; MCP Events' RpcError (from_json, from_service), fenced where it comes in with its code and structured data kept for the decisions made on them; rmcp's refresh errors carrying the authorization server's own text. 3. Pinned what #149 left untested: the insufficient-scope (and auth-required) challenge among a failed call's causes (describe_cause), the legacy server/discover rejection's body, the direct-HTTP stream's `$`-stripping (an e2e forge, over HTTP and stdio), and the dropped-id sequence seeded from the host's secret. 4. params_members no longer copies a server's own `$`-scalars into the host's genuine stand-in. Tests, each failing with its fix reverted (12 mutations): no_lookalike_or_invisible_spelling_ survives_the_fence (the auditor's full hostile set, plus private-use and unassigned), a_json_rpc_errors_message_and_data_are_fenced_in_the_tool_error, a_challenge_header_is_fenced_among_a_failed_calls_causes, a_servers_rpc_error_message_is_fenced_where_it_comes_in, a_refresh_failure_fences_the_authorization_servers_text, a_legacy_discover_rejections_body_is_fenced, a_servers_host_keys_forge_nothing_over_{http,stdio}, a_dropped_message_id_is_not_a_guessable_sequence, a_servers_dollar_keys_are_not_copied_into_the_stand_in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk * agent: the fence never folds a letter to `/` UTS #39 maps Katakana ノ and Coptic Ⳇ to `/`, so a Japanese server message read "/ート". Letters are no longer folded to `/`; letters shaped like `<`, `>` or `"` (Canadian syllabics ᐸ/ᐳ) still are, and without a `<` the fence cannot be closed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the four low findings from #146's audit. Each fix has a test that fails without it. I checked this by reverting each fix in place and running its test; the item 1, 2 and 4 fixes were each reverted in two ways, and all seven reverts fail.
…) and control characters become spaces. It is fenced the way event payloads are,<mcp_server_message untrusted>…</mcp_server_message>.<,>and quotes are replaced so the message can't close its fence or a challenge parameter's quoted-string.mcp_unauthorized::a_servers_401_message_is_cut_short_and_fenced_as_untrusteduses a 10 KB message that tries to close its own fence and start a new line. Unit:mcp_wire::a_server_message_is_cut_short_and_cannot_close_its_fence.WWW-Authenticatechallenge and a JSON-RPC body lost the challenge: it becameUnexpectedServerResponse.AuthRequired, carrying the challenge for rmcp's auth client andauth_challenge. The server's reason is added as RFC 6750'serror_description. A failed tool call now prints its error's causes, so the challenge and the reason reach the model instead of a bare "Auth required". The post-refreshagent mcp-loginerror keeps the challenge too.mcp_unauthorized::a_401_with_a_challenge_and_a_json_rpc_body_keeps_both. Unit:mcp_wire::a_401_with_a_challenge_and_a_reason_keeps_the_challenge.possibly_not_an_eventto the client, and in the model's notice. On a stdio connection with no push stream open, it is logged as dropped instead of reported.mcp_message_capnow assert the label on both transports. Unit tests cover the ambiguity mark and the rendered label.mcp_message_cap::one_over_cap_message_reaching_two_subscriptions_is_one_notice_to_the_modelexpects two gap frames and one model notice. Unit:an_unrouted_stand_in_reaches_every_stream_once_with_one_id.What item 3's tests cover. Before this change, a stdio connection with no push streams already reported no gap: the broadcast reached nobody. So the "log instead" half has no failing-before test; the router unit test exercises that path. The behaviour that changes is the label, and that is tested.
Fixture controls added to the OAuth fixture:
challenge_with_json_bodysends the challenge together with the JSON-RPC body;reject_messagesets a customerror.message.Audit fixes (at 99919fd)
As above, each fix's test fails with that fix reverted in place.
tool_call_err's cause walk printed rmcp'sAuthRequiredError/InsufficientScopeErrorDisplay, which contains the rawWWW-Authenticateheader, unfenced and unbounded. So did the post-refreshagent mcp-loginerror.fenced_server_message. A grep found two more server bodies that reached model-visible text: a non-JSON-RPC success's body, and a legacyserver/discoverrejection's body. Both are fenced too.mcp_unauthorized::a_hostile_oversized_challenge_header_is_cut_short_and_fenceduses a 6 KBrealmcontaining fence-closing text. The bare-challenge login test now expects the header fenced. The non-JSON-RPC body test now expects the body fenced.fenced_server_messagecould be fooled by lookalikes and invisible characters.<,>,/and"are replaced. Bidi embeddings, overrides and isolates, zero-width and joiner characters, and the BOM are dropped.a_server_message_cannot_close_its_fence_with_lookalikes_or_hide_with_invisibles$oversized/$dropped_id/$ambiguouskeys were trusted.mcp_stdio::host_params). The exception is a notification carrying the per-process secret that the host stamps into its own stand-ins ($host). The dropped-id sequence now starts from that secret.a_servers_host_reserved_keys_are_stripped_at_ingressChecks
mcp_events_nested::a_nested_elicitation_during_an_events_poll_reaches_the_owning_session, passed 3/3 when rerun alone; it is a timing-sensitive test from agent: MCP Events follow-ups — panic exit, delivery by tag, permanent refusals, runtime restore #136 and failed under full-suite load. This includes everytests/mcp_*.rssuite,serve_reaper,serve_http,serve_drain,run_signal_handling,run_cli_flags,serve_harness_deadlines, and theagentandagent-corelib tests.cargo clippywith-D warningsis clean, both for the agent with code-mode and for the whole workspace.cargo fmt --checkanddprint checkare clean.check.pypasses 12/12 over HTTP and 12/12 over stdio.🤖 Generated with Claude Code
https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk