From c9bb9be72265131e5072b421262442609e8981bd Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Wed, 2 Sep 2026 16:00:45 +1000 Subject: [PATCH 1/3] docs(spec): one >=64-bit ratio-product rule, guarded in both specs (LAB-2594) The decompression-bomb ratio bound was specified twice with divergent normative text: interop v2 bound the integer width (LAB-1135), wire format said nothing, so an implementer reading wire format alone could legally compute the product in 32-bit pointer-width arithmetic. Both documents now carry the same rule, and CI fails if the copies drift. The rule names the operation rather than a property of the arithmetic -- widen the operand to >=64-bit unsigned BEFORE multiplying -- because that is what defeats the actual defect: 1000 * payload.len() on usize is 64-bit on the author's host and in CI, and wraps only on the shipped wasm32 target. "Compute in >=64 bits" never fires in that author's self-assessment. The "if max_allowed overflows: REJECT" pseudocode is removed rather than reworded. It is not an observable event as written in any target language, and once the operand is widened it is unreachable -- the two 512 MiB caps bound the product below 2^39. Rejecting on overflow is explicitly not a substitute for widening: at 32-bit width it refuses 99.2% of the legal compressed-size range. Narrowing "no floating point" to "no floating-point ratio" (needed to permit JavaScript Number, exact below 2^53) would have sanctioned a truncating integer division accepting up to 1000*cs + (cs-1), so the bound is now required to be computed by multiplication in any arithmetic. interop v2's "corrupts the bound in both directions" was wrong: wrapping can only tighten it, so the failure mode is spurious rejection, never a bypass -- but a total one, collapsing to 704 B at 4.29 MB and to 0 at the 512 MiB cap. Spec text only. cachekit-core already uses checked_mul on u64 and is unchanged; no limit values and no fixture bytes were touched. --- .github/workflows/verify.yml | 9 ++ CHANGELOG.md | 67 ++++++++++++++ spec/interop-v2.md | 55 +++++++++-- spec/wire-format.md | 56 ++++++++++- tools/check-spec-duplication.py | 134 +++++++++++++++++++++++++++ tools/test_check_spec_duplication.py | 122 ++++++++++++++++++++++++ 6 files changed, 429 insertions(+), 14 deletions(-) create mode 100644 tools/check-spec-duplication.py create mode 100644 tools/test_check_spec_duplication.py diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index a267e98..4e09ae7 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -61,6 +61,15 @@ jobs: - name: Python-frame JS cross-check (zero-dep independent reader, full round-trip) run: node tools/frame-crosscheck.mjs + # The >=64-bit ratio-product rule is stated in full in BOTH spec documents + # (an implementer reads one standalone), so the copies are guarded rather + # than trusted -- divergent normative text for this exact bound is the bug + # LAB-2594 closed. Mutation suite first, same doctrine as below. + - name: Ratio-product rule identical in both spec documents + run: | + python3 tools/test_check_spec_duplication.py + python3 tools/check-spec-duplication.py + # Narrow on purpose: catches one facet of ONE of the six matrix incidents # listed in decisions/matrix-version-verification.md — a version in the SDK # Overview table stated as a snapshot, true when written and false at the diff --git a/CHANGELOG.md b/CHANGELOG.md index 7954064..6387de7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,73 @@ All notable changes to the CacheKit Protocol Specification. ## [Unreleased] +### Decompression bomb bound — one ≥ 64-bit rule, guarded in both specs (LAB-2594) + +- [`spec/wire-format.md`](spec/wire-format.md) § Security Limits now carries the + same normative obligation as + [`spec/interop-v2.md`](spec/interop-v2.md) § Security Limits — *"The ratio + product MUST be computed in **at least 64-bit unsigned integers**"* — with an + identical rationale block (identical text, modulo each document's operand name, + `compressed_size` vs `payload.length`). The bound was previously specified + twice with divergent normative text: interop v2 bound the integer width + (LAB-1135, [#53](https://github.com/cachekit-io/protocol/pull/53)), wire format + said nothing, so an implementer reading only wire format could legally compute + the product in 32-bit pointer-width arithmetic. +- The rule now names the **operation**, not a property of the arithmetic: widen + the operand to ≥ 64-bit unsigned *before* multiplying. That is what defeats the + actual defect — `1000 * payload.len()` on `usize` is 64-bit on the author's + host and in 64-bit CI, and wraps only on the shipped wasm32 target, so a rule + phrased as "compute in ≥ 64 bits" never fires in the author's self-assessment. + Rust `u64`, Python `int` and JavaScript `Number` (exact below 2⁵³; the product + is < 2³⁹, so no `BigInt`) all satisfy it, so no fallback is offered. +- The `if max_allowed overflows: REJECT` pseudocode is **removed**, not reworded. + It is not an observable event as written (Rust release-mode arithmetic wraps + silently, JavaScript `Number` loses precision above 2⁵³ rather than + overflowing, Python integers are arbitrary-precision — the repo's own + `tools/interop-v2-reference.py` can never take that branch), and once the + operand is widened the branch is unreachable anyway: the two 512 MiB caps bound + the product below 2³⁹. Rejecting on overflow is explicitly **not** an accepted + substitute for widening — at 32-bit width it would refuse 99.2 % of the legal + compressed-size range. Both documents now also state the caps as a normative + precondition of the product bound, and wire format's pseudocode enforces them + inline instead of leaving them to a collapsed flow section. +- The bound MUST now be computed by **multiplication**; deriving it by division + is forbidden in any arithmetic. Narrowing the old blanket "no floating point" + rule to "no floating-point *ratio*" (needed to permit JavaScript `Number`) + would otherwise have sanctioned `original_size / compressed_size > 1000`, whose + truncation accepts up to `1000·compressed_size + (compressed_size − 1)` — a + looser bound than this spec permits. +- **`spec/interop-v2.md`'s rationale corrected**: 32-bit wrapping was said to + "silently corrupt the bound **in both directions**". It cannot. Wrapping begins + at `⌈2³²/1000⌉ = 4,294,968` B (~4.29 MB) and can only *tighten* the bound, so + the failure mode is **spurious rejection**, never a bomb bypass — but a total + one: the wrapped bound collapses to 704 B at that threshold and to 0 at the + 512 MiB cap, making every entry with a ≥ 4.29 MB compressed payload + permanently unreadable on such a target. Both documents now say so, marked + non-normative, and interop v2 records the corrected claim in place. +- **New CI guard**: [`tools/check-spec-duplication.py`](tools/check-spec-duplication.py) + compares the two copies of the rule (delimited by sentinel comments) modulo the + operand rename, with a 9-case mutation suite + ([`tools/test_check_spec_duplication.py`](tools/test_check_spec_duplication.py)) + that re-arms the LAB-2594 divergence and asserts the guard fails. Stating the + rule twice is deliberate — an implementer reads one document standalone — so + the duplication is guarded rather than trusted. +- **No implementation change.** `cachekit-core/src/byte_storage.rs` already + computes `MAX_COMPRESSION_RATIO: u64 * (len as u64)` via + `checked_mul(…).ok_or(DecompressionBomb)?`, correct and fail-closed on every + target including wasm32. cachekit-ts was verified to have no JS-side ratio + check and to inherit that arithmetic through the Rust core on the envelope + decode path; a JS-only decode path, if one is ever added, would be unbounded + and is out of scope here. This was a spec trust bug — the documented rule was + weaker and vaguer than the shipped one. No limit values changed and no fixture + bytes were touched (no SDK re-vendors). +- **Known gap, not closed here**: no published conformance vector can distinguish + a 64-bit decoder from a 32-bit wrapping one. The largest `original_size` in + `test-vectors/interop-v2.json` is 237 B while divergence begins at a 4,294,968 B + compressed payload, so a wrapping decoder passes 100 % of the suite. Closing it + needs a limits-only vector shape and a fixture version bump (forcing py/ts + re-vendors), which is out of scope for a spec-text change — filed separately. + ### Wire format — compressed-byte reproducibility scoped per-vector (LAB-1751) - LZ4 compressed bytes are **not canonical** across conforming block encoders. diff --git a/spec/interop-v2.md b/spec/interop-v2.md index 1b9d81f..e16d44f 100644 --- a/spec/interop-v2.md +++ b/spec/interop-v2.md @@ -232,7 +232,8 @@ bomb — that bare-MessagePack v1 does not have. These bounds reuse the ByteStorage constants from [wire-format.md → Security Limits](wire-format.md#security-limits) so the fleet carries **one** set of numbers, and all of them MUST be enforced **before** -decompressing (integer arithmetic only — no floating point): +decompressing (integer arithmetic only — no floating-point *ratio*; the ratio +product's integer-width requirement is stated below): | Limit | Value | Applies to | | :--- | ---: | :--- | @@ -247,18 +248,54 @@ reject if original_size > MAX_UNCOMPRESSED // 512 MiB reject if payload.length > MAX_COMPRESSED // 512 MiB if method == 1: reject if payload.length == 0 // zero-length compressed = bomb - max_allowed = 1000 * payload.length // MUST be computed in >= 64-bit integers + max_allowed = 1000 * uint64(payload.length) // widen BEFORE multiplying; see below reject if original_size > max_allowed if method == 0: reject if original_size != payload.length ``` -The ratio product MUST be computed in **at least 64-bit unsigned integers**. -After the two 512 MiB caps pass, both operands are < 2³⁰ and the product is -< 2⁴⁰, so it can never overflow a u64 — but it *does* overflow 32-bit `usize` -arithmetic (a real target: cachekit-ts ships a wasm32 build), where release-mode -wrapping would silently corrupt the bound in both directions. Do not compute -this in pointer-width arithmetic. + +The ratio product MUST be computed in **at least 64-bit unsigned integers**: +promote `payload.length` to a ≥ 64-bit unsigned (or arbitrary-precision) integer *before* +the multiply. Multiplying in pointer width and widening the result afterwards +does not satisfy this, and is invisible on a 64-bit host and in 64-bit CI — it +is the wasm32 defect described below. Every target language has a conforming +path: Rust `u64` (on every target, `wasm32` included), Python's +arbitrary-precision `int`, and JavaScript `Number` — an IEEE-754 double +represents every integer below 2⁵³ exactly and this product is < 2³⁹, so no +`BigInt` is required. Because `payload.length` ≤ 2²⁹ once the two 512 MiB caps have +passed, the product is < 2³⁹ and cannot overflow 64 bits; that is why the +pseudocode above carries no overflow branch, and why rejecting on overflow is +**not** a substitute for widening — at 32-bit width it would refuse 99.2 % of +the legal `payload.length` range (see the note below). + +The bound MUST be computed by **multiplication**. Deriving it by division, or +as a *ratio*, is forbidden in any arithmetic — integer or floating-point. +Truncating integer division (`original_size / payload.length > 1000`) accepts up to +`1000·payload.length + (payload.length − 1)`, which is looser than this specification permits, and a +floating-point ratio is the precision bypass the integer rule exists to prevent. + +> [!NOTE] +> **Non-normative rationale — the failure direction under pointer-width +> arithmetic is fail-closed, never a bypass.** 32-bit pointer width is a live +> target: cachekit-ts ships a `wasm32` build. (That build is *not* affected — it +> computes this bound through `cachekit-core`'s `u64`.) Wrapping begins at +> `payload.length ≥ ⌈2³²/1000⌉ = 4,294,968` B (~4.29 MB), and it can only ever *tighten* +> the bound: for any product `p ≥ 2³²`, `wrapped(p) = p mod 2³² < 2³² ≤ p`, +> while `original_size` (≤ 512 MiB < 2³²) cannot itself wrap, so the direction +> of the comparison is preserved. The failure mode is therefore **spurious +> rejection** and not a bomb bypass — but a total one: at 4,294,968 B the +> wrapped bound collapses to 704 B, and at the 512 MiB cap it collapses to 0. +> Every entry whose compressed payload is ≥ 4.29 MB becomes permanently +> unreadable on such a target — 99.2 % of the legal range, as a hard error +> rather than a cache miss. Those payloads are legal under this specification; +> an implementation that refuses them is non-conforming. + + +*An earlier revision of this section claimed 32-bit wrapping would corrupt the +bound "in both directions". It cannot: wrapping is fail-closed, as derived above. +Recorded because the mis-stated failure direction, not the bound, was the part an +implementer would have acted on — a believed bypass mis-prioritises the fix.* After `method 1` decompression, the output length MUST equal `original_size` exactly — shorter or longer output is a hard error (the @@ -454,7 +491,7 @@ An SDK implementation of interop/v2 MUST: | **Minimal 3-element container** | Reuse the ByteStorage envelope | The envelope drags xxHash3-64 into every SDK — a second native dependency per language (the PHP-fork class of cost) duplicating integrity the AES-GCM tag already provides when encrypted, and exceeding v1's posture when not. Its `format` field is also dead weight here (the content is always one plain-MessagePack document). What *is* kept from the envelope experience: `bin` payload encoding as normative from birth (protocol 1.1, [decisions/envelope-bin-encoding.md](../decisions/envelope-bin-encoding.md)). | | **Array-of-ints payload rejected** | Inherit cachekit-core's permanent dual-read leniency | That leniency serves a deployed installed base and falls out of rmp-serde for free; interop/v2 has no installed base, and hand-written readers in new languages would pay extra code to be lenient. One legal encoding is the lowest implementation bar. Pinned by vector. | | **Compressed bytes non-canonical, read-side conformance** | Pin one canonical LZ4 output | LZ4 encoders legally differ (implementation, level, version). Pinning writer bytes would freeze one library's output as protocol law and break on its next release. Keys stay byte-canonical; values never needed to be. | -| **Bounds reuse wire-format.md constants (512 MiB / 1000:1)** | Profile-specific numbers | One set of constants fleet-wide; the guards are already implemented, reviewed, and vector-tested in cachekit-core. Integer arithmetic rule carried over verbatim. | +| **Bounds reuse wire-format.md constants (512 MiB / 1000:1)** | Profile-specific numbers | One set of constants fleet-wide; the guards are already implemented, reviewed, and vector-tested in cachekit-core. The integer-width rule is stated in full in both documents — edit this section and [wire-format.md → Decompression Bomb Detection](wire-format.md#decompression-bomb-detection) together; `tools/check-spec-duplication.py` fails CI if they drift. | | **No length padding** | Bucketed padding vs CRIME | Quantized leakage at real cost is not elimination; documented guidance plus the `method 0` / stay-v1 escape hatches are honest. Length-hiding from the backend is explicitly out of protocol scope (v1 leaks exact lengths today). | --- diff --git a/spec/wire-format.md b/spec/wire-format.md index 74e5e89..fee7835 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -372,7 +372,7 @@ let checksum: [u8; 8] = xxh3_64(&original_data).to_be_bytes(); ## Security Limits > [!IMPORTANT] -> All three limits below MUST be enforced by every implementation of the ByteStorage envelope. The decompression bomb check uses integer arithmetic — do not substitute floating-point. +> All three limits below MUST be enforced by every implementation of the ByteStorage envelope. The decompression bomb check uses integer arithmetic — do not substitute a floating-point *ratio*, and see [Decompression Bomb Detection](#decompression-bomb-detection) for the normative integer-width requirement. > Additionally, a decoder MUST validate any declared MessagePack `bin`/array > length header against the remaining input bytes **before** allocating for it — > a 5-byte `bin32` header can otherwise declare a 4 GiB allocation from a @@ -387,20 +387,66 @@ let checksum: [u8; 8] = xxh3_64(&original_data).to_be_bytes(); ### Decompression Bomb Detection -The ratio check uses **integer arithmetic** to prevent floating-point precision bypass: +All three limits above are enforced here, and the step order is normative — +the ratio check relies on both size caps having already passed. The check uses +**integer-valued arithmetic** and never a floating-point *ratio*: ``` +if original_size > MAX_UNCOMPRESSED: + REJECT // 512 MiB cap + +if compressed_size > MAX_COMPRESSED: + REJECT // 512 MiB cap + if compressed_size == 0: REJECT // Zero-length compressed with non-zero original = bomb -max_allowed = MAX_COMPRESSION_RATIO * compressed_size -if max_allowed overflows: - REJECT // Overflow = bomb +// MAX_COMPRESSION_RATIO = 1000. Widen compressed_size to >= 64-bit unsigned +// BEFORE multiplying (see below); the two caps above bound the product < 2^39. +max_allowed = MAX_COMPRESSION_RATIO * uint64(compressed_size) if original_size > max_allowed: REJECT // Ratio exceeded ``` + +The ratio product MUST be computed in **at least 64-bit unsigned integers**: +promote `compressed_size` to a ≥ 64-bit unsigned (or arbitrary-precision) integer *before* +the multiply. Multiplying in pointer width and widening the result afterwards +does not satisfy this, and is invisible on a 64-bit host and in 64-bit CI — it +is the wasm32 defect described below. Every target language has a conforming +path: Rust `u64` (on every target, `wasm32` included), Python's +arbitrary-precision `int`, and JavaScript `Number` — an IEEE-754 double +represents every integer below 2⁵³ exactly and this product is < 2³⁹, so no +`BigInt` is required. Because `compressed_size` ≤ 2²⁹ once the two 512 MiB caps have +passed, the product is < 2³⁹ and cannot overflow 64 bits; that is why the +pseudocode above carries no overflow branch, and why rejecting on overflow is +**not** a substitute for widening — at 32-bit width it would refuse 99.2 % of +the legal `compressed_size` range (see the note below). + +The bound MUST be computed by **multiplication**. Deriving it by division, or +as a *ratio*, is forbidden in any arithmetic — integer or floating-point. +Truncating integer division (`original_size / compressed_size > 1000`) accepts up to +`1000·compressed_size + (compressed_size − 1)`, which is looser than this specification permits, and a +floating-point ratio is the precision bypass the integer rule exists to prevent. + +> [!NOTE] +> **Non-normative rationale — the failure direction under pointer-width +> arithmetic is fail-closed, never a bypass.** 32-bit pointer width is a live +> target: cachekit-ts ships a `wasm32` build. (That build is *not* affected — it +> computes this bound through `cachekit-core`'s `u64`.) Wrapping begins at +> `compressed_size ≥ ⌈2³²/1000⌉ = 4,294,968` B (~4.29 MB), and it can only ever *tighten* +> the bound: for any product `p ≥ 2³²`, `wrapped(p) = p mod 2³² < 2³² ≤ p`, +> while `original_size` (≤ 512 MiB < 2³²) cannot itself wrap, so the direction +> of the comparison is preserved. The failure mode is therefore **spurious +> rejection** and not a bomb bypass — but a total one: at 4,294,968 B the +> wrapped bound collapses to 704 B, and at the 512 MiB cap it collapses to 0. +> Every entry whose compressed payload is ≥ 4.29 MB becomes permanently +> unreadable on such a target — 99.2 % of the legal range, as a hard error +> rather than a cache miss. Those payloads are legal under this specification; +> an implementation that refuses them is non-conforming. + + --- ## Store Flow diff --git a/tools/check-spec-duplication.py b/tools/check-spec-duplication.py new file mode 100644 index 0000000..5c5594d --- /dev/null +++ b/tools/check-spec-duplication.py @@ -0,0 +1,134 @@ +#!/usr/bin/env python3 +"""Fail if the ratio-product rule drifts between wire-format.md and interop-v2.md. + +The >=64-bit ratio-product rule is stated in full in BOTH spec documents rather +than in one with a cross-link, because a third-party implementer reads one +document standalone and a bound stated only elsewhere is a bound they can miss. +That is a deliberate duplication, and it is exactly the shape of the bug LAB-2594 +closed: the two documents already carried divergent normative text for this bound +once (interop-v2 bound the integer width, wire-format said nothing), so an +implementer working from wire-format alone could legally compute the product in +32-bit pointer-width arithmetic. Hand-maintained duplicate prose re-arms that bug +silently -- nothing else in this repo reads spec prose for agreement. + +So the duplication is guarded instead of trusted. Each copy is delimited by +sentinel HTML comments; this compares them modulo each document's operand name +(`compressed_size` in wire-format, `payload.length` in interop-v2), which is the +only difference the two copies are permitted to have. + +**Scope, and what this does NOT catch.** It proves the two blocks say the same +thing. It cannot prove either one is *correct*, and it does not police any other +shared text in the repo -- extending it means adding a sentinel pair and a row to +BLOCKS, not writing a second tool. + +Fails closed: a missing sentinel, an unterminated block, or an empty block is an +error, not a pass. A guard that silently checks nothing is worse than no guard. +Uses explicit failures rather than `assert`, so it cannot be defanged by `-O`. + +Usage: python3 tools/check-spec-duplication.py [repo-root] (exit 1 on drift) +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +# block-id -> [(spec path, operand name normalised away), ...] +BLOCKS: dict[str, list[tuple[str, str]]] = { + "ratio-product-rule": [ + ("spec/wire-format.md", "compressed_size"), + ("spec/interop-v2.md", "payload.length"), + ], +} +PLACEHOLDER = "" + + +def extract(text: str, block_id: str) -> str: + """Return the block body, or raise ValueError naming the exact defect.""" + begin = f"" + if text.count(begin) != 1: + raise ValueError(f"expected exactly 1 BEGIN sentinel, found {text.count(begin)}") + if text.count(end) != 1: + raise ValueError(f"expected exactly 1 END sentinel, found {text.count(end)}") + start = text.index(begin) + start = text.index("-->", start) + len("-->") + stop = text.index(end) + if stop < start: + raise ValueError("END sentinel precedes BEGIN sentinel") + body = text[start:stop].strip() + if not body: + raise ValueError("block is empty") + return body + + +def main(argv: list[str]) -> int: + root = Path(argv[1]) if len(argv) > 1 else Path(__file__).resolve().parent.parent + failures: list[str] = [] + checked = 0 + + for block_id, members in BLOCKS.items(): + bodies: list[tuple[str, str]] = [] + for rel, operand in members: + path = root / rel + try: + body = extract(path.read_text(encoding="utf-8"), block_id) + except OSError as exc: + failures.append(f"{rel}: cannot read ({exc})") + continue + except ValueError as exc: + failures.append(f"{rel}: shared-block '{block_id}' — {exc}") + continue + if operand not in body: + failures.append( + f"{rel}: shared-block '{block_id}' never mentions its operand " + f"'{operand}' — the normalisation cannot be trusted" + ) + continue + bodies.append((rel, body.replace(operand, PLACEHOLDER))) + + if len(bodies) != len(members): + continue # already reported; a partial comparison would be misleading + checked += 1 + + (ref_path, ref_body), *rest = bodies + for rel, body in rest: + if body == ref_body: + continue + import difflib + + diff = "\n".join( + difflib.unified_diff( + ref_body.splitlines(), body.splitlines(), + fromfile=ref_path, tofile=rel, lineterm="", + ) + ) + failures.append( + f"shared-block '{block_id}' differs between {ref_path} and {rel} " + f"(after normalising operand names):\n{diff}" + ) + + if failures: + print( + "check-spec-duplication: the ratio-product rule has drifted between the\n" + "spec documents. Both copies are normative and MUST agree — see the\n" + "rationale at the top of this tool.\n", + file=sys.stderr, + ) + for f in failures: + print(f" {f}", file=sys.stderr) + return 1 + + if checked != len(BLOCKS): + print("check-spec-duplication: no block was fully compared", file=sys.stderr) + return 1 + + print( + f"check-spec-duplication: OK — {checked} shared block(s), " + "every copy identical modulo its operand name" + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/tools/test_check_spec_duplication.py b/tools/test_check_spec_duplication.py new file mode 100644 index 0000000..7017308 --- /dev/null +++ b/tools/test_check_spec_duplication.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python3 +"""Mutation tests for check-spec-duplication.py. + +A drift guard that cannot be shown to FAIL is indistinguishable from a guard that +reports OK unconditionally -- and this one guards prose, where the plausible +mutation is a one-word edit to a single copy, not a structural break. So each case +below poisons a copy of the real spec tree and asserts the guard notices. + +Run: python3 tools/test_check_spec_duplication.py (exit 1 on any failure) +""" + +from __future__ import annotations + +import shutil +import subprocess +import sys +import tempfile +from collections.abc import Callable +from pathlib import Path + +HERE = Path(__file__).resolve().parent +ROOT = HERE.parent +CHECKER = HERE / "check-spec-duplication.py" +WIRE = "spec/wire-format.md" +INTEROP = "spec/interop-v2.md" + +# The obligation sentence, present in both copies -- the realistic drift target. +MUST = "The ratio product MUST be computed in **at least 64-bit unsigned integers**" + + +def edit(rel: str, old: str, new: str, *, once: bool = True) -> Callable[[Path], None]: + def mutate(root: Path) -> None: + path = root / rel + text = path.read_text(encoding="utf-8") + if text.count(old) < 1: + raise SystemExit(f"test setup broken: {old!r} not in {rel}") + path.write_text(text.replace(old, new, 1 if once else -1), encoding="utf-8") + + return mutate + + +# (name, mutate(root) -> None, expected_exit) +CASES: list[tuple[str, Callable[[Path], None], int]] = [ + ("unmodified tree", lambda root: None, 0), + # --- must be CAUGHT (exit 1) --- + ( + "one copy weakened to 32-bit (the LAB-2594 bug, re-armed)", + edit(INTEROP, "at least 64-bit unsigned integers", "at least 32-bit unsigned integers"), + 1, + ), + ( + "MUST downgraded to SHOULD in one copy", + edit(WIRE, MUST, MUST.replace("MUST", "SHOULD")), + 1, + ), + ( + "sentence deleted from one copy", + edit( + INTEROP, + "The bound MUST be computed by **multiplication**.", + "", + ), + 1, + ), + ("BEGIN sentinel removed", edit(WIRE, " *An earlier revision of this section claimed 32-bit wrapping would corrupt the diff --git a/spec/wire-format.md b/spec/wire-format.md index fee7835..5747c44 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -391,7 +391,7 @@ All three limits above are enforced here, and the step order is normative — the ratio check relies on both size caps having already passed. The check uses **integer-valued arithmetic** and never a floating-point *ratio*: -``` +```text if original_size > MAX_UNCOMPRESSED: REJECT // 512 MiB cap @@ -439,12 +439,21 @@ floating-point ratio is the precision bypass the integer rule exists to prevent. > the bound: for any product `p ≥ 2³²`, `wrapped(p) = p mod 2³² < 2³² ≤ p`, > while `original_size` (≤ 512 MiB < 2³²) cannot itself wrap, so the direction > of the comparison is preserved. The failure mode is therefore **spurious -> rejection** and not a bomb bypass — but a total one: at 4,294,968 B the -> wrapped bound collapses to 704 B, and at the 512 MiB cap it collapses to 0. -> Every entry whose compressed payload is ≥ 4.29 MB becomes permanently -> unreadable on such a target — 99.2 % of the legal range, as a hard error -> rather than a cache miss. Those payloads are legal under this specification; -> an implementation that refuses them is non-conforming. +> rejection** and not a bomb bypass. It is not, however, uniform: the wrapped +> bound sweeps the whole `[0, 2³²)` range in steps of 1000 as `compressed_size` +> grows, so it falls below the 512 MiB uncompressed cap — the only region where +> it can reject a legal entry at all — for exactly ⅛ of each `2³²/1000 ≈ 4.29` MB +> wrap cycle (`2²⁹/2³² = 1/8`). Elsewhere in the cycle the wrapped bound still +> exceeds every permitted `original_size`, and the entry is accepted. Within +> that ⅛, an entry is rejected only when its `original_size` exceeds the wrapped +> bound; the worst positions are severe — at `compressed_size = 4,294,968` B the +> bound collapses to 704 B, and at the 512 MiB cap to 0. Those payloads are +> legal under this specification; an implementation that refuses them is +> non-conforming. The two figures in this section measure different things and +> must not be conflated: *wrapping* rejects within ⅛ of each cycle, whereas +> *rejecting on overflow* refuses every payload past the same 4.29 MB threshold +> — the whole 99.2 % of the legal range — which is precisely why a checked +> multiply is not a substitute for widening the operand. --- diff --git a/tools/test_check_spec_duplication.py b/tools/test_check_spec_duplication.py index 7017308..7c6de99 100644 --- a/tools/test_check_spec_duplication.py +++ b/tools/test_check_spec_duplication.py @@ -41,7 +41,7 @@ def mutate(root: Path) -> None: # (name, mutate(root) -> None, expected_exit) CASES: list[tuple[str, Callable[[Path], None], int]] = [ - ("unmodified tree", lambda root: None, 0), + ("unmodified tree", lambda _: None, 0), # --- must be CAUGHT (exit 1) --- ( "one copy weakened to 32-bit (the LAB-2594 bug, re-armed)", From aca70529574c8af2a38215e2bf765c4f442a7e14 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Wed, 2 Sep 2026 18:30:07 +1000 Subject: [PATCH 3/3] fix: apply expert-panel findings on the wrapping-impact rationale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Panel re-run at high stakes on cd840aa, because that commit changed crypto/protocol spec text after the previous panel had reviewed it — the gate keys off current HEAD, not "a panel ran on this ticket once". All four agents confirmed the arithmetic correction is right; all four found defects in how I wrote it. Every figure below re-verified independently before acting. Restored, and this is the one that mattered. The rewrite silently dropped "permanently unreadable ... as a hard error rather than a cache miss" — the only clause in the note that told an implementer what happens to their application. Two agents flagged it independently. The note had been left stating frequency with no symptom and no persistence, which reads as a rare recoverable edge case; the truth is the wrapped bound is a pure function of the size, so an affected entry fails identically on every read, forever. Correcting an overstatement into an understatement is not a fix. Fixed a backwards argument, also found independently by two agents. The closing sentence cited 99.2 % for reject-on-overflow against 1/8 for wrapping and concluded "which is precisely why a checked multiply is not a substitute for widening" — on those figures it ranks the silent defect as 8x milder than the loud one, steering a reader toward the unchecked multiply. The real reason a checked multiply is no substitute is that it is unnecessary: after both size caps the product is < 2^39 and cannot overflow 64 bits. Both are non-conforming; the numbers compare blast radius, not acceptability. Dropped a false superlative I introduced. "The worst positions are severe — 704 B" is wrong: the wrapped bound lands only on multiples of gcd(1000, 2^32) = 8, and its nonzero floor is 8 B at compressed_size = 115,964,117, which is 88x below 704. A third party calibrating a conformance test to "the worst case" would have built it 88x too weak. Now states the floor and names the gcd that produces it. Fixed a false equation: `2^32/1000 ~= 4.29` MB put the unit outside the code span, so the span asserted 2^32/1000 ~= 4.29 — off by 10^6. Dropped "exactly" from the 1/8 claim: measured density is 0.1249998, not 1/8 exactly, and "exactly" is the wrong word in a section whose subject is that approximate claims about integer arithmetic caused this bug. Dropped the "exceeds every permitted original_size" clause, false at compressed_size = 335,544,320 where the wrapped bound equals 2^29. Cut per the pragmatism review: the sweep sentence, the 7/8 complement, and the restatement of the normative pseudocode inside a non-normative note. Restating a normative rule in the rationale is how the two drift. The CHANGELOG carried a third copy of the derivation; trimmed to the outcome. The one-off vulgar-fraction glyph is gone. Scoped the note's headline. It claimed "the failure direction under pointer-width arithmetic is fail-closed, never a bypass", which is true of the ratio product and false in general: nothing in either document binds the width of original_size, and truncating it IS a bypass — a declared 4,294,968,296 becomes 1000, clears the 512 MiB cap, clears the ratio bound, and is accepted where 64-bit rejects. Verified. The headline now says "the ratio product's failure direction" and flags original_size as a separate unbound obligation, so the spec no longer claims a property it does not have. Closing the gap is out of scope for a docs-only PR on a different bound and is filed as LAB-2734, together with the conformance vector that cannot detect it: reject_declared_size_bomb declares 2^40, whose low 32 bits are zero, so a truncating reader sees 0 and rejects on the length mismatch instead — passing the vector for the wrong reason. One correction to my own previous commit message. It rebutted TRY003 on the grounds that no such rule is configured here, then applied ARG005, which is equally unconfigured — arguing both sides in one commit. The honest line is cost, not provenance: ARG005 and MD040 are one token each and match conventions the repo already follows, while TRY003 wants custom exception classes for four raise sites in a 134-line stdlib guard whose documented contract is to name the exact defect. The rebuttal stands on that basis. Not applied: a second erratum footnote for this correction. The existing one exists because the old text pointed the wrong *direction*, which mis-prioritises a fix. Overstating severity errs safe and changes nothing an implementer does. check-spec-duplication passes, its 9-case mutation suite still fails closed, and all stdlib verify legs are green. --- CHANGELOG.md | 17 ++++++++--------- spec/interop-v2.md | 34 +++++++++++++++++----------------- spec/wire-format.md | 34 +++++++++++++++++----------------- 3 files changed, 42 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29338a6..ea1e4e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,15 +43,14 @@ All notable changes to the CacheKit Protocol Specification. - **`spec/interop-v2.md`'s rationale corrected**: 32-bit wrapping was said to "silently corrupt the bound **in both directions**". It cannot. Wrapping begins at `⌈2³²/1000⌉ = 4,294,968` B (~4.29 MB) and can only *tighten* the bound, so - the failure mode is **spurious rejection**, never a bomb bypass. The rejection - is not uniform: the wrapped bound sweeps `[0, 2³²)` in steps of 1000, so it - drops below the 512 MiB cap — the only region where it can refuse a legal - entry — for exactly ⅛ of each 4.29 MB wrap cycle (`2²⁹/2³²`), and inside that - eighth only when `original_size` exceeds it. The worst positions are still - severe (704 B at the threshold, 0 at the 512 MiB cap). Distinct from the - 99.2 % figure above, which measures *reject-on-overflow*, not wrapping. Both - documents now say so, marked non-normative, and interop v2 records the - corrected claim in place. + the failure mode is **spurious rejection**, never a bomb bypass — a hard + error rather than a cache miss, and permanent for an affected entry. The + rejection is not uniform (a density of `2²⁹/2³² = 1/8` over the legal range), + but where it bites the collapse is near-total: 704 B at the threshold, 8 B at + its floor, 0 at the 512 MiB cap. The 99.2 % figure four bullets above measures + *reject-on-overflow*, a different mechanism, and is unaffected. Both documents + now say so, marked non-normative, and interop v2 records the corrected claim + in place. - **New CI guard**: [`tools/check-spec-duplication.py`](tools/check-spec-duplication.py) compares the two copies of the rule (delimited by sentinel comments) modulo the operand rename, with a 9-case mutation suite diff --git a/spec/interop-v2.md b/spec/interop-v2.md index ca3a453..995b532 100644 --- a/spec/interop-v2.md +++ b/spec/interop-v2.md @@ -276,29 +276,29 @@ Truncating integer division (`original_size / payload.length > 1000`) accepts up floating-point ratio is the precision bypass the integer rule exists to prevent. > [!NOTE] -> **Non-normative rationale — the failure direction under pointer-width -> arithmetic is fail-closed, never a bypass.** 32-bit pointer width is a live +> **Non-normative rationale — the *ratio product's* failure direction under +> pointer-width arithmetic is fail-closed, never a bypass.** (This covers the +> product only. The width of `original_size` itself is a separate obligation +> not bound by this rule.) 32-bit pointer width is a live > target: cachekit-ts ships a `wasm32` build. (That build is *not* affected — it > computes this bound through `cachekit-core`'s `u64`.) Wrapping begins at > `payload.length ≥ ⌈2³²/1000⌉ = 4,294,968` B (~4.29 MB), and it can only ever *tighten* > the bound: for any product `p ≥ 2³²`, `wrapped(p) = p mod 2³² < 2³² ≤ p`, > while `original_size` (≤ 512 MiB < 2³²) cannot itself wrap, so the direction > of the comparison is preserved. The failure mode is therefore **spurious -> rejection** and not a bomb bypass. It is not, however, uniform: the wrapped -> bound sweeps the whole `[0, 2³²)` range in steps of 1000 as `payload.length` -> grows, so it falls below the 512 MiB uncompressed cap — the only region where -> it can reject a legal entry at all — for exactly ⅛ of each `2³²/1000 ≈ 4.29` MB -> wrap cycle (`2²⁹/2³² = 1/8`). Elsewhere in the cycle the wrapped bound still -> exceeds every permitted `original_size`, and the entry is accepted. Within -> that ⅛, an entry is rejected only when its `original_size` exceeds the wrapped -> bound; the worst positions are severe — at `payload.length = 4,294,968` B the -> bound collapses to 704 B, and at the 512 MiB cap to 0. Those payloads are -> legal under this specification; an implementation that refuses them is -> non-conforming. The two figures in this section measure different things and -> must not be conflated: *wrapping* rejects within ⅛ of each cycle, whereas -> *rejecting on overflow* refuses every payload past the same 4.29 MB threshold -> — the whole 99.2 % of the legal range — which is precisely why a checked -> multiply is not a substitute for widening the operand. +> rejection**, not a bomb bypass — but a hard error rather than a cache miss, +> and a permanent one: the wrapped bound is a pure function of +> `payload.length`, so an affected entry fails identically on every read. It +> does not bite everywhere — only where the wrapped bound falls below the +> 512 MiB cap, a density of `2²⁹/2³² = 1/8` over the legal range — but where it +> does, the collapse is near-total: 704 B at that first threshold, 8 B at +> `payload.length = 115,964,117` (the wrapped bound only ever lands on +> multiples of `gcd(1000, 2³²) = 8`), and 0 at the 512 MiB cap. Those payloads +> are legal under this specification; an implementation that refuses them is +> non-conforming. So is one that rejects on overflow instead of widening: that +> refuses the entire 99.2 % of the legal range above the same threshold, and it +> is unnecessary besides — once the two size caps have passed, the product is +> < 2³⁹ and cannot overflow 64 bits at all. *An earlier revision of this section claimed 32-bit wrapping would corrupt the diff --git a/spec/wire-format.md b/spec/wire-format.md index 5747c44..72676bc 100644 --- a/spec/wire-format.md +++ b/spec/wire-format.md @@ -431,29 +431,29 @@ Truncating integer division (`original_size / compressed_size > 1000`) accepts u floating-point ratio is the precision bypass the integer rule exists to prevent. > [!NOTE] -> **Non-normative rationale — the failure direction under pointer-width -> arithmetic is fail-closed, never a bypass.** 32-bit pointer width is a live +> **Non-normative rationale — the *ratio product's* failure direction under +> pointer-width arithmetic is fail-closed, never a bypass.** (This covers the +> product only. The width of `original_size` itself is a separate obligation +> not bound by this rule.) 32-bit pointer width is a live > target: cachekit-ts ships a `wasm32` build. (That build is *not* affected — it > computes this bound through `cachekit-core`'s `u64`.) Wrapping begins at > `compressed_size ≥ ⌈2³²/1000⌉ = 4,294,968` B (~4.29 MB), and it can only ever *tighten* > the bound: for any product `p ≥ 2³²`, `wrapped(p) = p mod 2³² < 2³² ≤ p`, > while `original_size` (≤ 512 MiB < 2³²) cannot itself wrap, so the direction > of the comparison is preserved. The failure mode is therefore **spurious -> rejection** and not a bomb bypass. It is not, however, uniform: the wrapped -> bound sweeps the whole `[0, 2³²)` range in steps of 1000 as `compressed_size` -> grows, so it falls below the 512 MiB uncompressed cap — the only region where -> it can reject a legal entry at all — for exactly ⅛ of each `2³²/1000 ≈ 4.29` MB -> wrap cycle (`2²⁹/2³² = 1/8`). Elsewhere in the cycle the wrapped bound still -> exceeds every permitted `original_size`, and the entry is accepted. Within -> that ⅛, an entry is rejected only when its `original_size` exceeds the wrapped -> bound; the worst positions are severe — at `compressed_size = 4,294,968` B the -> bound collapses to 704 B, and at the 512 MiB cap to 0. Those payloads are -> legal under this specification; an implementation that refuses them is -> non-conforming. The two figures in this section measure different things and -> must not be conflated: *wrapping* rejects within ⅛ of each cycle, whereas -> *rejecting on overflow* refuses every payload past the same 4.29 MB threshold -> — the whole 99.2 % of the legal range — which is precisely why a checked -> multiply is not a substitute for widening the operand. +> rejection**, not a bomb bypass — but a hard error rather than a cache miss, +> and a permanent one: the wrapped bound is a pure function of +> `compressed_size`, so an affected entry fails identically on every read. It +> does not bite everywhere — only where the wrapped bound falls below the +> 512 MiB cap, a density of `2²⁹/2³² = 1/8` over the legal range — but where it +> does, the collapse is near-total: 704 B at that first threshold, 8 B at +> `compressed_size = 115,964,117` (the wrapped bound only ever lands on +> multiples of `gcd(1000, 2³²) = 8`), and 0 at the 512 MiB cap. Those payloads +> are legal under this specification; an implementation that refuses them is +> non-conforming. So is one that rejects on overflow instead of widening: that +> refuses the entire 99.2 % of the legal range above the same threshold, and it +> is unnecessary besides — once the two size caps have passed, the product is +> < 2³⁹ and cannot overflow 64 bits at all. ---