From 1e759a16ec3bfe199ae672e010d43e1c393c44d2 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 6 Aug 2026 08:06:57 +0200 Subject: [PATCH] R-224/R-226 Part 1: why the unlock failed decides what we say MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The failure branch was 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 2026-08-05 with a CORRECT current code: hub firewalled off 0.0556s, agent stopped 0.0299s, against ~1.0s for a real unseal. Five classes, from the VALUE and never the text: hub-unreachable 502/503 from the agent — the code was NOT used agent-unreachable no agent verdict at all (transport) — NOT used no-bundle 404 bundle-too-old 409 asked-and-refused 400 — the ONLY class that may mention typing unknown everything else -> NEUTRAL, the safe default agentapi.RecoveryRefusal carries the status as a value (refusalError flattened it into a sentence, and a sentence is not something a caller can branch on). THE OLD-AGENT CASE IS WHY THIS NEEDS A COUPLING. Agent < 0.126.0 answers 400 for both a fetch failure and a wrong code, so a 400 from one cannot be read as a refusal. FeatureRecoveryFailureClass (MinAgent 0.126.0) withholds that reading and the 400 degrades to neutral. The gate BLOCKS NOTHING — it only decides whether the customer may be told to check their typing. R-226: the superseded message now names BOTH possibilities and restores the ten-words prompt. The two are indistinguishable at the engine; the honest message says so. It still does not promise the earlier package can be opened. Elapsed time is logged (it is what diagnosed this) and is NEVER a classifier. Tests: scenarios A-E at the HANDLER + the classifier table asserting the same sentence under two statuses classifies two ways. Red-proofs, each demonstrated failing then restored: delete the 502 case (A), remove the mistype clause (C), default to the accusation (D), route an instant transport failure to the typing message (E). Two existing tests encoded the defect and were corrected, not deleted: the web fake returned a BARE error for 'wrong code' (which is the shape of a failure we cannot classify), and R-222's test forbade any mention of typing on a superseded box — half of which R-226 deliberately reverses. 28 packages ok, vet clean, all controller gates OK. --- controller/internal/agentapi/escrow.go | 114 +++++++- controller/internal/agentapi/features.go | 22 ++ .../internal/web/recovery_class_test.go | 244 ++++++++++++++++++ controller/internal/web/recovery_gate_test.go | 20 +- controller/internal/web/recovery_handlers.go | 113 +++++++- controller/internal/web/recovery_test.go | 20 +- controller/internal/web/server.go | 6 + 7 files changed, 529 insertions(+), 10 deletions(-) create mode 100644 controller/internal/web/recovery_class_test.go 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.