diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index cdfa32c..9013669 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -174,6 +174,12 @@ type Manager struct { // volume leg is unit-testable without Docker. Nil → the real restoreDockerVolumesFrom. volumeReplayFrom func(stackName, dumpDir string) (int, error) + // R-403 unit-REHYDRATE seam — the copy that refills the primary unit from the Tier-2 mirror after + // a restore, overridable so the "only when hollow" condition and the byte-identical non-effect are + // testable without rsync. Nil → the real rsyncMirror. It is a SEPARATE seam from tier2Mirror on + // purpose: a test that guards the nightly copy must not silently also stub the rehydrate. + unitRehydrate func(src, dst string) error + // F7 tar seam — the ONE docker exec inside DumpAppVolumes, overridable so the atomic-write // behaviour (tmp+fsync+rename; the last good `.tar` survives a mid-write failure) is unit-testable // without Docker. It must write the tar to `/.tar.tmp` and return the combined diff --git a/controller/internal/backup/r403_hollow.go b/controller/internal/backup/r403_hollow.go new file mode 100644 index 0000000..1231cd0 --- /dev/null +++ b/controller/internal/backup/r403_hollow.go @@ -0,0 +1,50 @@ +package backup + +// 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 +// 120 082 104 bytes (4 database dumps + 3 named-volume tars) to 7 036 bytes (none of either) in one +// nightly run, and the run recorded itself as a SUCCESS — `Tier 2 copied docmost → … (14.9 KB, +// 0 leg(s), 0s)`. Evidence: `felhom.eu/documentation/audits/DRILL-r403-tier2-delete-2026-08-31/`. +// +// THE MECHANISM, in three lines of existing code that were each individually correct: +// 1. `RunTier2` guards the unit leg with `os.Stat(unitDir)` — *does the folder exist*. +// 2. `rsyncMirror` is `rsync -a --delete` — an exact mirror, which is what a derived copy must be. +// 3. Nothing between them compares the source to the destination. +// An EMPTY recovery unit is a folder that exists. So a primary unit that had lost its dumps — after a +// restore, a failed dump run, a crash mid-capture, a remount — was mirrored over a complete copy, and +// `--delete` removed the customer's last surviving package. +// +// WHAT THIS FILE DELIBERATELY DOES **NOT** DO: it does not make the Tier-2 copy un-shrinkable. +// `07-backup-architecture.md` §8 row 5 records that the secondary is a DERIVED copy, rebuilt on the +// next run ("Migration = rebuild, not preserve"), and `tier2.go`'s own header records that a +// classified app's copy legitimately shrinks as `export` drops out of its class set. Fencing +// shrinkage would be calling a decision a defect. The fence here is exactly one shape: a source that +// carries NO data replacing a destination that carries some. + +// unitCarriesData reports whether a recovery-unit DIRECTORY holds RECOVERABLE DATA — the app's +// database dumps or its named-volume tars. +// +// IT ASKS THE MANIFEST, NEVER THE BYTE SIZE, and that is the whole design of the predicate. A unit +// with a large compose tree and no dumps is dangerous; a tiny unit belonging to a tiny app is fine. +// Size answers "how big", and the question here is "is there anything to recover". `dirSizeBytes` +// exists two files away and would have been the obvious wrong answer — TestR403_SizeIsNeverConsulted +// is the guard that keeps it out. +// +// FAIL CLOSED on an absent or unparseable manifest: `readManifest` returns nil for both, and a unit +// whose manifest cannot be read is a unit whose contents cannot be vouched for. Treating it as +// data-bearing would let an unreadable source authorise a delete. +// +// ONE predicate, every caller. The mirror guard and the post-restore rehydrate both ask this +// function; two copies of the definition is how the two halves of a fix drift apart. +func unitCarriesData(unitDir string) bool { + man := readManifest(UnitManifestFile(unitDir)) + if man == nil { + return false + } + return len(man.DBDumps) > 0 || len(man.VolumeDumps) > 0 +} + +// 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) } diff --git a/controller/internal/backup/r403_hollow_test.go b/controller/internal/backup/r403_hollow_test.go new file mode 100644 index 0000000..c268bd2 --- /dev/null +++ b/controller/internal/backup/r403_hollow_test.go @@ -0,0 +1,134 @@ +package backup + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// R-403 Group A — the hollowness predicate. +// +// It decides whether a nightly copy is allowed to delete a customer's last package, so it is worth +// pinning precisely. Every case below is a shape that exists on a real box. + +// r403Unit writes a recovery unit directory carrying the given dump lists, plus whatever extra files +// the caller asks for. The manifest is written by the PRODUCTION writeManifest, so the predicate and +// the capture meet at real bytes rather than at a hand-rolled JSON literal. +func r403Unit(t *testing.T, dbDumps, volDumps []string, extra map[string]string) string { + t.Helper() + dir := filepath.Join(t.TempDir(), "recovery-unit") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + man := &RecoveryManifest{SchemaVersion: 2, AppName: "app", DBDumps: dbDumps, VolumeDumps: volDumps} + if err := writeManifest(UnitManifestFile(dir), man); err != nil { + t.Fatal(err) + } + for rel, body := range extra { + mustWrite(t, filepath.Join(dir, rel), body) + } + return dir +} + +// A1 — a manifest listing neither kind of dump is HOLLOW. This is the exact shape measured on +// demo-hp: `"db_dumps": []`, `"volume_dumps": null`. +func TestR403_ManifestWithNoDumpsIsHollow(t *testing.T) { + for _, tc := range []struct { + name string + dbs, vols []string + }{ + {"both empty slices", []string{}, []string{}}, + {"both nil (the measured shape: db_dumps [] and volume_dumps null)", nil, nil}, + {"empty db, nil volumes", []string{}, nil}, + } { + dir := r403Unit(t, tc.dbs, tc.vols, nil) + if !unitIsHollow(dir) { + t.Errorf("%s: unitIsHollow()=false, want true", tc.name) + } + if unitCarriesData(dir) { + t.Errorf("%s: unitCarriesData()=true, want false", tc.name) + } + } +} + +// A2 — volume tars alone are enough. For the 45 class-B apps that archive is the entire dataset, so +// treating a volume-only unit as hollow would let the guard delete exactly the material it exists to +// protect. +func TestR403_ManifestWithVolumeDumpsOnlyIsNotHollow(t *testing.T) { + dir := r403Unit(t, nil, []string{"app_data.tar"}, nil) + if unitIsHollow(dir) { + t.Error("a unit carrying a volume tar was called hollow") + } +} + +// A3 — a database dump alone is enough, for the same reason from the other side. +func TestR403_ManifestWithDBDumpsOnlyIsNotHollow(t *testing.T) { + dir := r403Unit(t, []string{"app-postgres.sql"}, nil, nil) + if unitIsHollow(dir) { + t.Error("a unit carrying a database dump was called hollow") + } +} + +// A4 — an absent manifest is HOLLOW. FAIL CLOSED: a unit whose contents cannot be vouched for must +// never authorise a delete of one whose contents can. +func TestR403_AbsentManifestIsHollow(t *testing.T) { + dir := filepath.Join(t.TempDir(), "recovery-unit") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if !unitIsHollow(dir) { + t.Error("a unit directory with no manifest was treated as data-bearing") + } + // And a directory that does not exist at all. + if !unitIsHollow(filepath.Join(t.TempDir(), "nope")) { + t.Error("an absent directory was treated as data-bearing") + } +} + +// A5 — an unparseable manifest is HOLLOW, for the same fail-closed reason. +func TestR403_UnparseableManifestIsHollow(t *testing.T) { + dir := filepath.Join(t.TempDir(), "recovery-unit") + mustWrite(t, UnitManifestFile(dir), "{ this is not json") + if !unitIsHollow(dir) { + t.Error("a unit with an unparseable manifest was treated as data-bearing") + } +} + +// A6 — TestR403_SizeIsNeverConsulted. THE DESIGN OF THE PREDICATE, as a test. +// +// A unit with a large compose tree and no dumps is DANGEROUS — it is exactly the shape that deleted +// 120 MB of dumps on demo-hp, and `dirSizeBytes` sits two files away and would call it substantial. +// A tiny unit belonging to a tiny app is FINE. Size answers "how big"; the question is "is there +// anything to recover", and only the manifest answers that. +// +// Red-proof (recorded in REPORT.md): switch `unitCarriesData` to a `dirSizeBytes` threshold and this +// test fails on the big-but-empty case. +func TestR403_SizeIsNeverConsulted(t *testing.T) { + // BIG and hollow: a fat compose capture, no dumps. + big := r403Unit(t, nil, nil, map[string]string{ + "compose/docker-compose.yml": strings.Repeat("# padding\n", 20000), + "compose/app.yaml": strings.Repeat("# padding\n", 20000), + }) + bigBytes := dirSizeBytes(big) + if bigBytes < 100000 { + t.Fatalf("fixture is not big enough to make the point: %d bytes", bigBytes) + } + if !unitIsHollow(big) { + t.Errorf("a %d-byte unit listing NO dumps was called data-bearing — size was consulted", bigBytes) + } + + // TINY and data-bearing: one small dump, nothing else. + small := r403Unit(t, nil, []string{"v.tar"}, map[string]string{"volume-dumps/v.tar": "x"}) + smallBytes := dirSizeBytes(small) + if unitIsHollow(small) { + t.Errorf("a %d-byte unit listing a volume tar was called hollow — size was consulted", smallBytes) + } + if smallBytes >= bigBytes { + t.Fatalf("fixture inverted: small=%d big=%d", smallBytes, bigBytes) + } + // The consequence stated plainly: the SMALLER unit is the one that carries data. + if !unitCarriesData(small) || unitCarriesData(big) { + t.Error("the predicate ordered these by size rather than by contents") + } +} diff --git a/controller/internal/backup/r403_mirror_guard_test.go b/controller/internal/backup/r403_mirror_guard_test.go new file mode 100644 index 0000000..5035fe8 --- /dev/null +++ b/controller/internal/backup/r403_mirror_guard_test.go @@ -0,0 +1,270 @@ +package backup + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" +) + +// R-403 Group B — the mirror guard, driven through the REAL RunTier2. +// +// The `tier2Mirror` seam records every (src,dst) the run asks for, so a skip is provable as an +// ABSENCE OF A CALL rather than inferred from a log line — and the destination's own bytes are +// compared before and after, which is the consequence the customer actually cares about. + +// r403Recorder is a mirror seam that records its calls and then performs a REAL MIRROR — the +// destination is emptied first, so `rsync -a --delete`'s defining behaviour is present in the test. +// +// This matters more than it looks: `copyTree` (the additive helper next door) would let every +// assertion here pass while proving nothing, because the deletion IS the defect. A seam that is +// gentler than the thing it stands in for is a test that cannot see the bug. +type r403Recorder struct{ calls []string } + +func (r *r403Recorder) mirror(src, dst string) error { + r.calls = append(r.calls, dst) + if err := os.RemoveAll(dst); err != nil { + return err + } + return copyTree(src, dst) +} + +func (r *r403Recorder) calledFor(sub string) bool { + for _, c := range r.calls { + if strings.Contains(c, sub) { + return true + } + } + return false +} + +// r403Fixture builds a Manager with a source drive and a registered off-drive target, seeds the +// SOURCE unit and (optionally) a pre-existing DESTINATION unit, and returns the recorder. +func r403Fixture(t *testing.T, srcDB, srcVols []string, destDB, destVols []string, destExists bool) ( + m *Manager, rec *r403Recorder, src, destBase string) { + t.Helper() + m, src, target, prov := newTier2V2(t, "app") + rec = &r403Recorder{} + m.tier2Mirror = rec.mirror + prov.has["app"] = false // legacy path, no classified binds → the unit leg is the only leg + destBase = filepath.Join(target, "backups", "secondary", "app") + + // SOURCE unit — newTier2V2 already wrote a `{}` manifest; replace it with a real one. + srcUnit := RecoveryUnitPath(src, "app") + man := &RecoveryManifest{SchemaVersion: 2, AppName: "app", DBDumps: srcDB, VolumeDumps: srcVols} + if err := writeManifest(UnitManifestFile(srcUnit), man); err != nil { + t.Fatal(err) + } + for _, d := range srcDB { + mustWrite(t, filepath.Join(UnitDBDumpDir(srcUnit), d), "SOURCE-"+d) + } + for _, v := range srcVols { + mustWrite(t, filepath.Join(UnitVolumeDumpDir(srcUnit), v), "SOURCE-"+v) + } + + // DESTINATION unit — the copy that must survive. + if destExists { + destUnit := filepath.Join(destBase, "recovery-unit") + // atomicWrite needs the parent to exist — the real capture creates it before writing. + if err := os.MkdirAll(destUnit, 0o755); err != nil { + t.Fatal(err) + } + dman := &RecoveryManifest{SchemaVersion: 2, AppName: "app", DBDumps: destDB, VolumeDumps: destVols} + if err := writeManifest(UnitManifestFile(destUnit), dman); err != nil { + t.Fatal(err) + } + for _, d := range destDB { + mustWrite(t, filepath.Join(UnitDBDumpDir(destUnit), d), "DEST-"+d) + } + for _, v := range destVols { + mustWrite(t, filepath.Join(UnitVolumeDumpDir(destUnit), v), "DEST-"+v) + } + mustWrite(t, filepath.Join(destBase, tier2LayoutMarker), tier2LayoutVersion) + } + return m, rec, src, destBase +} + +// B1 — TestR403_HollowSourceOverCompleteDestIsSkipped. THE ACCEPTANCE TEST. +// +// This is the state measured live on demo-hp 2026-08-31: a hollow primary unit and a complete +// secondary copy. On the shipped v0.229.0 the run deleted 4 database dumps and 3 volume tars — +// 120 082 104 B → 7 036 B — and reported success. +// +// Red-proof (recorded in REPORT.md): remove the guard and this fails on BOTH assertions — the seam +// is called for the unit leg, and the destination's fingerprint changes. +func TestR403_HollowSourceOverCompleteDestIsSkipped(t *testing.T) { + m, rec, _, destBase := r403Fixture(t, nil, nil, + []string{"app-postgres.sql"}, []string{"app_data.tar", "app_cache.tar"}, true) + destUnit := filepath.Join(destBase, "recovery-unit") + + before := fingerprintTree(t, destUnit) + if err := m.RunTier2("app"); err != nil { + t.Fatalf("RunTier2 must still succeed — the other legs are not held hostage: %v", err) + } + + // THE CONSEQUENCE, first: the customer's package is byte-identical. + if after := fingerprintTree(t, destUnit); after != before { + t.Error("the destination unit CHANGED — a hollow source was mirrored over a complete copy (R-403)") + } + for _, f := range []string{"db-dumps/app-postgres.sql", "volume-dumps/app_data.tar", "volume-dumps/app_cache.tar"} { + if _, err := os.Stat(filepath.Join(destUnit, f)); err != nil { + t.Errorf("%s was DELETED from the copy: %v", f, err) + } + } + // And the mechanism: the mirror was never asked to touch the unit leg. + if rec.calledFor("recovery-unit") { + t.Errorf("the mirror seam WAS called for the unit leg: %v", rec.calls) + } +} + +// B2 — a complete source still mirrors. The guard must not become a general refusal. +func TestR403_CompleteSourceStillMirrors(t *testing.T) { + m, rec, _, destBase := r403Fixture(t, []string{"app-postgres.sql"}, []string{"app_data.tar"}, + []string{"old.sql"}, []string{"old.tar"}, true) + if err := m.RunTier2("app"); err != nil { + t.Fatalf("RunTier2: %v", err) + } + if !rec.calledFor("recovery-unit") { + t.Fatalf("the unit leg did not run for a complete source: %v", rec.calls) + } + destUnit := filepath.Join(destBase, "recovery-unit") + if _, err := os.Stat(filepath.Join(destUnit, "volume-dumps/app_data.tar")); err != nil { + t.Errorf("the source's tar did not reach the copy: %v", err) + } + // --delete still works: the stale destination file is gone. The mirror is still a MIRROR. + if _, err := os.Stat(filepath.Join(destUnit, "volume-dumps/old.tar")); err == nil { + t.Error("the stale destination tar survived — --delete was neutered, which is NOT the fix") + } +} + +// B3 — hollow over hollow still mirrors. An app that genuinely has no database and no volumes is not +// a defect, and both sides agree, so nothing is at risk. +func TestR403_HollowOverHollowStillMirrors(t *testing.T) { + m, rec, _, _ := r403Fixture(t, nil, nil, nil, nil, true) + if err := m.RunTier2("app"); err != nil { + t.Fatalf("RunTier2: %v", err) + } + if !rec.calledFor("recovery-unit") { + t.Errorf("hollow→hollow was skipped; it must mirror: %v", rec.calls) + } +} + +// B3b — a first copy (no destination unit at all) still mirrors, hollow source or not. +func TestR403_FirstCopyStillMirrors(t *testing.T) { + m, rec, _, _ := r403Fixture(t, nil, nil, nil, nil, false) + if err := m.RunTier2("app"); err != nil { + t.Fatalf("RunTier2: %v", err) + } + if !rec.calledFor("recovery-unit") { + t.Errorf("the first copy was skipped: %v", rec.calls) + } +} + +// B4 — the other legs still run when the unit leg is skipped. A preserved package must not cost the +// customer their file legs. +func TestR403_OtherLegsStillRunWhenTheUnitLegIsSkipped(t *testing.T) { + m, rec, src, _ := r403Fixture(t, nil, nil, []string{"a.sql"}, []string{"a.tar"}, true) + // Give the app a classified file leg with real content. + prov := m.stackProvider.(*t2v2Provider) + prov.has["app"] = true + prov.binds["app"] = []ClassifiedBind{mHDD("appdata/app")} + mustWrite(t, filepath.Join(src, "appdata", "app", "user.txt"), "USERDATA") + + if err := m.RunTier2("app"); err != nil { + t.Fatalf("RunTier2: %v", err) + } + if rec.calledFor("recovery-unit") { + t.Error("the unit leg ran despite a hollow source over a complete destination") + } + if !rec.calledFor(filepath.Join("hdd", "appdata", "app")) { + t.Errorf("the FILE leg was held hostage by the skipped unit leg: %v", rec.calls) + } +} + +// B5 — TestR403_SkipIsRecordedForTheSurface. Not only logged. +// +// A refusal that lives only in a container log is a refusal the customer cannot see, and the whole +// point of preserving the copy is lost if the page then calls it fresh. +func TestR403_SkipIsRecordedForTheSurface(t *testing.T) { + m, _, _, destBase := r403Fixture(t, nil, nil, []string{"a.sql"}, []string{"a.tar"}, true) + // Stamp the destination manifest with a date so the surface has a package date to name. + destUnit := filepath.Join(destBase, "recovery-unit") + dman := &RecoveryManifest{SchemaVersion: 2, AppName: "app", CreatedAt: "2026-08-25T03:30:00Z", + DBDumps: []string{"a.sql"}, VolumeDumps: []string{"a.tar"}} + if err := os.MkdirAll(destUnit, 0o755); err != nil { + t.Fatal(err) + } + if err := writeManifest(UnitManifestFile(destUnit), dman); err != nil { + t.Fatal(err) + } + + if err := m.RunTier2("app"); err != nil { + t.Fatalf("RunTier2: %v", err) + } + cd := m.settings.GetCrossDriveConfig("app") + if cd == nil { + t.Fatal("no cross-drive record was written at all") + } + if !cd.UnitLegSkipped { + t.Error("UnitLegSkipped is false — the surface cannot tell the package was preserved") + } + if cd.UnitPackageDate != "2026-08-25T03:30:00Z" { + t.Errorf("UnitPackageDate = %q, want the DESTINATION manifest's own created_at", cd.UnitPackageDate) + } + if !strings.Contains(cd.LastWarning, "hi") { // ASCII fragment of "hiányos" (R-364) + t.Errorf("LastWarning does not carry the preserved-copy notice: %q", cd.LastWarning) + } + if cd.LastWarning != tier2UnitPreservedWarning && + !strings.Contains(cd.LastWarning, tier2UnitPreservedWarning) { + t.Errorf("LastWarning is not the named constant: %q", cd.LastWarning) + } + // And the coverage the restore surface reads agrees, from the ARTIFACT rather than the record. + cov, err := m.Tier2RestoreCoverage("app") + if err != nil { + t.Fatalf("coverage: %v", err) + } + date, older := cov.UnitRestoreDate() + if date != "2026-08-25T03:30:00Z" || !older { + t.Errorf("UnitRestoreDate() = (%q,%v), want the package date and older-than-the-run", date, older) + } +} + +// B6 — TestR403_DataLegShrinkIsUnaffected. The guard is on the UNIT leg only. +// +// `07-backup-architecture.md` §8 row 5 records that the secondary is a derived copy, rebuilt on the +// next run, and tier2.go's header records that a classified app's copy legitimately shrinks as +// `export` drops out of its class set. Fencing that would be calling a decision a defect. +// +// Red-proof (recorded in REPORT.md): widen the guard to the data legs and this fails — the stale file +// survives in the copy. +func TestR403_DataLegShrinkIsUnaffected(t *testing.T) { + m, _, src, destBase := r403Fixture(t, []string{"a.sql"}, []string{"a.tar"}, nil, nil, true) + prov := m.stackProvider.(*t2v2Provider) + prov.has["app"] = true + prov.binds["app"] = []ClassifiedBind{mHDD("appdata/app")} + mustWrite(t, filepath.Join(src, "appdata", "app", "kept.txt"), "KEPT") + // A file that exists ONLY in the destination — the shrink case. + mustWrite(t, filepath.Join(destBase, "hdd", "appdata", "app", "dropped.txt"), "SHOULD-GO") + + if err := m.RunTier2("app"); err != nil { + t.Fatalf("RunTier2: %v", err) + } + if _, err := os.Stat(filepath.Join(destBase, "hdd", "appdata", "app", "kept.txt")); err != nil { + t.Errorf("the live file did not reach the copy: %v", err) + } + if _, err := os.Stat(filepath.Join(destBase, "hdd", "appdata", "app", "dropped.txt")); err == nil { + t.Error("a data leg stopped shrinking — the guard is TOO WIDE and is fencing a design decision") + } +} + +// A guard that quietly widened would also be caught by the class constant staying where it belongs. +func TestR403_GuardUsesTheSharedPredicate(t *testing.T) { + // The predicate answers the same for the same directory whichever caller asks — one definition. + dir := r403Unit(t, nil, []string{"v.tar"}, nil) + if unitIsHollow(dir) != !unitCarriesData(dir) { + t.Error("unitIsHollow is not the negation of unitCarriesData") + } + _ = appbackup.UnitManifestFile // the unit-dir-relative helper is what both callers resolve through +} diff --git a/controller/internal/backup/r403_rehydrate_test.go b/controller/internal/backup/r403_rehydrate_test.go new file mode 100644 index 0000000..79a6f90 --- /dev/null +++ b/controller/internal/backup/r403_rehydrate_test.go @@ -0,0 +1,160 @@ +package backup + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +// R-403 Group C — the rehydrate: after a Tier-2 unit restore, the app's own drive gets its package +// back, INSIDE the call. +// +// The cause half. On 2026-08-31 the hollow primary manifest was written TWO SECONDS after a restore +// of exactly this shape, by the 5-minute `backup-cache` job. Anything that runs after the call +// returns races that job; only doing it before returning cannot lose. + +// C1 — a hollow primary is refilled from the mirror that was just restored from. +func TestR403_HollowPrimaryIsRefilledFromTheMirror(t *testing.T) { + f := r102Tier2Fixture(t, []string{"vol_a.tar", "vol_b.tar"}, pgDump(1)) + primaryUnit := RecoveryUnitPath(f.liveDrive, "app") + + // The R-102 scenario: the app's own package is gone. + if err := os.RemoveAll(primaryUnit); err != nil { + t.Fatal(err) + } + if unitCarriesData(primaryUnit) { + t.Fatal("fixture wrong: the primary still carries data") + } + + var copied [][2]string + f.m.unitRehydrate = func(src, dst string) error { + copied = append(copied, [2]string{src, dst}) + return copyTree(src, dst) + } + if _, err := f.m.RestoreTier2Unit("app"); err != nil { + t.Fatalf("restore: %v", err) + } + + if len(copied) != 1 { + t.Fatalf("rehydrate ran %d time(s), want 1: %v", len(copied), copied) + } + mirrorUnit := tier2UnitDir(f.destBase) + if copied[0][0] != mirrorUnit || copied[0][1] != primaryUnit { + t.Errorf("rehydrate copied %v → %v, want %v → %v", copied[0][0], copied[0][1], mirrorUnit, primaryUnit) + } + // THE CONSEQUENCE: the primary is a real package again, so the next capture has nothing hollow to + // describe and the next Tier-2 run has nothing poorer to mirror. + if !unitCarriesData(primaryUnit) { + t.Error("the primary unit is still hollow after the restore — R-403's cause is not closed") + } + for _, rel := range []string{"volume-dumps/vol_a.tar", "volume-dumps/vol_b.tar", "db-dumps/app-postgres.sql"} { + if _, err := os.Stat(filepath.Join(primaryUnit, rel)); err != nil { + t.Errorf("%s did not come back to the app's own drive: %v", rel, err) + } + } +} + +// C2 — TestR403_CompletePrimaryIsLeftByteIdentical. Scenario F, and it is R-403 pointed the other +// way: a primary that already carries data may be NEWER than the mirror, and overwriting it with an +// older copy is the same defect this task exists to remove. +// +// Red-proof (recorded in REPORT.md): drop the `unitCarriesData(primaryUnit)` condition and this +// fails on the fingerprint. +func TestR403_CompletePrimaryIsLeftByteIdentical(t *testing.T) { + f := r102Tier2Fixture(t, []string{"vol_a.tar"}, pgDump(1)) + primaryUnit := RecoveryUnitPath(f.liveDrive, "app") + if !unitCarriesData(primaryUnit) { + t.Fatal("fixture wrong: the primary must start complete") + } + // Make the primary distinguishable from the mirror, so an overwrite would show. + mustWrite(t, filepath.Join(primaryUnit, "volume-dumps", "only-on-the-primary.tar"), "NEWER") + before := fingerprintTree(t, primaryUnit) + + var ran int + f.m.unitRehydrate = func(src, dst string) error { ran++; return copyTree(src, dst) } + if _, err := f.m.RestoreTier2Unit("app"); err != nil { + t.Fatalf("restore: %v", err) + } + + if ran != 0 { + t.Errorf("the rehydrate ran %d time(s) over a COMPLETE primary — it would overwrite newer material", ran) + } + if after := fingerprintTree(t, primaryUnit); after != before { + t.Error("the complete primary unit was modified; it must be byte-identical") + } + if _, err := os.Stat(filepath.Join(primaryUnit, "volume-dumps", "only-on-the-primary.tar")); err != nil { + t.Error("the primary-only tar was destroyed by the rehydrate") + } +} + +// C3 — a FAILED restore writes no package. A package written from a run that did not succeed would +// look like a backup and describe data that never landed. +func TestR403_FailedRestoreDoesNotWriteAPackage(t *testing.T) { + f := r102Tier2Fixture(t, []string{"vol_a.tar"}, pgDump(1)) + primaryUnit := RecoveryUnitPath(f.liveDrive, "app") + if err := os.RemoveAll(primaryUnit); err != nil { + t.Fatal(err) + } + // Make the restore itself fail, after it has started. + f.fake.startSvcErr = errors.New("injected: the database service would not start") + f.m.volumeReplayFrom = func(string, string) (int, error) { + return 0, errors.New("injected: the volume replay failed") + } + + var ran int + f.m.unitRehydrate = func(src, dst string) error { ran++; return copyTree(src, dst) } + if _, err := f.m.RestoreTier2Unit("app"); err == nil { + t.Fatal("the fixture did not make the restore fail") + } + if ran != 0 { + t.Errorf("the rehydrate ran %d time(s) after a FAILED restore", ran) + } + if unitCarriesData(primaryUnit) { + t.Error("a package was written on the app's drive by a restore that failed") + } +} + +// C4 — TestR403_RehydrateHappensBeforeTheCallReturns. Asserted as ORDERING, never as a timer. +// +// The whole requirement is that the 5-minute capture job cannot observe the hollow state. A test +// that slept and then looked would pass on a racing implementation on a fast machine. +func TestR403_RehydrateHappensBeforeTheCallReturns(t *testing.T) { + f := r102Tier2Fixture(t, []string{"vol_a.tar"}, pgDump(1)) + primaryUnit := RecoveryUnitPath(f.liveDrive, "app") + if err := os.RemoveAll(primaryUnit); err != nil { + t.Fatal(err) + } + var rehydrated bool + f.m.unitRehydrate = func(src, dst string) error { rehydrated = true; return copyTree(src, dst) } + + // The observation is taken on the line AFTER the call returns, with nothing waited for. If the + // implementation deferred the work to a goroutine or a scheduled job, this reads false. + _, err := f.m.RestoreTier2Unit("app") + if err != nil { + t.Fatalf("restore: %v", err) + } + if !rehydrated { + t.Fatal("the rehydrate had NOT run when the call returned — a follow-up job races the capture (R-403)") + } + if !unitCarriesData(primaryUnit) { + t.Error("the primary was still hollow at the instant the call returned") + } +} + +// A rehydrate failure must not turn a successful restore into a reported failure — the app is back. +func TestR403_RehydrateFailureDoesNotFailTheRestore(t *testing.T) { + f := r102Tier2Fixture(t, []string{"vol_a.tar"}, pgDump(1)) + if err := os.RemoveAll(RecoveryUnitPath(f.liveDrive, "app")); err != nil { + t.Fatal(err) + } + f.m.unitRehydrate = func(string, string) error { return errors.New("injected: disk full") } + + res, err := f.m.RestoreTier2Unit("app") + if err != nil { + t.Fatalf("a failed rehydrate must not fail the restore: %v", err) + } + if res.VolumesReplayed != 1 { + t.Errorf("the restore's own result was disturbed: %+v", res) + } +} diff --git a/controller/internal/backup/tier2.go b/controller/internal/backup/tier2.go index e608bb8..8b29192 100644 --- a/controller/internal/backup/tier2.go +++ b/controller/internal/backup/tier2.go @@ -365,8 +365,25 @@ func (m *Manager) RunTier2(stackName string) error { } } - // Unit leg (always). - if err := mirror(unitDir, filepath.Join(destBase, "recovery-unit")); err != nil { + // Unit leg — always, EXCEPT the one case R-403 measured (see r403_hollow.go for the mechanism). + // + // `mirror` is `rsync -a --delete`. That is correct for a derived copy and it stays. What was + // missing is the precondition: a source unit that carries NO data must not be mirrored over a + // destination unit that does, because `--delete` then removes the customer's last package. Proven + // on demo-hp 2026-08-31: 120 082 104 B → 7 036 B in one run, reported as a success. + // + // The fence is EXACTLY this shape and no wider. Complete→complete, complete→hollow and + // hollow→hollow all mirror as before; a data leg that legitimately shrinks is untouched (this + // guard is on the unit leg only). §8 row 5's derived-copy rule is unchanged. + destUnit := filepath.Join(destBase, "recovery-unit") + unitLegSkipped := unitIsHollow(unitDir) && unitCarriesData(destUnit) + if unitLegSkipped { + // Loud, and it names the app, the reason and the consequence. Paths and counts only — a unit's + // compose/app.yaml carries portable secrets and nothing from inside it is logged here. + m.logger.Printf("[WARN] [backup] Tier 2 %s: unit leg SKIPPED — the recovery unit on the source drive lists no database dumps and no volume tars, while the existing copy at %s does. The copy was PRESERVED rather than replaced with an empty one (R-403). The other legs continue.", + stackName, destUnit) + warns = append(warns, tier2UnitPreservedWarning) + } else if err := mirror(unitDir, destUnit); err != nil { m.recordTier2Failure(stackName, target, err) if m.tier2Notify != nil { m.tier2Notify(stackName, target.Label, time.Since(start), err) @@ -395,13 +412,19 @@ func (m *Manager) RunTier2(stackName string) error { } dur := time.Since(start) - m.recordTier2Success(stackName, target, mirroredSize, strings.Join(warns, " "), dur) + // R-403: the package date is read from the DESTINATION unit's own manifest, so it describes what + // is actually in the copy whether the leg mirrored or was preserved. Reading the artifact rather + // than assuming the run's own timestamp is what stops the surface calling a preserved package + // fresh — a data loss traded for a comforting lie is not a fix. + m.recordTier2SuccessWithUnit(stackName, target, mirroredSize, strings.Join(warns, " "), dur, + unitLegSkipped, unitPackageDate(destUnit)) if m.tier2Notify != nil { m.tier2Notify(stackName, target.Label, dur, nil) } - m.logger.Printf("[INFO] [backup] Tier 2 copied %s → %s (%s, %d leg(s), %s)%s", + m.logger.Printf("[INFO] [backup] Tier 2 copied %s → %s (%s, %d leg(s), %s)%s%s", stackName, destBase, humanizeBytes(mirroredSize), len(legs), dur.Round(time.Second), - map[bool]string{true: " [SSD: state-only]", false: ""}[target.StateOnly]) + map[bool]string{true: " [SSD: state-only]", false: ""}[target.StateOnly], + map[bool]string{true: " [unit leg SKIPPED — existing package preserved, R-403]", false: ""}[unitLegSkipped]) return nil } @@ -571,9 +594,24 @@ func (m *Manager) tier2Update(stackName string, mutate func(*settings.CrossDrive } } +// recordTier2Success records a completed run for a path that HAS NO UNIT LEG — the shares +// pseudo-stack mirrors share legs and carries no recovery unit, so there is no leg to skip and no +// package whose date could be older than the run. It is the thin caller; ONE implementation lives +// below. func (m *Manager) recordTier2Success(stackName string, target *Tier2Target, sizeBytes int64, warning string, dur time.Duration) { + m.recordTier2SuccessWithUnit(stackName, target, sizeBytes, warning, dur, false, "") +} + +// recordTier2SuccessWithUnit records a completed run INCLUDING what happened to its unit leg. +// `unitLegSkipped` and `unitPkgDate` (R-403) travel with the rest rather than through a second write, +// because two writers to one record is how a status and the thing it describes drift apart. +// `unitPkgDate` is read from the destination unit's OWN manifest, so it is a fact about the copy and +// not about the run. +func (m *Manager) recordTier2SuccessWithUnit(stackName string, target *Tier2Target, sizeBytes int64, warning string, dur time.Duration, unitLegSkipped bool, unitPkgDate string) { now := time.Now().Format(time.RFC3339) m.tier2Update(stackName, func(c *settings.CrossDriveBackup) { + c.UnitLegSkipped = unitLegSkipped + c.UnitPackageDate = unitPkgDate c.Enabled = true c.Method = "rsync" c.DestinationPath = target.NamespaceRoot @@ -652,6 +690,21 @@ func rsyncMirror(src, dst string) error { return nil } +// tier2UnitPreservedWarning is the customer-facing half of the R-403 skip. It rides in +// CrossDriveBackup.LastWarning, which the per-app card already renders, so the refusal reaches the +// SURFACE and not only the log — the shape recordTier2NoTarget established. +const tier2UnitPreservedWarning = "A fő meghajtón lévő adatcsomag hiányos volt, ezért a másodlagos másolatban meglévő, teljes csomagot megőriztük. A másolat adatcsomagja ezért régebbi, mint ez a mentés." + +// unitPackageDate returns the `created_at` of a recovery unit's manifest — WHEN the package in that +// directory was captured. "" when the manifest is absent or unparseable, which the surface must read +// as UNKNOWN and never as "now". +func unitPackageDate(unitDir string) string { + if man := readManifest(UnitManifestFile(unitDir)); man != nil { + return man.CreatedAt + } + return "" +} + // dirSizeBytes returns the total size of a directory via `du -sb` (0 if absent/error). func dirSizeBytes(dir string) int64 { if _, err := os.Stat(dir); err != nil { diff --git a/controller/internal/backup/tier2_restore.go b/controller/internal/backup/tier2_restore.go index 56c3d94..13c44a6 100644 --- a/controller/internal/backup/tier2_restore.go +++ b/controller/internal/backup/tier2_restore.go @@ -81,6 +81,19 @@ type Tier2Coverage struct { // not present it as the other. CopyLastRun string CopyLastSuccess string + + // UnitPackageDate / UnitLegPreserved (R-403) — WHEN the package in this copy was actually + // captured, and whether the newest run PRESERVED it instead of refreshing it. + // + // They exist because after an R-403 skip, CopyLastRun and CopyLastSuccess stop describing the + // package: the run really did succeed and really is from today, and the package in the copy is + // from before it. A surface that names the run date as the package date would be trading a data + // loss for a comforting lie, which is the failure family this project keeps finding. + // + // UnitPackageDate is read from the MIRRORED UNIT'S OWN MANIFEST, not from the recorded status, so + // it is a fact about the artifact the restore will actually open. "" means UNKNOWN. + UnitPackageDate string + UnitLegPreserved bool } // CanRestore reports whether the FILE restore has any subtree to read at all. @@ -126,6 +139,9 @@ func tier2CoverageAt(destBase string) Tier2Coverage { c.HasUnit = true } c.UnitRestorable = tier2UnitIsOpenable(unitDir) + // R-403: ask the package itself when it was made. Reading the artifact rather than the status + // record is what makes this date impossible to overstate. + c.UnitPackageDate = unitPackageDate(unitDir) return c } @@ -144,6 +160,7 @@ func (m *Manager) Tier2RestoreCoverage(stackName string) (Tier2Coverage, error) if m.settings != nil { if cfg := m.settings.GetCrossDriveConfig(stackName); cfg != nil { cov.CopyLastRun, cov.CopyLastSuccess = cfg.LastRun, cfg.LastSuccess + cov.UnitLegPreserved = cfg.UnitLegSkipped } } return cov, nil @@ -178,7 +195,79 @@ func (m *Manager) RestoreTier2Unit(stackName string) (UnitRestoreResult, error) // path and never a secret, and naming it is what makes "the SECONDARY mirror was the source" a // positive observable in the log rather than an absence to be argued from. m.logger.Printf("[WARN] [backup] Tier-2 UNIT restore for %s from the secondary mirror %s — this OVERWRITES live app data", stackName, unitDir) - return m.RestoreFromRecoveryUnitAt(stackName, unitDir) + res, restoreErr := m.RestoreFromRecoveryUnitAt(stackName, unitDir) + + // R-403, the CAUSE half. Refill the primary unit from the mirror we just restored from, INSIDE + // this call, before it returns. + // + // THE TIMING IS THE REQUIREMENT, NOT A DETAIL. On 2026-08-31 the hollow primary manifest was + // written TWO SECONDS after a restore of exactly this shape, by the 5-minute `backup-cache` job + // (`backup.go` → `captureAllRecoveryUnits`). Any follow-up job, scheduled refresh or goroutine + // races that capture and can lose. Doing it here is the only shape that cannot. + // + // The capture itself is NOT guarded and must not be: a capture that describes an empty drive as + // empty is CORRECT. With the primary refilled there is no hollow state left for it to describe, + // which is why the fix is here and not there. Guarding the capture would make the manifest lie. + m.rehydratePrimaryUnit(stackName, unitDir, restoreErr) + return res, restoreErr +} + +// rehydratePrimaryUnit copies a mirrored recovery unit back onto the app's own drive when the primary +// unit is ABSENT or HOLLOW — the state a Tier-2 unit restore leaves behind, and the state that armed +// R-403's delete on the following night. +// +// Three refusals, each earned: +// - the restore FAILED → write nothing. A package written from a run that did not succeed is worse +// than no package: it would look like a backup and describe data that never landed. +// - the primary already CARRIES DATA → leave it byte-identical. It may be NEWER than the mirror +// (the customer restored while their own drive was fine), and overwriting it with an older copy +// is the very move this whole task exists to prevent, pointed the other way. +// - anything goes wrong copying → WARN and carry on. The restore itself succeeded; the app is back. +// Failing the restore because a convenience copy failed would report a success as a failure. +// +// It is best-effort by design and says so in the log either way, because an absent log line is not +// evidence that it ran. +func (m *Manager) rehydratePrimaryUnit(stackName, mirrorUnitDir string, restoreErr error) { + if restoreErr != nil { + m.logger.Printf("[INFO] [backup] %s: primary unit NOT refilled — the restore itself failed (R-403: a package from a failed run is worse than none)", stackName) + return + } + drivePath := m.GetAppDrivePath(stackName) + if drivePath == "" || !filepath.IsAbs(drivePath) { + m.logger.Printf("[WARN] [backup] %s: primary unit NOT refilled — cannot resolve the app's drive", stackName) + return + } + primaryUnit := RecoveryUnitPath(m.namespaceRoot(drivePath), stackName) + if unitCarriesData(primaryUnit) { + m.logger.Printf("[INFO] [backup] %s: primary unit already carries data — left untouched (R-403 never overwrites a richer package with a poorer one)", stackName) + return + } + copier := m.unitRehydrate + if copier == nil { + copier = rsyncMirror + } + if err := copier(mirrorUnitDir, primaryUnit); err != nil { + m.logger.Printf("[ERROR] [backup] %s: refilling the primary unit from the mirror FAILED: %v — the restore itself SUCCEEDED and the app is running; the local package stays incomplete until the next backup", stackName, err) + return + } + m.logger.Printf("[INFO] [backup] %s: primary unit refilled from the secondary mirror (R-403) — %d volume tar(s), %d database dump(s) now on the app's own drive", + stackName, countUnitFiles(UnitVolumeDumpDir(primaryUnit), ".tar"), countUnitFiles(UnitDBDumpDir(primaryUnit), ".sql")) +} + +// countUnitFiles counts files with a suffix in a unit leg directory — for the log line only, so the +// refill states WHAT it put back rather than merely that it ran. Never a secret: counts, not names. +func countUnitFiles(dir, suffix string) int { + entries, err := os.ReadDir(dir) + if err != nil { + return 0 + } + n := 0 + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), suffix) { + n++ + } + } + return n } // Tier2CopyDate returns the date the surface should name for this app's Tier-2 copy, preferring the @@ -194,6 +283,21 @@ func (c Tier2Coverage) Tier2CopyDate() (date string, proven bool) { return c.CopyLastRun, false } +// UnitRestoreDate returns the date of the PACKAGE the unit restore would actually open, and whether +// that package is OLDER than the copy's newest run. +// +// R-403. `Tier2CopyDate` answers "when was this copy last written to" and is right for the file +// restore, whose legs really were refreshed by that run. It is the WRONG answer for the unit restore +// after a preserved leg, because the package is then older than the run that reports success. This +// asks the manifest first and falls back to the copy date only when the package cannot say. +func (c Tier2Coverage) UnitRestoreDate() (date string, olderThanTheRun bool) { + copyDate, _ := c.Tier2CopyDate() + if c.UnitPackageDate == "" { + return copyDate, c.UnitLegPreserved + } + return c.UnitPackageDate, c.UnitLegPreserved || (copyDate != "" && c.UnitPackageDate < copyDate) +} + // tier2RecordedCopyDir resolves the RECORDED Tier-2 copy dir for a stack, applying every // source-side refusal in one place so the pre-flight check and the restore itself cannot drift. func (m *Manager) tier2RecordedCopyDir(stackName string) (string, error) { diff --git a/controller/internal/backup/tier2_shares.go b/controller/internal/backup/tier2_shares.go index b84a420..2ee21c6 100644 --- a/controller/internal/backup/tier2_shares.go +++ b/controller/internal/backup/tier2_shares.go @@ -249,6 +249,8 @@ func (m *Manager) RunSharesTier2() error { if len(noTargetWhy) > 0 { warns = append(warns, "Néhány meghajtón lévő megosztásnak nincs másodlagos célja: "+strings.Join(noTargetWhy, "; ")) } + // The no-unit-leg wrapper, deliberately: shares carry no recovery unit, so R-403's guard has + // nothing to guard here (see recordTier2Success). m.recordTier2Success(SharesPseudoStack, lastTarget, totalSize, strings.Join(warns, " "), dur) if m.tier2Notify != nil { m.tier2Notify(DisplayStackName(SharesPseudoStack), lastTarget.Label, dur, nil) diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index cb0edee..293dbbb 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -461,6 +461,13 @@ type CrossDriveBackup struct { // 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) + // 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 + // the RUN, and after a skip they no longer describe the PACKAGE. UnitPackageDate is read from the + // 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" diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index ace7bd6..39eb16b 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -1203,6 +1203,9 @@ type AppBackupRow struct { // Tier2UnitConfirm is the assembled destructive-confirm sentence (tier2UnitConfirmMsg). Built in // Go, not in the attribute, so it is one named string a test can assert verbatim. Tier2UnitConfirm string + // Tier2UnitStaleNotice (R-403) — non-empty when the newest run PRESERVED this copy's package + // instead of refreshing it, so the card cannot render a preserved package as a fresh one. + Tier2UnitStaleNotice string // Drive disconnected — app's home drive is currently disconnected DriveDisconnected bool @@ -1428,8 +1431,14 @@ func (s *Server) buildAppBackupRows(status *backup.FullBackupStatus) []AppBackup // row keeps rendering everything else it already showed. if cov, covErr := s.backupMgr.Tier2RestoreCoverage(app.StackName); covErr == nil { row.Tier2UnitRestorable = cov.CanRestoreUnit() - row.Tier2CopyDate, row.Tier2CopyDateProven = cov.Tier2CopyDate() - row.Tier2UnitConfirm = tier2UnitConfirmMsg(row.Tier2CopyDate, row.Tier2CopyDateProven) + // R-403: the UNIT action names the PACKAGE's date, not the run's. After a + // preserved leg those are different dates and the run's is the flattering one. + pkgDate, stale := cov.UnitRestoreDate() + row.Tier2CopyDate, row.Tier2CopyDateProven = pkgDate, cov.CopyLastSuccess != "" + row.Tier2UnitConfirm = tier2UnitConfirmWithStaleness(pkgDate, row.Tier2CopyDateProven, stale) + if stale && pkgDate != "" { + row.Tier2UnitStaleNotice = fmt.Sprintf(tier2UnitStaleNoticeFmt, fmtRFC3339Local(pkgDate)) + } } switch cd.LastStatus { case "ok": @@ -1669,6 +1678,16 @@ const ( tier2UnitConfirmDateUnprovenFmt = " A másolat kelte: %s – ez az utolsó mentési kísérlet ideje, azt nem tudjuk igazolni, hogy sikeres volt." tier2UnitConfirmContrast = " A mellette lévő „Fájlok visszaállítása” ezzel szemben csak a hiányzó fájlokat pótolja, és semmit nem ír felül. Az alkalmazás a művelet idejére leáll." + + // tier2UnitStaleClause (R-403) — the package in this copy is OLDER than the copy's newest run, + // because that run PRESERVED it rather than replacing it with an empty one. Without this the + // confirm would name a date the customer reads as "last night" over a package from before it. + // The whole point of preserving the copy is lost if the surface then misdescribes what it kept. + tier2UnitStaleClause = " FIGYELEM: ennek a másolatnak az adatcsomagja régebbi, mint a legutóbbi mentés — a fő meghajtón lévő csomag hiányos volt, ezért a meglévő, teljes másolatot megőriztük. A visszaállítás a fent megadott csomagot használja." + + // tier2UnitStaleNoticeFmt (R-403) — the same fact on the per-app backup card, where the customer + // looks BEFORE deciding anything. %s is the package's own date. + tier2UnitStaleNoticeFmt = "A másolat adatcsomagja régebbi, mint a legutóbbi mentés (%s): a fő meghajtón lévő csomag hiányos volt, ezért a meglévő, teljes másolatot megőriztük." ) // tier2UnitConfirmMsg assembles the destructive confirm for one app's Tier-2 unit restore. Pure, so @@ -1677,6 +1696,13 @@ const ( // A copy with no recorded date at all still gets a confirm — it just cannot name one. Dropping the // whole confirm because a date is missing would remove the warning and keep the destruction. func tier2UnitConfirmMsg(copyDate string, proven bool) string { + return tier2UnitConfirmWithStaleness(copyDate, proven, false) +} + +// tier2UnitConfirmWithStaleness is tier2UnitConfirmMsg for a copy whose PACKAGE may be older than its +// newest run (R-403). ONE implementation, two callers — the two-argument form above is the ordinary +// case where the run really did refresh the package. +func tier2UnitConfirmWithStaleness(copyDate string, proven bool, stale bool) string { msg := tier2UnitConfirmBase if copyDate != "" { if proven { @@ -1685,7 +1711,12 @@ func tier2UnitConfirmMsg(copyDate string, proven bool) string { msg += fmt.Sprintf(tier2UnitConfirmDateUnprovenFmt, fmtRFC3339Local(copyDate)) } } - return msg + tier2UnitConfirmContrast + msg += tier2UnitConfirmContrast + // R-403 last, so it is the sentence the customer is left holding before they press. + if stale { + msg += tier2UnitStaleClause + } + return msg } // tier2UnitSourceMsg renders the "which copy" clause for the Tier-2 unit restore's outcome, or "" if diff --git a/controller/internal/web/r403_surface_test.go b/controller/internal/web/r403_surface_test.go new file mode 100644 index 0000000..c8be60d --- /dev/null +++ b/controller/internal/web/r403_surface_test.go @@ -0,0 +1,101 @@ +package web + +import ( + "strings" + "testing" +) + +// R-403 Group D — the surfaces must not call a PRESERVED package a FRESH one. +// +// The guard keeps the customer's data. That gain is thrown away if the page then reports the run's +// own timestamp as the package's date: the customer would restore a week-old package believing it was +// last night's. Trading a data loss for a comforting lie is the failure family this project keeps +// finding, and it is not a fix. +// +// Every assertion compares against the NAMED CONSTANT rather than a Hungarian literal retyped here +// (R-364): a re-typed accented string can differ from the shipped one by a character nobody sees. + +// D1 — TestR403_SkippedUnitLegIsNotRenderedAsFresh. +func TestR403_SkippedUnitLegIsNotRenderedAsFresh(t *testing.T) { + const runDate = "2026-08-31T03:30:00Z" + const pkgDate = "2026-08-25T03:30:00Z" + + stale := r103Row(true, pkgDate, true) + stale.Tier2LastRun, stale.Tier2LastSuccess = runDate, runDate + stale.Tier2UnitStaleNotice = staleNoticeFor(pkgDate) + + html := renderBackupPage(t, "backups_apps", baseBackupData([]AppBackupRow{stale})) + + if !strings.Contains(html, stale.Tier2UnitStaleNotice) { + t.Error("the preserved-package notice is not on the page — a preserved copy renders as a fresh one") + } + // The PACKAGE's date is shown, not only the run's. + if !strings.Contains(html, fmtRFC3339Local(pkgDate)) { + t.Errorf("the package's own date %q is not on the page", fmtRFC3339Local(pkgDate)) + } + + // NEGATIVE CONTROL: an ordinary row must NOT carry the notice, or D1 would pass on a page that + // shows the warning to everybody. + fresh := r103Row(true, runDate, true) + freshHTML := renderBackupPage(t, "backups_apps", baseBackupData([]AppBackupRow{fresh})) + if strings.Contains(freshHTML, staleNoticeFor(pkgDate)) { + t.Error("an ordinary row carried the preserved-package notice") + } + if !strings.Contains(freshHTML, "/backup/tier2/unit-restore") { + t.Fatal("the ordinary row did not render at all — the negative control proves nothing") + } +} + +func staleNoticeFor(pkgDate string) string { + return strings.Replace(tier2UnitStaleNoticeFmt, "%s", fmtRFC3339Local(pkgDate), 1) +} + +// D2 — TestR403_UnitRestoreOfferNamesTheOlderPackageDate. +// +// The confirm is the last thing between the customer and an overwrite of their live data. After a +// preserved leg it must name the PACKAGE's date and say why it is older than the copy's newest run. +func TestR403_UnitRestoreOfferNamesTheOlderPackageDate(t *testing.T) { + const pkgDate = "2026-08-25T03:30:00Z" + staleConfirm := tier2UnitConfirmWithStaleness(pkgDate, true, true) + freshConfirm := tier2UnitConfirmWithStaleness(pkgDate, true, false) + + if !strings.Contains(staleConfirm, tier2UnitStaleClause) { + t.Error("the confirm does not say the package is older than the newest run") + } + if !strings.Contains(staleConfirm, fmtRFC3339Local(pkgDate)) { + t.Error("the confirm does not name the package's date") + } + // Everything the ordinary confirm promised is still promised. + if !strings.Contains(staleConfirm, tier2UnitConfirmBase) || !strings.Contains(staleConfirm, tier2UnitConfirmContrast) { + t.Error("the stale confirm lost the overwrite warning or the additive contrast") + } + // NEGATIVE CONTROL: the ordinary confirm must NOT carry the clause. + if strings.Contains(freshConfirm, tier2UnitStaleClause) { + t.Error("an ordinary confirm carried the preserved-package clause") + } + if staleConfirm == freshConfirm { + t.Error("a preserved package and a fresh one produced the SAME confirm") + } + + // And it reaches the rendered markup, not only the constant. + row := r103Row(true, pkgDate, true) + row.Tier2UnitConfirm = staleConfirm + html := renderBackupPage(t, "backups_apps", baseBackupData([]AppBackupRow{row})) + if !strings.Contains(html, "FIGYELEM") { // ASCII-only fragment, R-364 + t.Error("the stale clause never reached the page") + } + // The ASCII control: the same page WITHOUT the clause must not match. + rowFresh := r103Row(true, pkgDate, true) + freshHTML := renderBackupPage(t, "backups_apps", baseBackupData([]AppBackupRow{rowFresh})) + if strings.Contains(freshHTML, "FIGYELEM") { + t.Error("the ASCII fragment matches a page that has no stale clause — the control fails") + } +} + +// The two-argument wrapper still produces the ordinary confirm — yesterday's callers are unchanged. +func TestR403_TheOrdinaryConfirmIsUnchanged(t *testing.T) { + const d = "2026-08-25T03:30:00Z" + if tier2UnitConfirmMsg(d, true) != tier2UnitConfirmWithStaleness(d, true, false) { + t.Error("the two-argument confirm is no longer the not-stale case") + } +} diff --git a/controller/internal/web/templates/backups_apps.html b/controller/internal/web/templates/backups_apps.html index 5e4a560..2ff569f 100644 --- a/controller/internal/web/templates/backups_apps.html +++ b/controller/internal/web/templates/backups_apps.html @@ -245,6 +245,10 @@ {{end}} {{if .Tier2SizeHuman}}{{.Tier2SizeHuman}}{{end}} {{if .Tier2LastWarning}}{{.Tier2LastWarning}}{{end}} + {{/* R-403: a run that PRESERVED the copy's package instead of refreshing it must + not render as a plain fresh copy. The status line above is about the RUN; + this one is about the PACKAGE, and after a preserved leg they differ. */}} + {{if .Tier2UnitStaleNotice}}{{.Tier2UnitStaleNotice}}{{end}} {{.BackupContents}}