From a3499d18076c36e8b0ebbabd4b9a731277a5fc18 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 5 Aug 2026 17:48:08 +0200 Subject: [PATCH] =?UTF-8?q?v0.201.0=20=E2=80=94=20a=20correct=20recovery?= =?UTF-8?q?=20code=20is=20never=20called=20wrong=20again=20(CAMPAIGN-11)?= =?UTF-8?q?=20=E2=80=94=20MinAgent=200.125.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R-216: the offsite key recovery is a coupled feature and now says so. featureProbes + featureMinAgent 0.125.0 + a Supports gate at the unlock entry point, FAILING CLOSED — an agent that cannot answer is named as such instead of the customer's code being blamed. Measured live: a 404 from agent 0.120.0 came back as "we did not accept your recovery code, check that all ten words", in 0.134 s, against a perfect code. R-218: delete the repo-password short-circuit in needsOffsiteCredential. The declaration stops when the TIER WORKS, not when a key exists — installing a key is the recovery screen's whole job, so succeeding at recovery was switching off the mechanism that would have delivered the coordinates to use it. R-219: the unlock finishes the job — place the key, bring the tier up, then list. Without it the promised listing could never render on the shape the screen exists for. R-217: an unreadable store no longer claims to have opened with unattributable content (the OffsiteInventory{} zero value). Opened / empty / unreadable are three states. R-222: a code that is right about a RETAINED earlier package is named, not blamed. States what the hub knows and promises nothing — no read path exists. R-215: GET /recovery is gated on the same predicate as the interception. Five red-proofs, each demonstrated failing and restored. --- CHANGELOG.md | 60 +++ controller/cmd/controller/main.go | 17 +- controller/internal/agentapi/features.go | 43 +++ controller/internal/backup/offbox.go | 29 +- .../internal/backup/offbox_declare_test.go | 51 ++- controller/internal/report/escrow_confirm.go | 21 +- controller/internal/settings/settings.go | 38 ++ controller/internal/web/recovery_gate_test.go | 344 ++++++++++++++++++ controller/internal/web/recovery_handlers.go | 175 ++++++++- controller/internal/web/recovery_test.go | 6 + controller/internal/web/server.go | 7 + .../internal/web/templates/recovery.html | 7 +- 12 files changed, 778 insertions(+), 20 deletions(-) create mode 100644 controller/internal/web/recovery_gate_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3809f42..2719cee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,63 @@ +## v0.201.0 — a correct recovery code is never called wrong again (2026-08-05, CAMPAIGN-11) — MinAgent 0.125.0 + +CAMPAIGN-11 walked the whole recovery journey end to end for the first time. **The data came back +byte-identical; the journey did not exist** — four operator interventions stood between a customer and +their files, and the first one was the machine telling them their perfectly correct recovery code was +wrong. This closes six of the nine findings. + +**R-216 — the headline. A 404 was reported to the customer as a bad code.** The unseal lives in the +agent behind `POST /escrow/recover-offsite-password`, which ships in agent **v0.125.0**. On an older +agent the route 404s, and the unlock was attempted anyway: *„A megadott helyreállítási kódot nem +fogadtuk el. Ellenőrizd, hogy mind a tíz szót pontosan…"* — measured live in 0.134 s, far too fast for +`age`'s scrypt, against a code that was perfect. **It was the default state, not a misconfiguration**: +the day-0 manifest vouches agent 0.120.0, and a reinstall actively *downgrades* a hand-fixed box back +to it. Three parts: + +- `FeatureOffsiteKeyRecovery` now exists in `featureProbes` + `featureMinAgent` (0.125.0) with a + `Supports` gate at the unlock entry point — **the convention `features.go:72-76` and + `publish-train-rules.md` rule 4 already required, which v0.195.0/v0.196.0 declared in their headers + and v0.200.0 dropped when it put the same coupled feature behind a customer-facing button.** +- **This gate FAILS CLOSED**, alone in that table: anything but `SupportYes` says *the machine* cannot + ask yet, and no unlock is attempted. An attempt that cannot succeed must never be made, because its + failure is what got attributed to the code. The asymmetry is documented beside the row. +- The hub's own guard could not catch it (see the hub changelog, v0.97.0). + +**R-218 — succeeding at recovery stopped the box asking for what it still needed.** +`needsOffsiteCredential` short-circuited on a repository password existing — and installing one is the +recovery screen's whole job. 32 seconds after the hub re-staged the credential, the customer's success +switched off the mechanism that would have delivered the coordinates for the key they had just +recovered. **The short-circuit is deleted**; the declaration now stops when the tier works, not when a +key exists. Scenario E (a deliberately disabled target stays silent) is pinned by its own test. + +**R-219 — the promised listing could never render on the shape the screen exists for.** Listing needs +a target; a target cannot exist without a repository password; shape (a) is defined by having none. The +unlock now **finishes the job** — place the key, bring the tier up, then list — and a tier that is not +up yet says *waiting for the connection details* rather than reporting a failure. + +**R-217 — an unreadable store claimed to have opened.** The failure path passed +`backup.OffsiteInventory{}`, whose `Empty=false` the template read as *„A tároló megnyílt, és van benne +tartalom, de nem tudtuk alkalmazásokhoz rendelni."* — three assertions, none of them known. The field +built to prevent exactly this says so in its own doc comment. Opened / empty / unreadable are now three +distinguishable states. + +**R-222 — reaching for a retained earlier package read as a wrong code.** The hub's ACK carries +`superseded_present` / `superseded_at` (hub v0.97.0); the screen now names that situation. It states +the two facts the hub knows and **does not promise the earlier package can be opened** — that read path +does not exist. + +**R-215 — `GET /recovery` rendered the recovery story on a box that never had backups.** The predicate +was right and the page never asked it. Gated on the same predicate as the interception. + +**Four messages where there was one**, each saying what to do next and only one mentioning typing. + +Left open deliberately: **R-214** (the console never stops showing a stale pairing code), **R-220** +(drives cannot be re-enrolled after a rebuild), **R-221** (a rebuilt box cannot run the escrow ceremony +— a real blocker for that flow). + +Tests: `recovery_gate_test.go` (Scenarios A/F/G/H/I, handler-level), `offbox_declare_test.go` +(Scenarios D/E). **Five red-proofs, each demonstrated failing and restored** — removing the capability +gate brings the accusation back verbatim. + ## v0.200.0 — the recovery screen: unlocking, and only unlocking (2026-08-05, R-193) R-193's remaining half. The drill proved a customer's file comes back after a machine is destroyed; diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index bda22cb..b9f2507 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -494,6 +494,10 @@ func main() { // updater's floor/update-running state and defer the one-time-password consume past a managed // day-0 floor-update (the F10 race). The gate is wired ONLY when an updater exists — with no update // mechanism there is no floor-update to race, so the bridge reconciles immediately (Settle nil). + // offsiteBridge is hoisted out of the block below ONLY so the recovery screen can drive one + // reconcile between placing a recovered key and reading the repository (R-219). nil when off-site + // is not configured for this customer, which the web seam treats as "skip". + var offsiteBridge *offsiteapply.Bridge if backupMgr != nil && cfg.Offsite.Enabled && cfg.Hub.URL != "" && cfg.Hub.APIKey != "" { bridge := &offsiteapply.Bridge{ Cfg: cfg, @@ -526,6 +530,7 @@ func main() { return Version, floor, u.IsUpdateRunning(), floor != "" }) } + offsiteBridge = bridge go func() { // ReconcileWhenSettled runs the settle-gate FIRST (its own bounds), then Reconcile under a // fresh 3-minute context — the gate's wait never eats the reconcile budget. @@ -670,7 +675,10 @@ func main() { // is precisely the case that needs it. Half of the stranded-rebuild predicate in // backup.needsOffsiteCredential; the other half (no repository password) is local. RecordPresence: sett.SetHubEscrowIdentityPresent, - Logger: logger, + // 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, } // Wire hub verification: update settings when hub reports customer status hubPusher.OnPushResponse = func(resp *report.PushResponse) { @@ -1268,6 +1276,13 @@ func main() { // before a code is entered. A timestamp, never a secret. webServer.SetEscrowSealedAt(escrowConfirmer.SealedAt) } + // R-219 / R-218: the unlock brings the off-site tier up between placing the recovered key and + // reading the repository. Without it the listing the screen promises can never render on the + // pristine rebuilt box — no key ⇒ no target ⇒ no inventory — and the apply-bridge's only other + // trigger is a config refresh or a restart, neither of which the customer can cause. + if offsiteBridge != nil { + webServer.SetRecoveryTierUp(offsiteBridge.Reconcile) + } webServer.SetIntegrationManager(integrationMgr) if reportTrigger != nil { // Out-of-cycle report push after hub-relevant user actions (escrow claim, settings diff --git a/controller/internal/agentapi/features.go b/controller/internal/agentapi/features.go index b296b35..9844ed3 100644 --- a/controller/internal/agentapi/features.go +++ b/controller/internal/agentapi/features.go @@ -35,6 +35,26 @@ const FeatureGuestMemoryResize Feature = "guest_memory_resize" // an empty field means legacy. const FeatureBackupAgeState Feature = "backup_age_state" +// FeatureOffsiteKeyRecovery is the customer-facing off-site key recovery (agent v0.125.0, R-199 +// links 7–8): POST /escrow/recover-offsite-password fetches this host's sealed bundle, unseals it +// with R and returns the single repository-password field. +// +// ⚠ THIS GATE FAILS CLOSED, and it is the ONLY feature in this table that does. Read §7.1 of the +// R-216 fix before "correcting" it back to the package default. +// +// The package default is fail-OPEN: SupportUnknown proceeds, because for every other coupled feature +// a wrong "unsupported" would block something harmless while a down agent already speaks through the +// normal error paths. **That default is what produced R-216.** Measured live on 2026-08-05 +// (CAMPAIGN-11 Phase 1): an agent 0.120.0 answered the recovery route with 404, the unlock attempt +// went ahead anyway, and the customer was told — in Hungarian, on the one screen whose whole purpose +// is to be believed about backups — that their perfectly correct recovery code was not accepted and +// they should check their typing. A correct code, refused in 0.134 s, blamed on the customer. +// +// So here: anything other than SupportYes means the screen says THE MACHINE cannot ask yet. The +// unlock is never attempted when it cannot complete, because the failure of an attempt that could +// never have worked is attributed to the code. +const FeatureOffsiteKeyRecovery Feature = "offsite_key_recovery" + // SupportState is a probe verdict. The zero value is SupportUnknown (fail-open: unknown never // refuses — the existing agent-error paths speak honestly when the agent is down). type SupportState int @@ -92,12 +112,27 @@ var featureProbes = map[Feature]func(ctx context.Context, p SupportProber) error _, err := gm.GuestMemory(ctx) return err }, + // The recovery route is a POST that performs work and consumes a recovery code — it cannot be + // probed. Like the memory prober's negative case this returns a sentinel that classifies to + // SupportUnknown, so the decision falls to the VERSION path above. + // + // The row must exist even though it cannot probe: SupportsWithSource looks up featureProbes + // FIRST and returns "unregistered"/SupportUnknown on a table gap, before the version path runs. + // A featureMinAgent row without a featureProbes row is therefore never consulted at all. + FeatureOffsiteKeyRecovery: func(ctx context.Context, p SupportProber) error { + return errNoRecoveryProbe + }, } // errNoMemoryProbe classifies to SupportUnknown (not a *StatusError 404), so a prober that cannot be // asked never reads as "unsupported". var errNoMemoryProbe = errors.New("agentapi: prober does not support the guest-memory probe") +// errNoRecoveryProbe classifies to SupportUnknown: the off-site key recovery route is a POST that +// consumes a recovery code and so cannot be probed, leaving the VERSION path to decide. Its caller +// fails CLOSED on Unknown — see FeatureOffsiteKeyRecovery. +var errNoRecoveryProbe = errors.New("agentapi: the offsite key recovery route cannot be probed") + // featureMinAgent maps each coupled feature to the MINIMUM agent version that carries its coupled // semantics (the CHANGELOG `MinAgent:` header value). Used by Supports when the agent's version is // KNOWN (the v0.82.0 X-Felhom-Agent-Version channel) — a direct comparison, no probe traffic. A @@ -107,8 +142,16 @@ var featureMinAgent = map[Feature]string{ FeatureGuestMemoryResize: "0.90.0", // R-88 Part 2: /backup/due carries age_state, distinguishing "never backed up" from "cannot tell". FeatureBackupAgeState: "0.105.0", + // R-199 links 7–8: POST /escrow/recover-offsite-password. R-216 — this row is the whole reason a + // correct recovery code can no longer be reported as wrong on an agent that cannot answer. + FeatureOffsiteKeyRecovery: "0.125.0", } +// MinAgentFor returns the declared minimum agent version for a feature ("" when the feature has no +// row). Read-only accessor over featureMinAgent so a refusal can NAME the version it needs instead of +// hard-coding the number a second time at the call site. +func MinAgentFor(f Feature) string { return featureMinAgent[f] } + // AgentVersionReporter is optionally implemented by a SupportProber (*Client is one): it reports // the last strictly-validated agent version seen on its traffic ("" = unknown → probe fallback). type AgentVersionReporter interface { diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index cd38119..2e5375d 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -1228,6 +1228,32 @@ const OffsiteStateNeedsCredential = "needs_credential" // it". A target that exists but is DISABLED is not stranded either — the customer switched it off — // so the caller only consults this when there is no enabled target, and a non-nil disabled target // short-circuits to false here. +// +// ⚠ THE DECLARATION STOPS WHEN THE TIER WORKS, NOT WHEN A KEY EXISTS (R-218, v0.201.0). +// +// This used to carry a third condition: hold a repository password ⇒ not stranded, stop declaring. +// It reads as a sound freshness test and it is the exact opposite on the one path that matters, +// because installing a repository password is the RECOVERY SCREEN'S WHOLE JOB. Measured live on +// 2026-08-05 (CAMPAIGN-11 Phase 1): +// +// 13:39:54 needs_credential the box asks +// 13:42:43 needs_credential the hub's 2-report debounce is satisfied +// 13:47:03 hub re-stages the credential — "the box re-consumes on its next cycle" +// 13:47:35 the customer's unlock succeeds and places the recovered key +// 13:53:42 (silence) …and never asks again +// +// Thirty-two seconds after the remedy fired, the customer's own success switched off the mechanism +// that would have delivered the coordinates for the key they had just recovered. The hub held an +// unconsumed credential the box had no reason to collect; the box held a correct key and nowhere to +// use it; the screen said "a few minutes"; nothing was ever going to happen. Two features, each +// correct alone, cancelling on the path both were built for. +// +// The two remaining conditions are the honest ones: no target at all, and the hub is keeping a sealed +// package for us. Both stay true exactly until the tier is configured — which is the moment the box +// genuinely no longer needs a credential — and `OffboxReportStatus` stops consulting this predicate +// the instant a target exists. **Scenario E is unaffected and is pinned by its own test**: a disabled +// target is non-nil and still short-circuits at the first line, so a box whose customer switched +// off-site off stays silent. func (m *Manager) needsOffsiteCredential(t *settings.OffboxTarget) bool { if t != nil { return false // a target exists (merely disabled) — the customer's own choice, not a rebuild @@ -1235,9 +1261,6 @@ func (m *Manager) needsOffsiteCredential(t *settings.OffboxTarget) bool { if m.settings == nil || !m.settings.GetHubEscrowIdentityPresent() { return false // the hub holds nothing for us: never had off-site backups } - if _, ok := m.OffboxRepoPasswordHash(); ok { - return false // we still hold our repository password: not a fresh data area - } return true } diff --git a/controller/internal/backup/offbox_declare_test.go b/controller/internal/backup/offbox_declare_test.go index 4685812..c3388ae 100644 --- a/controller/internal/backup/offbox_declare_test.go +++ b/controller/internal/backup/offbox_declare_test.go @@ -88,23 +88,60 @@ func TestOffsiteDeclare_NeverHadOffsiteSaysNothing(t *testing.T) { } } -// The other half of the conjunction: a box that still holds its repository password is NOT stranded, -// even though the hub holds an escrow for it. That is simply a healthy box between configurations. -func TestOffsiteDeclare_BoxThatStillHoldsItsRepoPasswordDoesNotDeclare(t *testing.T) { +// SCENARIO D (R-218) — THE DECLARATION STOPS WHEN THE TIER WORKS, NOT WHEN A KEY EXISTS. +// +// ⚠ THIS TEST ASSERTED THE OPPOSITE until v0.201.0, and it was green the whole time. It required a +// box holding a repository password to stay SILENT — which reads as a sound freshness test and is the +// exact opposite on the one path that matters, because installing a repository password is the +// RECOVERY SCREEN'S WHOLE JOB. Measured live 2026-08-05 (CAMPAIGN-11 Phase 1): 32 seconds after the +// hub re-staged the credential, the customer's successful unlock switched off the mechanism that +// would have delivered the coordinates for the key they had just recovered. Deadlock, both halves. +// +// RED-PROOF: restore the `if _, ok := m.OffboxRepoPasswordHash(); ok { return false }` short-circuit +// in needsOffsiteCredential and this test FAILS — the box goes silent again with no target, which is +// the deadlock. Demonstrated failing before this test was kept. +func TestOffsiteDeclare_StillDeclaresAfterARecoveredKeyIsPlaced(t *testing.T) { m, sett := bareManager(t) if err := sett.SetHubEscrowIdentityPresent(true); err != nil { t.Fatal(err) } - // Place a repository password exactly where the manager looks for it. + // The post-unlock shape: the recovered repository password is on disk, and there is STILL no + // off-site target — so the box cannot use what it just recovered. if err := os.MkdirAll(m.offboxDir(), 0o700); err != nil { t.Fatal(err) } - if err := os.WriteFile(m.offboxPwPath(), []byte("a-repository-password"), 0o600); err != nil { + if err := os.WriteFile(m.offboxPwPath(), []byte("a-recovered-repository-password"), 0o600); err != nil { t.Fatal(err) } - if st := m.OffboxReportStatus(); st != nil { - t.Fatalf("a box holding its repository password declared a need: %+v", st) + st := m.OffboxReportStatus() + if st == nil { + t.Fatal("R-218: the box went SILENT after recovering its key while still having no off-site target — the hub's staged credential is never collected and nothing ever asks again") + } + if st.State != OffsiteStateNeedsCredential { + t.Fatalf("declared state = %q, want %q", st.State, OffsiteStateNeedsCredential) + } +} + +// SCENARIO E — and once the tier ACTUALLY WORKS the box goes quiet. This is the condition that +// replaces the deleted one, and the pair above/below is what makes the deletion safe. +func TestOffsiteDeclare_ConfiguredTierIsSilent(t *testing.T) { + m, sett := bareManager(t) + if err := sett.SetHubEscrowIdentityPresent(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) + } + st := m.OffboxReportStatus() + if st == nil { + t.Fatal("a configured tier must still report its ordinary off-site object") + } + if st.State == OffsiteStateNeedsCredential { + t.Fatal("a box whose tier is configured must not keep asking for a credential") } } diff --git a/controller/internal/report/escrow_confirm.go b/controller/internal/report/escrow_confirm.go index c80fde3..03c595a 100644 --- a/controller/internal/report/escrow_confirm.go +++ b/controller/internal/report/escrow_confirm.go @@ -20,6 +20,11 @@ type EscrowStatus struct { IdentityBlobPresent bool `json:"identity_blob_present"` ResticPwSHA256 string `json:"restic_pw_sha256"` CreatedAt string `json:"created_at"` + // SupersededPresent / SupersededAt (v0.201.0, R-222) — the hub is ALSO keeping an earlier sealed + // package, and when it was set aside. Absent on a pre-0.97.0 hub, which reads as "no earlier + // package" and simply keeps today's message: an older hub cannot make the screen say anything new. + SupersededPresent bool `json:"superseded_present"` + SupersededAt string `json:"superseded_at"` } // EscrowAutoConfirmer runs the auto-confirm check on each report ACK. Long-lived (one per process) so @@ -54,7 +59,13 @@ type EscrowAutoConfirmer struct { // immediately, so the one fact that distinguishes a REBUILT box from a box that never had // off-site backups was thrown away on every cycle. nil → not recorded (older wiring, tests). RecordPresence func(present bool) error - Logger *log.Logger + // RecordSuperseded persists the ACK's `superseded_present` / `superseded_at` (v0.201.0, R-222) — + // whether the hub is ALSO keeping an EARLIER sealed package for this box. Wired here for the same + // reason as RecordPresence: this is the one place the ACK's escrow object already arrives, and a + // 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 mu sync.Mutex warnedHash string // last mismatched hub hash we warned about (dedupe; shared by both branches) @@ -111,6 +122,14 @@ func (c *EscrowAutoConfirmer) Reconcile(es *EscrowStatus) { c.logf("[WARN] [escrow-confirm] could not record the hub's identity-blob presence (present=%v): %v", es.IdentityBlobPresent, err) } } + // Same discipline, same place, same reason (R-222): recorded unconditionally, because the box that + // needs it most is the unconfigured rebuilt one every gate below skips. Failure is logged, never + // swallowed, and never blocks the auto-confirm. + if c.RecordSuperseded != nil { + if err := c.RecordSuperseded(es.SupersededPresent, es.SupersededAt); err != nil { + c.logf("[WARN] [escrow-confirm] could not record the hub's superseded-package state (present=%v): %v", es.SupersededPresent, err) + } + } c.mu.Lock() c.sealedAt = es.CreatedAt // in-memory only; a timestamp, never a secret c.mu.Unlock() diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index 55b1e04..e697278 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -70,6 +70,22 @@ type Settings struct { // first ACK — which is correct, because the hub is the authority on what the hub holds. HubEscrowIdentityPresent bool `json:"hub_escrow_identity_present,omitempty"` + // HubEscrowSupersededPresent / HubEscrowSupersededAt (v0.201.0, R-222) cache the report ACK's + // `escrow.superseded_present` / `superseded_at` — whether the hub is ALSO keeping an EARLIER + // sealed package for this box, and when it was set aside. + // + // They exist for one message and nothing else. Without them the recovery screen cannot tell a + // genuinely wrong recovery code from a code that is RIGHT about an earlier package, because both + // look identical from here: the unseal fails closed against the package the hub currently serves. + // Measured on 2026-08-05 (CAMPAIGN-11 Phase 3 step 7) — the customer entered the correct code for + // their orphaned history and was told to check their typing. + // + // ⚠ These grant NO read path. The hub serves the CURRENT package only; retrieving a superseded one + // is a link that has never been built (R-199's inventory). The screen may say an earlier package is + // kept and when — both true, both non-secret — and must not promise it can be opened. + HubEscrowSupersededPresent bool `json:"hub_escrow_superseded_present,omitempty"` + HubEscrowSupersededAt string `json:"hub_escrow_superseded_at,omitempty"` + // 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 @@ -647,6 +663,28 @@ func (s *Settings) SetHubEscrowIdentityPresent(present bool) error { return s.save() } +// GetHubEscrowSuperseded reports whether the hub is keeping an EARLIER sealed package for this box, +// and when it was set aside (R-222). Both zero until an ACK has said so. +func (s *Settings) GetHubEscrowSuperseded() (present bool, at string) { + s.mu.RLock() + defer s.mu.RUnlock() + return s.HubEscrowSupersededPresent, s.HubEscrowSupersededAt +} + +// SetHubEscrowSuperseded records the ACK's `escrow.superseded_present` / `superseded_at`. Same +// last-write-wins mirror discipline as SetHubEscrowIdentityPresent — the hub is the authority, and a +// customer RESET that clears the retained rows must be able to turn the box's message back off. +// Saves only on a change. +func (s *Settings) SetHubEscrowSuperseded(present bool, at string) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.HubEscrowSupersededPresent == present && s.HubEscrowSupersededAt == at { + return nil + } + s.HubEscrowSupersededPresent, s.HubEscrowSupersededAt = present, at + return s.save() +} + // ── Recovery screen (v0.200.0, R-193) ────────────────────────────────────────── // GetRecoveryNoticePostponed reports whether the customer chose "most nem" on the recovery page. diff --git a/controller/internal/web/recovery_gate_test.go b/controller/internal/web/recovery_gate_test.go new file mode 100644 index 0000000..90cb2b9 --- /dev/null +++ b/controller/internal/web/recovery_gate_test.go @@ -0,0 +1,344 @@ +package web + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// CAMPAIGN-11 fixes — R-216, R-217, R-219, R-222, R-215. +// +// HANDLER-LEVEL throughout. Every assertion below is on the RENDERED PAGE or an on-disk effect, +// because each of these defects lived in the handler while a helper-level test looked green: R-215's +// predicate was correct and its page never asked, and R-217's failure branch built a struct whose +// zero value the TEMPLATE misread. A test that stops at the helper observes neither. + +// postUnlockWith drives the real unlock handler with a code. +func postUnlockWith(t *testing.T, s *Server, code string) *httptest.ResponseRecorder { + t.Helper() + form := url.Values{"recovery_code": {code}} + req := httptest.NewRequest(http.MethodPost, "/recovery/unlock", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rr := httptest.NewRecorder() + s.recoveryUnlockHandler(rr, req) + return rr +} + +// blamesTyping reports whether the page tells the customer to check how they typed their code. It is +// the ONE sentence that may only ever appear for a genuinely wrong code. +func blamesTyping(body string) bool { + return strings.Contains(body, "z szót pontosan") || strings.Contains(body, "nem fogadtuk el") +} + +// ── SCENARIO A — an agent that cannot answer is NAMED, never the customer's code (R-216) ───────── +// +// THE HEADLINE. Measured live 2026-08-05: agent 0.120.0 404s the recovery route, the unlock was +// attempted anyway, and a CORRECT code came back as "we did not accept your recovery code, check +// that all ten words…" in 0.134 s. +// +// RED-PROOF: delete the `if support := s.recoverySupport(...)` block from recoveryUnlockHandler and +// this test FAILS on the blamesTyping assertion — the accusation returns verbatim. Demonstrated +// failing before this test was kept. +func TestRecoveryGate_A_OldAgentIsNamed_NeverTheCode(t *testing.T) { + for _, tc := range []struct { + name string + state agentapi.SupportState + }{ + {"agent predates the route (404)", agentapi.SupportNo}, + {"agent cannot be asked at all", agentapi.SupportUnknown}, // fail-CLOSED: §7.1 + } { + t.Run(tc.name, func(t *testing.T) { + f := newRecoveryFixture(t) + f.s.SetRecoverySupport(func(context.Context) agentapi.SupportState { return tc.state }) + // Model the REAL agent: one that cannot answer the route also fails the call. Without + // this the fake would succeed and the red-proof could not show the accusation returning — + // the mutation would fail the test for weaker reasons than the finding itself. + f.rec.fail = true + + rr := postUnlockWith(t, f.s, testRecoveryCode) + if rr.Code != http.StatusOK { + t.Fatalf("unlock = %d", rr.Code) + } + body := rr.Body.String() + + // EFFECT 1 — the customer is NOT blamed. This is the whole finding. + if blamesTyping(body) { + t.Fatal("R-216 RETURNED: an agent that cannot answer is still reported as a wrong recovery code") + } + // EFFECT 2 — the machine is named as the thing that cannot do it yet. + if !strings.Contains(body, "Ez a gép még nem tudja") { + t.Errorf("the refusal must say THE MACHINE cannot do this yet; got: %q", firstAlert(body)) + } + // EFFECT 3 — it says the code is fine and was not consumed, so the customer keeps it. + if !strings.Contains(body, "kódoddal semmi baj") { + t.Error("the refusal must reassure the customer that their code is fine") + } + // EFFECT 4 — no attempt was made. An attempt that cannot succeed must never be made, + // because its failure is what got attributed to the code. + if n := len(f.rec.codes); n != 0 { + t.Fatalf("the handler attempted an unlock it could not complete (%d attempt(s))", n) + } + }) + } +} + +// The supported case still works — the gate must not refuse everything. +func TestRecoveryGate_A_SupportedAgentProceeds(t *testing.T) { + f := newRecoveryFixture(t) + rr := postUnlockWith(t, f.s, testRecoveryCode) + if rr.Code != http.StatusOK { + t.Fatalf("unlock = %d", rr.Code) + } + if len(f.rec.codes) != 1 { + t.Fatalf("a supported agent must be asked exactly once, got %d", len(f.rec.codes)) + } + if _, ok := f.mgr.OffboxRepoPasswordHash(); !ok { + t.Fatal("the recovered key was not placed") + } +} + +// The declared coupling itself: the table must carry the row, at the version that ships the route. +func TestRecoveryGate_A_CouplingIsDeclared(t *testing.T) { + if got := agentapi.MinAgentFor(agentapi.FeatureOffsiteKeyRecovery); got != "0.125.0" { + t.Fatalf("FeatureOffsiteKeyRecovery MinAgent = %q, want 0.125.0 (the version shipping POST /escrow/recover-offsite-password)", got) + } +} + +// ── SCENARIO G — opened / empty / unreadable are THREE sentences (R-217) ───────────────────────── +// +// RED-PROOF: replace `s.renderRecoveryUnlockedNoList(w, r, msg)` with +// `s.renderRecovery(w, r, msg, "", &backup.OffsiteInventory{})` and this test FAILS — the zero value's +// Empty=false falls through to .InvUntagged and the page claims the store opened with content it +// could not attribute. Demonstrated failing before this test was kept. +func TestRecoveryGate_G_UnreadableStoreNeverClaimsToHaveOpened(t *testing.T) { + f := newRecoveryFixture(t) + // The tier is up, but reading the repository fails. + f.mgr.SetOffboxRunner(func(context.Context, []string, ...string) ([]byte, error) { + return nil, context.DeadlineExceeded + }) + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + + if strings.Contains(body, "van benne tartalom") { + t.Fatal("R-217 RETURNED: an UNREADABLE store is reported as opened, with unattributable content") + } + if strings.Contains(body, "nincs benne egyetlen") { + t.Fatal("an unreadable store must not be reported as opened-and-empty either") + } + // It says the key is safe and what is pending — no claim about the contents. + if !strings.Contains(body, "kulcs visszakerült") { + t.Errorf("the customer must be told the key came back; got: %q", firstAlert(body)) + } +} + +// The three states must be DISTINGUISHABLE, which is the actual requirement. +func TestRecoveryGate_G_ThreeDistinctStates(t *testing.T) { + // (1) opened, with apps. + f1 := newRecoveryFixture(t) + f1.runner.snapshots = []map[string]any{{"short_id": "aaa1111", "time": "2026-08-01T10:00:00Z", "tags": []string{"immich"}}} + b1 := postUnlockWith(t, f1.s, testRecoveryCode).Body.String() + + // (2) opened, empty. + f2 := newRecoveryFixture(t) + f2.runner.snapshots = nil + b2 := postUnlockWith(t, f2.s, testRecoveryCode).Body.String() + + // (3) could not be read. + f3 := newRecoveryFixture(t) + f3.mgr.SetOffboxRunner(func(context.Context, []string, ...string) ([]byte, error) { + return nil, context.DeadlineExceeded + }) + b3 := postUnlockWith(t, f3.s, testRecoveryCode).Body.String() + + if !strings.Contains(b1, "immich") { + t.Error("state 1 (opened, with apps) must list the app") + } + if !strings.Contains(b2, "nincs benne egyetlen") { + t.Error("state 2 (opened, empty) must say so explicitly") + } + if strings.Contains(b3, "nincs benne egyetlen") || strings.Contains(b3, "van benne tartalom") { + t.Error("state 3 (unreadable) must claim nothing about the contents") + } +} + +// ── SCENARIO H — the retained earlier package is NAMED, not blamed on the customer (R-222) ─────── +func TestRecoveryGate_H_SupersededPackageIsNamed(t *testing.T) { + f := newRecoveryFixture(t) + f.rec.fail = true // the unseal fails against the CURRENT package — the real engine behaviour + if err := f.sett.SetHubEscrowSuperseded(true, "2026-08-05T15:03:14Z"); err != nil { + t.Fatal(err) + } + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + + if blamesTyping(body) { + t.Fatal("R-222 RETURNED: a code that is RIGHT about a retained earlier package is blamed on the customer's typing") + } + if !strings.Contains(body, "nem töröltük") { + t.Errorf("the customer must be told the earlier package is kept; got: %q", firstAlert(body)) + } + if !strings.Contains(body, "2026-08-05T15:03:14Z") { + t.Error("when the hub knows WHEN the earlier package was set aside, say so") + } + // §7.6 — it may state the facts and must NOT promise the earlier package can be opened. + for _, forbidden := range []string{"vissza tudod állítani", "megnyithatod", "vissza fogod kapni"} { + if strings.Contains(body, forbidden) { + t.Errorf("the message promises the earlier package can be opened (%q) — the read path does not exist", forbidden) + } + } +} + +// And with NO retained package the wrong-code message is unchanged — the only one mentioning typing. +func TestRecoveryGate_H_WithoutASupersededPackageItIsStillTheWrongCodeMessage(t *testing.T) { + f := newRecoveryFixture(t) + f.rec.fail = true + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + if !blamesTyping(body) { + t.Fatal("a genuinely wrong code must still get the wrong-code message") + } + if strings.Contains(body, "nem töröltük") { + t.Fatal("a box with no retained earlier package must not claim one exists") + } +} + +// ── SCENARIO I — /recovery is not reachable by typing the path (R-215) ────────────────────────── +// +// RED-PROOF: delete the `if !s.recoveryOffer()` guard from recoveryPageHandler and this test FAILS — +// a box that never had off-site backups renders "this machine was reinstalled, your off-site backups +// are there". Demonstrated failing before this test was kept. +func TestRecoveryGate_I_DirectGetRefusedOnABoxThatNeverHadBackups(t *testing.T) { + f := newRecoveryFixture(t) + // The defining fact: the hub holds nothing for this box. + if err := f.sett.SetHubEscrowIdentityPresent(false); err != nil { + t.Fatal(err) + } + if f.s.recoveryOffer() { + t.Fatal("fixture error: the box still reads as offered") + } + + rr := getRecoveryPage(t, f.s) + if rr.Code == http.StatusOK && strings.Contains(rr.Body.String(), "jratelep") { + t.Fatal("R-215 RETURNED: a box that never had off-site backups renders the recovery story on a direct GET") + } + if rr.Code != http.StatusFound { + t.Errorf("a direct GET on a non-offered box should redirect away, got %d", rr.Code) + } +} + +// A box that IS in the situation still reaches the page directly — including after "most nem", +// which is what makes the permanent backups-area entry point work at all. +func TestRecoveryGate_I_DirectGetStillWorksWhenOffered(t *testing.T) { + f := newRecoveryFixture(t) + if err := f.sett.SetRecoveryNoticePostponed(true); err != nil { + t.Fatal(err) + } + if f.s.recoveryInterrupts() { + t.Fatal("fixture error: postpone did not suppress the interruption") + } + rr := getRecoveryPage(t, f.s) + if rr.Code != http.StatusOK { + t.Fatalf("an offered box must still reach /recovery directly after a postpone, got %d", rr.Code) + } +} + +// ── SCENARIO F — the unlock finishes the job: key placed, tier brought up, then the listing ────── +func TestRecoveryGate_F_UnlockBringsTheTierUpBeforeListing(t *testing.T) { + f := newRecoveryFixture(t) + // The pristine rebuilt shape: no coordinates at all, so the listing is impossible until the tier + // comes up. Removing the target is what makes this the shape (a) the screen exists for. + if err := f.sett.SetOffboxTarget(nil); err != nil { + t.Fatal(err) + } + called := 0 + f.s.SetRecoveryTierUp(func(context.Context) error { + called++ + // Bringing the tier up = coordinates exist afterwards. + return f.sett.SetOffboxTarget(&settings.OffboxTarget{ + Enabled: true, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo", + Schedule: "daily", EscrowState: "escrowed", + }) + }) + f.runner.snapshots = []map[string]any{{"short_id": "bbb2222", "time": "2026-08-02T09:00:00Z", "tags": []string{"calibre-web"}}} + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + + if called != 1 { + t.Fatalf("the unlock must bring the tier up exactly once, got %d", called) + } + if !strings.Contains(body, "calibre-web") { + t.Fatalf("the listing the screen PROMISES did not render; got: %q", firstAlert(body)) + } +} + +// A tier that cannot come up yet is PENDING, not a failure, and never a wrong code. +func TestRecoveryGate_F_TierNotUpYetSaysPendingNotFailed(t *testing.T) { + f := newRecoveryFixture(t) + if err := f.sett.SetOffboxTarget(nil); err != nil { + t.Fatal(err) + } + f.s.SetRecoveryTierUp(func(context.Context) error { return context.DeadlineExceeded }) + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + + if blamesTyping(body) { + t.Fatal("a tier that has not come up yet must never read as a wrong code") + } + if !strings.Contains(body, "kapcsolódási adatait") { + t.Errorf("say what is pending — the connection details; got: %q", firstAlert(body)) + } + // The key IS placed regardless: nothing failed. + if _, ok := f.mgr.OffboxRepoPasswordHash(); !ok { + t.Fatal("the recovered key must be placed even when the tier cannot come up yet") + } +} + +// firstAlert extracts the first rendered alert body, for readable failure output. +func firstAlert(body string) string { + i := strings.Index(body, "alert-error") + if i < 0 { + i = strings.Index(body, "alert-info") + } + if i < 0 { + return "(no alert rendered)" + } + seg := body[i:] + if j := strings.Index(seg, ""); j > 0 { + seg = seg[:j] + } + if len(seg) > 400 { + seg = seg[:400] + } + return strings.TrimSpace(seg) +} + +// dataDirHasNoCode asserts the recovery code never reached disk — the sweep, with a positive control +// in the caller. Kept here because every path above handles a code. +func dataDirHasNoCode(t *testing.T, dir, code string) { + t.Helper() + var hits []string + _ = filepath.Walk(dir, func(p string, info os.FileInfo, err error) error { + if err != nil || info == nil || info.IsDir() { + return nil + } + b, rerr := os.ReadFile(p) + if rerr == nil && strings.Contains(string(b), code) { + hits = append(hits, p) + } + return nil + }) + if len(hits) > 0 { + t.Fatalf("the recovery code was written to disk: %v", hits) + } +} + +var _ = backup.HashResticPassword diff --git a/controller/internal/web/recovery_handlers.go b/controller/internal/web/recovery_handlers.go index 699edf9..04d0c20 100644 --- a/controller/internal/web/recovery_handlers.go +++ b/controller/internal/web/recovery_handlers.go @@ -5,6 +5,7 @@ import ( "net/http" "time" + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" "gitea.dooplex.hu/admin/felhom-controller/internal/backup" ) @@ -62,7 +63,23 @@ func recoveryNoStore(w http.ResponseWriter) { // recoveryPageHandler renders the screen (GET /recovery). Reachable whenever the situation holds — // including after "most nem", which is how the backups-area entry point can point at it forever. +// +// ⚠ GATED ON THE SAME PREDICATE AS THE INTERCEPTION (R-215, v0.201.0). It was not, and the page +// rendered its whole story — *"Ezt a gépet újratelepítették. A korábbi, házon kívüli mentéseid +// megvannak"* — on any claimed box, including one installed hours earlier that had never had an +// off-site backup in its life. Measured on 2026-08-05 (CAMPAIGN-11 §F9): the hub held no escrow row +// for that host at all, so `recoveryOffer()` was correctly false; the page simply never asked it. +// +// The predicate was right, the POST sibling below already consulted it, and the backups-area template +// gates the identical sentence on `.RecoveryOffer` — this GET was the one door left open. A box with +// no link to /recovery is only reached by typing the path or following a stale bookmark, but what it +// then asserts is false in two ways at once, on the one screen whose purpose is to be believed about +// backups. func (s *Server) recoveryPageHandler(w http.ResponseWriter, r *http.Request) { + if !s.recoveryOffer() { + http.Redirect(w, r, "/backups/remote", http.StatusFound) + return + } s.renderRecovery(w, r, "", "", nil) } @@ -70,6 +87,19 @@ func (s *Server) recoveryPageHandler(w http.ResponseWriter, r *http.Request) { // read-only listing). errorMsg/flash are already-customer-facing Hungarian; NEITHER EVER CONTAINS THE // CODE — see unlockHandler. func (s *Server) renderRecovery(w http.ResponseWriter, r *http.Request, errorMsg, flash string, inv *backup.OffsiteInventory) { + s.renderRecoveryState(w, r, errorMsg, flash, inv, false) +} + +// renderRecoveryUnlockedNoList is the R-217 shape: the unlock SUCCEEDED but the repository could not +// be listed. It renders the unlocked page with NO listing block at all — never the zero-value +// inventory, whose Empty=false made the template claim the store had opened with unattributable +// content. "Opened with apps", "opened and empty" and "could not be read" are three states and this +// is the third. +func (s *Server) renderRecoveryUnlockedNoList(w http.ResponseWriter, r *http.Request, errorMsg string) { + s.renderRecoveryState(w, r, errorMsg, "", nil, true) +} + +func (s *Server) renderRecoveryState(w http.ResponseWriter, r *http.Request, errorMsg, flash string, inv *backup.OffsiteInventory, unlockedNoList bool) { recoveryNoStore(w) data := s.baseData("recovery", "Adatok visszaszerzése") data["CSRFField"] = s.csrfField(r) @@ -87,7 +117,13 @@ func (s *Server) renderRecovery(w http.ResponseWriter, r *http.Request, errorMsg data["Unlocked"] = true data["InvApps"] = inv.Apps data["InvEmpty"] = inv.Empty + // InvUntagged is "the repository OPENED and holds snapshots we could not attribute to an app". + // It is only ever computed from a real reading — a failed read reaches the branch below and + // renders no listing at all (R-217). data["InvUntagged"] = !inv.Empty && len(inv.Apps) == 0 + } else if unlockedNoList { + data["Unlocked"] = true + data["InvUnavailable"] = true } w.Header().Set("Content-Type", "text/html; charset=utf-8") if err := s.tmpl.ExecuteTemplate(w, "recovery", data); err != nil { @@ -124,6 +160,48 @@ func (s *Server) SetRecoveryRecoverer(fn func() (backup.OffsiteKeyRecoverer, err s.recoveryRecovererFn = fn } +// recoverySupport is the R-216 capability verdict for the unlock path. It uses the SAME machinery the +// NAS gate uses (`netFeatures` + the agent client as prober), so the version channel decides without +// probe traffic whenever the agent's version is known. +// +// It returns SupportUnknown when the agent handle cannot be built — and the CALLER treats Unknown as +// "cannot ask". That is deliberate and is the whole point: a box that cannot reach its agent must not +// attempt an unlock whose failure would be read as a bad code. +func (s *Server) recoverySupport(ctx context.Context) agentapi.SupportState { + if s.recoverySupportFn != nil { + return s.recoverySupportFn(ctx) + } + agent, err := s.agentClient() + if err != nil { + return agentapi.SupportUnknown + } + state, source := s.netFeatures.SupportsWithSource(ctx, agent, agentapi.FeatureOffsiteKeyRecovery) + s.logger.Printf("[DEBUG] [web] recovery capability gate: %s=%s (source=%s)", agentapi.FeatureOffsiteKeyRecovery, state, source) + return state +} + +// SetRecoverySupport overrides the capability verdict (tests). INIT-ONLY. +func (s *Server) SetRecoverySupport(fn func(context.Context) agentapi.SupportState) { + s.recoverySupportFn = fn +} + +// recoverySuperseded reports the hub's statement that an EARLIER sealed package is kept, and when +// (R-222). Both zero on a pre-0.97.0 hub, which keeps the old message — an older hub simply cannot +// make the screen claim anything new. +func (s *Server) recoverySuperseded() (bool, string) { + if s.settings == nil { + return false, "" + } + return s.settings.GetHubEscrowSuperseded() +} + +// SetRecoveryTierUp wires the off-site apply-bridge reconcile (R-219 / R-218). INIT-ONLY. +// +// The unlock calls it between placing the recovered key and reading the repository — the step that +// turns the screen's promised listing from structurally impossible into merely conditional. nil (or +// an error) is survivable: the key is placed either way and the listing branch says what is pending. +func (s *Server) SetRecoveryTierUp(fn func(context.Context) error) { s.recoveryTierUp = fn } + // recoveryUnlockHandler takes the recovery code and drives the SHARED core (POST /recovery/unlock). // // R HANDLING, and it must not drift from the CLI's: @@ -155,6 +233,23 @@ func (s *Server) recoveryUnlockHandler(w http.ResponseWriter, r *http.Request) { s.renderRecovery(w, r, "A gép házon belüli kapcsolata most nem elérhető — próbáld újra néhány perc múlva.", "", nil) return } + // ── MESSAGE 2 of 4 — THE MACHINE CANNOT ASK (R-216). This gate FAILS CLOSED. ──────────────── + // + // The unseal happens in the agent, behind POST /escrow/recover-offsite-password, which ships in + // agent v0.125.0. On an older agent that route 404s — and until v0.201.0 the unlock was attempted + // anyway and the 404 came back as "we did not accept your recovery code, check your ten words". + // A correct code, refused in 0.134 s, blamed on the customer (CAMPAIGN-11 Phase 1, the headline). + // + // So: anything other than SupportYes stops here. Unknown counts as cannot-ask, against this + // package's fail-open default — see FeatureOffsiteKeyRecovery for why this one feature inverts it. + // An attempt that cannot succeed must never be made, because its failure is attributed to the code. + if support := s.recoverySupport(r.Context()); support != agentapi.SupportYes { + code = "" + s.logger.Printf("[WARN] [web] recovery: refused before attempting — agent capability %s=%s (needs %s)", + agentapi.FeatureOffsiteKeyRecovery, support, agentapi.MinAgentFor(agentapi.FeatureOffsiteKeyRecovery)) + s.renderRecovery(w, r, "Ez a gép még nem tudja megnyitni a mentéseidet — a hozzá tartozó házon belüli szolgáltatás régebbi, mint amit ehhez a lépéshez használunk. A kódoddal semmi baj, és nem is használtuk fel: tedd el biztonságos helyen. A gép magától frissül; próbáld újra később, vagy szólj a Felhom ügyfélszolgálatának, ha egy nap múlva sem működik.", "", nil) + return + } ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) defer cancel() // THE SHARED CORE — the same function the command line drives. There is no second recovery @@ -165,6 +260,31 @@ func (s *Server) recoveryUnlockHandler(w http.ResponseWriter, r *http.Request) { // The agent's error names the STEP (fetch / unseal / place) and carries no secret. It is not // shown raw: a customer needs to know what to check, not what age's KDF returned. s.logger.Printf("[WARN] [web] recovery: unlock failed: %v", rerr) + // ── MESSAGE 4 of 4 — AN EARLIER PACKAGE IS KEPT (R-222) ──────────────────────────────── + // + // The unseal failed against the package the hub CURRENTLY holds. That is the right outcome for + // a wrong code — and it is also the right outcome for a code that is perfectly correct about an + // EARLIER package the hub is deliberately keeping. From here the two are indistinguishable: the + // engine fails closed either way, which is exactly what it should do. + // + // So when the hub has told us an earlier package exists, say that instead of blaming typing. + // Measured on 2026-08-05 (CAMPAIGN-11 Phase 3 step 7): the customer entered the correct code + // for their orphaned history, got a real 1.111 s unseal attempt, and was told to check their + // ten words — while the same screen showed a seal date from after the code they were holding. + // + // ⚠ It states two facts the hub knows and STOPS. It does not promise the earlier package can be + // opened, because it cannot be: serving a superseded blob is an unbuilt link (R-199's + // inventory), and a conditional promise that turns out false on this screen is worse than + // saying less (the R-202 lesson). + if present, at := s.recoverySuperseded(); present { + when := "" + if at != "" { + when = " (" + at + ")" + } + s.renderRecovery(w, r, "Ez a kód nem nyitja meg azt a csomagot, amit most őrzünk ehhez a géphez. Ha egy korábbi kódot adtál meg: a géped azóta új mentési kulcsot kapott, és a régebbi csomagot"+when+" nem töröltük — megőrizzük. Megnyitni viszont innen egyelőre nem lehet, ezért ha a régebbi mentéseidre van szükséged, keresd a Felhom ügyfélszolgálatát. A kódoddal semmi nem történt, és semmi nem változott.", "", nil) + return + } + // ── MESSAGE 1 of 4 — THE CODE DID NOT OPEN IT. The ONLY one that mentions typing. ────────── s.renderRecovery(w, r, "A megadott helyreállítási kódot nem fogadtuk el. Ellenőrizd, hogy mind a tíz szót pontosan, szóközökkel elválasztva írtad be — a kis- és nagybetűk nem számítanak. Semmi nem változott, nyugodtan próbáld újra.", "", nil) return } @@ -175,21 +295,62 @@ func (s *Server) recoveryUnlockHandler(w http.ResponseWriter, r *http.Request) { return } s.logger.Printf("[INFO] [web] recovery: the offsite repository key was recovered and placed (outcome=%s)", res.Outcome) + + // ── FINISH THE JOB (R-219, v0.201.0) ─────────────────────────────────────────────────────── + // + // The screen promises: *"feloldjuk a mentéseid zárolását és megmutatjuk, mi van bennük"*. On the + // shape this screen exists for that promise was structurally unkeepable, and this call is what + // makes it possible at all: + // + // - listing needs an off-site TARGET (OffsiteInventoryList returns errNoOffsiteTarget without); + // - a target cannot exist without a repository password, because ApplyOffsiteTarget → + // WriteOffboxSecrets MINTS one whenever none is present; + // - shape (a) is DEFINED by having no repository password. So shape (a) ⇒ no target ⇒ the + // listing could never render. Not sometimes. Ever. + // + // And there was no second chance: placing the key flips recoveryOffer() false (a password now + // exists and the box is not orphaned), so the unlock response was the customer's ONLY opportunity + // to see the listing — and it was guaranteed not to contain it. + // + // So bring the tier up here, synchronously, between placing the key and reading the repository. + // It is also the second half of R-218's deadlock: the apply-bridge only retried "on next config + // refresh/restart", and nothing triggered either. + if s.recoveryTierUp != nil { + tctx, tcancel := context.WithTimeout(context.Background(), 90*time.Second) + if terr := s.recoveryTierUp(tctx); terr != nil { + // NOT a failure of the unlock — the key IS placed. Logged, never swallowed, and the + // listing branch below says what is pending rather than what failed. + s.logger.Printf("[WARN] [web] recovery: the offsite tier could not be brought up yet: %v", terr) + } + tcancel() + } + // Unlocked. Now show what is in there — read-only. ictx, icancel := context.WithTimeout(context.Background(), 2*time.Minute) defer icancel() inv, ierr := s.backupMgr.OffsiteInventoryList(ictx) if ierr != nil { + // ── MESSAGE 3 of 4 — THE STORE COULD NOT BE READ (R-217) ──────────────────────────────── + // + // ⚠ PASS nil, NOT backup.OffsiteInventory{}. The zero value has Empty=false and Apps=nil, which + // the template reads as InvUntagged — *"A tároló megnyílt, és van benne tartalom, de nem tudtuk + // alkalmazásokhoz rendelni."* Three assertions, none of them known: it did not open, its + // contents are unknown, and attribution is not the problem. Measured on 2026-08-05 (CAMPAIGN-11 + // Phase 1), rendered directly beneath the honest message, contradicting it. + // + // OffsiteInventory.Empty exists precisely to prevent this — its own doc comment says it is + // "named rather than inferred from len(Apps)==0, WHICH IS ALSO WHAT A FAILED READ LOOKS LIKE". + // The field built for the hazard was defaulted past. nil means "no inventory to render", and + // the template shows no listing block at all. s.logger.Printf("[WARN] [web] recovery: unlocked but the inventory could not be read: %v", ierr) - empty := backup.OffsiteInventory{} - msg := "A kulcs visszakerült, de a mentések listáját most nem sikerült beolvasni. Nézd meg a Biztonsági mentés oldalt néhány perc múlva." + msg := "A kulcs visszakerült, de a mentések listáját most nem sikerült beolvasni. A mentéseid nincsenek veszélyben — nézd meg a Biztonsági mentés oldalt néhány perc múlva." if backup.ErrNoOffsiteTarget(ierr) { - // The pristine rebuilt shape: the key is in place but the box has no off-site coordinates - // yet. It resolves by itself once the tier is re-applied, so say that rather than showing a - // failure the customer cannot act on. - msg = "A kulcs visszakerült. A gép még most kapcsolódik újra a házon kívüli tárhelyhez — a mentéseid listája néhány perc múlva jelenik meg a Biztonsági mentés oldalon." + // The tier could not be brought up in time — the transport credential has not arrived yet. + // Nothing has failed; something is pending, and the declaration stays alive (R-218) so the + // hub's self-heal can still act. + msg = "A kulcs visszakerült, és biztonságban van. A gép még várja a házon kívüli tárhely kapcsolódási adatait — amint megvannak, a mentéseid listája megjelenik a Biztonsági mentés oldalon. Nincs teendőd." } - s.renderRecovery(w, r, msg, "", &empty) + s.renderRecoveryUnlockedNoList(w, r, msg) return } s.renderRecovery(w, r, "", "A mentéseid zárolása feloldva. Az alábbiakat találtuk a tárolóban — semmit nem állítottunk vissza.", &inv) diff --git a/controller/internal/web/recovery_test.go b/controller/internal/web/recovery_test.go index 6de1ea4..0674b6e 100644 --- a/controller/internal/web/recovery_test.go +++ b/controller/internal/web/recovery_test.go @@ -16,6 +16,7 @@ import ( "testing" "time" + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" "gitea.dooplex.hu/admin/felhom-controller/internal/backup" "gitea.dooplex.hu/admin/felhom-controller/internal/config" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" @@ -127,6 +128,11 @@ func newRecoveryFixture(t *testing.T) *recoveryFixture { s := &Server{cfg: cfg, settings: sett, backupMgr: mgr, stackMgr: stackMgr, logger: lg, version: "test"} s.loadTemplates() s.SetRecoveryRecoverer(func() (backup.OffsiteKeyRecoverer, error) { return rec, nil }) + // R-216: the capability gate FAILS CLOSED, so the default fixture states the supported case + // explicitly. Without this every unlock test would exercise the refusal instead — which is exactly + // the protection working, and exactly not what those tests are about. The refusal has its own + // tests in recovery_gate_test.go, each overriding this. + s.SetRecoverySupport(func(context.Context) agentapi.SupportState { return agentapi.SupportYes }) return &recoveryFixture{s: s, mgr: mgr, sett: sett, rec: rec, runner: rr, dataDir: cfg.Paths.DataDir} } diff --git a/controller/internal/web/server.go b/controller/internal/web/server.go index 9793546..5b4830b 100644 --- a/controller/internal/web/server.go +++ b/controller/internal/web/server.go @@ -104,6 +104,13 @@ type Server struct { // recoveryRecovererFn is the recovery screen's agent seam (nil → the shared agentClient(), the // same channel the CLI uses). Tests inject a fake so the HANDLER itself can be driven. recoveryRecovererFn func() (backup.OffsiteKeyRecoverer, error) + // recoverySupportFn overrides the R-216 agent-capability verdict for the unlock path (tests). + // nil → the real gate over netFeatures. See recoverySupport: Unknown means CANNOT ASK here. + recoverySupportFn func(context.Context) agentapi.SupportState + // recoveryTierUp brings the off-site tier up between placing a recovered key and reading the + // repository (R-219). Wired from main.go to the apply-bridge's Reconcile; nil → skipped, and the + // listing branch then reports what is pending rather than claiming a failure. + recoveryTierUp func(context.Context) error // NAS add orchestration (verify-before-commit): the single-flight job slot + the two seams. // netAgentFn nil → the shared agentClient(); netProbeFn nil → runNetProbe (the uid-1000 re-exec). diff --git a/controller/internal/web/templates/recovery.html b/controller/internal/web/templates/recovery.html index 9cc665b..4286c81 100644 --- a/controller/internal/web/templates/recovery.html +++ b/controller/internal/web/templates/recovery.html @@ -18,7 +18,12 @@ {{if .Flash}}
{{.Flash}}
{{end}} {{if .Error}}
{{.Error}}
{{end}} - {{if .InvEmpty}} + {{if .InvUnavailable}} + + {{else if .InvEmpty}}
A tároló megnyílt, de nincs benne egyetlen mentés sem. Ez azt jelenti, hogy a kulcs jó volt, de ehhez a géphez nem tartozik korábbi mentés. Ha korábban biztosan készültek