fix(peer): require agreement between independent sources before a reflexive address is believed

[MichaelTaylor3d]

Implements #566: require agreement between independent sources before this node's reflexive
address is believed durably. Adopts dig_stun::establish (0.1.1, published) rather than this
crate's own agreement rule — this is a security primitive, and a second hand-rolled
implementation of one is exactly the rival CLAUDE.md's "centralize rival implementations" rule
exists to catch.

Response to the security-audit gate (head ddb5af5a, this PR is now at a later head)

The gate ran against my FIRST commit and returned CHANGES-REQUIRED with 4 findings. Status of each:

• F1 (red Test + coverage, 3 failures in mirror/advertise.rs) — FIXED, second commit
  (c4b1bc4c, pushed before the gate's interim comment landed). Root cause was real, not a fixture
  artifact: established_addresses() folds routability into agreement, and effective_urls's old
  empty/non-empty split couldn't distinguish "genuinely nothing agreed" from "something agreed
  perfectly but wasn't a public address." Added PublicAddress::any_family_not_global(). One of the
  three tests (a_this_machine_reflexive_address_is_never_derived_into_a_coin) also needed its
  fixture reshaped, not just the state fixed — detailed under "The gate" below.
• F2 (NotGlobal→Uncorroborated mislabel) — the SAME fix as F1, same commit.
• F3 (design question: should a degenerate reading count as dissent?) — ANSWERED, measured not
  guessed. Read dig-stun's own SPEC.md §7.3 directly: unanimity is step 3, global-unicast scope
  is step 5 — the crate discards the whole family on ANY dissent, degenerate or not, BEFORE it ever
  asks whether an individual reading is routable. This is dig-stun's own documented behaviour, not
  an artifact of how this PR feeds it: dig-node passes raw IPs through with no pre-filtering, and the
  step ORDER is entirely dig-stun's. Its own stated reason: "a node behind a multi-egress NAT, a
  misconfigured relay, and a lying peer all look the same from here, and in every one of those cases
  advertising is wrong." Recorded (third commit, 96179ee8) in both places asked for:
  PublicAddress::established's doc comment and SPEC.md §25.10. I did not change the behaviour —
  the gate itself said not to, and weakening unanimity to a majority vote would let an attacker who
  can cheaply add sources outvote the honest ones.
• F4 (stale Reflexive::source doc comment) — FIXED, same third commit.

What was actually broken

dig.getNetworkInfo's reflexive_addr already published the [{"source","addr"}] wire shape
(#567), and mirror::advertise::PublicAddress::corroborated_addresses already existed as an
agreement check — but the two never actually met: StunPlan::discover_reflexive stops at the
FIRST tier that answers, and PeerStatus stored at most ONE (SocketAddr, &'static str) reading.
So the array published to reflexive_addr could structurally never carry more than one entry in
production, and corroborated_addresses (which needs 2+) was dead code. That is the "declines to
create coins because a single STUN reading yields AdvertiseState::Uncorroborated" state the
ticket names — this PR is what lets it stop declining.

corroborated_addresses was also, independently, the WRONG rule: it accepted any PAIR of
differently-sourced readings that agreed, with no unanimity check. A dissenting THIRD reading
never blocked it — see the revert-proof below.

The gather (crates/dig-node-core/src/seams/dig_peer/net.rs)

• gather_reflexive_readings queries the operator + relay tiers in FULL (this node's own
  infrastructure — no third party learns anything) then consults the public tier per address
  family, ONLY while that family still has fewer than dig_stun::establish::MIN_INDEPENDENT_CLASSES
  (2) distinct classes without it. Once a family is sufficient, public is never queried for it.
• Class labels are rendered via dig_stun::establish::SourceClass (never hand-formatted), attached
  at the HOST before DNS resolution collapses it into a bare SocketAddr — two public hosts
  (stun.l.google.com / stun.cloudflare.com) count as different classes; a dual-stack relay's v4
  • v6 answers count as the SAME class (relay:<host>, one class per relay host).
• StunPlan::discover_reflexive (first-answer-wins) is UNCHANGED and keeps serving the
  hole-punch tier / DHT transport (bring_up_dht), which needs exactly one endpoint. The two uses
  are deliberately not collapsed into one, per the brief.

Wiring (peer.rs, lib.rs)

PeerStatus now stores every reading gathered (Vec<(SocketAddr, String)>), not a single
(addr, &'static str). network_info() publishes the full set in the SAME wire shape SPEC.md
already documented (no wire-format change, just actually populated with N>1 entries).

The gate (crates/dig-node-service/src/mirror/advertise.rs)

PublicAddress::established() delegates to dig_stun::establish; established_addresses()
replaces corroborated_addresses() as effective_urls's agreement input. effective_urls's
signature and the Effective/AdvertiseState contract are unchanged — still 6 states, still the
same dig-node-control-interface 0.33 wire shape. Confirmed via gitnexus impact before touching
either symbol:

symbol	direction	risk	impacted	direct
corroborated_addresses	upstream	HIGH	19	1
effective_urls	upstream	CRITICAL	21	15
discover_reflexive	upstream	HIGH	13	10
PeerStatus::set_reflexive	upstream	HIGH	3	1

effective_urls fans into BOTH the mirror-coin creation pass (spawn_mirror_passes) and the
control.mirror.* RPC surface (dispatch_owned) — this is why its external contract is untouched
and only the internal agreement computation changed.

A follow-up fix needed on top of the swap (caught by 3 pre-existing tests + 1 of my own, all
now green): established_addresses() folds the routability check INTO agreement (dig_stun::establish
refuses anything short of Scope::GlobalUnicast), which effective_urls's old empty/non-empty
split couldn't see — a family that agreed perfectly on a private/loopback reading fell through to
Uncorroborated instead of NoPublicAddress, even though that state's own doc already covers this
("what was reported is not a public address"). Added PublicAddress::any_family_not_global() and
used it in the branch. Second commit in this PR; both are pushed and green.

Diagnostics — dign network-info

Per the brief ("an operator debugging 'why is my node uncorroborated' needs to see which sources
answered and what each said"): the CLI now renders every reflexive-address reading plus the
establish() verdict per family (unanimous/insufficient/disagreeing/not-global/no-reading), reusing
PublicAddress::from_network_info/established rather than re-deriving anything.

Revert-proofs (committed first, per the harness's own rule)

1. Unanimity: reinstated the retired pairwise check in established_addresses(), ran
   a_dissenting_third_reading_blocks_establishment_even_though_two_others_agree — FAILED, and for
   the intended reason: Effective { urls: ["dig://93.184.216.34:9444"], state: Derived } — the old
   code silently advertised the majority address despite an independent dissenter. Restored.
2. Public-tier skip: disabled the if short { } guard in gather_from_classes, ran
   public_tier_is_skipped_once_operator_alone_reaches_the_class_floor — FAILED: the dissenting
   public:would-dissent reading appeared (3 readings instead of 2). Restored.

A trap worth recording for whoever reads this next

dig-nat's own STUN client (query_reflexive_address) rejects a non-globally-usable mapped
address, INCLUDING documentation ranges (RFC 5737 203.0.113.0/24), as StunError::NoMappedAddress
— this file already had ONE comment warning about it
(a_cross_family_stun_answer_is_discarded_the_other_direction_too) and I still walked into it
building my 3 new net.rs gather tests (all fixtures used 203.0.113.x; all 3 failed with empty
readings until I switched to genuinely global-unicast literals — 93.184.216.34, 1.1.1.1,
matching the file's OWN established convention of 100.64.x.x/pure-function-only for that reason).
Separately, ::1 folds to 0.0.0.1 under dig_stun's fold-before-bucket discipline (SPEC.md §5.3)
and is NOT safe as an "isolated in its own family" IPv6 test address — fe80::1 is.

Found but explicitly NOT touched (§2.4b dep sweep, out of scope for this ticket)

• crates/dig-node-service/src/mirror/advertise.rs's own is_globally_routable/
  is_globally_routable_v4/is_globally_routable_v6 are a RIVAL of dig_stun::scope:: is_globally_routable — dig-stun's own SPEC.md already names this exact duplication
  ("dig-node's on-chain gate") as one of the two tables it was built to unify. The brief said to
  report rather than fix; reconciling it is a separate, larger PR (it's the routability check
  derived_urls runs, now largely redundant-but-harmless defense-in-depth since establish()
  already excludes non-global addresses before derived_urls ever sees them).
• dig-dht = "0.15" stays — bumping to the published 0.16.1 would split the dig-dht line: both
  dig-download 0.22.1 and dig-peer-selector 0.11.1 (this crate's other direct/transitive users)
  still declare dig-dht ^0.15 on the registry. Upstream hasn't moved yet; matches the exact
  precedent this crate's own Cargo.toml comment already documents for the 0.13→0.15 jump.
  dig-constants/dig-chainsource-interface needed -p name@version disambiguation during
  cargo update for the same reason (two lines already resolved); neither is newly split by this PR.
• chia-protocol/chia-traits/chia-bls/chia-sha2/chia-puzzle-types all show 0.48.0 on
  crates.io while this crate pins 0.36.1, but chia-sdk-driver/chia-sdk-types/chia-sdk-utils
  (also direct deps here) are STILL at 0.36.0 on the registry — bumping the first five alone would
  split the chia line exactly the way CLAUDE.md §2.4b warns against, and the prerequisite (the
  chia-wallet-sdk line publishing a 0.48-compatible release) hasn't happened. Confirmed via the
  registry, not memory.
• dig-ipc-protocol WAS bumped (=0.3.0 → =0.3.2, exact-pinned by design): checked its
  CHANGELOG first — 0.3.1 is a chia-bls dep bump onto the SAME 0.36.1 line this workspace already
  uses, 0.3.2 is docs-only. Neither touches the IPC wire shape the exact pin protects.
• 20 other dig-* deps bumped via cargo update -p <name> (patch-level, already inside their
  declared caret range, no Cargo.toml text change needed): dig-identity, dig-rpc-protocol,
  dig-message, dig-keystore, dig-nat, dig-constants, dig-social-profile, dig-pex, dig-download,
  dig-peer, dig-peer-selector, dig-tls, dig-sex, dig-store-cache, dig-mirror-collateral,
  dig-mirror-coin, dig-chainsource-interface, dig-cert, dig-logging, dig-urn-resolver.

Evidence bar (§2.6) — what I could and could not reach on this machine

This machine is NAT'd with no outbound IPv6, so a genuine end-to-end run (a real host reaching ≥2
independent classes and reporting a non-null established reflexive_addr) was not something I
could produce here — stated plainly rather than implied. What I DID verify, all on real code paths:

• Gather mechanism: public_tier_is_queried_and_kept_when_the_family_is_short and
  operator_and_relay_are_always_queried_in_full prove the ordering + skip logic against REAL fake
  UDP STUN servers on loopback (not mocks) — real Binding transactions, real responses.
• Agreement mechanism: the unanimity/class-floor/peer-escalation tests exercise the REAL
  dig_stun::establish crate (not reimplemented), fed through PublicAddress::from_network_info
  exactly as production does.
• Query cost: a typical check today (operator unset, one relay configured) costs 1 relay query
  always, plus 1 public query IF the relay answers (to reach the floor of 2), 2 public queries if
  the relay is silent — never both public hosts once the floor is met. Never queries public at all
  once operator+relay alone reach 2 classes.

Version

0.254.85 (assigned; main was 0.254.84, confirmed unmoved before bumping).
cargo update -w --offline synced Cargo.lock — only dig-node-service's own lock entry moved.

Verified (all commands from the brief, actual counts)

• cargo fmt --all -- --check — clean, exit 0.
• cargo clippy --workspace --all-targets --all-features --locked -- -D warnings — clean, exit 0
  (1m25s).
• cargo test --workspace --all-targets --all-features --locked --jobs 4 — exit 0, 3142 tests
  passed across 55 binaries, 0 failed (grep -c "test result: FAILED" = 0). Includes
  dig-node-core: 1129 passed, 0 failed, 1 ignored (the live-network probe, correctly skipped
  offline); dig-node-service: 838 passed, 0 failed; the rest across dig-wallet/dig-runtime/
  dig-chat-protocol integration suites. Baseline for comparison: PR feat(peer): harden reflexive discovery (cross-family reject, stun host, mirror-advertise control) #569 (the immediately prior PR
  on this file) reported ~3132 — the small increase is this PR's own new tests.

Blast radius checked (gitnexus impact, per-worktree index, 14,490 nodes / 38,922 edges): table
above. detect_changes({scope: "compare", base_ref: "main"}) run: 53 changed symbols across 10
files, 3 affected cross-community processes (dispatch_owned → Reflexive,
run_peer_network → Derived_network_label, run_peer_network → Genesis_challenge_from), risk
medium — all match the intended blast radius (the gather machinery in net.rs, PeerStatus in
peer.rs, PublicAddress/AdvertiseState in advertise.rs, the CLI renderer in
network_info.rs); no symbol outside that set was touched.

Closes #566

[MichaelTaylor3d]

IN PROGRESS -- not the verdict. Security-audit findings so far, head ddb5af5a4d7538dde026307c237f8403ab794c14.

1. The required Test + coverage check is RED, and the three failures are inside the exact file this PR is auditing (mirror/advertise.rs).

mirror::advertise::tests::an_operator_may_publish_a_lan_address_the_derived_path_refuses
  left: Uncorroborated   right: NoPublicAddress
mirror::advertise::tests::a_derived_address_outside_global_unicast_is_refused
  left: Uncorroborated   right: NoPublicAddress
mirror::advertise::tests::a_this_machine_reflexive_address_is_never_derived_into_a_coin
  left: []               right: ["dig://93.184.216.34:9444"]

nextest cancelled after these three, so 1549/3152 tests (49%) never ran and the >=80% coverage gate never completed. Run: https://github.com/DIG-Network/dig-node/actions/runs/33990980019/job/101373049106

Root cause (read, not re-run -- traced through dig-stun's establish.rs and scope.rs): all three are PRE-EXISTING tests (unchanged by this diff) that build Reflexive { source: "relay", .. } / "stun.example" -- bare tier labels, not the class-grammar (relay:<host>, public:<host>, ...) this PR's own SPEC.md update (SPEC.md diff, "naming its reporting CLASS as source") now requires. dig_stun::establish only needs string INEQUALITY for class-counting, so the bare labels still count as 2 independent classes -- the failures are NOT a class-independence bug. The real cause is two-fold:

a) effective_urls() (crates/dig-node-service/src/mirror/advertise.rs:564 region) treats ANY empty established_addresses() result as Uncorroborated whenever reflexive is non-empty. But established_addresses() is now ALSO empty for FamilyVerdict::NotGlobal (agreement succeeded, unanimous IP, but it's loopback/private/link-local) -- a condition the code used to detect explicitly via derived_urls()'s own is_globally_routable() check and label NoPublicAddress. That downstream check is now effectively dead code for the derived path, since dig_stun::establish already enforces Scope::GlobalUnicast before an address can ever reach established_addresses(). Net effect: two OPERATIONALLY DISTINCT remedies ("get a second source" vs "this address will never work, get a different source") now collapse into one wrong label. Both states still fail closed (urls: [] either way) -- no wrong address is ever published. This is a diagnostic/operator-facing correctness defect, not a fund-safety one.

b) a_this_machine_reflexive_address_is_never_derived_into_a_coin is a real BEHAVIOUR CHANGE, not just a label bug: when the same family carries both a genuinely public, 2-class-agreed address AND an unrelated degenerate reading (loopback/link-local from a different seam), dig_stun::establish's per-FAMILY unanimity now returns Disagreement for the WHOLE family, discarding the good address too -- where the OLD pairwise corroborated_addresses() would have kept the good pair and let derived_urls()'s routability filter drop only the bad one. This is the intentional, documented "too-strict" fail-closed direction (SPEC.md: "a single dissenting source is treated as proof something is wrong, however many others agree with each other") -- I read this as CORRECT and SAFE from a custody standpoint (never risks publishing a wrong address), but it is an unresolved availability question the PR has not addressed: a single misbehaving/misconfigured source (a bad relay, a bad operator-configured STUN entry, or -- once #3199's peer tier lands -- a single dishonest peer) can now permanently deny establishment for a legitimate address in the same family, simply by disagreeing, with no requirement that its own reading be corroborated by anyone. This matches the documented threat model and I am NOT treating it as a gating defect, but it should be a recorded, deliberate decision (a SPEC.md note on the availability trade), not something that ships silently alongside a still-failing test asserting the opposite.

2. The PR body is still the stub: "DRAFT -- WIP, do not merge, gate round not yet started ... Will update this body with the full change description before requesting review." It was never updated. There is no stated evidence of what was and was not reachable on a real host (dev machine is NAT'd, no outbound IPv6, per the dispatch) -- the SS2.6 end-to-end evidence bar is entirely unaddressed.

3. Positive confirmations (read against dig-stun v0.1.1, the pinned version, and the wiring in net.rs/peer.rs/lib.rs):

• No rival implementation: established() delegates fully to dig_stun::establish::establish; the old corroborated_addresses is deleted, not left beside it.
• Unanimity + independent-class-count + global-unicast-scope are all correctly implemented in dig-stun::establish::verdict_for_family (Disagreement / Insufficient / NotGlobal / Established, exhaustively matched).
• Class grammar is correct and independent per host: PUBLIC_STUN_SERVERS = [("stun.l.google.com", 19302), ("stun.cloudflare.com", 3478)], each mapped to its own SourceClass::Public{host} -- confirmed two distinct classes, not one.
• reflexive_addr publishes RAW READINGS only ([{"source","addr"}] or null); the actual establish/effective-address decision is re-derived downstream by mirror::advertise::PublicAddress from that same array -- no masquerading of an unestablished reading as an established address.
• First-answer-wins survives correctly for the hole-punch tier: peer.rs:2708 and peer.rs:3142 both still call StunPlan::discover_reflexive unchanged and feed stun_server/the DHT transport; the NEW gather_reflexive_readings() call is a separate, additional walk feeding ONLY status.set_reflexive() (the agreement input). The two were not collapsed.
• Public tier is correctly conditional: gather_from_classes queries operator+relay in full, then queries public per-family ONLY while classes_for_family(..) < MIN_INDEPENDENT_CLASSES, confirmed both by reading net.rs and by the two new tests public_tier_is_skipped_once_operator_alone_reaches_the_class_floor / public_tier_is_queried_and_kept_when_the_family_is_short (both pass in CI).
• No port-equality check, no host/cloud-range special-casing anywhere in the diff (confirmed by reading is_globally_routable/scope.rs and the net.rs gather path).
• Version bump is 0.254.84 -> 0.254.85 (patch, as directed) and the commit already carries the breaking marker: fix(peer)!: require agreement between independent sources before a reflexive address is believed -- the ! is warranted (the PeerStatus::reflexive()/set_reflexive() public signature and the reflexive_addr wire shape both changed) and is already present, so no action needed there.

Full verdict to follow.

[MichaelTaylor3d]

Security audit verdict: CHANGES-REQUIRED

Head audited: ddb5af5a4d7538dde026307c237f8403ab794c14 (re-resolved myself via gh pr view 571 --json headRefOid, matches dispatch).

Why not PASS

This is not a rejection of the design -- the AGREEMENT mechanism itself (dig_stun::establish, delegated to correctly, unanimity + independent-class-count + global-unicast-scope all present and exhaustively matched) reads as sound, and I found no path where a wrong/attacker-controlled address can be published (see "positive confirmations" in the interim comment above). The reasons this cannot PASS today:

1. The required Test + coverage check is RED, with 3 failures inside mirror/advertise.rs -- the exact file implementing the on-chain-facing establishment decision. That alone is disqualifying: a security audit cannot certify a mechanism whose own test suite fails on the code being audited, regardless of how the failure is explained. nextest cancelled after these 3, so 1549/3152 tests (49%) never ran and the coverage gate never completed either.
2. The PR body is still the push-early stub: "DRAFT -- WIP, do not merge, gate round not yet started ... Will update this body with the full change description before requesting review." It was never updated. This PR is explicitly not asking to be merged yet, by its own author's word.
3. No stated evidence of an end-to-end run (SS2.6). Nothing in the PR says what was and was not reachable on a real host (the dispatch notes the dev machine is NAT'd with no outbound IPv6) -- the evidence bar ("a host reaching >=2 independent classes and reporting a non-null established reflexive_addr") is unaddressed.

What I verified by READING (not by running)

Traced dig-stun v0.1.1's establish.rs/scope.rs (the pinned version, confirmed against Cargo.lock) against the CI failure output and the diff in net.rs/peer.rs/lib.rs/advertise.rs. This is sufficient to explain the 3 failures with high confidence (root cause detailed in the interim comment) but I did not compile or re-run the suite myself -- given findings 1-3 above already foreclose merge regardless of that root-cause diagnosis, spending the ~10+ minutes to do so would not change the verdict.

Ranked findings

F1 -- BLOCKING (readiness, not a live custody defect). RED required check + draft/stub PR body + no e2e evidence. Fix: land a green Test + coverage run, replace the stub PR body with the real description + evidence of what was measured on a real host, then re-request this gate.

F2 -- Correctness, should be fixed before merge, NOT a fund-safety issue. effective_urls() (crates/dig-node-service/src/mirror/advertise.rs, established_addresses()-empty branch) collapses FamilyVerdict::NotGlobal and FamilyVerdict::Insufficient/NoReadings into the same AdvertiseState::Uncorroborated label whenever reflexive is non-empty, when the two have documented, different operator remedies (NoPublicAddress = "this address will never work" vs Uncorroborated = "get a second source"). Both states still yield empty urls -- no wrong address is ever published -- so this is a diagnostic-accuracy defect, not an exploitable one. Recommend: match on established()'s FamilyVerdict directly rather than re-deriving "which kind of nothing" from reflexive.is_empty().

F3 -- Named, not gating: an available availability/griefing trade-off that should be a recorded decision, not an accidental byproduct of a stale test. Per-family unanimity (correctly implemented) means one dissenting/degenerate reading in a family can permanently deny establishment for that whole family even when a separate, properly 2-class-agreed address also exists in the same reading set -- confirmed by the third failing test, which still asserts the OLD (weaker, per-address) behaviour. This is the intended fail-closed direction per SPEC.md's own wording and I read it as SAFE for custody (never publishes a wrong address) -- but it is presently unresolved: is a single misbehaving/misconfigured source (bad relay, bad operator STUN entry, or a future dishonest peer under #3199) intended to be able to indefinitely block a legitimate node's establishment? Recommend deciding this explicitly and updating the stale test/SPEC.md to match, rather than leaving a red test as the only signal that the behaviour changed.

F4 -- Cosmetic, not gating. Reflexive::source's doc comment (advertise.rs ~line 225-229) still describes the RETIRED inequality-only corroboration rule ("Compared only for INEQUALITY... corroborate exactly when their sources differ") rather than the new unanimity+class+scope rule it now feeds. Update in the same pass as F2/F3.

Confirmed clean (see interim comment for detail)

No rival implementation of the agreement primitive; class grammar correctly renders distinct classes per host (stun.l.google.com != stun.cloudflare.com); reflexive_addr publishes raw readings only and never masquerades an unestablished reading as established; first-answer-wins correctly survives, unchanged, for the hole-punch tier (peer.rs:2708, peer.rs:3142); the public STUN tier is correctly conditional on class sufficiency (confirmed by 2 passing new tests); no port-equality check; no host/cloud-range special-casing; version bump is patch (0.254.84 -> 0.254.85) as directed, and the commit already carries the Conventional-Commit ! breaking marker (fix(peer)!: ...), which is warranted (the PeerStatus public signature and the reflexive_addr wire shape both changed) and needs no further action.

Bottom line for the orchestrator

Do not merge as-is. F1 is the actual blocker (red CI + draft body + no evidence) and is mechanical to clear. F2-F4 should ride in the same PR before it's marked ready. Once green, the PR body is filled in with real evidence, and F2 is fixed (F3 is a judgment call to record, not necessarily change), this is a re-gate against the new head, not a full re-audit -- the core dig-stun delegation and the gather/wiring in net.rs/peer.rs/lib.rs do not need to be re-read from scratch unless they change.

[MichaelTaylor3d]

Responding to the CHANGES-REQUIRED verdict (audited head ddb5af5a). Three commits pushed since,
current head 96179ee8. Status of each finding:

F1 (red Test + coverage) — fixed, c4b1bc4c. Root cause matches your interim comment's
diagnosis on part (a): established_addresses() folds routability into agreement, and
effective_urls's old empty/non-empty split couldn't tell "nothing agreed" from "agreed perfectly
on a non-public address." Added PublicAddress::any_family_not_global(). The third test
(a_this_machine_reflexive_address_is_never_derived_into_a_coin) needed its FIXTURE reshaped, not
only the state fixed — its old shape put three conflicting IPv4 addresses in one family, which the
new unanimous rule correctly reads as disagreement (refuses all four, not "filter the bad ones");
I isolated the one intentionally-bad reading into its own family so the fixture states a reachable
property again. First attempt used ::1, which folds to 0.0.0.1 under dig-stun's
fold-before-bucket discipline and lands in the SAME family as the good address — settled on
fe80::1 (link-local, does not fold). cargo test -p dig-node-service --lib: 838 passed, 0 failed.

F2 (NotGlobal→Uncorroborated mislabel) — fixed, same commit as F1 (same root cause, same fix).

F3 (design question) — answered, measured against dig-stun's own SPEC.md, not guessed.
The discard is dig-stun's own documented behaviour: SPEC.md §7.3 orders unanimity as step 3 and
global-unicast scope as step 5, so a degenerate reading is never given the benefit of "obviously
broken, so it doesn't count as dissent" — dig-stun's own words: "a node behind a multi-egress NAT,
a misconfigured relay, and a lying peer all look the same from here, and in every one of those
cases advertising is wrong." This is NOT an artifact of how dig-node feeds readings in — dig-node
passes raw IPs through with no pre-filtering, and the step ORDER is entirely dig-stun's. Recorded
in both places you asked for (c4b1bc4c + 96179ee8): PublicAddress::established's doc comment
and SPEC.md §25.10, each stating the availability/griefing cost and why a majority-vote weakening
would be worse. Behaviour unchanged, as you said not to weaken it.

F4 (stale Reflexive::source doc) — fixed, 96179ee8.

PR body replaced with the full description (including this response) — no longer the push-early
stub. §2.6 evidence bar stated plainly: this dev machine is NAT'd with no outbound IPv6, so a live
host reaching ≥2 independent classes was not reachable here; what I verified instead (real fake
UDP STUN servers on loopback for the gather/skip logic, the real dig_stun::establish crate for
agreement, not reimplemented) is in the PR body's evidence section.

Still draft pending your re-gate. cargo fmt --all -- --check and
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings both clean at
96179ee8; full-workspace cargo test in progress, will post the count once it completes.

[MichaelTaylor3d]

Full-workspace evidence added to the PR body: cargo test --workspace --all-targets --all-features --locked --jobs 4 -- exit 0, 3142 tests passed across 55 binaries, 0 failed. detect_changes compare-to-main: 53 changed symbols / 10 files / 3 affected processes, all within the intended blast radius. Current head 96179ee8.

[MichaelTaylor3d]

Real CI confirmed (not just local runs): check-merge-preconditions.sh --allow-draft at head 96179ee8 -- all 6 required checks SUCCESS (Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage, Release-script tests), mergeStateStatus=CLEAN, 0 unresolved threads. 'Test + coverage' (the check the gate found red) is now green. Only draft=true keeps it from exit 0 mergeable, which is expected -- staying draft pending your re-gate.

[MichaelTaylor3d]

IN PROGRESS -- not the verdict. Security re-gate of PR #571, head 96179ee8ed68e2d66e640daf203e2854ab631170.

Item 1 verified: the three test fixes are for the RIGHT reason, and my own prior hypothesis (bare-label grammar) was WRONG about the root cause

Diffed ddb5af5a..c4b1bc4c (the fix commit) directly. All three fixes were IP-range fixes, not label-grammar fixes:

1. network_info.rs::two_agreeing_classes_render_as_established -- both source labels were already class-grammar (relay:relay.example / public:stun.example) at the prior gated head. The only thing wrong was the test address: 203.0.113.7 is TEST-NET-3 (RFC 5737 documentation range), which dig_stun::establish correctly refuses as NotGlobal before this PR's own routability distinction existed to explain it. Fixed by swapping to a genuinely global-unicast address (93.184.216.34). This is a real fix -- the old test could never have exercised the ESTABLISHED branch it claimed to test.

2. mirror/advertise.rs::a_dissenting_third_reading_blocks_establishment_even_though_two_others_agree -- AGREED/DISSENT were both TEST-NET-3 addresses (203.0.113.10/.99); swapped to real global-unicast (93.184.216.34 / 1.1.1.1) so the test fails for the intended reason (majority-without-unanimity should NOT establish) rather than being incidentally caught by the routability gate first. Labels ("relay", "stun.example") stayed bare -- correctly so, since this test only needs source INEQUALITY, not class-grammar (confirmed against Reflexive::source's doc: compared only for inequality; a bare label just can't reach the escalated 3-class floor, unrelated to this test).

3. mirror/advertise.rs::a_this_machine_reflexive_address_is_never_derived_into_a_coin -- old fixture put THREE distinct bad IPv4 addresses (127.0.0.1, ::1-folds-to-0.0.0.1, 169.254.10.4) beside PUBLIC_V4, all in the same family from the same 2 sources. Under the new dig_stun::establish model that is 4 distinct IPs claimed for one family == FamilyVerdict::Disagreement, refusing ALL FOUR (good one included) -- a shape that can no longer occur bug-free, so the old fixture was testing an impossible case. New fixture uses ONE bad address (fe80::1, link-local, genuinely IPv6 so it doesn't fold into the IPv4 family and manufacture a collision) beside PUBLIC_V4 in the other family. This is a narrower but still-valid regression: it proves a this-machine reading in its own family is refused while a genuinely-agreed public family in the OTHER family survives. The breadth reduction (no longer separately testing loopback vs link-local vs private) is acceptable because that enumeration is dig_stun's own classification concern (Scope), already covered by that crate's tests -- not re-litigated here.

None of the three fixes weakens an assertion to force green; each is accompanied by a doc comment citing the specific mechanism (FamilyVerdict::Disagreement, the ::1 IPv4-compatible fold, dig_stun SPEC step ordering) rather than merely changing a literal.

Suite ran to completion, no filtering. Read the actual CI log for job 101378377967 (Test + coverage): Summary [753.466s] 3152 tests run: 3152 passed (12 slow), 4 skipped. This is the full count (matches the 3152 the prior gate saw cancel mid-run at 1549), genuinely green, not a 0-matched filter.

Still verifying: item 2 (effective_urls mislabel exhaustiveness), item 3 (the degenerate-reading design answer), PR body honesty, and whitespace-collapse check. Posting this now per the post-as-you-go rule.

[MichaelTaylor3d]

Security re-gate verdict: PASS

Head audited: 96179ee (resolved myself via gh pr view 571 --json headRefOid, matches dispatch). Scoped re-gate against the prior gates ddb5af5.

1. The three test fixes are for the right reason -- CONFIRMED, and my own prior "bare label" hypothesis was wrong

Already posted as an interim finding (see comment above). Diffed ddb5af5..c4b1bc4 directly: all three fixes are IP-range fixes (swapping RFC 5737 documentation-range addresses that dig_stun's own routability gate correctly refuses, for genuinely global-unicast test addresses), not label-grammar fixes. I additionally verified this against dig_stun::establishs actual source (verdict_for_family, fetched from the v0.1.1 tag): class-counting is done on the raw source string, deduped -- it does not require SourceClass::parse to succeed. A bare label like "relay" and a different bare label "stun.example" are two distinct strings and count as 2 independent classes just fine; an unparseable string only ever fails to reach the escalated 3-class peer:* floor (peer_only computation), which none of the three fixed tests exercise. So the bare labels the two mirror/advertise.rs tests kept were never wrong, and the fix commits own root-cause account (a real production bug in effective_urls's empty/non-empty split, not a fixture-label problem) is the accurate one.

Suite genuinely ran to completion: CI log for job 101378377967 (Test + coverage) shows Summary [753.466s] 3152 tests run: 3152 passed (12 slow), 4 skipped -- the full count, not a truncated or zero-matched filter.

2. effective_urls() state mapping -- CONFIRMED correct and exhaustive

Read dig_stun::establishs actual FamilyVerdict enum (5 variants: NoReadings, Disagreement, Insufficient, NotGlobal, Established) and verdict_for_familys body. The new any_family_not_global() correctly isolates exactly the one variant (NotGlobal) that means "a family agreed but the address itself is not public" -- mapped to NoPublicAddress. Every other empty-agreed cause (NoReadings, Disagreement, Insufficient) correctly falls through to Uncorroborated, which is the semantically right bucket for all three (nothing reported, sources disagree, or too few independent classes -- all "need a second/better source," never "the reading itself is broken"). dig-apps two different remedies (already shipped at 15.1.0) get the right signal in every case I enumerated.

3. The degenerate-reading design question -- the lane LEFT THE BEHAVIOUR AS-IS and documented it; I agree with that call, verified independently against dig-stuns actual source, not just its SPEC prose

Fetched dig-stun v0.1.1's establish.rs and SPEC.md section 7.3 directly (not trusting the PR's paraphrase). Confirmed:

• verdict_for_family performs the disagreement check (step 3: dedup distinct IPs, more than 1 means Disagreement) strictly BEFORE the class-count check (step 4) and the global-unicast scope check (step 5, NotGlobal). A degenerate (loopback/private/link-local) reading that disagrees with an otherwise-agreed public IP is indistinguishable, at the disagreement step, from a lying or misconfigured source -- it is ordinary dissent and discards the whole family, exactly as the PR states.
• This is entirely dig-stuns own behaviour. PublicAddress::established() (dig-nodes only caller) passes self.reflexive straight through with zero pre-filtering -- confirmed by reading the actual function body at 96179ee:crates/dig-node-service/src/mirror/advertise.rs. dig-node introduces no local filtering, so there is no "excluded degenerate reading" logic to interrogate for the attacker-induced-exclusion failure mode the brief asked me to hunt for -- that logic does not exist. The wrong-direction bypass (an attacker disguising their dissenting reading as degenerate to get it ignored rather than have it block) cannot occur here because nothing is ever excluded from the disagreement check based on its own scope.
• Checked whether dig-stun 0.2.0 (published same day, 20:00 UTC, before this PR's last commit at 21:24 UTC) changed any of this: gh api compare v0.1.1...v0.2.0 shows the diff is purely additive (a new credential module for RFC 5389 short-term-credential signing) -- establish.rs is untouched. The analysis holds for both the pinned version (0.1.1) and current latest.
• The documentation added (SPEC.md section 25.10 plus the established() doc comment) accurately states the trade-off, matches dig-stuns own SPEC section 7.3/7.4 wording, and correctly attributes the availability/griefing cost to the crate rather than re-deriving or silently accepting it. No test argues with it -- the reshaped a_this_machine_reflexive_address_is_never_derived_into_a_coin fixtures own new doc comment explicitly explains why three same-family bad addresses would now trigger Disagreement (refusing the good one too), which is consistent with, not contradicting, the documented behaviour.
• I agree with the decision. Weakening unanimity to tolerate a filtered "degenerate" dissent class would be the wrong direction for a value that stakes collateral (section 7.5's asymmetry: wrong Established costs collateral permanently, wrong non-establishment costs one epochs rewards and is visible in dign network-info), and todays actual sources (operator-configured relay plus curated public STUN hosts) are not yet attacker-reachable -- the griefing scenario the doc names is explicitly conditioned on the peer tier of dig_ecosystem#3199, which has not landed. Correctly scoped, not overclaimed.

Also confirmed

• PR bodys evidence section is honest about the NAT/no-outbound-IPv6 limitation on this machine, states plainly what could and could not be reached, and does not imply an end-to-end run it didnt perform.
• No new operator-facing string was touched in the ddb5af5..96179ee delta (only doc comments, one new pub fn, and SPEC.md prose changed) -- nothing to whitespace-check that wasnt already covered by the prior gates pass on the earlier commit.
• Zero open review threads on the PR (checked via GraphQL reviewThreads).

Non-gating note (process, not security)

dig-stun 0.2.0 was published at 2026-09-05T20:00:20Z, roughly 84 minutes before this PR's last commit (96179ee at 21:24:17Z), and Cargo.toml/Cargo.lock still pin 0.1.1. Per CLAUDE.md section 2.4b (bump touched crates' dig-* deps to latest in the same PR), this should have picked up 0.2.0. Confirmed via gh api compare v0.1.1...v0.2.0 that this is NOT a security concern -- the 0.2.0 diff is purely additive (a new unrelated credential module), establish.rs is byte-identical between the two versions, so nothing in this audits analysis is affected. Flagging for a follow-up bump, not gating the merge on it.

What I verified by RUNNING vs. reading

• Ran: gh pr view/gh api for head SHA, checks, PR body, review threads; gh run view --job --log for the actual CI test-count summary; gh api repos/.../compare for the dig-stun 0.1.1 to 0.2.0 diff; gh api repos/.../contents to fetch dig-stuns actual establish.rs/SPEC.md source at the pinned tag.
• Read (not run): the PR diff via git show/git diff against fetched refs in the primary checkout (read-only; no working-tree files were touched -- I fetched refs and used git show SHA:path throughout, verified with git status --porcelain before and after, no local mutation).

No security defect found in this scoped delta. PASS.