diff --git a/src/korvid/mcp/server.py b/src/korvid/mcp/server.py index e2adedbe..98a6c773 100644 --- a/src/korvid/mcp/server.py +++ b/src/korvid/mcp/server.py @@ -32,8 +32,13 @@ from mcp import types from mcp.server.lowlevel import Server from mcp.server.streamable_http_manager import StreamableHTTPSessionManager -from mcp.server.transport_security import TransportSecuritySettings +from mcp.server.transport_security import ( + TransportSecurityMiddleware, + TransportSecuritySettings, +) from starlette.applications import Starlette +from starlette.requests import Request +from starlette.responses import Response from starlette.routing import Mount from starlette.types import Receive, Scope, Send @@ -287,7 +292,27 @@ async def run(self) -> None: security_settings=_SECURITY_SETTINGS, ) + security = TransportSecurityMiddleware(_SECURITY_SETTINGS) + async def handle(scope: Scope, receive: Receive, send: Send) -> None: + # Only POST carries MCP traffic here. Stateless + JSON responses + # means no server-initiated messages ever exist, so the SDK's + # standalone GET SSE stream could only hang open — holding + # uvicorn's graceful shutdown hostage until the controller + # hard-cancels it mid-request ("Exception in ASGI application", + # issue #136). The MCP spec allows a server that offers no SSE + # stream to answer GET with 405, so refuse everything but POST + # before it reaches the session manager — after the same + # DNS-rebinding Host/Origin validation the manager would apply, + # so a hostile origin is refused, never acknowledged with a 405. + if scope["type"] == "http" and scope["method"] != "POST": + request = Request(scope, receive) + rejection = await security.validate_request(request, is_post=False) + response = rejection or Response( + "Method Not Allowed", status_code=405, headers={"Allow": "POST"} + ) + await response(scope, receive, send) + return await manager.handle_request(scope, receive, send) @contextlib.asynccontextmanager diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index 0c9069df..c019eb27 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -179,6 +179,104 @@ async def test_streamable_http_roundtrip(tmp_path: Path) -> None: assert not endpoint_file.exists() +async def test_get_is_refused_with_405_instead_of_an_sse_stream() -> None: + """A standalone GET must be answered with 405, not an infinite SSE stream. + + This server is stateless with JSON responses: it never sends + server-initiated messages, so the SDK's standalone GET SSE stream can + only ever hang open — holding uvicorn's graceful shutdown hostage until + the controller hard-cancels it (issue #136). The MCP spec allows a + server that offers no SSE stream to answer GET with 405. + """ + import httpx + + server = make_server(port=0) + task = asyncio.create_task(server.run()) + try: + port = await asyncio.wait_for(server.wait_started(), timeout=10) + async with httpx.AsyncClient() as client: + resp = await client.get( + f"http://127.0.0.1:{port}/mcp/", + headers={"Accept": "text/event-stream"}, + ) + assert resp.status_code == 405 + assert "text/event-stream" not in resp.headers.get("content-type", "") + finally: + server.request_shutdown() + await asyncio.wait_for(task, timeout=10) + + +async def test_shutdown_completes_while_a_client_holds_a_get_connection() -> None: + """Graceful shutdown must finish even while a GET connection is open. + + Regression for issue #136: the SDK served GET as a never-ending SSE + stream, uvicorn's graceful shutdown waited forever on the connection, + and the controller's hard cancel tore down the stream mid-request — + "Exception in ASGI application" (CancelledError) on the TUI's terminal. + The GET is held open (response headers received, body still streaming) + while shutdown is requested, so the old behavior fails this test by + timing out instead of merely racing the connection close. + """ + import httpx + + server = make_server(port=0) + task = asyncio.create_task(server.run()) + port = await asyncio.wait_for(server.wait_started(), timeout=10) + response_started = asyncio.Event() + + async def issue_get() -> None: + async with ( + httpx.AsyncClient(timeout=30) as client, + client.stream( + "GET", + f"http://127.0.0.1:{port}/mcp/", + headers={"Accept": "text/event-stream"}, + ) as resp, + ): + response_started.set() + async for _ in resp.aiter_lines(): # drains nothing on a 405 + pass + + get_task = asyncio.create_task(issue_get()) + try: + await asyncio.wait_for(response_started.wait(), timeout=10) + server.request_shutdown() + # Must complete gracefully — no hard-cancel fallback needed. + await asyncio.wait_for(task, timeout=5) + assert task.done() + await asyncio.wait_for(get_task, timeout=10) + finally: + if not get_task.done(): + get_task.cancel() + server.request_shutdown() + if not task.done(): + await asyncio.wait_for(task, timeout=10) + + +async def test_hostile_origin_get_is_rejected_not_answered_405() -> None: + """DNS-rebinding protection must run before the GET refusal: a + non-loopback Origin on a GET is refused by the transport security + check (403), not acknowledged with the generic 405.""" + import httpx + + server = make_server(port=0) + task = asyncio.create_task(server.run()) + try: + port = await asyncio.wait_for(server.wait_started(), timeout=10) + async with httpx.AsyncClient() as client: + resp = await client.get( + f"http://127.0.0.1:{port}/mcp/", + headers={ + "Origin": "http://evil.example", + "Accept": "text/event-stream", + }, + ) + assert resp.status_code == 403 + finally: + server.request_shutdown() + await asyncio.wait_for(task, timeout=10) + + async def test_hostile_origin_is_rejected() -> None: """DNS-rebinding protection: loopback binding alone does not stop a malicious webpage from reaching 127.0.0.1, so requests carrying a