From 30b18beecdc5ce5d7f0890c313949e10bd6a178f Mon Sep 17 00:00:00 2001 From: mikecarper Date: Sat, 3 Oct 2026 18:26:26 -0700 Subject: [PATCH] Harden management report persistence and CLI input handling --- src/helpers/CommonCLI.h | 2 + src/helpers/CommonCLI_Management.cpp | 25 ++- src/helpers/ManagementReporter.cpp | 43 +++-- test/fixtures/management/CommonCLI.h | 11 +- test/fixtures/management/route_test.cpp | 194 ++++++++++++++++++++++ test/fixtures/management/runtime_test.cpp | 107 +++++++++++- test/fixtures/management/wrapper_test.cpp | 177 ++++++++++++++++++++ test/test_management_cli_wrapper.py | 79 +++++++++ 8 files changed, 620 insertions(+), 18 deletions(-) create mode 100644 test/fixtures/management/route_test.cpp create mode 100644 test/fixtures/management/wrapper_test.cpp create mode 100644 test/test_management_cli_wrapper.py diff --git a/src/helpers/CommonCLI.h b/src/helpers/CommonCLI.h index ade157f14..0d77242ea 100644 --- a/src/helpers/CommonCLI.h +++ b/src/helpers/CommonCLI.h @@ -850,6 +850,8 @@ public: bool resolveDataTxScope(TransportKey& scope, char* resolved_name = nullptr, size_t resolved_name_size = 0, bool* ambiguous = nullptr) const; + // True if a saved shared route already takes precedence or the legacy path + // was committed. False means migration failed and reporting must stop. bool adoptLegacyDataTxPath(const uint8_t* path, uint8_t path_len); bool setDataTxPath(const char* spec, char* reply, size_t reply_size); bool saveCommonPrefs(); diff --git a/src/helpers/CommonCLI_Management.cpp b/src/helpers/CommonCLI_Management.cpp index f845b5d6e..f538bf835 100644 --- a/src/helpers/CommonCLI_Management.cpp +++ b/src/helpers/CommonCLI_Management.cpp @@ -89,7 +89,10 @@ static bool parseDataPath(char* raw, uint8_t path[MAX_PATH_SIZE], char* token = spec; while (token && *token) { char* comma = strchr(token, ','); - if (comma) *comma = 0; + if (comma) { + if (!comma[1]) { error = "Err - missing path hash after comma"; return false; } + *comma = 0; + } token = trimDataRoute(token); const size_t chars = strlen(token); const uint8_t token_width = uint8_t(chars / 2); @@ -275,8 +278,11 @@ bool CommonCLI::resolveDataTxScope(TransportKey& scope, char* name, } bool CommonCLI::adoptLegacyDataTxPath(const uint8_t* path, uint8_t path_len) { - if (!_data_route || !_data_route->healthy || _data_route->persisted - || path_len == OUT_PATH_UNKNOWN || !mesh::Packet::isValidPathLen(path_len) + if (!_data_route || !_data_route->healthy) return false; + // A saved shared route is authoritative, including an explicit "none". + // This is successful migration, not a storage failure to fault on. + if (_data_route->persisted) return true; + if (path_len == OUT_PATH_UNKNOWN || !mesh::Packet::isValidPathLen(path_len) || ((path_len & 63) && !path)) return false; const uint8_t previous_len = _data_route->path_len; uint8_t previous[MAX_PATH_SIZE]; memcpy(previous, _data_route->path, sizeof(previous)); @@ -373,7 +379,10 @@ bool CommonCLI::handleManagementCommand(char* command, char* reply) { } if (!strncmp(command, "set mgmt.path ", 14)) { char alias[160]; - snprintf(alias, sizeof(alias), "set data.tx path %s", command + 14); + const int length = snprintf(alias, sizeof(alias), "set data.tx path %s", command + 14); + if (length < 0 || size_t(length) >= sizeof(alias)) { + strcpy(reply, "Err - data.tx path too long"); return true; + } return handleDataTxCommand(alias, reply); } if (strcmp(command, "get mgmt") && strncmp(command, "get mgmt.", 9) @@ -383,7 +392,13 @@ bool CommonCLI::handleManagementCommand(char* command, char* reply) { *_management_mesh, *_board, *_sensors, *_acl, *_prefs, *_callbacks, *this, _management_fs); } - if (!_management) { strcpy(reply, "ERR: management unavailable"); return true; } + if (!_management) { + // No reporter can consume/wipe the value when initialization or its + // nothrow allocation failed. Apply the same recognized-password hygiene. + if (!strncmp(command, "set mgmt.password ", 18)) + mesh::management::erase(command + 18, strlen(command + 18)); + strcpy(reply, "ERR: management unavailable"); return true; + } if (!_management->command(command, reply, 160)) strcpy(reply, "ERR: unknown management setting"); return true; } diff --git a/src/helpers/ManagementReporter.cpp b/src/helpers/ManagementReporter.cpp index 0c5b4eea1..5ff817362 100644 --- a/src/helpers/ManagementReporter.cpp +++ b/src/helpers/ManagementReporter.cpp @@ -3,7 +3,6 @@ #include "FileRead.h" #include "PersistentStoreFormat.h" #include -#include #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) #include "AtomicFileWriter.h" #else @@ -22,6 +21,19 @@ namespace mesh { using namespace management; static constexpr char STATE_FILE[] = "/management"; static constexpr size_t STATE_SIZE = 120; +// Return zero for malformed/out-of-range input before any integer can wrap. +// Signs, whitespace and suffixes are not decimal day counts; leading zeroes +// remain valid. All management intervals have the same upper bound. +static unsigned parseDays(const char* text) { + unsigned days = 0; + if (!*text) return 0; + do { + if (*text < '0' || *text > '9') return 0; + days = days * 10 + (*text++ - '0'); + if (days > 90) return 0; + } while (*text); + return days; +} struct ManagementReporter::Working { History history; Extrema during_report; @@ -70,9 +82,13 @@ bool ManagementReporter::load() { !direct_valid || !flood_valid || (b[4] && (!b[6] || (!b[5] && !b[7]))) || read32(b + 116) != storage::updateCRC32(0xffffffff, b, 116) || read32(b + 104) > 90 * DAY + 3600 || read32(b + 108) > 90 * DAY + 3600) continue; + // Do not discard the only durable legacy route in a later MGC2 checkpoint + // unless the shared route was successfully adopted (or already saved). + if (legacy && b[7] != OUT_PATH_UNKNOWN && !common_cli.adoptLegacyDataTxPath(b + 8, b[7])) { + erase(b, sizeof(b)); return false; + } enabled = b[4]; direct_days = b[5]; keyed = b[6]; flood_days = legacy ? (direct_days < 21 ? 21 : direct_days) : b[7]; - if (legacy && b[7] != OUT_PATH_UNKNOWN) common_cli.adoptLegacyDataTxPath(b + 8, b[7]); memcpy(key, b + 72, 32); schedule.direct = read32(b + 104); schedule.flood = read32(b + 108); if (!direct_days) schedule.direct = 0; @@ -320,9 +336,10 @@ bool ManagementReporter::command(char* command, char* reply, size_t size) { return true; } ++value; - if (!healthy) { snprintf(reply, size, "ERR: management state fault; repair storage/reboot first"); return true; } - // Failed persistence stops reporting; no uncommitted state is transmitted. - cancel(); + if (!healthy) { + if (!strncmp(command, "set mgmt.password ", 18)) erase(value, strlen(value)); + snprintf(reply, size, "ERR: management state fault; repair storage/reboot first"); return true; + } if (!strncmp(command, "set mgmt.password ", 18)) { const size_t len = strlen(value); if (len < 12 || len > 96) { snprintf(reply, size, "ERR: password must be 12..96 bytes"); erase(value, len); return true; } @@ -352,9 +369,9 @@ bool ManagementReporter::command(char* command, char* reply, size_t size) { } direct_days = 0; schedule.direct = 0; } else { - char* end; const unsigned long n = strtoul(value, &end, 10); + const unsigned n = parseDays(value); const uint8_t* route_path = nullptr; uint8_t route_path_len = OUT_PATH_UNKNOWN; - if (!*value || *end || !Schedule::validDirect(n) + if (!Schedule::validDirect(n) || !common_cli.getDataTxPath(route_path, route_path_len)) { snprintf(reply, size, "ERR: direct needs data.tx path and 5..90 days"); return true; } @@ -367,19 +384,19 @@ bool ManagementReporter::command(char* command, char* reply, size_t size) { } flood_days = 0; schedule.flood = 0; } else { - char* end; const unsigned long n = strtoul(value, &end, 10); + const unsigned n = parseDays(value); TransportKey scope; - if (!*value || *end || !Schedule::validFlood(n) + if (!Schedule::validFlood(n) || !common_cli.resolveDataTxScope(scope)) { snprintf(reply, size, "ERR: flood needs data.tx region and 21..90 days"); return true; } flood_days = n; schedule.flood = flood_days * DAY; } } else if (!strncmp(command, "set mgmt.interval ", 18)) { - char* end; const unsigned long n = strtoul(value, &end, 10); + const unsigned n = parseDays(value); const uint8_t* route_path = nullptr; uint8_t route_path_len = OUT_PATH_UNKNOWN; TransportKey scope; - if (!*value || *end || !Schedule::validDirect(n) + if (!Schedule::validDirect(n) || !common_cli.getDataTxPath(route_path, route_path_len) || !common_cli.resolveDataTxScope(scope)) { snprintf(reply, size, "ERR: interval needs path, region and 5..90 days"); return true; @@ -388,6 +405,10 @@ bool ManagementReporter::command(char* command, char* reply, size_t size) { direct_days = n; flood_days = n < 21 ? 21 : n; schedule.direct = direct_days * DAY; schedule.flood = flood_days * DAY; } else { snprintf(reply, size, "ERR: unknown management setting"); return true; } + // Only accepted changes abandon a frozen report. Invalid commands must not + // discard its remaining pages after the multi-day budget was reserved. + // Failed persistence still stops reporting; no uncommitted state leaves. + cancel(); if (!save()) { healthy = false; snprintf(reply, size, "ERR: save failed; reporting stopped"); return true; } if (!enabled) { delete work; work = nullptr; } snprintf(reply, size, "OK"); return true; diff --git a/test/fixtures/management/CommonCLI.h b/test/fixtures/management/CommonCLI.h index e47558d83..86517e792 100644 --- a/test/fixtures/management/CommonCLI.h +++ b/test/fixtures/management/CommonCLI.h @@ -7,6 +7,8 @@ class CommonCLI { public: uint8_t path_len = 0; bool scope_available = true; + bool adoption_succeeds = true, route_persisted = false; + unsigned adoption_calls = 0; bool getDataTxPath(const uint8_t*& out, uint8_t& len) const { out = path; len = path_len; return path_len != 0xff && mesh::Packet::isValidPathLen(path_len); @@ -16,7 +18,14 @@ public: if (ambiguous) *ambiguous = false; scope = TransportKey(); return scope_available; } - bool adoptLegacyDataTxPath(const uint8_t*, uint8_t) { return true; } + bool adoptLegacyDataTxPath(const uint8_t* legacy_path, uint8_t legacy_len) { + ++adoption_calls; + if (route_persisted) return true; // the explicitly saved shared route wins + if (!adoption_succeeds) return false; + path_len = mesh::Packet::copyPath(path, legacy_path, legacy_len); + route_persisted = true; + return true; + } }; class CommonCLICallbacks { public: diff --git a/test/fixtures/management/route_test.cpp b/test/fixtures/management/route_test.cpp new file mode 100644 index 000000000..7ad5ba543 --- /dev/null +++ b/test/fixtures/management/route_test.cpp @@ -0,0 +1,194 @@ +#include "IdentityStore.h" +#include +#include "ContactFileTransaction.h" +#include "PersistentStoreFormat.h" +#include +#include +#include +#include + +class RegionMap; +namespace mesh { struct DataRouteState; } + +class CommonCLI { +public: + mesh::DataRouteState* _data_route = nullptr; + bool adoptLegacyDataTxPath(const uint8_t* path, uint8_t path_len); +}; + +// Generated from the production source by the host test runner. Includes the +// real route state, save helper, and adoption method rather than copies of their +// behavior. The filesystem and packet boundary use the existing host fixtures. +#include "production_route.inc" + +struct Fixture { + MemoryFS fs; + mesh::DataRouteState route; + CommonCLI cli; + + Fixture() { + route.fs = &fs; + cli._data_route = &route; + } +}; + +static void verifySavedRoute(const Fixture& f) { + assert(f.route.persisted); + const auto& data = f.fs.files.at("/data_tx"); + assert(data.size() == 108); + assert(!memcmp(data.data(), "DTX1", 4)); + assert(data[4] == f.route.path_len && data[5] == f.route.region_mode); + assert(data[6] == 0 && data[7] == 0 && data[103] == 0); + assert(!memcmp(data.data() + 8, f.route.path, MAX_PATH_SIZE)); + assert(!memcmp(data.data() + 72, f.route.region, sizeof(f.route.region))); + assert(mesh::storage::readLE32(data.data() + 104) + == mesh::storage::updateCRC32(0xffffffff, data.data(), 104)); + assert(!f.fs.exists("/data_tx.tmp") && !f.fs.exists("/data_tx.bak")); +} + +static void checkPersistedRouteWins() { + const uint8_t legacy[] = {0x12, 0x34, 0x56, 0x78}; + for (const uint8_t explicit_len : {uint8_t(0), uint8_t(0x42), + uint8_t(OUT_PATH_UNKNOWN)}) { + Fixture f; + f.route.path_len = explicit_len; + if (explicit_len == 0x42) memcpy(f.route.path, "\xab\xcd\xef\x01", 4); + f.route.region_mode = mesh::DATA_REGION_NAMED; + strcpy(f.route.region, "shared-scope"); + assert(mesh::dataRouteSave(f.route)); + verifySavedRoute(f); + const auto saved_files = f.fs.files; + const mesh::DataRouteState previous = f.route; + + // A configured shared path, including an explicit none, takes precedence + // over a legacy path. Legacy none must also leave that shared route intact. + assert(f.cli.adoptLegacyDataTxPath(legacy, 0x42)); + assert(f.cli.adoptLegacyDataTxPath(nullptr, OUT_PATH_UNKNOWN)); + assert(f.cli.adoptLegacyDataTxPath(nullptr, 0)); + assert(f.route.path_len == previous.path_len); + assert(!memcmp(f.route.path, previous.path, sizeof(f.route.path))); + assert(f.route.region_mode == previous.region_mode); + assert(!memcmp(f.route.region, previous.region, sizeof(f.route.region))); + assert(f.route.healthy && f.route.persisted); + assert(f.fs.files == saved_files); + } +} + +static void checkSuccessfulMigration() { + const uint8_t legacy[] = {0x12, 0x34, 0x56, 0x78, 0x9a, 0xbc}; + for (const uint8_t path_len : {uint8_t(0), uint8_t(2), uint8_t(0x42), + uint8_t(0x82)}) { + Fixture f; + memset(f.route.path, 0xa5, sizeof(f.route.path)); + assert(f.cli.adoptLegacyDataTxPath(path_len ? legacy : nullptr, path_len)); + assert(f.route.path_len == path_len && f.route.healthy); + const size_t bytes = (path_len & 63) * ((path_len >> 6) + 1); + assert(!memcmp(f.route.path, legacy, bytes)); + for (size_t i = bytes; i < sizeof(f.route.path); ++i) assert(f.route.path[i] == 0); + assert(f.route.region_mode == mesh::DATA_REGION_AUTO && !f.route.region[0]); + verifySavedRoute(f); + } +} + +static void checkIsolatedPublishFailureAndRetry() { + Fixture f; + f.route.path_len = 1; + memset(f.route.path, 0xa5, sizeof(f.route.path)); + f.route.region_mode = mesh::DATA_REGION_NAMED; + strcpy(f.route.region, "shared-scope"); + f.fs.files["/management"] = std::vector(120, 0); + memcpy(f.fs.files["/management"].data(), "MGC1", 4); + const auto saved_files = f.fs.files; + const mesh::DataRouteState previous = f.route; + const uint8_t legacy[] = {0x12, 0x34, 0x56, 0x78}; + + f.fs.fail_rename_from = {"/data_tx.tmp"}; + assert(!f.cli.adoptLegacyDataTxPath(legacy, 0x42)); + assert(f.route.path_len == previous.path_len && !f.route.persisted); + assert(!memcmp(f.route.path, previous.path, sizeof(f.route.path))); + assert(f.route.region_mode == previous.region_mode); + assert(!memcmp(f.route.region, previous.region, sizeof(f.route.region))); + assert(f.route.healthy); + assert(f.fs.files == saved_files); + + // The failed /data_tx publish is isolated: a later /management transaction + // can succeed. The production loader must therefore act on adoption failure + // before allowing a management checkpoint to discard the legacy route. + uint8_t management[120] = {}; + memcpy(management, "MGC2", 4); + mesh::ContactFileTransaction writer(&f.fs, "/management"); + assert(writer && writer.write(management, sizeof(management)) == sizeof(management)); + assert(writer.commit()); + assert(f.fs.files.at("/management") + == std::vector(management, management + sizeof(management))); + assert(!f.fs.exists("/data_tx")); + assert(!f.fs.exists("/management.tmp") && !f.fs.exists("/management.bak")); + + f.fs.fail_rename_from.clear(); + assert(f.cli.adoptLegacyDataTxPath(legacy, 0x42)); + verifySavedRoute(f); + assert(f.route.path_len == 0x42 && !memcmp(f.route.path, legacy, sizeof(legacy))); +} + +static void checkUnavailableUnhealthyAndInvalidRoutes() { + const uint8_t legacy[MAX_PATH_SIZE] = {}; + CommonCLI unavailable; + assert(!unavailable.adoptLegacyDataTxPath(legacy, 1)); + + for (const bool persisted : {false, true}) { + Fixture f; + f.route.healthy = false; + f.route.persisted = persisted; + assert(!f.cli.adoptLegacyDataTxPath(legacy, 1)); + assert(f.fs.files.empty() && f.route.path_len == 0); + } + + for (const uint8_t invalid : {uint8_t(0xc0), uint8_t(0x61), uint8_t(0x96)}) { + Fixture f; + assert(!f.cli.adoptLegacyDataTxPath(legacy, invalid)); + assert(f.fs.files.empty() && f.route.path_len == 0 && !f.route.persisted); + } + Fixture missing_path; + assert(!missing_path.cli.adoptLegacyDataTxPath(nullptr, 1)); + assert(missing_path.fs.files.empty() && !missing_path.route.persisted); +} + +static void checkPathParsing() { + struct Case { const char* text; uint8_t encoded; const char* bytes; }; + for (const auto& valid : {Case{"direct", 0, ""}, Case{" none ", OUT_PATH_UNKNOWN, ""}, + Case{"clear", OUT_PATH_UNKNOWN, ""}, Case{"-", OUT_PATH_UNKNOWN, ""}, + Case{"1:12ab", 2, "\x12\xab"}, Case{" 12, AB ", 2, "\x12\xab"}, + Case{"2:12abcd34", 0x42, "\x12\xab\xcd\x34"}, + Case{"12ab, cd34", 0x42, "\x12\xab\xcd\x34"}, + Case{"3:123456abcdef", 0x82, "\x12\x34\x56\xab\xcd\xef"}}) { + std::vector text(valid.text, valid.text + strlen(valid.text) + 1); + uint8_t path[MAX_PATH_SIZE], encoded = OUT_PATH_UNKNOWN; + const char* error = nullptr; + assert(mesh::parseDataPath(text.data(), path, encoded, error)); + assert(encoded == valid.encoded && !error); + const size_t bytes = encoded == OUT_PATH_UNKNOWN ? 0 : (encoded & 63) * ((encoded >> 6) + 1); + assert(!memcmp(path, valid.bytes, bytes)); + for (size_t i = bytes; i < sizeof(path); ++i) assert(path[i] == 0); + } + for (const char* malformed : {"", " ", ",12", "12,,ab", "12,", "12, ", + "12ab,cd34,", "12ab,cd34, ", "1:", "1:123", "2:12ab34", + "3:123456ab", "4:12345678", "12,abcd", "12;ab", "12,xx"}) { + std::vector text(malformed, malformed + strlen(malformed) + 1); + uint8_t path[MAX_PATH_SIZE], encoded = OUT_PATH_UNKNOWN; + const char* error = nullptr; + if (mesh::parseDataPath(text.data(), path, encoded, error)) { + fprintf(stderr, "malformed data.tx path accepted: '%s'\n", malformed); + assert(false); + } + assert(error); + } +} + +int main() { + checkPersistedRouteWins(); + checkSuccessfulMigration(); + checkIsolatedPublishFailureAndRetry(); + checkUnavailableUnhealthyAndInvalidRoutes(); + checkPathParsing(); + puts("management route adoption checks passed"); +} diff --git a/test/fixtures/management/runtime_test.cpp b/test/fixtures/management/runtime_test.cpp index 1cc5a3884..d379c70e4 100644 --- a/test/fixtures/management/runtime_test.cpp +++ b/test/fixtures/management/runtime_test.cpp @@ -1,5 +1,6 @@ #include "ManagementReporter.h" #include "CommonCLI.h" +#include "PersistentStoreFormat.h" #include #include #include @@ -29,7 +30,7 @@ struct Fixture { if (!strncmp(text, "set ", 4) && ((!strncmp(reply, "OK", 2)) != ok)) { fprintf(stderr, "%s: %s\n", text, reply); assert(false); } - if (!strncmp(text, "set mgmt.password ", 18) && strlen(text + 18) >= 12) assert(b[18] == 0); + if (!strncmp(text, "set mgmt.password ", 18)) assert(b[18] == 0); return reply; } void advance(uint32_t seconds) { @@ -49,6 +50,13 @@ struct Fixture { const auto& file = fs.files["/management"]; assert(std::string(file.begin(), file.end()).find("management test password") == std::string::npos); } + std::vector installLegacyRoute() { + auto legacy = fs.files["/management"]; + memcpy(legacy.data(), "MGC1", 4); legacy[7] = 1; legacy[8] = 0x12; + write32(legacy.data() + 116, storage::updateCRC32(0xffffffff, legacy.data(), 116)); + fs.files["/management"] = legacy; + return legacy; + } }; int main() { { @@ -166,4 +174,101 @@ int main() { Fixture f; f.configure(false); f.cmd("set mgmt.flood 90"); f.advance(89 * DAY); assert(f.mesh.packets.empty()); f.advance(DAY); assert(f.mesh.packets.size() == 1); } + { + Fixture f; f.configure(); + const auto saved = f.fs.files; + const std::string state = f.cmd("get mgmt"); + for (const char* field : {"direct", "flood", "interval"}) { + for (const char* value : {"", "-", "-5", "-4294967291", "-4294967275", "+21", + " 21", "21 ", "21x", "4294967301", "4294967317", + "18446744073709551637", "999999999999999999999999"}) { + const std::string command = std::string("set mgmt.") + field + " " + value; + f.cmd(command.c_str(), false); + assert(f.cmd("get mgmt") == state && f.fs.files == saved); + } + } + f.cmd("set mgmt.direct 0005"); f.cmd("set mgmt.flood 0021"); f.cmd("set mgmt.interval 0005"); + assert(f.cmd("get mgmt").find("direct=5d") != std::string::npos); + assert(f.cmd("get mgmt").find("flood=21d") != std::string::npos); + f.cmd("set mgmt.direct 90"); f.cmd("set mgmt.flood 90"); + f.cmd("set mgmt.direct 4", false); f.cmd("set mgmt.flood 20", false); + f.cmd("set mgmt.interval 91", false); + } + { + Fixture f; + for (unsigned i = 0; i < 36; ++i) { ClientInfo c; c.id.pub_key[0] = i; f.acl.clients.push_back(c); } + f.configure(false); f.advance(21 * DAY); assert(f.mesh.packets.size() == 1); + const auto saved = f.fs.files; + const std::string state = f.cmd("get mgmt"); + for (const char* command : {"set mgmt.unknown value", "set mgmt.enabled maybe", + "set mgmt.direct 4", "set mgmt.flood 20", "set mgmt.interval 91", + "set mgmt.password short", "set mgmt.direct 5", "set mgmt.enabled on"}) { + f.cmd(command, false); + assert(f.cmd("get mgmt") == state && f.fs.files == saved); + } + f.cli.scope_available = false; + const std::string unscoped = f.cmd("get mgmt"); + f.cmd("set mgmt.flood 21", false); + assert(f.cmd("get mgmt") == unscoped && f.fs.files == saved); + f.cli.scope_available = true; + f.advance(60); assert(f.mesh.packets.size() == 2); + f.advance(7 * 60); assert(f.mesh.packets.size() == 9 && usb_status_calls == 1); + for (unsigned page = 0; page < 9; ++page) assert(f.mesh.packets[page].payload[78] == page); + } + for (bool fail_save : {false, true}) { + Fixture f; + for (unsigned i = 0; i < 8; ++i) { ClientInfo c; c.id.pub_key[0] = i; f.acl.clients.push_back(c); } + f.configure(false); f.advance(21 * DAY); assert(f.mesh.packets.size() == 1); + const auto saved = f.fs.files; + f.fs.fail_write = fail_save; + f.cmd("set mgmt.flood 30", !fail_save); + f.advance(3600); assert(f.mesh.packets.size() == 1); + assert((f.cmd("get mgmt").find("FAULT") != std::string::npos) == fail_save); + if (fail_save) assert(f.fs.files == saved); + else assert(f.cmd("get mgmt").find("flood=30d") != std::string::npos); + } + { + Fixture f; f.configure(false); + f.fs.files["/management"][72] ^= 1; f.reboot(); + assert(f.cmd("get mgmt").find("FAULT") != std::string::npos); + const auto saved = f.fs.files; + f.cmd("set mgmt.password another valid secret", false); + f.cmd("set mgmt.password short", false); + assert(f.fs.files == saved); + } + for (bool migration_succeeds : {false, true}) { + Fixture f; f.configure(); + const auto legacy = f.installLegacyRoute(); + f.cli.adoption_succeeds = migration_succeeds; + f.reboot(); f.advance(3600); + assert(f.cli.adoption_calls == 1 && f.mesh.packets.empty()); + if (!migration_succeeds) { + // The route write alone failed, while management writes still work. + // Do not let an hourly MGC2 rewrite destroy the only saved legacy path. + assert(f.fs.files["/management"] == legacy); + const std::string fault = f.cmd("get mgmt"); + assert(fault.find("FAULT") != std::string::npos); + assert(fault.find("> off key=unset") != std::string::npos); + f.cli.adoption_succeeds = true; + f.reboot(); f.advance(3600); + assert(f.cli.adoption_calls == 2); + } + const uint8_t* path = nullptr; uint8_t path_len = OUT_PATH_UNKNOWN; + assert(f.cli.getDataTxPath(path, path_len) && path_len == 1 && path[0] == 0x12); + assert(!memcmp(f.fs.files["/management"].data(), "MGC2", 4)); + assert(f.cmd("get mgmt").find("FAULT") == std::string::npos); + const auto calls = f.cli.adoption_calls; + f.reboot(); assert(f.cli.adoption_calls == calls); // no repeated migration + } + { + Fixture f; f.configure(); f.installLegacyRoute(); + const uint8_t explicit_path[] = {0x34}; + assert(f.cli.adoptLegacyDataTxPath(explicit_path, 1)); // already saved /data_tx + f.cli.adoption_succeeds = false; // no additional route write should be needed + f.reboot(); f.advance(3600); + const uint8_t* path = nullptr; uint8_t path_len = OUT_PATH_UNKNOWN; + assert(f.cli.getDataTxPath(path, path_len) && path_len == 1 && path[0] == 0x34); + assert(f.cmd("get mgmt").find("FAULT") == std::string::npos); + assert(!memcmp(f.fs.files["/management"].data(), "MGC2", 4)); + } } diff --git a/test/fixtures/management/wrapper_test.cpp b/test/fixtures/management/wrapper_test.cpp new file mode 100644 index 000000000..d5bf79749 --- /dev/null +++ b/test/fixtures/management/wrapper_test.cpp @@ -0,0 +1,177 @@ +#include "ManagementReporter.h" +#include "CommonCLI.h" +#include +#include +#include +#include +#include + +static bool fail_nothrow_allocation = false; +static unsigned failed_allocations = 0; + +// Exercise the standard public nothrow allocation-failure path, not a +// substituted ManagementReporter constructor or command implementation. +void* operator new(size_t size, const std::nothrow_t&) noexcept { + if (fail_nothrow_allocation) { ++failed_allocations; return nullptr; } + try { return ::operator new(size); } catch (const std::bad_alloc&) { return nullptr; } +} + +namespace mesh { +UsbLoggingStatus usbLoggingStatus() { return UsbLoggingStatus(); } +} + +#include "production_path_parser.inc" + +// The route parser has independent source-linked storage tests. Its routing +// boundary records what this wrapper delegates; management commands do not +// depend on or change route state in this focused fixture. +bool CommonCLI::handleDataTxCommand(char* command, char* reply) { + data_commands.emplace_back(command); + if (!strncmp(command, "set data.tx path ", 17)) { + uint8_t path[MAX_PATH_SIZE], path_len = OUT_PATH_UNKNOWN; + const char* error = nullptr; + if (!mesh::parseDataPath(command + 17, path, path_len, error)) { + snprintf(reply, 160, "%s", error ? error : "Err - invalid path"); + return true; + } + } + if (!strncmp(command, "get data.tx", 11) || !strncmp(command, "set data.tx", 11)) { + strcpy(reply, "route boundary"); + return true; + } + return false; +} + +#include "production_wrapper.inc" + +struct Fixture { + MemoryFS fs; + mesh::Mesh mesh; + mesh::MainBoard board; + SensorManager sensors; + ClientACL acl; + NodePrefs prefs; + CommonCLICallbacks callbacks; + CommonCLI cli; + + Fixture() { + cli._board = &board; cli._sensors = &sensors; cli._acl = &acl; + cli._prefs = &prefs; cli._callbacks = &callbacks; + } + ~Fixture() { delete cli._management; } + void enableContext() { cli._management_mesh = &mesh; cli._management_fs = &fs; } +}; + +static void password(Fixture& fixture, const std::string& value, + const char* expected_reply) { + unsigned char guarded[320]; memset(guarded, 0xa5, sizeof(guarded)); + char* command = reinterpret_cast(guarded + 8); + memcpy(command, "set mgmt.password ", 18); + memcpy(command + 18, value.c_str(), value.size() + 1); + const size_t length = 18 + value.size(); + char reply[160] = {}; + assert(fixture.cli.handleManagementCommand(command, reply)); + assert(!strcmp(reply, expected_reply)); + assert(!memcmp(command, "set mgmt.password ", 18)); + for (size_t index = 18; index <= length; ++index) { + if (command[index] != 0) { + fprintf(stderr, "password byte %zu survived reply '%s'\n", index - 18, reply); + assert(false); + } + } + for (size_t index = 0; index < 8; ++index) assert(guarded[index] == 0xa5); + for (size_t index = 8 + length + 1; index < sizeof(guarded); ++index) + assert(guarded[index] == 0xa5); +} + +int main() { + for (unsigned missing : {0u, 1u, 2u}) { + Fixture fixture; + if (missing == 1) fixture.cli._management_mesh = &fixture.mesh; + if (missing == 2) fixture.cli._management_fs = &fixture.fs; + for (size_t length : {0u, 5u, 12u, 96u, 97u, 200u}) + password(fixture, std::string(length, 's'), "ERR: management unavailable"); + assert(!fixture.cli._management && fixture.fs.files.empty()); + } + { + Fixture fixture; fixture.enableContext(); fail_nothrow_allocation = true; + const auto before = failed_allocations; + for (size_t length : {0u, 5u, 12u, 96u, 97u, 200u}) + password(fixture, std::string(length, 's'), "ERR: management unavailable"); + assert(failed_allocations == before + 6 && !fixture.cli._management); + assert(fixture.fs.files.empty()); fail_nothrow_allocation = false; + password(fixture, "a real management password", "OK"); + assert(fixture.cli._management && fixture.fs.files.count("/management")); + } + { + Fixture fixture; fixture.enableContext(); + for (size_t length : {0u, 5u, 97u, 200u}) + password(fixture, std::string(length, 's'), "ERR: password must be 12..96 bytes"); + assert(fixture.fs.files.empty()); + password(fixture, "a real management password", "OK"); + fixture.fs.fail_write = true; + password(fixture, "another management password", "ERR: save failed; reporting stopped"); + fixture.fs.fail_write = false; + password(fixture, "third management password", "ERR: management state fault; repair storage/reboot first"); + } + { + Fixture fixture; + for (const char* untouched : {"get mgmt", "set mgmt.enabled on", "get mgmt.password", + "set mgmt.passwordExtra secret", "set mgmt.password\tsecret"}) { + char command[160], reply[160] = {}; strcpy(command, untouched); + assert(fixture.cli.handleManagementCommand(command, reply)); + assert(!strcmp(command, untouched)); + assert(!strcmp(reply, "ERR: management unavailable")); + } + for (const char* unknown : {"get mgm", "set mgm.password secret", "get management", + "set management.password secret"}) { + char command[160], reply[160] = "not handled"; strcpy(command, unknown); + assert(!fixture.cli.handleManagementCommand(command, reply)); + assert(!strcmp(command, unknown) && !strcmp(reply, "not handled")); + } + for (const char* alias : {"get mgmt.path", "set mgmt.path 2:12abcd34"}) { + char command[160], reply[160] = {}; strcpy(command, alias); + assert(fixture.cli.handleManagementCommand(command, reply)); + assert(!strcmp(reply, "route boundary")); + assert(fixture.cli.data_commands.back() + == (!strncmp(alias, "get", 3) ? "get data.tx path" : "set data.tx path 2:12abcd34")); + assert(!fixture.cli._management); + } + const std::string largest_spec = "1:" + std::string(126, '1') + std::string(14, ' '); + const std::string largest_alias = "set mgmt.path " + largest_spec; + assert(largest_alias.size() == 156); + char boundary[160], boundary_reply[160] = {}; strcpy(boundary, largest_alias.c_str()); + assert(fixture.cli.handleManagementCommand(boundary, boundary_reply)); + assert(!strcmp(boundary_reply, "route boundary")); + // Exact expanded capacity is accepted, including its null terminator. + // trimDataRoute mutates the delegated buffer after validation. + assert(fixture.cli.data_commands.back().size() == 159); + // This fits the 159-byte ordinary repeater CLI input limit. Expanding the + // alias prefix by three bytes must not discard malformed suffix bytes and + // turn an invalid setting into an accepted 63-hop direct route. + const std::string spec = largest_spec + "bad"; + std::string direct = spec; + uint8_t path[MAX_PATH_SIZE], path_len = OUT_PATH_UNKNOWN; + const char* error = nullptr; + assert(!mesh::parseDataPath(&direct[0], path, path_len, error)); + const std::string alias = "set mgmt.path " + spec; + assert(alias.size() == 159); + char command[160], reply[160] = {}; strcpy(command, alias.c_str()); + const auto delegated = fixture.cli.data_commands.size(); + assert(fixture.cli.handleManagementCommand(command, reply)); + if (!strcmp(reply, "route boundary")) { + fprintf(stderr, "malformed alias accepted after truncation: '%s'\n", reply); + assert(false); + } + assert(!strcmp(reply, "Err - data.tx path too long")); + // The wrapper probes the original command once before recognizing aliases; + // rejection must not make a second call with a truncated expanded alias. + assert(fixture.cli.data_commands.size() == delegated + 1); + for (size_t excess : {1u, 2u, 100u}) { + std::string long_alias = largest_alias + std::string(excess, ' '); + char long_reply[160] = {}; + assert(fixture.cli.handleManagementCommand(&long_alias[0], long_reply)); + assert(!strcmp(long_reply, "Err - data.tx path too long")); + } + } +} diff --git a/test/test_management_cli_wrapper.py b/test/test_management_cli_wrapper.py new file mode 100644 index 000000000..1fbcc17bf --- /dev/null +++ b/test/test_management_cli_wrapper.py @@ -0,0 +1,79 @@ +"""Run the production management wrapper and reporter on existing host boundaries.""" + +from pathlib import Path +import os +import shutil +import subprocess +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + + +ROOT = Path(__file__).resolve().parents[1] +FIXTURE = ROOT / "test/fixtures/management" + + +class ManagementCliWrapperTests(unittest.TestCase): + def test_unavailable_password_wipe_allocation_failure_and_routing(self): + candidates = [Path(os.environ.get("MESHCORE_CRYPTO_DIR", "/nonexistent"))] + candidates += sorted((ROOT / ".pio/libdeps").glob("*/Crypto")) + crypto = next((path for path in candidates if (path / "AES128.cpp").is_file()), None) + if crypto is None: + raise RuntimeError("Install rweather/Crypto 0.4.0 or set MESHCORE_CRYPTO_DIR (no mock fallback)") + compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++") + if not compiler: + self.skipTest("a host C++17 compiler is required") + source = (ROOT / "src/helpers/CommonCLI_Management.cpp").read_text(encoding="ascii") + wrapper = extract_braced(source, "bool CommonCLI::handleManagementCommand(") + # Keep existing hardware/route fixtures, adding only the member surface + # needed to run the actual wrapper with the actual production reporter. + header = (FIXTURE / "CommonCLI.h").read_text(encoding="ascii") + header = header.replace("class CommonCLI {", "class SensorManager;\nclass ClientACL;\nclass CommonCLICallbacks;\nclass CommonCLI {", 1) + header = header.replace("public:\n uint8_t path_len", """public: + mesh::ManagementReporter* _management = nullptr; + mesh::Mesh* _management_mesh = nullptr; + FILESYSTEM* _management_fs = nullptr; + mesh::MainBoard* _board = nullptr; + SensorManager* _sensors = nullptr; + ClientACL* _acl = nullptr; + NodePrefs* _prefs = nullptr; + CommonCLICallbacks* _callbacks = nullptr; + std::vector data_commands; + bool handleManagementCommand(char* command, char* reply); + bool handleDataTxCommand(char* command, char* reply); + uint8_t path_len""", 1) + header = header.replace("#include ", "#include \n#include \nnamespace mesh { class ManagementReporter; }") + sources = ["AES128.cpp", "AESCommon.cpp", "BlockCipher.cpp", "Crypto.cpp", "SHA256.cpp", "Hash.cpp"] + sanitizer_flags = [] if os.name == "nt" else [ + "-DHOST_BUILD", "-fsanitize=address,undefined", "-fno-sanitize-recover=all", "-fno-pie", "-no-pie"] + with tempfile.TemporaryDirectory(prefix="management-wrapper-") as directory: + work = Path(directory) + for name in ("ManagementReporter.cpp", "ManagementReporter.h", "FileRead.h", + "ContactFileTransaction.h", "PersistentStoreFormat.h"): + shutil.copyfile(ROOT / "src/helpers" / name, work / name) + shutil.copyfile(FIXTURE / "Mesh.h", work / "Mesh.h") + shutil.copyfile(FIXTURE / "wrapper_test.cpp", work / "wrapper_test.cpp") + shutil.copyfile(ROOT / "test/fixtures/radio_profiles/mocks/helpers/IdentityStore.h", work / "IdentityStore.h") + (work / "CommonCLI.h").write_text(header, encoding="ascii") + (work / "production_wrapper.inc").write_text(wrapper, encoding="ascii") + parser = "namespace mesh {\n" + "\n".join( + extract_braced(source, signature) for signature in ( + "static uint8_t nibble(", "static char* trimDataRoute(", + "static bool parseDataPath(")) + "\n}\n" + (work / "production_path_parser.inc").write_text(parser, encoding="ascii") + executable = work / "wrapper-test" + command = [compiler, "-std=c++17", "-O1", "-g", "-Wall", "-Wextra", "-DRP2040_PLATFORM", + *sanitizer_flags, "-I", directory, "-I", str(crypto), + "-I", str(ROOT / "src/helpers"), "-I", str(ROOT / "src"), + str(work / "ManagementReporter.cpp"), str(ROOT / "src/helpers/ManagementReport.cpp"), + str(work / "wrapper_test.cpp")] + command += [str(crypto / name) for name in sources] + ["-o", str(executable)] + built = subprocess.run(command, capture_output=True, text=True, timeout=60) + self.assertEqual(built.returncode, 0, built.stderr) + tested = subprocess.run([str(executable)], capture_output=True, text=True, timeout=10) + self.assertEqual(tested.returncode, 0, tested.stderr) + + +if __name__ == "__main__": + unittest.main()