From 554855fb77c6693dcd189c05085a1ba8007a33c7 Mon Sep 17 00:00:00 2001 From: Imran Siddique Date: Mon, 10 Aug 2026 21:43:04 -0700 Subject: [PATCH] fix(cose): bound payload nesting explicitly, not via RecursionError main has been red since #274 merged. Two deeply-nested-payload tests fail on ubuntu 3.12 and 3.13 while passing on 3.11 and on Windows, and the cause is not a flaky test: the DOS-006 guard did not exist on Linux. _parse_payload relied on json.loads raising RecursionError to refuse a deeply nested payload. CPython on Linux parses thousands of levels without raising, so the 5000-level payload simply parsed, _check_version then raised CoseVersionError("unsupported manifest version None"), and the test expecting CoseStructureError failed. Windows tripped its own recursion limit and therefore looked protected. A control whose behaviour depends on which platform it runs on is not a control. Replaced with an explicit bound: _MAX_PAYLOAD_NESTING = 64, checked by a string-aware scan of the decoded text before json.loads rather than after. Before, because refusing the work after paying for it defeats the purpose; string-aware, because a brace inside a member value must not inflate the count and make a legitimate manifest look like an attack. The RecursionError catch stays as a second line. The new test proves the bound rather than the accident: it refuses a 65-level payload, which is far below any interpreter's recursion limit and therefore cannot be failing for the old reason. 64 is generous - the deepest path this spec defines is about six levels. Found while cutting python-v0.11.0: the release PR's CI failed, and the failures predated it. Co-Authored-By: Claude Opus 5 (1M context) --- python/src/agent_manifest/_cose.py | 64 +++++++++++++++++++++++++++++- python/tests/test_cose.py | 38 ++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/python/src/agent_manifest/_cose.py b/python/src/agent_manifest/_cose.py index dbf1451..67b9064 100644 --- a/python/src/agent_manifest/_cose.py +++ b/python/src/agent_manifest/_cose.py @@ -668,14 +668,74 @@ def _reject_non_json_constant(token: str) -> Any: ) +#: Maximum nesting depth accepted in a COSE payload (DOS-006). +#: +#: A manifest is a shallow document: the deepest path this specification defines +#: is roughly ``artifacts.tool_manifest.tools[].approved_scope``, well under ten. +#: Sixty-four leaves generous headroom for a future revision while staying far +#: below any interpreter's recursion limit. +_MAX_PAYLOAD_NESTING = 64 + + +def _payload_nesting_depth(text: str) -> int: + """Maximum ``{``/``[`` nesting depth in *text*, ignoring string contents. + + Scanned rather than measured after parsing, because the point is to refuse + the work before doing it: a depth check that runs after ``json.loads`` has + already paid for the structure it was supposed to prevent. + + String-aware, so a brace inside a member value cannot inflate the count and + make a legitimate manifest look like an attack. Backslash escapes are skipped + so ``"\\\\"`` does not swallow the closing quote. + """ + depth = 0 + deepest = 0 + in_string = False + escaped = False + for char in text: + if in_string: + if escaped: + escaped = False + elif char == "\\": + escaped = True + elif char == '"': + in_string = False + continue + if char == '"': + in_string = True + elif char in "{[": + depth += 1 + deepest = max(deepest, depth) + elif char in "}]": + depth -= 1 + return deepest + + def _parse_payload(payload: bytes) -> dict[str, Any]: + try: + text = payload.decode("utf-8") + except UnicodeDecodeError as exc: + raise CoseStructureError(f"payload is not valid JSON: {exc}") from exc + + # Before json.loads, not after. Relying on RecursionError alone made this + # guard platform-dependent: CPython on Linux parses thousands of levels + # without raising, so on that platform there was no bound at all, while + # Windows tripped its own recursion limit and appeared to be protected. An + # explicit bound behaves identically everywhere. + depth = _payload_nesting_depth(text) + if depth > _MAX_PAYLOAD_NESTING: + raise CoseStructureError( + f"payload is nested {depth} levels deep, above the " + f"{_MAX_PAYLOAD_NESTING}-level limit" + ) + try: manifest = json.loads( - payload.decode("utf-8"), + text, object_pairs_hook=_reject_duplicate_keys, parse_constant=_reject_non_json_constant, ) - except (UnicodeDecodeError, json.JSONDecodeError) as exc: + except json.JSONDecodeError as exc: raise CoseStructureError(f"payload is not valid JSON: {exc}") from exc except RecursionError as exc: # A manifest is untrusted input, so nesting must produce a verdict diff --git a/python/tests/test_cose.py b/python/tests/test_cose.py index c8f3c44..22c1484 100644 --- a/python/tests/test_cose.py +++ b/python/tests/test_cose.py @@ -1318,3 +1318,41 @@ def test_a_v01_manifest_with_the_v01_envelope_is_unaffected(): assert verify_manifest(manifest, base_context(), store()).result == ( OverallResult.VALID ) + + +def test_nesting_bound_is_explicit_not_a_recursion_accident(): + """DOS-006 must behave identically on every platform. + + Relying on RecursionError made this guard platform-dependent: CPython on + Linux parses thousands of levels without raising, so on that platform there + was no bound at all, while Windows tripped its own recursion limit and + looked protected. Main went red on ubuntu 3.12/3.13 for exactly this. + """ + from agent_manifest._cose import _MAX_PAYLOAD_NESTING, _parse_payload + + at_limit = ('{"a":' * (_MAX_PAYLOAD_NESTING - 1)) + '{"version":"0.2"}' + ("}" * (_MAX_PAYLOAD_NESTING - 1)) + assert _parse_payload(at_limit.encode()) is not None + + too_deep = ('{"a":' * (_MAX_PAYLOAD_NESTING + 1)) + "1" + ("}" * (_MAX_PAYLOAD_NESTING + 1)) + with pytest.raises(CoseStructureError, match="levels deep"): + _parse_payload(too_deep.encode()) + + +def test_nesting_scan_ignores_braces_inside_strings(): + """A brace in a member value must not be counted, or a legitimate manifest + with JSON-ish text in a field would be refused as an attack.""" + from agent_manifest._cose import _payload_nesting_depth + + assert _payload_nesting_depth('{"a": "{{{{{{"}') == 1 + assert _payload_nesting_depth('{"a": "\\"} {{{"}') == 1 + assert _payload_nesting_depth('{"a": {"b": [1, 2]}}') == 3 + assert _payload_nesting_depth("{}") == 1 + + +def test_a_deeply_nested_payload_is_refused_before_it_is_parsed(): + """The refusal must come from the bound, not from whatever json.loads does + with 5000 levels on this particular platform.""" + from agent_manifest._cose import _parse_payload + + with pytest.raises(CoseStructureError, match="levels deep"): + _parse_payload((('{"a":' * 5000) + "1" + ("}" * 5000)).encode())