Serve MCP 2026-07-28 stateless requests through the transparent proxy#5884
Conversation
The streamable proxy rendered ClassifyRevision errors as HTTP 400 JSON-RPC error bodies via a local helper. The transparent proxy will need the same rendering (issue #5831), so lift it into pkg/mcp as a single source of truth: - WriteClassificationError writes to an http.ResponseWriter (streamable). - ClassificationErrorResponse builds an *http.Response for use from tracingTransport.RoundTrip, where no ResponseWriter is available. Both share one body-computation helper, mirroring the session.NotFoundBody/WriteNotFound/NotFoundResponse precedent. Wire output is byte-identical; the streamable local copy is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The transparent proxy only rejected GET/HEAD/DELETE with 405 in --stateless mode. A Modern (2026-07-28, stateless) MCP request has no protocol use for those methods either, regardless of proxy mode, so the gate should reject them whenever the request is tagged Modern. Replace statelessMethodGate with a p.methodGate method that 405s GET/HEAD/DELETE when p.stateless OR the MCP-Protocol-Version header is exactly the Modern version, and wire it unconditionally. The gate stays outermost (pre-auth) and header-only -- it never reads the body, so it does not call ClassifyRevision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The transparent proxy forwarded every POST unclassified, so a malformed 2026-07-28 (Modern, stateless) request reached the backend instead of being rejected at the edge, and no revision signal was available for the health readiness fix. Parse the JSON-RPC body once (parseRPCRequest, replacing detectInitialize) to recover method, params, and id. Only a single request (non-empty method and a valid non-null id) is classified via mcp.ClassifyRevision; batches, notifications, and responses forward as Legacy so a stray Modern header can never trigger a spurious rejection. A malformed Modern request returns a 400 JSON-RPC error (echoing the id) before the backend is contacted. A well-formed request falls through unchanged, so the session-ID guard still keys on Mcp-Session-Id presence, not on the client-forgeable revision. The computed revision is consumed by the readiness fix in a following change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The transparent proxy's serverInitialized latch — which gates the background monitorHealth loop — only flipped on Legacy signals (a response Mcp-Session-Id or an initialize request). A pure-Modern (2026-07-28, stateless) backend has neither, so its health monitor never began probing. Flip the latch on the first successful (HTTP 200) request classified Modern, mirroring Legacy's existing any-200 flip. This gates only background monitoring, not the /health response, and starts monitoring on a proven-live backend, avoiding startup flapping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The session-ID guard, backend-SID rewrite, re-init recovery, and DELETE cleanup deliberately key on Mcp-Session-Id presence, not on the client-forgeable classified revision. Add RoundTrip-level regression tests proving a well-formed Modern _meta signal does not let a request bypass any of them: an unknown session still gets session-not-found, a known session still gets the backend-SID rewrite and 404/dial re-init recovery, and a DELETE carrying body _meta (no header) still performs session-keyed cleanup. The unknown-session case was mutation-verified (re-keying the guard on revision makes it fail). Also fix a pre-existing data race in method_gate_test.go, where parallel subtests shared a single gotBody/inner handler; move them into each subtest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address external review: move the exported WriteClassificationError and ClassificationErrorResponse above the private helper per go-style file organization, and document that the revision guard regression suite covers single-request Modern-signal forgery only (batch initialize guard exemption is pre-existing Legacy behavior, tracked in #5883). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JAORMX
left a comment
There was a problem hiding this comment.
LGTM. ✅ A three-lens panel (security · MCP-conformance · Go/reuse) verified this against the real code; it's carefully scoped and the security-critical invariant holds.
Security — anti-spoofing invariant holds (the crux). All four session guards stay keyed on Mcp-Session-Id presence, never on the client-forgeable classified revision: unknown-session (:614), backend-SID rewrite (:634), DELETE cleanup (:696), re-init recovery (:711). I confirmed revision is referenced in exactly one behavioral spot — the monotonic readiness latch (:759) — plus a debug log and the 405 message; none of the guards touch it. A forged MCP-Protocol-Version: 2026-07-28 or reserved _meta therefore can't bypass anything, and initialize is hard-pinned to Legacy in ClassifyRevision. The revision_guard_regression_test.go suite is genuine mutation-testing (each test first asserts the body classifies Modern-nil, then drives real RoundTrip and asserts the Legacy machinery still fires).
MCP conformance. Method gate is spec-correct (GET/HEAD/DELETE→405 with Allow: POST, OPTIONS for modern-only servers; exact 2026-07-28 header keying; no-op for Legacy so the SSE GET stream is preserved). Single-request classification is spec-safe — a conformant Modern client only sends a single request with a valid id (no batching, no client→server notifications/responses over Streamable HTTP), so forwarding those shapes as Legacy can never misclassify. Body restore + json.RawMessage id echo preserve large-int precision; every failure path returns HTTP 400 (exceeds the intermediary minimum).
Go/reuse. The centralized WriteClassificationError/ClassificationErrorResponse in pkg/mcp is exactly the dedup our #5839 review asked for — the streamable proxy's local writer is fully removed, both proxies route through the shared helper (which mirrors session.NotFoundBody/WriteNotFound/NotFoundResponse field-for-field and references the CodeInvalidParams const). Body parse/restore correct; shared helper unit-tested for byte-identical output across both seams.
Real checks (lint, unit tests) are green.
Minor / non-blocking:
MissingModernMetadataErrormaps to-32602rather than the draft's-32020for a header-signalled-but-body-missing case — but that's #5834's classifier (with an existing TODO), and the proxy returns 400 either way, so it stays intermediary-compliant. Track with the deferred POST-side header-validation work.- Classification fires for streamable-HTTP JSON POSTs on non-
/mcppaths too — only ever rejects the client's own malformed Modern request (no leak); confirm that broadening is intended. - The marshal-failure fallback hardcodes
-32602(matches thesessionprecedent — a raw string can't interpolate the const; a "keep in sync" comment would suffice). - Pre-existing batch-guard gap #5883 confirmed not worsened here (batches set
singleRequest=false, never classified).
Nicely done — clean sibling to #5839 and it closes the dedup loop.
🤖 AI-assisted panel review via Claude Code (security · MCP-spec · Go/reuse). Line numbers/verification against c30fd35.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5884 +/- ##
==========================================
+ Coverage 71.35% 71.44% +0.09%
==========================================
Files 693 694 +1
Lines 70630 70664 +34
==========================================
+ Hits 50395 50485 +90
+ Misses 16600 16530 -70
- Partials 3635 3649 +14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add a focused test that drives classificationErrorBody's defensive marshal-failure branch with an unmarshalable request id, bringing the helper to full coverage and verifying the fallback returns a valid JSON-RPC error body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JAORMX
left a comment
There was a problem hiding this comment.
Still LGTM on e9ec367 — the only change since my approval is a test-only addition (+15 lines in classification_response_test.go) covering the marshal-failure fallback branch, addressing the earlier coverage NIT. Production code is unchanged from the reviewed c30fd35 (which passed all 46 checks), and the security invariant / dedup / conformance findings all still stand. Nice to see the fallback now exercised. Re-approving.
🤖 AI-assisted panel review via Claude Code. Test-only delta since c30fd35.
Summary
The transparent proxy (
pkg/transport/proxy/transparent) reverse-proxies to MCPbackends, but every request path assumed the Legacy (≤2025-11-25),
initialize-handshake, session-based model. A pure-Modern (2026-07-28,stateless) backend was therefore mishandled:
serverInitializednever flipped,so the background
monitorHealthloop never began active failure-monitoringfor it; GET/DELETE were only rejected in
--statelessmode; and malformedModern requests were forwarded to the backend instead of being rejected at the
edge. This mirrors the classification the sibling streamable proxy already does.
What changed:
pkg/mcp(
WriteClassificationErrorfor thehttp.ResponseWriterseam,ClassificationErrorResponsefor theRoundTripperseam), replacing thestreamable proxy's local copy — one source of truth for the 400 JSON-RPC body.
is
--statelessor the request carriesMCP-Protocol-Version: 2026-07-28.Header-only and deliberately pre-auth (a method rejection needs no identity
and reads no body).
RoundTrip: parse the body once(
parseRPCRequest); only a single JSON-RPC request (non-empty method + validid) is classified. A malformed Modern request returns a 400 (echoing the id)
before the backend is contacted. Batches, notifications, and responses forward
as Legacy, so a stray Modern header can never trigger a spurious rejection.
serverInitializedon thefirst successful (HTTP 200) Modern request, so the background health monitor
engages (parity with Legacy's any-200 flip).
Security invariant: the session-ID guard, backend-SID rewrite, and re-init/
DELETE recovery remain keyed on
Mcp-Session-Idpresence, never on theclient-forgeable classified revision. Regression tests prove a forged Modern
_metacannot bypass any of them (the unknown-session case wasmutation-verified).
Closes #5831
Type of change
Test plan
task test) — full-racesuite greentask test-e2e)task lint-fix) — 0 issuesEnd-to-end against a live 2026-07-28 MCP server was not run and is left as
manual verification: point a transparent proxy at a backend that never receives
initializeand always sends Modern_meta+ theMCP-Protocol-Versionheader; confirm requests succeed, background health-monitoring engages after the
first Modern 200, GET/DELETE return 405 without
--stateless, and a malformedModern request gets a 400 instead of being forwarded.
API Compatibility
v1beta1API. (No operator/CRD surface touched.)Changes
pkg/mcp/classification_response.goWriteClassificationError,ClassificationErrorResponse).pkg/transport/proxy/streamable/streamable_proxy.gowriteClassificationError.pkg/transport/proxy/transparent/transparent_proxy.goparseRPCRequest+ POST classification inRoundTrip; Modern-200 readiness flip.pkg/mcp/classification_response_test.go,pkg/transport/proxy/transparent/{revision_classification,revision_guard_regression,method_gate}_test.goDoes this introduce a user-facing change?
Yes. The transparent proxy now serves MCP 2026-07-28 (stateless) backends: it
rejects GET/HEAD/DELETE for Modern requests, rejects malformed Modern requests
with a JSON-RPC 400 at the edge, and reports health readiness for pure-Modern
backends.
Implementation plan
Approved implementation plan (MoE-reviewed)
Implemented in 5 commits, each independently reviewed by two domain experts
(worker: go-expert-developer; reviewers selected by change area):
pkg/mcp) — lift thestreamable proxy's local
writeClassificationErrorinto a shared helper withtwo seams (
http.ResponseWriterand*http.Response), mirroring thesession.NotFoundBody/WriteNotFound/NotFoundResponseprecedent.statelessMethodGatewith amethodGatemethod; 405 GET/HEAD/DELETE whenp.stateless || header == MCPVersionModern; wired unconditionally, outermost (pre-auth), header-only.RoundTrip—parseRPCRequestparsesthe body once; classify only a single request (method + valid id); 400 a
malformed Modern request before the backend; batches/notifications/responses
forward as Legacy. Session guards left keyed on
Mcp-Session-Idpresence.serverInitializedon the first Modern HTTP 200 (parity with Legacy's any-200 flip); gates only
the background monitor, not the
/healthresponse._metadoes not bypass the unknown-session guard, backend-SID rewrite, re-init
recovery, or DELETE cleanup.
Deliberately out of scope (follow-ups): streamable proxy's own GET/DELETE
revision-awareness; POST-side unsupported
MCP-Protocol-Versionheadervalidation (tracked by an existing TODO in
pkg/mcp/revision.go).Special notes for reviewers
now rebased onto
mainafter that merged — the diff is only thetransparent-proxy changes.
before auth. This is intentional and leak-free — the decision is a pure
function of the request method +
MCP-Protocol-Versionheader + the static--statelessflag, reads no body, and reveals nothing auth-gated.fires on any 200 (including a JSON-RPC error inside a 200 body), exactly like
Legacy's existing flip. It gates only when background monitoring starts, not
what
/healthreturns, so it cannot make/healthlie.DEBUG log in
RoundTripfires for every request; theRoundTripclassificationblock could later be extracted to shrink the existing
gocyclosuppression.initializeskips the unknown-session guard (sawInitializeis set for anybatch member named
initialize). This is Legacy behavior predating this PR(Wire session-aware backend routing in proxy transports (RC-12) #4318 / Route MCP sessions to the originating backend pod using httptrace #4673, with a deliberate test), unrelated to the Modern/stateless
work here, and filed as Transparent proxy: batch containing initialize skips the unknown-session guard #5883 for a scope decision. The revision-guard
regression suite documents that batches are out of its (Modern-spoofing)
scope.
🤖 Generated with Claude Code