From a7c1cc631f71c4a1734f9da94cec82f57b279e3d Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 18 Jul 2026 09:34:11 -0700 Subject: [PATCH] feat(webconfig): add web configuration portal and MQTT enhancements Introduce a web configuration portal for easier node management and provisioning without serial CLI. Enhance MQTT functionality with improved IATA code validation, dynamic slot management, and background NTP synchronization. Update web UI elements for better user experience and security notes regarding open AP usage. --- MQTT_IMPLEMENTATION.md | 60 ++ WEB_CONFIG_REVIEW.md | 516 ++++++++++++++++++ examples/simple_repeater/MyMesh.h | 7 +- examples/simple_room_server/MyMesh.h | 7 +- scripts/generate_webconfig_html.py | 49 +- src/helpers/CommonCLI_Observer.cpp | 57 +- src/helpers/bridges/MQTTBridge.cpp | 74 ++- src/helpers/bridges/MQTTBridge.h | 5 + src/helpers/esp32/WebConfigServer.cpp | 129 ++++- src/helpers/esp32/WebConfigServer.h | 4 + .../HeltecTrackerV2Board.cpp | 7 + webui/index.html | 185 +++++-- 12 files changed, 983 insertions(+), 117 deletions(-) create mode 100644 WEB_CONFIG_REVIEW.md diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index 5bc75004..21735931 100644 --- a/MQTT_IMPLEMENTATION.md +++ b/MQTT_IMPLEMENTATION.md @@ -474,6 +474,66 @@ These are standard MeshCore commands, not MQTT-specific, but important for obser See [MQTT_SNMP.md](MQTT_SNMP.md) for full SNMP documentation. +### Web Configuration Portal + +The observer builds include a browser-based configuration portal so a node can +be provisioned and managed without the serial CLI. It is started from the CLI +(serial or remote admin) and is never on by default on a configured node. + +#### CLI commands +- `start webconfig` — start the portal. If WiFi is already configured and + connected, it binds to the node's **LAN** IP and requires the admin password + to log in. If WiFi is **not** configured (`wifi.ssid` empty), it raises the + setup AP instead (same as first boot). +- `start webconfig ap` — force the **setup AP** even when WiFi is configured. + The MQTT bridge must be stopped first (`set bridge off`); the AP owns the + radio. Used for re-provisioning in the field. +- `stop webconfig` — stop the portal and free its resources. LAN mode runs until + this is issued; the setup AP also auto-stops after an idle timeout (default 10 + minutes with no station associated). + +#### First-boot / setup-AP behavior +On a node with no WiFi configured, the portal comes up automatically as an open +SoftAP named `MeshCore-Setup-XXXX` (last two bytes of the public key), with a +captive-portal redirect. The device display shows the AP name and portal URL +(`http://192.168.4.1/`). Walk through the wizard (WiFi → radio → MQTT → review), +then **Save & reboot**; the node reboots and joins the configured network. + +Optionally set a WPA2 password for the setup AP at build time with +`-D WEBCONFIG_AP_PASSWORD='"yourpassword"'`. + +#### Modes and authentication +- **Setup AP**: unauthenticated. Trust is based on physical proximity to the + open/PSK AP. Only the SoftAP interface serves the API — on `start webconfig + ap` the STA is explicitly disassociated so the API is **not** exposed on the + LAN the node was attached to. +- **LAN**: requires the admin password (same one used for remote CLI admin). + Sessions use a cookie with a sliding idle expiry (default 20 minutes); five + failed logins trigger a 30-second lockout. + +> **Security note:** the open setup AP transports WiFi/MQTT credentials over +> plain HTTP. Provision on a trusted, non-public frequency/location, set +> `WEBCONFIG_AP_PASSWORD` where feasible, and prefer LAN mode for ongoing +> management. The setup AP is intended for initial provisioning, not +> long-running operation. + +#### Applying changes +- **Radio** (freq/BW/SF/CR): persisted but applied only on reboot; the UI shows + a "reboot to apply" hint. +- **WiFi SSID/password**: changing these in LAN mode saves and reboots so the + node reconnects on the new network (the page will drop; find the new IP on + your router). In the setup wizard, saving always reboots. +- **MQTT publishing toggles / slot config**: applied live to the running bridge + (no reboot needed). +- **NTP server**: saved immediately; the time sync runs in the background — + verify with `get mqtt.ntp.diag`. + +#### Recovery +If provisioning fails or you're locked out of the portal, connect over USB +serial and use the CLI directly (e.g. `set wifi.ssid ...`, `set wifi.pwd ...`, +`get wifi.status`, `stop webconfig`). Serial access always works regardless of +the portal state. + ## Command Architecture The CLI commands are organized into two levels: diff --git a/WEB_CONFIG_REVIEW.md b/WEB_CONFIG_REVIEW.md new file mode 100644 index 00000000..71c7f202 --- /dev/null +++ b/WEB_CONFIG_REVIEW.md @@ -0,0 +1,516 @@ +# WebConfig Branch Review + +## Scope + +This review covers the fork-owned WebConfig and Heltec Tracker additions on the `webconfig` branch, principally commits `d7a7e1b6`, `dfee21a0`, and `639c07a4`, plus the fork-owned MQTT/CLI paths they invoke. Issues inherited unchanged from `meshcore-dev/MeshCore` are intentionally excluded. + +No implementation changes are included in this document. + +## Executive Summary + +The portal builds successfully and has a sound high-level design: HTTP handlers avoid directly running CLI/radio operations, configuration writes are marshalled to the loop task, secrets are represented by placeholders, and the UI is self-contained for offline provisioning. + +Before deployment, the most important work is: + +1. Secure setup/forced-AP reachability. +2. Correlate each save with its own result. +3. Prevent reboot after partially failed wizard saves. +4. Make persisted Wi-Fi and MQTT settings match live runtime behavior. +5. Remove cross-task preference/statistics races and long loop-task blocking. + +## Priority 1: Security and Data Integrity + +### 1. Forced AP mode exposes the unauthenticated setup API on the existing LAN + +**Severity:** High + +**Locations:** + +- `src/helpers/esp32/WebConfigServer.cpp:125-134` +- `src/helpers/esp32/WebConfigServer.cpp:346-349` +- `src/helpers/bridges/MQTTBridge.cpp:801-866` +- `src/helpers/bridges/MQTTBridge.cpp:919-926` +- `examples/simple_repeater/MyMesh.cpp:1329-1339` +- `examples/simple_room_server/MyMesh.cpp:974-984` + +**Problem:** + +`MQTTBridge::end()` deliberately leaves the STA association connected. Forced setup then selects `WIFI_AP_STA`, starts the server on port 80, and disables authentication for all requests while `MODE_SETUP` is active. The server is therefore reachable through both the setup AP and the existing LAN connection. + +The comment that setup mode implies physical proximity is not valid in forced-AP mode. Any host on the existing LAN can read or change configuration and reboot the node without the admin password. + +**Suggested fix:** + +Choose one of these approaches: + +- Disconnect and disable STA before entering unauthenticated setup mode, leaving only the SoftAP interface active. If scanning requires STA mode, keep the interface enabled but explicitly disconnect it and prevent auto-reconnect. +- Bind the setup listener only to the SoftAP interface if the ESPAsyncWebServer/network stack supports reliable interface binding. +- Keep authentication enabled for setup-mode requests arriving through STA, while allowing unauthenticated requests only through the SoftAP interface. + +Add a hardware test that starts from an associated STA connection, stops the bridge, enters forced AP mode, and confirms that another LAN host cannot access `/api/config` or `/api/reboot` without authentication. + +### 2. Default provisioning sends credentials over an open HTTP network + +**Severity:** High + +**Locations:** + +- `src/helpers/esp32/WebConfigServer.cpp:130-134` +- `src/helpers/esp32/WebConfigServer.cpp:346-349` +- `src/helpers/esp32/WebConfigServer.cpp:530-598` + +**Problem:** + +No reviewed observer environment defines `WEBCONFIG_AP_PASSWORD`, so setup creates an open AP. Setup requests are unauthenticated and use plain HTTP. Wi-Fi passwords, MQTT passwords, and access tokens can be captured by another nearby station. A nearby party can also provision the device before the intended operator. + +**Suggested fix:** + +- Generate a unique per-device setup password from secure random data during first boot and show it on the display or a physical label. +- Alternatively require a short-lived setup PIN displayed on-device and validated by the API before secrets can be submitted. +- Add an explicit provisioning-session expiry and invalidate the setup credential after successful setup. +- If an intentionally open AP remains supported, document the threat model prominently and avoid describing proximity as authentication. + +### 3. Save results are not correlated with the submitted batch + +**Severity:** High + +**Locations:** + +- `webui/index.html:630-680` +- `src/helpers/esp32/WebConfigServer.cpp:547-550` +- `src/helpers/esp32/WebConfigServer.cpp:601-641` + +**Problem:** + +The server keeps the last completed result readable. The frontend polls `/api/config/result` after nearly every POST failure because the POST may have reached the device even if its response was lost. There is no request or batch identity. + +A rejected, lost, or concurrent save can therefore consume a previous or different tab's result, report success, clear `st.dirty`, and overwrite unsaved values with the device configuration. + +**Suggested fix:** + +- Have the browser generate a random request ID and include it in the POST body. +- Store that ID with the batch and include it in the 202 response and every result response. +- Require the frontend to accept `pending` or `done` only when the ID matches. +- Treat definite HTTP responses such as 400, 409, and 413 as final rejection; only poll after an ambiguous network failure. +- Return the active request ID with 409 so the browser can distinguish its own retry from another client's batch. + +Add regression tests for stale `DONE`, two tabs, 409, lost 202, lost result response, and a POST that never reached the server. + +### 4. Failed wizard commands still cause partial application and reboot + +**Severity:** High + +**Locations:** + +- `src/helpers/esp32/WebConfigServer.cpp:265-297` +- `src/helpers/esp32/WebConfigServer.cpp:624-637` +- `webui/index.html:950-957` + +**Problem:** + +Commands are persisted independently. Error replies are recorded, but `_batch_reboot` still schedules a fallback reboot, and reading the result arms the three-second reboot without checking aggregate success. The wizard can display rejected settings while the device reboots with a partially applied configuration. + +**Suggested fix:** + +- Track aggregate batch success while draining commands. +- Arm automatic reboot only if every required command succeeded. +- If partial persistence cannot be rolled back, return an explicit `partial` state listing applied and rejected keys. +- Keep the portal active after partial failure and present deliberate choices: correct and retry, or reboot with the partial configuration. +- Longer term, validate all values before executing any setter, or stage a complete preference snapshot and commit it atomically. + +## Priority 2: Runtime Correctness + +### 5. LAN Wi-Fi edits do not perform the reconnect promised by the UI + +**Severity:** High + +**Locations:** + +- `webui/index.html:339-350` +- `webui/index.html:630-641` +- `src/helpers/CommonCLI_Observer.cpp:278-285` + +**Problem:** + +The Wi-Fi tab says the connection will restart and the page will drop. Normal editor saves do not request reboot, and the underlying setters only persist the SSID/password. The active connection remains on the old network, while subsequent config reads show the new persisted values. + +**Suggested fix:** + +Use an explicit `Save and reconnect` or `Save and reboot` flow for SSID/password changes. Deliver the result first, then schedule the reconnect/reboot. Do not replace the displayed active network with the persisted network until the transition has begun successfully. + +### 6. MQTT publishing controls report success without changing live behavior + +**Severity:** High + +**Locations:** + +- `webui/index.html:303-319` +- `src/helpers/CommonCLI_Observer.cpp:215-238` +- `src/helpers/bridges/MQTTBridge.cpp:637-645` + +**Problem:** + +Status, packet, raw, RX, and TX options are copied into cached bridge fields during initialization. Their CLI setters persist preferences but do not refresh those fields or restart the bridge. The UI reports success although the running bridge continues using old values. + +**Suggested fix:** + +- Add one task-safe bridge method that reloads the publishing flags from preferences on the MQTT task, or +- coalesce these changes into one full bridge restart after the batch. + +If live application is not desirable, label these controls as restart-required and provide a reboot action instead of claiming immediate success. + +### 7. Custom MQTT endpoint edits do not reliably refresh the live slot + +**Severity:** High + +**Locations:** + +- `src/helpers/CommonCLI_Observer.cpp:385-425` +- `examples/simple_repeater/MyMesh.cpp:1354-1367` +- `examples/simple_room_server/MyMesh.cpp:999-1012` +- `src/helpers/bridges/MQTTBridge.cpp:1066-1072` +- `src/helpers/bridges/MQTTBridge.cpp:2076-2095` + +**Problem:** + +Custom server and port setters persist without requesting slot reconfiguration. Credential changes request reconfiguration, but the custom branch of `applySlotPreset()` reuses the existing slot fields instead of copying the current host, port, username, and password from preferences. + +**Suggested fix:** + +Create a single `reloadSlotFromPrefs(slot)` operation executed on the MQTT task. It should tear down the slot, copy every custom field and preset-dependent credential from `MQTTPrefs`, validate the complete endpoint, and then reconnect. Every slot-affecting CLI setter should queue that same operation rather than implementing partial restart behavior. + +### 8. Wizard review and validation mishandle intentionally cleared values + +**Severity:** Medium + +**Locations:** + +- `webui/index.html:907-929` +- `webui/index.html:931-933` + +**Problem:** + +Review calculations use `dirtyValue || originalValue`. An intentional empty string is treated as absent, so Review shows the original SSID/password/name/identity/slot while the submitted batch clears it. Final SSID validation can pass using the old SSID even though the effective new value is empty. + +**Suggested fix:** + +Resolve values by key presence, not truthiness. Add a helper such as `effectiveValue(key)` that returns `st.dirty[key]` when the key exists in `st.dirty`, including `""`, and otherwise returns `st.orig[key]`. Use it for review, validation, and reboot messaging. + +### 9. A credential equal to `********` cannot be configured + +**Severity:** Low + +**Locations:** + +- `webui/index.html:422` +- `webui/index.html:527-538` +- `src/helpers/esp32/WebConfigServer.cpp:564` + +**Problem:** + +The secret sentinel is also a valid possible password/token. The UI either considers it unchanged or the backend silently drops it. + +**Suggested fix:** + +Track secret-field edit state separately from the displayed placeholder. Prefer an empty password control with an adjacent “stored credential unchanged” indicator. If retaining the sentinel, reject that exact value with a clear validation message rather than silently ignoring it. + +## Priority 3: Concurrency, Responsiveness, and Lifecycle + +### 10. The preference mutex does not synchronize reads with writes + +**Severity:** Medium + +**Locations:** + +- `src/helpers/esp32/WebConfigServer.cpp:454-523` +- `src/helpers/esp32/WebConfigServer.cpp:265-280` +- `src/helpers/CommonCLI.cpp:1074-1216` +- `src/helpers/CommonCLI_Observer.cpp:198-486` + +**Problem:** + +`handleConfigGet()` takes `_mux` while reading preferences, but `drainBatch()` invokes CLI setters outside `_mux`. Those setters mutate the same strings and scalar fields. The lock therefore does not protect the data from concurrent async HTTP reads. + +**Suggested fix:** + +Prefer loop-task ownership: have the HTTP handler request a configuration snapshot, let `tick()` build it on the loop task, and return the immutable snapshot. If retaining direct reads, every writer must take the same mutex, with careful review to avoid holding it through flash writes or callbacks. + +### 11. Changing `mqtt.ntp` can block the main loop for up to 30 seconds + +**Severity:** Medium + +**Locations:** + +- `src/helpers/CommonCLI_Observer.cpp:249-272` +- `src/helpers/bridges/MQTTBridge.cpp:3230-3247` +- `src/helpers/esp32/WebConfigServer.cpp:275-280` + +**Problem:** + +Web batches run from `tick()` on the Arduino loop task. The NTP setter waits for the MQTT task in a polling loop that can last 30 seconds. During that period mesh/radio processing, portal DNS, further batch work, stats, and reboot timers stop progressing. + +**Suggested fix:** + +Persist and validate hostname syntax synchronously, then queue NTP synchronization without waiting. Represent NTP validation as a separate asynchronous operation/status result. The config batch should finish immediately and the UI can poll NTP validation independently. + +### 12. MQTT status snapshots read mutable cross-core state without synchronization + +**Severity:** Medium + +**Locations:** + +- `src/helpers/bridges/MQTTBridge.cpp:295-320` +- `examples/simple_repeater/MyMesh.cpp:1403-1408` +- `examples/simple_room_server/MyMesh.cpp:1048-1053` + +**Problem:** + +The loop task reads slot state and client counters while the MQTT task and callbacks mutate them. This can yield inconsistent snapshots and formal C++ data races. + +**Suggested fix:** + +Build a plain-data `SlotStatusSnapshot` array on the MQTT task and publish it atomically or under a shared lock. The web/loop side should read only the copied snapshot and should not call client methods cross-task. + +### 13. Fixed-delay server deletion does not prove connections have ended + +**Severity:** Medium, hardware/library stress-test required + +**Locations:** + +- `src/helpers/esp32/WebConfigServer.cpp:188-204` + +**Problem:** + +Stopping the listener and waiting two seconds does not establish that accepted slow or stalled clients have completed. Deleting the server and route lambdas while a request remains active risks use-after-free or crashes. + +**Suggested fix:** + +Use connection/request reference counting and finalize deletion only after all accepted requests disconnect, with an upper-bound recovery policy. If the library cannot expose lifecycle state safely, consider retaining one server instance for the firmware lifetime and enabling/disabling routes/listening without deleting captured handler state. + +### 14. Repeated starts permanently grow the global default-header list + +**Severity:** Medium + +**Location:** + +- `src/helpers/esp32/WebConfigServer.cpp:177-185` + +**Problem:** + +`DefaultHeaders::Instance().addHeader()` appends to a process-lifetime list on every server creation. Repeated start/stop cycles consume heap and add duplicate `Cache-Control` headers to every response. + +**Suggested fix:** + +Register the global header once through a static one-time guard, or avoid global headers and add `Cache-Control: no-store` to each WebConfig response. + +### 15. Stats polling can accumulate overlapping requests + +**Severity:** Low + +**Locations:** + +- `webui/index.html:717-747` + +**Problem:** + +A three-second `setInterval` starts a new request even if the previous fetch is still pending. Degraded Wi-Fi can accumulate requests and pressure both browser and ESP32 memory. + +**Suggested fix:** + +Schedule the next poll with `setTimeout` only after the current request settles. Add an in-flight guard and use the existing API timeout support. + +## Priority 4: Validation, Build, and Maintainability + +### 16. Malformed keys can cause out-of-bounds reads and invalid JSON errors + +**Severity:** Low + +**Locations:** + +- `src/helpers/esp32/WebConfigServer.cpp:41-59` +- `src/helpers/esp32/WebConfigServer.cpp:554-562` + +**Problem:** + +Short attacker-supplied keys are indexed at positions 4 and 5 without first establishing their length. Rejected keys are interpolated into hand-built JSON without escaping quotes or backslashes. + +**Suggested fix:** + +Check the key length before `memcmp` and indexed access. Construct error responses with ArduinoJson rather than string interpolation. + +### 17. Input lengths do not match fixed firmware buffers + +**Severity:** Low + +**Locations:** + +- `webui/index.html` configuration controls +- `src/helpers/CommonCLI.h:106-159` + +**Problem:** + +Several controls accept more text than the fixed `MQTTPrefs` fields retain. The CLI truncates values while the UI reports success. + +**Suggested fix:** + +Add `maxlength` values matching each destination buffer minus the NUL terminator, and validate lengths in the backend because client-side constraints are bypassable. Return a clear error instead of silently truncating. + +### 18. Generated HTML freshness relies only on timestamps + +**Severity:** Low + +**Location:** + +- `scripts/generate_webconfig_html.py:30-38` + +**Problem:** + +A generated header with a future timestamp can survive later source edits and embed stale UI code. + +**Suggested fix:** + +Use an explicit SCons source/target dependency or store the source hash in the generated header and regenerate whenever it differs. Deterministic gzip output can remain unchanged. + +### 19. Tracker v1.1 reports itself as Tracker V2 + +**Severity:** Medium + +**Locations:** + +- `boards/heltec_tracker_v1_1.json:19,28` +- `variants/heltec_tracker_v2/platformio.ini:61-109` +- `variants/heltec_tracker_v2/HeltecTrackerV2Board.cpp:82-84` + +**Problem:** + +The v1.1 environment reuses the V2 board implementation, whose manufacturer name is hardcoded as `Heltec Tracker V2`. WebConfig and MQTT metadata therefore report the wrong board. + +**Suggested fix:** + +Return `Heltec Tracker V1.1` when `HELTEC_TRACKER_V1_1` is defined, and retain the V2 value otherwise. Add a build-time or host-side assertion for both target identities. + +### 20. WebConfig operation and security behavior are undocumented + +**Severity:** Documentation gap + +**Locations:** + +- Commands added in `src/helpers/CommonCLI_Observer.cpp:982-994` +- `MQTT_IMPLEMENTATION.md` + +**Problem:** + +The build targets are documented, but operators cannot discover `start webconfig`, `start webconfig ap`, `stop webconfig`, first-boot AP behavior, authentication, timeout, or the security implications of setup mode. + +**Suggested fix:** + +Add an operator section to `MQTT_IMPLEMENTATION.md` covering: + +- first-boot setup behavior; +- AP name and setup credential; +- LAN versus AP modes; +- exact CLI commands; +- authentication requirements; +- idle and absolute timeout behavior; +- how Wi-Fi changes are applied; +- how to recover through serial if provisioning fails. + +### 21. Repeater and room-server integration is duplicated + +**Severity:** Optimization + +**Locations:** + +- `examples/simple_repeater/MyMesh.*` +- `examples/simple_room_server/MyMesh.*` +- Corresponding `UITask.cpp` files + +**Problem:** + +Lifecycle, restart coalescing, stats construction, and display behavior are implemented twice. Fixes can easily land in only one role. + +**Suggested fix:** + +Extract shared WebConfig callbacks, lifecycle ownership, and JSON snapshot helpers into a common observer helper. Keep role-specific radio/stat sources as injected callbacks. + +### 22. The HTML generator runs for every ESP32 build + +**Severity:** Optimization + +**Locations:** + +- `platformio.ini:57-72` + +**Problem:** + +The generator runs from `esp32_base` even when WebConfig is not compiled into the target. + +**Suggested fix:** + +Move the pre-script to MQTT observer environments or have the script inspect build flags and return immediately unless `WITH_MQTT_BRIDGE`/WebConfig is enabled. + +### 23. New observer targets enable MQTT debug logging + +**Severity:** Optimization + +**Locations:** + +- `variants/heltec_tracker_v2/platformio.ini:190,226,262,298` + +**Problem:** + +`MQTT_DEBUG=1` increases serial activity and code/logging overhead in targets that otherwise appear intended for deployment. + +**Suggested fix:** + +Remove it from production environments or create explicit debug variants. Confirm that no diagnostic output includes credentials or tokens. + +## Test Recommendations + +No WebConfig-specific automated tests were found. Add focused tests for: + +1. Save request/result correlation, including stale and concurrent batches. +2. Partial command failures and reboot gating. +3. Effective-value handling when fields are intentionally cleared. +4. Secret placeholder/edit semantics. +5. CLI parsing and backend length validation. +6. Runtime application of Wi-Fi and cached MQTT settings. +7. Complete custom-slot reconfiguration. +8. NTP updates without loop-task blocking. +9. Config and MQTT status snapshots under concurrent updates. +10. Repeated server start/stop heap behavior and duplicate headers. +11. Forced AP reachability from both SoftAP and STA networks. +12. Setup AP absolute expiry with an idle associated station. +13. Correct board identity for Tracker v1.1 and V2 targets. + +## Verification Already Performed + +The review ran the following checks successfully: + +- `git diff --check` +- Extracted JavaScript with `node --check` +- PlatformIO builds: + - `heltec_tracker_v1_1_repeater_observer_mqtt` + - `heltec_tracker_v2_repeater_observer_mqtt` + - `heltec_tracker_v1_1_room_server_observer_mqtt` + - `heltec_tracker_v2_room_server_observer_mqtt` + +Observed static usage: + +- Repeater: 76,960 bytes RAM (23.5%), approximately 47.4% flash. +- Room server: 80,656 bytes RAM (24.6%), approximately 47.3% flash. + +Successful compilation does not validate the AP/STA security boundary, async teardown, concurrency, reconnection, or partial-save behavior; those require the targeted host/hardware tests above. + +## Suggested Implementation Order + +1. Fix forced-AP LAN exposure and define secure provisioning authentication. +2. Introduce request IDs for save/result correlation. +3. Gate reboot on aggregate success and represent partial application explicitly. +4. Correct effective empty-value handling in the wizard. +5. Align Wi-Fi, MQTT flags, and custom-slot runtime behavior with the UI. +6. Remove NTP blocking and marshal config/status snapshots to their owning tasks. +7. Fix server lifecycle/global-header accumulation. +8. Add input-length/backend validation and polling safeguards. +9. Fix Tracker v1.1 identity and add operator documentation. +10. Add automated tests, then run the full hardware matrix. diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index 127ad7d1..ee6b935d 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -363,8 +363,11 @@ public: bool syncMqttNtp() override { if (!bridge || !bridge->isRunning()) return false; - // Marshal onto the MQTT task (Core 0); this runs on the CLI thread (Core 1). - return bridge->requestForcedNtpSync(); + // Queue the sync onto the MQTT task (Core 0) without blocking: this runs on + // the Arduino loop task (serial CLI and the web config batch both drain + // here), and blocking up to 30 s would stall mesh/radio forwarding. Returns + // true once queued; verify with `get mqtt.ntp.diag`. + return bridge->requestForcedNtpSync(0); } bool runMqttNtpDiag(char* reply, size_t reply_size, bool verbose) override { diff --git a/examples/simple_room_server/MyMesh.h b/examples/simple_room_server/MyMesh.h index 466fb09d..41377965 100644 --- a/examples/simple_room_server/MyMesh.h +++ b/examples/simple_room_server/MyMesh.h @@ -338,8 +338,11 @@ public: bool syncMqttNtp() override { if (!bridge || !bridge->isRunning()) return false; - // Marshal onto the MQTT task (Core 0); this runs on the CLI thread (Core 1). - return bridge->requestForcedNtpSync(); + // Queue the sync onto the MQTT task (Core 0) without blocking: this runs on + // the Arduino loop task (serial CLI and the web config batch both drain + // here), and blocking up to 30 s would stall mesh/radio forwarding. Returns + // true once queued; verify with `get mqtt.ntp.diag`. + return bridge->requestForcedNtpSync(0); } bool runMqttNtpDiag(char* reply, size_t reply_size, bool verbose) override { diff --git a/scripts/generate_webconfig_html.py b/scripts/generate_webconfig_html.py index d587fa5d..143687d3 100644 --- a/scripts/generate_webconfig_html.py +++ b/scripts/generate_webconfig_html.py @@ -3,7 +3,16 @@ # Pre-build script: gzip webui/index.html into a PROGMEM C header so the # webconfig portal can serve the page straight from flash with # Content-Encoding: gzip. The generated header is .gitignored; this script -# regenerates it whenever the source page (or this script) is newer. +# regenerates it whenever the source page (or this script) content changes. +# +# Freshness is decided by a content hash of the build inputs (not timestamps): +# a generated header written with a skewed/future mtime would otherwise mask a +# later source edit and embed stale UI. gzip output is deterministic (mtime=0), +# so an unchanged source produces an unchanged header. +# +# The generator is idempotent and cheap: when the inputs are unchanged it only +# hashes two small files and returns, so running it from esp32_base on every +# ESP32 build is negligible even for targets that don't compile the portal. # # Output: src/helpers/esp32/WebConfigHtml.h # WEBCONFIG_HTML_GZ[] - gzipped page (PROGMEM) @@ -21,21 +30,37 @@ SOURCE = os.path.join("webui", "index.html") OUTPUT = os.path.join("src", "helpers", "esp32", "WebConfigHtml.h") # __file__ is not defined inside PIO/SCons-executed extra_scripts SCRIPT = os.path.join("scripts", "generate_webconfig_html.py") +HASH_MARKER = "// build-inputs-sha256: " def status(msg): sys.stderr.write("WebConfig HTML: %s\n" % msg) -def needs_rebuild(): - if not os.path.isfile(OUTPUT): - return True - out_mtime = os.path.getmtime(OUTPUT) - if os.path.getmtime(SOURCE) > out_mtime: - return True - if os.path.isfile(SCRIPT) and os.path.getmtime(SCRIPT) > out_mtime: - return True - return False +def content_hash(): + # Hash the source page and this generator so any change to either forces a + # regenerate, independent of file timestamps. + h = hashlib.sha256() + with open(SOURCE, "rb") as f: + h.update(f.read()) + if os.path.isfile(SCRIPT): + h.update(b"\0") + with open(SCRIPT, "rb") as f: + h.update(f.read()) + return h.hexdigest() + + +def stored_hash(): + try: + with open(OUTPUT, "r") as f: + for line in f: + if line.startswith(HASH_MARKER): + return line[len(HASH_MARKER):].strip() + if line.startswith("#"): # reached the C preprocessor lines + break + except OSError: + return None + return None def main(): @@ -43,7 +68,8 @@ def main(): status("ERROR: %s not found" % SOURCE) sys.exit(2) - if not needs_rebuild(): + src_hash = content_hash() + if os.path.isfile(OUTPUT) and stored_hash() == src_hash: return with open(SOURCE, "rb") as f: @@ -56,6 +82,7 @@ def main(): lines = [] lines.append("// Auto-generated by scripts/generate_webconfig_html.py from %s" % SOURCE.replace(os.sep, "/")) lines.append("// DO NOT EDIT - edit webui/index.html instead.") + lines.append("%s%s" % (HASH_MARKER, src_hash)) lines.append("#pragma once") lines.append("#include ") lines.append("#include ") diff --git a/src/helpers/CommonCLI_Observer.cpp b/src/helpers/CommonCLI_Observer.cpp index 75d14d48..ab724717 100644 --- a/src/helpers/CommonCLI_Observer.cpp +++ b/src/helpers/CommonCLI_Observer.cpp @@ -205,13 +205,35 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf savePrefs(); strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.iata ", 10) == 0) { - StrHelper::strncpy(_mqtt_prefs.mqtt_iata, &config[10], sizeof(_mqtt_prefs.mqtt_iata)); - for (int i = 0; _mqtt_prefs.mqtt_iata[i]; i++) { - _mqtt_prefs.mqtt_iata[i] = toupper(_mqtt_prefs.mqtt_iata[i]); + const char* iata = &config[10]; + size_t iata_len = strlen(iata); + if (iata_len == 0) { + // 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(); + _callbacks->restartBridge(); + strcpy(reply, "OK - IATA cleared"); + } else { + // A region code goes straight into MQTT topic paths, so require exactly + // three alphanumeric characters (real IATA codes are 3 letters, e.g. DEN). + bool valid = (iata_len == 3); + for (size_t i = 0; valid && i < iata_len; i++) { + char c = iata[i]; + valid = (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9'); + } + if (!valid) { + strcpy(reply, "Error: IATA code must be exactly 3 letters/digits (e.g. DEN)"); + } else { + StrHelper::strncpy(_mqtt_prefs.mqtt_iata, iata, sizeof(_mqtt_prefs.mqtt_iata)); + for (int i = 0; _mqtt_prefs.mqtt_iata[i]; i++) { + _mqtt_prefs.mqtt_iata[i] = toupper(_mqtt_prefs.mqtt_iata[i]); + } + savePrefs(); + _callbacks->restartBridge(); + strcpy(reply, "OK"); + } } - savePrefs(); - _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(); @@ -260,16 +282,18 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } savePrefs(); #ifdef ESP_PLATFORM - // Validate by running an immediate sync. syncMqttNtp() marshals onto the MQTT - // task (Core 0) so no NTP I/O happens on this (Core 1) CLI thread. + // 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 + // web config batch, so a synchronous wait of up to 30 s would stall the + // node. The sync runs in the background; verify with `get mqtt.ntp.diag`. if (WiFi.status() != WL_CONNECTED) { strcpy(reply, "OK - saved (WiFi not connected; NTP sync pending)"); } else if (!_callbacks->isMqttBridgeRunning()) { strcpy(reply, "OK - saved (MQTT bridge not running)"); } else if (_callbacks->syncMqttNtp()) { - strcpy(reply, "OK - time synced"); + strcpy(reply, "OK - saved (NTP sync started; check 'get mqtt.ntp.diag')"); } else { - strcpy(reply, "Error: NTP sync failed"); + strcpy(reply, "OK - saved (NTP sync unavailable)"); } #else strcpy(reply, "OK - saved"); @@ -385,12 +409,17 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(subcmd, "server ", 7) == 0) { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_host[slot], &subcmd[7], sizeof(_mqtt_prefs.mqtt_slot_host[slot])); savePrefs(); + // 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). + _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(subcmd, "port ", 5) == 0) { int port = atoi(&subcmd[5]); if (port > 0 && port <= 65535) { _mqtt_prefs.mqtt_slot_port[slot] = port; savePrefs(); + _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else { strcpy(reply, "Error: port must be between 1 and 65535"); @@ -460,7 +489,13 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "mqtt.owner ", 11) == 0) { const char* owner_key = &config[11]; int key_len = strlen(owner_key); - if (key_len == 64) { + if (key_len == 0) { + // 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(); + strcpy(reply, "OK - owner key cleared"); + } else if (key_len == 64) { bool valid_key = true; for (int i = 0; i < key_len; i++) { if (!((owner_key[i] >= '0' && owner_key[i] <= '9') || diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index c7e7e66d..0bed6211 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -320,6 +320,17 @@ bool MQTTBridge::getSlotStatusSnapshot(int slot_index, SlotStatusSnapshot* out) return true; } +int MQTTBridge::getMaxActiveSlots() { + // Each WSS/TLS connection needs ~40KB for mbedTLS buffers. Without PSRAM even + // 3 concurrent connections would exhaust internal heap, so cap at 2; with + // PSRAM cap at 5 (6 configurable but 5 active max). +#if defined(ESP_PLATFORM) && defined(BOARD_HAS_PSRAM) + return psramFound() ? 5 : 2; +#else + return 2; +#endif +} + uint8_t MQTTBridge::getLastWifiDisconnectReason() { return s_wifi_disconnect_reason; } unsigned long MQTTBridge::getLastWifiDisconnectTime() { return s_wifi_disconnect_time; } @@ -605,15 +616,8 @@ void MQTTBridge::begin() { MQTT_DEBUG_PRINTLN("PSRAM: not configured for this board (no BOARD_HAS_PSRAM)"); #endif - // Limit active slots based on available memory. - // Each WSS/TLS connection needs ~40KB for mbedTLS buffers. - // Without PSRAM, even 3 concurrent connections would exhaust internal heap. - // With PSRAM, cap at 5 for safety (6 configurable but 5 active max). - #if defined(ESP_PLATFORM) && defined(BOARD_HAS_PSRAM) - _max_active_slots = psramFound() ? 5 : 2; - #else - _max_active_slots = 2; - #endif + // Limit active slots based on available memory (see getMaxActiveSlots()). + _max_active_slots = getMaxActiveSlots(); MQTT_DEBUG_PRINTLN("Max active slots: %d", _max_active_slots); // Check if WiFi credentials are configured first @@ -634,7 +638,10 @@ void MQTTBridge::begin() { _iata[i] = toupper(_iata[i]); } - // Update enabled flags from preferences + // Initial snapshot of the publish toggles. NOTE: the publish hot paths read + // these live from _obs->mqtt_* (status/packets/raw/rx/tx) so a CLI/web `set` + // takes effect without a bridge restart; these members are kept only for + // startup logging/back-compat and are not the source of truth. _status_enabled = _obs->mqtt_status_enabled; _packets_enabled = _obs->mqtt_packets_enabled; _raw_enabled = _obs->mqtt_raw_enabled; @@ -1107,8 +1114,10 @@ void MQTTBridge::mqttTaskLoop() { refreshNTP(); } - // Publish status updates (handle millis() overflow correctly) - if (_status_enabled) { + // Publish status updates (handle millis() overflow correctly). + // Read the toggle live from prefs (like mqtt.packets/rx/tx below) so a + // CLI/web `set mqtt.status` change applies without a bridge restart. + if (_obs->mqtt_status_enabled) { bool has_destinations = _cached_has_connected_slots; // Early exit if no destinations - skip all the expensive logic below @@ -2086,10 +2095,24 @@ void MQTTBridge::applySlotPreset(int slot_index, const char* preset_name) { } if (strcmp(preset_name, MQTT_PRESET_CUSTOM) == 0) { - slot.enabled = true; slot.preset = nullptr; - // Custom broker settings should already be set via setSlotCustomBroker - if (_initialized && customEndpointComplete(slot.host, slot.port)) { + // Re-sync every custom field from prefs (same copy begin() does at startup) + // so a CLI/web edit to the host, port, credentials, or JWT audience is + // actually picked up on reconfigure. Previously this branch reused the + // stale slot fields, so e.g. changing mqttN.server or mqttN.username had no + // effect on the live connection. Token and topic are read live from _obs in + // setupSlot()/buildTopicForSlot(), so they don't need copying here. + strncpy(slot.host, _obs->mqtt_slot_host[slot_index], sizeof(slot.host) - 1); + slot.host[sizeof(slot.host) - 1] = '\0'; + slot.port = _obs->mqtt_slot_port[slot_index]; + strncpy(slot.username, _obs->mqtt_slot_username[slot_index], sizeof(slot.username) - 1); + slot.username[sizeof(slot.username) - 1] = '\0'; + strncpy(slot.password, _obs->mqtt_slot_password[slot_index], sizeof(slot.password) - 1); + slot.password[sizeof(slot.password) - 1] = '\0'; + strncpy(slot.audience, _obs->mqtt_slot_audience[slot_index], sizeof(slot.audience) - 1); + slot.audience[sizeof(slot.audience) - 1] = '\0'; + slot.enabled = (slot.host[0] != '\0'); + if (_initialized && slot.enabled && customEndpointComplete(slot.host, slot.port)) { setupSlot(slot_index); } return; @@ -2336,8 +2359,10 @@ void MQTTBridge::loop() { refreshNTP(); } - // Publish status updates (handle millis() overflow correctly) - if (_status_enabled) { + // Publish status updates (handle millis() overflow correctly). + // Read the toggle live from prefs so a CLI/web `set mqtt.status` change + // applies without a bridge restart. + if (_obs->mqtt_status_enabled) { bool has_destinations = _cached_has_connected_slots; if (has_destinations) { @@ -2522,9 +2547,10 @@ void MQTTBridge::processPacketQueue() { queued.snr, queued.rssi); taskYIELD(); // allow higher-priority tasks to run between packet publishes - // Publish raw if enabled + // Publish raw if enabled (live from prefs so `set mqtt.raw` applies without + // a bridge restart) bool raw_published = false; - if (_raw_enabled) { + if (_obs->mqtt_raw_enabled) { raw_published = publishRaw(&queued.packet_copy); } @@ -2623,8 +2649,9 @@ void MQTTBridge::processPacketQueue() { queued.snr, queued.rssi); // No taskYIELD() on non-ESP32 platforms (non-FreeRTOS, cooperative scheduling not needed) + // Live from prefs so `set mqtt.raw` applies without a bridge restart. bool raw_published = false; - if (_raw_enabled) { + if (_obs->mqtt_raw_enabled) { raw_published = publishRaw(&queued.packet_copy); } @@ -3236,6 +3263,13 @@ bool MQTTBridge::requestForcedNtpSync(uint32_t timeout_ms) { _ntp_force_result = false; _ntp_force_requested = true; + // Fire-and-forget: callers on the Arduino loop task (web config batch, and + // the CLI which shares that task) must not block up to 30 s polling the MQTT + // task — that stalls mesh/radio forwarding, portal DNS, and reboot timers. + // The task still performs the sync; the result is observable via + // `get mqtt.ntp.diag`. Blocking callers pass a non-zero timeout. + if (timeout_ms == 0) return true; + unsigned long start = millis(); while (!_ntp_force_done) { if (millis() - start >= timeout_ms) { diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index ca065b6c..fee9845d 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -446,6 +446,11 @@ public: bool isSlotEnabledAndAttempted(int slot_index) const; const char* getSlotPresetName(int slot_index) const; static int getRuntimeSlotCount() { return RUNTIME_MQTT_SLOTS; } + /** Max slots that can be connected at once: 5 with PSRAM, 2 without (each + * WSS/TLS connection needs ~40KB for mbedTLS buffers). This is the number of + * usefully-configurable servers; RUNTIME_MQTT_SLOTS carries a spare for + * reconfiguration. Safe to call before begin(). */ + static int getMaxActiveSlots(); /** Resolved origin for MQTT JSON: node_name when mqtt_origin is empty, else mqtt_origin (with quote stripping). */ static void getEffectiveMqttOrigin(const NodePrefs* np, const MQTTPrefs* obs, char* buf, size_t buf_size); static const char* effectiveNtpPrimary(const MQTTPrefs* obs); diff --git a/src/helpers/esp32/WebConfigServer.cpp b/src/helpers/esp32/WebConfigServer.cpp index 60578df1..f7ee3354 100644 --- a/src/helpers/esp32/WebConfigServer.cpp +++ b/src/helpers/esp32/WebConfigServer.cpp @@ -38,13 +38,21 @@ static const char* const ALLOWED_SLOT_KEYS[] = { "preset", "server", "port", "username", "password", "token", "topic", "audience", }; +// Shortest slot key is "mqttN.x" (mqtt + digit + '.' + 1-char field) = 7 chars. +// The prefix probe below indexes key[4..6], so it must never run on a shorter +// string — an attacker-supplied "mqtt" or "m" would otherwise read past the +// terminator. strcmp() is null-safe, so the exact-match loop needs no guard. +static bool isSlotKeyPrefix(const char* key) { + return strlen(key) >= 7 && memcmp(key, "mqtt", 4) == 0 + && key[4] >= '1' && key[4] <= ('0' + MAX_MQTT_SLOTS) && key[5] == '.'; +} + static bool isAllowedSetKey(const char* key) { for (size_t i = 0; i < sizeof(ALLOWED_SET_KEYS) / sizeof(ALLOWED_SET_KEYS[0]); i++) { if (strcmp(key, ALLOWED_SET_KEYS[i]) == 0) return true; } // mqtt<1-6>. - if (memcmp(key, "mqtt", 4) == 0 && key[4] >= '1' && key[4] <= ('0' + MAX_MQTT_SLOTS) - && key[5] == '.') { + if (isSlotKeyPrefix(key)) { for (size_t i = 0; i < sizeof(ALLOWED_SLOT_KEYS) / sizeof(ALLOWED_SLOT_KEYS[0]); i++) { if (strcmp(&key[6], ALLOWED_SLOT_KEYS[i]) == 0) return true; } @@ -54,7 +62,7 @@ static bool isAllowedSetKey(const char* key) { static bool isSecretKey(const char* key) { if (strcmp(key, "wifi.pwd") == 0) return true; - if (memcmp(key, "mqtt", 4) == 0 && key[5] == '.' + if (isSlotKeyPrefix(key) && (strcmp(&key[6], "password") == 0 || strcmp(&key[6], "token") == 0)) return true; return false; } @@ -126,6 +134,15 @@ bool WebConfigServer::startSetupMode(char reply[]) { // the AP is up. STA stays unconnected - the bridge won't touch WiFi // while wifi_ssid is empty, and `start webconfig ap` requires it stopped. WiFi.mode(WIFI_AP_STA); + // Setup mode serves an UNAUTHENTICATED API on the trust of physical AP + // proximity. `start webconfig ap` can be run with the STA still associated + // to the operator's LAN (MQTTBridge::end() leaves the STA link up and + // auto-reconnect on), which would expose that open API to every host on the + // LAN. Drop the association and disable auto-reconnect so only the SoftAP + // interface is reachable; the STA interface itself stays up (unassociated) + // purely so WiFi.scanNetworks() below can populate the SSID picker. + WiFi.setAutoReconnect(false); + WiFi.disconnect(false /*keep radio on*/, true /*erase stored AP so it can't reconnect*/); snprintf(_ap_ssid, sizeof(_ap_ssid), "MeshCore-Setup-%02X%02X", _pub_key[0], _pub_key[1]); #ifdef WEBCONFIG_AP_PASSWORD bool ap_ok = WiFi.softAP(_ap_ssid, WEBCONFIG_AP_PASSWORD); @@ -180,7 +197,16 @@ void WebConfigServer::createServer() { // device reflash behind the same IP), which poisons /api/config/result and // friends with stale responses from earlier sessions. Forbid caching on // every response; the HTML is small enough to refetch per visit. - DefaultHeaders::Instance().addHeader("Cache-Control", "no-store"); + // + // DefaultHeaders::Instance() is a process-lifetime singleton and addHeader() + // appends unconditionally, so registering here on every start/stop cycle + // would leak heap and stack duplicate "Cache-Control" headers on every + // response. Register exactly once for the firmware lifetime. + static bool s_default_headers_registered = false; + if (!s_default_headers_registered) { + DefaultHeaders::Instance().addHeader("Cache-Control", "no-store"); + s_default_headers_registered = true; + } registerRoutes(); _server->begin(); } @@ -276,8 +302,19 @@ void WebConfigServer::drainBatch(uint32_t now) { BatchEntry& e = _batch[_batch_next++]; e.reply[0] = 0; uint32_t t0 = millis(); - _cb->execCommand(e.cmd, e.reply); - if (e.reply[0] == 0) strcpy(e.reply, "OK"); + // Hold _mux across the setter: it mutates the same _prefs/_obs strings that + // handleConfigGet() serializes on the async_tcp task, so without the lock + // the "read" side is unprotected and a GET can observe a half-written value. + // One command per tick keeps the hold brief; the flash write inside stalls + // WiFi regardless, so a concurrent GET waiting on it costs nothing extra. + { + WCLock lock(_mux); + _cb->execCommand(e.cmd, e.reply); + if (e.reply[0] == 0) strcpy(e.reply, "OK"); + // Success convention across every allowlisted setter is an "OK" prefix + // (the UI relies on the same test); anything else is a rejection. + if (strncmp(e.reply, "OK", 2) != 0) _batch_all_ok = false; + } _batch_last_cmd = millis(); Serial.printf("WC: cmd %d/%d '%s' took %lums\n", (int)_batch_next, (int)_batch_count, e.key, (unsigned long)(_batch_last_cmd - t0)); @@ -286,12 +323,13 @@ void WebConfigServer::drainBatch(uint32_t now) { _cb->onConfigBatchEnd(); WCLock lock(_mux); _batch_state = BATCH_DONE; - if (_batch_reboot) { - // Fallback only: the real 3 s reboot timer is armed when the client reads - // /api/config/result (handleConfigResult), so the browser gets its - // confirmation before the AP/WiFi drops. This covers a client that - // disconnected and never polls — generous enough for a phone that got - // bounced off the AP mid-save to rejoin and fetch its confirmation. + if (_batch_reboot && _batch_all_ok) { + // Fallback only, and only when every command succeeded: rebooting into a + // partially-applied config would strand the node. The real 3 s reboot timer + // is armed when the client reads /api/config/result (handleConfigResult), so + // the browser gets its confirmation before the AP/WiFi drops. This covers a + // client that disconnected and never polls — generous enough for a phone + // that got bounced off the AP mid-save to rejoin and fetch its confirmation. _reboot_at = now + 30000; if (_reboot_at == 0) _reboot_at = 1; } @@ -397,6 +435,9 @@ void WebConfigServer::handleStatus(AsyncWebServerRequest* req) { doc["uptime_s"] = millis() / 1000; doc["runtime_slots"] = RUNTIME_MQTT_SLOTS; doc["max_slots"] = MAX_MQTT_SLOTS; + // Servers the UI should expose: only as many as can actually be active at + // once (2 without PSRAM, 5 with). Configuring more never connects. + doc["active_slots"] = MQTTBridge::getMaxActiveSlots(); AsyncResponseStream* res = req->beginResponseStream("application/json"); serializeJson(doc, *res); @@ -541,13 +582,21 @@ void WebConfigServer::handleConfigPost(AsyncWebServerRequest* req) { return; } bool reboot_after = doc["reboot"] | false; + const char* reqid = doc["reqid"] | ""; JsonObject set = doc["set"]; WCLock lock(_mux); // A DONE batch stays readable until the next POST claims the slot, so a // client that lost the result response can re-poll instead of failing. if (_batch_state == BATCH_PENDING) { - req->send(409, "application/json", "{\"error\":\"busy\"}"); + // Echo the in-flight batch's reqid so the caller can tell its own retry + // (same reqid — landed, keep polling) from another client's save. + StaticJsonDocument<96> bd; + bd["error"] = "busy"; + bd["reqid"] = (const char*)_batch_reqid; + String out; + serializeJson(bd, out); + req->send(409, "application/json", out); return; } @@ -556,9 +605,17 @@ void WebConfigServer::handleConfigPost(AsyncWebServerRequest* req) { const char* key = kv.key().c_str(); const char* val = kv.value().as(); if (!val || !isAllowedSetKey(key)) { - char err[96]; - snprintf(err, sizeof(err), "{\"error\":\"bad key\",\"key\":\"%.32s\"}", key); - req->send(400, "application/json", err); + // Build with ArduinoJson so an attacker-supplied key containing quotes or + // backslashes is escaped rather than breaking out of the JSON string. + char safe_key[33]; + strncpy(safe_key, key, sizeof(safe_key) - 1); + safe_key[sizeof(safe_key) - 1] = 0; + StaticJsonDocument<128> ed; + ed["error"] = "bad key"; + ed["key"] = safe_key; + String out; + serializeJson(ed, out); + req->send(400, "application/json", out); return; } if (isSecretKey(key) && strcmp(val, SECRET_SENTINEL) == 0) continue; // unchanged @@ -587,15 +644,22 @@ void WebConfigServer::handleConfigPost(AsyncWebServerRequest* req) { _batch_next = 0; _batch_reboot = reboot_after; _batch_reboot_armed = false; + _batch_all_ok = true; + strncpy(_batch_reqid, reqid, sizeof(_batch_reqid) - 1); + _batch_reqid[sizeof(_batch_reqid) - 1] = 0; _batch_state = BATCH_PENDING; // tick() picks it up on the loop task uint32_t du = millis() + 60000; if (du == 0) du = 1; _diag_until = du; Serial.printf("WC: config POST accepted, %d cmds, reboot=%d\n", count, (int)reboot_after); - char msg[64]; - snprintf(msg, sizeof(msg), "{\"state\":\"pending\",\"count\":%d}", count); - req->send(202, "application/json", msg); + StaticJsonDocument<96> ack; + ack["state"] = "pending"; + ack["count"] = count; + ack["reqid"] = (const char*)_batch_reqid; + String out; + serializeJson(ack, out); + req->send(202, "application/json", out); } void WebConfigServer::handleConfigResult(AsyncWebServerRequest* req) { @@ -612,14 +676,23 @@ void WebConfigServer::handleConfigResult(AsyncWebServerRequest* req) { return; } if (_batch_state == BATCH_PENDING) { - req->send(200, "application/json", "{\"state\":\"pending\"}"); + StaticJsonDocument<96> pd; + pd["state"] = "pending"; + pd["reqid"] = (const char*)_batch_reqid; + String out; + serializeJson(pd, out); + req->send(200, "application/json", out); return; } - Serial.printf("WC: result read -> done (reboot=%d armed=%d)\n", - (int)_batch_reboot, (int)_batch_reboot_armed); + Serial.printf("WC: result read -> done (reboot=%d armed=%d all_ok=%d)\n", + (int)_batch_reboot, (int)_batch_reboot_armed, (int)_batch_all_ok); DynamicJsonDocument doc(6144); doc["state"] = "done"; - doc["reboot"] = _batch_reboot; + // Only advertise a reboot when it will actually happen: a partially-failed + // batch is not rebooted (see below), so the UI must not show a reboot screen. + doc["reboot"] = _batch_reboot && _batch_all_ok; + doc["all_ok"] = _batch_all_ok; + doc["reqid"] = (const char*)_batch_reqid; JsonArray results = doc.createNestedArray("results"); for (int i = 0; i < _batch_count; i++) { JsonObject r = results.createNestedObject(); @@ -627,10 +700,12 @@ void WebConfigServer::handleConfigResult(AsyncWebServerRequest* req) { r["reply"] = (const char*)_batch[i].reply; } // State stays DONE (re-readable) until the next POST claims the slot. - if (_batch_reboot && !_batch_reboot_armed) { - // Confirmation delivered — reboot 3 s from now (replaces the 15 s - // drain-time fallback) so the UI can show its countdown first. Armed - // once; re-reads must not keep pushing the deadline out. + if (_batch_reboot && _batch_all_ok && !_batch_reboot_armed) { + // Confirmation delivered and every command succeeded — reboot 3 s from now + // (replaces the 30 s drain-time fallback) so the UI can show its countdown + // first. Armed once; re-reads must not keep pushing the deadline out. A + // partially-failed batch is deliberately left running so the operator can + // correct and retry instead of rebooting into a broken config. _batch_reboot_armed = true; _reboot_at = millis() + 3000; if (_reboot_at == 0) _reboot_at = 1; diff --git a/src/helpers/esp32/WebConfigServer.h b/src/helpers/esp32/WebConfigServer.h index 1dd5f31f..79b195c8 100644 --- a/src/helpers/esp32/WebConfigServer.h +++ b/src/helpers/esp32/WebConfigServer.h @@ -118,6 +118,10 @@ private: uint32_t _batch_last_cmd = 0; bool _batch_reboot = false; bool _batch_reboot_armed = false; + bool _batch_all_ok = true; // every drained command replied "OK" (gates reboot) + // Client-supplied batch identity, echoed in the 202/result/409 responses so a + // reused/lost/concurrent result can't be mistaken for this client's own. + char _batch_reqid[24] = {0}; BatchEntry _batch[MAX_BATCH]; // LAN-mode session (single slot; new login evicts the old session) diff --git a/variants/heltec_tracker_v2/HeltecTrackerV2Board.cpp b/variants/heltec_tracker_v2/HeltecTrackerV2Board.cpp index f182c905..245b71a7 100644 --- a/variants/heltec_tracker_v2/HeltecTrackerV2Board.cpp +++ b/variants/heltec_tracker_v2/HeltecTrackerV2Board.cpp @@ -80,7 +80,14 @@ void HeltecTrackerV2Board::begin() { } const char* HeltecTrackerV2Board::getManufacturerName() const { + // The v1.1 environment reuses this V2 board implementation (same variant + // dir), so report the correct identity per build flag — this string feeds + // WebConfig and MQTT status/board metadata. +#ifdef HELTEC_TRACKER_V1_1 + return "Heltec Tracker V1.1"; +#else return "Heltec Tracker V2"; +#endif } bool HeltecTrackerV2Board::setLoRaFemLnaEnabled(bool enable) { diff --git a/webui/index.html b/webui/index.html index 4be74971..16e8fd4c 100644 --- a/webui/index.html +++ b/webui/index.html @@ -148,11 +148,11 @@ canvas{width:100%;height:56px;display:block}

Connect this node to your WiFi network so it can reach the MQTT servers.

- +
- +
Leave blank for an open network. 2.4 GHz networks only.
@@ -171,6 +171,8 @@ canvas{width:100%;height:56px;display:block}
Maximum legal power varies by region — check local rules.
+
RepeatForward mesh traffic. Off = listen-only (still observes and publishes). +
Need settings that aren't listed? Choose Keep current settings, finish setup, then fine-tune in the Advanced editor.
@@ -183,16 +185,16 @@ canvas{width:100%;height:56px;display:block}

Step 3 · MQTT

- +
How this observer identifies itself in published status/packet messages. Informational only — not used for topics or authentication. Prefilled with the node name.
- +
Nearest airport code, e.g. DEN. Used in topic paths.
Optional — 64-char hex public key of your companion node. Included in auth JWTs so services that support it can let you claim this node.
- +
Optional — also included in auth JWTs for claiming this node on some services.

Servers

@@ -293,12 +295,12 @@ canvas{width:100%;height:56px;display:block}

Identity

-
+
How this observer identifies itself in published messages; informational only. Blank = node name.
-
+
64-char hex public key of the owner's companion node (optional).
-
+

Publishing

@@ -324,15 +326,15 @@ canvas{width:100%;height:56px;display:block}

Time & SNMP

-
+
Enter none to clear.
-
+
SNMP agentRestart required
-
+
@@ -342,10 +344,10 @@ canvas{width:100%;height:56px;display:block}
Changing WiFi restarts the connection — this page will drop and the node reappears on the new network. Find its new IP from your router or the serial console.
- +
-
+
@@ -379,7 +381,7 @@ canvas{width:100%;height:56px;display:block}
- +
@@ -420,7 +422,7 @@ canvas{width:100%;height:56px;display:block}