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}}