Fail closed after uncertain shared data route publication

src/helpers/CommonCLI_Management.cpp

@@ -1,6 +1,7 @@
 #include "CommonCLI.h"
 #include "ManagementReporter.h"
 #include "FileRead.h"
+#include "FilePresence.h"
 #include "PersistentStoreFormat.h"
 #include "HilStartupTrace.h"
 #include <new>
@@ -138,19 +139,35 @@ static bool dataRouteSave(DataRouteState& route) {
 #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
   AtomicFileWriter writer(route.fs, DATA_ROUTE_FILE);
 #else
-  ContactFileTransaction writer(route.fs, DATA_ROUTE_FILE);
+  ContactFileTransaction writer(route.fs, DATA_ROUTE_FILE, filePresence<FILESYSTEM>);
 #endif
-  const bool ok = writer && writer.write(data, sizeof(data)) == sizeof(data)
+  bool ok = writer && writer.write(data, sizeof(data)) == sizeof(data)
       && writer.commit();
+  if (writer.outcome() == decltype(writer)::Outcome::Indeterminate) route.healthy = false;
+#if defined(ESP32_PLATFORM) || defined(RP2040_PLATFORM)
+  if (ok || writer.outcome() == ContactFileTransaction::Outcome::Indeterminate) {
+    // A moved stage alone does not confirm the public bytes. Never resume the
+    // old RAM route when the new durable authority cannot be established.
+    auto file = openFileRead(route.fs, DATA_ROUTE_FILE);
+    uint8_t verified[DATA_ROUTE_SIZE];
+    ok = file && !file.isDirectory() && file.size() == sizeof(verified)
+        && file.read(verified, sizeof(verified)) == (int)sizeof(verified)
+        && memcmp(data, verified, sizeof(data)) == 0;
+    file.close();
+    route.healthy = ok;
+  }
+#endif
   if (ok) route.persisted = true;
   return ok;
 }
 
 static bool dataRouteLoad(DataRouteState& route) {
 #if defined(ESP32_PLATFORM) || defined(RP2040_PLATFORM)
-  if (!ContactFileTransaction::recover(route.fs, DATA_ROUTE_FILE)) return false;
+  if (!ContactFileTransaction::recover(route.fs, DATA_ROUTE_FILE, filePresence<FILESYSTEM>)) return false;
 #endif
-  if (!route.fs->exists(DATA_ROUTE_FILE)) return true;
+  bool present = false;
+  if (!filePresence(route.fs, DATA_ROUTE_FILE, present)) return false;
+  if (!present) return true;
   uint8_t data[DATA_ROUTE_SIZE] = {};
   for (unsigned retry = 0; retry < 3; ++retry) {
     auto file = openFileRead(route.fs, DATA_ROUTE_FILE);
@@ -401,7 +418,9 @@ bool CommonCLI::handleDataTxCommand(char* command, char* reply) {
     }
   }
   if (!mesh::dataRouteSave(*_data_route)) {
-    *_data_route = previous; strcpy(reply, "Err - unable to save data.tx"); return true;
+    const bool healthy = _data_route->healthy;
+    *_data_route = previous; _data_route->healthy = healthy;
+    strcpy(reply, "Err - unable to save data.tx"); return true;
   }
   strcpy(reply, "OK"); return true;
 }

test/fixtures/management/route_test.cpp

@@ -1,6 +1,8 @@
 #include "IdentityStore.h"
 #include <Mesh.h>
 #include "ContactFileTransaction.h"
+#include "FilePresence.h"
+#include "FileRead.h"
 #include "PersistentStoreFormat.h"
 #include <cassert>
 #include <cstdio>

test/fixtures/telemetry_tx_persistence/IdentityStore.h

@@ -40,6 +40,8 @@ public:
   bool fail_metadata = false, fail_final_read = false;
   int final_read_limit = -1;
   bool corrupt_publish = false, fail_metadata_after_publish = false;
+  std::string publication_path = "/telemetry_tx";
+  bool no_op_publish = false, late_publish_error = false;
   unsigned writes = 0, renames = 0;
 #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
   bool rename_replaces = true;
@@ -54,12 +56,14 @@ public:
     ++renames;
     if (fail_rename.count({from, to}) || !exists(from)
         || (!rename_replaces && exists(to))) return false;
+    if (from == publication_path + ".tmp" && no_op_publish) return true;
     files[to] = files[from]; files.erase(from);
-    if (!strcmp(from, "/telemetry_tx.tmp")) {
+    if (from == publication_path + ".tmp") {
       if (corrupt_publish) files[to][0] ^= 1;
       if (fail_final_read) fail_read_open.insert(to);
       if (final_read_limit >= 0) read_limit[to] = size_t(final_read_limit);
       if (fail_metadata_after_publish) fail_metadata = true;
+      if (late_publish_error) return false;
     }
     return true;
   }
@@ -82,6 +86,7 @@ public:
     fail_metadata = fail_final_read = corrupt_publish = false;
     final_read_limit = -1;
     fail_metadata_after_publish = false;
+    no_op_publish = late_publish_error = false;
   }
 };
 

test/fixtures/telemetry_tx_persistence/fixture.cpp

@@ -45,12 +45,33 @@ struct Packet {
 struct Utils { static void toHex(char*, const uint8_t*, size_t); };
 struct DataRouteState;
 }
-class RegionMap;
+struct TransportKey {};
+struct RegionEntry { const char* name; };
+class RegionMap {
+public:
+  RegionEntry* findByNamePrefix(const char*) { return nullptr; }
+};
+namespace mesh {
+// Region selection is a boundary here; route storage and command handling are real.
+static bool usableScope(const DataRouteState&, RegionEntry*, TransportKey&) { return false; }
+}
 class CommonCLI {
 public:
   mesh::DataRouteState* _data_route = nullptr;
+  RegionMap* _region_map = nullptr;
   bool adoptLegacyDataTxPath(const uint8_t*, uint8_t);
   bool getDataTxPath(const uint8_t*&, uint8_t&) const;
+  bool resolveDataTxScope(TransportKey&, char*, size_t, bool* ambiguous = nullptr) const {
+    if (ambiguous) *ambiguous = false;
+    return false;
+  }
+  bool handleDataTxCommand(char*, char*);
+  std::string routeCommand(const char* text) {
+    std::vector<char> command(text, text + strlen(text) + 1);
+    char reply[160] = {};
+    assert(handleDataTxCommand(command.data(), reply));
+    return reply;
+  }
 };
 struct ClientInfo { bool admin = true; bool isAdmin() const { return admin; } };
 class MyMesh {
@@ -397,6 +418,119 @@ static void checkAmbiguousPublicationAndServiceGate() {
   assert(healthy.node.command("send telemetry.tx now").find("OK - telemetry.tx queued") == 0);
 }
 
+static void checkRoutePublicationAuthority() {
+  for (const bool migration : {false, true}) {
+    Fixture f; f.boot();
+    if (!migration) assert(f.node._cli.routeCommand("set data.tx path 12") == "OK");
+    f.fs.publication_path = "/data_tx";
+#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
+    // Atomic publication succeeds, but its mandatory public readback fails.
+    f.fs.fail_final_read = true;
+#else
+    // The old image is backed up but neither publication nor rollback happens.
+    if (migration) assert(mesh::dataRouteSave(f.route));
+    f.fs.fail_rename.insert({"/data_tx.tmp", "/data_tx"});
+    f.fs.fail_rename.insert({"/data_tx.bak", "/data_tx"});
+#endif
+    if (migration) {
+      const uint8_t path = 0x34;
+      // Force the legacy adoption path without discarding the existing disk image.
+      f.route.persisted = false;
+      assert(!f.node._cli.adoptLegacyDataTxPath(&path, 1));
+    } else {
+      assert(f.node._cli.routeCommand("set data.tx path 34") == "Err - unable to save data.tx");
+    }
+    // A reported failure cannot authorize old RAM when disk authority is unknown.
+    assert(!f.route.healthy);
+    const uint8_t* path = nullptr; uint8_t length = 0;
+    assert(!f.node._cli.getDataTxPath(path, length));
+    assert(f.node._cli.routeCommand("get data.tx") == "Err - data.tx storage fault");
+    const auto writes = f.fs.writes;
+    assert(f.node._cli.routeCommand("set data.tx path 56") == "Err - data.tx storage fault");
+    assert(f.fs.writes == writes);
+    f.fs.clearFaults(); f.boot();
+    assert(f.route.healthy);
+#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
+    assert(f.route.path_len == 1 && f.route.path[0] == 0x34);
+#else
+    assert(f.route.path_len == (migration ? 0 : 1));
+    if (!migration) assert(f.route.path[0] == 0x12);
+#endif
+  }
+}
+
+static void checkRouteReadbackAndRetry() {
+  for (const bool region : {false, true}) {
+    const char* command = region ? "set data.tx region none" : "set data.tx path 34";
+    for (const int failure : {0, 1, 2, 3, 4, 5}) {
+      Fixture f; f.boot();
+      assert(f.node._cli.routeCommand("set data.tx path 12") == "OK");
+      const auto previous = f.fs.files.at("/data_tx");
+      f.fs.publication_path = "/data_tx";
+      if (failure == 0) f.fs.fail_write_open.insert("/data_tx.tmp");
+      if (failure == 1) f.fs.write_limit["/data_tx.tmp"] = 107;
+      if (failure == 2) f.fs.fail_read_open.insert("/data_tx.tmp");
+      if (failure == 3) f.fs.fail_final_read = true;
+      if (failure == 4) f.fs.final_read_limit = 107;
+      if (failure == 5) f.fs.corrupt_publish = true;
+      assert(f.node._cli.routeCommand(command) == "Err - unable to save data.tx");
+      assert(f.route.healthy == (failure < 3));
+      assert(f.route.path_len == 1 && f.route.path[0] == 0x12);
+      assert(f.route.region_mode == mesh::DATA_REGION_AUTO);
+      if (failure < 3) {
+        assert(f.fs.files.at("/data_tx") == previous);
+        f.fs.clearFaults();
+        assert(f.node._cli.routeCommand(command) == "OK");
+        f.boot(); assert(f.route.healthy);
+      } else {
+        const auto writes = f.fs.writes;
+        const uint8_t* path = nullptr; uint8_t length = 0;
+        assert(!f.node._cli.getDataTxPath(path, length));
+        assert(f.node._cli.routeCommand(command) == "Err - data.tx storage fault");
+        assert(f.fs.writes == writes);
+        f.fs.clearFaults(); f.boot();
+        assert(f.route.healthy == (failure != 5));
+      }
+    }
+  }
+  for (const bool existing : {false, true}) {
+    for (const bool no_op : {false, true}) {
+      Fixture f; f.boot();
+      if (existing) assert(f.node._cli.routeCommand("set data.tx path 12") == "OK");
+      f.fs.publication_path = "/data_tx";
+      f.fs.no_op_publish = no_op;
+      f.fs.late_publish_error = !no_op;
+      const auto reply = f.node._cli.routeCommand("set data.tx path 34");
+      if (no_op) {
+        assert(reply == "Err - unable to save data.tx");
+#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
+        assert(!f.route.healthy); // Atomic writer cannot prove old authority.
+#else
+        assert(f.route.healthy); // Observed unchanged move + successful rollback.
+#endif
+      } else {
+        assert(reply == "OK" && f.route.healthy && f.route.path[0] == 0x34);
+      }
+      f.fs.clearFaults(); f.boot(); assert(f.route.healthy);
+      if (!no_op) assert(f.route.path_len == 1 && f.route.path[0] == 0x34);
+      else assert(f.route.path_len == (existing ? 1 : 0));
+    }
+  }
+#if !defined(RP2040_PLATFORM)
+  for (const bool existing : {false, true}) {
+    Fixture f; f.boot();
+    if (existing) assert(f.node._cli.routeCommand("set data.tx path 12") == "OK");
+    const auto files = f.fs.files;
+    f.fs.fail_metadata = true; f.boot();
+    assert(!f.route.healthy && f.fs.files == files);
+    const auto writes = f.fs.writes;
+    assert(f.node._cli.routeCommand("set data.tx path 34") == "Err - data.tx storage fault");
+    assert(f.fs.writes == writes);
+    f.fs.clearFaults(); f.boot(); assert(f.route.healthy);
+  }
+#endif
+}
+
 int main() {
   checkFreshAndValid();
   checkLoadFailures();
@@ -404,5 +538,7 @@ int main() {
   checkWriteAndValidationFailures();
   checkRecoveryAndRenameFailures();
   checkAmbiguousPublicationAndServiceGate();
+  checkRoutePublicationAuthority();
+  checkRouteReadbackAndRetry();
   puts("telemetry persistence checks passed");
 }

test/test_management_report.py

@@ -74,7 +74,8 @@ class ManagementTests(unittest.TestCase):
         # Compile the unchanged production runtime and transaction writer with
         # a memory filesystem/radio in place of the hardware-facing headers.
         fixture = ROOT / "test/fixtures/management"
-        for name in ("ManagementReporter.cpp", "ManagementReporter.h", "FileRead.h", "ContactFileTransaction.h", "PersistentStoreFormat.h"):
+        for name in ("ManagementReporter.cpp", "ManagementReporter.h", "FileRead.h", "FilePresence.h",
+                     "ContactFileTransaction.h", "PersistentStoreFormat.h"):
             shutil.copyfile(ROOT / "src/helpers" / name, Path(cls.work.name) / name)
         for name in ("CommonCLI.h", "Mesh.h"):
             shutil.copyfile(fixture / name, Path(cls.work.name) / name)

test/test_telemetry_tx_persistence.py

@@ -30,9 +30,12 @@ class TelemetryPersistenceTests(unittest.TestCase):
         generated += extract_braced(utils, "void Utils::toHex(") + "\n"
         generated += "\n".join(extract_braced(common, signature) for signature in (
             "static uint32_t readRoute32(", "static void writeRoute32(",
+            "static uint8_t nibble(", "static char* trimDataRoute(",
+            "static bool parseDataPath(", "static void formatDataPath(",
             "static bool dataRouteSave(", "static bool dataRouteLoad(")) + "\n}\n"
         generated += "\n".join(extract_braced(common, signature) for signature in (
-            "bool CommonCLI::adoptLegacyDataTxPath(", "bool CommonCLI::getDataTxPath(")) + "\n"
+            "bool CommonCLI::adoptLegacyDataTxPath(", "bool CommonCLI::getDataTxPath(",
+            "bool CommonCLI::handleDataTxCommand(")) + "\n"
         generated += '#define TELEMETRY_HISTORY_TX_PREFS_FILE "/telemetry_tx"\n'
         for name in ("RETRY_MILLIS", "PACKET_SPACING_MILLIS", "DEFAULT_DAYS", "MAX_DAYS",
                      "TEMPERATURE", "VOLTAGE", "EXTERNAL_VOLTAGE"):