Reduce ESP ACL save metadata opens while preserving verified recovery

.github/workflows/run-unit-tests.yml

@@ -216,6 +216,7 @@ jobs:
           python3 -B test/test_ota_device_deflate_encoder.py -v
           python3 -B test/test_ota_device_deflate_role.py -v
           python3 -B test/test_client_acl_spiffs.py
+          python3 -B test/test_client_acl_metadata.py -v
           python3 -B test/test_client_acl_cli.py -v
           python3 -B test/test_client_acl_response.py -v
 

src/helpers/ClientACL.cpp

@@ -13,8 +13,23 @@
 static const uint8_t CONTACT_RECORD_VERSION_ALT_PATH = 1;
 static const uint8_t EMPTY_OUT_PATH[MAX_PATH_SIZE] = {};
 
-static File openRead(FILESYSTEM* fs, const char* filename) {
+static File openRead(FILESYSTEM* fs, const char* filename,
+                     bool* metadata_ok = nullptr) {
+#if defined(ESP32_PLATFORM)
+  bool present = false;
+  if (!mesh::filePresence(fs, filename, present)) {
+    if (metadata_ok) *metadata_ok = false;
+    return mesh::emptyFile(fs);
+  }
+  if (!present) return mesh::emptyFile(fs);
+  File file = fs->open(filename);
+  if (!file && metadata_ok) *metadata_ok = false;
+  if (file && file.isDirectory()) file.close();
+  return file;
+#else
+  (void)metadata_ok;
   return mesh::openFileRead(fs, filename);
+#endif
 }
 
 #if !defined(NRF52_PLATFORM)
@@ -27,9 +42,12 @@ static const size_t LEGACY_CONTACT_RECORD_SIZE =
 static bool readPersistedClientPath(FILESYSTEM* fs,
                                     const uint8_t pubkey[PUB_KEY_SIZE],
                                     uint8_t* path_len,
-                                    uint8_t path[MAX_PATH_SIZE]) {
+                                    uint8_t path[MAX_PATH_SIZE],
+                                    bool* metadata_ok = nullptr) {
+#if !defined(ESP32_PLATFORM)
   if (!fs->exists(mesh::CLIENT_ACL_PRIMARY_PATH)) return false;
-  File file = openRead(fs, mesh::CLIENT_ACL_PRIMARY_PATH);
+#endif
+  File file = openRead(fs, mesh::CLIENT_ACL_PRIMARY_PATH, metadata_ok);
   if (!file) return false;
 
   bool found = false;
@@ -87,10 +105,11 @@ static mesh::StoredClientPathView storedClientPathForSave(
     FILESYSTEM* fs,
     const ClientInfo* client,
     uint8_t prior_path[MAX_PATH_SIZE],
-    uint8_t* prior_path_len) {
+    uint8_t* prior_path_len,
+    bool* metadata_ok = nullptr) {
   const bool prior_exists = !client->out_path_is_persistable
       && readPersistedClientPath(
-          fs, client->id.pub_key, prior_path_len, prior_path);
+          fs, client->id.pub_key, prior_path_len, prior_path, metadata_ok);
   return mesh::selectStoredClientPath(
       client->out_path_is_persistable,
       client->out_path_len,
@@ -132,25 +151,32 @@ static bool readMatches(File& file, const uint8_t* expected, size_t length) {
   return true;
 }
 
-static bool validateContactsFileIntegrity(FILESYSTEM* fs,
-                                          const char* filename) {
-  File file = openRead(fs, filename);
-  if (!file) return false;
+static mesh::ClientACLFileValidation validateContactsFileIntegrityChecked(
+    FILESYSTEM* fs, const char* filename) {
+  bool metadata_ok = true;
+  File file = openRead(fs, filename, &metadata_ok);
+  if (!metadata_ok) return mesh::ClientACLFileValidation::MetadataError;
+  if (!file) return mesh::ClientACLFileValidation::Invalid;
   const size_t size = file.size();
   const bool has_crc = size >= 8 && (size - 8) % CONTACT_RECORD_SIZE == 0;
   // A backup means publication was interrupted or is being finalized. In that
   // state a primary without its CRC trailer is a torn new image, not a legacy
   // image, even when truncation lands exactly on a record-size multiple.
-  const bool crc_required = strcmp(filename, mesh::CLIENT_ACL_PRIMARY_PATH) == 0
-      && fs->exists(mesh::CLIENT_ACL_BACKUP_PATH);
+  bool crc_required = false;
+  if (strcmp(filename, mesh::CLIENT_ACL_PRIMARY_PATH) == 0
+      && !mesh::clientACLFilePresence(fs, mesh::CLIENT_ACL_BACKUP_PATH,
+                                     crc_required)) {
+    file.close();
+    return mesh::ClientACLFileValidation::MetadataError;
+  }
   if (!has_crc && size % CONTACT_RECORD_SIZE != 0
       && size % LEGACY_CONTACT_RECORD_SIZE != 0) {
     file.close();
-    return false;
+    return mesh::ClientACLFileValidation::Invalid;
   }
   if (!has_crc) {
     file.close();
-    return !crc_required; // standalone legacy fixed-record image
+    return mesh::clientACLValidation(!crc_required); // standalone legacy image
   }
   uint32_t crc = 0xFFFFFFFFUL;
   size_t remaining = size - 8;
@@ -159,7 +185,7 @@ static bool validateContactsFileIntegrity(FILESYSTEM* fs,
     const size_t chunk = remaining < sizeof(buffer) ? remaining : sizeof(buffer);
     if (file.read(buffer, chunk) != (int)chunk) {
       file.close();
-      return false;
+      return mesh::ClientACLFileValidation::Invalid;
     }
     crc = mesh::updateClientACLCRC(crc, buffer, chunk);
     remaining -= chunk;
@@ -172,7 +198,12 @@ static bool validateContactsFileIntegrity(FILESYSTEM* fs,
       && memcmp(magic, mesh::CLIENT_ACL_CRC_MAGIC, sizeof(magic)) == 0
       && stored_crc == (crc ^ 0xFFFFFFFFUL);
   file.close();
-  return valid;
+  return mesh::clientACLValidation(valid);
+}
+
+static bool validateContactsFileIntegrity(FILESYSTEM* fs, const char* filename) {
+  return validateContactsFileIntegrityChecked(fs, filename)
+      == mesh::ClientACLFileValidation::Valid;
 }
 
 static bool verifyContactsFile(
@@ -197,10 +228,11 @@ static bool verifyContactsFile(
         || (filter && !filter(c))) continue;
     uint8_t prior_path_len = OUT_PATH_UNKNOWN;
     uint8_t prior_path[MAX_PATH_SIZE] = {};
+    bool metadata_ok = true;
     const mesh::StoredClientPathView persisted_path =
         storedClientPathForSave(
-            fs, c, prior_path, &prior_path_len);
-    matches = readMatches(file, c->id.pub_key, 32)
+            fs, c, prior_path, &prior_path_len, &metadata_ok);
+    matches = metadata_ok && readMatches(file, c->id.pub_key, 32)
         && readMatches(file, &c->permissions, 1)
         && readMatches(file, (uint8_t*)&c->extra.room.sync_since, 4)
         && readMatches(file, unused, sizeof(unused))
@@ -609,7 +641,7 @@ void ClientACL::load(FILESYSTEM* fs, const mesh::LocalIdentity& self_id) {
   }
 #if !defined(NRF52_PLATFORM)
   if (!mesh::recoverClientACLFilesVerified(
-          _fs, validateContactsFileIntegrity)) {
+          _fs, validateContactsFileIntegrityChecked)) {
     MESH_DEBUG_PRINTLN("ERROR: ClientACL::load could not recover contacts files");
     return;
   }
@@ -748,7 +780,7 @@ bool ClientACL::save(FILESYSTEM* fs, bool (*filter)(ClientInfo*)) {
   mesh::AtomicFileWriter file(_fs, "/s_contacts");
 #else
   if (!mesh::recoverClientACLFilesVerified(
-          _fs, validateContactsFileIntegrity)) {
+          _fs, validateContactsFileIntegrityChecked)) {
     MESH_DEBUG_PRINTLN("ERROR: ClientACL::save recovery is incomplete");
     return false;
   }
@@ -780,10 +812,11 @@ bool ClientACL::save(FILESYSTEM* fs, bool (*filter)(ClientInfo*)) {
     success = success && (file.write(unused, 2) == 2);
     uint8_t prior_path_len = OUT_PATH_UNKNOWN;
     uint8_t prior_path[MAX_PATH_SIZE] = {};
+    bool metadata_ok = true;
     const mesh::StoredClientPathView persisted_path =
         storedClientPathForSave(
-            _fs, c, prior_path, &prior_path_len);
-    success = success
+            _fs, c, prior_path, &prior_path_len, &metadata_ok);
+    success = success && metadata_ok
         && (file.write(&persisted_path.encoded_path_len, 1) == 1);
     success = success
         && (file.write(persisted_path.path, MAX_PATH_SIZE) == MAX_PATH_SIZE);
@@ -837,7 +870,7 @@ bool ClientACL::save(FILESYSTEM* fs, bool (*filter)(ClientInfo*)) {
     return false;
   }
   success = mesh::publishVerifiedClientACLTemp(
-      _fs, true, validateContactsFileIntegrity);
+      _fs, true, validateContactsFileIntegrityChecked);
   if (!success) {
     MESH_DEBUG_PRINTLN("ERROR: ClientACL::save atomic publish failed");
   }

src/helpers/ClientACLFileTransaction.h

@@ -1,16 +1,44 @@
 #pragma once
 
+#if defined(ESP32_PLATFORM)
+#include "FilePresence.h"
+#endif
+
 namespace mesh {
 
 static const char CLIENT_ACL_PRIMARY_PATH[] = "/s_contacts";
 static const char CLIENT_ACL_TEMP_PATH[] = "/s_contacts.tmp";
 static const char CLIENT_ACL_BACKUP_PATH[] = "/s_contacts.bak";
 
+// ESP ACL stores are mounted at /spiffs. Unlike FS::exists(), the metadata
+// probe does not open/close the file and distinguishes absent from unreadable.
+template <typename Filesystem>
+bool clientACLFilePresence(Filesystem* fs, const char* path, bool& present) {
+#if defined(ESP32_PLATFORM)
+  return filePresence(fs, path, present);
+#else
+  present = fs->exists(path);
+  return true;
+#endif
+}
+
+enum class ClientACLFileValidation { Invalid, Valid, MetadataError };
+
+inline ClientACLFileValidation clientACLValidation(bool valid) {
+  return valid ? ClientACLFileValidation::Valid
+               : ClientACLFileValidation::Invalid;
+}
+inline ClientACLFileValidation clientACLValidation(ClientACLFileValidation valid) {
+  return valid;
+}
+
 template <typename Filesystem>
 bool removeClientACLArtifact(Filesystem* fs, const char* path) {
-  if (!fs->exists(path)) return true;
+  bool present;
+  if (!clientACLFilePresence(fs, path, present)) return false;
+  if (!present) return true;
   fs->remove(path);
-  return !fs->exists(path);
+  return clientACLFilePresence(fs, path, present) && !present;
 }
 
 // Recover the last published image after any power-loss boundary in the
@@ -18,7 +46,9 @@ bool removeClientACLArtifact(Filesystem* fs, const char* path) {
 // if publication had not completed, the backup is restored instead.
 template <typename Filesystem>
 bool recoverClientACLFiles(Filesystem* fs) {
-  if (fs->exists(CLIENT_ACL_PRIMARY_PATH)) {
+  bool primary_exists, backup_exists;
+  if (!clientACLFilePresence(fs, CLIENT_ACL_PRIMARY_PATH, primary_exists)) return false;
+  if (primary_exists) {
     // The primary is already authoritative.  Cleanup is best-effort: a
     // filesystem which cannot remove a stale artifact must not make the
     // committed image appear unavailable.
@@ -26,7 +56,8 @@ bool recoverClientACLFiles(Filesystem* fs) {
     removeClientACLArtifact(fs, CLIENT_ACL_BACKUP_PATH);
     return true;
   }
-  if (fs->exists(CLIENT_ACL_BACKUP_PATH)) {
+  if (!clientACLFilePresence(fs, CLIENT_ACL_BACKUP_PATH, backup_exists)) return false;
+  if (backup_exists) {
     if (!fs->rename(CLIENT_ACL_BACKUP_PATH, CLIENT_ACL_PRIMARY_PATH)) {
       return false;
     }
@@ -38,25 +69,33 @@ bool recoverClientACLFiles(Filesystem* fs) {
 
 template <typename Filesystem, typename Validator>
 bool recoverClientACLFilesVerified(Filesystem* fs, Validator is_valid) {
-  if (fs->exists(CLIENT_ACL_PRIMARY_PATH)
-      && is_valid(fs, CLIENT_ACL_PRIMARY_PATH)) {
-    // A validated primary remains authoritative even when stale-artifact
-    // cleanup is temporarily unavailable.
-    removeClientACLArtifact(fs, CLIENT_ACL_TEMP_PATH);
-    removeClientACLArtifact(fs, CLIENT_ACL_BACKUP_PATH);
-    return true;
-  }
-  if (fs->exists(CLIENT_ACL_BACKUP_PATH)
-      && is_valid(fs, CLIENT_ACL_BACKUP_PATH)) {
-    if (!removeClientACLArtifact(fs, CLIENT_ACL_PRIMARY_PATH)
-        || !fs->rename(CLIENT_ACL_BACKUP_PATH, CLIENT_ACL_PRIMARY_PATH)) {
-      return false;
+  bool primary_exists, backup_exists;
+  if (!clientACLFilePresence(fs, CLIENT_ACL_PRIMARY_PATH, primary_exists)) return false;
+  if (primary_exists) {
+    const auto result = clientACLValidation(is_valid(fs, CLIENT_ACL_PRIMARY_PATH));
+    if (result == ClientACLFileValidation::MetadataError) return false;
+    if (result == ClientACLFileValidation::Valid) {
+      // A validated primary remains authoritative even when stale-artifact
+      // cleanup is temporarily unavailable.
+      removeClientACLArtifact(fs, CLIENT_ACL_TEMP_PATH);
+      removeClientACLArtifact(fs, CLIENT_ACL_BACKUP_PATH);
+      return true;
     }
-    removeClientACLArtifact(fs, CLIENT_ACL_TEMP_PATH);
-    return true;
   }
-  return !fs->exists(CLIENT_ACL_PRIMARY_PATH)
-      && !fs->exists(CLIENT_ACL_BACKUP_PATH)
+  if (!clientACLFilePresence(fs, CLIENT_ACL_BACKUP_PATH, backup_exists)) return false;
+  if (backup_exists) {
+    const auto result = clientACLValidation(is_valid(fs, CLIENT_ACL_BACKUP_PATH));
+    if (result == ClientACLFileValidation::MetadataError) return false;
+    if (result == ClientACLFileValidation::Valid) {
+      if (!removeClientACLArtifact(fs, CLIENT_ACL_PRIMARY_PATH)
+          || !fs->rename(CLIENT_ACL_BACKUP_PATH, CLIENT_ACL_PRIMARY_PATH)) {
+        return false;
+      }
+      removeClientACLArtifact(fs, CLIENT_ACL_TEMP_PATH);
+      return true;
+    }
+  }
+  return !primary_exists && !backup_exists
       && removeClientACLArtifact(fs, CLIENT_ACL_TEMP_PATH);
 }
 
@@ -66,9 +105,11 @@ bool publishVerifiedClientACLTemp(Filesystem* fs, bool temp_verified) {
     removeClientACLArtifact(fs, CLIENT_ACL_TEMP_PATH);
     return false;
   }
-  if (fs->exists(CLIENT_ACL_BACKUP_PATH)) return false;
+  bool backup_exists, had_primary;
+  if (!clientACLFilePresence(fs, CLIENT_ACL_BACKUP_PATH, backup_exists)
+      || backup_exists
+      || !clientACLFilePresence(fs, CLIENT_ACL_PRIMARY_PATH, had_primary)) return false;
 
-  const bool had_primary = fs->exists(CLIENT_ACL_PRIMARY_PATH);
   if (had_primary
       && !fs->rename(CLIENT_ACL_PRIMARY_PATH, CLIENT_ACL_BACKUP_PATH)) {
     return false;
@@ -76,7 +117,9 @@ bool publishVerifiedClientACLTemp(Filesystem* fs, bool temp_verified) {
   if (!fs->rename(CLIENT_ACL_TEMP_PATH, CLIENT_ACL_PRIMARY_PATH)) {
     // Restore the last committed image immediately when possible. If this
     // rename also fails, leave both artifacts for recoverClientACLFiles().
-    if (had_primary && !fs->exists(CLIENT_ACL_PRIMARY_PATH)) {
+    bool primary_exists;
+    if (had_primary && clientACLFilePresence(fs, CLIENT_ACL_PRIMARY_PATH, primary_exists)
+        && !primary_exists) {
       fs->rename(CLIENT_ACL_BACKUP_PATH, CLIENT_ACL_PRIMARY_PATH);
     }
     return false;
@@ -95,16 +138,18 @@ bool publishVerifiedClientACLTemp(Filesystem* fs, bool temp_verified,
     removeClientACLArtifact(fs, CLIENT_ACL_TEMP_PATH);
     return false;
   }
-  if (fs->exists(CLIENT_ACL_BACKUP_PATH)) return false;
-  const bool had_primary = fs->exists(CLIENT_ACL_PRIMARY_PATH);
+  bool backup_exists, had_primary;
+  if (!clientACLFilePresence(fs, CLIENT_ACL_BACKUP_PATH, backup_exists)
+      || backup_exists
+      || !clientACLFilePresence(fs, CLIENT_ACL_PRIMARY_PATH, had_primary)) return false;
   if (had_primary
       && !fs->rename(CLIENT_ACL_PRIMARY_PATH, CLIENT_ACL_BACKUP_PATH)) {
     return false;
   }
   if (!fs->rename(CLIENT_ACL_TEMP_PATH, CLIENT_ACL_PRIMARY_PATH)
-      || !is_valid(fs, CLIENT_ACL_PRIMARY_PATH)) {
-    removeClientACLArtifact(fs, CLIENT_ACL_PRIMARY_PATH);
-    if (had_primary) {
+      || clientACLValidation(is_valid(fs, CLIENT_ACL_PRIMARY_PATH))
+          != ClientACLFileValidation::Valid) {
+    if (removeClientACLArtifact(fs, CLIENT_ACL_PRIMARY_PATH) && had_primary) {
       fs->rename(CLIENT_ACL_BACKUP_PATH, CLIENT_ACL_PRIMARY_PATH);
     }
     return false;

test/fixtures/client_acl_metadata/mocks/sys/stat.h

@@ -0,0 +1,25 @@
+#pragma once
+#include <cerrno>
+#include <cstring>
+
+struct stat {};
+namespace acl_metadata {
+inline std::map<std::string, unsigned> calls;
+inline std::map<std::string, std::set<unsigned>> failures;
+inline void reset() { calls.clear(); failures.clear(); }
+}
+inline int stat(const char* path, struct stat*) {
+  if (!metadata_filesystem || strncmp(path, "/spiffs", 7) != 0) {
+    errno = EIO;
+    return -1;
+  }
+  const std::string name(path + 7);
+  const unsigned occurrence = ++acl_metadata::calls[name];
+  if (metadata_filesystem->metadata_error
+      || acl_metadata::failures[name].count(occurrence)) {
+    errno = EIO;
+    return -1;
+  }
+  if (!metadata_filesystem->files.count(name)) { errno = ENOENT; return -1; }
+  return 0;
+}

test/fixtures/client_acl_metadata/test.cpp

@@ -0,0 +1,200 @@
+// Compile the real implementation and transaction templates against metadata
+// failures. The FS mock keeps SDK-internal namespace operations independent of
+// caller-visible exists() so redundant source probes can be counted honestly.
+#include <ClientACL.cpp>
+#include <cstdio>
+#include <cstdlib>
+
+#define CHECK(condition) do { if (!(condition)) { \
+  std::fprintf(stderr, "FAIL line %d: %s\n", __LINE__, #condition); std::exit(1); \
+} } while (0)
+
+static const char* P = mesh::CLIENT_ACL_PRIMARY_PATH;
+static const char* T = mesh::CLIENT_ACL_TEMP_PATH;
+static const char* B = mesh::CLIENT_ACL_BACKUP_PATH;
+static mesh::LocalIdentity self;
+static const uint8_t key[PUB_KEY_SIZE] = {0x12, 0x34};
+
+static std::vector<uint8_t> crc_image(uint8_t value) {
+  std::vector<uint8_t> bytes(201, value);
+  const uint32_t crc = mesh::updateClientACLCRC(
+      0xffffffffu, bytes.data(), bytes.size()) ^ 0xffffffffu;
+  bytes.insert(bytes.end(), mesh::CLIENT_ACL_CRC_MAGIC,
+               mesh::CLIENT_ACL_CRC_MAGIC + 4);
+  const uint8_t* raw = reinterpret_cast<const uint8_t*>(&crc);
+  bytes.insert(bytes.end(), raw, raw + sizeof(crc));
+  return bytes;
+}
+
+static void admission_benchmark() {
+  FakeFilesystem fs;
+  ClientACL acl;
+  acl.load(&fs, self);
+  for (unsigned i = 0; i < 32; ++i) {
+    uint8_t public_key[PUB_KEY_SIZE] = {};
+    public_key[0] = uint8_t(i + 1);
+    CHECK(acl.applyPermissions(self, public_key, PUB_KEY_SIZE,
+        i == 0 ? PERM_ACL_ADMIN : PERM_ACL_READ_ONLY));
+  }
+  CHECK(acl.save(&fs));
+  fs.exists_calls = 0;
+  fs.read_open_count.clear();
+  CHECK(acl.save(&fs));
+  unsigned read_opens = 0;
+  for (const auto& item : fs.read_open_count) read_opens += item.second;
+  const unsigned existence_probes = fs.exists_calls;
+  CHECK(fs.files[P].size() == 32 * 201 + 8);
+  CHECK(validateContactsFileIntegrity(&fs, P));
+  // Exclude this final assertion's extra reader from the benchmark count.
+  std::printf("BENCH exists=%u reads=%u bytes=%zu\n",
+      existence_probes, read_opens, fs.files[P].size());
+}
+
+#if !defined(ACL_METADATA_BENCHMARK_ONLY)
+static void absent_and_error_are_distinct() {
+  FakeFilesystem fs;
+  acl_metadata::reset();
+  bool present = true;
+  CHECK(mesh::clientACLFilePresence(&fs, P, present) && !present);
+  CHECK(fs.exists_calls == 0);
+  acl_metadata::failures[P].insert(2);
+  present = true;
+  CHECK(!mesh::clientACLFilePresence(&fs, P, present) && present);
+  CHECK(fs.remove_calls == 0 && fs.rename_calls == 0);
+  bool metadata_ok = true;
+  fs.metadata_error = true;
+  CHECK(!openRead(&fs, P, &metadata_ok) && !metadata_ok);
+  CHECK(fs.read_open_count.empty());
+}
+
+static void decisions_fail_before_mutation() {
+  for (const char* failed : {P, B}) {
+    FakeFilesystem fs;
+    fs.files[P] = crc_image(0x11);
+    fs.files[T] = crc_image(0x22);
+    const auto before = fs.files;
+    acl_metadata::reset();
+    acl_metadata::failures[failed].insert(1);
+    CHECK(!mesh::publishVerifiedClientACLTemp(&fs, true,
+        validateContactsFileIntegrityChecked));
+    CHECK(fs.files == before && fs.remove_calls == 0 && fs.rename_calls == 0);
+  }
+  FakeFilesystem fs;
+  fs.files[P] = crc_image(0x11);
+  fs.files[T] = crc_image(0x22);
+  const auto before = fs.files;
+  acl_metadata::reset();acl_metadata::failures[P].insert(1);
+  CHECK(!mesh::recoverClientACLFiles(&fs));
+  CHECK(fs.files == before && fs.remove_calls == 0 && fs.rename_calls == 0);
+}
+
+static void backup_error_cannot_make_legacy_valid_or_delete_primary() {
+  FakeFilesystem fs;
+  fs.files[P] = std::vector<uint8_t>(201, 0x11);
+  fs.files[B] = crc_image(0x22);
+  const auto before = fs.files;
+  acl_metadata::reset();acl_metadata::failures[B].insert(1);
+  CHECK(!mesh::recoverClientACLFilesVerified(&fs,
+      validateContactsFileIntegrityChecked));
+  CHECK(fs.files == before && fs.remove_calls == 0 && fs.rename_calls == 0);
+  acl_metadata::reset();acl_metadata::failures[B].insert(1);
+  CHECK(validateContactsFileIntegrityChecked(&fs, P)
+      == mesh::ClientACLFileValidation::MetadataError);
+  CHECK(fs.files == before);
+}
+
+static void missing_path_lookup_keeps_original_policy() {
+  FakeFilesystem fs;
+  ClientInfo client = {};
+  client.out_path_is_persistable = false;
+  client.out_path_len = 1;
+  uint8_t previous[MAX_PATH_SIZE] = {};
+  uint8_t length = OUT_PATH_UNKNOWN;
+  bool metadata_ok = true;
+  acl_metadata::reset();
+  auto view = storedClientPathForSave(&fs, &client, previous, &length,
+                                     &metadata_ok);
+  CHECK(metadata_ok && view.encoded_path_len == OUT_PATH_UNKNOWN);
+  CHECK(fs.missing_read_opens == 0);
+}
+
+static void stored_path_metadata_failure_never_publishes_unknown() {
+  for (const std::set<unsigned>& faults : {
+      std::set<unsigned>{3}, std::set<unsigned>{4}, std::set<unsigned>{3, 4}}) {
+    FakeFilesystem fs;
+    ClientACL acl;acl_metadata::reset();acl.load(&fs, self);
+    CHECK(acl.applyPermissions(self, key, PUB_KEY_SIZE, PERM_ACL_ADMIN));
+    auto* client = acl.getClient(key, PUB_KEY_SIZE);
+    client->out_path_len = 1;client->out_path[0] = 0x5a;
+    CHECK(acl.save(&fs));
+    const auto original = fs.files[P];
+    client->out_path_is_persistable = false;client->out_path[0] = 0x7b;
+    fs.exists_calls = 0;fs.remove_calls = fs.rename_calls = 0;
+    acl_metadata::reset();acl_metadata::failures[P] = faults;
+    CHECK(!acl.save(&fs));
+    CHECK(fs.files[P] == original && fs.rename_calls == 0);
+    CHECK(fs.exists_calls == 0);
+  }
+}
+
+static void postrename_error_rolls_back_only_with_confirmed_presence() {
+  for (bool persistent : {false, true}) {
+    FakeFilesystem fs;
+    const auto old = crc_image(0x11), next = crc_image(0x22);
+    fs.files[P] = old;fs.files[T] = next;
+    acl_metadata::reset();acl_metadata::failures[P].insert(2);
+    if (persistent) acl_metadata::failures[P].insert(3);
+    CHECK(!mesh::publishVerifiedClientACLTemp(&fs, true,
+        validateContactsFileIntegrityChecked));
+    if (!persistent) {
+      CHECK(fs.files[P] == old && !fs.files.count(B));
+    } else {
+      // A failed metadata probe is never absence. Keep both recoverable
+      // artifacts when even rollback cannot prove the new target present.
+      CHECK(fs.files[P] == next && fs.files[B] == old && fs.remove_calls == 0);
+      acl_metadata::reset();
+      CHECK(mesh::recoverClientACLFilesVerified(&fs,
+          validateContactsFileIntegrityChecked));
+      CHECK(fs.files[P] == next && !fs.files.count(B));
+    }
+  }
+}
+
+static void committed_primary_survives_cleanup_metadata_error() {
+  FakeFilesystem fs;
+  const auto old = crc_image(0x11), next = crc_image(0x22);
+  fs.files[P] = old;fs.files[T] = next;
+  acl_metadata::reset();acl_metadata::failures[B].insert(3);
+  CHECK(mesh::publishVerifiedClientACLTemp(&fs, true,
+      validateContactsFileIntegrityChecked));
+  CHECK(fs.files[P] == next && fs.files[B] == old && fs.remove_calls == 0);
+}
+
+static void complete_readback_and_crc_checks_remain_required() {
+  for (unsigned fault = 0; fault < 3; ++fault) {
+    FakeFilesystem fs;ClientACL acl;acl_metadata::reset();acl.load(&fs, self);
+    CHECK(acl.applyPermissions(self, key, PUB_KEY_SIZE, PERM_ACL_ADMIN));
+    CHECK(acl.save(&fs));const auto old = fs.files[P];
+    if (fault == 0) fs.corrupt_on_close = T;
+    if (fault == 1) fs.write_budget = 3;
+    if (fault == 2) fs.unreadable.insert(T);
+    CHECK(!acl.save(&fs));CHECK(fs.files[P] == old);
+  }
+}
+#endif
+
+int main() {
+#if !defined(ACL_METADATA_BENCHMARK_ONLY)
+  absent_and_error_are_distinct();
+  decisions_fail_before_mutation();
+  backup_error_cannot_make_legacy_valid_or_delete_primary();
+  missing_path_lookup_keeps_original_policy();
+  stored_path_metadata_failure_never_publishes_unknown();
+  postrename_error_rolls_back_only_with_confirmed_presence();
+  committed_primary_survives_cleanup_metadata_error();
+  complete_readback_and_crc_checks_remain_required();
+  std::puts("ACL metadata failure checks passed");
+#else
+  admission_benchmark();
+#endif
+}

test/test_client_acl_metadata.py

@@ -0,0 +1,139 @@
+"""Execute ESP ACL metadata decisions, errors and old/new probe admission."""
+from pathlib import Path
+import re
+import shutil
+import subprocess
+import sys
+import tempfile
+import unittest
+
+ROOT = Path(__file__).resolve().parents[1]
+FIXTURE = ROOT / "test/fixtures/client_acl_metadata"
+MOCKS = ROOT / "test/fixtures/client_acl_spiffs/mocks"
+
+
+class ClientAclMetadataTest(unittest.TestCase):
+    def run_fixture(self, *, source_overrides=None, benchmark=False):
+        compiler = shutil.which("g++") or shutil.which("clang++")
+        if not compiler:
+            self.skipTest("a C++17 compiler is required")
+        with tempfile.TemporaryDirectory(prefix="meshcore-acl-metadata-") as directory:
+            work = Path(directory)
+            mocks = work / "mocks"
+            shutil.copytree(MOCKS, mocks)
+            shutil.copyfile(FIXTURE / "mocks/sys/stat.h", mocks / "sys/stat.h")
+            # The existing functional mock models the namespace, not VFS's
+            # extra fopen in exists(). Count calls from production separately
+            # from mock open/rename internals, which consult namespace maps.
+            arduino = (mocks / "Arduino.h").read_text()
+            replacements = {
+                "  bool metadata_error = false;": "  bool metadata_error = false;\n"
+                    "  mutable unsigned exists_calls = 0;\n"
+                    "  unsigned remove_calls = 0, rename_calls = 0;",
+                "    return files.count(path) != 0 && unreadable.count(path) == 0;":
+                    "    ++exists_calls;\n"
+                    "    return files.count(path) != 0 && unreadable.count(path) == 0;",
+                "      if (!exists(path)) {": "      if (!files.count(name)) {",
+                "  bool remove(const char* path) {":
+                    "  bool remove(const char* path) {\n    ++remove_calls;",
+                "  bool rename(const char* from, const char* to) {":
+                    "  bool rename(const char* from, const char* to) {\n    ++rename_calls;",
+            }
+            for old, new in replacements.items():
+                self.assertEqual(arduino.count(old), 1)
+                arduino = arduino.replace(old, new, 1)
+            (mocks / "Arduino.h").write_text(arduino)
+            helpers = work / "helpers"
+            helpers.mkdir()
+            for name, source in (source_overrides or {}).items():
+                (helpers / name).write_text(source)
+            binary = work / "acl-metadata"
+            sanitizer = ["-fsanitize=address,undefined", "-fno-sanitize-recover=all",
+                         "-fno-pie", "-no-pie"] if sys.platform.startswith("linux") else []
+            compile_result = subprocess.run([
+                compiler, "-std=c++17", "-Wall", "-Wextra", *sanitizer,
+                "-DESP32=1", "-DESP32_PLATFORM=1",
+                *( ["-DACL_METADATA_BENCHMARK_ONLY=1"] if benchmark else [] ),
+                f"-I{mocks}", f"-I{helpers}", f"-I{ROOT / 'src/helpers'}",
+                f"-I{ROOT / 'src'}", str(FIXTURE / "test.cpp"), "-o", str(binary),
+            ], capture_output=True, text=True, timeout=60)
+            self.assertEqual(compile_result.returncode, 0,
+                             compile_result.stdout + compile_result.stderr)
+            return subprocess.run([str(binary)], capture_output=True, text=True, timeout=10)
+
+    def test_actual_acl_metadata_failures_and_publication(self):
+        result = self.run_fixture()
+        self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+        self.assertIn("ACL metadata failure checks passed", result.stdout)
+
+    def test_actual_save_reduces_source_exists_without_skipping_readback(self):
+        # Recreate the former FS::exists admission only. Both runs retain the
+        # current production serializer, readback, CRC and rename bodies. No
+        # Git history is needed in a shallow CI checkout, and this measures
+        # source admission calls, not SDK-internal fopen costs or wall time.
+        old = {name: (ROOT / "src/helpers" / name).read_text()
+               for name in ("ClientACL.cpp", "ClientACLFileTransaction.h")}
+        header_anchor = "#if defined(ESP32_PLATFORM)\n  return filePresence(fs, path, present);"
+        self.assertEqual(old["ClientACLFileTransaction.h"].count(header_anchor), 1)
+        old["ClientACLFileTransaction.h"] = old["ClientACLFileTransaction.h"].replace(
+            header_anchor, "#if 0\n  return filePresence(fs, path, present);", 1)
+        reader_anchor = "bool* metadata_ok = nullptr) {\n#if defined(ESP32_PLATFORM)"
+        self.assertEqual(old["ClientACL.cpp"].count(reader_anchor), 1)
+        old["ClientACL.cpp"] = old["ClientACL.cpp"].replace(
+            reader_anchor, "bool* metadata_ok = nullptr) {\n#if 0", 1)
+        before = self.run_fixture(source_overrides=old, benchmark=True)
+        after = self.run_fixture(benchmark=True)
+        for result in (before, after):
+            self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+        pattern = r"BENCH exists=(\d+) reads=(\d+) bytes=(\d+)"
+        first = tuple(map(int, re.search(pattern, before.stdout).groups()))
+        second = tuple(map(int, re.search(pattern, after.stdout).groups()))
+        self.assertGreater(first[0], 0)
+        self.assertEqual(second[0], 0)
+        self.assertEqual(first[1:], second[1:])
+        self.assertEqual(second[2], 32 * 201 + 8)
+        print(f"ACL save former-probe model {first[0]}->0 source exists calls; "
+              f"actual read opens {second[1]}; "
+              f"same {second[2]}-byte image. SDK-internal rename/remove opens excluded.")
+
+    def test_exists_fallback_negative_control_is_detected(self):
+        source = (ROOT / "src/helpers/ClientACLFileTransaction.h").read_text()
+        anchor = "#if defined(ESP32_PLATFORM)\n  return filePresence(fs, path, present);"
+        self.assertEqual(source.count(anchor), 1)
+        result = self.run_fixture(source_overrides={
+            "ClientACLFileTransaction.h": source.replace(anchor,
+                "#if 0\n  return filePresence(fs, path, present);", 1),
+            # Make the local quoted include resolve to the isolated header.
+            "ClientACL.cpp": (ROOT / "src/helpers/ClientACL.cpp").read_text(),
+        })
+        self.assertNotEqual(result.returncode, 0)
+        self.assertIn("fs.exists_calls == 0", result.stderr)
+
+    def test_stored_path_status_negative_control_is_detected(self):
+        source = (ROOT / "src/helpers/ClientACL.cpp").read_text()
+        anchors = {
+            "success = success && metadata_ok": "success = success && true",
+            "matches = metadata_ok && readMatches": "matches = true && readMatches",
+        }
+        for old, new in anchors.items():
+            self.assertEqual(source.count(old), 1)
+            source = source.replace(old, new, 1)
+        result = self.run_fixture(source_overrides={"ClientACL.cpp": source})
+        self.assertNotEqual(result.returncode, 0)
+        self.assertIn("!acl.save(&fs)", result.stderr)
+
+    def test_metadata_error_cannot_be_treated_as_corruption(self):
+        source = (ROOT / "src/helpers/ClientACLFileTransaction.h").read_text()
+        anchor = "if (result == ClientACLFileValidation::MetadataError) return false;"
+        self.assertEqual(source.count(anchor), 2)
+        source = source.replace(anchor, "// negative control: swallow metadata failure")
+        result = self.run_fixture(source_overrides={
+            "ClientACLFileTransaction.h": source,
+            "ClientACL.cpp": (ROOT / "src/helpers/ClientACL.cpp").read_text(),
+        })
+        self.assertNotEqual(result.returncode, 0)
+        self.assertIn("recoverClientACLFilesVerified", result.stderr)
+
+
+if __name__ == "__main__":
+    unittest.main()