From 31fad9e2af5b2bd47f2c33ff784e10026136ffca Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:14:08 +0000 Subject: [PATCH 1/6] ci(miri): check the unsafe decode path under the interpreter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `Yokeable`/`StableDeref` impls behind `OwnedNodeRef` transmute lifetimes over borrowed decode output, and `zlib_pool` extends a Vec's length over inflate's uninitialized spare capacity. Neither clippy nor a native test observes an aliasing violation or an uninit read there — the first symptom would be a miscompile. Adds a Miri workflow over wacore-binary (both the portable-SIMD and the scalar decode paths), wacore-appstate and wacore-noise: the pure crates that carry the unsafe and drive it over real protocol payloads. The tokio/SQLite crates stay out; Miri has no epoll or FFI. The zlib fixtures only reach window refill and buffer growth at hundreds of KB to MB, which the interpreter cannot finish in CI time, so they are ignored under `cfg(miri)` and a 1 KB twin keeps `set_len` covered. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V1mXhXv7K1bxPnCL3cYF2b --- .github/workflows/miri.yml | 92 ++++++++++++++++++++++++++++++++++ AGENTS.md | 2 + wacore/binary/src/zlib_pool.rs | 25 +++++++++ 3 files changed, 119 insertions(+) create mode 100644 .github/workflows/miri.yml diff --git a/.github/workflows/miri.yml b/.github/workflows/miri.yml new file mode 100644 index 000000000..7c43c851a --- /dev/null +++ b/.github/workflows/miri.yml @@ -0,0 +1,92 @@ +permissions: + contents: read +name: Miri + +on: + push: + branches: [main] + pull_request: + branches: [main] + workflow_call: + +env: + CARGO_TERM_COLOR: always + # Keep in sync with PROTOC_VERSION in main.yml — env is per-workflow, so the + # value cannot be shared across files without a repo-level Actions variable. + PROTOC_VERSION: '3.25.3' + CARGO_INCREMENTAL: "0" + # Miri interprets MIR and never links, so the workspace's lld/ICF and + # -Zshare-generics rustflags buy nothing here; a set-but-empty RUSTFLAGS takes + # precedence over .cargo/config.toml (same lever main.yml's stable job pulls). + RUSTFLAGS: "" + +jobs: + miri: + name: Miri (${{ matrix.name }}) + runs-on: ubuntu-latest + # Interpretation costs two orders of magnitude over native. The fixtures the + # gate covers are small by construction (the MB-scale zlib ones are + # `#[cfg_attr(miri, ignore)]`), but a cold sysroot build alone is minutes. + timeout-minutes: 30 + strategy: + # Each leg is an independent UB question; one failing should not hide the + # verdict on the others. + fail-fast: false + matrix: + include: + # wacore-binary owns the workspace's only load-bearing `unsafe`: the + # `Yokeable`/`StableDeref` impls behind `OwnedNodeRef` (two lifetime + # transmutes over borrowed decode output) and the `set_len` over + # inflate's uninitialized spare capacity in `zlib_pool`. Both are + # invisible to clippy and to native tests — nothing observes the + # aliasing violation or the uninit read until it miscompiles. + - name: wacore-binary + cache-key: binary-simd + args: -p wacore-binary --lib + # The portable-SIMD scanners in the decoder/encoder and their scalar + # fallbacks are separate code paths, and `--no-default-features` is the + # only way to reach the latter. + - name: wacore-binary (no simd) + cache-key: binary-scalar + args: -p wacore-binary --no-default-features --lib + # No `unsafe` of our own, but both drive wacore-binary's zero-copy + # decode over real protocol payloads and pull the crypto stack + # (aes/sha2/curve25519), whose unsafe backends this exercises. + - name: wacore-appstate + cache-key: appstate + args: -p wacore-appstate --lib + - name: wacore-noise + cache-key: noise + args: -p wacore-noise --lib + steps: + - uses: actions/checkout@v6 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: nightly-2026-06-16 + # rust-src is what `cargo miri setup` compiles the interpreter's + # sysroot from; installing `miri` does not pull it in. + components: miri, rust-src + # waproto is in the appstate/noise graphs, and its build script needs protoc. + - name: Install protoc + uses: taiki-e/install-action@v2 + with: + tool: protoc@${{ env.PROTOC_VERSION }} + # No sccache here: cargo-miri drives the build through its own + # RUSTC_WRAPPER and the two cannot share that slot. + - name: Cache Rust build (registry + target + Miri sysroot) + uses: Swatinem/rust-cache@v2 + with: + cache-targets: "true" + # `cargo miri setup` builds the interpreted sysroot here; without it + # every run recompiles core/std from rust-src. + cache-directories: ~/.cache/miri + # Each leg interprets a different feature set into target/miri. + key: ${{ matrix.cache-key }} + - name: Build Miri sysroot + run: cargo miri setup + # Default flags: Stacked Borrows, isolation on. Deliberately no + # -Zmiri-strict-provenance — `bytes` rebuilds its tagged `Shared` pointer + # out of an integer, which strict provenance rejects on sight and which is + # not the class of bug this gate is looking for. + - name: Run tests under Miri + run: cargo miri test ${{ matrix.args }} diff --git a/AGENTS.md b/AGENTS.md index 04b23d1d3..300fc4b5a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,6 +20,8 @@ cargo clippy --workspace --all-targets -- -D warnings # what CI enforces Workspace clippy takes minutes — pushing and letting CI parallelize the matrix is usually faster. E2E tests (`cargo test -p e2e-tests`) need the mock server running; see `agent_docs/e2e_testing.md`. +Touching `unsafe` — the `Yokeable`/`StableDeref` impls in `wacore-binary`'s `node.rs`, the `set_len` in `zlib_pool.rs` — means CI's Miri gate (`.github/workflows/miri.yml`) is what proves it, since neither clippy nor a native test observes an aliasing violation or an uninit read. Locally: `rustup component add miri rust-src && cargo miri test -p wacore-binary --lib`. Interpretation is ~100× native, so a fixture that only makes sense at hundreds of KB (zlib window refill, buffer growth) belongs behind `#[cfg_attr(miri, ignore)]` with a small twin that keeps the `unsafe` covered. + ## Gotchas Things that look correct and are not: diff --git a/wacore/binary/src/zlib_pool.rs b/wacore/binary/src/zlib_pool.rs index 56483d112..e16966fe1 100644 --- a/wacore/binary/src/zlib_pool.rs +++ b/wacore/binary/src/zlib_pool.rs @@ -390,7 +390,24 @@ mod tests { .collect() } + // Every other test here is sized in hundreds of KB to MB — the only way to + // reach window refill, the growth projection and shrink-on-return — which + // puts a full deflate+inflate cycle hours out of reach of Miri's + // interpreter, so they are `#[cfg_attr(miri, ignore)]`. This one keeps the + // `set_len` in `inflate_into_spare` under Miri on a fixture it can finish. #[test] + fn pooled_roundtrip_small_input() { + let original = varied(1024); + let compressed = zlib(&original); + assert_eq!( + decompress_zlib_pooled(&compressed, 64 * 1024).unwrap(), + original + ); + assert_eq!(drain_reader(&compressed, original.len()), original); + } + + #[test] + #[cfg_attr(miri, ignore)] fn inflate_reader_roundtrip_across_chunks() { // >128 KB so the stream spans multiple 64 KB decompress windows, and read // it back in tiny odd steps to exercise refill + compaction. @@ -408,6 +425,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn inflate_reader_ensure_larger_than_chunk() { // A single record bigger than the 64 KB window must be fully buffered. let original: Vec = (0..150 * 1024).map(|i| (i % 256) as u8).collect(); @@ -418,6 +436,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn inflate_reader_keeps_one_window_for_smaller_records() { INFLATE_POOL.with(|p| p.borrow_mut().clear()); const RECORD: usize = 30 * 1024; @@ -440,6 +459,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn inflate_reader_enforces_max() { let original = vec![0u8; 1024 * 1024]; let compressed = zlib(&original); @@ -448,6 +468,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn pooled_high_ratio_stream_roundtrips() { // ~50x expansion: the 2x up-front guess undershoots badly, so this // exercises the ratio-projected growth path end to end. @@ -470,6 +491,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn pooled_oneshot_matches_streaming() { let original = varied(100_000); let compressed = zlib(&original); @@ -490,6 +512,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn inflate_reader_reuses_pool_state_correctly() { // Back-to-back readers each checkout the pooled Decompress and reset it, so // no state may carry over between streams. Verify several sizes in sequence. @@ -500,6 +523,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn inflate_reader_reuse_after_error() { // A reader aborted mid-stream (max exceeded) returns partial zlib state to // the pool; the next checkout must reset it and decompress a full stream. @@ -513,6 +537,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn drop_shrinks_oversized_buffer_before_pooling() { // Buffering a large record grows `buf` to many MB; on return to the pool it // must be shrunk back toward the bounded steady-state capacity, not parked From d4031e2fd6ac12eba9e66ad01accd1e238b2dcab Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:21:45 +0000 Subject: [PATCH 2/6] ci(miri): drop the appstate leg, blocked on a dependency's aliasing bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first run found real UB, in `inout` 0.2.2 rather than in this workspace: `PaddedInOutBuf::into_out` builds a shared slice from a raw pointer that invalidates the `&mut [u8]` still protected as `cbc::encrypt_padded`'s argument, reached from `RustCryptoProvider::aes_256_cbc_encrypt`. That is the AES-CBC path every appstate record takes, so the leg cannot be green until the dependency is, and per-test `cfg_attr(miri, ignore)` would delete the coverage while appearing to keep it. The blocker is named in the workflow so the criterion for restoring the leg is written down. wacore-noise stays: it passed, AES-GCM not going through the padded path. Also stops the checkout persisting its token, as signal-durability-nightly.yml already does — this job never pushes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V1mXhXv7K1bxPnCL3cYF2b --- .github/workflows/miri.yml | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/miri.yml b/.github/workflows/miri.yml index 7c43c851a..8bb5846dd 100644 --- a/.github/workflows/miri.yml +++ b/.github/workflows/miri.yml @@ -49,17 +49,23 @@ jobs: - name: wacore-binary (no simd) cache-key: binary-scalar args: -p wacore-binary --no-default-features --lib - # No `unsafe` of our own, but both drive wacore-binary's zero-copy - # decode over real protocol payloads and pull the crypto stack + # No `unsafe` of its own, but it drives wacore-binary's zero-copy + # decode over real Noise frames and pulls the crypto stack # (aes/sha2/curve25519), whose unsafe backends this exercises. - - name: wacore-appstate - cache-key: appstate - args: -p wacore-appstate --lib + # + # wacore-appstate is deliberately absent: `inout` 0.2.2's + # `PaddedInOutBuf::into_out` invalidates the `&mut [u8]` that `cbc`'s + # `encrypt_padded` still holds protected, which Miri rejects under + # Stacked Borrows. That is on the AES-CBC path every appstate record + # takes, so the leg cannot be green until the dependency is fixed — + # nothing in this workspace can make it so. - name: wacore-noise cache-key: noise args: -p wacore-noise --lib steps: - uses: actions/checkout@v6 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@master with: toolchain: nightly-2026-06-16 From d0615b5c30c3400d160dc6d2442d923c4f7bb7b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:28:15 +0000 Subject: [PATCH 3/6] test(binary): cover OwnedNodeRef outside the serde gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `OwnedNodeRef::new` was constructed in exactly one place: a test inside `#[cfg(feature = "serde")]`. Neither Miri leg enables that feature, so the `Yokeable` transmutes and the `StableDeref` impl — two of the three unsafe sites the gate exists for — were never interpreted. The gate would have reported green over an aliasing regression it was supposed to catch. Both tests are worth having on their own: nothing previously moved an `OwnedNodeRef` after construction, which is the entire `StableDeref` claim, and `slice_bytes`' pointer arithmetic had no coverage at all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V1mXhXv7K1bxPnCL3cYF2b --- wacore/binary/src/node.rs | 51 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/wacore/binary/src/node.rs b/wacore/binary/src/node.rs index 803184015..d5ef115ec 100644 --- a/wacore/binary/src/node.rs +++ b/wacore/binary/src/node.rs @@ -1008,6 +1008,57 @@ impl OwnedNodeRef { } } +#[cfg(test)] +mod owned_node_ref_tests { + use super::*; + + /// Raw binary-protocol bytes, as `OwnedNodeRef::new` wants them: `marshal` + /// writes a leading format byte that `unmarshal_ref` does not expect. + fn encoded(node: &Node) -> Bytes { + let bytes = crate::marshal::marshal(node).unwrap(); + Bytes::from(bytes[1..].to_vec()) + } + + fn sample() -> Node { + Node::new( + "iq", + Attrs(vec![(Cow::Borrowed("id"), NodeValue::String("abc".into()))].into()), + Some(NodeContent::Bytes(b"payload".to_vec())), + ) + } + + #[test] + fn borrowed_payloads_survive_moving_the_cart() { + let node = sample(); + let owned = OwnedNodeRef::new(encoded(&node)).unwrap(); + + // Move the value twice — through a Box and into a Vec — before reading + // anything back. That is the whole `StableDeref` claim: the yoked + // `NodeRef` keeps pointing at live bytes even though the wrapper it + // borrows from has moved. Nothing but an interpreter notices when it + // stops being true, which is why this test exists separately from the + // serde one it used to be a side effect of. + let mut moved = vec![*Box::new(owned)]; + let owned = moved.pop().unwrap(); + + assert_eq!(owned.tag(), "iq"); + assert!(owned.get_attr("id").unwrap() == "abc"); + assert_eq!(owned.content_bytes(), Some(&b"payload"[..])); + assert_eq!(owned.to_owned_node(), node); + } + + #[test] + fn slice_bytes_views_the_backing_buffer_without_copying() { + let owned = OwnedNodeRef::new(encoded(&sample())).unwrap(); + let content = owned.content_bytes().unwrap(); + + let view = owned.slice_bytes(content); + + assert_eq!(view.as_ref(), b"payload"); + assert_eq!(view.as_ptr(), content.as_ptr(), "slice_bytes copied"); + } +} + #[cfg(feature = "serde")] impl serde::Serialize for OwnedNodeRef { fn serialize(&self, serializer: S) -> Result { From 4e4fffeafba9fd497685a62acd2218198b605349 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:30:13 +0000 Subject: [PATCH 4/6] test(binary): keep the token mutation sweep off the interpreter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lookup_matches_reference_under_byte_mutation` walks every byte position of every token against all 256 values — ~3M probes, a second natively and minutes under Miri, which is what left the wacore-binary leg still running after twelve. `token.rs` holds no `unsafe` and no raw pointers, so the interpreter has nothing to find there. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V1mXhXv7K1bxPnCL3cYF2b --- wacore/binary/src/token.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/wacore/binary/src/token.rs b/wacore/binary/src/token.rs index 84e873404..e7b414ee4 100644 --- a/wacore/binary/src/token.rs +++ b/wacore/binary/src/token.rs @@ -205,7 +205,13 @@ mod tests { /// `None`). Probes the length-bucketed lookup for any discriminator collision /// that would silently map a non-token (a JID/id) onto a token and corrupt the /// wire, and stays durable against future token-set edits. + /// + /// Ignored under Miri: ~3M lookups (every byte position of every token, + /// times 256) is minutes of interpretation for a table probe that holds no + /// `unsafe` and no raw pointers, so there is nothing there for the + /// interpreter to find. #[test] + #[cfg_attr(miri, ignore)] fn lookup_matches_reference_under_byte_mutation() { use std::collections::HashMap; From 8f2e229b736d0ddcd4a5d0e19913e869f3250fe1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:45:43 +0000 Subject: [PATCH 5/6] test(binary): build the Miri zlib fixture without a compressor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit zlib-rs 0.6.6's deflate state frees its buffers from `deflate::end` while a `&mut` into them is still protected, which Miri rejects — so the one zlib test left running under the interpreter died in `flate2`'s Drop, inside the helper that built its fixture rather than in anything this crate runs. Compression is not a path wacore-binary has; inflate is. The fixture is now a hand-built stored deflate block plus its adler32, which keeps `inflate_into_spare`'s `set_len` covered and drops the compressor from the interpreted set entirely. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V1mXhXv7K1bxPnCL3cYF2b --- wacore/binary/src/zlib_pool.rs | 37 ++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/wacore/binary/src/zlib_pool.rs b/wacore/binary/src/zlib_pool.rs index e16966fe1..81e6e5565 100644 --- a/wacore/binary/src/zlib_pool.rs +++ b/wacore/binary/src/zlib_pool.rs @@ -390,15 +390,44 @@ mod tests { .collect() } + /// A zlib stream carrying `data` verbatim in one stored (uncompressed) + /// deflate block, hand-built so the fixture costs no compressor. + /// + /// `zlib()` above cannot be used under Miri: zlib-rs 0.6.6's *deflate* state + /// frees its buffers from `deflate::end` while a `&mut` into them is still + /// protected, which Miri rejects. That is the compression half, which this + /// crate never runs — inflate is the whole production path — so the fixture + /// side steps around it rather than the test being dropped. + fn stored_zlib(data: &[u8]) -> Vec { + assert!(data.len() <= u16::MAX as usize, "one stored block only"); + // 0x78 0x01: deflate, 32 KB window, and (0x78 << 8 | 0x01) % 31 == 0 as + // the header check requires. + let mut out = vec![0x78, 0x01]; + let len = data.len() as u16; + // BFINAL=1, BTYPE=00 (stored), then the byte-aligned LEN/!LEN pair. + out.push(0x01); + out.extend_from_slice(&len.to_le_bytes()); + out.extend_from_slice(&(!len).to_le_bytes()); + out.extend_from_slice(data); + + let (mut a, mut b) = (1u32, 0u32); + for &byte in data { + a = (a + byte as u32) % 65521; + b = (b + a) % 65521; + } + out.extend_from_slice(&(((b << 16) | a).to_be_bytes())); + out + } + // Every other test here is sized in hundreds of KB to MB — the only way to // reach window refill, the growth projection and shrink-on-return — which - // puts a full deflate+inflate cycle hours out of reach of Miri's - // interpreter, so they are `#[cfg_attr(miri, ignore)]`. This one keeps the - // `set_len` in `inflate_into_spare` under Miri on a fixture it can finish. + // puts a full inflate cycle hours out of reach of Miri's interpreter, so + // they are `#[cfg_attr(miri, ignore)]`. This one keeps the `set_len` in + // `inflate_into_spare` under Miri on a fixture it can finish. #[test] fn pooled_roundtrip_small_input() { let original = varied(1024); - let compressed = zlib(&original); + let compressed = stored_zlib(&original); assert_eq!( decompress_zlib_pooled(&compressed, 64 * 1024).unwrap(), original From 89757cdfb9b14c677a944fad608af529ca087399 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:58:05 +0000 Subject: [PATCH 6/6] test(binary): interpret the hand-written Yokeable, not only yoke's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Yoke::with_mut` is called nowhere in the workspace, so `transform_mut` was uninterpreted — and yoking a `NodeRef` would not have reached the `AttrsRef` impl anyway: the derive transmutes the whole struct in one go and never calls a field's methods. The hand-written impl is there to satisfy that bound, which leaves its three transmutes reachable only through a yoke of `AttrsRef` itself. That yoke is what this test builds, borrowing the attribute key and value straight out of the cart so `make`, `transform` and `transform_mut` all carry a real lifetime across. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01V1mXhXv7K1bxPnCL3cYF2b --- wacore/binary/src/node.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/wacore/binary/src/node.rs b/wacore/binary/src/node.rs index d5ef115ec..5ef1f0498 100644 --- a/wacore/binary/src/node.rs +++ b/wacore/binary/src/node.rs @@ -1047,6 +1047,41 @@ mod owned_node_ref_tests { assert_eq!(owned.to_owned_node(), node); } + #[test] + fn yoked_attrs_ref_survives_make_transform_and_mutation() { + // `NodeRef`'s derived `Yokeable` transmutes the whole struct in one go, + // so yoking a node never calls `AttrsRef`'s hand-written impl — that one + // is there to satisfy the derive's bound on the field. Reaching its + // three methods takes a yoke of `AttrsRef` itself, and it is the only + // way to put our own transmutes, rather than yoke's generated ones, in + // front of the interpreter. + let cart = BytesCart(Bytes::from_static(b"idabc")); + let mut yoke: Yoke, BytesCart> = Yoke::attach_to_cart(cart, |buf| { + // Borrowed from the cart, which is what makes the transmutes load-bearing. + let (key, value) = buf.split_at(2); + AttrsRef::from_vec(vec![( + NodeStr::Borrowed(std::str::from_utf8(key).expect("ascii")), + ValueRef::String(NodeStr::Borrowed( + std::str::from_utf8(value).expect("ascii"), + )), + )]) + }); + + // `make` ran on attach; `transform` runs here. + let (key, value) = &yoke.get().as_slice()[0]; + assert!(*key == "id"); + assert!(*value == "abc"); + + // `transform_mut`, which nothing in the workspace calls. + yoke.with_mut(|attrs| { + *attrs = AttrsRef::from_vec(vec![( + NodeStr::Owned("k".into()), + ValueRef::String(NodeStr::Owned("v".into())), + )]); + }); + assert!(yoke.get().as_slice()[0].1 == "v"); + } + #[test] fn slice_bytes_views_the_backing_buffer_without_copying() { let owned = OwnedNodeRef::new(encoded(&sample())).unwrap();