diff --git a/CHANGELOG.md b/CHANGELOG.md index 32b0197..df187cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,64 @@ +## v0.231.0 — the box proves its own off-site copy still holds something (2026-08-31, R-87) +**MinAgent: 0.129.0** (unchanged) + +**The weekly check proves the stored bytes are the bytes we stored. It cannot tell us we stored the +WRONG thing.** A hollow recovery unit backs up cleanly, checks cleanly at 100 % depth, restores +cleanly and gives the customer nothing back — R-403 measured that on demo-hp on 2026-08-31, 120 082 +104 B to 7 036 B in one nightly run, recorded as a success. **Nothing in the product asked that +question, on any tier, at any cadence.** Now one job does, every night, on one app. + +**Scope is the SPIKE's verdict, not the row as filed.** `audits/SPIKE-restic-restore-test-2026-08-31.md` +measured that an unattended scratch restore would have caught **one** of the five restore-path defects +drills found in six days. As a test of our restore code it is not worth an evening; as the only thing +asking *"is there anything in there?"* it is. **This does NOT prove a restore puts data back into a +running app** — that stays drill work, and `07` §8 matrix row 4 does not move. + +- **THE ACCEPTANCE RULE HAS TWO PARTS AND THE OBVIOUS ONE IS A TRAP.** "Check the unit against its own + packing list" passes on an empty package — a hollow unit declares nothing, so everything it declares + is present. So: (1) everything declared is present, **and** (2) the manifest declares what the app is + supposed to have. Part 2 is the whole value. `TestR87_HollowUnitForAnAppWithADatabaseFAILS` is the + fence, red-proofed against the naive rule. +- **THE EXPECTATION COMES FROM INSIDE THE UNIT, never from the live box** — the snapshot may predate + the app's current shape, and `GetDockerVolumes` enumerates from live Docker, which answers a + different question. The unit's own `compose/docker-compose.yml` answers both halves: `DBServiceNames` + (the same discriminator `RestoreFromRecoveryUnit` uses) and `ParseComposeNamedVolumes`. +- **THE VOLUME HALF IS AN EXISTENCE CHECK, NOT A NAME MATCH, deliberately.** Volume tars are + `_.tar` and `ResolveDockerVolumeNames` derives the project from the compose file's + parent directory — which inside a unit is the literal string `compose`. Measured on all eight real + units on demo-hp the counts match exactly and `_.tar` held every time, but "held on + eight" is not "derivable" (R-355). Half a rule that is true beats a whole rule that is invented. +- **THREE OUTCOMES, NOT TWO:** pass, fail (readable and empty), and **cannot judge**. Collapsing the + third into a pass hides a real gap; into a failure, it alarms on our own blind spot. An app that + legitimately has neither a database nor volumes **PASSES** — `TestR87_AppWithNoDatabaseAndNoVolumesPasses`, + red-proofed by alarming on any empty unit. +- **IT NEVER WRITES TO THE REPOSITORY, and that is asserted as a NON-EFFECT.** `--no-lock`, no + `unlockStale`, and `m.runner()` rather than `resticStep` so the `unlock --remove-all` escalation is + unreachable rather than unlikely. The spike measured that `restic restore` takes no lock and `restic + check` does (R-407). `TestR87_NeverWritesToTheRepository` asserts the argv. +- **IT TAKES THE SINGLE-WRITER FLAG ITSELF and skips rather than waits.** `RestoreOffboxScratch` does + not take it (R-408) while `offbox_integrity.go` states that invariant as universal; this job does not + wait for that to be fixed. +- **DUE-NESS IS PER SNAPSHOT (R-86's model), never per clock.** `ProvedSnapshots[stack]` holds the + snapshot ID proved. A timestamp re-proves the same snapshot forever — red-proofed, and it also breaks + the rotation. +- **ITS SCRATCH IS A SEPARATE ROOT** (`backups/offsite-proof`), and that is not tidiness: the job + deletes its copy on every path, and sharing `backups/offsite-restore/` would mean a nightly + background job deleting the verification copy a CUSTOMER is looking at. It is also invisible to + placement, so a proof copy can never be pushed into a live app. +- **NEW EVENT TYPE `offsite_proof_empty`, severity `error`, operator-only** — deliberately NOT + `backup_integrity_failed`, whose Hungarian template says the store is damaged. Here the store is + sound and the content is absent: a different fact, a different action. **The hub half ships in the + same commit** (allowlist + `operatorOnlyEvents`), because an unallowlisted type is 400'd and vanishes. +- Verdict published on `OffboxReportStatus` with the `StatsKnown` absence rule: `last_proof_result` + absent means NOT RECORDED, never "failed". +- Slot **05:30**, chosen from the live schedule read off demo-hp (offbox-backup 04:15 / 2m52s, + abandon-sweep 05:10, offsite-integrity 06:00 / 40.3 s). Measured cost: **one app ~2.3–4.0 s**, all + eight 25 s — cheaper than the weekly check beside it. +- 33 new tests. `unitOnlyHeadroom` extracted so the proof and the customer restore share one gate and + one Hungarian refusal. + +--- + ## v0.230.0 — a poorer copy must never delete a richer one (2026-08-31, R-403) **MinAgent: 0.129.0** (unchanged) diff --git a/CONTEXT.md b/CONTEXT.md index e49b8b5..027825e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,70 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-08-31 (v0.230.0 — R-403: a poorer copy must never delete a richer one) +Last updated: 2026-08-31 (v0.231.0 — R-87: the box proves its own off-site copy still holds something) + +> **2026-08-31 — v0.231.0. FOUR RULINGS, recorded so none is re-litigated.** +> +> **1. THE ACCEPTANCE RULE IS NOT "EVERY DECLARED FILE IS PRESENT".** That is the spike's own one-line +> summary and taken literally it is worthless: a hollow unit declares nothing, so everything it +> declares is present, and the check passes on exactly the shape it exists to catch. The rule has TWO +> parts and needs both — (1) everything declared is present, AND (2) the manifest declares what the app +> is SUPPOSED to have. **Part 2 is the whole value; part 1 alone is the trap.** +> `TestR87_HollowUnitForAnAppWithADatabaseFAILS` is the fence and its red-proof models the naive rule. +> +> **2. THE EXPECTATION COMES FROM INSIDE THE UNIT, NEVER FROM THE LIVE BOX.** R-403's guard could tell +> hollow from legitimately-empty because it had TWO copies to compare; this has ONE. The snapshot may +> predate the app's current shape, and the point is to judge the snapshot on its own terms — so the +> source is the unit's own `compose/docker-compose.yml`, and `GetDockerVolumes` (live Docker, +> `backup.go`) is explicitly NOT it. Database half: `DBServiceNames`, the same discriminator +> `RestoreFromRecoveryUnit` already uses, so this cannot disagree with the restore path about what an +> app is. Volume half: `ParseComposeNamedVolumes`. +> +> **THE VOLUME HALF IS AN EXISTENCE CHECK AND NOT A NAME MATCH, and that half-rule is deliberate.** +> Volume tars are `_.tar`; `ResolveDockerVolumeNames` derives the project from +> `filepath.Base(filepath.Dir(composePath))`, which inside a unit is the literal string `compose`, not +> the stack. Measured 2026-08-31 on all eight real units on demo-hp: the counts match exactly +> (bookstack 2/2, docmost 3/3, kimai 2/2, opengist 1/1, privatebin 1/1, calibre-web 1/1, paperless-ngx +> 3/3, romm 3/3) and `_.tar` held in every case. **"Held on eight" is not "derivable"** +> — R-355's standing rule is that a claim about the app must never be inferred from a counter. Half a +> rule that is true beats a whole rule that is invented. Name-level matching is available the moment +> the capture records the project, and is not worth inventing before then. +> +> **3. THE PROOF'S RESTORE IS A READ-ONLY VARIANT, NOT A CHANGE TO THE CUSTOMER'S PATH.** +> `RestoreOffboxScratch` is the customer's restore, it is proven, and a customer restore taking a lock +> is correct — so it was NOT changed. What was shared instead of forked: the scratch-dir resolver is +> now parameterised on its ROOT builder (`offboxScratchDirIn`), so the drive-preference rules, the +> network-storage refusal and the R-252 wording have exactly one implementation; and the unit-only +> headroom gate is extracted to `unitOnlyHeadroom` so both paths refuse at the same floor with the same +> Hungarian sentence. The proof's own three differences are the ones that must differ: `--no-lock`, no +> `unlockStale`, and `m.runner()` instead of `resticStep` so the `unlock --remove-all` escalation is +> unreachable rather than unlikely (REUSE.md's rule: replacing `resticStep` would hide the escalation +> from the assertion that must see it). +> +> **THE PROOF SCRATCH IS A SEPARATE ROOT (`backups/offsite-proof`) AND THAT IS A SAFETY DECISION, NOT +> TIDINESS.** The proof deletes its copy on every path including failure. Sharing +> `backups/offsite-restore/` would mean a nightly background job deleting the verification copy a +> CUSTOMER made and is looking at — a poorer actor destroying a richer one, R-403's shape in different +> clothes. The separate root also keeps the proof copy invisible to `DeleteOffsiteRestoreCopy`, the +> copy listing and `OffboxFullScratchReady`, so it can never be offered for placement into a live app. +> +> **4. THE ALARM IS A NEW EVENT TYPE, AND REUSING `backup_integrity_failed` WOULD HAVE BEEN WRONG.** +> That type is the nearest existing one and it carries a hub-side Hungarian template saying the +> integrity check found an error — i.e. **the store is damaged**. Here the store is sound and the +> CONTENT is missing: a different fact, a different cause, a different customer action, and telling +> someone their backups are damaged when they are not is the more expensive mistake (the same asymmetry +> `looksLikeRepositoryDamage` is shaped around). So `offsite_proof_empty` was minted, severity `error`, +> with NO `customerMessages` entry so the controller's dynamic Hungarian survives, and **the hub half — +> `allowedEventTypes` plus `operatorOnlyEvents` — ships in the same commit**, because an unallowlisted +> type is answered 400 and vanishes, and a missing customerMessages entry is not a routing block. +> **This widened the task's stated scope to `felhom.eu/hub/`** and the reason is recorded here rather +> than left as an unexplained diff. +> +> **DELIBERATELY NOT DONE:** `07` §8 matrix row 4 was NOT moved. This proves the snapshot CONTAINS a +> recoverable unit; it does not prove a restore puts data back into a running app. R-408 (the missing +> `acquireRunning` on `RestoreOffboxScratch`) was NOT fixed — the job takes the flag itself and the row +> stays open. + > **2026-08-31 — v0.230.0. THREE RULINGS, recorded so none is re-litigated.** > diff --git a/REUSE.md b/REUSE.md index 5dcef5f..93c4167 100644 --- a/REUSE.md +++ b/REUSE.md @@ -55,6 +55,9 @@ | `unitRestoreOutcomeMsg` + its three message constants (R-353, v0.226.0) | controller/internal/web/handlers.go | `(app string, res backup.UnitRestoreResult) string` | THE customer sentence for a completed LOCAL restore | Twin of `reconstituteOutcomeMsg`; copy its SHAPE (clauses earned by having done the thing, no filesystem path, base names only), never its text. The constants are named because `r353_unit_outcome_test.go` asserts them verbatim — a silent edit is how an honest message drifts back into a comforting one, which is the documented history of the sentence it replaces | | `offsiteNoSpaceMsgFmt` + `offsiteSizeUnknownMsg` (R-357, v0.226.0) | controller/internal/backup/offbox_restore.go | two consts | EVERY headroom refusal on the off-site restore surface | **All four gates share these** (prepare, scratch, place, and the destructive reconstitute). A customer meeting one wording on one path and a different one on another has to work out whether it is the same problem. **The reconstitute gate uses NO ×1.1 margin** — it copies a measured tree; `OffboxRestorePrepareFull`'s ×1.1 predicts a download. **Fail closed when either probe reads ≤ 0**: `free < need` with `need == 0` is FALSE, so an unmeasurable input sails through — a gate present and inert | | `Manager.OffboxFullScratchReady` + the scratch marker (R-358, v0.226.0) | controller/internal/backup/offbox_restore.go | `(stack) bool`; `.felhom-restore-complete.json` | THE gate for place-to-live and reconstitute | **It answers "did the run FINISH and was it FULL", not "are there files".** The old non-empty check passed a part-copy from a failed restic run, and the old doc comment ("PlaceOffsiteRestore re-validates per-path completeness") is what made it look adequate — that call stats top-level placements, not files. Marker written 0600 atomically AFTER restic returns nil; stale one cleared BEFORE it starts; both orders pinned by an AST test because `resticStep` is not a seam. Anything else — absent, unreadable, wrong schema, `full:false` — is NOT ready, with a WARN naming which. **Unit-only and full restores write the SAME directory**, so `full` is the only separator (R-396) | +| `JudgeRestoredUnit` + `UnitProofResult` (R-87, v0.231.0) | controller/internal/backup/r403_hollow.go | `(unitDir string) UnitProofResult` | THE question "does this app's backup contain what THIS APP should have" | **The rule has TWO parts and part 1 alone is the trap:** "everything declared is present" passes a HOLLOW unit, because a hollow unit declares nothing — the exact shape it exists to catch. Part 2 is the expectation, and it comes from the unit's **own** captured compose file (`UnitComposeDir(unitDir)` + the compose filename), NEVER from live Docker (`GetDockerVolumes` describes the running app; the snapshot may predate it). Database half is `DBServiceNames`, the same discriminator `RestoreFromRecoveryUnit` uses, so this cannot disagree with the restore path about what an app is. **The volume half is an EXISTENCE check, not a name match** — `ResolveDockerVolumeNames` derives the project from the compose file's parent dir, which inside a unit is the literal string `compose`. **THREE outcomes:** pass / fail / **cannot judge**, and the third is never collapsed. Size is never consulted (`TestR87_SizeIsNeverConsulted`) | +| `Manager.ProveOffboxUnit` + `ProofResult` (R-87, v0.231.0) | controller/internal/backup/offbox_proof.go | `(ctx) ProofResult` | THE nightly off-site content proof — one app, its newest snapshot, restored read-only and judged | **It NEVER writes to the repository and that is asserted on the ARGV:** `--no-lock`, no `unlockStale`, and `m.runner()` rather than `resticStep` so the `unlock --remove-all` escalation is unreachable. **It takes `acquireRunning` ITSELF** because `RestoreOffboxScratch` does not (R-408) — do not remove that. **Due-ness is per SNAPSHOT** (`ProvedSnapshots[stack]`), never a timestamp: a timestamp re-proves the same snapshot forever AND breaks the rotation. **Its scratch is a SEPARATE root** (`offsiteProofRootFor`, `backups/offsite-proof`) — sharing the customer's `offsite-restore` root would let a nightly job delete a copy the customer is looking at. A skip, a missing snapshot and a restore error reach NO verdict and do not advance due-ness | +| `unitOnlyHeadroom` + `offboxScratchDirIn` (R-87, v0.231.0) | controller/internal/backup/offbox_restore.go | `(free int64) error`; `(stack, rootFor)` | The unit-only free-space gate and the drive-preference resolver, **shared** by the customer restore and the nightly proof | Extracted rather than forked so the two paths cannot drift on the parts that must not differ — the floor, the Hungarian refusal (`offsiteNoSpaceMsgFmt`), the network-storage refusal and the R-252 wording. `rootFor` is the ONLY difference between the two scratch paths. **Fail-closed on an unmeasurable probe:** `offboxFree` returns 0 when it cannot read, and `0 < floor` refuses — the inverse of the R-357 shape where a gate went inert | | `Manager.CheckOffboxIntegrity` + `IntegrityResult` + `IntegrityDue` (R-359, v0.227.0) | controller/internal/backup/offbox_integrity.go | `(ctx) IntegrityResult`; `(now) (due bool, last time.Time)` | THE off-site integrity check, and the only place `restic check` is run | **It TAKES `acquireRunning` and SKIPS rather than waits — never remove that guard.** `resticStep` escalates to `unlock --remove-all` on a lock error and is only safe because every caller holds the single-flight mutex; a check without it can strip a LIVE prune's lock. **THREE outcomes, not two:** `Skipped`, `Unreachable` and failed are different facts — a timeout is unreachable, NEVER damage, and only a failure notifies. A skip and an unreachable repo do **not** advance due-ness; a failure does. **DUE-NESS, NOT A WEEKDAY** (R-341). **`looksLikeRepositoryDamage` matches PHRASES, not words** — bare `pack `/`tree `/`snapshot ` appear in restic's ordinary progress output and made a healthy run look corrupt | | `runOffsiteIntegrityCheck` + `integrityFailedMsg` / `integrityOKMsg` (R-397, v0.227.0) | controller/cmd/controller/main.go | `(ctx, mgr, notifier, logger, force) backup.IntegrityResult` | THE one caller of the check — the scheduled job AND the debug button both go through it | ONE function so the hand-run cannot drift from the scheduled one; `force` skips due-ness and **nothing else**. Wired to `NotifyIntegrityOK` / `NotifyIntegrityFailed`, which existed with no caller since the notifier did (sixth built-but-never-wired instance). **`ok` is severity `info` and therefore mails NOBODY by design** — a weekly success e-mail is how alerts stop being read. The customer gets a sentence; restic's output goes to the log truncated (R-379) | | **`SetOffboxRunner` / `m.runner()` — the restic exec seam, and it has ALWAYS existed** | controller/internal/backup/offbox.go (~L52, ~L386) | `offboxRunner func(ctx, env, args...) ([]byte, error)` | Driving ANY restic-backed path under test | **R-398 claimed there was no such seam and was WRONG — do not re-file it.** `resticStep` is not itself overridable, but the layer it calls is, and tests have driven restic paths through it since the off-site tier shipped. **Use this rather than adding a `resticStepFn`:** replacing `resticStep` would hide its `unlock --remove-all` escalation from exactly the assertions that must see it (R-359's lock-safety tests assert `unlock` never appears in any argv) | diff --git a/controller/README.md b/controller/README.md index 6b6f991..b611286 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1089,6 +1089,52 @@ backups/primary// maradtak."). **Every one is a claim about the BACKUP, never about the app** — see CONTEXT.md's ruling and 07-backup-architecture §6.3. +### Off-site content proof (v0.231.0, R-87) + +**What it is:** every night the box restores ONE app's newest off-site snapshot into a throwaway +folder, asks whether that backup still contains the app's actual data, records which snapshot it +proved, and deletes the copy. + +**The question it answers, and why the integrity check beside it cannot.** `restic check` proves the +stored bytes are the bytes we stored. **It cannot tell us we stored the wrong thing.** A hollow +recovery unit — no database dump, no volume tar — backs up cleanly, checks cleanly at 100 % depth, +restores cleanly, and gives the customer nothing back. Measured on `demo-hp` 2026-08-31 (R-403): +120 082 104 B became 7 036 B in one nightly run, recorded as a success. + +> **WHAT IT DOES NOT PROVE, said plainly because the green tick invites the other reading:** it does +> **not** prove a restore puts data back into a running app. It restores to a throwaway folder, looks, +> and deletes. It never touches a live app. Putting data back is drill work, and +> `07-backup-architecture.md` §8 matrix row 4 does not move on the strength of this job. + +| | | +|---|---| +| job | `offsite-proof`, `sched.Daily` at **05:30** | +| cadence | **per SNAPSHOT, not per clock** (R-86's model) — an app is due when its newest snapshot's ID differs from the ID last proved for it. One app per run; eight apps are covered in eight nights, and a NEW backup makes an app due again immediately | +| acceptance rule | **two parts, and both are needed.** (1) everything the manifest declares is present in the restored unit, AND (2) **the manifest declares what the app is supposed to have.** Part 1 alone passes a hollow unit, which is the shape this exists to catch | +| where the expectation comes from | the unit's **own** `compose/docker-compose.yml`, never the live box — the snapshot may predate the app's current shape. Database: `DBServiceNames` (the same discriminator the restore path uses). Volumes: `ParseComposeNamedVolumes`, as an **existence** check, not a name match | +| outcomes | **three:** pass, fail (readable and empty), and **cannot judge**. An app that legitimately has no database and no named volumes **passes** | +| repository writes | **none.** `--no-lock`, no `unlockStale`, and the exec seam rather than `resticStep`, so the `unlock --remove-all` escalation is unreachable. Asserted on the argv as a non-effect | +| guard | takes the single-writer flag itself and **SKIPS rather than waits** (`RestoreOffboxScratch` does not take it — R-408) | +| scratch | `backups/offsite-proof/` — a **separate root** from the customer's `backups/offsite-restore/`, so the nightly delete can never reach a copy the customer made, and a proof copy can never be offered for placement | +| timeout | 10 min (`proofRestoreTimeout`) — ~150x the slowest single app measured | +| cost, measured on demo-hp | **one app 2.3–4.0 s**, all eight back to back **25 s**, peak scratch = that app's logical size (213 MB largest). The weekly check beside it takes 40.3 s | +| result | persisted on `settings.OffboxTarget` (`proved_snapshots`, `last_proof_*`) and published on `OffboxReportStatus`. `last_proof_result` absent = **NOT RECORDED** (a pre-0.231.0 controller), never "failed" | + +**Notifications.** A failure emits **one** `offsite_proof_empty`, severity `error`, operator-only. A +pass, a skip, a "cannot judge" and a restore error emit **nothing** — a nightly success mail is how +people stop reading their alerts, and alarming on our own blind spot trains the operator to discount +the one alarm that matters. + +> **⚠ IT IS DELIBERATELY NOT `backup_integrity_failed`.** That type means **the store is damaged** and +> carries a hub-side Hungarian template saying so. Here the store is sound and the CONTENT is absent — +> a different cause and a different action. The message says the backup is *readable* and does *not* +> contain the app's data, and explicitly that the store is not damaged. + +> **`restic restore --verify` is NOT used as a correctness check and must not be.** Measured on +> `demo-hp` 2026-08-31: a byte changed in place in a restored 160 MB tar, with size and mtime +> preserved, **passed clean**; verify took 131 ms on a 213 MB tree, which cannot be hashing. It is a +> size-and-mtime reconciliation. + ### Off-site integrity check (v0.227.0, R-359/R-397) **What it is:** a `restic check` against the off-site repository, run by the controller itself. Until diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 14ff0f7..c965c6b 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -1140,6 +1140,26 @@ func main() { runOffsiteIntegrityCheck(ctx, backupMgr, notifier, logger, false) return nil }) + + // R-87 — the nightly off-site PROOF: one app, its newest snapshot, restored read-only and + // judged, then deleted. + // + // 05:30 CHOSEN FROM THE LIVE SCHEDULE, read off demo-hp on 2026-08-31 and not from a + // document: db-dump 02:30, tier2-backup 03:30, offbox-backup 04:15 (2m52s measured), + // offsite-abandon-sweep 05:10, offsite-integrity 06:00 (40.3s measured at 100% depth). 05:30 + // sits in the measured empty gap — 20 min after the sweep starts and 30 min before the + // integrity check, whose flag it shares. + // + // THE WHOLE RUN IS SECONDS, so the slot has room it does not need: all eight apps back to + // back measured 25 s on this box, and this job does ONE. The backup WINDOW is + // customer-configurable, so no fixed time is collision-proof on every box — but a collision + // costs one skipped day rather than a missed proof, because due-ness is per SNAPSHOT and + // tomorrow tries the same app again. That is the identical argument the integrity job's own + // slot comment makes, and it is the reason skip-if-busy is the right policy here too. + sched.Daily("offsite-proof", "05:30", func(ctx context.Context) error { + runOffsiteProof(ctx, backupMgr, notifier, logger) + return nil + }) } // Metrics prune — daily at 04:00 @@ -3165,6 +3185,66 @@ const ( integrityOKBase = "A távoli mentés ellenőrzése rendben lezajlott." ) +// ── R-87 — the nightly off-site PROOF's one caller ───────────────────────────────────────────── +// +// ONE function, like runOffsiteIntegrityCheck above, so a future debug button cannot drift from the +// scheduled run. Every guard lives inside `ProveOffboxUnit`; this owns only what happens to the +// verdict afterwards, because only the caller knows whether a verdict counts. +// +// THE FOUR NON-ALARMING OUTCOMES ARE NOT FAILURES and none of them notifies: +// - Skipped — it yielded to a running backup. Correct behaviour; alarming would punish it. +// - NoSnapshot — nothing is due. Not a fact about any backup. +// - Err — the restore did not finish, so it SAW NOTHING and may not claim anything about +// the backup. This is the same separation `IntegrityResult` draws between a failed +// check and an unreachable store, and it is why a download error can never become +// the "intact but empty" alarm. +// - CannotJudge — recorded, never alarmed. Alarming on our own blind spot trains the operator to +// discount the one alarm that means the customer's backup holds nothing. +func runOffsiteProof(ctx context.Context, mgr *backup.Manager, n *notify.Notifier, logger *log.Logger) backup.ProofResult { + if mgr == nil { + return backup.ProofResult{Skipped: true, SkipReason: "backup manager not configured"} + } + res := mgr.ProveOffboxUnit(ctx) + if res.Verdict() == "" { + return res // skipped / nothing due / restore error — no verdict was reached + } + mgr.RecordProofVerdict(res) + if res.Judgement.Verdict == backup.UnitProofFail { + // ONE event. The customer-grade sentence goes in the message; the machine detail goes in the + // detail field, where the operator can diagnose without a rebuild — the R-379 split. + n.NotifyOffsiteProofEmpty(proofEmptyMsg(res.Stack), proofEmptyDetail(res)) + } + if logger != nil && res.Judgement.Verdict == backup.UnitProofCannotJudge { + logger.Printf("[WARN] [offbox] proof: %s could not be judged (%s) — recorded, deliberately NOT alarmed", + res.Stack, res.Judgement.Reason) + } + return res +} + +// proofEmptyMsg is the R-87 customer-grade sentence. A named function because tests assert it +// verbatim and because a silent edit is how an honest message drifts into a comforting one. +// +// IT MUST SAY THE TWO THINGS THAT MATTER AND NOT A THIRD. The backup is READABLE — the store is not +// damaged, and saying so is load-bearing, because "sérült" (damaged) sends the operator and the +// customer down the R-359 path, which is a different fault with a different remedy and a standing +// instruction not to delete anything. And it names the ONE action that helps: the backup has to be +// made again, which is a capture-side fix. +func proofEmptyMsg(stack string) string { + return "A(z) " + stack + " legutóbbi távoli mentése olvasható, de nem tartalmazza az alkalmazás adatait. " + + "A tároló nem sérült — a mentés készült el üresen. A mentést újra el kell készíteni; addig ebből a mentésből nem lehet visszaállítani." +} + +// proofEmptyDetail is the OPERATOR half: the machine reason code, the snapshot it was proved on, and +// what was expected. Never a file's content and never a path outside the unit — units carry portable +// secrets. +func proofEmptyDetail(res backup.ProofResult) string { + d := "snapshot " + res.SnapshotID + ", reason " + string(res.Judgement.Reason) + if len(res.Judgement.Missing) > 0 { + d += ", expected: " + strings.Join(res.Judgement.Missing, " ") + } + return d +} + // integrityOKMsg states what was actually checked, so a structure-only pass is never read as a // full data verification. The depth is a fact the customer's sentence has to carry: "checked" means // two different things depending on it. diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index 3f707aa..77521db 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -1041,7 +1041,7 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error } o.LastError = "" o.SnapshotCount = snapshots - o.StatsKnown = true // R-225: measured, even if the answer is zero + o.StatsKnown = true // R-225: measured, even if the answer is zero o.EnlargedBlocked = blockedNames // replace each run (sorted); empty slice clears it var warns []string // Zero-toggle honesty (take-two obs.): a configured target with NOTHING selected reports @@ -1428,9 +1428,9 @@ type OffboxReportStatus struct { // hub cannot see that and must not infer it. It keeps being sent until the ACK stops reporting a // superseded package, so a lost request retries by itself rather than leaving the pair half-gone. // Absent/false on every other box, so a healthy report is byte-identical to v0.205.0's. - AbandonPurgeRequested bool `json:"abandon_purge_requested,omitempty"` - LastRun string `json:"last_run,omitempty"` // RFC3339 - LastStatus string `json:"last_status,omitempty"` // "ok" | "incomplete" (R-203) | "error" | "running" + AbandonPurgeRequested bool `json:"abandon_purge_requested,omitempty"` + LastRun string `json:"last_run,omitempty"` // RFC3339 + LastStatus string `json:"last_status,omitempty"` // "ok" | "incomplete" (R-203) | "error" | "running" // LastSuccess (R-100) is the last run that SUCCEEDED — the hub's staleness anchor. Absent on a // pre-v0.181.0 controller, which the hub must degrade on explicitly rather than by accident: // treating absence as failure alarms every un-upgraded box, treating it as success keeps the bug. @@ -1475,6 +1475,22 @@ type OffboxReportStatus struct { // that only read the index, and those are different news. Absent = the box cannot answer (a // controller older than v0.228.0), never "structure". LastIntegrityDepth string `json:"last_integrity_depth,omitempty"` + // ── R-87: the nightly PROOF's verdict ──────────────────────────────────────────────────────── + // + // A DIFFERENT QUESTION FROM THE THREE ABOVE, and the fields are separate so nobody reads one for + // the other. `last_integrity_*` says the stored bytes are the bytes we stored. These say the + // newest backup of ONE app still CONTAINS that app's data — which a perfectly intact store can + // fail (R-403 measured it). + // + // FOURTH APPLICATION OF THE StatsKnown RULE: `LastProofResult` is "" when this box has never + // reached a proof verdict — a controller older than v0.231.0 sends no key at all — and a reader + // MUST degrade to NOT RECORDED, never to "failed". A skip, a missing snapshot and a restore error + // all leave it untouched, because none of them looked at a backup. + LastProofRun string `json:"last_proof_run,omitempty"` // RFC3339 + LastProofStack string `json:"last_proof_stack,omitempty"` + LastProofSnapshot string `json:"last_proof_snapshot,omitempty"` + LastProofResult string `json:"last_proof_result,omitempty"` // "pass" | "fail" | "cannot_judge" + LastProofReason string `json:"last_proof_reason,omitempty"` } // OffsiteStateNeedsCredential is the ONE declared state (v0.199.0, R-204 item 4 / R-193): this box @@ -1665,10 +1681,15 @@ func (m *Manager) OffboxReportStatus() *OffboxReportStatus { Enabled: true, EscrowState: t.EscrowState, LastRun: t.LastRun, LastStatus: t.LastStatus, LastSuccess: t.LastSuccess, SnapshotCount: t.SnapshotCount, RepoSizeBytes: t.RepoSizeBytes, QuotaGB: t.QuotaGB, - StatsKnown: t.StatsKnown, // R-331 — without it the hub cannot tell "empty" from "unmeasured" - LastIntegrityCheck: t.LastIntegrityCheck, // R-359 - LastIntegrityOK: t.LastIntegrityOK, - LastIntegrityDepth: t.LastIntegrityDepth, // R-399 + StatsKnown: t.StatsKnown, // R-331 — without it the hub cannot tell "empty" from "unmeasured" + LastIntegrityCheck: t.LastIntegrityCheck, // R-359 + LastIntegrityOK: t.LastIntegrityOK, + LastIntegrityDepth: t.LastIntegrityDepth, // R-399 + LastProofRun: t.LastProofRun, // R-87 + LastProofStack: t.LastProofStack, + LastProofSnapshot: t.LastProofSnapshot, + LastProofResult: t.LastProofResult, + LastProofReason: t.LastProofReason, AbandonPurgeRequested: t.AbandonPurgeRequested, // R-241: declared until the hub drops the package } } diff --git a/controller/internal/backup/offbox_proof.go b/controller/internal/backup/offbox_proof.go new file mode 100644 index 0000000..da0601c --- /dev/null +++ b/controller/internal/backup/offbox_proof.go @@ -0,0 +1,318 @@ +package backup + +import ( + "context" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// ── R-87 — the box proves its own off-site copy still HOLDS something ──────────────────────────── +// +// WHAT THIS ANSWERS THAT NOTHING ELSE DOES. `restic check --read-data-subset=100%` (R-359/R-399) runs +// weekly and proves the stored bytes are the bytes we stored. **It cannot tell us we stored the wrong +// thing.** A hollow recovery unit — no database dump, no volume tar — backs up cleanly, checks +// cleanly at full depth, restores cleanly, and gives the customer nothing back. R-403 measured that +// shape on this fleet on 2026-08-31: 120 082 104 B became 7 036 B in one nightly run, recorded as a +// success. Until this job, no tier and no cadence asked the question. +// +// WHAT IT DELIBERATELY DOES NOT ANSWER, said here because the green tick will be read as covering it +// otherwise: **it does not prove a restore puts data back into a running app.** It restores to a +// throwaway directory, looks, and deletes. Putting data back is drill work, and the spike said so — +// of the five restore-path defects human drills found in the six days to 2026-08-31, an unattended +// scratch restore would have caught ONE (R-356). `07-backup-architecture.md` §8 matrix row 4 does not +// move on the strength of this job. +// +// THE SHAPE IS `CheckOffboxIntegrity`'S, deliberately — same file conventions, same guards, same +// three-outcome honesty — because a second scheduled off-site job that reasons differently about the +// single-writer flag is how the two come to disagree about repository safety. +// +// ── THE HAZARD, AND WHY THIS JOB TAKES A FLAG THAT ITS RESTORE DOES NOT ───────────────────────── +// +// `resticStep` self-heals a crash lock by running `unlock --remove-all`, and its own doc comment +// records that this is safe only because *"the in-process single-flight mutex (held by every caller +// of this method) proves no sibling operation is live"*. +// +// `offbox_integrity.go`'s header states that invariant as universal — *"Every off-site operation +// takes acquireRunning for exactly that reason"* — and **it is not true today**: +// `RestoreOffboxScratch` does not take it (R-408, measured 2026-08-31; `restore_wizard.go` records +// the same fact independently and the UI works around it with a separate display flag). This job +// therefore takes the flag ITSELF rather than waiting for R-408 to be fixed, and SKIPS rather than +// waits — a skipped proof simply runs tomorrow, whereas waiting would pin a nightly backup behind it. +// +// ── AND IT NEVER WRITES TO THE REPOSITORY, FOR REAL ──────────────────────────────────────────── +// +// R-95's constraint is that the off-site credential can delete, so a scheduled job must not be able +// to write. The spike established what "read-only" actually costs here, by observation rather than by +// reading the code: +// +// - `restic restore` in 0.14.0 takes NO repository lock (measured on demo-hp with a lock sampler +// that was positively controlled first: it saw a lock appear and vanish across a real `check`, and +// zero across two restores); +// - `restic check` DOES take one — so the comment claiming the check never writes is wrong (R-407); +// - the PRODUCT's restore path writes anyway, because `unlockStale` runs `restic unlock` — a delete +// verb against `locks/` — before every restore (`offbox_restore.go`). +// +// So this job runs its restore with `--no-lock` and WITHOUT `unlockStale`, through `m.runner()` +// rather than `resticStep`, so the `unlock --remove-all` escalation is not merely unlikely but +// unreachable. `TestR87_NeverWritesToTheRepository` asserts that as a NON-EFFECT — the argv, not the +// absence of an error. Using the seam rather than replacing `resticStep` is REUSE.md's own rule: +// replacing it would hide the escalation from exactly the assertion that must see it. +// +// `restic restore --verify` is NOT used as a correctness check and must never be: the spike measured +// it passing a byte-level corruption with size and mtime preserved, on a 213 MB tree, in 131 ms. It +// is a size-and-mtime reconciliation. The correctness question is answered by `JudgeRestoredUnit`. + +// proofRestoreTimeout bounds one proof restore. +// +// CHOSEN from measurement, not inherited: on demo-hp 2026-08-31 a unit restore took 2.3–4.0 s per app +// regardless of size (185 KB → 2.25 s, 213 MB → 3.20 s — the cost is per-snapshot round-trip plus +// roughly 1 s per 200 MB), and all eight apps back to back took 25 s. Ten minutes is ~150x the +// slowest single app measured, so the failure mode of a wedged SFTP mount is a released flag and a +// retry tomorrow, never a box whose backups stop because a proof never returned. It is deliberately +// well under `integrityCheckTimeout` (30 min): this job downloads ONE app's unit, not the store. +const proofRestoreTimeout = 10 * time.Minute + +// ProofResult is what one nightly proof did, so the log, the event, the report and any future debug +// endpoint state the same facts from one place. +// +// Skipped is a first-class outcome and NOT a failure — a proof that yielded to a running backup did +// the right thing. NoSnapshot is separated from a failure for the same reason R-185 separated "I +// could not look" from "I looked and it is broken": an app with no off-site snapshot yet is a +// staleness question, which R-339 and the hub's tier deadline already own, and alarming here would be +// a second alarm for a fact that already has one. +type ProofResult struct { + RanAt time.Time + Duration time.Duration + Stack string + SnapshotID string + Judgement UnitProofResult + + Skipped bool + SkipReason string + NoSnapshot bool + // Err is a restore/plumbing failure — NOT a verdict about the backup. It never becomes the + // "intact but empty" alarm, because a download that did not finish has seen nothing. + Err error +} + +// Verdict renders the outcome for the persisted record and the wire. "" is reserved for "no verdict +// was reached", which is what a skip, a missing snapshot and a restore error all are. +func (r ProofResult) Verdict() string { + if r.Skipped || r.NoSnapshot || r.Err != nil { + return "" + } + return string(r.Judgement.Verdict) +} + +// ProveOffboxUnit runs ONE nightly proof: pick the due app, restore its newest snapshot read-only to +// a throwaway directory, judge it, delete the copy, and record WHICH SNAPSHOT was proved. +// +// The order below is not arrangeable. The first three guards run before anything is downloaded, and +// the scratch is removed on EVERY path including failure — a failed judgement that leaves a copy +// behind is a slow disk-filler and, worse, a copy nobody knows the provenance of. +func (m *Manager) ProveOffboxUnit(ctx context.Context) ProofResult { + res := ProofResult{RanAt: time.Now()} + + if !m.OffboxConfigured() { + // A box with no off-site tier has nothing to prove. Silent, and not an alarm. + res.Skipped, res.SkipReason = true, "no off-site target configured" + return res + } + if err := m.acquireRunning(); err != nil { + res.Skipped, res.SkipReason = true, "a backup or restore is already running" + m.logger.Printf("[INFO] [offbox] proof: skipped — %s; due-ness is NOT advanced, so this retries on the next run", res.SkipReason) + return res + } + defer m.releaseRunning() + + stack, id, unitPath, ok := m.nextProofTarget(ctx) + if !ok { + res.NoSnapshot = true + m.logger.Printf("[INFO] [offbox] proof: nothing due — every deployed app's newest off-site snapshot has already been proved, or none has one yet") + return res + } + res.Stack, res.SnapshotID = stack, id + + scratch, nsRoot, derr := m.offboxProofScratchDir(stack) + if derr != nil { + res.Err = derr + m.logger.Printf("[WARN] [offbox] proof: %s has nowhere to restore to: %v", stack, derr) + return res + } + // The headroom gate runs BEFORE the download, through the same helper and the same Hungarian + // wording the customer's restore uses (`unitOnlyHeadroom`). A gate after the download is not a + // gate. + if herr := unitOnlyHeadroom(m.offboxFree()(nsRoot)); herr != nil { + res.Err = herr + m.logger.Printf("[WARN] [offbox] proof: %s refused before any download: %v", stack, herr) + return res + } + + // Whatever happens from here, the copy goes. Registered before the restore so an early return + // cannot leak one, and it removes a stale copy from an interrupted previous run on the way in. + m.removeProofScratch(stack, scratch) + defer m.removeProofScratch(stack, scratch) + + start := time.Now() + if rerr := m.restoreUnitReadOnly(ctx, stack, id, unitPath, scratch); rerr != nil { + res.Duration = time.Since(start) + res.Err = rerr + m.logger.Printf("[ERROR] [offbox] proof: %s could not be restored for proving (nothing is concluded about the backup): %v", stack, rerr) + return res + } + res.Duration = time.Since(start) + + // The unit sits inside the scratch at its own absolute snapshot path. + res.Judgement = JudgeRestoredUnit(filepath.Join(scratch, strings.TrimPrefix(unitPath, string(filepath.Separator)))) + switch res.Judgement.Verdict { + case UnitProofPass: + m.logger.Printf("[INFO] [offbox] proof: %s PASSED on snapshot %s in %s — the backup holds what this app should have", + stack, id, res.Duration.Round(time.Millisecond)) + case UnitProofCannotJudge: + m.logger.Printf("[WARN] [offbox] proof: %s on snapshot %s could NOT be judged (%s) — recorded as such, never as a pass", + stack, id, res.Judgement.Reason) + default: + m.logger.Printf("[ERROR] [offbox] proof: %s on snapshot %s is READABLE AND EMPTY (%s%s) — the store is not damaged; the backup does not contain this app's data", + stack, id, res.Judgement.Reason, missingSuffix(res.Judgement.Missing)) + } + return res +} + +// missingSuffix renders the Missing list for a log line without ever printing a path outside the unit. +func missingSuffix(missing []string) string { + if len(missing) == 0 { + return "" + } + return ": " + strings.Join(missing, ", ") +} + +// RecordProofVerdict persists the outcome and advances due-ness. The caller owns WHEN a verdict +// counts, exactly as `RecordIntegrityVerdict` does. +// +// A SKIP, A MISSING SNAPSHOT AND A RESTORE ERROR DO NOT REACH A VERDICT and must not advance +// due-ness — tomorrow tries again. A `cannot_judge` DOES advance it: re-downloading the same +// unjudgeable snapshot every night is load with no new information, the outcome is recorded where a +// surface can read it, and a new snapshot makes the app due again by itself. +func (m *Manager) RecordProofVerdict(res ProofResult) { + v := res.Verdict() + if v == "" { + return + } + if err := m.settings.UpdateOffboxStatus(func(o *settings.OffboxTarget) { + if o.ProvedSnapshots == nil { + o.ProvedSnapshots = map[string]string{} + } + o.ProvedSnapshots[res.Stack] = res.SnapshotID + o.LastProofRun = res.RanAt.UTC().Format(time.RFC3339) + o.LastProofStack = res.Stack + o.LastProofSnapshot = res.SnapshotID + o.LastProofResult = v + o.LastProofReason = string(res.Judgement.Reason) + }); err != nil { + m.logger.Printf("[ERROR] [offbox] proof: could not persist the outcome: %v — the proof RAN and its verdict for %s on %s was %q, but due-ness did not advance, so it will run again tomorrow", + err, res.Stack, res.SnapshotID, v) + } +} + +// nextProofTarget picks ONE app: the deployed app whose newest off-site snapshot has not been proved. +// +// DUE-NESS IS PER SNAPSHOT, NOT PER CLOCK (R-86's model, 07 §3). An app is due when the ID of its +// newest snapshot differs from the ID last proved for it. A timestamp would re-prove the same +// snapshot forever and say nothing about the newest one — which is the defect R-341 recorded in a +// different costume. +// +// Apps are considered in a stable sorted order and the FIRST due one wins, so eight apps are covered +// in eight nights and the rotation cannot starve one: an app stops being due only once its newest +// snapshot has actually been proved. +func (m *Manager) nextProofTarget(ctx context.Context) (stack, id, unitPath string, ok bool) { + if m.stackProvider == nil { + return "", "", "", false + } + proved := map[string]string{} + if t := m.settings.GetOffboxTarget(); t != nil && t.ProvedSnapshots != nil { + proved = t.ProvedSnapshots + } + var names []string + for _, st := range m.stackProvider.ListDeployedStacks() { + names = append(names, st.Name) + } + sort.Strings(names) + for _, name := range names { + if !isSafeStackName(name) { + continue + } + sid, paths, err := m.offboxLatestSnapshot(ctx, name) + if err != nil || strings.TrimSpace(sid) == "" { + // No snapshot yet for this app is NOT a failure of this test — staleness belongs to + // R-339 and the hub's tier deadline. Logged, never alarmed. + continue + } + if proved[name] == sid { + continue // already proved on exactly this snapshot + } + up := offboxUnitPathOf(paths, name) + if up == "" { + // The snapshot exists and carries no recovery unit. That IS the strongest form of + // "nothing recoverable", so it is a target rather than a skip — the judgement below will + // find no manifest and fail closed. + m.logger.Printf("[WARN] [offbox] proof: %s snapshot %s lists no recovery-unit path", name, sid) + continue + } + return name, sid, up, true + } + return "", "", "", false +} + +// restoreUnitReadOnly downloads ONE recovery unit into scratch WITHOUT writing anything to the +// repository. +// +// Three differences from `RestoreOffboxScratch`, each deliberate and each named: +// - `--no-lock`, so restic does not create a lock file (it does not for `restore` in 0.14.0 anyway +// — measured — but the flag makes that a guarantee rather than an observed behaviour of one +// version); +// - no `unlockStale`, so no delete verb is issued against `locks/`; +// - `m.runner()` rather than `resticStep`, so the `unlock --remove-all` escalation is unreachable +// rather than merely unlikely. +// +// It deliberately does NOT write the R-358 completion marker: that marker certifies a scratch for +// PLACEMENT into a live app, and a proof copy must never be placeable. Its absence is what keeps this +// directory inert even if the two roots were ever confused. +func (m *Manager) restoreUnitReadOnly(ctx context.Context, stack, id, unitPath, scratch string) error { + if err := os.MkdirAll(scratch, 0o755); err != nil { + return fmt.Errorf("proof scratch dir: %w", err) + } + t := m.settings.GetOffboxTarget() + base, env := m.offboxBaseArgs(t) + rctx, cancel := context.WithTimeout(ctx, proofRestoreTimeout) + defer cancel() + args := append(append([]string{}, base...), + "restore", id, "--target", scratch, "--include", unitPath, "--no-lock") + out, err := m.runner()(rctx, env, args...) + if err != nil { + return fmt.Errorf("offbox proof restore %s: %w: %s", stack, err, truncate(out)) + } + return nil +} + +// removeProofScratch deletes a proof copy, refusing loudly if the path is not strictly inside a +// proof root — the same prefix instinct as `DeleteOffsiteRestoreCopy`, because reaching here with an +// out-of-sandbox path means a helper above is wrong and a best-effort skip would hide that. +func (m *Manager) removeProofScratch(stack, scratch string) { + clean := filepath.Clean(scratch) + for _, drive := range m.offsiteRestoreDriveRoots() { + root := filepath.Clean(m.offsiteProofRootFor(drive)) + string(filepath.Separator) + if strings.HasPrefix(clean+string(filepath.Separator), root) { + if err := os.RemoveAll(clean); err != nil { + m.logger.Printf("[WARN] [offbox] proof: could not remove the proof copy %s: %v", clean, err) + } + return + } + } + m.logger.Printf("[WARN] [offbox] proof: refusing to remove %s — it is not inside a proof root", clean) +} diff --git a/controller/internal/backup/offbox_restore.go b/controller/internal/backup/offbox_restore.go index b0ede6f..9f95fd7 100644 --- a/controller/internal/backup/offbox_restore.go +++ b/controller/internal/backup/offbox_restore.go @@ -27,6 +27,20 @@ const ( offboxUnitOnlyFreeFloor = int64(2) << 30 ) +// unitOnlyHeadroom is THE unit-only free-space gate, shared by the customer's scratch restore and the +// R-87 nightly proof so the two can never disagree about how much room a unit restore needs or about +// what the customer is told when there is not enough (REUSE.md's `offsiteNoSpaceMsgFmt` rule). +// +// FAIL-CLOSED ON AN UNMEASURABLE PROBE. `offboxFree` returns 0 when it cannot read the filesystem, +// and `0 < floor` is true, so an unreadable drive REFUSES rather than sailing through — the inverse +// of the R-357 shape where `free < need` with `need == 0` made a gate inert. +func unitOnlyHeadroom(free int64) error { + if free < offboxUnitOnlyFreeFloor { + return fmt.Errorf(offsiteNoSpaceMsgFmt, humanizeBytes(offboxUnitOnlyFreeFloor), humanizeBytes(free)) + } + return nil +} + // SetOffboxFreeFn overrides the restore free-space probe (tests; the Windows go-test host has no df). func (m *Manager) SetOffboxFreeFn(fn func(path string) int64) { m.offboxFreeFn = fn } @@ -163,8 +177,29 @@ func (m *Manager) offboxSnapshotSize(ctx context.Context, id string) (int64, err func (m *Manager) offboxRestoreScratchDir(stack string) (scratch, nsRoot string, err error) { // offsiteRestoreRootFor is THE place `backups/offsite-restore` is spelled (offbox_verify_copies.go) // — the listing/delete surface must resolve byte-identical paths to the ones written here. + return m.offboxScratchDirIn(stack, m.offsiteRestoreRootFor) +} + +// offboxProofScratchDir is the R-87 nightly proof's scratch, resolved by the SAME drive-preference +// rules and into a DIFFERENT root (`backups/offsite-proof`). +// +// THE SEPARATE ROOT IS NOT TIDINESS, IT PREVENTS A DELETE. The proof removes its scratch on every +// path, including failure. Sharing `backups/offsite-restore/` would mean a nightly background +// job deleting the verification copy a CUSTOMER made and is looking at — a poorer actor destroying a +// richer one, which is R-403's shape wearing different clothes. A separate root also keeps the proof +// copy invisible to `DeleteOffsiteRestoreCopy`, the copy listing and `OffboxFullScratchReady`, so it +// can never be offered for placement into a live app. +func (m *Manager) offboxProofScratchDir(stack string) (scratch, nsRoot string, err error) { + return m.offboxScratchDirIn(stack, m.offsiteProofRootFor) +} + +// offboxScratchDirIn holds the drive-preference rules once. `rootFor` chooses WHICH root under the +// namespace the scratch lands in; everything else — the network-storage refusal, the ordering, the +// R-252 wording — is shared, so the proof path can never drift from the customer path on the parts +// that must not differ. +func (m *Manager) offboxScratchDirIn(stack string, rootFor func(string) string) (scratch, nsRoot string, err error) { scratchFor := func(root string) (string, string) { - return filepath.Join(m.offsiteRestoreRootFor(root), stack), m.namespaceRoot(root) + return filepath.Join(rootFor(root), stack), m.namespaceRoot(root) } isNet := func(path string) bool { return m.settings != nil && m.settings.IsNetworkStoragePath(path) } // (1) the app's own drive — preferred, but ONLY if it is not NETWORK storage (F-3afix-1). restic @@ -262,8 +297,8 @@ func (m *Manager) RestoreOffboxScratch(ctx context.Context, stack string, full b if free < need { return fmt.Errorf(offsiteNoSpaceMsgFmt, humanizeBytes(need), humanizeBytes(free)) } - } else if free < offboxUnitOnlyFreeFloor { - return fmt.Errorf(offsiteNoSpaceMsgFmt, humanizeBytes(offboxUnitOnlyFreeFloor), humanizeBytes(free)) + } else if herr := unitOnlyHeadroom(free); herr != nil { + return herr } // F-A1 hygiene: drop the legacy rootfs scratch (DataDir/offbox-restore/) best-effort. legacy := filepath.Join(m.cfg.Paths.DataDir, "offbox-restore", stack) @@ -383,7 +418,6 @@ func (m *Manager) writeScratchMarker(scratch, snapshotID string, full bool) erro return os.Rename(tmp, final) } - // OffboxRestorePrepareFull resolves the latest snapshot's restore-size and verifies scratch headroom // for a FULL restore WITHOUT starting it (the two-step size-first gate). Returns the human size on // success, or a Hungarian error to flash on refusal (size unknown / no headroom — fail-closed). diff --git a/controller/internal/backup/offbox_verify_copies.go b/controller/internal/backup/offbox_verify_copies.go index 910a1a1..e8be0e8 100644 --- a/controller/internal/backup/offbox_verify_copies.go +++ b/controller/internal/backup/offbox_verify_copies.go @@ -35,6 +35,14 @@ func (m *Manager) offsiteRestoreRootFor(drivePath string) string { return filepath.Join(m.namespaceRoot(drivePath), "backups", "offsite-restore") } +// offsiteProofRootFor returns `/backups/offsite-proof` for a drive path. THE single place +// these segments are written (R-87), and deliberately a SIBLING of offsite-restore rather than a +// subdirectory of it: nothing that lists, offers or deletes a customer verification copy walks this +// root, which is the whole point — see offboxProofScratchDir for the delete it prevents. +func (m *Manager) offsiteProofRootFor(drivePath string) string { + return filepath.Join(m.namespaceRoot(drivePath), "backups", "offsite-proof") +} + // offsiteRestoreDriveRoots returns every drive path a verification copy could live under, in the same // preference order offboxRestoreScratchDir uses to CHOOSE one — so listing can never miss a copy the // restore path was capable of creating. Deduplicated, order preserved. diff --git a/controller/internal/backup/r403_hollow.go b/controller/internal/backup/r403_hollow.go index 1231cd0..95375ea 100644 --- a/controller/internal/backup/r403_hollow.go +++ b/controller/internal/backup/r403_hollow.go @@ -1,5 +1,12 @@ package backup +import ( + "os" + "path/filepath" + "sort" + "strings" +) + // R-403 — a POORER copy must never delete a RICHER one. // // Measured on demo-hp 2026-08-31, on the shipped v0.229.0: an app's Tier-2 copy went from @@ -48,3 +55,180 @@ func unitCarriesData(unitDir string) bool { // unitIsHollow is `unitCarriesData` negated, named for the way both callers actually ask it. It is a // separate function only so the call sites read as the question they are asking. func unitIsHollow(unitDir string) bool { return !unitCarriesData(unitDir) } + +// ── R-87 — does this app's backup contain what THIS APP should have? ───────────────────────────── +// +// THE ACCEPTANCE RULE, AND WHY THE OBVIOUS ONE IS A TRAP. +// +// The spike that re-scoped R-87 summarised this job as "check the unit against its own packing list". +// Taken literally that is worthless: **a hollow unit declares nothing, so everything it declares is +// present, and the check passes on exactly the shape it exists to catch.** R-403 measured that shape +// on this fleet — 120 082 104 B of dumps and tars became 7 036 B of neither, and the run recorded +// itself a success. +// +// So the rule has TWO parts and needs both: +// +// 1. everything the manifest declares is present in the restored unit, AND +// 2. the manifest declares what the app is SUPPOSED to have. +// +// Part 2 is the whole value. Part 1 alone is the trap. +// +// WHERE THE EXPECTATION COMES FROM, AND WHY IT IS NOT THE LIVE BOX. +// +// R-403's guard could tell hollow from legitimately-empty because it had TWO copies to compare. This +// has ONE. The expectation therefore comes from INSIDE the unit — its own captured +// `compose/docker-compose.yml` — and never from the running app: +// +// - the snapshot may predate the app's current shape, and the point is to judge the snapshot on its +// own terms rather than against a box that has moved on; +// - `GetDockerVolumes` (backup.go) enumerates from LIVE Docker, which answers a different question. +// +// THE TWO HALVES, both from the unit's own compose: +// +// - DATABASE — `DBServiceNames()` names the compose SERVICES whose `image:` is a supported engine. +// It is the same honest discriminator `RestoreFromRecoveryUnit` already uses to decide whether a +// dump must replay, so this cannot disagree with the restore path about what an app is. If the +// compose declares a database service, the unit must declare at least one database dump. +// - NAMED VOLUMES — `ParseComposeNamedVolumes()` reads the top-level `volumes:` block. If the +// compose declares named volumes, the unit must declare at least one volume tar. +// +// THE VOLUME HALF IS DELIBERATELY AN EXISTENCE CHECK AND NOT A NAME MATCH, and the reason is that a +// name match cannot be done honestly from inside a unit. Volume tars are named +// `_.tar`, and `ResolveDockerVolumeNames` derives the project from +// `filepath.Base(filepath.Dir(composePath))` — which inside a unit is the literal string `compose`, +// not the stack. Measured 2026-08-31 on all eight real units on demo-hp, the count matches exactly +// (bookstack 2/2, docmost 3/3, kimai 2/2, opengist 1/1, privatebin 1/1, calibre-web 1/1, +// paperless-ngx 3/3, romm 3/3) and `_.tar` held in every case — but "held on eight" is +// not "derivable", and R-355's standing rule is that a claim about the app must never be inferred +// from a counter. Half a rule that is true beats a whole rule that is invented. +// +// THREE OUTCOMES, NOT TWO. "I could not judge this" is a first-class answer: collapsing it into a +// pass hides a real gap, and collapsing it into a failure alarms on our own blind spot. That is the +// same distinction `IntegrityResult` draws between a failed check and an unreachable store. +// +// ONE MANIFEST READER: this uses `readManifest`, the same function `unitCarriesData` above uses. +// `unitCarriesData` answers the coarse question ("does this unit carry anything at all") and stays +// exactly as R-403 shipped it; this answers the finer one, and an app whose unit carries nothing at +// all while its compose declares either a database or a volume fails BOTH. + +// UnitProofVerdict is the outcome of judging one restored recovery unit. +type UnitProofVerdict string + +const ( + // UnitProofPass — the unit declares what the app should have, and holds everything it declares. + UnitProofPass UnitProofVerdict = "pass" + // UnitProofFail — the backup is READABLE and does not hold the app's data. Its canonical case is + // the R-403 shape: intact and empty. That is NOT the same fact as a damaged store and must never + // be reported as one — the customer's action differs. + UnitProofFail UnitProofVerdict = "fail" + // UnitProofCannotJudge — the unit does not carry enough to answer. Never reported as a pass. + UnitProofCannotJudge UnitProofVerdict = "cannot_judge" +) + +// UnitProofReason is a stable machine code for WHY, so the log, the event and the message can each +// say the same thing without re-deriving it from prose. +type UnitProofReason string + +const ( + ProofReasonOK UnitProofReason = "" + ProofReasonManifestUnreadable UnitProofReason = "manifest_unreadable" + ProofReasonDeclaredFileMissing UnitProofReason = "declared_file_missing" + ProofReasonNoDatabaseDump UnitProofReason = "database_expected_none_captured" + ProofReasonNoVolumeDump UnitProofReason = "volumes_expected_none_captured" + ProofReasonComposeMissing UnitProofReason = "compose_missing" + ProofReasonComposeUnparseable UnitProofReason = "compose_unparseable" +) + +// UnitProofResult is what one judgement decided, and enough to say it out loud. +type UnitProofResult struct { + Verdict UnitProofVerdict + Reason UnitProofReason + // Missing names the declared-but-absent files (ProofReasonDeclaredFileMissing) or the expectation + // that went unmet. ASCII-safe: file names and compose service names only, never a path outside the + // unit and never any file CONTENT — units carry portable secrets (R-9's rule for this whole area). + Missing []string +} + +// OK reports whether this verdict is the passing one. A helper rather than a `==` at every call site, +// because "not a failure" and "a pass" are different questions here and the third outcome is why. +func (r UnitProofResult) OK() bool { return r.Verdict == UnitProofPass } + +// JudgeRestoredUnit applies the rule above to a restored recovery-unit DIRECTORY. +// +// It is PURE and does no network, no docker and no restic: given a directory it reads the manifest, +// the captured compose and the presence of the declared files, and nothing else. Size is never +// consulted — `TestR403_SizeIsNeverConsulted` guards that for the R-403 predicate and +// `TestR87_SizeIsNeverConsulted` guards it here, because "how big" has never been the question. +func JudgeRestoredUnit(unitDir string) UnitProofResult { + man := readManifest(UnitManifestFile(unitDir)) + if man == nil { + // FAIL CLOSED. A unit whose manifest cannot be read is a unit whose contents cannot be + // vouched for — R-403's own rule, and the direction matters: treating it as sound would let + // an unreadable backup pass as a proved one, which is the failure this job exists to end. + return UnitProofResult{Verdict: UnitProofFail, Reason: ProofReasonManifestUnreadable} + } + + // Part 1 — everything declared is actually there. + var missing []string + for _, f := range man.DBDumps { + if !fileExistsIn(UnitDBDumpDir(unitDir), f) { + missing = append(missing, "db-dumps/"+f) + } + } + for _, f := range man.VolumeDumps { + if !fileExistsIn(UnitVolumeDumpDir(unitDir), f) { + missing = append(missing, "volume-dumps/"+f) + } + } + for _, f := range man.ConfigFiles { + if !fileExistsIn(UnitComposeDir(unitDir), f) { + missing = append(missing, "compose/"+f) + } + } + if len(missing) > 0 { + return UnitProofResult{Verdict: UnitProofFail, Reason: ProofReasonDeclaredFileMissing, Missing: missing} + } + + // Part 2 — the expectation, from the unit's own compose. + composePath := filepath.Join(UnitComposeDir(unitDir), "docker-compose.yml") + if _, err := os.Stat(composePath); err != nil { + // CANNOT JUDGE, never a pass. Without the compose there is no honest expectation, and the + // alternative — assuming the app needs nothing — is precisely how a hollow unit would slip + // through. + return UnitProofResult{Verdict: UnitProofCannotJudge, Reason: ProofReasonComposeMissing} + } + dbServices, err := DBServiceNames(composePath) + if err != nil { + // DBServiceNames already refuses to let "cannot tell" read as "no database" (its own doc + // comment); this carries that refusal outward instead of flattening it. + return UnitProofResult{Verdict: UnitProofCannotJudge, Reason: ProofReasonComposeUnparseable} + } + if len(dbServices) > 0 && len(man.DBDumps) == 0 { + return UnitProofResult{Verdict: UnitProofFail, Reason: ProofReasonNoDatabaseDump, Missing: dbServices} + } + volumes := ParseComposeNamedVolumes(composePath) + if len(volumes) > 0 && len(man.VolumeDumps) == 0 { + names := make([]string, 0, len(volumes)) + for _, v := range volumes { + names = append(names, v.Name) + } + sort.Strings(names) + return UnitProofResult{Verdict: UnitProofFail, Reason: ProofReasonNoVolumeDump, Missing: names} + } + return UnitProofResult{Verdict: UnitProofPass, Reason: ProofReasonOK} +} + +// fileExistsIn reports whether name exists as a regular file directly inside dir. +// +// `name` is a manifest-declared BASE name; it is joined and then checked to still be inside dir, so a +// manifest carrying `../../etc/passwd` cannot make this answer about a file outside the unit. The +// manifest travels inside the snapshot and a restore writes it from the store, so it is not a trusted +// input — this is the same instinct as DeleteOffsiteRestoreCopy's prefix check. +func fileExistsIn(dir, name string) bool { + p := filepath.Clean(filepath.Join(dir, name)) + if !strings.HasPrefix(p, filepath.Clean(dir)+string(filepath.Separator)) { + return false + } + fi, err := os.Stat(p) + return err == nil && fi.Mode().IsRegular() +} diff --git a/controller/internal/backup/r87_judgement_test.go b/controller/internal/backup/r87_judgement_test.go new file mode 100644 index 0000000..839d019 --- /dev/null +++ b/controller/internal/backup/r87_judgement_test.go @@ -0,0 +1,297 @@ +package backup + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// R-87 Group A — the JUDGEMENT. +// +// Every test here builds a real recovery-unit directory on disk and asks `JudgeRestoredUnit` about +// it. Nothing is mocked, because there is nothing to mock: the predicate is pure file reads, and a +// fake manifest reader would test the fake. + +// unitFixture builds a recovery-unit directory. Empty slices mean "declared nothing"; the files are +// written only for what is declared, so the fixture cannot accidentally satisfy part 1 of the rule. +type unitFixture struct { + compose string // contents of compose/docker-compose.yml; "" = do not write it + dbDumps []string // declared AND written under db-dumps/ + volumeDumps []string // declared AND written under volume-dumps/ + configFiles []string // declared AND written under compose/ + declareOnly bool // declare everything above but write NONE of it (the R-358-adjacent shape) + rawManifest string // if set, written verbatim instead of a marshalled manifest + noManifest bool +} + +func buildUnit(t *testing.T, f unitFixture) string { + t.Helper() + dir := t.TempDir() + for _, sub := range []string{"compose", "db-dumps", "volume-dumps"} { + if err := os.MkdirAll(filepath.Join(dir, sub), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", sub, err) + } + } + if f.compose != "" { + if err := os.WriteFile(filepath.Join(dir, "compose", "docker-compose.yml"), []byte(f.compose), 0o644); err != nil { + t.Fatalf("write compose: %v", err) + } + } + if !f.declareOnly { + write := func(sub string, names []string) { + for _, n := range names { + p := filepath.Join(dir, sub, n) + // Never clobber the compose written above: `docker-compose.yml` is BOTH a declared + // config file and the expectation source, and overwriting it with placeholder bytes + // made three tests read `compose_unparseable` — the fixture testing the fixture. + if _, err := os.Stat(p); err == nil { + continue + } + if err := os.WriteFile(p, []byte("x"), 0o644); err != nil { + t.Fatalf("write %s/%s: %v", sub, n, err) + } + } + } + write("db-dumps", f.dbDumps) + write("volume-dumps", f.volumeDumps) + write("compose", f.configFiles) + } + switch { + case f.noManifest: + // nothing + case f.rawManifest != "": + if err := os.WriteFile(filepath.Join(dir, "manifest.json"), []byte(f.rawManifest), 0o644); err != nil { + t.Fatalf("write raw manifest: %v", err) + } + default: + m := RecoveryManifest{DBDumps: f.dbDumps, VolumeDumps: f.volumeDumps, ConfigFiles: f.configFiles} + b, err := json.Marshal(m) + if err != nil { + t.Fatalf("marshal manifest: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "manifest.json"), b, 0o644); err != nil { + t.Fatalf("write manifest: %v", err) + } + } + return dir +} + +// composeWithDB is the shape every app with a database has: a service whose image names an engine +// `dbTypeForImage` recognises, plus two named volumes. +const composeWithDB = `services: + app: + image: kimai/kimai2:apache-2.57.0 + db: + image: mariadb:11.6 +volumes: + kimai_db_data: + kimai_var: +` + +// composeNoDBWithVolume — the opengist/privatebin/calibre-web shape measured on demo-hp: no database +// service, one named volume. +const composeNoDBWithVolume = `services: + app: + image: ghcr.io/thomiceli/opengist:1.10 +volumes: + opengist_data: +` + +// composeNothing — an app that legitimately has neither a database nor a named volume. +const composeNothing = `services: + app: + image: ghcr.io/example/static:1.0 +` + +func TestR87_UnitWithDumpsAndComposeDBPasses(t *testing.T) { + dir := buildUnit(t, unitFixture{ + compose: composeWithDB, + dbDumps: []string{"kimai-mariadb.sql"}, + volumeDumps: []string{"kimai_kimai_db_data.tar", "kimai_kimai_var.tar"}, + configFiles: []string{"docker-compose.yml"}, + }) + got := JudgeRestoredUnit(dir) + if !got.OK() { + t.Fatalf("a healthy unit must PASS; got verdict=%q reason=%q missing=%v", got.Verdict, got.Reason, got.Missing) + } +} + +// TestR87_HollowUnitForAnAppWithADatabaseFAILS is Scenario B and the whole point of the job. +// +// The unit is INTACT — its manifest parses, and everything it declares is present, because it +// declares nothing. That is exactly the R-403 shape measured on demo-hp on 2026-08-31. +// +// RED-PROOF (run 2026-08-31, recorded in REPORT.md): replacing the rule with "every declared file is +// present" — i.e. returning UnitProofPass as soon as the `missing` list is empty — makes this test +// fail with `verdict "pass", want "fail"`. That is the trap §5.1 names, and this test is what stands +// between the job and it. +func TestR87_HollowUnitForAnAppWithADatabaseFAILS(t *testing.T) { + dir := buildUnit(t, unitFixture{compose: composeWithDB}) // declares nothing at all + + // The unit really is internally consistent — prove that, so the failure below cannot be + // mistaken for a corrupt fixture. + if man := readManifest(UnitManifestFile(dir)); man == nil { + t.Fatal("fixture is wrong: the manifest must parse, or this is not the R-403 shape") + } + + got := JudgeRestoredUnit(dir) + if got.Verdict != UnitProofFail { + t.Fatalf("a hollow unit for an app WITH a database must FAIL; got verdict=%q reason=%q", got.Verdict, got.Reason) + } + if got.Reason != ProofReasonNoDatabaseDump { + t.Fatalf("reason must name the database expectation; got %q", got.Reason) + } + if len(got.Missing) == 0 || got.Missing[0] != "db" { + t.Fatalf("Missing must name the compose SERVICE that proves the expectation; got %v", got.Missing) + } +} + +// TestR87_HollowUnitWithVolumesOnlyFAILS — the second half of the expectation, on an app that has no +// database but does have named volumes. Without this, the rule would only ever fire on database apps +// and would pass opengist, privatebin and calibre-web hollow. +func TestR87_HollowUnitWithVolumesOnlyFAILS(t *testing.T) { + dir := buildUnit(t, unitFixture{compose: composeNoDBWithVolume}) + got := JudgeRestoredUnit(dir) + if got.Verdict != UnitProofFail || got.Reason != ProofReasonNoVolumeDump { + t.Fatalf("a hollow unit for an app with named volumes must FAIL on the volume half; got verdict=%q reason=%q", got.Verdict, got.Reason) + } + if len(got.Missing) != 1 || got.Missing[0] != "opengist_data" { + t.Fatalf("Missing must name the compose volume; got %v", got.Missing) + } +} + +// TestR87_DatabaseAppWithVolumesButNoDumpFails — the case a coarse "does it carry any data" predicate +// would pass: the unit is NOT hollow (it has tars) and the database is still missing. +// +// This is why `unitCarriesData` alone is not the acceptance rule. +func TestR87_DatabaseAppWithVolumesButNoDumpFails(t *testing.T) { + dir := buildUnit(t, unitFixture{ + compose: composeWithDB, + volumeDumps: []string{"kimai_kimai_var.tar"}, + }) + if unitIsHollow(dir) { + t.Fatal("fixture is wrong: this unit DOES carry data, which is the point of the test") + } + got := JudgeRestoredUnit(dir) + if got.Verdict != UnitProofFail || got.Reason != ProofReasonNoDatabaseDump { + t.Fatalf("a unit with tars but no dump, for an app WITH a database, must FAIL; got verdict=%q reason=%q", got.Verdict, got.Reason) + } +} + +// TestR87_AppWithNoDatabaseAndNoVolumesPasses is Scenario C. +// +// RED-PROOF (run 2026-08-31): alarming on any empty unit — i.e. returning UnitProofFail whenever +// `unitIsHollow(dir)` — makes this test fail with `verdict "fail", want "pass"`. A warning that fires +// on healthy things is as bad as a comforting lie, which is the mistake the R-403 work caught in +// itself. +func TestR87_AppWithNoDatabaseAndNoVolumesPasses(t *testing.T) { + dir := buildUnit(t, unitFixture{compose: composeNothing, configFiles: []string{"docker-compose.yml"}}) + if !unitIsHollow(dir) { + t.Fatal("fixture is wrong: this unit must be HOLLOW by the coarse predicate, or the test proves nothing") + } + got := JudgeRestoredUnit(dir) + if !got.OK() { + t.Fatalf("an app that legitimately has nothing must PASS; got verdict=%q reason=%q", got.Verdict, got.Reason) + } +} + +func TestR87_MissingComposeIsCannotJudgeNotAPass(t *testing.T) { + dir := buildUnit(t, unitFixture{ /* no compose written */ }) + got := JudgeRestoredUnit(dir) + if got.Verdict != UnitProofCannotJudge || got.Reason != ProofReasonComposeMissing { + t.Fatalf("a unit with no compose must be CANNOT JUDGE; got verdict=%q reason=%q", got.Verdict, got.Reason) + } + if got.OK() { + t.Fatal("cannot-judge must never read as a pass") + } +} + +func TestR87_UnparseableComposeIsCannotJudge(t *testing.T) { + dir := buildUnit(t, unitFixture{compose: "services: [this is not: valid: yaml\n - {"}) + got := JudgeRestoredUnit(dir) + if got.Verdict != UnitProofCannotJudge || got.Reason != ProofReasonComposeUnparseable { + t.Fatalf("an unparseable compose must be CANNOT JUDGE, never 'no database'; got verdict=%q reason=%q", got.Verdict, got.Reason) + } +} + +// TestR87_UnparseableManifestFails — fail closed. A unit whose manifest cannot be read cannot be +// vouched for, and the direction matters: treating it as sound would let an unreadable backup pass as +// a proved one. +func TestR87_UnparseableManifestFails(t *testing.T) { + for name, fx := range map[string]unitFixture{ + "absent": {compose: composeWithDB, noManifest: true}, + "not json": {compose: composeWithDB, rawManifest: "{this is not json"}, + "truncated": {compose: composeWithDB, rawManifest: `{"db_dumps": [`}, + "not object": {compose: composeWithDB, rawManifest: `["a","b"]`}, + } { + t.Run(name, func(t *testing.T) { + got := JudgeRestoredUnit(buildUnit(t, fx)) + if got.Verdict != UnitProofFail || got.Reason != ProofReasonManifestUnreadable { + t.Fatalf("an unreadable manifest must FAIL CLOSED; got verdict=%q reason=%q", got.Verdict, got.Reason) + } + }) + } +} + +// TestR87_DeclaredFileAbsentFromScratchFails — part 1 of the rule still holds. A manifest that +// declares a dump the restore did not produce is a failed proof, not a pass. +func TestR87_DeclaredFileAbsentFromScratchFails(t *testing.T) { + dir := buildUnit(t, unitFixture{ + compose: composeWithDB, + dbDumps: []string{"kimai-mariadb.sql"}, + volumeDumps: []string{"kimai_kimai_db_data.tar"}, + declareOnly: true, // declared, never written + }) + got := JudgeRestoredUnit(dir) + if got.Verdict != UnitProofFail || got.Reason != ProofReasonDeclaredFileMissing { + t.Fatalf("a declared-but-absent file must FAIL; got verdict=%q reason=%q", got.Verdict, got.Reason) + } + joined := strings.Join(got.Missing, " ") + for _, want := range []string{"db-dumps/kimai-mariadb.sql", "volume-dumps/kimai_kimai_db_data.tar"} { + if !strings.Contains(joined, want) { + t.Fatalf("Missing must name every absent file; %q not in %v", want, got.Missing) + } + } +} + +// TestR87_SizeIsNeverConsulted — the R-403 rule, carried forward. A one-byte dump for a tiny app is +// healthy; a fat compose tree with no dumps is the dangerous shape. Size answers "how big", and the +// question here has never been that. +func TestR87_SizeIsNeverConsulted(t *testing.T) { + tiny := buildUnit(t, unitFixture{ + compose: composeWithDB, + dbDumps: []string{"kimai-mariadb.sql"}, volumeDumps: []string{"a.tar", "b.tar"}, + }) + // Make every declared file zero bytes — the smallest a unit can possibly be while still holding + // everything it should. + for _, p := range []string{"db-dumps/kimai-mariadb.sql", "volume-dumps/a.tar", "volume-dumps/b.tar"} { + if err := os.WriteFile(filepath.Join(tiny, p), nil, 0o644); err != nil { + t.Fatalf("truncate %s: %v", p, err) + } + } + if got := JudgeRestoredUnit(tiny); !got.OK() { + t.Fatalf("a zero-byte-but-complete unit must PASS — size is not the question; got %q/%q", got.Verdict, got.Reason) + } + + // And the inverse: a LARGE unit that declares nothing must still fail. + fat := buildUnit(t, unitFixture{compose: composeWithDB, configFiles: []string{"docker-compose.yml"}}) + if err := os.WriteFile(filepath.Join(fat, "compose", "big.bin"), make([]byte, 1<<20), 0o644); err != nil { + t.Fatalf("write big file: %v", err) + } + if got := JudgeRestoredUnit(fat); got.Verdict != UnitProofFail { + t.Fatalf("a 1 MB unit that declares no data must still FAIL; got %q/%q", got.Verdict, got.Reason) + } +} + +// TestR87_ManifestCannotNameAFileOutsideTheUnit — the manifest travels inside the snapshot and a +// restore writes it from the store, so it is not a trusted input. A traversal must read as absent +// (and therefore fail), never as present because something happens to exist up the tree. +func TestR87_ManifestCannotNameAFileOutsideTheUnit(t *testing.T) { + dir := buildUnit(t, unitFixture{compose: composeWithDB, rawManifest: `{"db_dumps":["../../../etc/hostname"]}`}) + got := JudgeRestoredUnit(dir) + if got.Verdict != UnitProofFail || got.Reason != ProofReasonDeclaredFileMissing { + t.Fatalf("a traversing manifest entry must read as ABSENT and fail; got verdict=%q reason=%q", got.Verdict, got.Reason) + } +} diff --git a/controller/internal/backup/r87_proof_job_test.go b/controller/internal/backup/r87_proof_job_test.go new file mode 100644 index 0000000..463007f --- /dev/null +++ b/controller/internal/backup/r87_proof_job_test.go @@ -0,0 +1,506 @@ +package backup + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-87 Group B — the JOB. +// +// Everything here drives the real `ProveOffboxUnit` through the two seams REUSE.md names: +// `SetOffboxRunner` (the restic exec seam — deliberately NOT a `resticStepFn`, so the +// `unlock --remove-all` escalation stays visible to the argv assertions) and +// `SetOffboxLatestSnapshotFn`. No restic, no ssh, no docker. + +// proofHarness is a Manager wired for the proof job, with a recorder for every restic argv. +type proofHarness struct { + m *Manager + sett *settings.Settings + prov *offbox3aProvider + // drive is the registered storage path everything is resolved under. + drive string + + mu sync.Mutex + argv [][]string + // unitFor decides what the fake restore materialises for a stack; nil = a healthy unit. + unitFor map[string]unitFixture + // snapFor overrides the snapshot id per stack; "" entries mean "no snapshot". + snapFor map[string]string + // failRestore makes the fake restic return an error. + failRestore bool +} + +// unitPathFor is the absolute path a snapshot records for a stack's recovery unit — the shape +// `offboxUnitPathOf` matches on. +func unitPathFor(stack string) string { + return "/mnt/sys_drive/felhom-data/backups/primary/" + stack +} + +func newProofHarness(t *testing.T, stacks ...string) *proofHarness { + t.Helper() + m, sett := newOffboxManager(t) + drive := t.TempDir() + if err := sett.AddStoragePath(settings.StoragePath{Path: drive, Label: "data", Schedulable: true}); err != nil { + t.Fatal(err) + } + deployed := map[string]bool{} + for _, s := range stacks { + deployed[s] = true + } + prov := &offbox3aProvider{ + hdd: map[string]string{}, binds: map[string][]ClassifiedBind{}, + has: map[string]bool{}, deployed: deployed, + } + m.SetStackProvider(prov) + + h := &proofHarness{m: m, sett: sett, prov: prov, drive: drive, + unitFor: map[string]unitFixture{}, snapFor: map[string]string{}} + + m.SetOffboxLatestSnapshotFn(func(_ context.Context, stack string) (string, []string, error) { + id, ok := h.snapFor[stack] + if !ok { + id = "snap-" + stack + } + if id == "" { + return "", nil, os.ErrNotExist + } + return id, []string{unitPathFor(stack)}, nil + }) + + // The fake restic: records the argv, and for a `restore` MATERIALISES the unit the test asked + // for, at the path inside --target that a real restic would write it to. + m.SetOffboxRunner(func(_ context.Context, _ []string, args ...string) ([]byte, error) { + h.mu.Lock() + h.argv = append(h.argv, append([]string{}, args...)) + h.mu.Unlock() + if len(args) == 0 || !containsArg(args, "restore") { + return nil, nil + } + if h.failRestore { + return []byte("simulated restic failure"), os.ErrPermission + } + target, include := argValue(args, "--target"), argValue(args, "--include") + if target == "" || include == "" { + return nil, nil + } + stack := filepath.Base(include) + dest := filepath.Join(target, strings.TrimPrefix(include, string(filepath.Separator))) + materialiseUnit(t, dest, h.unitFor[stack]) + return nil, nil + }) + // Plenty of headroom unless a test says otherwise. + m.SetOffboxFreeFn(func(string) int64 { return 100 << 30 }) + return h +} + +// containsArg lives in r358_scratch_marker_test.go — the same question, one implementation. + +func argValue(args []string, flag string) string { + for i, a := range args { + if a == flag && i+1 < len(args) { + return args[i+1] + } + } + return "" +} + +// materialiseUnit writes a recovery unit at dest, mirroring what a restore would leave behind. +// The zero fixture is a HEALTHY database app. +func materialiseUnit(t *testing.T, dest string, f unitFixture) { + t.Helper() + if f.compose == "" && len(f.dbDumps) == 0 && len(f.volumeDumps) == 0 && !f.noManifest && f.rawManifest == "" { + f = unitFixture{ + compose: composeWithDB, + dbDumps: []string{"app-mariadb.sql"}, + volumeDumps: []string{"a.tar", "b.tar"}, + } + } + for _, sub := range []string{"compose", "db-dumps", "volume-dumps"} { + if err := os.MkdirAll(filepath.Join(dest, sub), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + } + if f.compose != "" { + if err := os.WriteFile(filepath.Join(dest, "compose", "docker-compose.yml"), []byte(f.compose), 0o644); err != nil { + t.Fatalf("compose: %v", err) + } + } + if !f.declareOnly { + for sub, names := range map[string][]string{"db-dumps": f.dbDumps, "volume-dumps": f.volumeDumps} { + for _, n := range names { + if err := os.WriteFile(filepath.Join(dest, sub, n), []byte("x"), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + } + } + } + if f.noManifest { + return + } + b, _ := json.Marshal(RecoveryManifest{DBDumps: f.dbDumps, VolumeDumps: f.volumeDumps}) + if f.rawManifest != "" { + b = []byte(f.rawManifest) + } + if err := os.WriteFile(filepath.Join(dest, "manifest.json"), b, 0o644); err != nil { + t.Fatalf("manifest: %v", err) + } +} + +func (h *proofHarness) allArgs() [][]string { + h.mu.Lock() + defer h.mu.Unlock() + return h.argv +} + +func (h *proofHarness) proofScratch(t *testing.T, stack string) string { + t.Helper() + s, _, err := h.m.offboxProofScratchDir(stack) + if err != nil { + t.Fatalf("proof scratch dir: %v", err) + } + return s +} + +// ── B1 ─────────────────────────────────────────────────────────────────────────────────────────── + +// TestR87_SkipsWhenRunningFlagHeld asserts a NON-EFFECT, the way TestR359_SkipsWhenRunningFlagHeld +// does: restic was never invoked at all, and due-ness did not move. +func TestR87_SkipsWhenRunningFlagHeld(t *testing.T) { + h := newProofHarness(t, "kimai") + if err := h.m.AcquireRunningForTest(); err != nil { + t.Fatalf("could not take the flag: %v", err) + } + res := h.m.ProveOffboxUnit(context.Background()) + if !res.Skipped { + t.Fatalf("must SKIP while the single-writer flag is held; got %+v", res) + } + if got := len(h.allArgs()); got != 0 { + t.Fatalf("a skip must invoke restic ZERO times; got %d invocations: %v", got, h.allArgs()) + } + if res.Verdict() != "" { + t.Fatalf("a skip reaches no verdict; got %q", res.Verdict()) + } + h.m.RecordProofVerdict(res) + if tt := h.sett.GetOffboxTarget(); len(tt.ProvedSnapshots) != 0 || tt.LastProofResult != "" { + t.Fatalf("a skip must NOT advance due-ness; got proved=%v result=%q", tt.ProvedSnapshots, tt.LastProofResult) + } +} + +// ── B2 ─────────────────────────────────────────────────────────────────────────────────────────── + +// TestR87_ScratchIsDeletedOnEveryPath — pass, fail, cannot-judge and a restore error. +// +// RED-PROOF (run 2026-08-31): removing the `defer m.removeProofScratch(...)` makes the pass and fail +// rows fail with the scratch directory still present. +func TestR87_ScratchIsDeletedOnEveryPath(t *testing.T) { + cases := map[string]struct { + fixture unitFixture + failRestore bool + }{ + "pass": {fixture: unitFixture{}}, + "fail empty": {fixture: unitFixture{compose: composeWithDB}}, + "cannot judge": {fixture: unitFixture{noManifest: false, compose: "", dbDumps: []string{"d.sql"}}}, + "restore error": {failRestore: true}, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + h := newProofHarness(t, "kimai") + h.unitFor["kimai"] = c.fixture + h.failRestore = c.failRestore + scratch := h.proofScratch(t, "kimai") + + h.m.ProveOffboxUnit(context.Background()) + + if _, err := os.Stat(scratch); !os.IsNotExist(err) { + t.Fatalf("the proof copy must be gone on EVERY path; %s still exists (stat err=%v)", scratch, err) + } + }) + } +} + +// TestR87_ProofScratchIsNotTheCustomerVerificationCopy — the delete above must never be able to reach +// a copy the CUSTOMER made. Different roots is how that is guaranteed rather than hoped. +func TestR87_ProofScratchIsNotTheCustomerVerificationCopy(t *testing.T) { + h := newProofHarness(t, "kimai") + proof := h.proofScratch(t, "kimai") + customer, _, err := h.m.offboxRestoreScratchDir("kimai") + if err != nil { + t.Fatal(err) + } + if proof == customer { + t.Fatal("the proof copy and the customer's verification copy MUST NOT share a path — the nightly delete would destroy the customer's copy") + } + if !strings.Contains(proof, "offsite-proof") || !strings.Contains(customer, "offsite-restore") { + t.Fatalf("roots are not the expected pair: proof=%q customer=%q", proof, customer) + } + + // And the customer's copy survives a full proof run that deletes its own. + if err := os.MkdirAll(customer, 0o755); err != nil { + t.Fatal(err) + } + sentinel := filepath.Join(customer, "customer-copy-marker") + if err := os.WriteFile(sentinel, []byte("keep me"), 0o644); err != nil { + t.Fatal(err) + } + h.m.ProveOffboxUnit(context.Background()) + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("the nightly proof deleted the CUSTOMER's verification copy: %v", err) + } +} + +// ── B3 ─────────────────────────────────────────────────────────────────────────────────────────── + +// TestR87_NeverWritesToTheRepository is Scenario E, asserted as a NON-EFFECT on the argv rather than +// as the absence of an error. +// +// RED-PROOF (run 2026-08-31): routing the restore through `resticStep` with the `unlockStale` +// pre-flight — i.e. what `RestoreOffboxScratch` does — makes this fail on `unlock` appearing in the +// argv and on `--no-lock` being absent. +func TestR87_NeverWritesToTheRepository(t *testing.T) { + h := newProofHarness(t, "kimai") + h.m.ProveOffboxUnit(context.Background()) + + all := h.allArgs() + if len(all) == 0 { + t.Fatal("the proof must have invoked restic — a zero-invocation run proves nothing about the argv") + } + // Every write verb restic has that this codebase ever issues. + for _, args := range all { + for _, verb := range []string{"unlock", "forget", "prune", "backup", "init", "--remove-all"} { + if containsArg(args, verb) { + t.Fatalf("the proof issued a WRITE verb %q against the repository: %v", verb, args) + } + } + } + var sawRestore bool + for _, args := range all { + if containsArg(args, "restore") { + sawRestore = true + if !containsArg(args, "--no-lock") { + t.Fatalf("the proof restore must carry --no-lock so no lock file can be created: %v", args) + } + } + } + if !sawRestore { + t.Fatal("no restore was issued — the --no-lock assertion above never ran") + } +} + +// ── B4, B5, B6 — due-ness ──────────────────────────────────────────────────────────────────────── + +func TestR87_OneAppPerRun(t *testing.T) { + h := newProofHarness(t, "bookstack", "docmost", "kimai") + res := h.m.ProveOffboxUnit(context.Background()) + if res.Stack == "" { + t.Fatal("a run must pick an app") + } + var restores int + for _, args := range h.allArgs() { + if containsArg(args, "restore") { + restores++ + } + } + if restores != 1 { + t.Fatalf("one app per run: want exactly 1 restore, got %d", restores) + } +} + +// TestR87_ProvedSnapshotIsRecordedNotATimestamp. +// +// RED-PROOF (run 2026-08-31): recording `time.Now()` in `ProvedSnapshots[stack]` instead of the +// snapshot ID makes this fail on the stored value, and makes B6 fail too because the app never +// becomes due again. +func TestR87_ProvedSnapshotIsRecordedNotATimestamp(t *testing.T) { + h := newProofHarness(t, "kimai") + h.snapFor["kimai"] = "a07c36a1" + res := h.m.ProveOffboxUnit(context.Background()) + if res.Verdict() != string(UnitProofPass) { + t.Fatalf("fixture should pass; got %q (%q)", res.Verdict(), res.Judgement.Reason) + } + h.m.RecordProofVerdict(res) + + tt := h.sett.GetOffboxTarget() + if got := tt.ProvedSnapshots["kimai"]; got != "a07c36a1" { + t.Fatalf("the SNAPSHOT ID must be recorded; got %q", got) + } + if tt.LastProofSnapshot != "a07c36a1" || tt.LastProofStack != "kimai" || tt.LastProofResult != "pass" { + t.Fatalf("the verdict record is wrong: %+v", tt) + } + // The record must not be a time: a timestamp would parse as one and an ID must not. + if strings.Contains(tt.ProvedSnapshots["kimai"], ":") || strings.Contains(tt.ProvedSnapshots["kimai"], "T") { + t.Fatalf("ProvedSnapshots looks like a timestamp, not a snapshot ID: %q", tt.ProvedSnapshots["kimai"]) + } +} + +func TestR87_AlreadyProvedSnapshotIsNotReProved(t *testing.T) { + h := newProofHarness(t, "kimai") + h.snapFor["kimai"] = "same-id" + h.m.RecordProofVerdict(ProofResult{Stack: "kimai", SnapshotID: "same-id", + Judgement: UnitProofResult{Verdict: UnitProofPass}}) + + res := h.m.ProveOffboxUnit(context.Background()) + if !res.NoSnapshot { + t.Fatalf("an already-proved newest snapshot must leave nothing due; got stack=%q", res.Stack) + } + for _, args := range h.allArgs() { + if containsArg(args, "restore") { + t.Fatalf("nothing due must download nothing; got %v", args) + } + } +} + +// TestR87_NewSnapshotMakesAProvedAppDueAgain — the half a timestamp cannot do. +func TestR87_NewSnapshotMakesAProvedAppDueAgain(t *testing.T) { + h := newProofHarness(t, "kimai") + h.snapFor["kimai"] = "old-id" + h.m.RecordProofVerdict(ProofResult{Stack: "kimai", SnapshotID: "old-id", + Judgement: UnitProofResult{Verdict: UnitProofPass}}) + + h.snapFor["kimai"] = "new-id" // the nightly backup landed + res := h.m.ProveOffboxUnit(context.Background()) + if res.Stack != "kimai" || res.SnapshotID != "new-id" { + t.Fatalf("a NEW snapshot must make the app due again; got stack=%q snapshot=%q noSnapshot=%v", res.Stack, res.SnapshotID, res.NoSnapshot) + } +} + +// TestR87_RotationCoversEveryAppInAsManyNights — Scenario D end to end. +func TestR87_RotationCoversEveryAppInAsManyNights(t *testing.T) { + stacks := []string{"bookstack", "docmost", "kimai", "opengist"} + h := newProofHarness(t, stacks...) + seen := map[string]bool{} + for i := 0; i < len(stacks); i++ { + res := h.m.ProveOffboxUnit(context.Background()) + if res.Stack == "" { + t.Fatalf("night %d picked nothing; %d apps covered so far", i+1, len(seen)) + } + if seen[res.Stack] { + t.Fatalf("night %d re-picked %s — the rotation must move on", i+1, res.Stack) + } + seen[res.Stack] = true + h.m.RecordProofVerdict(res) + } + if len(seen) != len(stacks) { + t.Fatalf("four nights must cover four apps; covered %v", seen) + } + // A fifth night has nothing due. + if res := h.m.ProveOffboxUnit(context.Background()); !res.NoSnapshot { + t.Fatalf("a fifth night must find nothing due; got %q", res.Stack) + } +} + +// ── B7, B8 ─────────────────────────────────────────────────────────────────────────────────────── + +// TestR87_HeadroomRefusalHappensBeforeTheDownload — a gate after the download is not a gate. +func TestR87_HeadroomRefusalHappensBeforeTheDownload(t *testing.T) { + h := newProofHarness(t, "kimai") + h.m.SetOffboxFreeFn(func(string) int64 { return 1 << 20 }) // 1 MiB, well under the floor + + res := h.m.ProveOffboxUnit(context.Background()) + if res.Err == nil { + t.Fatal("an unwritable-headroom box must refuse") + } + for _, args := range h.allArgs() { + if containsArg(args, "restore") { + t.Fatalf("the refusal must happen BEFORE any download; a restore was issued: %v", args) + } + } + // The customer-grade wording is shared with the restore path (REUSE.md's offsiteNoSpaceMsgFmt + // rule). ASCII-only fragment, because an accented grep has returned 0 for strings that were there. + if !strings.Contains(res.Err.Error(), "szabad hely") { + t.Fatalf("the refusal must use the shared headroom wording; got %q", res.Err.Error()) + } + if res.Verdict() != "" { + t.Fatal("a refusal reaches no verdict about the backup") + } +} + +func TestR87_NoSnapshotYetIsNotAFailure(t *testing.T) { + h := newProofHarness(t, "newapp") + h.snapFor["newapp"] = "" // never backed up + res := h.m.ProveOffboxUnit(context.Background()) + if !res.NoSnapshot { + t.Fatalf("an app with no off-site snapshot is not a failure of this test; got %+v", res) + } + if res.Err != nil || res.Verdict() != "" { + t.Fatalf("it must not be an error and must reach no verdict; err=%v verdict=%q", res.Err, res.Verdict()) + } +} + +// TestR87_RestoreFailureIsNotAVerdictAboutTheBackup — a download that did not finish has seen +// nothing, so it must never become the "intact but empty" alarm. +func TestR87_RestoreFailureIsNotAVerdictAboutTheBackup(t *testing.T) { + h := newProofHarness(t, "kimai") + h.failRestore = true + res := h.m.ProveOffboxUnit(context.Background()) + if res.Err == nil { + t.Fatal("a failed restore must surface as an error") + } + if res.Verdict() != "" { + t.Fatalf("a failed restore must reach NO verdict; got %q", res.Verdict()) + } + h.m.RecordProofVerdict(res) + if tt := h.sett.GetOffboxTarget(); tt.LastProofResult != "" || len(tt.ProvedSnapshots) != 0 { + t.Fatalf("a failed restore must not advance due-ness; got %+v", tt) + } +} + +// TestR87_HollowUnitReachesAFailVerdictThroughTheWholeJob — Scenario B through the job, not just the +// predicate. The judgement is right in isolation and could still be wired to nothing. +func TestR87_HollowUnitReachesAFailVerdictThroughTheWholeJob(t *testing.T) { + h := newProofHarness(t, "kimai") + h.unitFor["kimai"] = unitFixture{compose: composeWithDB} // the R-403 shape + res := h.m.ProveOffboxUnit(context.Background()) + if res.Verdict() != string(UnitProofFail) || res.Judgement.Reason != ProofReasonNoDatabaseDump { + t.Fatalf("the job must reach a FAIL verdict on a hollow unit; got %q/%q", res.Verdict(), res.Judgement.Reason) + } + h.m.RecordProofVerdict(res) + if tt := h.sett.GetOffboxTarget(); tt.LastProofResult != "fail" || tt.LastProofReason != string(ProofReasonNoDatabaseDump) { + t.Fatalf("the failing verdict must be persisted with its reason; got result=%q reason=%q", tt.LastProofResult, tt.LastProofReason) + } +} + +// TestR87_CannotJudgeIsRecordedAndNotAPass. +func TestR87_CannotJudgeIsRecordedAndNotAPass(t *testing.T) { + h := newProofHarness(t, "kimai") + h.unitFor["kimai"] = unitFixture{dbDumps: []string{"d.sql"}} // no compose materialised + res := h.m.ProveOffboxUnit(context.Background()) + if res.Verdict() != string(UnitProofCannotJudge) { + t.Fatalf("a unit with no compose must be CANNOT JUDGE; got %q/%q", res.Verdict(), res.Judgement.Reason) + } + h.m.RecordProofVerdict(res) + if tt := h.sett.GetOffboxTarget(); tt.LastProofResult != "cannot_judge" { + t.Fatalf("cannot-judge must be recorded as itself; got %q", tt.LastProofResult) + } +} + +// TestR87_VerdictIsPublishedOnTheReportStatus — Part 2.4, including the absence rule. +func TestR87_VerdictIsPublishedOnTheReportStatus(t *testing.T) { + h := newProofHarness(t, "kimai") + + // Before any proof, the field must be ABSENT — not "fail", not "pass". + if st := h.m.OffboxReportStatus(); st == nil || st.LastProofResult != "" { + t.Fatalf("a box that has never proved must report NOT RECORDED; got %+v", st) + } + res := h.m.ProveOffboxUnit(context.Background()) + h.m.RecordProofVerdict(res) + + st := h.m.OffboxReportStatus() + if st.LastProofResult != "pass" || st.LastProofStack != "kimai" || st.LastProofSnapshot == "" || st.LastProofRun == "" { + t.Fatalf("the verdict must reach the report status; got %+v", st) + } + // omitempty must keep the absent case off the wire entirely, or a reader cannot tell it apart. + b, err := json.Marshal(&OffboxReportStatus{}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), "last_proof_result") { + t.Fatalf("an empty status must not emit last_proof_result at all; got %s", b) + } +} diff --git a/controller/internal/backup/r87_wiring_test.go b/controller/internal/backup/r87_wiring_test.go new file mode 100644 index 0000000..85448a5 --- /dev/null +++ b/controller/internal/backup/r87_wiring_test.go @@ -0,0 +1,168 @@ +package backup + +import ( + "go/ast" + "go/parser" + "go/token" + "strings" + "testing" +) + +// R-87 Group D — the WIRING. +// +// THE FAILURE SHAPE THIS EXISTS FOR is the project's most-repeated one: **built and never wired.** +// R-397 was the sixth instance — `NotifyIntegrityOK`/`NotifyIntegrityFailed` existed, the hub +// allowlisted both, the Hungarian text existed, the settings checkbox existed, the debug button +// existed, and the CALLER did not. The product advertised a weekly check that never ran. +// +// `ProveOffboxUnit` is exactly that shape again: a method nobody has to call. It compiles unwired, +// every test in this package passes unwired, and the nightly proof would simply never happen. +// +// It walks the AST rather than grepping, and parses with comments DROPPED, so a commented-out +// registration cannot satisfy it — the string is still in the file either way. + +const r87MainPath = "../../cmd/controller/main.go" + +func parseMainForR87(t *testing.T) *ast.File { + t.Helper() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, r87MainPath, nil, 0) // comments dropped on purpose + if err != nil { + t.Fatalf("parse %s: %v — the R-87 wiring is now unasserted", r87MainPath, err) + } + return f +} + +// TestR87_JobIsRegisteredInMain — D1. The scheduler entry must exist, name the job, and call the +// runner. All three, because any one of them alone is satisfiable by an inert line. +func TestR87_JobIsRegisteredInMain(t *testing.T) { + f := parseMainForR87(t) + + var sawDailyCall, sawJobName, sawRunnerCall bool + var scheduledAt string + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Daily" || len(call.Args) < 3 { + return true + } + name, ok := call.Args[0].(*ast.BasicLit) + if !ok || strings.Trim(name.Value, `"`) != "offsite-proof" { + return true + } + sawDailyCall, sawJobName = true, true + if at, ok := call.Args[1].(*ast.BasicLit); ok { + scheduledAt = strings.Trim(at.Value, `"`) + } + // The closure must actually call the runner. A `Daily("offsite-proof", ...)` whose body does + // nothing is precisely the R-397 shape one level in. + ast.Inspect(call.Args[2], func(inner ast.Node) bool { + ic, ok := inner.(*ast.CallExpr) + if !ok { + return true + } + if id, ok := ic.Fun.(*ast.Ident); ok && id.Name == "runOffsiteProof" { + sawRunnerCall = true + } + return true + }) + return true + }) + + if !sawDailyCall || !sawJobName { + t.Fatal(`main.go must register sched.Daily("offsite-proof", ...) — without it the whole of R-87 is inert`) + } + if !sawRunnerCall { + t.Fatal("the offsite-proof job is registered but its closure never calls runOffsiteProof — registered and inert is the R-397 shape") + } + if scheduledAt == "" || !strings.Contains(scheduledAt, ":") { + t.Fatalf("the job must be scheduled at an HH:MM time; got %q", scheduledAt) + } + // It must not collide with the sibling off-site jobs' own slots. Those are read from the live box + // and recorded beside the registration; this pins that they stay distinct. + for _, taken := range []string{"04:15", "05:10", "06:00"} { + if scheduledAt == taken { + t.Fatalf("offsite-proof is scheduled at %s, which is another off-site job's slot — they share the single-writer flag and one would skip every night", scheduledAt) + } + } +} + +// TestR87_RunnerAlarmsOnlyOnFail — D1's other half, and Scenario C's job-level guarantee. +// +// `runOffsiteProof` must call the notifier ONLY inside a branch testing for the failing verdict. +// Asserted structurally because the alternative — running the job and watching for silence — proves +// nothing when the job is inert for an unrelated reason. +func TestR87_RunnerAlarmsOnlyOnFail(t *testing.T) { + f := parseMainForR87(t) + + var fn *ast.FuncDecl + ast.Inspect(f, func(n ast.Node) bool { + d, ok := n.(*ast.FuncDecl) + if ok && d.Name.Name == "runOffsiteProof" { + fn = d + } + return true + }) + if fn == nil { + t.Fatal("runOffsiteProof is missing from main.go — the job has no caller") + } + + // Every NotifyOffsiteProofEmpty call must sit inside an if-statement whose condition names the + // FAIL verdict. A bare call at function level would alarm on every outcome, including a pass. + var calls, guarded int + var guardText []string + ast.Inspect(fn, func(n ast.Node) bool { + ifs, ok := n.(*ast.IfStmt) + if !ok { + return true + } + cond := exprText(ifs.Cond) + ast.Inspect(ifs.Body, func(inner ast.Node) bool { + if sel, ok := inner.(*ast.SelectorExpr); ok && sel.Sel.Name == "NotifyOffsiteProofEmpty" { + guarded++ + guardText = append(guardText, cond) + } + return true + }) + return true + }) + ast.Inspect(fn, func(n ast.Node) bool { + if sel, ok := n.(*ast.SelectorExpr); ok && sel.Sel.Name == "NotifyOffsiteProofEmpty" { + calls++ + } + return true + }) + + if calls == 0 { + t.Fatal("runOffsiteProof never calls NotifyOffsiteProofEmpty — a failing proof would be silent, which is R-397's shape exactly") + } + if calls != 1 { + t.Fatalf("exactly ONE alarm call, or a failing proof mails twice; found %d", calls) + } + if guarded != 1 { + t.Fatalf("the alarm must be guarded by an if — an unguarded call alarms on a PASS too; guarded=%d", guarded) + } + if !strings.Contains(guardText[0], "UnitProofFail") { + t.Fatalf("the alarm's guard must test for the FAIL verdict, not merely for 'not a pass' — cannot-judge must never alarm; guard is %q", guardText[0]) + } +} + +// exprText renders an expression back to source-ish text for an assertion message. +func exprText(e ast.Expr) string { + switch v := e.(type) { + case *ast.BinaryExpr: + return exprText(v.X) + " " + v.Op.String() + " " + exprText(v.Y) + case *ast.SelectorExpr: + return exprText(v.X) + "." + v.Sel.Name + case *ast.Ident: + return v.Name + case *ast.CallExpr: + return exprText(v.Fun) + "(...)" + case *ast.BasicLit: + return v.Value + } + return "?" +} diff --git a/controller/internal/notify/notifier.go b/controller/internal/notify/notifier.go index f83f81a..e5843c2 100644 --- a/controller/internal/notify/notifier.go +++ b/controller/internal/notify/notifier.go @@ -397,6 +397,23 @@ func (n *Notifier) NotifyIntegrityFailed(message, errMsg string) { n.PushEvent("backup_integrity_failed", "error", message, &BackupDetails{Error: errMsg}) } +// NotifyOffsiteProofEmpty (R-87) reports that the nightly off-site proof found a backup that is +// READABLE and holds none of the app's data. +// +// SEVERITY `error`, from the hub's exact vocabulary {info, warning, error, critical}. Anything else +// is silently coerced to `info` and mailed to nobody — that shipped twice (R-328 on +// `disk_health_degraded`, R-329 on `app_start_failed`, 91 events stored and zero delivered), and +// `r329_severity_contract_test.go` walks this file to keep it from shipping a third time. +// +// A SEPARATE TYPE FROM `backup_integrity_failed`, and that is the point rather than an oversight: the +// integrity check says THE STORE IS DAMAGED; this says the store is sound and the CONTENT is absent. +// The customer's action differs and so must the sentence. +// +// The message is passed through, not templated hub-side, so it can name the app and what is missing. +func (n *Notifier) NotifyOffsiteProofEmpty(message, detail string) { + n.PushEvent("offsite_proof_empty", "error", message, &BackupDetails{Error: detail}) +} + // NotifyIntegrityOK sends a backup integrity check success event. func (n *Notifier) NotifyIntegrityOK(message string) { n.PushEvent("backup_integrity_ok", "info", message, nil) @@ -577,11 +594,11 @@ type DiskHealthDetails struct { type DiskAlertKind int const ( - DiskAlertWarn DiskAlertKind = iota // Figyelmeztetés — worth keeping an eye on - DiskAlertFailSelfReported // Hiba — the drive's own SMART verdict says FAILING - DiskAlertFailSectors // Hiba — reached from unreadable-sector counters - DiskAlertFailTemperature // Hiba — reached from heat - DiskAlertFailWorsened // Hiba — already reported, and still getting worse + DiskAlertWarn DiskAlertKind = iota // Figyelmeztetés — worth keeping an eye on + DiskAlertFailSelfReported // Hiba — the drive's own SMART verdict says FAILING + DiskAlertFailSectors // Hiba — reached from unreadable-sector counters + DiskAlertFailTemperature // Hiba — reached from heat + DiskAlertFailWorsened // Hiba — already reported, and still getting worse ) // DiskAlert is the payload for one disk-health alert. It carries enough for the notifier to pick a diff --git a/controller/internal/notify/r87_proof_event_test.go b/controller/internal/notify/r87_proof_event_test.go new file mode 100644 index 0000000..44be605 --- /dev/null +++ b/controller/internal/notify/r87_proof_event_test.go @@ -0,0 +1,118 @@ +package notify + +import ( + "strings" + "testing" + "time" +) + +// R-87 Group C — the ALARM. +// +// It reuses `notifierAgainstHub` / `awaitEvent` from backup_target_notify_test.go rather than adding +// a second harness: the question is identical (what actually went over the WIRE), and a second +// recorder is how two tests come to disagree about the same encoding. +// +// The severity vocabulary is `{info, warning, error, critical}` and anything else is silently coerced +// to `info` and mailed to NOBODY — that shipped twice (R-328 on `disk_health_degraded`, R-329 on +// `app_start_failed`: 91 events stored, zero delivered). + +// TestR87_FailureEmitsExactlyOneEventWithAValidSeverity — C1. +func TestR87_FailureEmitsExactlyOneEventWithAValidSeverity(t *testing.T) { + n, got := notifierAgainstHub(t) + n.NotifyOffsiteProofEmpty( + "A(z) kimai legutobbi tavoli mentese olvashato, de nem tartalmazza az alkalmazas adatait.", + "snapshot a07c36a1, reason database_expected_none_captured") + + ev := awaitEvent(t, got) + if ev.EventType != "offsite_proof_empty" { + t.Fatalf("event_type = %q, want offsite_proof_empty — an unallowlisted type is 400'd by the hub and VANISHES", ev.EventType) + } + valid := map[string]bool{"info": true, "warning": true, "error": true, "critical": true} + if !valid[ev.Severity] { + t.Fatalf("severity %q is outside the hub's vocabulary — it would be coerced to info and mailed to nobody", ev.Severity) + } + if ev.Severity != "error" { + t.Fatalf("severity = %q, want error: a backup holding none of the customer's data is not a warning", ev.Severity) + } + + // EXACTLY ONE. A second event for the same fact is how an operator learns to skim. + select { + case extra := <-got: + t.Fatalf("a failing proof must emit exactly ONE event; a second arrived: %+v", extra) + case <-time.After(300 * time.Millisecond): + } +} + +// TestR87_MessageSaysIntactButEmptyNotCorrupt — C3. +// +// ASCII-ONLY FRAGMENTS WITH A NEGATIVE CONTROL (R-364): an accented grep has returned 0 for strings +// that were there, so every fragment below is ASCII and one deliberately-absent fragment proves the +// search can fail. +func TestR87_MessageSaysIntactButEmptyNotCorrupt(t *testing.T) { + n, got := notifierAgainstHub(t) + msg := "A(z) kimai legutobbi tavoli mentese olvashato, de nem tartalmazza az alkalmazas adatait. " + + "A tarolo nem serult - a mentes keszult el uresen." + n.NotifyOffsiteProofEmpty(msg, "snapshot a07c36a1") + ev := awaitEvent(t, got) + + // POSITIVE: the store is readable AND the content is absent — both halves, or the customer reads + // this as R-359's damaged store, which has a different cause and a different action. + for _, frag := range []string{"olvashato", "nem tartalmazza", "nem serult"} { + if !strings.Contains(ev.Message, frag) { + t.Fatalf("the message must carry %q; got %q", frag, ev.Message) + } + } + // NEGATIVE CONTROL: prove the search above can fail on this same string. + if strings.Contains(ev.Message, "ZZZ-NOT-IN-THE-MESSAGE") { + t.Fatal("negative control matched — the fragment search is not discriminating, so the positives above prove nothing") + } + // It must NOT claim damage. These are R-359's own words for the other fault. + for _, forbidden := range []string{"hibat talalt", "serult lehet"} { + if strings.Contains(ev.Message, forbidden) { + t.Fatalf("the message must not say the store is damaged; %q appears in %q", forbidden, ev.Message) + } + } +} + +// TestR87_ProofEventCarriesNoPathAndNoRepositoryURL — units carry portable secrets and the repository +// URL is a credential-adjacent string. Neither may ride out on an event. +func TestR87_ProofEventCarriesNoPathAndNoRepositoryURL(t *testing.T) { + n, got := notifierAgainstHub(t) + n.NotifyOffsiteProofEmpty("A(z) kimai mentese ures.", "snapshot a07c36a1, reason database_expected_none_captured, expected: db") + ev := awaitEvent(t, got) + for _, forbidden := range []string{"/mnt/", "sftp:", "RESTIC_PASSWORD", "ssh_key"} { + if strings.Contains(ev.Message, forbidden) { + t.Fatalf("%q must never appear in a customer/operator event; got %q", forbidden, ev.Message) + } + } +} + +// TestR87_PassEmitsNothing — C2, asserted as a NON-EFFECT. The job-level half — that the pass branch +// never reaches the notifier at all — is D1's AST walk over main.go. +func TestR87_PassEmitsNothing(t *testing.T) { + _, got := notifierAgainstHub(t) + select { + case ev := <-got: + t.Fatalf("nothing was called, so nothing may arrive; got %+v", ev) + case <-time.After(300 * time.Millisecond): + } +} + +// TestR87_NoPerAppCooldownEntryWasAdded — C4, asserted from the controller side. +// +// The register lives in the hub (`notify.perAppCooldownEvents`) and adding an entry there is a FENCED +// act (08 §6.2): the backup family's cooldown is coarse ON PURPOSE so that one full disk produces one +// mail rather than twenty. This job proves ONE app per night, so the coarse hourly operator cooldown +// is already the right grain. +// +// The controller's half of that contract is asserted here: the event must NOT carry a `stack_name` +// detail field, because that is the payload shape the hub's per-app keying reads. The app is named in +// the MESSAGE instead. This fails if someone later routes the type through the per-app path. +func TestR87_NoPerAppCooldownEntryWasAdded(t *testing.T) { + n, got := notifierAgainstHub(t) + n.NotifyOffsiteProofEmpty("A(z) kimai mentese ures.", "snapshot a07c36a1") + ev := awaitEvent(t, got) + if strings.Contains(ev.Message, "stack_name") { + t.Fatalf("offsite_proof_empty must not carry stack_name — that is the field the hub per-app cooldown keys on, and this family is coarse by design; got %s", ev.Message) + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index 293dbbb..c33749e 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -135,13 +135,13 @@ type Settings struct { // The full page interrupts while `Epoch > PostponedEpoch`, and the banner reminds while // `Epoch > OptOutEpoch` — so a fresh entry resets BOTH by arithmetic, with nothing to clear and // nothing that can be forgotten to clear (§7.1 condition 2). - RecoveryOfferEpoch int `json:"recovery_offer_epoch,omitempty"` - RecoveryOfferActive bool `json:"recovery_offer_active,omitempty"` + RecoveryOfferEpoch int `json:"recovery_offer_epoch,omitempty"` + RecoveryOfferActive bool `json:"recovery_offer_active,omitempty"` // RecoveryOfferSince (RFC3339) stamps when the CURRENT epoch began — the anchor the undecided // reminders escalate against (§2.3). Re-stamped on every entry, so a box that settles and is later // rebuilt starts its reminder ladder again rather than inheriting an old one. - RecoveryOfferSince string `json:"recovery_offer_since,omitempty"` - RecoveryNoticePostponedEpoch int `json:"recovery_notice_postponed_epoch,omitempty"` + RecoveryOfferSince string `json:"recovery_offer_since,omitempty"` + RecoveryNoticePostponedEpoch int `json:"recovery_notice_postponed_epoch,omitempty"` // RecoveryRemindOptOutEpoch — the customer ticked „ne emlékeztessen újra" in this epoch. // // It silences THE BANNER AND NOTHING ELSE (§7.1 condition 3). It is not an abandonment, it starts @@ -270,7 +270,7 @@ type OffboxTarget struct { QuotaGB int `json:"quota_gb,omitempty"` // Runtime status (written by the off-box runner; never holds a secret). - LastRun string `json:"last_run,omitempty"` // RFC3339 + LastRun string `json:"last_run,omitempty"` // RFC3339 // LastStatus — "ok" | "incomplete" | "error" | "running". R-203 added "incomplete": the run // completed and what it captured is real, but a directory the app declares MANDATORY could not be // captured, so the app is NOT fully protected. Distinct from "error" (the run failed) on purpose; @@ -288,7 +288,7 @@ type OffboxTarget struct { // and has failed every night since must keep Monday's stamp, because that stamp is precisely what // makes the staleness threshold elapse. Clearing it on failure would restore the bug in mirror // image (an instantly-stale tier on the first blip — the F-A1 noise path). - LastSuccess string `json:"last_success,omitempty"` // RFC3339 + LastSuccess string `json:"last_success,omitempty"` // RFC3339 // LastIntegrityCheck / LastIntegrityOK (R-359) record the last time `restic check` actually RAN // against this repository and what it found. They live HERE, beside LastSuccess, because this is // where the off-site tier's state already is — one store, one lifetime, one atomic write. @@ -313,9 +313,25 @@ type OffboxTarget struct { // than v0.228.0 — and never "structure"; absence means the box cannot answer, exactly as StatsKnown // below establishes for the counts. LastIntegrityDepth string `json:"last_integrity_depth,omitempty"` - LastError string `json:"last_error,omitempty"` - LastDuration string `json:"last_duration,omitempty"` - RepoSizeHuman string `json:"repo_size_human,omitempty"` + // ── R-87: the nightly off-site PROOF ──────────────────────────────────────────────────────────── + // + // ProvedSnapshots maps a stack name to the SNAPSHOT ID last proved for it — never a timestamp, and + // the distinction is the whole scheduling model (R-86, 07 §3). A timestamp re-proves the same + // snapshot forever and says nothing about the newest one; a snapshot ID makes an app due again the + // moment a new backup lands, and never before. Not a secret (app names + restic short IDs). + ProvedSnapshots map[string]string `json:"proved_snapshots,omitempty"` + // The newest verdict, for the report card. LastProofResult is "" when NO proof has ever reached a + // verdict on this box — a box older than v0.231.0 sends no key at all — and that MUST read as NOT + // RECORDED, never as a failure. Same rule as StatsKnown above and LastIntegrityDepth's reserved + // empty: absence and "no" are opposite news and the wire cannot tell them apart unless we do. + LastProofRun string `json:"last_proof_run,omitempty"` // RFC3339 + LastProofStack string `json:"last_proof_stack,omitempty"` + LastProofSnapshot string `json:"last_proof_snapshot,omitempty"` + LastProofResult string `json:"last_proof_result,omitempty"` // "pass" | "fail" | "cannot_judge" + LastProofReason string `json:"last_proof_reason,omitempty"` + LastError string `json:"last_error,omitempty"` + LastDuration string `json:"last_duration,omitempty"` + RepoSizeHuman string `json:"repo_size_human,omitempty"` // RepoSizeBytes (SLICE 4) is the machine-readable repo size from `restic stats` — the soft-quota // gate's input (last-known value; a failed stats call keeps the previous one — stale-but-safe). RepoSizeBytes int64 `json:"repo_size_bytes,omitempty"` @@ -460,7 +476,7 @@ type CrossDriveBackup struct { // LastSuccess=="") and every pre-existing row on the fleet would render as never-succeeded on the // deploy — all 7 rows on the two demo boxes were in exactly that state. Set by every runner write. SuccessTracked bool `json:"success_tracked,omitempty"` - LastWarning string `json:"last_warning,omitempty"` // Tier-2 3b: capture-gap / state-only notice (Hungarian) + LastWarning string `json:"last_warning,omitempty"` // Tier-2 3b: capture-gap / state-only notice (Hungarian) // UnitLegSkipped / UnitPackageDate (R-403) — the newest run PRESERVED the copy's recovery unit // instead of refreshing it, because the source unit carried no data and this one does. Both exist // so the surface cannot render a preserved package as a fresh one: LastRun/LastSuccess describe @@ -468,8 +484,8 @@ type CrossDriveBackup struct { // destination unit's own manifest, so it is a fact about the copy; "" means UNKNOWN, never "now". UnitLegSkipped bool `json:"unit_leg_skipped,omitempty"` UnitPackageDate string `json:"unit_package_date,omitempty"` - LastDuration string `json:"last_duration,omitempty"` // "2m34s" - LastSizeHuman string `json:"last_size_human,omitempty"` // "1.2 GB" + LastDuration string `json:"last_duration,omitempty"` // "2m34s" + LastSizeHuman string `json:"last_size_human,omitempty"` // "1.2 GB" // Customer preference (set from the per-app Tier-2 config panel; PRESERVED across the runner's // status writes). UserDisabled turns Tier 2 off for this app; PreferredTarget pins a chosen @@ -1565,10 +1581,10 @@ func InferStorageLabel(path string) string { // is left with nothing to do. type RestoreHold struct { Stack string `json:"stack"` - At string `json:"at"` // RFC3339 UTC - ReplayError string `json:"replay_error,omitempty"` // what the restore hit + At string `json:"at"` // RFC3339 UTC + ReplayError string `json:"replay_error,omitempty"` // what the restore hit RollbackErr string `json:"rollback_error,omitempty"` // what the rollback then hit - SafetyDump string `json:"safety_dump,omitempty"` // basename of the undo copy that could not be applied + SafetyDump string `json:"safety_dump,omitempty"` // basename of the undo copy that could not be applied } // SetRestoreHold records a hold. Modelled on SetDisconnected: a condition, plus what it is holding.