Skip to content

fix(serve-mcp): session TTL eviction, SSE emitter leak, subscription race, tool name sanitization#66

Merged
deer merged 1 commit into
mainfrom
mcp_security
Jun 22, 2026
Merged

fix(serve-mcp): session TTL eviction, SSE emitter leak, subscription race, tool name sanitization#66
deer merged 1 commit into
mainfrom
mcp_security

Conversation

@deer

@deer deer commented Jun 22, 2026

Copy link
Copy Markdown
Owner
  • Adds a background session reaper to McpServer — sessions idle for longer than sessionIdleTimeout (default 1 hour, configurable via Builder.sessionIdleTimeout(Duration)) are evicted automatically, breaking the permanent-503 deadlock that occurred once maxSessions was hit after the cap-without-TTL fix in fix(serve-mcp): harden MCP surface against DoS, info-leak, and missing DELETE #53. Every JSON-RPC dispatch and SSE connection establishment touches lastAccessed; the reaper sweeps at min(idleTimeout / 2, 5 min) using a daemon virtual thread.
  • Fixes an SSE emitter leak on reconnect: SessionState.emitter is now an AtomicReference<SseEmitter>; a second GET on the same session atomically swaps in the new emitter via getAndSet and calls close() on the old one (unblocking its awaitClose() virtual thread), while compareAndSet in finally prevents the exiting old thread from nulling out a newer reconnect's emitter.
  • Fixes a TOCTOU race on the per-session subscription cap: the check-and-add in handleResourcesSubscribe is now synchronized on the subscriptions set, preventing two concurrent subscribes near the cap from both passing and slightly overshooting it.
  • Sanitizes client-supplied tool names through sanitizeMessage() in the "Unknown tool: X" JSON-RPC error, matching the path used everywhere else error text is constructed.
  • Adds a class-level Javadoc callout that McpServer has no built-in authentication and must be wrapped (e.g. with serve auth's AuthMiddleware) before exposure to untrusted clients.
  • Two new tests: shouldEvictIdleSessionAfterTimeout (configures a 200ms idle timeout, fills the session cap, waits for the reaper to sweep, asserts the slot is freed) and shouldCloseOldSseEmitterOnReconnect (opens an SSE stream, reconnects with the same session ID, asserts the first stream's done future completes — proving the emitter was closed rather than leaked).

@deer deer merged commit ee0629b into main Jun 22, 2026
2 checks passed
@deer deer deleted the mcp_security branch June 22, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant