diff --git a/crates/epiphany-bundle/DECISIONS.md b/crates/epiphany-bundle/DECISIONS.md index 353638f..46676cf 100644 --- a/crates/epiphany-bundle/DECISIONS.md +++ b/crates/epiphany-bundle/DECISIONS.md @@ -446,18 +446,28 @@ lenient, **non-injective** codec, inherited by every structure embedding a Its visibility depended entirely on whether the embedder had a whole-value re-encode guard: -- `Manifest::decode` **has** one, and it is *total* — verified by exhaustive - single-byte perturbation, every one rejected. It caught this. (`encode_body` - sorts and deduplicates every vector, and `manifest_id` is derived from the - body, which is why the guard is complete here where `MaterializedState`'s is - not — see `epiphany-ops/DECISIONS.md` §"Push 5 / P2".) +- `Manifest::decode` **has** one, and it is total *by argument*: `manifest_id` + is derived from the body, so a body edit fails the id check and an id edit + fails the derivation; and `encode_body` sorts and deduplicates every vector, + so an out-of-order or duplicated encoding cannot round-trip. That is what + makes the guard complete here where `MaterializedState`'s is not (see + `epiphany-ops/DECISIONS.md` §"Push 5 / P2"). It caught this defect. + + The accompanying test is exhaustive over every single-byte *replacement* of + one constructed manifest (each byte × the 255 other values) — evidence for the + argument, not a proof of totality, and blind to multi-byte perturbations. An + earlier revision of this record claimed "verified by exhaustive single-byte + perturbation" while the test actually tried three XOR deltas per byte. Caught + in review; the test now does what the sentence says, and the sentence no + longer carries the weight of the proof. - `OperationIndex::decode` has **no** guard; it validates per-site instead. It accepted both byte strings while its own doc promised to *"reject (never normalizing) any non-canonical form"*. That promise was false. Fixed at the source rather than papered over at the index: a non-zero `None` -parameter is now rejected. Exhaustive sweep (every byte × every value, plus an -8-byte extreme-integer window) finds no remaining non-injective site. +parameter is now rejected, for every one of the 255 non-zero values. A sweep of +one `OperationIndex` payload (every byte × every value, plus an 8-byte +extreme-integer window) finds no remaining non-injective site in it. **This contradicted ratified spec text**, which said the byte was *"present but zero, and ignored on read"*. Escalated rather than fixed unilaterally; the user @@ -474,8 +484,12 @@ index corpus was built from real `OperationIndex::build` output. The smoke tests now assert on a `WireFuzzCoverage` so that can never silently regress. 1.5M inputs across five seeds, ~1s each, clean after the fix. -Three regressions, each mutation-verified by restoring the leniency: -`compression_none_rejects_a_non_zero_parameter_byte` (the codec), -`a_lenient_compression_byte_is_rejected_rather_than_normalized` (the index, the -surface that exposed it), and `every_single_byte_perturbation_of_a_manifest_is_rejected` -(the guard's totality, and the asymmetry that hid the bug). +Three regressions. Restoring the leniency fails exactly two of them: +`compression_none_rejects_a_non_zero_parameter_byte` (the codec) and +`a_lenient_compression_byte_is_rejected_rather_than_normalized` (the index — +the surface that exposed it). The third, +`every_single_byte_replacement_of_a_manifest_is_rejected`, stays **green** under +that mutation, because the guard rejects the bytes whatever the sub-codec does. +That is not a weak test; it is the asymmetry, and it locks the guard rather than +the codec. A regression suite where every test fails on every mutation would be +telling us less, not more. diff --git a/crates/epiphany-bundle/src/chunk.rs b/crates/epiphany-bundle/src/chunk.rs index c9729ff..6299973 100644 --- a/crates/epiphany-bundle/src/chunk.rs +++ b/crates/epiphany-bundle/src/chunk.rs @@ -327,26 +327,30 @@ mod tests { CompressionAlgorithm::None ); - for param in [1u8, 0x7F, 0xFF] { - let lenient = vec![0, param]; + // Exhaustive: every non-zero parameter byte, not three representatives. + for param in 1u16..=255 { + let lenient = vec![0, param as u8]; assert!( CompressionAlgorithm::decode(&mut Reader::new(&lenient)).is_err(), "None with parameter {param:#04x} must be rejected, never normalized to zero" ); } - // The parameter is meaningful for the other two, so it round-trips. - for algo in [ - CompressionAlgorithm::Zstd { level: 0xFF }, - CompressionAlgorithm::Reserved(0xFF), - ] { - let mut w = Writer::new(); - algo.encode(&mut w); - let bytes = w.into_bytes(); - assert_eq!( - CompressionAlgorithm::decode(&mut Reader::new(&bytes)).unwrap(), - algo - ); + // And the parameter round-trips for every value of the two variants that + // give it meaning, so the strictness is confined to `None`. + for param in 0u16..=255 { + for algo in [ + CompressionAlgorithm::Zstd { level: param as u8 }, + CompressionAlgorithm::Reserved(param as u8), + ] { + let mut w = Writer::new(); + algo.encode(&mut w); + let bytes = w.into_bytes(); + assert_eq!( + CompressionAlgorithm::decode(&mut Reader::new(&bytes)).unwrap(), + algo + ); + } } } diff --git a/crates/epiphany-bundle/src/manifest.rs b/crates/epiphany-bundle/src/manifest.rs index ba4cff0..8c050d9 100644 --- a/crates/epiphany-bundle/src/manifest.rs +++ b/crates/epiphany-bundle/src/manifest.rs @@ -709,19 +709,24 @@ mod tests { use crate::chunk::{chunk_id, ChunkKind}; use crate::ids::SnapshotId; - /// The manifest's whole-value re-encode guard is **total**: every single-byte - /// perturbation is rejected. `manifest_id` is derived from the body, so a - /// body edit fails the id check and an id edit fails the derivation; and - /// `encode_body` sorts and deduplicates every vector, so an out-of-order - /// encoding cannot round-trip either. + /// Exhaustive over **every single-byte replacement of this constructed + /// manifest**: each of its bytes, each of the 255 other values. All rejected. /// - /// This is what makes the guard complete *here* and not in + /// That is a finite check, not a proof of totality. Totality rests on the + /// *argument*: `manifest_id` is derived from the body, so a body edit fails + /// the id check and an id edit fails the derivation; and `encode_body` sorts + /// and deduplicates every vector, so an out-of-order or duplicated encoding + /// cannot round-trip. The test is evidence for that argument over one + /// manifest, not a substitute for it — and multi-byte perturbations are out + /// of its reach entirely. + /// + /// The argument is what makes the guard complete *here* and not in /// `MaterializedState` (epiphany-ops), whose encoder writes its `Vec` fields /// verbatim, nor in `OperationIndex`, which has no guard at all. The lenient /// `CompressionAlgorithm::None` parameter byte was invisible through the /// manifest for exactly this reason, and visible through the index. #[test] - fn every_single_byte_perturbation_of_a_manifest_is_rejected() { + fn every_single_byte_replacement_of_a_manifest_is_rejected() { use crate::chunk::{ChunkRef, CompressionAlgorithm}; use crate::ids::SchemaVersion; @@ -743,16 +748,19 @@ mod tests { assert_eq!(Manifest::decode(&bytes).unwrap().encode(), bytes); for i in 0..bytes.len() { - for delta in [1u8, 0x7F, 0xFF] { - let mut b = bytes.clone(); - b[i] ^= delta; - if b == bytes { + let original = bytes[i]; + for value in 0u16..=255 { + let value = value as u8; + if value == original { continue; } + let mut b = bytes.clone(); + b[i] = value; match Manifest::decode(&b) { Err(_) => {} Ok(decoded) => panic!( - "byte {i} ^ {delta:#04x} was accepted; re-encode matches input: {}", + "byte {i} = {value:#04x} (was {original:#04x}) was accepted; \ + re-encode matches input: {}", decoded.encode() == b ), }