diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index cc297f7..edc18a9 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -678,7 +678,12 @@ func main() { // v0.201.0 (R-222): whether the hub is ALSO keeping an EARLIER sealed package, so the // recovery screen can name that situation instead of blaming the customer's typing. RecordSuperseded: sett.SetHubEscrowSuperseded, - Logger: logger, + // v0.206.0 (R-241): the hash the hub's package COVERS. This is the fact the recovery + // screen's shape (c) reads, and the comparison against it was already being computed here + // on every ACK and discarded. Wired at the same point as the two above, deliberately — + // a second wiring site in this file is how six features got built and never reached. + RecordEscrowKeyHash: sett.SetHubEscrowKeySHA256, + Logger: logger, } // Wire hub verification: update settings when hub reports customer status hubPusher.OnPushResponse = func(resp *report.PushResponse) { diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index a34abaa..7a435cb 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -1522,13 +1522,49 @@ func (m *Manager) needsOffsiteCredential(t *settings.OffboxTarget) bool { // // Scenario B still holds exactly: a healthy box has its own password and is not orphaned; a box that // never had off-site backups fails fact 1; an unclaimed box never reaches an authenticated page. +// ── SHAPE (c), v0.206.0, R-241 — THE DISCRIMINATOR THAT ANSWERS THE REAL QUESTION ─────────────── +// +// Shapes (a) and (b) are both PROXIES for one question — *does the hub hold a package for a key other +// than the one I am using?* — and both have now been wrong, in opposite directions: +// +// - (a) "no repository password" went false the moment anything minted one. Before v0.206.0's mint +// guard that happened by itself, ~30 minutes after a rebuild, and the customer who logged in the +// next morning never saw the screen. That is R-241. +// - (b) "a run proved the repo will not open" is unreachable on exactly that box: the only producer +// of RepoState=="orphaned" is ensureOffboxRepo, which is downstream of the escrow gate in +// runOffboxBackup, and the escrow can never confirm while the hub's package covers a different +// key. Self-locking. +// +// (c) asks the question directly, from two facts the box already holds: the hash the hub's package +// covers (ACK-cached) and the hash of the key on disk. **This comparison was already computed on every +// ACK and thrown away** — see settings.HubEscrowKeySHA256. +// +// ⚠ §7.2 — WHAT A STALE OR ABSENT READING RESOLVES TO, decided deliberately rather than by default: +// +// - **A KNOWN DIFFERENCE OFFERS, however old the reading.** Age is not gated on. Both sides of the +// comparison are local; only the hub's half can be stale, and what the hub holds does not change +// without a ceremony THIS BOX runs — which refreshes the hash on the next ACK. Gating on age would +// add a second failure mode (a box offline from the hub silently stops offering) to fix a window +// that closes itself. `HubEscrowKeyCheckedAt` is persisted for diagnosis, not as a gate. +// - **AN ABSENT HASH FALLS BACK TO (a)/(b), it does not offer.** "" is what the hub sends for a +// legacy hash-less package — one that provably seals no repository password. There is nothing for +// (c) to compare, and offering on it would put a permanent screen in front of every legacy box. +// This is the one place where "not knowing" resolves to silence, and it does so because an empty +// hash is not an unknown: it is the hub positively saying the package covers no key. +// +// So: fail-closed (offer) on a known difference; fall back on a hash never learned. Pinned by +// TestR241_ScenarioD_* and TestR241_StaleComparison_*. func (m *Manager) OffsiteRecoveryOffer() bool { if m.settings == nil || !m.settings.GetHubEscrowIdentityPresent() { return false // the hub holds nothing for us — nothing to recover } - if _, ok := m.OffboxRepoPasswordHash(); !ok { + localHash, hasLocal := m.OffboxRepoPasswordHash() + if !hasLocal { return true // (a) no repository password at all — the pristine rebuilt box } + if hubHash, _ := m.settings.GetHubEscrowKeySHA256(); hubHash != "" && hubHash != localHash { + return true // (c) the hub's package covers a DIFFERENT key than the one we are using + } return m.OffboxOrphaned() // (b) a password exists but the inherited history will not open under it } diff --git a/controller/internal/backup/offbox_offer_shapec_r241_test.go b/controller/internal/backup/offbox_offer_shapec_r241_test.go new file mode 100644 index 0000000..9b2f3c2 --- /dev/null +++ b/controller/internal/backup/offbox_offer_shapec_r241_test.go @@ -0,0 +1,143 @@ +package backup + +import ( + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-241 shape (c) — the recovery offer is driven by the comparison the box already makes. +// +// Scenarios C and D from the task, plus §7.2's two staleness cases. The point of shape (c) is that +// it asks the real question — *does the hub hold a package for a key other than the one I am +// using?* — rather than the two proxies that have each now been wrong in opposite directions. + +// offerFixture builds a manager holding a repository password, with the hub's cached facts settable. +// Returns the local key's hash so a test can make the hub's hash match or differ deliberately. +func offerFixture(t *testing.T, hubHoldsPackage bool) (*Manager, *settings.Settings, string) { + t.Helper() + m, sett, pwPath := mintGuardManager(t, false) // mint freely first + if err := sett.SetOffboxTarget(&settings.OffboxTarget{ + Enabled: true, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo", Schedule: "daily", + }); err != nil { + t.Fatal(err) + } + if err := m.WriteOffboxSecrets("KEYMATERIAL", "nas.local ssh-ed25519 HOSTKEY"); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(pwPath); err != nil { + t.Fatalf("fixture should hold a repository password: %v", err) + } + local, ok := m.OffboxRepoPasswordHash() + if !ok { + t.Fatal("fixture should be able to hash its own key") + } + if err := sett.SetHubEscrowIdentityPresent(hubHoldsPackage); err != nil { + t.Fatal(err) + } + return m, sett, local +} + +const otherKeyHash = "9b4a9a9dcec7898e7544f35b18470aac77c3d9064e5d3a302897617fa62edd65" + +// ── SCENARIO C — a differing key offers recovery, whatever the reason for the difference ──────── +// +// This is the venue's exact state on 2026-08-07: a key present, no orphan recorded, escrow stuck +// pending — and before shape (c), silence. +func TestR241_ScenarioC_DifferingKeyOffersRecovery(t *testing.T) { + m, sett, local := offerFixture(t, true) + if local == otherKeyHash { + t.Fatal("fixture precondition: the local key must differ from the hub's") + } + if err := sett.SetHubEscrowKeySHA256(otherKeyHash, "2026-08-07T03:28:03Z"); err != nil { + t.Fatal(err) + } + // Neither proxy fires: a key EXISTS (so not shape (a)) and nothing is orphaned (so not shape (b)). + if _, ok := m.OffboxRepoPasswordHash(); !ok { + t.Fatal("precondition: shape (a) must be false") + } + if m.OffboxOrphaned() { + t.Fatal("precondition: shape (b) must be false") + } + if !m.OffsiteRecoveryOffer() { + t.Fatal("R-241: the hub holds a package for a DIFFERENT key and the screen was not offered — this is the defect") + } +} + +// ── SCENARIO D — a healthy box is never offered recovery ──────────────────────────────────────── +// +// RED-PROOF: drop the `hubHash != localHash` conjunct in shape (c) (make it `hubHash != ""`). A +// healthy box is then offered recovery forever, and this test fails — which is how a screen stops +// being read. +func TestR241_ScenarioD_MatchingKeyOffersNothing(t *testing.T) { + m, sett, local := offerFixture(t, true) + if err := sett.SetHubEscrowKeySHA256(local, "2026-08-07T09:00:00Z"); err != nil { + t.Fatal(err) + } + if m.OffsiteRecoveryOffer() { + t.Fatal("a box whose key the hub's package covers must never be offered recovery") + } +} + +// A box the hub holds nothing for is never offered, even if a stale hash lingers in settings. Fact 1 +// stays required — the spike's comment block calls dropping it "the plausible wrong fix". +func TestR241_ShapeC_NeverHadOffsiteIsStillSilent(t *testing.T) { + m, sett, _ := offerFixture(t, false) // the hub holds NOTHING + if err := sett.SetHubEscrowKeySHA256(otherKeyHash, "2026-08-07T09:00:00Z"); err != nil { + t.Fatal(err) + } + if m.OffsiteRecoveryOffer() { + t.Fatal("a box that never had off-site backups must never be greeted by a recovery screen") + } +} + +// ── §7.2 — the staleness decision, both halves ────────────────────────────────────────────────── + +// A KNOWN DIFFERENCE OFFERS, however old the reading. Age is deliberately not gated on: gating would +// make a box offline from the hub silently stop offering, which is the failure this session exists +// to remove. +func TestR241_StaleComparison_KnownDifferenceStillOffers(t *testing.T) { + m, sett, _ := offerFixture(t, true) + if err := sett.SetHubEscrowKeySHA256(otherKeyHash, "2020-01-01T00:00:00Z"); err != nil { // ancient + t.Fatal(err) + } + if !m.OffsiteRecoveryOffer() { + t.Fatal("a known difference must offer regardless of how old the reading is (§7.2)") + } +} + +// AN ABSENT HASH FALLS BACK TO (a)/(b) — it does not offer. "" is the hub positively saying its +// package seals no repository password (legacy hash-less escrow); there is nothing to compare, and +// offering would put a permanent screen in front of every legacy box. +func TestR241_StaleComparison_AbsentHashFallsBackAndDoesNotOffer(t *testing.T) { + m, sett, _ := offerFixture(t, true) + if err := sett.SetHubEscrowKeySHA256("", ""); err != nil { + t.Fatal(err) + } + if m.OffsiteRecoveryOffer() { + t.Fatal("a hash never learned must fall back to (a)/(b), not offer (§7.2)") + } + // ...and the fallback still works: mark the repo orphaned and shape (b) fires as before. + if err := m.settings.UpdateOffboxStatus(func(o *settings.OffboxTarget) { o.RepoState = "orphaned" }); err != nil { + t.Fatal(err) + } + if !m.OffsiteRecoveryOffer() { + t.Fatal("shape (b) must still work when the hub's hash was never learned") + } +} + +// Shape (a) is untouched: a box with no key at all is still offered, which is the pristine rebuild. +func TestR241_ShapeAStillWorks(t *testing.T) { + m, sett, _ := offerFixture(t, true) + if err := os.Remove(filepath.Join(m.cfg.Paths.DataDir, "offbox", "repo_password")); err != nil { + t.Fatal(err) + } + if err := sett.SetHubEscrowKeySHA256(otherKeyHash, "2026-08-07T09:00:00Z"); err != nil { + t.Fatal(err) + } + if !m.OffsiteRecoveryOffer() { + t.Fatal("shape (a) — no repository password at all — must still offer") + } +} diff --git a/controller/internal/report/escrow_confirm.go b/controller/internal/report/escrow_confirm.go index 03c595a..15f69b4 100644 --- a/controller/internal/report/escrow_confirm.go +++ b/controller/internal/report/escrow_confirm.go @@ -65,7 +65,23 @@ type EscrowAutoConfirmer struct { // second wiring point in main.go is how this project accumulated six features that were built and // never wired. nil → not recorded (older wiring, tests). RecordSuperseded func(present bool, at string) error - Logger *log.Logger + // RecordEscrowKeyHash persists the ACK's `restic_pw_sha256` — the hash of the repository password + // the hub's sealed package COVERS — with the time it was recorded (v0.206.0, R-241). + // + // WHY IT LIVES HERE, and it is the point of the whole change: the comparison between this hash and + // the local key is ALREADY MADE in Reconcile, on every ACK, and has been since SLICE 3 — and the + // result was used for one warning line and then discarded. On the final-walk venue that line + // (03:28:03Z) was the correct answer to the recovery screen's real question, thirty-five minutes + // before the customer looked at a screen that could not see it. + // + // Recorded UNCONDITIONALLY, before every gate below, for exactly the reason RecordPresence is: the + // box that needs this most is the rebuilt one with no configured target, on which `Pending()` and + // `Escrowed()` are both false and Reconcile used to return immediately. nil → not recorded. + RecordEscrowKeyHash func(sha, checkedAt string) error + // Now returns the current time; nil → time.Now. Injected so the persisted "when we last heard" + // stamp is testable without a sleep. + Now func() time.Time + Logger *log.Logger mu sync.Mutex warnedHash string // last mismatched hub hash we warned about (dedupe; shared by both branches) @@ -130,6 +146,14 @@ func (c *EscrowAutoConfirmer) Reconcile(es *EscrowStatus) { c.logf("[WARN] [escrow-confirm] could not record the hub's superseded-package state (present=%v): %v", es.SupersededPresent, err) } } + // R-241: persist the hash the hub's package covers, with the moment we heard it. Same discipline, + // same place, same reason as the two above — and this one is the fact the recovery screen has been + // unable to see. A record failure is logged, never swallowed, and never blocks the auto-confirm. + if c.RecordEscrowKeyHash != nil { + if err := c.RecordEscrowKeyHash(es.ResticPwSHA256, c.now().UTC().Format(time.RFC3339)); err != nil { + c.logf("[WARN] [escrow-confirm] could not record the hub's escrowed-key hash (%.12s…): %v", es.ResticPwSHA256, err) + } + } c.mu.Lock() c.sealedAt = es.CreatedAt // in-memory only; a timestamp, never a secret c.mu.Unlock() @@ -216,3 +240,11 @@ func (c *EscrowAutoConfirmer) reconcileEscrowed(es *EscrowStatus) { } c.logf("[WARN] [escrow-confirm] STALE escrow: the hub's current blob does not cover the CURRENT repo password (hub hash %.12s… != local %.12s…) — create a new recovery code (wizard /backup/escrow). State stays escrowed; runs continue", hubHash, localHash) } + +// now returns the injected clock or time.Now. +func (c *EscrowAutoConfirmer) now() time.Time { + if c.Now != nil { + return c.Now() + } + return time.Now() +} diff --git a/controller/internal/report/escrow_presence_wiring_test.go b/controller/internal/report/escrow_presence_wiring_test.go index 4631d9c..56facd7 100644 --- a/controller/internal/report/escrow_presence_wiring_test.go +++ b/controller/internal/report/escrow_presence_wiring_test.go @@ -66,3 +66,61 @@ func TestMainWiresRecordPresence(t *testing.T) { t.Fatal("EscrowAutoConfirmer is constructed WITHOUT RecordPresence — the box will never learn the hub holds its recovery package, and R-204 item 4 ships inert") } } + +// confirmerFieldIsWired is the generalised form of the walk above: it reports whether +// `EscrowAutoConfirmer{...}` in main.go assigns `field`, and whether the literal was found at all. +// Comments are dropped on purpose, so a commented-out assignment cannot satisfy it. +func confirmerFieldIsWired(t *testing.T, field string) (found, sawLiteral bool) { + t.Helper() + const mainPath = "../../cmd/controller/main.go" + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, mainPath, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v — the wiring of %s is now unasserted", mainPath, err, field) + } + ast.Inspect(f, func(n ast.Node) bool { + lit, ok := n.(*ast.CompositeLit) + if !ok { + return true + } + name := "" + switch tt := lit.Type.(type) { + case *ast.SelectorExpr: + name = tt.Sel.Name + case *ast.Ident: + name = tt.Name + } + if name != "EscrowAutoConfirmer" { + return true + } + sawLiteral = true + for _, el := range lit.Elts { + kv, ok := el.(*ast.KeyValueExpr) + if !ok { + continue + } + if k, ok := kv.Key.(*ast.Ident); ok && k.Name == field { + found = true + } + } + return true + }) + return found, sawLiteral +} + +// TestMainWiresRecordEscrowKeyHash — R-241's seam-discipline test, and it matters more than most. +// +// `RecordEscrowKeyHash` is nil-able exactly like `RecordPresence`. Unwired, the confirmer still +// compiles, every test in this package still passes, the auto-confirm still works — and +// `OffsiteRecoveryOffer`'s shape (c) reads an empty hash forever, silently falling back to the two +// proxies that R-241 proved insufficient. **The fix would ship inert, in precisely the shape the +// spike found: a correct answer computed and kept nowhere.** +func TestMainWiresRecordEscrowKeyHash(t *testing.T) { + found, sawLiteral := confirmerFieldIsWired(t, "RecordEscrowKeyHash") + if !sawLiteral { + t.Fatal("no EscrowAutoConfirmer composite literal found in main.go — did the wiring move? This test can no longer see it") + } + if !found { + t.Fatal("EscrowAutoConfirmer is constructed WITHOUT RecordEscrowKeyHash — the hub's escrowed-key hash is never persisted, so the recovery screen's shape (c) can never fire and R-241 ships inert") + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index 22b8a2c..d4448e7 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -86,6 +86,32 @@ type Settings struct { HubEscrowSupersededPresent bool `json:"hub_escrow_superseded_present,omitempty"` HubEscrowSupersededAt string `json:"hub_escrow_superseded_at,omitempty"` + // HubEscrowKeySHA256 / HubEscrowKeyCheckedAt (v0.206.0, R-241) cache the report ACK's + // `escrow.restic_pw_sha256` — the sha256 of the repository password the hub's sealed package + // COVERS — and when it was last recorded. + // + // ⚠ THIS FACT WAS ALREADY COMPUTED EVERY CYCLE AND KEPT NOWHERE, and that is the whole of R-241's + // second half. `report.EscrowAutoConfirmer.Reconcile` has compared this hash against the local key + // on every ACK since SLICE 3; on the final-walk venue it logged, at 03:28:03Z and thirty-five + // minutes before the customer looked, *"the hub's escrow blob does not cover the CURRENT repo + // password (hub hash 30ef574f… != local 9b4a9a9d…)"* — and then dropped it on the floor. The + // recovery screen, evaluating in the same process, went on asking a question that could not see it. + // + // It is the ONE fact that answers the screen's real question directly: *does the hub hold a package + // for a key other than the one I am using?* Shape (a) ("no key at all") and shape (b) ("a run + // proved the repo will not open") are both proxies for it, and both have now been wrong in + // opposite directions — (a) goes false the moment anything mints, (b) is unreachable while the + // escrow is pending. + // + // NON-SECRET: the sha256 of a 256-bit random secret is non-reversible and is already logged and + // served over the ACK. It must still never reach a customer-facing message. + // + // EMPTY IS MEANINGFUL AND IS NOT "THEY DIFFER": the hub sends "" for a legacy hash-less escrow + // (a package that provably seals no repository password). Shape (c) requires a NON-EMPTY hash — + // see backup.OffsiteRecoveryOffer for the staleness reasoning. + HubEscrowKeySHA256 string `json:"hub_escrow_key_sha256,omitempty"` + HubEscrowKeyCheckedAt string `json:"hub_escrow_key_checked_at,omitempty"` // RFC3339 + // RecoveryNoticePostponed (v0.200.0, R-193) — the customer chose "most nem" on the full-page // recovery screen. It suppresses THE FULL-PAGE INTERRUPTION ONLY. The entry point in the backups // area stays, permanently, for as long as the situation lasts: the data is still there whether or @@ -2052,3 +2078,46 @@ func (s *Settings) GetIntegrationsForTarget(target string) map[string]Integratio } return result } + +// GetHubEscrowKeySHA256 returns the sha256 the hub's sealed package covers, and when it was last +// recorded from an ACK ("" / "" when never learned). See the field comment: empty is "the hub never +// told us", not "they match". +func (s *Settings) GetHubEscrowKeySHA256() (sha, checkedAt string) { + s.mu.RLock() + defer s.mu.RUnlock() + return s.HubEscrowKeySHA256, s.HubEscrowKeyCheckedAt +} + +// SetHubEscrowKeySHA256 records the ACK's `escrow.restic_pw_sha256` and stamps when. Same +// last-write-wins mirror discipline as SetHubEscrowIdentityPresent — the hub is the authority on +// what the hub holds, and a re-ceremony legitimately moves this. +// +// The timestamp is refreshed on EVERY ack that carries a hash, including an unchanged one, because +// it records *when we last heard*, not *when it last changed* — a distinction this project has got +// wrong before (R-100: LastRun recorded an attempt and was read as a result). A no-op save is +// avoided only when BOTH the hash and the day are unchanged, so an idle box does not rewrite +// settings.json every fifteen minutes. +func (s *Settings) SetHubEscrowKeySHA256(sha, checkedAt string) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.HubEscrowKeySHA256 == sha && sameDayStamp(s.HubEscrowKeyCheckedAt, checkedAt) { + return nil + } + s.HubEscrowKeySHA256, s.HubEscrowKeyCheckedAt = sha, checkedAt + return s.save() +} + +// sameDayStamp reports whether two RFC3339 stamps fall on the same UTC day — the write-damper for +// SetHubEscrowKeySHA256. Unparseable stamps are treated as different, so a malformed value always +// gets replaced rather than sticking. +func sameDayStamp(a, b string) bool { + if a == "" || b == "" { + return false + } + ta, erra := time.Parse(time.RFC3339, a) + tb, errb := time.Parse(time.RFC3339, b) + if erra != nil || errb != nil { + return false + } + return ta.UTC().Format("2006-01-02") == tb.UTC().Format("2006-01-02") +}