diff --git a/.github/workflows/standards-build.yml b/.github/workflows/standards-build.yml index 9e9f7d9fe..a824df3db 100644 --- a/.github/workflows/standards-build.yml +++ b/.github/workflows/standards-build.yml @@ -279,8 +279,32 @@ jobs: # # No path argument: these binaries take none. They check the # implementation against in-code expectations, which is a different claim - # from validating the published `conformance/v1` files on disk — nothing - # in CI does that yet, and this step does not pretend to. + # from validating the published `conformance/v1` files on disk. + # + # That second claim is made elsewhere, by tests inside the reference + # implementations that read `conformance/v1` off disk, which the `Test` + # step above already runs — so it needs no entry here and a missing + # `conformance:` key is not evidence a standard's pack is unchecked. + # + # Deliberately not "a `tests/pack.rs` in each conformance crate": that + # is the convention in several of them, not a rule, and some crates read + # the pack from a test named something else or from another crate + # entirely. So counting files by that name does not audit pack coverage, + # and finding a crate without one is not a finding. The question is + # which tests read the published bytes: + # + # grep -rl --include=*.rs -E 'conformance/v[0-9]' / + # + # That mistake has been made here twice, both times concluding standards + # were uncovered when they were not. + # + # `joule-code` is deliberately absent from that key. Its verifier does read + # the published pack, but it requires a `verify ` argument and + # prints usage with a non-zero exit when given none, so it cannot be + # invoked by this step's no-argument form. It has its own workflow, + # `joule-code-conformance.yml`, plus a `tests/pack.rs` that the `Test` step + # above runs — the latter because that workflow is `paths:`-filtered to + # `joule-code/**`, while the crate depends on `jcp-*` across workspaces. - name: Conformance suite if: matrix.conformance working-directory: ${{ matrix.dir }} diff --git a/joule-code/joule-code-rs/crates/joule-code-conformance/tests/pack.rs b/joule-code/joule-code-rs/crates/joule-code-conformance/tests/pack.rs new file mode 100644 index 000000000..25b09d8fa --- /dev/null +++ b/joule-code/joule-code-rs/crates/joule-code-conformance/tests/pack.rs @@ -0,0 +1,150 @@ +//! Run the published `joule-code/conformance/v1` pack from inside the test suite. +//! +//! This crate ships a complete pack verifier — `verify_pack`, reachable as +//! `joule-code-conformance verify `, which returns a FAIL count that +//! `main()` turns into a non-zero exit. A dedicated workflow, +//! `.github/workflows/joule-code-conformance.yml`, already runs it. So the pack is +//! not unchecked, and this file is not what makes it checked. +//! +//! What it adds is coverage in the two places that gate misses. +//! +//! **It cannot tell a verified pack from a skipped one.** The gate is +//! `grep -Eq '0 failed'` over the summary line. `verify_pack` is documented to let +//! SKIPs pass — they mark derived fields pending regeneration — so +//! `0 passed, 0 failed, 24 skipped` satisfies that grep exactly as well as +//! `24 passed, 0 failed, 0 skipped` does, and so does a pack that shrank to one +//! vector. This file pins the count and requires `skip == 0`, which is the +//! distinction the grep cannot draw. +//! +//! **It is `paths:`-filtered to `joule-code/**`, and this crate reaches outside +//! that.** `joule-code-conformance` depends on `jcp-schema`, `jcp-grant` and +//! `jcp-receipt` by relative path across workspaces, and `joule-code-joulecontract` +//! on `joulecontract-schema`. A change in `jcp` to canonical CBOR or the receipt +//! preimage would change the `op_id`s and signatures this pack pins — and would not +//! touch `joule-code/**`, so that workflow would not run. `standards-build.yml` +//! carries a deliberate "NO `paths:` filter" and runs `cargo test --workspace`, so +//! hosting the call here covers the cross-standard case. +//! +//! Hosting it as a test also sidesteps an argument problem worth recording: the +//! other five conformance binaries in this repo take no arguments, so the shared +//! matrix step invokes them as `cargo run -p --bin `. This one +//! requires `verify ` and exits FAILURE given none, which is why it needs +//! its own workflow rather than a `conformance:` key. +//! +//! The second test is the one that matters most. A verifier that reports PASS on a +//! pack it never really read is worse than no verifier, because it manufactures +//! confidence — a tampered copy of another standard's pack once still reported +//! 15/15. So the tamper case does not trust `verify_pack`'s output; it corrupts a +//! copy of the pack and requires the verifier to notice. +//! +use std::path::PathBuf; + +use joule_code_conformance::verify_pack; + +/// Every vector in the published pack, counted so that losing one is not silent. +/// +/// Pinned rather than bounded: `>= 24` would let a vector be deleted as long as +/// another was added. When you legitimately add a case, update this number — that +/// edit is the point, it makes the corpus growing a reviewed event. +const PUBLISHED_VECTORS: u32 = 24; + +fn pack_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../../../conformance/v1") + .canonicalize() + .expect("the conformance pack must exist next to the spec") +} + +/// The published pack still passes against the current implementation. +/// +/// `verify_pack` recomputes each vector's derived fields — every `op_id` is BLAKE3 +/// over canonical CBOR of its `CodingOp`, every binding and receipt signature is a +/// real Ed25519 signature over the spec's preimage — so this fails if the +/// implementation drifts from the bytes a third party downloads. +#[test] +fn the_published_pack_still_verifies() { + let report = verify_pack(&pack_dir()).expect("the pack must be readable and well-formed"); + + assert_eq!( + report.fail, + 0, + "published vectors failed against the implementation:\n{}", + report + .lines + .iter() + .filter(|l| l.contains("FAIL")) + .cloned() + .collect::>() + .join("\n") + ); + + // A SKIP is an unverified vector. `verify_pack` is documented to let skips pass + // — they mark derived fields pending regeneration — so a skip would otherwise be + // indistinguishable from a check that ran. The pack currently has none, and + // `conformance/v1/regeneration.md` states that as a property ("all vectors pass, + // none skipped"). Pin it, so reintroducing a placeholder is a deliberate act. + assert_eq!(report.skip, 0, "a vector is skipped, so it is published but unverified"); + + assert_eq!( + report.pass, PUBLISHED_VECTORS, + "expected {PUBLISHED_VECTORS} vectors to be checked, {} were — a vector file or \ + case was added or lost", + report.pass + ); +} + +/// The verifier actually reads the bytes: corrupt them and it must object. +/// +/// Without this, `the_published_pack_still_verifies` proves only that `verify_pack` +/// returns `fail == 0`, which a verifier that silently skipped everything would also +/// do. Here a signature is flipped in a scratch copy — the pack on disk is never +/// touched — and the run must either fail a vector or refuse the pack outright. +#[test] +fn a_tampered_pack_is_not_reported_as_passing() { + let src = pack_dir(); + let tmp = std::env::temp_dir().join(format!("jc-pack-tamper-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(&tmp).expect("scratch dir"); + + for entry in std::fs::read_dir(&src).expect("pack dir reads") { + let entry = entry.expect("dir entry"); + if entry.path().extension().is_some_and(|e| e == "json") { + std::fs::copy(entry.path(), tmp.join(entry.file_name())).expect("copy vector file"); + } + } + + // Flip one hex digit of the first signature in the file. The value stays + // well-formed hex of the same length, so this is a cryptographic failure and not + // a parse error — the verifier has to actually check the signature to catch it. + // + // The offset is found by scanning to the opening quote of the value rather than + // by matching a literal `"signature": "`, because the pack is written with the + // values column-aligned and the run of spaces is not something a test should + // depend on. + let path = tmp.join("signature_vectors.json"); + let text = std::fs::read_to_string(&path).expect("signature vectors read"); + let key = text.find("\"signature\":").expect("pack has a signature field"); + let at = 1 + key + + text[key + "\"signature\":".len()..] + .find('"') + .expect("signature field has a value") + + "\"signature\":".len(); + let mut bytes = text.into_bytes(); + assert!(bytes[at].is_ascii_hexdigit(), "did not land on the signature's hex"); + bytes[at] = if bytes[at] == b'a' { b'b' } else { b'a' }; + std::fs::write(&path, &bytes).expect("write tampered vectors"); + + // Refusing the pack outright is also a detection rather than a miss, so only the + // `Ok` case carries an assertion: what must not happen is a clean bill of health. + if let Ok(report) = verify_pack(&tmp) { + assert!( + report.fail > 0, + "a forged signature was reported as {} passed / {} failed — the verifier is \ + not checking what it publishes", + report.pass, + report.fail + ); + } + + let _ = std::fs::remove_dir_all(&tmp); +}