* fix(NodeDB): require a full 32-byte key when demoting to the warm tier
meshtastic_User.public_key is a wire `bytes` field with max_size 32, so any
size in 0..32 decodes off the air, and nothing validates it on ingress:
NodeInfoModule hands the decoded User straight to NodeDB::updateUser, whose
PKI gates are all `== 32` and so fall through for a partial key, and
TypeConversions::CopyUserToNodeInfoLite then stores it with the short size.
demoteOldestHotNodesToWarm() admitted that partial key into the warm tier on
a `size > 0` gate. WarmNodeEntry has no length field - it distinguishes "has
a key" from "no key" purely by all-zero - so N real bytes plus 32-N zeros
become indistinguishable from a genuine key. copyPublicKeyAuthoritative()
then hands that fabricated key back with size = 32 and reports it
AUTHORITATIVE, and re-admission writes size = 32 into the hot store. From
then on updateUser's key pin permanently rejects the node's real NodeInfo,
and DMs to it are encrypted to a key nobody holds.
Require a full 32-byte key, so a partial one is absorbed as "no key"
(nullptr) rather than as a truncated one. WarmNodeStore::place() already
treats a null key as keyless and clears the slot's stale key when
repurposing it. This aligns the site with its two siblings, which both
already gate on `size == 32` (the purge path in cleanupMeshDB and the
runtime eviction in getOrCreateMeshNode).
The ingress gap - updateUser accepting a 1..31-byte key at all - is a
separate, larger change and is left for its own review.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(NodeDB): shorten warm-demotion comment to two lines
Repo guideline (AGENTS.md): keep code comments to one or two lines. Retains the
non-obvious invariant - warm entries have no key length field - and drops the
restated detail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(NodeDB): cover short-key demotion into the warm tier
A warm record stores 32 raw key bytes with no length field, so a partial
hot-store key is indistinguishable from a real one once demoted. The
public_key.size == 32 gate in demoteOldestHotNodesToWarm() is what keeps a
truncated key from being laundered into a full-looking warm key, but nothing
exercised it.
test_migration_dropsShortKeyOnDemotion overflows the hot store with one node
carrying a 31-byte key and asserts it lands as a keyless placeholder while a
genuine 32-byte key still survives. push() grows a keySize parameter to seed
the partial key, and clearWarm() gives the test an empty warm tier, which it
needs because the warm store outlives setUp() and a prior run's warm.dat.
Verified to discriminate: with the size gate reverted to size > 0 the new
test fails on "a 31-byte key must not be demoted as if it were a full key",
and passes again once restored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(NodeDB): assert the keyless placeholder carries last_heard
The test only proved a warm metadata row survived the demotion, not that the
placeholder does the job the nullptr is there for, which is preserving
last_heard when the key is dropped.
Asserting the value needed the seeds fixing first. Warm entries pack role,
protected category and the xeddsa flag into the low 7 bits of last_heard
(WARM_TIME_MASK is 0xFFFFFF80), so warm time has 128 second granularity and the
old seeds of 1, 2, 3 all quantised to 0. They are now multiples of 128, which
keeps the demotion ordering identical and makes the values survive the round
trip. Real last_heard is epoch seconds, so this is closer to production than
the old counter was.
Reads the entry through WarmNodeStore::take() rather than getOrCreateMeshNode(),
which does not restore last_heard from the warm tier and would have been
asserting a path that does not exist.
Reported by CodeRabbit on #11431.
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
90a6dec3f3