Skip to content

Commit 3515d83

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 ed897fa commit 3515d83

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
@@ -894,7 +894,7 @@ async def _validate_session(self, request: Request, send: Send) -> bool:
894894
return False
895895

896896
# If session ID doesn't match, return error
897-
if request_session_id != self.mcp_session_id: # pragma: no cover
897+
if request_session_id != self.mcp_session_id:
898898
response = self._create_error_response(
899899
"Not Found: Invalid or expired session ID",
900900
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
@@ -2264,3 +2264,106 @@ async def asgi_receive() -> Message:
22642264
assert body_chunks[-1] == {"type": "http.response.body", "body": b"", "more_body": False}
22652265
assert "Error in standalone SSE writer" not in caplog.text
22662266
assert "Error in standalone SSE response" not in caplog.text
2267+
2268+
2269+
@pytest.mark.anyio
2270+
async def test_standalone_transport_pre_session_get_returns_405() -> None:
2271+
"""A stateful transport (mcp_session_id set) rejects a GET without session ID with 405.
2272+
2273+
This tests the transport-level defensive check that prevents pre-session GETs from
2274+
establishing an SSE stream. When used via the session manager, the manager rejects
2275+
these requests first; this check ensures the transport is also safe for standalone use.
2276+
"""
2277+
# Create a transport with a session ID (stateful mode)
2278+
transport = StreamableHTTPServerTransport(
2279+
mcp_session_id="test-session-id",
2280+
security_settings=TransportSecuritySettings(enable_dns_rebinding_protection=False),
2281+
)
2282+
2283+
# Set up the read stream writer so handle_request doesn't fail
2284+
read_stream_writer, read_stream = create_context_streams[SessionMessage | Exception](0)
2285+
transport._read_stream_writer = read_stream_writer # pyright: ignore[reportPrivateUsage]
2286+
2287+
sent: list[Message] = []
2288+
2289+
async def asgi_send(message: Message) -> None:
2290+
sent.append(message)
2291+
2292+
async def asgi_receive() -> Message:
2293+
return {"type": "http.request", "body": b"", "more_body": False}
2294+
2295+
# Send a GET request without a session ID header
2296+
scope: Scope = {
2297+
"type": "http",
2298+
"method": "GET",
2299+
"path": "/mcp",
2300+
"query_string": b"",
2301+
"headers": [(b"accept", b"text/event-stream")],
2302+
}
2303+
2304+
async with read_stream_writer, read_stream:
2305+
await transport.handle_request(scope, asgi_receive, asgi_send)
2306+
2307+
# Verify 405 Method Not Allowed response
2308+
response_start = sent[0]
2309+
assert response_start["type"] == "http.response.start"
2310+
assert response_start["status"] == 405
2311+
2312+
# Verify Allow header
2313+
headers = dict(response_start.get("headers", []))
2314+
assert headers.get(b"allow") == b"GET, POST, DELETE"
2315+
2316+
# Verify response body contains the error message
2317+
body = b"".join(m.get("body", b"") for m in sent if m["type"] == "http.response.body")
2318+
assert b"Method Not Allowed" in body
2319+
assert b"GET requires an established session" in body
2320+
2321+
2322+
@pytest.mark.anyio
2323+
async def test_standalone_transport_get_with_wrong_session_returns_404() -> None:
2324+
"""A stateful transport rejects a GET with a mismatched session ID with 404.
2325+
2326+
This tests the transport-level session validation for standalone transport use.
2327+
When used via the session manager, the manager validates session IDs first.
2328+
"""
2329+
# Create a transport with a specific session ID
2330+
transport = StreamableHTTPServerTransport(
2331+
mcp_session_id="correct-session-id",
2332+
security_settings=TransportSecuritySettings(enable_dns_rebinding_protection=False),
2333+
)
2334+
2335+
# Set up the read stream writer so handle_request doesn't fail
2336+
read_stream_writer, read_stream = create_context_streams[SessionMessage | Exception](0)
2337+
transport._read_stream_writer = read_stream_writer # pyright: ignore[reportPrivateUsage]
2338+
2339+
sent: list[Message] = []
2340+
2341+
async def asgi_send(message: Message) -> None:
2342+
sent.append(message)
2343+
2344+
async def asgi_receive() -> Message:
2345+
return {"type": "http.request", "body": b"", "more_body": False}
2346+
2347+
# Send a GET request with a WRONG session ID header
2348+
scope: Scope = {
2349+
"type": "http",
2350+
"method": "GET",
2351+
"path": "/mcp",
2352+
"query_string": b"",
2353+
"headers": [
2354+
(b"accept", b"text/event-stream"),
2355+
(b"mcp-session-id", b"wrong-session-id"),
2356+
],
2357+
}
2358+
2359+
async with read_stream_writer, read_stream:
2360+
await transport.handle_request(scope, asgi_receive, asgi_send)
2361+
2362+
# Verify 404 Not Found response (session validation failed)
2363+
response_start = sent[0]
2364+
assert response_start["type"] == "http.response.start"
2365+
assert response_start["status"] == 404
2366+
2367+
# Verify response body contains the error message
2368+
body = b"".join(m.get("body", b"") for m in sent if m["type"] == "http.response.body")
2369+
assert b"Invalid or expired session ID" in body

0 commit comments

Comments
 (0)