diff --git a/docs/CLAIMS.md b/docs/CLAIMS.md index deba7e03..aa17d4e6 100644 --- a/docs/CLAIMS.md +++ b/docs/CLAIMS.md @@ -53,7 +53,7 @@ catalogue, `GUARANTEES` (`verify/guarantees.py:39`). | "An action the policy does not list is denied" | `Policy.evaluate` — `policy.py:678` | `test_T6_unknown_action_is_denied_with_reason_unknown_action` | | "Authority first ... then policy" / "authority first" | `Control.execute` evaluates authority before policy and a denial appends `AUTHORITY_DENIED` and never `POLICY_EVALUATED` — `control.py:542` | `test_T74_a_denial_leaves_no_pending_approval_request` | | "Neither axis reads the agent's instructions" | `Policy.evaluate` — `policy.py:678` — sees the action's name and arguments; `Authority.evaluate` — `authority.py:807` — sees the action and the principal; neither is handed a prompt, a message or a tool result | `test_T6_an_action_name_is_matched_exactly`, `test_T67_a_principal_with_no_grant_is_denied` | -| "canonical arguments (sorted keys, no floats) ... Its SHA-256 is the action hash" | `canonicalize` / `action_hash` — `action.py`; `float` refused at any depth — `action.py:47` | `test_T7_canonical_form_is_exactly_the_specified_serialization`, `test_T7_nested_dicts_are_sorted_recursively` | +| "canonical arguments (sorted keys, no floats) ... Its SHA-256 is the action hash" | `canonicalize` / `action_hash` — `action.py`; `float` refused at any depth — `action.py:71` | `test_T7_canonical_form_is_exactly_the_specified_serialization`, `test_T7_nested_dicts_are_sorted_recursively` | | "The approval is single-use, expires, and matches nothing but that exact action." | `_authorize_and_reserve` — `state.py:780` — checks expiry at consumption | `test_T5_expiry_is_checked_at_consumption_not_only_at_grant`, `test_T4_replaying_the_approval_raises_ApprovalMismatch_with_reason_consumed` | | "Only `NotExecuted`, raised by you, means `FAILED`." | `control.py:1036`; `NotExecuted` — `errors.py:157` | `test_T1_a_lost_response_leaves_the_effect_ambiguous` | | "the hash of the policy that decided it, chained to the receipt before it" | `Policy.policy_hash` — `policy.py:559`; `prev_hash`, `GENESIS_HASH` for the first — `receipt.py:284` | `test_T172_every_receipt_carries_the_hash_and_the_declared_version`, `test_T164_an_altered_receipt_is_content_altered_at_its_seq` | @@ -84,7 +84,7 @@ catalogue, `GUARANTEES` (`verify/guarantees.py:39`). |---|---|---| | "cheap to undo is autonomous, anything that leaves the building needs a human, money is by amount with both ends bound" | `Decision` — `policy.py:234` — is exactly `allow`, `approve`, `deny`; rules match first-wins over `Condition` (`policy.py:298`) with the operators `eq`, `neq`, `in`, `lt`, `lte`, `gt`, `gte` — `_OPERATORS` — `policy.py:83` | `test_T6_an_action_name_is_matched_exactly`, `test_T176_the_operators_behave_as_they_do_everywhere_else` | | "Unknown actions are denied; there is no default-allow." | `Policy.evaluate` — `policy.py:678` | `test_T6_unknown_action_is_denied_with_reason_unknown_action` | -| "Amounts are integer minor units; floats are rejected outright" | `float` refused at any depth — `action.py:47` | `test_T7_canonical_form_is_exactly_the_specified_serialization` | +| "Amounts are integer minor units; floats are rejected outright" | `float` refused at any depth — `action.py:71` | `test_T7_canonical_form_is_exactly_the_specified_serialization` | | "The policy cannot see who is asking — deliberately, since v0.1" | `Policy.evaluate` still takes only the action's name and arguments; `RESERVED_ARGUMENTS` — `policy.py:150` — refuses `agent_eq` and every other principal-addressing condition at load, in a document of **every** schema version | `test_T74b_a_reserved_name_in_a_policy_rule_is_a_load_error`, `test_T74b_a_reserved_name_in_a_grant_constraint_is_a_load_error` | | "the second axis, `authority:`" | `Authority.evaluate` — `authority.py:807`; `Control._authority_result` — `control.py:408` | `test_T67_a_principal_with_no_grant_is_denied` | | "opt-in, and then fail-closed" | `_optional_authority` returns `None` for a document with no section — `control.py`; `Control.authority is None` is v0.2 behaviour exactly | `test_T66_a_document_with_no_authority_section_leaves_control_authority_none`, `test_T66_no_authority_event_is_appended_without_a_section`, and T66's session-wide guard in `tests/conftest.py` | @@ -189,7 +189,7 @@ restating the code; the ones that are new to the site carry their own code and p | `get-started/install` | "importing `ctrlrun` imports nothing from an extra" | `test_T30_a_subprocess_importing_ctrlrun_pulls_in_no_module_from_an_extra` | | `get-started/install` | "raises `MissingDependency` with the install command in the message" | `test_a_missing_extra_raises_MissingDependency_naming_the_install_command` | | `get-started/quickstart` | every block on the page, and the outputs shown | the blocks are `runnable` and pass `tools/docs_audit/snippets.py` in one temporary directory, in order; the outputs are pasted from one run of the same blocks | -| `concepts/action-and-hash` | "The action hash is the SHA-256 of that canonical form"; sorted keys, no whitespace, UTF-8, `float` rejected; `action_id` excluded | `canonicalize` / `action_hash` — `action.py`; `float` refused — `action.py:47`; `test_T7_canonical_form_is_exactly_the_specified_serialization`, `test_T7_nested_dicts_are_sorted_recursively`, `test_T60_claims_do_not_change_the_action_hash` | +| `concepts/action-and-hash` | "The action hash is the SHA-256 of that canonical form"; sorted keys, no whitespace, UTF-8, `float` rejected; `action_id` excluded | `canonicalize` / `action_hash` — `action.py`; `float` refused — `action.py:71`; `test_T7_canonical_form_is_exactly_the_specified_serialization`, `test_T7_nested_dicts_are_sorted_recursively`, `test_T60_claims_do_not_change_the_action_hash` | | `concepts/decisions` | three decisions, first match wins, unknown denied, principal-addressing conditions refused at load | the "Write down what the agent may do" rows above | | `concepts/approval-binding` | A1–A4, the mismatch leaving the approval granted, one core provider writing every grant | the matrix row "An approval is bound to the exact action…", the "Three ways to use it" adapter rows, and `test_T2_a_mutated_action_leaves_the_approval_granted` | | `concepts/approval-binding` | the `DENY` and `ALLOW` rows when the policy changed between grant and consumption | "the approval is re-checked against the policy in force at execution" above | diff --git a/docs/reference/api/Action.mdx b/docs/reference/api/Action.mdx index 49b76235..11f38106 100644 --- a/docs/reference/api/Action.mdx +++ b/docs/reference/api/Action.mdx @@ -5,7 +5,7 @@ description: "A proposed agent action: what, with which arguments, by whom, on w {/* generated by tools/docs_audit/render_api.py from the docstrings — edit the docstring, never this page */} -`ctrlrun.Action` — class, defined at `src/ctrlrun/action.py:151` +`ctrlrun.Action` — class, defined at `src/ctrlrun/action.py:178` ```python from ctrlrun import Action diff --git a/docs/reference/api/Principal.mdx b/docs/reference/api/Principal.mdx index 3fe2a10b..d7fe8528 100644 --- a/docs/reference/api/Principal.mdx +++ b/docs/reference/api/Principal.mdx @@ -5,7 +5,7 @@ description: "Who is acting: an agent, optionally on behalf of a human." {/* generated by tools/docs_audit/render_api.py from the docstrings — edit the docstring, never this page */} -`ctrlrun.Principal` — class, defined at `src/ctrlrun/action.py:112` +`ctrlrun.Principal` — class, defined at `src/ctrlrun/action.py:139` ```python from ctrlrun import Principal diff --git a/docs/reference/api/action_hash.mdx b/docs/reference/api/action_hash.mdx index 8dd0124c..65d62390 100644 --- a/docs/reference/api/action_hash.mdx +++ b/docs/reference/api/action_hash.mdx @@ -5,7 +5,7 @@ description: "Return the action hash used to bind approvals to an exact action ( {/* generated by tools/docs_audit/render_api.py from the docstrings — edit the docstring, never this page */} -`ctrlrun.action_hash` — function, defined at `src/ctrlrun/action.py:283` +`ctrlrun.action_hash` — function, defined at `src/ctrlrun/action.py:327` ```python from ctrlrun import action_hash diff --git a/docs/reference/api/canonical_bytes.mdx b/docs/reference/api/canonical_bytes.mdx index bb0fced9..abe195ac 100644 --- a/docs/reference/api/canonical_bytes.mdx +++ b/docs/reference/api/canonical_bytes.mdx @@ -5,7 +5,7 @@ description: "The canonical form of an arbitrary mapping: UTF-8 JSON, sorted key {/* generated by tools/docs_audit/render_api.py from the docstrings — edit the docstring, never this page */} -`ctrlrun.canonical_bytes` — function, defined at `src/ctrlrun/action.py:201` +`ctrlrun.canonical_bytes` — function, defined at `src/ctrlrun/action.py:228` ```python from ctrlrun import canonical_bytes diff --git a/docs/reference/api/canonicalize.mdx b/docs/reference/api/canonicalize.mdx index cd248219..92147712 100644 --- a/docs/reference/api/canonicalize.mdx +++ b/docs/reference/api/canonicalize.mdx @@ -5,7 +5,7 @@ description: "Return the canonical form of an Action: UTF-8 JSON, sorted keys, n {/* generated by tools/docs_audit/render_api.py from the docstrings — edit the docstring, never this page */} -`ctrlrun.canonicalize` — function, defined at `src/ctrlrun/action.py:264` +`ctrlrun.canonicalize` — function, defined at `src/ctrlrun/action.py:308` ```python from ctrlrun import canonicalize diff --git a/fuzz/README.md b/fuzz/README.md index 028356c9..0788f569 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -70,23 +70,34 @@ and requires the check to notice. A property that cannot fail proves nothing. are recorded rather than worked around in the decoder, because a fuzzer whose corpus is pruned to avoid its own findings reports zero forever. -`test_the_known_findings_still_reproduce` asserts each one **still happens**. The day it is -fixed that test goes red and the entry must be deleted — which is the point. A recorded limit -that quietly starts passing is the failure mode this directory is about. +`test_the_seed_corpus_reproduces_every_known_finding` compares the dict against what the corpus +actually produces, in **both** directions: an entry cannot be added without a reproducer, and a +reproducer cannot quietly stop reproducing. -### `lone-surrogate-in-a-string` +**It is currently empty, and that is a result rather than a default.** -`canonical_bytes` raises `UnicodeEncodeError`, not `InvalidArgument`, for a string holding an -unpaired UTF-16 surrogate. +### `lone-surrogate-in-a-string` — found here, fixed in `action.py` -Reachable: `json.loads('"\ud800"')` produces one, so an MCP tool call can carry it into the -action path. `Action(...)` accepts it and `action_hash` is where it fails. +`canonical_bytes` raised `UnicodeEncodeError`, not `InvalidArgument`, for a string holding an +unpaired UTF-16 surrogate. Reachable the ordinary way: `json.loads` produces one from a +six-character escape, so an MCP tool call carried it into the action path; `Action(...)` +accepted it and `action_hash` was where it failed. -Fail-closed **holds** — both the gateway and `Control` wrap the action path in -`except Exception` — so this is a contract violation and a crash, not an authorization bypass. -What it breaks is the closed error set in `errors.py` and `InvalidArgument`'s documented -promise: a caller catching `CTRLRunError` does not catch this. +Fail-closed **held** — the gateway and `Control` both wrap the action path in +`except Exception` — so it was a contract violation and a crash, not an authorization bypass. +What it broke was the closed error set in `errors.py`: a caller catching `CTRLRunError` did not +catch it. -The fix is narrow and belongs to whoever owns `src/`: reject unencodable strings in -`_no_floats` alongside the float and non-string-key checks. It changes no hash that previously -succeeded, so `v0.1 §2.3`'s "old hashes still verify" rule is satisfied without a schema bump. +`_encodable` in `action.py` now refuses it at construction, beside the `float` and +non-string-key checks, and `canonical_bytes` refuses it again at the encode. A refusal and not +a repair: `errors="replace"` would map two distinct arguments onto one canonical form, which is +the collision §2.3 exists to prevent. A *paired* surrogate is unaffected, and a negative control +asserts that, because a check keyed on "contains a surrogate code point" rather than on +encodability would reject an ordinary emoji and pass every other test here. + +No hash that previously succeeded changes, so §2.3's "old hashes still verify" rule holds +without a schema bump. + +**The entry did not have to be noticed and removed by hand.** Its own test asserted the finding +*still reproduced*, so fixing the bug turned that test red — which is what forced the entry out +and the reproducer in `corpus/canonical/seed-10-surrogate` into a regression input. diff --git a/fuzz/properties.py b/fuzz/properties.py index 1f408dd7..1e18249c 100644 --- a/fuzz/properties.py +++ b/fuzz/properties.py @@ -37,32 +37,20 @@ def policy_from_yaml(text: str, *, source: str = "") -> Policy: # --- known findings --------------------------------------------------------------------------- -#: A finding that is real, reported, and not yet fixed. It is recorded here rather than worked -#: around in the decoder, because a fuzzer whose corpus is pruned to avoid its own findings is -#: a fuzzer that reports zero forever. +#: Findings that are real, reported, and not yet fixed -- recorded here rather than worked +#: around in the decoder, because a fuzzer whose corpus is pruned to avoid its own findings +#: reports zero forever. #: -#: `test_the_known_findings_still_reproduce` asserts each of these **still happens**. The day -#: one is fixed that test goes red and the entry must be deleted -- which is the point. A -#: documented limit that quietly starts passing is the failure mode this whole file is about. -KNOWN_FINDINGS: dict[str, str] = { - "lone-surrogate-in-a-string": ( - "canonical_bytes raises UnicodeEncodeError, not InvalidArgument, for a string holding " - "an unpaired UTF-16 surrogate. Reachable: json.loads('\"\\\\ud800\"') produces one, so " - "an MCP tool call can carry it. Fail-closed holds -- the gateway and Control both wrap " - "the action path in `except Exception` -- but the error escapes the closed set in " - "errors.py and violates InvalidArgument's documented contract." - ), -} - - -def _has_surrogate(value: object) -> bool: - if isinstance(value, str): - return any(0xD800 <= ord(char) <= 0xDFFF for char in value) - if isinstance(value, dict): - return any(_has_surrogate(k) or _has_surrogate(v) for k, v in value.items()) - if isinstance(value, list | tuple): - return any(_has_surrogate(item) for item in value) - return False +#: **Empty, and the machinery stays.** It held `lone-surrogate-in-a-string` -- +#: `canonical_bytes` raising `UnicodeEncodeError` instead of `InvalidArgument` for an unpaired +#: UTF-16 surrogate -- until that was fixed in `action.py`. The entry's own test asserted the +#: finding *still reproduced*, so the fix turned it red and the entry had to go. That is the +#: mechanism working, and it is why the dict is left in place for the next one. +#: +#: `test_the_seed_corpus_reproduces_every_known_finding` compares this to what the corpus +#: actually produces, in both directions, so an entry cannot be added without a reproducer and +#: a reproducer cannot start failing unnoticed. +KNOWN_FINDINGS: dict[str, str] = {} # --- decoders --------------------------------------------------------------------------------- @@ -211,10 +199,14 @@ def check_canonical(document: dict[Any, Any]) -> str | None: first = _encode(document, "the first pass") except InvalidArgument: return None # a refusal in words is the contract working - except UnicodeEncodeError: - if _has_surrogate(document): - return "lone-surrogate-in-a-string" - raise AssertionError("UnicodeEncodeError with no surrogate in the input") from None + except UnicodeEncodeError as exc: + # Was `KNOWN_FINDINGS["lone-surrogate-in-a-string"]`. `action.py` refuses an + # unencodable string with `InvalidArgument` now, so this escaping again is a + # regression and not a recorded limit. + raise AssertionError( + f"canonicalization raised UnicodeEncodeError, which is outside the closed error " + f"set in errors.py: {exc}" + ) from exc assert not must_refuse, "canonicalization accepted a document holding " + " and ".join( must_refuse diff --git a/src/ctrlrun/action.py b/src/ctrlrun/action.py index 1d2cdadd..dc57cf6f 100644 --- a/src/ctrlrun/action.py +++ b/src/ctrlrun/action.py @@ -42,6 +42,30 @@ def _new_action_id() -> str: return f"act_{secrets.token_hex(_ID_HEX_BYTES)}" +def _encodable(text: str, path: str) -> str: + """Refuse a `str` that UTF-8 cannot represent (SPEC-v0.1 §2.3). + + A lone UTF-16 surrogate is such a string: Python accepts it, UTF-8 has no encoding for it, + and it arrives the ordinary way -- `json.loads('"\\ud800"')` produces one, so an MCP tool + call carries it into the action path. It is refused here for the reason `float` is: an + argument that cannot be canonicalized cannot be hashed, and an `Action` that can be built + but never hashed is a trap set at construction and sprung somewhere else. + + A refusal and not a repair. `errors="replace"` would map two distinct arguments to one + canonical form, which is the collision §2.3 exists to prevent. A *paired* surrogate is not + affected: Python has already decoded it to the character it denotes. + """ + try: + text.encode("utf-8") + except UnicodeEncodeError as exc: + raise InvalidArgument( + f"{path} holds a string UTF-8 cannot encode at position {exc.start}: {exc.reason}. " + "A lone surrogate has no canonical form, and substituting one would give two " + "distinct actions the same hash (v0.1 §2.3)" + ) from exc + return text + + def _frozen_value(value: object, path: str) -> FrozenValue: """Validate an argument value and return a deep-frozen copy of it.""" if isinstance(value, float): @@ -49,7 +73,9 @@ def _frozen_value(value: object, path: str) -> FrozenValue: f"float is not an allowed argument type at {path}: " "use integer minor units (amount=200000) or a decimal string ('2000.00')" ) - if value is None or isinstance(value, str | int): # bool is a subclass of int + if isinstance(value, str): + return _encodable(value, path) + if value is None or isinstance(value, int): # bool is a subclass of int return value if isinstance(value, Mapping): return _frozen_mapping(value, path) @@ -65,6 +91,7 @@ def _frozen_mapping(value: Mapping[Any, Any], path: str) -> Mapping[str, FrozenV for key, item in value.items(): if not isinstance(key, str): raise InvalidArgument(f"argument keys must be str, got {type(key).__name__} at {path}") + _encodable(key, f"{path} key {key!r}") result[key] = _frozen_value(item, f"{path}.{key}") return MappingProxyType(result) @@ -214,13 +241,30 @@ def canonical_bytes(payload: Mapping[str, Any]) -> bytes: `ctrlrun.action/v1` is unchanged by this promotion and T164b is the corpus that proves it. """ - return json.dumps( + encoded = json.dumps( _no_floats(payload, "payload"), sort_keys=True, separators=(",", ":"), ensure_ascii=False, allow_nan=False, - ).encode("utf-8") + ) + try: + return encoded.encode("utf-8") + except UnicodeEncodeError as exc: + # A lone UTF-16 surrogate: a `str` Python accepts and UTF-8 cannot represent. It + # arrives the ordinary way -- `json.loads('"\ud800"')` produces one -- so an MCP tool + # call carries it here, and this used to escape as `UnicodeEncodeError`, outside the + # closed set in `errors.py`. A caller catching `CTRLRunError` did not catch it. + # + # Checked here rather than per string in `_no_floats`: the encode already walks every + # character, so this costs nothing on the path that succeeds. It is a refusal, not a + # repair -- `errors="replace"` would map two distinct arguments to one canonical form, + # which is the collision the whole of §2.3 exists to prevent. + raise InvalidArgument( + f"an argument holds a string that UTF-8 cannot encode at position {exc.start}: " + f"{exc.reason}. A lone surrogate has no canonical form, and substituting one " + "would give two distinct actions the same hash (v0.1 §2.3)" + ) from exc def _no_floats(value: object, path: str) -> PlainValue: diff --git a/tests/test_action.py b/tests/test_action.py index e955f431..55d722c3 100644 --- a/tests/test_action.py +++ b/tests/test_action.py @@ -251,6 +251,44 @@ def test_T7_non_string_argument_key_raises_invalid_argument(arguments: dict[Any, _action(arguments=arguments) +@pytest.mark.parametrize( + "arguments", + [ + pytest.param({"note": "\ud800"}, id="lone-high-surrogate-value"), + pytest.param({"note": "\udfff"}, id="lone-low-surrogate-value"), + pytest.param({"\ud800": "note"}, id="surrogate-key"), + pytest.param({"meta": {"note": "ok\ud83d"}}, id="nested-surrogate"), + pytest.param({"items": ["fine", "\udc00"]}, id="surrogate-in-a-list"), + ], +) +def test_T7_unencodable_string_raises_invalid_argument(arguments: dict[Any, Any]) -> None: + """A lone UTF-16 surrogate is a `str` Python accepts and UTF-8 cannot represent. + + It arrives the ordinary way: `json.loads('"\\ud800"')` produces one, so an MCP tool call + carries it into the action path. Canonicalization used to raise `UnicodeEncodeError` from + inside `json.dumps(...).encode()`, which is outside the closed error set in `errors.py` -- + a caller catching `CTRLRunError` did not catch it. Found by `fuzz/`.""" + with pytest.raises(InvalidArgument): + _action(arguments=arguments) + + +def test_T7_an_unencodable_string_is_refused_as_a_ctrlrun_error() -> None: + """The half that matters to a caller: it is in the closed set, not merely refused.""" + from ctrlrun.errors import CTRLRunError + + with pytest.raises(CTRLRunError): + assert _action(arguments=json.loads('{"note": "\\ud800"}')).action_hash + + +def test_T7_a_paired_surrogate_is_an_ordinary_character_and_is_accepted() -> None: + """The negative control. `\\ud83d\\ude00` is a *pair* -- Python decodes it to one + emoji -- so a refusal keyed on "contains a surrogate code point" rather than on + encodability would reject a string every JSON encoder in the world accepts.""" + action = _action(arguments={"note": json.loads('"\\ud83d\\ude00"')}) + assert action.action_hash.startswith("sha256:") + assert "\U0001f600" in canonicalize(action).decode("utf-8") + + def test_T7_a_tuple_and_the_equivalent_list_produce_the_same_hash() -> None: from_tuple = _action(arguments={"items": (1, "a", {"k": ("deep",)})}) from_list = _action(arguments={"items": [1, "a", {"k": ["deep"]}]}) diff --git a/tests/test_fuzzing.py b/tests/test_fuzzing.py index 291d2c9a..c37f95cb 100644 --- a/tests/test_fuzzing.py +++ b/tests/test_fuzzing.py @@ -280,42 +280,60 @@ def drifting(text, *, source=""): # --- the findings that are real and not yet fixed ----------------------------------------------- -def test_the_known_findings_still_reproduce(): - """`KNOWN_FINDINGS` records a defect that is real, reported, and unfixed. This asserts each - one **still happens**. - - The day it is fixed this goes red and the entry must be deleted. That is the point: a - recorded limit that quietly starts passing is exactly the false green the rest of this file - exists to prevent, and an excuse list nobody re-checks becomes permanent.""" - assert properties.KNOWN_FINDINGS, "no known findings; delete this test with the last entry" - - assert properties.check_canonical({"note": "\ud800"}) == "lone-surrogate-in-a-string", ( - "canonical_bytes no longer raises UnicodeEncodeError for an unpaired surrogate -- " - "if it now raises InvalidArgument, delete the KNOWN_FINDINGS entry and this assertion" +def test_there_are_no_recorded_findings_and_the_machinery_stays(): + """`KNOWN_FINDINGS` is empty, and that is a result rather than a default. + + It held `lone-surrogate-in-a-string` -- `canonical_bytes` raising `UnicodeEncodeError` + instead of `InvalidArgument` -- and the entry's test asserted the finding *still + reproduced*. Fixing it in `action.py` turned that test red, which is what forced the entry + out. The dict stays for the next one. + """ + assert properties.KNOWN_FINDINGS == {}, ( + "a finding was recorded; add a reproducer to the corpus and a test asserting it still " + "reproduces, so that fixing it forces the entry out" ) -def test_the_surrogate_finding_is_reachable_from_a_json_payload(): - """Severity, asserted rather than asserted-in-prose. `json.loads` produces a lone surrogate - from a six-character escape, so an MCP tool call can carry one into the action path, and - what comes back out is not in the closed error set that `errors.py` defines.""" +def test_the_surrogate_that_was_a_finding_is_now_refused_in_words(): + """The regression test for the finding this directory turned up. + + `json.loads` produces a lone surrogate from a six-character escape, so an MCP tool call can + carry one into the action path. It used to reach `action_hash` and raise + `UnicodeEncodeError`, outside the closed error set in `errors.py`.""" from ctrlrun import Action, Principal - from ctrlrun.errors import CTRLRunError + from ctrlrun.errors import CTRLRunError, InvalidArgument arguments = json.loads('{"note": "\\ud800"}') - assert arguments == {"note": "\ud800"} + assert arguments == {"note": "\ud800"}, "the payload no longer carries a surrogate" + + with pytest.raises(InvalidArgument) as raised: + Action(name="pay", environment="prod", principal=Principal(agent="a"), arguments=arguments) + + # The half that matters to a caller, asserted rather than implied: the refusal is *in* the + # closed set, so `except CTRLRunError` catches it, and it is not the `UnicodeEncodeError` + # this used to raise, which no such handler would have caught. + # + # A `try/except CTRLRunError: pass` stood here and CodeQL flagged the bare `pass`. It was + # right, and for a better reason than the style: `InvalidArgument` is a `CTRLRunError`, so + # the line above already proved that branch and the `except UnicodeEncodeError` beside it + # was unreachable from any input. A guard no test can reach is not a guard. + assert isinstance(raised.value, CTRLRunError) + assert not isinstance(raised.value, UnicodeEncodeError) + + +def test_a_paired_surrogate_is_still_accepted(): + """The negative control on the fix. A refusal keyed on "contains a surrogate code point" + rather than on encodability would reject an ordinary emoji, and every test above would + still pass.""" + from ctrlrun import Action, Principal action = Action( - name="pay", environment="prod", principal=Principal(agent="a"), arguments=arguments + name="pay", + environment="prod", + principal=Principal(agent="a"), + arguments=json.loads('{"note": "\\ud83d\\ude00"}'), ) - with pytest.raises(UnicodeEncodeError): - _ = action.action_hash - try: - _ = action.action_hash - except CTRLRunError: # pragma: no cover - the finding is that this does not happen - raise AssertionError("the finding is fixed; update KNOWN_FINDINGS") from None - except UnicodeEncodeError: - pass + assert action.action_hash.startswith("sha256:") def test_the_seed_corpus_reproduces_every_known_finding():