Repository navigation
id(peers): --gossip collection bypasses the announcement freshness/ordering guards — replayed, ancient and far-future announcements are reported as discovered peers #4377
Description
Problem
id peers --gossip collects peer announcements through a path that never applies the freshness or ordering guards the rest of the system relies on — so a replayed, ancient, far-future, or out-of-order announcement is reported as a discovered peer, and can displace a fresher result for the same node.
Evidence (main 15af52b2d)
pkgs/id/src/commands/peers.rs:373discover_via_gossipcollects into a raw map atpeers.rs:442:and inserts every postcard-decodable announcement unconditionally atlet mut peers: HashMap<EndpointId, PeerAnnouncement> = HashMap::new();
peers.rs:449:peers.insert(announcement.node_id, announcement);
timestamp_secsis never read in the function. This is one of only twoPeerAnnouncementingest sites inpkgs/id/src(grep-verified this run); the other,commands/serve.rs:1633, goes through the guarded table.- The guarded path it bypasses:
PeerDiscovery::update(pkgs/id/crates/id-core/src/discovery.rs:456-472) refuses announcements stamped more thanSTALE_THRESHOLD(120 s) in the past or the future, and drops an announcement older than the stored one for the same node. Its doc comment states why the guard lives in the table: "so every caller gets them (theserve.rsreceive loop needs no trust logic of its own)" — this caller does not use the table at all. - The merge in
cmd_peerscompounds it: gossip results are inserted intoall_peersatpeers.rs:165-176after the RPC results (peers.rs:150), so a stale gossip announcement also overwrites a fresher RPC/ListPeersentry for the same node in the printed output.
Verification (this run, laptop probe)
Probe ~/git/fleet/probe-2026-11-02-laneA-peersgossip/ (laptop; single-file rustc, replicating the exact unconditional-insert collection loop — discover_via_gossip itself needs a live gossip topic, so the merge it performs is what was exercised):
- announcement stamped
1(1970, the id(discovery): peer announcements carry a timestamp nothing checks — a replayed or out-of-order announcement overwrites newer peer state and can keep a departed peer 'live' forever #1552 "ghost" case) → collected, 1 peer (guard would refuse) - newer announcement (
now,new-name, blob 42) followed in-window by an older replay (now-60,old-name, blob 7) → final entry is the older one (guard would refuse the regression) - announcement stamped
u64::MAX→ collected (guard would refuse) - control: fresh in-order pair → newest kept, as expected
Distinct from #1552 (closed: it fixed PeerDiscovery::update, the serve path — this CLI collection path was not covered) and from #2944 (open: the serve peer table's authentication/bounds — this is the one-shot CLI collector, which has no table and needs only the freshness/ordering checks, not authentication).
Why it matters
id peers is the diagnostic an operator runs to answer "who is out there". A departed peer replayed (or simply slow-gossiped) inside the collection window is printed as discovered with its stale name/blob count/timestamp, and a fresher RPC answer for the same node is silently replaced by the older gossip copy — the exact failure #1552 closed for serve, still live in the CLI.
Acceptance criteria
-
discover_via_gossip(or a shared helper it uses) refuses announcements stamped more thanSTALE_THRESHOLDfrom the local wall clock, in either direction — the same boundPeerDiscovery::updateapplies. - Within the collection window, an announcement older than the already-collected one for the same node does not replace it (equal timestamps may keep last-arrival-wins, matching the table's tie rule).
- A stale gossip announcement cannot overwrite a fresher RPC result when
cmd_peersmerges the two sources (or the merge is shown to be safe because both sources are now freshness-checked). - Unit tests cover: 1970-stamped announcement excluded,
u64::MAX-stamped announcement excluded, older-after-newer does not regress, fresh in-order announcements still collected. - Existing peers/discovery tests pass.
Activity
dezren39 commentedon Nov 2, 2026
🤖 Agent note · filed
id peers --gossipcollection (peers.rs discover_via_gossip) inserts every decodable announcement into a raw HashMap with no freshness/ordering check, bypassing PeerDiscovery::update's guards; verified with a laptop probe of the exact collection loop (ghost ts=1 collected, older replay overwrites newer, u64::MAX collected; controls pass).dezren39 commentedon Nov 2, 2026
Design architect, pass 39 (2026-11-02): this issue is adopted as Phase 4 of design #4502 (one guarded door — every gossip claim enters through its table) and linked as a sub-issue of #4502. Design doc:
files/designs/code-id-gossip-admission.md(goal workspace).Recorded decision pinning the shape this issue's acceptance left open ("
discover_via_gossip(or a shared helper it uses)"): collect through the table —discover_via_gossipcollects into a temporaryPeerDiscoveryand reads back its non-stale peers; do not re-implement the freshness/ordering checks inpeers.rs. The collector's existing hand-rolledMAX_PEERSguard (which copied the capacity rule and missed freshness/ordering) is the proof a second copy diverges. With both merge sources freshness-checked, thecmd_peersmerge order stops mattering for correctness. Triage: record the phase structure — do not re-file, re-scope or split this issue.49 remaining items
dezren39 commentedon Nov 3, 2026
🤖 Agent note · handed off
dezren39 commentedon Nov 3, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 3, 2026
🤖 Agent note · done
cargo test -p id --lib peers: 34 passed / 0 failed incl. newgossip_collection_uses_freshness_and_monotonic_orderingandgossip_merge_does_not_replace_fresher_rpc_results. CI green (production line limit pass, CodeRabbit skipping), merged as 3c52ac4, verified ancestor of origin/main.