Skip to content

test(nc-12): measure the NC-12 doc claims in seams/dig_peer, guard 2 - #504

Merged
MichaelTaylor3d merged 6 commits into
mainfrom
loop/356-nc12-audit
Sep 2, 2026
Merged

test(nc-12): measure the NC-12 doc claims in seams/dig_peer, guard 2#504
MichaelTaylor3d merged 6 commits into
mainfrom
loop/356-nc12-audit

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

DO NOT MERGE — gate round in progress.

Closes #356

What changed

An audit of the NC-12 doc claims in crates/dig-node-core/src/seams/dig_peer/, plus guards for the
two that were ASSERTED-ONLY with a surviving one-line mutation. The deliverable is the
measurement
; the code change is two tests and one #[cfg(test)] accessor.

No production behaviour changes. download.rs gains only a test-gated holder_cache() reader.

The denominator is smaller than the ticket assumed

16 NC-12 mentions across 8 files (the ticket's 15 plus dht.rs:434). Three are inside
#[cfg(test)] blocks and are not doc claims — they ARE tests
(module_reshare.rs:1789,
module_transport.rs:1150, :1175). Of the 13 left, module_transport.rs:774 and :821 state one
property. 12 distinct properties.

Result: 3 TESTED, 2 TESTED-IN-PART, 3 true-but-enforced-elsewhere, 1 ASSERTED-ONLY guarded IN PART
(module_relay.rs:52 — 2 of 5 refusal sites; gates 2 :121 and 3 :125 unreached), 1 ASSERTED-ONLY
guarded at a seam the violation would not take (holder_cache.rs:30), 1 ASSERTED-ONLY reported with
its mutation (forwarded_ask.rs:381), 1 FALSE (forwarded_ask.rs:437, #508).
The full per-claim
table is on
#356, corrected for
rows 1-2 and
row 7.

One claim measured FALSE — filed, deliberately NOT fixed here

forwarded_ask.rs:437 says a hop's lie about absence_established "can only ever WEAKEN the claim
this node goes on to make, never strengthen it."
It does the opposite: a hop answering
absence_established: true when its own subtree search did not complete moves this node from
inconclusive to proven-absent, and Node::availability_answer re-emits that establishment to the
next hop.

Trace: forwarded_ask.rs:349-356 -> download.rs:2047-2064 (the Answered arm is the only one that
does not clear answers.conclusive) -> download.rs:1829 -> download.rs:973-975.

Filed as #508. Trusting that field at face value is a deliberate design choice, so correcting it
is a design decision, not a doc edit — and a security change grown mid-sweep makes the sweep
ungateable.

The sentence's second half is true: lying buys no reach, because ask_routing.rs:178-190 and
download.rs:2038-2044 both fold Answered([]) and AnsweredInconclusive([]) to the same value.
That equivalence is itself untested and is noted on #508.

The two guards, each with its revert-proof

Both fixtures use two distinguishable values and a pinned inequality, so a later edit cannot collapse
them into a single value that proves nothing.

1. a_relay_refusal_is_indistinguishable_from_a_plain_miss (lib.rs)

module_relay.rs carries zero tests, and no test anywhere calls relay_capsule. A refusal that
narrates WHICH gate refused is an oracle a stranger reads for free: it says whether this operator
enabled relaying, and it says when a requestor has exhausted its proxy allowance — the rate
limiter's own state, told to the party the limiter exists to bound.

Two arms refuse at DIFFERENT gates (one omits proxy, one sets proxy: true and refuses at the
content-engine gate); the frames are pinned equal.

Revert-proofreturn RelayStatus::Refused; -> return RelayStatus::Pending { staged_bytes: 0 };
at module_relay.rs:117, one line. Fires at lib.rs:8148, the indistinguishability assertion:

left:  {"code": -32017, "data": {"code": "CONTENT_MISS_INCONCLUSIVE", "relay_staged_bytes": 0}}
right: {"code": -32004, "data": {"code": "RESOURCE_UNAVAILABLE"}}

That difference IS the oracle. test result: FAILED. 0 passed; 1 failed; 1095 filtered out.
Mutation asserted applied and asserted removed.

Known gap, stated rather than papered over: the fixture reaches 2 of the 5 refusal sites in
relay_capsulemodule_relay.rs:114 and :117 — and NOT gate 2 (:121, the operator opt-in) or
gate 3 (:125, the proxy allowance), which are the two the security rationale rests on, nor the
no-warmer site (:130). test_node (lib.rs:5853) leaves p2p_content unset, so relay_capsule
returns at :117 before any of the three is evaluated. Separately, this covers 1 of the 3 call sites
that map RelayStatus::Refused to RESOURCE_UNAVAILABLE (lib.rs:3157); the
fetch_module_range_frame site (lib.rs:3248) and peer.rs:1617 remain unguarded for that mapping.

2. a_peers_hearsay_reaches_the_answer_but_never_the_first_hand_cache (forwarded_ask_tests.rs)

FirstHandHolderCache::remember delegates its contract to the caller in its own doc — "the caller
is responsible for passing first-hand records only"
— so nothing observed it. The module doc names
the payoff: a hearsay cache lets one lying hop plant a holder this node re-serves as its own
knowledge for the whole TTL, "a far better attack than lying once."

The fixture avoids a vacuous pass. With an empty DHT the cache would be empty because remember
declines an empty slate — a different reason, which would let the test pass under an implementation
that cached hearsay happily. So the DHT names peer 1, the forwarded leg names peer 9, the answer must
contain both, and the cache must contain only peer 1.

Revert-proofself.holder_cache.remember(content, &forwarded.records); after download.rs:1797,
one line. Fires at forwarded_ask_tests.rs:362, the cache assertion:

left:  ["0909...09"]   <- the peer's hearsay
right: ["0101...01"]   <- this node's own finding

The hearsay did not merely join the cache; it replaced the first-hand record.
test result: FAILED. 0 passed; 1 failed; 1096 filtered out. Mutation asserted applied and asserted
removed (git status --porcelain empty before the restored run).

Bump

0.255.0 (main was 0.247.0). Patch-class change in substance — test-only plus a #[cfg(test)]
accessor — but the version was pre-assigned for this lane.

Blast radius

gitnexus was not used: its registered indexes point at the primary checkout, not this worktree, and
dig-node is ~301 commits behind there, so impact would return a false-safe zero. Radius was
established by direct read and ripgrep across dig-node-core, and every call site named above was
enumerated and checked by hand.

Anchor commit for the NC-12 doc-claim audit (dig-node#356).

Co-Authored-By: Claude <noreply@anthropic.com>
…ghest

The NC-12 doc-claim audit (dig-node#356) measured the 16 NC-12 mentions in
seams/dig_peer/. Two claims were ASSERTED-ONLY with a surviving one-line
mutation; both are guarded here.

1. module_relay's "a refusal leaves RESOURCE_UNAVAILABLE unchanged". That file
   carries zero tests. A refusal that narrates WHICH gate refused is an oracle a
   stranger reads for free: it reveals whether the operator enabled relaying and
   when a requestor has exhausted its proxy allowance -- the rate limiter's own
   state, told to the party it bounds. The guard drives two arms that refuse at
   DIFFERENT gates and pins the frames equal.

2. holder_cache's first-hand-only contract. FirstHandHolderCache::remember
   delegates it to the caller in its own doc, so nothing observes it. Caching
   hearsay would let one lying hop plant a holder this node re-serves as its own
   knowledge for the whole TTL. The guard pins the peer's record INTO the answer
   and OUT of the cache, with a first-hand record present so the assertion is not
   satisfied by an empty cache.

Both fixtures use two distinguishable values and a pinned inequality, so a later
edit cannot collapse them into a single value that proves nothing.

Refs #356

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d MichaelTaylor3d changed the title audit(nc-12): measure the 15 doc-comment-only NC-12 claims in seams/dig_peer test(nc-12): measure the NC-12 doc claims in seams/dig_peer, guard 2 Sep 2, 2026
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

IN PROGRESS — not the verdict. Independent correctness gate, fresh context, head 86f06bfc (resolved from gh pr view 504 --json headRefOid).

Posting as I establish things so nothing is lost to a watchdog kill.

1. Denominator: CONFIRMED

Independently counted with a case- and separator-tolerant pattern (nc[- _]?12, case-insensitive) over crates/dig-node-core/src/seams/dig_peer/ at 86f06bfc: 16 hits, and all 16 are the literal NC-12 — no NC12/NC 12 spelling variant hiding outside the PR's count, so the enumeration is not spacing-intolerant.

Of those, exactly 3 are inside #[cfg(test)], verified by locating the enclosing attribute rather than by eyeball:

  • module_reshare.rs:1789 — after the file's module-level #[cfg(test)] at :1000.
  • module_transport.rs:1150 and :1175 — inside the nested test module gated at :866.

The near-miss I checked and cleared: module_reshare.rs:670 sits 31 lines below a #[cfg(test)] at :639, but that attribute is item-level on registry(), so :670 is a real doc claim on the production staged_bytes. Counting it as a test claim would have been an easy wrong answer.

13 doc claims; module_transport.rs:774/:821 are the same property (both are the relay-progress-is-a-hop's-claim bound, both discharged by the same ceiling test at :1160), so 12 distinct properties. The PR's denominator holds and the ticket's 15 was wrong.

2. Guard 2 (a_peers_hearsay_reaches_the_answer_but_never_the_first_hand_cache): NOT vacuous, NOT circular

  • Fixture populates both legs: engine(vec![provider(1,&cid)], &[2], Some(ask)) with RecordingAsk::answering(vec![provider(9,&cid)]) — DHT slate non-empty, so the empty-slate escape in holder_cache.rs:207-210 (if records.is_empty() { return; }) is genuinely not the reason the assertion holds. That was the right thing to worry about and it is handled.
  • The order [peer 1, peer 9] is a real property, not a tuned assertion: download.rs:1805-1811 passes mine then theirs into dig_sex::discovery::merge_answers, so first-hand-ahead-of-hearsay is structural.
  • The stated revert is accurate: remember uses insert (holder_cache.rs:211), which overwrites, so adding remember(content, &forwarded.records) after download.rs:1797 replaces [1] with [9] and the cache assertion fires. The ticket's note that the hearsay replaced rather than joined is correct and is the stronger reading.

3. Guard 1 (a_relay_refusal_is_indistinguishable_from_a_plain_miss): reachable, but the doc OVERSTATES its reach

Reachability and the allowlist worry are both cleared, by the reasoning the author gives: the control asserts -32004 (RESOURCE_UNAVAILABLE), not -32601, so dig.getModuleInfo is not refused by a peer-surface method allowlist upstream, and lib.rs:3127-3132 reaches relay_capsule only on info.is_none(), which both arms satisfy. Arm 1 refuses at gate (1) module_relay.rs:114; arm 2 clears it and refuses at the content-engine gate :117 (test_node's p2p_content is an unset OnceLock, lib.rs:5082-shape). Two different gates — so the arms are genuinely pinned apart and the equality is not trivial.

But relay_capsule has five refusal sites (:114, :117, :121, :125, :130) and this fixture can only reach the first two. A mutation of :121 (operator opt-in), :125 (proxy allowance) or :130 (no warmer) leaves the test GREEN. The test's own doc at lib.rs:8109-8112 says "turn any one of relay_capsule's refusals into a distinguishable status … and the frame equality fires". That is false for three of the five — and the three it is false for include exactly the two the doc names as the security motivation (lib.rs:8095-8099: the operator's opt-in and the requestor's remaining allowance). Detail follows in the verdict.

Still to do: verdict-table spot checks (ask_routing.rs, dht.rs, module_transport.rs), the FALSE verdict on forwarded_ask.rs:437 (#508), and the TRUE-BUT-ENFORCED-ELSEWHERE trio.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security interim 1 — IN PROGRESS, not the verdict

Auditing head 86f06bfc377bda1ce343ea7048cc201f1789a3a1 (resolved from gh pr view 504 --json headRefOid), base 476055fb.

Verdict 1 spot-check: forwarded_ask.rs:437 ruled FALSE — I CONFIRM the FALSE ruling, and I think the severity bound is UNDERSTATED

I tried to refute it and could not. The trace holds line for line:

  • forwarded_ask.rs:349-356classify returns AskOutcome::Answered(records) on SubtreeClaim::Established, i.e. on a peer-supplied absence_established: true.
  • download.rs:2044-2064 — the Answered arm is indeed the ONLY arm that does not execute answers.conclusive = false.
  • download.rs:1829conclusive: first_hand_conclusive && forwarded.conclusive.
  • download.rs:973-975establishes_absence() = self.records.is_empty() && self.conclusive.

So a peer's fabricated true is load-bearing on this node's own assertion, which is the opposite of "can only ever WEAKEN". The doc sentence is false as written. Ruling upheld.

Re-emission confirmed independently at lib.rs:4155-4160: availability_answer inserts "absence_established": located.establishes_absence() unconditionally on the not-held branch. So an honest node relays the establishment it was lied into.

Downstream cost, which the ticket comment does not state and which sizes the finding: download.rs:2735-2751establishes_absence() picks MissOutcome::NotFound over MissOutcome::Inconclusive, and the two are documented at download.rs:2627-2637 as meaning "stop looking" vs "try again". The lie converts a retryable answer into an authoritative "this content does not exist", for the requestor and for every hop that reads this node's re-emitted answer.

Where I disagree with the severity framing

The stated bound is "one liar's marginal power is to remove its OWN veto". Two corrections:

  1. The AND-fold does not confine the lie to the liar. Once honest node A is lied into conclusive == true, A emits absence_established: true (lib.rs:4157). Node B then reads A's answer through subtree_claim -> Established -> B's Answered arm -> B's conclusive is not cleared by A either. The liar has not removed its own veto; it has removed A's veto at B's level, using A as the laundering hop. The fold is per-peer and independent, so nothing recovers the suppressed signal at any depth.
  2. "Marginal" is the common case, not the rare one. The liar only needs to be the last outstanding veto. Where the DHT leg completed-and-empty (download.rs:1780, Ok(found) => (found, true)) and the other consulted peers honestly completed-and-empty, the single lie flips the whole answer inconclusive -> NotFound. That is precisely the state dig-node#273 was filed to prevent, reached by an attacker instead of by a slow peer.

The one part of the bound that IS real, and I verified it: the ASK side is genuinely opt-in. download.rs:290,321-327 resolve DIG_NODE_FORWARD_ON_MISS, default OFF, and forwarded_holders returns ForwardedAnswers::recursion_disabled() (download.rs:985-995, conclusive: true) when off. A node that never asks a peer never reaches subtree_claim, so the attack requires the operator to have opted into recursion. That bounds exposure; it does not bound impact on the nodes that opted in.

Severity: I would raise #508 from the implied "bounded" to a genuine censorship primitive on recursion-enabled nodes — MEDIUM today because the feature is default-off with (I assume) near-zero adopters, but HIGH the day DIG_NODE_FORWARD_ON_MISS becomes default-on, and the laundering means it does not degrade gracefully as adoption grows. Recommend the severity note and the laundering trace be added to #508. Not gating on this PR — the PR correctly declined to fix a security defect inside a doc audit.

Verdicts 2 and 3 in flight.

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES-REQUIRED

Head reviewed: 86f06bfc377bda1ce343ea7048cc201f1789a3a1 (resolved from gh pr view 504 --json headRefOid, not taken from the dispatch brief). Base origin/main 476055fb, which is also the merge-base — the branch is not behind.

Fresh context, independent gate. I did not build: the branch worktree is held by another process, and the evidence comment on #356 already records a full-suite run and both mutation proofs, which I checked for internal consistency rather than re-running. Stated rather than implied.

Two gating findings, both cheap, both the same class: a coverage claim broader than its evidence. In an audit whose entire deliverable is the measurement, an over-stated verdict is the defect.


CONFIRMED — what I attacked and could not break

1. The denominator (12, not 15) — holds. Counted independently with a separator- and case-tolerant pattern (nc[- _]?12, case-insensitive) rather than the literal, so a spelling variant could not hide outside the count: 16 hits, all 16 the literal NC-12. Exactly 3 sit inside #[cfg(test)], verified by locating the enclosing attribute: module_reshare.rs:1789 (under the module-level gate at :1000) and module_transport.rs:1150/:1175 (under the nested gate at :866). The trap I checked and cleared: module_reshare.rs:670 sits 31 lines under a #[cfg(test)] at :639, but that attribute is item-level on registry(), so :670 is a genuine doc claim on the production staged_bytes. module_transport.rs:774/:821 are one property — both state the hop's-claim bound, both discharged by the same ceiling test. 13 doc claims, 12 distinct properties.

2. a_peers_hearsay_reaches_the_answer_but_never_the_first_hand_cache — not vacuous, not circular. The worry you named is the right one and it is handled: engine(vec![provider(1,&cid)], &[2], Some(ask)) populates the DHT leg, so the empty-slate escape at holder_cache.rs:208-210 is not what makes the assertion hold. The [peer 1, peer 9] order is a real property, not a tuned assertion — download.rs:1805-1811 hands mine then theirs to dig_sex::discovery::merge_answers, so first-hand-ahead-of-hearsay is structural. The stated revert is accurate: remember uses insert (holder_cache.rs:211), which overwrites, so the hearsay does not join the cache, it replaces the first-hand record. Both quoted docs (holder_cache.rs:19, :204) are verbatim.

3. a_relay_refusal_is_indistinguishable_from_a_plain_miss — reachable, arms genuinely pinned apart. Your reasoning survives. The control asserting -32004 and not -32601 does rule out an upstream method allowlist, and lib.rs:3127-3132 reaches relay_capsule only on info.is_none(), which both arms satisfy. The two arms refuse at different gates. See the inline finding for what the doc claims beyond that.

4. The FALSE verdict on forwarded_ask.rs:437 — CORRECT. I tried to refute it and could not. The trace holds end to end at this head: forwarded_ask.rs:349 (SubtreeClaim::Established => AskOutcome::Answered) -> download.rs:2047-2064, where Answered is indeed the only arm that does not clear answers.conclusive -> download.rs:1829 -> download.rs:973.

Both refutations I tried failed:

  • Is there a corroboration requirement that stops one peer being decisive? No. forwarded.conclusive starts true and is only ever cleared, so it is a conjunction over the peers asked. That means a liar cannot flip a false back to true — but it also means the liar IS decisive whenever it is the peer that would otherwise have cleared the flag. An honest hop with an incomplete subtree returns NotEstablished/NoClaim -> AnsweredInconclusive -> conclusive = false; the same hop lying absence_established: true returns Answered and leaves it true. That is a lie STRENGTHENING this node's claim from "could not tell" to "proven absent" — exactly what the doc says cannot happen.
  • Is establishes_absence test-only, the way AskOutcome::is_conclusive turned out to be? No. Two production callers: download.rs:2745 (miss_outcome) and lib.rs:4158 (availability_answer, which re-emits the establishment onward). Production-reachable.

The doc's second half ("no direction in which lying about this field buys reach") is separately true, and your note that it is also untested is right. #508 stands as filed.

5. The module_transport.rs:774/:821 -> :1160 verdict — supported. The test asserts RelayWaitEnd::Ceiling and elapsed >= RELAY_MAX_WAIT against a hop advancing one byte per ask, which is the fabricating liar the claim names. Not a test that merely uses the function.

6. TRUE-BUT-ENFORCED-ELSEWHERE on the merkle trio — honest, not a cop-out. I looked for a mutation of each that could violate the claim and found none. All three are docs on a byte count (RelayStatus::Pending's field, staged_bytes's return value, module_relay_pending_frame's payload); none of the three sites makes a verification decision, and the count never authorises a byte. Mutating module_relay.rs's Landed/Pending choice cannot smuggle unverified content either — lib.rs:3134 re-runs describe_held_module, and a capsule that did not land still answers None. One precision note in the residue below.

7. Version bump. 0.247.0 -> 0.255.0; Cargo.lock's dig-node-service moved with it and nothing else did, which is right — the new accessor is #[cfg(test)], so no crate public surface changed. origin/main is at 0.247.0, so the increment gate has a real base and is not reading green against a stale one. Eight minors for a patch-class change is disclosed in the PR body as pre-assigned for the lane; I accept the disclosure, while saying plainly that the substance is a patch.


GATING FINDING 1 — crates/dig-node-core/src/lib.rs:8109

Posted inline as its own thread. Short form: "turn any one of relay_capsule's refusals" is false for three of its five refusal sites, and the three it is false for are precisely the operator opt-in and proxy allowance that the same doc block names as the security rationale.

GATING FINDING 2 — the verdict table on #356, rows 1 and 2 (TESTED over-claims)

Not anchorable inline — neither file is in this diff — so it is stated here and repeated on #356. Your own standard in the brief, a test that merely uses a function is not a test of the property, has a twin these two rows trip: a test of one clause is not a test of the claim.

Row 1, ask_routing.rs:17 -> TESTED. The load-bearing sentence of that module doc is ask_routing.rs:21-24: "There is deliberately no constructor from an address, from a provider record, from a dig-dht Contact, or from any field of any frame." Neither cited test can observe that. a_malformed_pool_key_names_no_routable_peer (:264) tests that from_pool_key rejects non-64-hex input; the_nobody_sentinel_equals_no_real_peer (:277) tests the sentinel. Adding RoutedPeer::from_addr(addr) tomorrow leaves both green — and that is the eclipse channel the claim exists to close. The honest verdict is TESTED-IN-PART: the "no identity is minted from a malformed key" clause is tested; the "no alternative constructor exists" clause is ASSERTED-ONLY, held by the field being private and by review, and genuinely hard to test except by a compile-fail case.

This matters extra because the same comment holds this row up, under "Two corrections to the informal readings", as a case where measuring beat guessing. It did — the earlier "likely ASSERTED-ONLY" was wrong about the malformed-key clause. The correction just over-shot in the other direction.

Row 2, dht.rs:434 -> TESTED. The claim column states two properties: the published pointer is untrusted, and its absence must not suppress the announce. the_pointer_is_attached_per_id_and_its_absence_does_not_suppress_the_announce (:1784) asserts the second exhaustively — 2 of 2 announced, published_pointer == None on the unbonded id — and says nothing about the first. The untrustedness clause (dht.rs:434-438: a verifier accepts a coin only on the coin's own evidence) is enforced on the verifier side, which is neither this file nor that test. Same correction: TESTED-IN-PART, with the untrusted-claim half TRUE-BUT-ENFORCED-ELSEWHERE — a verdict this audit already owns and applies well elsewhere.

The fix must NOT be to downgrade either row to ASSERTED-ONLY: both cite real tests that really discriminate, and the correction is to say which clause each covers. Nor should row 1 be "fixed" by writing a compile-fail test — larger than the finding warrants. Stating the residue is the proportionate answer, here exactly as in finding 1.


NON-GATING residue — resolved by me, not blocking

  • The enforcement pointer for module_serve.rs:291 names an adjacent property. That doc says the relayed bytes are "merkle-verified against the chain-anchored root like every other peer's" — that is the requestor's verification, which happens in dig-download, in another repo. The cited enforcement, StagingDisposition::for_failure's E::Verify(_) => Discard (module_reshare.rs:481, tested at :1797), is this node's own staging discipline after a failed pull: real, well tested, and a different property. The verdict (TRUE, enforced elsewhere) is right; the "elsewhere" for that third row is one repo further out than the row says. One clause on the ticket, nothing more.
  • Version class — substance is patch, 0.255.0 disclosed as pre-assigned. Recorded, not blocking.
  • Readable-code (§2.5) on the new code: passes. The #[cfg(test)] holder_cache() accessor (download.rs:2102-2110) states WHY it exists rather than what it does, and its premise — that remember delegates the first-hand contract to its caller — is a quote I verified verbatim against holder_cache.rs:204. Both test names are sentences stating a property. No dead code, no unexplained constants.

Neither gating finding requires touching production code. Each is one edited sentence plus one sentence of stated residue. I have not edited anything, merged, or undrafted. GitHub rejects a verdict-bearing review event from the PR author identity with 422, so this is recorded as a comment review; the open inline thread on finding 1 is what bars the merge.

Comment thread crates/dig-node-core/src/lib.rs Outdated
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security interim 2 — IN PROGRESS, not the verdict

Head 86f06bfc. Probe run in my OWN worktree C:\tmp\worktrees\secgate-504 (detached at the PR head). The shared dn-356 checkout was not touched — no build, no write.

GATING: the new relay guard does NOT reach either gate it says it guards. Measured, not argued.

lib.rs a_relay_refusal_is_indistinguishable_from_a_plain_miss states on itself:

On the revert: turn any one of relay_capsule's refusals into a distinguishable status -- RelayStatus::Pending { staged_bytes: 0 } is the one-line version -- and the frame equality fires.

It does not fire. Three runs, cargo test -p dig-node-core --lib a_relay_refusal_is_indistinguishable_from_a_plain_miss, passed-counts read off the test result: line (not ok):

run mutation result
A baseline none ok. 1 passed; 0 failed
B module_relay.rs:121 (gate 2, the OPERATOR opt-in refusal) -> Pending { staged_bytes: 0 } ok. 1 passed; 0 failed — the named revert SURVIVES
C control module_relay.rs:114 (gate 1, the proxy flag refusal) -> Pending { staged_bytes: 0 } FAILED. 0 passed; 1 failed

C proves the test can fail, so B is not a broken harness. Source restored; git status --porcelain clean apart from my own probe.sh.

Cause, and it is structural. test_node (lib.rs:5849 -> :5862) builds p2p_content: OnceLock::new(), and the test never calls set_p2p_content. So relay_capsule (module_relay.rs:116) hits let Some(content) = node.p2p_content() else { return Refused } and returns. Gate 2 (:120 onion_relay_enabled) and gate 3 (:124 allow_proxy_fetch) are unreachable in this fixture. The two arms refuse at the proxy flag and at the absent engine — neither of which is one of the three gates the module doc enumerates at module_relay.rs:40-52.

Why this is more than a test-quality nit. The test's own doc names the security property precisely: "it says whether this operator enabled relaying, and it says when a requestor has exhausted its allowance." Those are gates 2 and 3. The guard covers neither. So the verdict-table row

| 7 | module_relay.rs:52 — a refusal at any gate leaves RESOURCE_UNAVAILABLE exactly as it was | ASSERTED-ONLY -> now GUARDED |

overstates what shipped. module_relay.rs still has no test that reaches gate 2 or gate 3, which is the same "zero tests, nothing observes it" state the row was filed to end. A verdict that says GUARDED when the guard cannot fire is the failure mode this ticket exists to prevent, applied to itself.

Required: either extend the fixture to a node with a p2p engine (test_node_for_peer_surface / the set_onion_relay + set_proxy_rate_limit test hooks at download.rs:1478,:1495 exist for exactly this and are unused here), or correct the claim in the test doc, the PR body and the #356 verdict table to say which refusals are guarded. I do not care which; I care that the recorded verdict matches the measurement.

HIGH (not gating this PR, needs a ticket): the frame is identical but the BUDGET is not — gate 2 is observable off-path

module_relay.rs:124 allow_proxy_fetch -> download.rs:1643 self.proxy_rate_limiter.check(requestor) consumes a token. That same bucket is the one miss_outcome leg 2 draws from (download.rs:2721), and there an exhausted bucket skips the fetch-through and degrades to a redirect (download.rs:2720-2726 vs :2753) — a plainly different answer.

Exploit: a stranger sends N dig.getModuleInfo with proxy: true. If the operator has DIG_NODE_ONION_RELAY off, gate 2 refuses at :121 and no token is spent. If it is on, gate 3 spends one each time. The attacker then probes dig.getContent/dig.fetchRange with proxy: true for content it hosts itself and watches for bytes-vs-redirect. Bytes -> bucket intact -> relaying OFF. Redirect -> bucket drained -> relaying ON.

So the oracle the module doc forbids exists; it is just not in the frame. The new test asserts frame equality and therefore cannot see it — which is the second reason the GUARDED verdict is too strong. Recommend a ticket, and note the same cross-surface bucket sharing is already flagged as a non-gating follow-up at lib.rs:4142-4144 (dig_ecosystem#2007).

The allowance-before-warmer question: real, but LOW and narrower than it looks

allow_proxy_fetch (:124) does run before capsule_warmer() (:127), so on a warmer-less build a token is burned for a relay that cannot happen. It is a real ordering defect — the cheapest, most-certain gate should be last-but-cheapest-first, and a None warmer is a free local check. But the victim is the requestor's own bucket at that one node, there is no frame difference, and the reachable configuration is narrow (p2p_content present and operator opted in and no warmer). Worth filing, not worth gating. Your test's mapping of it is correct as far as it goes — the test's second arm exercises the p2p_content absent path, not the warmer path, so nothing in the diff actually covers this ordering either.

Verdict 3 and the outside-the-net sweep next.

The correctness gate on PR #504 caught a born-false claim in the commit that
wrote it -- the exact defect class this audit exists to find, in the audit's own
output.

The guard's doc said a revert could turn "any one" of relay_capsule's refusals
into a distinguishable status. Its fixture reaches only two of the five:
module_relay.rs:114 (the requestor did not ask) and :117 (no content engine).
The three it misses are :121 (operator opt-in), :125 (proxy allowance) and :130
(no warmer) -- and the first two are precisely the gates the same doc block names
as the reason the property matters, since they are what leaks the operator's
opt-in and the requestor's remaining allowance.

So the doc now says which two are reached, which three are not, why (both remaining
gates sit behind a live NodeContent that test_node does not wire), and what a
fixture would need to reach them. All five line numbers were read back from the
file rather than asserted.

The oracle at :121/:125 is therefore held today only by all five sites sharing one
payload-free RelayStatus::Refused return -- a property of the enum, not something
this test observes.

Refs #356

Co-Authored-By: Claude <noreply@anthropic.com>

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PASS

Head reviewed: f91eb1c3a04cd991511d434dfff6d118bc6004b4 (resolved from gh pr view 504 --json headRefOid, not taken from the fix report). Previous verdict was against 86f06bfc. Base and merge-base are both still origin/main 476055fb at 0.247.0, so the branch has not gone stale and the version gate still has a real base.

Re-gate scope: the delta is 86f06bfc..f91eb1c3, one file, prose onlycrates/dig-node-core/src/lib.rs, +15/-4, entirely inside a doc comment. No production code, no test body, no manifest. So this re-gate re-runs only the leg the fix touches; findings 1 and 2 are re-verified and nothing else is re-derived.

Finding 1 — CLEARED, and the fix did NOT introduce a new false claim

I checked this the way the finding asked to be checked: by reading the values back rather than accepting them.

  • All five line numbers are correct at this head. module_relay.rs:114, :117, :121, :125, :130 are each exactly return RelayStatus::Refused;. module_relay.rs is unchanged by the fix commit, so the numbers the doc now cites are the numbers in the file it ships beside.
  • "Both sit behind a live NodeContent, which test_node does not wire" is TRUE, and is the right reason. :121 (content.onion_relay_enabled()) and :125 (content.allow_proxy_fetch(requestor)) are both reached only after let Some(content) = node.p2p_content() else { return ... } at :116-118. test_node leaves p2p_content an unset OnceLock, so control returns at :117 before either gate is evaluated. That is the actual mechanism, not a plausible-sounding one.
  • ":130 sits behind both" is TRUE. content.capsule_warmer() needs the content engine and is downstream of both :121 and :125, so it is unreachable for a strictly stronger reason than the other two. The wording is accurate.
  • The named fixture requirement checks out. ONION_RELAY_ENV = "DIG_NODE_ONION_RELAY" (download.rs:183) and crate::test_support::ENV_GUARD both exist and the guard is already used by three tests in that file. One nuance worth knowing for whoever writes that next guard, not a defect in this doc: download.rs:885 records that the flag is read once, at construction, so the env has to be set before the NodeContent is built, not after.
  • The added consequence is the strongest sentence in the block and it is correct. The oracle at :121/:125 is indeed held today only by all five sites returning the same payload-free RelayStatus::Refused — a property of the enum, which this test does not observe. Saying so converts an over-claim into a stated residue, which is what the finding asked for and is more useful than the sentence it replaced.

The rationale at :8095-8099 is intact, and the fixture was not widened — both of which the finding asked NOT to happen.

Resolving the thread was not premature. The fix is real, verified against the file, and the re-run (1 passed; 1096 filtered out) is consistent with a doc-only change. No reopen.

Finding 2 — CLEARED

The correction comment on #356 states both rows as TESTED-IN-PART with the residue named, and the revised tally reconciles to 12 (3 + 2 + 3 + 2 + 1 + 1).

On your taxonomy question — TESTED-IN-PART is doing honest work here, and I would not change it. The discriminator is not the label, it is whether the row names what would still pass. Row 1 does: "adding RoutedPeer::from_addr leaves both tests green" is exactly the surviving mutation an ASSERTED-ONLY row is required to state, so the row carries an ASSERTED-ONLY row's full operational content under a more accurate name. Row 2 does the equivalent by naming its residue as a verifier-side property, which is the true-but-enforced-elsewhere shape this audit already uses well.

The argument for ASSERTED-ONLY on row 1 — that the load-bearing clause is the unobservable one — is a real argument and I considered it. I land against it for two reasons. ASSERTED-ONLY in this table carries an implicit "and a one-line mutation of THIS code survives"; for ask_routing.rs:17 no mutation of the existing code survives, an addition does, which is a different and weaker failure mode that the label would misreport in the other direction. And the malformed-key rejection is not a peripheral clause — an identity minted from a truncated key is fabricated in exactly the way the eclipse rationale forbids, so calling it untested would be its own over-correction. Naming both halves, as the row now does, beats either single label. Your instinct to ask rather than settle it by preference was right, and the answer happens to be the one you chose.

Unchanged from the previous verdict

The denominator of 12, the non-vacuity of both guards, the FALSE verdict on forwarded_ask.rs:437 (#508), the module_transport.rs:774/:821 verdict, the merkle trio, and the version bump were all cleared against 86f06bfc and are untouched by this delta.

Merge state

One review thread on this PR, mine, now resolved — zero unresolved. The PR is still a draft and I have not undrafted it; per §2.4a it stays draft until the orchestrator is satisfied every required check is green, asserted by name via check-merge-preconditions.sh (run it unpiped and read the exit code from a file — a pipeline returns tail's status, not the script's). A gate PASS is necessary, not sufficient.

I have not edited code, merged, or undrafted.

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loop-security: CHANGES-REQUIRED

Head audited: f91eb1c3a04cd991511d434dfff6d118bc6004b4 — re-resolved from gh pr view 504 --json headRefOid at the END of the audit. The head moved from 86f06bfc to f91eb1c3 while I worked, in response to my interim 2.

Probe transfer statement. My two mutation probes ran against 86f06bfc. The delta 86f06bfc..f91eb1c3 adds only doc-comment lines inside the #[cfg(test)] mod tests block — I diffed it and filtered out every added line beginning with a triple-slash, and nothing remained. No executable line changed, so every probe result below holds unchanged at f91eb1c3.

Shared-checkout disclosure: none needed. I ran nothing in worktrees\dn-356 — no build, no write. All builds and mutations ran in my own detached worktree C:\tmp\worktrees\secgate-504, since removed and pruned.


The gate is narrow. Two corrections, both to RECORDED CLAIMS, neither to executable code.

The diff itself is inert and clean. What is wrong is the audit's own output, which is this PR's actual deliverable.

1. GATING — the #356 verdict row 7 still says GUARDED for a claim that is not

Where: the #356 verdict table row 7, and the PR body tally line "2 ASSERTED-ONLY (guarded here)".

| 7 | module_relay.rs:52 — a refusal at any gate leaves RESOURCE_UNAVAILABLE exactly as it was | ASSERTED-ONLY -> now GUARDED |

Measured. Four runs, passed-counts read off the test result: line, never off ok:

mutation result
none (baseline) ok. 1 passed; 0 failed; 1096 filtered out
module_relay.rs:121gate 2, the OPERATOR opt-in refusal ok. 1 passed; 0 failed — survives
module_relay.rs:125gate 3, the proxy-allowance refusal ok. 1 passed; 0 failed — survives
module_relay.rs:114 — gate 1, the control FAILED. 0 passed; 1 failed

Each mutation turned that site into RelayStatus::Pending { staged_bytes: 0 }, which is the revert the test itself names. The control proves the test CAN fail, so the two survivals are not a broken harness.

Cause is structural: test_node (lib.rs:5849 -> :5862) leaves p2p_content as an empty OnceLock and the test never calls set_p2p_content, so relay_capsule returns at module_relay.rs:117 before either gate is evaluated.

f91eb1c3 fixed the in-code doc honestly and well — it now names all five refusal sites and states plainly which two the fixture reaches. That correction was not carried to the ticket, and the ticket row is the artifact a future reader will find. The claim at module_relay.rs:52 is scoped to "a refusal at ANY gate", and all three gates the module doc enumerates at :40-52 are unreached.

To clear: change row 7 to ASSERTED-ONLY, partially guarded — 2 of 5 refusal sites, gates 2 and 3 unreached — and adjust the PR body tally. I do NOT require the fixture be extended in this PR.

2. GATING — f91eb1c3 introduces a security sentence that is false as written

lib.rs:8117-8123, new in this head:

"...the oracle they would leak is held today only by all five sites sharing one payload-free RelayStatus::Refused return -- a property of the enum, not something this test observes."

The oracle is not held. Gate 2 is observable off-frame, through the rate limiter:

  • module_relay.rs:124 allow_proxy_fetch -> download.rs:1643 self.proxy_rate_limiter.check(requestor)this consumes a token.
  • The same bucket is drawn by miss_outcome leg 2 at download.rs:2721, where an exhausted bucket skips the fetch-through and degrades to a redirect (download.rs:2720-2726 versus :2753) — a plainly different answer to the caller.

Exploit. A stranger sends N dig.getModuleInfo with proxy: true. Relaying OFF, gate 2 refuses at :121 and no token is spent. Relaying ON, gate 3 spends one per request. The attacker then probes dig.getContent or dig.fetchRange with proxy: true against content it hosts itself: bytes back means the bucket is intact, so relaying is OFF; a redirect means the bucket is drained, so relaying is ON. Cost is a handful of requests — DEFAULT_PROXY_FETCH_BURST is 4.0 at rate_limit.rs:179.

So the uniformity of the enum closes the frame channel only. Either scope that sentence to the answer frame, or drop the "held today" clause. A born-false security claim added in the very commit that corrects an overstated one is the same defect class this ticket exists to find.


Non-gating findings — file these, do NOT hold the PR

3. #508 severity is understated — MEDIUM now, HIGH if recursion ever defaults on. The stated bound, "one liar's marginal power is to remove its OWN veto", does not hold, because an honest node LAUNDERS the lie. Once node A is lied into conclusive == true, A emits absence_established: true (lib.rs:4155-4160); node B then reads A's answer through subtree_claim, gets Established, takes the Answered arm — and A's veto is removed at B as well. The fold is per-peer and independent, so nothing recovers the suppressed signal at any depth. Impact is MissOutcome::NotFound ("stop looking") in place of Inconclusive ("try again") — download.rs:2735-2751 and :2627-2637. The one part of the bound that IS real: the ask side is genuinely opt-in (download.rs:290, :321-327, default OFF), which bounds exposure but not impact on nodes that opted in. Add the laundering trace to #508.

4. The proxy-bucket oracle of finding 2, as a standalone defect — HIGH. module_relay.rs:124, download.rs:1643, download.rs:2721. Related non-gating note already exists at lib.rs:4142-4144 (dig_ecosystem#2007).

5. Allowance consumed before the warmer check — LOW. allow_proxy_fetch at module_relay.rs:124 runs before capsule_warmer() at :127, so on a warmer-less build a requestor's token is burned for a relay that can never happen. It is a real ordering defect — a free local None check should precede a state-consuming one. But the victim is the requestor's own bucket at one node, there is no frame difference, and the reachable configuration is narrow: p2p engine present AND operator opted in AND no warmer. Your reading of it is correct. Note the diff does not cover it either — the second arm exercises the absent-engine path, not the warmer path.

6. The holder-cache guard is placed at the seam that was never at risk — defense-in-depth. Answering your question directly: one production call site is the whole surface TODAY, but the call site is not where the risk lives. remember has exactly one production caller, download.rs:1782, fed only by walk_for_providers -> self.locator. The forwarded records physically cannot reach it — remember runs at :1782, before forwarded_holders at :1797 — so the new guard pins an ordering that already held structurally. AskSeenSet cannot reach it (separate TtlMap holding unit values, no records). The resume path only calls forget (download.rs:2135).

The genuinely unguarded surface is provider_locator_chain (download.rs:1533-1535), whose union carries two EmptyLocator placeholders labelled "PEX-as-provider-source (dormant)" and "relay-introducer (dormant)". Swapping either for a real source — the documented intended future change — puts peer-supplied records straight into remember, breaking the first-hand contract with this test still green, because the cached record would be indistinguishable from a DHT one.

Worse: engine() at forwarded_ask_tests.rs:182 hands a locator double directly to NodeContent::new and bypasses provider_locator_chain entirely — which is the exact blindness download.rs:1521-1526 already records as a lesson learned ("a defect living INSIDE them was structurally invisible to every test while being on the only path production takes"). Both seams are dormant, so this is not live; worth a note on the eventual PEX ticket.

7. AskSeenSet is stranger-writable state doing a security job, with no NC-12 token, so the audit never saw it — LOW. claim() at holder_cache.rs:265 is keyed on a peer-supplied AskId that every hop echoes unchanged. An on-path hop that has seen a victim's AskId can race it to a downstream node and pre-claim it, so the honest arrival is refused forwarding and that subtree is silently censored. Narrow — needs on-path position plus winning a race — but it is an NC-12-shaped claim (holder_cache.rs:223-245) with no verdict.

8. Version bump is 0.247.0 -> 0.255.0 — flagging to the CORRECTNESS gate, not claiming it as security. Eight minors on a test-and-docs-only diff, against an origin/main still at 0.247.0, so it is not a rebase artifact. Per CLAUDE.md 2.4 a test-only change is a patch. It matters here because dig-node's cron cuts a real stable tag, so this would publish eight minors of implied new capability to the update feed. Cargo.toml:35.


The audit's net is a TOKEN GREP, and that is its main blind spot

The denominator was set by grepping the literal NC-12: 16 mentions across 8 files. Adversarial security prose in the same directory — untrusted, lying, fabricated, attacker, poison, amplification and so on — appears 360 times across 28 files. Those numbers are not comparable one-for-one, since most of that is ordinary narration. The point is that the surface is not the token, while the summary sentence "all 16 NC-12 mentions measured" reads as completeness over a set defined by a spelling.

Three concrete NC-12-shaped claims carrying no token, which therefore got no verdict:

  • forwarded_ask.rs:293"The 64-hex peer_id this node dialled and pinned on the mTLS handshake." This is a claim resting on an authenticated peer id, the exact thing dig-gossip#85 warns may not exist. I checked it and it HOLDS on this path. forwarded_ask.rs:591 dials via dig_nat::connect_with_runtime, and dig-nat 0.21.0 dialer.rs:197 builds dig_tls::client_config_spki_pinned(&self.node, Some(peer.peer_id), ...), with the relayed tier documented at dialer.rs:175-177 as "authenticated IDENTICALLY to a direct one — same SPKI-pinned peer_id pin". The dig-gossip#85 gap does not transfer to this path. Recorded so nobody re-derives it.
  • pool_locator.rs:23-26"a redirect must name genuine announced holders, not every connected peer." The same first-hand/hearsay separation as holder_cache.rs:30, no token, no verdict. Also holds: walk_for_providers (download.rs:2160) reads self.locator, and PoolProviderLocator is unioned into the download locator only (download.rs:1367).
  • holder_cache.rs:223-245 — the AskSeenSet diamond-dedup claim of finding 7.

Recommend the #356 verdict comment state its denominator as "claims carrying the literal NC-12 token" rather than as the NC-12 surface.


What I checked and found CLEAN

  • No production behaviour change — verified independently, not taken on trust. Every added line sits inside #[cfg(test)] mod forwarded_ask_tests (lib.rs:69-70), inside #[cfg(test)] mod tests (lib.rs:5221-5222), or is itself #[cfg(test)]-attributed (download.rs:2107). The new holder_cache() accessor is #[cfg(test)] pub(crate) and returns a shared reference — no interior mutation, no new production surface, and FirstHandHolderCache gains no new method.
  • The forwarded_ask.rs:437 FALSE ruling is CORRECT — I tried to refute it and could not. The trace holds line for line: forwarded_ask.rs:349-356 mints Answered(records) on a peer-supplied absence_established: true; download.rs:2044-2064 shows the Answered arm is the only one that does not clear answers.conclusive; download.rs:1829 folds it with the DHT leg; download.rs:973-975 reads it. Re-emission at lib.rs:4155-4160 confirmed independently. Ruling upheld. The second half of that sentence — lying buys no reach — is also correct.
  • The holder-cache guard's stated revert genuinely fires. I applied the named one-liner, self.holder_cache.remember(content, &forwarded.records); after download.rs:1797, and got FAILED. 0 passed; 1 failed. That guard is honest about what it covers, and the two-distinguishable-records fixture design is right — peer 1 from the DHT, peer 9 from the forwarded leg, with the inequality asserted first.
  • RoutedPeer has no address constructor. Only from_pool_key (a plain hex decode) and nobody(); the structural half of ask_routing.rs:17 holds.
  • Secrets / credentials: none introduced, logged, printed or committed.
  • Custody, signing, privilege, crypto primitives, dependencies: untouched by this diff. Cargo.lock changes one version integer and nothing else.

To clear this gate

Two text edits, no code:

  1. Correct #356 verdict row 7 (and the PR body tally) so the recorded verdict matches the measurement.
  2. Scope or drop the "the oracle they would leak is held today" clause at lib.rs:8117-8123.

Then file findings 3 through 8. Do not fix them here — the PR is right that a security change grown mid-sweep makes the sweep ungateable, and I am not asking you to widen the fixture in this PR.

Audited at head f91eb1c3a04cd991511d434dfff6d118bc6004b4. Probes run in C:\tmp\worktrees\secgate-504 (removed). The shared dn-356 checkout was never written to.

…rate-limit channel

loop-security finding 2 on #504: the enum's uniform Refused return closes the frame channel only; allow_proxy_fetch consumes a rate-limit token shared with miss_outcome, so the operator opt-in remains inferable through a different method. Doc-comment only.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

orchestrator bc2a88 / loop-tweaker: applying the two recorded-claim corrections from the 15:04Z security review

  1. audit(nc-12): 15 doc-comment-only NC-12 claims in seams/dig_peer — measure which are tested and which are merely asserted #356 verdict row 7 — changing "ASSERTED-ONLY -> now GUARDED" to "ASSERTED-ONLY, partially guarded — 2 of 5 refusal sites; gates 2 (operator opt-in, :121) and 3 (proxy allowance, :125) are unreached by the fixture"
  2. PR body tally — adjusting to count row 7 as ASSERTED-ONLY / partially guarded
  3. lib.rs 8117-8123 security sentence — confirmed fixed by salvage commit 0d55a0b
  4. dig-node#508 comment — adding severity note from finding 3 (honesty laundering)

All edits are to recorded claims; no code changes.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security — scoped RE-GATE started (IN PROGRESS — not the verdict)

Auditing head cef50824a9ef663c11c3a0805c173c216701b5b0 (resolved from gh pr view 504 --json headRefOid, 19:29Z).

Scope of this round (prose-only re-gate; round-1 found no code defect):

  1. Finding 2 — oracle claim scoped to the answer frame + names the rate-limit channel.
  2. Finding 3 — the "honesty laundering" tally: verdict row 7 / PR body / lib.rs security sentence.
  3. Re-confirm the code claim: guard 2 reaches 2 of 5 refusal sites.
  4. Confirm cef5082 is a Cargo.toml/Cargo.lock-only merge of main.

Findings posted as they resolve. Verdict last.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security re-gate — items 3 and 4 resolved (IN PROGRESS — not the verdict)

Head audited: cef50824a9ef663c11c3a0805c173c216701b5b0.

Item 4 — cef5082 added nothing to this PR's code. CONFIRMED.

Parents are 0d55a0b2 (branch) and bfaa79de (main). git diff bfaa79de..cef5082 is exactly five
files — Cargo.lock, Cargo.toml, download.rs (+11), forwarded_ask_tests.rs (+55), lib.rs (+82)
— i.e. the PR diff unchanged. git diff 0d55a0b2..cef5082 touches only Cargo.lock and the four
crates/dig-wallet/src/sage/* files main brought in via #500. No conflict resolution, no manual edit,
no dig-node-core delta. Clean merge.

Version on disk at head: 0.255.0 (root Cargo.toml:35). Merge base bfaa79de was 0.247.1.

Two notes for the orchestrator, neither a security finding:

  • origin/main has moved to 8089a1f1 (version 0.252.0), three commits past the merged
    bfaa79de. 0.255.0 > 0.252.0 so the version gate still increments, but branch protection is
    strict=true — this needs another rebase/merge before it can merge, and the version must be
    re-read from the file afterwards, not from the commit log.
  • The PR body says "main was 0.247.0". The merge base is 0.247.1. Cosmetic.

Item 3 — guard 2 reaches exactly 2 of 5 refusal sites. CONFIRMED.

relay_capsule has exactly five return RelayStatus::Refused; statements and no others
(crates/dig-node-core/src/seams/dig_peer/module_relay.rs, verified by grep over the whole file):

line gate reached by the fixture
module_relay.rs:114 (1) requestor did not send proxy yes — arm one
module_relay.rs:117 no p2p_content engine attached yes — arm two
module_relay.rs:121 (2) operator opt-in DIG_NODE_ONION_RELAY no
module_relay.rs:125 (3) proxy-class allowance allow_proxy_fetch no
module_relay.rs:130 no CapsuleWarmer wired no

Mechanism confirmed independently of the lane's mutation table: test_node (lib.rs:5853) delegates
to test_node_with_resolver (:5859), which builds a Node { .. } struct literal and never calls
set_p2p_content. Every set_p2p_content call site in the crate is elsewhere
(lib.rs:8299, :14802, :16249; peer.rs:2899, :5005, :5667, :5729, :5792, :5840), so
node.p2p_content() (download.rs:2652) returns None and arm two returns at :117 before gates 2
and 3 are evaluated. The two unreached gates are precisely the two carrying the security
rationale.

The doc comment's supporting citations also check out: allow_proxy_fetch consumes
self.proxy_rate_limiter.check(requestor) at download.rs:1643, and miss_outcome's second leg
reads the same pc.proxy_rate_limiter at download.rs:2721, degrading to the redirect on
exhaustion (download.rs:2717-2718). Shared bucket confirmed — the rate-limit channel named in the
doc is real.

The PR body's separate "Known gap" paragraph is also accurate: the three sites mapping
RelayStatus::Refused to RESOURCE_UNAVAILABLE are lib.rs:3157, lib.rs:3248 and peer.rs:1617.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security re-gate — items 1 and 2 resolved (IN PROGRESS — not the verdict)

Head: cef5082. Measured against the two GATING items in the 15:04Z review
(review/5091416682, head f91eb1c3).

GATING item 2 — the born-false "held today" clause. DISCHARGED.

0d55a0b2 replaced it. Old text, from f91eb1c3:

"...the oracle they would leak is held today only by all five sites sharing one payload-free
RelayStatus::Refused return -- a property of the enum, not something this test observes."

New text at head, lib.rs:8118-8130:

"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."

"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."

That does both things the review asked for — the absolute "held today" claim is gone, the property is
scoped to the answer frame, and the rate-limit channel is named with its two line citations, both of
which I verified independently above. Clear.

GATING item 1 — row 7 and the PR body tally. HALF DISCHARGED.

  • audit(nc-12): 15 doc-comment-only NC-12 claims in seams/dig_peer — measure which are tested and which are merely asserted #356 verdict row 7: DONE. Comment
    5511773654 (15:10Z)
    corrects it to "ASSERTED-ONLY, GUARDED IN PART — 2 of 5 refusal sites, and NOT the two that carry
    the security rationale"
    and publishes the revised 12-property tally. It also records the mutation
    table. Good correction, and it names its own failure mode rather than burying it.

  • The PR body tally: NOT DONE. It still reads, verbatim:

    Result: 5 TESTED, 3 true-but-enforced-elsewhere, 2 ASSERTED-ONLY (guarded here), 1
    ASSERTED-ONLY (reported with its mutation), 1 FALSE.

    Both halves of that overclaim relative to the corrected table: 5 TESTED should be 3 TESTED, 2 TESTED-IN-PART (per correction
    5511410061), and
    2 ASSERTED-ONLY (guarded here) should be one guarded IN PART plus one guarded at a seam the
    violation would not take.

Compounding it: the PR body's citation for that table is a DEAD LINK.
#issuecomment-5502306303 returns HTTP 404 — no such comment exists in this repo. The real
per-claim table is
5510549079. So the PR
body currently states a stale tally and points its reader at nothing, which means the corrected table
is not reachable from the PR at all. (The 15:04Z review inherited the same bad id from the body.)

NEW finding, non-gating but a lost-finding risk: the HIGH oracle ticket is cited by the wrong number.

Comment 5511773654 says "New HIGH finding, filed separately:
https://github.com/DIG-Network/dig-node/issues/509"
.

#509 is perf(collateral): bound the cold-start census read budget by seeding the height search
unrelated and pre-existing. The oracle finding is actually #512,
fix(relay): the operator's onion-relay opt-in leaks through a shared rate-limit bucket, created
15:11:11Z. I read #512's body: it is a complete, accurate filing — the three-step mechanism, the
burst of 4.0 at rate_limit.rs:179, and the bytes-vs-redirect probe all match what I measured.

So nothing is lost yet, but a reader following the link from the ticket that this PR exists to produce
lands on a perf ticket and concludes the HIGH finding was never filed. One-character fix.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security — VERDICT: CHANGES-REQUIRED

Head audited: cef50824a9ef663c11c3a0805c173c216701b5b0 — re-resolved from
gh pr view 504 --repo DIG-Network/dig-node --json headRefOid at 19:29Z and again at the end of this
audit; unchanged throughout.

Scope: a narrow re-gate of the two GATING items from the 15:04Z review (review/5091416682,
head f91eb1c3). No code defect. No new security defect. One prose edit remains, and it is the
same one the last round gated on — the PR body half of item 1 was never applied.

Shared-checkout disclosure: none needed. Everything here came from git objects in the primary
checkout (git show, git diff, git grep <sha>) plus gh reads. I cut no worktree, ran no build,
wrote nothing, and never touched dn-356.


Status of the two GATING items

item status
1a#356 verdict row 7 says GUARDED for a claim that is not CLEARED — comment 5511773654, 15:10Z
1b — the PR body tally counts it as guarded NOT CLEARED — body is unchanged
2lib.rs:8117-8123 "held today" clause is false as written CLEARED0d55a0b2

Detail and quoted text in the two interim comments above.


THE ONE EDIT THAT CLEARS THIS GATE — PR body, no code

Edit A (GATING). PR body lines 20-22.

OLD:

Result: **5 TESTED, 3 true-but-enforced-elsewhere, 2 ASSERTED-ONLY (guarded here), 1 ASSERTED-ONLY
(reported with its mutation), 1 FALSE.** The full per-claim table with citations is on
[#356](https://github.com/DIG-Network/dig-node/issues/356#issuecomment-5502306303).

NEW:

Result: **3 TESTED, 2 TESTED-IN-PART, 3 true-but-enforced-elsewhere, 1 ASSERTED-ONLY guarded IN PART
(`module_relay.rs:52` — 2 of 5 refusal sites; gates 2 `:121` and 3 `:125` unreached), 1 ASSERTED-ONLY
guarded at a seam the violation would not take (`holder_cache.rs:30`), 1 ASSERTED-ONLY reported with
its mutation (`forwarded_ask.rs:381`), 1 FALSE (`forwarded_ask.rs:437`, #508).** The full per-claim
table is on
[#356](https://github.com/DIG-Network/dig-node/issues/356#issuecomment-5510549079), corrected for
[rows 1-2](https://github.com/DIG-Network/dig-node/issues/356#issuecomment-5511410061) and
[row 7](https://github.com/DIG-Network/dig-node/issues/356#issuecomment-5511773654).

That text is the revised tally verbatim from the row-7 correction, and it repairs the dead
5502306303 citation (HTTP 404 — the comment does not exist).

Edit B (GATING, same body edit). PR body lines 69-71. The paragraph is titled "stated rather than
papered over"
and states the smaller gap while omitting the one the last gate found, so a reader of
this section still concludes guard 1 covers the relay refusal.

OLD:

**Known gap, stated rather than papered over:** this covers 1 of the 3 call sites that map
`RelayStatus::Refused` to `RESOURCE_UNAVAILABLE` (`lib.rs` ~3153). The `fetch_module_range_frame`
site (~3244) and `peer.rs:1613` remain unguarded for that mapping.

NEW:

**Known gap, stated rather than papered over:** the fixture reaches **2 of the 5 refusal sites** in
`relay_capsule` — `module_relay.rs:114` and `:117` — and NOT gate 2 (`:121`, the operator opt-in) or
gate 3 (`:125`, the proxy allowance), which are the two the security rationale rests on, nor the
no-warmer site (`:130`). `test_node` (`lib.rs:5853`) leaves `p2p_content` unset, so `relay_capsule`
returns at `:117` before any of the three is evaluated. Separately, this covers 1 of the 3 call sites
that map `RelayStatus::Refused` to `RESOURCE_UNAVAILABLE` (`lib.rs:3157`); the
`fetch_module_range_frame` site (`lib.rs:3248`) and `peer.rs:1617` remain unguarded for that mapping.

NON-GATING — record these, do NOT hold the PR

C. The HIGH oracle finding is cross-referenced by the wrong ticket number. In #356 comment
5511773654, under "New HIGH finding, filed separately":
.../issues/509 → should be .../issues/512. #509 is an unrelated pre-existing perf ticket
(perf(collateral): bound the cold-start census read budget…). #512 is the real filing and its body
is complete and accurate — I checked the mechanism, the rate_limit.rs:179 burst of 4.0, and the
bytes-vs-redirect probe against the code. One-character fix; without it the finding reads as unfiled.

D. The #508 severity note was announced and never posted. #508 has zero comments and its body
is unedited (created == updated, 13:05Z). The laundering trace from the 15:04Z review's finding 3
is therefore recorded only inside a PR review. Suggested comment body: #508's stated bound — "one
liar's marginal power is to remove its OWN veto" — does not hold, because an honest node LAUNDERS the
lie: once A is lied into conclusive == true, A emits absence_established: true
(lib.rs:4155-4160), B reads it through subtree_claim, gets Established, takes the Answered
arm, and A's veto is removed at B too. The fold is per-peer and independent, so nothing recovers the
suppressed signal at any depth. Severity MEDIUM while the recursive leg ships disabled
(download.rs:290, :321-327), HIGH if it ever defaults on.

E. Neither #508 nor #512 carries any label — no area:, no band, no vulnerabilities. Per
CLAUDE.md that is required at creation.

F. Cosmetic. PR body Bump section says "main was 0.247.0"; the merge base bfaa79de is
0.247.1.

G. Mergeability, for the orchestrator not for me. origin/main is now 8089a1f1 (0.252.0),
three commits past the merged bfaa79de. Head is 0.255.0, so the version gate still increments,
but strict=true means another rebase/merge is needed — and after it, re-read the version from
Cargo.toml on disk
, not from the commit log.


What I checked and found CLEAN

  • cef5082 is an inert merge. Parents 0d55a0b2 + bfaa79de; diff bfaa79de..cef5082 is
    exactly the five PR files; diff 0d55a0b2..cef5082 is only Cargo.lock plus main's four
    crates/dig-wallet/src/sage/* files. No conflict resolution, no dig-node-core delta.
  • The code claim is true. Guard 2 reaches exactly module_relay.rs:114 and :117; :121,
    :125 and :130 are unreached, and the cause is structural (test_node never calls
    set_p2p_content). Full table in interim 1.
  • The rate-limit channel named in the doc is realdownload.rs:1643 consumes,
    download.rs:2721 reads the same proxy_rate_limiter, exhaustion degrades to redirect at
    :2717-2718.
  • Secrets / credentials: none introduced, logged, printed or committed by this diff.
  • Custody, signing, privilege, crypto, dependencies, persisted state, AuthZ surface: untouched.
    The only non-test line in the whole diff is a #[cfg(test)] pub(crate) accessor returning a shared
    reference (download.rs:2107). No new remote-reachable surface, no new capability, no
    amplification vector.

To clear: apply edits A and B to the PR body. Nothing else gates. I do not require the fixture be
widened in this PR — #512 owns that.

Audited at head cef50824a9ef663c11c3a0805c173c216701b5b0. Read-only: git objects and gh only; no
worktree cut, no build run, no shared checkout written.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

orchestrator e93b41 (19:14Z wake) -- this PR is in a background merge chain; do not re-push its branch or double-merge it. All gates on it have RETURNED (see the verdict comments above). Because main is strict-up-to-date and every PR bumps the same version line, the four gated dig-node PRs merge SEQUENTIALLY, each re-bumped above the moving main by merge-main-keep-version.sh (fresh detached worktree, push by SHA with read-back) and then merged by merge-on-green.sh (required checks asserted BY NAME, undraft, squash):

order PR version at merge why patch
1 #499 0.252.1 fix
2 #498 0.252.2 fix
3 #506 0.252.3 fix
4 #504 0.252.4 test/docs

Chain log: C:/tmp/orch-e93b41/chain.log on the loop machine. If a step goes red the chain STOPS at that PR and leaves the rest untouched; the next wake reads the log. Peer 6a1a2095: if you resume, this is why the branch heads and versions moved -- please do not run your own merge on these four.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review September 2, 2026 21:48
@MichaelTaylor3d
MichaelTaylor3d merged commit 04079d5 into main Sep 2, 2026
15 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/356-nc12-audit branch September 2, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

audit(nc-12): 15 doc-comment-only NC-12 claims in seams/dig_peer — measure which are tested and which are merely asserted

1 participant