diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e196c91..7ca17cba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security - The MCP ingress now rejects non-object JSON-RPC messages, non-string methods, and non-object `tools/call` parameters with a bounded `MCP_INVALID_REQUEST` response. Structurally invalid attacker input no longer reaches attribute errors, HTTP 500 responses, or exception trace logging. +- The unauthenticated health/readiness rate limiter now expires inactive source-address entries and caps tracked clients at 10,000. Source-address churn can no longer grow the in-memory limiter map for the lifetime of the gateway. ### Changed diff --git a/docs/spec/threat-model.md b/docs/spec/threat-model.md index 58672010..42d2385b 100644 --- a/docs/spec/threat-model.md +++ b/docs/spec/threat-model.md @@ -65,7 +65,8 @@ Status: Draft v0.1 | Covers: Phase 1 cMCP Runtime | Tampering | A3 injects instructions in tool response payload | Response inspection (Stage 4: injection detection patterns) | Pattern-based detection has false negatives; sophisticated injection may evade patterns | | Repudiation | Tool server denies a call was made | Audit entry records call, tool server identity, and response hash | Tool server can deny it produced a specific response (only response hash is recorded, not content) | | Information Disclosure | A3 returns more data than requested | Response schema validation strips surplus fields (redact mode) | Strict mode may be too disruptive; redact mode requires correct schema in catalog | -| Denial of Service | A3 returns oversized responses | Stage 1 size check (default 2MB limit) | DDoS via many simultaneous large responses | +| Denial of Service | A3 returns oversized responses | Stage 1 size check (default 2MB limit) | DDoS via many simultaneous large responses | +| Denial of Service | A4 churns source addresses against unauthenticated probes | Per-IP windows expire and the limiter caps tracked client cardinality | Distributed traffic can still exhaust the configured request budget | | Elevation of Privilege | A5 calls escalating sequence of individually-authorized tools crossing compliance boundary | Call graph tracking + session sensitivity policy | Runtime uses temporal adjacency, not true data provenance; sophisticated cross-system flows may not be detected | ### Tool Catalog diff --git a/src/cmcp_runtime/mcp/server.py b/src/cmcp_runtime/mcp/server.py index df2028c2..0ce101b0 100644 --- a/src/cmcp_runtime/mcp/server.py +++ b/src/cmcp_runtime/mcp/server.py @@ -86,14 +86,24 @@ def __init__( *, paths: frozenset[str], requests_per_minute: int = 60, + max_clients: int = 10_000, ) -> None: super().__init__(app) self._paths = paths self._limit = requests_per_minute self._window = 60.0 + self._max_clients = max_clients self._counts: dict[str, list[float]] = defaultdict(list) self._lock = asyncio.Lock() + def _prune_inactive_clients(self, cutoff: float) -> None: + expired_clients = [ + client for client, timestamps in self._counts.items() + if not timestamps or timestamps[-1] <= cutoff + ] + for client in expired_clients: + del self._counts[client] + async def dispatch(self, request: Request, call_next: Any) -> Response: if request.url.path not in self._paths: return await call_next(request) @@ -101,6 +111,16 @@ async def dispatch(self, request: Request, call_next: Any) -> Response: now = time.monotonic() async with self._lock: cutoff = now - self._window + # Reclaim clients whose complete window has expired. Without this, + # one request from each spoofed/churned address grows the map for + # the process lifetime. + self._prune_inactive_clients(cutoff) + if ip not in self._counts and len(self._counts) >= self._max_clients: + return JSONResponse( + {"error": "Too Many Requests", "error_code": "RATE_LIMITED"}, + status_code=429, + headers={"Retry-After": "60"}, + ) hits = self._counts[ip] # Prune timestamps outside the window while hits and hits[0] <= cutoff: diff --git a/tests/unit/test_mcp_server_auth.py b/tests/unit/test_mcp_server_auth.py index 53b5012d..16fd8d4e 100644 --- a/tests/unit/test_mcp_server_auth.py +++ b/tests/unit/test_mcp_server_auth.py @@ -156,7 +156,9 @@ def test_structurally_invalid_json_rpc_returns_bounded_400(payload): # ── NET-002: /health rate limit ─────────────────────────────────────────────── -def _make_server_with_low_rate_limit(requests_per_minute: int = 3) -> MCPServer: +def _make_server_with_low_rate_limit( + requests_per_minute: int = 3, max_clients: int = 10_000 +) -> MCPServer: """Create a server with a very low rate limit for testing.""" from starlette.middleware import Middleware @@ -178,6 +180,7 @@ def _make_server_with_low_rate_limit(requests_per_minute: int = 3) -> MCPServer: _RateLimitMiddleware, paths=frozenset({"/health"}), requests_per_minute=requests_per_minute, + max_clients=max_clients, ) ], exception_handlers={}, @@ -241,6 +244,31 @@ def test_rate_limit_middleware_paths_only(): assert resp.status_code == 200 +def test_rate_limit_caps_tracked_client_addresses(): + server = _make_server_with_low_rate_limit(max_clients=2) + + with TestClient(server.app, client=("192.0.2.1", 1001)) as first_client: + assert first_client.get("/health").status_code == 200 + with TestClient(server.app, client=("192.0.2.2", 1002)) as second_client: + assert second_client.get("/health").status_code == 200 + with TestClient(server.app, client=("192.0.2.3", 1003)) as third_client: + response = third_client.get("/health") + + assert response.status_code == 429 + assert response.json()["error_code"] == "RATE_LIMITED" + + +def test_rate_limit_reclaims_inactive_client_addresses(): + from cmcp_runtime.mcp.server import _RateLimitMiddleware + + limiter = _RateLimitMiddleware(MagicMock(), paths=frozenset({"/health"}), max_clients=1) + limiter._counts["192.0.2.1"] = [100.0] + + limiter._prune_inactive_clients(cutoff=100.0) + + assert "192.0.2.1" not in limiter._counts + + # ── CONF-007: /readyz structured readiness probe ────────────────────────────────────