From 2e936f43bf906632bd1bc4adeaa005f6856217ed Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 5 Aug 2026 07:17:20 +0200 Subject: [PATCH] R-204 item 3: a restore says what it restored, and what it did not (v0.198.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mode=unit restores the recovery unit — the app's definition, configuration and database dumps — and NOT the customer's own files: RestoreOffboxScratch passes --include and the userdata in the same snapshot is excluded by it. The outcome was one sentence for both modes and named neither scope, so on the last step of a disaster recovery the customer was told the app had been restored after the thing they were looking for had not been. restoreScratchOutcomeMsg states what came back, what did not, and the next step that gets it. The wizard's intent card states its scope before the choice. The full-restore size gate is untouched and pinned as unchanged; the default stays unit, since all three wizard forms set mode explicitly. --- controller/internal/web/offbox_handlers.go | 35 ++- .../internal/web/offbox_restore_scope_test.go | 239 ++++++++++++++++++ .../internal/web/restore_wizard_test.go | 6 +- .../web/templates/backups_restore_wizard.html | 8 +- 4 files changed, 280 insertions(+), 8 deletions(-) create mode 100644 controller/internal/web/offbox_restore_scope_test.go diff --git a/controller/internal/web/offbox_handlers.go b/controller/internal/web/offbox_handlers.go index e8032ec..a423748 100644 --- a/controller/internal/web/offbox_handlers.go +++ b/controller/internal/web/offbox_handlers.go @@ -345,15 +345,40 @@ func (s *Server) offboxRestoreHandler(w http.ResponseWriter, r *http.Request) { // customer had no way to look at what they had just asked for. Resolve the real path and say // it. Fall back to the vague wording only if the path can no longer be resolved. where := s.backupMgr.OffsiteRestoreScratchPath(app) - msg := "A(z) " + app + " visszaállítva ellenőrző mappába a meghajtón (a meglévő adatok változatlanok)." - if where != "" { - msg = "A(z) " + app + " visszaállítva ellenőrző mappába: " + where + " (a meglévő adatok változatlanok)." - } - s.backupMgr.EndRestoreOp(true, msg) + s.backupMgr.EndRestoreOp(true, restoreScratchOutcomeMsg(app, where, full)) }() offboxRedirectTo(w, r, restoreWizardPath(app), "A távoli visszaállítás elindult — az állapot itt frissül.", false) } +// restoreScratchOutcomeMsg builds the OUTCOME flash for a completed scratch restore. Pure, so the +// wording is unit-testable — this string is the customer's only evidence of WHAT they now have. +// +// R-204 item 3 (v0.198.0) — THE DEFECT IT CLOSES. The default restore (`mode=unit`) recovers the +// recovery unit: the app's definition, its configuration and its database dumps. It does NOT recover +// the customer's own files; `RestoreOffboxScratch` passes `--include ` and the userdata +// paths that ARE in the same snapshot are excluded by it. The old message was one sentence for both +// modes and named neither scope, so a customer on the last step of a disaster recovery was told +// „visszaállítva" after the thing they were looking for had not been restored. A success message +// that does not name its scope is a silent wrong answer, which is this project's most-repeated +// failure shape. +// +// So the unit case states three things in order: what came back, what did NOT, and the next step +// that gets it. The full case says the files came with it, because otherwise the absence of the +// warning would be the only difference and an absence is not a statement. +func restoreScratchOutcomeMsg(app, where string, full bool) string { + at := " ellenőrző mappába" + if where != "" { + at = " ellenőrző mappába: " + where + } + if full { + return "A(z) " + app + " teljes mentése visszaállítva" + at + + " — a saját fájljaiddal együtt. A meglévő adatok változatlanok." + } + return "A(z) " + app + " beállításai és adatbázisa visszaállítva" + at + + ". A saját fájljaid (dokumentumok, képek, feltöltések) NEM kerültek vissza — ez az ellenőrző visszaállítás csak az alkalmazás beállításait és adatbázisát hozza vissza. " + + "Ha a fájljaidra van szükséged, indítsd el a „Teljes visszaállítás előkészítése” lépést ezen az oldalon. A meglévő adatok változatlanok." +} + // offboxReconstituteHandler is the TRUE offsite restore (R-43, v0.148.0): files overwritten to the // snapshot's version + that same snapshot's database replayed + the app restarted, with a safety // dump of the current database taken first. diff --git a/controller/internal/web/offbox_restore_scope_test.go b/controller/internal/web/offbox_restore_scope_test.go new file mode 100644 index 0000000..66f4395 --- /dev/null +++ b/controller/internal/web/offbox_restore_scope_test.go @@ -0,0 +1,239 @@ +package web + +import ( + "context" + "encoding/json" + "io" + "log" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-204 item 3 — a restore must say WHAT IT RESTORED, and for the default mode, what it did not. +// +// THESE TESTS DRIVE THE REAL HANDLER, not the message helper (task §10: a test that reaches a helper +// while the mutation lives in the handler cannot observe it). offboxRestoreHandler runs, the restic +// exec is the only thing stubbed, and the assertion is on the flash the customer actually receives — +// backupMgr.RestoreStatus().Last.Message, the same field the wizard renders. + +// scopeRunner is the restic exec seam. It answers the three calls a scratch restore makes +// (snapshots / unlock / restore) and RECORDS the restore argv, so Scenario F can assert the +// unit-vs-full distinction is still carried where it matters. +type scopeRunner struct { + mu sync.Mutex + unitPath string + restoreArgs []string + sizeBytes int64 +} + +func (sr *scopeRunner) run(_ context.Context, _ []string, args ...string) ([]byte, error) { + sr.mu.Lock() + defer sr.mu.Unlock() + joined := strings.Join(args, " ") + switch { + case strings.Contains(joined, " snapshots ") || strings.HasSuffix(joined, " snapshots"): + out, _ := json.Marshal([]map[string]any{{ + "short_id": "abc1234", + "id": "abc1234deadbeef", + "time": time.Now().UTC().Format(time.RFC3339), + // The snapshot holds BOTH the recovery unit and the customer's userdata — which is the + // whole point: a unit restore leaves the second one behind. + "paths": []string{sr.unitPath, filepath.Dir(filepath.Dir(sr.unitPath)) + "/userdata/immich"}, + }}) + return out, nil + case strings.Contains(joined, " stats "): + out, _ := json.Marshal(map[string]any{"total_size": sr.sizeBytes}) + return out, nil + case strings.Contains(joined, " restore "): + sr.restoreArgs = append([]string{}, args...) + return []byte("restored"), nil + } + return []byte(""), nil // unlock and anything else: a clean no-op +} + +func (sr *scopeRunner) lastRestoreArgs() []string { + sr.mu.Lock() + defer sr.mu.Unlock() + return append([]string{}, sr.restoreArgs...) +} + +// scopeServer wires a Server with a configured offbox manager whose restic exec is the stub above. +// The drive is a real temp dir registered as schedulable, so the scratch path resolves for real. +func scopeServer(t *testing.T) (*Server, *backup.Manager, *scopeRunner) { + t.Helper() + lg := log.New(io.Discard, "", 0) + dir := t.TempDir() + drive := filepath.Join(dir, "usb") + if err := os.MkdirAll(drive, 0o755); err != nil { + t.Fatal(err) + } + cfg := &config.Config{} + cfg.Paths.DataDir = filepath.Join(dir, "data") + cfg.Paths.SystemDataPath = filepath.Join(dir, "sys") + cfg.Paths.StacksDir = filepath.Join(dir, "stacks") + cfg.Web.SessionSecret = "test-session-secret-abcdef" + + sett, err := settings.Load(filepath.Join(dir, "settings.json"), lg) + if err != nil { + t.Fatal(err) + } + if err := sett.AddStoragePath(settings.StoragePath{Path: drive, Label: "USB", Schedulable: true}); err != nil { + t.Fatal(err) + } + if err := sett.SetOffboxTarget(&settings.OffboxTarget{ + Enabled: true, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo", + Schedule: "daily", EscrowState: "escrowed", + }); err != nil { + t.Fatal(err) + } + m := backup.NewManager(cfg, sett, lg) + if err := m.WriteOffboxSecrets("PRIVATE-KEY-MATERIAL", "nas.local ssh-ed25519 AAAAhostkey"); err != nil { + t.Fatal(err) + } + if !m.OffboxConfigured() { + t.Fatal("offbox target not configured — the handler would refuse before reaching the outcome") + } + sr := &scopeRunner{ + // The unit path as it appears INSIDE the snapshot: /backups/primary/. + unitPath: strings.TrimSuffix(m.OffsiteRestoreScratchPath("immich"), "/backups/offsite-restore/immich") + "/backups/primary/immich", + sizeBytes: 4 << 20, // 4 MiB — comfortably inside the headroom of a temp dir + } + m.SetOffboxRunner(sr.run) + + s := &Server{cfg: cfg, settings: sett, backupMgr: m, logger: lg, version: "test"} + s.loadTemplates() + return s, m, sr +} + +// postRestore drives the REAL handler and waits for the async restore to finish. +func postRestore(t *testing.T, s *Server, m *backup.Manager, form url.Values) backup.RestoreOpStatus { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/backup/offbox/restore", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rr := httptest.NewRecorder() + s.offboxRestoreHandler(rr, req) + + // Wait on a REAL completion marker (a finished Last with a FinishedAt), never a fixed sleep. + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + st := m.RestoreStatus() + if !st.Running && st.Last != nil && !st.Last.FinishedAt.IsZero() { + return st + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("restore did not finish within the deadline (status=%+v)", m.RestoreStatus()) + return backup.RestoreOpStatus{} +} + +// SCENARIO E — the DEFAULT (unit) restore's outcome names what it did NOT restore, and the next step. +// +// RED-PROOF: delete the „NEM kerültek vissza" sentence from restoreScratchOutcomeMsg (or revert the +// function to the single pre-R-204 sentence). The handler still succeeds and still flashes a +// „visszaállítva" message — and this test fails, which is exactly the silence being closed. +func TestOffboxRestore_UnitOutcomeNamesWhatItDidNotRestore(t *testing.T) { + s, m, _ := scopeServer(t) + + st := postRestore(t, s, m, url.Values{"app": {"immich"}, "mode": {"unit"}}) + if !st.Last.OK { + t.Fatalf("unit restore failed: %q", st.Last.Message) + } + msg := st.Last.Message + + // It must name what CAME BACK… + for _, want := range []string{"immich", "be" + "állításai és adatbázisa visszaállítva"} { + if !strings.Contains(msg, want) { + t.Errorf("outcome does not state what was restored (missing %q): %q", want, msg) + } + } + // …and, the point of R-204 item 3, what did NOT. + if !strings.Contains(msg, "NEM kerültek vissza") { + t.Errorf("outcome does not state that the customer's own files were NOT restored: %q", msg) + } + if !strings.Contains(msg, "dokumentumok") { + t.Errorf("outcome does not name the files it left behind: %q", msg) + } + // …and the next step that actually gets them. + if !strings.Contains(msg, "Teljes vissza"+"állítás előkészítése") { + t.Errorf("outcome does not name the next step that returns the files: %q", msg) + } + // The scratch path is still named (the v0.147.0 4a guarantee must not regress). + if !strings.Contains(msg, m.OffsiteRestoreScratchPath("immich")) { + t.Errorf("outcome no longer names the folder it restored into: %q", msg) + } +} + +// The DEFAULT is `unit` (mode absent) and it must produce the SAME scoped outcome — the wizard always +// sets mode, but a mode-less POST must not fall into a message that overstates what it did. +func TestOffboxRestore_DefaultModeGetsTheScopedOutcome(t *testing.T) { + s, m, _ := scopeServer(t) + st := postRestore(t, s, m, url.Values{"app": {"immich"}}) // no mode at all + if !strings.Contains(st.Last.Message, "NEM kerültek vissza") { + t.Fatalf("the DEFAULT restore did not state its scope: %q", st.Last.Message) + } +} + +// SCENARIO F — the full restore is unchanged: still two-step and size-gated, and its outcome does NOT +// carry the unit warning (a full restore did bring the files). +func TestOffboxRestore_FullPathUnchanged(t *testing.T) { + s, m, sr := scopeServer(t) + + // Step 1: mode=full WITHOUT confirm must NOT restore — it computes and redirects with the reveal. + req := httptest.NewRequest(http.MethodPost, "/backup/offbox/restore", + strings.NewReader(url.Values{"app": {"immich"}, "mode": {"full"}}.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rr := httptest.NewRecorder() + s.offboxRestoreHandler(rr, req) + if rr.Code != http.StatusFound { + t.Fatalf("full step 1: want a redirect, got %d", rr.Code) + } + loc := rr.Header().Get("Location") + if !strings.Contains(loc, "full_prep=immich") || !strings.Contains(loc, "full_size=") { + t.Fatalf("full step 1 did not reveal the size gate: Location=%q", loc) + } + if len(sr.lastRestoreArgs()) != 0 { + t.Fatal("full step 1 ran a restore before the customer confirmed — the size gate is bypassed") + } + + // Step 2: the revealed confirm executes, and the outcome says the files came with it. + st := postRestore(t, s, m, url.Values{"app": {"immich"}, "mode": {"full"}, "confirm": {"1"}}) + if !st.Last.OK { + t.Fatalf("full restore failed: %q", st.Last.Message) + } + if strings.Contains(st.Last.Message, "NEM kerültek vissza") { + t.Fatalf("the FULL restore wrongly claims the files were left behind: %q", st.Last.Message) + } + if !strings.Contains(st.Last.Message, "saját fájljaiddal együtt") { + t.Fatalf("the full outcome does not state that the files came with it: %q", st.Last.Message) + } + // And the mechanism that makes the two modes differ is still carried: unit passes --include, full + // does not. Asserted on the REAL argv the manager built. + if args := sr.lastRestoreArgs(); strings.Contains(strings.Join(args, " "), "--include") { + t.Fatalf("a FULL restore must not restrict to the unit: %v", args) + } +} + +// The unit restore's mechanism half: it DOES restrict to the unit path. Without this, Scenario E's +// message could be true today and quietly become a lie if --include were dropped. +func TestOffboxRestore_UnitRestrictsToTheUnitPath(t *testing.T) { + s, m, sr := scopeServer(t) + postRestore(t, s, m, url.Values{"app": {"immich"}, "mode": {"unit"}}) + joined := strings.Join(sr.lastRestoreArgs(), " ") + if !strings.Contains(joined, "--include") { + t.Fatalf("a unit restore must restrict to the unit path, argv=%q", joined) + } + if !strings.Contains(joined, "/backups/primary/immich") { + t.Fatalf("a unit restore did not include the recovery unit path, argv=%q", joined) + } +} diff --git a/controller/internal/web/restore_wizard_test.go b/controller/internal/web/restore_wizard_test.go index 9ea13cc..900b055 100644 --- a/controller/internal/web/restore_wizard_test.go +++ b/controller/internal/web/restore_wizard_test.go @@ -192,7 +192,11 @@ func TestRestoreWizard_ThreeIntentCards(t *testing.T) { // Each intent must state its CONSEQUENCE, not just its name. for _, want := range []string{ "Ellenőrzés külön mappába", - "az élő adataid nem változnak", + "Az élő adataid nem változnak", + // R-204 item 3: intent 1's SCOPE is stated before the choice, not only in the outcome. A + // disaster-recovery customer picked this card expecting their documents; it does not return + // them. If this substring goes, the card is back to promising „a mentés tartalma". + "A saját fájljaidat (dokumentumok, képek, feltöltések) nem hozza vissza", "Hiányzó fájlok visszahozása", "törölt tartalom ettől nem jelenik meg újra", "Teljes visszaállítás (fájlok + adatbázis)", diff --git a/controller/internal/web/templates/backups_restore_wizard.html b/controller/internal/web/templates/backups_restore_wizard.html index ccf2728..1886b2f 100644 --- a/controller/internal/web/templates/backups_restore_wizard.html +++ b/controller/internal/web/templates/backups_restore_wizard.html @@ -74,8 +74,12 @@ harmless first, irreversible-looking last. -->
-

1. Ellenőrzés külön mappába

-

A mentés tartalma egy külön ellenőrző mappába kerül — az élő adataid nem változnak.

+

1. Ellenőrzés külön mappába (beállítások és adatbázis)

+ +

Az alkalmazás beállításait és adatbázisát hozza vissza egy külön ellenőrző mappába. A saját fájljaidat (dokumentumok, képek, feltöltések) nem hozza vissza — azokhoz a 3. pont teljes visszaállítása kell. Az élő adataid nem változnak.

{{.CSRFField}}