From 6412242b7b378ac70298f38ab2228346054f36a3 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Mon, 31 Aug 2026 10:35:29 +1000 Subject: [PATCH 01/12] docs(wire-format): scope compressed-byte reproducibility per-vector (LAB-1751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The large_compressible pair pins lz4_flex's 15 B block; the spec's own reference liblz4 mapping emits a valid 14 B block for the same input (encode-only divergence, decode correct — found by execution in the LAB-868 panel review). Regeneration rejected: every SDK compresses via cachekit-core's lz4_flex, whose CI asserts re-encode byte-identity, so re-pinning to liblz4 would break the canonical writer and merely swap which compressor diverges. Remediation (path b): spec/wire-format.md gains a 'Compressed-byte reproducibility' section — compressed bytes are not canonical across conforming encoders (interop-v2 doctrine, LAB-1135), conformance for compressed_data is read-side only, writers are never byte-compared against fixtures, and large_compressible is marked known encode-divergent / decode-verified only. wire-format-reference.py verify gains an optional liblz4 decode-conformance leg (dep already installed in CI) plus --require-extras, passed in verify.yml's optional-deps step, so dependency drift cannot silently disable the deeper checks. Fixture bytes untouched (1.1.1) — no SDK re-vendors. Expert panel (high stakes) findings applied: OverflowError/MemoryError from lz4.block.decompress converted to the guarded AssertionError so a poisoned vector fails itself, not the run (mutation-tested both ways); --require-extras closes the silent-optional gap; doctrine prose deduplicated per catchphrase cut list. --- .github/workflows/verify.yml | 2 +- CHANGELOG.md | 27 ++++++++++++++ spec/wire-format.md | 45 ++++++++++++++++++++++- tools/wire-format-reference.py | 66 +++++++++++++++++++++++++++------- 4 files changed, 126 insertions(+), 14 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 5911736..6dd0f5c 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -38,7 +38,7 @@ jobs: python3 tools/interop-reference.py verify python3 tools/interop-v2-reference.py verify python3 tools/encryption-verify.py --require-seal - python3 tools/wire-format-reference.py verify + python3 tools/wire-format-reference.py verify --require-extras - name: JS cross-check (independent encoder + @noble/hashes + WebCrypto) run: | diff --git a/CHANGELOG.md b/CHANGELOG.md index 27235e5..7058781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,33 @@ All notable changes to the CacheKit Protocol Specification. ## [Unreleased] +### Wire format — compressed-byte reproducibility scoped per-vector (LAB-1751) + +- LZ4 compressed bytes are **not canonical** across conforming block encoders. + [`spec/wire-format.md`](spec/wire-format.md) now states this explicitly + (new "Compressed-byte reproducibility" section, mirroring interop v2's + doctrine): `compressed_data` conformance is read-side only, writers are + never validated by byte-comparing compressor output against fixtures, and + only the canonical writer (`lz4_flex` via `cachekit-core`) has enforced + byte-reproducibility. The `large_compressible` / `large_compressible_bin` + pair is marked **known encode-divergent, decode-verified only** under the + spec's own reference liblz4 mapping — `lz4.block.compress(store_size=False)` + emits a 14 B block where the fixture pins `lz4_flex`'s 15 B. Found by + execution during the LAB-868 panel review; resolves the trust bug of a + fixture implying a reproducibility property the reference toolchain cannot + produce. Regeneration was rejected: every SDK compresses through + `cachekit-core`'s `lz4_flex`, whose CI asserts re-encode byte-identity, so + re-pinning to liblz4 output would break the canonical writer and merely swap + which compressor diverges. +- [`tools/wire-format-reference.py`](tools/wire-format-reference.py) `verify` + gains an optional `lz4` leg (the dependency was already installed in CI's + optional-deps step): liblz4 MUST decompress every pinned `compressed_data` + to the pinned input; encoder agreement with the pin is reported per vector + but never asserted. The CI invocation now passes `--require-extras` + (precedent: `encryption-verify.py --require-seal`) so a dependency drift + cannot silently turn the deeper checks off. Fixture bytes untouched + (version stays 1.1.1) — no downstream SDK re-vendors required. + ### Interop v2 — compressed-values profile (DRAFT) - New [`spec/interop-v2.md`](spec/interop-v2.md) (LAB-1135, protocol#52): diff --git a/spec/wire-format.md b/spec/wire-format.md index b1a18c6..19c7147 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -40,7 +40,11 @@ This document specifies two layers: vendors the file sha256-pinned in `tests/wire_format_vectors.rs`, asserting decode byte-identity for every vector and re-encode byte-identity for the canonical `*_bin` vectors only — legacy array-of-integers vectors are - decode-only, retained as legacy-read proof. + decode-only, retained as legacy-read proof. Byte-canonicity scopes to the + envelope's MessagePack encoding and to the **canonical writer's** output: + the LZ4 bytes inside `compressed_data` are not reproducible across + conforming compressors — see + [Compressed-byte reproducibility](#compressed-byte-reproducibility-per-vector-scoping). 2. **[SDK storage containers](#sdk-storage-containers-auto-mode)** — what each SDK *actually stores* in a backend in default (auto) mode. These differ per SDK, are **SDK-internal**, and are documented here so their bytes are identifiable — not so @@ -248,6 +252,45 @@ the generic shortest-width selection property at fixture level, while > [!WARNING] > **PHP**: Standard `php-ext-lz4`'s `lz4_compress()` is **not compliant** — it prepends a proprietary 4-byte size header. Use `lz4_compress_raw()` from the forked extension at `27Bslash6/php-ext-lz4`. +### Compressed-byte reproducibility (per-vector scoping) + +The LZ4 block **format** is fixed, but conforming **encoders** are not: the +format constrains only what a block must decompress to, so two compliant +compressors may legally emit different bytes for the same input. Compressed +bytes are therefore +**not canonical**, and conformance for `compressed_data` is **read-side**: + +- A conforming reader MUST decompress every pinned vector's `compressed_data` + to its pinned input. +- A writer is NOT required to reproduce the pinned compressed bytes, and + MUST NOT be conformance-tested by byte-comparing its compressor output + against the fixture — validate a writer by decoding its envelopes per the + [Retrieve Flow](#retrieve-flow) and checking its MessagePack encoding against + [Byte Layout](#byte-layout-canonical-encoding). + +This is the same doctrine [interop v2](interop-v2.md) records for its +compressed-values profile. The pinned bytes are the **canonical implementation's** output +(`lz4_flex` via `cachekit-core`), and only that writer's reproducibility is +enforced — by the re-encode byte-identity assertions in +`cachekit-core/tests/wire_format_vectors.rs`. The reference liblz4 mapping +above (`lz4.block`) is **decode-verified against every vector** in this repo's +CI (`tools/wire-format-reference.py verify`, optional `lz4` leg); on encode it +happens to reproduce six of the seven pairs byte-for-byte, which is an +observation, not a guarantee. + +> [!NOTE] +> **Known encode divergence — `large_compressible` / `large_compressible_bin` +> (decode-verified only).** For this pair's input (1024 × `'A'`), liblz4 +> (observed at 1.9.4 via `python-lz4` 4.4.5) emits a **14-byte** block where +> the fixture pins `lz4_flex`'s **15-byte** block. The blocks differ only in +> the end-of-block match/literal split: `lz4_flex` ends the long match one byte +> earlier and emits six trailing literals (`… e9 60` + `41`×6) where liblz4 +> emits five (`… ea 50` + `41`×5). Both are valid LZ4 blocks and both +> decompress to the input; the divergence is encode-only. A third-party writer +> following the Library Mapping will therefore produce a different — equally +> conforming — envelope for this input. (LAB-1751; found by execution during +> the LAB-868 panel review.) + --- ## Checksum: xxHash3-64 diff --git a/tools/wire-format-reference.py b/tools/wire-format-reference.py index 7fa1a3a..d8e37be 100644 --- a/tools/wire-format-reference.py +++ b/tools/wire-format-reference.py @@ -1,12 +1,12 @@ #!/usr/bin/env python3 """Reference encoder/verifier for the ByteStorage envelope (spec/wire-format.md). -Stdlib-only (one optional extra, see below). Scope: the **MessagePack encoding** +Stdlib-only (optional extras, see below). Scope: the **MessagePack encoding** of the StorageEnvelope positional array — both the legacy element[0] encoding (array of integers, pre-1.1 writers) and the canonical one (msgpack `bin`, -protocol 1.1+ writers). LZ4 decompression and xxHash3-64 recomputation are NOT -verified here (neither is stdlib); that enforcement lives in cachekit-core's CI -(`tests/wire_format_vectors.rs`, LAB-423). +protocol 1.1+ writers). xxHash3-64 recomputation is NOT verified here (not +stdlib); byte-level enforcement for the canonical writer lives in +cachekit-core's CI (`tests/wire_format_vectors.rs`, LAB-423). What `verify` proves, for every vector pair in ../test-vectors/wire-format.json: 1. Codec fidelity — decoding a legacy vector and re-encoding it in legacy form @@ -23,11 +23,20 @@ Usage: python3 tools/wire-format-reference.py verify # default + python3 tools/wire-format-reference.py verify --require-extras + # fail if optional deps are + # missing (CI optional-deps leg) python3 tools/wire-format-reference.py generate # (re)derive *_bin vectors -One optional-dependency check deepens `verify` when importable (runs in CI): +Two optional-dependency checks deepen `verify` when importable (both run in CI): - `msgpack`: third-encoder conformance — msgpack-python re-encodes both forms from decoded fields and must reproduce the pinned bytes byte-identically. + - `lz4`: C-implementation (liblz4) DECODE conformance — liblz4 must + decompress every vector's pinned compressed_data to the pinned input. + Compressed bytes are not canonical across conforming LZ4 block encoders + (spec/wire-format.md 'Compressed-byte reproducibility', LAB-1751), so + encoder agreement with the pinned lz4_flex bytes is reported per vector + but never asserted — liblz4 is known to diverge on `large_compressible`. """ from __future__ import annotations @@ -257,7 +266,7 @@ def generate() -> int: return 0 -def _verify_vector(base: dict, bins: dict, msgpack) -> str: +def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: """Validate one legacy vector against its bin twin (popped from `bins`). Returns the one-line size-delta summary on success, or raises @@ -315,11 +324,30 @@ def _verify_vector(base: dict, bins: dict, msgpack) -> str: "msgpack-python legacy re-encode mismatch" ) + # optional: liblz4 DECODE-only conformance — encode agreement reported, never + # asserted; see module docstring / spec 'Compressed-byte reproducibility' (LAB-1751). + lz4_note = "" + if lz4_block is not None: + inp = bytes.fromhex(base["input_hex"]) + try: + got = lz4_block.decompress(data, uncompressed_size=size) + except (lz4_block.LZ4BlockError, OverflowError, MemoryError) as e: + # convert to the guarded type so a bad vector (corrupt stream, or an + # oversized size that liblz4 rejects/pre-allocates) fails itself, not the run + raise AssertionError(f"liblz4 rejects pinned compressed_data: {e}") from e + assert got == inp, "liblz4 does not decompress pinned compressed_data to the input" + theirs = lz4_block.compress(inp, store_size=False) + lz4_note = ( + "; liblz4 decode ok, encode reproduces pin" + if theirs == data + else f"; liblz4 decode ok, encode diverges ({len(theirs)} B vs {len(data)} B pinned — decode-verified only)" + ) + delta = len(new_env) - len(old_env) - return f"legacy {len(old_env)} B -> bin {len(new_env)} B ({delta:+d} B)" + return f"legacy {len(old_env)} B -> bin {len(new_env)} B ({delta:+d} B){lz4_note}" -def verify() -> int: +def verify(require_extras: bool = False) -> int: fixture = _load() legacy, bins = _split_vectors(fixture) if not legacy: @@ -333,12 +361,23 @@ def verify() -> int: import msgpack # type: ignore[import-untyped] except ImportError: msgpack = None + try: + import lz4.block as lz4_block # type: ignore[import-untyped] + except ImportError: + lz4_block = None + if require_extras and (msgpack is None or lz4_block is None): + # CI's optional-deps leg passes --require-extras so a dependency drift + # cannot silently turn the deeper conformance checks off (exit-0 with + # a "stdlib-only" banner would be an unflagged loss of coverage). + missing = [n for n, mod in (("msgpack", msgpack), ("lz4", lz4_block)) if mod is None] + print(f"FAIL: --require-extras set but not importable: {', '.join(missing)}", file=sys.stderr) + return 1 failures = 0 for base in legacy: name = base["name"] try: - print(f" ok {name}: {_verify_vector(base, bins, msgpack)}") + print(f" ok {name}: {_verify_vector(base, bins, msgpack, lz4_block)}") except (AssertionError, ValueError, IndexError, KeyError) as e: # Per-vector isolation: a malformed vector (truncated hex, missing # field) fails only itself with a named FAIL line, not the whole run. @@ -349,7 +388,8 @@ def verify() -> int: failures += 1 print(f" FAIL {orphan}: bin vector without a legacy base", file=sys.stderr) - conformance = "with msgpack-python conformance" if msgpack else "stdlib-only" + extras = [label for label, mod in (("msgpack-python", msgpack), ("liblz4 decode", lz4_block)) if mod] + conformance = "with " + " + ".join(extras) + " conformance" if extras else "stdlib-only" if failures: print(f"FAIL: {failures} failure(s) ({conformance})", file=sys.stderr) return 1 @@ -358,11 +398,13 @@ def verify() -> int: def main() -> int: - cmd = sys.argv[1] if len(sys.argv) > 1 else "verify" + args = [a for a in sys.argv[1:] if a != "--require-extras"] + require_extras = "--require-extras" in sys.argv[1:] + cmd = args[0] if args else "verify" if cmd == "generate": return generate() if cmd == "verify": - return verify() + return verify(require_extras=require_extras) print(__doc__, file=sys.stderr) return 2 From a0643c5aeca5d089332cf3490dd7e8f274cb8659 Mon Sep 17 00:00:00 2001 From: Mark S Date: Mon, 31 Aug 2026 16:51:05 +1000 Subject: [PATCH 02/12] fix(reference): validate original_size against the spec's 512 MiB limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit, PR #57. The stronger argument is not the OOM: spec/wire-format.md's decode sequence validates original_size <= 512 MiB at step 4, BEFORE step 6 decompresses, and this file is the spec's executable witness — it was running step 6 without step 4. The reference implementation now implements the sequence it documents. The OOM path is real but narrower than the finding claims. Reaching the liblz4 decompress with an oversized size means defeating three earlier guards (size vs input_size, twin field drift, bin re-encode byte-identity), so it takes a fully coherent fixture — the shape a bad regeneration produces, not a one-field tamper. Verified by building exactly that fixture: base and twin envelopes re-encoded with original_size at 512 MiB + 1, input_size matching. Before, that handed lz4.block.decompress a 512 MiB allocation bound; now it fails as "original_size 536870913 exceeds the spec's 536870912 B limit". Also scoped the CHANGELOG's SDK claim: cachekit-rs writes plain MessagePack with no envelope (spec 'Per-SDK'), so "every SDK compresses through lz4_flex" was overstated. Now "every envelope-using SDK". Verify still passes all 7 vector pairs with msgpack-python + liblz4. Refs LAB-1751 --- CHANGELOG.md | 5 +++-- tools/wire-format-reference.py | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7058781..15e1c66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,9 @@ All notable changes to the CacheKit Protocol Specification. emits a 14 B block where the fixture pins `lz4_flex`'s 15 B. Found by execution during the LAB-868 panel review; resolves the trust bug of a fixture implying a reproducibility property the reference toolchain cannot - produce. Regeneration was rejected: every SDK compresses through - `cachekit-core`'s `lz4_flex`, whose CI asserts re-encode byte-identity, so + produce. Regeneration was rejected: every envelope-using SDK compresses + through `cachekit-core`'s `lz4_flex` (`cachekit-rs` writes plain MessagePack + with no envelope — spec 'Per-SDK'), whose CI asserts re-encode byte-identity, so re-pinning to liblz4 output would break the canonical writer and merely swap which compressor diverges. - [`tools/wire-format-reference.py`](tools/wire-format-reference.py) `verify` diff --git a/tools/wire-format-reference.py b/tools/wire-format-reference.py index d8e37be..d55dd9a 100644 --- a/tools/wire-format-reference.py +++ b/tools/wire-format-reference.py @@ -48,6 +48,12 @@ FIXTURE_PATH = Path(__file__).resolve().parent.parent / "test-vectors" / "wire-format.json" FIXTURE_VERSION = "1.1.1" +# spec/wire-format.md 'Size Limits' — 512 MiB, and the decode sequence validates +# original_size against it BEFORE decompressing (step 4, ahead of step 6). This +# file is the spec's executable witness, so it has to run that step too: liblz4 +# pre-allocates uncompressed_size, so a fixture whose original_size was mutated +# upward gets the run OOM-killed rather than failing the vector by name. +MAX_UNCOMPRESSED_SIZE = 536_870_912 ENVELOPE_FORMAT = ( "MessagePack positional array (rmp_serde::to_vec): " "[compressed_data, checksum, original_size, format]. Vectors without an " @@ -329,6 +335,9 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: lz4_note = "" if lz4_block is not None: inp = bytes.fromhex(base["input_hex"]) + assert size <= MAX_UNCOMPRESSED_SIZE, ( + f"original_size {size} exceeds the spec's {MAX_UNCOMPRESSED_SIZE} B limit" + ) try: got = lz4_block.decompress(data, uncompressed_size=size) except (lz4_block.LZ4BlockError, OverflowError, MemoryError) as e: From d131e449e8053681d588f233e1549d5a5af3c914 Mon Sep 17 00:00:00 2001 From: Mark S Date: Mon, 31 Aug 2026 16:56:21 +1000 Subject: [PATCH 03/12] fix(reference): raise instead of assert for the 512 MiB bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kody (critical, team rule "Don't Use assert for Data Validation") on the line added in a0643c5. Correct, and for a sharper reason than the rule states. Every other check in _verify_vector is an assert, and that is fine for them: they are conformance checks, so if `python -O` strips them the tool verifies nothing and the silence is self-announcing. A memory-safety bound behaves differently under -O — it disappears while the tool still looks like it works, right up to the point an oversized fixture takes the process out. Same keyword, opposite failure mode, which is why the blanket rule lands hardest on exactly this line. ValueError is already in verify()'s per-vector guard, so the named FAIL line and the per-vector isolation are unchanged. Verified: all 7 vector pairs pass; the coherent-mutation fixture still fails as ValueError("original_size 536870913 exceeds the spec's 536870912 B limit"); and that failure now survives `python -OO`, which it did not before. Refs LAB-1751 --- tools/wire-format-reference.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/wire-format-reference.py b/tools/wire-format-reference.py index d55dd9a..1644851 100644 --- a/tools/wire-format-reference.py +++ b/tools/wire-format-reference.py @@ -335,9 +335,15 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: lz4_note = "" if lz4_block is not None: inp = bytes.fromhex(base["input_hex"]) - assert size <= MAX_UNCOMPRESSED_SIZE, ( - f"original_size {size} exceeds the spec's {MAX_UNCOMPRESSED_SIZE} B limit" - ) + # `raise`, not `assert`, unlike every conformance check around it: `python -O` + # strips asserts. For the conformance checks that is self-announcing — the + # tool verifies nothing and says so. A memory-safety bound stripped under -O + # looks fine right up to the point an oversized fixture takes the process out. + # ValueError is in verify()'s per-vector guard, so the named FAIL line is kept. + if size > MAX_UNCOMPRESSED_SIZE: + raise ValueError( + f"original_size {size} exceeds the spec's {MAX_UNCOMPRESSED_SIZE} B limit" + ) try: got = lz4_block.decompress(data, uncompressed_size=size) except (lz4_block.LZ4BlockError, OverflowError, MemoryError) as e: From c662d86e69bbec0e0aaaf01a7aea4d666aa10e33 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Mon, 31 Aug 2026 18:16:51 +1000 Subject: [PATCH 04/12] fix(reference): close three fail-open paths in the wire-format verifier (LAB-1751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expert-panel review (crypto/protocol gate — the diff touches the ByteStorage wire format). Each finding was reproduced by poisoning test-vectors/wire-format.json and re-confirmed after the fix; the fixture itself is byte-unchanged. 1. original_size drift was undetectable. lz4.block.decompress(uncompressed_size=N) sizes the output buffer, it does not assert the decoded length -- verified: decompress(compress(b'hello world 1234'), uncompressed_size=100000) returns 16 bytes without error. The pre-existing guard compared original_size against input_size, and both live IN the file under test, so they drift together. A vector declaring 100,000,000 for 16 bytes of real input verified green while printing "liblz4 decode ok". Now checked against len(input_hex), the only field the pinned bytes derive from, and placed outside the optional-deps gate so spec decode step 9 runs on the stdlib leg too. 2. python -O stripped every check. All conformance checks here are asserts, so an optimised run printed "all 7 vector pairs verified" against a poisoned fixture. verify() now refuses to run when __debug__ is false. 3. --require-extras failed open on a typo. Unrecognised args were dropped, so `verify --require-extra` exited 0 with the extras legs off -- the exact silent coverage loss the flag was added to prevent. Unknown args now exit 2. Also: pin the liblz4 encode-divergence set (LZ4_ENCODE_DIVERGENT) and assert it, so a toolchain bump that changes which vectors diverge fails CI instead of quietly making the new spec section's prose wrong; and stop catching MemoryError as a per-vector conformance failure, since that would hide a host OOM. spec/wire-format.md, same panel: - Scope "a writer MUST NOT be conformance-tested by byte-comparing its compressor output" to non-canonical writers. Unscoped, it forbade the cachekit-core re-encode assertions the next paragraph relies on as the enforcement mechanism -- the fleet's only detector for an unintended lz4_flex change. - Scope the cachekit-core enforcement claim to the vectors that repo vendors: it pins version == "1.1.0", so width_boundary_bin16 (added at 1.1.1) has no encode-side check anywhere today. Recorded in the spec; closed by re-vendoring. Verified: all verify.yml legs green (stdlib + optional-deps python, both node cross-checks), generate is a no-op, fixture byte-identical. --- CHANGELOG.md | 30 ++++++++++++ spec/wire-format.md | 23 +++++++--- tools/wire-format-reference.py | 83 ++++++++++++++++++++++++++-------- 3 files changed, 109 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15e1c66..7fbc5fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,36 @@ All notable changes to the CacheKit Protocol Specification. (precedent: `encryption-verify.py --require-seal`) so a dependency drift cannot silently turn the deeper checks off. Fixture bytes untouched (version stays 1.1.1) — no downstream SDK re-vendors required. +- Expert-panel hardening of the same verifier (crypto/protocol gate; every item + below was reproduced by poisoning the fixture and re-run after the fix): + - `original_size` is now checked against `len(input_hex)`, not just the + co-located `input_size` field. Both declared sizes live *in* the file under + test, so a regeneration bug that inflates them drifts them together and the + old check still passed — a vector declaring 100 MB for 16 bytes of input + verified green, and liblz4 did not catch it because + `decompress(uncompressed_size=…)` sizes the output buffer rather than + asserting the length. Runs on both CI legs (stdlib and optional-deps). + - `verify` refuses to run under `-O`/`PYTHONOPTIMIZE`: every conformance check + is an `assert`, so an optimised run reported "all 7 vector pairs verified" + against a poisoned fixture. + - Unrecognised arguments now exit 2 instead of being dropped, closing a + fail-open in the new flag itself: `verify --require-extra` (one character + short) exited 0 with the extras legs silently off. + - The set of vectors liblz4 fails to reproduce on encode is pinned in + `LZ4_ENCODE_DIVERGENT` and asserted, so a toolchain bump that changes it + fails CI instead of quietly making the new spec section's prose wrong. + - `MemoryError` is no longer caught as a per-vector conformance failure (it is + a host signal, and relabelling it would hide an OOM). +- [`spec/wire-format.md`](spec/wire-format.md) corrections from the same panel: + the "MUST NOT byte-compare a writer's compressor output" rule is scoped to + **non-canonical** writers — unscoped, it forbade the `cachekit-core` re-encode + assertions that the very next paragraph relies on as the enforcement + mechanism, i.e. the fleet's only `lz4_flex` drift detector. The claim that + cachekit-core enforces canonical-writer reproducibility is now scoped to the + vectors that repo actually vendors: core pins `version == "1.1.0"`, so + `width_boundary_bin16` (added at 1.1.1) currently has no encode-side check + anywhere — recorded in the spec, closed by re-vendoring 1.1.1 into + cachekit-core. ### Interop v2 — compressed-values profile (DRAFT) diff --git a/spec/wire-format.md b/spec/wire-format.md index 19c7147..42a6ce3 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -262,21 +262,30 @@ bytes are therefore - A conforming reader MUST decompress every pinned vector's `compressed_data` to its pinned input. -- A writer is NOT required to reproduce the pinned compressed bytes, and - MUST NOT be conformance-tested by byte-comparing its compressor output - against the fixture — validate a writer by decoding its envelopes per the +- A writer **other than the canonical `lz4_flex` writer** is NOT required to + reproduce the pinned compressed bytes, and MUST NOT be conformance-tested by + byte-comparing its compressor output against the fixture — validate such a + writer by decoding its envelopes per the [Retrieve Flow](#retrieve-flow) and checking its MessagePack encoding against - [Byte Layout](#byte-layout-canonical-encoding). + [Byte Layout](#byte-layout-canonical-encoding). The carve-out below is + deliberate: byte-comparing the *canonical* writer is the fleet's only + detector for an unintended `lz4_flex` behaviour change, so it stays. This is the same doctrine [interop v2](interop-v2.md) records for its compressed-values profile. The pinned bytes are the **canonical implementation's** output (`lz4_flex` via `cachekit-core`), and only that writer's reproducibility is enforced — by the re-encode byte-identity assertions in -`cachekit-core/tests/wire_format_vectors.rs`. The reference liblz4 mapping +`cachekit-core/tests/wire_format_vectors.rs`, **for the vectors present in the +fixture that repo vendors**. That matters today: cachekit-core vendors 1.1.0 +and pins `version == "1.1.0"`, so `width_boundary_bin16` (added at 1.1.1) is +not yet covered by any encode-side check anywhere — re-vendoring 1.1.1 into +cachekit-core closes that gap. The reference liblz4 mapping above (`lz4.block`) is **decode-verified against every vector** in this repo's CI (`tools/wire-format-reference.py verify`, optional `lz4` leg); on encode it -happens to reproduce six of the seven pairs byte-for-byte, which is an -observation, not a guarantee. +reproduces every pair except `large_compressible` byte-for-byte, which is an +observation, not a guarantee — but one this repo's CI pins (see +`LZ4_ENCODE_DIVERGENT`), so a toolchain change that alters the divergent set +fails CI rather than quietly making this paragraph wrong. > [!NOTE] > **Known encode divergence — `large_compressible` / `large_compressible_bin` diff --git a/tools/wire-format-reference.py b/tools/wire-format-reference.py index 1644851..246b05e 100644 --- a/tools/wire-format-reference.py +++ b/tools/wire-format-reference.py @@ -20,6 +20,9 @@ bin form reproduces the `*_bin` bytes exactly. 3. Documented size bound — a bin envelope is never more than 1 byte larger than its legacy twin (the header-arithmetic bound stated in the spec). + 4. Declared sizes match ground truth — `original_size` equals the real length + of `input_hex`, not merely the co-located `input_size` field (both declared + sizes live in the file under test, so they drift together). Usage: python3 tools/wire-format-reference.py verify # default @@ -35,8 +38,13 @@ decompress every vector's pinned compressed_data to the pinned input. Compressed bytes are not canonical across conforming LZ4 block encoders (spec/wire-format.md 'Compressed-byte reproducibility', LAB-1751), so - encoder agreement with the pinned lz4_flex bytes is reported per vector - but never asserted — liblz4 is known to diverge on `large_compressible`. + encoder agreement is never a pass/fail criterion per vector. What IS + asserted is that the set of divergent vectors still equals + LZ4_ENCODE_DIVERGENT, so a toolchain bump cannot silently invalidate the + spec's statement of which vectors diverge. + +`verify` refuses to run under -O/PYTHONOPTIMIZE: every check above is an +`assert`, so an optimised run would report a pass having tested nothing. """ from __future__ import annotations @@ -48,12 +56,16 @@ FIXTURE_PATH = Path(__file__).resolve().parent.parent / "test-vectors" / "wire-format.json" FIXTURE_VERSION = "1.1.1" -# spec/wire-format.md 'Size Limits' — 512 MiB, and the decode sequence validates -# original_size against it BEFORE decompressing (step 4, ahead of step 6). This -# file is the spec's executable witness, so it has to run that step too: liblz4 -# pre-allocates uncompressed_size, so a fixture whose original_size was mutated -# upward gets the run OOM-killed rather than failing the vector by name. -MAX_UNCOMPRESSED_SIZE = 536_870_912 +# spec/wire-format.md 'Size Limits' step 4 — liblz4 pre-allocates uncompressed_size, +# so an inflated original_size must be rejected by name rather than sized into RAM. +# Only this one bound; the envelope/compressed_data length caps and the 1000:1 bomb +# check are a reader's obligations, not this fixture verifier's. +MAX_UNCOMPRESSED_SIZE = 512 * 1024 * 1024 +# spec/wire-format.md 'Compressed-byte reproducibility' names WHICH vectors liblz4 +# fails to reproduce on encode. Encoder agreement is not a conformance rule, but the +# spec's claim about the set is a fact, so a change to it must fail CI and force the +# text to be re-read — otherwise the next `lz4==` bump rots the spec silently. +LZ4_ENCODE_DIVERGENT = frozenset({"large_compressible"}) ENVELOPE_FORMAT = ( "MessagePack positional array (rmp_serde::to_vec): " "[compressed_data, checksum, original_size, format]. Vectors without an " @@ -290,6 +302,15 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: assert len(old_env) == base["envelope_size"], "envelope_size mismatch" assert fmt == base["format"], "format field mismatch" assert size == base["input_size"], "original_size != input_size" + # ...and against ground truth. original_size and input_size both live IN the + # file under test, so a regeneration bug that inflates them drifts them + # together and the line above still passes (LAB-903's lesson). input_hex is + # the only field the pinned bytes are actually derived from, so it is the + # one to measure against. Stdlib, and outside the optional-deps gate below, + # so spec decode step 9 (data.length == original_size) runs on both CI legs. + assert size == len(bytes.fromhex(base["input_hex"])), ( + "original_size != len(input_hex)" + ) # 2. twin equivalence twin = bins.pop(base["name"] + "_bin", None) @@ -335,10 +356,7 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: lz4_note = "" if lz4_block is not None: inp = bytes.fromhex(base["input_hex"]) - # `raise`, not `assert`, unlike every conformance check around it: `python -O` - # strips asserts. For the conformance checks that is self-announcing — the - # tool verifies nothing and says so. A memory-safety bound stripped under -O - # looks fine right up to the point an oversized fixture takes the process out. + # `raise`, not `assert`: this is a memory bound, and -O strips asserts. # ValueError is in verify()'s per-vector guard, so the named FAIL line is kept. if size > MAX_UNCOMPRESSED_SIZE: raise ValueError( @@ -346,16 +364,24 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: ) try: got = lz4_block.decompress(data, uncompressed_size=size) - except (lz4_block.LZ4BlockError, OverflowError, MemoryError) as e: - # convert to the guarded type so a bad vector (corrupt stream, or an - # oversized size that liblz4 rejects/pre-allocates) fails itself, not the run + except (lz4_block.LZ4BlockError, OverflowError) as e: + # convert to the guarded type so a corrupt stream fails itself, not the + # run. MemoryError is deliberately NOT caught: it is a host signal, and + # relabelling it as a per-vector conformance failure would hide an OOM. raise AssertionError(f"liblz4 rejects pinned compressed_data: {e}") from e assert got == inp, "liblz4 does not decompress pinned compressed_data to the input" theirs = lz4_block.compress(inp, store_size=False) + diverges = theirs != data + assert diverges == (base["name"] in LZ4_ENCODE_DIVERGENT), ( + f"liblz4 encode-divergence set changed: {base['name']} " + f"{'now diverges from' if diverges else 'now reproduces'} the pin — " + "update spec/wire-format.md 'Compressed-byte reproducibility' and " + "LZ4_ENCODE_DIVERGENT together" + ) lz4_note = ( - "; liblz4 decode ok, encode reproduces pin" - if theirs == data - else f"; liblz4 decode ok, encode diverges ({len(theirs)} B vs {len(data)} B pinned — decode-verified only)" + f"; liblz4 decode ok, encode diverges ({len(theirs)} B vs {len(data)} B pinned — decode-verified only)" + if diverges + else "; liblz4 decode ok, encode reproduces pin" ) delta = len(new_env) - len(old_env) @@ -363,6 +389,15 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: def verify(require_extras: bool = False) -> int: + if not __debug__: + # Every conformance check in this file is an `assert`, so -O strips all of + # them and an optimised run prints "all N vector pairs verified" having + # verified nothing. Refuse instead of reporting a vacuous pass. + print( + "FAIL: assertions disabled (-O / PYTHONOPTIMIZE) — verify proves nothing", + file=sys.stderr, + ) + return 1 fixture = _load() legacy, bins = _split_vectors(fixture) if not legacy: @@ -413,8 +448,16 @@ def verify(require_extras: bool = False) -> int: def main() -> int: - args = [a for a in sys.argv[1:] if a != "--require-extras"] - require_extras = "--require-extras" in sys.argv[1:] + argv = sys.argv[1:] + require_extras = "--require-extras" in argv + args = [a for a in argv if a != "--require-extras"] + # Reject anything unrecognised rather than dropping it. `--require-extras` is + # matched by exact string and it gates the deepest coverage, so a silently + # ignored `--require-extra` typo used to exit 0 with the extras legs off — + # the exact fail-open the flag was added to close. + if len(args) > 1 or any(a.startswith("-") for a in args): + print(__doc__, file=sys.stderr) + return 2 cmd = args[0] if args else "verify" if cmd == "generate": return generate() From 9e58292f724a9d7caf6bcafa040a55c9e51aa347 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Mon, 31 Aug 2026 18:17:54 +1000 Subject: [PATCH 05/12] docs(wire-format): version-stamp the canonical writer beside the liblz4 pin (LAB-1751) Panel MIN: the encode-divergence NOTE pinned liblz4's version but not lz4_flex's, while the section's own doctrine is that encoder output is version-dependent. large_compressible's 15 B pin comes from cachekit-core v0.2.0 per the fixture generator field. --- spec/wire-format.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/wire-format.md b/spec/wire-format.md index 42a6ce3..8681546 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -291,7 +291,10 @@ fails CI rather than quietly making this paragraph wrong. > **Known encode divergence — `large_compressible` / `large_compressible_bin` > (decode-verified only).** For this pair's input (1024 × `'A'`), liblz4 > (observed at 1.9.4 via `python-lz4` 4.4.5) emits a **14-byte** block where -> the fixture pins `lz4_flex`'s **15-byte** block. The blocks differ only in +> the fixture pins the **15-byte** block emitted by `lz4_flex` as shipped in +> `cachekit-core` v0.2.0 (this vector's generator). Both sides are +> version-stamped deliberately: encoder output is version-dependent, which is +> the whole reason compressed bytes are not canonical. The blocks differ only in > the end-of-block match/literal split: `lz4_flex` ends the long match one byte > earlier and emits six trailing literals (`… e9 60` + `41`×6) where liblz4 > emits five (`… ea 50` + `41`×5). Both are valid LZ4 blocks and both From 7dbc1c463d65e8408c2a4f53c4bb86bb8892589f Mon Sep 17 00:00:00 2001 From: Mark S Date: Mon, 31 Aug 2026 20:05:12 +1000 Subject: [PATCH 06/12] fix(reference): refuse to run under -O, generate included (LAB-1751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The -O refusal added earlier guarded `verify` only. `generate` shares the same all-assert integrity model and is the path that *writes* test-vectors/wire-format.json — the fixture every SDK conforms against. Under -O its input checks vanish silently: a bin-encoded base vector was observed producing a garbage twin and exit 0, with the fixture rewritten. Hoisted the guard to main() so it covers every command rather than the one that happened to be audited. No command in this tool is meaningful with assertions stripped, so refusing before dispatch is both smaller and complete — it also removes the "which entry points did we remember?" question the per-function placement kept open. Added tools/test_wire_format_reference.py, mirroring the doctrine already written down for the version-floor guard: a guard with no mutation test degrades to reporting OK. It asserts both commands refuse under -O and -OO, that the refusal is the guard's and not an unrelated crash, and keeps a positive control so a guard that refuses everything cannot pass. Verified failing (3 cases) with the guard stripped. Kody flagged the assert-for-validation class on this file; this closes it at the choke point instead of rewriting 24 asserts into if/raise, which would have left the conformance failures indistinguishable from real errors in verify()'s per-vector guard. --- .github/workflows/verify.yml | 5 +++ tools/test_wire_format_reference.py | 63 +++++++++++++++++++++++++++++ tools/wire-format-reference.py | 21 +++++----- 3 files changed, 80 insertions(+), 9 deletions(-) create mode 100755 tools/test_wire_format_reference.py diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 6dd0f5c..c5d56be 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -27,6 +27,11 @@ jobs: - name: Python reference verify (stdlib only) run: | + # Runs first, same doctrine as the version-floor mutation suite below: + # every check in wire-format-reference.py is an assert, so -O turns + # `verify` into a vacuous pass and `generate` into an unvalidated + # fixture rewrite. Prove the refusal holds before trusting the verify. + python3 tools/test_wire_format_reference.py python3 tools/interop-reference.py verify python3 tools/interop-v2-reference.py verify python3 tools/encryption-verify.py diff --git a/tools/test_wire_format_reference.py b/tools/test_wire_format_reference.py new file mode 100755 index 0000000..dbbb089 --- /dev/null +++ b/tools/test_wire_format_reference.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python3 +"""Regression test for wire-format-reference.py's -O refusal. + +Every integrity check in that tool is an `assert`, so `python -O` strips all of +them: `verify` would report "all N vector pairs verified" having verified +nothing, and `generate` would rewrite the fixture every SDK conforms against +with its input checks removed. A one-line guard in `main()` is all that stands +between the tool and a vacuous pass — and a guard with no test is one refactor +away from being deleted by someone who cannot see what it holds up. + +Run: python3 tools/test_wire_format_reference.py (exit 1 on any failure) +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +TOOL = Path(__file__).resolve().parent / "wire-format-reference.py" + +# (name, python flags, argv, expected_exit) +CASES = [ + # Positive control: without -O the tool must still do its job, otherwise a + # guard that refuses everything would pass the two cases below. + ("verify, assertions on", [], ["verify"], 0), + # The regression itself: both commands must refuse, not just `verify`. + ("verify under -O", ["-O"], ["verify"], 1), + ("generate under -O", ["-O"], ["generate"], 1), + # -OO strips docstrings as well as asserts; same refusal must hold. + ("verify under -OO", ["-OO"], ["verify"], 1), +] + + +def main() -> int: + failures = [] + for name, flags, argv, expected in CASES: + proc = subprocess.run( + [sys.executable, *flags, str(TOOL), *argv], + capture_output=True, + text=True, + ) + ok = proc.returncode == expected + # An -O run must say why it refused; a bare non-zero exit could just as + # easily be an unrelated crash, which would let the guard rot unnoticed. + if ok and flags and "assertions disabled" not in proc.stderr: + ok = False + name += " (exited 1 but not via the guard)" + print(f" [{'ok' if ok else 'FAIL'}] {name}: expected exit {expected}, got {proc.returncode}") + if not ok: + failures.append(name) + + if failures: + print(f"\n{len(failures)} case(s) failed:", file=sys.stderr) + for f in failures: + print(f" - {f}", file=sys.stderr) + return 1 + print(f"\nall {len(CASES)} cases passed") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/wire-format-reference.py b/tools/wire-format-reference.py index 246b05e..cdb186d 100644 --- a/tools/wire-format-reference.py +++ b/tools/wire-format-reference.py @@ -389,15 +389,6 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: def verify(require_extras: bool = False) -> int: - if not __debug__: - # Every conformance check in this file is an `assert`, so -O strips all of - # them and an optimised run prints "all N vector pairs verified" having - # verified nothing. Refuse instead of reporting a vacuous pass. - print( - "FAIL: assertions disabled (-O / PYTHONOPTIMIZE) — verify proves nothing", - file=sys.stderr, - ) - return 1 fixture = _load() legacy, bins = _split_vectors(fixture) if not legacy: @@ -448,6 +439,18 @@ def verify(require_extras: bool = False) -> int: def main() -> int: + if not __debug__: + # Every integrity check in this file is an `assert`, so -O strips all of + # them. That makes `verify` print "all N vector pairs verified" having + # verified nothing, and lets `generate` write the fixture every SDK + # conforms against with its input checks removed. No command in this + # tool is meaningful with assertions off, so refuse before dispatch + # rather than emit a vacuous pass or an unvalidated fixture. + print( + "FAIL: assertions disabled (-O / PYTHONOPTIMIZE) — this tool proves nothing", + file=sys.stderr, + ) + return 1 argv = sys.argv[1:] require_extras = "--require-extras" in argv args = [a for a in argv if a != "--require-extras"] From aa0b1e0cb063c530f2ecffcfbceeaef347a72309 Mon Sep 17 00:00:00 2001 From: Mark S Date: Mon, 31 Aug 2026 20:20:18 +1000 Subject: [PATCH 07/12] fix(reference): close the generate deletion path; panel round 2 (LAB-1751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second expert panel on the current HEAD (crypto/protocol gate — this diff edits spec/wire-format.md). The gate keys off HEAD, not "a panel ran on this ticket once", and two commits had landed since the last one. CRIT — generate could permanently erase a committed vector, exit 0. `fixture["vectors"] = legacy + twins` rebuilt from the legacy set alone, so any vector that is not a derived twin was dropped with no diagnostic. The trap was baited: verify's orphan FAIL names `generate` as the remedy, so the documented repair step completed the data loss. Reproduced end to end — dropping legacy width_boundary_bin16 (the fleet's only bin16 coverage, and per this PR's own spec text already uncovered by any encode-side check) left generate reporting success on a fixture two vectors smaller, verify green, ready to be re-vendored by 4+ SDKs that sha256-pin this file. generate is now append-only: it refuses to write when the rebuild would lose a name. MAJ — the -O guard was bypassable by import. It sat in main(), so `exec_module(m); m.verify()` under -O printed a full pass having run zero asserts. Moved to module scope, which closes the CLI and the import path together; sibling tools reuse this envelope codec, so the import path is real. Spec and comment accuracy (an SDK author in another language reads these as contract): - The "MUST NOT byte-compare a non-canonical writer's compressor output" rule was over-broad. liblz4 reproduces 6 of 7 pins byte-for-byte, so read literally it told every liblz4-based SDK to delete a working drift detector — and it forbade exactly what this repo's own verifier does at LZ4_ENCODE_DIVERGENT. Now forbids the wrong *conclusion* (judging a writer non-conforming for differing bytes), explicitly allowing byte-comparison as a declared-divergence tripwire. - CHANGELOG restated that rule unscoped — the pre-fix wording the previous panel overturned, contradicting its own later bullet. - Two comments and the CHANGELOG claimed encoder agreement is "never asserted". It is, against the divergence set. A false comment on a gate is what the next maintainer trusts when deciding the assert is safe to relax. - A comment claimed the ground-truth assert made spec decode step 9 run on both CI legs. Step 9 compares decompressed length; the stdlib leg never decompresses. Same over-claim class trimmed once already in this ticket. - Scope stated cachekit-core's re-encode coverage unqualified, contradicting the 1.1.0/1.1.1 gap this diff documents 240 lines later. Scope is read first. - MAX_UNCOMPRESSED_SIZE was unreachable behind the ground-truth assert and inside the lz4-only branch; moved ahead of both so it fires on both legs. - 'Size Limits' / 'Per-SDK' section citations named sections that do not exist ('Security Limits', 'SDK Storage Containers (auto mode)'). `--require-extras` was accepted and ignored on generate — the same accepted-and-dropped fail-open the unrecognised-arg check exists to close. Now exit 2. Mutation suite extended to 11 cases across three guard classes, each verified failing with its guard stripped; scratch-tree mirroring keeps the fixture out of reach. Both CI legs run green locally, liblz4 divergence exactly as the spec NOTE states (14 B vs 15 B on large_compressible). Fixture bytes untouched. Trimmed ~25 lines of duplicated normative prose across spec, docstring and workflow: each doctrine was written out four or five times, and the copies had already started contradicting each other. --- .github/workflows/verify.yml | 6 +- CHANGELOG.md | 32 ++++-- spec/wire-format.md | 28 +++-- tools/test_wire_format_reference.py | 161 ++++++++++++++++++++++------ tools/wire-format-reference.py | 100 ++++++++++------- 5 files changed, 235 insertions(+), 92 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index c5d56be..a267e98 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -27,10 +27,8 @@ jobs: - name: Python reference verify (stdlib only) run: | - # Runs first, same doctrine as the version-floor mutation suite below: - # every check in wire-format-reference.py is an assert, so -O turns - # `verify` into a vacuous pass and `generate` into an unvalidated - # fixture rewrite. Prove the refusal holds before trusting the verify. + # Mutation suite first, same doctrine as the version-floor guard below: + # prove the fail-closed guards still fail before trusting the verify. python3 tools/test_wire_format_reference.py python3 tools/interop-reference.py verify python3 tools/interop-v2-reference.py verify diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fbc5fb..91baac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,9 @@ All notable changes to the CacheKit Protocol Specification. - LZ4 compressed bytes are **not canonical** across conforming block encoders. [`spec/wire-format.md`](spec/wire-format.md) now states this explicitly (new "Compressed-byte reproducibility" section, mirroring interop v2's - doctrine): `compressed_data` conformance is read-side only, writers are - never validated by byte-comparing compressor output against fixtures, and + doctrine): `compressed_data` conformance is read-side only, a **non-canonical** + writer is never judged non-conforming for differing from the pinned bytes + (byte-comparison as a declared-divergence tripwire remains allowed), and only the canonical writer (`lz4_flex` via `cachekit-core`) has enforced byte-reproducibility. The `large_compressible` / `large_compressible_bin` pair is marked **known encode-divergent, decode-verified only** under the @@ -20,14 +21,14 @@ All notable changes to the CacheKit Protocol Specification. fixture implying a reproducibility property the reference toolchain cannot produce. Regeneration was rejected: every envelope-using SDK compresses through `cachekit-core`'s `lz4_flex` (`cachekit-rs` writes plain MessagePack - with no envelope — spec 'Per-SDK'), whose CI asserts re-encode byte-identity, so + with no envelope — spec 'SDK Storage Containers (auto mode)'), whose CI asserts re-encode byte-identity, so re-pinning to liblz4 output would break the canonical writer and merely swap which compressor diverges. - [`tools/wire-format-reference.py`](tools/wire-format-reference.py) `verify` gains an optional `lz4` leg (the dependency was already installed in CI's optional-deps step): liblz4 MUST decompress every pinned `compressed_data` - to the pinned input; encoder agreement with the pin is reported per vector - but never asserted. The CI invocation now passes `--require-extras` + to the pinned input; encoder agreement is asserted only as a set-level drift + tripwire against `LZ4_ENCODE_DIVERGENT`, never as a per-vector conformance rule. The CI invocation now passes `--require-extras` (precedent: `encryption-verify.py --require-seal`) so a dependency drift cannot silently turn the deeper checks off. Fixture bytes untouched (version stays 1.1.1) — no downstream SDK re-vendors required. @@ -40,17 +41,28 @@ All notable changes to the CacheKit Protocol Specification. verified green, and liblz4 did not catch it because `decompress(uncompressed_size=…)` sizes the output buffer rather than asserting the length. Runs on both CI legs (stdlib and optional-deps). - - `verify` refuses to run under `-O`/`PYTHONOPTIMIZE`: every conformance check - is an `assert`, so an optimised run reported "all 7 vector pairs verified" - against a poisoned fixture. + - **Both** commands refuse to run under `-O`/`PYTHONOPTIMIZE`: every conformance + check is an `assert`, so an optimised `verify` reported "all 7 vector pairs + verified" against a poisoned fixture, and an optimised `generate` rewrote the + fixture with its input checks stripped. The guard is at module scope, not in + `main()`, because importing the module (sibling tools reuse this envelope + codec) walked straight past a CLI-only guard. + - `generate` is now **append-only**: it refuses to write when the rebuild would + drop a committed vector. It previously rebuilt `vectors` from the legacy set + alone, so a bin vector with no legacy base was erased silently — and because + `verify`'s orphan FAIL names `generate` as the remedy, the documented repair + step completed the data loss. Reproduced end to end: dropping legacy + `width_boundary_bin16` (the fleet's only bin16 coverage) left `generate` + reporting success on a fixture two vectors smaller, with CI green. + - `--require-extras` is rejected outside `verify` (exit 2). It was accepted and + silently ignored on `generate`, the fixture-writing path — the same + accepted-and-dropped fail-open the unrecognised-argument check closes. - Unrecognised arguments now exit 2 instead of being dropped, closing a fail-open in the new flag itself: `verify --require-extra` (one character short) exited 0 with the extras legs silently off. - The set of vectors liblz4 fails to reproduce on encode is pinned in `LZ4_ENCODE_DIVERGENT` and asserted, so a toolchain bump that changes it fails CI instead of quietly making the new spec section's prose wrong. - - `MemoryError` is no longer caught as a per-vector conformance failure (it is - a host signal, and relabelling it would hide an OOM). - [`spec/wire-format.md`](spec/wire-format.md) corrections from the same panel: the "MUST NOT byte-compare a writer's compressor output" rule is scoped to **non-canonical** writers — unscoped, it forbade the `cachekit-core` re-encode diff --git a/spec/wire-format.md b/spec/wire-format.md index 8681546..1b5e186 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -40,7 +40,10 @@ This document specifies two layers: vendors the file sha256-pinned in `tests/wire_format_vectors.rs`, asserting decode byte-identity for every vector and re-encode byte-identity for the canonical `*_bin` vectors only — legacy array-of-integers vectors are - decode-only, retained as legacy-read proof. Byte-canonicity scopes to the + decode-only, retained as legacy-read proof. That re-encode assertion covers + only the vectors the pinned file contains (core currently vendors 1.1.0; see + [Compressed-byte reproducibility](#compressed-byte-reproducibility-per-vector-scoping) + for the resulting gap). Byte-canonicity scopes to the envelope's MessagePack encoding and to the **canonical writer's** output: the LZ4 bytes inside `compressed_data` are not reproducible across conforming compressors — see @@ -263,20 +266,23 @@ bytes are therefore - A conforming reader MUST decompress every pinned vector's `compressed_data` to its pinned input. - A writer **other than the canonical `lz4_flex` writer** is NOT required to - reproduce the pinned compressed bytes, and MUST NOT be conformance-tested by - byte-comparing its compressor output against the fixture — validate such a + reproduce the pinned compressed bytes, and MUST NOT be judged non-conforming + because its compressor output differs from the fixture — validate such a writer by decoding its envelopes per the [Retrieve Flow](#retrieve-flow) and checking its MessagePack encoding against - [Byte Layout](#byte-layout-canonical-encoding). The carve-out below is - deliberate: byte-comparing the *canonical* writer is the fleet's only - detector for an unintended `lz4_flex` behaviour change, so it stays. + [Byte Layout](#byte-layout-canonical-encoding). +- A writer MAY still byte-compare its compressor output against the pins as a + **drift tripwire**, provided the expected divergences are declared per vector + rather than treated as failures. This repo's own verifier does exactly that + (`LZ4_ENCODE_DIVERGENT` in `tools/wire-format-reference.py`), and it is how + the *canonical* writer's byte-reproducibility stays enforced — the fleet's + only detector for an unintended `lz4_flex` behaviour change. This is the same doctrine [interop v2](interop-v2.md) records for its -compressed-values profile. The pinned bytes are the **canonical implementation's** output -(`lz4_flex` via `cachekit-core`), and only that writer's reproducibility is -enforced — by the re-encode byte-identity assertions in -`cachekit-core/tests/wire_format_vectors.rs`, **for the vectors present in the -fixture that repo vendors**. That matters today: cachekit-core vendors 1.1.0 +compressed-values profile. The pinned bytes are the **canonical implementation's** +output (`lz4_flex` via `cachekit-core`), enforced by the re-encode byte-identity +assertions in `cachekit-core/tests/wire_format_vectors.rs` — **but only for the +vectors present in the fixture that repo vendors**. That matters today: cachekit-core vendors 1.1.0 and pins `version == "1.1.0"`, so `width_boundary_bin16` (added at 1.1.1) is not yet covered by any encode-side check anywhere — re-vendoring 1.1.1 into cachekit-core closes that gap. The reference liblz4 mapping diff --git a/tools/test_wire_format_reference.py b/tools/test_wire_format_reference.py index dbbb089..aaa6c7d 100755 --- a/tools/test_wire_format_reference.py +++ b/tools/test_wire_format_reference.py @@ -1,61 +1,160 @@ #!/usr/bin/env python3 -"""Regression test for wire-format-reference.py's -O refusal. +"""Mutation tests for wire-format-reference.py's fail-closed guards. -Every integrity check in that tool is an `assert`, so `python -O` strips all of -them: `verify` would report "all N vector pairs verified" having verified -nothing, and `generate` would rewrite the fixture every SDK conforms against -with its input checks removed. A one-line guard in `main()` is all that stands -between the tool and a vacuous pass — and a guard with no test is one refactor -away from being deleted by someone who cannot see what it holds up. +Two classes, both proven reachable by execution rather than argued from reading +(LAB-903: do not reason about a conformance gate, poison the fixture and watch it): + + 1. -O refusal. Every integrity check in that tool is an `assert`, so `python -O` + strips all of them: `verify` would report "all N vector pairs verified" having + verified nothing, and `generate` would rewrite the fixture every SDK conforms + against with its input checks removed. The guard sits at MODULE scope, not in + `main()`, because importing the module walks straight past a CLI-only guard. + + 2. generate's append-only refusal. Rebuilding `vectors` from the legacy set alone + silently drops any committed vector that is not a derived twin — and `verify`'s + orphan FAIL names `generate` as the remedy, so the repair step completed the + data loss. test-vectors/wire-format.json is vendored and sha256-pinned by 4+ + SDKs; a deletion here is invisible until an SDK's coverage has already shrunk. + +A guard with no mutation test is one refactor away from being deleted by someone +who cannot see what it holds up. Run: python3 tools/test_wire_format_reference.py (exit 1 on any failure) """ from __future__ import annotations +import json +import shutil import subprocess import sys +import tempfile from pathlib import Path -TOOL = Path(__file__).resolve().parent / "wire-format-reference.py" +HERE = Path(__file__).resolve().parent +TOOL = HERE / "wire-format-reference.py" +FIXTURE = HERE.parent / "test-vectors" / "wire-format.json" -# (name, python flags, argv, expected_exit) -CASES = [ - # Positive control: without -O the tool must still do its job, otherwise a - # guard that refuses everything would pass the two cases below. - ("verify, assertions on", [], ["verify"], 0), - # The regression itself: both commands must refuse, not just `verify`. - ("verify under -O", ["-O"], ["verify"], 1), - ("generate under -O", ["-O"], ["generate"], 1), - # -OO strips docstrings as well as asserts; same refusal must hold. - ("verify under -OO", ["-OO"], ["verify"], 1), -] +# A vector whose legacy base is dropped by a bad merge, leaving an orphan twin. +# LAB-868's width-boundary vector: the only bin16 coverage in the fleet. +ORPHANED_BASE = "width_boundary_bin16" -def main() -> int: +def _run(flags: list[str], argv: list[str], tool: Path = TOOL) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, *flags, str(tool), *argv], capture_output=True, text=True + ) + + +def _scratch(tmp: Path, drop: str | None = None) -> Path: + """Mirror tool + fixture into a scratch tree so mutations never touch the repo.""" + (tmp / "tools").mkdir() + (tmp / "test-vectors").mkdir() + shutil.copy(TOOL, tmp / "tools" / TOOL.name) + fixture = json.loads(FIXTURE.read_text()) + if drop: + fixture["vectors"] = [v for v in fixture["vectors"] if v["name"] != drop] + (tmp / "test-vectors" / FIXTURE.name).write_text(json.dumps(fixture, indent=2) + "\n") + return tmp / "tools" / TOOL.name + + +def check_optimised_refusals() -> list[str]: + """-O must stop every entry point, including `import`.""" failures = [] - for name, flags, argv, expected in CASES: - proc = subprocess.run( - [sys.executable, *flags, str(TOOL), *argv], - capture_output=True, - text=True, - ) + # Positive control: without -O the tool must still work, otherwise a guard that + # refuses everything would pass every case below. + cases = [ + ("verify, assertions on", [], ["verify"], 0), + ("verify under -O", ["-O"], ["verify"], 1), + ("generate under -O", ["-O"], ["generate"], 1), + ("verify under -OO", ["-OO"], ["verify"], 1), + ] + for name, flags, argv, expected in cases: + proc = _run(flags, argv) ok = proc.returncode == expected - # An -O run must say why it refused; a bare non-zero exit could just as - # easily be an unrelated crash, which would let the guard rot unnoticed. + # An -O run must say why it refused; a bare non-zero exit could be an + # unrelated crash, which would let the guard rot behind a passing test. if ok and flags and "assertions disabled" not in proc.stderr: - ok = False - name += " (exited 1 but not via the guard)" + ok, name = False, name + " (exited 1 but not via the guard)" + print(f" [{'ok' if ok else 'FAIL'}] {name}: expected exit {expected}, got {proc.returncode}") + if not ok: + failures.append(name) + + # The guard is at module scope precisely so this path cannot skip it. + probe = "import importlib.util as u;s=u.spec_from_file_location('w',r'%s');m=u.module_from_spec(s);s.loader.exec_module(m);print('RAN',m.verify())" + proc = subprocess.run( + [sys.executable, "-O", "-c", probe % TOOL], capture_output=True, text=True + ) + ok = proc.returncode != 0 and "assertions disabled" in proc.stderr + print(f" [{'ok' if ok else 'FAIL'}] import under -O refuses: got exit {proc.returncode}") + if not ok: + failures.append("import under -O bypassed the guard") + return failures + + +def check_generate_is_append_only() -> list[str]: + """generate must refuse to drop a committed vector, not silently erase it.""" + failures = [] + with tempfile.TemporaryDirectory() as td: + tmp = Path(td) + tool = _scratch(tmp, drop=ORPHANED_BASE) + before = json.loads((tmp / "test-vectors" / FIXTURE.name).read_text()) + proc = _run([], ["generate"], tool=tool) + after = json.loads((tmp / "test-vectors" / FIXTURE.name).read_text()) + + refused = proc.returncode == 1 and "REFUSED" in proc.stderr + print(f" [{'ok' if refused else 'FAIL'}] generate refuses to drop a committed vector: exit {proc.returncode}") + if not refused: + failures.append("generate did not refuse to drop a committed vector") + + # The refusal must be a no-op on disk, not a refusal after the write. + untouched = before == after + print(f" [{'ok' if untouched else 'FAIL'}] refusal left the fixture byte-untouched") + if not untouched: + failures.append("generate mutated the fixture despite refusing") + + # Positive control: on an intact fixture, generate is still a working no-op. + tool2 = _scratch(Path(tempfile.mkdtemp(dir=td))) + proc2 = _run([], ["generate"], tool=tool2) + ok2 = proc2.returncode == 0 + print(f" [{'ok' if ok2 else 'FAIL'}] generate still succeeds on an intact fixture: exit {proc2.returncode}") + if not ok2: + failures.append("generate broke on an intact fixture") + return failures + + +def check_flag_rejections() -> list[str]: + """A flag accepted-and-ignored on the fixture-writing path is a fail-open.""" + failures = [] + for name, argv, expected in [ + ("generate --require-extras rejected", ["generate", "--require-extras"], 2), + ("unknown command rejected", ["bogus"], 2), + ("typo'd flag rejected", ["verify", "--require-extra"], 2), + ]: + proc = _run([], argv) + ok = proc.returncode == expected print(f" [{'ok' if ok else 'FAIL'}] {name}: expected exit {expected}, got {proc.returncode}") if not ok: failures.append(name) + return failures + + +def main() -> int: + failures = [] + for label, check in ( + ("-O refusal", check_optimised_refusals), + ("generate append-only", check_generate_is_append_only), + ("flag rejection", check_flag_rejections), + ): + print(f"{label}:") + failures += check() if failures: print(f"\n{len(failures)} case(s) failed:", file=sys.stderr) for f in failures: print(f" - {f}", file=sys.stderr) return 1 - print(f"\nall {len(CASES)} cases passed") + print("\nall cases passed") return 0 diff --git a/tools/wire-format-reference.py b/tools/wire-format-reference.py index cdb186d..595a8c4 100644 --- a/tools/wire-format-reference.py +++ b/tools/wire-format-reference.py @@ -34,17 +34,17 @@ Two optional-dependency checks deepen `verify` when importable (both run in CI): - `msgpack`: third-encoder conformance — msgpack-python re-encodes both forms from decoded fields and must reproduce the pinned bytes byte-identically. - - `lz4`: C-implementation (liblz4) DECODE conformance — liblz4 must - decompress every vector's pinned compressed_data to the pinned input. - Compressed bytes are not canonical across conforming LZ4 block encoders - (spec/wire-format.md 'Compressed-byte reproducibility', LAB-1751), so - encoder agreement is never a pass/fail criterion per vector. What IS - asserted is that the set of divergent vectors still equals - LZ4_ENCODE_DIVERGENT, so a toolchain bump cannot silently invalidate the - spec's statement of which vectors diverge. - -`verify` refuses to run under -O/PYTHONOPTIMIZE: every check above is an -`assert`, so an optimised run would report a pass having tested nothing. + - `lz4`: C-implementation (liblz4) decode conformance — liblz4 must decompress + every vector's pinned compressed_data to the pinned input. Encoder agreement + is asserted only as a set-level drift tripwire against LZ4_ENCODE_DIVERGENT, + never as a per-vector conformance rule (spec 'Compressed-byte + reproducibility', LAB-1751). + +Both commands refuse to run under -O/PYTHONOPTIMIZE: every check in this file +is an `assert`, so an optimised `verify` would report a pass having tested +nothing, and an optimised `generate` would rewrite the fixture with its input +checks stripped. Guarded in `main()`; regression-tested by +tools/test_wire_format_reference.py. """ from __future__ import annotations @@ -56,8 +56,9 @@ FIXTURE_PATH = Path(__file__).resolve().parent.parent / "test-vectors" / "wire-format.json" FIXTURE_VERSION = "1.1.1" -# spec/wire-format.md 'Size Limits' step 4 — liblz4 pre-allocates uncompressed_size, -# so an inflated original_size must be rejected by name rather than sized into RAM. +# spec/wire-format.md 'Security Limits' (512 MiB table); the allocating consumer is +# 'Retrieve Flow' step 4. Checked on both CI legs, before the ground-truth compare, so +# an inflated original_size is rejected by name rather than sized into RAM by liblz4. # Only this one bound; the envelope/compressed_data length caps and the 1000:1 bomb # check are a reader's obligations, not this fixture verifier's. MAX_UNCOMPRESSED_SIZE = 512 * 1024 * 1024 @@ -65,6 +66,7 @@ # fails to reproduce on encode. Encoder agreement is not a conformance rule, but the # spec's claim about the set is a fact, so a change to it must fail CI and force the # text to be re-read — otherwise the next `lz4==` bump rots the spec silently. +# Base names only: twins carry the same compressed_data and are not iterated. LZ4_ENCODE_DIVERGENT = frozenset({"large_compressible"}) ENVELOPE_FORMAT = ( "MessagePack positional array (rmp_serde::to_vec): " @@ -274,8 +276,25 @@ def _bin_twin(base: dict) -> dict: def generate() -> int: fixture = _load() - legacy, _ = _split_vectors(fixture) - fixture["vectors"] = legacy + [_bin_twin(v) for v in legacy] + legacy, bins = _split_vectors(fixture) + rebuilt = legacy + [_bin_twin(v) for v in legacy] + # Append-only, as the fixture's own contract requires (LAB-783) and as the sibling + # python-frame-reference.py already enforces by upsert (LAB-1203). Rebuilding from + # the legacy set alone silently drops any committed vector that is not a derived + # twin, and `verify`'s orphan FAIL names `generate` as the remedy — so the repair + # step completes the data loss. Refuse instead: this file is vendored and + # sha256-pinned downstream, and a deletion here is invisible until an SDK's + # conformance coverage has already shrunk. + lost = {v["name"] for v in fixture["vectors"]} - {v["name"] for v in rebuilt} + if lost: + print( + f"REFUSED: generate would drop committed vector(s): {', '.join(sorted(lost))}. " + "A bin vector with no legacy base is not regenerable from the legacy set — " + "restore the missing base vector rather than regenerating.", + file=sys.stderr, + ) + return 1 + fixture["vectors"] = rebuilt fixture["envelope_format"] = ENVELOPE_FORMAT fixture["generator"] = GENERATOR fixture["version"] = FIXTURE_VERSION @@ -307,7 +326,15 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: # together and the line above still passes (LAB-903's lesson). input_hex is # the only field the pinned bytes are actually derived from, so it is the # one to measure against. Stdlib, and outside the optional-deps gate below, - # so spec decode step 9 (data.length == original_size) runs on both CI legs. + # so fixture self-consistency holds on both CI legs. This is NOT spec decode + # step 9 (data.length == original_size after decompression) — that compares + # decompressed output and only runs on the lz4 leg, as `got == inp` below. + if size > MAX_UNCOMPRESSED_SIZE: + # Before the compare, and before any decompression: an inflated declared + # size must fail by name on both legs, not be sized into RAM downstream. + raise ValueError( + f"original_size {size} exceeds the spec's {MAX_UNCOMPRESSED_SIZE} B limit" + ) assert size == len(bytes.fromhex(base["input_hex"])), ( "original_size != len(input_hex)" ) @@ -351,17 +378,12 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: "msgpack-python legacy re-encode mismatch" ) - # optional: liblz4 DECODE-only conformance — encode agreement reported, never - # asserted; see module docstring / spec 'Compressed-byte reproducibility' (LAB-1751). + # optional: liblz4 decode conformance. Encode agreement is asserted only against + # the declared LZ4_ENCODE_DIVERGENT set — a drift tripwire, never a per-vector + # conformance rule; see spec 'Compressed-byte reproducibility' (LAB-1751). lz4_note = "" if lz4_block is not None: inp = bytes.fromhex(base["input_hex"]) - # `raise`, not `assert`: this is a memory bound, and -O strips asserts. - # ValueError is in verify()'s per-vector guard, so the named FAIL line is kept. - if size > MAX_UNCOMPRESSED_SIZE: - raise ValueError( - f"original_size {size} exceeds the spec's {MAX_UNCOMPRESSED_SIZE} B limit" - ) try: got = lz4_block.decompress(data, uncompressed_size=size) except (lz4_block.LZ4BlockError, OverflowError) as e: @@ -439,18 +461,6 @@ def verify(require_extras: bool = False) -> int: def main() -> int: - if not __debug__: - # Every integrity check in this file is an `assert`, so -O strips all of - # them. That makes `verify` print "all N vector pairs verified" having - # verified nothing, and lets `generate` write the fixture every SDK - # conforms against with its input checks removed. No command in this - # tool is meaningful with assertions off, so refuse before dispatch - # rather than emit a vacuous pass or an unvalidated fixture. - print( - "FAIL: assertions disabled (-O / PYTHONOPTIMIZE) — this tool proves nothing", - file=sys.stderr, - ) - return 1 argv = sys.argv[1:] require_extras = "--require-extras" in argv args = [a for a in argv if a != "--require-extras"] @@ -462,6 +472,12 @@ def main() -> int: print(__doc__, file=sys.stderr) return 2 cmd = args[0] if args else "verify" + if require_extras and cmd != "verify": + # Same rule as the unrecognised-arg rejection above: a flag that is accepted + # and ignored on the fixture-WRITING path is the fail-open this guard exists + # to close. + print(f"FAIL: --require-extras is not valid for '{cmd}'", file=sys.stderr) + return 2 if cmd == "generate": return generate() if cmd == "verify": @@ -470,5 +486,17 @@ def main() -> int: return 2 +if not __debug__: + # Every integrity check in this file is an `assert`, so -O/-OO strips all of them: + # `verify` reports a pass having tested nothing and `generate` rewrites the fixture + # with its input checks gone. Module scope, not main(), because importing this + # module (sibling tools reuse its envelope codec) would otherwise walk straight + # past a CLI-only guard. Regression-tested by tools/test_wire_format_reference.py. + print( + "FAIL: assertions disabled (-O / PYTHONOPTIMIZE) — this tool proves nothing", + file=sys.stderr, + ) + sys.exit(1) + if __name__ == "__main__": sys.exit(main()) From 4c80bbfb91bbf98bf290f7ac600ce2f188667d88 Mon Sep 17 00:00:00 2001 From: Mark S Date: Mon, 31 Aug 2026 20:23:02 +1000 Subject: [PATCH 08/12] test(reference): bind the annotated case label separately (LAB-1751) CodeRabbit PLW2901: the guard-provenance annotation clobbered the loop variable mid-iteration. Separate binding; the case name stays the case name. S603/PLW1510 from the same review rebutted on the thread: no Ruff config or lint job exists in this repo, and the sibling suite CI already runs (tools/test_check_version_floors.py:92) has the identical subprocess.run shape, so suppressing here alone would only make the two inconsistent. --- tools/test_wire_format_reference.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/test_wire_format_reference.py b/tools/test_wire_format_reference.py index aaa6c7d..6079fcf 100755 --- a/tools/test_wire_format_reference.py +++ b/tools/test_wire_format_reference.py @@ -74,11 +74,12 @@ def check_optimised_refusals() -> list[str]: ok = proc.returncode == expected # An -O run must say why it refused; a bare non-zero exit could be an # unrelated crash, which would let the guard rot behind a passing test. + label = name if ok and flags and "assertions disabled" not in proc.stderr: - ok, name = False, name + " (exited 1 but not via the guard)" - print(f" [{'ok' if ok else 'FAIL'}] {name}: expected exit {expected}, got {proc.returncode}") + ok, label = False, f"{name} (exited {expected} but not via the guard)" + print(f" [{'ok' if ok else 'FAIL'}] {label}: expected exit {expected}, got {proc.returncode}") if not ok: - failures.append(name) + failures.append(label) # The guard is at module scope precisely so this path cannot skip it. probe = "import importlib.util as u;s=u.spec_from_file_location('w',r'%s');m=u.module_from_spec(s);s.loader.exec_module(m);print('RAN',m.verify())" From 491490ee0bfc3541b0a3c74e88df4328270676cf Mon Sep 17 00:00:00 2001 From: Mark S Date: Mon, 31 Aug 2026 20:28:15 +1000 Subject: [PATCH 09/12] fix(wire-format): separate the liblz4 tripwire from canonical enforcement (LAB-1751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit caught a real error in the previous commit's spec wording, and it was mine: the new bullet welded two different mechanisms into one sentence. LZ4_ENCODE_DIVERGENT compares *liblz4's* compressor output. It watches the reference mapping the spec names for divergence-set drift and cannot observe lz4_flex at all. Canonical-writer byte-reproducibility is enforced somewhere else entirely — cachekit-core's re-encode assertions, as the very next paragraph says. Calling the former "how the canonical writer's byte-reproducibility stays enforced" would have told an SDK author the fleet has a drift detector it does not have. Also from the same review: - docstring still said the -O guard was in main(); it moved to module scope last commit, which is the whole point (import cannot skip it). - generate() unpacked `bins` and never read it (RUF059); the append-only check works off the full committed name set, not the bin split. --- spec/wire-format.md | 8 +++++--- tools/wire-format-reference.py | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/spec/wire-format.md b/spec/wire-format.md index 1b5e186..d8ca73d 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -274,9 +274,11 @@ bytes are therefore - A writer MAY still byte-compare its compressor output against the pins as a **drift tripwire**, provided the expected divergences are declared per vector rather than treated as failures. This repo's own verifier does exactly that - (`LZ4_ENCODE_DIVERGENT` in `tools/wire-format-reference.py`), and it is how - the *canonical* writer's byte-reproducibility stays enforced — the fleet's - only detector for an unintended `lz4_flex` behaviour change. + (`LZ4_ENCODE_DIVERGENT` in `tools/wire-format-reference.py`) — which watches + the reference **liblz4** mapping for divergence-set drift, and cannot observe + `lz4_flex` at all. Canonical-writer byte-reproducibility is a separate + mechanism, enforced only by the re-encode assertions in `cachekit-core` + described below. This is the same doctrine [interop v2](interop-v2.md) records for its compressed-values profile. The pinned bytes are the **canonical implementation's** diff --git a/tools/wire-format-reference.py b/tools/wire-format-reference.py index 595a8c4..514bd74 100644 --- a/tools/wire-format-reference.py +++ b/tools/wire-format-reference.py @@ -43,8 +43,8 @@ Both commands refuse to run under -O/PYTHONOPTIMIZE: every check in this file is an `assert`, so an optimised `verify` would report a pass having tested nothing, and an optimised `generate` would rewrite the fixture with its input -checks stripped. Guarded in `main()`; regression-tested by -tools/test_wire_format_reference.py. +checks stripped. The guard is at module scope, so `import` cannot skip it +either; regression-tested by tools/test_wire_format_reference.py. """ from __future__ import annotations @@ -276,7 +276,7 @@ def _bin_twin(base: dict) -> dict: def generate() -> int: fixture = _load() - legacy, bins = _split_vectors(fixture) + legacy, _ = _split_vectors(fixture) rebuilt = legacy + [_bin_twin(v) for v in legacy] # Append-only, as the fixture's own contract requires (LAB-783) and as the sibling # python-frame-reference.py already enforces by upsert (LAB-1203). Rebuilding from From 534bf079c4dc08ce1b5da254062a21cce5498bd6 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Tue, 1 Sep 2026 02:24:47 +1000 Subject: [PATCH 10/12] fix(reference): close three whole-file fail-opens in the wire-format verifier (LAB-1751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expert-panel round 3 (crypto/protocol gate keys off current HEAD, not "a panel ran on this ticket once" — commits 4c80bbf and 491490e landed after round 2). All three findings exited 0 before the fix and are caught after; the mutation matrix is committed so they cannot rot back. The unifying defect: every existing check iterates the fixture's own vector list, so none of them can see a whole-file property. That is the original_size/input_size lesson one level up — a name list derived from the artifact under test pins nothing. - The base-vector set is now pinned in code (EXPECTED_BASE_VECTORS). Dropping a legacy base AND its _bin twin together — the realistic bad-merge shape, which the orphan-twin refusal does not cover — netted to zero in generate's append-only diff: verify reported "all 6 vector pairs verified" and generate WROTE the 12-vector fixture, both exit 0. It also silently disarmed LZ4_ENCODE_DIVERGENT, since the divergent vector stopped being iterated. - The fixture's declared `limits` block is now compared against the spec's Security Limits table. SDKs read their bounds from that block and nothing pinned it either way, so a fixture rewriting max_uncompressed_size to 1 verified green while handing every downstream reader a wrong bound. - A declared-divergent vector's compressed_data is now byte-pinned. `assert diverges == (name in LZ4_ENCODE_DIVERGENT)` is a one-bit check that any other valid LZ4 block satisfies, so re-pinning large_compressible to an unrelated, correctly-decompressing block passed both CI legs. The byte-pin sits OUTSIDE the optional-deps gate (same reasoning as the ground-truth compare) so the one vector this section exists to document is enforced on the stdlib leg too — it has no canonical-writer check anywhere else in the fleet. Harness: mutation cases for all three, each proven non-vacuous by deleting the guard and confirming the matching case fails. Its own invocations that can reach `generate` now run against a scratch mirror instead of the repo's sha256-pinned fixture — with the guard regressed, this suite (CI's first step) rewrote the vendored artifact. Exit-code-only assertions gained guard-marker checks: python exits 2 on a bad script path and 1 on a traceback, which made an exit-code-only case pass vacuously. Spec/CHANGELOG accuracy, same class as the two false claims round 2 caught: - Read-side conformance for compressed_data was fully satisfiable by a reader enforcing none of Security Limits. Every pinned vector is well-formed with a truthful original_size, so they evidence none of Retrieve Flow steps 4/5/9 and a reader omitting all three decompresses all of them. Now stated explicitly. - "width_boundary_bin16 is not yet covered by any encode-side check anywhere" was too broad: this repo asserts its legacy and bin re-encode byte-identity on every run, and liblz4 reproduces its compressed bytes on the optional leg. The real gap is narrower — no canonical-writer (lz4_flex) compressed-byte check, and its xxh3-64 checksum is recomputed nowhere. - The stated remedy failed on contact. Re-vendoring 1.1.1 into cachekit-core needs three changes, not one: bump FIXTURE_SHA256, bump the version pin, and relax `assert_eq!(twin_bytes[1], 0xc4)` to accept 0xc5 — that assertion requires every twin to be bin8 and width_boundary_bin16_bin is bin16 (303 B compressed_data), which is the vector's entire purpose. Verified against cachekit-core@main. A remedy that fails leaves the gap open longer. - Two comments credited the wrong mechanism: the sibling python-frame-reference uses the same refusal guard for its whole-fixture rebuild (its upsert applies only to the single-vector append mode), and nothing in the repo imports this module's codec — the -O guard's real justification is the harness's importlib probe and the sibling loader pattern. Cut: an unreachable, message-less `assert t_encoding == "bin"` and a dead `startswith("-")` disjunct whose job the arity check already does (57-combination argv sweep: zero divergence). Fixture byte-untouched (sha256 b902db88…, version stays 1.1.1) — no downstream SDK re-vendors. Both CI legs green, 22 harness cases, 10/10 mutations caught (7 escaped before), no new lint. --- CHANGELOG.md | 55 ++++++- spec/wire-format.md | 46 ++++-- tools/test_wire_format_reference.py | 239 +++++++++++++++++++++------- tools/wire-format-reference.py | 202 +++++++++++++++++++---- 4 files changed, 433 insertions(+), 109 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91baac2..7954064 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ All notable changes to the CacheKit Protocol Specification. - [`tools/wire-format-reference.py`](tools/wire-format-reference.py) `verify` gains an optional `lz4` leg (the dependency was already installed in CI's optional-deps step): liblz4 MUST decompress every pinned `compressed_data` - to the pinned input; encoder agreement is asserted only as a set-level drift + to the pinned input; encoder agreement is asserted only as a drift tripwire against `LZ4_ENCODE_DIVERGENT`, never as a per-vector conformance rule. The CI invocation now passes `--require-extras` (precedent: `encryption-verify.py --require-seal`) so a dependency drift cannot silently turn the deeper checks off. Fixture bytes untouched @@ -45,8 +45,9 @@ All notable changes to the CacheKit Protocol Specification. check is an `assert`, so an optimised `verify` reported "all 7 vector pairs verified" against a poisoned fixture, and an optimised `generate` rewrote the fixture with its input checks stripped. The guard is at module scope, not in - `main()`, because importing the module (sibling tools reuse this envelope - codec) walked straight past a CLI-only guard. + `main()`, because a CLI-only guard is bypassed by importing the module and + calling `verify()` directly — which the regression harness's `importlib` + probe does, and which is how the sibling tools load each other's codecs. - `generate` is now **append-only**: it refuses to write when the rebuild would drop a committed vector. It previously rebuilt `vectors` from the legacy set alone, so a bin vector with no legacy base was erased silently — and because @@ -63,6 +64,40 @@ All notable changes to the CacheKit Protocol Specification. - The set of vectors liblz4 fails to reproduce on encode is pinned in `LZ4_ENCODE_DIVERGENT` and asserted, so a toolchain bump that changes it fails CI instead of quietly making the new spec section's prose wrong. +- Second expert-panel round on the remediated verifier (crypto/protocol gate + keys off current HEAD, not "a panel ran once"). Three whole-file fail-opens, + all reproduced by execution and all previously exit-0: + - **The base-vector set is now pinned in code** (`EXPECTED_BASE_VECTORS`). + Every other check iterates the fixture's own vector list and so is + structurally blind to a vector that is simply *absent*. Dropping a legacy + base **and** its `_bin` twin together — the realistic bad-merge shape, which + the orphan-twin refusal does not cover — netted to zero in `generate`'s + append-only diff: `verify` reported "all 6 vector pairs verified" and + `generate` wrote the 12-vector fixture, both exit 0. It also silently + disarmed `LZ4_ENCODE_DIVERGENT`, since the divergent vector was no longer + iterated. Same lesson as `original_size`/`input_size` one level up: a name + list derived from the artifact under test pins nothing. + - **The fixture's declared `limits` block is now compared against the spec's + Security Limits table.** SDKs read their bounds from that block and nothing + pinned it either way, so a fixture rewriting `max_uncompressed_size` to `1` + verified green while handing every downstream reader a wrong bound. + - **A declared-divergent vector's `compressed_data` is now byte-pinned.** + `assert diverges == (name in LZ4_ENCODE_DIVERGENT)` is a one-bit check that + any other valid LZ4 block satisfies, so re-pinning `large_compressible` to + an unrelated (valid, correctly-decompressing) block passed both CI legs. The + byte-pin sits outside the optional-deps gate, so the one vector this section + exists to document is enforced on the stdlib leg too — it has no + canonical-writer check anywhere else in the fleet. + - `tools/test_wire_format_reference.py` gains mutation cases for all three, + each verified non-vacuous by deleting the guard and confirming the case + fails. Its own invocations that can reach `generate` now run against a + scratch mirror rather than the repo's sha256-pinned fixture — with the + guard regressed, the suite (CI's first step) rewrote the vendored artifact. + Exit-code-only assertions gained guard-marker checks, because python itself + exits 2 on a bad script path and 1 on a traceback, which made an + exit-code-only case pass vacuously. + - Fixture-shape rejections now name the offending vector instead of exiting + via a bare traceback. - [`spec/wire-format.md`](spec/wire-format.md) corrections from the same panel: the "MUST NOT byte-compare a writer's compressor output" rule is scoped to **non-canonical** writers — unscoped, it forbade the `cachekit-core` re-encode @@ -70,9 +105,17 @@ All notable changes to the CacheKit Protocol Specification. mechanism, i.e. the fleet's only `lz4_flex` drift detector. The claim that cachekit-core enforces canonical-writer reproducibility is now scoped to the vectors that repo actually vendors: core pins `version == "1.1.0"`, so - `width_boundary_bin16` (added at 1.1.1) currently has no encode-side check - anywhere — recorded in the spec, closed by re-vendoring 1.1.1 into - cachekit-core. + `width_boundary_bin16` (added at 1.1.1) has no **canonical-writer + (`lz4_flex`) compressed-byte** check anywhere in the fleet, and its pinned + xxh3-64 checksum is recomputed nowhere. The earlier phrasing — "no + encode-side check anywhere" — was too broad and is corrected: this repo's + verifier does assert that vector's legacy and bin re-encode byte-identity on + every run, and liblz4 reproduces its compressed bytes on the optional leg. The + spec also now names what re-vendoring 1.1.1 into cachekit-core actually + requires: bump `FIXTURE_SHA256`, bump the version pin, **and** relax + `assert_eq!(twin_bytes[1], 0xc4)` to accept `0xc5` — that assertion demands + every twin be bin8, and `width_boundary_bin16_bin` is bin16, so a drop-in + re-vendor fails it. A remedy that fails on contact leaves the gap open longer. ### Interop v2 — compressed-values profile (DRAFT) diff --git a/spec/wire-format.md b/spec/wire-format.md index d8ca73d..74e5e89 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -41,9 +41,8 @@ This document specifies two layers: decode byte-identity for every vector and re-encode byte-identity for the canonical `*_bin` vectors only — legacy array-of-integers vectors are decode-only, retained as legacy-read proof. That re-encode assertion covers - only the vectors the pinned file contains (core currently vendors 1.1.0; see - [Compressed-byte reproducibility](#compressed-byte-reproducibility-per-vector-scoping) - for the resulting gap). Byte-canonicity scopes to the + only the vectors the pinned file contains (core currently vendors 1.1.0, with + the resulting gap detailed below). Byte-canonicity scopes to the envelope's MessagePack encoding and to the **canonical writer's** output: the LZ4 bytes inside `compressed_data` are not reproducible across conforming compressors — see @@ -264,7 +263,13 @@ bytes are therefore **not canonical**, and conformance for `compressed_data` is **read-side**: - A conforming reader MUST decompress every pinned vector's `compressed_data` - to its pinned input. + to its pinned input, **and MUST enforce [Retrieve Flow](#retrieve-flow) steps + 4, 5 and 9 while doing so.** Read-side conformance is not "the vectors pass": + every pinned vector is well-formed and declares a truthful `original_size`, so + they evidence **none** of those bounds, and a reader that omits all three + decompresses all of them successfully. The vectors prove decode + interoperability; the bounds in [Security Limits](#security-limits) are a + separate, non-negotiable obligation that no fixture can demonstrate. - A writer **other than the canonical `lz4_flex` writer** is NOT required to reproduce the pinned compressed bytes, and MUST NOT be judged non-conforming because its compressor output differs from the fixture — validate such a @@ -274,20 +279,35 @@ bytes are therefore - A writer MAY still byte-compare its compressor output against the pins as a **drift tripwire**, provided the expected divergences are declared per vector rather than treated as failures. This repo's own verifier does exactly that - (`LZ4_ENCODE_DIVERGENT` in `tools/wire-format-reference.py`) — which watches - the reference **liblz4** mapping for divergence-set drift, and cannot observe - `lz4_flex` at all. Canonical-writer byte-reproducibility is a separate - mechanism, enforced only by the re-encode assertions in `cachekit-core` - described below. + (`LZ4_ENCODE_DIVERGENT` in `tools/wire-format-reference.py`), in two halves: a + set-level half that watches the reference **liblz4** mapping for + divergence-set drift, and a byte-level half that pins the exact + `compressed_data` of each declared-divergent vector. Both are needed — + "differs from liblz4's output" alone is a one-bit assertion that any other + valid LZ4 block satisfies, so it accepts a re-pin to unrelated bytes. Neither + half runs `lz4_flex`, so neither can detect an `lz4_flex` **behaviour** change; + that remains the job of the re-encode assertions in `cachekit-core` described + below, subject to the vendored-version gap noted there. This is the same doctrine [interop v2](interop-v2.md) records for its compressed-values profile. The pinned bytes are the **canonical implementation's** output (`lz4_flex` via `cachekit-core`), enforced by the re-encode byte-identity assertions in `cachekit-core/tests/wire_format_vectors.rs` — **but only for the -vectors present in the fixture that repo vendors**. That matters today: cachekit-core vendors 1.1.0 -and pins `version == "1.1.0"`, so `width_boundary_bin16` (added at 1.1.1) is -not yet covered by any encode-side check anywhere — re-vendoring 1.1.1 into -cachekit-core closes that gap. The reference liblz4 mapping +vectors present in the fixture that repo vendors**. That matters today: +cachekit-core vendors 1.1.0 and pins `version == "1.1.0"`, so +`width_boundary_bin16` (added at 1.1.1) has **no canonical-writer (`lz4_flex`) +compressed-byte check anywhere in the fleet**, and its pinned xxh3-64 checksum +is recomputed nowhere. Its MessagePack encoding *is* covered: this repo's +`tools/wire-format-reference.py verify` asserts legacy and bin re-encode +byte-identity for it on every run, and liblz4 reproduces its compressed bytes +on the optional `lz4` leg — so do not read this gap as "the vector is +unverified". Closing it means re-vendoring 1.1.1 into cachekit-core, which +requires three changes together, not one: bump `FIXTURE_SHA256`, bump the +`version == "1.1.0"` pin to `1.1.1`, and relax +`assert_eq!(twin_bytes[1], 0xc4)` to accept `0xc5` — that assertion currently +requires *every* twin to be bin8, and `width_boundary_bin16_bin` is bin16 +(marker `0xc5`, 303-byte `compressed_data`), which is the whole point of the +vector. A drop-in re-vendor fails that test. The reference liblz4 mapping above (`lz4.block`) is **decode-verified against every vector** in this repo's CI (`tools/wire-format-reference.py verify`, optional `lz4` leg); on encode it reproduces every pair except `large_compressible` byte-for-byte, which is an diff --git a/tools/test_wire_format_reference.py b/tools/test_wire_format_reference.py index 6079fcf..01034df 100755 --- a/tools/test_wire_format_reference.py +++ b/tools/test_wire_format_reference.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """Mutation tests for wire-format-reference.py's fail-closed guards. -Two classes, both proven reachable by execution rather than argued from reading +Every class below is proven reachable by execution rather than argued from reading (LAB-903: do not reason about a conformance gate, poison the fixture and watch it): 1. -O refusal. Every integrity check in that tool is an `assert`, so `python -O` @@ -16,9 +16,27 @@ data loss. test-vectors/wire-format.json is vendored and sha256-pinned by 4+ SDKs; a deletion here is invisible until an SDK's coverage has already shrunk. + 3. Whole-file properties, which every per-vector check is structurally blind to + because they all iterate the fixture's own vector list (LAB-1751 panel round 3; + all three exited 0 before the guards existed): + - the base-vector SET. Dropping a legacy base AND its `_bin` twin together net + to zero in generate's append-only diff, so `verify` reported "all 6 vector + pairs verified" and `generate` wrote the shrunken fixture. + - the fixture's declared `limits` block, which SDKs read their bounds from and + which nothing compared against the spec's Security Limits table. + - the pinned bytes of an encode-divergent vector. The lz4 tripwire asserts only + "differs from liblz4's output" — a one-bit check any other valid LZ4 block + satisfies, so a re-pin to unrelated bytes passed. + A guard with no mutation test is one refactor away from being deleted by someone who cannot see what it holds up. +SAFETY: every invocation that could reach `generate` runs against a scratch mirror, +never the repo's sha256-pinned fixture. A regressed guard must fail this suite, not +rewrite the vendored artifact — this suite is CI's first step, so it runs before +anything else has confirmed the tool is sane. `main` asserts the repo fixture is +byte-identical after the whole run. + Run: python3 tools/test_wire_format_reference.py (exit 1 on any failure) """ @@ -29,6 +47,7 @@ import subprocess import sys import tempfile +from collections.abc import Callable from pathlib import Path HERE = Path(__file__).resolve().parent @@ -38,6 +57,9 @@ # A vector whose legacy base is dropped by a bad merge, leaving an orphan twin. # LAB-868's width-boundary vector: the only bin16 coverage in the fleet. ORPHANED_BASE = "width_boundary_bin16" +# The realistic bad-merge shape the orphan case does NOT cover: base and twin go +# together, so the append-only diff is empty. +DROPPED_PAIR = "large_compressible" def _run(flags: list[str], argv: list[str], tool: Path = TOOL) -> subprocess.CompletedProcess: @@ -46,40 +68,72 @@ def _run(flags: list[str], argv: list[str], tool: Path = TOOL) -> subprocess.Com ) -def _scratch(tmp: Path, drop: str | None = None) -> Path: +def _scratch(tmp: Path, mutate: Callable[[dict], None] | None = None) -> Path: """Mirror tool + fixture into a scratch tree so mutations never touch the repo.""" - (tmp / "tools").mkdir() - (tmp / "test-vectors").mkdir() + (tmp / "tools").mkdir(parents=True, exist_ok=True) + (tmp / "test-vectors").mkdir(parents=True, exist_ok=True) shutil.copy(TOOL, tmp / "tools" / TOOL.name) fixture = json.loads(FIXTURE.read_text()) - if drop: - fixture["vectors"] = [v for v in fixture["vectors"] if v["name"] != drop] + if mutate: + mutate(fixture) (tmp / "test-vectors" / FIXTURE.name).write_text(json.dumps(fixture, indent=2) + "\n") return tmp / "tools" / TOOL.name +def _drop(*names: str) -> Callable[[dict], None]: + def mutate(fixture: dict) -> None: + fixture["vectors"] = [v for v in fixture["vectors"] if v["name"] not in names] + + return mutate + + +def _expect( + failures: list[str], + label: str, + proc: subprocess.CompletedProcess, + expected: int, + marker: str | None = None, +) -> None: + """Assert exit code and, when given, that the refusal came from the right guard. + + The marker is not decoration: python itself exits 2 on a bad script path and 1 on + an unhandled traceback, so an exit-code-only assertion passes vacuously when the + invocation never reached the guard at all. + """ + ok = proc.returncode == expected + if ok and marker and marker not in (proc.stdout + proc.stderr): + ok, label = False, f"{label} (exited {expected} but not via the guard)" + print(f" [{'ok' if ok else 'FAIL'}] {label}: expected exit {expected}, got {proc.returncode}") + if not ok: + failures.append(label) + + def check_optimised_refusals() -> list[str]: """-O must stop every entry point, including `import`.""" failures = [] - # Positive control: without -O the tool must still work, otherwise a guard that - # refuses everything would pass every case below. - cases = [ - ("verify, assertions on", [], ["verify"], 0), - ("verify under -O", ["-O"], ["verify"], 1), - ("generate under -O", ["-O"], ["generate"], 1), - ("verify under -OO", ["-OO"], ["verify"], 1), - ] - for name, flags, argv, expected in cases: - proc = _run(flags, argv) - ok = proc.returncode == expected - # An -O run must say why it refused; a bare non-zero exit could be an - # unrelated crash, which would let the guard rot behind a passing test. - label = name - if ok and flags and "assertions disabled" not in proc.stderr: - ok, label = False, f"{name} (exited {expected} but not via the guard)" - print(f" [{'ok' if ok else 'FAIL'}] {label}: expected exit {expected}, got {proc.returncode}") - if not ok: - failures.append(label) + with tempfile.TemporaryDirectory() as td: + # `generate` under -O is run against a scratch mirror: if the guard regresses, + # the write lands on a throwaway copy instead of the vendored fixture. + scratch_tool = _scratch(Path(td) / "opt") + cases = [ + # Positive control: without -O the tool must still work, otherwise a guard + # that refuses everything would pass every case below. + ("verify, assertions on", [], ["verify"], 0, TOOL, None), + ("verify under -O", ["-O"], ["verify"], 1, TOOL, "assertions disabled"), + ("generate under -O", ["-O"], ["generate"], 1, scratch_tool, "assertions disabled"), + ("verify under -OO", ["-OO"], ["verify"], 1, TOOL, "assertions disabled"), + ] + for name, flags, argv, expected, tool, marker in cases: + _expect(failures, name, _run(flags, argv, tool=tool), expected, marker) + + # The scratch fixture must be untouched even though the invocation asked to + # write it — proves the -O refusal precedes the write, not follows it. + scratch_fixture = scratch_tool.parent.parent / "test-vectors" / FIXTURE.name + pristine = json.loads(FIXTURE.read_text()) + untouched = json.loads(scratch_fixture.read_text()) == pristine + print(f" [{'ok' if untouched else 'FAIL'}] -O generate wrote nothing") + if not untouched: + failures.append("generate under -O rewrote the fixture before refusing") # The guard is at module scope precisely so this path cannot skip it. probe = "import importlib.util as u;s=u.spec_from_file_location('w',r'%s');m=u.module_from_spec(s);s.loader.exec_module(m);print('RAN',m.verify())" @@ -97,59 +151,132 @@ def check_generate_is_append_only() -> list[str]: """generate must refuse to drop a committed vector, not silently erase it.""" failures = [] with tempfile.TemporaryDirectory() as td: - tmp = Path(td) - tool = _scratch(tmp, drop=ORPHANED_BASE) - before = json.loads((tmp / "test-vectors" / FIXTURE.name).read_text()) - proc = _run([], ["generate"], tool=tool) - after = json.loads((tmp / "test-vectors" / FIXTURE.name).read_text()) - - refused = proc.returncode == 1 and "REFUSED" in proc.stderr - print(f" [{'ok' if refused else 'FAIL'}] generate refuses to drop a committed vector: exit {proc.returncode}") - if not refused: - failures.append("generate did not refuse to drop a committed vector") - - # The refusal must be a no-op on disk, not a refusal after the write. - untouched = before == after - print(f" [{'ok' if untouched else 'FAIL'}] refusal left the fixture byte-untouched") - if not untouched: - failures.append("generate mutated the fixture despite refusing") + for label, mutate, marker in ( + ("orphan twin (base dropped)", _drop(ORPHANED_BASE), "REFUSED"), + (f"whole pair dropped ({DROPPED_PAIR})", _drop(DROPPED_PAIR, f"{DROPPED_PAIR}_bin"), "REFUSED"), + ): + tmp = Path(tempfile.mkdtemp(dir=td)) + tool = _scratch(tmp, mutate=mutate) + fixture_path = tmp / "test-vectors" / FIXTURE.name + before = json.loads(fixture_path.read_text()) + proc = _run([], ["generate"], tool=tool) + _expect(failures, f"generate refuses: {label}", proc, 1, marker) + + # The refusal must be a no-op on disk, not a refusal after the write. + untouched = before == json.loads(fixture_path.read_text()) + print(f" [{'ok' if untouched else 'FAIL'}] refusal left the fixture byte-untouched: {label}") + if not untouched: + failures.append(f"generate mutated the fixture despite refusing: {label}") # Positive control: on an intact fixture, generate is still a working no-op. tool2 = _scratch(Path(tempfile.mkdtemp(dir=td))) - proc2 = _run([], ["generate"], tool=tool2) - ok2 = proc2.returncode == 0 - print(f" [{'ok' if ok2 else 'FAIL'}] generate still succeeds on an intact fixture: exit {proc2.returncode}") - if not ok2: - failures.append("generate broke on an intact fixture") + _expect(failures, "generate still succeeds on an intact fixture", _run([], ["generate"], tool=tool2), 0) + return failures + + +def check_whole_file_properties() -> list[str]: + """Drift no per-vector check can see: the vector set, the limits block, the pins.""" + failures = [] + + def repin_divergent(fixture: dict) -> None: + """Swap the divergent vector's compressed_data for a DIFFERENT valid LZ4 block. + + All-literals encoding: token litlen nibble 15 + extension bytes, matchlen 0. + liblz4 decompresses it to the same input, and it differs from liblz4's own + output, so every check except the byte-pin accepts it. + """ + base = next(v for v in fixture["vectors"] if v["name"] == DROPPED_PAIR) + twin = next(v for v in fixture["vectors"] if v["name"] == f"{DROPPED_PAIR}_bin") + inp = bytes.fromhex(base["input_hex"]) + rem = len(inp) - 15 + alt = bytes([0xF0]) + bytes([255] * (rem // 255) + [rem % 255]) + inp + for vec, encoding in ((base, "int-array"), (twin, "bin")): + env = _encode(alt, base, encoding) + vec["envelope_hex"] = env.hex() + vec["envelope_size"] = len(env) + + def _encode(data: bytes, base: dict, encoding: str) -> bytes: + import importlib.util as u + + spec = u.spec_from_file_location("_wfr", TOOL) + assert spec and spec.loader + mod = u.module_from_spec(spec) + spec.loader.exec_module(mod) + _d, checksum, size, fmt, _e = mod.decode_envelope(bytes.fromhex(base["envelope_hex"])) + return mod.encode_envelope(data, checksum, size, fmt, encoding=encoding) + + def limits_drift(fixture: dict) -> None: + fixture["limits"]["max_uncompressed_size"] = 1 + + def limits_missing(fixture: dict) -> None: + del fixture["limits"]["max_compression_ratio"] + + def unclassifiable(fixture: dict) -> None: + next(v for v in fixture["vectors"] if v["name"] == "simple_string_bin")["envelope_encoding"] = "bin16" + + cases = [ + ( + f"dropped pair is not 'all 6 verified' ({DROPPED_PAIR})", + _drop(DROPPED_PAIR, f"{DROPPED_PAIR}_bin"), + "base-vector set drifted", + ), + ( + f"dropped pair is not 'all 6 verified' ({ORPHANED_BASE})", + _drop(ORPHANED_BASE, f"{ORPHANED_BASE}_bin"), + "base-vector set drifted", + ), + ("fixture limits may not contradict the spec table", limits_drift, "limits' drifted"), + ("a missing declared limit is drift, not a skip", limits_missing, "limits' drifted"), + ("divergent vector keeps its pinned bytes", repin_divergent, "no longer carries its pinned"), + ("unusable fixture fails by name, not by traceback", unclassifiable, "simple_string_bin"), + ] + with tempfile.TemporaryDirectory() as td: + for label, mutate, marker in cases: + tool = _scratch(Path(tempfile.mkdtemp(dir=td)), mutate=mutate) + _expect(failures, label, _run([], ["verify"], tool=tool), 1, marker) return failures def check_flag_rejections() -> list[str]: """A flag accepted-and-ignored on the fixture-writing path is a fail-open.""" failures = [] - for name, argv, expected in [ - ("generate --require-extras rejected", ["generate", "--require-extras"], 2), - ("unknown command rejected", ["bogus"], 2), - ("typo'd flag rejected", ["verify", "--require-extra"], 2), - ]: - proc = _run([], argv) - ok = proc.returncode == expected - print(f" [{'ok' if ok else 'FAIL'}] {name}: expected exit {expected}, got {proc.returncode}") - if not ok: - failures.append(name) + with tempfile.TemporaryDirectory() as td: + # Scratch mirror: `generate --require-extras` is one regressed guard away from + # writing the fixture, and this suite is the first thing CI runs. + tool = _scratch(Path(td) / "flags") + fixture_path = tool.parent.parent / "test-vectors" / FIXTURE.name + before = json.loads(fixture_path.read_text()) + for name, argv, expected, marker in [ + ("generate --require-extras rejected", ["generate", "--require-extras"], 2, "not valid for"), + ("unknown command rejected", ["bogus"], 2, "Usage:"), + ("typo'd flag rejected", ["verify", "--require-extra"], 2, "Usage:"), + ]: + _expect(failures, name, _run([], argv, tool=tool), expected, marker) + + untouched = before == json.loads(fixture_path.read_text()) + print(f" [{'ok' if untouched else 'FAIL'}] no rejected invocation wrote the fixture") + if not untouched: + failures.append("a rejected invocation still wrote the fixture") return failures def main() -> int: + pristine = FIXTURE.read_bytes() failures = [] for label, check in ( ("-O refusal", check_optimised_refusals), ("generate append-only", check_generate_is_append_only), + ("whole-file properties", check_whole_file_properties), ("flag rejection", check_flag_rejections), ): print(f"{label}:") failures += check() + # Belt and braces on the whole suite: nothing here may touch the vendored artifact. + if FIXTURE.read_bytes() != pristine: + print("\nFATAL: the suite modified test-vectors/wire-format.json", file=sys.stderr) + failures.append("suite modified the repo fixture") + if failures: print(f"\n{len(failures)} case(s) failed:", file=sys.stderr) for f in failures: diff --git a/tools/wire-format-reference.py b/tools/wire-format-reference.py index 514bd74..166ad35 100644 --- a/tools/wire-format-reference.py +++ b/tools/wire-format-reference.py @@ -23,6 +23,11 @@ 4. Declared sizes match ground truth — `original_size` equals the real length of `input_hex`, not merely the co-located `input_size` field (both declared sizes live in the file under test, so they drift together). + 5. The base-vector set is exactly EXPECTED_BASE_VECTORS, and the fixture's own + declared `limits` block matches the spec's Security Limits table. Both are + whole-file properties: checks 1-4 iterate the fixture's vector list and so + are structurally blind to a vector that is simply absent, or to a bound the + fixture misdeclares to every SDK that reads it. Usage: python3 tools/wire-format-reference.py verify # default @@ -36,9 +41,11 @@ from decoded fields and must reproduce the pinned bytes byte-identically. - `lz4`: C-implementation (liblz4) decode conformance — liblz4 must decompress every vector's pinned compressed_data to the pinned input. Encoder agreement - is asserted only as a set-level drift tripwire against LZ4_ENCODE_DIVERGENT, - never as a per-vector conformance rule (spec 'Compressed-byte - reproducibility', LAB-1751). + is a drift tripwire against LZ4_ENCODE_DIVERGENT, never a per-vector + conformance rule (spec 'Compressed-byte reproducibility', LAB-1751): a vector + liblz4 reproduces must keep reproducing, and a declared-divergent vector must + keep the exact bytes mapped to its name — "differs from liblz4" alone is a + one-bit check that any other valid LZ4 block would satisfy. Both commands refuse to run under -O/PYTHONOPTIMIZE: every check in this file is an `assert`, so an optimised `verify` would report a pass having tested @@ -56,18 +63,57 @@ FIXTURE_PATH = Path(__file__).resolve().parent.parent / "test-vectors" / "wire-format.json" FIXTURE_VERSION = "1.1.1" -# spec/wire-format.md 'Security Limits' (512 MiB table); the allocating consumer is -# 'Retrieve Flow' step 4. Checked on both CI legs, before the ground-truth compare, so -# an inflated original_size is rejected by name rather than sized into RAM by liblz4. -# Only this one bound; the envelope/compressed_data length caps and the 1000:1 bomb -# check are a reader's obligations, not this fixture verifier's. -MAX_UNCOMPRESSED_SIZE = 512 * 1024 * 1024 +# The base vectors this fixture is pinned to contain. Checked as a SET, because every +# other integrity check here iterates the fixture's own vector list and therefore +# cannot see a vector that is simply absent: dropping a legacy base AND its `_bin` +# twin together left `generate`'s append-only diff empty and `verify` reporting "all 6 +# vector pairs verified", exit 0 (LAB-1751 panel round 3). Same lesson as the +# original_size/input_size drift one level up — a name list derived from the artifact +# under test pins nothing, so the expected set has to live in code. +EXPECTED_BASE_VECTORS = frozenset( + { + "empty", + "simple_string", + "binary_data", + "large_compressible", + "json_like", + "single_byte", + "width_boundary_bin16", + } +) +# spec/wire-format.md 'Security Limits'. Pinned here so the fixture's own declared +# `limits` block cannot drift from the spec table: SDKs read their bounds from that +# block, so a fixture shipping max_uncompressed_size=1 with CI green would hand every +# downstream reader a wrong bound. `verify` compares the two. The per-vector check +# below additionally rejects an inflated declared original_size by name; note the +# ground-truth `original_size == len(input_hex)` assert would catch that case anyway, +# so this bound is about naming the spec limit that was breached, not about bounding +# an allocation. The compressed_data length cap and the 1000:1 bomb check are a +# reader's obligations, not this fixture verifier's. +SPEC_LIMITS = { + "max_uncompressed_size": 512 * 1024 * 1024, + "max_compressed_size": 512 * 1024 * 1024, + "max_compression_ratio": 1000, +} +MAX_UNCOMPRESSED_SIZE = SPEC_LIMITS["max_uncompressed_size"] # spec/wire-format.md 'Compressed-byte reproducibility' names WHICH vectors liblz4 -# fails to reproduce on encode. Encoder agreement is not a conformance rule, but the -# spec's claim about the set is a fact, so a change to it must fail CI and force the -# text to be re-read — otherwise the next `lz4==` bump rots the spec silently. +# fails to reproduce on encode, and maps each to the bytes actually pinned. Encoder +# agreement is not a conformance rule, but the spec's claim about the set is a fact, +# so a change to it must fail CI and force the text to be re-read — otherwise the next +# `lz4==` bump rots the spec silently. The value is load-bearing: asserting only +# "these bytes differ from liblz4's output" is a one-bit check that ANY other valid +# LZ4 block satisfies, so a re-pin of the divergent vector to unrelated bytes passed +# (LAB-1751 panel round 3). Byte-pinning the divergent vector is the only encode-side +# enforcement it has anywhere in the fleet. # Base names only: twins carry the same compressed_data and are not iterated. -LZ4_ENCODE_DIVERGENT = frozenset({"large_compressible"}) +LZ4_ENCODE_DIVERGENT = {"large_compressible": "1f410100ffffffe960414141414141"} +if not LZ4_ENCODE_DIVERGENT.keys() <= EXPECTED_BASE_VECTORS: + # A phantom name here is never compared against anything in the loop below, so it + # would leave the spec's named set wrong with CI green. + raise SystemExit( + "BUG: LZ4_ENCODE_DIVERGENT names vectors not in EXPECTED_BASE_VECTORS: " + f"{', '.join(sorted(LZ4_ENCODE_DIVERGENT.keys() - EXPECTED_BASE_VECTORS))}" + ) ENVELOPE_FORMAT = ( "MessagePack positional array (rmp_serde::to_vec): " "[compressed_data, checksum, original_size, format]. Vectors without an " @@ -251,12 +297,47 @@ def _split_vectors(fixture: dict) -> tuple[list[dict], dict[str, dict]]: bins = {v["name"]: v for v in fixture["vectors"] if v.get("envelope_encoding") == "bin"} # Fail closed: every vector must classify as exactly one of the two sets, # with no duplicate names — otherwise verify would silently skip it (and - # generate would silently drop it from the append-only fixture). - if len(legacy) + len(bins) != len(fixture["vectors"]) or len({v["name"] for v in legacy}) != len(legacy): - raise ValueError("fixture contains unclassifiable or duplicate-named vectors") + # generate would silently drop it from the append-only fixture). Named + # separately, and caught by both callers, so the failure says WHICH vector + # is unusable instead of exiting via a bare traceback. + if len(legacy) + len(bins) != len(fixture["vectors"]): + classified = {v["name"] for v in legacy} | set(bins) + stray = [v["name"] for v in fixture["vectors"] if v["name"] not in classified] + raise ValueError( + "fixture vector(s) classify as neither legacy nor bin (envelope_encoding " + f"must be absent or 'bin'): {', '.join(sorted(stray))}" + ) + names = [v["name"] for v in legacy] + if len(set(names)) != len(names): + dupes = sorted({n for n in names if names.count(n) > 1}) + raise ValueError(f"fixture contains duplicate-named legacy vector(s): {', '.join(dupes)}") return legacy, bins +def _base_set_error(legacy: list[dict]) -> str | None: + """Compare the fixture's base-vector set against EXPECTED_BASE_VECTORS. + + Absence is the one drift class every per-vector check is blind to, because they + all iterate the fixture's own list. Returns a message, or None when the set is + exactly as pinned. + """ + present = {v["name"] for v in legacy} + missing = EXPECTED_BASE_VECTORS - present + unexpected = present - EXPECTED_BASE_VECTORS + if not missing and not unexpected: + return None + parts = [] + if missing: + parts.append(f"missing base vector(s): {', '.join(sorted(missing))}") + if unexpected: + parts.append(f"unexpected base vector(s): {', '.join(sorted(unexpected))}") + return ( + f"fixture base-vector set drifted — {'; '.join(parts)}. A dropped base and its " + "'_bin' twin are invisible to every per-vector check; add a genuinely new " + "vector to EXPECTED_BASE_VECTORS deliberately, and never remove one." + ) + + def _bin_twin(base: dict) -> dict: data, checksum, size, fmt, encoding = decode_envelope(bytes.fromhex(base["envelope_hex"])) assert encoding == "int-array", f"[{base['name']}] base vector is not legacy-encoded" @@ -275,16 +356,29 @@ def _bin_twin(base: dict) -> dict: def generate() -> int: - fixture = _load() - legacy, _ = _split_vectors(fixture) + try: + fixture = _load() + legacy, _ = _split_vectors(fixture) + except ValueError as e: + print(f"REFUSED: unusable fixture: {e}", file=sys.stderr) + return 1 + # Refuse before the write if the base set itself has drifted. The `lost` diff below + # is derived from the fixture on both sides, so dropping a base AND its twin + # together nets to zero there and `generate` would happily write the shrunken + # fixture (LAB-1751 panel round 3). + set_error = _base_set_error(legacy) + if set_error: + print(f"REFUSED: {set_error}", file=sys.stderr) + return 1 rebuilt = legacy + [_bin_twin(v) for v in legacy] - # Append-only, as the fixture's own contract requires (LAB-783) and as the sibling - # python-frame-reference.py already enforces by upsert (LAB-1203). Rebuilding from - # the legacy set alone silently drops any committed vector that is not a derived - # twin, and `verify`'s orphan FAIL names `generate` as the remedy — so the repair - # step completes the data loss. Refuse instead: this file is vendored and - # sha256-pinned downstream, and a deletion here is invisible until an SDK's - # conformance coverage has already shrunk. + # Append-only, as the fixture's own contract requires (LAB-783), and the same + # refusal the sibling python-frame-reference.py uses for its whole-fixture rebuild + # (its upsert-by-name applies only to the single-vector append mode, LAB-1203 — do + # not "align" this to an upsert). Rebuilding from the legacy set alone silently + # drops any committed vector that is not a derived twin, and `verify`'s orphan FAIL + # names `generate` as the remedy — so the repair step completes the data loss. + # Refuse instead: this file is vendored and sha256-pinned downstream, and a + # deletion here is invisible until an SDK's conformance coverage has already shrunk. lost = {v["name"] for v in fixture["vectors"]} - {v["name"] for v in rebuilt} if lost: print( @@ -338,6 +432,20 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: assert size == len(bytes.fromhex(base["input_hex"])), ( "original_size != len(input_hex)" ) + # A vector the spec declares encode-divergent keeps the exact bytes mapped to its + # name. Stdlib, and deliberately OUTSIDE the optional-deps gate below (same reason + # as the ground-truth compare above): the set-level tripwire on the lz4 leg only + # asserts "these bytes differ from liblz4's output", which ANY other valid LZ4 + # block satisfies — so a re-pin to unrelated bytes passed both legs. This is the + # only encode-side enforcement the divergent vector has anywhere in the fleet; + # cachekit-core re-encodes via lz4_flex and so cannot check it either. + pinned_hex = LZ4_ENCODE_DIVERGENT.get(base["name"]) + assert pinned_hex is None or data.hex() == pinned_hex, ( + f"declared-divergent vector {base['name']} no longer carries its pinned " + f"compressed_data ({data.hex()} != {pinned_hex}) — re-pin deliberately in " + "LZ4_ENCODE_DIVERGENT and re-read spec/wire-format.md 'Compressed-byte " + "reproducibility'" + ) # 2. twin equivalence twin = bins.pop(base["name"] + "_bin", None) @@ -348,8 +456,7 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: ) assert new_env[0] == 0x94, "outer fixarray(4) not preserved" assert new_env[1] in (0xC4, 0xC5, 0xC6), "element[0] not bin-encoded" - t_data, t_checksum, t_size, t_fmt, t_encoding = decode_envelope(new_env) - assert t_encoding == "bin" + t_data, t_checksum, t_size, t_fmt, _t_encoding = decode_envelope(new_env) assert (t_data, t_checksum, t_size, t_fmt) == (data, checksum, size, fmt), ( "twin decodes to different fields" ) @@ -394,7 +501,10 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: assert got == inp, "liblz4 does not decompress pinned compressed_data to the input" theirs = lz4_block.compress(inp, store_size=False) diverges = theirs != data - assert diverges == (base["name"] in LZ4_ENCODE_DIVERGENT), ( + # Set-level half of the tripwire: WHICH vectors liblz4 fails to reproduce is a + # spec fact, so a change either direction must fail CI. The byte-level half + # (the pinned bytes themselves) is asserted above on both legs. + assert diverges == (pinned_hex is not None), ( f"liblz4 encode-divergence set changed: {base['name']} " f"{'now diverges from' if diverges else 'now reproduces'} the pin — " "update spec/wire-format.md 'Compressed-byte reproducibility' and " @@ -412,13 +522,33 @@ def _verify_vector(base: dict, bins: dict, msgpack, lz4_block) -> str: def verify(require_extras: bool = False) -> int: fixture = _load() - legacy, bins = _split_vectors(fixture) + try: + legacy, bins = _split_vectors(fixture) + except ValueError as e: + print(f"FAIL: unusable fixture: {e}", file=sys.stderr) + return 1 if not legacy: print("FAIL: no legacy vectors found", file=sys.stderr) return 1 if fixture.get("version") != FIXTURE_VERSION: print(f"FAIL: fixture version {fixture.get('version')} != {FIXTURE_VERSION}", file=sys.stderr) return 1 + set_error = _base_set_error(legacy) + if set_error: + print(f"FAIL: {set_error}", file=sys.stderr) + return 1 + # The fixture declares the bounds SDKs read; the spec table is normative. Neither + # pinned the other, so a fixture rewriting max_uncompressed_size to 1 verified + # green (LAB-1751 panel round 3). + declared = fixture.get("limits", {}) + drifted = sorted(k for k, v in SPEC_LIMITS.items() if declared.get(k) != v) + if drifted: + detail = ", ".join(f"{k}: fixture {declared.get(k)!r} != spec {SPEC_LIMITS[k]}" for k in drifted) + print( + f"FAIL: fixture 'limits' drifted from spec/wire-format.md 'Security Limits' — {detail}", + file=sys.stderr, + ) + return 1 try: import msgpack # type: ignore[import-untyped] @@ -467,8 +597,10 @@ def main() -> int: # Reject anything unrecognised rather than dropping it. `--require-extras` is # matched by exact string and it gates the deepest coverage, so a silently # ignored `--require-extra` typo used to exit 0 with the extras legs off — - # the exact fail-open the flag was added to close. - if len(args) > 1 or any(a.startswith("-") for a in args): + # the exact fail-open the flag was added to close. The arity check is what + # closes it: the typo survives the strip above, leaving two positionals. A + # lone unrecognised token falls through to the unknown-command exit 2 below. + if len(args) > 1: print(__doc__, file=sys.stderr) return 2 cmd = args[0] if args else "verify" @@ -489,9 +621,11 @@ def main() -> int: if not __debug__: # Every integrity check in this file is an `assert`, so -O/-OO strips all of them: # `verify` reports a pass having tested nothing and `generate` rewrites the fixture - # with its input checks gone. Module scope, not main(), because importing this - # module (sibling tools reuse its envelope codec) would otherwise walk straight - # past a CLI-only guard. Regression-tested by tools/test_wire_format_reference.py. + # with its input checks gone. Module scope, not main(), because a CLI-only guard is + # bypassed by importing this module and calling verify() directly — which the + # regression harness's importlib probe does, and which is how the sibling tools + # load each other's codecs (interop-v2-reference.py loads interop-reference.py by + # spec_from_file_location). Regression-tested by tools/test_wire_format_reference.py. print( "FAIL: assertions disabled (-O / PYTHONOPTIMIZE) — this tool proves nothing", file=sys.stderr, From afc254328e7cf213908a4e6e4362ac4d27ad9149 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Tue, 1 Sep 2026 02:32:43 +1000 Subject: [PATCH 11/12] fix(harness): byte-snapshot the immutability checks; drop a narrowing assert (LAB-1751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review on 534bf07, both items valid. The three "fixture untouched" checks compared parsed JSON, so a rewrite that only reindented or reordered keys satisfied a claim whose label says byte-untouched. Proved: json.loads() calls an indent=2 -> indent=4 rewrite untouched, read_bytes() does not. Now byte snapshots throughout. The -O case had a second, sharper bug CodeRabbit also caught: its baseline was read AFTER the invocation, and from the repo fixture rather than the scratch mirror. Since _scratch re-serialises the fixture, the mirror is not byte-identical to the repo copy — so a naive switch to bytes there would have failed rather than passed vacuously. Snapshot is now taken from the scratch file before the loop. Kody's narrowing `assert spec and spec.loader` in the importlib helper becomes an explicit raise: that one is genuinely not a conformance check, so the team rule applies to it cleanly. Harness still 22/22, both CI legs green, all five guard-deletion regressions still detected, fixture sha256 b902db88... unchanged. --- tools/test_wire_format_reference.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tools/test_wire_format_reference.py b/tools/test_wire_format_reference.py index 01034df..18f6928 100755 --- a/tools/test_wire_format_reference.py +++ b/tools/test_wire_format_reference.py @@ -115,6 +115,12 @@ def check_optimised_refusals() -> list[str]: # `generate` under -O is run against a scratch mirror: if the guard regresses, # the write lands on a throwaway copy instead of the vendored fixture. scratch_tool = _scratch(Path(td) / "opt") + # Byte snapshot, taken BEFORE the invocations and of the scratch file itself: + # comparing parsed JSON would call a reformatting rewrite "byte-untouched", + # and comparing against the repo fixture would compare the wrong file (the + # mirror is re-serialised by _scratch, so it is not byte-identical to it). + scratch_fixture = scratch_tool.parent.parent / "test-vectors" / FIXTURE.name + pristine_scratch = scratch_fixture.read_bytes() cases = [ # Positive control: without -O the tool must still work, otherwise a guard # that refuses everything would pass every case below. @@ -128,9 +134,7 @@ def check_optimised_refusals() -> list[str]: # The scratch fixture must be untouched even though the invocation asked to # write it — proves the -O refusal precedes the write, not follows it. - scratch_fixture = scratch_tool.parent.parent / "test-vectors" / FIXTURE.name - pristine = json.loads(FIXTURE.read_text()) - untouched = json.loads(scratch_fixture.read_text()) == pristine + untouched = scratch_fixture.read_bytes() == pristine_scratch print(f" [{'ok' if untouched else 'FAIL'}] -O generate wrote nothing") if not untouched: failures.append("generate under -O rewrote the fixture before refusing") @@ -158,12 +162,14 @@ def check_generate_is_append_only() -> list[str]: tmp = Path(tempfile.mkdtemp(dir=td)) tool = _scratch(tmp, mutate=mutate) fixture_path = tmp / "test-vectors" / FIXTURE.name - before = json.loads(fixture_path.read_text()) + before = fixture_path.read_bytes() proc = _run([], ["generate"], tool=tool) _expect(failures, f"generate refuses: {label}", proc, 1, marker) # The refusal must be a no-op on disk, not a refusal after the write. - untouched = before == json.loads(fixture_path.read_text()) + # Bytes, not parsed JSON: a rewrite that only reorders keys or reindents + # is still a write, and the claim below says byte-untouched. + untouched = before == fixture_path.read_bytes() print(f" [{'ok' if untouched else 'FAIL'}] refusal left the fixture byte-untouched: {label}") if not untouched: failures.append(f"generate mutated the fixture despite refusing: {label}") @@ -199,7 +205,8 @@ def _encode(data: bytes, base: dict, encoding: str) -> bytes: import importlib.util as u spec = u.spec_from_file_location("_wfr", TOOL) - assert spec and spec.loader + if spec is None or spec.loader is None: + raise RuntimeError(f"cannot load {TOOL} as a module") mod = u.module_from_spec(spec) spec.loader.exec_module(mod) _d, checksum, size, fmt, _e = mod.decode_envelope(bytes.fromhex(base["envelope_hex"])) @@ -245,7 +252,7 @@ def check_flag_rejections() -> list[str]: # writing the fixture, and this suite is the first thing CI runs. tool = _scratch(Path(td) / "flags") fixture_path = tool.parent.parent / "test-vectors" / FIXTURE.name - before = json.loads(fixture_path.read_text()) + before = fixture_path.read_bytes() for name, argv, expected, marker in [ ("generate --require-extras rejected", ["generate", "--require-extras"], 2, "not valid for"), ("unknown command rejected", ["bogus"], 2, "Usage:"), @@ -253,7 +260,7 @@ def check_flag_rejections() -> list[str]: ]: _expect(failures, name, _run([], argv, tool=tool), expected, marker) - untouched = before == json.loads(fixture_path.read_text()) + untouched = before == fixture_path.read_bytes() print(f" [{'ok' if untouched else 'FAIL'}] no rejected invocation wrote the fixture") if not untouched: failures.append("a rejected invocation still wrote the fixture") From 618ffc0e021ea484a8cbbdf3294437606f37ff03 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Tue, 1 Sep 2026 02:38:01 +1000 Subject: [PATCH 12/12] style(harness): move the module-load message into a dedicated exception (TRY003) (LAB-1751) CodeRabbit on afc2543. The raise it flags is the one Kody asked for in place of a narrowing assert, so the two rules meet on the same line; a 3-line private exception satisfies both. Harness 22/22, verify green, fixture untouched. --- tools/test_wire_format_reference.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tools/test_wire_format_reference.py b/tools/test_wire_format_reference.py index 18f6928..53ad8ee 100755 --- a/tools/test_wire_format_reference.py +++ b/tools/test_wire_format_reference.py @@ -57,6 +57,15 @@ # A vector whose legacy base is dropped by a bad merge, leaving an orphan twin. # LAB-868's width-boundary vector: the only bin16 coverage in the fleet. ORPHANED_BASE = "width_boundary_bin16" + + +class _ModuleLoadError(RuntimeError): + """The reference tool could not be loaded as a module (message lives here per TRY003).""" + + def __init__(self, tool: Path) -> None: + super().__init__(f"cannot load {tool} as a module") + + # The realistic bad-merge shape the orphan case does NOT cover: base and twin go # together, so the append-only diff is empty. DROPPED_PAIR = "large_compressible" @@ -206,7 +215,7 @@ def _encode(data: bytes, base: dict, encoding: str) -> bytes: spec = u.spec_from_file_location("_wfr", TOOL) if spec is None or spec.loader is None: - raise RuntimeError(f"cannot load {TOOL} as a module") + raise _ModuleLoadError(TOOL) mod = u.module_from_spec(spec) spec.loader.exec_module(mod) _d, checksum, size, fmt, _e = mod.decode_envelope(bytes.fromhex(base["envelope_hex"]))