From 6e2c47720cf632ab4922b7ff3cbc577f3ae7378f Mon Sep 17 00:00:00 2001 From: Rafael Poyiadzi Date: Wed, 25 Feb 2026 11:36:07 +0000 Subject: [PATCH] Fix DOS-03 and DOS-09: remove server-side sleep, extend body size limit DOS-03: Remove the 12-second asyncio.sleep from everyrow_progress that held connections open server-side. Polling cadence is now client-driven via retry_after_seconds in the response text. Removes the connection exhaustion vector identified in the audit. DOS-09: Extend BodySizeLimitMiddleware to accept multiple path prefixes and cover both /api/uploads/ and /mcp endpoints. Previously only upload paths were protected, leaving the MCP endpoint with no body size limit. Co-Authored-By: Claude Opus 4.6 --- everyrow-mcp/manifest.json | 2 +- everyrow-mcp/src/everyrow_mcp/http_config.py | 4 +- everyrow-mcp/src/everyrow_mcp/middleware.py | 10 +++-- everyrow-mcp/src/everyrow_mcp/tool_helpers.py | 10 +++-- everyrow-mcp/src/everyrow_mcp/tools.py | 12 ++---- everyrow-mcp/tests/test_mcp_e2e.py | 2 - everyrow-mcp/tests/test_middleware.py | 37 +++++++++++++++++++ everyrow-mcp/tests/test_server.py | 5 --- everyrow-mcp/tests/test_stdio_content.py | 8 ---- 9 files changed, 57 insertions(+), 33 deletions(-) diff --git a/everyrow-mcp/manifest.json b/everyrow-mcp/manifest.json index a601a8af..fe417c05 100644 --- a/everyrow-mcp/manifest.json +++ b/everyrow-mcp/manifest.json @@ -59,7 +59,7 @@ }, { "name": "everyrow_progress", - "description": "Check progress of a running task. Blocks briefly to limit the polling rate." + "description": "Check progress of a running task." }, { "name": "everyrow_results", diff --git a/everyrow-mcp/src/everyrow_mcp/http_config.py b/everyrow-mcp/src/everyrow_mcp/http_config.py index faa0db2a..94075085 100644 --- a/everyrow-mcp/src/everyrow_mcp/http_config.py +++ b/everyrow-mcp/src/everyrow_mcp/http_config.py @@ -213,7 +213,9 @@ def _wrapped(): # Pure-ASGI middlewares — outermost wraps first. # SecurityHeaders → BodySizeLimit → Starlette app asgi_app = BodySizeLimitMiddleware( - app, max_bytes=settings.max_upload_size_bytes + app, + max_bytes=settings.max_upload_size_bytes, + path_prefixes=("/api/uploads/", "/mcp"), ) asgi_app = SecurityHeadersMiddleware(asgi_app) return asgi_app diff --git a/everyrow-mcp/src/everyrow_mcp/middleware.py b/everyrow-mcp/src/everyrow_mcp/middleware.py index 70552f53..1c249478 100644 --- a/everyrow-mcp/src/everyrow_mcp/middleware.py +++ b/everyrow-mcp/src/everyrow_mcp/middleware.py @@ -98,7 +98,7 @@ class BodySizeLimitMiddleware: when the limit is exceeded — even for chunked transfer-encoding requests that lack a Content-Length header. - Only active on paths matching ``path_prefix``. + Active on paths matching any of the given ``path_prefixes``. """ def __init__( @@ -106,14 +106,16 @@ def __init__( app: ASGIApp, *, max_bytes: int, - path_prefix: str = "/api/uploads/", + path_prefixes: tuple[str, ...] = ("/api/uploads/",), ) -> None: self._app = app self._max_bytes = max_bytes - self._path_prefix = path_prefix + self._path_prefixes = path_prefixes async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: - if scope["type"] != "http" or not scope["path"].startswith(self._path_prefix): + if scope["type"] != "http" or not any( + scope["path"].startswith(p) for p in self._path_prefixes + ): await self._app(scope, receive, send) return diff --git a/everyrow-mcp/src/everyrow_mcp/tool_helpers.py b/everyrow-mcp/src/everyrow_mcp/tool_helpers.py index af355739..084439a1 100644 --- a/everyrow-mcp/src/everyrow_mcp/tool_helpers.py +++ b/everyrow-mcp/src/everyrow_mcp/tool_helpers.py @@ -280,15 +280,17 @@ def progress_message(self, task_id: str) -> str: return f"{completed_msg}\n{next_call}" return f"Task {self.status.value}. Report the error to the user." + retry = redis_store.PROGRESS_POLL_DELAY + if self.is_screen: return dedent(f"""\ - Screen running ({self.elapsed_s}s elapsed). - Immediately call everyrow_progress(task_id='{task_id}').""") + Screen running ({self.elapsed_s}s elapsed). retry_after_seconds={retry} + Wait {retry} seconds, then call everyrow_progress(task_id='{task_id}').""") fail_part = f", {self.failed} failed" if self.failed else "" return dedent(f"""\ - Running: {self.completed}/{self.total} complete, {self.running} running{fail_part} ({self.elapsed_s}s elapsed) - Immediately call everyrow_progress(task_id='{task_id}').""") + Running: {self.completed}/{self.total} complete, {self.running} running{fail_part} ({self.elapsed_s}s elapsed) retry_after_seconds={retry} + Wait {retry} seconds, then call everyrow_progress(task_id='{task_id}').""") def write_initial_task_state( diff --git a/everyrow-mcp/src/everyrow_mcp/tools.py b/everyrow-mcp/src/everyrow_mcp/tools.py index 0363bd94..878f73b3 100644 --- a/everyrow-mcp/src/everyrow_mcp/tools.py +++ b/everyrow-mcp/src/everyrow_mcp/tools.py @@ -1,6 +1,5 @@ """MCP tool functions for the everyrow MCP server.""" -import asyncio import json import logging from pathlib import Path @@ -732,11 +731,11 @@ async def everyrow_progress( params: ProgressInput, ctx: EveryRowContext, ) -> list[TextContent]: - """Check progress of a running task. Blocks briefly to limit the polling rate. + """Check progress of a running task. - After receiving a status update, immediately call everyrow_progress again - unless the task is completed or failed. The tool handles pacing internally. - Do not add commentary between progress calls, just call again immediately. + After receiving a status update, wait at least ``retry_after_seconds`` + before calling everyrow_progress again (unless the task is completed or + failed). Do not add commentary between progress calls. """ client = _get_client(ctx) task_id = params.task_id @@ -754,9 +753,6 @@ async def everyrow_progress( ) ] - # Block server-side before polling — controls the cadence - await asyncio.sleep(redis_store.PROGRESS_POLL_DELAY) - try: status_response = handle_response( await get_task_status_tasks_task_id_status_get.asyncio( diff --git a/everyrow-mcp/tests/test_mcp_e2e.py b/everyrow-mcp/tests/test_mcp_e2e.py index 1eeb17ca..5c1c3e35 100644 --- a/everyrow-mcp/tests/test_mcp_e2e.py +++ b/everyrow-mcp/tests/test_mcp_e2e.py @@ -260,7 +260,6 @@ async def test_call_progress_tool(self, _http_state): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.redis_store.PROGRESS_POLL_DELAY", 0), ): result = await session.call_tool( "everyrow_progress", @@ -358,7 +357,6 @@ async def test_completed_progress_suggests_results(self, _http_state): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.redis_store.PROGRESS_POLL_DELAY", 0), ): result = await session.call_tool( "everyrow_progress", diff --git a/everyrow-mcp/tests/test_middleware.py b/everyrow-mcp/tests/test_middleware.py index 7bd80711..d6583bec 100644 --- a/everyrow-mcp/tests/test_middleware.py +++ b/everyrow-mcp/tests/test_middleware.py @@ -233,3 +233,40 @@ def test_exact_limit_passes(self): client = TestClient(app) resp = client.put("/api/uploads/abc", content=b"x" * 10) assert resp.status_code == 200 + + def test_multiple_path_prefixes(self): + """BodySizeLimitMiddleware enforces limits on all configured paths.""" + + async def _handler(request: Request): + body = await request.body() + return PlainTextResponse(f"received {len(body)} bytes") + + inner = Starlette( + routes=[ + Route("/api/uploads/{upload_id}", _handler, methods=["PUT"]), + Route("/mcp", _handler, methods=["POST"]), + Route("/other", _handler, methods=["POST"]), + ], + ) + app = BodySizeLimitMiddleware( + inner, + max_bytes=10, + path_prefixes=("/api/uploads/", "/mcp"), + ) + client = TestClient(app) + + # Upload path — enforced + resp = client.put("/api/uploads/abc", content=b"x" * 50) + assert resp.status_code == 413 + + # MCP path — enforced + resp = client.post("/mcp", content=b"x" * 50) + assert resp.status_code == 413 + + # MCP path — under limit passes + resp = client.post("/mcp", content=b"x" * 5) + assert resp.status_code == 200 + + # Other path — not enforced + resp = client.post("/other", content=b"x" * 50) + assert resp.status_code == 200 diff --git a/everyrow-mcp/tests/test_server.py b/everyrow-mcp/tests/test_server.py index ad7e4971..9e53e248 100644 --- a/everyrow-mcp/tests/test_server.py +++ b/everyrow-mcp/tests/test_server.py @@ -501,7 +501,6 @@ async def test_progress_api_error(self): new_callable=AsyncMock, side_effect=RuntimeError("API error"), ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), ): params = ProgressInput(task_id=task_id) result = await everyrow_progress(params, ctx) @@ -532,7 +531,6 @@ async def test_progress_running_task(self): new_callable=AsyncMock, return_value=status_response, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), ): params = ProgressInput(task_id=task_id) @@ -567,7 +565,6 @@ async def test_progress_completed_task(self): new_callable=AsyncMock, return_value=status_response, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), ): params = ProgressInput(task_id=task_id) @@ -1467,7 +1464,6 @@ async def test_progress_stdio_returns_single_content(self): new_callable=AsyncMock, return_value=status_response, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), ): result = await everyrow_progress(ProgressInput(task_id=task_id), ctx) @@ -1492,7 +1488,6 @@ async def test_progress_http_returns_text_only(self): new_callable=AsyncMock, return_value=status_response, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), patch( "everyrow_mcp.tools._check_task_ownership", diff --git a/everyrow-mcp/tests/test_stdio_content.py b/everyrow-mcp/tests/test_stdio_content.py index f9cada23..87fda2dd 100644 --- a/everyrow-mcp/tests/test_stdio_content.py +++ b/everyrow-mcp/tests/test_stdio_content.py @@ -344,7 +344,6 @@ async def test_running_status(self): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), ): result = await everyrow_progress(ProgressInput(task_id=task_id), ctx) @@ -368,7 +367,6 @@ async def test_completed_status(self): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), ): result = await everyrow_progress(ProgressInput(task_id=task_id), ctx) @@ -399,7 +397,6 @@ async def test_completed_screen_status(self): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), ): result = await everyrow_progress(ProgressInput(task_id=task_id), ctx) @@ -427,7 +424,6 @@ async def test_running_screen_status(self): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), ): result = await everyrow_progress(ProgressInput(task_id=task_id), ctx) @@ -448,7 +444,6 @@ async def test_error_status(self): new_callable=AsyncMock, side_effect=RuntimeError("API timeout"), ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), ): result = await everyrow_progress(ProgressInput(task_id=task_id), ctx) @@ -473,7 +468,6 @@ async def test_failed_task_with_error_message(self): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), ): result = await everyrow_progress(ProgressInput(task_id=task_id), ctx) @@ -661,7 +655,6 @@ async def test_progress_http_returns_text_only(self): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), patch( "everyrow_mcp.tools._check_task_ownership", @@ -689,7 +682,6 @@ async def test_progress_http_completed_no_output_path_hint(self): new_callable=AsyncMock, return_value=status_resp, ), - patch("everyrow_mcp.tools.asyncio.sleep", new_callable=AsyncMock), patch("everyrow_mcp.tools.write_initial_task_state"), patch( "everyrow_mcp.tools._check_task_ownership",