feat: add notification isolation tests for GHSA-345p-7cg4-v4c7#137
Draft
feat: add notification isolation tests for GHSA-345p-7cg4-v4c7#137
Conversation
Add two new conformance test scenarios for server re-use notification cross-talk (Issue 2 from the cross-talk CVE report): - notification-isolation: deterministic 2-client test that verifies progress notifications from a slow tool don't leak to a second client that connects mid-handler - notification-isolation-fuzz: N=10 concurrent clients each calling test_tool_with_progress with unique progress tokens, verifying no cross-contamination across any pair of clients Also adds a vulnerable server example (no-notification-isolation.ts) that mirrors Cloudflare's createMcpHandler pattern: shared McpServer with per-request transports.
commit: |
- Each fuzz client now emits its own check so you can see per-client results (token, notification counts, pass/fail) - Client count defaults to 10, overridable via FUZZ_CLIENT_COUNT env var
…st server - parseSSEStreamFull now reads all events until stream closes (not just until first response), catching leaked notifications after the response - Both notification-isolation and notification-isolation-fuzz now fail when a client receives zero own notifications (indicates they were routed to another client's stream) - Remove stagger between fuzz requests (fire all concurrently) - Add naive-shared-state-server.ts: a minimal non-SDK server with a subtle bug (inFlightRequests keyed by request ID, not session+request ID). Verified to fail both notification isolation tests.
- Tests now verify each client receives all 3 expected progress
notifications (not just > 0)
- Failure messages explain what happened:
- 'Received 1/3 expected progress notifications (2 likely routed
to another client's stream)'
- 'Received 4 notification(s) from other clients: fuzz-client-0
(2x), fuzz-client-1 (2x) — the server wrote them to this
client's SSE stream'
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
1 participant
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.
Adds conformance test scenarios for the cross-client data leak vulnerability (GHSA-345p-7cg4-v4c7), focused on Issue 2 (server re-use /
this._transportoverwrite).New scenarios
session-isolationnotification-isolationnotification-isolation-fuzzHow to run
# Against any MCP server implementing test_tool_with_progress: node dist/index.js server --url http://localhost:3000/mcp --scenario notification-isolation node dist/index.js server --url http://localhost:3000/mcp --scenario notification-isolation-fuzzServer requirements
The server under test needs to implement
test_tool_with_progresswhich sends 3notifications/progressevents (0/100, 50/100, 100/100) with ~50ms delays, using theprogressTokenfrom_meta.Context
Built to share with Cloudflare so they can test their
createMcpHandler(stateless Workers path) against the notification cross-talk vulnerability.