diff --git a/controller/internal/agentapi/escrow.go b/controller/internal/agentapi/escrow.go index 82cbd82..5979e4b 100644 --- a/controller/internal/agentapi/escrow.go +++ b/controller/internal/agentapi/escrow.go @@ -3,6 +3,7 @@ package agentapi import ( "context" "encoding/json" + "errors" "fmt" "net/http" ) @@ -132,8 +133,11 @@ func (c *Client) RecoverOffsiteRepoPassword(ctx context.Context, recoveryCode st if perr != nil { return "", "", perr } - if rerr := refusalError("/escrow/recover-offsite-password", status, env); rerr != nil { - return "", "", rerr + // R-224: this route's refusal keeps its STATUS as a value. `refusalError` flattens status into a + // sentence, and a sentence is not something a caller can branch on — which is exactly how a failed + // fetch and a wrong recovery code came to produce one customer-facing message. + if status < 200 || status > 299 || !env.OK { + return "", "", &RecoveryRefusal{Status: status, Reason: truncateErr(env.Error, 300)} } var out struct { ResticRepoPassword string `json:"restic_repo_password"` @@ -147,3 +151,109 @@ func (c *Client) RecoverOffsiteRepoPassword(ctx context.Context, recoveryCode st } return out.ResticRepoPassword, out.ResticPwSHA256, nil } + +// ── R-224 — CLASSIFYING A FAILED UNLOCK ───────────────────────────────────────────────────────── +// +// CAMPAIGN-11 measured what happens without this. On 2026-08-05, with a CORRECT current recovery +// code: the hub firewalled off returned the customer "this code does not open your package" in +// 0.0556 s, and this agent stopped returned the same in 0.0299 s — against ~1.0 s for a genuine +// unseal. Neither attempted one. The failure path had exactly two branches, both of them statements +// about the customer's code, and `rerr` was never inspected. +// +// The rule this type exists to enforce: **the customer is blamed only after a real attempt refused +// their code.** Everything else — including anything we cannot classify — says something else. + +// RecoveryRefusal is the agent's refusal of an unlock, carrying the STATUS as a value so callers +// classify on it rather than on the sentence. The message keeps `refusalError`'s shape so operator +// logs read as they did. +type RecoveryRefusal struct { + Status int + Reason string +} + +func (e *RecoveryRefusal) Error() string { + reason := e.Reason + if reason == "" { + reason = "(no reason in agent response)" + } + return fmt.Sprintf("agentapi: POST /escrow/recover-offsite-password: HTTP %d: %s", e.Status, reason) +} + +// RecoveryFailure is what went wrong, as far as it can be known. +type RecoveryFailure int + +const ( + // RecoveryUnknown — the cause could not be determined. **The safe default**, and deliberately the + // zero value: a new status, a transport shape nobody anticipated, or an agent too old to + // distinguish fetch from refusal all land here, and none of them may blame the customer. + RecoveryUnknown RecoveryFailure = iota + // RecoveryHubUnreachable — the agent answered, and it could not FETCH the sealed package: the hub + // refused, was unreachable, or recovery is not configured on this agent. **The code was not used.** + RecoveryHubUnreachable + // RecoveryAskedAndRefused — the bundle was fetched and the code did not open it. The ONLY class + // from which the customer may be told to check their typing. + RecoveryAskedAndRefused + // RecoveryNoBundle — the hub holds no sealed package for this host at all. + RecoveryNoBundle + // RecoveryBundleTooOld — the bundle opened but predates the repository-password field. + RecoveryBundleTooOld + // RecoveryAgentUnreachable — the machine's own in-house service never answered, so there is no + // agent verdict at all. **The code was not used.** Distinct from RecoveryHubUnreachable because + // it is a different fault, with different words and a different remedy. + RecoveryAgentUnreachable +) + +// ClassifyRecoveryFailure maps an unlock error to its class, from the VALUE and never the text. +// +// ⚠ `trustRefusal` is the agent-version gate and it is not optional. An agent older than v0.126.0 +// answers **400 for BOTH** a fetch failure and a wrong code, so a 400 from one cannot be read as +// "the code was refused" — it means "one of two things, and we cannot tell which". Pass false there +// and the 400 degrades to RecoveryUnknown, which is neutral. That degradation is the point: it is +// safe, it is silent, and it heals itself when the agent updates. +func ClassifyRecoveryFailure(err error, trustRefusal bool) RecoveryFailure { + if err == nil { + return RecoveryUnknown + } + var ref *RecoveryRefusal + if !errors.As(err, &ref) { + // Not a refusal at all — the request never produced an agent verdict (dial failure, TLS, + // timeout, or the channel could not be built). The machine could not even ASK its own service, + // which is a different sentence from "the hub was unreachable" and a different thing to fix. + return RecoveryAgentUnreachable + } + switch ref.Status { + case http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout: + // 502 is agent >= v0.126.0's "the sealed bundle could not be fetched". 503 is its + // "recovery is not configured on this agent (no hub client)". Neither used the code. + return RecoveryHubUnreachable + case http.StatusNotFound: + return RecoveryNoBundle + case http.StatusConflict: + return RecoveryBundleTooOld + case http.StatusBadRequest: + if trustRefusal { + return RecoveryAskedAndRefused + } + return RecoveryUnknown + default: + return RecoveryUnknown + } +} + +// String names the class for the operator log. The customer never sees these words. +func (f RecoveryFailure) String() string { + switch f { + case RecoveryHubUnreachable: + return "hub-unreachable" + case RecoveryAgentUnreachable: + return "agent-unreachable" + case RecoveryAskedAndRefused: + return "asked-and-refused" + case RecoveryNoBundle: + return "no-bundle" + case RecoveryBundleTooOld: + return "bundle-too-old" + default: + return "unknown" + } +} diff --git a/controller/internal/agentapi/features.go b/controller/internal/agentapi/features.go index 9844ed3..b6f6858 100644 --- a/controller/internal/agentapi/features.go +++ b/controller/internal/agentapi/features.go @@ -55,6 +55,21 @@ const FeatureBackupAgeState Feature = "backup_age_state" // never have worked is attributed to the code. const FeatureOffsiteKeyRecovery Feature = "offsite_key_recovery" +// FeatureRecoveryFailureClass is agent v0.126.0's SPLIT of a failed unlock into distinguishable +// statuses (R-224): 502 the sealed bundle could not be FETCHED · 400 it was fetched and the code was +// refused · 404 no bundle · 409 the bundle predates the repository-password field. +// +// ⚠ WHAT THIS GATE ACTUALLY GUARDS is the meaning of **400**, and nothing else. An agent older than +// v0.126.0 answers 400 for BOTH a fetch failure and a wrong code — one status, one sentence, two +// situations — so on such an agent a 400 cannot be read as "the code was refused". It means "one of +// two things and we cannot tell which", which is `RecoveryUnknown`, which is neutral. +// +// So this gate does not block anything and has no fail-closed behaviour to get wrong: the unlock is +// attempted either way (FeatureOffsiteKeyRecovery already decides THAT). It only decides whether the +// customer may be told to check their typing. Unknown → they may not. **That is the safe direction, +// and it heals itself the moment the agent updates.** +const FeatureRecoveryFailureClass Feature = "recovery_failure_class" + // 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 @@ -122,6 +137,10 @@ var featureProbes = map[Feature]func(ctx context.Context, p SupportProber) error FeatureOffsiteKeyRecovery: func(ctx context.Context, p SupportProber) error { return errNoRecoveryProbe }, + // Same POST route, same reason it cannot be probed — the decision falls to the VERSION path. + FeatureRecoveryFailureClass: func(ctx context.Context, p SupportProber) error { + return errNoRecoveryProbe + }, } // errNoMemoryProbe classifies to SupportUnknown (not a *StatusError 404), so a prober that cannot be @@ -145,6 +164,9 @@ var featureMinAgent = map[Feature]string{ // 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", + + // R-224 — the four-way status split of a failed unlock. + FeatureRecoveryFailureClass: "0.126.0", } // MinAgentFor returns the declared minimum agent version for a feature ("" when the feature has no diff --git a/controller/internal/web/recovery_class_test.go b/controller/internal/web/recovery_class_test.go new file mode 100644 index 0000000..aba8fbd --- /dev/null +++ b/controller/internal/web/recovery_class_test.go @@ -0,0 +1,244 @@ +package web + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" +) + +// ── R-224 / R-226 — WHY IT FAILED DECIDES WHAT WE SAY ──────────────────────────────────────────── +// +// CAMPAIGN-11 measured the defect these tests pin. With a CORRECT, CURRENT recovery code: +// +// hub firewalled off → "this code does not open your package" in 0.0556 s +// agent stopped → the same sentence in 0.0299 s +// genuinely wrong code → the same sentence after 1.194 / 1.004 / 1.014 s +// +// A real unseal costs ~1 s of scrypt, so the first two had not attempted one. Three different causes, +// one accusation, and the failure path never inspected the error. +// +// Every test here asserts the EFFECT — which sentence the customer is shown — at the HANDLER, because +// a helper-level test cannot observe a mutation that lives in the handler. + +// namesTyping / isBareAccusation split what the old single predicate conflated. R-226 requires the +// typing HINT on a re-escrowed box; what stays forbidden is the bare accusation that says only that. +func namesTyping(body string) bool { return strings.Contains(body, "z szót pontosan") } +func isBareAccusation(body string) bool { return strings.Contains(body, "nem fogadtuk el") } + +// saysCodeWasNotUsed is the load-bearing half of every could-not-ask message: the customer must be +// told their code was not spent, because that is what makes "keep it safe and try again" honest. +func saysCodeWasNotUsed(body string) bool { return strings.Contains(body, "NEM használtuk fel") } + +// refusal builds the error shape agent >= v0.126.0 returns for a given status. +func refusal(status int, reason string) error { + return &agentapi.RecoveryRefusal{Status: status, Reason: reason} +} + +// ── SCENARIO A — the hub is unreachable and the customer is not blamed ─────────────────────────── +// +// RED-PROOF: delete the `agentapi.RecoveryHubUnreachable` case from recoveryUnlockHandler so a 502 +// falls through to the wrong-code branch → this FAILS on namesTyping, and the accusation returns +// exactly as CAMPAIGN-11 F3 measured it. Demonstrated failing before this test was kept. +func TestRecoveryClass_A_HubUnreachableNamesTheConnection(t *testing.T) { + f := newRecoveryFixture(t) + f.rec.failWith = refusal(502, "the sealed recovery bundle could not be fetched from the hub — the recovery code was NOT used and nothing was written") + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + + if namesTyping(body) || isBareAccusation(body) { + t.Fatalf("R-224 RETURNED: a hub outage is reported as a bad recovery code; got %q", firstAlert(body)) + } + if !strings.Contains(body, "központi rendszer") { + t.Errorf("the message must name the connection that failed; got %q", firstAlert(body)) + } + if !saysCodeWasNotUsed(body) { + t.Errorf("the customer must be told their code was NOT used; got %q", firstAlert(body)) + } + // It must not invent an earlier package either — that is a different situation. + if strings.Contains(body, "nem töröltük") { + t.Errorf("a hub outage must not be dressed up as a retained earlier package; got %q", firstAlert(body)) + } +} + +// ── SCENARIO B — the agent is stopped and the customer is not blamed ───────────────────────────── +// +// A dial failure produces NO agent verdict at all, so it is not a *RecoveryRefusal and classifies as +// RecoveryAgentUnreachable. This is the F4 shape: connection refused to 169.254.253.1:8443. +func TestRecoveryClass_B_AgentUnreachableNamesTheMachine(t *testing.T) { + f := newRecoveryFixture(t) + f.rec.failWith = errors.New(`agentapi: POST /escrow/recover-offsite-password: dial tcp 169.254.253.1:8443: connect: connection refused`) + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + + if namesTyping(body) || isBareAccusation(body) { + t.Fatalf("R-224 RETURNED: a stopped agent is reported as a bad recovery code; got %q", firstAlert(body)) + } + if !strings.Contains(body, "házon belüli") { + t.Errorf("the message must name the machine's own service; got %q", firstAlert(body)) + } + if !saysCodeWasNotUsed(body) { + t.Errorf("the customer must be told their code was NOT used; got %q", firstAlert(body)) + } + // The raw technical error must never reach the customer. + if strings.Contains(body, "dial tcp") || strings.Contains(body, "169.254") { + t.Errorf("the raw transport error was rendered to the customer; got %q", firstAlert(body)) + } +} + +// ── SCENARIO C — a genuinely wrong code says so, even on a re-escrowed box (R-226) ─────────────── +// +// RED-PROOF: remove the mistype clause from the superseded message → this FAILS on namesTyping, and +// the ten-words prompt is unreachable again on exactly the population most likely to need it. +func TestRecoveryClass_C_MistypeOnAReEscrowedBoxNamesBoth(t *testing.T) { + f := newRecoveryFixture(t) + f.rec.failWith = refusal(400, "the recovery code did not open the sealed bundle — nothing was written") + 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 !namesTyping(body) { + t.Fatalf("R-226 RETURNED: a mistype on a re-escrowed box is never told to re-check the words; got %q", firstAlert(body)) + } + if !strings.Contains(body, "nem töröltük") { + t.Fatalf("the retained earlier package must still be named; got %q", firstAlert(body)) + } + if isBareAccusation(body) { + t.Fatalf("R-222 RETURNED: the bare accusation, with no mention of the retained package") + } + // §7.3 — it 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)", forbidden) + } + } +} + +// ── SCENARIO D — an unclassifiable failure never blames the customer ───────────────────────────── +// +// THE RULE THAT WAS MISSING WHEN THIS DEFECT WAS FIXED THE FIRST TIME. The default must be neutral, +// and it must claim NEITHER that the code was wrong NOR that it went unused — neither is known. +// +// RED-PROOF: change the `default:` arm to render the wrong-code message → this FAILS. That mutation +// is precisely the pre-R-224 shape, where everything unrecognised fell through to an accusation. +func TestRecoveryClass_D_UnclassifiableIsNeutral(t *testing.T) { + for _, tc := range []struct { + name string + err error + }{ + {"an unrecognised status", refusal(418, "something nobody anticipated")}, + {"a 400 from an agent too old to split fetch from refusal", refusal(400, "the recovery code did not open the sealed bundle, or the bundle could not be fetched")}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newRecoveryFixture(t) + f.rec.failWith = tc.err + if tc.name != "an unrecognised status" { + // The OLD-agent case: a 400 may not be read as a refusal. + f.s.SetRecoveryRefusalTrusted(func(context.Context) bool { return false }) + } + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + + if namesTyping(body) || isBareAccusation(body) { + t.Fatalf("an unclassifiable failure blamed the customer; got %q", firstAlert(body)) + } + // And it must not claim the opposite either — "we did not use your code" is also a claim. + if saysCodeWasNotUsed(body) { + t.Fatalf("an unclassifiable failure asserted the code was unused — that is not known; got %q", firstAlert(body)) + } + if !strings.Contains(body, "nem tudjuk biztosan") { + t.Errorf("the neutral message must say the cause is not known; got %q", firstAlert(body)) + } + }) + } +} + +// ── SCENARIO E — the accusing message requires a REAL attempt ──────────────────────────────────── +// +// §7.2 asks for this as a TEST rather than production logic, and the distinction matters: elapsed +// time is the symptom that DIAGNOSED R-224, never a classifier. A production guard on duration would +// be a second thing that can be wrong, and §5 forbids it outright. +// +// So the guard is STRUCTURAL: the typing hint is reachable from exactly ONE class — the one that can +// only arise from a 400, which by the agent's contract means the bundle was fetched and age ran. This +// asserts that exhaustively, and the clock is injected so the assertion needs no sleeping. +// +// RED-PROOF: route any instant-failing class (502, a transport error, an unrecognised status) to the +// wrong-code message → this FAILS on that row. +func TestRecoveryClass_E_OnlyARealRefusalMayMentionTyping(t *testing.T) { + instant := []struct { + name string + err error + }{ + {"hub unreachable (502)", refusal(502, "could not be fetched")}, + {"recovery not configured (503)", refusal(503, "not configured")}, + {"agent unreachable (transport)", errors.New("dial tcp: connection refused")}, + {"no bundle (404)", refusal(404, "no sealed bundle")}, + {"bundle too old (409)", refusal(409, "predates the field")}, + {"unrecognised (418)", refusal(418, "unanticipated")}, + } + for _, tc := range instant { + t.Run(tc.name, func(t *testing.T) { + f := newRecoveryFixture(t) + // A clock that NEVER advances: none of these performs an unseal, and the assertion below + // must hold without any wall-clock time passing. + frozen := time.Date(2026, 8, 6, 4, 0, 0, 0, time.UTC) + f.s.SetRecoveryClock(func() time.Time { return frozen }) + f.rec.failWith = tc.err + + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + if namesTyping(body) || isBareAccusation(body) { + t.Fatalf("a failure that performed NO unseal mentioned typing; got %q", firstAlert(body)) + } + }) + } + + // The positive half: the one class that DID perform an unseal may say it. Without this the test + // would pass with the typing message deleted outright. + t.Run("a real refusal (400) may mention typing", func(t *testing.T) { + f := newRecoveryFixture(t) + f.rec.failWith = refusal(400, "the recovery code did not open the sealed bundle") + body := postUnlockWith(t, f.s, testRecoveryCode).Body.String() + if !namesTyping(body) { + t.Fatalf("a genuinely refused code must be told what to check; got %q", firstAlert(body)) + } + }) +} + +// The classifier itself, at the value level — the table the handler switches on. Kept separate from +// the handler tests so a mapping change is named directly rather than inferred from Hungarian copy. +func TestClassifyRecoveryFailure_MapsFromTheValueNotTheText(t *testing.T) { + cases := []struct { + name string + err error + trusted bool + want agentapi.RecoveryFailure + }{ + {"fetch failure", refusal(502, ""), true, agentapi.RecoveryHubUnreachable}, + {"not configured", refusal(503, ""), true, agentapi.RecoveryHubUnreachable}, + {"refused, trusted", refusal(400, ""), true, agentapi.RecoveryAskedAndRefused}, + {"refused, NOT trusted (old agent)", refusal(400, ""), false, agentapi.RecoveryUnknown}, + {"no bundle", refusal(404, ""), true, agentapi.RecoveryNoBundle}, + {"bundle too old", refusal(409, ""), true, agentapi.RecoveryBundleTooOld}, + {"unrecognised status", refusal(418, ""), true, agentapi.RecoveryUnknown}, + {"transport", errors.New("dial tcp"), true, agentapi.RecoveryAgentUnreachable}, + {"nil", nil, true, agentapi.RecoveryUnknown}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := agentapi.ClassifyRecoveryFailure(tc.err, tc.trusted); got != tc.want { + t.Fatalf("got %v, want %v", got, tc.want) + } + }) + } + // The text must be irrelevant: the SAME sentence under two statuses classifies two ways. + same := "the recovery code did not open the sealed bundle" + if agentapi.ClassifyRecoveryFailure(refusal(400, same), true) == agentapi.ClassifyRecoveryFailure(refusal(502, same), true) { + t.Fatal("classification followed the TEXT — it must follow the status") + } +} diff --git a/controller/internal/web/recovery_gate_test.go b/controller/internal/web/recovery_gate_test.go index 90cb2b9..0aa1de8 100644 --- a/controller/internal/web/recovery_gate_test.go +++ b/controller/internal/web/recovery_gate_test.go @@ -180,8 +180,24 @@ func TestRecoveryGate_H_SupersededPackageIsNamed(t *testing.T) { 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") + // ⚠ R-226 DELIBERATELY REVERSED HALF OF THIS ASSERTION (2026-08-06), so it is spelled out. + // + // As first shipped, this test forbade ANY mention of typing here — R-222's guarantee was that a + // customer holding the RIGHT code for a retained earlier package must not be told they mistyped. + // That was right about the accusation and wrong about the omission: because this branch is tested + // BEFORE the wrong-code message, it made the ten-words prompt UNREACHABLE on every box the hub + // keeps an earlier package for — which is exactly the box whose customer has just been given a new + // code and is most likely to be typing one. Measured 2026-08-05 (CAMPAIGN-11 F1): three genuinely + // wrong codes, three real ~1 s unseals, three copies of the earlier-package message. + // + // The two causes are INDISTINGUISHABLE at the engine, so the message now names BOTH. What stays + // forbidden is the bare ACCUSATION — the M1 opener that asserts the code was simply not accepted + // and says nothing about the retained package. + if strings.Contains(body, "nem fogadtuk el") { + t.Fatal("R-222 RETURNED: the bare wrong-code accusation, with no mention of the retained earlier package") + } + if !blamesTyping(body) { + t.Fatal("R-226 RETURNED: a genuine mistype on a re-escrowed box is given no way to discover it was a mistype") } 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)) diff --git a/controller/internal/web/recovery_handlers.go b/controller/internal/web/recovery_handlers.go index 04d0c20..b3ca65c 100644 --- a/controller/internal/web/recovery_handlers.go +++ b/controller/internal/web/recovery_handlers.go @@ -185,6 +185,47 @@ func (s *Server) SetRecoverySupport(fn func(context.Context) agentapi.SupportSta s.recoverySupportFn = fn } +// recoveryRefusalTrusted reports whether a 400 from the agent may be read as "the bundle was fetched +// and the code was REFUSED" (R-224). +// +// Only agent >= v0.126.0 splits a failed fetch out to its own status. Before it, 400 covered both, +// and reading one as a refusal is precisely how a hub outage became an accusation. Anything other +// than a definite yes therefore withholds that reading, and the caller falls to the neutral message. +// +// ⚠ This gate BLOCKS NOTHING. The unlock is attempted either way — FeatureOffsiteKeyRecovery already +// decides that, fail-closed. This only decides whether the customer may be told to check their +// typing, and "not sure" means they may not. +func (s *Server) recoveryRefusalTrusted(ctx context.Context) bool { + if s.recoveryRefusalTrustedFn != nil { + return s.recoveryRefusalTrustedFn(ctx) + } + agent, err := s.agentClient() + if err != nil { + return false + } + state, _ := s.netFeatures.SupportsWithSource(ctx, agent, agentapi.FeatureRecoveryFailureClass) + return state == agentapi.SupportYes +} + +// SetRecoveryRefusalTrusted overrides the R-224 version gate (tests). INIT-ONLY. +func (s *Server) SetRecoveryRefusalTrusted(fn func(context.Context) bool) { + s.recoveryRefusalTrustedFn = fn +} + +// recoveryNow is the clock the unlock path measures itself against. Real time in production; tests +// inject so §7.2's guard — the typing message may only follow a REAL unseal — can be asserted +// without sleeping. It is an observability seam and a test seam: **it must never become a +// classifier.** Time is the symptom that diagnosed R-224; the agent's status is the fact. +func (s *Server) recoveryNow() time.Time { + if s.recoveryNowFn != nil { + return s.recoveryNowFn() + } + return time.Now() +} + +// SetRecoveryClock overrides the unlock clock (tests). INIT-ONLY. +func (s *Server) SetRecoveryClock(fn func() time.Time) { s.recoveryNowFn = 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. @@ -254,12 +295,67 @@ func (s *Server) recoveryUnlockHandler(w http.ResponseWriter, r *http.Request) { defer cancel() // THE SHARED CORE — the same function the command line drives. There is no second recovery // implementation in this codebase (R-193 §8.5). + unsealStart := s.recoveryNow() res, rerr := backup.RecoverInstallCore(ctx, s.backupMgr, rec, code, true) + unsealTook := s.recoveryNow().Sub(unsealStart) code = "" // cleared here, before any branch below, on success and failure alike if rerr != nil { // 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) + // + // ── R-224 — WHY IT FAILED DECIDES WHAT WE SAY. ───────────────────────────────────────── + // + // This branch used to be a two-way choice — superseded? M4 : M1 — and BOTH are statements + // about the customer's code. `rerr` was never inspected, so a hub that refused, an agent that + // was stopped, and a genuinely mistyped code all produced the same accusation. + // + // Measured live on 2026-08-05 with a CORRECT current code: hub firewalled off → 0.0556 s; + // agent stopped → 0.0299 s. A genuine unseal costs ~1.0 s of scrypt, so neither had attempted + // one. **The machine accused the customer of something it had not tried.** + // + // The duration is logged because it is what DIAGNOSED this and it is the cheapest possible + // tell for the operator — but it is NEVER the classifier. Time is a symptom; the status is + // the fact. + class := agentapi.ClassifyRecoveryFailure(rerr, s.recoveryRefusalTrusted(r.Context())) + s.logger.Printf("[WARN] [web] recovery: unlock failed after %s (class=%s): %v", unsealTook.Round(time.Millisecond), class, rerr) + switch class { + case agentapi.RecoveryHubUnreachable: + // The agent answered and could not FETCH the package. The code was NEVER USED. Say that, + // name the connection, and say nothing whatever about whether the code is right — we do + // not know, and guessing here is the whole defect. + s.renderRecovery(w, r, "Most nem sikerült elérni a Felhom központi rendszerét, ezért a mentéseidet nem tudtuk megnyitni. A kódodat NEM használtuk fel, és semmi nem változott — tedd el biztonságos helyen, és próbáld újra néhány perc múlva. Ha egy óra múlva sem megy, szólj a Felhom ügyfélszolgálatának.", "", nil) + return + case agentapi.RecoveryAgentUnreachable: + // The machine's own in-house service never answered, so there is no verdict at all. A + // different fault from the one above, with a different remedy — and, again, the code was + // not used. + s.renderRecovery(w, r, "A gép házon belüli szolgáltatása most nem válaszol, ezért a mentéseidet nem tudtuk megnyitni. A kódodat NEM használtuk fel, és semmi nem változott — tedd el biztonságos helyen. A gép magától rendbe jöhet; próbáld újra néhány perc múlva, és ha egy óra múlva sem megy, szólj a Felhom ügyfélszolgálatának.", "", nil) + return + case agentapi.RecoveryNoBundle: + // The hub answered, and it holds nothing for this machine. Not the customer's doing, and + // not something a different code would fix. + s.renderRecovery(w, r, "Ehhez a géphez nem őrzünk lezárt csomagot, ezért nincs mit megnyitni. Ez nem a kódoddal van összefüggésben. Ha korábban készültek házon kívüli mentéseid, szólj a Felhom ügyfélszolgálatának.", "", nil) + return + case agentapi.RecoveryBundleTooOld: + // The code WORKED — the bundle opened. It simply predates the field we need. + s.renderRecovery(w, r, "A kódod megnyitotta a csomagot, de az még nem tartalmazza a házon kívüli tárhely kulcsát — régebben készült, mint amikor ezt elkezdtük belerakni, és utólag nem pótolható. A kódoddal semmi baj. Keresd a Felhom ügyfélszolgálatát.", "", nil) + return + case agentapi.RecoveryAskedAndRefused: + // The bundle was fetched and the code did not open it. THIS is the only class from which + // the customer may be told to check their typing — see the two messages below. + default: + // ── THE SAFE DEFAULT (R-224 §7.1). ──────────────────────────────────────────────── + // + // The cause could not be determined: an unrecognised status, an unexpected transport + // shape, or an agent older than v0.126.0 whose 400 means "wrong code OR failed fetch" + // and cannot be told apart. **We do not know, so we do not guess — and we certainly do + // not guess the customer.** + // + // It claims neither that the code was wrong nor that it went unused; both would be + // inventions. This is the branch whose ABSENCE let the defect survive being fixed once. + s.renderRecovery(w, r, "A művelet nem fejeződött be, és nem tudjuk biztosan, miért. Semmi nem változott, és a mentéseid érintetlenek. Próbáld újra néhány perc múlva — ha másodszorra sem sikerül, szólj a Felhom ügyfélszolgálatának.", "", nil) + return + } // ── 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 @@ -276,12 +372,25 @@ func (s *Server) recoveryUnlockHandler(w http.ResponseWriter, r *http.Request) { // 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). + // + // ── R-226 — AND IT MUST NAME THE MISTYPE TOO. ───────────────────────────────────────── + // + // As shipped, this message spoke only of an earlier package and sent the customer to support. + // But it is tested BEFORE the typing message, so on every box the hub keeps an earlier package + // for — precisely the boxes whose customer has just been handed a NEW recovery code and is + // most likely to be typing one — a genuine mistype produced this text and the ten-words prompt + // became unreachable. Measured on 2026-08-05 (CAMPAIGN-11 F1): three deliberately wrong codes, + // three real ~1 s unseals, three copies of this message. + // + // The two are INDISTINGUISHABLE at the engine — both fail closed against the current package — + // so the honest message names both and does not pretend to know which. Do not try to tell them + // apart; there is nothing to tell them apart with. 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) + s.renderRecovery(w, r, "Ez a kód nem nyitotta meg azt a csomagot, amit most őrzünk ehhez a géphez. Két oka lehet, és innen nem tudjuk megkülönböztetni őket. Lehet elgépelés: ellenőrizd, hogy mind a tíz szót pontosan, szóközökkel elválasztva írtad-e be — a kis- és nagybetűk nem számítanak. Vagy 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. Ha újrapróbálod és úgy sem megy, és 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. ────────── diff --git a/controller/internal/web/recovery_test.go b/controller/internal/web/recovery_test.go index 0674b6e..f4f077a 100644 --- a/controller/internal/web/recovery_test.go +++ b/controller/internal/web/recovery_test.go @@ -3,7 +3,6 @@ package web import ( "context" "encoding/json" - "fmt" "io" "log" "net/http" @@ -38,16 +37,26 @@ type fakeRecoverer struct { pw string sha string fail bool - codes []string + // failWith overrides `fail` with an EXACT error value, so a test can drive one specific + // R-224 class (a 502 fetch failure, a transport error, an unrecognised status). + failWith error + codes []string } func (f *fakeRecoverer) RecoverOffsiteRepoPassword(_ context.Context, code string) (string, string, error) { f.mu.Lock() f.codes = append(f.codes, code) f.mu.Unlock() + if f.failWith != nil { + return "", "", f.failWith + } if f.fail { - // The shape the agent returns: names the STEP, never the code. - return "", "", fmt.Errorf("unseal failed: age: incorrect passphrase") + // R-224: the shape agent >= v0.126.0 returns for a code that was TRIED AND REFUSED — HTTP 400, + // meaning the bundle was fetched and `age -d` rejected it. It used to be a bare error here, + // which is the shape of a failure we could NOT classify; under the R-224 rule that now renders + // the neutral message, and rightly so. Saying "wrong code" in a test requires saying it the way + // the agent says it. + return "", "", &agentapi.RecoveryRefusal{Status: 400, Reason: "the recovery code did not open the sealed bundle — nothing was written"} } return f.pw, f.sha, nil } @@ -133,6 +142,9 @@ func newRecoveryFixture(t *testing.T) *recoveryFixture { // 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 }) + // R-224: the fixture's agent is a current one, so a 400 may be read as a genuine refusal. Tests + // that need the OLD-agent behaviour override this explicitly. + s.SetRecoveryRefusalTrusted(func(context.Context) bool { return true }) 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 5b4830b..b863133 100644 --- a/controller/internal/web/server.go +++ b/controller/internal/web/server.go @@ -107,6 +107,12 @@ type Server struct { // 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 + // recoveryRefusalTrustedFn overrides the R-224 gate deciding whether a 400 may be read as a + // genuine refusal (tests). nil → the agent-version path. + recoveryRefusalTrustedFn func(context.Context) bool + // recoveryNowFn is the unlock path's clock (tests inject; nil → time.Now). Observability and + // tests only — never a classifier. + recoveryNowFn func() time.Time // 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.