You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This issue proposes a change in default behavior for the StreamableServerTransport. Specifically, I propose changing the the default behavior documented here:
// If nil, a [MemoryEventStore] with the default maximum size will be used.
To the following: // If nil, stream replayability is disabled..
While this is technically a breaking change in behavior, I believe the current behavior is unlikely to be useful, and is problematic in a number of ways:
As reported in MCP go-sdk implementation Gap-1: provide APIs to bind event storage with user identifiers #571, replayability may be insecure if the authorization middleware does not validate the Mcp-Session-Id header against the authorized user, meaning the server author must be explicitly aware of the session hijacking attack pattern. This would require the attacker to have access to the cryptographically random session ID and have the ability to terminate a client request, so I think this is unlikely, but still possible.
Even with bounded buffer size (as currently implemented), the MemoryEventStore incurs memory cost on the server, and the accounting metadata (the stream mapping) is a slow memory leak. This is a barrier to scalability.
With replayability enabled, message delivery is necessarily asynchronous. When a client cancels their HTTP request, we can't automatically cancel the corresponding server operation, because we can never know if a subsequent request will resume the stream. This is yet another performance concern.
With replayability enabled, we can't know if server->client calls might eventually succeed, so server operations that interact with the client must hang until they timeout (assuming the server is using a reasonable timeout, which is not guaranteed!).
Servers that care about replayability are likely to be hosted in ways where an in-memory event store is not sufficient. For example, the current in-memory event store provides no benefit to stateless serving.
With all these considerations, I feel strongly that the current default behavior is not worth the marginal benefit it provides, and the best path forward is to disable replayability by default.
Subsequent proposals will provide APIs for enabling secure, robust replayability.
This issue proposes a change in default behavior for the
StreamableServerTransport. Specifically, I propose changing the the default behavior documented here:go-sdk/mcp/streamable.go
Line 366 in 31e97ad
To the following:
// If nil, stream replayability is disabled..While this is technically a breaking change in behavior, I believe the current behavior is unlikely to be useful, and is problematic in a number of ways:
Mcp-Session-Idheader against the authorized user, meaning the server author must be explicitly aware of the session hijacking attack pattern. This would require the attacker to have access to the cryptographically random session ID and have the ability to terminate a client request, so I think this is unlikely, but still possible.MemoryEventStoreincurs memory cost on the server, and the accounting metadata (the stream mapping) is a slow memory leak. This is a barrier to scalability.With all these considerations, I feel strongly that the current default behavior is not worth the marginal benefit it provides, and the best path forward is to disable replayability by default.
Subsequent proposals will provide APIs for enabling secure, robust replayability.
CC @jba @samthanawalla @neild