diff --git a/src/cmcp_runtime/mcp/stdio.py b/src/cmcp_runtime/mcp/stdio.py index 1373d4f..a21a4f0 100644 --- a/src/cmcp_runtime/mcp/stdio.py +++ b/src/cmcp_runtime/mcp/stdio.py @@ -223,6 +223,11 @@ async def start(self) -> None: stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, env=self._env, + # asyncio's default stream limit is 64 KiB, which is smaller than a + # perfectly ordinary tool response. Left at the default, readline() + # raises before MAX_RESPONSE_BYTES is ever consulted, so the bound + # the gateway actually enforces has to be the bound it declares. + limit=MAX_RESPONSE_BYTES, ) logger.info( "stdio server spawned: %s (%s, digest %s)", @@ -256,7 +261,20 @@ async def call(self, call_id: str, tool_name: str, arguments: dict[str, Any]) -> f"stdio server closed its input: {self._spawn.command}", detail=str(exc) ) from exc - raw = await self._proc.stdout.readline() + try: + raw = await self._proc.stdout.readline() + except ValueError as exc: + # A line longer than the limit, or one still unterminated at it. + # asyncio reports both as a bare ValueError, and an over-sized + # response is a refusal with a reason rather than a stray + # exception surfacing from inside the enclave. + await self.close() + raise UpstreamUnavailable( + f"stdio server response exceeds {MAX_RESPONSE_BYTES} bytes without " + f"a newline, so the JSON-RPC stream cannot be framed and the session " + f"is terminated: {self._spawn.command}", + detail=str(exc), + ) from exc if not raw: await self._collect_stderr() diff --git a/tests/unit/test_stdio_upstream.py b/tests/unit/test_stdio_upstream.py index 52f3130..8543127 100644 --- a/tests/unit/test_stdio_upstream.py +++ b/tests/unit/test_stdio_upstream.py @@ -57,6 +57,21 @@ def _script(tmp_path, body: str, name: str = "server.py"): """ +SIZED_SERVER = """ + import json, sys + for line in sys.stdin: + req = json.loads(line) + sys.stdout.write(json.dumps({ + "jsonrpc": "2.0", + "id": req["id"], + "result": {"content": [ + {"type": "text", "text": "x" * req["params"]["arguments"]["n"]}, + ]}, + }) + "\\n") + sys.stdout.flush() +""" + + def _spawn_for(script: str, digest: str | None) -> StdioSpawn: """Pin the script, not the interpreter. @@ -151,6 +166,40 @@ async def test_round_trip(tmp_path) -> None: await server.close() +async def test_response_over_the_asyncio_default_limit_still_round_trips(tmp_path) -> None: + """A 200 KiB response is ordinary, and must not depend on asyncio's default. + + ``create_subprocess_exec`` gives the child's stdout a 64 KiB stream limit + unless told otherwise, and ``readline`` raises a bare ``ValueError`` past it. + Left at the default, a file read or a search result would fail with a stray + exception and ``MAX_RESPONSE_BYTES`` would never be reached at all. + """ + script = _script(tmp_path, SIZED_SERVER) + server = StdioServer(_spawn_for(script, None), allow_unmeasured=True) + await server.start() + try: + assert await server.call("c1", "read", {"n": 200 * 1024}) == "x" * (200 * 1024) + finally: + await server.close() + + +async def test_oversized_response_is_refused_with_a_reason(tmp_path, monkeypatch) -> None: + """Past the declared bound the answer is a refusal, not a raw ValueError. + + The limit is monkeypatched rather than fed 8 MB so the test stays quick; it + is read at spawn time, so the child inherits whatever it is set to here. + """ + monkeypatch.setattr("cmcp_runtime.mcp.stdio.MAX_RESPONSE_BYTES", 8 * 1024) + script = _script(tmp_path, SIZED_SERVER) + server = StdioServer(_spawn_for(script, None), allow_unmeasured=True) + await server.start() + try: + with pytest.raises(UpstreamUnavailable, match="exceeds"): + await server.call("c1", "read", {"n": 64 * 1024}) + finally: + await server.close() + + # --- framing, where a wrong answer is worse than an error ------------------