Skip to content

agent: MCP OAuth hardening — any 401 by status, RFC 6749 refusals definitive, pinned reload rule - #140

Merged
jaredLunde merged 1 commit into
mainfrom
jared/mcp-oauth-hardening
Oct 7, 2026
Merged

jaredLunde merged 1 commit into
mainfrom
jared/mcp-oauth-hardening

Conversation

@jaredLunde

Copy link
Copy Markdown
Contributor

Fixes the three low items from #138's verification audit. Each has a test that fails without its fix. I checked that by turning each fix off in turn.

# Gap Fix Proving test Fix turned off →
1 A 401 whose body is an application/json JSON-RPC error came back from rmcp's client as a normal error response, so is_unauthorized never saw it and the transport path didn't refresh. The events wire did. For a server with a login (HttpClient { oauth: true }, set in connect_http), every POST is answered by mcp_wire::HttpClient::post_bounded. It turns any 401 into AuthRequired from the status, before the body is read. post_bounded also picks up the rest of rmcp's behavior for general requests: an empty or non-JSON-RPC success for a notification is Accepted, and a 4xx to server/discover gets the legacy-handshake answer. JSON bodies and SSE events are capped at the host's per-message cap (64 MiB, the same as stdio). mcp_oauth::tests::a_401_whose_body_is_a_json_rpc_error_still_refreshes_and_retries FAILED
2 Only invalid_grant counted as definitive. Other permanent OAuth errors backed off forever and never showed the mcp-login hint. Refresh failures are now classified from the token endpoint's own answer. OAuth HTTP goes through a new RecordingOAuthHttp (an rmcp OAuthHttpClient), because rmcp's refresh error keeps neither the status nor the code. Every RFC 6749 §5.2 code is definitive: invalid_grant, invalid_client, unauthorized_client, unsupported_grant_type, invalid_scope, invalid_request. So is a token endpoint that isn't there (404/405/410). 5xx and 429 stay transient and back off. mcp_oauth_refresh::every_rfc_6749_refusal_is_definitive_and_names_mcp_login and …a_token_endpoint_that_is_not_there_is_definitive_and_names_mcp_login (both end to end through rmcp's real refresh against the fixture); mcp_oauth::tests::the_token_endpoints_answer_classifies_a_refresh_failure both e2e tests FAILED
3 No test pinned that a reload clears a remembered definitive failure only when the stored token actually changed (mutant T11 survived). test only mcp_oauth::tests::a_reload_clears_a_remembered_definitive_failure_only_when_the_stored_token_changed FAILED (T11 killed)

The OAuth fixture gains a refresh_reply control, which answers refresh_token grants with any status and body.

The ARCHITECTURE.md "MCP Server OAuth" mid-session refresh paragraph and the module doc are updated.

Local: all mcp_* suites (incl. mcp_oauth, mcp_oauth_refresh) + lib units + serve_service_mcp: 1773 passed. Also clean: clippy --workspace --all-targets -D warnings, cargo fmt --check, dprint.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

… definitive, pinned reload rule

- A 401 whose body is a JSON-RPC error was read by rmcp's client as an
  ordinary error response, so the transport path never refreshed (the events
  path did). For a server with a login, every POST is now answered by
  mcp_wire::HttpClient::post_bounded, which turns any 401 into AuthRequired
  from the status before the body is read; post_bounded gains rmcp's
  remaining semantics for general requests (an empty or non-JSON-RPC success
  for a notification is accepted; a 4xx to server/discover is the legacy
  handshake's cue).
- Refresh failures are classified from the token endpoint's own answer: OAuth
  HTTP goes through RecordingOAuthHttp (rmcp's refresh error keeps neither
  status nor code). Every RFC 6749 §5.2 code (invalid_grant, invalid_client,
  unauthorized_client, unsupported_grant_type, invalid_scope,
  invalid_request) and a missing token endpoint (404/405/410) are definitive
  and name `agent mcp-login`; 5xx/429 stay transient (backoff).
- A test pins that a reload clears a remembered definitive failure only when
  the stored token changed.

Each fix has a test that fails without it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
@jaredLunde
jaredLunde merged commit 082a625 into main Oct 7, 2026
21 checks passed
@jaredLunde
jaredLunde deleted the jared/mcp-oauth-hardening branch October 7, 2026 05:33
jaredLunde added a commit that referenced this pull request Oct 7, 2026
…h flag, pin the 401 divergence

#140 routed an OAuth server's POSTs through post_bounded (HttpClient { oauth }) so a 401 stays
visible; this branch already routes every server's POSTs there, so the flag selected nothing: it is
removed, with OAUTH_MAX_MESSAGE_BYTES (the one per-message cap, mcp_stdio::max_message_bytes,
applies). The rmcp-parity test now pins the one deliberate divergence: a 401 with no challenge and
a JSON-RPC error body is AuthRequired here, an error response from rmcp's own client.

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
…h flag, pin the 401 divergence

#140 routed an OAuth server's POSTs through post_bounded (HttpClient { oauth }) so a 401 stays
visible; this branch already routes every server's POSTs there, so the flag selected nothing: it is
removed, with OAUTH_MAX_MESSAGE_BYTES (the one per-message cap, mcp_stdio::max_message_bytes,
applies). The rmcp-parity test now pins the one deliberate divergence: a 401 with no challenge and
a JSON-RPC error body is AuthRequired here, an error response from rmcp's own client.

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
… refusals, runtime restore (#136)

* agent: MCP Events follow-ups — panic exit, delivery by tag, permanent refusals, runtime restore

- A panic out of `run`/`serve` no longer hangs in runtime drop on the parked stdin reader: `main`
  catches the unwind, abandons the runtime's threads, sweeps stdio servers and exits 101.
- Steered MCP Events batches are delivered by tag: `SteeringMessage::tag` comes back in
  `AgentEvent::Steered` (emitted after the checkpoint on both paths), and the batch is recorded
  as delivered right then — a later compaction can no longer make it look undelivered.
- Permanent refusals (event not offered, extension unsupported, invalid/denied, no mode) are
  classified, reported as `refused` (frame + `mcp_events_list.unestablished`), retried hourly,
  and no longer keep the events session alive.
- Runtime subscriptions are persisted and restored after a restart; a daemon boots the sessions
  that hold them, and their webhook tokens are held like configured ones.
- Tests for nested elicitation during `events/poll` and `events/stream` (stdio and daemon).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: MCP Events follow-ups — audit fixes (F1–F6), session panic containment, events-session restart

- Unknown/removed servers are permanent refusals; a restore refused for good forgets the runtime
  subscription; runtime subscriptions expire after their session's last client command
  (RUNTIME_TTL_MS, 7 d); boot restoration is bounded (MAX_RESTORED_SESSIONS, 32).
- Steered-batch delivery relies only on the Steered tag (the redundant received list is gone); the
  compaction test now asserts the durable done records and an empty pending queue.
- agent-core reports Steered before ending a terminating batch.
- Direct-HTTP events requests refuse server->client requests with an error instead of ignoring
  them (unary SSE and events/stream).
- -32012 is retried with fresh credentials on a short backoff, refused only after 5 in a row.
- The unused blob fixture tool is gone.
- A panicking daemon session is contained (error frame, session ends, daemon carries on); the MCP
  Events session is restarted after a panic, with backoff.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: one MCP message cap on every path — the events HTTP wire and every HttpClient POST

Extends #134's single per-message cap (mcp_stdio::max_message_bytes,
BEYOND_AI_AGENT_MCP_MAX_MESSAGE_BYTES) to the paths it did not cover: the MCP Events direct-HTTP
wire (unary JSON, unary SSE, events/stream — its own 4 MiB / 1 MiB caps are gone) and every
mcp_wire::HttpClient POST (not only skills/*; a request's over-cap answer becomes its JSON-RPC
error), plus the GET stream through rmcp's SSE-event limit. The skills-only 32 MiB constant is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: an over-cap MCP Events push event is skipped and reported, not reconnected into forever

The over-cap event's bounded head is read structurally (mcp_stdio::oversized_stand_in — the same
member walk as scan_head) for its routing, cursor and id; a small $oversized stand-in replaces it on
both transports (the stdio pump, and the events SSE reader, which skips the rest of the event
unread). The subscription keeps the cursor (else the next heartbeat's), records an 'oversized' gap
(frame + model notice) and carries on — no reconnect into the same giant event.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent tests: follow-up tests on #135's race-free daemon ports

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: MCP Events re-audit fixes (N1–N7) — runtime TTL from creation, refusal Accept, inline refusals, boot ranking, purge, restart, one POST path

- N1: creating a runtime subscription records client activity (set_runtime), and a state with
  runtime subscriptions but no record is no longer trusted forever: one made by a single command
  expires one TTL after creation. (a_runtime_subscription_made_by_one_command_expires_with_no_client)
- N2: a direct-HTTP server->client request's refusal POST carries the same Accept as every events
  POST; the fixture now 406s an answer without it, as the Python SDK would. (over_direct_http_*)
- N3: refusals on an events/stream are answered inline, one at a time — a flooding server cannot
  make the reader spawn without limit.
  (a_flood_of_server_requests_on_an_events_stream_is_answered_one_at_a_time)
- N4: the boot cap keeps the sessions a client used most recently, not the first alphabetically.
  (the_boot_cap_keeps_the_most_recently_used_sessions)
- N5: a forgotten runtime subscription (TTL expiry, permanent refusal on restore, server ended)
  is forgotten whole: its webhook token and secret leave the snapshot.
  (a_runtime_subscription_to_a_removed_server_is_forgotten_not_resurrected)
- N6: a panicked session holding runtime subscriptions is restarted with per-session backoff,
  like the events session. (a_session_with_runtime_subscriptions_is_restarted_after_a_panic)
- N7: ordinary_limit reads the request id directly instead of serializing the whole message;
  every streamable-HTTP POST goes through post_bounded (rmcp's post_message is unused) — pinned
  against rmcp's own client on request headers and response shapes
  (an_ordinary_request_is_sent_as_rmcps_own_client_would / ..._answered_as_...).
- ARCHITECTURE.md: the above, and the over-cap push-event skip from the previous commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: a refused server request is answered with the current OAuth token (after #138)

#138 made every direct events/* POST carry the server's current shared token and refresh-and-resend
once on a 401; the refusal POST for a server->client request still sent the headers built at dial,
so after a refresh it spent a 401 and the server kept waiting. It now goes through the same
Conn::send (Accept included; no Mcp-Method, since an answer has no method).
(a_refused_server_request_is_answered_with_the_current_token_and_refreshed_on_401)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: reconcile the one POST path with #140 — drop HttpClient's oauth flag, pin the 401 divergence

#140 routed an OAuth server's POSTs through post_bounded (HttpClient { oauth }) so a 401 stays
visible; this branch already routes every server's POSTs there, so the flag selected nothing: it is
removed, with OAUTH_MAX_MESSAGE_BYTES (the one per-message cap, mcp_stdio::max_message_bytes,
applies). The rmcp-parity test now pins the one deliberate divergence: a 401 with no challenge and
a JSON-RPC error body is AuthRequired here, an error response from rmcp's own client.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent tests: widen the steered-compaction test's window so full-suite load cannot close it

Its second event must be polled, coalesced and steered within one stalled turn; 2.5 s was missed
once under full-suite load on the host. 6 s per turn, and a 90 s bound on the run's response.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: reconcile the one POST path with #142 — every server, two pinned divergences from rmcp

#142's routing docs and tests described post_bounded as the OAuth servers' path, chosen by the
removed oauth flag; it answers every server's POSTs. ARCHITECTURE.md now names both deliberate
differences from rmcp's client (any 401 is AuthRequired; a non-JSON-RPC success answering a request
is an error), each with its pinning test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent tests: read the MCP Events suites' stdout through #141's guarded Frames

#141's ChildGuard takes the stdout pipe at spawn and its lint refuses any other route to it; the
follow-up tests built Frames from child.stdout.take(). They now pass the guard, like every suite.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant