From 508dd2367398df75fab459300c8a7ed063d093ec Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 10:59:11 -0700 Subject: [PATCH] Keep channel faults from disabling healthy Companion contacts --- .github/workflows/run-unit-tests.yml | 2 + examples/companion_radio/DataStore.cpp | 173 ++++++++------ examples/companion_radio/DataStore.h | 3 +- examples/companion_radio/MyMesh.cpp | 5 +- .../companion_channel_storage/checks.h | 214 ++++++++++++++++++ .../test_secondary_migration.cpp | 1 + test/test_companion_channel_storage.py | 172 ++++++++++++++ ...companion_settings_persistence_contract.py | 15 +- test/test_companion_tx_routing.py | 40 ++-- test/test_companion_uncached_storage.py | 13 +- test/test_contact_cache.py | 16 +- test/test_nrf52_extrafs_contract.py | 16 +- 12 files changed, 572 insertions(+), 98 deletions(-) create mode 100644 test/fixtures/companion_channel_storage/checks.h create mode 100644 test/test_companion_channel_storage.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 28099b18c..f3af9817a 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -349,6 +349,8 @@ jobs: python3 -B test/test_companion_identity_cli.py -v python3 -B test/test_companion_packet_stream.py -v python3 -B test/test_companion_uncached_storage.py -v + python3 -B test/test_companion_channel_storage.py -v + python3 -B test/test_nrf52_extrafs_contract.py -v python3 -B test/test_companion_ota_config.py -v python3 -B test/test_ota_identity_policy.py -v python3 -B test/test_companion_radio_gain_restore.py -v diff --git a/examples/companion_radio/DataStore.cpp b/examples/companion_radio/DataStore.cpp index 98e622e9a..f48c0efff 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -574,6 +574,7 @@ void DataStore::useVolatilePrimaryFS(FILESYSTEM& fs) { _secondary_authority_unknown = false; _identity_creation_blocked = false; _contact_load_incomplete = false; + _channel_load_incomplete = false; _prefs_load_incomplete = false; } @@ -835,6 +836,7 @@ bool DataStore::formatFileSystem() { // failed erase must keep the boot's load quarantine latched. if (success) { _contact_load_incomplete = false; + _channel_load_incomplete = false; #if MESH_CONTACT_CACHE _cache_load_incomplete = false; #endif @@ -3132,10 +3134,11 @@ bool DataStore::hasPendingContactWrites() const { #endif } +// Share the repeated admission/persistence checks instead of duplicating them under LTO. +__attribute__((noinline)) bool DataStore::hasIncompleteContactLoad() const { if (contactRecoveryPending()) return true; #if !defined(NRF52_PLATFORM) - if (_channel_load_incomplete) return true; #if !MESH_CONTACT_CACHE if (_uncached_contact_load_incomplete) return true; #endif @@ -3164,10 +3167,15 @@ void DataStore::loadChannels(DataStoreHost* host) { if (!file) continue; static const uint32_t RECORD_SIZE = 4 + 32 + 32; const size_t size = file.size(); - if (size % RECORD_SIZE != 0 || size / RECORD_SIZE > MAX_GROUP_CHANNELS) { + if (size % RECORD_SIZE != 0) { file.close(); break; } + if (size / RECORD_SIZE > MAX_GROUP_CHANNELS) { + file.close(); + _channel_load_incomplete = true; + return; + } const uint8_t count = size / RECORD_SIZE; ChannelDetails* loaded = count == 0 ? nullptr : static_cast(malloc(sizeof(ChannelDetails) * count)); @@ -3209,10 +3217,11 @@ void DataStore::loadChannels(DataStoreHost* host) { return; #else #if defined(NRF52_PLATFORM) - bool& incomplete = _contact_load_incomplete; -#else - bool& incomplete = _channel_load_incomplete; + // Contact/migration failures can still make the shared filesystem's + // authority unknowable. A channel-only failure must not set that veto. + if (_contact_load_incomplete) return; #endif + bool& incomplete = _channel_load_incomplete; if (incomplete) return; FILESYSTEM* contacts_fs = _getContactsChannelsFS(); #if defined(ESP32_PLATFORM) || defined(RP2040_PLATFORM) @@ -3234,74 +3243,98 @@ void DataStore::loadChannels(DataStoreHost* host) { #else if (!contacts_fs->exists("/channels2")) return; #endif - File file = openRead(contacts_fs, "/channels2"); - if (!file) { - MESH_DEBUG_PRINTLN("DataStore: channels file open failed; storage quarantined"); - incomplete = true; - return; - } -#if defined(NRF52_PLATFORM) - if (file.size() != channels_size) { - file.close(); - MESH_DEBUG_PRINTLN("DataStore: channels file changed while opening; storage quarantined"); - incomplete = true; - return; - } -#else - channels_size = file.size(); -#endif - static const uint32_t CHANNEL_RECORD_SIZE = 4 + 32 + 32; - if ((channels_size % CHANNEL_RECORD_SIZE) != 0 - || channels_size / CHANNEL_RECORD_SIZE > MAX_GROUP_CHANNELS) { - file.close(); - MESH_DEBUG_PRINTLN("DataStore: invalid channels file size; storage quarantined"); - incomplete = true; - return; - } - const uint8_t channel_count = - static_cast(channels_size / CHANNEL_RECORD_SIZE); - ChannelDetails* loaded = channel_count == 0 ? nullptr - : static_cast( - malloc(sizeof(ChannelDetails) * channel_count)); - if (channel_count != 0 && loaded == nullptr) { - file.close(); - MESH_DEBUG_PRINTLN( - "DataStore: no memory for channels snapshot; storage quarantined"); - incomplete = true; - return; - } - bool success = true; - for (uint8_t channel_idx = 0; channel_idx < channel_count; channel_idx++) { - uint8_t unused[4]; - success = file.read(unused, sizeof(unused)) == sizeof(unused) - && file.read(reinterpret_cast(loaded[channel_idx].name), - sizeof(loaded[channel_idx].name)) - == sizeof(loaded[channel_idx].name) - && file.read(loaded[channel_idx].channel.secret, - sizeof(loaded[channel_idx].channel.secret)) - == sizeof(loaded[channel_idx].channel.secret); - if (!success) break; - loaded[channel_idx].name[sizeof(loaded[channel_idx].name) - 1] = 0; - loaded[channel_idx].channel.tx_radio = mesh::decodeRadioTxPolicy(unused[0]); - } - file.close(); - if (!success) { - free(loaded); - MESH_DEBUG_PRINTLN( - "DataStore: channels file read failed; storage quarantined"); - incomplete = true; - return; - } - for (uint8_t channel_idx = 0; channel_idx < channel_count; channel_idx++) { - if (!host->onChannelLoaded(channel_idx, loaded[channel_idx])) { - free(loaded); - MESH_DEBUG_PRINTLN( - "DataStore: channel host refused durable record; storage quarantined"); + const char* channel_path = "/channels2"; + for (;;) { + File file = openRead(contacts_fs, channel_path); + if (!file) { + MESH_DEBUG_PRINTLN("DataStore: channels file open failed; storage quarantined"); incomplete = true; return; } +#if defined(NRF52_PLATFORM) + if (file.size() != channels_size) { + file.close(); + MESH_DEBUG_PRINTLN("DataStore: channels file changed while opening; storage quarantined"); + incomplete = true; + return; + } +#else + channels_size = file.size(); +#endif + static const uint32_t CHANNEL_RECORD_SIZE = 4 + 32 + 32; + const bool malformed = (channels_size % CHANNEL_RECORD_SIZE) != 0; + if (malformed || channels_size / CHANNEL_RECORD_SIZE > MAX_GROUP_CHANNELS) { + file.close(); +#if defined(RP2040_PLATFORM) + // A rename-gap backup takes precedence over replacing a malformed image. + // Read and admit that complete snapshot before changing either name. + if (malformed && channel_path[sizeof("/channels2") - 1] == 0 + && contacts_fs->exists("/channels2.bak")) { + channel_path = "/channels2.bak"; + continue; + } +#endif + MESH_DEBUG_PRINTLN("DataStore: invalid channels file size"); + incomplete = !malformed; + return; + } + const uint8_t channel_count = + static_cast(channels_size / CHANNEL_RECORD_SIZE); + ChannelDetails* loaded = channel_count == 0 ? nullptr + : static_cast( + malloc(sizeof(ChannelDetails) * channel_count)); + if (channel_count != 0 && loaded == nullptr) { + file.close(); + MESH_DEBUG_PRINTLN( + "DataStore: no memory for channels snapshot; storage quarantined"); + incomplete = true; + return; + } + bool success = true; + for (uint8_t channel_idx = 0; channel_idx < channel_count; channel_idx++) { + uint8_t unused[4]; + success = file.read(unused, sizeof(unused)) == sizeof(unused) + && file.read(reinterpret_cast(loaded[channel_idx].name), + sizeof(loaded[channel_idx].name)) + == sizeof(loaded[channel_idx].name) + && file.read(loaded[channel_idx].channel.secret, + sizeof(loaded[channel_idx].channel.secret)) + == sizeof(loaded[channel_idx].channel.secret); + if (!success) break; + loaded[channel_idx].name[sizeof(loaded[channel_idx].name) - 1] = 0; + loaded[channel_idx].channel.tx_radio = mesh::decodeRadioTxPolicy(unused[0]); + } + file.close(); + if (!success) { + free(loaded); + MESH_DEBUG_PRINTLN( + "DataStore: channels file read failed; storage quarantined"); + incomplete = true; + return; + } + for (uint8_t channel_idx = 0; channel_idx < channel_count; channel_idx++) { + if (!host->onChannelLoaded(channel_idx, loaded[channel_idx])) { + free(loaded); + MESH_DEBUG_PRINTLN( + "DataStore: channel host refused durable record; storage quarantined"); + incomplete = true; + return; + } + } + free(loaded); +#if defined(RP2040_PLATFORM) + if (channel_path[sizeof("/channels2") - 1] != 0) { + // Only the proven malformed target is retired. Failed promotion keeps + // the validated RAM table and protects the surviving backup from saves. + contacts_fs->remove("/channels2"); + if (contacts_fs->exists("/channels2") + || !mesh::ContactFileTransaction::recover(contacts_fs, "/channels2") + || !contacts_fs->exists("/channels2") + || contacts_fs->exists("/channels2.bak")) incomplete = true; + } +#endif + return; } - free(loaded); #endif } @@ -3310,9 +3343,7 @@ bool DataStore::saveChannels(DataStoreHost* host) { if (!promoteCompanionRecoveryFile(_getContactsChannelsFS(), "/channels2", _channel_recovery_source)) return false; #endif -#if !defined(NRF52_PLATFORM) if (_channel_load_incomplete) return false; -#endif #if defined(NRF52_PLATFORM) if (_contact_load_incomplete) return false; #endif diff --git a/examples/companion_radio/DataStore.h b/examples/companion_radio/DataStore.h index e0904bf86..098a1ff83 100644 --- a/examples/companion_radio/DataStore.h +++ b/examples/companion_radio/DataStore.h @@ -146,8 +146,8 @@ class DataStore void invalidateAdvertWrite(const uint8_t key[], int key_len); void retireAdvertWrite(const uint8_t key[], int key_len); #endif -#if !defined(NRF52_PLATFORM) bool _channel_load_incomplete = false; +#if !defined(NRF52_PLATFORM) #if !MESH_CONTACT_CACHE bool _uncached_contact_load_incomplete = false; #endif @@ -255,6 +255,7 @@ public: bool flushContactWrites(DataStoreHost* host, bool (*filter)(const ContactInfo& c) = NULL); bool hasPendingContactWrites() const; bool hasIncompleteContactLoad() const; + bool hasIncompleteChannelLoad() const { return _channel_load_incomplete; } void loadChannels(DataStoreHost* host); bool saveChannels(DataStoreHost* host); bool migrateToSecondaryFS(); diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index 4697cc05f..5540e9cd2 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -3696,8 +3696,9 @@ bool MyMesh::handleTxRoutingCommand(const char* command, char* reply, size_t rep kind, parsed.channel ? "index" : "key:hex"); return true; } - if (parsed.set && !canMutateContacts()) { - snprintf(reply, reply_size, "Error: contact/channel storage unavailable"); + if (parsed.set && (!canMutateContacts() + || (parsed.channel && _store->hasIncompleteChannelLoad()))) { + snprintf(reply, reply_size, "Error: storage unavailable"); return true; } diff --git a/test/fixtures/companion_channel_storage/checks.h b/test/fixtures/companion_channel_storage/checks.h new file mode 100644 index 000000000..0a924925b --- /dev/null +++ b/test/fixtures/companion_channel_storage/checks.h @@ -0,0 +1,214 @@ +// Included after the existing real-contact persistence fixture and methods. +static std::vector channelImage(const char* name, unsigned count = 1) { + std::vector image(count * 68, 0); + for (unsigned i = 0; i < count; ++i) { + image[i * 68] = mesh::encodeRadioTxPolicy(mesh::RADIO_TX_SECONDARY); + strncpy(reinterpret_cast(image.data() + i * 68 + 4), name, 31); + memset(image.data() + i * 68 + 36, 0x51, 32); + } + return image; +} + +#if MESH_CONTACT_CACHE +using ChannelFixture = Fixture; +static FakeFilesystem& channelFs() { return SPIFFS; } +#else +struct ChannelFixture { + Host host; + DataStore store; + explicit ChannelFixture(unsigned) { + const auto image = make_original(); + filesystem = FakeFilesystem(); + filesystem.files["/contacts3"] = image; + } +}; +static FakeFilesystem& channelFs() { return filesystem; } +#endif + +static void setShortChannelRead(bool active) { +#if MESH_CONTACT_CACHE + SPIFFS.max_channel_read = active ? 2 : std::numeric_limits::max(); +#else + filesystem.max_read = active ? 2 : std::numeric_limits::max(); +#endif +} + +static void failChannelOpenOnce() { +#if MESH_CONTACT_CACHE + SPIFFS.read_open_failures["/channels2"] = 1; +#else + filesystem.fail_open = "/channels2"; + filesystem.fail_open_remaining = 1; +#endif +} + +static void initializeContacts(ChannelFixture& f) { +#if MESH_CONTACT_CACHE + f.host.contacts.clear(); +#endif + f.store.loadContacts(&f.host); + f.store.recoverContacts(&f.host); + assert(!f.host.contacts.empty() && !f.store.hasIncompleteContactLoad()); + uint8_t path[64]; + assert(f.host.contacts[0].copyPathTo(path)); + ChannelDetails defaults{}; + strcpy(defaults.name, "Public"); + f.host.channels.push_back(defaults); +} + +class MyMesh { + DataStore* _store; +public: + explicit MyMesh(DataStore& store) : _store(&store) {} + bool canMutateContacts() const; +}; +#include "channel_admission_under_test.h" + +static void channel_faults_do_not_poison_contacts() { + // 0 healthy, 1 malformed framing, 2 one open failure, 3 short read, + // 4 allocation failure, 5 host refusal, 6 whole-record overcapacity, + // 7 NRF unknown metadata, 8 preexisting shared-store authority failure. + for (unsigned fault = 0; fault <= 8; ++fault) { +#if !defined(NRF52_PLATFORM) + if (fault >= 7) continue; +#endif +#if defined(ESP32_PLATFORM) + if (fault == 2 || fault == 3) continue; // Existing bounded fallback policy. +#endif + ChannelFixture f(30); + initializeContacts(f); + auto& fs = channelFs(); + fs.files["/channels2"] = channelImage("Saved"); + if (fault == 1) fs.files["/channels2"].pop_back(); + if (fault == 2) failChannelOpenOnce(); + if (fault == 3) setShortChannelRead(true); + if (fault == 4) fail_channel_allocation = true; + if (fault == 5) f.host.channel_accepts = false; + if (fault == 6) fs.files["/channels2"] = channelImage("Larger profile", MAX_GROUP_CHANNELS + 1); +#if defined(NRF52_PLATFORM) + if (fault == 7) fail_channel_metadata = true; + if (fault == 8) f.store.forceSharedAuthorityFailure(); +#endif + const auto channel_bytes = fs.files.at("/channels2"); + const auto durable = fs.files; + const unsigned writes = fs.writes, renames = fs.renames; + f.store.loadChannels(&f.host); + setShortChannelRead(false); + fail_channel_allocation = false; + f.host.channel_accepts = true; +#if defined(NRF52_PLATFORM) + fail_channel_metadata = false; +#endif + MyMesh admission(f.store); + uint8_t path[64]; + if (fault == 8) { + assert(f.store.hasIncompleteContactLoad() && !admission.canMutateContacts()); + assert(!f.store.saveContacts(&f.host, nullptr)); + assert(!f.store.saveChannels(&f.host)); +#if MESH_CONTACT_CACHE + assert(!f.host.contacts[0].copyPathTo(path)); +#endif + assert(fs.files == durable && fs.writes == writes && fs.renames == renames); + continue; + } + // This assertion is the runtime failure checked by the old-coupling + // negative controls, not a source-text or fixture-compilation failure. + assert(!f.store.hasIncompleteContactLoad()); + assert(admission.canMutateContacts() && !f.store.contactRecoveryPending()); + assert(f.host.contacts[0].copyPathTo(path)); + const bool quarantine = fault >= 2; + assert(f.store.hasIncompleteChannelLoad() == quarantine); + assert(fs.files == durable && fs.writes == writes && fs.renames == renames); + if (quarantine) { + assert(!f.store.saveChannels(&f.host)); + // Contact-only recovery must not authorize a channel rewrite. + f.store.requireContactRecovery(); + f.store.recoverContacts(&f.host); + assert(!f.store.hasIncompleteContactLoad() && !f.store.contactRecoveryPending()); + assert(f.store.hasIncompleteChannelLoad() && !f.host.contacts.empty()); + assert(f.host.contacts[0].copyPathTo(path)); + assert(!f.store.saveChannels(&f.host)); + assert(fs.files.at("/channels2") == channel_bytes); + } + strcpy(f.host.contacts[0].name, "Accepted contact change"); + assert(f.store.saveContacts(&f.host, nullptr)); + assert(!f.store.hasIncompleteContactLoad() && f.host.contacts[0].copyPathTo(path)); + assert(fs.files.at("/channels2") == channel_bytes); + if (!quarantine) { + if (fault == 1) assert(!strcmp(f.host.channels[0].name, "Public")); + else assert(!strcmp(f.host.channels[0].name, "Saved")); + assert(f.store.saveChannels(&f.host)); + assert(fs.files.at("/channels2").size() == 68); + } + } +} + +#if defined(RP2040_PLATFORM) +static void malformed_primary_channels_prefer_a_valid_backup() { + // Valid, malformed, unreadable, overcapacity, blocked remove, failed move, + // and a move whose acknowledgement is false despite successful publication. + for (unsigned fault = 0; fault < 7; ++fault) { + ChannelFixture f(30); + initializeContacts(f); + auto& fs = channelFs(); + fs.files["/channels2"] = {1, 2, 3}; + fs.files["/channels2.bak"] = channelImage("Recovered backup"); + if (fault == 1) fs.files["/channels2.bak"].pop_back(); + if (fault == 2) fs.fail_open = "/channels2.bak"; + if (fault == 3) fs.files["/channels2.bak"] = channelImage("Larger profile", MAX_GROUP_CHANNELS + 1); + if (fault == 4) fs.fail_remove = true; + if (fault == 5) fs.fail_rename = fs.renames + 1; + if (fault == 6) fs.late_channel_rename_error = true; + const auto original = fs.files; + f.store.loadChannels(&f.host); + assert(!f.store.hasIncompleteContactLoad() && !f.store.contactRecoveryPending()); + assert(f.store.hasIncompleteChannelLoad() == (fault >= 2 && fault <= 5)); + if (fault == 0 || fault >= 4) assert(!strcmp(f.host.channels[0].name, "Recovered backup")); + else assert(!strcmp(f.host.channels[0].name, "Public")); + if (fault == 0 || fault == 6) { + assert(!fs.exists("/channels2.bak")); + assert(fs.files.at("/channels2") == original.at("/channels2.bak")); + assert(f.store.saveChannels(&f.host)); + } else if (fault == 1) { + assert(fs.files == original); // Both malformed images await a normal save. + assert(f.store.saveChannels(&f.host)); + assert(fs.files.at("/channels2").size() == 68); + } else { + assert(!f.store.saveChannels(&f.host)); + assert(fs.files.at("/channels2.bak") == original.at("/channels2.bak")); + } + } +} +#endif + +#if !defined(ESP32_PLATFORM) +static void explicit_format_alone_clears_channel_quarantine() { + ChannelFixture f(30); + initializeContacts(f); + auto& fs = channelFs(); + fs.files["/channels2"] = channelImage("Protected"); + fail_channel_allocation = true; + f.store.loadChannels(&f.host); + fail_channel_allocation = false; + assert(f.store.hasIncompleteChannelLoad() && !f.store.hasIncompleteContactLoad()); + const auto original = fs.files; + fs.fail_channel_format = true; + assert(!f.store.formatFileSystem()); + assert(f.store.hasIncompleteChannelLoad() && fs.files == original); + fs.fail_channel_format = false; + assert(f.store.formatFileSystem()); + assert(!f.store.hasIncompleteChannelLoad() && !f.store.hasIncompleteContactLoad()); + assert(fs.files.empty() && f.store.saveChannels(&f.host)); +} +#endif + +int main() { + channel_faults_do_not_poison_contacts(); +#if defined(RP2040_PLATFORM) + malformed_primary_channels_prefer_a_valid_backup(); +#endif +#if !defined(ESP32_PLATFORM) + explicit_format_alone_clears_channel_quarantine(); +#endif + puts("PASS: channel/contact storage isolation"); +} diff --git a/test/fixtures/contact_cache/test_secondary_migration.cpp b/test/fixtures/contact_cache/test_secondary_migration.cpp index a4ffb0af3..dabb379a9 100644 --- a/test/fixtures/contact_cache/test_secondary_migration.cpp +++ b/test/fixtures/contact_cache/test_secondary_migration.cpp @@ -26,6 +26,7 @@ class DataStore { FakeFilesystem* _fsExtra; bool _contact_load_incomplete = false, _identity_creation_blocked = false; bool _prefs_load_incomplete = false; + bool _channel_load_incomplete = false; DataStore(FakeFilesystem& primary, FakeFilesystem& secondary) : _fs(&primary), _fsExtra(&secondary) {} File openRead(FakeFilesystem* fs, const char* path) { return fs->open(path); } FakeFilesystem* _getContactsChannelsFS() { return _fsExtra ? _fsExtra : _fs; } diff --git a/test/test_companion_channel_storage.py b/test/test_companion_channel_storage.py new file mode 100644 index 000000000..a007cc683 --- /dev/null +++ b/test/test_companion_channel_storage.py @@ -0,0 +1,172 @@ +#!/usr/bin/env python3 +"""Run real contact and channel persistence together under sanitizers.""" +from pathlib import Path +import unittest + +import test_contact_cache as cached +import test_companion_uncached_storage as uncached +from test_t096_full_memory import method + +ROOT = Path(__file__).resolve().parents[1] + +CHANNEL_HOST = r''' + bool channel_accepts = true; + std::vector channels; + bool onChannelLoaded(uint8_t index, const ChannelDetails& channel) { + if (!channel_accepts) return false; + if (channels.size() <= index) channels.resize(index + 1); + channels[index] = channel; + return true; + } + bool getChannelForSave(uint8_t index, ChannelDetails& channel) { + if (index >= channels.size()) return false; + channel = channels[index]; + return true; + } +''' + + +def fixture_builder(platform, old_coupling=False): + """Extend existing faulting filesystem adapters, not persistence logic.""" + is_cached = platform in ("NRF52_PLATFORM", "ESP32_PLATFORM") + + def build(work): + store = (ROOT / "examples/companion_radio/DataStore.cpp").read_text() + header = (ROOT / "examples/companion_radio/DataStore.h").read_text() + mesh = (ROOT / "examples/companion_radio/MyMesh.cpp").read_text() + implementation = (work / "store_under_test.h").read_text() + load = method(store, "void DataStore::loadChannels(") + if old_coupling: + # Restore the exact pre-fix channel-to-contact flag coupling. + contact_health = method(store, "bool DataStore::hasIncompleteContactLoad() const") + self_guard = " if (contactRecoveryPending()) return true;" + assert self_guard in contact_health + implementation = implementation.replace(contact_health, contact_health.replace( + self_guard, self_guard + "\n if (_channel_load_incomplete) return true;")) + if platform == "NRF52_PLATFORM": + assert " bool& incomplete = _channel_load_incomplete;" in load + load = load.replace(" bool& incomplete = _channel_load_incomplete;", + " bool& incomplete = _contact_load_incomplete;") + if platform == "ESP32_PLATFORM": + implementation += "\n" + method(store, "static bool promoteCompanionRecoveryFile(") + implementation += r''' +static bool fail_channel_allocation = false; +static void* allocateChannelSnapshot(size_t size) { + return fail_channel_allocation ? nullptr : malloc(size); +} +#define malloc allocateChannelSnapshot +''' + implementation += load + "\n#undef malloc\n" + implementation += method(store, "bool DataStore::saveChannels(") + "\n" + if platform != "ESP32_PLATFORM": + implementation += method(store, "bool DataStore::formatFileSystem()") + "\n" + (work / "store_under_test.h").write_text(implementation) + (work / "channel_admission_under_test.h").write_text( + method(mesh, "bool MyMesh::canMutateContacts() const")) + + fixture = (ROOT / ("test/fixtures/contact_cache/test_contact_cache.cpp" if is_cached + else "test/fixtures/companion_uncached_storage/test.cpp")) + source = fixture.read_text() + group = method((ROOT / "src/Mesh.h").read_text(), "class GroupChannel {") + source = ("#include \n#include \nnamespace mesh {\n" + group + + ";\n}\n#include \n#define MAX_GROUP_CHANNELS 8\n" + + "#define ADV_TYPE_NONE 0\n") + source + source = source.replace("struct Host {", "struct Host {" + CHANNEL_HOST, 1) + declarations = """ + void loadChannels(DataStoreHost*); + bool saveChannels(DataStoreHost*); + bool formatFileSystem(); + void requireContactRecovery() { queueContactRecovery(); } +""" + method(header, " bool hasIncompleteChannelLoad() const") + "\n" + if platform == "NRF52_PLATFORM": + declarations += " void forceSharedAuthorityFailure() { _contact_load_incomplete = true; }\n" + source = source.replace(" bool hasIncompleteContactLoad() const;", + declarations + " bool hasIncompleteContactLoad() const;", 1) + if is_cached: + source = source.replace(" bool _channel_load_incomplete = false;\n", "") + source = source.replace(" bool _cache_load_incomplete = false;", + " bool _cache_load_incomplete = false;\n bool _channel_load_incomplete = false;", 1) + if platform == "ESP32_PLATFORM": + source = source.replace(" uint32_t _secret_retry_at = 0;", + " const char* _channel_recovery_source = nullptr;\n uint32_t _secret_retry_at = 0;", 1) + else: + source = source.replace(" bool _contact_load_incomplete = false;", + " bool _contact_load_incomplete = false;\n" + " bool _identity_creation_blocked = false, _prefs_load_incomplete = false;\n" + " FakeFilesystem* _fs = &SPIFFS;\n FakeFilesystem* _fsExtra = nullptr;", 1) + source = source.replace( + "bool contactPathPresence(FakeFilesystem* fs, const char* path, bool& present) {", + "static bool fail_channel_metadata = false;\n" + "bool contactPathPresence(FakeFilesystem* fs, const char* path, bool& present, uint32_t* size = nullptr) {\n" + " if (fail_channel_metadata && !strcmp(path, \"/channels2\")) return false;") + source = source.replace(" present = fs->exists(path);\n return true;", + " present = fs->exists(path);\n" + " if (size) *size = present ? fs->files.at(path).size() : 0;\n return true;", 1) + fake = (ROOT / "test/fixtures/contact_cache/FakeFilesystem.h").read_text() + fake = fake.replace(" size_t largest_read =", + " size_t max_channel_read = std::numeric_limits::max();\n size_t largest_read =", 1) + fake = fake.replace( + " const size_t count = std::min(length, data.size() - std::min(data.size(), _position));", + " const size_t count = std::min({length, _fs->max_channel_read, data.size() - std::min(data.size(), _position)});") + fake = fake.replace(" bool mkdir(const char*) { return true; }", + " bool fail_channel_format = false;\n" + " bool format() { if (fail_channel_format) return false; files.clear(); return true; }\n" + " bool mkdir(const char*) { return true; }", 1) + (work / "FakeFilesystem.h").write_text(fake) + else: + source = source.replace(" FakeFilesystem* _fs = &filesystem;", + " FakeFilesystem* _fs = &filesystem;\n FakeFilesystem* _fsExtra = nullptr;\n" + " bool _prefs_load_incomplete = false;", 1) + source = source.replace(" bool mkdir(const char*) { return true; }", + " bool fail_channel_format = false, late_channel_rename_error = false;\n" + " bool format() { if (fail_channel_format) return false; files.clear(); return true; }\n" + " bool mkdir(const char*) { return true; }", 1) + source = source.replace(" return true;\n }\n} filesystem;", + " return !late_channel_rename_error;\n }\n} filesystem;", 1) + source = source.replace('#include "store_under_test.h"', + '#define LittleFS filesystem\n#include "store_under_test.h"', 1) + for signature in ("static void identity_checks()", "static void contact_checks()", + "static void recovery_checks()"): + source = source.replace(method(source, signature), "") + source = source[:source.index("int main() {")] + source += (ROOT / "test/fixtures/companion_channel_storage/checks.h").read_text() + generated = work / "channel_storage.cpp" + generated.write_text(source) + return generated + + return build + + +class CompanionChannelStorageTest(unittest.TestCase): + def run_platform(self, platform, old_coupling=False): + transform = fixture_builder(platform, old_coupling) + failure = "!f.store.hasIncompleteContactLoad()" if old_coupling else None + if platform in ("NRF52_PLATFORM", "ESP32_PLATFORM"): + cached.ContactCacheTest().run_cache_test( + platform, fixture_transform=transform, expected_failure=failure, + expected_marker="PASS: channel/contact storage isolation") + else: + uncached.CompanionUncachedStorageTest().run_platform( + platform, fixture_transform=transform, expected_failure=failure) + + def test_nrf52_cached_channels_do_not_quarantine_contacts(self): + self.run_platform("NRF52_PLATFORM") + + def test_esp32_channel_allocation_and_refusal_are_local(self): + self.run_platform("ESP32_PLATFORM") + + def test_stm32_uncached_channels_do_not_quarantine_contacts(self): + self.run_platform("STM32_PLATFORM") + + def test_rp2040_channels_recover_without_quarantining_contacts(self): + self.run_platform("RP2040_PLATFORM") + + def test_old_nrf52_shared_channel_flag_is_detected(self): + self.run_platform("NRF52_PLATFORM", old_coupling=True) + + def test_old_stm32_channel_contact_veto_is_detected(self): + self.run_platform("STM32_PLATFORM", old_coupling=True) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_companion_settings_persistence_contract.py b/test/test_companion_settings_persistence_contract.py index 831ea7a50..73a05ba31 100644 --- a/test/test_companion_settings_persistence_contract.py +++ b/test/test_companion_settings_persistence_contract.py @@ -223,7 +223,8 @@ class CompanionSettingsPersistenceContractTests(unittest.TestCase): presence_at = load.index( 'contactPathPresence(contacts_fs, "/channels2",' ) - open_at = load.index('openRead(contacts_fs, "/channels2")') + self.assertIn('const char* channel_path = "/channels2";', load) + open_at = load.index("openRead(contacts_fs, channel_path)") self.assertLess(presence_at, open_at) # Stat and open errors are distinct from a cleanly absent file, and @@ -250,8 +251,20 @@ class CompanionSettingsPersistenceContractTests(unittest.TestCase): self.assertGreaterEqual(load.count("incomplete = true;"), 4) save = function_body(self.store, "bool DataStore::saveChannels(") + self.assertIn("if (_channel_load_incomplete) return false;", save) self.assertIn("if (_contact_load_incomplete) return false;", save) + def test_channel_quarantine_does_not_poison_contact_authority(self): + contact_health = function_body(self.store, "bool DataStore::hasIncompleteContactLoad(") + self.assertNotIn("_channel_load_incomplete", contact_health) + self.assertIn("contactRecoveryPending()", contact_health) + self.assertIn("_contact_load_incomplete", contact_health) + load = function_body(self.store, "void DataStore::loadChannels(") + self.assertIn("bool& incomplete = _channel_load_incomplete;", load) + self.assertNotIn("_contact_load_incomplete = true", load) + clear = function_body(self.store, "void DataStore::clearContactLoadFailure(") + self.assertNotIn("_channel_load_incomplete", clear) + def test_failed_legacy_preferences_recovery_latches_preferences(self): migrate = function_body(self.store, "bool DataStore::migrateToSecondaryFS()") primary = function_body( diff --git a/test/test_companion_tx_routing.py b/test/test_companion_tx_routing.py index 42de6258d..0f6dc8b11 100644 --- a/test/test_companion_tx_routing.py +++ b/test/test_companion_tx_routing.py @@ -51,6 +51,7 @@ struct DataStore { enum : uint8_t { ContactRecoveryPending=1 }; uint8_t _contact_recovery=0; @RECOVERY_METHOD@ + @CHANNEL_METHOD@ const char* _channel_recovery_source=nullptr; bool hasIncompleteContactLoad() const; FILESYSTEM* _getContactsChannelsFS() { return &fs; } @@ -65,6 +66,7 @@ public: ChannelDetails channels[MAX_GROUP_CHANNELS]; Radio radio; Radio* _radio=&radio; DataStore store; + DataStore* _store=&store; bool writable=true, writes_ok=true; unsigned writes=0; bool canMutateContacts() const { return writable && !store.hasIncompleteContactLoad(); } int getTotalContactSlots() const { return contacts.size(); } @@ -146,8 +148,8 @@ int main() { assert(disk.size()==MAX_GROUP_CHANNELS*68); assert(disk[0]==mesh::encodeRadioTxPolicy(mesh::RADIO_TX_BOTH)); assert(disk[68]==mesh::encodeRadioTxPolicy(mesh::RADIO_TX_SECONDARY)); - // Loading is all-or-nothing. ESP32 retries recovery and then permits an - // explicitly requested defaults save; the other backends remain fail-closed. + // Channels have their own quarantine. A partial fixed record may be + // replaced with defaults, but readable overcapacity and IO failures cannot. for(int fault : {0,1,2,3,4}) { MyMesh boot; boot.store.fs.files["/channels2"]=disk; @@ -159,22 +161,29 @@ int main() { if(fault==4) boot.store.fs.files["/channels2"].resize((MAX_GROUP_CHANNELS+1)*68); auto durable=boot.store.fs.files["/channels2"]; boot.store.loadChannels(&boot); -#if defined(ESP32_PLATFORM) assert(!boot.store.hasIncompleteContactLoad()); + const bool quarantined = +#if defined(ESP32_PLATFORM) + fault==4; #else - assert(boot.store.hasIncompleteContactLoad()); + fault!=3; #endif + assert(boot.store.hasIncompleteChannelLoad()==quarantined); assert(!strcmp(boot.channels[0].name,"keep existing")); assert(!boot.channels[1].name[0]); boot.store.fs.fail_read_open=false;boot.store.fs.fail_read_after=-1; -#if defined(ESP32_PLATFORM) assert(boot.store.fs.files["/channels2"]==durable); - assert(boot.canMutateContacts()&&boot.store.saveChannels(&boot)); - assert(boot.store.fs.files["/channels2"]!=disk); -#else - assert(!boot.canMutateContacts()&&!boot.store.saveChannels(&boot)); - assert(boot.store.fs.files["/channels2"]==durable); -#endif + assert(boot.canMutateContacts()); + const unsigned attempts=boot.writes; + assert(boot.handleTxRoutingCommand("set tx.channel 0 auto",reply,sizeof(reply))); + if(quarantined) { + assert(strstr(reply,"storage unavailable")&&boot.writes==attempts); + assert(!boot.store.saveChannels(&boot)); + assert(boot.store.fs.files["/channels2"]==durable); + } else assert(!strncmp(reply,"OK",2)); + strcpy(boot.contacts[1].name,"healthy contact"); + assert(boot.handleTxRoutingCommand("set tx.user \"healthy contact\" both",reply,sizeof(reply))); + assert(!strncmp(reply,"OK",2)&&boot.contacts[1].tx_radio==mesh::RADIO_TX_BOTH); } MyMesh fresh;fresh.store.loadChannels(&fresh); assert(!fresh.store.hasIncompleteContactLoad()); // absent file is a fresh boot @@ -221,10 +230,12 @@ int main() { assert(failed_boot.store.saveChannels(&failed_boot)); assert(failed_boot.store.fs.files["/channels2"]==disk); #else - assert(failed_boot.store.hasIncompleteContactLoad()); + assert(!failed_boot.store.hasIncompleteContactLoad()); + assert(failed_boot.store.hasIncompleteChannelLoad()); failed_boot.store.fs.fail_rename_from.clear(); failed_boot.store.loadChannels(&failed_boot); - assert(failed_boot.store.hasIncompleteContactLoad()); + assert(!failed_boot.store.hasIncompleteContactLoad()); + assert(failed_boot.store.hasIncompleteChannelLoad()); assert(!failed_boot.store.saveChannels(&failed_boot)); assert(failed_boot.store.fs.files["/channels2.bak"]==disk); #endif @@ -291,7 +302,8 @@ class CompanionTxRoutingTest(unittest.TestCase): (work / 'ContactFileTransaction.h').write_text(transaction.replace( '#include "IdentityStore.h"', '#include ')) (work / 'test.cpp').write_text(HARNESS.replace('@METHODS@', methods).replace( - '@RECOVERY_METHOD@', method(store_header, ' bool contactRecoveryPending() const'))) + '@RECOVERY_METHOD@', method(store_header, ' bool contactRecoveryPending() const')).replace( + '@CHANNEL_METHOD@', method(store_header, ' bool hasIncompleteChannelLoad() const'))) for platform in ('ESP32_PLATFORM', 'RP2040_PLATFORM', 'STM32_PLATFORM'): with self.subTest(platform=platform): build = subprocess.run(['g++', '-std=c++17', '-Wall', '-Wextra', '-D'+platform+'=1', diff --git a/test/test_companion_uncached_storage.py b/test/test_companion_uncached_storage.py index c3e237048..e2f8d247f 100644 --- a/test/test_companion_uncached_storage.py +++ b/test/test_companion_uncached_storage.py @@ -20,7 +20,7 @@ class CompanionUncachedStorageTest(unittest.TestCase): def test_rp2040_uncached_storage(self): self.run_platform("RP2040_PLATFORM") - def run_platform(self, platform): + def run_platform(self, platform, fixture_transform=None, expected_failure=None): source = (ROOT / "examples/companion_radio/DataStore.cpp").read_text() signatures = [ "static bool companionPathPresence(", @@ -78,6 +78,9 @@ class CompanionUncachedStorageTest(unittest.TestCase): (temp / "InternalFileSystem.h").write_text( "#pragma once\n#define InternalFS filesystem\n") binary = temp / "test" + fixture = ROOT / "test/fixtures/companion_uncached_storage/test.cpp" + if fixture_transform is not None: + fixture = fixture_transform(temp) compiled = subprocess.run([ "c++", "-std=c++17", "-O1", "-g", "-Wall", "-Wextra", "-Werror", "-Wno-unused-parameter", "-fsanitize=address,undefined", "-fno-pie", "-no-pie", @@ -86,12 +89,16 @@ class CompanionUncachedStorageTest(unittest.TestCase): "-I", str(ROOT / "test/fixtures/contact_cache/mocks"), "-I", str(ROOT / "src"), "-I", str(ROOT / "lib/ed25519"), - str(ROOT / "test/fixtures/companion_uncached_storage/test.cpp"), + str(fixture), "-o", str(binary), ], capture_output=True, text=True, timeout=60) self.assertEqual(compiled.returncode, 0, compiled.stdout + compiled.stderr) result = subprocess.run([str(binary)], capture_output=True, text=True, timeout=30) - self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + if expected_failure is None: + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + else: + self.assertNotEqual(result.returncode, 0) + self.assertIn(expected_failure, result.stdout + result.stderr) if __name__ == "__main__": diff --git a/test/test_contact_cache.py b/test/test_contact_cache.py index cfaf8f930..7a29b1142 100644 --- a/test/test_contact_cache.py +++ b/test/test_contact_cache.py @@ -64,7 +64,9 @@ class ContactCacheTest(unittest.TestCase): def run_cache_test(self, platform, disable_crc_batch=False, disable_recovery=False, disable_raw_recovery=False, - unsafe_legacy_discovery=False, unsafe_page_discovery=False): + unsafe_legacy_discovery=False, unsafe_page_discovery=False, + fixture_transform=None, expected_failure=None, + expected_marker='PASS: 114 shared request/login payload and routing traces'): with tempfile.TemporaryDirectory(prefix="mesh-contact-cache-") as temp: temp = Path(temp) store = (ROOT / "examples/companion_radio/DataStore.cpp").read_text() @@ -180,6 +182,9 @@ class ContactCacheTest(unittest.TestCase): "int BaseChatMesh::sendRequest(const ContactInfo& recipient, uint8_t")) ) binary = temp / "test" + fixture = ROOT / "test/fixtures/contact_cache/test_contact_cache.cpp" + if fixture_transform is not None: + fixture = fixture_transform(temp) result = subprocess.run([ "c++", "-std=c++17", "-O1", "-g", "-Wall", "-Wextra", "-fsanitize=address,undefined", "-fno-omit-frame-pointer", @@ -189,13 +194,16 @@ class ContactCacheTest(unittest.TestCase): "-I", str(ROOT / "test/fixtures/contact_cache/mocks"), "-I", str(ROOT / "lib/ed25519"), "-I", str(ROOT / "src"), "-I", str(temp), - str(ROOT / "test/fixtures/contact_cache/test_contact_cache.cpp"), + str(fixture), str(ROOT / "src/helpers/ContactInfo.cpp"), *crypto_objects(temp), "-lcrypto", "-o", str(binary), ], capture_output=True, text=True) self.assertEqual(result.returncode, 0, result.stderr) result = subprocess.run([str(binary)], capture_output=True, text=True) - if disable_raw_recovery: + if expected_failure is not None: + self.assertNotEqual(result.returncode, 0) + self.assertIn(expected_failure, result.stdout + result.stderr) + elif disable_raw_recovery: self.assertNotEqual(result.returncode, 0) self.assertIn("raw_read_recovery_preserves_accepted_ram_state", result.stderr) self.assertIn("f.host.recovery_resets == 0", result.stderr) @@ -215,7 +223,7 @@ class ContactCacheTest(unittest.TestCase): self.assertIn("crc_read_passes == 104", result.stderr) else: self.assertEqual(result.returncode, 0, result.stdout + result.stderr) - self.assertIn('PASS: 114 shared request/login payload and routing traces', result.stdout) + self.assertIn(expected_marker, result.stdout) if __name__ == "__main__": diff --git a/test/test_nrf52_extrafs_contract.py b/test/test_nrf52_extrafs_contract.py index 32c87e128..08b804f2b 100644 --- a/test/test_nrf52_extrafs_contract.py +++ b/test/test_nrf52_extrafs_contract.py @@ -758,10 +758,22 @@ class Nrf52ExtraFsContractTest(unittest.TestCase): save_channels = function_body( store, "bool DataStore::saveChannels(DataStoreHost* host)" ) - self.assertIn("bool& incomplete = _contact_load_incomplete;", load_channels) + # Unknown shared-filesystem authority still blocks channel loading, + # but channel-only failures must not poison the contact health latch. + self.assertIn("bool& incomplete = _channel_load_incomplete;", load_channels) + self.assertLess( + load_channels.index("if (_contact_load_incomplete) return;"), + load_channels.index("bool& incomplete = _channel_load_incomplete;"), + ) self.assertLess( load_channels.index("if (incomplete) return;"), - load_channels.index('openRead(contacts_fs, "/channels2")'), + load_channels.index("openRead(contacts_fs,"), + ) + self.assertNotIn("_contact_load_incomplete", function_body( + load_channels, "if (!success)")) + self.assertLess( + save_channels.index("if (_channel_load_incomplete) return false;"), + save_channels.index("mesh::AtomicFileWriter file("), ) self.assertLess( save_channels.index("if (_contact_load_incomplete) return false;"),