From 6a9ea0a24617c683fe134d515da437badf555911 Mon Sep 17 00:00:00 2001 From: arpan Date: Fri, 4 Sep 2026 10:27:31 +0530 Subject: [PATCH 1/2] gateway: compare a mirrored header against the body as text, not by re-parsing it SPEC-v0.2 section 6.4 has the gateway validate every mirrored MCP header against the body, because "different components in the network rely on different sources of truth (e.g., a load balancer routing on the header value while the MCP server executes based on the body value)". The comparison did that by running the header through int(), and Python's int() is lenient. So a body carrying amount: 2000 was certified as agreeing with headers spelling it 2_000, +2000, 02000, " 2000", "2000 ", "2000\n", 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("2_000") is an error in Go. A gateway that calls that agreement is telling a load balancer it is safe to route on a header they read as a different number, which is the hazard the section exists to prevent rather than a convenience. CTRLRun's own decisions were never affected. The action is built from the body and the headers are only checked, so the action hash, the approval binding and the effect key were always right. What was wrong is the certificate the gateway hands to everything upstream of it. A header value must now be the body value's canonical rendering, compared as text: a string's own characters, and for everything else exactly what JSON writes. One rendering per value, produced by the serializer the body already went through, so there is no parser in the comparison and therefore no parser to disagree with. This also declines the revision's SHOULD that a server compare integers numerically, under which 42.0 equals 42. A float cannot reach the body at all -- v0.1 section 2.3 refuses it and section 6.6 turns it into -41008 -- so the leniency has no legitimate case here, and taking it would mean accepting a spelling of a number the body could never hold. Section 6.4 now states both rules and why. Mutation table, PYTHONDONTWRITEBYTECODE=1 with __pycache__ cleared between runs: restore int() and the case-insensitive bool -> 11 failed (the 8 integer spellings and the 3 boolean ones), restored -> 13 passed remove the str branch, so strings go through json.dumps and gain quotes -> 2 failed (the existing Mcp-Param-payment_id tests), restored -> 71 passed Neither guard is subsumed and neither test is a negative test against something the environment already refuses: the first mutation shows all eleven spellings were accepted before, and the canonical spellings keep their own passing tests as the control, so an implementation that refused every integer header would fail them. Two existing tests were renamed rather than left saying "numerically", and the docstring claiming that comparing as text "would refuse every well-formed request" is gone -- comparing "2000" against str(2000) was always going to work, which is why the parse was never needed. --- CHANGELOG.md | 21 +++++++++- docs/SPEC-v0.2.md | 21 +++++++++- src/ctrlrun/gateway/mcp.py | 29 +++++++++----- tests/test_mcp.py | 82 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 135 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d31564e0..c86c5c92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,26 @@ 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. 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 diff --git a/docs/SPEC-v0.2.md b/docs/SPEC-v0.2.md index 06892b75..e9e7347a 100644 --- a/docs/SPEC-v0.2.md +++ b/docs/SPEC-v0.2.md @@ -513,8 +513,25 @@ 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: a string's + own characters, and for everything else exactly what JSON writes — `2000`, `true`, `null`. + One rendering per value, and it is the one the body already went through a serializer to get. + + 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 diff --git a/src/ctrlrun/gateway/mcp.py b/src/ctrlrun/gateway/mcp.py index 2306e654..4b27e526 100644 --- a/src/ctrlrun/gateway/mcp.py +++ b/src/ctrlrun/gateway/mcp.py @@ -273,18 +273,25 @@ 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. """ - 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=(",", ":")) diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 09329d28..8a4ac8c2 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -241,22 +241,96 @@ 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) + + +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" From 96e6784b30cc7b50645778c71bd0c242208b703b Mon Sep 17 00:00:00 2001 From: arpan Date: Fri, 4 Sep 2026 10:35:50 +0530 Subject: [PATCH 2/2] gateway: a header can only agree with a string, an integer or a boolean Reviewing the previous commit against the revision's Value Encoding section turned up the same defect one layer down. Having stopped re-parsing the header, the comparison still fell through to json.dumps for every remaining type -- so a body carrying extra: null agreed with a header spelling it null, and one carrying a list agreed with the compact JSON rendering of that list. The revision defines encodings for exactly three types: a string as-is, an integer as a decimal string, a boolean as lowercase true or false. It permits x-mcp-header on those three alone, and a null parameter omits its header entirely. So there is no conforming header value for a list, a mapping or a null, and comparing against one CTRLRun made up is the same thing the previous commit removed: certifying an agreement that holds under nobody's rules but ours. Anything that is not a string or an int is now False. Mutation: restore the json.dumps fall-through -> 3 failed (null, list, object), restored -> 3 passed. The list and object cases needed the test tightening first -- written with a plain json.dumps they passed against the old code for an incidental reason, the default separator spacing, rather than on the rule. A float is deliberately not in the parametrization: SPEC-v0.2 section 6.6 refuses one in the body with -41008 before this comparison is reached, so a case for it here would be a negative test against something the environment already prevents. --- CHANGELOG.md | 5 ++++- docs/SPEC-v0.2.md | 11 ++++++++--- src/ctrlrun/gateway/mcp.py | 9 ++++++++- tests/test_mcp.py | 26 ++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c86c5c92..d105951f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,10 @@ any change to one appears here. 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. This also declines the revision's SHOULD that servers compare + 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. diff --git a/docs/SPEC-v0.2.md b/docs/SPEC-v0.2.md index e9e7347a..5d0dca0f 100644 --- a/docs/SPEC-v0.2.md +++ b/docs/SPEC-v0.2.md @@ -515,9 +515,14 @@ for. So: `MCP-Protocol-Version`, `Mcp-Method`, `Mcp-Name` and every `Mcp-Param-{Name}` against it, 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: a string's - own characters, and for everything else exactly what JSON writes — `2000`, `true`, `null`. - One rendering per value, and it is the one the body already went through a serializer to get. +- **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 diff --git a/src/ctrlrun/gateway/mcp.py b/src/ctrlrun/gateway/mcp.py index 4b27e526..e218f7d3 100644 --- a/src/ctrlrun/gateway/mcp.py +++ b/src/ctrlrun/gateway/mcp.py @@ -291,10 +291,17 @@ def _agrees(declared: str, value: object) -> bool: 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, 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: diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 8a4ac8c2..0d80da8c 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -321,6 +321,32 @@ def test_the_canonical_boolean_spelling_is_still_accepted(): 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