From 1170b48e59f746d730f2fa682e3e136b80d1ef6d Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 26 Oct 2026 20:20:59 +0100 Subject: [PATCH] fix(R-1218, R-1219): stage the off-site password when its file exists; no recovery ceremony during a recovery offer R-1218: escrowStart stages when OffboxRepoPasswordOnBox() (not OffboxConfigured()). R-1219: the top bar hides during an offer; the start handler answers 409 (api.escrow.recovery_offer_open, hu+en) before staging. Red-proved. Commit only. --- CHANGELOG.md | 9 +++ controller/internal/backup/offbox.go | 8 +++ controller/internal/i18n/locales/en.json | 1 + controller/internal/i18n/locales/hu.json | 1 + controller/internal/web/escrow_banner.go | 5 ++ controller/internal/web/escrow_handlers.go | 16 ++++- .../r1218_stage_when_password_file_test.go | 56 +++++++++++++++ .../r1219_no_ceremony_during_offer_test.go | 69 +++++++++++++++++++ 8 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 controller/internal/web/r1218_stage_when_password_file_test.go create mode 100644 controller/internal/web/r1219_no_ceremony_during_offer_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f5a67c74..b58bb845 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,15 @@ hub answers 400 to both and nothing else changes. lists — and never offers to delete — that app's live folder; one WARN names the app. Before: a parse error gave a nil `AppConfig`, the app read as not installed, its folder was a „leftover". Test `TestR1220_AnUnreadableAppYamlKeepsTheFolderLive` (with a real leftover as the control), red-proved against the old code. +- **R-1218 (P2, key code):** the recovery ceremony stages the off-site repository password whenever the password FILE is + on the box (`backup.Manager.OffboxRepoPasswordOnBox`), not only when off-site is configured — a cleared or switched-off + target keeps the file, and a ceremony that sealed no password let the hub demote the household's good package (the + hub half refuses that demotion: felhom.eu hub, same night). Test `TestR1218_TheCeremonyStagesWhenThePasswordFileExists` + (off, cleared, and a box with no password as the control), red-proved („preflight,start" — no stage). +- **R-1219 (P2, key code):** during a recovery offer the top bar no longer urges the ceremony, and `POST + /api/escrow/start` answers 409 with `api.escrow.recovery_offer_open` (hu + en) before anything is staged or started. + Tests `TestR1219_*` (the bar with a real session, and the same box without an offer as the control), red-proved (bar + shown, start 200). - `settings.RestoreHold.Why()` / `WireReason()`: the hold sentence moved out of `cmd/controller` (`holdWhy` calls it), so `--restore-holds` and the hub's list say the same thing. - Tests `internal/holdreport/holdreport_test.go` (8): the report field, the daily repeat, cleared once, cleared while diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index de34bcb5..1a6ca351 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -714,6 +714,14 @@ func (m *Manager) OffboxConfigured() bool { return true } +// OffboxRepoPasswordOnBox reports whether the off-site repository password FILE is on the box — whatever the target's +// state. R-1218: ClearOffboxTarget keeps that file on purpose, and a switched-off target keeps it too, so a recovery +// ceremony run then must still seal it; OffboxConfigured() is the wrong question for „is there a key to escrow". +func (m *Manager) OffboxRepoPasswordOnBox() bool { + _, err := os.Stat(m.offboxPwPath()) + return err == nil +} + // offboxRepoPwPattern matches a valid restic repo password (generateOffboxPassword = 32 rand bytes → 64 hex). var offboxRepoPwPattern = regexp.MustCompile(`^[0-9a-fA-F]{64}$`) diff --git a/controller/internal/i18n/locales/en.json b/controller/internal/i18n/locales/en.json index da1443f2..65b0c81f 100644 --- a/controller/internal/i18n/locales/en.json +++ b/controller/internal/i18n/locales/en.json @@ -2663,6 +2663,7 @@ "api.escrow.too_many_attempts": "Too many failed attempts, try again in 1 minute.", "api.escrow.stage_failed": "Preparing the remote backup password for escrow failed — the process did not start. Try again.", "api.escrow.already_running": "A code is already being created — wait until it finishes.", + "api.escrow.recovery_offer_open": "You cannot create a new recovery code right now: first open your old off-site backups with your existing code (Backups page). A new code would replace the key of the old backups.", "api.escrow.code_not_shown": "The code was created but not shown — for security it cannot be fetched again. Start the process again: the new code replaces the old one.", "api.escrow.no_active_job": "There is no active recovery process — start creating the code first.", "api.escrow.confirm_expired": "The time to type the new code back has run out — your previous code still works. Start the process again.", diff --git a/controller/internal/i18n/locales/hu.json b/controller/internal/i18n/locales/hu.json index 775f5119..39a6e9c4 100644 --- a/controller/internal/i18n/locales/hu.json +++ b/controller/internal/i18n/locales/hu.json @@ -2651,6 +2651,7 @@ "api.escrow.too_many_attempts": "Túl sok sikertelen próbálkozás, próbáld újra 1 perc múlva.", "api.escrow.stage_failed": "A távoli mentés jelszavának letéti előkészítése nem sikerült — a folyamat nem indult el. Próbáld újra.", "api.escrow.already_running": "Egy kódkészítés már folyamatban van — várd meg, míg befejeződik.", + "api.escrow.recovery_offer_open": "Most nem készíthetsz új helyreállítási kódot: előbb nyisd meg a régi, házon kívüli mentéseidet a meglévő kódoddal (Mentések oldal). Egy új kód lecserélné a régi mentések kulcsát.", "api.escrow.code_not_shown": "A kód létrejött, de nem lett megjelenítve — biztonsági okból újra nem kérhető le. Indítsd újra a folyamatot: az új kód a régit érvényteleníti.", "api.escrow.no_active_job": "Nincs aktív helyreállítási folyamat — előbb indítsd el a kódkészítést.", "api.escrow.confirm_expired": "Lejárt az idő az új kód visszaírására — a régi kódod továbbra is érvényes. Indítsd újra a folyamatot.", diff --git a/controller/internal/web/escrow_banner.go b/controller/internal/web/escrow_banner.go index 559951fd..23d2e6e7 100644 --- a/controller/internal/web/escrow_banner.go +++ b/controller/internal/web/escrow_banner.go @@ -53,6 +53,11 @@ func (s *Server) escrowBannerVisible(r *http.Request) bool { if r == nil || !s.hasAdminSession(r) || !s.escrowPaused() { return false } + // R-1219: during a recovery offer a NEW code would replace the key that opens the old off-site copy (R-199) — the + // backups page hides the ceremony then, and so does this bar. Pinned by TestR1219_NoCeremonyDuringARecoveryOffer. + if s.recoveryOffer() { + return false + } // R-546: while the agent says the ceremony cannot run yet, do not urge the household into it. // Unknown readiness keeps the bar (fail loud). if ready, known := s.escrowReadiness(r.Context(), false); known && !ready { diff --git a/controller/internal/web/escrow_handlers.go b/controller/internal/web/escrow_handlers.go index c9460d90..71d67448 100644 --- a/controller/internal/web/escrow_handlers.go +++ b/controller/internal/web/escrow_handlers.go @@ -213,8 +213,20 @@ func (s *Server) escrowStartAPIHandler(w http.ResponseWriter, r *http.Request) { return } - // (2) Re-stage-first (only when offsite is configured — Scenario C boxes skip it). - if s.backupMgr != nil && s.backupMgr.OffboxConfigured() { + // (1c) R-1219: no ceremony during a recovery offer — a new code would replace the key that opens the old off-site + // copy (R-199). The page hides the form; a direct POST (or the old top-bar link) is refused here, before anything is + // staged or started. Pinned by TestR1219_NoCeremonyDuringARecoveryOffer. + if s.recoveryOffer() { + s.logger.Printf("[INFO] [web] escrow start refused: a recovery offer is open (R-1219)") + escrowJSON(w, http.StatusConflict, nil, s.msg(r, "api.escrow.recovery_offer_open")) + return + } + + // (2) Re-stage-first whenever the repository password FILE is on the box (R-1218) — not only when off-site is + // configured: a cleared or switched-off target keeps the file, and a ceremony that seals no password makes the hub + // demote the household's good package. A box with no password (Scenario C, R-241's awaiting key) skips it. + // Pinned by TestR1218_TheCeremonyStagesWhenThePasswordFileExists. + if s.backupMgr != nil && s.backupMgr.OffboxRepoPasswordOnBox() { if err := s.escrowStage(r.Context()); err != nil { s.logger.Printf("[WARN] [web] escrow start: re-stage failed (ceremony NOT started): %v", err) // err carries no secret escrowJSON(w, http.StatusBadGateway, nil, s.msg(r, "api.escrow.stage_failed")) diff --git a/controller/internal/web/r1218_stage_when_password_file_test.go b/controller/internal/web/r1218_stage_when_password_file_test.go new file mode 100644 index 00000000..107f7fe8 --- /dev/null +++ b/controller/internal/web/r1218_stage_when_password_file_test.go @@ -0,0 +1,56 @@ +package web + +import ( + "net/http" + "strings" + "testing" +) + +// R-1218: a recovery ceremony run while off-site is switched OFF (or its target cleared) must still stage the off-site +// repository password when the password FILE is on the box — ClearOffboxTarget keeps that file on purpose. Before the +// fix the stage ran only when OffboxConfigured() was true, the agent sealed no password, uploaded sha "", and the hub +// demoted the household's good package: the family's code no longer opened its off-site copy. Red-proved against the +// OffboxConfigured() gate (order „preflight,start", no stage). +func TestR1218_TheCeremonyStagesWhenThePasswordFileExists(t *testing.T) { + for _, tc := range []struct { + name string + off func(h *escrowWizardHarness) + }{ + {"off-site switched off", func(h *escrowWizardHarness) { + tg := h.sett.GetOffboxTarget() + tg.Enabled = false + if err := h.sett.SetOffboxTarget(tg); err != nil { + t.Fatal(err) + } + }}, + {"off-site target cleared", func(h *escrowWizardHarness) { + if err := h.sett.SetOffboxTarget(nil); err != nil { + t.Fatal(err) + } + }}, + } { + t.Run(tc.name, func(t *testing.T) { + h := newEscrowWizardHarness(t) + h.configureOffbox(t, "escrowed") + tc.off(h) + if h.m.OffboxConfigured() { + t.Fatal("setup: off-site should read as not configured") + } + w := postStart(t, h.s, wizardPassword) + if w.Code != http.StatusOK { + t.Fatalf("start: %d %s", w.Code, w.Body.String()) + } + if got := strings.Join(h.order, ","); got != "preflight,stage,start" { + t.Fatalf("call order = %q — the repository password on the box was NOT staged, so the new package would cover no key", got) + } + }) + } + // The control: no password file at all (a box that never had off-site, R-241's awaiting-key shape) → nothing staged. + h := newEscrowWizardHarness(t) + if w := postStart(t, h.s, wizardPassword); w.Code != http.StatusOK { + t.Fatalf("start without off-site: %d %s", w.Code, w.Body.String()) + } + if got := strings.Join(h.order, ","); got != "preflight,start" { + t.Fatalf("a box with no repository password staged something: %q", got) + } +} diff --git a/controller/internal/web/r1219_no_ceremony_during_offer_test.go b/controller/internal/web/r1219_no_ceremony_during_offer_test.go new file mode 100644 index 00000000..54259d2b --- /dev/null +++ b/controller/internal/web/r1219_no_ceremony_during_offer_test.go @@ -0,0 +1,69 @@ +package web + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// offerShapeC puts the harness box in the recovery offer, shape (c): the hub holds a package for a DIFFERENT repository +// key than the one the box uses — the old off-site copy opens only with the household's EXISTING recovery code. +func offerShapeC(t *testing.T, h *escrowWizardHarness) { + t.Helper() + h.configureOffbox(t, "pending") + if err := h.sett.SetHubEscrowIdentityPresent(true); err != nil { + t.Fatal(err) + } + if err := h.sett.SetHubEscrowKeySHA256("0000000000000000000000000000000000000000000000000000000000000000", "2026-10-26T20:00:00Z"); err != nil { + t.Fatal(err) + } + if !h.m.OffsiteRecoveryOffer() { + t.Fatal("setup: the box should be in the recovery offer") + } +} + +// R-1219: during a recovery offer a NEW recovery code would replace the key that opens the old off-site copy (R-199). +// The backups page already hid the ceremony; the global top bar still linked to it and the start handler ran it. Now +// the bar is not shown and the start handler answers 409 with a sentence in the household's language, BEFORE anything +// is staged or started. COMPANION RED-PROOF: before the fix the bar was visible and the start answered 200 +// („preflight,stage,start"). +func TestR1219_NoCeremonyDuringARecoveryOffer(t *testing.T) { + h := newEscrowWizardHarness(t) + offerShapeC(t, h) + if !h.s.escrowPaused() { + t.Fatal("setup: the tier should read as paused (pending) — the bar would otherwise be hidden for another reason") + } + r := httptest.NewRequest("GET", "/", nil) + r.AddCookie(&http.Cookie{Name: sessionCookieName, Value: newTestSession(h.s)}) + if h.s.escrowBannerVisible(r) { + t.Error("the top bar urges the recovery-code ceremony during a recovery offer") + } + w := postStart(t, h.s, wizardPassword) + if w.Code != http.StatusConflict { + t.Fatalf("start during an offer = %d (%s), want 409", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "helyre") { // ASCII fragment of the Hungarian sentence (ui-hungarian rule) + t.Errorf("the refusal does not say why in Hungarian: %s", w.Body.String()) + } + for _, step := range h.order { + if step == "stage" || step == "start" { + t.Fatalf("the agent was reached during an offer: %v", h.order) + } + } +} + +// The control for the bar half: the SAME paused box with no recovery offer, the same logged-in request — the bar IS +// shown. Without this, a bar hidden for any other reason (no session, readiness) would pass the test above. +func TestR1219_TheBarStillShowsWithoutAnOffer(t *testing.T) { + h := newEscrowWizardHarness(t) + h.configureOffbox(t, "pending") + if h.m.OffsiteRecoveryOffer() { + t.Fatal("setup: no offer expected") + } + r := httptest.NewRequest("GET", "/", nil) + r.AddCookie(&http.Cookie{Name: sessionCookieName, Value: newTestSession(h.s)}) + if !h.s.escrowBannerVisible(r) { + t.Fatal("the bar is hidden on a paused box with no offer — the offer test above would prove nothing") + } +}