fix: enforce a per-string length cap on tool arguments (#562)

scripts/mock_upstream.py

@@ -43,6 +43,26 @@
 _MAX_ARG_DEPTH = 20
 _MAX_ARG_KEYS = 256
 
+# #562: docs/spec/proxy-security.md's Fuzzing Definition of Done specs
+# MAX_STRING_LENGTH at 1MB per string field, separate from the depth/key
+# caps above. A single oversized string can sit inside an otherwise
+# shallow, low-key-count payload and slip past both of those unbounded,
+# up to whatever the whole-body byte cap happens to be.
+#
+# Not set to the spec's literal 1MB: that would equal MAX_REQUEST_BYTES
+# itself, and a 1MB string plus any JSON structure around it already
+# exceeds the whole-body cap, so the check could never fire before
+# DOS-001's size rejection already had. Set to half of MAX_REQUEST_BYTES
+# instead, so a single string cannot consume the whole request budget and
+# the cap is actually reachable. Worth revisiting to the spec's literal
+# value if MAX_REQUEST_BYTES itself is ever raised toward the spec's
+# stated 10MB (see #562).
+#
+# Checked as UTF-8 byte length, not character count, since a codepoint
+# count understates the actual memory and processing cost of multi-byte
+# text.
+_MAX_ARG_STRING_LENGTH = MAX_REQUEST_BYTES // 2
+
 logger = logging.getLogger("mock_upstream")
 
 
@@ -56,23 +76,41 @@ def _valid_rpc_id(value: Any) -> bool:
     return value is None or (isinstance(value, (str, int, float)) and not isinstance(value, bool))
 
 
+def _over_string_cap(text: str) -> bool:
+    return len(text.encode("utf-8")) > _MAX_ARG_STRING_LENGTH
+
+
+def _object_shape_violation(value: dict[str, Any], depth: int) -> str | None:
+    if len(value) > _MAX_ARG_KEYS:
+        return f"object has more than {_MAX_ARG_KEYS} keys"
+    for key, child in value.items():
+        # Keys carry the same cost as values and are not covered by the key
+        # *count* cap above, so a single huge key would otherwise slip
+        # through every check here.
+        if isinstance(key, str) and _over_string_cap(key):
+            return f"object key over the length cap of {_MAX_ARG_STRING_LENGTH} bytes"
+        violation = _arg_shape_violation(child, depth=depth + 1)
+        if violation is not None:
+            return violation
+    return None
+
+
 def _arg_shape_violation(value: Any, *, depth: int = 0) -> str | None:
-    """Return a message describing the first depth/key-count violation found
-    under `value`, or None if it fits within `_MAX_ARG_DEPTH` / `_MAX_ARG_KEYS`."""
+    """Return a message describing the first depth/key-count/string-length
+    violation found under `value`, or None if it fits within `_MAX_ARG_DEPTH` /
+    `_MAX_ARG_KEYS` / `_MAX_ARG_STRING_LENGTH`."""
     if depth > _MAX_ARG_DEPTH:
         return f"arguments nested past the depth cap of {_MAX_ARG_DEPTH}"
     if isinstance(value, dict):
-        if len(value) > _MAX_ARG_KEYS:
-            return f"object has more than {_MAX_ARG_KEYS} keys"
-        for child in value.values():
-            violation = _arg_shape_violation(child, depth=depth + 1)
-            if violation is not None:
-                return violation
-    elif isinstance(value, list):
+        return _object_shape_violation(value, depth)
+    if isinstance(value, list):
         for child in value:
             violation = _arg_shape_violation(child, depth=depth + 1)
             if violation is not None:
                 return violation
+        return None
+    if isinstance(value, str) and _over_string_cap(value):
+        return f"string value over the length cap of {_MAX_ARG_STRING_LENGTH} bytes"
     return None
 
 

src/cmcp_runtime/mcp/server.py

@@ -41,6 +41,12 @@
 # Endpoints exempt from bearer-token auth (Kubernetes liveness / readiness probes)
 _AUTH_EXEMPT_PATHS = {"/health", "/readyz"}
 
+# DOS-001: default ceiling on a single request body. Overridable per
+# deployment via MCPServer(max_request_bytes=...). Named here rather than
+# left inline on the constructor so the argument-shape caps below can be
+# derived from it instead of restating the number.
+_DEFAULT_MAX_REQUEST_BYTES = 1_000_000
+
 # #518: DOS-001's byte cap bounds total size, not shape. A payload well under
 # the limit can still push toward Python's recursion limit through deep
 # nesting, or cost real time to iterate through a flat object with thousands
@@ -51,6 +57,29 @@
 _MAX_ARG_DEPTH = 20
 _MAX_ARG_KEYS = 256
 
+# #562: docs/spec/proxy-security.md's Fuzzing Definition of Done specs
+# MAX_STRING_LENGTH at 1MB per string field, separate from the depth/key
+# caps above. A single oversized string can sit inside an otherwise
+# shallow, low-key-count payload and slip past both of those unbounded,
+# up to whatever the whole-body byte cap happens to be.
+#
+# Not set to the spec's literal 1MB: that equals the whole-body default
+# below, and a 1MB string plus any JSON structure around it already
+# exceeds that cap, so the check could never fire before DOS-001's size
+# rejection already had. Derived from the default instead, so a single
+# string cannot consume the whole request budget, the cap is actually
+# reachable, and raising the default carries this along with it rather
+# than silently leaving it behind. This is scoped against the *default*
+# max_request_bytes; a deployment that configures a smaller value simply
+# has the whole-body cap bind first, which is a safe direction to fail
+# in, not a gap. Worth revisiting to the spec's literal value if the
+# default is ever raised toward the spec's stated 10MB (see #562).
+#
+# Checked as UTF-8 byte length, not character count, since a codepoint
+# count understates the actual memory and processing cost of multi-byte
+# text.
+_MAX_ARG_STRING_LENGTH = _DEFAULT_MAX_REQUEST_BYTES // 2
+
 
 def _reject_nan_and_infinity(text: str) -> float:
     raise ValueError(f"non-standard JSON value not allowed: {text}")
@@ -62,23 +91,41 @@ def _valid_rpc_id(value: Any) -> bool:
     return value is None or (isinstance(value, (str, int, float)) and not isinstance(value, bool))
 
 
+def _over_string_cap(text: str) -> bool:
+    return len(text.encode("utf-8")) > _MAX_ARG_STRING_LENGTH
+
+
+def _object_shape_violation(value: dict[str, Any], depth: int) -> str | None:
+    if len(value) > _MAX_ARG_KEYS:
+        return f"object has more than {_MAX_ARG_KEYS} keys"
+    for key, child in value.items():
+        # Keys carry the same cost as values and are not covered by the key
+        # *count* cap above, so a single huge key would otherwise slip
+        # through every check here.
+        if isinstance(key, str) and _over_string_cap(key):
+            return f"object key over the length cap of {_MAX_ARG_STRING_LENGTH} bytes"
+        violation = _arg_shape_violation(child, depth=depth + 1)
+        if violation is not None:
+            return violation
+    return None
+
+
 def _arg_shape_violation(value: Any, *, depth: int = 0) -> str | None:
-    """Return a message describing the first depth/key-count violation found
-    under `value`, or None if it fits within `_MAX_ARG_DEPTH` / `_MAX_ARG_KEYS`."""
+    """Return a message describing the first depth/key-count/string-length
+    violation found under `value`, or None if it fits within `_MAX_ARG_DEPTH` /
+    `_MAX_ARG_KEYS` / `_MAX_ARG_STRING_LENGTH`."""
     if depth > _MAX_ARG_DEPTH:
         return f"arguments nested past the depth cap of {_MAX_ARG_DEPTH}"
     if isinstance(value, dict):
-        if len(value) > _MAX_ARG_KEYS:
-            return f"object has more than {_MAX_ARG_KEYS} keys"
-        for child in value.values():
-            violation = _arg_shape_violation(child, depth=depth + 1)
-            if violation is not None:
-                return violation
-    elif isinstance(value, list):
+        return _object_shape_violation(value, depth)
+    if isinstance(value, list):
         for child in value:
             violation = _arg_shape_violation(child, depth=depth + 1)
             if violation is not None:
                 return violation
+        return None
+    if isinstance(value, str) and _over_string_cap(value):
+        return f"string value over the length cap of {_MAX_ARG_STRING_LENGTH} bytes"
     return None
 
 
@@ -255,7 +302,7 @@ def __init__(
         audit_chain: AuditChain | None = None,
         bearer_token: str | None = None,
         session: SessionState | None = None,
-        max_request_bytes: int = 1_000_000,
+        max_request_bytes: int = _DEFAULT_MAX_REQUEST_BYTES,
     ) -> None:
         self._proxy = proxy
         self._session_manager = session_manager
@@ -506,7 +553,8 @@ async def _handle_tool_call(self, rpc_id: Any, params: dict[str, Any]) -> Respon
         tool_name: str = params.get("name", "").lower()
         arguments: dict[str, Any] = params.get("arguments", {})
 
-        # #518: depth/key-count cap, matching scripts/mock_upstream.py.
+        # #518/#562: depth, key-count and string-length caps, matching
+        # scripts/mock_upstream.py.
         violation = _arg_shape_violation(arguments)
         if violation is not None:
             return JSONResponse(

tests/unit/test_mcp_server_auth.py

@@ -2,13 +2,19 @@
 
 from __future__ import annotations
 
+import json
 from typing import Any
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
 from starlette.testclient import TestClient
 
-from cmcp_runtime.mcp.server import _MAX_ARG_DEPTH, _MAX_ARG_KEYS, MCPServer
+from cmcp_runtime.mcp.server import (
+    _MAX_ARG_DEPTH,
+    _MAX_ARG_KEYS,
+    _MAX_ARG_STRING_LENGTH,
+    MCPServer,
+)
 
 
 def _make_server(bearer_token: str | None = None) -> MCPServer:
@@ -666,6 +672,27 @@ def test_depth_cap_violation_inside_a_list_is_caught():
     assert resp.json()["error"]["code"] == -32602
 
 
+def test_well_formed_list_in_arguments_is_accepted():
+    """The list walk must fall through cleanly when nothing inside it violates a
+    cap. Without this the only list coverage is the rejection path, which would
+    still pass if the walk rejected every list it saw."""
+    server = _make_server()
+    client = TestClient(server.app, raise_server_exceptions=False)
+    resp = client.post(
+        "/mcp",
+        json={
+            "jsonrpc": "2.0",
+            "method": "tools/call",
+            "params": {
+                "name": "t",
+                "arguments": {"items": [1, "ok", {"nested": ["fine"]}, None]},
+            },
+            "id": 1,
+        },
+    )
+    assert resp.status_code != 400
+
+
 def test_arguments_within_key_cap_is_allowed():
     server = _make_server()
     client = TestClient(server.app, raise_server_exceptions=False)
@@ -699,6 +726,91 @@ def test_arguments_exceeding_key_cap_returns_invalid_params():
     assert resp.json()["error"]["code"] == -32602
 
 
+# ── #562: argument string-length cap, matching scripts/mock_upstream.py ─────
+
+def test_string_within_length_cap_is_allowed():
+    server = _make_server()
+    client = TestClient(server.app, raise_server_exceptions=False)
+    resp = client.post(
+        "/mcp",
+        json={
+            "jsonrpc": "2.0",
+            "method": "tools/call",
+            "params": {"name": "t", "arguments": {"text": "a" * 1000}},
+            "id": 1,
+        },
+    )
+    assert resp.status_code == 200
+
+
+def test_string_exceeding_length_cap_returns_invalid_params():
+    server = _make_server()
+    client = TestClient(server.app, raise_server_exceptions=False)
+    resp = client.post(
+        "/mcp",
+        json={
+            "jsonrpc": "2.0",
+            "method": "tools/call",
+            "params": {"name": "t", "arguments": {"text": "a" * (_MAX_ARG_STRING_LENGTH + 1)}},
+            "id": 1,
+        },
+    )
+    assert resp.status_code == 400
+    assert resp.json()["error"]["code"] == -32602
+
+
+def test_oversized_object_key_returns_invalid_params():
+    """A huge key is as expensive as a huge value and is not bounded by the
+    key *count* cap, so it must be rejected on its own."""
+    server = _make_server()
+    client = TestClient(server.app, raise_server_exceptions=False)
+    key = "a" * (_MAX_ARG_STRING_LENGTH + 1)
+    resp = client.post(
+        "/mcp",
+        json={
+            "jsonrpc": "2.0",
+            "method": "tools/call",
+            "params": {"name": "t", "arguments": {key: 1}},
+            "id": 1,
+        },
+    )
+    assert resp.status_code == 400
+    assert resp.json()["error"]["code"] == -32602
+
+
+def test_multibyte_string_length_measured_in_bytes_not_characters():
+    """A 4-byte-per-char string well under the byte cap in character count
+    but over it in UTF-8 bytes must still be rejected."""
+    # U+1F600 (😀) is 4 bytes in UTF-8, 1 codepoint in Python's len(). Choose
+    # a character count that clears the string cap in bytes while staying
+    # well under the server's default max_request_bytes once JSON-encoded.
+    char_count = (_MAX_ARG_STRING_LENGTH // 4) + 1
+    oversized_by_bytes = "\U0001F600" * char_count
+    assert len(oversized_by_bytes.encode("utf-8")) > _MAX_ARG_STRING_LENGTH
+    server = _make_server()
+    client = TestClient(server.app, raise_server_exceptions=False)
+    # Built and encoded by hand with ensure_ascii=False, rather than passed
+    # via the json= kwarg: both json.dumps' default and httpx's json=
+    # encoder escape each emoji to a 12-byte \uXXXX\uXXXX surrogate pair,
+    # inflating the wire size far past the string's actual UTF-8 length and
+    # tripping max_request_bytes instead of the check this test targets.
+    body = json.dumps(
+        {
+            "jsonrpc": "2.0",
+            "method": "tools/call",
+            "params": {"name": "t", "arguments": {"text": oversized_by_bytes}},
+            "id": 1,
+        },
+        ensure_ascii=False,
+    ).encode("utf-8")
+    assert len(body) < 1_000_000
+    resp = client.post(
+        "/mcp", content=body, headers={"Content-Type": "application/json"}
+    )
+    assert resp.status_code == 400
+    assert resp.json()["error"]["code"] == -32602
+
+
 # ── #518: non-standard JSON values (NaN, Infinity, -Infinity) ───────────────
 
 @pytest.mark.parametrize("literal", ["NaN", "Infinity", "-Infinity"])