diff --git a/CONTEXT.md b/CONTEXT.md index a5ff93f..7994013 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,6 +7,23 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" +Last updated: 2026-06-22 (v0.76.0 — campaign-#3 hardening: settings recovery, restore-name validation, quiesce-marker quarantine) + +> **2026-06-22 — v0.76.0 (deployed on 9201): three campaign-#3 hardening fixes.** +> - **S1**: corrupt `settings.json` no longer crash-loops — `save()` writes a last-known-good `.bak` +> (after the primary rename); `Load()` recovers from `.bak`, else preserves the corrupt file as +> `*.corrupt-` + safe defaults. New `Settings.LoadWarning` → dashboard banner. (`main.go` Fatalf +> now only on the IO-unreadable path.) +> - **F2**: `web.validStackName` gates `backupRestoreHandler` + `apiExportStart` (reject `/ \ .. NUL`) +> before any restore/export — closes the traversal defense-in-depth gap (no escape had occurred, but +> it relied on downstream map-lookups). +> - **S3**: `quiesce.readMarker` logs `[WARN]` + quarantines a corrupt marker to `*.corrupt-` +> instead of silently dropping it. +> All three live-validated on 9201 (truncate settings → recover from .bak no crash-loop; traversal +> restore → rejected, /etc intact; corrupt quiesce marker → quarantined). Tests + red-proofs. +> Still-open campaign-#3 deferrals (NOT done): time-chaos on a dedicated VM, host reboot (supervised), +> `.fab` import compose-fuzzing. See `felhom.eu/documentation/tests/test-campaign-3-2026-06-22-findings.md`. + Last updated: 2026-06-22 (v0.75.0 — gate userdata MkdirAll on a live mountpoint) > **2026-06-22 — v0.75.0 (deployed on 9201): userdata MkdirAll gated on a live mountpoint.** diff --git a/REPORT.md b/REPORT.md index e160bec..a23c005 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,55 +1,49 @@ -# REPORT — controller v0.75.0: gate userdata MkdirAll on a live mountpoint +# REPORT — controller v0.76.0: campaign-#3 hardening (3 fixes) ## Baseline → target -`felhom-controller` `main` v0.74.0 → **v0.75.0**. Controller-only; agent/hub untouched. +`felhom-controller` `main` v0.75.0 → **v0.76.0**. Controller-only; agent/hub untouched. -## Problem -Two `MkdirAll`-into-`/userdata` sites fired without checking the drive was mounted (campaign-#2 -findings #2/#3): the deploy belt (`ensureUserdataMounts`) and the FileBrowser sync. During a -drive-absent window they logged `mkdir …/userdata: permission denied` + caused transient `Created` -container flapping — and worse, writing into an unmounted mountpoint lands app data on the guest -**rootfs**, shadowed when the drive returns (data-integrity + rootfs-fill hazard). +## What changed (commit `b0dd131`) +Three findings from chaos campaign #3, severity-ordered: -## Change (commit `0b2605c`) -- `internal/stacks/manager.go` — `ensureUserdataMounts` skips when the `HDD_PATH` drive root is an - **external** path (`!= sysDataPath`) that is **not a live mountpoint**; the app is held by - `planDriveGates`. New injectable `Manager.isMountPoint` seam (defaults to `system.IsMountPoint`). - The system/local path is never gated. -- `internal/web/handlers.go` — the FileBrowser sync loop skips (and doesn't mount) a path under - `StableParentDir` that isn't a live mountpoint, via the new pure `skipFileBrowserPath` helper. -- `EnsureUserdataDir`/`EnsureUserdataSkeleton`/`planDriveGates` unchanged (gated the callers). +- **S1 [MEDIUM] — corrupt `settings.json` no longer crash-loops the controller.** + `internal/settings/settings.go`: `save()` writes a last-known-good `.bak` **after** the primary + rename succeeds (best-effort). `Load()` on a parse error recovers from `.bak` (re-promotes to primary), + else preserves the corrupt file as `*.corrupt-` and starts on safe defaults — never returns the + error that drove `main.go`'s `Fatalf`/crash-loop. New `Settings.LoadWarning` → dashboard banner + (`handlers.go` + `dashboard.html`). `main.go` `Fatalf` retained (now only the IO-unreadable path). +- **F2 [MEDIUM, defense-in-depth] — `stack_name` validated against path traversal.** + `internal/web/validate.go` `validStackName` (single segment; rejects `/ \ .. NUL`), gated in + `backupRestoreHandler` (`handlers.go`) and `apiExportStart` (`handler_export.go`) before any + restore/export work. +- **S3 [LOW] — corrupt quiesce marker quarantined, not silently dropped.** + `internal/quiesce/quiesce.go` `readMarker` logs a `[WARN]` + renames a bad-JSON marker to + `*.corrupt-` (still returns "no marker" → no recovery, the correct contract). -## Tests (green; red-proofed) -`go build ./... && go vet ./... && go test ./...` all green. -- T1 `TestEnsureUserdataMounts_SkipsAbsentExternalDrive` — belt skips when external drive absent. - Red-proof: removing the gate → dir created → FAIL (shown, reverted). -- T2 `TestEnsureUserdataMounts_EnsuresWhenMounted` — belt ensures when drive mounted. -- T3 `TestEnsureUserdataMounts_SystemPathNeverSkipped` — system/local path never gated (must-not-over-gate). -- T4 `TestSkipFileBrowserPath` — only an absent external path is skipped; mounted + system paths kept. - Red-proof: gate→false → absent `usb` not skipped → FAIL (shown, reverted). +## Tests (green; all red-proofed) +`go build ./... && go vet ./... && go test ./...` — green. +- T-S1a recover-from-bak, T-S1b unrecoverable→safe-defaults+preserve, T-S1c save-writes-bak, T-S1d happy. + Red-proof: revert `Load` to error-on-parse → T-S1a/b FAIL. +- T-F2a `validStackName` (pure), T-F2b restore handler rejects traversal (no nil-deref), T-F2c export + handler → 400. Red-proof: drop the restore gate → traversal falls through to the nil-backupMgr + redirect → T-F2b FAIL. +- T-S3a quarantine+log, T-S3b happy. Red-proof: revert `readMarker` to silent → T-S3a FAIL. ## Deploy -Built+pushed `gitea.dooplex.hu/admin/felhom-controller:0.75.0` on 192.168.0.180 (digest -`sha256:f735676e…`; **build-server `git pull` done first** — the v0.74.0 stale-source gotcha), -deployed to guest 9201 via the bootstrap path. `docker inspect`: `image=:0.75.0 running healthy`. +Built+pushed `:0.76.0` on 192.168.0.180 (build-server `git pull` done first), deployed to 9201 via +bootstrap. `docker inspect`: `image=:0.76.0 running healthy`. -## LIVE acceptance — campaign-#2 C6 re-run, now clean -Disconnected felhom-flash (8 apps) and observed the drive-absent window: +## LIVE re-test — the three campaign-#3 scenarios, now fixed +1. **S1:** ensured `.bak`, truncated `settings.json` → restart → `status=running RestartCount=0` + (no crash-loop), logs `[ERROR] primary settings corrupt … — attempting recovery from .bak` → + `[WARN] recovered settings from .bak; re-promoting to primary`; config intact (3 storage_paths), 25/0. +2. **F2:** `POST /backup/restore stack_name=../../../etc` → `flash_error=Érvénytelen alkalmazásnév`, + log `[WARN] restore rejected: invalid stack_name "../../../etc"`, **no "Starting app-data restore"** + (gate fired before the restore logic); `/etc/passwd` intact (24 lines). +3. **S3:** planted a corrupt `quiesce-state.json` → restart → `[WARN] [quiesce] marker … is corrupt … + — quarantining; stacks not auto-recovered`, original renamed to `*.corrupt-`, `RestartCount=0`, + 25/0 (stacks not stranded). -| Check | campaign #2 (pre-fix) | **v0.75.0 (fixed)** | -|---|---|---| -| `mkdir …/userdata: permission denied` | many | **0** | -| "could not ensure userdata skeleton" | present | **0** | -| rootfs shadow dir at unmounted mountpoint | created | **none** — `/mnt/felhom-drives/felhom-flash/userdata` does not exist (bare mountpoint) | -| apps during absent window | transient `Created` flapping | held cleanly (down) | -| reconnect | recovered | **recovered: flash remounted /dev/sdc1, 8 apps restarted → 25 running / 0 unhealthy** | - -The integrity hazard (app data shadowed onto the rootfs) is closed: the now-unmounted mountpoint is -empty, no `userdata/...` was written to the rootfs. - -## Not touched / design note (see CONTEXT.md) -The **boot-time** `mkdir … permission denied` (daemon boot-restore) is a different cause — **docker** -auto-starts drive-backed containers (`restart: unless-stopped`) before the agent mounts the drives, so -docker (not the belt) tries to create the bind source; `planDriveGates` recovers them after mount -convergence. This task ships only the runtime gate; the boot-ordering decision is documented, not -implemented. +## Still-open campaign-#3 deferrals (NOT in this task) +Time-chaos on a dedicated VM / injectable clock; host-reboot (supervised); `.fab` import +compose-fuzzing. Agent/hub untouched.