From 5d36625400f5e38c9c9ab748a64990bc69feb473 Mon Sep 17 00:00:00 2001 From: hellices Date: Sat, 1 Aug 2026 13:24:56 +0900 Subject: [PATCH 1/2] fix: refuse standalone GET on the MCP endpoint with 405 (#136) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mcp SDK (1.28.x) serves GET as an infinite standalone SSE stream even in stateless mode with JSON responses — a stream korvid can never send anything on. Because korvid neutralizes uvicorn's signal capture (the TUI owns signals), sse-starlette's shutdown watcher never fires, uvicorn's graceful shutdown waits forever on the open connection, and the controller's 5s deadline falls back to a hard cancel that tears the stream down mid-request: 'Exception in ASGI application' (CancelledError) plus a lifespan traceback land on the terminal the TUI is drawing on. The MCP spec allows a server that offers no SSE stream to answer GET with 405 Method Not Allowed (the TypeScript SDK's stateless mode does exactly this), so refuse everything but POST in the ASGI wrapper before it reaches the SDK session manager. No connection is ever held open, and graceful shutdown completes within the controller's deadline. Fixes #136 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/korvid/mcp/server.py | 15 +++++++++ tests/mcp/test_server.py | 68 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/src/korvid/mcp/server.py b/src/korvid/mcp/server.py index e2adedbe..88483215 100644 --- a/src/korvid/mcp/server.py +++ b/src/korvid/mcp/server.py @@ -34,6 +34,7 @@ from mcp.server.streamable_http_manager import StreamableHTTPSessionManager from mcp.server.transport_security import TransportSecuritySettings from starlette.applications import Starlette +from starlette.responses import Response from starlette.routing import Mount from starlette.types import Receive, Scope, Send @@ -288,6 +289,20 @@ async def run(self) -> None: ) 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. + if scope["type"] == "http" and scope["method"] != "POST": + response = 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..4e90019d 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -179,6 +179,74 @@ 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 when a host has issued a GET. + + 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. + """ + import httpx + + server = make_server(port=0) + task = asyncio.create_task(server.run()) + port = await asyncio.wait_for(server.wait_started(), timeout=10) + + 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, + ): + async for _ in resp.aiter_lines(): # drains nothing on a 405 + pass + + get_task = asyncio.create_task(issue_get()) + try: + await asyncio.wait_for(get_task, timeout=10) + server.request_shutdown() + # Must complete gracefully — no hard-cancel fallback needed. + await asyncio.wait_for(task, timeout=5) + assert task.done() + 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_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 From 3e4794e72a3b76b83da905602f2c355b636ca076 Mon Sep 17 00:00:00 2001 From: hellices Date: Sat, 1 Aug 2026 13:41:15 +0900 Subject: [PATCH 2/2] fix: validate transport security before the non-POST 405; pin the open-connection shutdown regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 on #140: - The early 405 bypassed the DNS-rebinding Host/Origin validation for non-POST requests. Run the same TransportSecurityMiddleware check the session manager applies before answering 405, so a hostile Origin on a GET is refused (403), never acknowledged. New test: test_hostile_origin_get_is_rejected_not_answered_405. - The shutdown regression test awaited the GET to completion before requesting shutdown, so it never exercised shutdown with an open connection. It now synchronizes on response start and requests shutdown while the GET is held open — verified to fail (timeout, then CancelledError tracebacks) against the pre-fix server. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/korvid/mcp/server.py | 16 +++++++++++++--- tests/mcp/test_server.py | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/korvid/mcp/server.py b/src/korvid/mcp/server.py index 88483215..98a6c773 100644 --- a/src/korvid/mcp/server.py +++ b/src/korvid/mcp/server.py @@ -32,8 +32,12 @@ 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 @@ -288,6 +292,8 @@ 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 @@ -296,9 +302,13 @@ async def handle(scope: Scope, receive: Receive, send: Send) -> None: # 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. + # 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": - response = Response( + 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) diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index 4e90019d..c019eb27 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -207,18 +207,22 @@ async def test_get_is_refused_with_405_instead_of_an_sse_stream() -> None: async def test_shutdown_completes_while_a_client_holds_a_get_connection() -> None: - """Graceful shutdown must finish even when a host has issued a GET. + """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 ( @@ -229,16 +233,18 @@ async def issue_get() -> None: 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(get_task, timeout=10) + 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() @@ -247,6 +253,30 @@ async def issue_get() -> None: 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