From f62a1158913c2cde3055804ccf21deaa32e8e985 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 5 Aug 2026 10:48:18 +0200 Subject: [PATCH] R-204 item 4 (hub half): the hub answers a rebuilt box's request (hub v0.96.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New internal/offsiteheal, the sibling of pbsdrheal: it acts ONLY on the state the box declares, sustained across two distinct reports, re-staging the stored credential before ever minting a new one. A healthy box is a pure no-op; it never blind-timer-reissues and never re-runs a provisioning step. RESTAGE IS POSSIBLE because the stored value survives a consume — established from the schema and ConsumeOneTimeSecret (which stamps consumed_at and nothing else), not inherited from the PBS analogy, and pinned by a test that asserts the SAME value comes back. reportHasOffsite is TIGHTENED to require enabled:true. Its comment asserted that presence == applied-on-the-box, and the declaration deliberately breaks that premise; left alone it would have read a request for help as proof the tier was applied. Provably a no-op for every report shape that existed before, because an attached object has always carried enabled:true. R-192's guard half is CLOSED BY REPLACEMENT: the delivery checker's counting inference read the OLDEST 500 reports after a consume — all predating a rebuild, which is why demo-hp sat stranded for 108 reports under a confident regressed-shape verdict. A declaration outranks both inferred shapes, and the checker stands down with a record so the two mechanisms cannot double-issue. No escrow ceremony is ever run or requested: credential automatic, key customer-present. --- hub/cmd/hub/main.go | 28 +- hub/internal/monitor/offsite_declared_test.go | 47 +++ hub/internal/monitor/offsite_delivery.go | 51 ++- hub/internal/offsiteheal/reconciler.go | 284 ++++++++++++++++ hub/internal/offsiteheal/reconciler_test.go | 310 ++++++++++++++++++ hub/internal/offsiteheal/wiring_test.go | 58 ++++ hub/internal/store/offsite_restage_test.go | 177 ++++++++++ hub/internal/store/store.go | 87 ++++- 8 files changed, 1026 insertions(+), 16 deletions(-) create mode 100644 hub/internal/monitor/offsite_declared_test.go create mode 100644 hub/internal/offsiteheal/reconciler.go create mode 100644 hub/internal/offsiteheal/reconciler_test.go create mode 100644 hub/internal/offsiteheal/wiring_test.go create mode 100644 hub/internal/store/offsite_restage_test.go diff --git a/hub/cmd/hub/main.go b/hub/cmd/hub/main.go index 1c41178..56da5bd 100644 --- a/hub/cmd/hub/main.go +++ b/hub/cmd/hub/main.go @@ -20,14 +20,15 @@ import ( "gitea.dooplex.hu/admin/felhom-hub/internal/hetznerapi" "gitea.dooplex.hu/admin/felhom-hub/internal/intent" "gitea.dooplex.hu/admin/felhom-hub/internal/mailrelay" - "gitea.dooplex.hu/admin/felhom-hub/internal/offsite" - "gitea.dooplex.hu/admin/felhom-hub/internal/pbsdrheal" "gitea.dooplex.hu/admin/felhom-hub/internal/monitor" "gitea.dooplex.hu/admin/felhom-hub/internal/notify" + "gitea.dooplex.hu/admin/felhom-hub/internal/offsite" + "gitea.dooplex.hu/admin/felhom-hub/internal/offsiteheal" + "gitea.dooplex.hu/admin/felhom-hub/internal/pbsdrheal" "gitea.dooplex.hu/admin/felhom-hub/internal/poke" "gitea.dooplex.hu/admin/felhom-hub/internal/store" - "gitea.dooplex.hu/admin/felhom-hub/internal/web" "gitea.dooplex.hu/admin/felhom-hub/internal/tenantsync" + "gitea.dooplex.hu/admin/felhom-hub/internal/web" "gitea.dooplex.hu/admin/felhom-hub/internal/wgsync" "gopkg.in/yaml.v3" ) @@ -290,8 +291,8 @@ func main() { webServer := web.New(dataStore, cfg.Auth.PasswordHash, cfg.API.ReportAPIKey, Version, staleThreshold, logger) webServer.SetTemplateFetcher(templateFetcher) webServer.SetAssetManager(assetsMgr) - webServer.SetClaimEngine(claimEngine) // v0.50.0 — Setup-tab claim chip + resend button - webServer.SetSelfBindMailer(dispatcher) // v0.66.0 (R-27) — customer self-bind link button (sibling of claim mailer) + webServer.SetClaimEngine(claimEngine) // v0.50.0 — Setup-tab claim chip + resend button + webServer.SetSelfBindMailer(dispatcher) // v0.66.0 (R-27) — customer self-bind link button (sibling of claim mailer) // Day-0 artifact version dropdowns: let the operator pick a version and have the hub derive the // sha256 from Gitea (no hand-copied checksums). Reuses the registry creds; degrades to manual text // entry when they're absent. @@ -510,6 +511,23 @@ func main() { logger.Printf("[INFO] PBS-DR self-heal reconciler started (interval 5m)") } + // Off-site credential self-heal reconciler (internal/offsiteheal, R-193 / R-204 item 4). The + // sibling of the block above, for the last of the four manual interventions the 2026-08-04 drill + // needed: a REBUILT box has no off-site credential because its predecessor spent the one-time + // password. It acts ONLY on a state the box DECLARES (an absence has four meanings and the hub + // cannot tell them apart; the box can), sustained across two distinct reports, re-staging the + // stored secret before ever minting. A healthy box is a pure no-op. OFFSITEHEAL_ONLY_CUSTOMER + // scopes a supervised first rollout (empty = whole fleet — the steady state). + { + offsiteReconciler := offsiteheal.NewReconciler(dataStore, offsiteheal.NewActions(dataStore, webServer), logger) + if only := os.Getenv("OFFSITEHEAL_ONLY_CUSTOMER"); only != "" { + offsiteReconciler.RestrictToCustomer(only) + logger.Printf("[INFO] offsite credential self-heal RESTRICTED to customer %s (supervised rollout scope)", only) + } + go offsiteReconciler.Run(ctx) + logger.Printf("[INFO] offsite credential self-heal reconciler started (interval 5m, debounce 2 reports)") + } + // Session cleanup — removes expired sessions every hour go webServer.CleanupSessions(ctx) diff --git a/hub/internal/monitor/offsite_declared_test.go b/hub/internal/monitor/offsite_declared_test.go new file mode 100644 index 0000000..ff3e404 --- /dev/null +++ b/hub/internal/monitor/offsite_declared_test.go @@ -0,0 +1,47 @@ +package monitor + +import ( + "testing" + + "gitea.dooplex.hu/admin/felhom-hub/internal/offsite" + "gitea.dooplex.hu/admin/felhom-hub/internal/offsiteheal" +) + +// R-192's guard half, closed by REPLACEMENT (R-204 item 4). +// +// The counting guard inferred the situation from how many of the OLDEST 500 reports after a consume +// carried an offbox target. On demo-hp all 500 predated the rebuild, so the checker confidently +// reported the REGRESSED shape and declined to heal — for 108 reports, while the box sat stranded. +// A declaration needs no window and no count, and it OUTRANKS both inferred shapes. + +func TestShapeOf_DeclarationOutranksBothInferredShapes(t *testing.T) { + cases := []struct { + name string + status offsite.DeliveryStatus + declared bool + want deliveryShape + }{ + {"burned, undeclared", offsite.DeliveryStatus{OffsiteReportsSinceConsume: 0, ReportsSinceConsume: 9}, false, shapeBurned}, + {"regressed, undeclared", offsite.DeliveryStatus{OffsiteReportsSinceConsume: 500, ReportsSinceConsume: 500}, false, shapeRegressed}, + // THE demo-hp SHAPE: the counts say "regressed" from a window that predates the rebuild, and + // the box says it needs a credential. The declaration wins. + {"regressed counts BUT the box declares", offsite.DeliveryStatus{OffsiteReportsSinceConsume: 500, ReportsSinceConsume: 500}, true, shapeDeclared}, + {"burned counts AND the box declares", offsite.DeliveryStatus{OffsiteReportsSinceConsume: 0, ReportsSinceConsume: 9}, true, shapeDeclared}, + } + for _, tc := range cases { + if got := shapeOf(tc.status, tc.declared); got != tc.want { + t.Errorf("%s: shapeOf = %q, want %q", tc.name, got, tc.want) + } + } +} + +// The declared-state string is duplicated in three packages (the controller declares it, this checker +// recognises it, the reconciler acts on it). A silent drift between them would make the whole feature +// inert with every test still green — so the two hub-side copies are pinned to each other here. The +// controller's copy is pinned by its own report-shape test and by the live validation. +func TestDeclaredStateStringMatchesTheReconciler(t *testing.T) { + if declaredNeedsCredential != offsiteheal.StateNeedsCredential { + t.Fatalf("declared-state drift: monitor has %q, offsiteheal has %q — the checker would never stand down and both mechanisms would heal the same customer", + declaredNeedsCredential, offsiteheal.StateNeedsCredential) + } +} diff --git a/hub/internal/monitor/offsite_delivery.go b/hub/internal/monitor/offsite_delivery.go index 50e4984..1bd6bf1 100644 --- a/hub/internal/monitor/offsite_delivery.go +++ b/hub/internal/monitor/offsite_delivery.go @@ -94,8 +94,19 @@ func (c *OffsiteDeliveryChecker) Check() { if age < stuckAfter { continue // normal convergence window } - emitted := c.maybeEmitStuck(cfg.CustomerID, status, age) - c.maybeHeal(cfg.CustomerID, status, emitted) + // R-204 item 4 / R-192's guard half: does the BOX declare that it needs a credential? A + // declaration is stronger evidence than anything this checker can infer, and it is owned by + // internal/offsiteheal — see maybeHeal. A read error is treated as "no declaration", which is + // the conservative direction: this checker keeps its pre-v0.96.0 behaviour rather than + // silently standing down. + declared := false + if _, _, state, derr := c.store.LatestReportOffsiteDeclaration(cfg.CustomerID); derr != nil { + c.logger.Printf("[WARN] offsite-delivery: %s: read declaration: %v (treating as undeclared)", cfg.CustomerID, derr) + } else { + declared = state == declaredNeedsCredential + } + emitted := c.maybeEmitStuck(cfg.CustomerID, status, age, declared) + c.maybeHeal(cfg.CustomerID, status, emitted, declared) } } @@ -111,9 +122,25 @@ const ( // the credential worked and the target was later lost (a guest rebuild does exactly this, R-193). // Re-issue is NOT indicated; it treats a symptom whose cause is elsewhere. shapeRegressed deliveryShape = "regressed" + // shapeDeclared — the BOX itself says it needs a credential (controller >= v0.199.0). This + // OUTRANKS both shapes above and is R-192's guard half closed by REPLACEMENT rather than repair: + // the counting guard inferred the situation from how many of the OLDEST 500 reports after the + // consume carried an offbox target — all of which predate a rebuild, which is why demo-hp sat + // stranded for 108 reports while the checker confidently reported the regressed shape. A + // declaration needs no window, no count and no inference. The remediation belongs to + // internal/offsiteheal; this checker stands down and says so. + shapeDeclared deliveryShape = "declared" ) -func shapeOf(status offsite.DeliveryStatus) deliveryShape { +// declaredNeedsCredential mirrors backup.OffsiteStateNeedsCredential / offsiteheal.StateNeedsCredential. +// Duplicated as a literal rather than imported to avoid a monitor→offsiteheal dependency; the three +// are pinned together by TestDeclaredStateStringMatchesTheReconciler. +const declaredNeedsCredential = "needs_credential" + +func shapeOf(status offsite.DeliveryStatus, declared bool) deliveryShape { + if declared { + return shapeDeclared + } if status.OffsiteReportsSinceConsume == 0 { return shapeBurned } @@ -137,7 +164,7 @@ func shapeOf(status offsite.DeliveryStatus) deliveryShape { // assembled (R-199/R-200/R-201). Naming the window in the text is how it stays visible instead of // being laundered into a confident sentence — an instrument that can silently mis-scope its results // must say so where it reports them. -func (c *OffsiteDeliveryChecker) maybeEmitStuck(customerID string, status offsite.DeliveryStatus, age time.Duration) bool { +func (c *OffsiteDeliveryChecker) maybeEmitStuck(customerID string, status offsite.DeliveryStatus, age time.Duration, declared bool) bool { last, err := c.store.LastEventAt(customerID, eventDeliveryStuck) if err != nil { c.logger.Printf("[WARN] offsite-delivery: %s: cooldown read: %v", customerID, err) @@ -146,9 +173,12 @@ func (c *OffsiteDeliveryChecker) maybeEmitStuck(customerID string, status offsit if !last.IsZero() && c.now().Sub(last) < stuckCooldown { return false } - shape := shapeOf(status) + shape := shapeOf(status, declared) var msg string switch shape { + case shapeDeclared: + msg = fmt.Sprintf("Offsite delivery stuck (DECLARED by the box): the one-time password was consumed %s ago and the box now reports `offsite.state=%s` — it has been rebuilt, holds no repository password, and the hub is holding a sealed recovery package for it. No inference was needed: the box said so. The offsite self-heal reconciler owns this remediation (it re-arms the stored credential before minting a new one); no operator action is indicated unless this repeats.", + age.Round(time.Minute), declaredNeedsCredential) case shapeBurned: msg = fmt.Sprintf("Offsite delivery stuck (BURNED-credential shape): the one-time password was consumed %s ago; of the first %d report(s) after that consume, NONE carried an offbox target, and the latest report carries none either. The credential never reached a persisted apply. Re-issue delivers a fresh one.", age.Round(time.Minute), status.ReportsSinceConsume) @@ -185,10 +215,19 @@ func (c *OffsiteDeliveryChecker) maybeEmitStuck(customerID string, status offsit // the set of situations in which the heal fires is byte-for-byte what it was; only the silence is // gone. The two conditions are split into separate branches solely so each refusal can name its own // reason. -func (c *OffsiteDeliveryChecker) maybeHeal(customerID string, status offsite.DeliveryStatus, recordRefusal bool) { +func (c *OffsiteDeliveryChecker) maybeHeal(customerID string, status offsite.DeliveryStatus, recordRefusal bool, declared bool) { if c.reissuer == nil { return // no provisioner configured: the heal does not exist on this hub, so there is nothing to explain } + // R-204 item 4: a DECLARING box belongs to internal/offsiteheal, which re-arms the stored + // credential before minting a new one. Two mechanisms healing the same customer would double-issue + // — and this one can only mint, so it would also skip the cheap path. Stand down, with a record: + // "we chose not to act" and "the heal never ran" must not look identical (R-192 defect (b)). + if declared { + c.recordHealRefusal(customerID, recordRefusal, + "the box DECLARES offsite.state=needs_credential; internal/offsiteheal owns this remediation (it re-stages the stored credential before minting). A second mechanism minting here would double-issue.") + return + } if status.OffsiteReportsSinceConsume != 0 { c.recordHealRefusal(customerID, recordRefusal, fmt.Sprintf( "regressed-apply shape: %d of the first %d report(s) after the consume DID carry an offbox target, so a burned credential is ruled out — a restage would treat a symptom whose cause is elsewhere. Operator's call (R-193).", diff --git a/hub/internal/offsiteheal/reconciler.go b/hub/internal/offsiteheal/reconciler.go new file mode 100644 index 0000000..02169fa --- /dev/null +++ b/hub/internal/offsiteheal/reconciler.go @@ -0,0 +1,284 @@ +// Package offsiteheal is the hub-side OFF-SITE credential self-heal reconciler (R-193 / R-204 item 4, +// hub v0.96.0). It is the sibling of internal/pbsdrheal, deliberately: same shape, same restraint, +// same rule that one thing is left loud rather than healed. +// +// THE PROBLEM. The 2026-08-04 drill (R-201) proved a customer's file survives a machine rebuild and +// comes back — with a person present for four manual interventions. Three were closed in controller +// v0.198.0 / hub v0.95.0. The fourth is this one: a REBUILT box has no off-site credential of its own, +// because the one-time provider password was spent by its predecessor. Everything downstream is +// self-service; nothing gets the box past that first step. +// +// WHY THE TRIGGER IS A DECLARATION AND NOT AN INFERENCE (the operator ruling, 2026-08-05, and the +// whole design). From the hub, an ABSENT off-site object has FOUR meanings — never configured, +// mid-restart, a transient config read failure, and rebuilt-and-stranded — and the hub cannot tell +// them apart. The BOX can, from two local facts it holds with certainty: its data area is fresh (no +// repository password) AND the hub is holding a sealed recovery package for it. So the box says so, +// in its ordinary report, and this reconciler acts on a stated request rather than on a silence. +// That is also why R-192's counting guard is REPLACED rather than repaired: it inferred the same +// thing by counting reports over the OLDEST 500 after a consume, all of which predate a rebuild. +// +// WHAT IT DOES, once a declaration has held across a debounce: RE-STAGES the customer's stored +// one-time secret (store.RestageOneTimeSecret — no provider call, no new password) and escalates to +// the existing Re-issue ONLY when there is nothing stored to re-arm. It NEVER blind-timer-reissues +// and NEVER re-runs a provisioning step. A box that is not declaring is a pure no-op. +// +// WHAT IT DELIBERATELY DOES NOT DO: it never runs, or asks for, an escrow ceremony. A credential is +// replaceable; the recovery code is not, because only the customer holds it. Credential automatic, +// key customer-present — the ruling this session implements and must not quietly widen. +package offsiteheal + +import ( + "context" + "log" + "time" + + "gitea.dooplex.hu/admin/felhom-hub/internal/offsite" + "gitea.dooplex.hu/admin/felhom-hub/internal/store" +) + +// StateNeedsCredential is the ONE state acted on — the string controller v0.199.0 declares on its +// report's `offsite` object (backup.OffsiteStateNeedsCredential). Everything else, including "" (an +// older controller, a healthy box, or a box with no off-site object at all), is a no-op. +// +// THE §8.4 QUESTION — IS THERE AN EQUIVALENT OF pbsdrheal's DELIBERATELY-UNHEALED `verify_failed`? +// Yes, and it is handled by CONSTRUCTION rather than by a case here, which is worth stating plainly +// because "there is nothing like that here" is usually wrong. The analogue is the REGRESSED shape: +// a box that HAD a working off-site tier and lost its target while still holding its repository +// password. Re-arming a credential would not help it — its problem is whatever removed the target — +// and R-192's existing checker already refuses to heal that shape for exactly this reason. It cannot +// reach this reconciler at all, because the controller's declaration predicate requires the +// repository password to be ABSENT (backup.needsOffsiteCredential). So the unhealable case is +// excluded upstream, by the declaration itself, rather than filtered out here. +const StateNeedsCredential = "needs_credential" + +// Audit event types (store.SaveEvent; hub-internal, not gated by allowedEventTypes — the pbsdr_* +// precedent). Distinct per remediation so the operator sees exactly what was auto-done. +const ( + eventRestaged = "offsite_selfheal_restaged" // re-armed the stored secret (routine, no provider call) + eventReissued = "offsite_selfheal_reissued" // nothing stored to re-arm → minted a fresh credential +) + +// debounceReportsDefault — how many DISTINCT reports must carry the declaration before acting. +// +// TWO, and the interval is derived rather than chosen. The controller reports every ~15 minutes, so +// two distinct declarations mean the state has survived at least one full report cycle: a restart, a +// slow first report or a transient config read cannot produce it, because each of those resolves +// well inside one cycle. One report would act on a blip; three would leave a genuinely stranded +// customer waiting ~45 minutes for a credential they cannot obtain any other way. The reconciler's +// own tick (below) is deliberately FASTER than the report cadence so it never adds latency of its +// own — the debounce is counted in fresh evidence, not in ticks. +const debounceReportsDefault = 2 + +// tickIntervalDefault — how often the fleet is swept. Shorter than the 15-minute report cadence on +// purpose (see above); a sweep that finds nothing writes nothing. +const tickIntervalDefault = 5 * time.Minute + +// Actions is the mutation seam — fakes in tests count calls without touching a provider. Restage +// flips a stored secret's consumed flag (false when NO row exists → the caller escalates); Reissue +// mints a fresh provider credential and stores a fresh consume-once secret. +type Actions interface { + Restage(customerID string) (restaged bool, err error) + Reissue(ctx context.Context, customerID string) error +} + +// Reissuer is satisfied by *web.Server (its ReissueOffsiteForCustomer) — the same indirection +// pbsdrheal uses, so the escalation IS the designed Re-issue path and not a sibling mechanism. +type Reissuer interface { + ReissueOffsiteForCustomer(ctx context.Context, customerID string) error +} + +type storeActions struct { + st *store.Store + reissuer Reissuer +} + +func (a storeActions) Restage(customerID string) (bool, error) { + return a.st.RestageOneTimeSecret(customerID) +} +func (a storeActions) Reissue(ctx context.Context, customerID string) error { + return a.reissuer.ReissueOffsiteForCustomer(ctx, customerID) +} + +// NewActions builds the production mutation seam. +func NewActions(st *store.Store, reissuer Reissuer) Actions { return storeActions{st: st, reissuer: reissuer} } + +// debounceState tracks, per customer, the last DISTINCT report seen and how many consecutive +// distinct reports have carried the declaration. +type debounceState struct { + reportID int64 + state string + streak int +} + +// Reconciler re-arms stranded boxes. DECLARATIVE + IDEMPOTENT: a tick over a healthy fleet writes +// nothing. It reads the hub DB — never the box. +type Reconciler struct { + store *store.Store + act Actions + interval time.Duration + debounceReports int + onlyCustomer string // "" = whole fleet; non-empty restricts the work set (supervised rollout) + trigger chan struct{} + logger *log.Logger + + deb map[string]debounceState +} + +// NewReconciler builds the reconciler with the derived defaults. +func NewReconciler(st *store.Store, act Actions, logger *log.Logger) *Reconciler { + if logger == nil { + logger = log.Default() + } + return &Reconciler{ + store: st, + act: act, + interval: tickIntervalDefault, + debounceReports: debounceReportsDefault, + trigger: make(chan struct{}, 1), + logger: logger, + deb: map[string]debounceState{}, + } +} + +// RestrictToCustomer scopes the work set to one customer (empty = whole fleet) for a supervised first +// rollout — the pbsdrheal precedent. Set before Run. +func (r *Reconciler) RestrictToCustomer(customerID string) { r.onlyCustomer = customerID } + +// SetDebounceReports overrides the debounce (tests). Values < 1 are ignored. +func (r *Reconciler) SetDebounceReports(n int) { + if n >= 1 { + r.debounceReports = n + } +} + +// Trigger requests an immediate reconcile. Non-blocking. +func (r *Reconciler) Trigger() { + select { + case r.trigger <- struct{}{}: + default: + } +} + +// Run loops until ctx is done. Never exits on an error. +func (r *Reconciler) Run(ctx context.Context) { + ticker := time.NewTicker(r.interval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-r.trigger: + case <-ticker.C: + } + r.ReconcileOnce(ctx) + } +} + +// ReconcileOnce sweeps the fleet once. Exported so tests drive it directly instead of racing a +// ticker, and so the wiring can trigger it. Errors are logged and retried next tick; a read failure +// means this reconciler does NOTHING rather than acting on a partial view. +func (r *Reconciler) ReconcileOnce(ctx context.Context) { + configs, err := r.store.ListCustomerConfigs() + if err != nil { + r.logger.Printf("[ERROR] offsiteheal: list customer configs: %v (retry next tick)", err) + return + } + seen := make(map[string]bool, len(configs)) + for _, cfg := range configs { + if r.onlyCustomer != "" && cfg.CustomerID != r.onlyCustomer { + continue + } + if cfg.Status != "active" { + continue // blocked/inactive customers are never healed + } + // The descriptor must still say the customer HAS an off-site tier. A deliberately disabled + // tier must not be re-credentialed behind the operator's back; an unparseable config never + // drives a heal (the config UI owns that failure). + d, derr := offsite.ReadDescriptor(cfg.ConfigJSON) + if derr != nil || d == nil || !d.Enabled { + delete(r.deb, cfg.CustomerID) + continue + } + found, reportID, state, err := r.store.LatestReportOffsiteDeclaration(cfg.CustomerID) + if err != nil { + r.logger.Printf("[ERROR] offsiteheal: %s: read latest declaration: %v (retry next tick)", cfg.CustomerID, err) + continue // never act on a partial view + } + seen[cfg.CustomerID] = true + if !found || state != StateNeedsCredential { + delete(r.deb, cfg.CustomerID) // healthy, silent, or an older controller → pure no-op + continue + } + if !r.confirm(cfg.CustomerID, reportID, state) { + continue // one declaration is not evidence — wait for a second distinct report + } + r.heal(ctx, cfg.CustomerID, reportID, state) + } + for c := range r.deb { + if !seen[c] { + delete(r.deb, c) + } + } +} + +// confirm advances the per-customer debounce and reports whether the declaration has held across +// >= debounceReports DISTINCT reports. A re-observed same report never advances the streak — the +// debounce counts fresh evidence, not reconciler ticks, so a fast tick cannot shorten it. +func (r *Reconciler) confirm(customerID string, reportID int64, state string) bool { + st := r.deb[customerID] + if reportID != st.reportID { + if state == st.state { + st.streak++ + } else { + st.streak = 1 + } + st.state = state + st.reportID = reportID + r.deb[customerID] = st + } + return st.streak >= r.debounceReports +} + +// heal re-arms the stored secret; with nothing stored, escalates to a fresh mint. +// +// RESTAGE BEFORE MINT, because re-arming costs no provider call and converges in one report tick, +// while a mint is external churn for a credential the hub already holds. The stored value survives a +// consume (store.RestageOneTimeSecret documents how that was established), which is what makes the +// cheap path possible at all. +func (r *Reconciler) heal(ctx context.Context, customerID string, reportID int64, state string) { + restaged, err := r.act.Restage(customerID) + if err != nil { + r.logger.Printf("[ERROR] offsiteheal: re-stage %s: %v (retry next tick)", customerID, err) + return + } + if restaged { + r.logger.Printf("[INFO] offsiteheal: re-staged the stored one-time offsite secret for customer %s (declared %s across %d reports) — the box re-consumes on its next cycle; no provider credential was minted", + customerID, state, r.debounceReports) + r.event(customerID, eventRestaged, "info", + "Offsite self-heal: a rebuilt box asked for its storage credential and the stored one-time password was re-armed. No new credential was created at the storage provider.") + r.resetAfterHeal(customerID, reportID, state) + return + } + r.logger.Printf("[INFO] offsiteheal: customer %s declares %s with NO stored one-time secret — escalating to Re-issue", customerID, state) + if err := r.act.Reissue(ctx, customerID); err != nil { + r.logger.Printf("[ERROR] offsiteheal: re-issue for customer %s: %v (retry next tick)", customerID, err) + return + } + r.event(customerID, eventReissued, "warning", + "Offsite self-heal: a rebuilt box asked for its storage credential and the hub had none stored, so fresh credentials were issued at the storage provider.") + r.resetAfterHeal(customerID, reportID, state) +} + +// resetAfterHeal clears the streak (keeping the report id) so the SAME report cannot re-heal on the +// next tick — a fresh report must re-confirm the box is still stranded before acting again. This is +// what makes Scenario E's "one mint, not repeated" true. +func (r *Reconciler) resetAfterHeal(customerID string, reportID int64, state string) { + r.deb[customerID] = debounceState{reportID: reportID, state: state, streak: 0} +} + +// event records an audit row; a failure to write it must never break the heal loop. +func (r *Reconciler) event(customerID, eventType, severity, message string) { + if _, err := r.store.SaveEvent(customerID, eventType, severity, message, "", "hub"); err != nil { + r.logger.Printf("[WARN] offsiteheal: audit event %s for %s not stored: %v", eventType, customerID, err) + } +} diff --git a/hub/internal/offsiteheal/reconciler_test.go b/hub/internal/offsiteheal/reconciler_test.go new file mode 100644 index 0000000..83c88e2 --- /dev/null +++ b/hub/internal/offsiteheal/reconciler_test.go @@ -0,0 +1,310 @@ +package offsiteheal + +// Non-hollow reconciler tests mapping 1:1 to the task's integration scenarios A–F. A REAL store +// (t.TempDir sqlite) supplies the work set, the customer configs and the reports; a FAKE Actions seam +// counts restage/reissue calls without touching a storage provider. ReconcileOnce is driven directly +// for determinism — the debounce is exercised by feeding DISTINCT REPORTS, never by sleeping, because +// the debounce counts fresh evidence rather than elapsed time. + +import ( + "context" + "encoding/json" + "fmt" + "io" + "log" + "path/filepath" + "sync" + "testing" + + "gitea.dooplex.hu/admin/felhom-hub/internal/store" +) + +type fakeActions struct { + mu sync.Mutex + restaged []string + reissued []string + restageResult bool // does a stored secret row exist? + restageErr error + reissueErr error +} + +func (f *fakeActions) Restage(customerID string) (bool, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.restaged = append(f.restaged, customerID) + return f.restageResult, f.restageErr +} + +func (f *fakeActions) Reissue(_ context.Context, customerID string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.reissued = append(f.reissued, customerID) + return f.reissueErr +} + +func (f *fakeActions) restages() int { f.mu.Lock(); defer f.mu.Unlock(); return len(f.restaged) } +func (f *fakeActions) reissues() int { f.mu.Lock(); defer f.mu.Unlock(); return len(f.reissued) } + +func newHealStore(t *testing.T) *store.Store { + t.Helper() + st, err := store.New(filepath.Join(t.TempDir(), "test.db"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatalf("store.New: %v", err) + } + t.Cleanup(func() { st.Close() }) + return st +} + +// seedCustomer writes an ACTIVE customer whose offsite descriptor is enabled (or not). +func seedCustomer(t *testing.T, st *store.Store, customerID string, offsiteEnabled bool) { + t.Helper() + cfgJSON := `{"offsite":{"enabled":false}}` + if offsiteEnabled { + cfgJSON = `{"offsite":{"enabled":true,"type":"shared","host":"x.your-storagebox.de","user":"u","port":23,"repo_path":"/home/felhom-repo"}}` + } + if err := st.SaveCustomerConfig(&store.CustomerConfig{ + CustomerID: customerID, CustomerName: customerID, Domain: customerID + ".hu", + APIKey: "k-" + customerID, ConfigJSON: cfgJSON, + }); err != nil { + t.Fatalf("SaveCustomerConfig %s: %v", customerID, err) + } +} + +// pushReport writes ONE controller report. state=="" → a healthy configured box (enabled offsite +// object); otherwise the declaration shape controller v0.199.0 emits. Each call is a DISTINCT report, +// which is the debounce's currency. +func pushReport(t *testing.T, st *store.Store, customerID, state string, seq int) { + t.Helper() + var off map[string]any + if state == "" { + off = map[string]any{"enabled": true, "escrow_state": "escrowed", "last_status": "ok"} + } else { + off = map[string]any{"enabled": false, "state": state} + } + b, err := json.Marshal(map[string]any{ + "controller_version": fmt.Sprintf("0.199.0-%d", seq), // makes each row distinguishable + "offsite": off, + }) + if err != nil { + t.Fatal(err) + } + if err := st.SaveReport(customerID, b); err != nil { + t.Fatalf("SaveReport %s: %v", customerID, err) + } +} + +// pushReportNoOffsite writes a report with NO offsite object at all — the pre-v0.199.0 shape a +// stranded box used to send, and the shape a box that never had off-site backups still sends. +func pushReportNoOffsite(t *testing.T, st *store.Store, customerID string, seq int) { + t.Helper() + b, _ := json.Marshal(map[string]any{"controller_version": fmt.Sprintf("0.198.0-%d", seq)}) + if err := st.SaveReport(customerID, b); err != nil { + t.Fatalf("SaveReport %s: %v", customerID, err) + } +} + +func newRec(st *store.Store, act Actions) *Reconciler { + return NewReconciler(st, act, log.New(io.Discard, "", 0)) +} + +func countEvents(t *testing.T, st *store.Store, customerID, eventType string) int { + t.Helper() + ev, err := st.GetLatestEventByType(customerID, eventType) + if err != nil { + t.Fatalf("GetLatestEventByType: %v", err) + } + if ev == nil { + return 0 + } + return 1 +} + +// SCENARIO A + D — a declaring box is served, and the STORED secret is re-armed rather than a fresh +// credential minted. +// +// RED-PROOF (D): invert the order in heal() so Reissue is called before Restage — the assertion +// "no mint" fails, and the test names the cost: external churn for a credential the hub already has. +func TestScenarioAD_DeclaringBoxIsRestagedNotMinted(t *testing.T) { + st := newHealStore(t) + act := &fakeActions{restageResult: true} // a stored secret exists + r := newRec(st, act) + seedCustomer(t, st, "c1", true) + + pushReport(t, st, "c1", StateNeedsCredential, 1) + r.ReconcileOnce(context.Background()) + if act.restages() != 0 || act.reissues() != 0 { + t.Fatalf("acted on a SINGLE declaration — the debounce did not hold (restages=%d reissues=%d)", act.restages(), act.reissues()) + } + + pushReport(t, st, "c1", StateNeedsCredential, 2) // a second DISTINCT report + r.ReconcileOnce(context.Background()) + + if act.restages() != 1 { + t.Fatalf("a sustained declaration was not served: restages=%d", act.restages()) + } + if act.reissues() != 0 { + t.Fatalf("a fresh credential was minted although a stored one could be re-armed — external churn for nothing (reissues=%d)", act.reissues()) + } + if countEvents(t, st, "c1", eventRestaged) != 1 { + t.Error("no offsite_selfheal_restaged audit event — an automatic remediation must be visible") + } + if countEvents(t, st, "c1", eventReissued) != 0 { + t.Error("a reissue event was recorded for a restage") + } +} + +// SCENARIO B — a box that never had off-site backups says nothing, so nothing happens. (The +// declaration itself is the controller's guard; here we assert the hub side is a no-op on silence.) +func TestScenarioB_SilentBoxIsNeverActedOn(t *testing.T) { + st := newHealStore(t) + act := &fakeActions{restageResult: true} + r := newRec(st, act) + seedCustomer(t, st, "c1", true) + + for i := 1; i <= 4; i++ { + pushReportNoOffsite(t, st, "c1", i) + r.ReconcileOnce(context.Background()) + } + if act.restages() != 0 || act.reissues() != 0 { + t.Fatalf("a silent box was acted on: restages=%d reissues=%d", act.restages(), act.reissues()) + } +} + +// SCENARIO C — a healthy box is a PURE no-op, repeatedly. No restage, no mint, no event. +func TestScenarioC_HealthyBoxIsAPureNoOp(t *testing.T) { + st := newHealStore(t) + act := &fakeActions{restageResult: true} + r := newRec(st, act) + seedCustomer(t, st, "c1", true) + + for i := 1; i <= 5; i++ { + pushReport(t, st, "c1", "", i) // healthy: enabled offsite object, no declaration + r.ReconcileOnce(context.Background()) + } + if act.restages() != 0 || act.reissues() != 0 { + t.Fatalf("a healthy box was acted on: restages=%d reissues=%d", act.restages(), act.reissues()) + } + if countEvents(t, st, "c1", eventRestaged)+countEvents(t, st, "c1", eventReissued) != 0 { + t.Error("a healthy box produced a self-heal event") + } +} + +// SCENARIO E — nothing to re-arm escalates to a mint, ONCE, and does not repeat while the staged +// secret sits unconsumed (a fresh report must re-confirm before acting again). +func TestScenarioE_NothingToRestageEscalatesOnce(t *testing.T) { + st := newHealStore(t) + act := &fakeActions{restageResult: false} // NO stored secret + r := newRec(st, act) + seedCustomer(t, st, "c1", true) + + pushReport(t, st, "c1", StateNeedsCredential, 1) + r.ReconcileOnce(context.Background()) + pushReport(t, st, "c1", StateNeedsCredential, 2) + r.ReconcileOnce(context.Background()) + + if act.reissues() != 1 { + t.Fatalf("escalation did not happen exactly once: reissues=%d", act.reissues()) + } + if countEvents(t, st, "c1", eventReissued) != 1 { + t.Error("no offsite_selfheal_reissued audit event") + } + + // Repeated ticks on the SAME report must not mint again — this is the "a mint on every tick" + // failure the sibling's resetAfterHeal exists to prevent. + for i := 0; i < 5; i++ { + r.ReconcileOnce(context.Background()) + } + if act.reissues() != 1 { + t.Fatalf("minted repeatedly on an unchanged report: reissues=%d", act.reissues()) + } +} + +// SCENARIO F — a BLIP is absorbed: one declaring report followed by a healthy one triggers nothing. +// +// RED-PROOF: set debounceReports to 1 (i.e. remove the debounce) — the blip acts, and a credential is +// churned by what was only a restart. +func TestScenarioF_BlipIsAbsorbedByTheDebounce(t *testing.T) { + st := newHealStore(t) + act := &fakeActions{restageResult: true} + r := newRec(st, act) + seedCustomer(t, st, "c1", true) + + pushReport(t, st, "c1", StateNeedsCredential, 1) // the blip + r.ReconcileOnce(context.Background()) + pushReport(t, st, "c1", "", 2) // healthy again + r.ReconcileOnce(context.Background()) + + if act.restages() != 0 || act.reissues() != 0 { + t.Fatalf("a one-report blip triggered a credential action: restages=%d reissues=%d", act.restages(), act.reissues()) + } + + // And the streak must have been FORGOTTEN, not merely paused: a single later declaration must + // still not act. + pushReport(t, st, "c1", StateNeedsCredential, 3) + r.ReconcileOnce(context.Background()) + if act.restages() != 0 { + t.Fatal("the debounce streak survived a healthy report — a flapping box would be healed on every other cycle") + } +} + +// A DISABLED offsite descriptor is the operator's own choice and must never be re-credentialed, +// even if the box declares (e.g. an old declaration left in the newest report). +func TestDisabledDescriptorIsNeverHealed(t *testing.T) { + st := newHealStore(t) + act := &fakeActions{restageResult: true} + r := newRec(st, act) + seedCustomer(t, st, "c1", false) // offsite disabled in the config + + for i := 1; i <= 4; i++ { + pushReport(t, st, "c1", StateNeedsCredential, i) + r.ReconcileOnce(context.Background()) + } + if act.restages() != 0 || act.reissues() != 0 { + t.Fatalf("a customer whose offsite is DISABLED was re-credentialed: restages=%d reissues=%d", act.restages(), act.reissues()) + } +} + +// A blocked customer is never healed. +func TestBlockedCustomerIsNeverHealed(t *testing.T) { + st := newHealStore(t) + act := &fakeActions{restageResult: true} + r := newRec(st, act) + seedCustomer(t, st, "c1", true) + // SaveCustomerConfig always writes status 'active' (its INSERT does not carry the column), so the + // block must go through the real setter — asserted below, because a test that silently failed to + // block would pass for the wrong reason. + if err := st.SetCustomerConfigStatus("c1", "blocked"); err != nil { + t.Fatalf("SetCustomerConfigStatus: %v", err) + } + if !st.IsCustomerBlocked("c1") { + t.Fatal("precondition: the customer is not actually blocked — this test would pass vacuously") + } + for i := 1; i <= 4; i++ { + pushReport(t, st, "c1", StateNeedsCredential, i) + r.ReconcileOnce(context.Background()) + } + if act.restages() != 0 || act.reissues() != 0 { + t.Fatalf("a BLOCKED customer was healed: restages=%d reissues=%d", act.restages(), act.reissues()) + } +} + +// A restage ERROR must not escalate to a mint — the reconciler does nothing and retries. Acting on a +// failed read is the "never act on a partial view" rule. +func TestRestageErrorDoesNotEscalate(t *testing.T) { + st := newHealStore(t) + act := &fakeActions{restageResult: true, restageErr: fmt.Errorf("db is busy")} + r := newRec(st, act) + seedCustomer(t, st, "c1", true) + + pushReport(t, st, "c1", StateNeedsCredential, 1) + r.ReconcileOnce(context.Background()) + pushReport(t, st, "c1", StateNeedsCredential, 2) + r.ReconcileOnce(context.Background()) + + if act.reissues() != 0 { + t.Fatalf("a failed re-stage escalated to an external mint: reissues=%d", act.reissues()) + } + if countEvents(t, st, "c1", eventRestaged) != 0 { + t.Error("a failed re-stage recorded a success event") + } +} diff --git a/hub/internal/offsiteheal/wiring_test.go b/hub/internal/offsiteheal/wiring_test.go new file mode 100644 index 0000000..f6d0a85 --- /dev/null +++ b/hub/internal/offsiteheal/wiring_test.go @@ -0,0 +1,58 @@ +package offsiteheal + +import ( + "go/ast" + "go/parser" + "go/token" + "testing" +) + +// SCENARIO H — the reconciler is WIRED, asserted from main.go's source. +// +// Every test in this package passes on a reconciler that main.go never starts. That is this +// project's most-repeated failure shape: six features built and never wired, one of them an off-site +// restage event that existed and never fired once. The whole of R-204 item 4 is worth nothing if +// `Run` is not called. +// +// It walks the AST rather than grepping, because a commented-out call still contains the string, and +// it parses with comments DROPPED so a commented `go rec.Run(ctx)` cannot satisfy it. +// +// RED-PROOF: comment out the `go offsiteReconciler.Run(ctx)` line in cmd/hub/main.go → this fails. +func TestMainWiresTheOffsiteHealReconciler(t *testing.T) { + const mainPath = "../../cmd/hub/main.go" + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, mainPath, nil, 0) // comments dropped on purpose + if err != nil { + t.Fatalf("parse %s: %v — the reconciler's wiring is now unasserted", mainPath, err) + } + + var sawConstruct, sawRun bool + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + pkg, isIdent := sel.X.(*ast.Ident) + // offsiteheal.NewReconciler(...) + if isIdent && pkg.Name == "offsiteheal" && sel.Sel.Name == "NewReconciler" { + sawConstruct = true + } + // .Run(ctx) — the receiver is a local variable, so match on the method name and + // confirm the construction separately. Narrow enough: this file has one Run per reconciler. + if sel.Sel.Name == "Run" && isIdent && pkg.Name == "offsiteReconciler" { + sawRun = true + } + return true + }) + + if !sawConstruct { + t.Fatal("cmd/hub/main.go never calls offsiteheal.NewReconciler — R-204 item 4 ships inert") + } + if !sawRun { + t.Fatal("the offsite self-heal reconciler is CONSTRUCTED but never Run — a stranded box would declare forever and nothing would answer") + } +} diff --git a/hub/internal/store/offsite_restage_test.go b/hub/internal/store/offsite_restage_test.go new file mode 100644 index 0000000..12960b0 --- /dev/null +++ b/hub/internal/store/offsite_restage_test.go @@ -0,0 +1,177 @@ +package store + +import ( + "database/sql" + "errors" + "io" + "log" + "path/filepath" + "testing" +) + +func newRestageStore(t *testing.T) *Store { + t.Helper() + st, err := New(filepath.Join(t.TempDir(), "test.db"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatalf("store.New: %v", err) + } + t.Cleanup(func() { st.Close() }) + return st +} + +// R-204 item 4 / R-193 — THE FACT THE WHOLE DESIGN RESTS ON: the one-time offsite secret's VALUE +// survives consumption, so it can be re-armed without any storage-provider call. +// +// This is asserted rather than inherited from the PBS analogy on purpose. The two secrets are +// different objects with different lifecycles, and assuming a shared shape is exactly how two earlier +// sessions confused the two credentials. If a future change ever clears `value` on consume — a +// perfectly reasonable-looking hardening — restage-before-mint becomes silently impossible and every +// rebuild turns into an external mint. This test fails the moment that happens. +func TestRestageOneTimeSecret_ReArmsTheSameValue(t *testing.T) { + st := newRestageStore(t) + const cust, pw = "c1", "the-one-time-password" + + if err := st.SaveOneTimeSecret(cust, pw); err != nil { + t.Fatal(err) + } + got, err := st.ConsumeOneTimeSecret(cust) + if err != nil || got != pw { + t.Fatalf("first consume: got %q err=%v", got, err) + } + // Single-use holds before the restage. + if _, err := st.ConsumeOneTimeSecret(cust); !errors.Is(err, sql.ErrNoRows) { + t.Fatalf("a consumed secret was served twice: err=%v", err) + } + + restaged, err := st.RestageOneTimeSecret(cust) + if err != nil { + t.Fatal(err) + } + if !restaged { + t.Fatal("an existing secret row reported nothing to re-stage") + } + + // THE ASSERTION: the SAME value comes back. Not "a value" — the same one, which is what makes + // this a re-arm rather than a silent mint. + again, err := st.ConsumeOneTimeSecret(cust) + if err != nil { + t.Fatalf("consume after restage: %v", err) + } + if again != pw { + t.Fatalf("the re-armed secret changed value: got %q want %q — the value did NOT survive the consume, so restage-before-mint is impossible and the reconciler must mint instead", again, pw) + } + // And it is single-use again. + if _, err := st.ConsumeOneTimeSecret(cust); !errors.Is(err, sql.ErrNoRows) { + t.Fatalf("the re-armed secret was not single-use: err=%v", err) + } +} + +// No row → restaged=false, so the caller escalates to a fresh mint instead of silently doing nothing. +func TestRestageOneTimeSecret_NoRowReportsFalse(t *testing.T) { + st := newRestageStore(t) + restaged, err := st.RestageOneTimeSecret("nobody") + if err != nil { + t.Fatal(err) + } + if restaged { + t.Fatal("re-staging a customer with no stored secret claimed success — the caller would never escalate") + } +} + +// A restage must not touch the timestamps' meaning beyond the consumed flag: created_at stays put, so +// the delivery-state age math is unchanged. +func TestRestageOneTimeSecret_ClearsOnlyTheConsumedFlag(t *testing.T) { + st := newRestageStore(t) + const cust = "c1" + if err := st.SaveOneTimeSecret(cust, "pw"); err != nil { + t.Fatal(err) + } + before, err := st.GetOneTimeSecretInfo(cust) + if err != nil || before == nil { + t.Fatalf("info before: %v", err) + } + if _, err := st.ConsumeOneTimeSecret(cust); err != nil { + t.Fatal(err) + } + if _, err := st.RestageOneTimeSecret(cust); err != nil { + t.Fatal(err) + } + after, err := st.GetOneTimeSecretInfo(cust) + if err != nil || after == nil { + t.Fatalf("info after: %v", err) + } + if !after.ConsumedAt.IsZero() { + t.Errorf("consumed_at was not cleared: %v", after.ConsumedAt) + } + if !after.CreatedAt.Equal(before.CreatedAt) { + t.Errorf("created_at moved on a restage: %v → %v (the delivery-state age math would jump)", before.CreatedAt, after.CreatedAt) + } +} + +// reportHasOffsite is TIGHTENED in v0.96.0 to require enabled:true, because controller v0.199.0 sends +// an offsite object with enabled:false to ASK for a credential. This test pins BOTH halves: the +// tightening works, AND it is a no-op for every report shape that existed before — the equivalence is +// measured here rather than argued in a comment. +func TestReportHasOffsite_EnabledOnly(t *testing.T) { + cases := []struct { + name string + report string + want bool + }{ + // The PRE-v0.199.0 shapes. These are the equivalence claim: every object the old controller + // could ever attach carried enabled:true, so the tightening changes none of them. + {"configured box (pre-0.199 shape)", `{"offsite":{"enabled":true,"escrow_state":"escrowed","last_status":"ok"}}`, true}, + {"configured box, minimal", `{"offsite":{"enabled":true}}`, true}, + {"no offsite object", `{"controller_version":"0.198.0"}`, false}, + {"explicit null", `{"offsite":null}`, false}, + {"unparseable", `{not json`, false}, + + // The v0.199.0 DECLARATION — a request for help, not evidence of an applied tier. + {"stranded rebuild declaring", `{"offsite":{"enabled":false,"state":"needs_credential"}}`, false}, + {"disabled object without a state", `{"offsite":{"enabled":false}}`, false}, + } + for _, tc := range cases { + if got := reportHasOffsite(tc.report); got != tc.want { + t.Errorf("%s: reportHasOffsite = %v, want %v — a declaration read as an applied tier makes DeliveryStateFor say the stranded box is fine", tc.name, got, tc.want) + } + } +} + +// The declaration is readable as such, and an older controller's report yields "" rather than an error. +func TestLatestReportOffsiteDeclaration(t *testing.T) { + st := newRestageStore(t) + const cust = "c1" + + found, _, state, err := st.LatestReportOffsiteDeclaration(cust) + if err != nil { + t.Fatal(err) + } + if found { + t.Fatal("a customer with no reports was reported as found") + } + + if err := st.SaveReport(cust, []byte(`{"controller_version":"0.198.0"}`)); err != nil { + t.Fatal(err) + } + found, id1, state, err := st.LatestReportOffsiteDeclaration(cust) + if err != nil || !found { + t.Fatalf("found=%v err=%v", found, err) + } + if state != "" { + t.Errorf("an old controller's report declared %q", state) + } + + if err := st.SaveReport(cust, []byte(`{"offsite":{"enabled":false,"state":"needs_credential"}}`)); err != nil { + t.Fatal(err) + } + found, id2, state, err := st.LatestReportOffsiteDeclaration(cust) + if err != nil || !found { + t.Fatalf("found=%v err=%v", found, err) + } + if state != "needs_credential" { + t.Errorf("declared state = %q, want needs_credential", state) + } + if id2 <= id1 { + t.Errorf("the report id did not advance (%d → %d) — the debounce counts distinct reports and would never confirm", id1, id2) + } +} diff --git a/hub/internal/store/store.go b/hub/internal/store/store.go index fff7bed..a999453 100644 --- a/hub/internal/store/store.go +++ b/hub/internal/store/store.go @@ -978,11 +978,27 @@ func (s *Store) SaveReport(customerID string, reportJSON []byte) error { return err } -// reportOffsitePresence is the minimal parse for "does this controller report carry an offsite -// status object" — the R-70 delivery-state signal. The report builder attaches `offsite` only when -// the box actually has an offbox target configured, so presence == applied-on-the-box. +// reportOffsitePresence is the minimal parse for "does this controller report show an offsite tier +// that is actually CONFIGURED on the box" — the R-70 delivery-state signal, i.e. applied-on-the-box. +// +// ⚠ TIGHTENED 2026-08-05 (hub v0.96.0, R-204 item 4), and the reason is exactly the trap this +// project keeps recording. The predicate used to be bare PRESENCE, under a comment asserting *"the +// report builder attaches `offsite` only when the box actually has an offbox target configured, so +// presence == applied-on-the-box"*. Controller v0.199.0 breaks that premise deliberately: a REBUILT +// box now attaches an offsite object carrying `enabled:false` and a declared `state` in order to ASK +// for a credential. Left as presence-only, this function would have read that request for help as +// proof the tier was applied — turning `DeliveryStateFor` into DeliveryApplied for the exact boxes +// that are stranded, and suppressing the stuck event for them. +// +// The tightening is `enabled == true`, and it is PROVABLY A NO-OP for every report shape that exists +// today: `backup.OffboxReportStatus` returned nil unless `t != nil && t.Enabled`, so an attached +// object has ALWAYS carried `enabled:true`. Pinned by +// TestReportHasOffsite_EnabledOnly — including a case asserting the pre-v0.199.0 shape still reads +// true, so the equivalence is measured rather than argued. type reportOffsitePresence struct { - Offsite json.RawMessage `json:"offsite"` + Offsite *struct { + Enabled bool `json:"enabled"` + } `json:"offsite"` } func reportHasOffsite(reportJSON string) bool { @@ -990,7 +1006,7 @@ func reportHasOffsite(reportJSON string) bool { if err := json.Unmarshal([]byte(reportJSON), &p); err != nil { return false // unparseable report → no offsite evidence } - return len(p.Offsite) > 0 && string(p.Offsite) != "null" + return p.Offsite != nil && p.Offsite.Enabled } // LatestReportOffsitePresence reports whether the customer's most recent controller report exists @@ -1415,6 +1431,67 @@ func (s *Store) ConsumeOneTimeSecret(customerID string) (string, error) { return value, nil } +// RestageOneTimeSecret re-arms an ALREADY-STORED one-time offsite secret for re-consumption by +// clearing its consumed flag — WITHOUT changing the secret value, WITHOUT inserting a row, and +// WITHOUT any storage-provider call. It is the off-site sibling of RestageHostPBSSecret (pbsdr.go), +// and it exists because a REBUILT box has no credential of its own: its predecessor spent the +// one-time password (R-193 / R-204 item 4). +// +// IT IS POSSIBLE AT ALL ONLY BECAUSE THE VALUE SURVIVES A CONSUME, and that was established from +// this file rather than assumed from the PBS analogy (the two secrets are different objects with +// different lifecycles, and assuming a shared shape is how two sessions confused the credentials): +// ConsumeOneTimeSecret sets `consumed_at` and NOTHING ELSE — the `value` column is never cleared or +// overwritten, and the schema declares it `TEXT NOT NULL`. So the row still holds a serviceable +// password after consumption, and re-arming it costs no external call and converges in one report +// tick. Pinned by TestRestageOneTimeSecret_ReArmsTheSameValue. +// +// Returns restaged=true when a row existed (its consumed flag is now cleared, so +// ConsumeOneTimeSecret will serve it once more); restaged=false when NO secret is stored for the +// customer — the caller must then escalate to a fresh mint (ReissueCredentials). The value is never +// read or logged here. +func (s *Store) RestageOneTimeSecret(customerID string) (bool, error) { + res, err := s.db.Exec(`UPDATE one_time_secrets SET consumed_at = NULL WHERE customer_id = ?`, customerID) + if err != nil { + return false, err + } + n, err := res.RowsAffected() + if err != nil { + return false, err + } + return n > 0, nil +} + +// LatestReportOffsiteDeclaration returns the id of the customer's newest controller report and the +// state DECLARED on its `offsite` object (controller >= v0.199.0; "" on anything older, on a report +// with no offsite object, and on an unparseable one). +// +// The report id is the debounce currency: the reconciler counts DISTINCT reports, not its own ticks, +// so a stuck box is confirmed by fresh evidence rather than by the passage of time. +// +// found=false when the customer has no reports at all — distinguished from "a report that declares +// nothing" so the caller never treats an absent report as a healthy one. +func (s *Store) LatestReportOffsiteDeclaration(customerID string) (found bool, reportID int64, state string, err error) { + var id int64 + var reportJSON string + err = s.db.QueryRow(`SELECT id, report_json FROM reports WHERE customer_id = ? ORDER BY id DESC LIMIT 1`, + customerID).Scan(&id, &reportJSON) + if err == sql.ErrNoRows { + return false, 0, "", nil + } + if err != nil { + return false, 0, "", err + } + var p struct { + Offsite *struct { + State string `json:"state"` + } `json:"offsite"` + } + if json.Unmarshal([]byte(reportJSON), &p) == nil && p.Offsite != nil { + state = p.Offsite.State + } + return true, id, state, nil +} + // OneTimeSecretInfo is the delivery-state metadata of a customer's one-time offsite secret — // timestamps ONLY, the value column is deliberately never selected (R-70 detector input; the // customer-scoped sibling of PBSDRHealStates' SecretUnconsumedFor).