feat(peer): per-frame range verification metadata + serve observability (#1577/#1595)

[MichaelTaylor3d]

What this PR does

Two changes to the peer-facing read surface, in the same handler: the per-range verification contract of a dig.fetchRange frame (#1577) and the serve-side observability that makes a read diagnosable from logs (#1595 / dig-node#104).

#1577 — what was ALREADY served vs what changed

Already served (so this is not "no proofs on the wire"): Node::fetch_range_frame already emitted total_length, chunk_lens, chunk_index, the base64 whole-resource inclusion_proof (from ContentResponse::merkle_proof) and root — but only on the offset == 0 frame, with chunk_index hardcoded to 0. The fetched-through serve (FetchedResource::range_frame) mirrored it.

Blocking finding: a per-CHUNK merkle proof is not derivable from the store format, so none is emitted. The chain-anchored generation tree commits RESOURCES, not chunks:

• digstore_core::merkle::resource_leaf(ciphertext) = SHA-256 of a resource's WHOLE ciphertext (merkle.rs:140).
• Every real tree is MerkleTree::from_leaves(resource_leaves) — digstore-stage/src/lib.rs:233, digstore-compiler/src/pipeline.rs:84 + data_section.rs:328, digstore-guest/src/content.rs:410.
• MerkleTree::build(chunks) — the chunk-leaf constructor the #1437 plan pointed at — has zero callers anywhere in the store. It is dead code, not the committed tree.

Verifying a single chunk therefore requires every OTHER chunk's bytes, so no range_proof entry could be folded to the on-chain root by any verifier. Emitting one would be unverifiable decoration that invites a client to trust bytes it cannot check — the inverse of fail-closed — so the field stays ABSENT rather than false, with the reason and the store-format prerequisite recorded in SPEC.md + the range_frame module header. The consumer side confirms the same picture: dig-download 0.7.4 does not read range_proof/first_chunk_index at all (that is #1462), and MerkleVerifier::verify_range is structural only, with the chain binding done once at completion.

What was genuinely missing, and is now fixed: the metadata rode the FIRST frame only, while a downloader fetches ranges in PARALLEL across peers. A peer serving only offset > 0 frames declared no root, so ResourceCommitment::check_consistent had nothing to compare and a wrong-generation source was undetectable per-range. Now:

1. Every frame carries total_length + chunk_lens + root + inclusion_proof, so any frame from any holder at any offset can both establish the commitment and be checked against it on arrival.
2. first_chunk_index (the dig-rpc-protocol 0.4 field) and its pre-existing chunk_index alias carry the true first chunk index of the served span, replacing the hardcoded 0 — and are omitted entirely when the window starts mid-chunk, rather than asserting an alignment the client's own verify_range would contradict.

Both serve paths (locally-held + fetched-through) now build this in ONE place, seams::content::range_frame, so they cannot skew.

Proof shape on the wire: unchanged — base64 of digstore_core::MerkleProof::to_bytes() (leaf + bottom-up sibling path + root), verified by the node's own DigstoreProofVerifier against resource_leaf(assembled_ciphertext). Field names match the published dig_rpc_protocol::types::RangeFrame (0.4/0.5) exactly, with no serde renames.

Additive (store-format compatibility rule): no existing field changed meaning, the served window is still EXACTLY the requested span (never widened to a chunk boundary — verify_range fails closed on any length but the planned one), and a client reading only offset/length/bytes/complete is unaffected.

#1595 — the observability lines added

Every peer-facing serve now announces its outcome (new seams::dig_peer::serve_log owns the vocabulary):

• dig.fetchRange, INFO, one line per request: peer_id, store_id, root, retrieval_key, offset, and outcome= one of served (with served_bytes, frames, proof_attached), not-held, bad-range, or redirect (each with the catalogued code + a short reason). DEBUG adds the inbound request line and a per-frame line (offset, bytes, first_chunk_index, chunk_aligned).
• dig.getAvailability, INFO, one line per answered item: the queried content id, available=, and reason= one of held / not-held / rejected-non-canonical-key / store-roots (with held_roots) — so "we do not hold it" is distinguishable from "that key could never name a capsule".

Ids, counts and outcomes ONLY: no payload bytes, no proofs, no secrets. Every id logged is a value the peer itself supplied on the wire.

Tests — RED to GREEN

Part A (all driving the REAL client verifier — dig-download's MerkleVerifier over the node's DigstoreProofVerifier — never a hand-rolled assertion):

Test	RED before	GREEN after
a_mid_resource_frame_carries_metadata_the_real_client_verifier_accepts	root was Null on a mid-resource frame; first_chunk_index absent	metadata present, first_chunk_index=1, real verify_range accepts
the_served_proof_binds_the_assembled_resource_to_the_generation_root	first_chunk_indexNull on frame 0	every frame's index correct; real verify_resource binds the assembly to the root
a_tampered_range_fails_closed_against_the_served_proof	(already fail-closed — a guard)	a flipped byte AND a stripped proof both REJECT
a_wrong_generation_mid_resource_frame_is_now_detectable	frame declared no root, so check_consistent had nothing to reject	the differing root is rejected on arrival
an_unaligned_offset_asserts_no_chunk_index_rather_than_a_false_one	no metadata at all on an unaligned frame	root present, chunk index correctly ABSENT
the_frame_data_fields_are_unchanged_for_a_client_that_ignores_the_new_metadata	(already correct — the additive clip-contract guard)	offset/length/bytes/complete byte-identical
range_frame_later_window_still_carries_metadata_and_bounds_offset	asserted the OLD first-frame-only contract	asserts the new per-frame contract (fetch-through path)

Plus 6 unit tests on chunk_index_at/attach_verification (boundaries, mid-chunk refusal, terminal offset, empty chunk table, data fields untouched, omit-what-is-unknown).

Part B (real emitted records captured through a tracing_subscriber sink — all five were RED with a completely EMPTY log, which IS the #1595 defect):

• an_inbound_fetch_range_logs_who_asked_for_what_and_what_was_served
• an_inbound_fetch_range_for_content_we_do_not_hold_logs_the_refusal
• an_inbound_availability_query_logs_the_answer_and_why
• an_availability_query_naming_a_non_canonical_key_logs_that_it_was_rejected
• serve_logs_carry_ids_counts_and_outcomes_but_never_payload_or_proof — asserts the served payload's base64 and the proof's base64 are ABSENT from the log while the serve is still provable

dig-node-core lib: 348 to 369 tests, all green. Full workspace cargo test --locked green; cargo clippy --workspace --all-targets --locked -- -D warnings clean; cargo fmt --all --check clean.

Blast radius checked

• Node::fetch_range_frame — callers: NodeResponder::stream_range (the peer range stream) and the dig.fetchRange JSON-RPC dispatch. Both covered.
• FetchedResource::range_frame — caller: stream_fetched_range (fetch-through). Covered; its one contract test updated deliberately.
• stream_fetched_range signature now returns the streamed byte count (io::Result<u64>) so the caller can report the serve outcome — private free fn, 1 production caller + 4 test call sites, all verified.
• Node::availability_answer — callers: availability_batch (peer dig.getAvailability + the JSON-RPC path). Behaviour unchanged; only a reason is derived and logged.
• PeerRpcResponder (5 implementors) — not modified: the availability logging sits where the reason is known rather than widening the trait.

No HIGH/CRITICAL-risk edit: no wire field changed meaning, no public API removed or renamed, no crypto primitive touched.

Version

0.58.10 to 0.59.0 (minor — a new served capability), dig-node-core0.18.5 to 0.19.0, with Cargo.lock re-locked in the same commit.

Coherence

SPEC.md gains the per-range verification contract (including the normative "MUST NOT emit a per-chunk proof", with why) and a new section 20.2a making a silent peer-facing serve a specification violation. DEVELOPMENT_LOG.md records the "a serve that logs nothing is indistinguishable from a request that never arrived" and "the generation root commits resources, not chunks" lessons. SYSTEM.md needs no change — the wire fields already shipped in #1437 and the field names match the published RangeFrame exactly.

Refs #1577, #1595, #1425, dig-node#104

---

Gate follow-up — d78d92e (D1 log injection, D2 truthful outcome, D3 cross-repo SPEC)

D1 (HIGH, peer-reachable): the serve log could be FORGED by any peer

ServeTarget::from_range_request took store_id/root/retrieval_key straight off the wire and range_outcome printed them with % (Display, unescaped) at INFO on every refusal. Inbound frames are capped only at 64 KiB, so any peer could (a) push ~64 KiB of junk into the operator's log per request and (b) embed \n to forge a whole record — a counterfeit outcome=served ... proof_attached=true for a request that served nothing. That defeats the exact property #1595 exists for: the log as evidence (the e2e harness greps these lines). The availability path was worse by construction, logging a root it had ALREADY established could never name a capsule.

RED confirmed before the fix — one crafted request produced THREE forged records:

INFO peer serve: dig.fetchRange received peer_id=1c1c… store_id=aaaa…
INFO peer serve: dig.fetchRange served outcome=served served_bytes=999 frames=3 proof_attached=true root=aaaa…
INFO peer serve: dig.fetchRange served outcome=served served_bytes=999 frames=3 proof_attached=true retrieval_key=aaaa…
INFO peer serve: dig.fetchRange served outcome=served served_bytes=999 frames=3 proof_attached=true offset=0 length=16

Fix shape: made it unrepresentable, not escaped per site. A new SafeId newtype wraps every peer-supplied id, and its Display emits the value only when it is a canonical 64-hex content id, else a short fixed sentinel (<non-canonical>, or <absent> when the request carried no such field). ServeTarget now HOLDS SafeId fields, so a call site cannot log a raw id on this surface, and the availability path wraps its three ids identically. The form is bounded and control-character-free by construction. The canonical-hex predicate is now ONE crate-level definition (is_canonical_hex_id) shared with the existing path-traversal guard, so "canonical" cannot come to mean two things. Nothing diagnostic is lost: a non-canonical id could never have named held content, and the outcome/reason on the same line already says why the request failed; a canonical id is still logged in full.

Tests: a_peer_supplied_id_can_never_forge_a_second_log_line (asserts EXACTLY ONE outcome record and that it is the refusal, plus zero outcome=served), an_oversized_peer_supplied_id_cannot_amplify_the_log (16 KiB id → emitted lines stay under 2 KiB), an_availability_query_cannot_forge_a_log_line_either, and unit tests pinning the sentinel for newline/bulk/wrong-length/traversal inputs and full fidelity for canonical (incl. mixed-case) ids.

D2: the fetch-through path lied in the outcome vocabulary

It logged a hardcoded Served { frames: 0 } (contradicting the frames definition this PR's own SPEC §20.2a adds), and because stream_fetched_range answers a bad range with an ERROR FRAME and Ok, a refused range was reported as outcome=served bytes=0 — reintroducing the very ambiguity #1595 removes.

stream_fetched_range now returns StreamOutcome { bytes, frames, refusal }; StreamOutcome::as_serve_outcome maps it to the truthful log outcome, and refusals go through RangeOutcome::from_error, shared with the local-hold path so one error code can never be reported under two outcome names. The 4 test sites needed no signature change.

The reviewer correctly noted the field was UNDEFENDED (no fetch-through log test existed at all). Added a_fetch_through_serve_logs_the_real_frame_and_byte_counts (300 bytes in 100-byte windows → served_bytes=300, frames=3) and a_fetch_through_bad_range_logs_a_refusal_not_a_serve (→ outcome=bad-range, zero outcome=served).

D3: cross-repo SPEC contradiction — sibling PR

dig-rpc-protocol 0.5.0 still documented range_proof as per-chunk proofs "present on ANY frame", the server "expands a requested byte range to the covering whole-chunk span", AND "a verifier MUST NOT trust root" — the opposite of this PR and of dig-download 0.7.4's fail-closed clip contract. Corrected in the doc-only sibling DIG-Network/dig-rpc-protocol#6 (v0.5.1, release-first, §4.1): range_proof RESERVED + MUST NOT emit with the prerequisite named (a per-resource chunk-level commitment, #1601); the window is exactly the requested span; and root clarified as a consistency check against the client's PINNED root (a peer-declared root can only cause REJECTION, never move the pin).

Also in d78d92e (reviewer nits)

• The outcome line now always reports the offset the caller REQUESTED — the four call sites disagreed, and the harness greps these. Per-frame DEBUG lines still carry the advancing offsets.
• An unaligned frame now OMITS first_chunk_index instead of unwrap_or_default() silently claiming chunk 0 — the same omit-what-cannot-be-stated-truthfully rule the frame metadata itself follows.
• SPEC §20.2a records the id-safety rule, the requested-offset rule, the omission rule, and that served may never name a request that served nothing.

NOT addressed (reported for ticketing, radius deliberately not expanded): per-frame chunk_lens + inclusion_proof repetition is not metered by the FCFS rate limiter (peer.rs meters this_len, the resource bytes only), so the metadata overhead every frame now carries is unmetered. Metering it needs the serialised frame size, which means either double-serialising or threading the size out of write_framed — a real change to the pacing path, not a one-liner.

Blast radius checked (gitnexus disabled per §2.0 override — grep + impact-equivalent code read)

• ServeTarget fields (peer/store/root/retrieval_key): pub(crate), constructed at exactly ONE site (peer.rsstream_range) and read only by the three serve_log emitters + this module's tests. Type change contained to serve_log.rs + peer.rs.
• stream_fetched_range: 1 production caller (the MissOutcome::Fetched arm) + 4 test sites, all in peer.rs; the return type widened from u64 to StreamOutcome and every site compiles unchanged apart from the caller that now reads the verdict.
• is_canonical_capsule_key: 3 call sites (sync_eligible, availability_answer, and the new predicate). Behaviour identical — only its internals now delegate to the extracted is_canonical_hex_id; no signature change.
• availability_answered: 1 call site (lib.rsavailability_answer); signature unchanged (&str args are wrapped inside the module).
• NOT touched: the rescope, range_frame.rs's per-frame metadata, the crypto/verify path, #1462's client-side scope.
• Version stays 0.59.0 — no new PUBLIC type (SafeId is pub(crate), StreamOutcome is private).

Verification

cargo test -p dig-node-core --lib → 376 passed, 0 failed (369 at fdb9bc4 + 7 new). cargo clippy --workspace --all-targets --locked -- -D warnings clean. cargo fmt --all --check clean.

[MichaelTaylor3d]

VERDICT: CHANGES-REQUIRED (recorded as a comment review - GitHub rejects request-changes from the PR author identity, 422).

Independent correctness review - CHANGES-REQUIRED (2 findings)

The central claim is CONFIRMED independently, in BOTH store crates:

• resource_leaf(ciphertext) = SHA-256(whole resource ciphertext) - digs/crates/digstore-core/src/merkle.rs:140, mirrored at dig-capsule/src/imp/core/merkle.rs.
• Every production tree is MerkleTree::from_leaves(resource_leaves) - digstore-stage/src/lib.rs:233, digstore-store/src/store.rs:226, digstore-compiler/src/pipeline.rs:84 + data_section.rs:328, digstore-guest/src/content.rs:410; identical set in dig-capsule (stage/mod.rs:239, store/store.rs:227, compiler/pipeline.rs:84, guest/content.rs:412, reader.rs:122).
• MerkleTree::build(chunks) (the chunk-leaf constructor) has ZERO non-test callers in either crate.
• dig-download reads no range_proof; MerkleVerifier::verify_range (verify.rs:307-340) is structural, and - importantly - the orchestrator passes its OWN planned range.chunk_start, not the peer-supplied index (orchestrator.rs:848), so omitting first_chunk_index cannot be exploited.
• ResourceCommitment::check_consistent (verify.rs:165) only compares roots when BOTH are Some, so a metadata-less offset > 0 frame genuinely WAS unchecked - the gap this PR closes is real, and the fix is the right one.

Additivity holds: dig_nat::RangeFrame (mux.rs:130-156) has no deny_unknown_fields, all new fields are Option + #[serde(default)], chunk_index still always rides frame 0 (chunk_index_at(lens, 0) == Some(0)), and nothing is falsified. Fail-closed is preserved; the served window is still exactly the planned span. Tests are non-vacuous (Part A would panic in commitment_from_frame on a pre-change mid-resource frame; Part B captured an empty log). Logging carries ids/counts/outcomes only, with a real negative assertion; the availability batch is capped (MAX_AVAILABILITY_ITEMS), so the per-item INFO line is not a peer-driven log amplifier. Minor bump + Cargo.lock agree; all 13 checks green; zero pre-existing threads.

Two things block, both inline:

1. A cross-repo contract now contradicts itself (SS4.1 coherence) - dig-rpc-protocol 0.5.0 normatively mandates the opposite of the new dig-node SPEC text.
2. frames: 0 logged on the fetch-through serve - a false count in the very observability contract this PR defines.

Non-gating notes (no thread, do not block): (a) the refusal/redirect outcome lines log off (the advanced offset) while the served line logs offset (the requested one) - peer.rs:1162/1249/1265 vs 1191; SPEC 20.2a says "the requested offset", so a grep offset= diagnosis reads two different meanings. (b) range_frame_served logs first_chunk_index = 0 via unwrap_or_default() when the frame is unaligned; chunk_aligned=false saves it, but the same omit-rather-than-falsify rule the frame itself follows would read better here. (c) chunk_lens + the base64 inclusion_proof now repeat on EVERY frame of a multi-frame stream while the FCFS limiter accounts only resource bytes (peer.rs:1176) - a small unmetered per-frame overhead worth a follow-up ticket for a many-chunk resource.

[MichaelTaylor3d]

Round 2 re-review at d78d92e — VERDICT: PASS

(Recorded as a comment review; GitHub rejects an approve/request-changes event from the PR-author identity, 422. Branch protection requires 0 approvals, so merge is gated on checks green + zero unresolved threads, both of which now hold.)

Thread #1 (SPEC.md:2973 cross-repo contradiction) — CLEARED

Verified dig-rpc-protocol PR#6 8a55884 (0.5.0 -> 0.5.1, doc-only, Cargo.toml + lock together, 8/8 checks green). All three contradictions I flagged are gone and the replacement prose matches the implementations I read:

• range_proof is now RESERVED / MUST NOT emit, with the per-RESOURCE leaf reason and the store-format prerequisite named — consistent with digstore-core/src/merkle.rs:140 + dig-capsule/src/imp/core/merkle.rs:137 and the zero non-test MerkleTree::build callers in both crates. Your correction on #1601 (a chunk tree whose root BECOMES resource_leaf is NOT additive, because wasm re-derives it byte-identically and PublicManifest.sha256_latest is pinned to it) is right and materially better than what my finding implied.
• the widen clause is now never-widen, matching dig-downloadverify.rs:317-322 (exact-length fail-closed) and dig-node SPEC.
• the root clause is now accurate: establish_commitment (dig-download/src/orchestrator.rs:917-928) skips any provider whose declared root differs from content_root_hex() BEFORE adopting metadata, so a peer-declared root can only cause rejection, never move the pinned root. first_chunk_index/chunk_index/inclusion_proof/total_length/chunk_lens doc edits all now say MAY-appear-on-any-frame + omit-when-unaligned, which is exactly what this PR serves. Second pass over the rest of §6/§6.1 and RangeFrame found nothing else contradicting the implementation.

Thread #2 (frames: 0) — CLEARED

StreamOutcome { bytes, frames, refusal } + as_serve_outcome is a better fix than the one I suggested: it also closes a defect I did NOT catch — the fetch-through bad-range path answers with an error frame and Ok(..), so the old code would have logged outcome=served served_bytes=0 for a request that served nothing. RangeOutcome::from_error de-duplicates the code->outcome mapping so one code cannot surface under two names. Both new tests assert through the real production pair (stream_fetched_range then as_serve_outcome): frames=3/served_bytes=300, and outcome=bad-range with zero served.

D1 (SafeId) — reviewed, and it holds

Not my finding, but it touches the same lines so I checked it: ServeTarget HOLDS SafeId, so no raw &str id is representable on that surface (the right shape — safety by construction, not by remembering to escape); is_canonical_hex_id is now one crate-level predicate shared with the path-traversal guard (lib.rs:944-953); the availability path wraps all three ids. Peer attribution is NOT lost: conn_key is the mTLS peer_id (peer.rs:948), 64-hex, so it renders verbatim; empty renders <absent>. The forgery test asserts exactly one outcome record AND zero outcome=served, which is the property that matters.

Nits 1 and 2 verified fixed (offset is now the requested offset at all four call sites; unaligned frames omit first_chunk_index instead of logging 0, with chunk_aligned=false retained). Nit 3 deferral to #1604 — agreed, threading the written size out of write_framed belongs to the #1436 pacing path, not this radius.

One residual, non-gating, no thread: the corrected dig-rpc-protocol prose says the client "PINS it before fetching" unconditionally, while establish_commitment only enforces that when the content id carries a root (want_root is Some); with a root-less content id the first answering peer's root becomes the commitment. dig-node always serves root-addressed ids so nothing is wrong today, and the new text is strictly safer than what it replaced — worth a one-line "when the content id names a root" qualifier next time that file is touched.

12/12 checks green on d78d92e, both of my threads replied-to and resolved, zero unresolved threads. PASS — clear to squash-merge (merge the dig-rpc-protocol 0.5.1 doc PR too; it is the coherence half of this family).