From db1c4704e037812c0c1d366d2b0b2af4467656ff Mon Sep 17 00:00:00 2001 From: mikecarper Date: Wed, 21 Oct 2026 03:02:46 -0700 Subject: [PATCH] Fix firmware budget regressions and flood alias delimiters Share nRF52 checked-record reads and command-family lookups without changing storage formats, error protection, fleet authorization, or delegated scopes. Use the existing bounded Merkle proof workspace for qualified ESP32 repeaters while retaining source capacity, wire format, and every enabled feature. Restore advert tab/CRLF delimiters and expose nested qualification build logs. Validation: all 1932 native CI test cases; exact canonical RAK4631 Full, Wio-E5 Mini, Station G3 Full MQTT, and Wireless Tracker Full wrappers; linked RAK crypto/workspace checks and retained Reader code equivalence; focused persistence, authorization, source-proof, and RAM policy regressions. --- .github/workflows/run-unit-tests.yml | 18 +++++ examples/simple_repeater/MyMesh.cpp | 86 +++++++++--------------- scripts/check_firmware_ram.py | 4 +- src/helpers/CheckedSettingsFile.h | 21 ++++-- src/helpers/FleetCommand.cpp | 93 +++++++++++++++++--------- src/helpers/FloodCommandAliases.h | 38 +++++++---- src/helpers/MorseSettings.cpp | 16 ++++- src/helpers/ota/CompanionOtaConfig.cpp | 19 ++++-- src/helpers/ota/OtaMemoryPolicy.h | 8 ++- src/helpers/ota/OtaSpeedConfig.cpp | 19 ++++-- test/test_firmware_ram.py | 20 +++++- test/test_fleet_command_crypto.py | 26 +++++++ test/test_ota_source_proofs.py | 29 ++++++-- test/test_ota_speed.py | 54 +++++++++++++++ test/test_transport_filter_wiring.py | 49 +++++++++++++- 15 files changed, 370 insertions(+), 130 deletions(-) diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 13c8ba620..5ebf0a2e1 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -88,6 +88,15 @@ jobs: bash build.sh build-firmware heltec_v4_r8_tft_companion_radio_full --firmware-version v1.18.0.1-halo-keymind-cascade-dev --radio-preset usa-cascade-fixed --profile cascade --require-ota + - name: Show qualification build logs on failure + if: failure() + run: | + for log in out/build-logs/*.log; do + [ -f "$log" ] || continue + echo "::group::$log (last 120 lines)" + tail -n 120 "$log" + echo "::endgroup::" + done esp32-sensor-smoke: runs-on: ubuntu-latest @@ -109,6 +118,15 @@ jobs: run: | pio run -e meshnology_w12_repeater bash build.sh build-firmware meshnology_w12_companion_radio_full --firmware-version v1.17.1.10-halo-keymind-cascade-dev --radio-preset usa-cascade-fixed --profile cascade --require-ota + - name: Show qualification build logs on failure + if: failure() + run: | + for log in out/build-logs/*.log; do + [ -f "$log" ] || continue + echo "::group::$log (last 120 lines)" + tail -n 120 "$log" + echo "::endgroup::" + done esp32-ethernet-companion-smoke: runs-on: ubuntu-latest diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 51959b33d..85c087a30 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -11922,73 +11922,53 @@ static bool commandFamilyMatches(const char* command, const char* family) { return mesh::cli::floodCommandArguments(command, family) != nullptr; } -static bool isCommonManagerReadOnlyAllowed(const char* cmd) { - while (*cmd == ' ') cmd++; // skip leading spaces - if (commandFamilyMatches(cmd, "ver") || commandFamilyMatches(cmd, "board") - || commandFamilyMatches(cmd, "neighbors") || strcmp(cmd, "clock") == 0) return true; - // sensor reads only - if (commandFamilyMatches(cmd, "sensor get") || commandFamilyMatches(cmd, "sensor list")) return true; +// Keep delegated namespaces explicit while sharing the matcher loop. The +// generic get namespace includes credentials and must never be granted whole. +static bool commandListMatches(const char* command, const char* const* families) { + for (; *families; ++families) { + if (commandFamilyMatches(command, *families)) return true; + } + return false; +} - // Keep delegated reads explicit. CommonCLI's generic `get` namespace also - // contains credentials (guest.password, WiFi/MQTT secrets, bridge.secret, - // and similar settings), so it must never be granted wholesale. - return commandFamilyMatches(cmd, "get role") - || commandFamilyMatches(cmd, "get public.key") - || commandFamilyMatches(cmd, "get clock.sync") - || commandFamilyMatches(cmd, "get battery.alert") - || commandFamilyMatches(cmd, "get rx.watchdog") - || commandFamilyMatches(cmd, "get recent.repeater") - || commandFamilyMatches(cmd, "get recent.repeaters") - || commandFamilyMatches(cmd, "get outpath") - || commandFamilyMatches(cmd, "get altpath"); +static bool isCommonManagerReadOnlyAllowed(const char* cmd) { + while (*cmd == ' ') cmd++; + static const char* const families[] = { + "ver", "board", "neighbors", "sensor get", "sensor list", "get role", + "get public.key", "get clock.sync", "get battery.alert", "get rx.watchdog", + "get recent.repeater", "get recent.repeaters", "get outpath", "get altpath", nullptr + }; + return strcmp(cmd, "clock") == 0 || commandListMatches(cmd, families); } // Whitelist helpers for delegated manager roles. Admins bypass both lists. static bool isRegionMgrAllowed(const char* cmd) { while (*cmd == ' ') cmd++; - return commandFamilyMatches(cmd, "region") - || commandFamilyMatches(cmd, "get flood.channel.scope") - || commandFamilyMatches(cmd, "set flood.channel.scope") - || commandFamilyMatches(cmd, "del flood.channel.scope") - || commandFamilyMatches(cmd, "get flood.channel.scope.require") - || commandFamilyMatches(cmd, "set flood.channel.scope.require") - || commandFamilyMatches(cmd, "del flood.channel.scope.require") - || isCommonManagerReadOnlyAllowed(cmd); + static const char* const families[] = { + "region", "get flood.channel.scope", "set flood.channel.scope", "del flood.channel.scope", + "get flood.channel.scope.require", "set flood.channel.scope.require", + "del flood.channel.scope.require", nullptr + }; + return commandListMatches(cmd, families) || isCommonManagerReadOnlyAllowed(cmd); } static bool isFilterMgrAllowed(const char* cmd) { while (*cmd == ' ') cmd++; - if (isCommonManagerReadOnlyAllowed(cmd)) return true; - if (commandFamilyMatches(cmd, "get repeat") - || commandFamilyMatches(cmd, "get loop.detect") - || commandFamilyMatches(cmd, "get flood.max") - || commandFamilyMatches(cmd, "get flood.channel.data") - || commandFamilyMatches(cmd, "get flood.advert") - || commandFamilyMatches(cmd, "get flood.filter") - || commandFamilyMatches(cmd, "get flood.filter.blacklist") + static const char* const families[] = { + "get repeat", "get loop.detect", "get flood.max", "get flood.channel.data", + "get flood.advert", "get flood.filter", "get flood.filter.blacklist", #if MESH_ENABLE_FLOOD_RULE_ENGINE - || commandFamilyMatches(cmd, "get flood.rule") - || commandFamilyMatches(cmd, "get fr") + "get flood.rule", "get fr", #endif - || commandFamilyMatches(cmd, "get flood.moderation")) return true; - // General payload/hop filters plus the remaining flood-hop gates. - return commandFamilyMatches(cmd, "set flood.filter") - || commandFamilyMatches(cmd, "del flood.filter") - || commandFamilyMatches(cmd, "set flood.filter.blacklist") - || commandFamilyMatches(cmd, "del flood.filter.blacklist") + "get flood.moderation", "set flood.filter", "del flood.filter", + "set flood.filter.blacklist", "del flood.filter.blacklist", #if MESH_ENABLE_FLOOD_RULE_ENGINE - || commandFamilyMatches(cmd, "set flood.rule") - || commandFamilyMatches(cmd, "set fr") - || commandFamilyMatches(cmd, "del flood.rule") - || commandFamilyMatches(cmd, "del fr") + "set flood.rule", "set fr", "del flood.rule", "del fr", #endif - || commandFamilyMatches(cmd, "set flood.moderation") - || commandFamilyMatches(cmd, "del flood.moderation") - || commandFamilyMatches(cmd, "clear flood.advert") - || commandFamilyMatches(cmd, "set flood.channel.data") - || commandFamilyMatches(cmd, "set flood.max") - || commandFamilyMatches(cmd, "set loop.detect") - || commandFamilyMatches(cmd, "set repeat"); + "set flood.moderation", "del flood.moderation", "clear flood.advert", + "set flood.channel.data", "set flood.max", "set loop.detect", "set repeat", nullptr + }; + return isCommonManagerReadOnlyAllowed(cmd) || commandListMatches(cmd, families); } bool MyMesh::handleReplayResetCommand(ClientInfo* sender, const char* command, diff --git a/scripts/check_firmware_ram.py b/scripts/check_firmware_ram.py index 0fb74fcce..a85bfc7e3 100644 --- a/scripts/check_firmware_ram.py +++ b/scripts/check_firmware_ram.py @@ -308,8 +308,8 @@ def requirements(platform, defines, target): 16384 if platform == "ESP32_PLATFORM" else 8192 if "OTA_SD_STORE" in defines else 4096) bounded_default = (platform == "ESP32_PLATFORM" - and "COMPANION_RADIO_FULL" in defines - and "BOARD_HAS_PSRAM" not in defines + and (("COMPANION_RADIO_FULL" in defines and "BOARD_HAS_PSRAM" not in defines) + or qualified_repeater) and 4 <= leaf_limit <= 16384) bounded = integer(defines, "OTA_BOUNDED_SOURCE_PROOFS", int(bounded_default)) if bounded not in (0, 1): diff --git a/src/helpers/CheckedSettingsFile.h b/src/helpers/CheckedSettingsFile.h index 7ff17d4e6..de1d4757e 100644 --- a/src/helpers/CheckedSettingsFile.h +++ b/src/helpers/CheckedSettingsFile.h @@ -16,19 +16,28 @@ namespace mesh { namespace storage { // Share the filesystem boundary between small settings records. Callers keep // their own format/version checks; no preference layout changes here. -MESH_SETTINGS_FILE_SHARED inline bool readFixedSettingsFile( +enum class FixedSettingsRead : uint8_t { Complete, WrongSize, Unreadable }; + +MESH_SETTINGS_FILE_SHARED inline FixedSettingsRead readFixedSettingsFileResult( FILESYSTEM* fs, const char* path, uint8_t* data, size_t size) { - if (!fs) return false; + if (!fs) return FixedSettingsRead::Unreadable; #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) File file(*fs); - if (!file.open(path, FILE_O_READ)) return false; + if (!file.open(path, FILE_O_READ)) return FixedSettingsRead::Unreadable; #else File file = fs->open(path, "r"); - if (!file) return false; + if (!file) return FixedSettingsRead::Unreadable; #endif - const bool complete = file.size() == size && file.read(data, size) == size; + const bool right_size = file.size() == size; + const bool complete = right_size && file.read(data, size) == size; file.close(); - return complete; + return !right_size ? FixedSettingsRead::WrongSize + : complete ? FixedSettingsRead::Complete : FixedSettingsRead::Unreadable; +} + +inline bool readFixedSettingsFile( + FILESYSTEM* fs, const char* path, uint8_t* data, size_t size) { + return readFixedSettingsFileResult(fs, path, data, size) == FixedSettingsRead::Complete; } #if defined(NRF52_PLATFORM) diff --git a/src/helpers/FleetCommand.cpp b/src/helpers/FleetCommand.cpp index 66f8f5a57..7f909f2ed 100644 --- a/src/helpers/FleetCommand.cpp +++ b/src/helpers/FleetCommand.cpp @@ -230,6 +230,54 @@ bool rowKey(const char* key, size_t length, const char* family) { return true; } +enum class CommandKind : uint8_t { + Filter, Row, Advert, Schedule, SecondarySchedule, Radio, RadioStatus, Cross, Setting +}; + +struct CommandFamily { + const char* name; + CommandKind kind; +}; + +// Match the signed namespace once. Row families alone accept a numeric suffix; +// sharing the lookup also keeps the sender's full authorization policy compact. +const CommandFamily* commandFamily(const char* key, size_t length) { + static const CommandFamily families[] = { + {"flood.filter", CommandKind::Filter}, + {"flood.rule", CommandKind::Filter}, + {"fr", CommandKind::Filter}, + {"flood.filter.blacklist", CommandKind::Row}, + {"fb", CommandKind::Row}, + {"flood.moderation", CommandKind::Row}, + {"fm", CommandKind::Row}, + {"flood.channel.scope", CommandKind::Row}, + {"fs", CommandKind::Row}, + {"flood.channel.scope.require", CommandKind::Row}, + {"fsr", CommandKind::Row}, + {"flood.advert", CommandKind::Advert}, + {"fa", CommandKind::Advert}, + {"radioat", CommandKind::Schedule}, + {"tempradioat", CommandKind::Schedule}, + {"radioat2", CommandKind::SecondarySchedule}, + {"tempradioat2", CommandKind::SecondarySchedule}, + {"radio2", CommandKind::Radio}, + {"tempradio2", CommandKind::Radio}, + {"radio2.status", CommandKind::RadioStatus}, + {"radio2.timing", CommandKind::RadioStatus}, + {"radio2.cross", CommandKind::Cross}, + {"flood.channel.data", CommandKind::Setting}, + {"flood.channel.data.hops", CommandKind::Setting}, + {"flood.max", CommandKind::Setting}, + {"flood.max.unscoped", CommandKind::Setting}, + {"flood.max.advert", CommandKind::Setting}, + }; + for (const auto& family : families) { + const bool row = family.kind == CommandKind::Filter || family.kind == CommandKind::Row; + if (row ? rowKey(key, length, family.name) : keyEquals(key, length, family.name)) return &family; + } + return nullptr; +} + // Regex punctuation is data only in the two content-pattern fields. Keep the // existing management-command character policy for every other argument. bool filterPatternCharactersAllowed(const char* text) { @@ -520,12 +568,14 @@ bool FleetCommand::commandAllowed(const char* command) { const char* arguments = separator ? separator + 1 : ""; if (!key_length || (separator && (!*arguments || *arguments == ' '))) return false; - const bool filter_rows = rowKey(key, key_length, "flood.filter") - || rowKey(key, key_length, "flood.rule") || rowKey(key, key_length, "fr"); + const CommandFamily* family = commandFamily(key, key_length); + if (!family) return false; + const CommandKind kind = family->kind; + const bool filter_rows = kind == CommandKind::Filter; if (pattern_characters && (!set || !filter_rows || !filterPatternCharactersAllowed(arguments))) return false; - if (keyEquals(key, key_length, "flood.advert") || keyEquals(key, key_length, "fa")) { + if (kind == CommandKind::Advert) { if (get) { if (!*arguments) return true; const bool detail = !strncmp(arguments, "key ", 4); @@ -542,46 +592,25 @@ bool FleetCommand::commandAllowed(const char* command) { } if (clear) return false; - const bool scheduled_primary = keyEquals(key, key_length, "radioat") - || keyEquals(key, key_length, "tempradioat"); - const bool scheduled_secondary = keyEquals(key, key_length, "radioat2") - || keyEquals(key, key_length, "tempradioat2"); - if (scheduled_primary || scheduled_secondary) { + const bool scheduled_secondary = kind == CommandKind::SecondarySchedule; + if (kind == CommandKind::Schedule || scheduled_secondary) { return set ? scheduledRadioTuple(arguments, key[0] == 't', scheduled_secondary) : (get || del) && scheduleSelector(arguments, scheduled_secondary); } - if (keyEquals(key, key_length, "radio2") - || keyEquals(key, key_length, "tempradio2")) { + if (kind == CommandKind::Radio) { return get ? !*arguments : set && radioTuple(arguments, key_length == 10); } - if (keyEquals(key, key_length, "radio2.status") - || keyEquals(key, key_length, "radio2.timing")) return get && !*arguments; - if (keyEquals(key, key_length, "radio2.cross")) { + if (kind == CommandKind::RadioStatus) return get && !*arguments; + if (kind == CommandKind::Cross) { return get ? !*arguments : set && (!strcmp(arguments, "on") || !strcmp(arguments, "off") || !strcmp(arguments, "auto")); } - const bool rows = filter_rows - || rowKey(key, key_length, "flood.filter.blacklist") - || rowKey(key, key_length, "fb") - || rowKey(key, key_length, "flood.moderation") - || rowKey(key, key_length, "fm") - || rowKey(key, key_length, "flood.channel.scope") - || rowKey(key, key_length, "fs") - || rowKey(key, key_length, "flood.channel.scope.require") - || rowKey(key, key_length, "fsr"); - if (rows) return get ? (!*arguments || (filter_rows - && !keyEquals(key, key_length, "flood.filter") - && !keyEquals(key, key_length, "flood.rule") - && !keyEquals(key, key_length, "fr") + if (filter_rows || kind == CommandKind::Row) return get ? (!*arguments || (filter_rows + && key_length > strlen(family->name) && (!strcmp(arguments, "sender") || !strcmp(arguments, "n") || !strcmp(arguments, "text") || !strcmp(arguments, "x")))) : set ? *arguments != 0 : del && (!*arguments || !strcmp(arguments, "all")); - const bool settings = keyEquals(key, key_length, "flood.channel.data") - || keyEquals(key, key_length, "flood.channel.data.hops") - || keyEquals(key, key_length, "flood.max") - || keyEquals(key, key_length, "flood.max.unscoped") - || keyEquals(key, key_length, "flood.max.advert"); - return settings && (get ? !*arguments : set && *arguments); + return kind == CommandKind::Setting && (get ? !*arguments : set && *arguments); } size_t FleetCommand::encode(const LocalIdentity& publisher, const uint8_t* key, diff --git a/src/helpers/FloodCommandAliases.h b/src/helpers/FloodCommandAliases.h index 270ce0d6b..35e46995b 100644 --- a/src/helpers/FloodCommandAliases.h +++ b/src/helpers/FloodCommandAliases.h @@ -6,32 +6,44 @@ namespace mesh { namespace cli { +inline bool floodCommandBoundary(char c) { + return c == 0 || c == ' ' || c == '.' || c == '\t' || c == '\r' || c == '\n'; +} + // Return the untouched argument suffix. Aliases do not expand the caller's // bounded command buffer or change the bytes covered by a fleet signature. inline const char* floodCommandArguments(const char* command, const char* family) { if (!command || !family) return nullptr; const size_t length = strlen(family); if (strncmp(command, family, length) == 0 - && (command[length] == 0 || command[length] == ' ' || command[length] == '.')) + && floodCommandBoundary(command[length])) return command + length; const char* name = strchr(family, ' '); if (!name) return nullptr; const size_t verb_length = size_t(++name - family); if (strncmp(command, family, verb_length) != 0) return nullptr; - const char* alias = nullptr; - if (strcmp(name, "flood.rule") == 0) alias = "fr"; - else if (strcmp(name, "flood.moderation") == 0) alias = "fm"; - else if (strcmp(name, "flood.channel.scope") == 0) alias = "fs"; - else if (strcmp(name, "flood.channel.scope.require") == 0) alias = "fsr"; - else if (strcmp(name, "flood.filter.blacklist") == 0) alias = "fb"; - else if (strcmp(name, "flood.advert") == 0) alias = "fa"; - if (!alias) return nullptr; - const size_t alias_length = strlen(alias); const char* key = command + verb_length; - if (strncmp(key, alias, alias_length) != 0) return nullptr; - const char* suffix = key + alias_length; - return *suffix == 0 || *suffix == ' ' || *suffix == '.' ? suffix : nullptr; + struct Alias { + const char* name; + const char* short_name; + }; + static const Alias aliases[] = { + {"flood.rule", "fr"}, + {"flood.moderation", "fm"}, + {"flood.channel.scope", "fs"}, + {"flood.channel.scope.require", "fsr"}, + {"flood.filter.blacklist", "fb"}, + {"flood.advert", "fa"}, + }; + for (const auto& alias : aliases) { + if (strcmp(name, alias.name) != 0) continue; + const size_t alias_length = strlen(alias.short_name); + if (strncmp(key, alias.short_name, alias_length) != 0) return nullptr; + const char* suffix = key + alias_length; + return floodCommandBoundary(*suffix) ? suffix : nullptr; + } + return nullptr; } } // namespace cli diff --git a/src/helpers/MorseSettings.cpp b/src/helpers/MorseSettings.cpp index 9263c65d5..581336790 100644 --- a/src/helpers/MorseSettings.cpp +++ b/src/helpers/MorseSettings.cpp @@ -1,6 +1,8 @@ #include "MorseSettings.h" #include "FilePresence.h" -#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) +#if defined(NRF52_PLATFORM) +#include +#elif defined(STM32_PLATFORM) #include "AtomicFileWriter.h" #elif defined(ESP32_PLATFORM) || defined(RP2040_PLATFORM) #include "ContactFileTransaction.h" @@ -22,19 +24,27 @@ ReadResult read(const char* path, MorseSettings& result) { bool present = false; if (!filePresence(settings_fs, path, present)) return ReadResult::Unreadable; if (!present) return ReadResult::Missing; -#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) + uint8_t image[MorseSettingsImageSize]; +#if defined(NRF52_PLATFORM) + const storage::FixedSettingsRead read = storage::readFixedSettingsFileResult( + settings_fs, path, image, sizeof(image)); + if (read != storage::FixedSettingsRead::Complete) + return read == storage::FixedSettingsRead::WrongSize + ? ReadResult::Invalid : ReadResult::Unreadable; +#else +#if defined(STM32_PLATFORM) File file(*settings_fs); if (!file.open(path, FILE_O_READ)) return ReadResult::Unreadable; #else File file = settings_fs->open(path, "r"); if (!file) return ReadResult::Unreadable; #endif - uint8_t image[MorseSettingsImageSize]; const bool right_size = file.size() == sizeof(image); const bool complete = right_size && file.read(image, sizeof(image)) == int(sizeof(image)); file.close(); if (!right_size) return ReadResult::Invalid; if (!complete) return ReadResult::Unreadable; +#endif return decodeMorseSettings(image, sizeof(image), result) ? ReadResult::Valid : ReadResult::Invalid; } diff --git a/src/helpers/ota/CompanionOtaConfig.cpp b/src/helpers/ota/CompanionOtaConfig.cpp index c941f31ea..e9f3b3ddc 100644 --- a/src/helpers/ota/CompanionOtaConfig.cpp +++ b/src/helpers/ota/CompanionOtaConfig.cpp @@ -1,6 +1,10 @@ #if defined(ENABLE_OTA) && (defined(COMPANION_RADIO_FULL) || COMPANION_FEATURE_OTA_CLI) #include "CompanionOtaConfig.h" #include +#if defined(NRF52_PLATFORM) +#include +#include +#endif #include #if defined(ESP32_PLATFORM) #include @@ -21,7 +25,9 @@ const char* const Backup = "/ota_config.bak"; // Some FS::exists implementations open the file and report an unreadable // existing image as absent. Metadata errors must not authorize replacement. bool pathPresence(const char* path, bool& present) { -#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) +#if defined(NRF52_PLATFORM) + return filePresence(settings_fs, path, present); +#elif defined(STM32_PLATFORM) struct lfs_info info; settings_fs->_lockFS(); const int result = lfs_stat(settings_fs->_getFS(), path, &info); @@ -77,9 +83,13 @@ ImageReadResult readImage(const char* path, uint8_t* image, OtaConfigState& stat if (!settings_fs || !pathPresence(path, present)) return ImageReadResult::Unreadable; if (!present) return ImageReadResult::Missing; #if defined(NRF52_PLATFORM) - File file(*settings_fs); - if (!file.open(path, FILE_O_READ)) return ImageReadResult::Unreadable; -#elif defined(STM32_PLATFORM) + const storage::FixedSettingsRead read = storage::readFixedSettingsFileResult( + settings_fs, path, image, ImageSize); + if (read != storage::FixedSettingsRead::Complete) + return read == storage::FixedSettingsRead::WrongSize + ? ImageReadResult::Invalid : ImageReadResult::Unreadable; +#else +#if defined(STM32_PLATFORM) File file = settings_fs->open(path, FILE_O_READ); #else File file = settings_fs->open(path, "r"); @@ -90,6 +100,7 @@ ImageReadResult readImage(const char* path, uint8_t* image, OtaConfigState& stat file.close(); if (!right_size) return ImageReadResult::Invalid; if (!complete) return ImageReadResult::Unreadable; +#endif return decode(image, state) ? ImageReadResult::Valid : ImageReadResult::Invalid; } diff --git a/src/helpers/ota/OtaMemoryPolicy.h b/src/helpers/ota/OtaMemoryPolicy.h index 7481414d6..113c7009d 100644 --- a/src/helpers/ota/OtaMemoryPolicy.h +++ b/src/helpers/ota/OtaMemoryPolicy.h @@ -1,10 +1,12 @@ #pragma once -// Non-PSRAM Full Companions retain the complete source geometry without -// retaining every Merkle leaf in internal RAM. An explicit 0 keeps the legacy +// Non-PSRAM Full Companions and qualified ESP32 repeaters retain the complete source +// geometry without caching every Merkle leaf. An explicit 0 keeps the legacy // cache for matched qualification builds. #ifndef OTA_BOUNDED_SOURCE_PROOFS -#if defined(ESP32_PLATFORM) && defined(COMPANION_RADIO_FULL) && !defined(BOARD_HAS_PSRAM) && \ +#if defined(ESP32_PLATFORM) && \ + ((defined(COMPANION_RADIO_FULL) && !defined(BOARD_HAS_PSRAM)) || \ + (defined(MESH_CLIENT_REPEATER_ONLY) && MESH_CLIENT_REPEATER_ONLY)) && \ (!defined(OTA_PROOFGEN_SCRATCH) || \ (OTA_PROOFGEN_SCRATCH >= 4 && OTA_PROOFGEN_SCRATCH <= 16384)) #define OTA_BOUNDED_SOURCE_PROOFS 1 diff --git a/src/helpers/ota/OtaSpeedConfig.cpp b/src/helpers/ota/OtaSpeedConfig.cpp index 2a2ad1609..04d33a37c 100644 --- a/src/helpers/ota/OtaSpeedConfig.cpp +++ b/src/helpers/ota/OtaSpeedConfig.cpp @@ -2,6 +2,10 @@ #include "OtaSpeedConfig.h" #include #include +#if defined(NRF52_PLATFORM) +#include +#include +#endif #include #include #if defined(ESP32_PLATFORM) @@ -26,7 +30,9 @@ enum class ImageReadResult : uint8_t { Missing, Valid, Invalid, Unreadable }; // A failed open is not enough to prove a path is absent: some filesystems // report an I/O error as a failed exists() call. Keep those images protected. bool pathPresence(const char* path, bool& present) { -#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) +#if defined(NRF52_PLATFORM) + return filePresence(settings_fs, path, present); +#elif defined(STM32_PLATFORM) struct lfs_info info; settings_fs->_lockFS(); const int result = lfs_stat(settings_fs->_getFS(), path, &info); @@ -61,9 +67,13 @@ ImageReadResult readImage(const char* path, uint8_t* image) { if (!settings_fs || !pathPresence(path, present)) return ImageReadResult::Unreadable; if (!present) return ImageReadResult::Missing; #if defined(NRF52_PLATFORM) - File file(*settings_fs); - if (!file.open(path, FILE_O_READ)) return ImageReadResult::Unreadable; -#elif defined(STM32_PLATFORM) + const storage::FixedSettingsRead read = storage::readFixedSettingsFileResult( + settings_fs, path, image, ImageSize); + if (read != storage::FixedSettingsRead::Complete) + return read == storage::FixedSettingsRead::WrongSize + ? ImageReadResult::Invalid : ImageReadResult::Unreadable; +#else +#if defined(STM32_PLATFORM) File file = settings_fs->open(path, FILE_O_READ); #else File file = settings_fs->open(path, "r"); @@ -74,6 +84,7 @@ ImageReadResult readImage(const char* path, uint8_t* image) { file.close(); if (!right_size) return ImageReadResult::Invalid; if (!complete) return ImageReadResult::Unreadable; +#endif float value; memcpy(&value, image + 4, sizeof(value)); return !memcmp(image, "OS\1\0", 4) && validSpeed(value) diff --git a/test/test_firmware_ram.py b/test/test_firmware_ram.py index 6c9a0ea9b..55d8f8748 100644 --- a/test/test_firmware_ram.py +++ b/test/test_firmware_ram.py @@ -667,7 +667,7 @@ int main() { }, "board_companion_radio_full") def test_compact_tables_keep_capacity_and_reserve_active_ota_allocations(self): - for platform, leaf_limit in (("NRF52_PLATFORM", 4096), ("ESP32_PLATFORM", 16384), + for platform, leaf_limit in (("NRF52_PLATFORM", 4096), ("ESP32_PLATFORM", 1536), ("RP2040_PLATFORM", 4096), ("STM32_PLATFORM", 4096)): with self.subTest(platform=platform): flags = {"ENABLE_OTA": 1, "MESH_CLIENT_REPEATER_ONLY": 1, @@ -704,6 +704,22 @@ int main() { self.assertEqual(alias["components"]["neighbor_history"], 2048 * 9 + 16) self.assertEqual(alias["components"]["radio_packet_pool"], 10240) + def test_qualified_esp32_repeaters_stream_source_proofs_without_cutting_other_budgets(self): + for psram in (False, True): + flags = {"ENABLE_OTA": 1, "MESH_CLIENT_REPEATER_ONLY": 1, + "WITH_MQTT_BRIDGE": 1, "WIFI_SSID": "test", "MAX_CLIENTS": 32} + if psram: + flags["BOARD_HAS_PSRAM"] = 1 + with self.subTest(psram=psram): + legacy = ram.requirements("ESP32_PLATFORM", { + **flags, "OTA_BOUNDED_SOURCE_PROOFS": 0}, "board_repeater")["components"] + selected = ram.requirements("ESP32_PLATFORM", flags, "board_repeater")["components"] + expected = 1536 + self.assertEqual(selected["ota_generic_source_buffers"], expected + 2048 + 32) + for name, amount in legacy.items(): + if name != "ota_generic_source_buffers": + self.assertEqual(selected[name], amount) + def test_combined_rak_oled_bound_does_not_change_other_display_budgets(self): defines = { "RAK_4631": 1, "ETHERNET_ENABLED": 1, "OTA_RAK_AUTO_STORE": 1, @@ -732,7 +748,7 @@ int main() { } full = ram.requirements("ESP32_PLATFORM", defines, "Heltec_v3_repeater_observer_mqtt") parts = full["components"] - self.assertEqual(full["required_heap_bytes"], 218768) + self.assertEqual(full["required_heap_bytes"], 203920) self.assertEqual(parts["display_pixels_and_driver"], 2048) self.assertEqual(parts["uart_bridge_and_driver"], 3072 + 4096) self.assertEqual(parts["wireless_stacks"], 49152) diff --git a/test/test_fleet_command_crypto.py b/test/test_fleet_command_crypto.py index 1e9e69cde..32110e69f 100644 --- a/test/test_fleet_command_crypto.py +++ b/test/test_fleet_command_crypto.py @@ -422,6 +422,32 @@ class FleetCommandCryptoTests(unittest.TestCase): with self.subTest(command=command): self.assertEqual(self.decode(envelope(command)), "reject") + def test_every_row_family_retains_suffix_bounds_and_detail_permissions(self): + filters = ("flood.filter", "flood.rule", "fr") + rows = filters + ("flood.filter.blacklist", "fb", "flood.moderation", "fm", + "flood.channel.scope", "fs", "flood.channel.scope.require", "fsr") + for family in rows: + for suffix in ("", ".1", ".255"): + command = f"get {family}{suffix}" + with self.subTest(command=command): + self.assertEqual(self.decode(envelope(command)), f"{NOW}\n{NOW + 120}\n{command}") + for suffix in (".0", ".01", ".256", ".9999999999", ".1.secret", "x"): + command = f"get {family}{suffix}" + with self.subTest(command=command): + self.assertEqual(self.decode(envelope(command)), "reject") + for detail in ("sender", "n", "text", "x"): + for suffix in ("", ".1"): + command = f"get {family}{suffix} {detail}" + expected = f"{NOW}\n{NOW + 120}\n{command}" if family in filters and suffix else "reject" + with self.subTest(command=command): + self.assertEqual(self.decode(envelope(command)), expected) + for family in ("flood.channel.data", "flood.channel.data.hops", "flood.max", + "flood.max.unscoped", "flood.max.advert", "radio2.status", "radio2.timing"): + for suffix in (".1", ".255", ".secret"): + command = f"get {family}{suffix}" + with self.subTest(command=command): + self.assertEqual(self.decode(envelope(command)), "reject") + def test_scheduled_radio_commands_have_cross_language_real_signatures(self): for command in [ "set radioat 910.5,500,5,5,+1,auto", diff --git a/test/test_ota_source_proofs.py b/test/test_ota_source_proofs.py index dfeb6b0d9..c6e2cc9f8 100644 --- a/test/test_ota_source_proofs.py +++ b/test/test_ota_source_proofs.py @@ -23,13 +23,28 @@ static_assert(OTA_PROOFGEN_SCRATCH / 4 == 4095, "odd byte limit lost usable leaf #endif """ base = [os.environ.get("CXX", "g++"), "-std=c++17", "-fsyntax-only", "-x", "c++", "-", - "-I", str(ROOT / "src"), "-DESP32_PLATFORM=1", "-DCOMPANION_RADIO_FULL=1"] - for scratch in (None, 0, 1, 3, 4, 5, 16383, 16384, 16385, 32768): - bounded = scratch is None or 4 <= scratch <= 16384 - flags = [] if scratch is None else [f"-DOTA_PROOFGEN_SCRATCH={scratch}"] - with self.subTest(scratch=scratch): - result = subprocess.run([*base, *flags, f"-DEXPECT_BOUNDED={int(bounded)}"], - input=source, text=True, capture_output=True, timeout=30) + "-I", str(ROOT / "src"), "-DESP32_PLATFORM=1"] + for role in ("COMPANION_RADIO_FULL", "MESH_CLIENT_REPEATER_ONLY"): + for scratch in (None, 0, 1, 3, 4, 5, 16383, 16384, 16385, 32768): + bounded = scratch is None or 4 <= scratch <= 16384 + flags = [f"-D{role}=1"] + if scratch is not None: + flags.append(f"-DOTA_PROOFGEN_SCRATCH={scratch}") + with self.subTest(role=role, scratch=scratch): + result = subprocess.run([*base, *flags, f"-DEXPECT_BOUNDED={int(bounded)}"], + input=source, text=True, capture_output=True, timeout=30) + self.assertEqual(result.returncode, 0, result.stderr) + # Repeater proof buffers are internal even when the board has PSRAM. + result = subprocess.run([*base, "-DMESH_CLIENT_REPEATER_ONLY=1", + "-DBOARD_HAS_PSRAM=1", "-DEXPECT_BOUNDED=1"], + input=source, text=True, capture_output=True, timeout=30) + self.assertEqual(result.returncode, 0, result.stderr) + for flags in ([], ["-DMESH_CLIENT_REPEATER_ONLY=0"], + ["-DCOMPANION_RADIO_FULL=1", "-DBOARD_HAS_PSRAM=1"], + ["-DMESH_CLIENT_REPEATER_ONLY=1", "-DOTA_BOUNDED_SOURCE_PROOFS=0"]): + with self.subTest(legacy=flags): + result = subprocess.run([*base, *flags, "-DEXPECT_BOUNDED=0"], input=source, + text=True, capture_output=True, timeout=30) self.assertEqual(result.returncode, 0, result.stderr) for scratch in (0, 3, 16385): with self.subTest(forced_invalid=scratch): diff --git a/test/test_ota_speed.py b/test/test_ota_speed.py index d4e5bb680..572a9b709 100644 --- a/test/test_ota_speed.py +++ b/test/test_ota_speed.py @@ -98,6 +98,60 @@ class OtaSpeedTest(unittest.TestCase): self.assertEqual(result.returncode, 0, result.stderr) subprocess.run([str(binary)], check=True) + def test_nrf52_metadata_open_and_short_reads_preserve_committed_speed(self): + mock = (ROOT / "test/fixtures/radio_profiles/mocks/helpers/IdentityStore.h").read_text() + mock = mock.replace("bool fail_write = false;", "bool stat_error=false;\n bool fail_write = false;") + mock = mock.replace("bool mkdir(const char*)", "void _lockFS() {}\n void _unlockFS() {}\n" + " MemoryFS* _getFS() { return this; }\n" + " bool mkdir(const char*)") + mock += ("\nstruct lfs_info {};\nconstexpr int LFS_ERR_NOENT=-2;\n" + "inline int lfs_stat(MemoryFS* fs, const char* path, lfs_info*) {\n" + " if (fs->stat_error) return -5;\n" + " return fs->files.count(path) ? 0 : LFS_ERR_NOENT;\n}\n") + faults = r''' + const auto committed = fs.files; + for (unsigned fault = 0; fault < 3; ++fault) { + fs.stat_error = fault == 0; + fs.fail_read_open = fault == 1; + fs.fail_read_after = fault == 2 ? 4 : -1; + beginSpeedConfig(&fs); + assert(speedFactor() == 1.0f); // Unreadable is not a corrupt image to replace. + command("set ota.speed 2", "ERR"); + assert(fs.files == committed); + fs.stat_error = fs.fail_read_open = false; + fs.fail_read_after = -1; + beginSpeedConfig(&fs); + assert(speedFactor() == 1.125f); + } + fs.files["/ota_speed"].pop_back(); // A complete, wrong-sized image is invalid. + beginSpeedConfig(&fs); + assert(speedFactor() == 1.0f); + command("set ota.speed 2", "OK ota.speed=2x (saved)"); + beginSpeedConfig(&fs); + assert(speedFactor() == 2.0f); +''' + source = HARNESS.replace(" assert(speedFactor() == 1.125f);\n}", + " assert(speedFactor() == 1.125f);\n" + faults + "}") + compiler = shutil.which("g++") or shutil.which("clang++") + self.assertIsNotNone(compiler) + with tempfile.TemporaryDirectory() as directory: + work = Path(directory) + (work / "helpers").mkdir() + (work / "helpers/IdentityStore.h").write_text(mock) + cpp = work / "speed.cpp" + cpp.write_text(source) + binary = work / "speed" + result = subprocess.run([ + compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", "-Wno-sign-compare", + "-DENABLE_OTA=1", "-DNRF52_PLATFORM=1", "-I", directory, + "-I", str(ROOT / "test/fixtures/radio_profiles/mocks"), + "-I", str(ROOT / "test/mocks"), "-I", str(ROOT / "src"), + str(ROOT / "src/helpers/ota/OtaSpeedConfig.cpp"), str(cpp), + "-o", str(binary), + ], text=True, capture_output=True) + self.assertEqual(result.returncode, 0, result.stderr) + subprocess.run([str(binary)], check=True) + def test_shared_role_wiring_and_idle_context_access(self): common = (ROOT / "src/helpers/CommonCLI.cpp").read_text() companion = (ROOT / "examples/companion_radio/MyMesh.cpp").read_text() diff --git a/test/test_transport_filter_wiring.py b/test/test_transport_filter_wiring.py index 27bde3e71..e1ac3c623 100644 --- a/test/test_transport_filter_wiring.py +++ b/test/test_transport_filter_wiring.py @@ -237,6 +237,23 @@ void run(const char* command,char* reply) { else strcpy(reply,"unhandled"); } int main() { + // Preserve the advert CLI's whitespace delimiters for both full names and + // aliases, returning the original bytes for the caller's own validation. + struct Boundary {const char* command;const char* suffix;}; + for(const auto& row : { + Boundary{"get flood.advert", ""}, Boundary{"get fa", ""}, + Boundary{"get flood.advert\t2\r\n", "\t2\r\n"}, + Boundary{"get fa\t2\r\n", "\t2\r\n"}, + Boundary{"get flood.advert\r\n", "\r\n"}, + Boundary{"get fa\n", "\n"}, Boundary{"get fa 2", " 2"}, + Boundary{"get fa.2", ".2"}}) { + const char* suffix=mesh::cli::floodCommandArguments(row.command,"get flood.advert"); + assert(suffix==row.command+strlen(row.command)-strlen(row.suffix)); + assert(!strcmp(suffix,row.suffix)); + } + for(const char* command : {"get flood.advertisement", "get fax", "get fa/2", + "get fa=2", "get fa\v2", "get fa\f2", "get\tfa 2"}) + assert(!mesh::cli::floodCommandArguments(command,"get flood.advert")); struct Test {const char* command;const char* suffix;const char* action;}; for(const auto& row : { Test{"get fm.2",".2","formatFloodGroupModeration"}, @@ -287,11 +304,41 @@ int main() { def test_shorthand_uses_the_existing_filter_manager_permissions(self): source = (ROOT/'examples/simple_repeater/MyMesh.cpp').read_text() helpers = '\n'.join(method(source, signature) for signature in [ - 'static bool commandFamilyMatches(', 'static bool isCommonManagerReadOnlyAllowed(', + 'static bool commandFamilyMatches(', 'static bool commandListMatches(', + 'static bool isCommonManagerReadOnlyAllowed(', 'static bool isRegionMgrAllowed(', 'static bool isFilterMgrAllowed(']) code = (f'#include "{(ROOT/"src/helpers/FloodCommandAliases.h").as_posix()}"\n' '#include \n#include \n#include \n') + helpers + r''' int main() { + // Every common namespace retains both manager roles; sensitive get namespaces + // and partial command names stay denied. Clock deliberately requires equality. + for (const char* cmd : {"ver", "board", "neighbors", "sensor get", "sensor list", + "get role", "get public.key", "get clock.sync", "get battery.alert", + "get rx.watchdog", "get recent.repeater", "get recent.repeaters", + "get outpath", "get altpath", "clock", " get public.key"}) { + assert(isFilterMgrAllowed(cmd)); + assert(isRegionMgrAllowed(cmd)); + } + for (const char* cmd : {"clock secret", "version", "get rolepassword", "get public.keys", + "get admin.password", "get guest.password", "get wifi.password", "get bridge.secret"}) { + assert(!isFilterMgrAllowed(cmd)); + assert(!isRegionMgrAllowed(cmd)); + } + for (const char* cmd : {"get repeat", "get loop.detect", "get flood.max", + "get flood.channel.data", "get flood.advert", "get flood.filter", + "get flood.filter.blacklist", "get flood.moderation", "set flood.filter", + "del flood.filter", "set flood.filter.blacklist", "del flood.filter.blacklist", + "set flood.moderation", "del flood.moderation", "clear flood.advert", + "set flood.channel.data", "set flood.max", "set loop.detect", "set repeat"}) { + assert(isFilterMgrAllowed(cmd)); + assert(!isRegionMgrAllowed(cmd)); + } + for (const char* cmd : {"region", "get flood.channel.scope", "set flood.channel.scope", + "del flood.channel.scope", "get flood.channel.scope.require", + "set flood.channel.scope.require", "del flood.channel.scope.require"}) { + assert(isRegionMgrAllowed(cmd)); + assert(!isFilterMgrAllowed(cmd)); + } for (const char* cmd : {"get fr", "get fr.3", "set fr.3 any m=bc d", "del fr.3", "del fr all", "set fr.3 any c=public sender=^Alice$ text=BEACON len=1-80 prob=50 drop", "set flood.rule.3 type=grp_txt channel=public sender=^Alice$ text=BEACON drop",