diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index 0df96e67..f18fed75 100644 --- a/MQTT_IMPLEMENTATION.md +++ b/MQTT_IMPLEMENTATION.md @@ -285,7 +285,7 @@ flash regardless of target. #### Compile-time fresh-install defaults (`src/helpers/MQTTDefaults.h`) -Optional PlatformIO `build_flags` override defaults written when `/mqtt_prefs` is first created. They do **not** change existing saved prefs on upgrade or reflash (unless `/mqtt_prefs` is erased). +Optional PlatformIO `build_flags` override defaults used when `/mqtt.json` is first created. They do **not** change existing saved prefs on upgrade or reflash (unless `/mqtt.json` is erased). | Macro | Default | Notes | |-------|---------|-------| diff --git a/MQTT_INTERNALS.md b/MQTT_INTERNALS.md index d0de0398..005de530 100644 --- a/MQTT_INTERNALS.md +++ b/MQTT_INTERNALS.md @@ -8,7 +8,8 @@ Developer-facing notes on how the MQTT observer feature is structured in the cod - `src/helpers/bridges/MQTTBridge.h` - MQTT bridge class definition - `src/helpers/bridges/MQTTBridge.cpp` - MQTT bridge implementation - `src/helpers/MQTTPresets.h` - Preset definitions, CA certificates, and lookup functions -- `src/helpers/MQTTDefaults.h` - Compile-time defaults for fresh `/mqtt_prefs` +- `src/helpers/MQTTDefaults.h` - Compile-time defaults for fresh `/mqtt.json` +- `src/helpers/MQTTPrefsSerializer.h` - Versioned, semantic observer JSON schema - `src/helpers/MQTTMessageBuilder.h` - JSON message formatting utilities - `src/helpers/MQTTMessageBuilder.cpp` - JSON message formatting implementation - `src/helpers/JWTHelper.h` - JWT token generation for Ed25519-based authentication @@ -30,8 +31,9 @@ The observer feature is kept out of upstream-tracked files through three mechani `resolveAlertScope`, `beginDeferredOtaUpdate`). The example apps override them behind `#ifdef WITH_MQTT_BRIDGE`. - **Separate settings file** — observer settings (MQTT slots, WiFi, timezone, SNMP, - radio watchdog, fault alerts) live in the `MQTTPrefs` struct persisted to - `/mqtt_prefs`, keeping `NodePrefs` / `/com_prefs` aligned with the upstream layout. + radio watchdog, fault alerts) live in the runtime `MQTTPrefs` object and are + field-serialized to `/mqtt.json`, keeping `NodePrefs` / `/prefs.json` aligned with + upstream. The two files are independent transactions, not one atomic snapshot. Remaining integration points in upstream files: - `examples/simple_repeater/MyMesh.{h,cpp}`, `examples/simple_room_server/MyMesh.{h,cpp}` - @@ -164,75 +166,104 @@ which packet events non-MQTT bridges capture. The MQTT bridge ignores `bridge.so favour of independent `mqtt.rx` / `mqtt.tx` controls. Everything MQTT-specific lives under `mqtt.*` (shared settings), `mqttN.*` (per-slot broker config), `wifi.*`, and `timezone.*`. -### `/mqtt_prefs` file format +### `/mqtt.json` file format -`/mqtt_prefs` is written with an 8-byte `MQTTPrefsHeader` (`magic`, `version`, -`payload_len`) followed by the raw `MQTTPrefs` payload. The magic is -`{0xF5, 'M', 'Q', 'P'}` — its leading non-ASCII byte can never collide with the first -bytes of a legacy (headerless) file, whose payload begins with the `mqtt_origin` -string. Bump `MQTT_PREFS_VERSION` when the payload layout changes incompatibly; a file -whose version this firmware doesn't recognize is left untouched and the in-memory prefs -fall back to defaults (no downgrade, no misread). `saveMQTTPrefs()` also refuses to -write while such a file is present (`_mqtt_prefs_hold`), so a `set` command after a -firmware downgrade can't clobber the newer config — observer settings changed in that -state simply don't persist. The frozen legacy layouts are pinned with `static_assert`s -in `MQTTPrefsStorage.h`, so every target build re-verifies the fleet's file offsets. +Observer preferences use the same `ConfigSerializer` object notation as upstream +`/prefs.json`: semantic unquoted keys, quoted strings, decimal numbers, and nested +objects. Schema version 1 requires the root `version:1` field. The main shape is: -Adding a field to the current version stays backward compatible: append it to the end -of `MQTTPrefs`, give the older exact payload length an explicit decoder boundary, and -leave the missing tail at its default. The packet-filter addition follows that rule: -the prior 2864-byte v1 payload loads with all six filters set to `all`, while the -full payload is 2876 bytes. +```text +{version:1, + wifi:{ssid:"...",password:"...",power_save:1}, + time:{timezone:"...",utc_offset:0,ntp_server:"..."}, + mqtt:{origin:"...",iata:"SEA",packets_enabled:1,raw_enabled:0, + tx_enabled:2,rx_enabled:1, + status:{enabled:1,interval_ms:300000}, + neighbors:{enabled:0,interval_ms:86400000}, + owner:{public_key:"...",email:"..."}, + slot1:{preset:"analyzer-us",host:"",port:0,username:"",password:"", + token:"",topic:"",audience:"",packet_filter:65535}, + ... slot2 through slot6 ...}, + snmp:{enabled:0,community:"public"}, + radio:{watchdog_min:5}, + alert:{enabled:0,psk_hex:"",wifi_minutes:30,mqtt_minutes:240, + rate_limit_min:60,hashtag:"",region:""}} +``` -#### The downgrade contract +Keys may contain digits after their first character, which permits the readable +`slot1` ... `slot6` names. Every schema key fits `ConfigSerializer`'s 15-character +visible-key limit. Known strings and numbers use strict parsing: duplicates, overlong +strings, malformed decimals, and overflow reject the complete file. A supported file +with a semantically out-of-range value is repaired to that field's safe default and +rewritten atomically. Unknown fields are ignored only within the serializer's general +limits (15-character keys, 127-byte decoded values, and six nested object levels below the root). -**Within a version tag the layout is append-only, and a longer payload is always -readable.** A file written by a later build starts with this binary's exact baseline, -so `classify()` reads that prefix and ignores the tail. A downgraded node keeps its -WiFi credentials, broker slots, and every other setting it understands; the only thing -it loses is the settings the newer build added. +Loading always starts with defaults and parses into a separate heap scratch object. +The live preferences change only after the complete file parses, has an explicit +supported version, and passes validation. A missing/future version, syntax error, +overlength value, or allocation/read failure leaves `/mqtt.json` untouched, runs this +boot on defaults, and holds observer saves so a later CLI/WebConfig write cannot erase +the opaque source. If an observer setter cannot commit its JSON transaction, it restores +the pre-command in-memory preferences and does not restart or apply a bridge change as +though the setting were durable. -That asymmetry is the whole point. Refusing the file costs the operator the network -itself — `/mqtt_prefs` holds `wifi_ssid`/`wifi_password` as well as the broker config, -so a node that falls back to defaults has no WiFi, no portal, and no OTA, recoverable -only over serial. Reading it costs a feature's settings. Losing later settings is the -acceptable half of that trade; losing the node is not. +Saves stream to `/mqtt.json.tmp` through a sticky short-write detector while computing +size and checksum. The firmware closes and rereads the temp, verifies size/checksum, +parses it into another scratch object, then publishes with +`/mqtt.json` -> `/mqtt.json.bak` and temp -> primary. Boot recovery selects the usable +primary/temp/backup without overwriting an opaque future or corrupt primary. A valid +future-version temp that reached the rename phase wins over the stale backup and is held +for newer firmware. If a temp claims a future version but uses grammar this firmware +cannot parse, or cannot be classified because scratch allocation fails, recovery may run +the last usable backup but retains the uncertain temp and holds all observer writes. A +definitively corrupt/incomplete current-version temp may be discarded for that backup. +If power fails during the very first migration, there is no JSON primary or backup yet; +recovery discards a definitively invalid temp so the intact `/mqtt_prefs` source can be +loaded and the migration retried. An uncertain/future temp is still preserved and held. -The tail survives until something actually writes. `saveMQTTPrefs()` rewrites at this -binary's own length, so a rollback that changes no observer setting and is later rolled -forward keeps the newer fields intact — only an explicit `set` while downgraded drops -them. The boot log says so when it happens. +Unknown slot preset names are repaired to `none`, never to a build-specific default +broker. Duplicate known presets from historical firmware are preserved on load; current +CLI and WebConfig setters prevent creating new duplicates without silently changing an +existing deployment during migration. -**A change that is not a pure append MUST bump `MQTT_PREFS_VERSION`.** The version -check is what makes the rule above safe: a different tag is still refused outright and -the file preserved, because the bytes may no longer mean what this binary thinks. Never -reorder, resize, or repurpose an existing field within a version. +Schema changes that reinterpret existing names or values must increment `version`. +Additive fields may remain in version 1 only when old readers can safely ignore them +within the limits above. A future version is treated as opaque rather than partially +loading its known-looking fields. Literal schema keys are compile-time checked against +the 15-character visible-key limit so a new version-1 field cannot accidentally violate +that downgrade contract. -Note the rule is only as old as the build that implements it. Firmware already deployed -carries the *previous* decoder, which rejects any longer v1 payload — so rolling back -from this build to one shipped before it still falls back to defaults. -`MQTTPrefsCodec::payloadLenFor()` covers that gap from the writing side: it returns the -shortest length that still round-trips the configuration, so a node keeps writing 2864 -bytes until a packet filter actually holds something, and clearing the last non-default -filter puts it back. That mitigation can be retired once no supported downgrade target -predates the contract; the contract itself is the durable half. +#### Downgrade and rollback -Shorter payloads keep their existing, stricter treatment: a short length must match a -boundary that really shipped (`MQTT_PREFS_V1_*_PAYLOAD_SIZE`), because raw prefs have -no checksum and an arbitrary short size cannot be trusted to mean anything. +The old `/mqtt_prefs` binary is read only for one-time migration and is deliberately +not updated or deleted. When it exists and is usable, it is the exact pre-migration +rollback snapshot. Firmware older than this JSON change will therefore see stale +observer settings after a downgrade. A fresh JSON-only install has no binary observer +configuration to recover on downgrade. + +The two files are intentionally not reconciled. If an operator changes observer settings +while running old firmware, those changes update only `/mqtt_prefs`. Rolling forward to +this firmware makes the existing `/mqtt.json` authoritative again, so rollback-era edits +are ignored unless the operator exports and reapplies them. Conversely, JSON-only settings +cannot be recovered by old firmware. This is a rollback snapshot, not bidirectional sync. + +`LegacyV1MQTTPrefs` and the older pre-slot/3-slot/6-slot structs are frozen migration +ABIs with size/offset assertions. The runtime `MQTTPrefs` layout is not an on-flash ABI. ### Settings upgrade / migration `loadPrefs()` handles every historical on-device format one-time at boot: -- **`/mqtt_prefs`** — if the file has the version header it is read directly. Otherwise +- **`/mqtt_prefs` -> `/mqtt.json`** — if the legacy file has the version header its + frozen v1 layout is field-copied. Otherwise it is a legacy headerless file and its layout is detected by size: pre-slot (`OldMQTTPrefs`), 3-slot (`ThreeSlotMQTTPrefs`), or the 6-slot layout shipped on `observer-firmware` back when it was named `mqtt-bridge-implementation-flex` (`Legacy6SlotMQTTPrefs`). Each is field-copied into - the current compact `MQTTPrefs` and re-saved with the version header — which also + the runtime `MQTTPrefs` and saved as schema-versioned JSON — which also drops the vestigial `_legacy_*` fields the flex layout carried mid-struct. This is a one-time rewrite; every deployed device performs it on its first boot of versioned - firmware, after which all reads take the header path. + firmware, after which `/mqtt.json` is authoritative. The binary source remains as + a rollback snapshot and is never dual-written. The pre-slot (`OldMQTTPrefs`) copy maps the old single-broker keys onto slots: `mqtt.analyzer.us = on` → slot 1 `analyzer-us`, `mqtt.analyzer.eu = on` → slot 2 `analyzer-eu`, and a configured `mqtt.server` / `mqtt.port` / `mqtt.username` / @@ -241,7 +272,7 @@ no checksum and an arbitrary short size cannot be trusted to mean anything. - **`/com_prefs`** — a file written by fork firmware that predates the `MQTTPrefs` split (a zero-filled MQTT gap plus a trailing observer block) is detected by size; the trailing SNMP / radio-watchdog / fault-alert settings and the `rx_boosted_gain` / - `flood_max_*` fields are recovered, carried into `/mqtt_prefs`, and both files are + `flood_max_*` fields are recovered, carried into `/mqtt.json`, and the active files are rewritten in the current formats. - Settings the pre-split firmware stored *inside* the `/com_prefs` MQTT gap (the MQTT slot/WiFi config itself) are **not** recovered — users upgrading from firmware that diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 7e209cfb..22645503 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -1050,7 +1050,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc // _prefs.agc_reset_interval = 7; // 28 seconds (secs/4) #endif // Observer defaults (radio_watchdog, alert.*, snmp.*) moved to applyMQTTDefaults() - // in MQTTDefaults.h — they live in /mqtt_prefs now, not NodePrefs. + // in MQTTDefaults.h — they live in /mqtt.json now, not NodePrefs. // bridge defaults _prefs.bridge_enabled = 1; // enabled @@ -1066,7 +1066,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc _prefs.gps_interval = 0; _prefs.advert_loc_policy = ADVERT_LOC_PREFS; - // MQTT/WiFi/timezone/radio_watchdog defaults live in /mqtt_prefs now (see applyMQTTDefaults). + // MQTT/WiFi/timezone/radio_watchdog defaults live in /mqtt.json now (see applyMQTTDefaults). _prefs.adc_multiplier = 0.0f; // 0.0f means use default board multiplier diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index a232ef52..e4c3a3f7 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -285,6 +285,11 @@ public: void savePrefs() override { _cli.savePrefs(_fs); } +#ifdef WITH_MQTT_BRIDGE + bool saveObserverPrefs() override { + return _cli.saveObserverPrefs(_fs); + } +#endif void sendFloodScoped(const TransportKey& scope, mesh::Packet* pkt, uint32_t delay_millis, uint8_t path_hash_size); diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index 9fe5a9c8..02efbd43 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -891,7 +891,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc _prefs.radio_fem_txgain = 0; // Observer defaults (alert.*, etc.) moved to applyMQTTDefaults() — they live - // in /mqtt_prefs now, not NodePrefs. + // in /mqtt.json now, not NodePrefs. // bridge defaults (same as repeater) _prefs.bridge_enabled = 1; // enabled @@ -900,7 +900,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc _prefs.bridge_baud = 115200; // baud rate _prefs.bridge_channel = 1; // channel 1 - // MQTT/WiFi/timezone defaults live in /mqtt_prefs now (see applyMQTTDefaults). + // MQTT/WiFi/timezone defaults live in /mqtt.json now (see applyMQTTDefaults). next_post_idx = 0; next_client_idx = 0; diff --git a/examples/simple_room_server/MyMesh.h b/examples/simple_room_server/MyMesh.h index b0eb4e8f..65de77ed 100644 --- a/examples/simple_room_server/MyMesh.h +++ b/examples/simple_room_server/MyMesh.h @@ -289,6 +289,11 @@ public: void savePrefs() override { _cli.savePrefs(_fs); } +#ifdef WITH_MQTT_BRIDGE + bool saveObserverPrefs() override { + return _cli.saveObserverPrefs(_fs); + } +#endif void sendFloodScoped(const TransportKey& scope, mesh::Packet* pkt, uint32_t delay_millis, uint8_t path_hash_size); diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 427d456b..cf2e52eb 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -6,6 +6,7 @@ #include "MQTTPrefsAtomicStore.h" #include #include +#include #ifndef BRIDGE_MAX_BAUD #define BRIDGE_MAX_BAUD 115200 @@ -23,6 +24,7 @@ #include "MQTTDefaults.h" #include "MQTTPrefsCodec.h" #include "MQTTPrefsRecovery.h" +#include "MQTTPrefsSerializer.h" #endif // Believe it or not, this std C function is busted on some platforms! @@ -94,7 +96,8 @@ void CommonCLI::loadPrefs(FILESYSTEM* fs) { _prefs->bridge_pkt_src = 1; // Default to RX (logRx) for new installs } #ifdef WITH_MQTT_BRIDGE - // Load observer preferences (MQTT/WiFi/timezone/SNMP/alert) from /mqtt_prefs. + // Load observer preferences (MQTT/WiFi/timezone/SNMP/alert) from /mqtt.json, + // migrating the old /mqtt_prefs binary when JSON does not exist yet. // Readers (MQTTBridge, AlertReporter, observer CLI) use _mqtt_prefs directly — // these fields no longer exist in NodePrefs, so there is nothing to sync. MQTTPrefsAtomicStore::LegacyUpgradeGate legacy_upgrade( @@ -119,13 +122,13 @@ void CommonCLI::loadPrefs(FILESYSTEM* fs) { } } // mqtt_rx_enabled: new field appended to end of MQTTPrefs. On upgrade from older firmware, - // the shorter /mqtt_prefs file won't contain it, so it keeps the default value (1 = on) + // a shorter legacy /mqtt_prefs file won't contain it, so it keeps the default value (1 = on) // set by setMQTTPrefsDefaults(). No explicit migration needed. #endif // Republish legacy binary prefs as /prefs.json. Old-format files also carried a // trailing observer block, which loadPrefsInt() recovered into _legacy_tail; wait - // for loadMQTTPrefs() to commit that to /mqtt_prefs first. The legacy file is left + // for loadMQTTPrefs() to commit that to /mqtt.json first. The legacy file is left // on flash either way, so a deferred or failed save just retries on the next boot. #ifdef WITH_MQTT_BRIDGE if (loaded_from_legacy || _com_prefs_needs_upgrade) { @@ -134,7 +137,7 @@ void CommonCLI::loadPrefs(FILESYSTEM* fs) { legacy_upgrade.recordComPrefsRewrite(); _com_prefs_needs_upgrade = false; } else { - MESH_DEBUG_PRINTLN("Prefs: deferring /prefs.json migration until /mqtt_prefs commits"); + MESH_DEBUG_PRINTLN("Prefs: deferring /prefs.json migration until /mqtt.json commits"); } } #else @@ -376,13 +379,17 @@ bool CommonCLI::savePrefs(FILESYSTEM* fs, bool save_mqtt) { #ifdef WITH_MQTT_BRIDGE // Observer config (MQTT/WiFi/timezone/SNMP/alert) is persisted separately. The // observer CLI writes _mqtt_prefs directly, so no NodePrefs->MQTTPrefs sync runs. - // Runs regardless of the NodePrefs result so a failed JSON write cannot strand it. + // Ordinary NodePrefs callers leave save_mqtt false; migration is the only + // workflow that may explicitly combine these independent transactions. if (save_mqtt) saveMQTTPrefs(fs); #endif return success; } #ifdef WITH_MQTT_BRIDGE +static const uint32_t MQTT_JSON_FNV1A_OFFSET_BASIS = 2166136261u; +static const uint32_t MQTT_JSON_FNV1A_PRIME = 16777619u; + // Set default values for MQTT preferences (used when file doesn't exist or is corrupted) static void setMQTTPrefsDefaults(MQTTPrefs* prefs) { applyMQTTDefaults(prefs); @@ -396,6 +403,124 @@ static File openMqttPrefsRead(FILESYSTEM* fs, const char* path = "/mqtt_prefs") #endif } +enum class JsonPrefsLoadResult : uint8_t { + Loaded, + LoadedWithRepairs, + UnsupportedVersion, + FutureClaimed, + Invalid, +}; + +static JsonPrefsLoadResult loadMqttJsonFile(FILESYSTEM* fs, const char* path, + MQTTPrefs* output) { + if (output == nullptr) return JsonPrefsLoadResult::Invalid; + applyMQTTDefaults(output); + + // Probe the root version without applying the v1 schema. A future version + // may legitimately change an existing field's type, and must still be held + // opaquely rather than misclassified as corrupt and rolled back. + File version_file = openMqttPrefsRead(fs, path); + if (!version_file) return JsonPrefsLoadResult::Invalid; + MQTTPrefsVersionProbe version_probe; + const bool version_parsed = version_probe.loadSerial(version_file); + version_file.close(); + if (version_probe.hasFutureVersion()) { + return version_parsed ? JsonPrefsLoadResult::UnsupportedVersion + : JsonPrefsLoadResult::FutureClaimed; + } + + File file = openMqttPrefsRead(fs, path); + if (!file) return JsonPrefsLoadResult::Invalid; + MQTTPrefsSerializer serializer(output); + const bool parsed = serializer.loadSerial(file); + file.close(); + if (!parsed) return JsonPrefsLoadResult::Invalid; + if (serializer.hasFutureVersion()) return JsonPrefsLoadResult::UnsupportedVersion; + bool repaired = false; + if (!serializer.apply(&repaired)) return JsonPrefsLoadResult::Invalid; + return repaired ? JsonPrefsLoadResult::LoadedWithRepairs : JsonPrefsLoadResult::Loaded; +} + +static MQTTPrefsRecovery::FileState mqttJsonFileState(FILESYSTEM* fs, const char* path) { + if (!fs->exists(path)) return MQTTPrefsRecovery::FileState::Missing; + MQTTPrefs* scratch = new (std::nothrow) MQTTPrefs; + if (scratch == nullptr) return MQTTPrefsRecovery::FileState::Indeterminate; + const JsonPrefsLoadResult result = loadMqttJsonFile(fs, path, scratch); + delete scratch; + if (result == JsonPrefsLoadResult::UnsupportedVersion) { + // Syntax and the mandatory version field were valid, so this can be a + // fully verified transaction written by newer firmware. Keep it distinct + // from a torn/corrupt temp during recovery. + return MQTTPrefsRecovery::FileState::FutureUsable; + } + if (result == JsonPrefsLoadResult::FutureClaimed) { + return MQTTPrefsRecovery::FileState::FutureClaimed; + } + return result == JsonPrefsLoadResult::Loaded || + result == JsonPrefsLoadResult::LoadedWithRepairs + ? MQTTPrefsRecovery::FileState::Usable + : MQTTPrefsRecovery::FileState::Preserve; +} + +static bool recoverMqttJsonFiles(FILESYSTEM* fs) { + const MQTTPrefsRecovery::FileState primary = mqttJsonFileState(fs, "/mqtt.json"); + const MQTTPrefsRecovery::FileState temp = mqttJsonFileState(fs, "/mqtt.json.tmp"); + const MQTTPrefsRecovery::FileState backup = mqttJsonFileState(fs, "/mqtt.json.bak"); + const MQTTPrefsRecovery::Action action = MQTTPrefsRecovery::select(primary, temp, backup); + + if (action == MQTTPrefsRecovery::Action::KeepPrimary) { + if (primary == MQTTPrefsRecovery::FileState::Usable) { + if (!MQTTPrefsRecovery::uncertain(temp) && + temp != MQTTPrefsRecovery::FileState::Missing) { + fs->remove("/mqtt.json.tmp"); + } + if (!MQTTPrefsRecovery::uncertain(backup) && + backup != MQTTPrefsRecovery::FileState::Missing) { + fs->remove("/mqtt.json.bak"); + } + } + return MQTTPrefsRecovery::uncertain(primary) || + MQTTPrefsRecovery::uncertain(temp) || + MQTTPrefsRecovery::uncertain(backup); + } + if (action == MQTTPrefsRecovery::Action::DiscardTemp) { + if (fs->remove("/mqtt.json.tmp")) { + MESH_DEBUG_PRINTLN("MQTT: discarded incomplete first-migration JSON temp"); + return false; + } + MESH_DEBUG_PRINTLN("MQTT: could not discard incomplete /mqtt.json temp; source held"); + return true; + } + if (action == MQTTPrefsRecovery::Action::PromoteTemp) { + if (fs->rename("/mqtt.json.tmp", "/mqtt.json")) { + if (temp == MQTTPrefsRecovery::FileState::Usable && + backup != MQTTPrefsRecovery::FileState::Missing) { + fs->remove("/mqtt.json.bak"); + } + MESH_DEBUG_PRINTLN("MQTT: recovered /mqtt.json from transaction temp"); + return MQTTPrefsRecovery::uncertain(temp) || + MQTTPrefsRecovery::uncertain(backup); + } + MESH_DEBUG_PRINTLN("MQTT: could not recover /mqtt.json temp; files preserved"); + return true; + } + if (action == MQTTPrefsRecovery::Action::PromoteBackup) { + if (fs->rename("/mqtt.json.bak", "/mqtt.json")) { + if (backup == MQTTPrefsRecovery::FileState::Usable && + !MQTTPrefsRecovery::uncertain(temp) && + temp != MQTTPrefsRecovery::FileState::Missing) { + fs->remove("/mqtt.json.tmp"); + } + MESH_DEBUG_PRINTLN("MQTT: recovered /mqtt.json from transaction backup"); + return MQTTPrefsRecovery::uncertain(temp) || + MQTTPrefsRecovery::uncertain(backup); + } + MESH_DEBUG_PRINTLN("MQTT: could not recover /mqtt.json backup; files preserved"); + return true; + } + return false; +} + static MQTTPrefsRecovery::FileState mqttPrefsFileState(FILESYSTEM* fs, const char* path) { if (!fs->exists(path)) return MQTTPrefsRecovery::FileState::Missing; File file = openMqttPrefsRead(fs, path); @@ -432,6 +557,14 @@ static bool recoverMqttPrefsFiles(FILESYSTEM* fs) { } return false; } + if (action == MQTTPrefsRecovery::Action::DiscardTemp) { + if (fs->remove("/mqtt_prefs.tmp")) { + MESH_DEBUG_PRINTLN("MQTT: discarded incomplete legacy transaction temp"); + return false; + } + MESH_DEBUG_PRINTLN("MQTT: could not discard incomplete /mqtt_prefs temp; source held"); + return true; + } if (action == MQTTPrefsRecovery::Action::PromoteTemp) { if (fs->rename("/mqtt_prefs.tmp", "/mqtt_prefs")) { // A usable temp is now the committed primary. Its backup is necessarily @@ -463,30 +596,31 @@ static bool recoverMqttPrefsFiles(FILESYSTEM* fs) { return false; } -// Filesystem adapter for MQTTPrefsAtomicStore. It writes the new image to -// /mqtt_prefs.tmp and verifies its size. Publishing is a recoverable SPIFFS +// Filesystem adapter for the ConfigSerializer image. It writes to +// /mqtt.json.tmp and verifies its size and checksum. Publishing is a recoverable SPIFFS // transaction: primary -> .bak, then tmp -> primary, then best-effort backup // cleanup. A power loss at every boundary leaves at least one recoverable file. -class MQTTPrefsFileStore { +class MQTTPrefsJsonFileStore { public: - explicit MQTTPrefsFileStore(FILESYSTEM* fs) : _fs(fs) {} + explicit MQTTPrefsJsonFileStore(FILESYSTEM* fs) : _fs(fs) {} bool begin() { _finished = false; _open = false; _owns_temp = false; _bytes_written = 0; + _expected_crc = MQTT_JSON_FNV1A_OFFSET_BASIS; // Recovery owns stale artifacts. Do not delete them here: a failed commit // may have moved the old primary to .bak and left a verified temp that the // next boot must choose between. Refusing the save is safer than erasing an // image this firmware cannot decode. - if (_fs->exists("/mqtt_prefs.tmp") || _fs->exists("/mqtt_prefs.bak")) return false; + if (_fs->exists("/mqtt.json.tmp") || _fs->exists("/mqtt.json.bak")) return false; #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) - _file = _fs->open("/mqtt_prefs.tmp", FILE_O_WRITE); + _file = _fs->open("/mqtt.json.tmp", FILE_O_WRITE); #elif defined(RP2040_PLATFORM) - _file = _fs->open("/mqtt_prefs.tmp", "w"); + _file = _fs->open("/mqtt.json.tmp", "w"); #else - _file = _fs->open("/mqtt_prefs.tmp", "w", true); + _file = _fs->open("/mqtt.json.tmp", "w", true); #endif _open = _file; _owns_temp = _open; @@ -496,6 +630,9 @@ public: size_t write(const uint8_t* bytes, size_t size) { if (!_open) return 0; const size_t written = _file.write(bytes, size); + for (size_t i = 0; i < written; ++i) { + _expected_crc = (_expected_crc ^ bytes[i]) * MQTT_JSON_FNV1A_PRIME; + } _bytes_written += written; return written; } @@ -505,12 +642,33 @@ public: _file.close(); _open = false; #if defined(RP2040_PLATFORM) - File verify = _fs->open("/mqtt_prefs.tmp", "r"); + File verify = _fs->open("/mqtt.json.tmp", "r"); #else - File verify = _fs->open("/mqtt_prefs.tmp"); + File verify = _fs->open("/mqtt.json.tmp"); #endif if (!verify) return false; - const bool complete = verify.size() == _bytes_written; + uint32_t actual_crc = MQTT_JSON_FNV1A_OFFSET_BASIS; + size_t actual_size = 0; + bool read_failed = false; + uint8_t buf[64]; + while (verify.available() > 0) { + // Arduino File implementations normally return a byte count, but some + // Stream implementations use -1 for a read error. Keep that sentinel + // signed so it cannot become a huge size_t and overrun this buffer. + const int count = static_cast(verify.read(buf, sizeof(buf))); + if (count <= 0) { + read_failed = count < 0; + break; + } + actual_size += static_cast(count); + for (int i = 0; i < count; ++i) { + actual_crc = (actual_crc ^ buf[i]) * MQTT_JSON_FNV1A_PRIME; + } + } + const bool complete = !read_failed && + verify.size() == _bytes_written && + actual_size == _bytes_written && + actual_crc == _expected_crc; verify.close(); if (!complete) return false; _finished = true; @@ -523,25 +681,33 @@ public: // recoverable backup first, then publish temp into the now-empty primary. // Never remove either image after a failed boundary; boot recovery selects // the completed temp or restores the backup. - if (_fs->exists("/mqtt_prefs.bak")) return false; - if (_fs->exists("/mqtt_prefs") && !_fs->rename("/mqtt_prefs", "/mqtt_prefs.bak")) { + if (_fs->exists("/mqtt.json.bak")) return false; + if (_fs->exists("/mqtt.json") && !_fs->rename("/mqtt.json", "/mqtt.json.bak")) { return false; } - if (!_fs->rename("/mqtt_prefs.tmp", "/mqtt_prefs")) return false; + if (!_fs->rename("/mqtt.json.tmp", "/mqtt.json")) return false; // Cleanup failure is non-fatal: the new primary is published and recovery // will remove a known-good stale backup on a later boot. - if (_fs->exists("/mqtt_prefs.bak")) _fs->remove("/mqtt_prefs.bak"); + if (_fs->exists("/mqtt.json.bak")) _fs->remove("/mqtt.json.bak"); return true; } + void discardFinishedTemp() { + if (_open) _file.close(); + _open = false; + if (_owns_temp && _fs->exists("/mqtt.json.tmp")) _fs->remove("/mqtt.json.tmp"); + _finished = false; + _owns_temp = false; + } + void abort() { if (_open) _file.close(); _open = false; // Once finish() has verified the temp, commit may already have moved the // primary to .bak. Keep the temp on a commit failure so recovery can // publish it (or fall back to .bak) after reset. - if (_owns_temp && !_finished && _fs->exists("/mqtt_prefs.tmp")) { - _fs->remove("/mqtt_prefs.tmp"); + if (_owns_temp && !_finished && _fs->exists("/mqtt.json.tmp")) { + _fs->remove("/mqtt.json.tmp"); } _finished = false; _owns_temp = false; @@ -554,31 +720,90 @@ private: bool _finished = false; bool _owns_temp = false; size_t _bytes_written = 0; + uint32_t _expected_crc = MQTT_JSON_FNV1A_OFFSET_BASIS; +}; + +class MQTTPrefsStoreStream : public Stream { +public: + explicit MQTTPrefsStoreStream(MQTTPrefsJsonFileStore* store) : _store(store) {} + + size_t write(uint8_t byte) override { return write(&byte, 1); } + size_t write(const uint8_t* buffer, size_t size) override { + if (!_ok || _store == nullptr) return 0; + const size_t written = _store->write(buffer, size); + if (written != size) _ok = false; + return written; + } + int available() override { return 0; } + int read() override { return -1; } + int peek() override { return -1; } + bool ok() const { return _ok; } + +private: + MQTTPrefsJsonFileStore* _store; + bool _ok = true; }; #endif // WITH_MQTT_BRIDGE #ifdef WITH_MQTT_BRIDGE -static const char* mqttPrefsSaveResultName(MQTTPrefsAtomicStore::Result result) { - switch (result) { - case MQTTPrefsAtomicStore::Result::BeginFailed: return "begin"; - case MQTTPrefsAtomicStore::Result::HeaderWriteFailed: return "header write"; - case MQTTPrefsAtomicStore::Result::PayloadWriteFailed: return "payload write"; - case MQTTPrefsAtomicStore::Result::FinishFailed: return "close"; - case MQTTPrefsAtomicStore::Result::CommitFailed: return "rename"; - case MQTTPrefsAtomicStore::Result::Committed: return "committed"; - } - return "unknown"; -} - void CommonCLI::loadMQTTPrefs( FILESYSTEM* fs, MQTTPrefsAtomicStore::LegacyUpgradeGate* legacy_upgrade) { setMQTTPrefsDefaults(&_mqtt_prefs); + _mqtt_prefs_hold = recoverMqttJsonFiles(fs); + if (_mqtt_prefs_hold && !fs->exists("/mqtt.json") && + (fs->exists("/mqtt.json.tmp") || fs->exists("/mqtt.json.bak"))) { + _legacy_tail.valid = false; + MESH_DEBUG_PRINTLN("MQTT: unresolved /mqtt.json recovery files; using defaults (files preserved)"); + return; + } + + // The JSON file is authoritative once it exists. Never fall back to the + // stale binary snapshot when JSON is corrupt, unreadable, or from a future + // schema: preserve it and run defaults until an operator resolves it. + if (fs->exists("/mqtt.json")) { + MQTTPrefs* scratch = new (std::nothrow) MQTTPrefs; + if (scratch == nullptr) { + _mqtt_prefs_hold = true; + _legacy_tail.valid = false; + MESH_DEBUG_PRINTLN("MQTT: no memory to validate /mqtt.json; source preserved"); + return; + } + const JsonPrefsLoadResult json_result = loadMqttJsonFile(fs, "/mqtt.json", scratch); + if (json_result == JsonPrefsLoadResult::Loaded || + json_result == JsonPrefsLoadResult::LoadedWithRepairs) { + memcpy(&_mqtt_prefs, scratch, sizeof(_mqtt_prefs)); + delete scratch; + _legacy_tail.valid = false; + if (json_result == JsonPrefsLoadResult::LoadedWithRepairs) { + MESH_DEBUG_PRINTLN("MQTT: repaired out-of-range values in /mqtt.json"); + if (!_mqtt_prefs_hold && !saveMQTTPrefs(fs)) { + _mqtt_prefs_hold = true; + MESH_DEBUG_PRINTLN("MQTT: could not persist /mqtt.json repairs; source held"); + } + } + return; + } + delete scratch; + _mqtt_prefs_hold = true; + _legacy_tail.valid = false; + if (json_result == JsonPrefsLoadResult::UnsupportedVersion) { + MESH_DEBUG_PRINTLN("MQTT: /mqtt.json uses a future version; using defaults (file preserved)"); + } else if (json_result == JsonPrefsLoadResult::FutureClaimed) { + MESH_DEBUG_PRINTLN( + "MQTT: /mqtt.json claims a future version but uses unknown grammar; " + "using defaults (file preserved)"); + } else { + MESH_DEBUG_PRINTLN("MQTT: /mqtt.json is invalid or unreadable; using defaults (file preserved)"); + } + return; + } + // Complete or preserve an interrupted SPIFFS transaction before decoding. // A failed recovery leaves the artifacts untouched and blocks this boot from // replacing them with defaults through a later CLI save. - _mqtt_prefs_hold = recoverMqttPrefsFiles(fs); + _mqtt_prefs_hold = _mqtt_prefs_hold || recoverMqttPrefsFiles(fs); bool has_observer_fields = false; bool mqtt_rewrite_pending = false; bool migrated_legacy_mqtt = false; @@ -600,24 +825,34 @@ void CommonCLI::loadMQTTPrefs( } else if (plan.source == MQTTPrefsCodec::Source::Current) { file = openMqttPrefsRead(fs); MQTTPrefsHeader header; - if (!file || file.read((uint8_t *)&header, sizeof(header)) != sizeof(header) || - file.read((uint8_t *)&_mqtt_prefs, plan.payload_len) != plan.payload_len) { + LegacyV1MQTTPrefs* old_prefs = new (std::nothrow) LegacyV1MQTTPrefs; + if (old_prefs) memset(old_prefs, 0, sizeof(*old_prefs)); + if (!old_prefs || !file || file.read((uint8_t *)&header, sizeof(header)) != sizeof(header) || + file.read((uint8_t *)old_prefs, plan.payload_len) != plan.payload_len) { setMQTTPrefsDefaults(&_mqtt_prefs); _mqtt_prefs_hold = true; MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs read failed, using defaults (file preserved)"); + } else if (!MQTTPrefsCodec::isPlausibleV1(*old_prefs, plan.payload_len)) { + setMQTTPrefsDefaults(&_mqtt_prefs); + _mqtt_prefs_hold = true; + MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs v1 content failed plausibility checks; source preserved"); } else { + MQTTPrefsCodec::migrateV1(*old_prefs, plan.payload_len, &_mqtt_prefs); has_observer_fields = plan.observer_fields_present; + mqtt_rewrite_pending = true; + migrated_legacy_mqtt = true; // Written by a later build with appended fields. Everything this // binary knows loaded normally; say so, because the next `set` will // rewrite the file at this length and drop the newer settings. if (file_size - sizeof(MQTTPrefsHeader) > plan.payload_len) { MESH_DEBUG_PRINTLN( "MQTT: /mqtt_prefs written by newer firmware (%u > %u bytes); " - "config loaded, newer settings ignored and dropped on next save", + "known settings loaded; newer binary fields remain in the rollback snapshot", (unsigned)(file_size - sizeof(MQTTPrefsHeader)), (unsigned)plan.payload_len); } } + delete old_prefs; if (file) file.close(); } else if (plan.rewrite_legacy) { bool migrated = false; @@ -674,16 +909,18 @@ void CommonCLI::loadMQTTPrefs( case MQTTPrefsCodec::Source::LegacySixSlotAudience: case MQTTPrefsCodec::Source::LegacySixSlotAudienceRx: case MQTTPrefsCodec::Source::LegacySixSlot: { - Legacy6SlotMQTTPrefs old_prefs = {}; - if (file.read((uint8_t *)&old_prefs, plan.payload_len) == plan.payload_len) { + Legacy6SlotMQTTPrefs* old_prefs = new (std::nothrow) Legacy6SlotMQTTPrefs; + if (old_prefs) memset(old_prefs, 0, sizeof(*old_prefs)); + if (old_prefs && file.read((uint8_t *)old_prefs, plan.payload_len) == plan.payload_len) { if (MQTTPrefsCodec::isPlausibleLegacy(plan.source, - (const uint8_t *)&old_prefs, plan.payload_len)) { - MQTTPrefsCodec::migrateLegacySixSlot(old_prefs, plan.source, &_mqtt_prefs); + (const uint8_t *)old_prefs, plan.payload_len)) { + MQTTPrefsCodec::migrateLegacySixSlot(*old_prefs, plan.source, &_mqtt_prefs); migrated = true; } else { MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs legacy content failed plausibility checks"); } } + delete old_prefs; break; } default: @@ -742,9 +979,9 @@ void CommonCLI::loadMQTTPrefs( if (mqtt_rewrite_pending) { legacy_upgrade->requireMqttRewrite(); if (migrated_legacy_mqtt) { - MESH_DEBUG_PRINTLN("MQTT: Migrating headerless /mqtt_prefs to versioned layout"); + MESH_DEBUG_PRINTLN("MQTT: Migrating binary /mqtt_prefs to /mqtt.json"); } else { - MESH_DEBUG_PRINTLN("MQTT: Persisting observer tail into /mqtt_prefs before /com_prefs compaction"); + MESH_DEBUG_PRINTLN("MQTT: Persisting observer tail into /mqtt.json before /com_prefs compaction"); } if (saveMQTTPrefs(fs)) { legacy_upgrade->recordMqttSave(true); @@ -754,7 +991,7 @@ void CommonCLI::loadMQTTPrefs( // untouched; the next boot can recover the tail and retry the transaction. _mqtt_prefs_hold = true; legacy_upgrade->recordMqttSave(false); - MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs migration save failed; legacy files preserved and held"); + MESH_DEBUG_PRINTLN("MQTT: /mqtt.json migration save failed; legacy files preserved and held"); } } } @@ -763,26 +1000,71 @@ bool CommonCLI::saveMQTTPrefs(FILESYSTEM* fs) { if (_mqtt_prefs_hold) { // Loading deliberately preserved the source file. Do not replace it with this // boot's defaults after an unsupported, corrupt, or temporarily failed read. - MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs held, not overwriting"); + MESH_DEBUG_PRINTLN("MQTT: observer preference source held, not overwriting /mqtt.json"); return false; } - // Write header and payload sequentially so the transaction needs no second - // full-size (2.8 KiB) staging buffer on constrained targets. The length is - // the shortest that still round-trips this config, so a node with default - // packet filters keeps writing a payload older firmware can read. - const size_t payload_len = MQTTPrefsCodec::payloadLenFor(_mqtt_prefs); - const MQTTPrefsHeader header = MQTTPrefsCodec::makeHeader(payload_len); - MQTTPrefsFileStore store(fs); - const MQTTPrefsAtomicStore::Result result = MQTTPrefsAtomicStore::write( - store, (const uint8_t *)&header, sizeof(header), - (const uint8_t *)&_mqtt_prefs, payload_len); - if (!MQTTPrefsAtomicStore::committed(result)) { - MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt_prefs save failed at %s; source preserved", - mqttPrefsSaveResultName(result)); + MQTTPrefs* repair_defaults = new (std::nothrow) MQTTPrefs; + if (repair_defaults == nullptr) { + MESH_DEBUG_PRINTLN("MQTT: no memory to normalize observer settings before save"); return false; } - return true; + setMQTTPrefsDefaults(repair_defaults); + MQTTPrefsSerializer serializer(&_mqtt_prefs, repair_defaults); + // The serializer hierarchy copies the default values it needs. Release this + // large temporary before file I/O and the independent verification scratch. + delete repair_defaults; + bool repaired = false; + if (!serializer.normalize(&repaired)) return false; + if (repaired) MESH_DEBUG_PRINTLN("MQTT: normalized out-of-range observer settings before save"); + + MQTTPrefsJsonFileStore store(fs); + bool verify_oom = false; + const MQTTPrefsAtomicStore::VerifiedImageResult result = + MQTTPrefsAtomicStore::writeVerifiedImage( + store, + [&]() -> bool { + MQTTPrefsStoreStream stream(&store); + return serializer.saveSerial(stream) && stream.ok(); + }, + [&]() -> bool { + MQTTPrefs* verify = new (std::nothrow) MQTTPrefs; + if (verify == nullptr) { + verify_oom = true; + return false; + } + const JsonPrefsLoadResult verify_result = + loadMqttJsonFile(fs, "/mqtt.json.tmp", verify); + delete verify; + // saveSerial() emits already-normalized values. Needing another + // repair here means save/load is not idempotent. + return verify_result == JsonPrefsLoadResult::Loaded; + }); + + switch (result) { + case MQTTPrefsAtomicStore::VerifiedImageResult::Committed: + return true; + case MQTTPrefsAtomicStore::VerifiedImageResult::BeginFailed: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed at begin; source preserved"); + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::WriteFailed: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed during write; source preserved"); + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::FinishFailed: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed during checksum verification; source preserved"); + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::VerifyFailed: + if (verify_oom) { + MESH_DEBUG_PRINTLN("MQTT: no memory to validate /mqtt.json temp; source preserved"); + } else { + MESH_DEBUG_PRINTLN("MQTT: generated /mqtt.json temp failed schema validation; source preserved"); + } + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::CommitFailed: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed during rename; recovery files preserved"); + break; + } + return false; } #endif @@ -1419,6 +1701,15 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep strcpy(reply, "Error: delay must be between 0-10000 ms"); } } else if (memcmp(config, "bridge.source ", 14) == 0) { +#ifdef WITH_MQTT_BRIDGE + MQTTPrefs* observer_rollback = new (std::nothrow) MQTTPrefs; + if (observer_rollback == nullptr) { + strcpy(reply, "Error: insufficient memory to update observer setting"); + return; + } + memcpy(observer_rollback, &_mqtt_prefs, sizeof(*observer_rollback)); + const uint8_t old_bridge_pkt_src = _prefs->bridge_pkt_src; +#endif _prefs->bridge_pkt_src = memcmp(&config[14], "rx", 2) == 0; #ifdef WITH_MQTT_BRIDGE if (_prefs->bridge_pkt_src == 1) { @@ -1428,6 +1719,15 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep _mqtt_prefs.mqtt_rx_enabled = 0; _mqtt_prefs.mqtt_tx_enabled = 1; } + _observer_prefs_rollback = observer_rollback; + if (!persistObserverPrefs(reply)) { + _prefs->bridge_pkt_src = old_bridge_pkt_src; + _observer_prefs_rollback = nullptr; + delete observer_rollback; + return; + } + _observer_prefs_rollback = nullptr; + delete observer_rollback; #endif savePrefs(); strcpy(reply, "OK"); diff --git a/src/helpers/CommonCLI.h b/src/helpers/CommonCLI.h index e0a0a71b..2e2f02ef 100644 --- a/src/helpers/CommonCLI.h +++ b/src/helpers/CommonCLI.h @@ -73,7 +73,7 @@ public: uint8_t extra_sf[4]; // NOTE: observer settings (MQTT/WiFi/timezone/SNMP/alert) are not in NodePrefs. - // They live in MQTTPrefs, persisted separately to /mqtt_prefs, so this struct + // They live in MQTTPrefs, persisted separately to /mqtt.json, so this struct // stays aligned with upstream. See struct MQTTPrefs below. private: @@ -228,6 +228,13 @@ struct LegacyObserverTail { class CommonCLICallbacks { public: virtual void savePrefs() = 0; +#ifdef WITH_MQTT_BRIDGE + virtual bool saveObserverPrefs() = 0; +#else + virtual bool saveObserverPrefs() { + return false; + } +#endif virtual const char* getFirmwareVer() = 0; virtual const char* getBuildDate() = 0; virtual const char* getRole() = 0; @@ -359,8 +366,11 @@ class CommonCLI { char tmp[PRV_KEY_SIZE*2 + 4]; #ifdef WITH_MQTT_BRIDGE MQTTPrefs _mqtt_prefs; + // Points at a per-command snapshot only while an observer setter is running. + // persistObserverPrefs() uses it to undo RAM mutations when flash commit fails. + const MQTTPrefs* _observer_prefs_rollback = nullptr; LegacyObserverTail _legacy_tail; - // /mqtt_prefs is newer, corrupt, or temporarily unreadable. The in-memory prefs + // /mqtt.json is newer, corrupt, or temporarily unreadable. The in-memory prefs // run on defaults and saveMQTTPrefs() must not overwrite the source file. bool _mqtt_prefs_hold = false; #endif @@ -384,6 +394,7 @@ class CommonCLI { // false to fall through to the base get/set parsing. bool handleObserverSetCmd(uint32_t sender_timestamp, const char* config, char* reply); bool handleObserverGetCmd(uint32_t sender_timestamp, const char* config, char* reply); + bool persistObserverPrefs(char* reply); // Observer-only top-level commands (ota check/update, tls.bundletest, alert test) // also live in CommonCLI_Observer.cpp; returns true if it handled the command. bool handleObserverCommand(uint32_t sender_timestamp, char* command, char* reply); @@ -393,14 +404,17 @@ public: : _board(&board), _rtc(&rtc), _sensors(&sensors), _region_map(®ion_map), _acl(&acl), _prefs(prefs), _callbacks(callbacks) { } void loadPrefs(FILESYSTEM* _fs); - bool savePrefs(FILESYSTEM* _fs, bool save_mqtt = true); + // Node preferences and observer preferences are separate transactions. + // Callers must explicitly request an observer save when they changed it. + bool savePrefs(FILESYSTEM* _fs, bool save_mqtt = false); void handleCommand(uint32_t sender_timestamp, char* command, char* reply); mesh::MainBoard* getBoard() { return _board; } uint8_t buildAdvertData(uint8_t node_type, uint8_t* app_data); #ifdef WITH_MQTT_BRIDGE - // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt_prefs. + // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt.json. // Exposed so the app can hand it to MQTTBridge/AlertReporter, which read these // fields directly (they no longer live in NodePrefs). MQTTPrefs* getObserverPrefs() const { return const_cast(&_mqtt_prefs); } + bool saveObserverPrefs(FILESYSTEM* fs) { return saveMQTTPrefs(fs); } #endif }; diff --git a/src/helpers/CommonCLI_Observer.cpp b/src/helpers/CommonCLI_Observer.cpp index c1b7e74a..93c25776 100644 --- a/src/helpers/CommonCLI_Observer.cpp +++ b/src/helpers/CommonCLI_Observer.cpp @@ -16,6 +16,7 @@ #include "AlertReporter.h" // for alertReporterBannedChannelMatch[Hex]() #include "MQTTObserverValidation.h" // pure input validators (host-testable) #include +#include #ifdef ESP_PLATFORM #include #include @@ -109,6 +110,46 @@ static bool valueTooLong(const char* val, size_t bufsize, char* reply, const cha return false; } +static bool isObserverPrefsSetCommand(const char* config) { + return strncmp(config, "snmp", 4) == 0 || + strncmp(config, "radio.watchdog", 14) == 0 || + strncmp(config, "mqtt", 4) == 0 || + strncmp(config, "wifi.", 5) == 0 || + strncmp(config, "timezone", 8) == 0 || + strncmp(config, "alert", 5) == 0; +} + +// Keep observer setters atomic from the caller's perspective. The live object +// is restored if its JSON transaction fails, and the snapshot pointer is valid +// only for this synchronous command dispatch. +class ObserverPrefsRollbackScope { +public: + ObserverPrefsRollbackScope(const MQTTPrefs** active_snapshot, + const MQTTPrefs& live, + bool capture) + : _active_snapshot(active_snapshot) { + *_active_snapshot = nullptr; + if (capture) { + _snapshot = new (std::nothrow) MQTTPrefs; + if (_snapshot != nullptr) { + memcpy(_snapshot, &live, sizeof(*_snapshot)); + *_active_snapshot = _snapshot; + } + } + } + + ~ObserverPrefsRollbackScope() { + *_active_snapshot = nullptr; + delete _snapshot; + } + + bool available() const { return _snapshot != nullptr; } + +private: + const MQTTPrefs** _active_snapshot; + MQTTPrefs* _snapshot = nullptr; +}; + static const char* getMQTTPresetNameByIndex(int index) { if (index < MQTT_PRESET_COUNT) return MQTT_PRESETS[index].name; if (index == MQTT_PRESET_COUNT) return MQTT_PRESET_CUSTOM; @@ -161,17 +202,38 @@ static void formatMQTTPresetListReply(char* reply, size_t reply_size, int start) } #endif +bool CommonCLI::persistObserverPrefs(char* reply) { +#ifdef WITH_MQTT_BRIDGE + if (_callbacks->saveObserverPrefs()) return true; + if (_observer_prefs_rollback != nullptr) { + memcpy(&_mqtt_prefs, _observer_prefs_rollback, sizeof(_mqtt_prefs)); + } + strcpy(reply, "Error: setting not persisted; change rolled back"); + return false; +#else + (void)reply; + return false; +#endif +} + bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* config, char* reply) { #ifdef WITH_MQTT_BRIDGE + const bool needs_snapshot = isObserverPrefsSetCommand(config); + ObserverPrefsRollbackScope rollback_scope( + &_observer_prefs_rollback, _mqtt_prefs, needs_snapshot); + if (needs_snapshot && !rollback_scope.available()) { + strcpy(reply, "Error: insufficient memory to update observer setting"); + return true; + } bool handled = true; if (memcmp(config, "snmp.community ", 15) == 0) { if (valueTooLong(&config[15], sizeof(_mqtt_prefs.snmp_community), reply, "snmp.community")) return true; StrHelper::strncpy(_mqtt_prefs.snmp_community, &config[15], sizeof(_mqtt_prefs.snmp_community)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK - restart to apply"); } else if (memcmp(config, "snmp ", 5) == 0) { _mqtt_prefs.snmp_enabled = memcmp(&config[5], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK - restart to apply"); } else if (memcmp(config, "radio.watchdog ", 15) == 0) { const char* val = &config[15]; @@ -189,7 +251,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: radio.watchdog must be 0-120 minutes"); } else { _mqtt_prefs.radio_watchdog_minutes = (uint8_t)mins; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; if (mins == 0) { strcpy(reply, "OK - radio watchdog disabled"); } else { @@ -200,13 +262,13 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf #ifdef WITH_MQTT_BRIDGE } else if (strcmp(config, "mqtt.origin") == 0) { _mqtt_prefs.mqtt_origin[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.origin ", 12) == 0) { if (valueTooLong(&config[12], sizeof(_mqtt_prefs.mqtt_origin), reply, "origin")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_origin, &config[12], sizeof(_mqtt_prefs.mqtt_origin)); StrHelper::stripSurroundingQuotes(_mqtt_prefs.mqtt_origin, sizeof(_mqtt_prefs.mqtt_origin)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.iata ", 10) == 0) { const char* iata = &config[10]; @@ -215,7 +277,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // Empty clears the region code (meshcore-topic publishing stays disabled // until one is set). This keeps the pre-existing "clear IATA" capability. _mqtt_prefs.mqtt_iata[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridge(); strcpy(reply, "OK - IATA cleared"); } else { @@ -228,22 +290,22 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf for (int i = 0; _mqtt_prefs.mqtt_iata[i]; i++) { _mqtt_prefs.mqtt_iata[i] = toupper(_mqtt_prefs.mqtt_iata[i]); } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridge(); strcpy(reply, "OK"); } } } else if (memcmp(config, "mqtt.status ", 12) == 0) { _mqtt_prefs.mqtt_status_enabled = memcmp(&config[12], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.packets ", 13) == 0) { _mqtt_prefs.mqtt_packets_enabled = memcmp(&config[13], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.raw ", 9) == 0) { _mqtt_prefs.mqtt_raw_enabled = memcmp(&config[9], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.tx ", 8) == 0) { if (memcmp(&config[8], "advert", 6) == 0) { @@ -251,17 +313,17 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else { _mqtt_prefs.mqtt_tx_enabled = memcmp(&config[8], "on", 2) == 0 ? 1 : 0; } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.rx ", 8) == 0) { _mqtt_prefs.mqtt_rx_enabled = memcmp(&config[8], "on", 2) == 0 ? 1 : 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.interval ", 14) == 0) { uint32_t minutes = _atoi(&config[14]); if (minutes >= 1 && minutes <= 60) { _mqtt_prefs.mqtt_status_interval = minutes * 60000; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridge(); sprintf(reply, "OK - interval set to %u minutes (%lu ms), bridge restarted", minutes, (unsigned long)_mqtt_prefs.mqtt_status_interval); } else { @@ -274,7 +336,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf uint32_t hours = _atoi(&config[24]); if (hours >= MQTT_NEIGHBORS_MIN_INTERVAL_HOURS && hours <= MQTT_NEIGHBORS_MAX_INTERVAL_HOURS) { _mqtt_prefs.mqtt_neighbors_interval = hours * 3600000UL; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; sprintf(reply, "OK - neighbors interval set to %u hours (%lu ms)", (unsigned)hours, (unsigned long)_mqtt_prefs.mqtt_neighbors_interval); } else { @@ -284,7 +346,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // The mesh loop reads this live, so no bridge restart is needed; enabling it // triggers a discovery on the next eligible loop pass. _mqtt_prefs.mqtt_neighbors_enabled = memcmp(&config[15], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); #elif defined(WITH_MQTT_BRIDGE) } else if (memcmp(config, "mqtt.neighbors.interval ", 24) == 0 || @@ -303,7 +365,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else { StrHelper::strncpy(_mqtt_prefs.mqtt_ntp_server, host, sizeof(_mqtt_prefs.mqtt_ntp_server)); } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; #ifdef ESP_PLATFORM // Queue a sync on the MQTT task (Core 0) but do NOT block: this handler // runs on the Arduino loop task, shared with mesh/radio processing and the @@ -325,12 +387,12 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "wifi.ssid ", 10) == 0) { if (valueTooLong(&config[10], sizeof(_mqtt_prefs.wifi_ssid), reply, "wifi.ssid")) return true; StrHelper::strncpy(_mqtt_prefs.wifi_ssid, &config[10], sizeof(_mqtt_prefs.wifi_ssid)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "wifi.pwd ", 9) == 0) { if (valueTooLong(&config[9], sizeof(_mqtt_prefs.wifi_password), reply, "wifi.pwd")) return true; StrHelper::strncpy(_mqtt_prefs.wifi_password, &config[9], sizeof(_mqtt_prefs.wifi_password)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "wifi.powersave ", 15) == 0) { const char* value = &config[15]; @@ -350,7 +412,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: must be none, min, or max"); } else { _mqtt_prefs.wifi_power_save = ps_value; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; #ifdef ESP_PLATFORM if (WiFi.status() == WL_CONNECTED) { wifi_ps_type_t ps_mode = (ps_value == 1) ? WIFI_PS_NONE : @@ -374,13 +436,13 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "timezone ", 9) == 0) { if (valueTooLong(&config[9], sizeof(_mqtt_prefs.timezone_string), reply, "timezone")) return true; StrHelper::strncpy(_mqtt_prefs.timezone_string, &config[9], sizeof(_mqtt_prefs.timezone_string)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "timezone.offset ", 16) == 0) { int8_t offset = _atoi(&config[16]); if (offset >= -12 && offset <= 14) { _mqtt_prefs.timezone_offset = offset; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else { strcpy(reply, "Error: timezone offset must be between -12 and +14"); @@ -397,20 +459,14 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcmp(preset_name, MQTT_PRESET_CUSTOM) == 0 || strcmp(preset_name, MQTT_PRESET_NONE) == 0) { // Reject duplicate presets (except "none" and "custom") - int dup_slot = -1; - if (findMQTTPreset(preset_name) != nullptr) { - for (int s = 0; s < MAX_MQTT_SLOTS; s++) { - if (s != slot && strcmp(_mqtt_prefs.mqtt_slot_preset[s], preset_name) == 0) { - dup_slot = s; - break; - } - } - } + const int dup_slot = findMQTTPreset(preset_name) == nullptr ? -1 : + mqttAssignedPresetSlot(_mqtt_prefs.mqtt_slot_preset, + preset_name, slot); if (dup_slot >= 0) { sprintf(reply, "Error: preset '%s' is already assigned to slot %d", preset_name, dup_slot + 1); } else { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], preset_name, sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); // Check if the slot has everything it needs to connect const MQTTPresetDef* p = findMQTTPreset(preset_name); @@ -469,7 +525,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(subcmd, "server ", 7) == 0) { if (valueTooLong(&subcmd[7], sizeof(_mqtt_prefs.mqtt_slot_host[slot]), reply, "server")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_host[slot], &subcmd[7], sizeof(_mqtt_prefs.mqtt_slot_host[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; // Reconfigure the slot so the new host reaches the live connection (other // custom-slot setters do the same; without it the change only applies on // the next reboot/bridge restart). @@ -479,7 +535,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf int port = atoi(&subcmd[5]); if (port > 0 && port <= 65535) { _mqtt_prefs.mqtt_slot_port[slot] = port; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else { @@ -488,19 +544,19 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(subcmd, "username ", 9) == 0) { if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_username[slot]), reply, "username")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_username[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_username[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(subcmd, "password ", 9) == 0) { if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_password[slot]), reply, "password")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_password[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_password[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(subcmd, "token ", 6) == 0) { if (valueTooLong(&subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_token[slot]), reply, "token")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_token[slot], &subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_token[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); sprintf(reply, "OK - slot %d token set", slot + 1); } else if (memcmp(subcmd, "topic ", 6) == 0) { @@ -510,14 +566,14 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf return true; } else { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_topic[slot], &subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_topic[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); sprintf(reply, "OK - slot %d topic: %s", slot + 1, _mqtt_prefs.mqtt_slot_topic[slot]); } } else if (memcmp(subcmd, "audience ", 9) == 0) { if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_audience[slot]), reply, "audience")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_audience[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_audience[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); if (_mqtt_prefs.mqtt_slot_audience[slot][0] != '\0') { sprintf(reply, "OK - slot %d JWT audience: %s", slot + 1, _mqtt_prefs.mqtt_slot_audience[slot]); @@ -527,7 +583,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(subcmd, "audience", 8) == 0 && subcmd[8] == '\0') { // "set mqttN.audience" with no value — clear the audience _mqtt_prefs.mqtt_slot_audience[slot][0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); sprintf(reply, "OK - slot %d JWT audience cleared (using username/password auth)", slot + 1); } else if (strcmp(subcmd, "filter") == 0 || @@ -539,20 +595,18 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: filter must be all, none, or a CSV of types 0-15 / names (advert,txt_msg,...)"); } else { _mqtt_prefs.mqtt_slot_packet_filter[slot] = filter_mask; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; char filter_text[MQTTPacketFilter::kFilterTextSize]; MQTTPacketFilter::format(filter_mask, filter_text, sizeof(filter_text)); snprintf(reply, 160, "OK - slot %d packet types: %s", slot + 1, filter_text); - // A non-default filter extends /mqtt_prefs past what pre-filter - // firmware can read (see MQTTPrefsCodec::payloadLenFor), so say when - // that cost buys nothing: slots beyond the runtime array are never - // published to on this board, the same warning `preset` gives. + // Slots beyond the runtime array are never published to on this board, + // so retain the same inactive-hardware warning `preset` gives. if (slot >= RUNTIME_MQTT_SLOTS && filter_mask != MQTTPacketFilter::kAllPacketTypes) { size_t used = strlen(reply); if (used < 158) { snprintf(reply + used, 160 - used, - " (slot inactive on this hardware; blocks firmware rollback)"); + " (slot inactive on this hardware)"); } } } @@ -562,21 +616,35 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "mqtt.analyzer.us ", 17) == 0) { const int slot = 0; if (memcmp(&config[17], "on", 2) == 0) { + const int dup_slot = mqttAssignedPresetSlot( + _mqtt_prefs.mqtt_slot_preset, "analyzer-us", slot); + if (dup_slot >= 0) { + sprintf(reply, "Error: preset 'analyzer-us' is already assigned to slot %d", + dup_slot + 1); + return true; + } StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], "analyzer-us", sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); } else { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], MQTT_PRESET_NONE, sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.analyzer.eu ", 17) == 0) { const int slot = 1; if (memcmp(&config[17], "on", 2) == 0) { + const int dup_slot = mqttAssignedPresetSlot( + _mqtt_prefs.mqtt_slot_preset, "analyzer-eu", slot); + if (dup_slot >= 0) { + sprintf(reply, "Error: preset 'analyzer-eu' is already assigned to slot %d", + dup_slot + 1); + return true; + } StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], "analyzer-eu", sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); } else { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], MQTT_PRESET_NONE, sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.owner ", 11) == 0) { @@ -585,11 +653,11 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // Owner key is optional — empty clears it (previously this errored, so a // set key could never be removed via the portal/CLI). _mqtt_prefs.mqtt_owner_public_key[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK - owner key cleared"); } else if (mqttOwnerKeyValid(owner_key)) { StrHelper::strncpy(_mqtt_prefs.mqtt_owner_public_key, owner_key, sizeof(_mqtt_prefs.mqtt_owner_public_key)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else { strcpy(reply, "Error: public key must be 64 hex characters (32 bytes)"); @@ -597,7 +665,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "mqtt.email ", 11) == 0) { if (valueTooLong(&config[11], sizeof(_mqtt_prefs.mqtt_email), reply, "email")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_email, &config[11], sizeof(_mqtt_prefs.mqtt_email)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); #endif } else if (memcmp(config, "alert ", 6) == 0) { @@ -605,12 +673,12 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf const char* val = &config[6]; if (memcmp(val, "on", 2) == 0 && (val[2] == 0 || val[2] == ' ')) { _mqtt_prefs.alert_enabled = 1; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alerts on"); } else if (memcmp(val, "off", 3) == 0 && (val[3] == 0 || val[3] == ' ')) { _mqtt_prefs.alert_enabled = 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alerts off"); } else { @@ -625,7 +693,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf if (len == 0) { _mqtt_prefs.alert_psk_hex[0] = '\0'; _mqtt_prefs.alert_hashtag[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alert.psk cleared (alerts disabled until configured)"); } else if (val[0] == '#') { @@ -659,7 +727,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // The new PSK is operator-supplied, so any previously-derived // hashtag name is no longer accurate provenance — drop it. _mqtt_prefs.alert_hashtag[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alert.psk updated"); } @@ -672,7 +740,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf if (in_len == 0) { _mqtt_prefs.alert_psk_hex[0] = '\0'; _mqtt_prefs.alert_hashtag[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alert.hashtag cleared (alerts disabled until configured)"); } else { @@ -708,7 +776,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf mesh::Utils::toHex(hex, digest, 16); StrHelper::strncpy(_mqtt_prefs.alert_hashtag, hashtag, sizeof(_mqtt_prefs.alert_hashtag)); StrHelper::strncpy(_mqtt_prefs.alert_psk_hex, hex, sizeof(_mqtt_prefs.alert_psk_hex)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); sprintf(reply, "OK - alert.hashtag: %s", _mqtt_prefs.alert_hashtag); } @@ -726,7 +794,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf size_t len = strlen(val); if (len == 0) { _mqtt_prefs.alert_region[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alert.region cleared (using default scope)"); } else if (len >= sizeof(_mqtt_prefs.alert_region)) { @@ -734,7 +802,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else { StrHelper::strncpy(_mqtt_prefs.alert_region, val, sizeof(_mqtt_prefs.alert_region)); StrHelper::stripSurroundingQuotes(_mqtt_prefs.alert_region, sizeof(_mqtt_prefs.alert_region)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); sprintf(reply, "OK - alert.region: %s", _mqtt_prefs.alert_region); } @@ -744,7 +812,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: alert.wifi must be 0-1440 minutes (0=off)"); } else { _mqtt_prefs.alert_wifi_minutes = (uint16_t)mins; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; sprintf(reply, "OK - alert.wifi %d min%s", mins, mins == 0 ? " (disabled)" : ""); } } else if (memcmp(config, "alert.mqtt ", 11) == 0) { @@ -753,7 +821,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: alert.mqtt must be 0-10080 minutes (0=off)"); } else { _mqtt_prefs.alert_mqtt_minutes = (uint16_t)mins; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; sprintf(reply, "OK - alert.mqtt %d min%s", mins, mins == 0 ? " (disabled)" : ""); } } else if (memcmp(config, "alert.interval ", 15) == 0) { @@ -764,7 +832,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: alert.interval must be 60-10080 minutes"); } else { _mqtt_prefs.alert_min_interval_min = (uint16_t)mins; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; sprintf(reply, "OK - alert.interval %d min", mins); } } else { diff --git a/src/helpers/ConfigSerializer.cpp b/src/helpers/ConfigSerializer.cpp index adff147f..b8788bf5 100644 --- a/src/helpers/ConfigSerializer.cpp +++ b/src/helpers/ConfigSerializer.cpp @@ -1,5 +1,9 @@ #include "ConfigSerializer.h" +#include +#include +#include + bool ConfigSerializer::saveSerial(Stream& s) { Context context(&s, OP::WRITE); _context = &context; // set the context for structure() call @@ -22,9 +26,12 @@ bool ConfigSerializer::saveSerial(Stream& s) { static bool is_whitespace(char c) { return c == ' ' || c == '\t' || c == '\r' || c == '\n'; } -static bool is_key_char(char c) { +static bool is_key_start_char(char c) { return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_'; } +static bool is_key_char(char c) { + return is_key_start_char(c) || (c >= '0' && c <= '9'); +} static bool is_value_char(char c) { return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || c == '-' || c == '.'; } @@ -64,14 +71,26 @@ int ConfigSerializer::Context::readNext() { case EXPECT_KEY: if (rd_len > 0 && c == ':') { rd_buf[rd_len] = 0; rd_len = 0; rd_mode = EXPECT_VAL_OR_OBJ; return TOK_KEY; } if (rd_len == 0 && is_whitespace(c)) return TOK_WHITESPACE; - if (rd_len < CONFIG_MAX_KEYLEN-1 && is_key_char(c)) { rd_buf[rd_len++] = c; return TOK_WHITESPACE; } + if (rd_len < CONFIG_MAX_KEYLEN-1 && + ((rd_len == 0 && is_key_start_char(c)) || (rd_len > 0 && is_key_char(c)))) { + rd_buf[rd_len++] = c; + return TOK_WHITESPACE; + } return TOK_ERROR; case EXPECT_VAL_OR_OBJ: if (rd_len == 0 && is_whitespace(c)) return TOK_WHITESPACE; - if (rd_len == 0 && c == '"') { rd_mode = EXPECT_STRING_VAL; return TOK_WHITESPACE; } + if (rd_len == 0 && c == '"') { + rd_token_quoted = true; + rd_mode = EXPECT_STRING_VAL; + return TOK_WHITESPACE; + } if (rd_len == 0 && c == '{') { rd_mode = EXPECT_KEY; return TOK_START_OBJ; } - if (is_value_char(c) && rd_len < CONFIG_MAX_TOKEN_LEN-1) { rd_buf[rd_len++] = c; return TOK_WHITESPACE; } + if (is_value_char(c) && rd_len < CONFIG_MAX_TOKEN_LEN-1) { + if (rd_len == 0) rd_token_quoted = false; + rd_buf[rd_len++] = c; + return TOK_WHITESPACE; + } if (rd_len > 0 && (c == ',' || c == '}' || is_whitespace(c))) { pending = c; rd_buf[rd_len] = 0; rd_len = 0; rd_mode = EXPECT_COMMA_OR_CLOSE; return TOK_VALUE; } return TOK_ERROR; @@ -107,9 +126,19 @@ bool ConfigSerializer::loadSerial(Stream& s) { if (next_tok == TOK_KEY) { context.setKey(sp, context.getToken()); } else if (next_tok == TOK_VALUE) { + context.setValueEvent(sp, false); _depth = 1; // re-run the structure() hierarchy again (looking for specific key, at specific depth) structure(); } else if (next_tok == TOK_START_OBJ) { + // The root has no key. For every nested object, rerun the schema at the + // parent depth so strict scalar fields can reject an object value and + // object fields can confirm that their value has the expected shape. + if (sp > 0) { + context.setValueEvent(sp, true); + _depth = 1; + structure(); + if (!context.success) break; + } if (sp < CONFIG_MAX_DEPTH - 1) { sp++; } else { @@ -153,6 +182,10 @@ void ConfigSerializer::def(const char* key, void* value, size_t len) { _context->file()->print("\""); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } memset(value, 0, len); mesh::Utils::fromHex((uint8_t *)value, len, _context->getToken()); } @@ -181,6 +214,10 @@ void ConfigSerializer::def(const char* key, char* value, size_t max_len) { _context->file()->print("\""); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } strncpy(value, _context->getToken(), max_len - 1); value[max_len - 1] = 0; } @@ -195,6 +232,10 @@ void ConfigSerializer::def(const char* key, int32_t& value) { _context->file()->print(value); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atol(_context->getToken()); } } @@ -208,6 +249,10 @@ void ConfigSerializer::def(const char* key, uint32_t& value) { _context->file()->print(value); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atol(_context->getToken()); } } @@ -221,6 +266,10 @@ void ConfigSerializer::def(const char* key, int16_t& value) { _context->file()->print((int32_t) value, 10); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atol(_context->getToken()); } } @@ -234,6 +283,10 @@ void ConfigSerializer::def(const char* key, uint16_t& value) { _context->file()->print((uint32_t) value, 10); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atoi(_context->getToken()); } } @@ -247,6 +300,10 @@ void ConfigSerializer::def(const char* key, uint8_t& value) { _context->file()->print((uint32_t) value, 10); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atoi(_context->getToken()); } } @@ -260,6 +317,10 @@ void ConfigSerializer::def(const char* key, int8_t& value) { _context->file()->print((int32_t) value, 10); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atoi(_context->getToken()); } } @@ -273,6 +334,10 @@ void ConfigSerializer::def(const char* key, bool& value) { _context->file()->print(value ? "true" : "false"); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = strcmp(_context->getToken(), "true") == 0 || atoi(_context->getToken()) != 0; // 'true' or a non-zero number } } @@ -290,6 +355,10 @@ void ConfigSerializer::def(const char* key, double& value) { } } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atof(_context->getToken()); } } @@ -307,6 +376,10 @@ void ConfigSerializer::def(const char* key, float& value) { } } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = (float) atof(_context->getToken()); } } @@ -323,9 +396,64 @@ void ConfigSerializer::def(const char* key, ConfigSerializer& sub_obj) { if (_context->file()->print("}") != 1) _context->success = false; // failure detect } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() && _context->valueDepth() == _depth) { + return; // the object itself; descendant values recurse below + } + if (_context->valueDepth() <= _depth) { + _context->success = false; // known object supplied as a scalar + return; + } sub_obj._context = _context; // inherit the Context sub_obj._depth = _depth + 1; sub_obj.structure(); // recurse into sub object } } } + +bool ConfigSerializer::defStrict(const char* key, char* value, size_t max_len, bool& seen) { + if (_context->op() == OP::WRITE) { + def(key, value, max_len); + return true; + } + if (!_context->keyMatch(_depth, key)) return false; + if (seen || max_len == 0 || _context->objectStart() || + _context->valueDepth() != _depth || !_context->tokenQuoted()) { + _context->success = false; + return false; + } + seen = true; + const char* token = _context->getToken(); + const size_t len = strlen(token); + if (len >= max_len) { + _context->success = false; + return false; + } + memcpy(value, token, len + 1); + return true; +} + +bool ConfigSerializer::defStrict(const char* key, int32_t& value, bool& seen) { + if (_context->op() == OP::WRITE) { + def(key, value); + return true; + } + if (!_context->keyMatch(_depth, key)) return false; + if (seen || _context->objectStart() || _context->valueDepth() != _depth || + _context->tokenQuoted()) { + _context->success = false; + return false; + } + seen = true; + + const char* token = _context->getToken(); + char* end = nullptr; + errno = 0; + const long long parsed = strtoll(token, &end, 10); + if (token[0] == '\0' || end == token || *end != '\0' || errno == ERANGE || + parsed < INT32_MIN || parsed > INT32_MAX) { + _context->success = false; + return false; + } + value = static_cast(parsed); + return true; +} diff --git a/src/helpers/ConfigSerializer.h b/src/helpers/ConfigSerializer.h index 7e6d6f2a..e889e637 100644 --- a/src/helpers/ConfigSerializer.h +++ b/src/helpers/ConfigSerializer.h @@ -25,17 +25,35 @@ class ConfigSerializer { OP _op; uint8_t rd_len; uint8_t rd_mode; + uint8_t rd_value_depth; char pending; + bool rd_token_quoted; + bool rd_object_start; char rd_buf[CONFIG_MAX_TOKEN_LEN]; char _keys[CONFIG_MAX_DEPTH][CONFIG_MAX_KEYLEN]; public: bool success = true; - Context(Stream* f, OP op) : _f(f), _op(op) { rd_buf[rd_len = 0] = 0; rd_mode = 0; pending = 0; } + Context(Stream* f, OP op) : _f(f), _op(op) { + rd_buf[rd_len = 0] = 0; + rd_mode = 0; + rd_value_depth = 0; + pending = 0; + rd_token_quoted = false; + rd_object_start = false; + memset(_keys, 0, sizeof(_keys)); + } OP op() const { return _op; } Stream* file() const { return _f; } int readNext(); const char* getToken() const { return rd_buf; } + bool tokenQuoted() const { return rd_token_quoted; } + uint8_t valueDepth() const { return rd_value_depth; } + bool objectStart() const { return rd_object_start; } + void setValueEvent(uint8_t depth, bool object_start) { + rd_value_depth = depth; + rd_object_start = object_start; + } bool keyMatch(int8_t depth, const char* key) { return strcmp(key, _keys[depth]) == 0; } void setKey(uint8_t depth, const char* key) { strcpy(_keys[depth], key); } }; @@ -60,6 +78,67 @@ protected: void def(const char* key, bool& value); void def(const char* key, ConfigSerializer& sub_obj); + // Strict read helpers for schemas whose values may be edited outside the + // firmware. Unlike the legacy def() overloads, these reject duplicate keys, + // truncated strings, malformed integers, and integer overflow. `seen` must + // be a field owned by the schema object and initialized false before load. + bool defStrict(const char* key, char* value, size_t max_len, bool& seen); + bool defStrict(const char* key, int32_t& value, bool& seen); + + // Literal keys are checked where a schema defines them, so adding a field + // that an older ConfigSerializer cannot tokenize is a build failure rather + // than a silent downgrade trap. Dynamic keys retain the pointer overloads. + template static void checkKey(const char (&)[N]) { + static_assert(N <= CONFIG_MAX_KEYLEN, + "ConfigSerializer key exceeds the visible-key limit"); + } + template void def(const char (&key)[N], char* value, size_t max_len) { + checkKey(key); def(static_cast(key), value, max_len); + } + template void def(const char (&key)[N], void* value, size_t len) { + checkKey(key); def(static_cast(key), value, len); + } + template void def(const char (&key)[N], int32_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], int16_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], int8_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], uint32_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], uint16_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], uint8_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], float& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], double& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], bool& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], ConfigSerializer& sub_obj) { + checkKey(key); def(static_cast(key), sub_obj); + } + template + bool defStrict(const char (&key)[N], char* value, size_t max_len, bool& seen) { + checkKey(key); + return defStrict(static_cast(key), value, max_len, seen); + } + template + bool defStrict(const char (&key)[N], int32_t& value, bool& seen) { + checkKey(key); + return defStrict(static_cast(key), value, seen); + } + virtual void structure() = 0; public: diff --git a/src/helpers/MQTTDefaults.h b/src/helpers/MQTTDefaults.h index 5072e422..21c53bc4 100644 --- a/src/helpers/MQTTDefaults.h +++ b/src/helpers/MQTTDefaults.h @@ -8,7 +8,7 @@ #include "MQTTPacketFilter.h" #include "MQTTPresets.h" -// Compile-time defaults for fresh /mqtt_prefs (override via platformio build_flags). +// Compile-time defaults for fresh /mqtt.json (override via platformio build_flags). // Example: // -D MQTT_DEFAULT_SLOT1_PRESET='"meshcore-ca-1"' // -D MQTT_DEFAULT_IATA='"YYZ"' diff --git a/src/helpers/MQTTObserverValidation.h b/src/helpers/MQTTObserverValidation.h index 2042f226..a8d465a7 100644 --- a/src/helpers/MQTTObserverValidation.h +++ b/src/helpers/MQTTObserverValidation.h @@ -58,3 +58,20 @@ static inline bool mqttNtpHostnameValid(const char* host) { static inline bool mqttValueFits(const char* s, size_t bufsize) { return s != NULL && bufsize > 0 && strlen(s) < bufsize; } + +// Find a preset already assigned to another slot. The caller decides which +// names are singleton presets ("none" and "custom" intentionally are not). +template +static inline int mqttAssignedPresetSlot( + const char (&presets)[SlotCount][PresetSize], + const char* preset_name, + int ignored_slot) { + if (preset_name == NULL) return -1; + for (size_t slot = 0; slot < SlotCount; ++slot) { + if (static_cast(slot) != ignored_slot && + strcmp(presets[slot], preset_name) == 0) { + return static_cast(slot); + } + } + return -1; +} diff --git a/src/helpers/MQTTPacketFilter.h b/src/helpers/MQTTPacketFilter.h index df811828..47fbfe5c 100644 --- a/src/helpers/MQTTPacketFilter.h +++ b/src/helpers/MQTTPacketFilter.h @@ -71,8 +71,8 @@ inline bool resolveToken(const char* begin, size_t len, uint8_t* type_out) { } // Parse an allowlist value. Empty input means "all" so WebConfig can clear a -// field and retain the same backwards-compatible default as an older -// /mqtt_prefs file. Keywords and type names are deliberately lowercase; "all" +// field and retain the same all-packet-types default as an older preference +// file. Keywords and type names are deliberately lowercase; "all" // and "none" cannot be mixed into a list. Entries may be decimal (0..15) or // named, may carry surrounding ASCII whitespace, and may be repeated. inline bool parse(const char* input, uint16_t* mask_out) { @@ -186,8 +186,8 @@ inline uint16_t enabledUnion(const uint16_t* masks, const bool* enabled, size_t return combined; } -// True when every slot still carries the default all-types mask, i.e. nothing -// depends on the packet-filter tail of /mqtt_prefs being written. +// True when every slot still carries the default all-types mask. The legacy +// binary encoder uses this while producing migration fixtures. inline bool allMasksDefault(const uint16_t* masks, size_t count) { if (masks == nullptr) return true; for (size_t i = 0; i < count; ++i) { diff --git a/src/helpers/MQTTPrefsAtomicStore.h b/src/helpers/MQTTPrefsAtomicStore.h index be77bf99..c3b3ce20 100644 --- a/src/helpers/MQTTPrefsAtomicStore.h +++ b/src/helpers/MQTTPrefsAtomicStore.h @@ -3,12 +3,51 @@ #include #include -// Transactional writer for /mqtt_prefs. The Store interface is intentionally -// narrow so host tests can exercise every failure boundary without an Arduino -// filesystem: begin(), write(), finish(), commit(), and abort(). The caller -// supplies header and payload separately, avoiding a second full-size buffer. +// Transactional writer policy for preferences. The Store interface is +// intentionally narrow so host tests can exercise every failure boundary +// without an Arduino filesystem. namespace MQTTPrefsAtomicStore { +// Production /mqtt.json flow. finish() verifies the exact bytes written, +// verify_image reparses the finished temp through the schema, and only then is +// commit() allowed to publish it. A schema-verification failure discards the +// finished temp; a commit failure preserves it for boot recovery. +enum class VerifiedImageResult : uint8_t { + Committed, + BeginFailed, + WriteFailed, + FinishFailed, + VerifyFailed, + CommitFailed, +}; + +template +inline VerifiedImageResult writeVerifiedImage(Store& store, + ImageWriter write_image, + ImageVerifier verify_image) { + if (!store.begin()) { + store.abort(); + return VerifiedImageResult::BeginFailed; + } + if (!write_image()) { + store.abort(); + return VerifiedImageResult::WriteFailed; + } + if (!store.finish()) { + store.abort(); + return VerifiedImageResult::FinishFailed; + } + if (!verify_image()) { + store.discardFinishedTemp(); + return VerifiedImageResult::VerifyFailed; + } + if (!store.commit()) { + store.abort(); + return VerifiedImageResult::CommitFailed; + } + return VerifiedImageResult::Committed; +} + enum class Result : uint8_t { Committed, BeginFailed, @@ -59,7 +98,7 @@ inline ImageResult writeImage(Store& store, ImageWriter write_image) { } // Coordinates a two-file legacy upgrade. /com_prefs must not be compacted -// until the observer tail it carries has been published into /mqtt_prefs. +// until the observer tail it carries has been published into /mqtt.json. // Keeping this state in a tiny pure helper lets host tests cover power-cut // boundaries without an Arduino filesystem. class LegacyUpgradeGate { diff --git a/src/helpers/MQTTPrefsCodec.h b/src/helpers/MQTTPrefsCodec.h index 7bbfe30a..5ac073c0 100644 --- a/src/helpers/MQTTPrefsCodec.h +++ b/src/helpers/MQTTPrefsCodec.h @@ -53,6 +53,9 @@ static const size_t kEncodedSize = sizeof(MQTTPrefsHeader) + kV1BaselinePayloadS // refuses to overwrite the file, so the node cannot be recovered over the air. // Touching any filter opts that node into the longer payload — a deliberate, // operator-initiated trade rather than a side effect of upgrading. +// The encoder below is retained for host migration fixtures and downgrade +// compatibility tests. Production firmware reads legacy binary files but only +// writes the JSON schema. inline size_t payloadLenFor(const MQTTPrefs& prefs) { return MQTTPacketFilter::allMasksDefault(prefs.mqtt_slot_packet_filter, MQTT_PREFS_SLOT_COUNT) @@ -68,13 +71,60 @@ inline MQTTPrefsHeader makeHeader(size_t payload_len) { return header; } +inline void freezeV1(const MQTTPrefs& prefs, LegacyV1MQTTPrefs* frozen) { + if (frozen == nullptr) return; + memset(frozen, 0, sizeof(*frozen)); + memcpy(frozen->mqtt_origin, prefs.mqtt_origin, sizeof(frozen->mqtt_origin)); + memcpy(frozen->mqtt_iata, prefs.mqtt_iata, sizeof(frozen->mqtt_iata)); + frozen->mqtt_status_enabled = prefs.mqtt_status_enabled; + frozen->mqtt_packets_enabled = prefs.mqtt_packets_enabled; + frozen->mqtt_raw_enabled = prefs.mqtt_raw_enabled; + frozen->mqtt_tx_enabled = prefs.mqtt_tx_enabled; + frozen->mqtt_status_interval = prefs.mqtt_status_interval; + memcpy(frozen->wifi_ssid, prefs.wifi_ssid, sizeof(frozen->wifi_ssid)); + memcpy(frozen->wifi_password, prefs.wifi_password, sizeof(frozen->wifi_password)); + frozen->wifi_power_save = prefs.wifi_power_save; + memcpy(frozen->timezone_string, prefs.timezone_string, sizeof(frozen->timezone_string)); + frozen->timezone_offset = prefs.timezone_offset; + memcpy(frozen->mqtt_slot_preset, prefs.mqtt_slot_preset, sizeof(frozen->mqtt_slot_preset)); + memcpy(frozen->mqtt_slot_host, prefs.mqtt_slot_host, sizeof(frozen->mqtt_slot_host)); + memcpy(frozen->mqtt_slot_port, prefs.mqtt_slot_port, sizeof(frozen->mqtt_slot_port)); + memcpy(frozen->mqtt_slot_username, prefs.mqtt_slot_username, sizeof(frozen->mqtt_slot_username)); + memcpy(frozen->mqtt_slot_password, prefs.mqtt_slot_password, sizeof(frozen->mqtt_slot_password)); + memcpy(frozen->mqtt_owner_public_key, prefs.mqtt_owner_public_key, + sizeof(frozen->mqtt_owner_public_key)); + memcpy(frozen->mqtt_email, prefs.mqtt_email, sizeof(frozen->mqtt_email)); + memcpy(frozen->mqtt_slot_token, prefs.mqtt_slot_token, sizeof(frozen->mqtt_slot_token)); + memcpy(frozen->mqtt_slot_topic, prefs.mqtt_slot_topic, sizeof(frozen->mqtt_slot_topic)); + memcpy(frozen->mqtt_slot_audience, prefs.mqtt_slot_audience, + sizeof(frozen->mqtt_slot_audience)); + frozen->mqtt_rx_enabled = prefs.mqtt_rx_enabled; + memcpy(frozen->mqtt_ntp_server, prefs.mqtt_ntp_server, sizeof(frozen->mqtt_ntp_server)); + frozen->snmp_enabled = prefs.snmp_enabled; + memcpy(frozen->snmp_community, prefs.snmp_community, sizeof(frozen->snmp_community)); + frozen->radio_watchdog_minutes = prefs.radio_watchdog_minutes; + frozen->alert_enabled = prefs.alert_enabled; + memcpy(frozen->alert_psk_hex, prefs.alert_psk_hex, sizeof(frozen->alert_psk_hex)); + frozen->alert_wifi_minutes = prefs.alert_wifi_minutes; + frozen->alert_mqtt_minutes = prefs.alert_mqtt_minutes; + frozen->alert_min_interval_min = prefs.alert_min_interval_min; + memcpy(frozen->alert_hashtag, prefs.alert_hashtag, sizeof(frozen->alert_hashtag)); + memcpy(frozen->alert_region, prefs.alert_region, sizeof(frozen->alert_region)); + frozen->mqtt_neighbors_enabled = prefs.mqtt_neighbors_enabled; + frozen->mqtt_neighbors_interval = prefs.mqtt_neighbors_interval; + memcpy(frozen->mqtt_slot_packet_filter, prefs.mqtt_slot_packet_filter, + sizeof(frozen->mqtt_slot_packet_filter)); +} + inline size_t encode(const MQTTPrefs& prefs, uint8_t* output, size_t output_size) { const size_t payload_len = payloadLenFor(prefs); const size_t encoded_size = sizeof(MQTTPrefsHeader) + payload_len; if (output == nullptr || output_size < encoded_size) return 0; const MQTTPrefsHeader header = makeHeader(payload_len); + LegacyV1MQTTPrefs frozen; + freezeV1(prefs, &frozen); memcpy(output, &header, sizeof(header)); - memcpy(output + sizeof(header), &prefs, payload_len); + memcpy(output + sizeof(header), &frozen, payload_len); return encoded_size; } @@ -346,6 +396,120 @@ inline bool isPlausibleLegacy(Source source, const uint8_t* input, size_t size) } } +// A valid v1 header proves the intended layout, not that a torn or corrupted +// payload still contains bounded C strings. Validate every field present at a +// shipped v1 boundary before copying it into the runtime object; otherwise the +// later semantic validators and JSON writer could scan beyond a fixed array. +inline bool isPlausibleV1(const LegacyV1MQTTPrefs& prefs, size_t payload_len) { + if (payload_len != kV1PreObserverPayloadSize && + payload_len != kV1PreNeighborsPayloadSize && + payload_len != kV1PreFilterPayloadSize && + payload_len != kV1BaselinePayloadSize) { + return false; + } + const uint8_t* input = reinterpret_cast(&prefs); + // A v1 header already identifies the layout. At this stage only reject + // fields that could make later C-string validation/serialization read past a + // fixed array; numeric values are safely repaired by MQTTPrefsSerializer. + if (!hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, mqtt_origin), sizeof(prefs.mqtt_origin)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, mqtt_iata), sizeof(prefs.mqtt_iata)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, wifi_ssid), sizeof(prefs.wifi_ssid)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, wifi_password), sizeof(prefs.wifi_password)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, timezone_string), sizeof(prefs.timezone_string)) || + !hasPlausibleSharedAuth(input, payload_len, + offsetof(LegacyV1MQTTPrefs, mqtt_owner_public_key), + offsetof(LegacyV1MQTTPrefs, mqtt_email)) || + !hasPlausibleSlotText(input, payload_len, MQTT_PREFS_SLOT_COUNT, + offsetof(LegacyV1MQTTPrefs, mqtt_slot_preset), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_host), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_username), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_password), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_token), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_topic), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_audience)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, mqtt_ntp_server), + sizeof(prefs.mqtt_ntp_server))) { + return false; + } + if (payload_len >= kV1PreNeighborsPayloadSize) { + if (!hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, snmp_community), + sizeof(prefs.snmp_community)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, alert_psk_hex), + sizeof(prefs.alert_psk_hex)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, alert_hashtag), + sizeof(prefs.alert_hashtag)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, alert_region), + sizeof(prefs.alert_region))) { + return false; + } + } + return true; +} + +inline void migrateV1(const LegacyV1MQTTPrefs& old_prefs, size_t payload_len, + MQTTPrefs* prefs) { + if (prefs == nullptr || payload_len < kV1PreObserverPayloadSize) return; + memcpy(prefs->mqtt_origin, old_prefs.mqtt_origin, sizeof(prefs->mqtt_origin)); + memcpy(prefs->mqtt_iata, old_prefs.mqtt_iata, sizeof(prefs->mqtt_iata)); + prefs->mqtt_status_enabled = old_prefs.mqtt_status_enabled; + prefs->mqtt_packets_enabled = old_prefs.mqtt_packets_enabled; + prefs->mqtt_raw_enabled = old_prefs.mqtt_raw_enabled; + prefs->mqtt_tx_enabled = old_prefs.mqtt_tx_enabled; + prefs->mqtt_status_interval = old_prefs.mqtt_status_interval; + memcpy(prefs->wifi_ssid, old_prefs.wifi_ssid, sizeof(prefs->wifi_ssid)); + memcpy(prefs->wifi_password, old_prefs.wifi_password, sizeof(prefs->wifi_password)); + prefs->wifi_power_save = old_prefs.wifi_power_save; + memcpy(prefs->timezone_string, old_prefs.timezone_string, sizeof(prefs->timezone_string)); + prefs->timezone_offset = old_prefs.timezone_offset; + memcpy(prefs->mqtt_slot_preset, old_prefs.mqtt_slot_preset, sizeof(prefs->mqtt_slot_preset)); + memcpy(prefs->mqtt_slot_host, old_prefs.mqtt_slot_host, sizeof(prefs->mqtt_slot_host)); + memcpy(prefs->mqtt_slot_port, old_prefs.mqtt_slot_port, sizeof(prefs->mqtt_slot_port)); + memcpy(prefs->mqtt_slot_username, old_prefs.mqtt_slot_username, sizeof(prefs->mqtt_slot_username)); + memcpy(prefs->mqtt_slot_password, old_prefs.mqtt_slot_password, sizeof(prefs->mqtt_slot_password)); + memcpy(prefs->mqtt_owner_public_key, old_prefs.mqtt_owner_public_key, + sizeof(prefs->mqtt_owner_public_key)); + memcpy(prefs->mqtt_email, old_prefs.mqtt_email, sizeof(prefs->mqtt_email)); + memcpy(prefs->mqtt_slot_token, old_prefs.mqtt_slot_token, sizeof(prefs->mqtt_slot_token)); + memcpy(prefs->mqtt_slot_topic, old_prefs.mqtt_slot_topic, sizeof(prefs->mqtt_slot_topic)); + memcpy(prefs->mqtt_slot_audience, old_prefs.mqtt_slot_audience, + sizeof(prefs->mqtt_slot_audience)); + prefs->mqtt_rx_enabled = old_prefs.mqtt_rx_enabled; + memcpy(prefs->mqtt_ntp_server, old_prefs.mqtt_ntp_server, sizeof(prefs->mqtt_ntp_server)); + + if (payload_len >= kV1PreNeighborsPayloadSize) { + prefs->snmp_enabled = old_prefs.snmp_enabled; + memcpy(prefs->snmp_community, old_prefs.snmp_community, sizeof(prefs->snmp_community)); + prefs->radio_watchdog_minutes = old_prefs.radio_watchdog_minutes; + prefs->alert_enabled = old_prefs.alert_enabled; + memcpy(prefs->alert_psk_hex, old_prefs.alert_psk_hex, sizeof(prefs->alert_psk_hex)); + prefs->alert_wifi_minutes = old_prefs.alert_wifi_minutes; + prefs->alert_mqtt_minutes = old_prefs.alert_mqtt_minutes; + prefs->alert_min_interval_min = old_prefs.alert_min_interval_min; + memcpy(prefs->alert_hashtag, old_prefs.alert_hashtag, sizeof(prefs->alert_hashtag)); + memcpy(prefs->alert_region, old_prefs.alert_region, sizeof(prefs->alert_region)); + // The pre-neighbors payload includes the old zero padding byte at this + // offset; copying it preserves the deployed format's disabled default. + prefs->mqtt_neighbors_enabled = old_prefs.mqtt_neighbors_enabled; + } + if (payload_len >= kV1PreFilterPayloadSize) { + prefs->mqtt_neighbors_interval = old_prefs.mqtt_neighbors_interval; + } + if (payload_len >= kV1BaselinePayloadSize) { + memcpy(prefs->mqtt_slot_packet_filter, old_prefs.mqtt_slot_packet_filter, + sizeof(prefs->mqtt_slot_packet_filter)); + } +} + inline void migratePreSlot(const OldMQTTPrefs& old_prefs, MQTTPrefs* prefs) { memcpy(prefs->mqtt_origin, old_prefs.mqtt_origin, sizeof(prefs->mqtt_origin)); memcpy(prefs->mqtt_iata, old_prefs.mqtt_iata, sizeof(prefs->mqtt_iata)); diff --git a/src/helpers/MQTTPrefsRecovery.h b/src/helpers/MQTTPrefsRecovery.h index 7534678e..37073209 100644 --- a/src/helpers/MQTTPrefsRecovery.h +++ b/src/helpers/MQTTPrefsRecovery.h @@ -2,23 +2,29 @@ #include -// Pure recovery policy for the three MQTT preference transaction files. The +// Pure recovery policy for the three MQTT preference transaction files. The // writer first moves the old primary to .bak, then moves the verified .tmp to -// the primary name. On a reset, the loader uses this policy before decoding -// /mqtt_prefs. "Preserve" is deliberately distinct from "Usable": it covers -// an unsupported newer layout, corruption, or an unreadable file and must -// never be replaced by an older image. +// the primary name. On a reset, the loader uses this policy before decoding +// the primary. FutureUsable is syntactically valid but belongs to newer +// firmware. FutureClaimed and Indeterminate cannot be safely classified by +// this firmware, so recovery may use a known-good backup but must retain the +// uncertain image and hold further writes. Preserve is definitively invalid or +// unsupported and may be discarded only where the transaction policy permits. namespace MQTTPrefsRecovery { enum class FileState : uint8_t { Missing, Usable, + FutureUsable, + FutureClaimed, + Indeterminate, Preserve, }; enum class Action : uint8_t { None, KeepPrimary, + DiscardTemp, PromoteTemp, PromoteBackup, }; @@ -30,13 +36,23 @@ inline Action select(FileState primary, FileState temp, FileState backup) { if (primary != FileState::Missing) return Action::KeepPrimary; // A completed temp is the new image and wins over the old backup. - if (temp == FileState::Usable) return Action::PromoteTemp; + if (temp == FileState::Usable || temp == FileState::FutureUsable) { + return Action::PromoteTemp; + } - // If temp is opaque but a known-good backup exists, boot from the backup. - // The caller may discard the opaque temp once that usable backup has become - // primary. Otherwise, rename the opaque temp into the empty primary name so - // the normal loader can hold it. + // Preserve is definitively invalid, so it was never a committed image. If + // no backup exists, discard the interrupted temp and leave the primary name + // absent; this is what lets a first JSON migration retry from /mqtt_prefs. + // If a backup exists, it is the prior committed image and wins regardless + // of whether this firmware understands its schema. if (temp == FileState::Preserve) { + return backup == FileState::Missing ? Action::DiscardTemp : Action::PromoteBackup; + } + + // FutureClaimed and Indeterminate may be a completed image this firmware + // cannot classify. A known-good backup can run this boot, but without one + // preserve the only candidate under the authoritative name and hold writes. + if (temp == FileState::FutureClaimed || temp == FileState::Indeterminate) { return backup == FileState::Usable ? Action::PromoteBackup : Action::PromoteTemp; } @@ -46,4 +62,8 @@ inline Action select(FileState primary, FileState temp, FileState backup) { return Action::None; } +inline bool uncertain(FileState state) { + return state == FileState::FutureClaimed || state == FileState::Indeterminate; +} + } // namespace MQTTPrefsRecovery diff --git a/src/helpers/MQTTPrefsSerializer.h b/src/helpers/MQTTPrefsSerializer.h new file mode 100644 index 00000000..b4d12b18 --- /dev/null +++ b/src/helpers/MQTTPrefsSerializer.h @@ -0,0 +1,412 @@ +#pragma once + +#include +#include +#include +#include + +#ifdef WITH_MQTT_BRIDGE + +static const int32_t MQTT_PREFS_JSON_FORMAT_VERSION = 1; + +// Reads only the mandatory root version while ignoring every other schema +// field. Recovery uses this before the v1 decoder so a syntactically valid +// future file remains opaque even when that schema changes a v1 field's type. +class MQTTPrefsVersionProbe : public ConfigSerializer { + int32_t _version = 0; + bool _seen_version = false; +protected: + void structure() override { defStrict("version", _version, _seen_version); } +public: + bool hasFutureVersion() const { + return _seen_version && _version > MQTT_PREFS_JSON_FORMAT_VERSION; + } +}; + +// ConfigSerializer adapter for the observer preference POD. Keeping the +// serializer separate avoids making the runtime object layout part of the JSON +// format and adds no permanent per-slot serializer state to CommonCLI. +class MQTTPrefsSerializer : public ConfigSerializer { + + class WifiPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _power_save; + bool _seen_ssid = false, _seen_password = false, _seen_power_save = false; + protected: + void structure() override { + defStrict("ssid", _prefs->wifi_ssid, sizeof(_prefs->wifi_ssid), _seen_ssid); + defStrict("password", _prefs->wifi_password, sizeof(_prefs->wifi_password), _seen_password); + defStrict("power_save", _power_save, _seen_power_save); + } + public: + explicit WifiPrefs(MQTTPrefs* prefs) : _prefs(prefs), _power_save(prefs->wifi_power_save) {} + bool apply(bool* repaired) { + if (_power_save < 0 || _power_save > 2) { + _power_save = 1; + *repaired = true; + } + _prefs->wifi_power_save = static_cast(_power_save); + return true; + } + }; + + class TimePrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _utc_offset, _default_utc_offset; + char _default_ntp_server[sizeof(MQTTPrefs::mqtt_ntp_server)]; + bool _seen_timezone = false, _seen_utc_offset = false, _seen_ntp_server = false; + protected: + void structure() override { + defStrict("timezone", _prefs->timezone_string, sizeof(_prefs->timezone_string), _seen_timezone); + defStrict("utc_offset", _utc_offset, _seen_utc_offset); + defStrict("ntp_server", _prefs->mqtt_ntp_server, sizeof(_prefs->mqtt_ntp_server), _seen_ntp_server); + } + public: + TimePrefs(MQTTPrefs* prefs, const MQTTPrefs* defaults) + : _prefs(prefs), _utc_offset(prefs->timezone_offset), + _default_utc_offset(defaults->timezone_offset) { + if (_default_utc_offset < -12 || _default_utc_offset > 14) { + _default_utc_offset = 0; + } + memcpy(_default_ntp_server, defaults->mqtt_ntp_server, sizeof(_default_ntp_server)); + if (_default_ntp_server[0] != '\0' && + !mqttNtpHostnameValid(_default_ntp_server)) { + _default_ntp_server[0] = '\0'; + } + } + bool apply(bool* repaired) { + if (_utc_offset < -12 || _utc_offset > 14) { + _utc_offset = _default_utc_offset; + *repaired = true; + } + if (_prefs->mqtt_ntp_server[0] != '\0' && + !mqttNtpHostnameValid(_prefs->mqtt_ntp_server)) { + memcpy(_prefs->mqtt_ntp_server, _default_ntp_server, sizeof(_default_ntp_server)); + *repaired = true; + } + _prefs->timezone_offset = static_cast(_utc_offset); + return true; + } + }; + + class StatusPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _enabled, _interval_ms; + bool _seen_enabled = false, _seen_interval = false; + protected: + void structure() override { + defStrict("enabled", _enabled, _seen_enabled); + defStrict("interval_ms", _interval_ms, _seen_interval); + } + public: + explicit StatusPrefs(MQTTPrefs* prefs) + : _prefs(prefs), _enabled(prefs->mqtt_status_enabled), + _interval_ms(static_cast(prefs->mqtt_status_interval)) {} + void apply(bool* repaired) { + if (_enabled < 0 || _enabled > 1) { _enabled = 1; *repaired = true; } + if (_interval_ms < 60000 || _interval_ms > 3600000) { + _interval_ms = 300000; + *repaired = true; + } + _prefs->mqtt_status_enabled = static_cast(_enabled); + _prefs->mqtt_status_interval = static_cast(_interval_ms); + } + }; + + class NeighborPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _enabled, _interval_ms; + bool _seen_enabled = false, _seen_interval = false; + protected: + void structure() override { + defStrict("enabled", _enabled, _seen_enabled); + defStrict("interval_ms", _interval_ms, _seen_interval); + } + public: + explicit NeighborPrefs(MQTTPrefs* prefs) + : _prefs(prefs), _enabled(prefs->mqtt_neighbors_enabled), + _interval_ms(static_cast(prefs->mqtt_neighbors_interval)) {} + void apply(bool* repaired) { + if (_enabled < 0 || _enabled > 1) { _enabled = 0; *repaired = true; } + if (_interval_ms < static_cast(MQTT_NEIGHBORS_MIN_INTERVAL_MS) || + _interval_ms > static_cast(MQTT_NEIGHBORS_MAX_INTERVAL_MS)) { + _interval_ms = static_cast(MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS); + *repaired = true; + } + _prefs->mqtt_neighbors_enabled = static_cast(_enabled); + _prefs->mqtt_neighbors_interval = static_cast(_interval_ms); + } + }; + + class OwnerPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + bool _seen_public_key = false, _seen_email = false; + protected: + void structure() override { + defStrict("public_key", _prefs->mqtt_owner_public_key, + sizeof(_prefs->mqtt_owner_public_key), _seen_public_key); + defStrict("email", _prefs->mqtt_email, sizeof(_prefs->mqtt_email), _seen_email); + } + public: + explicit OwnerPrefs(MQTTPrefs* prefs) : _prefs(prefs) {} + void apply(bool* repaired) { + if (_prefs->mqtt_owner_public_key[0] != '\0' && + !mqttOwnerKeyValid(_prefs->mqtt_owner_public_key)) { + _prefs->mqtt_owner_public_key[0] = '\0'; + *repaired = true; + } + } + }; + + class SlotPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int _index; + int32_t _port, _filter; + bool _seen_preset = false, _seen_host = false, _seen_port = false; + bool _seen_username = false, _seen_password = false, _seen_token = false; + bool _seen_topic = false, _seen_audience = false, _seen_filter = false; + protected: + void structure() override { + defStrict("preset", _prefs->mqtt_slot_preset[_index], + sizeof(_prefs->mqtt_slot_preset[_index]), _seen_preset); + defStrict("host", _prefs->mqtt_slot_host[_index], + sizeof(_prefs->mqtt_slot_host[_index]), _seen_host); + defStrict("port", _port, _seen_port); + defStrict("username", _prefs->mqtt_slot_username[_index], + sizeof(_prefs->mqtt_slot_username[_index]), _seen_username); + defStrict("password", _prefs->mqtt_slot_password[_index], + sizeof(_prefs->mqtt_slot_password[_index]), _seen_password); + defStrict("token", _prefs->mqtt_slot_token[_index], + sizeof(_prefs->mqtt_slot_token[_index]), _seen_token); + defStrict("topic", _prefs->mqtt_slot_topic[_index], + sizeof(_prefs->mqtt_slot_topic[_index]), _seen_topic); + defStrict("audience", _prefs->mqtt_slot_audience[_index], + sizeof(_prefs->mqtt_slot_audience[_index]), _seen_audience); + defStrict("packet_filter", _filter, _seen_filter); + } + public: + SlotPrefs(MQTTPrefs* prefs, int index) + : _prefs(prefs), _index(index), _port(prefs->mqtt_slot_port[index]), + _filter(prefs->mqtt_slot_packet_filter[index]) {} + void apply(bool* repaired) { + if (_port < 0 || _port > 65535) { _port = 0; *repaired = true; } + if (_filter < 0 || _filter > 65535) { _filter = 0xffff; *repaired = true; } + const char* preset = _prefs->mqtt_slot_preset[_index]; + if (strcmp(preset, MQTT_PRESET_NONE) != 0 && + strcmp(preset, MQTT_PRESET_CUSTOM) != 0 && findMQTTPreset(preset) == nullptr) { + strcpy(_prefs->mqtt_slot_preset[_index], MQTT_PRESET_NONE); + *repaired = true; + } + _prefs->mqtt_slot_port[_index] = static_cast(_port); + _prefs->mqtt_slot_packet_filter[_index] = static_cast(_filter); + } + }; + + class MqttPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _packets_enabled, _raw_enabled, _tx_enabled, _rx_enabled; + char _default_iata[sizeof(MQTTPrefs::mqtt_iata)]; + bool _seen_origin = false, _seen_iata = false, _seen_packets = false; + bool _seen_raw = false, _seen_tx = false, _seen_rx = false; + StatusPrefs _status; + NeighborPrefs _neighbors; + OwnerPrefs _owner; + static_assert(MQTT_PREFS_SLOT_COUNT == 6, + "MQTTPrefsSerializer slot members and keys must be updated"); + SlotPrefs _slot1, _slot2, _slot3, _slot4, _slot5, _slot6; + protected: + void structure() override { + defStrict("origin", _prefs->mqtt_origin, sizeof(_prefs->mqtt_origin), _seen_origin); + defStrict("iata", _prefs->mqtt_iata, sizeof(_prefs->mqtt_iata), _seen_iata); + defStrict("packets_enabled", _packets_enabled, _seen_packets); + defStrict("raw_enabled", _raw_enabled, _seen_raw); + defStrict("tx_enabled", _tx_enabled, _seen_tx); + defStrict("rx_enabled", _rx_enabled, _seen_rx); + def("status", _status); + def("neighbors", _neighbors); + def("owner", _owner); + def("slot1", _slot1); + def("slot2", _slot2); + def("slot3", _slot3); + def("slot4", _slot4); + def("slot5", _slot5); + def("slot6", _slot6); + } + public: + MqttPrefs(MQTTPrefs* prefs, const MQTTPrefs* defaults) + : _prefs(prefs), _packets_enabled(prefs->mqtt_packets_enabled), + _raw_enabled(prefs->mqtt_raw_enabled), _tx_enabled(prefs->mqtt_tx_enabled), + _rx_enabled(prefs->mqtt_rx_enabled), _status(prefs), _neighbors(prefs), + _owner(prefs), _slot1(prefs, 0), _slot2(prefs, 1), + _slot3(prefs, 2), _slot4(prefs, 3), + _slot5(prefs, 4), _slot6(prefs, 5) { + memcpy(_default_iata, defaults->mqtt_iata, sizeof(_default_iata)); + if (_default_iata[0] != '\0' && !mqttIataValid(_default_iata)) { + _default_iata[0] = '\0'; + } + for (char* p = _default_iata; *p; ++p) { + if (*p >= 'a' && *p <= 'z') *p = static_cast(*p - ('a' - 'A')); + } + } + void apply(bool* repaired) { + if (_packets_enabled < 0 || _packets_enabled > 1) { _packets_enabled = 1; *repaired = true; } + if (_raw_enabled < 0 || _raw_enabled > 1) { _raw_enabled = 0; *repaired = true; } + if (_tx_enabled < 0 || _tx_enabled > 2) { _tx_enabled = 2; *repaired = true; } + if (_rx_enabled < 0 || _rx_enabled > 1) { _rx_enabled = 1; *repaired = true; } + _prefs->mqtt_packets_enabled = static_cast(_packets_enabled); + _prefs->mqtt_raw_enabled = static_cast(_raw_enabled); + _prefs->mqtt_tx_enabled = static_cast(_tx_enabled); + _prefs->mqtt_rx_enabled = static_cast(_rx_enabled); + if (_prefs->mqtt_iata[0] != '\0') { + if (!mqttIataValid(_prefs->mqtt_iata)) { + memcpy(_prefs->mqtt_iata, _default_iata, sizeof(_default_iata)); + *repaired = true; + } else { + for (char* p = _prefs->mqtt_iata; *p; ++p) { + if (*p >= 'a' && *p <= 'z') { + *p = static_cast(*p - ('a' - 'A')); + *repaired = true; + } + } + } + } + _status.apply(repaired); + _neighbors.apply(repaired); + _owner.apply(repaired); + _slot1.apply(repaired); _slot2.apply(repaired); _slot3.apply(repaired); + _slot4.apply(repaired); _slot5.apply(repaired); _slot6.apply(repaired); + } + }; + + class SnmpPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _enabled; + bool _seen_enabled = false, _seen_community = false; + protected: + void structure() override { + defStrict("enabled", _enabled, _seen_enabled); + defStrict("community", _prefs->snmp_community, + sizeof(_prefs->snmp_community), _seen_community); + } + public: + explicit SnmpPrefs(MQTTPrefs* prefs) : _prefs(prefs), _enabled(prefs->snmp_enabled) {} + void apply(bool* repaired) { + if (_enabled < 0 || _enabled > 1) { _enabled = 0; *repaired = true; } + _prefs->snmp_enabled = static_cast(_enabled); + } + }; + + class RadioPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _watchdog_min; + bool _seen_watchdog = false; + protected: + void structure() override { defStrict("watchdog_min", _watchdog_min, _seen_watchdog); } + public: + explicit RadioPrefs(MQTTPrefs* prefs) + : _prefs(prefs), _watchdog_min(prefs->radio_watchdog_minutes) {} + void apply(bool* repaired) { + if (_watchdog_min < 0 || _watchdog_min > 120) { _watchdog_min = 5; *repaired = true; } + _prefs->radio_watchdog_minutes = static_cast(_watchdog_min); + } + }; + + class AlertPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _enabled, _wifi_minutes, _mqtt_minutes, _rate_limit_min; + bool _seen_enabled = false, _seen_psk = false, _seen_wifi = false; + bool _seen_mqtt = false, _seen_rate = false, _seen_hashtag = false, _seen_region = false; + protected: + void structure() override { + defStrict("enabled", _enabled, _seen_enabled); + defStrict("psk_hex", _prefs->alert_psk_hex, sizeof(_prefs->alert_psk_hex), _seen_psk); + defStrict("wifi_minutes", _wifi_minutes, _seen_wifi); + defStrict("mqtt_minutes", _mqtt_minutes, _seen_mqtt); + defStrict("rate_limit_min", _rate_limit_min, _seen_rate); + defStrict("hashtag", _prefs->alert_hashtag, sizeof(_prefs->alert_hashtag), _seen_hashtag); + defStrict("region", _prefs->alert_region, sizeof(_prefs->alert_region), _seen_region); + } + public: + explicit AlertPrefs(MQTTPrefs* prefs) + : _prefs(prefs), _enabled(prefs->alert_enabled), + _wifi_minutes(prefs->alert_wifi_minutes), _mqtt_minutes(prefs->alert_mqtt_minutes), + _rate_limit_min(prefs->alert_min_interval_min) {} + void apply(bool* repaired) { + if (_enabled < 0 || _enabled > 1) { _enabled = 0; *repaired = true; } + if (_wifi_minutes < 0 || _wifi_minutes > 1440) { _wifi_minutes = 30; *repaired = true; } + if (_mqtt_minutes < 0 || _mqtt_minutes > 10080) { _mqtt_minutes = 240; *repaired = true; } + if (_rate_limit_min < 60 || _rate_limit_min > 10080) { _rate_limit_min = 60; *repaired = true; } + if (_prefs->alert_psk_hex[0] != '\0') { + const size_t len = strlen(_prefs->alert_psk_hex); + bool valid_hex = len == 32; + for (size_t i = 0; valid_hex && i < len; ++i) { + const char c = _prefs->alert_psk_hex[i]; + valid_hex = (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || + (c >= 'a' && c <= 'f'); + } + if (!valid_hex) { + _prefs->alert_psk_hex[0] = '\0'; + _prefs->alert_hashtag[0] = '\0'; + *repaired = true; + } + } + _prefs->alert_enabled = static_cast(_enabled); + _prefs->alert_wifi_minutes = static_cast(_wifi_minutes); + _prefs->alert_mqtt_minutes = static_cast(_mqtt_minutes); + _prefs->alert_min_interval_min = static_cast(_rate_limit_min); + } + }; + + MQTTPrefs* _prefs; + int32_t _version = MQTT_PREFS_JSON_FORMAT_VERSION; + bool _seen_version = false; + WifiPrefs _wifi; + TimePrefs _time; + MqttPrefs _mqtt; + SnmpPrefs _snmp; + RadioPrefs _radio; + AlertPrefs _alert; + +protected: + void structure() override { + defStrict("version", _version, _seen_version); + def("wifi", _wifi); + def("time", _time); + def("mqtt", _mqtt); + def("snmp", _snmp); + def("radio", _radio); + def("alert", _alert); + } + +public: + explicit MQTTPrefsSerializer(MQTTPrefs* prefs, const MQTTPrefs* repair_defaults = nullptr) + : _prefs(prefs), _wifi(prefs), + _time(prefs, repair_defaults ? repair_defaults : prefs), + _mqtt(prefs, repair_defaults ? repair_defaults : prefs), _snmp(prefs), + _radio(prefs), _alert(prefs) {} + + bool hasSupportedVersion() const { + return _seen_version && _version == MQTT_PREFS_JSON_FORMAT_VERSION; + } + bool hasFutureVersion() const { + return _seen_version && _version > MQTT_PREFS_JSON_FORMAT_VERSION; + } + + bool normalize(bool* repaired) { + if (repaired == nullptr) return false; + *repaired = false; + _wifi.apply(repaired); + _time.apply(repaired); + _mqtt.apply(repaired); + _snmp.apply(repaired); + _radio.apply(repaired); + _alert.apply(repaired); + return true; + } + + bool apply(bool* repaired) { + return hasSupportedVersion() && normalize(repaired); + } +}; + +#endif // WITH_MQTT_BRIDGE diff --git a/src/helpers/MQTTPrefsStorage.h b/src/helpers/MQTTPrefsStorage.h index e3c6a098..0bc0867e 100644 --- a/src/helpers/MQTTPrefsStorage.h +++ b/src/helpers/MQTTPrefsStorage.h @@ -62,10 +62,9 @@ struct PreWifiPowerOldMQTTPrefs { char mqtt_email[64]; }; -// MQTT preferences stored separately from NodePrefs to avoid upstream layout -// conflicts. The full layout is the frozen v1 payload baseline. The prefix -// before observer settings is also an explicitly supported v1 payload: it was -// used before the observer fields were appended. +// Runtime MQTT preferences, stored separately from NodePrefs to avoid upstream +// layout conflicts. JSON persistence is field-based; this type is deliberately +// free to evolve independently of the frozen binary migration layouts below. struct MQTTPrefs { char mqtt_origin[32]; char mqtt_iata[8]; @@ -110,17 +109,57 @@ struct MQTTPrefs { char alert_region[31]; // Neighbors publishing (PSRAM boards only). Appended at the end of the - // observer tail so a shorter (pre-neighbors) /mqtt_prefs payload from earlier - // firmware still loads with these defaulting off/24h; keeps the format at - // VERSION 1. Field order and sizes are kept byte-identical to the flex - // neighbors build so a /mqtt_prefs written by either firmware is - // interchangeable (see the offsetof static_asserts below). + // observer tail in the former binary format. Binary compatibility is now + // represented only by LegacyV1MQTTPrefs below. uint8_t mqtt_neighbors_enabled; uint32_t mqtt_neighbors_interval; // Per-slot payload-type allow masks. Bit N controls MeshCore packet type N - // for both packets and raw MQTT topics. Appended so older v1 payloads load - // with the default all-types masks intact. + // for both packets and raw MQTT topics. + uint16_t mqtt_slot_packet_filter[MQTT_PREFS_SLOT_COUNT]; +}; + +// Frozen payload written by the version-1 binary format. Keep this distinct +// from the runtime MQTTPrefs type: JSON persistence must not make the runtime +// object's padding or member order an on-flash ABI again. Legacy decoding reads +// into this type and field-copies into a defaulted runtime object. +struct LegacyV1MQTTPrefs { + char mqtt_origin[32]; + char mqtt_iata[8]; + uint8_t mqtt_status_enabled; + uint8_t mqtt_packets_enabled; + uint8_t mqtt_raw_enabled; + uint8_t mqtt_tx_enabled; + uint32_t mqtt_status_interval; + char wifi_ssid[32]; + char wifi_password[64]; + uint8_t wifi_power_save; + char timezone_string[32]; + int8_t timezone_offset; + char mqtt_slot_preset[MQTT_PREFS_SLOT_COUNT][24]; + char mqtt_slot_host[MQTT_PREFS_SLOT_COUNT][64]; + uint16_t mqtt_slot_port[MQTT_PREFS_SLOT_COUNT]; + char mqtt_slot_username[MQTT_PREFS_SLOT_COUNT][32]; + char mqtt_slot_password[MQTT_PREFS_SLOT_COUNT][64]; + char mqtt_owner_public_key[65]; + char mqtt_email[64]; + char mqtt_slot_token[MQTT_PREFS_SLOT_COUNT][48]; + char mqtt_slot_topic[MQTT_PREFS_SLOT_COUNT][96]; + char mqtt_slot_audience[MQTT_PREFS_SLOT_COUNT][64]; + uint8_t mqtt_rx_enabled; + char mqtt_ntp_server[64]; + uint8_t snmp_enabled; + char snmp_community[24]; + uint8_t radio_watchdog_minutes; + uint8_t alert_enabled; + char alert_psk_hex[33]; + uint16_t alert_wifi_minutes; + uint16_t alert_mqtt_minutes; + uint16_t alert_min_interval_min; + char alert_hashtag[24]; + char alert_region[31]; + uint8_t mqtt_neighbors_enabled; + uint32_t mqtt_neighbors_interval; uint16_t mqtt_slot_packet_filter[MQTT_PREFS_SLOT_COUNT]; }; @@ -272,20 +311,20 @@ static const size_t LEGACY6_AUDIENCE_RX_SIZE = 2840; // Frozen on-flash layouts; every firmware and native fixture build checks them. static_assert(sizeof(MQTTPrefsHeader) == 8, "versioned /mqtt_prefs header must stay 8 bytes"); -static_assert(offsetof(MQTTPrefs, snmp_enabled) == MQTT_PREFS_V1_PRE_OBSERVER_PAYLOAD_SIZE, +static_assert(offsetof(LegacyV1MQTTPrefs, snmp_enabled) == MQTT_PREFS_V1_PRE_OBSERVER_PAYLOAD_SIZE, "v1 pre-observer /mqtt_prefs boundary changed"); -static_assert(sizeof(MQTTPrefs) == MQTT_PREFS_V1_FULL_PAYLOAD_SIZE, +static_assert(sizeof(LegacyV1MQTTPrefs) == MQTT_PREFS_V1_FULL_PAYLOAD_SIZE, "v1 /mqtt_prefs payload layout changed"); // Lock the neighbors tail to the flex neighbors build's layout so a /mqtt_prefs // written by either firmware is byte-for-byte interchangeable. The enable flag // lands in the old struct's zeroed trailing padding (offset 2857), and the // interval begins exactly at the pre-neighbors payload size (2860) so a // pre-neighbors read stops right before it and the interval keeps its default. -static_assert(offsetof(MQTTPrefs, mqtt_neighbors_enabled) == 2857, +static_assert(offsetof(LegacyV1MQTTPrefs, mqtt_neighbors_enabled) == 2857, "neighbors enable flag must sit at the flex-compatible offset"); -static_assert(offsetof(MQTTPrefs, mqtt_neighbors_interval) == MQTT_PREFS_V1_PRE_NEIGHBORS_PAYLOAD_SIZE, +static_assert(offsetof(LegacyV1MQTTPrefs, mqtt_neighbors_interval) == MQTT_PREFS_V1_PRE_NEIGHBORS_PAYLOAD_SIZE, "neighbors interval offset must equal the pre-neighbors payload size"); -static_assert(offsetof(MQTTPrefs, mqtt_slot_packet_filter) == MQTT_PREFS_V1_PRE_FILTER_PAYLOAD_SIZE, +static_assert(offsetof(LegacyV1MQTTPrefs, mqtt_slot_packet_filter) == MQTT_PREFS_V1_PRE_FILTER_PAYLOAD_SIZE, "packet filters must begin at the pre-filter payload boundary"); static_assert(sizeof(OldMQTTPrefs) == 472, "frozen pre-slot /mqtt_prefs layout changed"); static_assert(sizeof(PreWifiPowerOldMQTTPrefs) == 472, "frozen pre-WiFi-power /mqtt_prefs layout changed"); diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index eb7ab943..43af42b2 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -509,7 +509,7 @@ private: LifecycleOps _lifecycle_ops; MQTTLifecycle::Coordinator _lifecycle; - // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt_prefs. + // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt.json. // _prefs (held by BridgeBase) still provides upstream fields (freq/sf/node_name…). MQTTPrefs* _obs = nullptr; diff --git a/test/README.md b/test/README.md index 711e6484..80182510 100644 --- a/test/README.md +++ b/test/README.md @@ -33,7 +33,8 @@ does not reflect the GoogleTest count — run the built binary directly | `test_mqtt_packet_filter` | `src/helpers/MQTTPacketFilter.h` | per-slot 0-15 allowlist parsing/formatting, numeric and named spellings; exact bounds; membership; candidate/eligible split and retry-completion policy; pre-queue union gate; default-mask detection | | `test_mqtt_runtime_buffer_lifecycle` | `src/helpers/MQTTRuntimeBufferLifecycle.h` | idempotent allocation/release; partial-allocation degradation; retry of only missing buffers | | `test_mqtt_prefs_codec` | `src/helpers/MQTTPrefsStorage.h`, `src/helpers/MQTTPrefsCodec.h` | binary pre-slot/3-slot/6-slot migration fixtures; v1 header integrity; downgrade preservation; shortest-payload write policy (default filters stay downgrade-readable) | -| `test_mqtt_prefs_atomic_store` | `src/helpers/MQTTPrefsAtomicStore.h` | transactional MQTT writes and legacy `/node_prefs` handoff; exact short-write detection; begin/finish/rename failure cleanup; original-file preservation | +| `test_mqtt_prefs_serializer` | `src/helpers/MQTTPrefsSerializer.h`, `src/helpers/ConfigSerializer.*` | semantic nested `/mqtt.json` round trips; numeric slot keys; required/future version handling; strict length/overflow/duplicate rejection; safe semantic repair; scratch-before-live loading | +| `test_mqtt_prefs_atomic_store` | `src/helpers/MQTTPrefsAtomicStore.h`, `src/helpers/MQTTPrefsRecovery.h` | production JSON begin/write/checksum-finish/schema-verify/commit orchestration; first-migration and rename-boundary recovery; legacy `/node_prefs` handoff; failure cleanup and original-file preservation | | `test_mqtt_payload_builder` | `src/helpers/MQTTPayloadBuilder.cpp` | status/packet/raw JSON contracts; optional fields; escaping; RX metrics and path; score handling; exact buffer bounds; maximum representative payloads | | `test_utils` | `src/Utils.cpp` | `Utils::toHex` (upstream) | diff --git a/test/test_config_serializer/test_config_serializer.cpp b/test/test_config_serializer/test_config_serializer.cpp index 27c3c811..90154047 100644 --- a/test/test_config_serializer/test_config_serializer.cpp +++ b/test/test_config_serializer/test_config_serializer.cpp @@ -214,6 +214,23 @@ TEST(NodePrefs, FemGainSettingsRoundTrip) { EXPECT_EQ(1, loaded.radio_fem_txgain); } +TEST(ConfigSerializer, LoadSerial_KeyDigitsAfterFirstCharacter) { + MockInputStream s("{age:1,flags:2,name:\"ok\",slot1:7}"); + TestStruct data; + data.age = data.flags = 0; + strcpy(data.name, "before"); + EXPECT_TRUE(data.loadSerial(s)); + EXPECT_EQ(1, data.age); + EXPECT_EQ(2, data.flags); + EXPECT_STREQ("ok", data.name); +} + +TEST(ConfigSerializer, LoadSerial_RejectsLeadingDigitKey) { + MockInputStream s("{1slot:7}"); + TestStruct data; + EXPECT_FALSE(data.loadSerial(s)); +} + // ── main ─────────────────────────────────────────────────────── diff --git a/test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp b/test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp index 67880a82..4d2ae120 100644 --- a/test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp +++ b/test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp @@ -20,6 +20,7 @@ enum class FailurePoint { PayloadWrite, ImageWrite, Finish, + Verify, Commit, }; @@ -44,7 +45,8 @@ public: ++write_calls; if (!_open) return 0; const bool should_fail = (write_calls == 1 && _failure == FailurePoint::HeaderWrite) || - (write_calls == 2 && _failure == FailurePoint::PayloadWrite); + (write_calls == 2 && _failure == FailurePoint::PayloadWrite) || + _failure == FailurePoint::ImageWrite; const size_t written = should_fail && size > 0 ? size - 1 : size; _staging.insert(_staging.end(), bytes, bytes + written); return written; @@ -68,6 +70,18 @@ public: return true; } + bool verify() { + ++verify_calls; + return _failure != FailurePoint::Verify; + } + + void discardFinishedTemp() { + ++discard_calls; + _files.erase("/mqtt_prefs.tmp"); + _finished = false; + _owns_temp = false; + } + void abort() { ++abort_calls; _open = false; @@ -88,6 +102,8 @@ public: int finish_calls = 0; int commit_calls = 0; int abort_calls = 0; + int verify_calls = 0; + int discard_calls = 0; private: FailurePoint _failure; @@ -113,6 +129,16 @@ AtomicStore::Result runWithObserverTail(InMemoryStore* store) { return AtomicStore::write(*store, header, sizeof(header), payload, sizeof(payload)); } +AtomicStore::VerifiedImageResult runVerifiedJson(InMemoryStore* store) { + const uint8_t json[] = "{version:1,wifi:{ssid:\"mesh\"}}"; + return AtomicStore::writeVerifiedImage( + *store, + [store, &json]() { + return store->write(json, sizeof(json) - 1) == sizeof(json) - 1; + }, + [store]() { return store->verify(); }); +} + class LegacyComPrefs { public: LegacyComPrefs() : bytes({'l', 'e', 'g', 'a', 'c', 'y', '-', 'c', 'o', 'm'}) {} @@ -286,9 +312,14 @@ public: } return; } + if (action == Recovery::Action::DiscardTemp) { + _files.erase("/mqtt_prefs.tmp"); + return; + } if (action == Recovery::Action::PromoteBackup) { rename("/mqtt_prefs.bak", "/mqtt_prefs"); - if (backup == Recovery::FileState::Usable && temp != Recovery::FileState::Missing) { + if (backup == Recovery::FileState::Usable && !Recovery::uncertain(temp) && + temp != Recovery::FileState::Missing) { _files.erase("/mqtt_prefs.tmp"); } return; @@ -298,12 +329,13 @@ public: // incomplete transaction artifact. It only preserves artifacts when the // primary itself is opaque. if (had_primary && primary == Recovery::FileState::Usable) { - _files.erase("/mqtt_prefs.tmp"); - _files.erase("/mqtt_prefs.bak"); + if (!Recovery::uncertain(temp)) _files.erase("/mqtt_prefs.tmp"); + if (!Recovery::uncertain(backup)) _files.erase("/mqtt_prefs.bak"); } } bool has(const char* path) const { return _files.count(path) != 0; } + void removePrimary() { _files.erase("/mqtt_prefs"); } bool canStartSave() const { return !has("/mqtt_prefs.tmp") && !has("/mqtt_prefs.bak"); } const std::vector& primary() const { return _files.at("/mqtt_prefs"); } static std::vector oldImage() { return {'o', 'l', 'd'}; } @@ -337,6 +369,50 @@ TEST(MQTTPrefsAtomicStore, CommitPublishesExactHeaderThenPayload) { EXPECT_EQ(0, store.abort_calls); } +TEST(MQTTPrefsAtomicStore, ProductionJsonPolicyCoversEveryVerificationBoundary) { + const std::vector old_source = { + 'o', 'l', 'd', '-', 'p', 'r', 'e', 'f', 's'}; + const struct { + FailurePoint point; + AtomicStore::VerifiedImageResult expected; + int writes; + int finishes; + int verifies; + int commits; + int aborts; + int discards; + bool keeps_temp; + } cases[] = { + {FailurePoint::None, AtomicStore::VerifiedImageResult::Committed, + 1, 1, 1, 1, 0, 0, false}, + {FailurePoint::Begin, AtomicStore::VerifiedImageResult::BeginFailed, + 0, 0, 0, 0, 1, 0, false}, + {FailurePoint::ImageWrite, AtomicStore::VerifiedImageResult::WriteFailed, + 1, 0, 0, 0, 1, 0, false}, + {FailurePoint::Finish, AtomicStore::VerifiedImageResult::FinishFailed, + 1, 1, 0, 0, 1, 0, false}, + {FailurePoint::Verify, AtomicStore::VerifiedImageResult::VerifyFailed, + 1, 1, 1, 0, 0, 1, false}, + {FailurePoint::Commit, AtomicStore::VerifiedImageResult::CommitFailed, + 1, 1, 1, 1, 1, 0, true}, + }; + + for (const auto& test_case : cases) { + InMemoryStore store(test_case.point); + EXPECT_EQ(test_case.expected, runVerifiedJson(&store)); + EXPECT_EQ(test_case.writes, store.write_calls); + EXPECT_EQ(test_case.finishes, store.finish_calls); + EXPECT_EQ(test_case.verifies, store.verify_calls); + EXPECT_EQ(test_case.commits, store.commit_calls); + EXPECT_EQ(test_case.aborts, store.abort_calls); + EXPECT_EQ(test_case.discards, store.discard_calls); + EXPECT_EQ(test_case.keeps_temp, store.tempExists()); + if (test_case.point != FailurePoint::None) { + EXPECT_EQ(old_source, store.source()); + } + } +} + TEST(MQTTPrefsAtomicStore, AnyFailureAbortsAndPreservesExistingSource) { const std::vector source = {'o', 'l', 'd', '-', 'p', 'r', 'e', 'f', 's'}; const struct { @@ -546,6 +622,23 @@ TEST(MQTTPrefsAtomicStore, PowerCutDuringTempWriteKeepsPrimaryAndAllowsNextSave) EXPECT_TRUE(store.canStartSave()); } +TEST(MQTTPrefsAtomicStore, TornFirstMigrationTempIsDiscardedSoLegacyCanRetry) { + SpiffsMqttTransaction store; + store.removePrimary(); + store.cutDuringTempWrite(); + + EXPECT_EQ(Recovery::Action::DiscardTemp, + Recovery::select(Recovery::FileState::Missing, + Recovery::FileState::Preserve, + Recovery::FileState::Missing)); + store.recover(Recovery::FileState::Missing, + Recovery::FileState::Preserve, + Recovery::FileState::Missing); + EXPECT_FALSE(store.has("/mqtt_prefs")); + EXPECT_FALSE(store.has("/mqtt_prefs.tmp")); + EXPECT_TRUE(store.canStartSave()); +} + TEST(MQTTPrefsAtomicStore, RecoveredUsablePrimaryClearsOpaqueTransactionArtifacts) { { SpiffsMqttTransaction store; @@ -605,8 +698,14 @@ TEST(MQTTPrefsAtomicStore, RecoveryNeverOverwritesOpaqueNewerLayout) { EXPECT_EQ(Recovery::Action::KeepPrimary, Recovery::select(Recovery::FileState::Preserve, Recovery::FileState::Usable, Recovery::FileState::Usable)); - // If there is no primary, a usable backup wins over an opaque temp. Once - // promoted, production treats the backup as authoritative and clears temp. + // A syntactically valid future temp may already have passed verification and + // reached the rename phase. It wins over the stale supported backup and is + // promoted into the authoritative name, where older firmware will hold it. + EXPECT_EQ(Recovery::Action::PromoteTemp, + Recovery::select(Recovery::FileState::Missing, Recovery::FileState::FutureUsable, + Recovery::FileState::Usable)); + // A corrupt or incomplete temp is different: the supported backup remains + // the last known committed image. EXPECT_EQ(Recovery::Action::PromoteBackup, Recovery::select(Recovery::FileState::Missing, Recovery::FileState::Preserve, Recovery::FileState::Usable)); @@ -617,6 +716,34 @@ TEST(MQTTPrefsAtomicStore, RecoveryNeverOverwritesOpaqueNewerLayout) { Recovery::FileState::Preserve)); } +TEST(MQTTPrefsAtomicStore, AmbiguousFutureOrOomTempIsNeverDeleted) { + for (const Recovery::FileState uncertain : { + Recovery::FileState::FutureClaimed, + Recovery::FileState::Indeterminate}) { + SpiffsMqttTransaction store; + store.cutAt(SpiffsMqttTransaction::Boundary::AfterBackupRename); + store.recover(Recovery::FileState::Missing, uncertain, + Recovery::FileState::Usable); + + // The supported backup can run this boot, but the candidate that this + // firmware could not classify remains available to newer firmware or a + // later boot with more heap. Its presence also blocks a new transaction. + EXPECT_EQ(SpiffsMqttTransaction::oldImage(), store.primary()); + EXPECT_TRUE(store.has("/mqtt_prefs.tmp")); + EXPECT_FALSE(store.canStartSave()); + } +} + +TEST(MQTTPrefsAtomicStore, UsablePrimaryDoesNotCleanIndeterminateArtifact) { + SpiffsMqttTransaction store; + store.cutDuringTempWrite(); + store.recover(Recovery::FileState::Usable, + Recovery::FileState::Indeterminate); + EXPECT_EQ(SpiffsMqttTransaction::oldImage(), store.primary()); + EXPECT_TRUE(store.has("/mqtt_prefs.tmp")); + EXPECT_FALSE(store.canStartSave()); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/test/test_mqtt_prefs_codec/test_mqtt_prefs_codec.cpp b/test/test_mqtt_prefs_codec/test_mqtt_prefs_codec.cpp index 7bf49746..73ec5ec8 100644 --- a/test/test_mqtt_prefs_codec/test_mqtt_prefs_codec.cpp +++ b/test/test_mqtt_prefs_codec/test_mqtt_prefs_codec.cpp @@ -28,6 +28,7 @@ MQTTPrefs defaults() { prefs.alert_wifi_minutes = 30; prefs.alert_mqtt_minutes = 240; prefs.alert_min_interval_min = 60; + prefs.mqtt_neighbors_interval = MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS; return prefs; } @@ -59,6 +60,21 @@ Codec::DecodePlan classify(const std::vector& bytes) { return Codec::classify(bytes.data(), prefix_size, bytes.size()); } +void writeV1Payload(std::vector* bytes, const MQTTPrefs& prefs, size_t payload_len) { + LegacyV1MQTTPrefs frozen; + Codec::freezeV1(prefs, &frozen); + ASSERT_GE(bytes->size(), sizeof(MQTTPrefsHeader) + payload_len); + memcpy(bytes->data() + sizeof(MQTTPrefsHeader), &frozen, payload_len); +} + +MQTTPrefs decodeV1(const std::vector& bytes, const Codec::DecodePlan& plan) { + LegacyV1MQTTPrefs frozen = {}; + memcpy(&frozen, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = defaults(); + Codec::migrateV1(frozen, plan.payload_len, &loaded); + return loaded; +} + void fillHighEntropy(std::vector* bytes) { uint32_t state = 0x89abcdef; for (size_t i = 0; i < bytes->size(); ++i) { @@ -244,8 +260,7 @@ TEST(MQTTPrefsCodec, CurrentVersionedPayloadRoundTripsExactly) { ASSERT_EQ(Codec::Source::Current, plan.source); ASSERT_FALSE(plan.preserve_file); ASSERT_TRUE(plan.observer_fields_present); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_EQ(0, memcmp(&source, &loaded, sizeof(source))); } @@ -277,8 +292,7 @@ TEST(MQTTPrefsCodec, DefaultFiltersKeepTheDowngradeReadablePayloadLength) { ASSERT_EQ(Codec::kV1PreFilterPayloadSize, plan.payload_len); ASSERT_FALSE(plan.preserve_file); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_EQ(0, memcmp(&source, &loaded, sizeof(source))); } @@ -299,8 +313,7 @@ TEST(MQTTPrefsCodec, AnyNonDefaultFilterOptsIntoTheLongerPayload) { const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::kV1BaselinePayloadSize, plan.payload_len); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_EQ(mask, loaded.mqtt_slot_packet_filter[slot]) << slot; EXPECT_EQ(0, memcmp(&source, &loaded, sizeof(source))); } @@ -338,7 +351,7 @@ TEST(MQTTPrefsCodec, PreFilterV1PayloadDefaultsEverySlotToAllTypes) { std::vector bytes(sizeof(MQTTPrefsHeader) + Codec::kV1PreFilterPayloadSize, 0); writeHeader(&bytes, MQTT_PREFS_VERSION, static_cast(Codec::kV1PreFilterPayloadSize)); - memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &source, Codec::kV1PreFilterPayloadSize); + writeV1Payload(&bytes, source, Codec::kV1PreFilterPayloadSize); const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::Source::Current, plan.source); @@ -346,8 +359,7 @@ TEST(MQTTPrefsCodec, PreFilterV1PayloadDefaultsEverySlotToAllTypes) { ASSERT_TRUE(plan.observer_fields_present); ASSERT_FALSE(plan.preserve_file); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_STREQ("pre-filter-node", loaded.mqtt_origin); EXPECT_EQ(1u, loaded.mqtt_neighbors_enabled); EXPECT_EQ(MQTT_NEIGHBORS_MAX_INTERVAL_MS, loaded.mqtt_neighbors_interval); @@ -368,7 +380,7 @@ TEST(MQTTPrefsCodec, CompatibleShortV1PayloadPreservesDefaultsBeyondObserverBoun std::vector bytes(sizeof(MQTTPrefsHeader) + Codec::kV1PreObserverPayloadSize, 0); writeHeader(&bytes, MQTT_PREFS_VERSION, static_cast(Codec::kV1PreObserverPayloadSize)); - memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &source, Codec::kV1PreObserverPayloadSize); + writeV1Payload(&bytes, source, Codec::kV1PreObserverPayloadSize); const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::Source::Current, plan.source); @@ -376,8 +388,7 @@ TEST(MQTTPrefsCodec, CompatibleShortV1PayloadPreservesDefaultsBeyondObserverBoun ASSERT_FALSE(plan.preserve_file); ASSERT_FALSE(plan.observer_fields_present); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_STREQ("short-v1-node", loaded.mqtt_origin); EXPECT_STREQ("ntp.short.example", loaded.mqtt_ntp_server); EXPECT_EQ(0, loaded.snmp_enabled); @@ -400,7 +411,7 @@ TEST(MQTTPrefsCodec, PreNeighborsV1PayloadLoadsObserverFieldsAndDefaultsNeighbor std::vector bytes(sizeof(MQTTPrefsHeader) + Codec::kV1PreNeighborsPayloadSize, 0); writeHeader(&bytes, MQTT_PREFS_VERSION, static_cast(Codec::kV1PreNeighborsPayloadSize)); - memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &source, Codec::kV1PreNeighborsPayloadSize); + writeV1Payload(&bytes, source, Codec::kV1PreNeighborsPayloadSize); const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::Source::Current, plan.source); @@ -408,10 +419,7 @@ TEST(MQTTPrefsCodec, PreNeighborsV1PayloadLoadsObserverFieldsAndDefaultsNeighbor ASSERT_FALSE(plan.preserve_file); ASSERT_TRUE(plan.observer_fields_present); - MQTTPrefs loaded = defaults(); - loaded.mqtt_neighbors_enabled = 1; // pretend stale - loaded.mqtt_neighbors_interval = MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS; // caller's defaulted tail - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_STREQ("pre-neighbors-node", loaded.mqtt_origin); EXPECT_STREQ("PNW", loaded.alert_region); @@ -475,6 +483,34 @@ TEST(MQTTPrefsCodec, CorruptOrShortVersionedInputsArePreserved) { EXPECT_TRUE(plan.preserve_file); } +TEST(MQTTPrefsCodec, HeaderValidV1StillRequiresBoundedPayloadStrings) { + MQTTPrefs source = defaults(); + LegacyV1MQTTPrefs frozen; + Codec::freezeV1(source, &frozen); + ASSERT_TRUE(Codec::isPlausibleV1(frozen, Codec::kV1BaselinePayloadSize)); + + memset(frozen.mqtt_slot_topic[5], 'x', sizeof(frozen.mqtt_slot_topic[5])); + EXPECT_FALSE(Codec::isPlausibleV1(frozen, Codec::kV1BaselinePayloadSize)); +} + +TEST(MQTTPrefsCodec, HeaderValidV1AllowsNumericValuesForSerializerRepair) { + MQTTPrefs source = defaults(); + LegacyV1MQTTPrefs frozen; + Codec::freezeV1(source, &frozen); + + // A valid v1 header fixes the layout. Numeric bytes cannot trigger an + // out-of-bounds read and are normalized after migration, so plausibility + // must not reject deployed files merely because these values need repair. + frozen.timezone_offset = 99; + frozen.mqtt_status_enabled = 0xff; + frozen.mqtt_rx_enabled = 0xff; + frozen.snmp_enabled = 0xff; + frozen.alert_enabled = 0xff; + frozen.mqtt_neighbors_enabled = 0xa5; + EXPECT_TRUE(Codec::isPlausibleV1(frozen, Codec::kV1BaselinePayloadSize)); + EXPECT_TRUE(Codec::isPlausibleV1(frozen, Codec::kV1PreNeighborsPayloadSize)); +} + TEST(MQTTPrefsCodec, LegacyPlausibilityRejectsHighEntropyBytesAtEveryWhitelistedSize) { // A headerless raw struct has no checksum, so this only reduces false // migrations; it cannot prove that a plausible-looking file is authentic. @@ -531,7 +567,9 @@ TEST(MQTTPrefsCodec, LongerSameVersionPayloadLoadsTheBaselineAndIgnoresTheTail) const size_t payload_len = Codec::kV1BaselinePayloadSize + kTail; std::vector bytes(sizeof(MQTTPrefsHeader) + payload_len, 0xA5); writeHeader(&bytes, MQTT_PREFS_VERSION, static_cast(payload_len)); - memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &source, sizeof(source)); + LegacyV1MQTTPrefs frozen; + Codec::freezeV1(source, &frozen); + memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &frozen, sizeof(frozen)); const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::Source::Current, plan.source); @@ -542,9 +580,11 @@ TEST(MQTTPrefsCodec, LongerSameVersionPayloadLoadsTheBaselineAndIgnoresTheTail) // Reading plan.payload_len bytes recovers this build's whole struct exactly, // and cannot run past it into the unknown tail. + LegacyV1MQTTPrefs loaded_frozen = {}; + memcpy(&loaded_frozen, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + ASSERT_TRUE(Codec::isPlausibleV1(loaded_frozen, plan.payload_len)); MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); - EXPECT_EQ(0, memcmp(&source, &loaded, sizeof(source))); + Codec::migrateV1(loaded_frozen, plan.payload_len, &loaded); EXPECT_STREQ("future-node", loaded.mqtt_origin); EXPECT_STREQ("field-ssid", loaded.wifi_ssid); EXPECT_STREQ("field-secret", loaded.wifi_password); diff --git a/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp b/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp new file mode 100644 index 00000000..ec2544e7 --- /dev/null +++ b/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp @@ -0,0 +1,358 @@ +#include + +#include + +#define WITH_MQTT_BRIDGE 1 +#define PROGMEM +#include "helpers/MQTTPrefsSerializer.h" + +class InputStream : public Stream { +public: + explicit InputStream(const std::string& text) : _text(text) {} + int available() override { return static_cast(_text.size() - _pos); } + int read() override { return _pos < _text.size() ? _text[_pos++] : -1; } + int peek() override { return _pos < _text.size() ? _text[_pos] : -1; } +private: + std::string _text; + size_t _pos = 0; +}; + +class OutputStream : public Stream { +public: + size_t write(uint8_t byte) override { _text.push_back(static_cast(byte)); return 1; } + size_t print(int value, int = DEC) override { return appendNumber(value); } + size_t print(unsigned int value, int = DEC) override { return appendNumber(value); } + size_t print(long value, int = DEC) override { return appendNumber(value); } + size_t print(unsigned long value, int = DEC) override { return appendNumber(value); } + size_t print(long long value, int = DEC) override { return appendNumber(value); } + size_t print(unsigned long long value, int = DEC) override { return appendNumber(value); } + int available() override { return 0; } + int read() override { return -1; } + int peek() override { return -1; } + const std::string& text() const { return _text; } +private: + template size_t appendNumber(T value) { + const std::string number = std::to_string(value); + _text += number; + return number.size(); + } + std::string _text; +}; + +class StickyFailingStream : public Stream { +public: + explicit StickyFailingStream(size_t limit) : _limit(limit) {} + size_t write(uint8_t) override { + if (_failed || _written >= _limit) { + _failed = true; + return 0; + } + ++_written; + return 1; + } + size_t print(int value, int = DEC) override { + const std::string number = std::to_string(value); + return Print::print(number.c_str()); + } + int available() override { return 0; } + int read() override { return -1; } + int peek() override { return -1; } + bool failed() const { return _failed; } +private: + size_t _limit; + size_t _written = 0; + bool _failed = false; +}; + +static MQTTPrefs defaults() { + MQTTPrefs prefs = {}; + prefs.mqtt_status_enabled = 1; + prefs.mqtt_packets_enabled = 1; + prefs.mqtt_tx_enabled = 2; + prefs.mqtt_rx_enabled = 1; + prefs.mqtt_status_interval = 300000; + prefs.wifi_power_save = 1; + prefs.timezone_offset = -7; + prefs.radio_watchdog_minutes = 5; + prefs.alert_wifi_minutes = 30; + prefs.alert_mqtt_minutes = 240; + prefs.alert_min_interval_min = 60; + prefs.mqtt_neighbors_interval = MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS; + strcpy(prefs.snmp_community, "public"); + for (int i = 0; i < MQTT_PREFS_SLOT_COUNT; ++i) { + strcpy(prefs.mqtt_slot_preset[i], "none"); + prefs.mqtt_slot_packet_filter[i] = 0xffff; + } + return prefs; +} + +TEST(MQTTPrefsSerializer, RoundTripsEveryGroupAndNumericSlotKeys) { + MQTTPrefs source = defaults(); + strcpy(source.wifi_ssid, "mesh-net"); + strcpy(source.wifi_password, "p\\\"ass\nword"); + strcpy(source.timezone_string, "MST7MDT,M3.2.0"); + strcpy(source.mqtt_ntp_server, "time.example"); + strcpy(source.mqtt_origin, "observer-one"); + strcpy(source.mqtt_iata, "SEA"); + source.mqtt_neighbors_enabled = 1; + source.mqtt_neighbors_interval = MQTT_NEIGHBORS_MAX_INTERVAL_MS; + strcpy(source.mqtt_owner_public_key, + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"); + strcpy(source.mqtt_email, "owner@example.com"); + strcpy(source.mqtt_slot_preset[5], "custom"); + strcpy(source.mqtt_slot_host[5], "broker.example"); + source.mqtt_slot_port[5] = 65535; + strcpy(source.mqtt_slot_username[5], "user-six"); + strcpy(source.mqtt_slot_password[5], "secret-six"); + strcpy(source.mqtt_slot_token[5], "token-six"); + strcpy(source.mqtt_slot_topic[5], "mesh/{iata}/{type}"); + strcpy(source.mqtt_slot_audience[5], "audience-six"); + source.mqtt_slot_packet_filter[5] = 0x8001; + source.snmp_enabled = 1; + source.radio_watchdog_minutes = 120; + source.alert_enabled = 1; + strcpy(source.alert_psk_hex, "0123456789abcdef0123456789abcdef"); + strcpy(source.alert_hashtag, "#ops"); + strcpy(source.alert_region, "PNW"); + + OutputStream output; + MQTTPrefsSerializer writer(&source); + ASSERT_TRUE(writer.saveSerial(output)); + EXPECT_NE(std::string::npos, output.text().find("slot6:{")) << output.text(); + EXPECT_NE(std::string::npos, output.text().find("packet_filter:32769")) << output.text(); + + MQTTPrefs loaded = defaults(); + InputStream input(output.text()); + MQTTPrefsSerializer reader(&loaded); + ASSERT_TRUE(reader.loadSerial(input)) << output.text(); + bool repaired = true; + ASSERT_TRUE(reader.apply(&repaired)); + EXPECT_FALSE(repaired); + EXPECT_STREQ(source.wifi_password, loaded.wifi_password); + EXPECT_STREQ(source.mqtt_owner_public_key, loaded.mqtt_owner_public_key); + EXPECT_STREQ(source.mqtt_slot_host[5], loaded.mqtt_slot_host[5]); + EXPECT_EQ(65535, loaded.mqtt_slot_port[5]); + EXPECT_EQ(0x8001, loaded.mqtt_slot_packet_filter[5]); + EXPECT_EQ(MQTT_NEIGHBORS_MAX_INTERVAL_MS, loaded.mqtt_neighbors_interval); + EXPECT_STREQ("PNW", loaded.alert_region); +} + +TEST(MQTTPrefsSerializer, MissingOptionalKeysKeepDefaults) { + MQTTPrefs prefs = defaults(); + InputStream input("{version:1,mqtt:{origin:\"changed\"}}"); + MQTTPrefsSerializer serializer(&prefs); + ASSERT_TRUE(serializer.loadSerial(input)); + bool repaired = true; + ASSERT_TRUE(serializer.apply(&repaired)); + EXPECT_FALSE(repaired); + EXPECT_STREQ("changed", prefs.mqtt_origin); + EXPECT_EQ(1, prefs.mqtt_packets_enabled); + EXPECT_EQ(300000u, prefs.mqtt_status_interval); + EXPECT_EQ(0xffff, prefs.mqtt_slot_packet_filter[5]); +} + +TEST(MQTTPrefsSerializer, RequiresSupportedVersion) { + MQTTPrefs missing = defaults(); + InputStream no_version("{wifi:{ssid:\"x\"}}"); + MQTTPrefsSerializer missing_serializer(&missing); + ASSERT_TRUE(missing_serializer.loadSerial(no_version)); + bool repaired = false; + EXPECT_FALSE(missing_serializer.apply(&repaired)); + + MQTTPrefs future = defaults(); + InputStream future_input("{version:2,wifi:{ssid:\"x\"}}"); + MQTTPrefsSerializer future_serializer(&future); + ASSERT_TRUE(future_serializer.loadSerial(future_input)); + EXPECT_TRUE(future_serializer.hasFutureVersion()); + EXPECT_FALSE(future_serializer.apply(&repaired)); +} + +TEST(MQTTPrefsSerializer, FutureVersionProbeIgnoresV1FieldTypeChanges) { + InputStream probe_input("{version:2,wifi:{power_save:\"max\"}}"); + MQTTPrefsVersionProbe probe; + ASSERT_TRUE(probe.loadSerial(probe_input)); + EXPECT_TRUE(probe.hasFutureVersion()); + + // The same image is not valid under v1, demonstrating why recovery must + // probe the version before invoking the current schema. + MQTTPrefs prefs = defaults(); + InputStream v1_input("{version:2,wifi:{power_save:\"max\"}}"); + MQTTPrefsSerializer v1(&prefs); + EXPECT_FALSE(v1.loadSerial(v1_input)); + + for (const char* future_text : { + "{version:2,this_key_is_too_long:1}", + "{version:2,x:[1]}", + "{version:2,wifi:{ssid:\"torn\"}"}) { + InputStream future_input(future_text); + MQTTPrefsVersionProbe future_probe; + EXPECT_FALSE(future_probe.loadSerial(future_input)) << future_text; + EXPECT_TRUE(future_probe.hasFutureVersion()) << future_text; + } +} + +TEST(MQTTPrefsSerializer, RejectsDuplicateKnownKey) { + MQTTPrefs prefs = defaults(); + InputStream input("{version:1,mqtt:{origin:\"one\",origin:\"two\"}}"); + MQTTPrefsSerializer serializer(&prefs); + EXPECT_FALSE(serializer.loadSerial(input)); +} + +TEST(MQTTPrefsSerializer, RejectsOverlongStringAndIntegerOverflow) { + MQTTPrefs prefs = defaults(); + InputStream long_string( + "{version:1,wifi:{ssid:\"12345678901234567890123456789012\"}}"); + MQTTPrefsSerializer string_serializer(&prefs); + EXPECT_FALSE(string_serializer.loadSerial(long_string)); + + prefs = defaults(); + InputStream overflow("{version:1,mqtt:{slot1:{port:999999999999}}}"); + MQTTPrefsSerializer number_serializer(&prefs); + EXPECT_FALSE(number_serializer.loadSerial(overflow)); + + prefs = defaults(); + InputStream quoted_number("{version:\"1\"}"); + MQTTPrefsSerializer quoted_number_serializer(&prefs); + EXPECT_FALSE(quoted_number_serializer.loadSerial(quoted_number)); + + prefs = defaults(); + InputStream bare_string("{version:1,wifi:{ssid:meshnet}}"); + MQTTPrefsSerializer bare_string_serializer(&prefs); + EXPECT_FALSE(bare_string_serializer.loadSerial(bare_string)); +} + +TEST(MQTTPrefsSerializer, RejectsScalarObjectShapeMismatches) { + MQTTPrefs prefs = defaults(); + InputStream object_version("{version:{x:1}}"); + MQTTPrefsSerializer object_version_serializer(&prefs); + EXPECT_FALSE(object_version_serializer.loadSerial(object_version)); + + prefs = defaults(); + InputStream object_port("{version:1,mqtt:{slot1:{port:{x:1883}}}}"); + MQTTPrefsSerializer object_port_serializer(&prefs); + EXPECT_FALSE(object_port_serializer.loadSerial(object_port)); + + prefs = defaults(); + InputStream scalar_mqtt("{version:1,mqtt:1}"); + MQTTPrefsSerializer scalar_mqtt_serializer(&prefs); + EXPECT_FALSE(scalar_mqtt_serializer.loadSerial(scalar_mqtt)); + + prefs = defaults(); + InputStream scalar_slot("{version:1,mqtt:{slot1:1}}"); + MQTTPrefsSerializer scalar_slot_serializer(&prefs); + EXPECT_FALSE(scalar_slot_serializer.loadSerial(scalar_slot)); +} + +TEST(MQTTPrefsSerializer, RepairsSemanticRanges) { + MQTTPrefs prefs = defaults(); + InputStream input( + "{version:1,wifi:{power_save:9},time:{utc_offset:99}," + "mqtt:{tx_enabled:7,status:{enabled:3,interval_ms:10}," + "neighbors:{enabled:2,interval_ms:100},slot1:{port:-1,packet_filter:-2}}," + "radio:{watchdog_min:121},alert:{rate_limit_min:1}}"); + MQTTPrefsSerializer serializer(&prefs); + ASSERT_TRUE(serializer.loadSerial(input)); + bool repaired = false; + ASSERT_TRUE(serializer.apply(&repaired)); + EXPECT_TRUE(repaired); + EXPECT_EQ(1, prefs.wifi_power_save); + EXPECT_EQ(-7, prefs.timezone_offset); + EXPECT_EQ(2, prefs.mqtt_tx_enabled); + EXPECT_EQ(300000u, prefs.mqtt_status_interval); + EXPECT_EQ(MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS, prefs.mqtt_neighbors_interval); + EXPECT_EQ(0, prefs.mqtt_slot_port[0]); + EXPECT_EQ(0xffff, prefs.mqtt_slot_packet_filter[0]); + EXPECT_EQ(5, prefs.radio_watchdog_minutes); + EXPECT_EQ(60, prefs.alert_min_interval_min); +} + +TEST(MQTTPrefsSerializer, RepairsTextValuesToSafeDefaults) { + MQTTPrefs prefs = defaults(); + InputStream input( + "{version:1,time:{ntp_server:\"bad/host\"},mqtt:{iata:\"sea\"," + "owner:{public_key:\"not-a-key\"},slot1:{preset:\"not-a-preset\"}," + "slot2:{preset:\"analyzer-us\"},slot3:{preset:\"analyzer-us\"}}," + "alert:{psk_hex:\"not-hex\",hashtag:\"#stale\"}}"); + MQTTPrefsSerializer serializer(&prefs); + ASSERT_TRUE(serializer.loadSerial(input)); + bool repaired = false; + ASSERT_TRUE(serializer.apply(&repaired)); + EXPECT_TRUE(repaired); + EXPECT_STREQ("SEA", prefs.mqtt_iata); + EXPECT_STREQ("", prefs.mqtt_ntp_server); + EXPECT_STREQ("", prefs.mqtt_owner_public_key); + EXPECT_STREQ("none", prefs.mqtt_slot_preset[0]); + EXPECT_STREQ("analyzer-us", prefs.mqtt_slot_preset[1]); + // Historical firmware allowed duplicate aliases. Preserve them on load; + // current setters prevent creating new duplicates without silently changing + // a deployed configuration during migration. + EXPECT_STREQ("analyzer-us", prefs.mqtt_slot_preset[2]); + EXPECT_STREQ("", prefs.alert_psk_hex); + EXPECT_STREQ("", prefs.alert_hashtag); +} + +TEST(MQTTPrefsSerializer, LateParseOrVersionFailureCannotMutateLivePrefs) { + MQTTPrefs live = defaults(); + strcpy(live.wifi_ssid, "live-network"); + + MQTTPrefs scratch = defaults(); + InputStream truncated("{wifi:{ssid:\"uncommitted\"},version:1"); + MQTTPrefsSerializer truncated_serializer(&scratch); + EXPECT_FALSE(truncated_serializer.loadSerial(truncated)); + EXPECT_STREQ("live-network", live.wifi_ssid); + + scratch = defaults(); + InputStream future("{wifi:{ssid:\"future-network\"},version:2}"); + MQTTPrefsSerializer future_serializer(&scratch); + ASSERT_TRUE(future_serializer.loadSerial(future)); + EXPECT_TRUE(future_serializer.hasFutureVersion()); + bool repaired = false; + EXPECT_FALSE(future_serializer.apply(&repaired)); + EXPECT_STREQ("live-network", live.wifi_ssid); +} + +TEST(MQTTPrefsSerializer, StickyShortWriteFailsTheCompleteSave) { + MQTTPrefs prefs = defaults(); + MQTTPrefsSerializer serializer(&prefs); + StickyFailingStream output(20); + EXPECT_FALSE(serializer.saveSerial(output)); + EXPECT_TRUE(output.failed()); +} + +TEST(MQTTPrefsSerializer, SaveNormalizationIsIdempotentAgainstKnownDefaults) { + MQTTPrefs prefs = defaults(); + prefs.timezone_offset = 99; + strcpy(prefs.mqtt_ntp_server, "bad/host"); + strcpy(prefs.mqtt_iata, "not-iata"); + strcpy(prefs.mqtt_slot_preset[0], "not-a-preset"); + + MQTTPrefs repair_defaults = defaults(); + repair_defaults.timezone_offset = -7; + strcpy(repair_defaults.mqtt_iata, "sea"); + strcpy(repair_defaults.mqtt_slot_preset[0], "analyzer-us"); + + MQTTPrefsSerializer writer(&prefs, &repair_defaults); + bool repaired = false; + ASSERT_TRUE(writer.normalize(&repaired)); + EXPECT_TRUE(repaired); + EXPECT_EQ(-7, prefs.timezone_offset); + EXPECT_STREQ("", prefs.mqtt_ntp_server); + EXPECT_STREQ("SEA", prefs.mqtt_iata); + EXPECT_STREQ("none", prefs.mqtt_slot_preset[0]); + + OutputStream output; + ASSERT_TRUE(writer.saveSerial(output)); + + MQTTPrefs loaded = defaults(); + InputStream input(output.text()); + MQTTPrefsSerializer reader(&loaded); + ASSERT_TRUE(reader.loadSerial(input)); + repaired = true; + ASSERT_TRUE(reader.apply(&repaired)); + EXPECT_FALSE(repaired) << output.text(); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/test_observer_validation/test_observer_validation.cpp b/test/test_observer_validation/test_observer_validation.cpp index 3045caed..8e9913ca 100644 --- a/test/test_observer_validation/test_observer_validation.cpp +++ b/test/test_observer_validation/test_observer_validation.cpp @@ -129,6 +129,18 @@ TEST(ValueFits, RejectsNullOrZeroBuffer) { EXPECT_FALSE(mqttValueFits("abc", 0)); } +TEST(AssignedPresetSlot, FindsDuplicatesOutsideTheTargetSlot) { + const char presets[4][24] = { + "analyzer-us", "none", "analyzer-eu", "analyzer-us" + }; + + EXPECT_EQ(3, mqttAssignedPresetSlot(presets, "analyzer-us", 0)); + EXPECT_EQ(0, mqttAssignedPresetSlot(presets, "analyzer-us", 3)); + EXPECT_EQ(-1, mqttAssignedPresetSlot(presets, "analyzer-eu", 2)); + EXPECT_EQ(-1, mqttAssignedPresetSlot(presets, "missing", 0)); + EXPECT_EQ(-1, mqttAssignedPresetSlot(presets, nullptr, 0)); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/test/test_webconfig_batch/test_webconfig_batch.cpp b/test/test_webconfig_batch/test_webconfig_batch.cpp index 813412c5..670c6ead 100644 --- a/test/test_webconfig_batch/test_webconfig_batch.cpp +++ b/test/test_webconfig_batch/test_webconfig_batch.cpp @@ -230,6 +230,7 @@ TEST(WebConfigBatch, CliFailureRepliesAreRecognisedInEveryShapeCommonCLIEmits) { "Err - bad params", // MyMesh setperm "ERR: bad pubkey", // neighbor.remove "Error: IATA code must be exactly 3 letters",// observer setters + "Error: setting not persisted; change rolled back", // observer storage failure "(ERR: clock cannot go backwards)", // clock sync, parenthesised "Unknown command", // top-level fallthrough "unknown config: mqtt.nope", // set fallthrough