mirror of
https://github.com/CyberMind-FR/secubox-deb.git
synced 2026-07-28 21:17:36 +00:00
fix(meshtastic): wire daemon main() to web.create_app(cache,send_cb,ctl_cb) (ref #897)
This commit is contained in:
parent
c9ab0fa4ad
commit
760c28eab5
|
|
@ -1,192 +1,115 @@
|
|||
# Task 10 Report: webui — lifecycle/wake_class/sleep-state + manual sleep/wake (secubox-profiles, ref #896)
|
||||
# Task 10 report — FastAPI webui backend (`api/web.py`)
|
||||
|
||||
**Status:** Done.
|
||||
## Status: DONE
|
||||
|
||||
(Note: this file previously held an unrelated stale Task-10 report for a
|
||||
different plan — a `secubox-metablogizer` packaging task — overwritten
|
||||
here since it did not belong to this plan.)
|
||||
## Files
|
||||
- `packages/secubox-meshtastic/api/web.py` (new)
|
||||
- `packages/secubox-meshtastic/tests/test_web.py` (new, 13 tests)
|
||||
|
||||
## What was implemented
|
||||
## Commit
|
||||
`c9ab0fa4` — `feat(meshtastic): FastAPI webui backend (ref #897)`
|
||||
|
||||
- `packages/secubox-profiles/api/web.py`:
|
||||
- `_build_status_payload(manifests, actuals)` — each module row now also
|
||||
carries `lifecycle` (the **effective** value from
|
||||
`lifecycle.effective_lifecycle(m)`, so a protected module reads
|
||||
`"always-on"` regardless of its declared manifest value — consistent
|
||||
with how `protected` is already surfaced), `wake_class` (raw
|
||||
`m.wake_class`), `sleep_state`, and `wake_budget_s`.
|
||||
`sleep_state ∈ {"up","asleep","n/a"}`: `"n/a"` when
|
||||
`effective_lifecycle(m)` is `always-on`/`manual`; otherwise `"up"` if
|
||||
`observe.is_on(a)` else `"asleep"`. **`"waking"` is deliberately NOT
|
||||
modeled** — the only candidate signal (`api/waker.py`'s
|
||||
`waker-active.json`) is a best-effort anti-storm lock, not a reliable
|
||||
"in-progress" probe; inventing a third state off it would be exactly
|
||||
the fragile probe the task brief warned against. Documented inline.
|
||||
`wake_budget_s` is `lifecycle.wake_budget(m)` for sleepable modules,
|
||||
`None` for `n/a` ones.
|
||||
- Refactored `_run_ctl_json(verb)` into a shared `_run_ctl_json_argv(argv)`
|
||||
(same parse-report-first-then-rc logic, now argv-agnostic) plus three
|
||||
thin builders: `_run_ctl_json(verb)` (apply/rollback, unchanged
|
||||
behavior), `_run_wake_ctl(module)`, `_run_sleep_ctl(module)`.
|
||||
- New `POST /api/v1/profiles/wake` and `POST /api/v1/profiles/sleep`
|
||||
(`ModuleAction{module: str}` body): both JWT-gated
|
||||
(`Depends(require_jwt)`), both refuse **locally** (structural refusal,
|
||||
before any sudo call — same posture as the pin protected-off check)
|
||||
via a shared `_sleepable_module_or_error(mod_dir, module)`: unknown
|
||||
module → 404, non-sleepable (`always-on`/`manual`, incl.
|
||||
protected-forced) → 409. Both then run under `_apply_lock` (same
|
||||
process-local serialization as apply/rollback) and delegate to the
|
||||
ctl, returning its JSON report as-is (refused/rolled_back statuses
|
||||
surface in the 200 body, exactly like apply's `rolled_back` already
|
||||
does — the caller/panel classifies the status, not the HTTP code).
|
||||
## Design
|
||||
`web.create_app(cache, send_cb, ctl_cb) -> FastAPI`, matching the brief's
|
||||
explicit dependency-injection signature (distinct from `secubox-profiles`'
|
||||
zero-arg `create_app()`, which closes over module-level globals — profiles
|
||||
was still studied as the pattern reference for `require_jwt` wiring,
|
||||
validate-before-delegate, and test structure).
|
||||
|
||||
- `packages/secubox-profiles/sudoers.d/secubox-profiles` — **two new
|
||||
grants** (visudo -c passes):
|
||||
```
|
||||
secubox ALL=(root) NOPASSWD: /usr/bin/systemd-run --wait --pipe --collect --quiet /usr/sbin/secubox-wakectl wake *
|
||||
secubox ALL=(root) NOPASSWD: /usr/bin/systemd-run --wait --pipe --collect --quiet /usr/sbin/secubox-profilectl apply --only * --yes --json
|
||||
```
|
||||
Both were **required**, not deferrable:
|
||||
- The existing `secubox-wakectl wake *` grant (from Task 6/waker) is
|
||||
**fire-and-forget** (no `--wait --pipe`) — used by `api/waker.py`'s
|
||||
`_fire_wake`, which never blocks a public request on the outcome. The
|
||||
panel's manual Wake button needs the *synchronous* report to show the
|
||||
operator what happened, which is a **different, additional** command
|
||||
line (`--wait --pipe` inserted) that does not match the existing rule.
|
||||
- `/sleep` reuses the existing `apply` actuator (no new privileged code
|
||||
path) scoped to one module via `--only <id>`, but the only sudoers rule
|
||||
for `apply` was the exact-string `apply --yes --json` (no `--only`) —
|
||||
a genuinely new, bounded wildcard grant was needed.
|
||||
Both wildcards are safe for the same reason already documented for the
|
||||
pre-existing wake grant (quoted/extended in the file): sudo matches argv
|
||||
via `execve`, never a shell, so glob metacharacters in a module id are
|
||||
inert; and the downstream CLI (`secubox-wakectl`/`secubox-profilectl`)
|
||||
only ever acts on ids it recognizes from `modules.d` — an unrecognized id
|
||||
either gets `status=refused` (wake) or silently filters the plan to
|
||||
nothing (sleep's `--only`), never arbitrary execution.
|
||||
Routes, all `Depends(require_jwt)`, prefix `/api/v1/meshtastic`:
|
||||
- `GET /status` → `cache.get()`
|
||||
- `GET /nodes` → `cache.get().get("nodes", [])`
|
||||
- `GET /messages` → `cache.get().get("messages_by_channel", {})`
|
||||
- `GET /packets` → `{"census": ..., "channel_stats": ...}` from cache
|
||||
- `POST /send {channel,text}` → `send_cb(channel, text)` (no validation —
|
||||
real-time action, no root, no config write)
|
||||
- `POST /mode {mode}` → 422 if `mode not in config.MODES`, else
|
||||
`ctl_cb("set-mode", mode=mode)`
|
||||
- `POST /grid {channel,grid}` → 422 if any `grid[i] not in config.GRIDS`,
|
||||
else `ctl_cb("set-grid", channel=channel, grid=grid)`
|
||||
|
||||
- `packages/secubox-profiles/www/profiles/index.html` — each module row
|
||||
gained one more grid column: a `sleep_state` pill (🟢 up / 🌙 asleep /
|
||||
the raw lifecycle label for n/a) plus, when sleepable, the one
|
||||
applicable manual action button (💤 Sleep when up, ⚡ Wake when asleep,
|
||||
titled with the `wake_budget_s` estimate; no button for n/a). Wired via
|
||||
a `data-act="wake"|"sleep"` click handler mirroring the existing
|
||||
pin-button pattern: POST to the new routes, refresh() on completion,
|
||||
toast/errorToast based on the returned report's `status`/`failed`.
|
||||
Sleep asks `confirm()` first (it stops a running module); Wake does not
|
||||
(idempotent no-op if already up). New column hidden on the existing
|
||||
mobile media query alongside meta/prio/rss/member (unchanged narrow
|
||||
layout otherwise). Verified with `node --check` on the extracted
|
||||
`<script>` body (no syntax errors).
|
||||
`require_jwt` imported from `secubox_core.auth` with the same dev-fallback
|
||||
stub as profiles' `web.py` (dist-packages first, ImportError fallback).
|
||||
`/channel` mentioned in the thin brief was NOT implemented — the detailed
|
||||
task instructions (routes list) omit it, so I followed the more specific
|
||||
spec rather than the brief.
|
||||
|
||||
## TDD: RED → GREEN
|
||||
## Tests (13, all pass)
|
||||
JWT gating (real `require_jwt`, no override) on `/status` and on all 6
|
||||
routes collectively; read routes return exactly the injected cache dict/sub-
|
||||
keys and default to `[]`/`{}` on a bare cache; `/send` calls `send_cb` with
|
||||
correct args and returns its result; `/mode` and `/grid` each get a
|
||||
bad-value → 422-and-ctl-NOT-called test plus a good-value →
|
||||
ctl-called-with-expected-args test; a grep-level guard (code lines only,
|
||||
skipping comments/docstrings) confirms `web.py` never imports `subprocess`
|
||||
or shells out directly.
|
||||
|
||||
- Added tests to `tests/test_web.py` (status payload extension, wake/sleep
|
||||
delegation-argv, 404, 409, JWT-gating) — ran first to confirm RED:
|
||||
`python -m pytest tests/test_web.py -q -k "lifecycle_wake_class or wake_route or sleep_route or wake_and_sleep"`
|
||||
→ **6 failed** (routes/fields didn't exist yet; the JWT test failed
|
||||
because the routes 404'd before ever reaching `require_jwt`, i.e. it
|
||||
wasn't a false-positive pass).
|
||||
- Implemented the payload extension, the two routes, and the sudoers
|
||||
grants.
|
||||
- GREEN: same command → **8 passed**.
|
||||
|
||||
## Test results
|
||||
|
||||
```
|
||||
cd packages/secubox-profiles && python -m pytest tests/test_web.py -q
|
||||
```
|
||||
**44 passed** (8 new, 36 pre-existing untouched).
|
||||
|
||||
Full suite:
|
||||
```
|
||||
cd packages/secubox-profiles && python -m pytest tests/ -q
|
||||
```
|
||||
**236 passed**, 3 warnings (pre-existing FastAPI `on_event` deprecation
|
||||
warnings, unrelated).
|
||||
|
||||
## mypy vs. baseline
|
||||
|
||||
Non-strict (`mypy api/`): **3 errors** before and after — identical
|
||||
(`api/scan.py:80`, `api/snapshot.py:55`, `api/web.py:49` import-not-found
|
||||
for `secubox_core.auth`, all pre-existing, none touched).
|
||||
|
||||
`--strict` (matches Task 7's comparison method): baseline **93 errors in
|
||||
12 files** (checked via `git stash` of this task's 4 changed files) →
|
||||
after this task **102 errors in 12 files** (+9). All 9 new errors are
|
||||
instances of the **same two categories already pervasive throughout
|
||||
`web.py`** before this task (every existing nested route handler in the
|
||||
file is already untyped the same way): `no-untyped-def` (missing
|
||||
return/param annotation — the 3 new nested route-scoped helpers
|
||||
`_sleepable_module_or_error`, `wake_module`, `sleep_module` follow the
|
||||
exact same unannotated-`_claims=Depends(...)` idiom as every pre-existing
|
||||
handler like `set_pin`/`apply_active`) and `type-arg` (bare `dict` return
|
||||
annotation — the 3 new top-level ctl helpers `_run_ctl_json_argv`,
|
||||
`_run_wake_ctl`, `_run_sleep_ctl` return bare `dict`, matching ~15
|
||||
pre-existing bare-`dict` returns elsewhere in the same file). No new
|
||||
error *category* was introduced.
|
||||
|
||||
## Delegation argv (webui→ctl)
|
||||
|
||||
- `POST /wake {module}` →
|
||||
`sudo -n /usr/bin/systemd-run --wait --pipe --collect --quiet /usr/sbin/secubox-wakectl wake <module> --json`
|
||||
- `POST /sleep {module}` →
|
||||
`sudo -n /usr/bin/systemd-run --wait --pipe --collect --quiet /usr/sbin/secubox-profilectl apply --only <module> --yes --json`
|
||||
|
||||
Both under `_apply_lock` (process-local asyncio.Lock, same one apply/
|
||||
rollback/active already share) and `Depends(require_jwt)`.
|
||||
|
||||
## What T12 must wire
|
||||
|
||||
- Nothing new beyond what earlier tasks already deferred to T12 (nginx
|
||||
`@waker` include wiring, WAF route file, etc.) — **no new sudoers grant
|
||||
is deferred here**; both grants this task needed were shipped and
|
||||
`visudo -c`'d in this task, since the brief called them out as a real
|
||||
requirement, not deferrable.
|
||||
- The panel's new column is additive CSS/markup only; no server-side
|
||||
wiring is needed beyond the two routes added here.
|
||||
|
||||
## Self-review
|
||||
|
||||
- Confirmed `test_web_module_has_no_actuation_helper` (greps `web.py` for
|
||||
`systemctl start/stop/enable/disable`, `lxc-start/lxc-stop`) still
|
||||
passes — the new code only ever shells out through `_ctl_run` to
|
||||
`sudo`/`secubox-wakectl`/`secubox-profilectl`, never touches
|
||||
systemd/LXC directly.
|
||||
- Confirmed `/wake` and `/sleep` reject unknown/non-sleepable modules
|
||||
**before** calling `_ctl_run` at all (asserted via a `called=[]` list in
|
||||
the 404/409 tests), matching the "never actuate for a refusal" posture
|
||||
already used for the pin protected-off check.
|
||||
- Confirmed the two new sudoers lines with `visudo -cf
|
||||
sudoers.d/secubox-profiles` (parses clean) — chose to ship them now
|
||||
rather than defer, per the brief's explicit instruction that a
|
||||
genuinely-needed grant is not deferrable.
|
||||
- Extracted the panel's `<script>` body and ran `node --check` on it to
|
||||
catch any JS syntax errors from the new markup-building code before
|
||||
committing.
|
||||
- Verified the `sleep_state`/`wake_budget_s` semantics against a 3rd,
|
||||
on-demand/urgent fixture module (`podcast`, deliberately left
|
||||
unobserved by the test's `_observe_all` stub) to prove the
|
||||
`is_on()`-driven None→False→"asleep" coalescing, not just the two
|
||||
pre-existing fixture modules (`lyrion` eager/observed-on, `auth`
|
||||
protected/always-on).
|
||||
## Full suite
|
||||
`60 passed` (was 47 before this task, +13 new — no regressions).
|
||||
No `meshtastic`/`paho` imports introduced anywhere in the new files.
|
||||
|
||||
## Concerns
|
||||
None. One iteration needed: my first grep-guard test tripped on the word
|
||||
"subprocess" appearing in my own docstring prose (not code) — narrowed the
|
||||
test to skip comment/docstring lines and to look for concrete offenders
|
||||
(`import subprocess`, `os.system`, `Popen(`) instead of loose substrings.
|
||||
|
||||
- `/sleep`'s reuse of `apply --only <module>` is correct **only** because
|
||||
`plan_changes` already computes a STOP for any currently-up sleepable
|
||||
module that is not desired ON (absent from the active profile / not
|
||||
pinned on) — this is the common case for genuinely on-demand modules,
|
||||
which typically aren't listed as profile members. If an operator adds a
|
||||
sleepable module to the active profile's `on` list (making it
|
||||
desired-ON), a manual Sleep click becomes a **safe no-op**
|
||||
(`changed: []`, no error) rather than a forced stop, because `apply`
|
||||
will always want to re-converge it back on. This is intentional
|
||||
(avoids fighting the declared desired state) but may surprise an
|
||||
operator who expects Sleep to always work — flagging in case a
|
||||
dedicated "force sleep" actuator (bypassing desired-state) is wanted
|
||||
later.
|
||||
- `sleep_state`'s `"waking"` value from the brief's enum is not emitted by
|
||||
this implementation (only `"up"`/`"asleep"`/`"n/a"`) — deliberately, per
|
||||
the brief's own permission to omit it absent a reliable signal. Noted
|
||||
in code and here in case a future task adds a proper in-progress probe.
|
||||
---
|
||||
|
||||
## Follow-up fix — daemon.py `main()` cross-task integration gap (ref #897)
|
||||
|
||||
### Status: DONE
|
||||
|
||||
### Problem
|
||||
`daemon.py::main()` still had Task-8-era placeholders (`web.create_app(engine)`,
|
||||
one arg) and stale `# Task 10: replace ...` comments. Once this task made
|
||||
`api/web.py` importable with a real 3-arg `create_app(cache, send_cb, ctl_cb)`,
|
||||
the `try/except ImportError` guard around `from . import web` no longer caught
|
||||
anything — `main()` would import `web` successfully and then raise `TypeError`
|
||||
calling `create_app(engine)` with the wrong arity. The webui was wired to
|
||||
nothing reachable end-to-end.
|
||||
|
||||
### Fix (`packages/secubox-meshtastic/api/daemon.py`)
|
||||
- Added module-level `_ctl_cb(verb, **kwargs) -> dict`: builds
|
||||
`["sudo","-n","/usr/bin/systemd-run","--wait","--pipe","--collect","--quiet",
|
||||
"/usr/sbin/secubox-meshtasticctl", verb, ...]`, mirroring
|
||||
`secubox-profiles/api/web.py::_run_ctl_json_argv`'s shape but run
|
||||
synchronously (`subprocess.run(..., timeout=30)`, no asyncio loop in
|
||||
daemon.main() to offload onto). Argv shape matches the REAL
|
||||
`api/ctl.py` argparse (positional args, not the brief's guessed `--mode`
|
||||
flag): `set-mode <mode>`, `set-grid <channel> --grid <csv>`. `api/ctl.py`
|
||||
today prints plain text, not `--json` (confirmed: no `--json`/`json.dumps`
|
||||
report path in ctl.py) — so `_ctl_cb` first *tries* a JSON-report parse
|
||||
(forward-compatible if ctl.py ever grows one) and otherwise derives the
|
||||
dict from `rc`/stdout/stderr: `{"status":"applied","output":...}` on
|
||||
rc==0, `{"status":"error","stderr":...}` otherwise. Unknown verb ->
|
||||
`{"status":"error","stderr":"ctl verb inconnu: ..."}` without shelling out.
|
||||
- In `main()`: `send_cb(channel, text)` closes over `engine` — returns
|
||||
`{"status":"radio-absent"}` when `engine.radio is None` (never crashes),
|
||||
else calls `engine.radio.send_text(text, channel)` (note argument order:
|
||||
`RadioInterface.send_text(text, channel=0)`) and returns
|
||||
`{"status":"sent","channel":channel}`.
|
||||
- `app = web.create_app(engine.cache, send_cb, _ctl_cb)`, served via
|
||||
`uvicorn.run(app, uds=SOCKET_PATH, log_level="warning")`.
|
||||
- `import uvicorn` moved inside the same `try/except ImportError` as
|
||||
`from . import web`, so the cache-refresh-only fallback path still
|
||||
degrades gracefully if either dependency is genuinely absent — safety net
|
||||
preserved, just no longer masking a real `TypeError`.
|
||||
- Removed both stale `# Task 10: replace with uvicorn UDS serve of api.web`
|
||||
comments (now resolved).
|
||||
|
||||
### create_app call line
|
||||
```python
|
||||
app = web.create_app(engine.cache, send_cb, _ctl_cb)
|
||||
```
|
||||
|
||||
### Verify
|
||||
- `python -c "import ast; ast.parse(open('packages/secubox-meshtastic/api/daemon.py').read())"` → parses.
|
||||
- `pytest tests/ -q` → `60 passed` (unchanged — `main()` remains untested by
|
||||
design, no device/UDS in CI).
|
||||
|
||||
### Concerns
|
||||
None new. `main()` stays outside test coverage on purpose (no real serial
|
||||
device or UDS bind in CI); correctness here rests on parse-check + matching
|
||||
`api/ctl.py`'s actual (not hypothetical) argparse surface and `api/web.py`'s
|
||||
actual (tested) `create_app` signature.
|
||||
|
|
|
|||
|
|
@ -2,7 +2,9 @@
|
|||
# Copyright (c) 2026 CyberMind — Gérald Kerma <devel@cybermind.fr>
|
||||
"""SecuBox-Deb :: meshtastic — daemon engine (wires radio/cache/passive/bridge)."""
|
||||
from __future__ import annotations
|
||||
import json
|
||||
import logging
|
||||
import subprocess
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
|
@ -109,6 +111,45 @@ class _NullMqtt:
|
|||
pass
|
||||
|
||||
|
||||
def _ctl_cb(verb: str, **kwargs) -> dict:
|
||||
"""webui -> ctl delegate for `web.create_app`'s `ctl_cb` (see
|
||||
secubox-profiles/api/web.py's `_run_ctl_json_argv` for the pattern this
|
||||
mirrors — same `sudo -n systemd-run --wait --pipe --collect --quiet`
|
||||
shape, run synchronously since daemon.main() has no asyncio loop to
|
||||
offload onto). `secubox-meshtasticctl` argv is positional-argument based
|
||||
(see api/ctl.py argparse: `set-mode <mode>`, `set-grid <channel> --grid
|
||||
<csv>`) and today prints plain text, not a `--json` report — so a
|
||||
successful (rc=0) run is reported as `{"status": "applied", ...}` from
|
||||
stdout, and only a genuine JSON report (if the ctl ever grows one) short-
|
||||
circuits that. Web-side validation (config.MODES / config.GRIDS, see
|
||||
api/web.py) has already refused bad values before this is ever called."""
|
||||
argv = ["sudo", "-n", "/usr/bin/systemd-run", "--wait", "--pipe",
|
||||
"--collect", "--quiet", "/usr/sbin/secubox-meshtasticctl", verb]
|
||||
if verb == "set-mode":
|
||||
argv.append(str(kwargs["mode"]))
|
||||
elif verb == "set-grid":
|
||||
argv.append(str(kwargs["channel"]))
|
||||
argv += ["--grid", ",".join(kwargs["grid"])]
|
||||
else:
|
||||
return {"status": "error", "stderr": f"ctl verb inconnu: {verb}"}
|
||||
|
||||
try:
|
||||
proc = subprocess.run(argv, capture_output=True, text=True, timeout=30)
|
||||
except Exception as exc: # sudo/systemd-run itself missing, timeout, ...
|
||||
return {"status": "error", "stderr": str(exc)}
|
||||
|
||||
try:
|
||||
report = json.loads(proc.stdout)
|
||||
if isinstance(report, dict) and "status" in report:
|
||||
return report
|
||||
except ValueError:
|
||||
pass
|
||||
|
||||
if proc.returncode == 0:
|
||||
return {"status": "applied", "verb": verb, "output": proc.stdout.strip()}
|
||||
return {"status": "error", "stderr": (proc.stderr or proc.stdout).strip()[:300]}
|
||||
|
||||
|
||||
def main() -> None:
|
||||
logging.basicConfig(level=logging.INFO)
|
||||
|
||||
|
|
@ -139,25 +180,29 @@ def main() -> None:
|
|||
stop = threading.Event()
|
||||
|
||||
try:
|
||||
from . import web # Task 10: replace with uvicorn UDS serve of api.web
|
||||
from . import web
|
||||
import uvicorn
|
||||
except ImportError:
|
||||
web = None
|
||||
|
||||
if web is not None:
|
||||
# Task 10: replace with uvicorn UDS serve of api.web
|
||||
import uvicorn
|
||||
app = web.create_app(engine)
|
||||
def send_cb(channel: int, text: str) -> dict:
|
||||
if engine.radio is None:
|
||||
return {"status": "radio-absent"}
|
||||
engine.radio.send_text(text, channel)
|
||||
return {"status": "sent", "channel": channel}
|
||||
|
||||
app = web.create_app(engine.cache, send_cb, _ctl_cb)
|
||||
cache.start_refresh(engine.snapshot, CACHE_REFRESH_INTERVAL, stop)
|
||||
try:
|
||||
uvicorn.run(app, uds=SOCKET_PATH, log_level="info")
|
||||
uvicorn.run(app, uds=SOCKET_PATH, log_level="warning")
|
||||
finally:
|
||||
stop.set()
|
||||
bridge.stop()
|
||||
if radio is not None:
|
||||
radio.close()
|
||||
else:
|
||||
# Task 10: replace with uvicorn UDS serve of api.web
|
||||
log.info("api.web not available yet — running cache-refresh only, no webui")
|
||||
log.info("api.web/uvicorn not available — running cache-refresh only, no webui")
|
||||
cache.start_refresh(engine.snapshot, CACHE_REFRESH_INTERVAL, stop)
|
||||
try:
|
||||
threading.Event().wait()
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user