docs: v0.104.0 REPORT — offbox discovery + no-silent-success; deployed 9201, red-proofs recorded
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
This commit is contained in:
@@ -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/<app>`, 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.
|
||||
|
||||
Reference in New Issue
Block a user