From 57a3c7a38e575fb24d3df672e7d3c5f46eec25e4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 14 May 2026 00:06:28 +0000 Subject: [PATCH] fix(mcp): reverse cleanup ordering to terminate transport before clearing owner Reverses _purge_expired_stateful_session_auth_contexts so the transport is popped from server_instances and terminated BEFORE owner/auth tracking is cleared. The previous order left a window where _stateful_session_owners was already empty but server_instances still served the session, so a concurrent request would observe expected_owner is None and bypass the owner-binding check. Addresses Greptile review on PR #26857. Co-authored-by: Mateo Wang --- litellm/proxy/_experimental/mcp_server/server.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index 77f1dfbac04..8d749ea5d80 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -327,10 +327,15 @@ if MCP_AVAILABLE: # ripped out from under it mid-flight. if _stateful_session_active_request_counts.get(session_id, 0) > 0: continue - _remove_stateful_session_tracking(session_id) + # Pop transport + terminate BEFORE removing owner/auth tracking. + # Reversing the order avoids a window where ``_stateful_session_owners`` + # is empty but ``server_instances`` still serves the session — a + # concurrent request in that window would observe ``expected_owner + # is None`` and bypass the owner-binding check. transport = server_instances.pop(session_id, None) if transport is not None: await transport.terminate() + _remove_stateful_session_tracking(session_id) for session_id in list(_stateful_session_auth_context_last_seen): if session_id not in _stateful_session_auth_contexts: @@ -2769,7 +2774,12 @@ if MCP_AVAILABLE: passthrough path), fall back to the client IP so two unrelated anonymous callers from different sources do not collapse to a single ``anonymous`` owner and end up able to drive each other's - stateful sessions. + stateful sessions. Note: when even client IP is unavailable + (exotic deployments without trusted X-Forwarded-For and direct + socket info), the fingerprint degrades to the ``anonymous`` + sentinel and cannot meaningfully protect against another + unauthenticated caller who learns the session id — owner-binding + is best-effort in that mode. """ def _bytes_for_hash(value: Any) -> Optional[bytes]: