diff --git a/Cargo.lock b/Cargo.lock index 61857d31..b827726e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.252.3" +version = "0.252.4" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index 1bf0afdc..d8a20e84 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.252.3" +version = "0.252.4" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over diff --git a/crates/dig-node-core/src/download.rs b/crates/dig-node-core/src/download.rs index ee805482..0768fc3e 100644 --- a/crates/dig-node-core/src/download.rs +++ b/crates/dig-node-core/src/download.rs @@ -2099,6 +2099,17 @@ impl NodeContent { &self.ask_routing } + /// This node's first-hand holder cache, so a test can ask what a completed walk LEFT BEHIND. + /// + /// `FirstHandHolderCache::remember` states that "the caller is responsible for passing first-hand + /// records only", which makes the first-hand property a discipline of THIS module rather than an + /// invariant the type can enforce. A discipline nothing observes is a discipline one line can + /// break, so the cache has to be readable from a test for that line to be catchable. + #[cfg(test)] + pub(crate) fn holder_cache(&self) -> &FirstHandHolderCache { + &self.holder_cache + } + /// [`Self::forget_stale_discovery`], reachable from the tests that drive the caches directly. #[cfg(test)] pub(crate) async fn forget_stale_discovery_for_test(&self, content: &ContentId) { diff --git a/crates/dig-node-core/src/forwarded_ask_tests.rs b/crates/dig-node-core/src/forwarded_ask_tests.rs index db49bc94..aaf3e7dc 100644 --- a/crates/dig-node-core/src/forwarded_ask_tests.rs +++ b/crates/dig-node-core/src/forwarded_ask_tests.rs @@ -312,6 +312,61 @@ async fn a_holder_only_a_peer_knows_about_reaches_the_answer() { ); } +/// **Proves (NC-12, `seams::dig_peer::holder_cache` module doc):** a peer's HEARSAY never enters the +/// first-hand holder cache, even though it is perfectly welcome in the answer. +/// +/// **Why this is the sharpest thing to pin about that cache.** `FirstHandHolderCache::remember` says +/// in its own doc that "the caller is responsible for passing first-hand records only", so the +/// property is a discipline of `locate_holders` rather than an invariant the type enforces. The +/// module doc names exactly what it buys: a cache that stored hearsay would let one lying hop plant a +/// fabricated holder that this node then re-serves as its OWN knowledge for the whole TTL -- "a far +/// better attack than lying once", because it converts a single lie into an hour of this node +/// repeating it to everyone who asks. +/// +/// **Fixture design -- two DISTINGUISHABLE records, one per leg, and the inequality is the test.** +/// The DHT names peer 1 and the forwarded leg names peer 9. Both are required: with an empty DHT the +/// cache would be empty because `remember` declines an empty slate, which is a DIFFERENT reason and +/// would let this test pass under an implementation that cached hearsay happily. With one shared +/// record there would be nothing to tell "cached mine" from "cached theirs" apart. So the answer must +/// contain BOTH and the cache must contain ONLY the first-hand one. +/// +/// **On the revert:** add `self.holder_cache.remember(content, &forwarded.records);` after the +/// forwarded leg in `NodeContent::locate_holders` -- one line -- and the cache assertion fires. +#[tokio::test] +async fn a_peers_hearsay_reaches_the_answer_but_never_the_first_hand_cache() { + let cid = content(); + let ask = RecordingAsk::answering(vec![provider(9, &cid)]); + let (pc, _dir) = engine(vec![provider(1, &cid)], &[2], Some(ask.clone())); + + let found = pc + .locate_holder_candidates( + &cid, + HopBudget::fresh(), + &RequestorId::Peer("caller".into()), + ) + .await; + + // The control: BOTH legs genuinely contributed, so the cache assertion below is comparing two + // populated sources rather than observing one empty one. + assert_eq!( + peer_ids(&found), + vec![mock_peer_hex(1), mock_peer_hex(9)], + "fixture precondition: the answer must carry this node's own finding AND the peer's \ + hearsay, first-hand ahead of hearsay" + ); + + let cached = pc + .holder_cache() + .get(&cid) + .expect("a completed walk that found a holder is remembered"); + assert_eq!( + peer_ids(&cached), + vec![mock_peer_hex(1)], + "only THIS node's own lookup may be remembered; caching the peer's hearsay would let one \ + lying hop plant a holder this node re-serves as its own for the whole TTL" + ); +} + /// **Proves:** with no forwarded-ask leg installed — the FFI/base path — the answer is exactly this /// node's own DHT findings, unchanged. /// diff --git a/crates/dig-node-core/src/lib.rs b/crates/dig-node-core/src/lib.rs index ed06f863..d155ea0e 100644 --- a/crates/dig-node-core/src/lib.rs +++ b/crates/dig-node-core/src/lib.rs @@ -8089,6 +8089,88 @@ mod tests { ); } + /// **Proves (NC-12, `seams::dig_peer::module_relay` module doc):** a relay REFUSAL leaves the + /// pre-existing `RESOURCE_UNAVAILABLE` answer byte-identical to the one a requestor that never + /// asked for a relay receives. A refusal must not narrate WHICH gate refused. + /// + /// **Why that is a security property and not tidiness.** The gates are, in order, the + /// requestor's own `proxy` flag, the OPERATOR's opt-in (`DIG_NODE_ONION_RELAY`, default off), + /// and the requestor's proxy-class allowance. A frame that differs by gate is an oracle a + /// stranger reads for free: it says whether this operator enabled relaying, and it says when a + /// requestor has exhausted its allowance -- which is the rate-limiter's own state, told to the + /// party the limiter exists to bound. + /// + /// **Fixture design -- the two arms refuse at DIFFERENT gates.** Arm one omits `proxy`, so gate + /// one refuses: this request never wanted a relay. Arm two sets `proxy: true`, clears gate one, + /// and refuses at the content-engine gate instead. Two arms refusing at the SAME gate would be + /// two spellings of one input and would pass under any implementation, so the inequality below + /// is asserted first and pins the arms apart. + /// + /// **On the revert:** turn EITHER of the two refusals this fixture actually reaches -- + /// `module_relay.rs:114` (the requestor did not ask) or `:117` (this build has no content + /// engine) -- into a distinguishable status. `RelayStatus::Pending { staged_bytes: 0 }` is the + /// one-line version, and the frame equality fires because that arm becomes the inconclusive-miss + /// code carrying a progress field while the other stays `RESOURCE_UNAVAILABLE`. + /// + /// **What this fixture does NOT reach, said plainly so it is not read as full cover.** + /// `relay_capsule` has FIVE refusal sites. The operator opt-in (`:121`) and the proxy allowance + /// (`:125`) both sit behind a live `NodeContent`, which `test_node` does not wire, and the + /// no-warmer refusal (`:130`) sits behind both. Those first two are exactly the gates the + /// rationale above names as the reason the property matters. What holds them today is that all + /// five sites return one payload-free `RelayStatus::Refused` -- a property of the enum, not + /// something this test observes. Reaching them needs a fixture that installs a `NodeContent` + /// and drives `DIG_NODE_ONION_RELAY` under the `test_support` env mutex; that is the next guard + /// here, not a gap this one closes. + /// + /// **And the frame is not the only channel, so do not read this test as closing the oracle.** + /// `allow_proxy_fetch` (`:125`) CONSUMES a rate-limit token (`download.rs:1643`) from a bucket + /// shared with `miss_outcome`'s second leg (`download.rs:2721`). Draining it flips a later + /// `dig.getContent` from fetch-through to redirect, so a stranger can still infer the operator's + /// opt-in by spending the allowance and watching a DIFFERENT method change shape. Equal frames + /// are necessary for the property in the module doc and are not sufficient for the secret. + #[tokio::test] + async fn a_relay_refusal_is_indistinguishable_from_a_plain_miss() { + let (node, _td) = test_node(None); + // Nothing is cached, so BOTH arms are genuine misses and the relay leg is the only thing + // that could make them differ. + let (store, root) = (id_hex(0x31), id_hex(0x32)); + + let never_asked = json!({"store_id": store, "root": root}); + let asked = json!({"store_id": store, "root": root, "proxy": true}); + + // Side effect first: the two params genuinely differ in the field gate one reads, so the + // assertions below compare two different journeys rather than one input written twice. + assert_ne!( + crate::download::proxy_requested(&never_asked), + crate::download::proxy_requested(&asked), + "fixture precondition: one arm must ask for a relay and the other must not, or the \ + arms refuse at the same gate and prove nothing" + ); + + let answer = |params: Value| { + handle_rpc( + &node, + json!({"jsonrpc":"2.0","id":7,"method":"dig.getModuleInfo","params":params}), + crate::download::ReadOrigin::Peer, + crate::download::RequestProvenance::FirstParty, + ) + }; + let plain_miss = answer(never_asked).await; + let refused_relay = answer(asked).await; + + assert_eq!( + plain_miss["error"]["code"], + json!(download::RESOURCE_UNAVAILABLE), + "the control: a miss with no relay asked for is the settled not-held answer" + ); + assert_eq!( + refused_relay, plain_miss, + "a refused relay must be INDISTINGUISHABLE from a plain miss; any difference tells a \ + stranger which gate refused, and the gates encode the operator's opt-in and the \ + requestor's remaining allowance" + ); + } + /// **Proves (#2022):** `dig.listInventory` with `store_id` omitted — the whole-inventory /// enumeration ("a free map of everything this node holds") — is REFUSED over the permissionless /// peer surface (`ReadOrigin::Peer`) with -32601, yet still answered from the loopback/control