fix(kerykeion): bound node and link cardinality, stop treating unknown routing codes as ACKs

[forkwright]

Closes #204
Closes #208
Closes #209

Provenance — read this first

Salvaged from a crashed session; nothing here had been gated or reviewed when this PR was
opened. The orchestrating machine died mid-wave with this work uncommitted on disk. The
test-fixture commit had reached the remote; the implementation had not. An independent
adversarial review has since run against the diff; its findings are fixed and folded in below.

The unit was assigned three issues and delivers three

#209 (AES-CTR nonce reuse) IS addressed by this diff. The PR as first opened claimed the
opposite — an error in the body, not a scope decision: the diff already touched packet_id.rs
(new, a complete PacketIdCounter monotonic-sequence type) and message.rs
(MessageBuilder::build's sole production signature, message.rs:177, draws packet_id from
that counter instead of OsRng.next_u32() per packet — the vulnerable random draw is fully
deleted, not left as an alternate path). sequential_builds_never_share_a_packet_id
(message.rs) demonstrates non-repeating ids across sequential builds, which is #209's literal
Done-when.

Two things worth a reviewer's attention rather than treated as closed by inference:

• PacketIdCounter's restart-safety half is documented but unenforced (see the WARNING on its
  doc, packet_id.rs:18-29) — nothing stops a future caller from calling seed_random() on every
  process start instead of persisting current() / calling resume(), which reproduces the
  original defect.
• There are currently zero production callers of MessageBuilder::build or PacketIdCounter
  anywhere in the repo (crates/akroasis, crates/akroasis-server both grep-empty), so this fix
  protects no live send path yet either way.

#204 — unbounded node and topology cardinality. The from field on an inbound frame is
unauthenticated, so an over-the-air peer could announce unlimited distinct identities and exhaust
memory. Node table and topology graph are now capped with least-recently-heard eviction.

The eviction is where this could have gone wrong, and the branch takes it seriously: update_link
is protected from evicting the two endpoints it is in the middle of adding — a bounded table whose
eviction discards the entry being inserted is its own denial of service, and
update_link_never_evicts_its_own_two_new_endpoints pins that. load_from_bytes's link-restore
loop originally missed the same protection — see "Fixed after adversarial review" below.

#208 — fail-open routing. An unrecognised routing error code fell back to Error::None, which
the caller reads as delivered. Unknown now means not-delivered. This is the fail-open shape: the
default for something you do not understand was success.

Fixed after adversarial review

• MeshTopology::load_from_bytes reproduced the exact self-eviction hazard update_link was
  fixed against. add_node's eviction capability (added for Bound node-table and topology cardinality to stop OTA memory-exhaustion DoS #204) makes any unprotected pair of
  add_node calls followed by add_edge unsafe once the graph is at cap. update_link was fixed
  with mutual add_node_protecting, but load_from_bytes's link-restore loop still called plain
  add_node for both endpoints (topology.rs:497-498 before this fix). A snapshot whose links
  fill the graph to MAX_LIVE_NODES with zero None-freshness entries, followed by one more link
  between two brand-new node ids, deterministically evicted the just-inserted from before its
  edge was created. In this repo's petgraph version that manifests as silent corruption rather
  than a panic — the freed graph slot gets reused by the very next insertion (to), so from_idx
  and to_idx collapse onto the same slot, add_edge succeeds, and the result is a bogus
  self-loop on to with from's identity dropped from the topology entirely. Fixed by routing
  both endpoints through add_node_protecting, same as update_link. Negative fixture:
  load_from_bytes_never_evicts_its_own_two_new_endpoints (topology_tests.rs) — watched red
  against the pre-fix code (CI run
  32044433789:
  thread '...load_from_bytes_never_evicts_its_own_two_new_endpoints' panicked at crates/kerykeion/src/topology_tests.rs:514:5: the new link's own source must survive its own restore) and green on this PR's head (CI run
  32043867713: 1085 tests run: 1085 passed, 0 skipped, including this test at 453/1085).
• Two pre-existing clippy errors, unrelated to the finding above, blocked the whole gate from
  running at all — the PR's own commits admit no gate, no review, no local build, and neither
  had ever been caught:
  • #[expect(clippy::unwrap_used)] attached directly to an assert_eq! macro-invocation
    statement — a position rustc silently ignores (unused_attributes, promoted to a hard error
    under this repo's -D warnings) — so cargo nextest run failed to compile
    kerykeion's test target, taking every test in the crate down with it, including the
    Bound node-table and topology cardinality to stop OTA memory-exhaustion DoS #204/Do not classify unknown routing error codes as delivery ACKs (fail-open) #208/Use a monotonic packet-id counter to avoid AES-CTR nonce reuse #209 tests above. Fixed by binding the unwrap to a name first, matching the working
    pattern already used one test up in the same file (packet_id.rs:142-144).
  • node_db.rs's evict_stalest used .map(f).unwrap_or(a) on an Option
    (clippy::map_unwrap_or); PacketIdCounter::next(&mut self) -> Result<u32, Error>'s name
    collides with Iterator::next (clippy::should_implement_trait) despite implementing no such
    trait and returning Result, not Option. Fixed with map_or(a, f) and a rename to
    next_id across its one call site and its own tests; no behavior change either way.

What a reviewer should attack

• Does the cap hold against the adversary who chose the input? An attacker who can trigger
  eviction can also choose what gets evicted. Can a hostile peer evict a legitimate neighbour by
  flooding, and does that cost anything real?
• Is MAX_LIVE_NODES reachable in normal operation? A bound set too low is a functional
  regression; too high and it does not bound anything on a constrained device.
• Do the tests exercise the defect or its neighbourhood? For each: what would have to be true
  for it to pass while the bug is present?
• Is Error::None the only fail-open default in that path, or does the same shape appear at
  another match arm?
• Does PacketIdCounter's restart-safety gap need enforcing before a real caller exists, or is
  it acceptable to leave as a documented caller obligation until one does?

Verification status

There is no build box in the fleet and no Gate-Passed trailer is obtainable from anyone right
now — do not read the absence of one as a gap specific to this PR. CI (hosted GitHub Actions) is
the verifier of record, and this PR's head has a genuine green run: fmt, check, clippy and all
1085 nextest cases pass
(run 32043867713). The
load_from_bytes fix was independently watched fail without the change and pass with it — see
"Fixed after adversarial review" above for both CI runs and the literal panic text.

Note for whoever reviews CI here: codeload.github.com was intermittently returning 429/502/503
to the hosted runners while this PR was being worked (unrelated Actions-infrastructure
congestion, not this repo) — a run failing at the "Set up job" step with no steps beyond it is
that, not a code regression. Re-run rather than debug the diff.

[forkwright]

T0 review of the crypto half. The implementation is right, and one scope caveat belongs on the record before #209 closes.

What I verified in the head tree, not the body:

• packet_id.rs — next_id() returns Result and refuses to wrap past u32::MAX, returning PacketIdSpaceExhausted. That is the correct call: wrapping would silently reissue ids, which is the nonce-reuse defect itself, so the fix declines to reintroduce its own bug class in its exhaustion path. resume(last_used) carries the sequence across restarts, and seed_random() is documented as valid only where no persisted value exists.
• message.rs:177 — MessageBuilder::build draws from packet_ids.next(). The OsRng.next_u32() per-packet draw is deleted, not left as an alternate path. One implementation, not two.
• Tests cover strictly-increasing, resume-continues-past-persisted (the restart repro named in Use a monotonic packet-id counter to avoid AES-CTR nonce reuse #209), and refusal to wrap.

The caveat, and the reason I am writing it down rather than blocking on it.

PacketIdCounter and MessageBuilder appear outside their own modules only in doc comments and pub use re-exports. No production call site in this repo constructs a MessageBuilder and calls build(). That is legitimate for a library surface — the consumer is external — but it means the guarantee is available, not in force: no packet this repo currently sends is protected by it.

So #209's Done-when is genuinely met, and nobody should read the closure as "nonce reuse is now impossible in production." When a send path is wired, the thing to check is that it calls resume(persisted) and not seed_random() on every start — the type documents that requirement but cannot enforce it, since only the caller knows whether a persisted value exists. That is the one place this design can still be defeated by a well-meaning consumer.

On the scope correction itself: the original body stated #209 was untouched while the diff fixed it. Worth naming why that mattered beyond bookkeeping — a crypto change shipping under a body that says "don't look here" defeats an independent reviewer who trusts stated scope, which is exactly how this nearly went unexamined. The diff is the claim; the body is a hypothesis about it.