security: MCP stdio dispatcher misdelivers server requests as responses (Z-052)

[PierrunoYT]

Description

The MCP (Model Context Protocol) stdio dispatcher in internal/mcp/client.go suffers from a message-type confusion bug. The current
eadLoop implementation filters primarily on whether message.ID is nil to determine if a message is a response or a request, but it does not explicitly validate the method or the specific JSON-RPC message type.

When an MCP server initiates its own request to the client, the dispatcher mistakenly treats this incoming request as a response to a previously sent client request. This leads to the dispatcher returning an empty result or a malformed response to the original caller, as the server's request is consumed and discarded without being routed to a proper request handler.

Impact

This causes silent failures and incorrect behavior when interacting with sophisticated MCP servers that utilize server-initiated requests (e.g., for capability negotiation or resource updates). The user sees a tool return 'no results' or an empty string even when the server is functioning correctly.

Recommended Fix

1. Explicit Type Validation: Update the
   eadLoop to explicitly check the JSON-RPC message structure. Only messages that are explicitly marked as responses (containing a matching id to a pending request) should be returned to the caller.
2. Request Routing: Implement a separate dispatch path for requests initiated by the server. These should be routed to a handler that allows the client to respond to the server, rather than letting them be swallowed by the response loop.
3. Improved Diagnostics: Add logging for unexpected message types to help diagnose protocol mismatches between the client and various MCP server implementations.

[vedantlavale]

I’m working on this issue. I’ll look into the dispatcher’s message-type handling and request routing, and share a PR once I have a fix ready.

[gnanam1990]

Approved after tracing the current dispatcher. readLoop can still deliver an incoming method-bearing server request to a pending client call when the ID matches, instead of routing or rejecting it as a request. PRs #935 and #942 are both active for this issue; contributors should coordinate on those rather than start a third implementation.

[Vasanthdev2004]

Fixed on main by 7c50a9f (#942), so closing.

readLoop now classifies a frame before it looks anything up: isRequestOrNotification() catches a method-bearing frame and refuses to route it as a response, replying -32601 for an echoable id, and only Method == "" && ID != nil reaches the pending map. The reply is queued to a dedicated writer rather than written from the reader, which was the part I insisted on when I declined #935 for writing it under client.mu. The method-present test also covers the "method": null and "method": "" spellings.

Verified rather than assumed. I read the guard out of current main rather than out of the commit, so a later refactor could not have dropped it, and I drove the reported symptom end to end through the public CallTool path on both sides of the fix: on 7c50a9f's parent a server frame interleaved ahead of the real result gives an empty tool result, which is exactly what this issue described, and on main the same probe returns the real result while the server receives the method-not-found error. Disabling the single guard line reproduces the empty result and fails all five shipped regression tests, so the guard is load-bearing rather than decorative. The package is green under the race detector.

Why it stayed open: #942's body opens "Fixes #935, #924", and only the number directly after the keyword is linked, so GitHub auto-closed #935 and left this one behind. Nothing was unfixed.

One adjacent thing I am not folding in here, because it is a different transport and cannot misdeliver across calls: network_client.go:219 matches on id with no method guard, so a streamable-HTTP body carrying a server request surfaces as a silently empty result instead of a protocol error. networkClient.request holds the lock across the POST, so there is only ever one in-flight call. Filed separately as #1050.