Commit Graph
150 Commits
Author SHA1 Message Date
21e3a583bd Yaml check for Meshtasticd (#11224)
* feat(portduino): add `meshtasticd --check` config validator

Users hand-writing files in /etc/meshtasticd/config.d/ get no feedback when a
key is misplaced, misspelled or duplicated: meshtasticd silently ignores what
it does not read, so a broken config looks identical to a working one.

Add a --check mode that loads the configuration exactly as startup does, then
reports what it found and exits:

- Duplicate keys, via the yaml-cpp Parser/EventHandler stream. The Node API
  cannot see them because the map is already collapsed by the time it exists,
  and yaml-cpp keeps the FIRST occurrence, so a later override is discarded.
- Unknown or misnested keys, against a schema mirroring what loadConfig()
  reads, with a hint naming the section a stray key actually belongs to.
- rfswitch_table validation: unrecognised pins, mode rows whose length does not
  match the pin list, values that are not HIGH/LOW, and unknown modes.
- Cross-file overlap: every .yaml in the config directory merges into one
  portduino_config, so the file loaded LAST wins, the opposite of the
  within-file rule. Those files are read in filesystem order, not alphabetical.
- A warning when more than one file defines a Lora section: spidev, spiSpeed,
  gpiochip, DIO2_AS_RF_SWITCH, DIO3_TCXO_VOLTAGE and USB_PID/VID/Serialnum are
  assigned unconditionally with a default every time one is seen, so any of
  them not repeated in the last file loaded is silently reset.
- The resolved gpiochip/line for each pin, since a line that exists on the
  wrong chip is claimed successfully and then silently does nothing.

Exits non-zero when errors were found so it can also gate CI over
bin/config.d/**, keeping one implementation rather than a second schema.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(portduino): flag pins that resolve to -1 in --check

A pin key whose value will not convert to a number falls back to RADIOLIB_NC
(-1) while still being marked enabled, and initGPIOPin() then trips an
assertion inside LinuxGPIOPin rather than failing cleanly. YAML indentation
makes this easy to hit by accident: a stray line under "CS: 8" folds into the
value as a multi-line scalar, so the file parses, the daemon crashes with a
stack trace from a library file, and --check reported "Configuration looks
good" while printing "pin -1" two lines above.

Report it as an error naming the likely cause instead.

Also correct a comment claiming unparseable config.d files are skipped
silently. They are not: loadConfig() prints "*** Exception ..." with the line
and column. It is the discarded return value, not the diagnostic, that makes
the file's absence from the merged config easy to miss.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(portduino): cover `meshtasticd --check` with fixtures and a fuzz suite

Adds the tests the config validator was missing, and the checks and fixes that
writing them turned up. The theme throughout is configuration that the YAML
parser accepts but that does not mean what it looks like it means.

Tests
-----

bin/test-config-check.sh - 57 assertions driving a built meshtasticd against
test/fixtures/portduino-config (50 fixtures plus two config.d trees). A shell
test rather than a Unity suite because both behaviours under test are properties
of the process: --check is judged by its exit status and printed report, and the
"a normal run rejects a bad config" path ends in exit() inside portduinoSetup(),
neither of which is reachable from a suite that links one translation unit.
Every fixture carries a comment header naming its planted fault and the expected
finding, so it can be read on its own. Coverage:

  * a clean config for each of the ten radio module families (RF95, sx1262,
    sx1268, LLCC68, sx1280, lr1110, lr1120, lr1121, sim, auto), asserted both
    findings-free and resolving to that module, so a silent fallback to sim
    cannot pass
  * LR11xx rfswitch tables: unrecognised pins, rows longer and shorter than the
    pin count, levels that are not exactly HIGH, a missing pins list, more than
    five pins, a scalar table, unknown MODE_ keys, a MODE_ row stranded one
    level out, and a legal partial table
  * the PA gain table in both accepted shapes, entries outside the uint16 range
    it is stored in, and more than the 22 points that are kept
  * values of the wrong type, split by consequence: the two settings read with
    no fallback stop meshtasticd starting, everything else is silently replaced
    by its default
  * out-of-range and unit mistakes: TCXO voltage written in millivolts, ports
    outside their usable range, an over-long StatusMessage
  * MAC sources: both keys set at once, a malformed address, an interface that
    does not exist
  * structural faults: duplicate keys, non-mapping and unknown sections, a key
    left at the top level, a sequence at the document root, an empty file,
    unreadable pins, unparseable YAML
  * cross-file behaviour over a config.d directory, including the switch tables
    that do not override each other
  * five configs run WITHOUT --check, each of which must still be refused, so
    check mode cannot quietly make the normal path permissive

test/test_fuzz_config - adversarial fuzzing of the checker itself, the "the tool
meant to diagnose your config crashes on it" failure mode. Scope is deliberately
narrow: yaml-cpp does the parsing and is fuzzed upstream, so what is exercised
here is our code above the parse, above all the duplicate-key detector, which is
the one hand-rolled piece and walks the raw parser event stream with its own
stack. Groups: the checked-in fixtures as a seed corpus, 3000 byte mutations of
them (flips, truncation, insertion, splicing, deletion), and structural torture
(nesting to 4096 in flow and block style, duplicate keys at depth, anchors,
aliases and merge keys, 64KB keys, 256KB scalars, multi-document files). A
fourth group of random bytes is present but disabled behind
FUZZ_CONFIG_RANDOM_BYTES: it was half the runtime for the least return, since
uniform noise is rejected on the first token. The contract is crash-freedom and
termination under AddressSanitizer, not any particular finding.

CI runs the shell test in the existing native simulator job; the fuzz suite is
picked up by the existing ^test_fuzz_ area rule. native-suite-count 40 -> 41.
The fixtures are exempt from trunk in .trunk/trunk.yaml, since prettier rejects
the duplicate keys and bad indentation that are the point of them.

Checker fixes found while writing the tests
-------------------------------------------

--check reported a clean exit 0 on configs meshtasticd then refuses to boot, the
worst failure a diagnostic tool can have. Four hard exits inside loadConfig()
killed the report before it printed: an unparseable file, an unknown Lora.Module,
MACAddress and MACAddressSource both set, and HUB75 on a build without it. All
are now reported as findings, and all are still refused on a normal run.

New validation: Lora.Module against the accepted spellings, which are matched
exactly and inconsistently cased, with a suggestion when only case differs; a
per-key value type table covering ~85 keys, tested by asking yaml-cpp to perform
the same conversion loadConfig() will so it cannot drift; the PA gain table;
DIO3_TCXO_VOLTAGE, which is in volts and multiplied by 1000, so the millivolt
value everything else uses silently asks for 1800V; APIPort and Webserver.Port
ranges; MaxNodes; StatusMessage truncation; MAC address and source; and an
unreadable ConfigDirectory.

Also fixes a crash: a ConfigDirectory that cannot be read threw an uncaught
filesystem_error from directory_iterator and aborted meshtasticd with SIGABRT,
taking --check down with it. It now fails cleanly.

Two smaller ones: cppcheck's uselessCallsSubstr on the ancestor walk, which was
failing every check job; and the duplicate-key detector's stack pop, which was
unguarded and relied on yaml-cpp emitting balanced events.

Switch tables are the one place "the file loaded last wins" is false. The loader
only ever writes HIGH and never writes LOW back, so a HIGH from an earlier file
survives a later file that clears it and the radio drives the OR of every table
loaded. Confirmed with --output-yaml. Reported as an error for now; the loader
itself is left alone, as that changes RF behaviour.

* fix(portduino): report CH341 pins as adapter indexes, not gpiochip lines

--check printed "Resolved GPIO lines (what meshtasticd will try to claim)" for
every config, listing a gpiochip and line for each Lora pin and advising they be
confirmed against gpiodetect and gpioinfo. For spidev: ch341 every part of that
is false. portduinoSetup() skips initGPIOPin() for every Lora pin when spidev is
ch341 and hands the raw numbers to Ch341Hal, so nothing is claimed from a
gpiochip -- and on Windows and macOS, where a USB adapter is the only way to
attach a radio, there is no gpiochip, gpiodetect or gpioinfo to check against in
the first place. The checker had no ch341 coverage at all: not one fixture used
it, so the whole USB-SPI path went unexercised.

The summary now splits on the transport. A ch341 device gets its pins listed as
adapter indexes with the gpiod advice dropped, and a gpiochip or line mapping
written alongside it is reported: those are read, stored, and never used.

Also: "RF switch table: not set" read as a gap on an SX126x, where there is
nothing to set. setRfSwitchTable() is only ever called for an LR11xx, so absence
is now "not needed for this module" everywhere else, and "not resolved yet" for
auto, which has no module to judge against.

Fixtures: usb-ch341.yaml (clean, the meshstick shape) and ch341-gpiochip.yaml.

CI fix
------

test-native was RED on "config.d overrides are reported", which wanted 2
warnings and got 1. The fixture's two config.d files name different modules, so
which one wins -- and whether the LR11xx-without-a-switch-table warning fires --
depends on the order the filesystem returns them in. That is the very thing the
fixture exists to demonstrate, so the count is no longer asserted; the report's
own order caveat is asserted instead.

Review fixes
------------

The unreadable-ConfigDirectory diagnostic was the one new print in
PortduinoGlue.cpp not gated behind !configCheck, so it landed ahead of the report
header and broke the clean output the rest of the change is careful to keep.

Docs: rfswitch-valid.yaml carries seven modes, not eight, and empty-file.yaml is
comments-only rather than zero bytes.

* style(portduino): trim --check comment blocks and reconcile suite count

Condense the multi-paragraph comment blocks in the --check validator to the
one-to-two-line convention, and bump test/native-suite-count to 42 for the
test_fuzz_config suite added here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 11:06:24 +00:00
Benjamin FaershteinandGitHub 9a37250438 fix(router): clear failed reliable send retries (#11267)
* fix(router): clear failed reliable send retries

* test(router): cover failed interface enqueue

* fix(router): retain retries after duty cycle limits
2026-07-30 11:00:32 +00:00
TomandGitHub 2024bb8384 Arrival time fix perhaps (#11274)
* Add explicit presence for MeshPacket.rx_time (arrival time)

rx_time is now proto3 optional with a has_rx_time presence bit, matching
the rx_rssi treatment. A node with no GPS and no phone connected yet has
no time source at all, so a bare 0 was indistinguishable from a genuine
1970-01-01 reading; downstream consumers (replay packets, JSON
serialization) now check has_rx_time instead of the value.

* Dedupe rx_time stamping into a shared helper; trim a debug log string

Extract the repeated haveTime/rx_time/has_rx_time stamp logic (5 call
sites across Router.cpp, MeshBeaconModule.cpp, MeshService.cpp) into
Router::computeRxTimeStamp()/stampRxTime(). Also shorten the new RTC.cpp
LOG_DEBUG string. Saves 48 bytes of flash on rak4631 (measured), no
behavior change.

* Fix has_rx_rssi presence carried unconditionally through StoreForward replay

preparePayload() set has_rx_rssi = true unconditionally on replay, regardless
of whether the packet's rx_rssi at store time was a genuine measurement (e.g.
MQTT-relayed packets carry no real RSSI). Store the presence bit alongside
rx_rssi in PacketHistoryStruct and restore it on replay instead.

Flagged by Copilot on #11271 (same root cause the has_rx_time explicit
presence work fixes) but never addressed before that PR merged.

* Trim comment blocks to the repo's 1-2 line guideline

.github/copilot-instructions.md:338 caps code comments at 1-2 lines; several
blocks added across the rx_time explicit-presence work ran well past that.
Also consolidates Time.cpp's file-level doc comment into Time.h, where the
rest of the Time:: API contract already lives.

No behavior change.

* Add rx_time explicit-presence test coverage

- test_meshpacket_serializer: has_rx_time=false fixture plus tests asserting
  JsonSerialize/JsonSerializeEncrypted emit 0 rather than leaking the
  millis() placeholder, alongside the has_rx_time=true baseline.
- test_stream_api: two tests driving a real PhoneAPI handshake (want_config_id
  through STATE_SEND_PACKETS) that simulate a phone time-giving transaction
  arriving before vs. after a queued packet is drained - covering both the
  reconciled and the ships-with-placeholder-absent paths of
  MeshService::reconcilePendingRxTimes().

* Fix three correctness issues flagged in review

- Time.h: drop the reserved-identifier include guard (_MT_TIME_H); pragma
  once already covers it, matching convention elsewhere (e.g. RTC.h).
- Time.cpp: rebase getMillis64()'s wrap accumulator when the test seam
  swaps clock sources, so a real<->injected clock jump isn't miscounted
  as a genuine 32-bit wrap.
- NodeInfoModule: the 12h reply-suppression window is a local dedup
  duration, not a wall-clock reading - switch it to Time::getMillis64()
  so RTC-quality jumps and replayed packets' stale rx_time can't perturb
  it.
- StoreForwardModule: has_rx_time was derived from *current* RTC quality
  at replay time rather than stored at capture time, so a history entry
  saved while time-blind could be misreported as a valid epoch once the
  clock later improved. Persist the presence bit in PacketHistoryStruct
  instead.

* tryfix CI

* post review fixes

* more test fixes
2026-07-29 14:03:25 +00:00
Benjamin FaershteinandGitHub 0fef83d434 Add configurable event mode hop limit (#11275)
* feat: resolve event mode hop limit

* feat: bake event mode hop limit

* fix: honor event mode hop cap in routing

* docs: expose event mode hop limit preference

* fix: enforce event hop defaults across routing

* docs: clarify event hop override behavior

* refactor: simplify event mode hop preference

* fix: cap equal event hop limit
2026-07-29 10:54:48 +00:00
TomandGitHub 2c57a17124 Phantom node fix perhaps (#11271)
* trying to fix phantom nodes

* fix comment spam

* oops - missed one
2026-07-28 18:32:18 +00:00
Ben MeadorsandGitHub a8623a60c5 Stream our own position to the phone/UI while mesh position sharing is opt-in (#11270)
* Position: stream our own position to the phone/UI while mesh sharing is opt-in

Position broadcasts became opt-in in 2.8 (#10929): with every public channel
at position_precision 0, sendOurPosition() finds no eligible channel and
returns without queueing anything, so the connected phone or on-device UI
never sees the node's own GPS fix ("GPS looks dead" on standalone MUI
devices even though the receiver has a lock).

Mirror device telemetry's local delivery: once a minute, when the toPhone
queue is idle, stream our own position to the connected client at full
precision. The packet is handed straight to sendToPhone() and never touches
the mesh, so the per-channel opt-in and the public-channel precision clamp
still govern everything on the air.

Also stop logging "Send pos ... to mesh" before the channel scan has found
an eligible channel; when sharing is disabled everywhere the skip is now
logged explicitly instead of pretending a send happened.

The cadence gate is a pure static (shouldSendPositionToPhone) alongside the
existing broadcast-policy helpers, with unit tests covering the first-send,
cadence, gating, and millis() rollover cases.

* Review: only advance phone cadence on a queued packet; drop the ms==0 sentinel

sendOurPositionToPhone() now reports whether a packet actually reached the
phone queue, and runOnce() stamps the cadence only on success, so a guard or
allocation failure retries on the next tick instead of waiting out a minute.

The never-sent state is a dedicated hasSentPositionToPhone flag rather than
lastPhoneSendMs == 0, so a send stamped exactly at millis() == 0 still holds
the cadence. New regression test covers that case; existing cases updated to
the explicit flag.

* Review: align the rollover test fixture with its documented elapsed times

lastSent now sits exactly 30,000 ms before the uint32 wrap, so the two cases
are precisely 70,000 ms (sends) and 40,000 ms (held) - the previous comments
claimed 70s/20s against actual elapsed values of 70,001/40,001 ms.
2026-07-28 14:17:09 +00:00
Ben MeadorsandGitHub ae16c052d5 Time out an abandoned admin edit transaction (#11254)
begin_edit_settings sets a bool that only the matching commit ever
clears. If the commit never arrives -- the client dropped its link
partway through a bulk config import, or went away entirely -- the
transaction stays open indefinitely, and every later config write from
any client is applied to RAM, acknowledged, and then never saved. The
writes look successful and are visible in get_config, but the node
reverts all of them at the next boot, and only a reboot clears it.

Give the transaction a one minute idle timeout. Each deferred write
restarts the clock, so the window bounds the gap between writes rather
than the length of the edit; a bulk import sends them milliseconds
apart. The next admin message after it lapses retires the transaction,
persisting the segments it had deferred and flushing the warnings it was
holding. The check runs after the auth gates and before the switch, so
every case sees consistent state and the recovery's flash write happens
in the main loop rather than a disconnect callback.

It saves rather than rolls back because there is nothing to roll back
to: each write already took effect in RAM and was acknowledged, so the
transaction only ever deferred the save. That also makes an early expiry
cheap -- the worst case for a client that really was still going is that
its remaining writes are saved individually instead of batched. Closing
on disconnect instead was tempting, but the reporter's captures show iOS
dropping and reconnecting within half a second several times per
session, which would abort imports that currently survive the blip.

Also drops the file-scope hasOpenEditTransaction, shadowed by the class
member at every use site and referenced nowhere else in the tree.

Reported in #11245
2026-07-27 15:22:33 +00:00
1e982fa78c Sign plaintext packets in licensed mode (#10969)
* feat: sign licensed plaintext packets

Preserve and publish identity keys in licensed mode so existing XEdDSA signatures can authenticate plaintext traffic. Sign licensed broadcasts and direct messages when they fit, while keeping PKI encryption disabled and normal routing behavior unchanged.

* fix(security): persist licensed channel sanitation

* fix(security): close licensed migration lifecycle gaps

* fix(security): address licensed signing review feedback

* test: restore signing globals from Unity teardown

* fix(baseui): allow confirmed ham region selection

* fix(baseui): guard region picker validation

* style: trunk fmt

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Austin <vidplace7@gmail.com>
2026-07-27 02:57:15 +00:00
45cb8e7500 feat: preserve normal radio profiles across event firmware (#11110)
* feat: isolate event radio profiles

* fix: preserve identity on degraded config boot

* fix: guard event profile storage

* style: align event profile storage names

* fix: discard event profile on normal boot

* refactor: simplify event profile cleanup

* fix: limit inactive profile migration to event firmware

* fix(event): make event-profile capacity check portable across filesystems

The USERPREFS_EVENT_MODE storage preflight called FSCom.totalBytes() and
FSCom.usedBytes(), which only exist on ESP32's LittleFS wrapper. Every other
backend failed to compile once event mode was enabled:

  error: 'class InternalFileSystem' has no member named 'totalBytes'   (nRF52)
  error: no member named 'totalBytes' in 'fs::FS'                      (Portduino)

This was not caught earlier because the event block is behind
#if USERPREFS_EVENT_MODE, and the existing unit tests only cover the pure
helpers, which compile identically either way.

Add fsTotalBytes()/fsUsedBytes() to FSCommon and implement them per backend:
littlefs v1 traversal for nRF52 (Adafruit_LittleFS) and STM32
(STM32_LittleFS), FSInfo for RP2040, statvfs for Portduino, and the native
methods for ESP32 and nRF54L15. The nRF52/STM32 path reports "full" if the
traversal errors so the capacity check fails safe.

Verified with USERPREFS_EVENT_MODE=1: native-macos test_event_profile_storage
passes 5/5, and heltec-v3, rak4631, rak11310 and rak3172 all build clean.

* fix(event): use std::filesystem for Portduino capacity, bump native-suite-count

Two CI fixes:

- native-windows has no <sys/statvfs.h>, so the Portduino branch of
  fsTotalBytes()/fsUsedBytes() broke the Windows build. Switch to
  std::filesystem::space(), which is already used under ARCH_PORTDUINO in
  HostMetrics.cpp and works on both POSIX and MinGW. Errors report "full"
  so the capacity check still fails safe.

- This PR adds test/test_event_profile_storage, taking test/ from 40 to 41
  suite directories, which trips the native-suite-count reconciliation check.

* fix(event): scope boot-write deferral to the radio profile, address review

Review feedback:

- Copilot: saveProto()'s boot-write deferral applied to every file, so
  loadFromDisk()'s recovery writes (e.g. restoring owner fields into
  devicestate) were silently dropped and never retried, because the ctor CRC
  baselines are computed after loadFromDisk(). Deferral now applies only to
  the radio-profile files, via isRadioProfileFile().

- jp-bennett: eventConfigFromStandard() wrapped a struct copy plus one field
  assignment in a header helper with its own unit test. Inlined at its single
  call site and removed; the behaviour is covered end-to-end by hardware
  validation instead (RAK4631 normal -> event -> normal preserves NodeNum and
  public key while swapping LoRa, and restores the original channel
  byte-identically).

- jp-bennett: the active-backup encrypted-storage migration ran for normal
  builds too, which changed non-event behaviour and contradicted the PR's
  stated event-only scope. Scoped to USERPREFS_EVENT_MODE; the adjacent block
  already covers the inactive standard backup in event builds.

New tests (all verified to fail under mutation, not vacuous):

- test_event_paths_never_collide_with_standard_or_shared_files: the core
  safety property. A path-table slip would make event firmware overwrite the
  user's real config, channels or backup, or capture a shared file like
  devicestate/nodedb. Nothing asserted this before.

- test_only_radio_profile_files_defer_boot_writes: guards the deferral fix
  above so re-widening it fails loudly.

- test_event_reservation_fits_smallest_supported_filesystem: the reservation
  is compile-time but must fit filesystems as small as 14 KiB (STM32WL) and
  28 KiB (nRF52840). Protobuf growth pushing it past those would silently
  stop event profiles persisting - the exact failure mode confirmed by
  fault injection on a RAK4631.

Verified with USERPREFS_EVENT_MODE both on and off: 7/7 tests pass, and
rak4631, heltec-v3, rak11310 and rak3172 all build clean.

* fix: preserve event profile storage recovery

---------

Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Benjamin Faershtein <benjaminfaershtein@Benjamins-MacBook-Pro-2.local>
2026-07-26 22:58:23 +00:00
8e104a909b fix(admin): persist TAK module config (team color / member role) (#11216)
Setting TAK team/role ACKed and rebooted but stored nothing:
handleSetModuleConfig had no tak case, saveToDisk never set has_tak,
and handleGetModuleConfig had no TAK_CONFIG case. Add all three, plus
a native suite sweeping every ModuleConfig submessage through
set -> save -> load -> get and a TAK value-fidelity suite.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-25 23:03:06 +00:00
Thomas GöttgensandGitHub b46c8c9f80 Router: defer nested local sends to avoid handleReceived re-entrancy (#11191)
Prevents an nRF52 task-stack overflow on config save. Replaces draft #11155.
2026-07-24 19:30:48 +00:00
Ben MeadorsandGitHub 01d1873bfc Deliver locally-generated replies addressed to us to the phone (#11185)
* Deliver locally-generated replies addressed to us to the phone

Config get/set from the phone times out on every device: the client sends an
admin request, the node handles it, and the response is silently dropped
before it reaches the phone queue.

#10967 changed Router::sendLocal's isToUs branch from enqueueReceivedMessage()
to handleReceived(p, src), so a local packet keeps its RxSource instead of
being relabeled RX_SRC_RADIO by the queue round-trip. That is the right call
for the new policy gates, but module replies go out through
MeshService::sendToMesh() with the default RX_SRC_LOCAL, and a reply to a
phone-originated request is addressed to our own node (setReplyTo resolves
from == 0 to ourNodeNum). Those replies now re-enter callModules as
RX_SRC_LOCAL, where the loopback gate skips every module whose loopbackOk is
false - including RoutingModule, whose promiscuous sniff is the only path that
moves a received packet into toPhoneQueue. The reply is released, never sent.

Requests still work, because the phone's own packets arrive as RX_SRC_USER and
pass the gate, so a set_config is applied and only its acknowledgement is lost.
That is why a client can connect and download config but times out on every
config screen and every setter.

Deliver the phone's copy from sendToMesh() instead: for a local packet
addressed to us, the loopback gate is doing its job in keeping the packet away
from module re-dispatch, and the phone copy is exactly what is missing. Setting
loopbackOk on RoutingModule would instead echo every locally-generated
broadcast back to the phone, and relabeling replies RX_SRC_RADIO would undo the
origin separation #10967 added.

Also stop reporting ERRNO_SHOULD_RELEASE (35) to the phone in the QueueStatus
for these packets. It means "caller frees", not a send failure, and the same
hunk changed it from the 0 the phone used to see.

* Address review: trim comments, assert the QueueStatus count

Condense the added comments to the one-or-two-line house style; the rationale
lives in the commit message and PR.

The reply test drained QueueStatus records in a while loop, which would have
passed just as happily on an empty queue. Count them and require both the
request's and the reply's.
2026-07-24 09:02:51 +00:00
Benjamin Faershtein 8acb7fec73 test: cover coerced packet signature policy 2026-07-22 12:25:19 -07:00
Ben MeadorsandGitHub 5e3d1ea606 Merge pull request #11129 from RCGV1/agent/include-statusmessage-config
phoneapi: send status message config
2026-07-21 19:37:35 -05:00
Benjamin Faershtein d195ec6748 fix(security): round-trip packet policy defaults 2026-07-21 14:25:53 -07:00
Benjamin Faershtein 566d95f7d2 test: terminate status message fixture 2026-07-21 10:15:44 -07:00
Benjamin Faershtein 91915a0539 Merge commit '5548bd3195c18ec9274b2ff8171504acfc2647b3' into agent/include-statusmessage-config 2026-07-21 10:11:16 -07:00
Benjamin Faershtein 9cce99400b phoneapi: send status message config 2026-07-21 10:10:34 -07:00
Ben MeadorsandGitHub 5548bd3195 Merge pull request #10967 from RCGV1/codex/packet-auth-policy 2026-07-21 11:32:40 -05:00
Ben Meadors 6fc7c1b1db test: unset force_simradio so admin request pinning is exercised
The native test harness boots Portduino in simulated mode, and
wouldEncryptWithPKC() short-circuits to false whenever
portduino_config.force_simradio is set. noteOutgoingAdminRequest() derives its
pin from that predicate:

    keyValid = haveDestKey && wouldEncryptWithPKC(&p, p.channel, haveDestKey);

so under the harness no outgoing admin request is ever key-pinned, and
responseIsSolicited() admits a plaintext response to a PKC-pinned request.
test_pinned_request_keeps_its_key_after_an_unpinned_request and
test_request_to_keyed_node_pins_the_stored_key have therefore failed since
#11092 added them, on develop and on every branch that merges it.

Instrumenting the predicate shows every other term already satisfied
(haveDestKey=1, isFromUs=1, private_key=32, unicast, ADMIN_APP, channel
LongFast) with wouldEncryptWithPKC=0, leaving only the simradio guard.

Clear the flag in setUp so the fixture models a real device. Skipping PKC under
force_simradio is correct for a simulated radio - there is no PKC to pin - so
the predicate is left alone rather than relaxed to make a test pass. The only
sim-mode path in this suite is AdminModule's exit_simulator intercept, which no
test here exercises.

Native suite: 38 suites, 723/723 cases, no sanitizer findings.
2026-07-21 10:49:35 -05:00
Ben MeadorsandGitHub e11635baa2 Merge branch 'develop' into codex/packet-auth-policy 2026-07-21 07:18:02 -05:00
Thomas GöttgensandGitHub d587f08481 Pin admin responses to the stored peer key and request id (#11092)
* Pin admin responses to the stored peer key and request id

noteOutgoingAdminRequest derived its PKC pin from p.public_key, which nothing
populates on the outgoing path, so keyValid was false for every client request
and the pin never engaged. The accepted-response predicate reduced to an
unauthenticated from plus variant and subtype.

Resolve the destination key from NodeDB the way perhapsEncode does, and pin it
only when the request would actually be PKC-encrypted. Extract that condition
from perhapsEncode as wouldEncryptWithPKC so both use one predicate. Also
record the request's packet id and require the response to echo it.

* Treat a zero request id as no pairing token
2026-07-21 07:16:22 -05:00
Ben Meadors 7d954e668d Merge branch 'develop' into codex/packet-auth-policy
Resolve conflicts against the NodeDB signer/key primitives (#11050) and the
admin-key PKI decrypt budget (#11100).

- NodeDB: drop this branch's hasSeenXeddsaSigner in favour of develop's
  isKnownXeddsaSigner. They answer the same question, but develop's reads the
  dedicated warm signer bit (warmSignerOf) rather than the WarmProtected
  category, and TrafficManagementModule already depends on it. Keep develop's
  copyPublicKey/copyPublicKeyAuthoritative, isVerifiedSignerForKey and
  commitRemoteKey/KeyCommitTrust.
- checkXeddsaReceivePolicy: keep this branch's Strict/Balanced/Compatible
  policy, which is a superset of develop's balanced-only downgrade gate, and
  call isKnownXeddsaSigner from it. develop's !pki_encrypted term is dropped
  because the policy returns early for PKI packets before that check.
- perhapsDecode: keep develop's key resolution (NodeDB then pending-key, only
  for real PKI candidates) plus its admin-key token bucket, and re-apply this
  branch's pkiAttempted flag feeding the DECODE_OPAQUE verdict. Keep both
  passesRoutingAuthGate and adminKeyFallbackAllowed/Refund.
- test_A17: model eviction the way NodeDB actually does it, passing the warm
  signer bit as well as the XeddsaSigner category, since isKnownXeddsaSigner
  reads the former.

Native suite: 38 suites, 743/743 cases, no sanitizer findings.
2026-07-21 06:09:57 -05:00
Ben Meadors e247f70c6d test: drain toPhoneQueue in MQTT test to fix ASan leak
sendLocal() now dispatches local packets through handleReceived() directly
instead of the mock-overridden enqueueReceivedMessage(). The MQTT implicit
ACK-to-self therefore runs the full receive pipeline (RoutingModule ->
MeshService::handleFromRadio -> sendToPhone), enqueuing pooled MeshPacket
copies into toPhoneQueue. Production drains that queue via PhoneAPI, but the
test has no phone reader, so the copies leaked at teardown (LeakSanitizer:
42824 bytes / 101 objects across test_receiveFuzzServiceEnvelope and
test_receiveAcksOwnSentMessages).

Give MockMeshService a destructor that drains toPhoneQueue like the phone
would. Test-only; the real firmware does not leak here.
2026-07-21 05:27:23 -05:00
1872e5b306 TMM extend to ephemeral 3rd tier store (#11050)
* Harden TMM NodeInfo direct-response: staleness, key hygiene, throttle

The NodeInfo direct-response path answered queries on behalf of other nodes
from an unauthenticated, never-expiring cache while suppressing the genuine
request (STOP). That let a long-gone or forged identity be served as a
fresh-looking, authoritative reply indefinitely. Three fixes:

- Staleness gate: refuse to spoof a reply for a node not actually heard within
  ~6h (PSRAM cache via lastObservedMs; NodeDB fallback via sinceLastSeen, which
  tolerates last_heard==0 when the clock is unset). A stale entry now falls
  through so the real request propagates instead of being answered for a dead
  node.

- Key hygiene: mirror NodeDB::updateUser() PKI checks when caching overheard
  NodeInfo - reject a packet advertising our own public key, and pin keys (drop
  a NodeInfo whose key mismatches an already-known key from NodeDB or from our
  own cache). Prevents cache poisoning / key substitution via the spoofed-reply
  path.

- Response throttle: at most one spoofed reply per target node per 30s. Direct
  responses bypass the per-sender rate limiter (they STOP the request first) and
  the reply target is attacker-controlled, so this bounds airtime exhaustion and
  reflected floods. Recorded in a new NodeInfoPayloadEntry.lastResponseMs
  (PSRAM; self-expiring timestamp compare, no sweep needed).

Tests: native NodeDB-fallback staleness (stale drops, fresh serves) plus
PSRAM-guarded staleness, throttle, and key-mismatch cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Add TODO(T1-T9) markers for code-review findings on NodeInfo hardening

Comment-only. Tags each site flagged in the review of the direct-response
throttle/staleness/key-hygiene change so the follow-ups are visible in-context:

T1 throttle black-holes distinct requestors; T2 per-target key doesn't bound
aggregate TX; T3 non-PSRAM fallback unthrottled; T4 millis-wrap defeats the
staleness gate; T5 redundant second O(n) scan for the throttle stamp;
T6 duplicated 32-byte key compares; T7 sinceLastSeen() called twice;
T8 two 6h constants can desync; T9 clockMs()==0 collides with the 0 sentinel.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Address code-review cleanups T6, T7, T8 in NodeInfo direct-response

T8: derive kNodeInfoMaxServeAgeSecs from kNodeInfoMaxServeAgeMs so the PSRAM
    and NodeDB-fallback staleness windows cannot desync.
T6: collapse the three duplicated 32-byte key compares in cacheNodeInfoPacket
    (owner-impersonation + NodeDB pin + cache TOFU pin) into one pubKeysEqual()
    helper, so the compare lives in one place.
T7: compute sinceLastSeen(node) once on the fallback staleness path so the
    tested age and the logged age cannot diverge (it calls getTime() each time).

Remaining review items still marked in-code: T1/T2/T3 (throttle design),
T4/T9 (millis-wrap + 0-sentinel), T5 (redundant throttle-stamp scan).

Native test_traffic_management suite: 48/48 passing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Throttle the NodeDB fallback direct-response path (T3)

The per-target NodeInfo response throttle lived only in the PSRAM
NodeInfoPayloadEntry, so non-PSRAM boards - which answer from the NodeDB
fallback path - emitted spoofed direct replies with no rate limit at all,
leaving the airtime/reflection surface the throttle exists to close.

Add a single module-global fallback stamp (nodeInfoFallbackLastResponseMs,
4 bytes, guarded by cacheLock) that throttles the fallback path to one
spoofed reply per window across all targets. Coarser than the PSRAM
per-target throttle, but the fallback path has no per-node slot to stamp,
and a global cap still bounds spoofed transmissions - which is the point.

Add a native test exercising the fallback throttle window.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Make NodeInfo staleness/throttle wrap- and sentinel-safe (T4, T9)

T9: the millis-based fields that use 0 as a "never" sentinel
(lastObservedMs, lastResponseMs, nodeInfoFallbackLastResponseMs) could be
stamped with a literal 0 during the one-millisecond clockMs()==0 instant at
each ~49.7-day wrap, momentarily colliding with the sentinel and disabling
the staleness/throttle gate for that entry. Route all such stamps through a
new nowStampMs() that maps 0 -> 1; the 1 ms skew is irrelevant to every
window these fields feed.

T4: the staleness gate's modular age compare is only unambiguous while true
age < 2^32 ms (~49.7 days); an entry that lingered unrefreshed that long
could wrap to a small age and read as fresh, defeating the gate. Add a
NodeInfo eviction pass to the maintenance sweep that drops entries past the
serve window, so an entry is removed long before its age can approach the
wrap boundary. This also frees slots holding stale identities.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Stamp PSRAM throttle entry by captured index, not a rescan (T5)

The post-send throttle stamp did a second full O(n) scan of the PSRAM
NodeInfo array under a fresh lock, even though findNodeInfoEntry() had
already located the slot at the top of shouldRespondToNodeInfo(). Capture
the slot index during that initial lookup and address the entry directly on
the stamp path. Because the cache lock is released between the two accesses,
the slot could have been evicted or reused, so re-validate node == p->to
under the lock before stamping.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Document accepted per-target throttle tradeoffs (T1, T2)

Convert the T1/T2 TODO markers into a NOTE: the per-target PSRAM throttle
keying and its lack of an aggregate bound are deliberate design choices, not
pending work. A distinct requestor being throttled for one window is
harmless on a redundant mesh, and keeping the PSRAM path per-target
preserves throughput for legitimate multi-target responders (the fallback
path already bounds aggregate via its global stamp).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Track signed-provenance of cached NodeInfo public keys

Add keySignerProven to NodeInfoPayloadEntry, set when an observed NODEINFO
frame's XEdDSA signature was verified (mp.xeddsa_signed). This distinguishes
a trust-on-first-use key from one proven to belong to a signer. The flag is
monotonic - once proven it stays proven, and the existing key-pin checks
forbid the underlying key from changing - so a later unsigned frame cannot
downgrade it. A signature can only be verified against a key we already
held, so a first-contact key is always TOFU until a later signed frame
upgrades it.

Groundwork for using the TMM cache as a last-resort public-key source, where
this flag serves as a trust tier.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Draw public keys from the TMM NodeInfo cache as a last resort

Add TrafficManagementModule::copyPublicKey() and consult it from
NodeDB::copyPublicKey() after the hot (NodeInfoLite) and warm
(WarmNodeStore) tiers miss. This extends the pool of peers the node can
encrypt to: a key the NodeInfo direct-response cache overheard for a node
that has since aged out of both NodeDB tiers can still be used.

The getter reports whether the key is signer-proven or trust-on-first-use.
NodeDB serves TOFU keys here too - the same first-contact trust NodeDB
already applies in updateUser() - so the pool actually expands to new
long-tail nodes rather than only re-confirming known keys.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Retain keyed NodeInfo entries as a pubkey pool; trust-tier eviction

Now that NodeDB::copyPublicKey() draws keys from the NodeInfo cache, the flat
6 h serve-window eviction would discard useful keys the moment a node stops
being servable. Split retention: an entry carrying a 32-byte public key is
kept for kNodeInfoKeyRetentionMs (7 d), while a keyless entry still expires
at the serve window. Both windows stay well under the ~49.7-day millis wrap,
preserving the T4 wrap-safety guarantee.

Make LRU victim selection trust-tiered to match: a keyless slot is sacrificed
before any keyed one, and a trust-on-first-use key before a signer-proven
key; within a tier the oldest loses. Mirrors WarmNodeStore's keyed-first
admission so the most valuable keys are the stickiest under memory pressure.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Test signed-provenance flag and copyPublicKey key-pool source

PSRAM-gated tests (run on the ESP32 build, alongside the existing NodeInfo
PSRAM tests):
- copyPublicKey serves a TOFU key learned from an unsigned NodeInfo and
  reports signerProven=false
- a later signature-verified NodeInfo upgrades provenance to signer-proven
  while the pinned key bytes stay unchanged
- copyPublicKey reports a miss for an uncached node

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Inherit signer provenance from NodeDB when caching a re-found node

When the TrafficManagement NodeInfo cache re-caches a node, mark its key
signer-proven if NodeDB already knows the node as a verified signer for that
same key - even if this particular (unicast/unsigned) frame carried no
signature. Previously the flag only upgraded on a frame we verified
ourselves, so a node we had already proven elsewhere looked TOFU here.

Add NodeDB::isVerifiedSignerForKey(), which checks both tiers - the hot
store's signed bitfield and the warm tier's cached signer bit - and requires
the key to match so a rotated/mismatched key never inherits a stale verdict.
Add WarmNodeStore::isVerifiedSigner() to expose the warm signer bit (the
rebase onto develop added the bit itself; this surfaces it for lookups).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Lock NodeInfoPayloadEntry packing with a size assert

keySignerProven cost zero bytes: sourceChannel, the two bools, and
decodedBitfield are four 1-byte fields that fill a single 4-byte tail word
(struct alignment is 4), so the flag consumed former padding rather than
growing the 2000-entry PSRAM array. Add a static_assert pinning the entry to
sizeof(meshtastic_User) + 20 so a future 5th trailing byte - which would open
a fresh word (~8 KB PSRAM across the array) - fails the build instead of
silently costing memory, prompting new flags to be packed into existing bytes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Pack NodeInfo cache booleans into 1-bit fields

Fold hasDecodedBitfield and keySignerProven into adjacent uint8_t:1 bitfields
so they share a single byte, reserving 6 spare bits for future flags without
growing the 2000-entry PSRAM array. sizeof is unchanged (still one packed tail
word, sizeof(meshtastic_User) + 20); access is by name exactly as before, so no
call sites change. Reorders decodedBitfield ahead of the flags so the two
1-bit members stay adjacent and the compiler packs them together.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Gate NodeInfo replay on signer-proven provenance (compile-time, default on)

Add TMM_NODEINFO_REPLAY_REQUIRE_SIGNED (default 1): the direct-response path
now only spoofs a reply for a node whose key is signer-proven, in addition to
the staleness gate - so replay is based on flagging AND staleness. Replay is a
courtesy feature, and vouching for an unverified (trust-on-first-use) identity
to other nodes is the risk this closes, so signed-only is the safer default.
Define the macro to 0 at build time to also serve fresh TOFU-only nodes.

Both paths are gated: the PSRAM path checks the cached keySignerProven flag,
the NodeDB fallback checks nodeInfoLiteHasXeddsaSigned(node). The effective
gate (TMM_NODEINFO_REPLAY_SIGNED_GATE) is bypassed when PKI is excluded from
the build, since nothing can be signed there and the courtesy feature would
otherwise be disabled outright.

Tests: existing reply-expecting tests establish signer-proven state (a new
markKeySignerProvenForTest hook for the PSRAM cache, the NodeDB signed bit for
the fallback path); add fallback and PSRAM tests asserting an unsigned/TOFU
node is withheld under the default gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Rehydrate re-admitted node names from the TMM NodeInfo cache

The warm tier keeps an evicted node's key but not its name (the 40-byte
record has no room), so a re-admitted long-tail node is nameless until its
next NodeInfo broadcast. The TrafficManagement NodeInfo cache is much larger
(2000 PSRAM entries) and commonly still holds the full User, so use it as a
name reservoir the warm tier structurally cannot be.

On re-admission (getOrCreateMeshNode), after the warm-tier restore, copy the
cached identity from the TMM cache via a new copyUser() getter - but only when
its cached key matches the key just restored from warm, so a name never
attaches to a different identity than the one we encrypt to (key-matched
trust). Guarded by HAS_TRAFFIC_MANAGEMENT and a null check; a no-op without
the PSRAM cache or when no key is present. CopyUserToNodeInfoLite sets only
user-related bits, so the warm-restored signer bit is preserved.

Limitations (by design): PSRAM-only, and the TMM cache is RAM-only, so this
helps within an uptime session, not across reboot.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Drop non-portable NodeInfoPayloadEntry size assert (fixes pico build)

The static_assert pinned sizeof(NodeInfoPayloadEntry) to
sizeof(meshtastic_User) + 20, but nanopb packs the generated User struct
differently per platform: on pico its size is not a multiple of 4, so
alignment padding before the uint32 timestamps makes the overhead 22, not
20, and the assert failed the build (native happened to be +20 and passed,
hiding it). The bitfield packing it was guarding still stands; replace the
fixed-count assert with a comment, since no portable byte count exists.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7QggdwMM1CJkguuCmMfZs

* Run the NodeInfo cache paths in native tests (TMM_HAS_NODEINFO_CACHE)

The NodeInfo direct-response cache and everything layered on it - key
pinning, signer provenance, staleness, throttle - was compiled only for
ARCH_ESP32 + BOARD_HAS_PSRAM, so its entire test set was skipped by the
native CI suite and ran nowhere but real hardware.

Fold the scattered platform guards into one TMM_HAS_NODEINFO_CACHE macro
that also enables the cache (plain heap, delete[] path already existed)
for ARCH_PORTDUINO unit-test builds. Production portduino and embedded
test builds are unchanged. Tests that exercise the NodeDB fallback path
drop the cache explicitly via a new dropNodeInfoCacheForTest() hook, so
both response paths are covered in one binary.

Native suite: 60/60 (was 50 with the 10 cache tests compiled out).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT
(cherry picked from commit b2dafbbbd6501b42fc27175e599fb38be292b0e3)

* Pin NodeInfo cache keys against the warm tier, not just the hot store

cacheNodeInfoPacket() mirrored NodeDB::updateUser()'s key pin but checked
only getMeshNode() - the hot store. updateUser's own pin effectively
covers the warm tier too (getOrCreateMeshNode rehydrates the warm key
before the check), so the mirror had a gap: for a node evicted to the
warm tier whose cache slot was also gone, an attacker's NodeInfo with a
bogus key passed the pin, and the cache's TOFU pin then locked the
genuine node's frames out until the poisoned entry aged away.

Split the authoritative lookup out of NodeDB::copyPublicKey() as
copyPublicKeyAuthoritative() (hot store, then warm tier - never the
opportunistic TMM tier, which would compare the cache against itself)
and pin against that.

Test: warm-tier-only key rejects a mismatching NodeInfo and accepts the
matching one. Native suite: 61/61.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT
(cherry picked from commit 28e880875ef1f8bf16bdf301a4e6036537a84b4b)

* Convert NodeInfo cache clocks to uint8 tick counters

Replace the entry's uint32 millis stamps (lastObservedMs,
lastResponseMs) with three uint8 free-running tick clocks - the same
modular scheme the UnifiedCache counters already use:

  obsTick   3 min/tick (12.8 h period) - replay staleness gate
  respTick  5 s/tick   (21.3 min)      - per-target response throttle
  retTick   1 h/tick   (10.67 d)       - retention TTL + LRU age

Validity is an explicit flag bit (hasObserved / hasResponded), not a 0
sentinel, and the maintenance sweep saturates a stamp (clears its flag)
once its window passes, so no stamp can age toward its ~256-tick
aliasing horizon. That retires the wrap/sentinel special cases (T4, T9)
- nowStampMs() survives only for the fallback path's module-global
millis stamp. obsTick is separated from retention by design: only a
genuinely heard NODEINFO frame stamps it, so later membership-based
retention refresh can never make a silent node look servable.

Also drops lastObservedRxTime, which was only echoed in a debug log.

Entry shrinks 136 -> 128 B; the 2000-entry array 272 kB -> 256 kB, 8 kB
below the original develop footprint while keeping the throttle and
retention features. Tick granularity costs at most one tick per window
(+-5 s on 30 s, +-3 min on 6 h, +-1 h on 7 d).

Native suite: 61/61.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT
(cherry picked from commit e48ed9cfacfed0131b77ed2f1992e8720720d387)

* Keep the NodeInfo cache a superset of NodeDB (seed + keep-alive)

The cache learned identities only from overheard NODEINFO frames, so its
membership was opportunistic: a NodeDB-tier node whose frame was never
heard this boot had no entry (no key pin to protect it, no name to
rehydrate), and the retention TTL evicted entries for nodes that still
lived in the hot store or warm tier.

Add an anti-entropy pass to the maintenance sweep
(reconcileNodeInfoMembershipLocked): every hot-store / warm-tier node
gets an entry - full identity from the hot store (hasFullUser), key-only
from the warm tier - with signer provenance inherited key-matched, and
NodeDB's key adopted wholesale on conflict. Member entries (isMember)
are re-stamped each sweep, so they never age toward the retention TTL;
when a node leaves both tiers the keep-alive stops and its entry ages
out from its final member re-stamp. Membership also outranks key trust
in LRU victim selection, and the reconcile pass skips seeding rather
than churn one member out for another when hot+warm exceeds the cache.

Two properties are load-bearing:
 - seeding/keep-alive never touch obsTick/hasObserved, so a seeded or
   retained entry is never served as a spoofed reply - only genuinely
   heard frames make a node servable;
 - copyUser() now requires hasFullUser, so a key-only warm seed can
   never stamp HAS_USER onto a nameless node via name-rehydration.

Native suite: 64/64.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT
(cherry picked from commit 4f1b30d838b2f7b11173373b8f1ba97cf91bef6b)

* Write-through NodeDB identity commits into the NodeInfo cache

updateUser() is the single chokepoint through which every remote
identity/key write enters NodeDB (key-verification keys stay in
CryptoEngine's pending buffer until a NodeInfo carries them here), so
one hook at its tail lets the NodeInfo cache reflect a commit
immediately instead of waiting up to a minute for the reconcile sweep -
which stays in place as the anti-entropy backstop.

onNodeIdentityCommitted() upserts the full User: NodeDB's key is
authoritative (a conflicting cached key is stale residue - replaced,
provenance dropped), a keyless commit keeps an already-cached TOFU key,
and signer provenance transfers only for the committed key. The
observation stamp is never touched: knowledge is not observation, so a
hook write can never make a never-heard node servable - covered by the
new test alongside immediate copyUser/copyPublicKey visibility.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT
(cherry picked from commit e160b899728be8d951d4e7bdf5106c777832b917)

* Purge TrafficManagement caches on explicit node removal

NodeDB::removeNodeByNum() already forgets the node everywhere NodeDB
owns - hot store, satellite stores, warm tier - but the TrafficManagement
caches kept the deleted identity: the NodeInfo entry went on feeding the
key pool (NodeDB::copyPublicKey) and name rehydration, and the unified
slot kept its role/next-hop/dedup state, resurrecting the node on next
contact.

Add purgeNode(): clears both the unified cache slot and the NodeInfo
cache entry, called from removeNodeByNum() alongside the warm-tier
removal. Removal means full removal; passive eviction never calls this,
and the reconcile sweep will not re-seed a node absent from both NodeDB
tiers.

Test: an observed identity plus a next-hop hint both vanish after
removeNodeByNum(). Left for CI to execute per request.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT
(cherry picked from commit b5564c173337de99e15ef998a5c698550238fd3c)

* Document the tmm-fix-2 / tmm-fix-superset reconciliation decisions

Two parallel implementations of the same superset design are being
combined on this branch. This decision matrix records every divergence
and which side each reconciled feature takes, ahead of the code commits
that apply them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT

* Reconcile retention: no timed eviction, obsTick LRU, hourly seeding

Adopt tmm-fix-superset's retention model (reconciliation decisions #3,
#4, #9 in .notes/tmm-super-superset-reconciliation.md):

- Entries are never evicted on a timer. The 7-day retention TTL and its
  third tick clock (retTick) are gone; wrap-safety rests entirely on the
  sweep's presence-bit saturation, and a quiet entry keeps its value
  (pubkey pool, name rehydration). Slots are reclaimed only by
  trust/membership-tiered LRU on insert - age scored by obsTick, with
  never-observed entries counting oldest - or by an explicit purge.
- Membership refresh (entry -> NodeDB contains checks) runs every sweep;
  the heavy seeding pass runs at boot and then hourly, with the
  write-through hooks carrying immediacy in between.
- Tick constants and helpers move into the header beside the existing
  UnifiedCache tick idiom, with static_asserts tying the tick windows to
  the fallback path's seconds/ms forms.

Kept from tmm-fix-2 (decision #4): the warm tier is still seeded
(key-only records via WarmNodeStore::entryAt) so the invariant remains
cache superset-of hot AND warm, and the spareMembers guard still stops
seeding from churning one member out for another when hot+warm exceeds
the cache. Entry shrinks to 128 B (2000 entries = 256 kB).

The TTL-based retention test is superseded by a no-timed-eviction test:
a quiet keyed entry survives 9 days of sweeps while the serve gate
saturates as before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT

* Reconcile hooks: key-commit write-through, purgeAll, key-matched signer

Adopt tmm-fix-superset's hook coverage (reconciliation decisions #5-#8):

- onNodeKeyCommitted(): write-through for the two key-write sites that
  bypass NodeDB::updateUser - the Router's admin-key learn (TOFU-grade)
  and KeyVerificationModule's manual-verification commit (proven=true,
  the strongest provenance the cache can carry). Both were coverage gaps
  in the tmm-fix-2 write-through design.
- updateUser's hook call now transfers signer status key-matched
  (isVerifiedSignerForKey) instead of the node's bare signed flag, and
  runs on acceptance rather than only on change.
- purgeAll(): factoryReset() and resetNodes() clear both TMM tables -
  removal-is-full-removal applies to bulk resets too, without relying on
  the usual post-reset reboot.
- purgeNode() reuses the existing finders instead of open-coded scans
  and logs the (user-initiated) purge.
- peekNodeInfoFlagsForTest(): flag introspection so saturation and
  membership tests can assert sweep effects directly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT

* Merge the two branches' test suites for the reconciled design

Port tmm-fix-superset's unique tests, adapted to run natively under
TMM_HAS_NODEINFO_CACHE (reconciliation decision #11):

- keyHook_upsertsAndGovernsProvenance: TOFU learn -> manual-verification
  upgrade -> NodeDB-senior rotation resets provenance.
- tickSaturation_sweepClearsObserved: the sweep saturates hasObserved
  past the serve window, the entry persists (no TTL), and a full
  256-tick clock wrap cannot alias a saturated stamp back to fresh.
- sweepMembershipMarking: reconciliation seeds a hot identity as
  member+fullUser+unobserved; the next sweep clears membership once the
  node leaves NodeDB, while the entry itself persists.

tmm-fix-2's tests (warm pin, hot/warm seeding, updateUser write-through,
removal purge - now also asserting the entry is gone via the new peek
hook - and the no-timed-eviction rewrite) were already on this branch.
Suite now counts 70 test functions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT

* Adapt cherry-picked and seeding tests to the reconciled semantics

Two tests needed updating for interactions the validation run surfaced:

- The #11035 test (ignoresUnsignedSignerIdentity) was written for a
  world without the native NodeInfo cache: it needs the NodeDB fallback
  path (dropNodeInfoCacheForTest) and the signed replay gate satisfied
  for its target, like the other fallback tests on this branch.
- The hot-seed test's "observed frame makes it servable" step now sends
  a signature-verified frame: its node is a known signer, and per #11035
  an unsigned frame from a signer must not (and does not) drive cache
  writes - the gate working as designed.

Full native suite: 70/70 under ASan.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWHDDxY4U2XLYW2WJJ2oLT

* trunk fml

* Doc tidyup

* TrafficManagement: key NodeInfo-cache maintenance to its own guard

runOnce() nested the entire NodeInfo maintenance block (tick-stamp
saturation, membership refresh, boot/hourly reconcile) inside
#if TRAFFIC_MANAGEMENT_CACHE_SIZE > 0, so a variant overriding the
unified cache to 0 on a PSRAM board would compile the NodeInfo cache
without its maintenance: hasObserved would never saturate, obsTick
would alias past its 12.8 h wrap, and the 6 h staleness gate - whose
wrap-safety explicitly depends on the sweep - would serve spoofed
replies for long-gone nodes. Extract maintainNodeInfoCacheLocked(),
guarded by TMM_HAS_NODEINFO_CACHE alone, and give runOnce() the same
independent-guard structure purgeAll() already has.

Also close the runtime cousin of the same mismatch: the write-through
hooks (onNodeIdentityCommitted / onNodeKeyCommitted) now no-op while
moduleConfig.has_traffic_management is off. Previously they kept
filling the cache from NodeDB commits while runOnce() returned
INT32_MAX, accumulating entries that were never swept or reconciled.
Purges and reads stay ungated: removal must always work, and reads
just miss an empty cache.

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

* TrafficManagement: forward throttled NodeInfo requests instead of consuming them

The direct-response throttle returned true, which made handleReceived()
STOP the request: within the 30 s window a repeat request was neither
answered nor forwarded toward the genuine target, and the call site
still logged "respond" and bumped nodeinfo_cache_hits for a reply that
was never sent. A requester whose first reply was lost on a noisy link
got silence for the whole window.

Return false instead: the request flows through normal relay handling,
so the genuine node (or another cache-holder) can answer, while our own
spoofed TX stays bounded. Repeats of the same packet id are already
absorbed by the router's duplicate detection, and the stats counter now
only counts replies actually sent.

Also log purgeNode() only when a slot was actually cleared - it runs
for every NodeDB removal, including nodes the caches never tracked.

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

* TrafficManagement: consolidate NodeInfo-cache #if sprawl into one region

Fourteen scattered #if TMM_HAS_NODEINFO_CACHE guards (one per function,
each with its own #else stub of (void) casts) collapse into a single
guarded region holding every NodeInfo-cache-only function, terminated
by one #else block of no-op stub definitions. Because the stubs now
exist on every build, the call sites in runOnce(), purgeNode() and
purgeAll() drop their guards too; inner guards remain only for
orthogonal features (PKI, warm tier) and for real conditional work
(constructor allocation, PSRAM paths in shouldRespondToNodeInfo).

No behavior change. Compile-checked on both sides of the macro: the
native app build (stubs) and the native unit-test build (region).

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

* Router: resolve sender key only for PKI-decrypt candidates

perhapsDecode() resolved the sender's public key unconditionally for
every encrypted packet, before even checking whether the packet could
be PKI-decrypted (channel 0, unicast to us). Since copyPublicKey()
grew its TrafficManagement fallback tier, a full hot+warm miss - the
common case for channel traffic from senders outside both NodeDB
tiers - additionally walked the 2000-entry NodeInfo cache under its
lock, per packet, for a key that was then discarded.

Move the resolution inside the PKI-candidate branch; remotePublic and
haveRemoteKey had no consumers outside it.

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

* TrafficManagement: refresh NodeInfo membership hourly, not per-sweep

The 60 s sweep re-derived isMember with a per-entry NodeDB lookup:
O(entries x members) - about 700k node-number comparisons per minute on
a full PSRAM cache (2000 entries x 250 hot + 100 warm), with strided
PSRAM reads, all while holding cacheLock against the packet path.

Move the refresh into the hourly reconcile pass, which is already
O(members x entries): after the seeding loops (so the upsert pass still
sees last pass's bits for spareMembers protection), clear every
isMember bit and re-mark from both NodeDB tiers - including keyless
warm records, which seed nothing but are still members.

Accepted tradeoff, now documented on the field: membership lags a
passive NodeDB eviction by up to an hour (the entry just stays
LRU-sticky slightly longer). Additions stay immediate via the
write-through hooks, explicit removals via purgeNode().

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

* NodeDB: centralize bare-key commits in commitRemoteKey()

The two key-write sites that bypass updateUser() (admin-channel learn
in Router::perhapsDecode, manual verification in KeyVerificationModule)
each carried their own #if HAS_TRAFFIC_MANAGEMENT write-through
boilerplate - the pattern this PR itself had to retrofit twice, and
which any future direct key write would silently miss, leaving the
TrafficManagement cache divergent until the next hourly reconcile.

Add NodeDB::commitRemoteKey(n, key32, KeyCommitTrust): writes the key
to the hot store and routes the TrafficManagement write-through in one
place, with provenance explicit at the call site (AdminChannelProven
maps to TOFU-grade, ManuallyVerified to proven). The bypass sites keep
their reason for existing - bare-key commits with provenance that
updateUser's User-payload/TOFU-pin path cannot express - but no longer
know about TrafficManagement at all; both stale includes are dropped
(Router's was already unused on develop).

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

* docs: tick wrap-safety analysis, updated cadences, CodeRabbit doc fixes

node_info_stores.md gains a "Tick clocks and wrap safety" section
recording which mechanism keeps each modular tick clock honest: the
unified-cache clocks pair the 60 s sweep with read-time window resets,
the NodeInfo obs/resp clocks are sweep-only (hence the compile-time
invariant that maintainNodeInfoCacheLocked() is guarded by
TMM_HAS_NODEINFO_CACHE alone), and the warm tier is immune by design -
absolute unix-seconds, no wrap until 2106.

Also brought current with this series: membership refresh moved to the
hourly reconcile, throttled direct-response requests forward instead of
being consumed, the commitRemoteKey() bare-key funnel, and the
module-disabled gate on the write-through hooks.

CodeRabbit doc review (PR #11050): present the NodeInfo payload cache
as the third *identity* tier with the unified cache beside the chain,
and describe NodeInfoLite's flattened fields / satellite copy-out
accessors instead of the removed nested members. Two stale
docs/tmm_node_stores.md references in the header now point at the real
file.

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

* TrafficManagement: adapt tests to forwarded throttles and hourly membership

Throttle tests (PSRAM and NodeDB-fallback paths): a request inside the
30 s window now CONTINUEs into normal relay handling instead of being
consumed - assert no spoofed TX, no NAK suppression, and (PSRAM path)
that nodeinfo_cache_hits counts only replies actually sent.

Membership test (renamed reconcileMembershipMarking): the per-minute
sweep no longer refreshes isMember, so the test now pins down both
halves of the new contract - the very next sweep after a passive NodeDB
eviction still shows the stale member bit (the documented up-to-an-hour
lag), and a reconcile interval's worth of sweeps clears it.

Disabled-module test additionally proves the write-through hooks share
the has_traffic_management gate: a key commit while disabled must not
land in the NodeInfo cache.

Not covered here: a build permutation with TRAFFIC_MANAGEMENT_CACHE_SIZE
overridden to 0 (the configuration the maintenance-guard fix protects)
would need its own PlatformIO env plus guards on every unified-cache
test - left as a follow-up.

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

* expand test coverage

* nitpicks and docs update

* more comment fixes

* nitpicks

* test: make TMM fixture cleanup abort-safe in tearDown()

Several tests reset per-test global state (trafficManagementModule pointing at a
stack `module`, owner.public_key, the RTC fake clock) only after their assertions.
A TEST_ASSERT_* failure longjmps out and skips that trailing cleanup, leaving the
state to dangle into later cases - concretely, the next setUp()'s resetNodes()
would call trafficManagementModule->purgeAll() on a destroyed object.

Move the resets into tearDown(), which runs unconditionally between tests, and drop
the now-redundant per-test cleanups. Addresses a CodeRabbit review comment on
PR #11050.

clod helped too

* docs tidy

* Throttle NodeInfo direct responses

A direct response is addressed to the requesting packet's from field, which
is unauthenticated, and is sent by every neighbour that holds the target in
cache. One request therefore makes several nodes transmit at an address the
requester chose, and nothing limited how often that could be repeated.

Replies are now spaced per requester, which bounds how much any single node
can be made to receive, plus a global floor on how much airtime the feature
can consume. The check sits where a reply is about to be sent, so requests
declined for other reasons do not consume the budget.

* test: reconcile TMM direct-response tests with #11104 throttle

Makes the suite green for now. #11104's per-requester + global-floor
throttle (60 s) is layered over the branch's per-target throttle, so the
three existing "served again" checks (psram, fallback, sweep) now use a
fresh requester to avoid the per-requester window masking the mechanism
each one actually exercises. Adds a dedicated test for the new
per-requester + 1 s global-floor behaviour (the reflector-flood gap the
per-target throttle leaves open, and the bound that survives on
non-PSRAM builds).

Deferred: the branch's now-redundant fallback global stamp
(nodeInfoFallbackLastResponseMs) is left in place as a follow-up cleanup.

Clod wants credit.

* TMM: unify direct-response throttles into per-sender + per-target RAM tables

Replace the two path-specific NodeInfo-spoof throttles (per-entry respTick on
the PSRAM cache path, single global stamp on the fallback path) with three
symmetric bounds that behave identically with and without PSRAM:

  - per requester (60 s): how much any one node can be made to receive;
  - per target   (60 s): how often we vouch for the same identity;
  - global floor (1 s):  total airtime, the backstop once an attacker cycles
    requester/target past the 8-slot tables.

Both axes are fixed 8-slot LRU tables in internal RAM (not the PSRAM NodeInfo
cache), compared by wrap-safe uint32 ms subtraction, so there is no tick clock
and no sweep to maintain. directResponseAllowed() resolves both slots before
stamping either, so a reply one axis throttles never consumes the other's
budget, and records the send itself.

Retires: nodeInfoFallbackLastResponseMs, kNodeInfoResponseThrottleMs,
nowStampMs, the respTick byte + hasResponded bit on every cache entry,
currentRespTick/kNodeInfoRespTickMs/kNodeInfoThrottleTicks, and the sweep's
respTick clear. Renumbers peekNodeInfoFlagsForTest (drops the responded bit).

Tests: the psram/fallback throttle tests now assert the per-target axis at 60 s,
isolated via a different requester, proving it holds with and without PSRAM;
perRequesterAndGlobalFloor isolates the other two axes; the obsolete respTick
wrap-safety test is removed. 83/83 native cases pass.

clod helped too

* whats up, doc?

* trimming the comments

* test: bump native-suite-count to 38 after upstream rebase

Upstream develop carries 38 test_* suite directories but its
native-suite-count file still reads 37 (a suite was added without
bumping the count). Rebasing onto develop inherits that stale file,
so the runner flags AMBER. Correct the registered total to 38.

clod helped too

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
2026-07-21 12:19:43 +02:00
Ben MeadorsandGitHub 0165e914a9 fix: gate module replies by request port (#11094) 2026-07-21 10:19:38 +02:00
Thomas GöttgensandGitHub 7bdc2569e8 Budget the admin-key PKI decrypt fallback (#11100) 2026-07-21 09:48:09 +02:00
Thomas GöttgensandGitHub 077c6e72f4 Corroborate traceroute next-hop updates against the relaying node (#11108) 2026-07-21 09:11:05 +02:00
Thomas GöttgensandGitHub b299cc2427 Honour which_payload_variant on MQTT client-proxy ingress (#11097) 2026-07-21 09:08:46 +02:00
Thomas GöttgensandGitHub d9f8839241 Accept a pending key only for the key-verification exchange (#11107)
perhapsDecode fell back to the not-yet-verified key held during a key
verification handshake for any incoming PKI unicast. That key is supplied
by whoever opened the handshake and proves only that they hold it, not that
they are the node they claim to be, so until the session ended they could
send DMs on any port that decrypted and were marked pki_encrypted.

perhapsEncode already restricts the pending key to KEY_VERIFICATION_APP.
The receive path now applies the same rule.
2026-07-20 20:28:30 -05:00
Ben Meadors 1317176f78 test: accept DECODE_OPAQUE/DECODE_POLICY_REJECT in perhapsDecode fuzz assertion
The packet-auth-policy change extends the DecodeState enum with DECODE_OPAQUE
and DECODE_POLICY_REJECT. test_E1_perhaps_decode_fuzz drives arbitrary
ciphertext through perhapsDecode and asserted the verdict was one of the
original three states; random ciphertext that matches no channel with no PKI
attempt now returns DECODE_OPAQUE, tripping the assertion. Broaden the check to
accept all five valid verdicts, matching the test's stated 'any verdict is
fine' contract.
2026-07-20 19:12:28 -05:00
Thomas GöttgensandGitHub 6f522aad17 Return the secret sentinel for remote admin config gets (#11093)
writeSecret is a setter, so calling it on the NETWORK_CONFIG get path was a no-op: the buffer
already holds the stored psk, never the sentinel. MQTT_CONFIG returned the broker password
verbatim.

Both get paths now return secretReserved when req.from != 0, and the matching set paths call
writeSecret so a read-modify-write round trip keeps the stored value.
2026-07-20 18:38:23 -05:00
Benjamin Faershtein 08cfb3d683 Merge upstream develop into packet authentication policy 2026-07-20 11:48:59 -07:00
Ben MeadorsandGitHub 5cf346311c fix: don't wipe admin keys when regenerating the keypair (#11088)
A client's "regenerate keys" action sends a blank SecurityConfig carrying
only the new private key rather than the config it read from the device,
so assigning it wholesale cleared admin_key, is_managed, serial_enabled,
debug_log_api_enabled, admin_channel_enabled and packet_signature_policy.
Losing the admin keys locks the owner out of remote admin with no recourse
but a physical connection to the node.

Detect that bare-rotation shape and swap in just the keypair, leaving the
rest of the security config intact. Deliberately clearing admin keys still
works through a SET that leaves the private key alone.

Fixes #11073
2026-07-20 07:19:29 -05:00
Thomas GöttgensandGitHub 290967f739 Release packets the interface declines to send (#11087) 2026-07-20 13:42:59 +02:00
Thomas GöttgensandGitHub 5ded0ec8c5 Gate identity learning on signature in NodeDB::updateUser (#11084) 2026-07-20 11:56:42 +02:00
Thomas GöttgensandGitHub 405a6bfd8a Strip inner-message padding when sizing unsigned broadcasts (#11083) 2026-07-20 11:55:55 +02:00
Thomas GöttgensandGitHub ba8b1b1038 Treat backslash as a path separator in XModem filename validation (#11085) 2026-07-20 10:46:33 +02:00
Benjamin Faershtein 59ab12675a Merge develop into packet authentication policy 2026-07-19 15:33:16 -07:00
TomandGitHub b8b5582943 Regioninfo (#11056)
* advertise a superset of EU regional presets to client devices.

* trunk
2026-07-19 06:22:39 -05:00
TomandGitHub ff951059cf test: correct native-suite-count to 37 (#11059)
native-suite-count drifted from the actual test/test_*/ directory count: #10669
added two suites (test_admin_session_repro, test_pki_admin_fallback) but bumped
the count by only one, and #11037 added test_xmodem without bumping it at all.
The file reads 35 against 37 real suites, which bin/run-tests.sh reports as AMBER
on every full run. Correct it to 37.
2026-07-18 14:20:59 -05:00
Thomas GöttgensandGitHub 8147970957 AdminModule: only accept admin responses to requests we sent (#11024)
* AdminModule: only accept admin responses to requests we sent

An admin *_response short-circuited the auth and session-passkey checks that gate
every other admin message, so any node could deliver one. On a channel the module
listens to unauthenticated, a get_module_config_response drives the remote-hardware
pin handler with attacker-supplied values.

Track the destination of outgoing admin requests (per remote, with the pinned PKC
key when there is one) and accept a response only from a node with a matching
outstanding request, inside the same window as the session passkey. Local (from == 0)
admin is unchanged; PhoneAPI already gates it.

Also fix the response dispatch: get_module_config_response.which_payload_variant is a
ModuleConfig oneof tag, but it was compared against the AdminMessage ModuleConfigType
enum (different numbering), so the handler never ran. Compare against the oneof tag.

* AdminModule: rollover-safe request window, bind response to request type

Two review refinements to the request/response pairing:

Use Throttle::isWithinTimespanMs for the outstanding-request expiry instead of
comparing millis()/1000 sums, which mis-expired across the millis() rollover.

Bind each accepted response to a request type actually sent to that node. Each
outstanding record now carries a bitmask of the response variants its requests
authorize, so a get_owner request no longer admits a get_module_config response.
The mask accumulates per remote, so a client may still pipeline several request
types to one node and have every answer accepted.

* AdminModule: track admin requests per-request, not per-node

Reworks the outstanding-request table so each request is its own entry with its own
expiry window and pinned key, replacing the per-node bitmask that shared one timestamp
and one key across every response variant.

That sharing let a later request to the same node extend an earlier one's window and,
worse, clear its PKC pin: an unpinned request cleared keyValid, so a plaintext response
to an earlier PKC-pinned request was then accepted. Per-request entries keep each pin
intact. Identical requests are de-duplicated (a client may fetch several config subtypes,
all answered by one response variant) and eviction compares elapsed time, which is
rollover-safe.

Test: a pinned request's response still requires its key after an unpinned request to the
same node.

* AdminModule: match module-config subtype and consume answered requests

Two refinements to the request/response pairing:

Only remote_hardware get_module_config_response mutates state (the pin table), so it
must answer a request for that exact ModuleConfigType, not just any module-config
request. Each entry records the requested subtype and the gate checks it.

A matched request is now consumed on accept, so a node cannot replay a state-mutating
response within the window. Because one request yields one response, request de-dup is
dropped (a client's N indexed get_channel requests are N entries, each consumed once).

Tests: a non-remote-hardware request does not admit a remote_hardware response, and a
second copy of an answered response is rejected.
2026-07-17 07:50:41 -05:00
d5f78a37d3 XModem: reject path-traversal filenames in the transfer handler (#11037)
The SOH/STX control frame carries a client-supplied filename that was passed
straight to FSCom open/remove/exists, so a ".." component could write, read, or
delete outside the filesystem root. On embedded LittleFS this is largely inert
(no parent of the partition root); on the Portduino daemon FSCom is the host
filesystem under a mountpoint, so it is a real arbitrary-path write/read/delete.

Validate the filename before any FS access: reject empty and any ".." path
component, and NAK the transfer. Absolute and subdirectory paths are still
accepted - the file manager transfers them from the manifest and PortduinoFS
confines them to its mountpoint - so only traversal out of the root is blocked.

Reachable only from a local client connection (PhoneAPI: BLE/USB/serial/TCP),
not over the RF mesh; on the daemon the TCP API makes it network-reachable.

native-suite-count goes to 34: +1 for the new test_xmodem suite and +1 correcting
a pre-existing miscount (it read 32 for 33 suite directories).

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2026-07-17 06:31:20 -05:00
a7fde715c6 PhoneAPI: gate local admin on the connection, not the wire from (#11033)
* PhoneAPI: gate local admin on the connection, not the wire from

The lockdown admin check in handleToRadioPacket only ran when p.from == 0. from is
a client-supplied wire field, and MeshService::handleToRadio rewrites it to 0 before
AdminModule sees the packet. A client could therefore set from != 0 to skip the
!getAdminAuthorized() drop, then have the packet normalized back to a local-admin
identity and executed - unauthorized admin from an unauthorized connection.

Every packet in handleToRadioPacket already comes from the local connection, so
locality is a property of the connection, not of from. Move the decision into
classifyLocalAdminPacket(), which ignores from and keys only on the admin variant and
the connection's authorization: lockdown_auth is delivered inline, any other admin
from an unauthorized connection is dropped, authorized admin passes through.

The classifier is compiled unconditionally and unit-tested; the guarded caller (built
only in the nRF52 lockdown config) calls it. Test: an unauthorized connection's
ADMIN_APP packet with from != 0 is classified DropUnauthorized.

* PhoneAPI: wipe the encoded lockdown passphrase, shorten comments

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
2026-07-17 05:21:32 -05:00
Thomas GöttgensandGitHub ed3afdbc61 TrafficManagement: don't learn identity from a signer's unsigned NodeInfo (#11035) 2026-07-17 09:43:22 +02:00
Jonathan BennettGitHubThomas Göttgenscoderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>IxitxachitlCopilot Autofix powered by AI
8d1fbbf55f Snake! (#10936)
* Snake!

* Add spiLock to snake score saving

* Check fixes

* More careful locking

* WIP: Big Display Node

* Update src/graphics/HUB75Display.cpp

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Add HUB75 Native

* Add Tetris game module with core logic and UI integration

- Implement TetrisGame class for game logic, including piece movement, rotation, and line clearing.
- Create TetrisModule class to manage game state, input handling, and rendering on OLED display.
- Introduce high score tracking with persistence and optional mesh broadcasting.
- Define UI states for title, playing, paused, game over, and high scores.
- Implement input handling for game controls and state transitions.
- Add rendering functions for the game board, high scores, and title screen.

* feat(snake): add snake graphics and update display logic in SnakeModule

* Prompt for Initials for Tetris, too

* refactor games module

* Games refactor

* hub75 native double buffer

* Games tuning

* Make joystick repeat events on held button

* Add clouds and colors

* Fix breakout and more color

* difficulty tuning

* trunk

* refactor game announcements, etc

* Scale chirpy gravity as game speed increases

* Portduino, check for hub75 display before reading in hub75 options

* Final(?) games tuning

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Properly ignore input when games screen not shown

* Fail gracefully when HUB75 is selected but not supported.

---------

Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Ixitxachitl <kramerfm@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-07-16 18:35:33 -05:00
Thomas GöttgensandGitHub d3d02af817 AdminModule: don't return the device private key to remote config reads (#11030)
A SECURITY_CONFIG get_config response copied config.security verbatim, so a remote
request was answered with the device identity private_key and sent over the air.
Only the local owner needs it, for backup.

Zero private_key in the SECURITY_CONFIG response when the request is remote
(from != 0); the local BLE/USB/TCP path (from == 0) still receives it. public_key
and admin_key are public and stay as they were.
2026-07-16 16:56:47 -05:00
Thomas GöttgensandGitHub 381f9c196d NodeDB: clear only the slots removeNodeByNum() vacates (#11022) 2026-07-16 21:48:25 +02:00
Thomas GöttgensandGitHub e219e24b09 WarmNodeStore: carry the XEdDSA signer flag through the warm tier (#11020) 2026-07-16 21:46:46 +02:00
Thomas GöttgensandGitHub f52d7753ad Router: size the fit check from the decoded Data (#11018) 2026-07-16 08:51:10 -05:00