fix(http): stop exposing active MCP session IDs on /health - #35
Closed
andesyteoss wants to merge 1 commit into
Closed
fix(http): stop exposing active MCP session IDs on /health#35andesyteoss wants to merge 1 commit into
andesyteoss wants to merge 1 commit into
Conversation
The /health endpoint returned an array of every active MCP session UUID via 'sessionIds: Array.from(transports.keys())'. Because the HTTP server binds on 0.0.0.0 by default and treats the 'mcp-session-id' header as the sole authentication token for reusing a transport on POST /mcp, any unauthenticated network caller could fetch /health, harvest live session UUIDs, and hijack another user's MCP transport - including wallet signing and transfer tools. Drop 'sessionIds' from the response and keep only the non-sensitive liveness fields (status, server, activeSessions count).
Author
|
Closing this as inactive — no maintainer response after 14 days. The security finding and fix remain valid. If this is still relevant, I'm happy to reopen, rebase, or re-submit against a different branch. Just drop a comment. |
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.
Summary
The
/healthendpoint insrc/server/http-server.tsreturns the full list of active MCP session IDs to any unauthenticated caller. Because those same session IDs are the sole bearer credential used to reuse an existing MCP transport onPOST /mcp,GET /mcp, andDELETE /mcp, disclosing them is equivalent to disclosing a session token — a network peer that can reach the health endpoint can hijack any live MCP session and invoke every registered tool (including wallet/transfer tools) as that session.src/server/http-server.ts(health handler around L154–L162 pre-fix)HOST=0.0.0.0andPORT=3001by default, so on any host without an external firewall the endpoint is reachable from the local network.Data flow
POST /mcpwith aninitializerequest. The server generates a UUID session ID and stores the transport intransports: Map<string, StreamableHTTPServerTransport>(L18, L75).POST /mcp,GET /mcp, andDELETE /mcprequests look up the transport purely by themcp-session-idheader (L61–L66, L117–L122, L138–L143). There is no other authentication.GET /healthreturnedsessionIds: Array.from(transports.keys())— the exact strings needed to satisfy step 2.Session IDs are unguessable UUIDs, so
/healthis the differentiator: without this leak the transport lookup is effectively bearer-token protected; with it, any network-adjacent process can enumerate live sessions.Fix
Drop the
sessionIdsfield from the/healthresponse.activeSessions: transports.sizeis retained because a count is useful for liveness/monitoring and does not disclose credentials.res.status(200).json({ status: "ok", server: server ? "initialized" : "initializing", - activeSessions: transports.size, - sessionIds: Array.from(transports.keys()) + activeSessions: transports.size });Minimal, one-file change; no behavioural impact on
/mcphandlers or on monitoring that readsstatus/activeSessions.Proof of concept
With the server running on its defaults (
bun start:http→http://0.0.0.0:3001):Post-fix, step 2 no longer yields the session UUID, so step 3 cannot succeed without out-of-band knowledge of the token.
Testing
bun run buildsucceeds; only the health handler is touched.GET /healthbefore and after the change — response shape matches the diff above.GET /mcp,POST /mcp,DELETE /mcphandlers are untouched; existing session lookup (transports.has(sessionId)/transports.get(sessionId)) continues to work for legitimate clients that already hold their own session ID.grep -rn "transports.keys\|Array.from(transports" srcconfirms no other endpoint leaks the same data.Adversarial review
Before submitting we tried to disprove this:
/mcphandlers treat a validmcp-session-idas sufficient to bind to an existing transport (L61–L66). There is no additional auth, origin check, or per-session secret. The UUID is the credential./healthis normally firewalled." The default bind is0.0.0.0, no auth middleware is registered, and the README instructs users to runbun start:httpdirectly. Any co-tenant or LAN peer reaches it. Even on a public deployment behind a reverse proxy,/healthis commonly whitelisted for uptime checks — that same path then leaks credentials./mcprequests without one are rejected. The/healthleak is precisely what turns network reach into session hijack.Fix is scoped to the one line that made the disclosure possible; monitoring consumers that only need liveness continue to work.