diff --git a/REPORT.md b/REPORT.md index 99a9f29..cb79f04 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,31 +1,79 @@ -# REPORT — v0.103.0: F-C2-1 config loader no longer corrupts a bcrypt password_hash +# REPORT — v0.104.0: off-box unit discovery (durable, deployment-independent) + no-silent-success -**Date:** 2026-07-07 · **Class:** implementation (controller). **Baseline:** `main` @ `cd0ebd2` -(v0.102.0) → `02d37a1`. Part C of the campaign-2 R1/R2 bundle. +**Date:** 2026-07-09 · **Class:** implementation (controller only). **Baseline:** `main` @ `59eb3be` +(v0.103.0) → `908e4b9` (v0.104.0). Fixes the off-box mis-resolution + silent-success landmine surfaced by +the Storage-Box spike and pinned by the DIAG report. -## The bug (F-C2-1, silent auth-integrity) -`loadAndParse` (config.go:234) and `LoadFromBytes` (:249) ran `os.ExpandEnv` over the ENTIRE YAML -before parse. A bcrypt hash (`$2a$10$…`) is full of `$word` sequences, so `ExpandEnv` silently -replaced each with its (usually empty) env value — corrupting `web.password_hash` on load. Proven: -`$2a$10$N9qo8uL…` → `"a0"`. Depending on the hash this bricks login or degrades toward a bypass. +## The bug (DIAG root cause) +`runOffboxInternal` located each toggled app's recovery unit via `RecoveryUnitPath(AppNamespaceRoot(stack), +stack)`. `AppNamespaceRoot`→`GetAppDrivePath` reads the app's **live** `app.yaml` `HDD_PATH` and returns +`""` when the app isn't currently deployed/scanned — which **silently falls back to `systemDataPath`**. So a +toggled-but-undeployed app (DIAG: on the demo, *no* app is deployed and primary backups sit on the USB drive) +was looked for on the wrong drive → `os.Stat` failed → skipped → the run returned `nil` → status `ok` with +0 snapshots and no operator alert. ## The fix -Removed both `os.ExpandEnv` calls — parse the raw bytes directly. The sanctioned typed env path -(`applyEnvOverrides` → `FELHOM_WEB_PASSWORD_HASH`, applied after parse) is unchanged; no shipped -`controller.yaml` relies on file-level `${VAR}` interpolation (only `docker-compose.yml`'s `${DOMAIN}`, -which is compose-level). Behavior change: a literal `${VAR}` in a value is now preserved verbatim. +- **Discovery over inference** (`internal/backup/offbox.go`): `offboxCandidateNSRoots` builds the durable + candidate set = every registered *schedulable, non-decommissioned* storage path (`GetSchedulableStoragePaths`) + ∪ `systemDataPath`, deduped by resolved nsRoot. `discoverOffboxUnit(app)` scans each for + `backups/primary/`, independent of live deploy state; if the same unit exists on >1 drive (drive + churn) it backs up the **newest by manifest `CreatedAt`** (`offboxUnitTime`, mtime fallback) and WARN-logs + the stale one. A disconnected drive's path is simply absent → `os.Stat` fails → "not here" (correct). +- **No silent success** (`RunOffboxBackup`): `runOffboxInternal` now returns `(backedUp, missing, err)`. + `len(apps)>0 && backedUp==0` → a hard error → `LastStatus="error"`, `LastError` names the missing apps, + and `offboxNotify` fires with a non-nil err (operator alert). A *partial* run stays `ok` but sets a + Hungarian **`OffboxTarget.LastWarning`** naming the skipped apps. +- **`AppNamespaceRoot` and the primary-WRITE paths (`CaptureRecoveryUnit`/dumps) are unchanged** — this only + changes offbox's read/discovery + status reporting. -## Tests + red-proof -`config_test.go`: bcrypt hash loads byte-identical (file + bytes paths) — **red-proof:** pre-fix -`ExpandEnv` mangles it to `"a0"` → FAIL, demonstrated + reverted; `FELHOM_WEB_PASSWORD_HASH` override -still wins; literal `${VAR}` preserved. Full gate `go build/vet/test ./...` = PASS. +## Files changed (commit `908e4b9`) +- `controller/internal/backup/offbox.go` — `offboxCandidateNSRoots`, `discoverOffboxUnit`, `offboxUnitTime`; + `runOffboxInternal` rewritten (discovery loop, new signature); `RunOffboxBackup` 0/N + `LastWarning` logic. +- `controller/internal/settings/settings.go` — `OffboxTarget.LastWarning string` (`last_warning,omitempty`, additive, no migration). +- `controller/internal/web/offbox_handlers.go` — preserve `LastWarning` across a config edit. +- `controller/internal/web/templates/backups.html` — render `LastWarning` on `/backups` in the `--warn` style. +- `controller/internal/backup/offbox_test.go` — 6 new non-hollow tests + helpers. +- `CHANGELOG.md`, `CONTEXT.md`, `controller/README.md` — docs. -## Deploy + live acceptance -Built + pushed `:0.103.0`, deployed to guest 9201 (golden/bootstrap), healthy. **F-C2-1 live belt:** -put a real bcrypt hash IN controller.yaml (the exact bug path), restarted → login with the correct -password **succeeded** (302, no "Hibás jelszó") — the hash loaded intact; pre-fix it would corrupt → -fail. Reverted controller.yaml to the auth-off pre-state (root page 200). +## Tests + companion red-proofs +**Green gate:** `go build ./... && go vet ./... && go test ./...` — all green (module `controller/`). +**Offbox suite: 8 → 14 tests** (before → after). New (all PASS), asserting the **effect** (exact discovered +`src`, `LastStatus`/`LastError`/`LastWarning`, notify-err — never just "no error"): +- `TestOffbox_DiscoversUnitOnRegisteredDrive` (A) — unit on a registered drive (not systemDataPath) is + discovered; backup `src` = the USB nsRoot path; status ok, no warning. +- `TestOffbox_NoUnitAnywhereIsHardError` (B) — 0/N → error, 0 backups, operator alerted, `LastError` names the app. +- `TestOffbox_PartialRunWarnsNotErrors` (C) — present app backed up, notify-err nil, status ok, `LastWarning` names the missing one. +- `TestOffbox_MultipleUnitsPicksNewest` (D) — two copies → exactly one backup, the newer by manifest `CreatedAt`. +- `TestOffbox_AllPresentHappyPath` (E) — both backed up, ok, no warning, snapshot count reflects stats. +- `TestOffbox_NoAppsToggledIsCleanOK` (edge) — 0 toggled → clean ok no-op, no backup call. -## Observation -Migration note: any hand-edited `controller.yaml` carrying a `${...}` that expected file-level -expansion would change behavior — none exist in the repo; the golden/bootstrap writes concrete values. +**Companion red-proofs (run live, then reverted — `git diff` clean):** +- **A:** temporarily reverted the loop to the old `AppNamespaceRoot`-based resolution → `TestOffbox_DiscoversUnitOnRegisteredDrive` + **FAILED** with `run: off-box backup produced no snapshots: 1 app(s) toggled … (missing: audiobookshelf)` + (the old resolution looked at `systemDataPath`, missed the unit on the registered drive). Proves discovery is load-bearing. +- **B:** temporarily guarded the 0/N promotion with `if false` → `TestOffbox_NoUnitAnywhereIsHardError` + **FAILED** with `0-of-N toggled apps backed up must ERROR (no silent success)` (the run silently returned `ok`). + Proves the landmine fix is load-bearing. + +## Deploy verification (guest 9201) +- Built + pushed `gitea.dooplex.hu/admin/felhom-controller:0.104.0` (digest `sha256:431f9f9f…d74`) on the build server. +- Deployed via the bootstrap mechanism (`docker pull` → write `/etc/felhom-controller-image` → restart `felhom-controller-bootstrap.service`). +- `docker ps`: `gitea.dooplex.hu/admin/felhom-controller:0.104.0 Up (healthy)`; `/etc/felhom-controller-image` = `:0.104.0`. +- Startup logs clean: settings loaded, scheduler registered (incl. daily `offbox-backup`), logscanner `errors=0 warnings=0 issues=0`, no panics. + +## NOT yet live-validated against the Storage Box +**This task did NOT re-point offbox at the live Storage Box or run a real offbox→box round-trip** — the spike's +box repos + credentials were torn down. The endpoint-level re-validation against the box is a separate +**supervised** runbook once box access is re-provisioned. The unit suite fully covers the discovery + status logic. + +## Observations (noticed, not acted on — out of scope) +- **Pre-existing flaky test:** `internal/web/async_restore_test.go::TestBackupTier2Restore_DoubleClickRefused` + flakes ~1–2/20 on the **clean baseline** too (confirmed by stashing my changes) — an inherent test race + (`waitFor(IsRunning())` returns once `running=true` is set, but `StopStack` may not have been called yet, so + `prov.stops` can read 0). Not touched (Tier-2, unrelated to this task). Worth a follow-up: have the test + `waitFor(prov.stops==1)` instead of asserting immediately. +- **Discovery vs the demo's reality (context for the supervised re-validation):** on the demo, `systemDataPath` + = `/mnt/sys_drive` (near-empty), the only registered schedulable path is `/mnt/felhom-drives/felhom-usb`, and + all recovery units live on that USB drive — so post-fix discovery will find them there regardless of any app's + (absent) live `app.yaml`. The declared `HDD_PATH` in the surviving backup artifacts (`/mnt/felhom-drives/felhom-flash`) + points at a drive that no longer exists — further reason the durable-registry scan (not the inferred drive) is correct.