diff --git a/docs/research/companion_contact_cache.md b/docs/research/companion_contact_cache.md index 2fe191962..627f7993b 100644 --- a/docs/research/companion_contact_cache.md +++ b/docs/research/companion_contact_cache.md @@ -44,7 +44,10 @@ If damaged contacts cannot be removed, the node remains usable with volatile contacts; explicit saves still report failure, while reboot/shutdown remain available. Identity, credentials, channels and unrelated settings are not erased. Allocation/capacity failures and shared filesystem-migration authority errors -are not corrupt-contact evidence and do not authorize this reset. The path checksum also rejects +are not corrupt-contact evidence and do not authorize this reset. Failed source +discovery also preserves contact files; if authority stays unknown, use volatile +contacts instead of deleting readable pages or selecting a potentially stale view. +The path checksum also rejects a cold path changed underneath a live handle; it does not add a new checksum to the legacy ESP32 on-disk format. diff --git a/examples/companion_radio/DataStore.cpp b/examples/companion_radio/DataStore.cpp index d009f93f3..1e24cfe28 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -2369,14 +2369,14 @@ void DataStore::loadContacts(DataStoreHost* host) { MESH_DEBUG_PRINTLN( "DataStore: HIL injecting contact page %u stat failure", page); _contact_load_incomplete = true; - _unread_contact_pages.mark(page); queueContactRecovery(); return; } #endif if (!contactPathPresence(contacts_fs, path, page_exists)) { _contact_load_incomplete = true; - _unread_contact_pages.mark(page); + // Discovery failed before the contact source was selected. This is not + // evidence that the page payload is unreadable or safe to discard. queueContactRecovery(); return; } @@ -2388,7 +2388,9 @@ void DataStore::loadContacts(DataStoreHost* host) { bool migration_marker_exists = false; if (!contactPathPresence(contacts_fs, "/contacts3", legacy_exists)) { _contact_load_incomplete = true; - queueContactRecovery(true); + // Unknown legacy presence does not make readable pages expendable: they + // may be the only authoritative store. Retry without authorizing erasure. + queueContactRecovery(); return; } if (!contactPathPresence(contacts_fs, CONTACT_MIGRATION_MARKER, diff --git a/test/fixtures/contact_cache/test_contact_cache.cpp b/test/fixtures/contact_cache/test_contact_cache.cpp index 9ffd9b80a..8fb4e2282 100644 --- a/test/fixtures/contact_cache/test_contact_cache.cpp +++ b/test/fixtures/contact_cache/test_contact_cache.cpp @@ -47,7 +47,14 @@ using DataStoreHost = Host; #if defined(NRF52_PLATFORM) static const char* CONTACT_MIGRATION_MARKER = "/contacts4.mig"; static bool fail_contact_migration_marker = false; +static bool fail_contact_legacy_stat = false; +static int fail_contact_page_stats = 0; bool contactPathPresence(FakeFilesystem* fs, const char* path, bool& present) { + if (fail_contact_page_stats && !strcmp(path, "/contacts4_00")) { + if (fail_contact_page_stats > 0) --fail_contact_page_stats; + return false; + } + if (fail_contact_legacy_stat && !strcmp(path, "/contacts3")) return false; if (fail_contact_migration_marker && !strcmp(path, CONTACT_MIGRATION_MARKER)) return false; present = fs->exists(path); return true; @@ -1090,6 +1097,108 @@ static void nrf52_marker_error_keeps_a_usable_contact_view() { assert(SPIFFS.files == durable && SPIFFS.writes == writes && SPIFFS.removes == removes); } +static void nrf52_unknown_legacy_stat_preserves_contact_authority() { + for (bool legacy_present : {false, true}) { + for (bool persistent_failure : {false, true}) { + Fixture f(30); + assert(f.store.writeContactPage(&f.host, 0, cachedContactFilter)); + assert(f.store.writeContactPage(&f.host, 1, cachedContactFilter)); + if (legacy_present) { + // A downgrade may leave a shorter authoritative legacy view beside + // stale, otherwise completely readable pages. No migration marker + // permits treating those extra records as a committed page tail. + SPIFFS.files["/contacts3"].resize(5 * mesh::storage::CONTACT_RECORD_SIZE); + } else { + SPIFFS.remove("/contacts3"); + } + const FakeFilesystem::Files unrelated = { + {"/_main.id", {1, 3}}, {"/new_prefs", {2, 4}}, {"/channels2", {5, 7}}, + {"/.extrafs.mig", {'M', 'C', 'X', 'F', 1, 2}}}; + for (const auto& file : unrelated) SPIFFS.files[file.first] = file.second; + const auto durable = SPIFFS.files; + const unsigned writes = SPIFFS.writes, removes = SPIFFS.removes; + f.host.contacts.clear(); + fail_contact_legacy_stat = true; + f.store.loadContacts(&f.host); + assert(f.store.contactRecoveryPending() && f.store.hasIncompleteContactLoad()); + assert(f.host.contacts.empty() && SPIFFS.files == durable); + if (!persistent_failure) fail_contact_legacy_stat = false; + f.store.recoverContacts(&f.host); + assert(!f.store.contactRecoveryPending() && !f.store.hasIncompleteContactLoad()); + assert(f.store.contactsAreVolatile() == persistent_failure); + const unsigned expected_contacts = legacy_present ? 5 : 30; + assert(f.host.contacts.size() == (persistent_failure ? 0 : expected_contacts)); + if (persistent_failure) { + // Unknown source authority is not a corrupt legacy image. Keep both + // candidate namespaces unchanged and permit usable volatile hardware. + assert(SPIFFS.files == durable); + assert(SPIFFS.writes == writes && SPIFFS.removes == removes); + assert(!f.store.saveContacts(&f.host, cachedContactFilter)); + f.store.recoverContacts(&f.host); + assert(SPIFFS.files == durable && f.host.contacts.empty()); + } + for (const auto& file : unrelated) assert(SPIFFS.files.at(file.first) == file.second); + fail_contact_legacy_stat = false; + f.host.contacts.clear(); + DataStore rebooted(f.host); + rebooted.loadContacts(&f.host); + assert(!rebooted.hasIncompleteContactLoad() && !rebooted.contactRecoveryPending()); + assert(!rebooted.contactsAreVolatile() && f.host.contacts.size() == expected_contacts); + for (unsigned i = 0; i < expected_contacts; ++i) f.check(i, route(i)); + } + } +} + +static void nrf52_unknown_page_stat_preserves_contact_authority() { + for (bool legacy_present : {false, true}) { + for (int failures : {1, 2, -1}) { + Fixture f(30); + assert(f.store.writeContactPage(&f.host, 0, cachedContactFilter)); + assert(f.store.writeContactPage(&f.host, 1, cachedContactFilter)); + if (legacy_present) { + SPIFFS.files["/contacts3"].resize(5 * mesh::storage::CONTACT_RECORD_SIZE); + } else { + SPIFFS.remove("/contacts3"); + } + const FakeFilesystem::Files unrelated = { + {"/_main.id", {1, 3}}, {"/new_prefs", {2, 4}}, {"/channels2", {5, 7}}, + {"/.extrafs.mig", {'M', 'C', 'X', 'F', 1, 2}}}; + for (const auto& file : unrelated) SPIFFS.files[file.first] = file.second; + const auto durable = SPIFFS.files; + const unsigned writes = SPIFFS.writes, removes = SPIFFS.removes; + f.host.contacts.clear(); + fail_contact_page_stats = failures; + f.store.loadContacts(&f.host); + assert(f.store.contactRecoveryPending() && f.store.hasIncompleteContactLoad()); + assert(f.host.contacts.empty() && SPIFFS.files == durable); + f.store.recoverContacts(&f.host); + const bool persistent_failure = failures < 0; + assert(!f.store.contactRecoveryPending() && !f.store.hasIncompleteContactLoad()); + assert(f.store.contactsAreVolatile() == persistent_failure); + const unsigned expected_contacts = legacy_present ? 5 : 30; + assert(f.host.contacts.size() == (persistent_failure ? 0 : expected_contacts)); + // In particular, two metadata failures clear just before the cleanup + // probe. That later successful stat is not permission to erase a page + // whose complete bytes never failed validation or even a read attempt. + assert(SPIFFS.files == durable); + assert(SPIFFS.writes == writes && SPIFFS.removes == removes); + for (const auto& file : unrelated) assert(SPIFFS.files.at(file.first) == file.second); + if (persistent_failure) { + assert(!f.store.saveContacts(&f.host, cachedContactFilter)); + f.store.recoverContacts(&f.host); + assert(SPIFFS.files == durable && f.host.contacts.empty()); + } + fail_contact_page_stats = 0; + f.host.contacts.clear(); + DataStore rebooted(f.host); + rebooted.loadContacts(&f.host); + assert(!rebooted.hasIncompleteContactLoad() && !rebooted.contactRecoveryPending()); + assert(!rebooted.contactsAreVolatile() && f.host.contacts.size() == expected_contacts); + for (unsigned i = 0; i < expected_contacts; ++i) f.check(i, route(i)); + } + } +} + static void nrf52_unread_legacy_and_unknown_marker_cannot_resurrect_stale_pages() { Fixture f(30); assert(f.store.writeContactPage(&f.host, 0, cachedContactFilter)); @@ -1225,6 +1334,8 @@ int main() { nrf52_migration_snapshots_and_failed_page_replacement(); nrf52_resource_refusals_never_discard_contact_pages(); nrf52_marker_error_keeps_a_usable_contact_view(); + nrf52_unknown_legacy_stat_preserves_contact_authority(); + nrf52_unknown_page_stat_preserves_contact_authority(); nrf52_unread_legacy_and_unknown_marker_cannot_resurrect_stale_pages(); nrf52_uncertain_page_publication_preserves_snapshots(); encoded_paths_and_copy_assignment(); diff --git a/test/test_contact_cache.py b/test/test_contact_cache.py index c9e9eea1c..b4a48923b 100644 --- a/test/test_contact_cache.py +++ b/test/test_contact_cache.py @@ -50,7 +50,14 @@ class ContactCacheTest(unittest.TestCase): def test_esp32_disabled_contact_recovery_is_detected(self): self.run_cache_test("ESP32_PLATFORM", disable_recovery=True) - def run_cache_test(self, platform, disable_crc_batch=False, disable_recovery=False): + def test_nrf52_unsafe_legacy_discovery_recovery_is_detected(self): + self.run_cache_test("NRF52_PLATFORM", unsafe_legacy_discovery=True) + + def test_nrf52_unsafe_page_discovery_recovery_is_detected(self): + self.run_cache_test("NRF52_PLATFORM", unsafe_page_discovery=True) + + def run_cache_test(self, platform, disable_crc_batch=False, disable_recovery=False, + unsafe_legacy_discovery=False, unsafe_page_discovery=False): with tempfile.TemporaryDirectory(prefix="mesh-contact-cache-") as temp: temp = Path(temp) store = (ROOT / "examples/companion_radio/DataStore.cpp").read_text() @@ -112,6 +119,22 @@ class ContactCacheTest(unittest.TestCase): self.assertIn(recovery, implementation) implementation = implementation.replace( recovery, "void DataStore::recoverContacts(DataStoreHost*) {}") + if unsafe_legacy_discovery: + discovery = method(store, + 'if (!contactPathPresence(contacts_fs, "/contacts3", legacy_exists))') + self.assertEqual(discovery.count("queueContactRecovery();"), 1) + self.assertEqual(implementation.count(discovery), 1) + implementation = implementation.replace(discovery, discovery.replace( + "queueContactRecovery();", "queueContactRecovery(true);")) + if unsafe_page_discovery: + discovery = method(store, + "if (!contactPathPresence(contacts_fs, path, page_exists))") + self.assertNotIn("_unread_contact_pages.mark(page);", discovery) + self.assertEqual(discovery.count("queueContactRecovery();"), 1) + self.assertEqual(implementation.count(discovery), 1) + implementation = implementation.replace(discovery, discovery.replace( + "queueContactRecovery();", + "_unread_contact_pages.mark(page);\n queueContactRecovery();")) if platform == "NRF52_PLATFORM": # Fault only the page snapshot allocation; keep production # loading/recovery logic and unrelated crypto allocations. @@ -159,7 +182,15 @@ class ContactCacheTest(unittest.TestCase): ], capture_output=True, text=True) self.assertEqual(result.returncode, 0, result.stderr) result = subprocess.run([str(binary)], capture_output=True, text=True) - if disable_recovery: + if unsafe_page_discovery: + self.assertNotEqual(result.returncode, 0) + self.assertIn("nrf52_unknown_page_stat_preserves_contact_authority", result.stderr) + self.assertIn("f.host.contacts.size()", result.stderr) + elif unsafe_legacy_discovery: + self.assertNotEqual(result.returncode, 0) + self.assertIn("nrf52_unknown_legacy_stat_preserves_contact_authority", result.stderr) + self.assertIn("SPIFFS.files == durable", result.stderr) + elif disable_recovery: self.assertNotEqual(result.returncode, 0) self.assertIn("!f.store.contactRecoveryPending()", result.stderr) elif disable_crc_batch: