docs(controller): backup-architecture — v0.99.0 restore-path fixes (F1 snapshots endpoint, F3 volume dumps real again, O4 resettable-secret generation + residual case)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
This commit is contained in:
2026-07-05 11:57:02 +02:00
parent cf1a9402f2
commit b5d92a8e45
@@ -1,6 +1,6 @@
# Controller backup architecture
Source of truth: felhom-controller `internal/appbackup/`, `internal/backup/`, `internal/quiesce/` at v0.59.0; whole-guest backup is the agent's (`felhom-agent`).
Source of truth: felhom-controller `internal/appbackup/`, `internal/backup/`, `internal/quiesce/` at v0.59.0, updated for the v0.99.0 restore-path fixes (F1/F3/O4 — TASK C1); whole-guest backup is the agent's (`felhom-agent`). Line numbers are v0.59.0-era landmarks — re-grep the symbol.
This document describes what the in-guest controller actually does for backup and restore at v0.59.0. The controller was de-privileged in slice 8C: restic, cross-drive-to-arbitrary-disks execution, drive-recovery and infra-backup were removed from the controller and now live in the host agent + PBS. The `restic` string still appears in the backup package only inside comments that record its removal (`backup.go:20`, `restore.go:14`, `restore.go:19`); there is no restic execution in the controller.
@@ -35,7 +35,9 @@ The agent surface the controller talks to is `internal/agentapi/client.go`: a TL
**Where dumps are stored.** `Manager.GetAppDrivePath(stack)` (`backup.go:88`) returns the app's `HDD_PATH` if it has one, else falls back to the configured `systemDataPath` (the internal SSD) for SSD-only apps. That drive path is mapped to its felhom-data namespace root by `namespaceRoot` (`backup.go:105` — Model A: an in-guest drive mount IS the namespace root, so it is used as-is; only the system-data fallback gets the `felhom-data` subdir appended). Dumps land in `AppDBDumpPath(nsRoot, stack)` = `<nsRoot>/backups/primary/<stack>/db-dumps/` (`paths.go:58`). The SSD-only fallback is the case that the C3 DR finding was about — and it is handled here: SSD-only apps get a correct path via `systemDataPath`, so there is no path gap. (The fallback only warns when `systemDataPath` itself is unconfigured, `backup.go:75`/`backup.go:94`.)
`RunDBDumps` (`backup.go:143`) acquires the running flag, discovers, dumps each DB to its app's path (skipping drives marked disconnected/decommissioned), persists each validation result to `settings.json`, and finally refreshes every recovery unit (`captureAllRecoveryUnits`, `backup.go:247`) — even on partial DB failure, so units never go stale.
`RunDBDumps` (`backup.go:143`) acquires the running flag, discovers, dumps each DB to its app's path (skipping drives marked disconnected/decommissioned), persists each validation result to `settings.json`, then — **v0.99.0 (F3)** — runs the named-volume dumps (`runVolumeDumps`) and finally refreshes every recovery unit (`captureAllRecoveryUnits`) — even on partial failure, so units never go stale.
**Volume dumps are real again (F3, v0.99.0).** After the restic removal, `DumpAppVolumesSafe` had no caller — no trigger ever produced `volume-dumps/`, so named-volume app data was never captured (drill finding F3). `runVolumeDumps` now runs inside the same nightly/manual backup run, per deployed stack: skip if protected (`IsProtectedStack`), skip if it has **no named volumes** (this check deliberately precedes `DumpAppVolumesSafe`, which stops the stack before its own check), skip disconnected/decommissioned drives, else stop → tar each volume (`docker run alpine tar`) → restart. It runs BEFORE the unit capture so `manifest.VolumeDumps` enumerates the fresh tars. A per-stack failure lands in the run summary (`FAIL <app> volumes:`), flips the run's Success flag and fails the run — no silent partials. Note the operational consequence: volume-bearing apps are briefly stopped during each nightly backup (the locked stop-first policy — a live tar of a database volume would be torn).
## 3. Recovery units (`internal/backup/recovery_unit.go`)
@@ -62,9 +64,11 @@ Two keep-side restore entry points exist; neither does a whole-guest restore (th
**The fail-closed data-key gate** is `reconcileRestoreSecrets` (`restore_unit.go:22`) — a pure, unit-tested function. It merges non-secret env with recovered secrets, then:
- A missing **resettable** secret (DB password, admin password) is non-fatal: returned in `missing`, the caller warns and proceeds (`restore_unit.go:117`).
- A missing **resettable** secret (DB password, admin password) is non-fatal: returned in `missing`. **v0.99.0 (O4):** the caller now generates a replacement from the field's catalog `generate` spec (`stacks.GenerateSecretForField` via the `backup.SetSecretGenerator` seam) instead of proceeding with the secret blank — pre-O4, the redeploy failed compose-up with "Defaulting to a blank string". The generated value rides `fullEnv` into `RecreateStackFromUnit``SaveAppConfig`, so it persists encrypted in the guest `app.yaml` and round-trips on later backups. Fields with no `generate` spec still proceed, with a WARN that the app may fail to start. Generation NEVER applies to data-keys (the gate below refuses first, and the generator itself rejects `data_key` fields). **Residual case (honesty note):** generation fully fixes the fresh-init path (empty DB volume → DB initialises with the new credential → dump replay restores the data). If a restored volume tar carries the OLD internal credential hash, the app may still fail auth until a manual in-DB credential reset — generation does not cover that.
- A missing **data-encrypting key** (`DataKeyEnvVars`) is **fatal**: the restore is refused with an explicit error directing the operator to do a PBS whole-guest restore first, because regenerating the key would render the stored data unreadable (`restore_unit.go:45-50`). This is the safety centerpiece: the controller never silently recreates an app whose data it can no longer decrypt.
**The restore panel's data source (F1, v0.99.0).** `GET /api/backup/snapshots?stack=<app>` is backed by `backup.Manager.ListRestorePoints` (`internal/backup/restore_points.go`). The route was a restic-era leftover the backups.html template still fetched; it was never registered, so the snapshot dropdown could never populate and the restore button never enabled (drill finding F1 — the whole keep-side restore UI was dead). It returns at most ONE entry — the current recovery unit (`time` = newest artifact mtime among manifest/db-dumps/volume-dumps, `short_id:"helyi"`, `tier:1`, `drive_label` from the storage registry). Tier-2 entries are never emitted: `POST /backup/restore` only reads the primary unit, so a tier-2 listing would silently restore tier-1 data while claiming tier-2. Guards: invalid/empty stack name → 400, unknown stack → 404, no unit yet → `ok:true, data:[]`.
### 4b. Security note — `.fab` import path validation (CTRL-001, v0.59.0)
The portable `.fab` import (`internal/appexport/`) validates every manifest path segment before it reaches a `filepath.Join` against a trusted base (`appexport.ValidateSegment`, `validate.go:28`; `validateManifestPaths`, `validate.go:51`, called from `UnmarshalManifest`). The attacker-controllable `AppName` / `HDDSubdirs` / `VolumeNames` are rejected on any `..`, path separator, or absolute path, closing the v0.59.0 path-traversal finding. This is cross-referenced here; the detail lives in the appexport doc and the v0.59.0 audit record.