Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,25 @@ Phased plan derived from a full codebase review. Detailed items live in the TODO

**Not scale-dependent, do sooner than phase 4**: the 2026-07-27 security review (see the Security section in TODO) found items that are risks today regardless of lab size — two hardcoded fleet-wide Samba passwords already committed to the repo, and an unauthenticated-ZMQ path to full RCE-as-root via `update_saviour` that doesn't require ever touching the web UI's login. The web UI *does* already have a real (if limited) auth system, contrary to this roadmap's original "auth on... web UI" framing — what it actually needs is TLS, rate-limiting, and closing the unauthenticated endpoints, not auth from scratch.

## v1.0 Milestone (proposed 2026-08-13, currently v0.6)

Scope for calling this v1.0: **safe to run unattended on a closed lab LAN without silent data loss or easy compromise** — not "fully hardened for internet exposure." Deliberately excludes the big structural refactors (they're real debt, but don't block correctness/safety): `web.py` blueprint split, the `examples/`→`variants/` manifest rename, Samba→rsync, and the module god-object composition refactor all stay post-1.0.

- [ ] **Correctness gate** — the small, already-diagnosed bugs still open from the 2026-07-31/08-03 reviews:
- [ ] `modules.py` heartbeat-timeout offline path bypasses `on_status_change()` (mid-recording dropout alert doesn't fire for this path)
- [ ] `controller/config.py` `set_all()`/`_recursive_update()` has no lock (racing reads see half-merged config)
- [ ] `controller/config.py` `set_all()` raises `KeyError` on a genuinely new config key (crashes controller-settings save)
- [ ] `health.py` `get_ptp_sync()` treats a perfect `0` offset as missing data
- [ ] `health.py` `get_health_summary()` crashes on any online module with a `None` metric
- [ ] `recording.py` `create_session()` busy-module overlap check is unlocked (double-submit/scheduler race can double-start a session)
- [ ] **Security floor** — the items that are real regardless of LAN-vs-WAN exposure, per the 2026-07-27 review:
- [ ] `update_saviour` / `deploy_update`: add package signature or checksum verification before rsync-over-install (currently only checks `zipfile.is_zipfile()`)
- [ ] Session recording downloads (`/api/sessions/<name>/download...`) have no auth check at all — add `_require_auth`, matching every other handler
- [ ] Gate the other unauthenticated data-leak endpoints: `/facade/list_modules`, `/facade/module_health`, `/facade/exported_recordings`, `/update/package`, `get_bug_report` (and fix `bug_report_ready` broadcasting to every socket instead of `room=request.sid`)
- [ ] ZMQ identity-hijack: either disable `ROUTER_HANDOVER` or require a shared pre-shared secret on `"hello"` so a spoofed `module_id` can't silently steal another module's command stream
- [ ] **Explicitly deferred, documented as known limitation for v1.0**: ZMQ CURVE auth/encryption, web UI TLS, Samba SMB3 sealing, fleet SSH-key/password rotation on clone — the roadmap already frames these as "closed-LAN only" caveats; v1.0 ships with that caveat stated plainly rather than blocked on the multi-day transport-layer work.
- [ ] **Process**: wire the existing `python-app.yml` CI job up as a *required* status check on `main`/`staging` before tagging v1.0 (see GitHub repo settings section below) — a tagged release should not be possible to cut from a red build.

## TODO

Known issues and planned improvements, grouped by priority. Check these off (`- [x]`) as they are completed.
Expand Down
44 changes: 37 additions & 7 deletions saviour-config
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,32 @@ OCTET_A=""; OCTET_B=""; OCTET_C=""
# 3>&1 1>&2 2>&3 swaps fd1 and fd2 so $() captures the selection.
wt() { whiptail "$@" 3>&1 1>&2 2>&3; }

# ── Progress reporting ───────────────────────────────────────────────────────
# Plain status lines, not a whiptail dialog. This used to be
# `whiptail --title "Configuring…" --infobox "\n $1" 7 $W` -- an
# eye-catching box, but each call is its own short-lived newt session: on
# terminals that support smcup/rmcup (virtually every SSH client -- PuTTY,
# xterm-256color, etc.) newt switches to the alternate screen to draw the
# box and switches straight back to the primary screen the instant it
# exits, which for a fire-and-forget infobox is near instantaneous. Between
# calls -- especially during multi-minute steps like `npm install`/
# `apt-get install`, which print nothing to the terminal at all (see
# run_logged) -- the primary screen shows whatever was there before the
# flow started (blank, since configure_device() clears it right before this
# phase begins), which looks like the whole TUI has vanished until the next
# call flashes it back into view. Over SSH specifically this reads as the
# session going completely black for the whole run, with no way to tell
# progress from a hang.
#
# Plain echo has none of that: nothing here touches terminal modes, so nothing
# to flash or restore. It's also unconditionally safe with no controlling
# terminal at all -- e.g. saviour-provision.service invoking
# `saviour-config --apply` at boot, fully headless -- where whiptail would
# fail outright with no tty and, under run_configuration()'s `set -e`, abort
# the entire boot-time provisioning run on its very first progress() call.
progress() {
whiptail --title "Configuring…" --infobox "\n $1" 7 $W
echo
echo "==> $1"
}

log() {
Expand Down Expand Up @@ -584,10 +608,10 @@ configure_device() {
return
fi

# run_configuration()'s progress() infoboxes don't block or get
# dismissed -- the last one shown just stays on screen (whiptail/newt
# never clears between dialogs) until something bigger happens to paint
# over the same region. Explicit clear so it can't linger.
# run_configuration() prints plain progress lines (see progress() above)
# rather than whiptail dialogs, so there's a scrollback's worth of that
# sitting above wherever the cursor is now -- explicit clear so the
# credentials/reboot dialogs below get a clean screen.
clear

# Step 6b: surface first-time login credentials while the operator is
Expand Down Expand Up @@ -1471,6 +1495,12 @@ run_configuration() {

systemctl restart saviour.service >> "$LOG" 2>&1 || true
log "Configuration complete"

# Matches run_clone_fix()'s own set -e/set +e bracketing above -- set -e
# is a global shell option, not function-scoped, so without this a
# successful run left errexit switched on for the rest of the script
# (the main_menu loop, any later reconfiguration, etc.).
set +e
}

# ── Non-interactive apply (boot-time provisioning) ─────────────────────────────
Expand Down Expand Up @@ -1540,6 +1570,6 @@ fi
main_menu
# whiptail/newt dialogs paint their own box without a full-screen clear, and
# never clear on the way out either -- whatever was last drawn (the main
# menu, or a lingering progress/infobox from a prior step) would otherwise
# still be sitting on screen once control returns to the shell prompt.
# menu) would otherwise still be sitting on screen once control returns to
# the shell prompt.
clear
Loading