From d7109c185caab4011c637719a025c6437fb7b13a Mon Sep 17 00:00:00 2001 From: agessaman Date: Fri, 7 Aug 2026 23:16:38 -0700 Subject: [PATCH] feat(webconfig): implement /api/cli on the device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal has been driving the mock since it was built. This is the firmware side, so it works on hardware. Same 202 + reqid + poll contract as a config save, for the same reason: CommonCLI touches prefs, the radio and the filesystem, none of which may be reached from the async_tcp task. Commands go into the deferred slot and tick() drains them on the loop task. Unlike a save this is not allowlisted — reaching what the serial console reaches is the point, and execCommand() already passes sender_timestamp 0, so the terminal gets exactly the serial console's privilege. Authentication is the boundary, as it is there. The CLI shares the config batch's slot rather than owning a second MAX_BATCH array: both drain on the loop task, both are single-slot, and a duplicate would cost ~8 KB of permanently resident RAM. Sharing also makes a save and a CLI run mutually exclusive, which they must be. Each reader checks the kind, so neither can serve the other's results. Three things the mock could not have taught us: - Board::reboot() does not return, so a drained `reboot` would take the node down before the client read a single result. It is answered rather than executed, and the batch arms the existing deferred-reboot path once the results have been read — withheld if any command failed, exactly as a save withholds one. clkreboot/poweroff/ota update do real work on the way down and cannot be faked, so they still drop the connection; the UI warns first. - `password ` echoes the new password in its reply. The config path already scrubbed that by key; a CLI entry has no key, so it is matched on the command. CLI commands are also kept out of the serial log entirely — the browser session and the serial console are different audiences. - MAX_BATCH is 24, not the 64 the page assumed. It is reported as status.max_cmds instead of hardcoded, so the cap cannot drift. Results stream and page (kCliResultPage = 8), and "done" means the client has been handed every result, not merely that execution finished — otherwise a client that stops polling at "done" loses the last page. Commands are never echoed back: they may carry a secret, and the client matches by index. New decisions live in WebConfigBatch.h with the rest, covered by three host tests. Builds clean for heltec_v4_repeater_observer_mqtt; 22 batch + 14 keys tests pass; the CLI audit reports 119/119 against the updated mock. --- scripts/webconfig_cli_audit.py | 32 ++- scripts/webconfig_mock_server.py | 44 ++- src/helpers/WebConfigBatch.h | 42 +++ src/helpers/esp32/WebConfigServer.cpp | 269 +++++++++++++++++- src/helpers/esp32/WebConfigServer.h | 12 +- .../test_webconfig_batch.cpp | 41 +++ webui/index.html | 43 ++- 7 files changed, 446 insertions(+), 37 deletions(-) diff --git a/scripts/webconfig_cli_audit.py b/scripts/webconfig_cli_audit.py index ca88ff6d..1bca892f 100644 --- a/scripts/webconfig_cli_audit.py +++ b/scripts/webconfig_cli_audit.py @@ -82,6 +82,9 @@ class Client: self.base = base r = self._open("/api/login", b'{"password":"password"}') self.cookie = r.headers["Set-Cookie"].split(";")[0] + # The node caps a sequence at MAX_BATCH and reports it; chunk to match + # rather than hardcoding a number that drifts when the slot is resized. + self.max_cmds = json.load(self._open("/api/status")).get("max_cmds", 24) def _open(self, path, data=None): headers = {"Content-Type": "application/json"} @@ -91,10 +94,16 @@ class Client: self.base + path, data=data, headers=headers, method="POST" if data is not None else "GET")) - def run(self, cmds, chunk=40): + def run(self, cmds): + """[(command, result)]. The node never echoes the command back — it may + carry a secret — so results pair with what was sent, by index.""" out = [] - for i in range(0, len(cmds), chunk): - out += self._sequence(cmds[i:i + chunk]) + for i in range(0, len(cmds), self.max_cmds): + chunk = cmds[i:i + self.max_cmds] + results = self._sequence(chunk) + if len(results) != len(chunk): + sys.exit("node returned %d results for %d commands" % (len(results), len(chunk))) + out += list(zip(chunk, results)) return out def _sequence(self, cmds): @@ -108,10 +117,14 @@ class Client: if e.code != 409: raise time.sleep(0.5) + # Results stream and page, so keep reading from a cursor until the node + # says done — "done" arrives only once every result has been handed over. + out = [] while True: - r = json.load(self._open("/api/cli/result?reqid=" + reqid)) + r = json.load(self._open("/api/cli/result?reqid=%s&from=%d" % (reqid, len(out)))) + out += r.get("results", []) if r["state"] == "done": - return r["results"] + return out time.sleep(0.05) @@ -126,15 +139,16 @@ def main(): cmds = table() unexpected = [] - for res in cli.run(cmds): + for cmd, res in cli.run(cmds): if res["ok"]: continue - want = EXPECTED_FAILURES.get(res["cmd"]) + want = EXPECTED_FAILURES.get(cmd) if want and want in res["reply"]: continue - unexpected.append((res["cmd"], res["reply"])) + unexpected.append((cmd, res["reply"])) print("commands offered by autocomplete : %d" % len(cmds)) print("answered : %d" % (len(cmds) - len(unexpected))) + print("sequence cap reported by the node: %d" % cli.max_cmds) for cmd, reply in unexpected: print(" FAIL %-30s %s" % (cmd, reply)) failures += unexpected @@ -142,7 +156,7 @@ def main(): results = cli.run([c for probe in ROUND_TRIPS for c in probe[:2]]) print("\nround-trips : %d" % len(ROUND_TRIPS)) for i, (setc, getc, want) in enumerate(ROUND_TRIPS): - setr, getr = results[i * 2], results[i * 2 + 1] + setr, getr = results[i * 2][1], results[i * 2 + 1][1] if setr["ok"] and getr["reply"] == want: continue print(" FAIL %-30s got %r, wanted %r (set: %s)" diff --git a/scripts/webconfig_mock_server.py b/scripts/webconfig_mock_server.py index 4623f9e5..103204bd 100644 --- a/scripts/webconfig_mock_server.py +++ b/scripts/webconfig_mock_server.py @@ -182,6 +182,7 @@ class State: "fw": "v1.7.1-mock", "role": "Repeater", "board": "Heltec V3 (mock)", "uptime_s": int(time.time() - self.start), "runtime_slots": 6, "max_slots": 6, "active_slots": self.active_slots, + "max_cmds": CLI_MAX_CMDS, } @@ -424,8 +425,14 @@ def is_secret_key(key): # the same surface the serial console reaches. Auth is the boundary — exactly # as it is for the serial console and for remote admin over the mesh. # --------------------------------------------------------------------------- -CLI_MAX_CMDS = 64 # per POSTed sequence +# MAX_BATCH in WebConfigServer.h: the CLI shares the config batch's fixed slot, +# so this is the real cap, reported to the page as status.max_cmds. +CLI_MAX_CMDS = 24 +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" # Commands the device answers but that have no config-key equivalent. GETTERS = { @@ -821,14 +828,14 @@ class Handler(BaseHTTPRequestHandler): if not isinstance(cmds, list) or not cmds: return self._json(400, {"error": "no commands"}) if len(cmds) > CLI_MAX_CMDS: - return self._json(413, {"error": "too many commands (max %d)" % CLI_MAX_CMDS}) + return self._json(413, {"error": "too many commands", "max": CLI_MAX_CMDS}) cmds = [str(c).replace("\r", "").replace("\n", "").strip() for c in cmds] cmds = [c for c in cmds if c] if not cmds: return self._json(400, {"error": "no commands"}) for c in cmds: if len(c) > BATCH_CMD_SIZE - 1: - return self._json(400, {"error": "command too long", "cmd": c[:32]}) + return self._json(400, {"error": "command too long"}) with ST.lock: self._cli_advance(ST.cli) @@ -838,6 +845,7 @@ 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, "next_at": time.time() + CLI_CMD_SECS} return self._json(202, {"state": "running", "reqid": reqid, "total": len(cmds)}) @@ -850,8 +858,16 @@ 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"])] - ok, reply = run_cli(ST.cfg, cmd) - job["results"].append({"cmd": cmd, "ok": ok, "reply": reply}) + if cmd == CLI_DEFERRED_REBOOT: + ok, reply = True, "OK - reboot queued" + else: + ok, 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 + # 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}) job["next_at"] = now + CLI_CMD_SECS if job.get("state") == "running" and len(job["results"]) == len(job["cmds"]): job["state"] = "done" # stays readable until the next POST @@ -874,10 +890,20 @@ class Handler(BaseHTTPRequestHandler): if j.get("reqid") != reqid: return self._json(404, {"error": "unknown request"}) self._cli_advance(j) # one command per CLI_CMD_SECS - return self._json(200, { - "state": j["state"], "reqid": reqid, "total": len(j["cmds"]), - "from": frm, "results": j["results"][frm:], - }) + # Results stream, capped per read so the device's JSON document + # stays small; a longer sequence pages across reads. "done" means + # the client has been handed everything, not just that execution + # finished — a client that stops polling at "done" must lose nothing. + page = j["results"][frm:frm + CLI_RESULT_PAGE] + final = j["state"] == "done" and frm + len(page) >= len(j["cmds"]) + body = {"state": "done" if final else "running", "reqid": reqid, + "total": len(j["cmds"]), "from": frm, "results": page} + if final: + body["all_ok"] = j["all_ok"] + body["reboot"] = j["reboot"] and j["all_ok"] + if j["reboot"] and not j["all_ok"]: + body["reboot_withheld"] = True + return self._json(200, body) def _scan(self): rescan = "rescan=1" in self.path diff --git a/src/helpers/WebConfigBatch.h b/src/helpers/WebConfigBatch.h index 2f93f56d..c69562e2 100644 --- a/src/helpers/WebConfigBatch.h +++ b/src/helpers/WebConfigBatch.h @@ -168,6 +168,48 @@ static inline uint32_t confirmRebootAt(uint32_t now) { return scheduleAt(now, kRebootConfirmMs); } +// -------------------------------------------------------------------------- +// CLI sequences (/api/cli). The terminal shares this one deferred-command slot +// with config saves rather than owning a second MAX_BATCH array: both drain on +// the loop task, both are single-slot, and a duplicate would cost ~8 KB of +// permanently resident RAM. Sharing also makes a save and a CLI run mutually +// exclusive, which they must be. +// +// Two things differ from a config save: +// 1. results stream. A save's results appear only when the whole batch is +// Done; a CLI read hands back whatever has executed so far, so a pasted +// sequence fills the terminal command by command. +// 2. the reboot is not requested by a `reboot` flag on the request but by the +// word `reboot` appearing in the sequence. It is deferred rather than +// executed, because Board::reboot() does not return and would take the node +// down before the client could read a single result. +// -------------------------------------------------------------------------- + +// Results returned by one read. Bounds the JSON document built on the +// async_tcp task; a longer sequence pages across successive reads. +static const int kCliResultPage = 8; + +static inline int cliPageCount(int from, int produced, int page) { + const int pending = produced - from; + if (pending <= 0) return 0; + return pending > page ? page : pending; +} + +// "done" means the client has been handed every result, not merely that +// execution finished: the last page may still be unread, and a client that +// stops polling at "done" would lose it. +static inline bool cliReadIsFinal(State state, int from, int page_count, int total) { + return state == State::Done && from + page_count >= total; +} + +// A trailing `reboot` is withheld when any command in the sequence failed, +// exactly as a config save's is. The operator asked for the reboot, but +// rebooting into a half-applied config — over a link they may not get back — +// is the worse failure, and the result body reports the refusal. +static inline bool cliRebootAllowed(bool has_reboot, bool all_ok) { + return has_reboot && all_ok; +} + // -------------------------------------------------------------------------- // 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 b9108001..92ab5e89 100644 --- a/src/helpers/esp32/WebConfigServer.cpp +++ b/src/helpers/esp32/WebConfigServer.cpp @@ -28,6 +28,22 @@ static const char SECRET_SENTINEL[] = "********"; static inline bool isAllowedSetKey(const char* key) { return wcIsAllowedSetKey(key); } static inline bool isSecretKey(const char* key) { return wcIsSecretKey(key); } +// Commands whose CLI handler never returns would take the node down mid-drain, +// before the client could read a single result. `reboot` is deferred instead: +// it is not passed to the CLI at all, and the batch arms the ordinary reboot +// 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. +static inline bool wcIsDeferredReboot(const char* cmd) { + return strcmp(cmd, "reboot") == 0; +} +// 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 +// by key; a CLI entry has no key, so match on the command itself. +static inline bool wcCliEchoesSecret(const char* cmd) { + return strncmp(cmd, "password ", 9) == 0; +} + // 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) { size_t la = strnlen(a, max_len), lb = strnlen(b, max_len); @@ -315,20 +331,41 @@ void WebConfigServer::drainBatch(uint32_t now) { // WiFi regardless, so a concurrent GET waiting on it costs nothing extra. { WCLock lock(_mux); - _cb->execCommand(e.cmd, e.reply); + // A CLI `reboot` is answered here rather than executed: Board::reboot() + // does not return, so running it would take the node down before the + // operator could read whether the preceding commands succeeded. The + // batch arms the ordinary deferred reboot on the first result read. + if (_batch_kind == BATCH_CLI && wcIsDeferredReboot(e.cmd)) { + // Deliberately non-committal: whether the reboot actually happens is + // not known until the whole sequence has run (it is withheld if any + // command failed), and a later command could still fail after this one. + strcpy(e.reply, "OK - reboot queued"); + } else { + _cb->execCommand(e.cmd, e.reply); + } if (e.reply[0] == 0) strcpy(e.reply, "OK"); // The upstream `password` command echoes the new password back in its // reply, and replies are served to the client over the open setup AP. // Overwrite it: the command cannot fail, so there is nothing to report. - if (wcIsAdminPasswordKey(e.key)) strcpy(e.reply, "OK"); + // Config entries carry the key; a CLI entry is matched on the command. + if (wcIsAdminPasswordKey(e.key) || wcCliEchoesSecret(e.cmd)) 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. _batch_all_ok = WebConfigBatch::nextAllOk(_batch_all_ok, strncmp(e.reply, "OK", 2) == 0); } _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)); + // Config entries are named by their (non-secret) key. A CLI command is + // deliberately not logged: the operator can see what they typed, and a + // `set wifi.pwd` or `password` from the terminal must not reach the serial + // log, which is a different audience from the browser session. + if (_batch_kind == BATCH_CLI) { + Serial.printf("WC: cli %d/%d took %lums\n", (int)_batch_next, (int)_batch_count, + (unsigned long)(_batch_last_cmd - t0)); + } else { + Serial.printf("WC: cmd %d/%d '%s' took %lums\n", (int)_batch_next, (int)_batch_count, + e.key, (unsigned long)(_batch_last_cmd - t0)); + } if (!WebConfigBatch::drainFinished(_batch_next, _batch_count)) { return; // more commands next tick } @@ -418,6 +455,10 @@ void WebConfigServer::registerRoutes() { _server->on("/api/config", HTTP_GET, [](AsyncWebServerRequest* r) { dispatchRequest(r, &WebConfigServer::handleConfigGet); }); _server->on("/api/config", HTTP_POST, [](AsyncWebServerRequest* r) { dispatchRequest(r, &WebConfigServer::handleConfigPost); }, NULL, collectBody); + // Same specific-route-first rule as /api/config above. + _server->on("/api/cli/result", HTTP_GET, [](AsyncWebServerRequest* r) { dispatchRequest(r, &WebConfigServer::handleCliResult); }); + _server->on("/api/cli", HTTP_POST, [](AsyncWebServerRequest* r) { dispatchRequest(r, &WebConfigServer::handleCliPost); }, + NULL, collectBody); _server->on("/api/stats", HTTP_GET, [](AsyncWebServerRequest* r) { dispatchRequest(r, &WebConfigServer::handleStats); }); _server->on("/api/scan", HTTP_GET, [](AsyncWebServerRequest* r) { dispatchRequest(r, &WebConfigServer::handleScan); }); _server->on("/api/reboot", HTTP_POST, [](AsyncWebServerRequest* r) { dispatchRequest(r, &WebConfigServer::handleReboot); }); @@ -495,6 +536,10 @@ void WebConfigServer::handleStatus(AsyncWebServerRequest* req) { // 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(); + // Commands the terminal may submit at once. The CLI shares the config + // batch's fixed slot, so the cap is MAX_BATCH — reported rather than + // duplicated in the page, which cannot know how this build was sized. + doc["max_cmds"] = MAX_BATCH; AsyncResponseStream* res = req->beginResponseStream("application/json"); serializeJson(doc, *res); @@ -781,6 +826,7 @@ void WebConfigServer::handleConfigPost(AsyncWebServerRequest* req) { req->send(400, "application/json", "{\"error\":\"no changes\"}"); return; } + _batch_kind = BATCH_CONFIG; _batch_count = count; _batch_next = 0; _batch_reboot = reboot_after; @@ -820,8 +866,13 @@ void WebConfigServer::handleConfigResult(AsyncWebServerRequest* req) { // fires but no branch print follows, the handler is blocked on _mux. Serial.printf("WC: result entry mode=%d state=%d\n", (int)_mode, (int)_batch_state); WCLock lock(_mux); - const WebConfigBatch::ResultOutcome outcome = WebConfigBatch::classifyResult( - toSpecState(_batch_state), strcmp(requested_reqid.c_str(), _batch_reqid) == 0); + // A CLI sequence occupying the shared slot is not a config save, whatever the + // reqid says: its entries have no `key` and its results belong to the + // terminal's reader. Treat it as unknown here (and vice versa there). + const bool mine = (_batch_kind == BATCH_CONFIG) && + (strcmp(requested_reqid.c_str(), _batch_reqid) == 0); + const WebConfigBatch::ResultOutcome outcome = + WebConfigBatch::classifyResult(toSpecState(_batch_state), mine); if (outcome == WebConfigBatch::ResultOutcome::Idle) { Serial.println("WC: result read -> idle"); StaticJsonDocument<64> idle; @@ -877,6 +928,212 @@ void WebConfigServer::handleConfigResult(AsyncWebServerRequest* req) { req->send(res); } +// --------------------------------------------------------------------------- +// CLI terminal (/api/cli). Same 202 + reqid + poll contract as a config save, +// and for the same reason: CommonCLI touches prefs, the radio and the +// filesystem, none of which may be reached from the async_tcp task. The +// commands go into the shared deferred slot and tick() drains them. +// +// Unlike a save this is NOT allowlisted. That is the point: the terminal exists +// to reach what the serial console reaches, and execCommand() passes +// sender_timestamp 0, so it gets the same local privilege the serial console +// has. Authentication is the boundary — as it is for serial (physical access) +// and for remote admin over the mesh (the admin password). +// --------------------------------------------------------------------------- + +// Commands whose CLI handler never returns would take the node down mid-drain, +// before the client could read a single result. `reboot` is deferred instead: +// it is not passed to the CLI at all, and the batch arms the ordinary reboot +// 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. +void WebConfigServer::handleCliPost(AsyncWebServerRequest* req) { + if (_mode == MODE_OFF) { req->send(503); return; } + if (!checkAuth(req)) { req->send(401, "application/json", "{\"error\":\"auth\"}"); return; } + if (req->_tempObject == NULL) { + req->send(413, "application/json", "{\"error\":\"body too large\"}"); + return; + } + const char* body = (const char*)req->_tempObject; + DynamicJsonDocument doc(6144); + if (!body || deserializeJson(doc, body) != DeserializationError::Ok) { + req->send(400, "application/json", "{\"error\":\"bad json\"}"); + return; + } + const char* reqid = doc["reqid"] | ""; + if (!wcIsValidReqId(reqid)) { + req->send(400, "application/json", "{\"error\":\"bad reqid\"}"); + return; + } + JsonArray cmds = doc["cmds"]; + if (cmds.isNull()) { + req->send(400, "application/json", "{\"error\":\"no commands\"}"); + return; + } + + WCLock lock(_mux); + // Replay/Busy exactly as a config save classifies them: a repeated POST is + // acknowledged rather than executed twice, and a different sequence while one + // is still draining is refused. + const WebConfigBatch::State bstate = toSpecState(_batch_state); + const bool reqid_matches = (strcmp(reqid, _batch_reqid) == 0); + const WebConfigBatch::PostOutcome pre = + WebConfigBatch::classifyPost(bstate, reqid_matches, 1 /* count unknown yet */, false); + if (pre == WebConfigBatch::PostOutcome::Replay) { + StaticJsonDocument<96> ack; + ack["state"] = (bstate == WebConfigBatch::State::Done) ? "done" : "running"; + ack["total"] = _batch_count; + ack["reqid"] = (const char*)_batch_reqid; + String out; + serializeJson(ack, out); + req->send(202, "application/json", out); + return; + } + if (pre == WebConfigBatch::PostOutcome::Busy) { + StaticJsonDocument<96> bd; + bd["error"] = "busy"; + bd["reqid"] = (const char*)_batch_reqid; + String out; + serializeJson(bd, out); + req->send(409, "application/json", out); + return; + } + + int count = 0; + bool defer_reboot = false; + for (JsonVariant v : cmds) { + const char* raw = v.as(); + if (!raw) continue; + if (count >= MAX_BATCH) { + StaticJsonDocument<96> ed; + ed["error"] = "too many commands"; + ed["max"] = MAX_BATCH; + String out; + serializeJson(ed, out); + req->send(413, "application/json", out); + return; + } + // Strip CR/LF so one entry cannot smuggle a second command past the + // operator's confirmation, and skip whatever is left blank. + BatchEntry& e = _batch[count]; + int pos = 0; + for (const char* p = raw; *p; p++) { + if (*p == '\r' || *p == '\n') continue; + if (pos == 0 && (*p == ' ' || *p == '\t')) continue; // leading space + if (pos >= (int)sizeof(e.cmd) - 1) { + req->send(400, "application/json", "{\"error\":\"command too long\"}"); + return; + } + e.cmd[pos++] = *p; + } + while (pos > 0 && (e.cmd[pos - 1] == ' ' || e.cmd[pos - 1] == '\t')) pos--; + e.cmd[pos] = 0; + if (pos == 0) continue; + e.key[0] = 0; // CLI entries have no config key + if (wcIsDeferredReboot(e.cmd)) defer_reboot = true; + count++; + } + if (count == 0) { + req->send(400, "application/json", "{\"error\":\"no commands\"}"); + return; + } + + _batch_kind = BATCH_CLI; + _batch_count = count; + _batch_next = 0; + _batch_reboot = defer_reboot; + _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 + Serial.printf("WC: cli POST accepted, %d cmds, reboot=%d\n", count, (int)defer_reboot); + + StaticJsonDocument<96> ack; + ack["state"] = "running"; + ack["total"] = count; + ack["reqid"] = (const char*)_batch_reqid; + String out; + serializeJson(ack, out); + req->send(202, "application/json", out); +} + +void WebConfigServer::handleCliResult(AsyncWebServerRequest* req) { + if (_mode == MODE_OFF) { req->send(503); return; } + if (!checkAuth(req)) { req->send(401, "application/json", "{\"error\":\"auth\"}"); return; } + if (!req->hasParam("reqid")) { + req->send(400, "application/json", "{\"error\":\"bad reqid\"}"); + return; + } + String requested_reqid = req->getParam("reqid")->value(); + if (!wcIsValidReqId(requested_reqid.c_str())) { + req->send(400, "application/json", "{\"error\":\"bad reqid\"}"); + return; + } + int from = 0; + if (req->hasParam("from")) { + from = req->getParam("from")->value().toInt(); + if (from < 0) from = 0; + } + + WCLock lock(_mux); + // A config save occupying the slot is not this client's sequence, whatever + // the reqid says; treat it as unknown rather than serving `set` results + // through the terminal's reader. + const bool mine = (_batch_kind == BATCH_CLI) && + (strcmp(requested_reqid.c_str(), _batch_reqid) == 0); + const WebConfigBatch::ResultOutcome outcome = + WebConfigBatch::classifyResult(toSpecState(_batch_state), mine); + if (outcome == WebConfigBatch::ResultOutcome::Idle) { + StaticJsonDocument<64> idle; + idle["state"] = "idle"; + idle["reqid"] = requested_reqid; + String out; + serializeJson(idle, out); + req->send(200, "application/json", out); + return; + } + if (outcome == WebConfigBatch::ResultOutcome::Unknown) { + req->send(404, "application/json", "{\"error\":\"unknown request\"}"); + return; + } + + // Results stream: hand back whatever has drained since the client's cursor, + // capped so the document stays small on the async_tcp task. + const int produced = _batch_next; + const int page = WebConfigBatch::cliPageCount(from, produced, WebConfigBatch::kCliResultPage); + const bool final_read = WebConfigBatch::cliReadIsFinal(toSpecState(_batch_state), + from, page, _batch_count); + DynamicJsonDocument doc(4096); + doc["state"] = final_read ? "done" : "running"; + doc["reqid"] = (const char*)_batch_reqid; + doc["total"] = _batch_count; + doc["from"] = from; + JsonArray results = doc.createNestedArray("results"); + for (int i = from; i < from + page; i++) { + 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"] = strncmp(_batch[i].reply, "OK", 2) == 0; + r["reply"] = (const char*)_batch[i].reply; + } + if (final_read) { + doc["all_ok"] = _batch_all_ok; + const bool rebooting = WebConfigBatch::cliRebootAllowed(_batch_reboot, _batch_all_ok); + doc["reboot"] = rebooting; + // Tell the operator why a `reboot` they asked for is not happening. + if (_batch_reboot && !_batch_all_ok) doc["reboot_withheld"] = true; + if (WebConfigBatch::shouldArmConfirmReboot(toSpecState(_batch_state), _batch_reboot, + _batch_all_ok, _batch_reboot_armed)) { + _batch_reboot_armed = true; + _reboot_at = WebConfigBatch::confirmRebootAt(millis()); + } + } + AsyncResponseStream* res = req->beginResponseStream("application/json"); + serializeJson(doc, *res); + req->send(res); +} + void WebConfigServer::handleStats(AsyncWebServerRequest* req) { if (_mode == MODE_OFF) { req->send(503); return; } if (!checkAuth(req)) { req->send(401, "application/json", "{\"error\":\"auth\"}"); return; } diff --git a/src/helpers/esp32/WebConfigServer.h b/src/helpers/esp32/WebConfigServer.h index e2d9e08a..7ef23a92 100644 --- a/src/helpers/esp32/WebConfigServer.h +++ b/src/helpers/esp32/WebConfigServer.h @@ -98,6 +98,11 @@ private: // than freeing memory still referenced by the async task. static const uint32_t STOP_WARN_MS = WebConfigBatch::kStopWarnMs; enum BatchState : uint8_t { BATCH_IDLE = 0, BATCH_PENDING, BATCH_DONE }; + // What filled the shared slot. A config save comes from allowlisted form + // fields; a CLI sequence is arbitrary commands typed into the terminal. They + // share the slot (see WebConfigBatch.h) but differ in how results are read + // and in whether `key` means anything, so every reader checks the kind. + enum BatchKind : uint8_t { BATCH_CONFIG = 0, BATCH_CLI }; // BatchState and WebConfigBatch::State are deliberately kept as separate // types (the enum is stored in a volatile member and used in prints); this @@ -110,9 +115,9 @@ private: } } struct BatchEntry { - char key[24]; // allowlisted config key (echoed back to the UI) + char key[24]; // allowlisted config key (echoed back to the UI); empty for CLI entries char cmd[160]; // full CLI command (may contain secrets - never echoed) - char reply[160]; + char reply[160]; // CLI reply budget, same 160 bytes the serial console gets }; NodePrefs* _prefs; @@ -141,6 +146,7 @@ private: // Command batch: filled by async_tcp under _mux, drained by tick(). volatile BatchState _batch_state = BATCH_IDLE; + volatile BatchKind _batch_kind = BATCH_CONFIG; uint8_t _batch_count = 0; uint8_t _batch_next = 0; // drain progress (one command per tick) uint32_t _batch_last_cmd = 0; @@ -199,6 +205,8 @@ private: void handleConfigGet(AsyncWebServerRequest* req); void handleConfigPost(AsyncWebServerRequest* req); void handleConfigResult(AsyncWebServerRequest* req); + void handleCliPost(AsyncWebServerRequest* req); + void handleCliResult(AsyncWebServerRequest* req); void handleStats(AsyncWebServerRequest* req); void handleScan(AsyncWebServerRequest* req); void handlePresets(AsyncWebServerRequest* req); diff --git a/test/test_webconfig_batch/test_webconfig_batch.cpp b/test/test_webconfig_batch/test_webconfig_batch.cpp index 9faddc4f..2b6deb82 100644 --- a/test/test_webconfig_batch/test_webconfig_batch.cpp +++ b/test/test_webconfig_batch/test_webconfig_batch.cpp @@ -190,6 +190,47 @@ TEST(WebConfigBatch, StopWarnsOnceAfterTheDeadlineThenKeepsWaiting) { EXPECT_EQ(Batch::StopAction::Wait, Batch::stopStep(2, false, 0, 999999)); } +// -------------------------------------------------------------------------- +// CLI sequences (/api/cli), which share the deferred-command slot +// -------------------------------------------------------------------------- +TEST(WebConfigBatch, CliReadPagesResultsAndNeverOverrunsWhatHasDrained) { + const int page = Batch::kCliResultPage; + // Nothing drained past the cursor yet. + EXPECT_EQ(0, Batch::cliPageCount(/*from=*/0, /*produced=*/0, page)); + EXPECT_EQ(0, Batch::cliPageCount(/*from=*/3, /*produced=*/3, page)); + // Partial progress: hand back exactly what exists. + EXPECT_EQ(3, Batch::cliPageCount(0, 3, page)); + EXPECT_EQ(2, Batch::cliPageCount(5, 7, page)); + // More available than fits in one read: cap at the page size. + EXPECT_EQ(page, Batch::cliPageCount(0, page + 5, page)); + // A cursor beyond what has drained (stale or crafted) yields nothing rather + // than a negative count that would index backwards through the batch. + EXPECT_EQ(0, Batch::cliPageCount(/*from=*/9, /*produced=*/4, page)); +} + +TEST(WebConfigBatch, CliReadIsDoneOnlyOnceEveryResultHasBeenHandedOver) { + // Still executing: never final, however much has been read. + EXPECT_FALSE(Batch::cliReadIsFinal(State::Pending, /*from=*/0, /*page=*/8, /*total=*/8)); + // Execution finished but the client has only seen the first page. Reporting + // "done" here would make a client that stops polling lose the rest. + EXPECT_FALSE(Batch::cliReadIsFinal(State::Done, /*from=*/0, /*page=*/8, /*total=*/20)); + EXPECT_FALSE(Batch::cliReadIsFinal(State::Done, /*from=*/8, /*page=*/8, /*total=*/20)); + // The read that hands over the last result is the final one. + EXPECT_TRUE(Batch::cliReadIsFinal(State::Done, /*from=*/16, /*page=*/4, /*total=*/20)); + // Re-reading past the end stays final (polls after the last page). + EXPECT_TRUE(Batch::cliReadIsFinal(State::Done, /*from=*/20, /*page=*/0, /*total=*/20)); +} + +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 + // over a link the operator may not get back. + EXPECT_FALSE(Batch::cliRebootAllowed(true, false)); + // No `reboot` in the sequence: nothing to allow either way. + EXPECT_FALSE(Batch::cliRebootAllowed(false, true)); + EXPECT_FALSE(Batch::cliRebootAllowed(false, false)); +} + // -------------------------------------------------------------------------- // Wrap-around guard shared with the production _reboot_at assignments // -------------------------------------------------------------------------- diff --git a/webui/index.html b/webui/index.html index 66e9a9f2..6d8e87ae 100644 --- a/webui/index.html +++ b/webui/index.html @@ -626,6 +626,7 @@ function boot(){ // (active_slots: 2 without PSRAM, 5 with). Fall back to the runtime array // size for older firmware that doesn't report it. st.mode=s.mode;st.authed=s.auth;st.needsSetup=!!s.needs_setup;st.nslots=s.active_slots||s.runtime_slots||6; + if(s.max_cmds>0)CLI_MAX=s.max_cmds; $("#h-name").textContent=s.name||"MeshCore"; $("#h-sub").textContent=s.role+" · "+s.fw+" · "+s.board; var b=$("#h-badge");b.classList.remove("hide"); @@ -1428,7 +1429,10 @@ function showReboot(msg,reconnect,title){ Drives autocomplete only — the node remains the authority on what it accepts. A trailing space marks a command that takes an argument, so accepting the completion leaves the cursor ready for the value. */ -var CLI_MAX=64; // commands per submitted sequence +// Commands per submitted sequence. The node reports its own limit (MAX_BATCH, +// the fixed slot the CLI shares with config saves) in /api/status; this is only +// the fallback for firmware that doesn't say. +var CLI_MAX=24; // [command, description] var CLI_VERBS=[ ["ver","Firmware version"], @@ -1967,13 +1971,17 @@ function cliPoll(reqid,cmds,from,status,errs,idles){ if(idles<5){setTimeout(function(){cliPoll(reqid,cmds,from,status,errs,idles+1)},400);return} cliEnd(status,"The node never received the command.");return; } - (r.results||[]).forEach(function(x){ - cliEcho("cmd",x.cmd); + (r.results||[]).forEach(function(x,i){ + // The node never echoes the command back — it may hold a password or a + // token, and we already have the sequence we sent. Match by index. + cliEcho("cmd",cmds[from+i]||""); // `ok` is advisory; the node's own OK/Error convention is authoritative var ok=(x.ok!=null)?x.ok:!/^\s*(err|error)\b/i.test(x.reply||""); if(x.reply)cliEcho(ok?"rep":"err",x.reply); }); from+=(r.results||[]).length; + // "running" also covers "finished, but more results are still to be paged + // over", so keep polling until the node says done. if(r.state!=="done"){ if(cmds.length>1)status.textContent="running "+from+"/"+cmds.length+"…"; setTimeout(function(){cliPoll(reqid,cmds,from,status,0,0)},250); @@ -1981,7 +1989,7 @@ function cliPoll(reqid,cmds,from,status,errs,idles){ } status.remove(); cliBusy(false); - cliAfter(cmds); + cliAfter(cmds,r); }).catch(function(e){ if(e.message==="auth"){cliBusy(false);status.remove();return} if(e.status===400||e.status===404){cliEnd(status,"The node no longer has a result for this sequence.");return} @@ -1991,18 +1999,31 @@ function cliPoll(reqid,cmds,from,status,errs,idles){ } // A CLI `set` writes the same prefs the forms edit, so re-read them or the // other tabs keep showing stale values. -function cliAfter(cmds){ - var touched=false,restart=null; +function cliAfter(cmds,r){ + var touched=false; cmds.forEach(function(c){ if(/^password\s/.test(c))cli.pwd=true; if(/^(set|password)\s/.test(c))touched=true; - if(/^(reboot|clkreboot)$/.test(c))restart=restart||"reboot"; - if(/^erase$/.test(c))restart=restart||"erase"; - if(/^(poweroff|shutdown)$/.test(c))restart="off"; }); if(touched)loadConfigSoft(); - if(restart==="off")showReboot("The node is powering off. Reconnect power to bring it back.",false,"Powering off…"); - else if(restart)showReboot("The node is restarting. This page will try to reconnect automatically.",true); + // 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."); + return; + } + if(r&&r.reboot){ + showReboot("The node is restarting. This page will try to reconnect automatically.",true); + return; + } + // 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); + }); } /* ---------- CLI: fit ----------