Files
felhom-controller/controller/internal/web/restore_wizard_test.go
T
admin 3a9d744360 v0.154.0 — R-48: one restore entry per app, and the intent is a described choice
The offsite restore list rendered up to five inline forms per app row. Two of them —
„Helyreállítás az élő adatok közé (csak a hiányzó fájlok)" and „Teljes visszaállítás
(fájlok + adatbázis)" — were sibling buttons whose difference is whether the customer's
data comes back at all. That mis-selection CAUSED the round-2 incident: an operator who
had read the source pressed the missing-only button and /backup/offbox/reconstitute was
never hit (DIAG-immich-restore-round2-2026-07-19, finding 1).

The rule this establishes: two adjacent controls whose difference is "your data comes
back" vs "your data cannot come back" must never be distinguishable only by layout.

Each row now carries ONE „Visszaállítás…" entry linking to a per-app wizard at
GET /backups/restore/app?name=<app>: three intent CARDS with consequence sentences, a
visible phase strip, danger styling plus the R-43 double-confirm carried over verbatim on
the destructive one, and the pair-honesty panel bound to real OffsiteScratchPair facts.

deriveWizardStep is pure over (op running, size-gate flash, scratch ready); the step is
never accepted from the request, and a running op outranks a stale ?full_prep= so no
commit button survives into a restore. While ANY op runs every mutation form is suppressed
server-side instead of being offered and refused.

No new mutation endpoint: every card posts to the pre-existing /backup/offbox/* handler
with the same field names and gates, and the page works with JavaScript disabled.
internal/{backup,appbackup,selfupdate} untouched. R-45's job registry stays its own item.

Fixes a latent bug found on the way: offboxRedirectTo hardcoded "?" when appending its
flash, which against the wizard's ?name=<app> target would have buried the flash inside
the name value.

No new agent coupling — MinAgent stays 0.90.0.
2026-07-21 08:30:42 +02:00

325 lines
14 KiB
Go

package web
import (
"net/http/httptest"
"regexp"
"sort"
"strings"
"testing"
"time"
"gitea.dooplex.hu/admin/felhom-controller/internal/backup"
)
// R-48 — the offsite restore wizard.
//
// What these tests actually guard is the finding's RULE, not the page: two adjacent controls whose
// difference is "your data comes back" vs "your data cannot come back" must not be distinguishable
// only by layout. Scenario A is therefore asserted STRUCTURALLY (the mutation forms are absent from
// the list page and present only in the wizard), not by eyeballing copy.
// wizardData builds the wizard template's data map. Mirrors what backupsRestoreWizardHandler puts
// there — the handler's own construction is exercised separately by the redirect tests.
func wizardData(app string, view restoreWizardView, pair backup.OffsitePairInfo) map[string]interface{} {
return map[string]interface{}{
"Page": "backups-restore", "Title": "Visszaállítás",
"Backup": &backup.FullBackupStatus{},
"App": app,
"AppDisplayName": strings.ToUpper(app[:1]) + app[1:],
"AppSlug": app,
"Wizard": view,
"Pair": pair,
"FullPrepSize": "1,2 GB",
"RunningStack": "",
}
}
func renderWizard(t *testing.T, data map[string]interface{}) string {
t.Helper()
return renderBackupPage(t, "backups_restore_wizard", data)
}
// --- Group B (Scenario B): the step derivation is server truth, and PURE ---------------------------
//
// COMPANION RED-PROOF (run + recorded in REPORT.md): replace the body of deriveWizardStep with the
// trivial `return restoreWizardView{Step: wizStepIntent, VerifyEnabled: true}` — the op-running,
// prepare-confirm and scratch-ready rows all FAIL. Restore → green.
func TestDeriveWizardStep_Table(t *testing.T) {
cases := []struct {
name string
in restoreWizardInput
want restoreWizardView
}{
{
name: "no scratch, no op → intent; only verification is offered, full restore must be prepared first",
in: restoreWizardInput{App: "immich"},
want: restoreWizardView{Step: wizStepIntent, VerifyEnabled: true, PrepareEnabled: true},
},
{
name: "scratch ready → intent, and BOTH data-touching intents unlock; preparation is done",
in: restoreWizardInput{App: "immich", ScratchReady: true},
want: restoreWizardView{Step: wizStepIntent, VerifyEnabled: true, PlaceEnabled: true, RestoreEnabled: true},
},
{
name: "full_prep flash for THIS app → prepare-confirm; only the commit is offered",
in: restoreWizardInput{App: "immich", FullPrepApp: "immich"},
want: restoreWizardView{Step: wizStepPrepareConfirm, CommitPrepareEnabled: true},
},
{
name: "full_prep flash for ANOTHER app → this app keeps its own intent step",
in: restoreWizardInput{App: "immich", FullPrepApp: "bookstack"},
want: restoreWizardView{Step: wizStepIntent, VerifyEnabled: true, PrepareEnabled: true},
},
{
name: "op running (this app) → execution; nothing offered",
in: restoreWizardInput{App: "immich", OpRunning: true},
want: restoreWizardView{Step: wizStepExecution},
},
{
name: "op running for ANOTHER app still suppresses THIS app (the single-flight is process-wide)",
in: restoreWizardInput{App: "immich", OpRunning: true, ScratchReady: true},
want: restoreWizardView{Step: wizStepExecution},
},
{
name: "op running OUTRANKS a stale full_prep flash — no commit button mid-restore",
in: restoreWizardInput{App: "immich", OpRunning: true, FullPrepApp: "immich"},
want: restoreWizardView{Step: wizStepExecution},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := deriveWizardStep(tc.in)
if got != tc.want {
t.Errorf("deriveWizardStep(%+v)\n got %+v\n want %+v", tc.in, got, tc.want)
}
})
}
}
// --- Group C (Scenario B error rows): refusals resolve, never 500 ----------------------------------
func TestResolveWizardApp_Refusals(t *testing.T) {
rows := []OffboxAppRow{
{Name: "immich", DisplayName: "Immich", Enabled: true},
{Name: "radarr", DisplayName: "Radarr", Enabled: false},
}
if got := resolveWizardApp(rows, "immich"); got == nil || got.DisplayName != "Immich" {
t.Fatalf("toggled app must resolve, got %+v", got)
}
if got := resolveWizardApp(rows, "radarr"); got != nil {
t.Errorf("an app that is NOT toggled for offsite has no snapshot to restore from — want nil, got %+v", got)
}
if got := resolveWizardApp(rows, "does-not-exist"); got != nil {
t.Errorf("unknown app must not resolve, got %+v", got)
}
if got := resolveWizardApp(nil, "immich"); got != nil {
t.Errorf("empty set must not resolve, got %+v", got)
}
}
// The customer-visible URL must redirect, not 500, when the offsite target is not configured at all.
func TestRestoreWizardHandler_UnconfiguredRedirects(t *testing.T) {
s := testServer(t)
rec := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/backups/restore/app?name=immich", nil)
s.backupsRestoreWizardHandler(rec, req)
if rec.Code != 302 {
t.Fatalf("want 302 redirect, got %d", rec.Code)
}
loc := rec.Header().Get("Location")
if !strings.HasPrefix(loc, "/backups/restore?flash_error=") {
t.Errorf("must redirect to the list with an error flash, got %q", loc)
}
}
// --- Group A + C (Scenarios A and C): one entry on the list, three described cards in the wizard ----
// Scenario A — the list page carries EXACTLY ONE restore control per app and ZERO offsite mutation
// forms. This is the finding itself: the five inline forms are gone from the row.
func TestRestoreList_SingleEntryPerApp(t *testing.T) {
data := splitTestData()
data["OffboxScratchReady"] = map[string]bool{"calibre-web": true}
data["OffboxPairInfo"] = map[string]backup.OffsitePairInfo{
"calibre-web": {Ready: true, HasDump: true, DumpsAt: time.Now().Add(-2 * time.Hour)},
}
html := renderBackupPage(t, "backups_restore", data)
// The scratch-ready fixture is precisely the state in which the OLD page rendered place and
// reconstitute as adjacent siblings. None of them may appear here now.
for _, banned := range []string{
`action="/backup/offbox/restore"`,
`action="/backup/offbox/place"`,
`action="/backup/offbox/reconstitute"`,
"Helyreállítás az élő adatok közé",
"Teljes visszaállítás (fájlok + adatbázis)",
"Teljes visszaállítás előkészítése",
} {
if strings.Contains(html, banned) {
t.Errorf("R-48 violated: the list page still renders %q", banned)
}
}
if n := strings.Count(html, `href="/backups/restore/app?name=calibre-web"`); n != 1 {
t.Errorf("want exactly ONE wizard entry for the app, got %d", n)
}
}
// Scenario C — the three intents are CARDS with their own consequence sentence, the dangerous one is
// styled as such, and the honesty panel is bound to the real pair info.
func TestRestoreWizard_ThreeIntentCards(t *testing.T) {
pair := backup.OffsitePairInfo{
Ready: true, HasDump: true,
DumpsAt: time.Date(2026, 7, 19, 3, 15, 0, 0, time.UTC),
Skewed: true, LooksEmpty: true,
}
view := deriveWizardStep(restoreWizardInput{App: "immich", ScratchReady: true})
html := renderWizard(t, wizardData("immich", view, pair))
// 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",
"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)",
"az adatbázist is visszatölti",
} {
if !strings.Contains(html, want) {
t.Errorf("intent card copy missing: %q", want)
}
}
// The dangerous intent is marked structurally, not only by wording/position.
if !strings.Contains(html, "restore-danger-card") {
t.Error("the full-restore card must carry the danger styling hook")
}
// Pair honesty is BOUND to the fixture, not hardcoded prose.
if !strings.Contains(html, "eltérő időpontból származnak") {
t.Error("Skewed pair must surface the skew warning")
}
if !strings.Contains(html, "üresnek tűnik") {
t.Error("LooksEmpty pair must surface the empty-dump warning")
}
if !strings.Contains(html, `data-restore-skewed="1"`) || !strings.Contains(html, `data-restore-empty="1"`) {
t.Error("the double-confirm must receive the pair facts it repeats back")
}
// The confirm copy moved VERBATIM — it is the good part of the old surface.
if !strings.Contains(html, "UTOLSÓ MEGERŐSÍTÉS") || !strings.Contains(html, "confirmFullRestore") {
t.Error("the double-confirm did not move with the action")
}
// A pair WITHOUT the warnings must not inherit them (guards a hardcoded-prose regression).
// NOTE: the confirm helper's JS repeats both sentences as string literals, so the assertion has
// to be on the RENDERED markup — the warning banners and the data-attributes that drive them —
// not on a bare substring, which is always present via the script block.
clean := renderWizard(t, wizardData("immich", view, backup.OffsitePairInfo{Ready: true, HasDump: true}))
// The two banners are identified by their opening markup, not by a bare substring: the layout's
// shared confirm helper also contains alert-warning literals.
const skewBanner = `alert alert-warning" style="margin-bottom:.75rem">Az adatbázis-mentés régebbi`
const emptyBanner = `alert alert-warning" style="margin-bottom:.75rem">A mentett adatbázis üresnek tűnik`
if !strings.Contains(html, skewBanner) || !strings.Contains(html, emptyBanner) {
t.Error("a skewed + empty-looking pair must render BOTH warning banners")
}
if strings.Contains(clean, skewBanner) || strings.Contains(clean, emptyBanner) {
t.Error("a clean pair must render neither warning banner")
}
if !strings.Contains(clean, `data-restore-skewed=""`) || !strings.Contains(clean, `data-restore-empty=""`) {
t.Error("a clean pair must pass empty skew/empty flags to the confirm helper")
}
}
// Without a prepared scratch the two data-touching intents are NOT offered — the card explains what
// has to happen first instead of showing a button that would fail.
func TestRestoreWizard_NoScratchLocksDataIntents(t *testing.T) {
view := deriveWizardStep(restoreWizardInput{App: "immich"})
html := renderWizard(t, wizardData("immich", view, backup.OffsitePairInfo{}))
if strings.Contains(html, `action="/backup/offbox/place"`) {
t.Error("missing-only merge must not be offered without a prepared scratch")
}
if strings.Contains(html, `action="/backup/offbox/reconstitute"`) {
t.Error("full restore must not be offered without a prepared scratch")
}
if !strings.Contains(html, "Teljes visszaállítás előkészítése") {
t.Error("the full-restore card must offer preparation instead")
}
}
// --- Group E (Scenario B, execution row): every mutation form suppressed while an op runs ----------
func TestRestoreWizard_OpRunningSuppressesAllMutations(t *testing.T) {
view := deriveWizardStep(restoreWizardInput{App: "immich", OpRunning: true, ScratchReady: true, FullPrepApp: "immich"})
data := wizardData("immich", view, backup.OffsitePairInfo{Ready: true, HasDump: true})
data["RunningStack"] = "bookstack"
html := renderWizard(t, data)
if strings.Contains(html, "<form") {
t.Error("the execution step must render NO mutation form — the single-flight would refuse it")
}
if !strings.Contains(html, "bookstack") {
t.Error("the execution card must name what is actually running")
}
}
// --- Group D (Scenario D): real flows only — no new mutation surface ------------------------------
var formActionRe = regexp.MustCompile(`action="([^"]+)"`)
// Every form in every wizard state posts to a PRE-EXISTING endpoint. If this test has to be updated
// to add a path, a new mutation endpoint was introduced — which R-48 explicitly does not do.
func TestRestoreWizard_NoNewMutationEndpoints(t *testing.T) {
preExisting := map[string]bool{
"/backup/offbox/restore": true,
"/backup/offbox/place": true,
"/backup/offbox/reconstitute": true,
}
states := []restoreWizardInput{
{App: "immich"},
{App: "immich", ScratchReady: true},
{App: "immich", FullPrepApp: "immich"},
{App: "immich", OpRunning: true},
}
seen := map[string]bool{}
for _, in := range states {
html := renderWizard(t, wizardData(in.App, deriveWizardStep(in), backup.OffsitePairInfo{Ready: true, HasDump: true}))
for _, m := range formActionRe.FindAllStringSubmatch(html, -1) {
seen[m[1]] = true
if !preExisting[m[1]] {
t.Errorf("wizard posts to a NON-pre-existing endpoint %q — R-48 adds no mutation surface", m[1])
}
}
}
var got []string
for p := range seen {
got = append(got, p)
}
sort.Strings(got)
if len(got) != 3 {
t.Errorf("expected all three existing endpoints to be reachable across the states, got %v", got)
}
}
// The field NAMES the wizard sends must match what the handlers read — a renamed field would make
// every action a silent no-op that still redirects with a success-shaped flash.
func TestRestoreWizard_FieldContract(t *testing.T) {
// intent step, scratch ready: unit-verify + place + reconstitute(confirm=1)
html := renderWizard(t, wizardData("immich",
deriveWizardStep(restoreWizardInput{App: "immich", ScratchReady: true}),
backup.OffsitePairInfo{Ready: true, HasDump: true}))
for _, want := range []string{
`<input type="hidden" name="app" value="immich">`,
`<input type="hidden" name="mode" value="unit">`,
`<input type="hidden" name="confirm" value="1">`,
} {
if !strings.Contains(html, want) {
t.Errorf("field contract broken, missing: %s", want)
}
}
// prepare-confirm step: mode=full AND confirm=1 together (the size-gated commit)
confirmHTML := renderWizard(t, wizardData("immich",
deriveWizardStep(restoreWizardInput{App: "immich", FullPrepApp: "immich"}),
backup.OffsitePairInfo{}))
if !strings.Contains(confirmHTML, `name="mode" value="full"`) || !strings.Contains(confirmHTML, `name="confirm" value="1"`) {
t.Error("the size-gated commit must post mode=full together with confirm=1")
}
if !strings.Contains(confirmHTML, "1,2 GB") {
t.Error("the confirm step must show the measured size before the customer commits")
}
}