diff --git a/CHANGELOG.md b/CHANGELOG.md index d869709..8ed8c1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,142 @@ +## v0.227.1 — the damage classifier matched restic's ordinary progress output (2026-08-30, R-359 follow-on) +**MinAgent: 0.129.0** (unchanged) + +**A patch rather than a rebuilt 0.227.0, deliberately** — 0.227.0 was already deployed to `demo-hp` +when this was found, and re-pushing changed bytes under a tag that is already running somewhere is the +`:latest` hazard with extra steps. + +`looksLikeRepositoryDamage` matched bare `"pack "`, `"tree "`, `"snapshot "` and `"blob "`. **A healthy +`restic check` prints `check all packs` and `check snapshots, trees and blobs`**, so any check that +failed for a NON-damage reason — a connection dropped mid-run, say — would have been classified as a +corrupted repository and told the customer their backups may be damaged. That is the false alarm that +teaches an operator to ignore the true one. + +The signatures are now phrases from restic's own error wording (`does not match`, `not found in index`, +`ciphertext verification failed`, `repository contains errors`, …), and the negative control that +caught it — `TestR359_HealthyRealOutputIsNotDamage`, built from the real bytes of a real passing +check — is what keeps it caught. + +## v0.227.0 — the off-site store gets checked, and the check that was advertised becomes real (2026-08-30, R-359 + R-397) +**MinAgent: 0.129.0** (unchanged) + +### R-359 — nothing ever verified that the off-site copies are still readable + +The whole-guest tier has verify jobs. The tier holding the customer's documents and photos had none: +the complete set of restic verbs this controller used was `restore, snapshots, backup, unlock, stats, +init, forget, prune, cat, config` — **no `check`**. We would have found out at restore time, with a +customer waiting. + +### ⚠ AND THE MEASUREMENT CHANGED WHAT THE FEATURE IS WORTH — read this before the rest + +Part 5's positive control corrupted one pack of a throwaway repository **without changing its size** +(64 zero bytes at offset 1024). Both depths were run against it: + +| depth | exit | verdict | +|---|---|---| +| `restic check` — **the depth that ships ON** | **0** | **`no errors were found`** | +| any `--read-data*` form — **ships OFF** | 1 | `Pack ID does not match, want 288afd3e…, got 4b6847bb…` | + +**The structure check reported a corrupted store as healthy.** It verifies the index, the pack +inventory and the snapshot graph — real failure modes, and it catches missing packs, broken indexes and +unreadable snapshots. It does **not** re-hash pack contents, so it cannot see rot inside a pack that is +still the right size. **R-399 is therefore not only a bandwidth question: at the shipped default a +class of damage is not checked at all.** + +**The cost curve, measured against the live store (134.3 MB, 67 snapshots) rather than reasoned:** + +| depth | wall | over structure-only | +|---|---|---| +| structure only | 35.0 s | — | +| `10%` | 35.9 s | +0.9 s (+3%) | +| `50%` | 37.3 s | +2.2 s (+6%) | +| `100%` | 39.2 s | **+4.2 s (+12%)** | + +At this size, re-reading **all** the data costs four seconds more than reading none — the wall clock is +dominated by SFTP round-trips, not transfer. **These figures do not extrapolate**: the structure +check's cost tracks the index, a read-data run's tracks the data. The default is still not chosen here; +R-399 now has numbers instead of guesses. + +### The hazard that shapes the whole design + +`resticStep` self-heals a crash lock by running **`unlock --remove-all`** and retrying, and its own +comment records why that is safe: *every caller holds the in-process single-flight mutex, so any lock +it meets is stale.* **A check that did not take that flag could meet a LIVE `forget --prune`'s lock +from this same box, remove it, and retry over the top of it** — on the tier holding customer data. + +So the check **takes the flag and SKIPS rather than waits**. Waiting would pin the nightly backup +behind it; a skip costs nothing because due-ness makes tomorrow try again. +`TestR359_SkipsWhenRunningFlagHeld` asserts the **non-effects** — restic never invoked, `unlock` never +in any argv — and its red-proof prints the real thing: restic running `check` with the flag held. + +**Proven live too.** The intended demonstration could not be run (`POST /api/backup/offbox/run` is a +404 — that is **R-279 and stays open**), so the same flag was exercised by its other holder: two checks +6 s apart. The second returned `skipped: true`, `"a backup or restore is already running"`, +**`duration_ms: 0`** — it never ran restic at all. + +### Due-ness, not a weekday + +A daily job asking *"is the last successful check older than 7 days?"*, not *"is it Sunday?"*. A box +switched off on its check day is checked the next day it is on. **R-341 is exactly the other shape** — +a dated check quietly missed and never caught up. No `Weekly` primitive was added; due-ness is smaller +and is what R-86 already chose for restore-tests. Registered at **06:00**, chosen from the live +schedule read off `demo-hp` (db-dump 02:30, tier2 + fill-watch 03:30, metrics-prune 04:00, offbox-backup +04:15, abandon-sweep 05:10, whole-guest gate 04:30–08:30). + +### Three outcomes, not two + +`Skipped`, `Unreachable` and failed are different facts. **"I could not look" is not "I looked and it +is broken"** — R-339 already owns reachability, and a second alarm for the same fact trains the +operator to discount the one alarm that means the backups are damaged. A timeout is unreachable, never +damage. A failure advances due-ness (a broken store must not be re-checked nightly); a skip and an +unreachable store do not. + +### R-397 — the notifiers get their caller + +`NotifyIntegrityOK` / `NotifyIntegrityFailed` existed with **no caller**; the hub allowlists both event +types and carries the Hungarian customer text for both; the settings checkbox exists; the debug button +posts to `/api/debug/backup/integrity` and **the dispatch had no such case**. Everything was built +except the part that runs. **Sixth instance of that shape in this project.** + +Success is severity `info`, which `severityNotifies` drops before either leg — it **mails nobody, by +design**. A weekly success e-mail is how people stop reading their alerts. Observed live: +`Event pushed: backup_integrity_ok (info) — A távoli mentés ellenőrzése rendben lezajlott. (35s)`. + +The customer gets a **sentence**; restic's words go to the log, truncated (R-379: 615 bytes of raw +database text reached a customer once). Published on `OffboxReportStatus`, **not** on +`report.BackupReport`'s `IntegrityOK` — those were retired by R-331 the day before, and +`TestBackupReport_DeadFieldsStayZero` still passes unmodified. + +`backup_integrity_failed` was checked against `perAppCooldownEvents`, `operatorOnlyEvents` and +`DefaultEnabledEvents` and **deliberately left out of all three** — it is already in the right shape. + +### Two defects this work introduced and then caught + +**The damage classifier matched restic's ordinary progress output.** The first draft looked for bare +`"pack "`, `"tree "`, `"snapshot "` — and a healthy run prints `check all packs` and +`check snapshots, trees and blobs`, so a check that failed for a non-damage reason would have alarmed +the customer that their backups were corrupt. **Caught by the negative control** +(`TestR359_HealthyRealOutputIsNotDamage`) using the real bytes of a real passing check. The signatures +are now phrases from restic's own error wording. + +**An exit code I misread.** An early run showed `exit=0` on the subset forms while they printed +`Fatal: repository contains errors`. That was not restic — the commands were piped through `tail`, so +`$?` was tail's. Re-measured without pipes, every read-data form exits 1. The project's own +"exit codes that lie" trap, caught by re-measuring rather than reasoning. + +### Part 0 was NOT built, and R-398 was my own mistake + +R-398 (filed by me yesterday) said `resticStep` is not a seam so no test can drive a restic-backed +path. **The first half is true and the conclusion was false:** `offboxRunner` / `SetOffboxRunner` / +`m.runner()` has been injectable since the off-site tier shipped, and other tests already drive restic +paths through it. A `resticStepFn` seam would have been **worse** here — it would replace the +`unlock --remove-all` escalation and hide it from the assertions that must observe it. R-358's AST +ordering test is converted to a real execution test instead, which immediately surfaced something the +AST walk could not: `unlockStale` legitimately runs before the restore. R-398 is **corrected, not +closed**. + +**Green gate:** 28 packages, rc 0. Four red-proofs (A2, B1, C2, D2), each printing the pre-fix +behaviour. Evidence: `felhom.eu/documentation/tests/r359-integrity-2026-08-30/`. + ## v0.226.1 — the unknown that the v0.226.0 fix drew as a zero (2026-08-30, R-353 follow-on) **MinAgent: 0.129.0** (unchanged) diff --git a/CONTEXT.md b/CONTEXT.md index c030cc7..0ee04dd 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,35 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-08-30 (v0.226.0 — R-353/R-357/R-358/R-360: the restore tells the truth) +Last updated: 2026-08-30 (v0.227.1 — R-359/R-397: the off-site store gets checked) + +> **2026-08-30 — v0.227.0/v0.227.1. THREE RULINGS, recorded so none is re-litigated.** +> +> **1. The integrity check TAKES the single-writer flag and SKIPS rather than waits.** `resticStep` +> self-heals a crash lock by running `unlock --remove-all` and retrying, and its own comment records +> why that is safe: every caller holds the in-process single-flight mutex, so any lock it meets is +> stale. A check that did not take the flag could meet a **live** `forget --prune`'s lock from this +> same box, remove it, and retry over the top of it. It skips rather than waits because waiting would +> pin the nightly backup behind a check, and a skip costs nothing — due-ness makes tomorrow try again. +> +> **2. Due-ness, not a weekday.** A daily job asking "is the last successful check older than 7 days?" +> catches up after downtime; a Sunday-gated job silently skips a week every time the box is off on a +> Sunday. **R-341 is that failure**, and no `Weekly` primitive was added to the scheduler. +> +> **3. The result is published on `OffboxReportStatus`, NOT on `report.BackupReport`'s `IntegrityOK` / +> `LastIntegrityCheck`.** R-331 retired those the day before, because the hub card rendering them read +> `Integrity Unknown` for every customer forever. Giving them a live value would resurrect a card that +> was deliberately removed and break `TestBackupReport_DeadFieldsStayZero`. +> +> **AND THE MEASUREMENT THAT MATTERS MOST, because it is the thing a future session will assume +> wrongly: the structure check that ships ON does NOT catch silent corruption.** Measured on demo-hp — +> a pack corrupted without changing its size returned `no errors were found`, exit 0. Only +> `--read-data*` caught it. The structure check does catch missing packs, broken indexes and unreadable +> snapshots, which are real; it does not re-hash pack contents. **R-399 is therefore not merely a +> bandwidth question.** The cost curve is measured and small at today's store size (100% costs +12% +> wall-clock over structure-only, 39.2 s vs 35.0 s on 134.3 MB) but does NOT extrapolate — the +> structure check's cost tracks the index, read-data's tracks the data. + > **2026-08-30 — v0.226.0. TWO RULINGS THIS SESSION MAKES, recorded so neither is re-litigated.** > diff --git a/REUSE.md b/REUSE.md index dd17493..95313f6 100644 --- a/REUSE.md +++ b/REUSE.md @@ -53,6 +53,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) | +| `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) | | `Manager.SetOffboxLatestSnapshotFn` (R-357, v0.226.0) | controller/internal/backup/offbox_restore.go | `(fn func(ctx, stack) (id string, paths []string, err error))` INIT/TEST-ONLY | Overriding the restic snapshot lookup in tests | Exists because R-357's gate could not otherwise be tested at the level that matters: reaching it requires getting past `offboxLatestSnapshot`, which shells to restic. **The assertion the seam enables is `StopStack` call count == 0** — a gate placed after the stop returns the right sentence and still takes the outage. Nil in production | | `CheckPlacement` + `PlacementMismatchMessage` | controller/internal/backup/offbox_placement.go | `(*RecoveryManifest, liveDrive, liveNS) PlacementCheck` / `(stack, PlacementCheck) string` | Comparing where a backup SAYS the data lived against where a restore is about to write | Pure and total — nil/empty/blank manifest all give the same honest "not known, no mismatch". **An UNKNOWN is never a mismatch** (refusing on an absence strands every pre-field unit). Compares the DRIVE only (the namespace root is derived from it), Cleaned, so a trailing slash is not a difference. The message names BOTH values on purpose | | `RecordedUnitForStack` + `RecordedAddress` | controller/internal/backup/offbox_placement.go | `(stack) (RecordedPlacement, RecordedAddress, bool)` | Reading back the address + data folder a backup recorded, for a reinstall prefill | Local file reads over every readable namespace root — **no network, no restic, no restore**; it exists for the NOT-INSTALLED case where `GetStackHDDPath` is `""`. **`RecordedAddress.Known()` requires BOTH halves:** an absent `SUBDOMAIN` makes the live deploy path fall back to the CATALOG default (`stacks/deploy.go:88-90`), and offering that back as "what your backup says" is a fabricated fact | diff --git a/controller/README.md b/controller/README.md index a7868bc..4f56a8d 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1089,6 +1089,43 @@ 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 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 +v0.227.0 nothing verified that the off-site copies were readable — the whole-guest tier had verify +jobs, the tier holding the customer's documents and photos had none. + +| | | +|---|---| +| job | `offsite-integrity`, `sched.Daily` at **06:00** | +| cadence | **due-ness, not a weekday** — runs when the last SUCCESSFUL check is older than `monitoring.integrity.max_age_days` (default **7**). A box switched off on its check day is checked the next day it is on | +| depth | structure + index by default. `monitoring.integrity.read_data_subset` (default **empty**) adds `--read-data-subset=`; a malformed value is refused at read time with a WARN and treated as empty | +| guard | takes the single-writer flag and **SKIPS rather than waits** | +| timeout | 30 min (`integrityCheckTimeout`) — bounds a hung repository so it cannot pin the flag | +| by hand | `POST /api/debug/backup/integrity` — same code path, due-ness ignored, **every other guard intact** | +| result | persisted on `settings.OffboxTarget` (`last_integrity_check`, `last_integrity_ok`) and published on `OffboxReportStatus` | + +**Three outcomes, not two.** `Skipped` (a sibling operation held the flag), `Unreachable` (the repo +could not be opened, or the check timed out) and failed are different facts. Only a failure notifies; +a skip and an unreachable repository do **not** advance due-ness, so tomorrow tries again. A failure +**does** advance it — re-checking a broken store nightly is load with no new information. + +**Notifications.** `backup_integrity_ok` is severity `info`, which `severityNotifies` drops — it mails +nobody, by design. `backup_integrity_failed` is `error` and reaches the operator; the customer leg is +switchable and OFF by default. The customer gets a sentence; restic's output goes to the log, +truncated. + +> **⚠ THE STRUCTURE CHECK DOES NOT CATCH SILENT CORRUPTION, and this is the thing to know before +> trusting it.** Measured on `demo-hp` 2026-08-30 against a throwaway repo whose pack was corrupted +> *without changing its size*: `restic check` returned **`no errors were found`, exit 0**; every +> `--read-data*` form returned `Pack ID does not match …` and exit 1. The structure check verifies the +> index, the pack inventory and the snapshot graph — it catches missing packs, broken indexes and +> unreadable snapshots — but it does **not** re-hash pack contents. Choosing the read-data depth is +> **R-399**, and the cost curve is measured: +> structure 35.0 s · 10% 35.9 s · 50% 37.3 s · **100% 39.2 s** on a 134.3 MB / 67-snapshot store. +> Those figures do not extrapolate: the structure check's cost tracks the index, read-data's tracks +> the data. + ### Restore refusals (v0.226.0) Three guards added on the off-site restore surface, all server-side: @@ -2830,7 +2867,7 @@ When `logging.level: "debug"` is set in `controller.yaml`, the controller expose |---|---------|-----------|-------------| | 1 | Rendszer diagnosztika | `GET /api/debug/dump` | Full state dump: controller info, storage, stacks, network (guest-netns interfaces/route/DNS via the samba door, R-66; best-effort per item), scheduler, health, alerts. JSON download. | | 2 | Értesítés teszt | `POST /api/debug/event/test`, `GET /api/debug/event/history` | Send test events with configurable type/severity, view event history ring buffer. | -| 3 | Mentés teszt | `POST /api/debug/backup/{dbdump,crossdrive,integrity,infra}` | Trigger individual backup phases independently. | +| 3 | Mentés teszt | `POST /api/debug/backup/dbdump` · `POST /api/debug/backup/integrity` | Trigger a DB dump, or run an off-site integrity check by hand. **`crossdrive` and `infra` are NOT implemented** — their buttons 404 (R-400). | | 4 | Tárhely teszt | `POST /api/debug/storage/simulate-{disconnect,reconnect}`, `GET /api/debug/storage/watchdog-status` | Simulate drive disconnect/reconnect without unmounting. Per-path probe state with 5s auto-refresh. | | 5 | Hub & Kapcsolatok | `POST /api/debug/hub/{push,infra-push,test-connectivity,preferences-sync}`, `POST /api/debug/gitea/test-connectivity` | Test Hub/Gitea connectivity with latency. Push reports and sync preferences. | | — | Telemetria teszt | `GET /api/debug/telemetry` | Run the full telemetry collection pipeline on-demand (metrics query + log scan). Returns per-app table: container list, memory current/avg/peak, CPU avg, catalog limit, log error/warning counts, and top issues. Useful for verifying container→stack mapping and testing log scanner patterns without waiting for the 15-minute report cycle. | diff --git a/controller/internal/backup/offbox_integrity.go b/controller/internal/backup/offbox_integrity.go index daea41a..8c04608 100644 --- a/controller/internal/backup/offbox_integrity.go +++ b/controller/internal/backup/offbox_integrity.go @@ -233,15 +233,26 @@ func (m *Manager) CheckOffboxIntegrity(ctx context.Context) IntegrityResult { // already alarms when the store cannot be reached. func looksLikeRepositoryDamage(out []byte) bool { s := strings.ToLower(string(out)) + // THE SIGNATURES ARE PHRASES, NOT WORDS, AND THE REASON IS A BUG THIS FILE ALREADY HAD. + // + // The first draft matched bare `"pack "`, `"tree "`, `"snapshot "` and `"blob "`. Those appear in + // restic's ORDINARY PROGRESS OUTPUT — a healthy run prints `check all packs` and + // `check snapshots, trees and blobs` — so a check that failed for a NON-damage reason (a dropped + // connection mid-run, say) would have been classified as a corrupted store and alarmed the customer + // that their backups were damaged. Caught by the negative control in + // TestR359_HealthyRealOutputIsNotDamage, using the real bytes of a real passing check. + // + // Every phrase below is restic's own error wording, taken from the 2026-08-30 damaged-pack run on + // demo-hp or from restic's check source — never paraphrased. for _, sig := range []string{ - "pack ", // "pack 1234abcd: not found in index" / "... size mismatch" - "load index", // a broken index - "blob ", // "blob not found" - "tree ", // "tree 1234: file ... blob not found" - "snapshot ", // "error for snapshot ...: ..." + "does not match", // "Pack ID does not match, want , got " — the measured one + "not found in index", // a blob or pack the index promises and the store lacks + "blob not found", // + "size mismatch", // "ciphertext verification failed", "integrity error", - "repository contains errors", + "repository contains errors", // restic's own summary verdict + "failed to load index", // } { if strings.Contains(s, sig) { return true diff --git a/controller/internal/backup/r359_integrity_test.go b/controller/internal/backup/r359_integrity_test.go index cc9ed68..bd2841f 100644 --- a/controller/internal/backup/r359_integrity_test.go +++ b/controller/internal/backup/r359_integrity_test.go @@ -259,3 +259,47 @@ func TestR359_NoTargetConfiguredIsASilentSkip(t *testing.T) { t.Fatal("a skip was reported as a passing check — nothing was checked") } } + +// TestR359_RealResticDamageOutputIsClassifiedAsDamage uses the EXACT bytes restic produced on +// `demo-hp` on 2026-08-30 against a deliberately corrupted throwaway repository (Part 5's positive +// control). Invented output would only prove the classifier agrees with my guess about restic; this +// closes the loop on real bytes. +// +// The damage was 64 zero bytes written at offset 1024 of one pack, leaving the file SIZE unchanged — +// the subtlest form, and the one a structure check cannot see. See the accompanying finding: plain +// `restic check` returned "no errors were found" and exit 0 over this very repository. +func TestR359_RealResticDamageOutputIsClassifiedAsDamage(t *testing.T) { + const realOutput = "Pack ID does not match, want 288afd3e868dc6bd210e33bd6f821e9f088a5fd71a0464c23ce82eb8217bf0cc, got 4b6847bb5eece6c56e69d7381733827330a4eb799fc26a92287a181edc496d2b\nFatal: repository contains errors" + + if !looksLikeRepositoryDamage([]byte(realOutput)) { + t.Fatal("restic's REAL damage output was not recognised as damage — the check would report a " + + "corrupted store as merely unreachable, and the customer would never be told") + } + + m, _ := newIntegrityManager(t, okRepo(func([]string) ([]byte, error) { + return []byte(realOutput), errFake + })) + res := m.CheckOffboxIntegrity(context.Background()) + + if res.OK { + t.Fatal("a repository restic called corrupt was reported as passing") + } + if res.Unreachable { + t.Fatal("readable-and-corrupt was reported as unreachable — the store WAS opened and read; " + + "that misclassification would suppress the one alarm that matters") + } + if !strings.Contains(res.Output, "288afd3e") { + t.Error("restic's own words did not reach the log") + } +} + +// TestR359_HealthyRealOutputIsNotDamage is the negative control for the classifier, from the same +// live run: the healthy repository's actual output must not trip the damage predicate. +func TestR359_HealthyRealOutputIsNotDamage(t *testing.T) { + const realHealthy = "using temporary cache in /tmp/restic-check-cache-962728151\ncreate exclusive lock for repository\nload indexes\ncheck all packs\ncheck snapshots, trees and blobs\n\nno errors were found" + + if looksLikeRepositoryDamage([]byte(realHealthy)) { + t.Fatalf("a HEALTHY check's real output was classified as damage — every weekly check would " + + "alarm, which is how an operator learns to ignore the alarm") + } +}