From 10f3b978f7317c71e7fd3a715e1f3eb9f57fbb50 Mon Sep 17 00:00:00 2001 From: arpan Date: Sun, 6 Sep 2026 22:29:03 +0530 Subject: [PATCH 1/2] Fuzz canonicalization and the policy loader, and record the finding it turned up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scorecard reads Fuzzing as 0/10. That is the reason this was picked up, but not the reason it is worth having: canonicalization is the one place in this codebase where a defect is silent. Two distinct actions sharing a canonical form share an approval, and nothing in a receipt would look wrong. This repository has already had one bug of exactly that shape -- `str(key)` folded `{"1": "a", 1: "b"}` into a single key, with the survivor decided by insertion order -- and a review found it. These are the properties that would have found it without a review. Two targets, chosen because each is a promise already written down: canonicalization (`v0.1 §2.3`), and `Policy.from_yaml`, whose docstring says "anything malformed raises `PolicyError`" -- the fail-closed rule in a sentence. **The invariants do not import Atheris.** `fuzz/properties.py` is plain stdlib, so `tests/test_fuzzing.py` is a second driver for the same properties and they run on every commit whether or not anybody has a fuzzing toolchain -- and a contributor can reproduce a finding without one. `fuzz/fuzz_*.py` hold no assertions of their own. CI runs the seed corpus as a gate that cannot be skipped by a toolchain problem, then a bounded two-minute campaign per target; a failure to install Atheris is a red build and not a skip. ## A finding, recorded rather than worked around `canonical_bytes` raises `UnicodeEncodeError`, not `InvalidArgument`, for a string holding an unpaired UTF-16 surrogate. It is reachable: `json.loads('"\ud800"')` produces one, so an MCP tool call can carry it; `Action(...)` accepts it and `action_hash` is where it fails. Fail-closed **holds** -- the gateway and `Control` both 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`: a caller catching `CTRLRunError` does not catch this. It is in `KNOWN_FINDINGS` with a reproducer in the corpus, not pruned out of the decoder, because a fuzzer whose corpus avoids its own findings reports zero forever. `test_the_known_findings_still_reproduce` asserts it **still happens**, so the day it is fixed that test goes red and the entry must be deleted. The fix belongs to whoever owns `src/` and is argued in `fuzz/README.md`. ## What the positive controls found Eleven controls, one per invariant. Writing them found three defects in the properties themselves, all fixed here: - **Only the first encode call was guarded.** An encoder broken on its second call raised `TypeError` straight out of `check_canonical`. A property claiming "nothing but InvalidArgument escapes" has to hold that for every call it makes. Every call goes through `_encode` now. - **Nothing asserted that floats and non-string keys are *refused*.** Checking only what comes back cannot tell "refused correctly" from "never refused at all". Both are now stated before the call. - **Every control was really testing one branch.** The generated corpus is full of non-string keys, so `must_refuse` fired first and each control -- whatever it was named for -- passed on that. They use purpose-built documents now and assert *which* message they got. Eighteen mutations, eighteen caught. ## One existing test changed `test_T124b_the_package_ships_nothing_from_research` substring-matched the whole of `MANIFEST.in` including its comments, so a comment mentioning `research/` beside the new `prune fuzz` failed a test about what setuptools ships. It parses directives now. Mutation-tested three ways -- `recursive-include research *.py`, `graft research`, `include research/soak/run.py` -- all caught, and the comment that broke it passes. --- .github/workflows/fuzz.yml | 63 ++++ MANIFEST.in | 6 + fuzz/README.md | 92 ++++++ fuzz/corpus/canonical/seed-00 | 0 fuzz/corpus/canonical/seed-01 | Bin 0 -> 1 bytes fuzz/corpus/canonical/seed-02 | 1 + fuzz/corpus/canonical/seed-03 | Bin 0 -> 32 bytes fuzz/corpus/canonical/seed-04 | 1 + fuzz/corpus/canonical/seed-05 | 1 + fuzz/corpus/canonical/seed-06 | Bin 0 -> 6 bytes fuzz/corpus/canonical/seed-07 | Bin 0 -> 4 bytes fuzz/corpus/canonical/seed-08 | Bin 0 -> 4 bytes fuzz/corpus/canonical/seed-09 | 1 + fuzz/corpus/canonical/seed-10-surrogate | 1 + fuzz/corpus/policy/seed-00 | 0 fuzz/corpus/policy/seed-01 | 2 + fuzz/corpus/policy/seed-02 | 4 + fuzz/corpus/policy/seed-03 | 1 + fuzz/corpus/policy/seed-04 | 1 + fuzz/corpus/policy/seed-05 | 1 + fuzz/corpus/policy/seed-06 | Bin 0 -> 11 bytes fuzz/corpus/policy/seed-07 | 1 + fuzz/corpus/policy/seed-08 | 5 + fuzz/fuzz_canonical.py | 57 ++++ fuzz/fuzz_policy.py | 57 ++++ fuzz/properties.py | 283 +++++++++++++++++ pyproject.toml | 1 + tests/test_framework_probe.py | 15 +- tests/test_fuzzing.py | 396 ++++++++++++++++++++++++ 29 files changed, 988 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/fuzz.yml create mode 100644 fuzz/README.md create mode 100644 fuzz/corpus/canonical/seed-00 create mode 100644 fuzz/corpus/canonical/seed-01 create mode 100644 fuzz/corpus/canonical/seed-02 create mode 100644 fuzz/corpus/canonical/seed-03 create mode 100644 fuzz/corpus/canonical/seed-04 create mode 100644 fuzz/corpus/canonical/seed-05 create mode 100644 fuzz/corpus/canonical/seed-06 create mode 100644 fuzz/corpus/canonical/seed-07 create mode 100644 fuzz/corpus/canonical/seed-08 create mode 100644 fuzz/corpus/canonical/seed-09 create mode 100644 fuzz/corpus/canonical/seed-10-surrogate create mode 100644 fuzz/corpus/policy/seed-00 create mode 100644 fuzz/corpus/policy/seed-01 create mode 100644 fuzz/corpus/policy/seed-02 create mode 100644 fuzz/corpus/policy/seed-03 create mode 100644 fuzz/corpus/policy/seed-04 create mode 100644 fuzz/corpus/policy/seed-05 create mode 100644 fuzz/corpus/policy/seed-06 create mode 100644 fuzz/corpus/policy/seed-07 create mode 100644 fuzz/corpus/policy/seed-08 create mode 100644 fuzz/fuzz_canonical.py create mode 100644 fuzz/fuzz_policy.py create mode 100644 fuzz/properties.py create mode 100644 tests/test_fuzzing.py diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml new file mode 100644 index 00000000..32dfe5eb --- /dev/null +++ b/.github/workflows/fuzz.yml @@ -0,0 +1,63 @@ +name: Fuzz + +# Canonicalization is security-critical (`v0.1 §2.3`): two distinct actions sharing a canonical +# form share an approval, and nothing in a receipt would look wrong. `Policy.from_yaml` promises +# in its docstring that anything malformed raises `PolicyError`. Both are properties rather than +# examples, so both are fuzzed. `fuzz/properties.py` holds the invariants; this runs them. + +on: + pull_request: + push: + branches: [main] + schedule: + - cron: "23 4 * * 1" + +permissions: + contents: read + +jobs: + fuzz: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.11" + + - name: Install + run: | + python -m pip install --upgrade pip + pip install -e . + + # The gate. No Atheris, so it cannot be skipped by a toolchain problem, and it fails on + # a seed that regressed rather than on a campaign that happened to find one. + - name: The seed corpus still holds + run: | + python fuzz/fuzz_canonical.py --corpus + python fuzz/fuzz_policy.py --corpus + + # Atheris is a real dependency and a failure to install it is a red build, not a skip: + # a fuzzing job that quietly stops fuzzing is the false green this repository keeps + # finding in other costumes. + - name: Install Atheris + run: pip install atheris + + # Bounded, or an unbounded `atheris.Fuzz()` hangs until the job timeout, which reads as a + # broken build rather than as a finding. `-runs` is not used: wall-clock is what makes a + # pull request's campaign the same size on a fast and a slow runner. + - name: Campaign + run: | + python fuzz/fuzz_canonical.py -max_total_time=120 -print_final_stats=1 fuzz/corpus/canonical + python fuzz/fuzz_policy.py -max_total_time=120 -print_final_stats=1 fuzz/corpus/policy + + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: failure() + with: + name: crashes + path: | + crash-* + timeout-* + oom-* + if-no-files-found: ignore + retention-days: 14 diff --git a/MANIFEST.in b/MANIFEST.in index f531bc7a..7e5d7e88 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -57,4 +57,10 @@ prune .github # a test, because `MANIFEST.in` resolves against the working tree rather than the index, which # is how v0.2 shipped four files it should not have. prune adapters +# `fuzz/` is a development tree like `research/`: the targets, their seed corpus and the +# invariants they assert. Belt and braces for the same reason the four lines above are -- +# `[tool.setuptools.packages.find]` looks only in `src/`, so nothing here is a package and the +# sdist omits it either way. `test_the_fuzz_corpus_is_not_packaged` asserts the line, and +# `test_T181_*` asserts the outcome. +prune fuzz global-exclude *.py[cod] .DS_Store diff --git a/fuzz/README.md b/fuzz/README.md new file mode 100644 index 00000000..028356c9 --- /dev/null +++ b/fuzz/README.md @@ -0,0 +1,92 @@ +# Fuzzing + +Two properties are fuzzed, chosen because each is a promise the project has written down. + +**Canonicalization** (`SPEC-v0.1.md` §2.3, which calls it security-critical and requires a +test proving old hashes still verify before anything here changes). Two distinct actions sharing a canonical form share an approval, and +nothing in a receipt would look wrong. This repository has already had one bug of exactly that +shape — `str(key)` folded `{"1": "a", 1: "b"}` into a single key, with the survivor decided by +insertion order — and a review found it. These are the properties that would have found it +without one. + +**`Policy.from_yaml`**, whose docstring says *anything malformed raises `PolicyError`*. That is +the fail-closed rule in a sentence. A loader raising `KeyError` still denies, but through a +crash nobody classified, and a caller catching `PolicyError` would not catch it. + +## Layout + +``` +properties.py the invariants and the decoders — no Atheris import +fuzz_canonical.py Atheris entry point +fuzz_policy.py Atheris entry point +corpus/ seed inputs, including a reproducer for every recorded finding +``` + +`properties.py` deliberately does not import Atheris. The fuzzer is one driver for these +invariants and `tests/test_fuzzing.py` is another, so they run on every commit whether or not +anybody has a fuzzing toolchain — and a contributor can reproduce a finding with nothing but +the standard library. + +## Running + +```sh +python fuzz/fuzz_canonical.py --corpus # seeds only, no Atheris +pip install atheris +python fuzz/fuzz_canonical.py -max_total_time=120 fuzz/corpus/canonical # a campaign +``` + +CI runs the first form as a gate and the second for two minutes per target on every pull +request, weekly on a schedule. A failure to install Atheris is a red build and not a skip: a +fuzzing job that quietly stops fuzzing is a false green. + +## The invariants + +For canonicalization, over every generated document: + +1. **Refusal is stated.** Nothing but `InvalidArgument` escapes — checked on *every* call, not + just the first. A positive control found that one: an encoder broken only on its second call + raised `TypeError` straight out of the property. +2. **Floats and non-string keys are refused**, asserted *before* the call. Checking only what + comes back cannot tell "refused correctly" from "never refused at all". +3. **Deterministic** — the same document twice gives the same bytes. +4. **Order-independent** — two mappings that compare equal encode identically, or the hash + depends on how the caller happened to build the dict. +5. **Stable through a round trip** — `json.loads` then re-encode gives the same bytes. A lossy + encoding shows up here. +6. **Keys sorted at every level**, which is what makes the form canonical and not merely + consistent. + +For the policy loader: nothing but `PolicyError` escapes, and a document that loads hashes the +same way on a second parse. (`RecursionError` on a document nested past the interpreter's limit +is out of scope, and is returned rather than swallowed silently — it is a stack-depth property, +not a policy one.) + +Every invariant has a positive control in `tests/test_fuzzing.py` that breaks it deliberately +and requires the check to notice. A property that cannot fail proves nothing. + +## Findings + +`properties.KNOWN_FINDINGS` records defects that are real, reported, and not yet fixed. They +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. + +### `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 into the +action path. `Action(...)` accepts it and `action_hash` is where it fails. + +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. + +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. diff --git a/fuzz/corpus/canonical/seed-00 b/fuzz/corpus/canonical/seed-00 new file mode 100644 index 00000000..e69de29b diff --git a/fuzz/corpus/canonical/seed-01 b/fuzz/corpus/canonical/seed-01 new file mode 100644 index 0000000000000000000000000000000000000000..f76dd238ade08917e6712764a16a22005a50573d GIT binary patch literal 1 IcmZPo000310RR91 literal 0 HcmV?d00001 diff --git a/fuzz/corpus/canonical/seed-02 b/fuzz/corpus/canonical/seed-02 new file mode 100644 index 00000000..8663f7de --- /dev/null +++ b/fuzz/corpus/canonical/seed-02 @@ -0,0 +1 @@ +ÿÿÿÿÿÿÿÿ \ No newline at end of file diff --git a/fuzz/corpus/canonical/seed-03 b/fuzz/corpus/canonical/seed-03 new file mode 100644 index 0000000000000000000000000000000000000000..fefa1cc823e0ec463b7923972b03901b52808fc1 GIT binary patch literal 32 ncmZQzWMXDvWn<^yMC+6cQE@6%&_`l#-T_m6Hbm6>tIZ literal 0 HcmV?d00001 diff --git a/fuzz/corpus/canonical/seed-04 b/fuzz/corpus/canonical/seed-04 new file mode 100644 index 00000000..bba994b1 --- /dev/null +++ b/fuzz/corpus/canonical/seed-04 @@ -0,0 +1 @@ +  \ No newline at end of file diff --git a/fuzz/corpus/canonical/seed-05 b/fuzz/corpus/canonical/seed-05 new file mode 100644 index 00000000..7256e277 --- /dev/null +++ b/fuzz/corpus/canonical/seed-05 @@ -0,0 +1 @@ +  \ No newline at end of file diff --git a/fuzz/corpus/canonical/seed-06 b/fuzz/corpus/canonical/seed-06 new file mode 100644 index 0000000000000000000000000000000000000000..d5843b80fec72e6db9f16fd5bc58a66be6b417c3 GIT binary patch literal 6 Ncmd;NW?^7q1ONbJ02TlM literal 0 HcmV?d00001 diff --git a/fuzz/corpus/canonical/seed-07 b/fuzz/corpus/canonical/seed-07 new file mode 100644 index 0000000000000000000000000000000000000000..33ee3187e780027ecd73869bcfc8ccdbf76df13b GIT binary patch literal 4 Lcmd;NVq*XR06G8? literal 0 HcmV?d00001 diff --git a/fuzz/corpus/canonical/seed-08 b/fuzz/corpus/canonical/seed-08 new file mode 100644 index 0000000000000000000000000000000000000000..78c7d73fa393906d782c333bf2bb51422167b0a4 GIT binary patch literal 4 Lcmd;NVrKvV06YK_ literal 0 HcmV?d00001 diff --git a/fuzz/corpus/canonical/seed-09 b/fuzz/corpus/canonical/seed-09 new file mode 100644 index 00000000..0890bb8e --- /dev/null +++ b/fuzz/corpus/canonical/seed-09 @@ -0,0 +1 @@ +  diff --git a/fuzz/corpus/canonical/seed-10-surrogate b/fuzz/corpus/canonical/seed-10-surrogate new file mode 100644 index 00000000..b4523155 --- /dev/null +++ b/fuzz/corpus/canonical/seed-10-surrogate @@ -0,0 +1 @@ +ÏÉüÂÚ1Î=Ñf½Í:3„~[»ý \ No newline at end of file diff --git a/fuzz/corpus/policy/seed-00 b/fuzz/corpus/policy/seed-00 new file mode 100644 index 00000000..e69de29b diff --git a/fuzz/corpus/policy/seed-01 b/fuzz/corpus/policy/seed-01 new file mode 100644 index 00000000..083d3389 --- /dev/null +++ b/fuzz/corpus/policy/seed-01 @@ -0,0 +1,2 @@ +schema: ctrlrun.policy/v3 +actions: {} diff --git a/fuzz/corpus/policy/seed-02 b/fuzz/corpus/policy/seed-02 new file mode 100644 index 00000000..488ea6ef --- /dev/null +++ b/fuzz/corpus/policy/seed-02 @@ -0,0 +1,4 @@ +schema: ctrlrun.policy/v3 +actions: + pay: + default: deny diff --git a/fuzz/corpus/policy/seed-03 b/fuzz/corpus/policy/seed-03 new file mode 100644 index 00000000..ca85d108 --- /dev/null +++ b/fuzz/corpus/policy/seed-03 @@ -0,0 +1 @@ +1: true diff --git a/fuzz/corpus/policy/seed-04 b/fuzz/corpus/policy/seed-04 new file mode 100644 index 00000000..521ece52 --- /dev/null +++ b/fuzz/corpus/policy/seed-04 @@ -0,0 +1 @@ +[ [ [ [ \ No newline at end of file diff --git a/fuzz/corpus/policy/seed-05 b/fuzz/corpus/policy/seed-05 new file mode 100644 index 00000000..0b7eb6d7 --- /dev/null +++ b/fuzz/corpus/policy/seed-05 @@ -0,0 +1 @@ +schema: ctrlrun.policy/v99 diff --git a/fuzz/corpus/policy/seed-06 b/fuzz/corpus/policy/seed-06 new file mode 100644 index 0000000000000000000000000000000000000000..f79cd00de536db9b063450b9d1d84cec7c263efe GIT binary patch literal 11 ScmezWk0B{BMWM7L%>n=+&jic> literal 0 HcmV?d00001 diff --git a/fuzz/corpus/policy/seed-07 b/fuzz/corpus/policy/seed-07 new file mode 100644 index 00000000..4a7799e8 --- /dev/null +++ b/fuzz/corpus/policy/seed-07 @@ -0,0 +1 @@ +actions: not-a-mapping diff --git a/fuzz/corpus/policy/seed-08 b/fuzz/corpus/policy/seed-08 new file mode 100644 index 00000000..eb7e7fa0 --- /dev/null +++ b/fuzz/corpus/policy/seed-08 @@ -0,0 +1,5 @@ +schema: ctrlrun.policy/v3 +actions: + pay: + default: allow + require_approval: true diff --git a/fuzz/fuzz_canonical.py b/fuzz/fuzz_canonical.py new file mode 100644 index 00000000..c8e8209a --- /dev/null +++ b/fuzz/fuzz_canonical.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Atheris entry point for the canonical property. The invariants live in `properties.py`. + +Two ways to run it: + + python fuzz/fuzz_canonical.py --corpus # the seed corpus, no Atheris needed + python fuzz/fuzz_canonical.py -max_total_time=60 fuzz/corpus/canonical + +The first is what `tests/test_fuzzing.py` and CI's quick gate run, so the target is executed +on every commit rather than only wherever a fuzzing toolchain happens to exist. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +FUZZ = Path(__file__).resolve().parent +if str(FUZZ) not in sys.path: + sys.path.insert(0, str(FUZZ)) + +import properties # noqa: E402 + + +def one_input(data: bytes) -> None: + properties.check_canonical(properties.document_from_bytes(data)) + + +def _run_corpus() -> int: + seeds = sorted(p for p in (FUZZ / "corpus" / "canonical").iterdir() if p.is_file()) + findings: dict[str, int] = {} + for path in seeds: + known = properties.check_canonical(properties.document_from_bytes(path.read_bytes())) + if known is not None: + findings[known] = findings.get(known, 0) + 1 + print(f"canonical: {len(seeds)} inputs, {len(findings)} known finding(s) reproduced") + for name, count in sorted(findings.items()): + print(f" {name}: {count}") + return 0 + + +def main() -> int: + if "--corpus" in sys.argv: + return _run_corpus() + + import atheris + + with atheris.instrument_imports(): + import properties as instrumented # noqa: F401 + + atheris.Setup(sys.argv, one_input) + atheris.Fuzz() + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/fuzz/fuzz_policy.py b/fuzz/fuzz_policy.py new file mode 100644 index 00000000..9ab97941 --- /dev/null +++ b/fuzz/fuzz_policy.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Atheris entry point for the policy property. The invariants live in `properties.py`. + +Two ways to run it: + + python fuzz/fuzz_policy.py --corpus # the seed corpus, no Atheris needed + python fuzz/fuzz_policy.py -max_total_time=60 fuzz/corpus/policy + +The first is what `tests/test_fuzzing.py` and CI's quick gate run, so the target is executed +on every commit rather than only wherever a fuzzing toolchain happens to exist. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +FUZZ = Path(__file__).resolve().parent +if str(FUZZ) not in sys.path: + sys.path.insert(0, str(FUZZ)) + +import properties # noqa: E402 + + +def one_input(data: bytes) -> None: + properties.check_policy(properties.policy_text_from_bytes(data)) + + +def _run_corpus() -> int: + seeds = sorted(p for p in (FUZZ / "corpus" / "policy").iterdir() if p.is_file()) + findings: dict[str, int] = {} + for path in seeds: + known = properties.check_policy(properties.policy_text_from_bytes(path.read_bytes())) + if known is not None: + findings[known] = findings.get(known, 0) + 1 + print(f"policy: {len(seeds)} inputs, {len(findings)} known finding(s) reproduced") + for name, count in sorted(findings.items()): + print(f" {name}: {count}") + return 0 + + +def main() -> int: + if "--corpus" in sys.argv: + return _run_corpus() + + import atheris + + with atheris.instrument_imports(): + import properties as instrumented # noqa: F401 + + atheris.Setup(sys.argv, one_input) + atheris.Fuzz() + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/fuzz/properties.py b/fuzz/properties.py new file mode 100644 index 00000000..1f408dd7 --- /dev/null +++ b/fuzz/properties.py @@ -0,0 +1,283 @@ +"""The invariants the fuzz targets assert, and the decoders that turn bytes into inputs. + +**No Atheris import lives here on purpose.** Atheris is one driver for these properties and +`tests/test_fuzzing.py` is another, so they run on every commit whether or not anybody has a +fuzzing toolchain -- and a contributor can reproduce a finding with nothing but the standard +library. `fuzz/fuzz_*.py` are the Atheris entry points and contain no assertions of their own. + +Two properties, chosen because each is a promise the project has written down: + +* **Canonicalization** (`v0.1 §2.3`, which calls it security-critical). + Two distinct actions sharing a canonical form share an approval, and nothing in a receipt + would look wrong. This repository has already had one such bug -- `str(key)` folding `{"1": + "a", 1: "b"}` into a single key, with the survivor decided by insertion order -- and a review + found it. These are the properties that would have found it without one. + +* **`Policy.from_yaml`**, whose docstring says "anything malformed raises `PolicyError`". That + is the fail-closed rule in one sentence. A loader raising `KeyError` still denies, but through + a crash nobody classified, and a caller catching `PolicyError` would not catch it. +""" + +from __future__ import annotations + +import json +import math +from typing import Any + +from ctrlrun import canonical_bytes +from ctrlrun.errors import InvalidArgument, PolicyError +from ctrlrun.policy import Policy + + +#: Indirection so the harness -- and the positive controls in `tests/test_fuzzing.py` -- can +#: substitute a deliberately broken loader. A property that cannot fail proves nothing. +def policy_from_yaml(text: str, *, source: str = "") -> Policy: + return Policy.from_yaml(text, source=source) + + +# --- 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. +#: +#: `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 + + +# --- decoders --------------------------------------------------------------------------------- + +_MAX_DEPTH = 5 +_MAX_ITEMS = 6 + +# Deliberately includes values canonicalization must *refuse*. A property saying "only +# InvalidArgument escapes" is vacuous if nothing is ever rejected. +_INTERESTING_STRINGS = ( + "", + "a", + "0", + "\x00", + "\\", + '"', + " ", + "é", + "中文", + "\U0001f511", + "\ud800", # unpaired surrogate -- see KNOWN_FINDINGS + "", + "1e309", +) + + +class _Reader: + def __init__(self, data: bytes) -> None: + self._data = data + self._i = 0 + + def byte(self) -> int: + if self._i >= len(self._data): + return 0 + value = self._data[self._i] + self._i += 1 + return value + + def spent(self) -> bool: + return self._i >= len(self._data) + + +def _value(reader: _Reader, depth: int) -> Any: + kind = reader.byte() % (10 if depth < _MAX_DEPTH else 8) + if kind == 0: + return None + if kind == 1: + return reader.byte() % 2 == 0 + if kind == 2: + return reader.byte() - 128 + if kind == 3: + return (reader.byte() << 56) * (10 ** (reader.byte() % 4)) + if kind == 4: + return _INTERESTING_STRINGS[reader.byte() % len(_INTERESTING_STRINGS)] + if kind == 5: + return bytes([reader.byte(), reader.byte()]).decode("latin-1") + if kind == 6: + # float: canonicalization must refuse it at any depth (v0.1 §2.3). + return (reader.byte() / 7.0, float("inf"), math.nan)[reader.byte() % 3] + if kind == 7: + # a type that is not JSON at all + return (b"bytes", {1, 2}, object())[reader.byte() % 3] + if kind == 8: + return [_value(reader, depth + 1) for _ in range(reader.byte() % _MAX_ITEMS)] + return _mapping(reader, depth + 1) + + +def _mapping(reader: _Reader, depth: int) -> dict[Any, Any]: + out: dict[Any, Any] = {} + for _ in range(reader.byte() % _MAX_ITEMS): + # Non-string keys included on purpose: `yaml.safe_load` produces them from `1:` and + # `true:`, which is how §7.1's policy hash reaches this path. + key: Any = ( + _INTERESTING_STRINGS[reader.byte() % len(_INTERESTING_STRINGS)] + if reader.byte() % 4 + else (reader.byte(), True, None)[reader.byte() % 3] + ) + out[key] = _value(reader, depth) + return out + + +def document_from_bytes(data: bytes) -> dict[Any, Any]: + """Total and deterministic: every byte string maps to a document, and the same bytes always + map to the same one. A decoder that raised would spend a campaign reporting its own crashes.""" + return _mapping(_Reader(data), 0) + + +def policy_text_from_bytes(data: bytes) -> str: + """Bytes as a policy document. Undecodable input becomes text the loader must still refuse + in words rather than crash on.""" + return data.decode("utf-8", "replace") + + +# --- the properties ----------------------------------------------------------------------------- + + +def _contains_float(value: object) -> bool: + if isinstance(value, float): + return True + if isinstance(value, dict): + return any(_contains_float(k) or _contains_float(v) for k, v in value.items()) + if isinstance(value, list | tuple): + return any(_contains_float(item) for item in value) + return False + + +def _has_non_string_key(value: object) -> bool: + if isinstance(value, dict): + return any(not isinstance(k, str) or _has_non_string_key(v) for k, v in value.items()) + if isinstance(value, list | tuple): + return any(_has_non_string_key(item) for item in value) + return False + + +def _encode(document: object, what: str) -> bytes: + """Every call goes through here, not just the first. + + A positive control found this: an encoder broken only on its *second* call raised + `TypeError` straight out of `check_canonical`, because the later calls were bare. A + property claiming "nothing but InvalidArgument escapes" has to hold that for every call it + makes, or it holds for the one the author happened to wrap. + """ + try: + return canonical_bytes(document) # type: ignore[arg-type] + except (InvalidArgument, UnicodeEncodeError): + raise + except Exception as exc: + raise AssertionError( + f"canonicalization raised {type(exc).__name__} on {what}, which is outside the " + f"closed error set in errors.py: {exc}" + ) from exc + + +def check_canonical(document: dict[Any, Any]) -> str | None: + """Assert every canonicalization invariant. Returns the name of a known finding if the + input reproduces one, else None. Raises AssertionError on anything else.""" + # Stated before the call, so that *accepting* one of these is a failure. Checking only + # what comes back cannot tell "refused correctly" from "never refused at all". + must_refuse = [] + if _contains_float(document): + must_refuse.append("a float, which is not portable between two hosts (v0.1 §2.3)") + if _has_non_string_key(document): + must_refuse.append("a non-string key, which two distinct documents can share") + + try: + 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 + + assert not must_refuse, "canonicalization accepted a document holding " + " and ".join( + must_refuse + ) + assert isinstance(first, bytes) + + # 1. Deterministic. + assert _encode(document, "a repeat pass") == first, "canonicalization is not deterministic" + + # 2. Independent of insertion order. Two dicts that compare equal must encode identically, + # or the hash depends on how the caller happened to build the mapping. + reordered = dict(reversed(list(document.items()))) + assert _encode(reordered, "a reordered mapping") == first, ( + "canonicalization depends on insertion order: two equal mappings, two canonical forms" + ) + + # 3. Valid UTF-8 JSON. + decoded = json.loads(first.decode("utf-8")) + + # 4. Stable through a round trip. A lossy encoding -- two distinct keys folding into one -- + # shows up here as a second pass that disagrees with the first. + assert _encode(decoded, "a round trip") == first, ( + "canonicalization is not stable through a round trip" + ) + + # 5. Keys sorted at every level, which is what makes the form canonical rather than merely + # consistent. + _assert_sorted(decoded) + return None + + +def _assert_sorted(value: object) -> None: + if isinstance(value, dict): + keys = list(value) + assert keys == sorted(keys), f"keys are not sorted: {keys}" + for item in value.values(): + _assert_sorted(item) + elif isinstance(value, list): + for item in value: + _assert_sorted(item) + + +def check_policy(text: str) -> str | None: + """`Policy.from_yaml` either returns a policy or raises `PolicyError`. Nothing else.""" + try: + policy = policy_from_yaml(text) + except PolicyError: + return None + except RecursionError: + # Out of scope and said so rather than caught silently: a document nested past the + # interpreter's limit is a stack-depth property, not a policy one. + return None + except Exception as exc: + raise AssertionError( + f"Policy.from_yaml raised {type(exc).__name__} for a document it should have " + f"refused with PolicyError: {exc}" + ) from exc + + # A policy that loaded must hash, and hash the same way twice: `policy_hash` is the answer + # to "what decided this action", and two parses disagreeing would make it useless. + first = policy.policy_hash + assert policy.policy_hash == first, "policy_hash is not stable within one policy" + assert policy_from_yaml(text).policy_hash == first, ( + "two parses of one document produced two policy hashes" + ) + return None diff --git a/pyproject.toml b/pyproject.toml index df0d45a3..6c4c4d04 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -216,6 +216,7 @@ select = ["E", "F", "W", "I", "N", "UP", "B", "ANN", "SIM", "RUF"] "research/framework-probe/*.py" = ["ANN401", "N818", "PLC0415"] "research/framework-probe/**/*.py" = ["ANN401", "N818", "PLC0415"] "research/soak/*.py" = ["ANN401", "N818", "PLC0415"] +"fuzz/*.py" = ["ANN401", "N818", "PLC0415"] "research/soak/**/*.py" = ["ANN401", "N818", "PLC0415"] # The reference generators carry the pages' own prose as string literals, and a page's line is # not a code line: the rendered file is what has a width. Everything else about them is linted. diff --git a/tests/test_framework_probe.py b/tests/test_framework_probe.py index df9047c7..e5efab97 100644 --- a/tests/test_framework_probe.py +++ b/tests/test_framework_probe.py @@ -344,8 +344,19 @@ def test_T124b_the_package_ships_nothing_from_research(): document = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8")) assert document["tool"]["setuptools"]["packages"]["find"]["where"] == ["src"] - manifest = (REPO_ROOT / "MANIFEST.in").read_text(encoding="utf-8") - assert "research" not in manifest or "prune research" in manifest + + # The **directives**, not the raw text. This read the whole file including its comments, + # so a comment merely mentioning `research/` -- one was added beside `prune fuzz` -- failed + # a test about what setuptools ships. Comments are not packaging instructions, and the + # thing being asserted is that no directive names the directory. + directives = [ + line.strip() + for line in (REPO_ROOT / "MANIFEST.in").read_text(encoding="utf-8").splitlines() + if line.strip() and not line.lstrip().startswith("#") + ] + assert directives, "MANIFEST.in has no directives at all; this test would pass on an empty file" + shipping = [d for d in directives if "research" in d and not d.startswith("prune ")] + assert shipping == [], shipping # --- the fairness rules are in the README, and they are normative --------------------------- diff --git a/tests/test_fuzzing.py b/tests/test_fuzzing.py new file mode 100644 index 00000000..fdefd6a6 --- /dev/null +++ b/tests/test_fuzzing.py @@ -0,0 +1,396 @@ +"""The fuzz targets under `fuzz/`, and the invariants they assert. + +`SPEC-v0.1.md` §2.3 calls canonicalization security-critical, and it is the one place where a +defect is silent: two distinct actions sharing a canonical form share an approval, and nothing in a +receipt would look wrong. `Policy.from_yaml`'s docstring makes the other promise -- "anything +malformed raises `PolicyError`" -- which is the fail-closed rule in a single sentence, and a +loader that raised `KeyError` instead would still deny, but through a crash nobody classified. + +Both are properties rather than examples, so both are fuzzed. The invariants live in +`fuzz/properties.py` and **carry no dependency on Atheris**: the fuzzer is one driver for them +and this suite is another, so they are exercised on every commit whether or not anybody has a +fuzzing toolchain installed. `fuzz/fuzz_*.py` are the Atheris entry points. + +A property that cannot fail proves nothing, so every invariant here has a positive control +that breaks it deliberately and requires the check to notice. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] +FUZZ = REPO_ROOT / "fuzz" + +# Same narrowness as `test_soak.py`: `fuzz/` is not in a distribution, and the packaging test +# is what asserts it should not be. In a checkout this module runs; in an sdist it skips and +# `test_the_fuzz_corpus_is_not_packaged` is what proves the absence was deliberate. +if not FUZZ.is_dir(): # pragma: no cover - running from a distribution + pytest.skip("fuzz/ is not in this distribution", allow_module_level=True) +if str(FUZZ) not in sys.path: + sys.path.insert(0, str(FUZZ)) + +import properties # noqa: E402 + +# --- the decoders are total ------------------------------------------------------------------ + + +@pytest.mark.parametrize("size", [0, 1, 2, 7, 33, 200, 1024]) +def test_the_document_decoder_accepts_any_bytes(size): + """A fuzzer hands the target arbitrary bytes. A decoder that raised on some of them would + spend the campaign reporting its own crashes instead of the library's.""" + for seed in range(16): + data = bytes((seed * 31 + i * 17) % 256 for i in range(size)) + properties.document_from_bytes(data) + + +@pytest.mark.parametrize("size", [0, 1, 5, 64, 512]) +def test_the_policy_text_decoder_accepts_any_bytes(size): + for seed in range(16): + data = bytes((seed * 13 + i * 7) % 256 for i in range(size)) + assert isinstance(properties.policy_text_from_bytes(data), str) + + +# --- the invariants hold ---------------------------------------------------------------------- + + +def _corpus(count: int = 400) -> list[bytes]: + return [ + bytes((n * 2654435761 >> (8 * i)) % 256 for i in range(n % 48 + 1)) for n in range(count) + ] + + +def test_canonicalization_holds_over_the_corpus(): + """The campaign this suite can afford to run on every commit.""" + for data in _corpus(): + properties.check_canonical(properties.document_from_bytes(data)) + + +def test_the_policy_loader_holds_over_the_corpus(): + for data in _corpus(): + properties.check_policy(properties.policy_text_from_bytes(data)) + + +def test_the_seed_corpus_on_disk_passes_both_checks(): + """The seeds are what a fresh campaign starts from. One that already fails is a finding + nobody filed, and one that cannot be decoded is a file the fuzzer will ignore.""" + seeds = sorted((FUZZ / "corpus").rglob("*")) + assert [p for p in seeds if p.is_file()], "the seed corpus is empty" + for path in seeds: + if not path.is_file(): + continue + data = path.read_bytes() + if path.parent.name == "canonical": + properties.check_canonical(properties.document_from_bytes(data)) + else: + properties.check_policy(properties.policy_text_from_bytes(data)) + + +# --- the positive controls -------------------------------------------------------------------- + + +def _plain_documents() -> list[dict]: + """String keys, no floats, at least two keys in non-sorted order, some nesting. + + The generated corpus is full of non-string keys, so `must_refuse` fires on it first and + every control -- whatever it was named for -- was really testing that one branch. These + documents reach the invariants further down. + """ + return [ + {"b": 1, "a": 2}, + {"z": {"y": 1, "x": 2}, "a": [1, 2, {"q": None}]}, + {"A": 1, "a": 2}, + {"b": "two", "a": True, "c": None}, + {"nested": {"deep": {"deeper": ["x", "y"]}}, "alpha": 0}, + ] + + +def test_the_canonical_check_catches_a_nondeterministic_encoder(monkeypatch): + """Two hosts disagreeing about a hash is the failure that makes an approval unbindable.""" + calls = {"n": 0} + + def wobbly(payload): + calls["n"] += 1 + return json.dumps(payload, sort_keys=calls["n"] % 2 == 0, separators=(",", ":")).encode() + + monkeypatch.setattr(properties, "canonical_bytes", wobbly) + # The message, not just the type. Several invariants can each catch a broken encoder, so a + # bare `raises(AssertionError)` stays green when the one being tested is deleted. + with pytest.raises(AssertionError, match="not deterministic"): + for document in _plain_documents(): + properties.check_canonical(document) + + +def test_the_canonical_check_catches_an_order_dependent_encoder(monkeypatch): + """Two mappings that compare equal must encode identically, or the hash depends on how the + caller happened to build the dict -- and the same action, assembled twice, hashes twice.""" + monkeypatch.setattr( + properties, + "canonical_bytes", + lambda payload: json.dumps(payload, sort_keys=False, separators=(",", ":")).encode(), + ) + with pytest.raises(AssertionError, match="depends on insertion order"): + for document in _plain_documents(): + properties.check_canonical(document) + + +def test_the_canonical_check_catches_a_lossy_encoder(monkeypatch): + """The bug shape this exists for, and one this repository has already had once: a + canonicalizer that folded two distinct keys into one. A review found that one; this is what + would have found it without a review. + + **Order-independence is the invariant that catches it**, not the round trip: the fold is + invisible to a re-encode of the already-folded output, but it makes the surviving value + depend on which key was written last. That is stated rather than left to the reader, + because a control whose docstring names a different invariant than the one that fires is + how a subsumed guard gets recorded as load-bearing.""" + + def lossy(payload): + return json.dumps( + {str(k).lower(): v for k, v in payload.items()}, sort_keys=True, separators=(",", ":") + ).encode() + + monkeypatch.setattr(properties, "canonical_bytes", lossy) + with pytest.raises(AssertionError, match="depends on insertion order"): + for document in _plain_documents(): + properties.check_canonical(document) + + +def test_the_canonical_check_catches_a_non_idempotent_encoder(monkeypatch): + """The invariant the other three do not reach: an encoder that is deterministic and + order-independent, and still disagrees with itself one round trip later.""" + monkeypatch.setattr( + properties, + "canonical_bytes", + lambda payload: json.dumps({"v": payload}, sort_keys=True, separators=(",", ":")).encode(), + ) + with pytest.raises(AssertionError, match="not stable through a round trip"): + for document in _plain_documents(): + properties.check_canonical(document) + + +def test_the_canonical_check_catches_an_encoder_that_admits_a_float(monkeypatch): + """`float` is rejected because a binary float is not portable between two hosts. An encoder + that let one through would produce a hash that verifies on the machine that made it.""" + monkeypatch.setattr( + properties, "canonical_bytes", lambda payload: json.dumps(payload, default=str).encode() + ) + with pytest.raises(AssertionError, match="a float"): + properties.check_canonical({"amount": 0.1}) + + +def test_the_canonical_check_catches_an_encoder_that_admits_a_non_string_key(monkeypatch): + """`yaml.safe_load` produces non-string keys from `1:` and `true:`, which is how the policy + hash reaches this path. `json.dumps` coerces them to strings silently, so two distinct + documents get one canonical form and nothing raises. + + `sort_keys=False` here on purpose: with it on, `json.dumps` refuses to order a `str` + against an `int` and the control would pass on a `TypeError` instead of on the guard.""" + monkeypatch.setattr( + properties, + "canonical_bytes", + lambda payload: json.dumps(payload, sort_keys=False, separators=(",", ":")).encode(), + ) + with pytest.raises(AssertionError, match="a non-string key"): + properties.check_canonical({"1": "a", 1: "b"}) + + +def test_the_canonical_check_catches_an_encoder_that_sorts_the_wrong_way(monkeypatch): + """Sortedness is what makes the form canonical rather than merely consistent, and it is the + one invariant the other controls cannot reach: a reverse-sorting encoder is deterministic, + order-independent and stable through a round trip, and only this check sees it.""" + monkeypatch.setattr( + properties, + "canonical_bytes", + lambda payload: json.dumps( + dict(sorted(payload.items(), reverse=True)), separators=(",", ":") + ).encode(), + ) + with pytest.raises(AssertionError, match="keys are not sorted"): + for document in _plain_documents(): + properties.check_canonical(document) + + +def test_the_canonical_check_catches_an_error_outside_the_closed_set(monkeypatch): + """`InvalidArgument` is the contract. Anything else reaching a caller is a crash nobody + classified, and `errors.py` calls its set closed.""" + + def raising(payload): + raise TypeError("not JSON serializable") + + monkeypatch.setattr(properties, "canonical_bytes", raising) + with pytest.raises(AssertionError, match="outside the closed error set"): + properties.check_canonical({"a": 1}) + + +def test_the_closed_set_check_covers_every_call_and_not_just_the_first(monkeypatch): + """The finding a positive control turned up while this file was being written: an encoder + broken only on its *second* call raised `TypeError` straight out of `check_canonical`, + because the later calls were bare. Every call goes through `_encode` now, and this is what + holds it there.""" + calls = {"n": 0} + + def late(payload): + calls["n"] += 1 + if calls["n"] > 1: + raise TypeError("broken on every call but the first") + return json.dumps(payload, sort_keys=True, separators=(",", ":")).encode() + + monkeypatch.setattr(properties, "canonical_bytes", late) + with pytest.raises(AssertionError, match="outside the closed error set"): + properties.check_canonical({"a": 1, "b": 2}) + + +def test_the_policy_check_catches_a_loader_that_raises_something_else(monkeypatch): + """`PolicyError` is the closed contract. A `KeyError` reaching a caller still denies, but + through a crash nobody classified -- and a caller catching `PolicyError` would not catch it.""" + + def raising(text, *, source=""): + raise KeyError("actions") + + monkeypatch.setattr(properties, "policy_from_yaml", raising) + with pytest.raises(AssertionError, match="KeyError"): + properties.check_policy("schema: ctrlrun.policy/v3") + + +def test_the_policy_check_catches_an_unstable_policy_hash(monkeypatch): + """Two parses of one document disagreeing about the hash would make `policy_hash` useless + as the answer to "what decided this action".""" + real = properties.policy_from_yaml + calls = {"n": 0} + + def drifting(text, *, source=""): + policy = real(text, source=source) + calls["n"] += 1 + if calls["n"] % 2 == 0: + object.__setattr__(policy, "_canonical", {"drifted": calls["n"]}) + return policy + + monkeypatch.setattr(properties, "policy_from_yaml", drifting) + with pytest.raises(AssertionError, match="two policy hashes"): + properties.check_policy((REPO_ROOT / "ctrlrun.example.yaml").read_text(encoding="utf-8")) + + +# --- 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_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.""" + from ctrlrun import Action, Principal + from ctrlrun.errors import CTRLRunError + + arguments = json.loads('{"note": "\\ud800"}') + assert arguments == {"note": "\ud800"} + + action = Action( + name="pay", environment="prod", principal=Principal(agent="a"), arguments=arguments + ) + 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 + + +def test_the_seed_corpus_reproduces_every_known_finding(): + """A finding whose reproducer is not in the corpus is a finding the next campaign has to + rediscover by luck.""" + reproduced = set() + for path in sorted((FUZZ / "corpus" / "canonical").iterdir()): + if path.is_file(): + found = properties.check_canonical(properties.document_from_bytes(path.read_bytes())) + if found: + reproduced.add(found) + assert reproduced == set(properties.KNOWN_FINDINGS), ( + f"corpus reproduces {reproduced}, recorded findings are {set(properties.KNOWN_FINDINGS)}" + ) + + +# --- the Atheris entry points ------------------------------------------------------------------- + + +@pytest.mark.parametrize("name", ["fuzz_canonical.py", "fuzz_policy.py"]) +def test_each_target_is_an_atheris_harness(name): + """Scorecard detects Python fuzzing by finding `import atheris` in a `.py` file, which is a + thing a decorative file could also do. These assert the harness is real: it instruments, + it feeds the decoder, and it hands control to the fuzzer.""" + source = (FUZZ / name).read_text(encoding="utf-8") + assert "import atheris" in source + assert "atheris.Setup(" in source and "atheris.Fuzz()" in source + assert "instrument_all" in source or "instrument_imports" in source + assert "properties." in source, "the harness asserts nothing of its own" + + +@pytest.mark.parametrize("name", ["fuzz_canonical.py", "fuzz_policy.py"]) +def test_each_target_runs_its_property_without_atheris_installed(name): + """The targets are importable and runnable as plain scripts against the seed corpus, so a + contributor with no fuzzing toolchain -- and this suite -- can still execute them. A target + that only runs under Atheris is a target that runs in one place, once a week.""" + result = subprocess.run( + [sys.executable, str(FUZZ / name), "--corpus"], + capture_output=True, + text=True, + cwd=REPO_ROOT, + ) + assert result.returncode == 0, result.stderr + assert "inputs" in result.stdout + + +# --- packaging and CI --------------------------------------------------------------------------- + + +def test_the_fuzz_corpus_is_not_packaged(): + """`fuzz/` is a development tree like `research/`. The wheel and the sdist carry neither.""" + manifest = (REPO_ROOT / "MANIFEST.in").read_text(encoding="utf-8") + # Line-exact: `prune fuzz-disabled` contains `prune fuzz`, and a substring check passed it. + assert "prune fuzz" in manifest.splitlines() + + +def test_ci_actually_runs_the_fuzzers(): + """A fuzz target nothing executes is a file that satisfies a scanner. This asserts a job + exists, that it runs both targets, and that it bounds them -- an unbounded `atheris.Fuzz()` + in CI hangs until the job timeout, which reads as a broken build and not as a finding.""" + import yaml + + workflow = yaml.safe_load((REPO_ROOT / ".github" / "workflows" / "fuzz.yml").read_text()) + job = workflow["jobs"]["fuzz"] + steps = {str(step.get("name", "")): str(step.get("run", "")) for step in job["steps"]} + + # The gate runs both targets without Atheris, so a toolchain problem cannot silence them. + gate = steps["The seed corpus still holds"] + # The campaign is the Atheris run, and is where a missing bound or a dropped target hides: + # both names also appear in the gate, so searching the whole job proves nothing. + campaign = steps["Campaign"] + for target in ("fuzz_canonical.py", "fuzz_policy.py"): + assert f"python fuzz/{target} --corpus" in gate, target + bounded = [ + line for line in campaign.splitlines() if target in line and "-max_total_time=" in line + ] + assert bounded, f"{target} is not run under a time bound in the campaign" + assert "atheris" in steps["Install Atheris"], "the campaign would run without Atheris" + assert workflow["permissions"] == {"contents": "read"} From e3425faf06a355a723a2290f324e0620d31d8bb7 Mon Sep 17 00:00:00 2001 From: arpan Date: Sun, 6 Sep 2026 22:42:36 +0530 Subject: [PATCH 2/2] The campaign was instrumented by nothing, and the test asserted the string that said it was CI's fuzz job passed: green tick, 9,788,746 executions on canonicalization and 5,074,499 on the policy loader. `stat::new_units_added: 0` on both is what gave it away. A coverage-guided fuzzer that finds no new coverage unit in ten million runs is not coverage-guided. `import properties` sat at module scope, so by the time `atheris.instrument_imports()` ran the module was already in `sys.modules` and the instrumented re-import was a no-op. Nothing was instrumented -- not `properties`, and not `ctrlrun.action` underneath it, which is the code the campaign exists to cover. It ran blind random input at full speed and reported success, which is worse than not running: the job's green tick was evidence for a claim nothing supported. Both harnesses now import `properties` for the first time inside the instrumentation block, so the hook applies to it and transitively to `ctrlrun`. **And the test that should have caught it asserted the string.** `"instrument_imports" in source` was true of the broken version -- the call was right there, doing nothing. It is an AST walk now: no `import properties` at module scope, and an `import properties` inside a `with` whose context manager is `instrument_imports`. A structural property, because the difference between the two versions is structural and invisible to a substring. Two mutations, two caught: reintroducing the top-level import, and removing the instrumentation block. Neither is detectable by the check this replaces. --- fuzz/fuzz_canonical.py | 19 ++++++++++++++++-- fuzz/fuzz_policy.py | 19 ++++++++++++++++-- tests/test_fuzzing.py | 45 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/fuzz/fuzz_canonical.py b/fuzz/fuzz_canonical.py index c8e8209a..6c4f21df 100644 --- a/fuzz/fuzz_canonical.py +++ b/fuzz/fuzz_canonical.py @@ -14,12 +14,20 @@ import sys from pathlib import Path +from typing import Any FUZZ = Path(__file__).resolve().parent if str(FUZZ) not in sys.path: sys.path.insert(0, str(FUZZ)) -import properties # noqa: E402 +# **`properties` is deliberately not imported here.** `atheris.instrument_imports()` hooks the +# import system and instruments what is imported *inside* it, transitively -- so `properties`, +# and through it `ctrlrun.action`, has to reach the interpreter for the first time in that +# block. An import at module scope puts it in `sys.modules` first and makes the instrumented +# re-import a silent no-op: the campaign still runs, at full speed, guided by nothing at all. +# `stat::new_units_added: 0` after ten million executions is what that looks like in a CI log, +# and it is what the first version of this file did. +properties: Any = None def one_input(data: bytes) -> None: @@ -41,12 +49,19 @@ def _run_corpus() -> int: def main() -> int: if "--corpus" in sys.argv: + import properties as module + + globals()["properties"] = module return _run_corpus() import atheris + # The first import of `properties` in this process, so the instrumentation actually + # applies to it and to `ctrlrun` underneath it. with atheris.instrument_imports(): - import properties as instrumented # noqa: F401 + import properties as module + + globals()["properties"] = module atheris.Setup(sys.argv, one_input) atheris.Fuzz() diff --git a/fuzz/fuzz_policy.py b/fuzz/fuzz_policy.py index 9ab97941..60838724 100644 --- a/fuzz/fuzz_policy.py +++ b/fuzz/fuzz_policy.py @@ -14,12 +14,20 @@ import sys from pathlib import Path +from typing import Any FUZZ = Path(__file__).resolve().parent if str(FUZZ) not in sys.path: sys.path.insert(0, str(FUZZ)) -import properties # noqa: E402 +# **`properties` is deliberately not imported here.** `atheris.instrument_imports()` hooks the +# import system and instruments what is imported *inside* it, transitively -- so `properties`, +# and through it `ctrlrun.action`, has to reach the interpreter for the first time in that +# block. An import at module scope puts it in `sys.modules` first and makes the instrumented +# re-import a silent no-op: the campaign still runs, at full speed, guided by nothing at all. +# `stat::new_units_added: 0` after ten million executions is what that looks like in a CI log, +# and it is what the first version of this file did. +properties: Any = None def one_input(data: bytes) -> None: @@ -41,12 +49,19 @@ def _run_corpus() -> int: def main() -> int: if "--corpus" in sys.argv: + import properties as module + + globals()["properties"] = module return _run_corpus() import atheris + # The first import of `properties` in this process, so the instrumentation actually + # applies to it and to `ctrlrun` underneath it. with atheris.instrument_imports(): - import properties as instrumented # noqa: F401 + import properties as module + + globals()["properties"] = module atheris.Setup(sys.argv, one_input) atheris.Fuzz() diff --git a/tests/test_fuzzing.py b/tests/test_fuzzing.py index fdefd6a6..291d2c9a 100644 --- a/tests/test_fuzzing.py +++ b/tests/test_fuzzing.py @@ -17,6 +17,7 @@ from __future__ import annotations +import ast import json import subprocess import sys @@ -337,15 +338,53 @@ def test_the_seed_corpus_reproduces_every_known_finding(): @pytest.mark.parametrize("name", ["fuzz_canonical.py", "fuzz_policy.py"]) def test_each_target_is_an_atheris_harness(name): """Scorecard detects Python fuzzing by finding `import atheris` in a `.py` file, which is a - thing a decorative file could also do. These assert the harness is real: it instruments, - it feeds the decoder, and it hands control to the fuzzer.""" + thing a decorative file could also do. These assert the harness is real: it instruments, it + feeds the decoder, and it hands control to the fuzzer.""" source = (FUZZ / name).read_text(encoding="utf-8") assert "import atheris" in source assert "atheris.Setup(" in source and "atheris.Fuzz()" in source - assert "instrument_all" in source or "instrument_imports" in source assert "properties." in source, "the harness asserts nothing of its own" +@pytest.mark.parametrize("name", ["fuzz_canonical.py", "fuzz_policy.py"]) +def test_the_target_under_test_is_imported_inside_the_instrumentation(name): + """The invariant behind `stat::new_units_added`, and the reason this is a structural check + and not `"instrument_imports" in source`. + + `atheris.instrument_imports()` instruments what is imported *inside* it. A module already + in `sys.modules` is not re-imported, so an `import properties` at file scope makes the + instrumented import a silent no-op -- and the campaign then runs at full speed, guided by + nothing, reporting a green job. The first version of this file did exactly that: ten + million executions in CI and `new_units_added: 0` on both targets. + + A string search could not tell the two apart, because the broken version contained the + string. This walks the tree instead.""" + tree = ast.parse((FUZZ / name).read_text(encoding="utf-8")) + + def imports_properties(node) -> bool: + return isinstance(node, ast.Import) and any(a.name == "properties" for a in node.names) + + top_level = [n for n in tree.body if imports_properties(n)] + assert top_level == [], ( + "`import properties` at module scope puts it in sys.modules before Atheris can " + "instrument it; move it inside the `instrument_imports()` block" + ) + + instrumented = [ + node + for parent in ast.walk(tree) + if isinstance(parent, ast.With) + and any( + isinstance(item.context_expr, ast.Call) + and getattr(item.context_expr.func, "attr", None) == "instrument_imports" + for item in parent.items + ) + for node in ast.walk(parent) + if imports_properties(node) + ] + assert instrumented, "nothing is imported inside instrument_imports(); nothing is instrumented" + + @pytest.mark.parametrize("name", ["fuzz_canonical.py", "fuzz_policy.py"]) def test_each_target_runs_its_property_without_atheris_installed(name): """The targets are importable and runnable as plain scripts against the seed corpus, so a