From c831e599ec6435eaf827d7e58a17a7f30ccd7d7d Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 8 Aug 2026 08:40:49 -0700 Subject: [PATCH] fix(webconfig): tighten CLI failure detection, reboot deferral and refusals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five findings from review, all confirmed against the source. Failure classification (P2). Testing replies for an "Err" prefix passed five other shapes off as success: "Unknown command", "unknown config: x", "??: x", "Can't find GPS", "(ERR: clock cannot go backwards)" and "File system erase: Err". They rendered green, and worse, left _batch_all_ok true — so a queued reboot went ahead after commands that had failed, defeating the gate entirely. Rather than lengthen one guess, the two questions are now asked separately, each erring safe: - colour asks "does this look like a failure", against every shape CommonCLI actually emits, enumerated in WebConfigBatch.h and pinned by a host test that uses the literal strings. Getting this wrong is cosmetic. - the reboot gate asks something narrower and answerable: "did every setting I asked for take". Only `set`/`password` gate it, and only on the "OK" prefix every setter keeps. Diagnostics no longer gate a reboot at all, so a harmless `memory` cannot strand one and no guess is made about "> value". Reboot deferral (P2). CommonCLI dispatches on a six-byte prefix, so `reboot now` and `rebooted` reach Board::reboot() too. Matching exactly meant those variants skipped both the confirmation and the deferral and took the node down mid-drain — the precise failure deferral exists to prevent. Both sides now anchor the way the firmware dispatches, and the UI's risk matcher with them. Three commands the portal cannot honestly serve are refused at POST with a reason, and dropped from autocomplete, instead of running and lying: - `start ota` builds a second AsyncWebServer on port 80 with no bind check and answers "Started" regardless; the portal already holds that port, so it could only leak the allocation and inhibit sleep. - `clock sync` takes its time from the caller's timestamp, which a web request has none of, so CommonCLI always rejected it. `time ` works and remains offered. - bare `log` and `get acl` write their real output to Serial and hand back a stub the terminal showed as success; `log` also streams a whole file from the loop task, stalling the mesh and radio while it does. The mock now emits the same failure shapes it used to fake as successes, so these are reproducible off-hardware. 24 batch + 14 keys tests pass; audit reports 119/119 answered, 0 missing, 4/4 refused with a reason. --- scripts/webconfig_cli_audit.py | 26 ++++++- scripts/webconfig_mock_server.py | 66 +++++++++++++--- src/helpers/WebConfigBatch.h | 63 +++++++++++++++ src/helpers/esp32/WebConfigServer.cpp | 77 +++++++++++++++---- .../test_webconfig_batch.cpp | 61 +++++++++++++++ webui/index.html | 38 +++++---- 6 files changed, 284 insertions(+), 47 deletions(-) diff --git a/scripts/webconfig_cli_audit.py b/scripts/webconfig_cli_audit.py index 4868c705..14294334 100644 --- a/scripts/webconfig_cli_audit.py +++ b/scripts/webconfig_cli_audit.py @@ -73,9 +73,15 @@ COMMAND_SOURCES = [ "examples/simple_repeater/MyMesh.cpp", ] -# Firmware commands the table deliberately does not offer. +# Firmware commands the table deliberately does not offer. Everything below +# except tls.bundletest is also rejected by /api/cli (wcCliUnavailable), so the +# portal never pretends to run something it cannot. NOT_OFFERED = { - "tls.bundletest", # TLS debugging, not an operator command + "tls.bundletest", # TLS debugging, not an operator command + "start ota", # binds port 80, which the portal is already using + "clock sync", # takes its time from the caller; a web request has none + "log", # streams to Serial and stalls the radio ("log start" is offered) + "get acl", # streams to Serial, returns nothing } @@ -202,6 +208,22 @@ def main(): % (getc, got, want, setr["reply"])) failures.append((getc, got)) + # Commands the portal refuses must be refused clearly, not run and fudged. + print("\nrefused with a reason : ", end="") + refused = [] + for cmd in sorted(NOT_OFFERED - {"tls.bundletest"}): + try: + cli._sequence([cmd]) + refused.append((cmd, "was accepted, expected a 400")) + except urllib.error.HTTPError as e: + body = json.load(e) if e.code == 400 else {} + if e.code != 400 or not body.get("error"): + refused.append((cmd, "HTTP %d, expected 400 with a reason" % e.code)) + print("%d/%d" % (len(NOT_OFFERED) - 1 - len(refused), len(NOT_OFFERED) - 1)) + for cmd, why in refused: + print(" FAIL %-30s %s" % (cmd, why)) + failures += refused + print("\n%s" % ("FAILED: %d" % len(failures) if failures else "all clear")) return 1 if failures else 0 diff --git a/scripts/webconfig_mock_server.py b/scripts/webconfig_mock_server.py index e85852c9..5b94ce54 100644 --- a/scripts/webconfig_mock_server.py +++ b/scripts/webconfig_mock_server.py @@ -333,7 +333,8 @@ def apply_set(cfg, key, val): # Strict fallthrough: this function is the single authority on what can be # set, for the batch and the CLI alike. Accepting unknown keys here once hid # the fact that the CLI could not reach `dutycycle` or `radio.fem.rxgain`. - return False, "Error: unknown config key '%s'" % key + # Verbatim shape from CommonCLI::handleSetCmd's fallthrough. + return False, "unknown config: %s" % key # Payload-type names accepted alongside the decimal form. Mirrors @@ -432,7 +433,38 @@ CLI_RESULT_PAGE = 8 # WebConfigBatch::kCliResultPage CLI_CMD_SECS = 0.25 # simulated per-command execution time # Board::reboot() does not return, so the firmware answers `reboot` itself and # arms the deferred reboot once results have been read (see wcIsDeferredReboot). -CLI_DEFERRED_REBOOT = "reboot" +CLI_DEFERRED_REBOOT = "reboot" # matched as a PREFIX, like CommonCLI does + +# Commands the CLI reaches but the portal cannot honestly serve; rejected at +# POST. Mirrors wcCliUnavailable() in WebConfigServer.cpp. +CLI_UNAVAILABLE = [ + ("start ota", True, "start ota needs port 80, which this portal is using. " + "Run it from the serial console, or use `ota update`."), + ("clock sync", True, "clock sync takes its time from the caller, which a web request " + "has no way to supply. Use `time ` instead."), + ("log", False, "log writes the packet log to the serial console, not here, and " + "blocks the radio while it does. Use `log start` / `log stop`."), + ("get acl", False, "get acl writes to the serial console, not here."), +] + + +def cli_unavailable(cmd): + for token, is_prefix, why in CLI_UNAVAILABLE: + if cmd.startswith(token) if is_prefix else cmd == token: + return why + return None + + +# Failure replies CommonCLI emits that do NOT start with "Err" — the shapes that +# made a naive prefix test call them success. Mirrors +# WebConfigBatch::cliReplyIsFailure. +def cli_reply_is_failure(reply): + if not reply: + return False + if reply.startswith(("Err", "ERR", "err", "(ERR", "Unknown command", + "unknown config", "??", "Can't find")): + return True + return ": Err" in reply # Commands the device answers but that have no config-key equivalent. GETTERS = { @@ -497,7 +529,7 @@ def _cli_get_value(cfg, key): return True, SENTINEL if cli_read_key(cfg, key) else "(not set)" val = cli_read_key(cfg, key) if val is None: - return False, "Error: unknown config key '%s'" % key + return False, "??: %s" % key # CommonCLI::handleGetCmd fallthrough return True, str(val) @@ -542,7 +574,7 @@ def run_cli(cfg, line): if cmd in ("poweroff", "shutdown"): return True, "OK - powering off" if cmd == "erase": - return True, "OK - filesystem erased, rebooting" + return True, "File system erase: OK" if cmd == "memory": return True, ("heap free: 142000\nheap min: 118000\n" "largest block: 96000\npsram free: 3980000") @@ -559,8 +591,10 @@ def run_cli(cfg, line): if len(parts) != 2 or not _hex64(parts[0]): return False, "Err - bad params" return True, "OK" - if cmd == "clock sync": - return True, "OK - clock set: %s UTC" % time.strftime("%H:%M - %d/%m/%Y", time.gmtime()) + if cmd.startswith("clock sync"): + # Rejected at POST, but modelled anyway: over the web the caller's + # timestamp is 0, so CommonCLI always takes this branch. + return False, "(ERR: clock cannot go backwards)" if cmd == "region": return True, "US915" if cmd == "sensor list": @@ -628,7 +662,7 @@ def run_cli(cfg, line): # key is *readable* rejected write-only and computed ones (`dutycycle`, # `prv.key`, `radio.fem.rxgain`). return apply_set(cfg, key, val.strip()) - return False, "Error: unknown command '%s'" % cmd[:40] + return False, "Unknown command" def valid_reqid(reqid): @@ -859,6 +893,9 @@ class Handler(BaseHTTPRequestHandler): for c in cmds: if len(c) > BATCH_CMD_SIZE - 1: return self._json(400, {"error": "command too long"}) + why = cli_unavailable(c) + if why: + return self._json(400, {"error": why}) with ST.lock: self._cli_advance(ST.cli) @@ -868,7 +905,8 @@ class Handler(BaseHTTPRequestHandler): if ST.cli.get("state") == "running": return self._json(409, {"error": "busy", "reqid": ST.cli.get("reqid", "")}) ST.cli = {"state": "running", "reqid": reqid, "cmds": cmds, "results": [], - "all_ok": True, "reboot": CLI_DEFERRED_REBOOT in cmds, + "all_ok": True, + "reboot": any(c.startswith(CLI_DEFERRED_REBOOT) for c in cmds), "next_at": time.time() + CLI_CMD_SECS} return self._json(202, {"state": "running", "reqid": reqid, "total": len(cmds)}) @@ -881,13 +919,17 @@ class Handler(BaseHTTPRequestHandler): while (job.get("state") == "running" and len(job["results"]) < len(job["cmds"]) and now >= job["next_at"]): cmd = job["cmds"][len(job["results"])] - if cmd == CLI_DEFERRED_REBOOT: - ok, reply = True, "OK - reboot queued" + if cmd.startswith(CLI_DEFERRED_REBOOT): + reply = "OK - reboot queued" else: - ok, reply = run_cli(ST.cfg, cmd) + _, reply = run_cli(ST.cfg, cmd) if cmd.startswith("password "): reply = "OK" # never echo the new password back - job["all_ok"] = job.get("all_ok", True) and ok + ok = not cli_reply_is_failure(reply) + # Only writes gate the reboot, and only on the "OK" convention every + # setter keeps (WebConfigBatch::cliReplyGatesReboot). + if cmd.startswith(("set ", "password ")): + job["all_ok"] = job.get("all_ok", True) and reply.startswith("OK") # The command is NOT echoed: it may carry a password or token, and # the client matches results to its own sequence by index. job["results"].append({"ok": ok, "reply": reply}) diff --git a/src/helpers/WebConfigBatch.h b/src/helpers/WebConfigBatch.h index c69562e2..5532bbad 100644 --- a/src/helpers/WebConfigBatch.h +++ b/src/helpers/WebConfigBatch.h @@ -1,5 +1,6 @@ #pragma once +#include // size_t / NULL for the reply classifiers below #include // Fork-owned, dependency-free spec for the WebConfig "config batch / reboot / @@ -210,6 +211,68 @@ static inline bool cliRebootAllowed(bool has_reboot, bool all_ok) { return has_reboot && all_ok; } +// CommonCLI has no single failure convention. Testing only for an "Err" prefix +// let five other shapes through as success — including "Unknown command" and +// "unknown config: x", the two an operator hits most — which coloured them +// green AND let a queued reboot proceed after them. +// +// Every shape below is a literal from CommonCLI.cpp / CommonCLI_Observer.cpp. +// This list is the fragile part of the design: a new failure string added there +// is silently a success here. Which is exactly why it must not be what decides +// whether to reboot — see cliReplyConfirmsWrite. +static inline bool cliReplyIsFailure(const char* r) { + if (r == NULL || r[0] == 0) return false; // empty is normalised to "OK" + static const char* const kPrefixes[] = { + "Err", "ERR", "err", // "Err - ", "ERR: ", "Error: " + "(ERR", // "(ERR: clock cannot go backwards)" + "Unknown command", + "unknown config", + "??", // "??: " from the get fallthrough + "Can't find", // "Can't find GPS" + }; + for (size_t i = 0; i < sizeof(kPrefixes) / sizeof(kPrefixes[0]); i++) { + const char* p = kPrefixes[i]; + size_t n = 0; + while (p[n]) n++; + bool match = true; + for (size_t j = 0; j < n; j++) { + if (r[j] != p[j]) { match = false; break; } + } + if (match) return true; + } + // "File system erase: Err" reports the failure at the END of the reply. + for (size_t i = 0; r[i]; i++) { + if (r[i] == ':' && r[i + 1] == ' ' && r[i + 2] == 'E' && r[i + 3] == 'r' && + r[i + 4] == 'r') return true; + } + return false; +} + +// Whether a command's reply is allowed to gate the deferred reboot. +// +// Only writes are, and only writes have a reply convention worth trusting: +// every setter answers with an "OK" prefix. Diagnostics do not — `memory` +// answers "Free: ...", a getter answers "> value" — so letting them gate would +// mean guessing, and guessing wrong here either strands the operator (a +// harmless `memory` blocks their reboot) or reboots into a config that did not +// apply. The question the gate exists to answer is narrower than "did anything +// fail": it is "did every setting I asked for actually take". +static inline bool cliReplyGatesReboot(const char* cmd) { + if (cmd == NULL) return false; + const char* set = "set "; + const char* pwd = "password "; + bool is_set = true, is_pwd = true; + for (int i = 0; i < 4; i++) if (cmd[i] != set[i]) { is_set = false; break; } + for (int i = 0; i < 9; i++) if (cmd[i] != pwd[i]) { is_pwd = false; break; } + return is_set || is_pwd; +} + +// A write took effect iff its reply starts with "OK" — the one convention every +// setter in CommonCLI actually keeps. +static inline bool cliWriteSucceeded(const char* reply) { + return reply != NULL && reply[0] == 'O' && reply[1] == 'K'; +} + // -------------------------------------------------------------------------- // Reboot fire (.cpp:262-265) and isRebootPending (.cpp:70-74). // -------------------------------------------------------------------------- diff --git a/src/helpers/esp32/WebConfigServer.cpp b/src/helpers/esp32/WebConfigServer.cpp index 3eb0fb4e..903036cd 100644 --- a/src/helpers/esp32/WebConfigServer.cpp +++ b/src/helpers/esp32/WebConfigServer.cpp @@ -34,8 +34,44 @@ static inline bool isSecretKey(const char* key) { return wcIsSecretKey(key); } // path once the operator has read the results. The rest (clkreboot, poweroff, // ota update) do real work on the way down and cannot be faked, so they run // normally and the connection drops — the UI warns before sending them. +// CommonCLI dispatches on a 6-byte PREFIX (memcmp(command, "reboot", 6)), so +// `reboot now` and `rebooted` reach Board::reboot() too. Matching exactly here +// let those through to the CLI, which took the node down mid-drain with no +// results and no deferral — the precise failure the deferral exists to avoid. +// Whatever CommonCLI would treat as a reboot, this must intercept. static inline bool wcIsDeferredReboot(const char* cmd) { - return strcmp(cmd, "reboot") == 0; + return strncmp(cmd, "reboot", 6) == 0; +} + +// Commands the CLI reaches but the portal cannot honestly serve. Rejected at +// POST so nothing in the sequence runs, rather than failing halfway with a +// reply that does not explain itself. Returns the reason, or NULL if fine. +static const char* wcCliUnavailable(const char* cmd) { + // ESP32Board::startOTAUpdate() does `new AsyncWebServer(80)` with no bind + // check and answers "Started" regardless. The portal already holds port 80, + // so from here it can only leak the allocation, inhibit sleep, and lie. + if (strncmp(cmd, "start ota", 9) == 0) { + return "start ota needs port 80, which this portal is using. " + "Run it from the serial console, or use `ota update`."; + } + // `clock sync` sets the clock from the CALLER's timestamp. Web requests carry + // none (execCommand passes 0), so CommonCLI always rejects it as moving the + // clock backwards. `time ` is the one that works over this transport. + if (strncmp(cmd, "clock sync", 10) == 0) { + return "clock sync takes its time from the caller, which a web request has " + "no way to supply. Use `time ` instead."; + } + // Both write their real output to Serial and hand back a stub the terminal + // would render as success. Bare `log` also streams a whole file from the loop + // task, stalling the mesh and the radio while it does. + if (strcmp(cmd, "log") == 0) { + return "log writes the packet log to the serial console, not here, and " + "blocks the radio while it does. Use `log start` / `log stop`."; + } + if (strcmp(cmd, "get acl") == 0) { + return "get acl writes to the serial console, not here."; + } + return NULL; } // The `password` command echoes the new password back in its reply, and replies // are served to the client over the open setup AP. The config path overwrites it @@ -43,16 +79,9 @@ static inline bool wcIsDeferredReboot(const char* cmd) { static inline bool wcCliEchoesSecret(const char* cmd) { return strncmp(cmd, "password ", 9) == 0; } -// Success has no single shape across the CLI: setters answer "OK...", getters -// answer "> value", and a few answer free-form ("File system erase: OK"). Only -// FAILURE is uniform — an "Err"/"ERR:"/"Error:" prefix. So the CLI must test for -// the failure prefix, where the config batch can test for "OK" because every -// allowlisted setter uses it. Testing for "OK" here marked every `get` a -// failure, which turned the terminal red and withheld requested reboots. -static inline bool wcCliReplyIsOk(const char* r) { - return !((r[0] == 'E' || r[0] == 'e') && (r[1] == 'R' || r[1] == 'r') && - (r[2] == 'R' || r[2] == 'r')); -} +// Reply classification lives in WebConfigBatch.h with the rest of the decisions +// (WebConfigBatch::cliReplyIsFailure / cliReplyGatesReboot / cliWriteSucceeded), +// so the shapes CommonCLI actually emits are enumerated in one host-tested place. // Constant-time-ish comparison so login timing doesn't leak a prefix match. static bool fixedTimeEquals(const char* a, const char* b, size_t max_len) { @@ -359,11 +388,14 @@ void WebConfigServer::drainBatch(uint32_t now) { // Overwrite it: the command cannot fail, so there is nothing to report. // Config entries carry the key; a CLI entry is matched on the command. if (wcIsAdminPasswordKey(e.key) || wcCliEchoesSecret(e.cmd)) strcpy(e.reply, "OK"); - // A config batch is all allowlisted setters, which uniformly answer "OK"; - // the CLI reaches the whole surface, where only failure has a fixed shape. - _batch_all_ok = WebConfigBatch::nextAllOk( - _batch_all_ok, _batch_kind == BATCH_CLI ? wcCliReplyIsOk(e.reply) - : strncmp(e.reply, "OK", 2) == 0); + // What gates the reboot is narrower than "did anything fail": only a + // write can leave the node in a config not worth rebooting into, and only + // a write has a reply convention ("OK") solid enough to test. Diagnostics + // in the sequence neither gate it nor get guessed at. + if (_batch_kind != BATCH_CLI || WebConfigBatch::cliReplyGatesReboot(e.cmd)) { + _batch_all_ok = WebConfigBatch::nextAllOk( + _batch_all_ok, WebConfigBatch::cliWriteSucceeded(e.reply)); + } } _batch_last_cmd = millis(); // Config entries are named by their (non-secret) key. A CLI command is @@ -1040,6 +1072,17 @@ void WebConfigServer::handleCliPost(AsyncWebServerRequest* req) { while (pos > 0 && (e.cmd[pos - 1] == ' ' || e.cmd[pos - 1] == '\t')) pos--; e.cmd[pos] = 0; if (pos == 0) continue; + // Reject before anything runs, so a sequence never half-applies and then + // stops on a command that was never going to work here. + const char* why = wcCliUnavailable(e.cmd); + if (why) { + StaticJsonDocument<256> ed; + ed["error"] = why; + String out; + serializeJson(ed, out); + req->send(400, "application/json", out); + return; + } e.key[0] = 0; // CLI entries have no config key if (wcIsDeferredReboot(e.cmd)) defer_reboot = true; count++; @@ -1125,7 +1168,7 @@ void WebConfigServer::handleCliResult(AsyncWebServerRequest* req) { JsonObject r = results.createNestedObject(); // The command is deliberately NOT echoed: it may hold a password or token, // and the client already has the sequence it sent. It matches by index. - r["ok"] = wcCliReplyIsOk(_batch[i].reply); + r["ok"] = !WebConfigBatch::cliReplyIsFailure(_batch[i].reply); r["reply"] = (const char*)_batch[i].reply; } if (final_read) { diff --git a/test/test_webconfig_batch/test_webconfig_batch.cpp b/test/test_webconfig_batch/test_webconfig_batch.cpp index 2b6deb82..813412c5 100644 --- a/test/test_webconfig_batch/test_webconfig_batch.cpp +++ b/test/test_webconfig_batch/test_webconfig_batch.cpp @@ -221,6 +221,67 @@ TEST(WebConfigBatch, CliReadIsDoneOnlyOnceEveryResultHasBeenHandedOver) { EXPECT_TRUE(Batch::cliReadIsFinal(State::Done, /*from=*/20, /*page=*/0, /*total=*/20)); } +// Every string below is a literal lifted from CommonCLI.cpp / +// CommonCLI_Observer.cpp. Testing only for an "Err" prefix passed five of these +// off as success, which both coloured them green and let a queued reboot go +// ahead after them. +TEST(WebConfigBatch, CliFailureRepliesAreRecognisedInEveryShapeCommonCLIEmits) { + const char* failures[] = { + "Err - bad params", // MyMesh setperm + "ERR: bad pubkey", // neighbor.remove + "Error: IATA code must be exactly 3 letters",// observer setters + "(ERR: clock cannot go backwards)", // clock sync, parenthesised + "Unknown command", // top-level fallthrough + "unknown config: mqtt.nope", // set fallthrough + "??: mqtt.nope", // get fallthrough + "Can't find GPS", // gps + "File system erase: Err", // failure reported at the end + }; + for (const char* f : failures) { + EXPECT_TRUE(Batch::cliReplyIsFailure(f)) << f; + } + + const char* successes[] = { + "OK", + "OK - slot 1 preset: meshrank", + "> 22", // getter value + "> msgs: on, 1: analyzer-us (ok)", // getter, contains "ok" + "File system erase: OK", // same shape, succeeded + "Free: 142832, Min: 126808", // memory + "v1.16.0 (Build: 6 Jun 2026)", // ver + }; + for (const char* s : successes) { + EXPECT_FALSE(Batch::cliReplyIsFailure(s)) << s; + } + // An empty reply is normalised to "OK" before it ever reaches the client. + EXPECT_FALSE(Batch::cliReplyIsFailure("")); + EXPECT_FALSE(Batch::cliReplyIsFailure(NULL)); +} + +TEST(WebConfigBatch, OnlyWritesGateTheDeferredReboot) { + // Writes gate it: these are what can leave a config not worth rebooting into. + EXPECT_TRUE(Batch::cliReplyGatesReboot("set tx 22")); + EXPECT_TRUE(Batch::cliReplyGatesReboot("set mqtt1.preset meshrank")); + EXPECT_TRUE(Batch::cliReplyGatesReboot("password hunter2")); + // Diagnostics do not. `memory` answering "Free: ..." must not be read as a + // failure and strand the operator's reboot, and a getter's "> value" must not + // be read as a success either — neither is asked. + EXPECT_FALSE(Batch::cliReplyGatesReboot("memory")); + EXPECT_FALSE(Batch::cliReplyGatesReboot("get tx")); + EXPECT_FALSE(Batch::cliReplyGatesReboot("reboot")); + EXPECT_FALSE(Batch::cliReplyGatesReboot("advert")); + EXPECT_FALSE(Batch::cliReplyGatesReboot(NULL)); + // "settle" must not be mistaken for a `set`; the space is part of the token. + EXPECT_FALSE(Batch::cliReplyGatesReboot("settle")); + + // A write counts only on the "OK" prefix every setter keeps. + EXPECT_TRUE(Batch::cliWriteSucceeded("OK")); + EXPECT_TRUE(Batch::cliWriteSucceeded("OK - reboot to apply")); + EXPECT_FALSE(Batch::cliWriteSucceeded("unknown config: nope")); + EXPECT_FALSE(Batch::cliWriteSucceeded("Error: expected a number")); + EXPECT_FALSE(Batch::cliWriteSucceeded("")); +} + TEST(WebConfigBatch, CliRebootIsWithheldWhenAnyCommandInTheSequenceFailed) { EXPECT_TRUE(Batch::cliRebootAllowed(/*has_reboot=*/true, /*all_ok=*/true)); // Same rule a config save follows: do not reboot into a half-applied config diff --git a/webui/index.html b/webui/index.html index 7d4a1107..77174b1a 100644 --- a/webui/index.html +++ b/webui/index.html @@ -1440,7 +1440,8 @@ var CLI_VERBS=[ ["ver","Firmware version"], ["board","Board and hardware info"], ["clock","Show the device clock (UTC)"], - ["clock sync","Set the device clock from this browser"], + // `clock sync` is absent on purpose: it takes its time from the caller's + // timestamp, which a web request has none of, so it can only ever fail here. ["time ","Set the clock {epoch-seconds}"], ["region","Show the configured region"], ["memory","Heap and PSRAM free/min/largest block"], @@ -1459,7 +1460,8 @@ var CLI_VERBS=[ ["stats-packets","Per-packet-type counters"], ["stats-radio","Radio counters (RSSI, SNR, noise)"], ["stats-radio-diag","Extended radio diagnostics"], - ["log","Show the packet log summary"], + // Bare `log` is absent: it streams the file to the serial console and stalls + // the radio doing it, and hands back only "EOF". ["log start","Start packet logging to the filesystem"], ["log stop","Stop packet logging"], ["log erase","Delete the stored packet logs"], @@ -1480,7 +1482,7 @@ var CLI_VERBS=[ ["alert test","Send a test alert on the configured channel"], ["ota check","Check for a newer build (does not flash)"], ["ota update","Download and flash the newer build, then reboot"], - ["start ota","Raise the manual firmware-upload AP"], + // `start ota` is absent: it binds port 80, which this portal is already using. ["start webconfig","Start this portal on the LAN"], ["start webconfig ap","Start this portal on its own setup AP"], ["stop webconfig","Stop this portal"], @@ -1498,7 +1500,7 @@ var CLI_KEYS=[ ["lat","Advert latitude",0], ["lon","Advert longitude",0], ["public.key","This node's public key",1], - ["acl","Access control list (per-node permissions)",1], + // `get acl` is absent: it prints to the serial console and returns nothing. ["prv.key","Restore an identity {64-hex-char-key}",2], ["role","Node role",1], ["radio","Radio parameters {freq,bw,sf,cr}",0], @@ -1852,18 +1854,21 @@ function cliParse(text){ /* ---------- CLI: confirmation ---------- Rendered into the scrollback rather than as a modal: it reads as part of the session, and on a phone it can't end up behind the keyboard. */ +// Anchored the way CommonCLI dispatches, which is mostly on a PREFIX: `reboot` +// matches the first six bytes, so `reboot now` reboots too. Matching these +// exactly (/^reboot$/) let those variants skip the confirmation entirely. var CLI_RISK=[ [/^erase$/,"erases the filesystem — stored settings and this node's identity"], - [/^(reboot|clkreboot)$/,"restarts the node"], - [/^(poweroff|shutdown)$/,"powers the node off"], - [/^ota update$/,"downloads and flashes new firmware, then reboots"], - [/^start ota$/,"drops this portal and raises the firmware-upload AP"], + [/^clkreboot/,"resets the clock and restarts the node"], + [/^reboot/,"restarts the node"], + [/^(poweroff|shutdown)/,"powers the node off"], + [/^ota update/,"downloads and flashes new firmware, then reboots"], [/^stop webconfig$/,"stops this portal"], - [/^set wifi\.(ssid|pwd)\b/,"changes WiFi — this page will drop"], - [/^set radio\b/,"changes radio parameters — a wrong value takes this node off the air"], - [/^set freq\b/,"changes the frequency — a wrong value takes this node off the air"], - [/^password\b/,"changes the admin password"], - [/^set prv\.key\b/,"replaces this node's identity"] + [/^set wifi\.(ssid|pwd)\s/,"changes WiFi — this page will drop"], + [/^set radio\s/,"changes radio parameters — a wrong value takes this node off the air"], + [/^set freq\s/,"changes the frequency — a wrong value takes this node off the air"], + [/^password\s/,"changes the admin password"], + [/^set prv\.key\s/,"replaces this node's identity"] ]; function cliRisks(cmds){ var seen={},out=[]; @@ -2019,13 +2024,14 @@ function cliAfter(cmds,r){ if(/^password\s/.test(c))cli.pwd=true; if(/^(set|password)\s/.test(c))touched=true; }); + // Same prefix dispatch as the risk list: `poweroff now` powers off too. if(touched)loadConfigSoft(); // A `reboot` in the sequence is not run by the CLI — Board::reboot() never // returns, so the node answers it and schedules the restart for after this // read. It withholds it when any command failed, exactly as a config save // does; say which happened rather than leaving the operator to guess. if(r&&r.reboot_withheld){ - cliEcho("err","Not rebooting — some commands failed. Fix them and run \"reboot\" again."); + cliEcho("err","Not rebooting — a setting was rejected. Fix it and run \"reboot\" again."); return; } if(r&&r.reboot){ @@ -2035,8 +2041,8 @@ function cliAfter(cmds,r){ // clkreboot / poweroff / erase-and-flash take the node down themselves, so // there is no result to wait for. cmds.forEach(function(c){ - if(/^(poweroff|shutdown)$/.test(c))showReboot("The node is powering off. Reconnect power to bring it back.",false,"Powering off…"); - else if(/^clkreboot$/.test(c))showReboot("The node is restarting. This page will try to reconnect automatically.",true); + if(/^(poweroff|shutdown)/.test(c))showReboot("The node is powering off. Reconnect power to bring it back.",false,"Powering off…"); + else if(/^clkreboot/.test(c))showReboot("The node is restarting. This page will try to reconnect automatically.",true); }); }