Skip to content

Commit 40642cc

Browse files
author
Veerendra Kumar
committed
fix: reject pre-session GET in the session manager before a session is created
A pre-session GET (one without a session ID header in stateful mode) was being handled AFTER a transport and session were already registered. The transport's _handle_get_request correctly returned 405, but by then an unused session existed indefinitely (with default no idle timeout). Restructured to reject pre-session GETs at the manager layer BEFORE any transport creation: 1. Manager now checks for GET without session ID first 2. Security validation (DNS rebinding protection) runs before the 405 so malformed/attack requests get their proper 421 response 3. Only after security passes does it return 405 Method Not Allowed The transport-level check remains as defensive code for standalone transport use (the class is public). Tests now cover both paths: - Manager path: test_pre_session_get_rejected_without_creating_transport - Standalone path: test_standalone_transport_pre_session_get_returns_405 Also added test_standalone_transport_get_with_wrong_session_returns_404 to cover the session ID mismatch validation (removed pragma: no cover). Addresses review finding: #3129
1 parent 8409a65 commit 40642cc

4 files changed

Lines changed: 182 additions & 1 deletion

File tree

src/mcp/server/streamable_http.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,7 @@ async def _validate_session(self, request: Request, send: Send) -> bool:
860860
return False
861861

862862
# If session ID doesn't match, return error
863-
if request_session_id != self.mcp_session_id: # pragma: no cover
863+
if request_session_id != self.mcp_session_id:
864864
response = self._create_error_response(
865865
"Not Found: Invalid or expired session ID",
866866
HTTPStatus.NOT_FOUND,

src/mcp/server/streamable_http_manager.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,38 @@ async def _handle_stateful_request(self, scope: Scope, receive: Receive, send: S
253253
request = Request(scope, receive)
254254
request_mcp_session_id = request.headers.get(MCP_SESSION_ID_HEADER)
255255

256+
# Per MCP spec, a GET without a session ID cannot establish an SSE stream in
257+
# stateful mode because no session exists yet. Reject with 405 BEFORE creating
258+
# any transport or session to avoid leaking resources. Security validation
259+
# (DNS rebinding protection) happens first so malformed/attack requests get
260+
# their proper 421 response rather than our 405.
261+
if request.method == "GET" and request_mcp_session_id is None:
262+
# Run security validation first if enabled
263+
if self.security_settings is not None:
264+
from mcp.server.transport_security import TransportSecurityMiddleware
265+
266+
security = TransportSecurityMiddleware(self.security_settings)
267+
error_response = await security.validate_request(request, is_post=False)
268+
if error_response:
269+
await error_response(scope, receive, send)
270+
return
271+
272+
response = Response(
273+
JSONRPCError(
274+
jsonrpc="2.0",
275+
id=None,
276+
error=ErrorData(
277+
code=INVALID_REQUEST,
278+
message="Method Not Allowed: GET requires an established session",
279+
),
280+
).model_dump_json(by_alias=True, exclude_unset=True),
281+
status_code=405,
282+
media_type="application/json",
283+
headers={"Allow": "GET, POST, DELETE"},
284+
)
285+
await response(scope, receive, send)
286+
return
287+
256288
user = scope.get("user")
257289
requestor = authorization_context(user) if isinstance(user, AuthenticatedUser) else None
258290

tests/server/test_streamable_http_manager.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,3 +746,49 @@ async def test_anonymous_session_accepts_anonymous_requests(
746746
session_id = await _open_session(manager, None)
747747

748748
assert await _request_session(manager, session_id, None) != 404
749+
750+
751+
@pytest.mark.anyio
752+
async def test_pre_session_get_rejected_without_creating_transport() -> None:
753+
"""A GET without a session ID returns 405 before any transport or session is created."""
754+
app = Server("test-pre-session-get")
755+
manager = StreamableHTTPSessionManager(app=app)
756+
757+
async with manager.run():
758+
sent_messages: list[Message] = []
759+
response_body = b""
760+
761+
async def mock_send(message: Message) -> None:
762+
nonlocal response_body
763+
sent_messages.append(message)
764+
if message["type"] == "http.response.body":
765+
response_body += message.get("body", b"")
766+
767+
scope: Scope = {
768+
"type": "http",
769+
"method": "GET",
770+
"path": "/mcp",
771+
"headers": [(b"accept", b"text/event-stream")],
772+
}
773+
774+
async def mock_receive() -> Message:
775+
return {"type": "http.request", "body": b"", "more_body": False}
776+
777+
await manager.handle_request(scope, mock_receive, mock_send)
778+
779+
# Should return 405 Method Not Allowed
780+
response_start = next(msg for msg in sent_messages if msg["type"] == "http.response.start")
781+
assert response_start["status"] == 405
782+
783+
# Verify Allow header is present
784+
headers = dict(response_start.get("headers", []))
785+
assert headers.get(b"allow") == b"GET, POST, DELETE"
786+
787+
# Verify JSON-RPC error format
788+
error_data = json.loads(response_body)
789+
assert error_data["jsonrpc"] == "2.0"
790+
assert error_data["id"] is None
791+
assert "Method Not Allowed" in error_data["error"]["message"]
792+
793+
# Most importantly: no session was created
794+
assert manager._server_instances == {}

tests/shared/test_streamable_http.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2283,3 +2283,106 @@ async def asgi_receive() -> Message:
22832283
assert body_chunks[-1] == {"type": "http.response.body", "body": b"", "more_body": False}
22842284
assert "Error in standalone SSE writer" not in caplog.text
22852285
assert "Error in standalone SSE response" not in caplog.text
2286+
2287+
2288+
@pytest.mark.anyio
2289+
async def test_standalone_transport_pre_session_get_returns_405() -> None:
2290+
"""A stateful transport (mcp_session_id set) rejects a GET without session ID with 405.
2291+
2292+
This tests the transport-level defensive check that prevents pre-session GETs from
2293+
establishing an SSE stream. When used via the session manager, the manager rejects
2294+
these requests first; this check ensures the transport is also safe for standalone use.
2295+
"""
2296+
# Create a transport with a session ID (stateful mode)
2297+
transport = StreamableHTTPServerTransport(
2298+
mcp_session_id="test-session-id",
2299+
security_settings=TransportSecuritySettings(enable_dns_rebinding_protection=False),
2300+
)
2301+
2302+
# Set up the read stream writer so handle_request doesn't fail
2303+
read_stream_writer, read_stream = create_context_streams[SessionMessage | Exception](0)
2304+
transport._read_stream_writer = read_stream_writer # pyright: ignore[reportPrivateUsage]
2305+
2306+
sent: list[Message] = []
2307+
2308+
async def asgi_send(message: Message) -> None:
2309+
sent.append(message)
2310+
2311+
async def asgi_receive() -> Message:
2312+
return {"type": "http.request", "body": b"", "more_body": False}
2313+
2314+
# Send a GET request without a session ID header
2315+
scope: Scope = {
2316+
"type": "http",
2317+
"method": "GET",
2318+
"path": "/mcp",
2319+
"query_string": b"",
2320+
"headers": [(b"accept", b"text/event-stream")],
2321+
}
2322+
2323+
async with read_stream_writer, read_stream:
2324+
await transport.handle_request(scope, asgi_receive, asgi_send)
2325+
2326+
# Verify 405 Method Not Allowed response
2327+
response_start = sent[0]
2328+
assert response_start["type"] == "http.response.start"
2329+
assert response_start["status"] == 405
2330+
2331+
# Verify Allow header
2332+
headers = dict(response_start.get("headers", []))
2333+
assert headers.get(b"allow") == b"GET, POST, DELETE"
2334+
2335+
# Verify response body contains the error message
2336+
body = b"".join(m.get("body", b"") for m in sent if m["type"] == "http.response.body")
2337+
assert b"Method Not Allowed" in body
2338+
assert b"GET requires an established session" in body
2339+
2340+
2341+
@pytest.mark.anyio
2342+
async def test_standalone_transport_get_with_wrong_session_returns_404() -> None:
2343+
"""A stateful transport rejects a GET with a mismatched session ID with 404.
2344+
2345+
This tests the transport-level session validation for standalone transport use.
2346+
When used via the session manager, the manager validates session IDs first.
2347+
"""
2348+
# Create a transport with a specific session ID
2349+
transport = StreamableHTTPServerTransport(
2350+
mcp_session_id="correct-session-id",
2351+
security_settings=TransportSecuritySettings(enable_dns_rebinding_protection=False),
2352+
)
2353+
2354+
# Set up the read stream writer so handle_request doesn't fail
2355+
read_stream_writer, read_stream = create_context_streams[SessionMessage | Exception](0)
2356+
transport._read_stream_writer = read_stream_writer # pyright: ignore[reportPrivateUsage]
2357+
2358+
sent: list[Message] = []
2359+
2360+
async def asgi_send(message: Message) -> None:
2361+
sent.append(message)
2362+
2363+
async def asgi_receive() -> Message:
2364+
return {"type": "http.request", "body": b"", "more_body": False}
2365+
2366+
# Send a GET request with a WRONG session ID header
2367+
scope: Scope = {
2368+
"type": "http",
2369+
"method": "GET",
2370+
"path": "/mcp",
2371+
"query_string": b"",
2372+
"headers": [
2373+
(b"accept", b"text/event-stream"),
2374+
(b"mcp-session-id", b"wrong-session-id"),
2375+
],
2376+
}
2377+
2378+
async with read_stream_writer, read_stream:
2379+
await transport.handle_request(scope, asgi_receive, asgi_send)
2380+
2381+
# Verify 404 Not Found response (session validation failed)
2382+
response_start = sent[0]
2383+
assert response_start["type"] == "http.response.start"
2384+
assert response_start["status"] == 404
2385+
2386+
# Verify response body contains the error message
2387+
body = b"".join(m.get("body", b"") for m in sent if m["type"] == "http.response.body")
2388+
assert b"Invalid or expired session ID" in body

0 commit comments

Comments
 (0)