Route SSE proxy responses to originating session

[rdimitrov]

Summary

The legacy HTTP+SSE proxy for stdio backends (pkg/transport/proxy/httpsse) fronts one shared backend process for every connected session. Backend responses arrive with no session attribution, and the proxy delivered each one to every live SSE stream. In an authenticated multi-user deployment, any user with their own valid session received every other user's tool results, and because independent MCP clients reuse ordinary request-id sequences, a victim client could accept another session's response as the answer to its own pending call. Fixes GHSA-wm2j-ch74-276r.

• Each client call's wire id is rewritten to <session_id>|n:<int64> or <session_id>|s:<string> before it reaches the backend, using the same session_id the ownership middleware just validated. The backend echoes it, and the proxy routes the response to exactly that session with the client's original id and id type restored. The id is read exactly from the raw body, because jsonrpc2.DecodeMessage parses numbers through float64 and rounds integers above 2^53; fractional ids are rejected with 400.
• Routing is stateless: the destination is derived from the echoed id, so there is no pending-request table and nothing to evict on response, disconnect, or timeout. A response whose id the proxy did not mint, or whose session has disconnected, is dropped and never queued or broadcast.
• A client's notifications/cancelled names its target request in params.requestId; that value is rewritten the same way so the backend can match it. Without this, client cancellation would silently stop working once ids are rewritten.
• A JSON-RPC response sent by a client is refused with 400. The proxy answers or rejects every server-initiated request itself before any client sees it, so a client response could only be an attempt to answer backend work belonging to another session.
• Server-to-client traffic follows the streamable proxy's existing policy (dispatcher.go): */list_changed notifications are broadcast to every live session, rebuilt from the method with no params since the spec allows _meta there; notifications/message, notifications/progress, notifications/resources/updated and unknown notifications are dropped because the shared backend cannot attribute them to a session; a backend ping is answered by the proxy with an empty result; other server-initiated requests such as sampling/createMessage are answered with JSON-RPC -32601 written back to the backend. The first drop or rejection per method is logged at Warn so operators can see the fail-closed behavior; later ones at Debug.
• The queue that replayed messages received while no client was connected to the next client to connect is removed. It was the second path by which one session's response could reach another, and a list_changed handed to a client that has not yet initialized is of no use to it. The streamable proxy has no replay buffer either. The ssecommon.PendingSSEMessage type it used is left untouched; removing it is a separate change.
• docs/arch/03-transport-architecture.md documents the legacy SSE routing table alongside the streamable one.

Type of change

• Bug fix

Test plan

• Unit tests (task test)
• E2E tests: the proxy stdio + --proxy-mode sse scenario in test/e2e/proxy_stdio_test.go run against a real time-server container, proving a real client's initialize with id: -1 round-trips through the rewrite and restore.
• Linting (task lint-fix) — cannot run locally on this machine (golangci-lint cannot read Go 1.27 export data, pre-existing and unrelated); go vet and gofmt are clean, relying on CI for lint.
• New regression tests in routing_test.go: two independently authenticated principals, victim's response reaches only the victim's stream; colliding id: 1 from both sessions stays isolated when the backend answers out of order; a client whose own string id is spelled like the victim's route gets it back as its own id and the victim sees nothing; a response arriving after its session disconnected reaches nobody; nothing received while no client is connected is replayed to a later client; notifications/cancelled carries the routed id, including above 2^53; integer ids above 2^53 round-trip exactly and fractional ids are refused; client responses are refused; list_changed broadcasts carry no params; a backend ping is answered; every row of the dispatch table. Non-delivery is proven by sentinel ordering on the FIFO per-session stream, not by waiting.
• Security-advisor and code-reviewer subagent reviews; findings addressed in this PR.

Does this introduce a user-facing change?

Yes, for deployments using the legacy SSE proxy mode with a stdio backend:

• Responses now reach only the session that issued the request. Clients that relied on seeing another session's responses were relying on the vulnerability.
• notifications/progress, notifications/message and notifications/resources/updated are no longer delivered on the SSE proxy. Porting the streamable proxy's progress-token rewriting and subscription tracking is a follow-up.
• Server-initiated requests (sampling/createMessage, elicitation/create) receive a -32601 error from the proxy instead of reaching a client, matching the streamable proxy. This applies even with a single connected client, because a request emitted on behalf of a disconnected session would otherwise land on whoever remains connected. A backend ping is the exception and is answered by the proxy.
• A client that posts a JSON-RPC response, or a call with a fractional numeric id, now receives 400 instead of 202.

Streamable HTTP proxy mode, the default for stdio backends, is unaffected.

Implementation plan

Approved implementation plan

Mechanism. Rewrite each outgoing call's wire id to a string encoding both the originating session and the original id with its type (<session_id>|n:<int64> / <session_id>|s:<string>). On a backend response, parse it back, restore the original id, deliver to exactly that session. This is the streamable proxy's compositeKey(sessID, idKey) shape without its two maps: streamable needs a per-request waiter channel, whereas the SSE destination is the session stream already indexed by session id, so the route is fully derivable from the echoed id. Collisions are solved by construction. The session id embedded is the same session_id query value the ownership middleware validated, read once in the handler.

Dispatch table. Response with a live routed session: that session only. Response otherwise: dropped. */list_changed: every live session. notifications/message, notifications/progress, notifications/resources/updated, anything else: dropped (SECURE-DROP, as streamable). Server-initiated request: -32601 back to the backend. Two deliberate fail-closed behavior changes: progress stops arriving on SSE until token rewriting is ported; server-initiated requests are rejected even with one client, because a departed session's still-running tool could otherwise route its sampling request to the next sole session.

Tests first. Two-principal HTTP-level regression test modeled on the ownership test, failing on the old code; colliding-id case; table-driven dispatch cases; adapt the existing tests that encoded the broadcast.

Review adjustments. notifications/cancelled rewrites params.requestId the same way (code review). Warn once per method on drops and rejections (both reviews). Replay queue removed rather than bounded (simplification pass).

Special notes for reviewers

• The design mirrors compositeKey(sessID, idKey) in the streamable proxy but without its waiters/idRestore maps, because the SSE destination (the session's stream) is already indexed by session id in liveSSESessions. The advisory's recommended remediation describes the same shape; it was used only as a cross-check after deriving the fix from the streamable precedent.
• Porting progress-token routing and subscription tracking from the streamable proxy to the SSE proxy is follow-up work with no tracking issue yet. The streamable proxy shares two of the gaps the review found here, verbatim list_changed broadcast and -32601 for backend ping; fixing those there is also a follow-up.
• The health-check pinger's ping responses were previously broadcast to every client as unsolicited responses with unknown ids; they now drop at Debug.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 88.57143% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.30%. Comparing base (1fa5e36) to head (e3861ca).

Files with missing lines	Patch %	Lines
pkg/transport/proxy/httpsse/routing.go	91.89%	9 Missing ⚠️
pkg/transport/proxy/httpsse/http_proxy.go	78.57%	6 Missing ⚠️
pkg/transport/stdio.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #6702    +/-   ##
========================================
  Coverage   79.30%   79.30%            
========================================
  Files         799      800     +1     
  Lines       80826    80936   +110     
========================================
+ Hits        64097    64185    +88     
- Misses      16724    16746    +22     
  Partials        5        5            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ChrisJBurns]

Multi-Agent Consensus Review

Agents consulted: ToolHive code reviewer, security advisor, MCP protocol expert, Codex cross-review

Consensus Summary

#	Finding	Consensus	Severity	Action
1	Client responses can forge replies to shared-backend requests	9/10	HIGH	Fix
2	List-change broadcasts preserve arbitrary cross-session metadata	10/10	HIGH	Fix
3	HTTP decoding corrupts valid numeric request IDs before routing	9/10	MEDIUM	Fix
4	Backend ping requests are rejected as unsupported	8/10	MEDIUM	Fix
5	Removing exported pending-message symbols breaks downstream builds	7/10	MEDIUM	Discuss

Overall

This is a security-focused routing refactor with a sound core: ownership-validated session IDs are embedded in backend request IDs, response IDs are restored, stale replay buffering is removed, and deterministic cross-principal tests cover collisions and disconnects. The stateless routing approach is substantially safer than the previous broadcast behavior.

Two remaining paths should be fixed before merge. Client-originated JSON-RPC responses still enter the shared backend even though no legitimate server request reaches a client, allowing a session to race replies to another session's backend work. Separately, whitelisted list-change notifications are broadcast verbatim even though MCP permits arbitrary _meta, retaining a cross-session disclosure channel. The numeric-ID and ping findings are protocol-correctness issues; the exported-type removal needs an explicit compatibility decision.

CI is green at the reviewed commit, including unit, race, lint, proxy E2E, MCP conformance, and CodeQL checks.

Documentation

If list-change notifications are sanitized or rejected when parameterized, update docs/arch/03-transport-architecture.md so the routing table states the exact safe payload shape. If the exported pending-message API remains removed, document the compatibility decision or deprecation path.

---

Generated with Codex using the pr-review workflow