gateway: compare a mirrored header against the body as text, not by re-parsing it

CHANGELOG.md

@@ -9,7 +9,29 @@ any change to one appears here.
 
 ## [Unreleased]
 
-Nothing yet.
+### Fixed
+
+- **The gateway compared mirrored header values by re-parsing them, and Python's parser is
+  lenient.** `Mcp-Param-{Name}` validation ran the header through `int()`, so a body carrying
+  `amount: 2000` was certified as agreeing with headers spelling it `2_000`, `+2000`, `02000`,
+  ` 2000`, `2000 `, and the Arabic-indic and fullwidth digit forms; booleans were matched
+  case-insensitively, so `TRUE`, `True` and `tRuE` all agreed with `true`. None of those is
+  what a JSON serializer writes, and each is read differently — or refused — by another
+  parser: `parseInt("2_000")` is 2 in JavaScript and `strconv.Atoi` errors in Go.
+
+  That is the exact hazard SPEC-v0.2 §6.4 exists to prevent. The gateway's job there is to
+  certify that a routing intermediary and CTRLRun are looking at the same value, and it was
+  certifying agreement that held only under Python's rules. CTRLRun's own decisions were never
+  affected — the action is built from the body, and the headers are only checked — so this
+  costs an intermediary's correctness rather than an approval binding.
+
+  A header value must now be the body value's canonical rendering, compared as text, with no
+  parser in the comparison — and only a string, an integer or a boolean has one. The revision
+  permits `x-mcp-header` on those three types alone and omits the header for a `null`, so a
+  header naming an argument of any other type is refused rather than compared against a
+  rendering CTRLRun invented for it. This also declines the revision's SHOULD that servers compare
+  integers numerically (`42.0` equals `42`): v0.1 §2.3 refuses a float in the body outright, so
+  the leniency has no legitimate case here. SPEC-v0.2 §6.4 states both rules and the reasoning.
 
 ## [0.2.0] — unreleased
 

docs/SPEC-v0.2.md

@@ -513,8 +513,30 @@ for. So:
 
 - The gateway MUST parse the body of every request it forwards and MUST validate
   `MCP-Protocol-Version`, `Mcp-Method`, `Mcp-Name` and every `Mcp-Param-{Name}` against it,
-  decoding the `=?base64?…?=` sentinel before comparing, and comparing integers numerically.
-  A mismatch, or a missing required header, is HTTP 400 with `-32020`.
+  decoding the `=?base64?…?=` sentinel before comparing. A mismatch, or a missing required
+  header, is HTTP 400 with `-32020`.
+- **A header value MUST be the body value's canonical rendering**, compared as text, and only
+  three types have one. The revision's Value Encoding is exact: *"`string`: Use the value
+  as-is"*, *"`integer`: Convert to decimal string representation (e.g., `42`, `-7`)"*,
+  *"`boolean`: Convert to lowercase `"true"` or `"false"`"* — and `x-mcp-header` *"**MUST**
+  only be applied to parameters with primitive types (integer, string, boolean)"*, with a
+  `null` parameter omitting its header entirely. A `Mcp-Param-{Name}` naming an argument of any
+  other type — a list, a mapping, a `null` — therefore cannot agree with it, and is refused
+  rather than compared against a rendering CTRLRun invented for it.
+
+  This is stricter than the revision's *"servers SHOULD compare the header value and the body
+  value numerically rather than as strings (e.g., `42.0` and `42` are considered equal)"*, and
+  the gateway **declines that SHOULD**, for two reasons. A float cannot appear in the body at
+  all — v0.1 §2.3 refuses it and §6.6 turns it into `-41008` — so the leniency has no
+  legitimate case here; taking it would mean accepting a spelling of a number the body could
+  never hold. And re-parsing is what creates the hazard this section is about: `int("2_000")`
+  is 2000 in Python, `parseInt("2_000")` is 2 in JavaScript, and `strconv.Atoi("2_000")` is an
+  error in Go, so a gateway that certified `Mcp-Param-Amount: 2_000` as agreeing with a body
+  carrying `2000` would be telling a load balancer that a header they read differently is
+  safe to route on. `+2000`, `02000`, leading or trailing whitespace, and the Arabic-indic and
+  fullwidth spellings of the digits are the same defect wearing other hats; so is matching
+  `true` case-insensitively. The comparison has no parser in it, so it has no parser to
+  disagree with.
 - The gateway MUST decide what to intercept from the **body's** `method` and `params.name`,
   never from the headers. The headers are checked; the body is believed.
 - The gateway MUST NOT rely on the upstream to perform this validation. A `-32020` arriving

src/ctrlrun/gateway/mcp.py

@@ -273,21 +273,35 @@ def encode_header_value(value: str) -> str:
 
 
 def _agrees(declared: str, value: object) -> bool:
-    """Whether a header value agrees with the body's, comparing integers numerically (§6.4).
-
-    `2000` in a header is text and the body's is an `int`; comparing them as strings would
-    refuse every well-formed request that annotated a numeric parameter.
+    """Whether a header value is the body value's canonical rendering (SPEC-v0.2 §6.4).
+
+    A string's header carries its bare text; everything else must be exactly what JSON writes
+    for it — `2000`, `true`, `null`. One rendering per value, produced by the serializer the
+    body already went through, so there is nothing to re-parse and nothing to be lenient about.
+
+    This used to call `int(declared)`, which reads `2_000`, `+2000`, `02000`, ` 2000`, and the
+    Arabic-indic and fullwidth spellings of 2000 as agreeing with a body carrying `2000`. None
+    of them is what a JSON serializer writes, and each is read differently — or not at all — by
+    another parser: `parseInt("2_000")` is 2 in JavaScript and `strconv.Atoi` refuses it in Go.
+    §6.4 exists because "different components in the network rely on different sources of
+    truth", so certifying agreement that only holds under Python's rules is the hazard rather
+    than a convenience. Booleans were compared case-insensitively, with the same consequence.
+
+    It also declines the revision's SHOULD that a server compare integers numerically, under
+    which `2000.0` equals `2000`. v0.1 §2.3 refuses a float in the body outright (§6.6), so the
+    leniency has no legitimate case here; taking it would mean accepting a spelling of a number
+    that the body could never hold.
+
+    Anything that is not a string, an integer or a boolean has **no** header encoding at all:
+    the revision permits `x-mcp-header` only on those three, and says a `null` parameter omits
+    its header entirely. So a header naming an argument of any other type cannot agree with it,
+    and the answer is `False` rather than a comparison against a rendering CTRLRun made up.
     """
-    if isinstance(value, bool):
-        return declared.lower() == str(value).lower()
-    if isinstance(value, int):
-        try:
-            return int(declared) == value
-        except ValueError:
-            return False
     if isinstance(value, str):
         return declared == value
-    return declared == json.dumps(value, separators=(",", ":"))
+    if isinstance(value, int):  # bool included, and JSON writes it as `true` / `false`
+        return declared == json.dumps(value)
+    return False
 
 
 def find_unrepresentable(arguments: object, pointer: str = "") -> str | None:

tests/test_mcp.py

@@ -241,22 +241,122 @@ def test_an_Mcp_Param_agreeing_with_the_body_is_accepted():
     assert not isinstance(parse_request(_body(), headers), Refusal)
 
 
-def test_an_Mcp_Param_integer_is_compared_numerically():
-    """§6.4 — `2000` in a header is a string; the body's is an int. Comparing them as text
-    would refuse every well-formed request that annotated a numeric parameter."""
+def test_an_Mcp_Param_integer_agrees_with_its_canonical_spelling():
+    """§6.4 — `2000` in a header is text and the body's is an `int`, so the comparison is
+    against what JSON writes for it. This is also the control for the parametrization below:
+    without it, an implementation that refused every integer header would pass those."""
     headers = _headers()
     headers["Mcp-Param-amount"] = "2000"
 
     assert not isinstance(parse_request(_body(), headers), Refusal)
 
 
-def test_an_Mcp_Param_integer_that_differs_numerically_is_refused():
+def test_an_Mcp_Param_integer_that_differs_is_refused():
     headers = _headers()
     headers["Mcp-Param-amount"] = "2001"
 
     assert isinstance(parse_request(_body(), headers), Refusal)
 
 
+#: Spellings that Python's `int()` accepts and that no JSON serializer produces. Each is a
+#: header whose text differs from the body's value, so an intermediary that parses it with a
+#: different set of rules reads a different number — the split-brain §6.4 exists to prevent.
+#: JavaScript's `parseInt("4_2")` is 4, Go's `strconv.Atoi("4_2")` is an error, and Python's
+#: `int("4_2")` is 42.
+_LENIENT_INTEGER_SPELLINGS = [
+    "2_000",  # PEP 515 underscore; JS parseInt reads 2
+    "+2000",  # leading plus
+    " 2000",  # leading whitespace
+    "2000 ",  # trailing whitespace
+    "2000\n",  # trailing newline
+    "02000",  # leading zero
+    "\u0662\u0660\u0660\u0660",  # arabic-indic digits for 2000
+    "\uff12\uff10\uff10\uff10",  # fullwidth digits for 2000
+]
+
+
+@pytest.mark.parametrize("declared", _LENIENT_INTEGER_SPELLINGS)
+def test_an_Mcp_Param_integer_must_be_the_body_value_as_text(declared):
+    """§6.4 — the header must be the body value's canonical rendering, not merely something
+    Python's `int()` maps onto it. Each of these parses to 2000 under `int()` and to something
+    else, or to nothing, under another parser."""
+    headers = _headers()
+    headers["Mcp-Param-amount"] = declared
+
+    refusal = parse_request(_body(), headers)
+
+    assert isinstance(refusal, Refusal), f"{declared!r} was accepted"
+    assert (refusal.http_status, refusal.code) == (400, -32020)
+
+
+@pytest.mark.parametrize("declared", ["TRUE", "True", "tRuE", "1", "yes"])
+def test_an_Mcp_Param_boolean_must_be_its_json_spelling(declared):
+    """§6.4 — `true` and `false`, as JSON writes them. A case-insensitive match accepts four
+    spellings of one value, and an intermediary comparing bytes sees four different headers."""
+    document = {
+        "jsonrpc": "2.0",
+        "id": 1,
+        "method": "tools/call",
+        "params": {"name": "create_refund", "arguments": {"partial": True}},
+    }
+    headers = _headers()
+    headers["Mcp-Param-partial"] = declared
+
+    refusal = parse_request(_body(document), headers)
+
+    assert isinstance(refusal, Refusal), f"{declared!r} was accepted"
+    assert (refusal.http_status, refusal.code) == (400, -32020)
+
+
+def test_the_canonical_boolean_spelling_is_still_accepted():
+    document = {
+        "jsonrpc": "2.0",
+        "id": 1,
+        "method": "tools/call",
+        "params": {"name": "create_refund", "arguments": {"partial": True}},
+    }
+    headers = _headers()
+    headers["Mcp-Param-partial"] = "true"
+
+    assert not isinstance(parse_request(_body(document), headers), Refusal)
+
+
+@pytest.mark.parametrize("value", [None, [1, 2], {"a": 1}], ids=["null", "list", "object"])
+def test_an_Mcp_Param_naming_a_non_primitive_argument_is_refused(value):
+    """§6.4 — the revision defines an encoding for exactly three types: string, integer and
+    boolean. It permits `x-mcp-header` on nothing else, and a `null` parameter omits the header
+    entirely. So no header value can agree with any of these, and comparing against a rendering
+    CTRLRun invented would certify an agreement under nobody's rules but its own.
+
+    The header carries the compact rendering the old code compared against, so each case fails
+    on the rule rather than on a stray space. A float is not here: §6.6 refuses one in the body
+    outright with `-41008`, so it never reaches this comparison.
+    """
+    document = {
+        "jsonrpc": "2.0",
+        "id": 1,
+        "method": "tools/call",
+        "params": {"name": "create_refund", "arguments": {"extra": value}},
+    }
+    headers = _headers()
+    headers["Mcp-Param-extra"] = json.dumps(value, separators=(",", ":"))
+
+    refusal = parse_request(_body(document), headers)
+
+    assert isinstance(refusal, Refusal), f"{value!r} was accepted"
+    assert (refusal.http_status, refusal.code) == (400, -32020)
+
+
+def test_a_float_spelling_of_an_integer_is_refused():
+    """§6.4 declines the revision's numeric-leniency SHOULD (`42.0` equals `42`). v0.1 §2.3
+    refuses a float in the body outright, so the leniency has no legitimate case here, and
+    honouring it would mean accepting a header spelling other parsers read differently."""
+    headers = _headers()
+    headers["Mcp-Param-amount"] = "2000.0"
+
+    assert isinstance(parse_request(_body(), headers), Refusal)
+
+
 def test_an_Mcp_Param_naming_an_argument_the_body_does_not_have_is_refused():
     headers = _headers()
     headers["Mcp-Param-nothing"] = "x"