diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 22558be..12d27c2 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -665,7 +665,12 @@ func main() { } return ac.WipeStagedEscrowSecret(ctx) }, - Logger: logger, + // v0.199.0 (R-204 item 4 / R-193): remember whether the HUB holds a sealed recovery + // package for us — recorded on EVERY ACK, including when no off-site target exists, which + // is precisely the case that needs it. Half of the stranded-rebuild predicate in + // backup.needsOffsiteCredential; the other half (no repository password) is local. + RecordPresence: sett.SetHubEscrowIdentityPresent, + Logger: logger, } // Wire hub verification: update settings when hub reports customer status hubPusher.OnPushResponse = func(resp *report.PushResponse) { diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index 39114e9..3df5be8 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -1188,6 +1188,15 @@ func offboxAnchorAfterRun(prev, at string, runErr error) string { type OffboxReportStatus struct { Enabled bool `json:"enabled"` EscrowState string `json:"escrow_state"` + // State (v0.199.0, R-204 item 4 / R-193) is a DECLARED condition — the box naming its own + // situation rather than the hub deducing it from a silence. Empty on every configured box, so a + // healthy report's JSON is byte-identical to v0.198.0's. + // + // WHY A DECLARATION AND NOT AN INFERENCE (the operator ruling, 2026-08-05). 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. So it says so. + State string `json:"state,omitempty"` LastRun string `json:"last_run,omitempty"` // RFC3339 LastStatus string `json:"last_status,omitempty"` // "ok" | "incomplete" (R-203) | "error" | "running" // LastSuccess (R-100) is the last run that SUCCEEDED — the hub's staleness anchor. Absent on a @@ -1199,11 +1208,63 @@ type OffboxReportStatus struct { QuotaGB int `json:"quota_gb"` } -// OffboxReportStatus returns the offsite summary for the hub report (nil = not configured; the hub's -// checker treats absence as "nothing to watch" — pre-v0.109 reports look the same). +// OffsiteStateNeedsCredential is the ONE declared state (v0.199.0, R-204 item 4 / R-193): this box +// has no off-site tier, holds no repository password, and the hub says it is keeping a sealed +// recovery package for it — i.e. it is a REBUILT box whose predecessor spent the one-time provider +// password, and it cannot configure its off-site tier without a credential it has no way to obtain. +// That was the last of the four manual interventions the 2026-08-04 drill needed. +const OffsiteStateNeedsCredential = "needs_credential" + +// needsOffsiteCredential is the stranded-rebuild predicate. BOTH facts are required and neither is +// sufficient on its own — this is the whole correctness of the feature: +// +// - the data area is FRESH (no repository password on disk). Alone, this is simply a box that never +// had off-site backups, and declaring on it would make every un-configured box in the fleet ask +// for a credential. +// - the HUB holds a sealed recovery package (the ACK's identity_blob_present, cached in settings). +// Alone, this is a healthy box that has run its ceremony. +// +// Only together do they mean "this box HAD an off-site tier, and no longer has what it needs to use +// it". A target that exists but is DISABLED is not stranded either — the customer switched it off — +// so the caller only consults this when there is no enabled target, and a non-nil disabled target +// short-circuits to false here. +func (m *Manager) needsOffsiteCredential(t *settings.OffboxTarget) bool { + if t != nil { + return false // a target exists (merely disabled) — the customer's own choice, not a rebuild + } + if m.settings == nil || !m.settings.GetHubEscrowIdentityPresent() { + return false // the hub holds nothing for us: never had off-site backups + } + if _, ok := m.OffboxRepoPasswordHash(); ok { + return false // we still hold our repository password: not a fresh data area + } + return true +} + +// OffboxReportStatus returns the offsite summary for the hub report. +// +// nil = nothing to say (not configured, and nothing to ask for) — the hub's checker treats absence as +// "nothing to watch"; pre-v0.109 reports look the same. +// +// v0.199.0: there is now ONE case where an UNCONFIGURED box still reports an object — the stranded +// rebuild, which DECLARES OffsiteStateNeedsCredential rather than leaving the hub to deduce it from a +// silence. An absent object has FOUR meanings (never configured / mid-restart / a transient config +// read failure / rebuilt-and-stranded) and the hub cannot tell them apart; the box can. +// +// WHY THE DECLARATION IS INERT TO EVERY EXISTING READER, established from their code rather than +// assumed: it carries Enabled=false and zero quota/size, and the hub's OffsiteChecker gates +// `isStale` on `!off.Enabled` (returns false) and `fillBand` on a zero quota/size (returns OK). So it +// raises no staleness and no fill alarm on a new hub OR an old one, and an unknown `state` string is +// ignored by encoding/json. The ONE reader that would have misread it is the store's +// `reportHasOffsite` ("presence == applied-on-the-box"), which hub v0.96.0 tightens to require +// enabled=true — provably a no-op for every report shape that exists today, because this function has +// never emitted a disabled object before. func (m *Manager) OffboxReportStatus() *OffboxReportStatus { t := m.settings.GetOffboxTarget() if t == nil || !t.Enabled { + if m.needsOffsiteCredential(t) { + return &OffboxReportStatus{Enabled: false, State: OffsiteStateNeedsCredential} + } return nil } return &OffboxReportStatus{ diff --git a/controller/internal/backup/offbox_declare_test.go b/controller/internal/backup/offbox_declare_test.go new file mode 100644 index 0000000..4685812 --- /dev/null +++ b/controller/internal/backup/offbox_declare_test.go @@ -0,0 +1,156 @@ +package backup + +import ( + "encoding/json" + "io" + "log" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-204 item 4 / R-193 — a REBUILT box declares that it needs an off-site credential, instead of +// reporting an absence the hub cannot interpret. +// +// THE POINT OF THESE TESTS is the conjunction. An absent off-site object has FOUR meanings (never +// configured / mid-restart / a transient read failure / rebuilt-and-stranded). The declaration has +// one, and it is only sound because BOTH halves are required: a fresh data area AND a hub-held +// recovery package. Scenario B is the one that matters most — drop the escrow half and every +// un-configured box in the fleet starts asking for a credential. + +// bareManager builds a Manager with NO off-site target and NO repository password — the shape of a +// freshly rebuilt box before anything is configured. +func bareManager(t *testing.T) (*Manager, *settings.Settings) { + t.Helper() + lg := log.New(io.Discard, "", 0) + dataDir := t.TempDir() + sett, err := settings.Load(filepath.Join(dataDir, "settings.json"), lg) + if err != nil { + t.Fatal(err) + } + cfg := &config.Config{} + cfg.Paths.DataDir = dataDir + cfg.Paths.SystemDataPath = filepath.Join(dataDir, "sys") + return NewManager(cfg, sett, lg), sett +} + +// SCENARIO A — a rebuilt box (fresh data area + a hub-held escrow) DECLARES the state. +// +// RED-PROOF: remove the `GetHubEscrowIdentityPresent()` condition from needsOffsiteCredential — +// Scenario A still passes (it has an escrow), and SCENARIO B FAILS, which is the point: the plausible +// wrong fix is to declare on freshness alone, and that would make every un-configured box in the +// fleet ask for a credential. +func TestOffsiteDeclare_RebuiltBoxDeclaresNeedsCredential(t *testing.T) { + m, sett := bareManager(t) + if err := sett.SetHubEscrowIdentityPresent(true); err != nil { + t.Fatal(err) + } + + st := m.OffboxReportStatus() + if st == nil { + t.Fatal("a rebuilt box reported NO off-site object — the hub cannot distinguish it from a box that never had off-site backups (this is the defect)") + } + if st.State != OffsiteStateNeedsCredential { + t.Fatalf("declared state = %q, want %q", st.State, OffsiteStateNeedsCredential) + } + // Enabled MUST be false and the sizes zero — that is what makes the declaration inert to the + // hub's existing fill and staleness checkers (and to a pre-upgrade hub). + if st.Enabled { + t.Error("a declaration must not claim the tier is enabled — the hub's staleness check keys on it") + } + if st.QuotaGB != 0 || st.RepoSizeBytes != 0 || st.SnapshotCount != 0 { + t.Errorf("a declaration must carry zero sizes (fill band keys on them): %+v", st) + } + // And it must be on the off-site object, not a new top-level field. + b, err := json.Marshal(st) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `"state":"needs_credential"`) { + t.Fatalf("declared state absent from the marshalled off-site object: %s", b) + } + if !strings.Contains(string(b), `"enabled":false`) { + t.Fatalf("marshalled object must carry enabled:false: %s", b) + } +} + +// SCENARIO B — a box that never had off-site backups says NOTHING. This is the guard on the +// conjunction; without it the feature churns credentials fleet-wide. +func TestOffsiteDeclare_NeverHadOffsiteSaysNothing(t *testing.T) { + m, _ := bareManager(t) // fresh data area, but NO hub-held escrow + + if st := m.OffboxReportStatus(); st != nil { + t.Fatalf("a box that never had off-site backups DECLARED a need: %+v — every un-configured box in the fleet would now ask for a credential", st) + } +} + +// The other half of the conjunction: a box that still holds its repository password is NOT stranded, +// even though the hub holds an escrow for it. That is simply a healthy box between configurations. +func TestOffsiteDeclare_BoxThatStillHoldsItsRepoPasswordDoesNotDeclare(t *testing.T) { + m, sett := bareManager(t) + if err := sett.SetHubEscrowIdentityPresent(true); err != nil { + t.Fatal(err) + } + // Place a repository password exactly where the manager looks for it. + if err := os.MkdirAll(m.offboxDir(), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(m.offboxPwPath(), []byte("a-repository-password"), 0o600); err != nil { + t.Fatal(err) + } + + if st := m.OffboxReportStatus(); st != nil { + t.Fatalf("a box holding its repository password declared a need: %+v", st) + } +} + +// A DISABLED target is the customer's own choice, not a rebuild — it must not declare either. +func TestOffsiteDeclare_DisabledTargetIsNotStranded(t *testing.T) { + m, sett := bareManager(t) + if err := sett.SetHubEscrowIdentityPresent(true); err != nil { + t.Fatal(err) + } + if err := sett.SetOffboxTarget(&settings.OffboxTarget{ + Enabled: false, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo", + }); err != nil { + t.Fatal(err) + } + + if st := m.OffboxReportStatus(); st != nil { + t.Fatalf("a deliberately DISABLED target declared a need: %+v", st) + } +} + +// A CONFIGURED box's report object must be byte-identical to v0.198.0's — no `state` key at all. +// This is what lets a pre-upgrade hub and every existing checker read the fleet unchanged. +func TestOffsiteDeclare_ConfiguredBoxJSONIsUnchanged(t *testing.T) { + m, sett := bareManager(t) + if err := sett.SetOffboxTarget(&settings.OffboxTarget{ + Enabled: true, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo", + Schedule: "daily", EscrowState: "escrowed", LastStatus: "ok", + }); err != nil { + t.Fatal(err) + } + + st := m.OffboxReportStatus() + if st == nil { + t.Fatal("a configured box must still report an off-site object") + } + if st.State != "" { + t.Errorf("a configured box must declare NO state, got %q", st.State) + } + b, err := json.Marshal(st) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), `"state"`) { + t.Fatalf("a healthy report's JSON gained a `state` key — it must stay byte-compatible: %s", b) + } + if !strings.Contains(string(b), `"enabled":true`) { + t.Fatalf("a configured box must report enabled:true: %s", b) + } +} diff --git a/controller/internal/report/escrow_confirm.go b/controller/internal/report/escrow_confirm.go index 6995859..d715662 100644 --- a/controller/internal/report/escrow_confirm.go +++ b/controller/internal/report/escrow_confirm.go @@ -39,8 +39,22 @@ type EscrowAutoConfirmer struct { // Flip transitions EscrowState pending→escrowed (settings.UpdateOffboxStatus). Flip func() error // Wipe removes the agent-staged secret (best-effort — the flip is the primary effect). - Wipe func(ctx context.Context) error - Logger *log.Logger + Wipe func(ctx context.Context) error + // RecordPresence persists the ACK's `identity_blob_present` — whether the HUB holds a sealed + // recovery package for this box (v0.199.0, R-204 item 4 / R-193). + // + // WHY IT LIVES HERE, in the auto-confirmer, rather than in its own ACK consumer: this is already + // the ONE place the ACK's escrow object arrives, and it is already wired. A second Reconcile call + // in main.go would be a second wiring point, and this project's count of features built but never + // wired is six. Pinned by TestEscrowConfirm_RecordsPresenceEvenWhenOffboxUnconfigured and by the + // wiring test. + // + // It is called FIRST, before every gate below, and that ordering is the whole fix: on a box with + // no off-site target `Pending()` and `Escrowed()` are both false and Reconcile returned + // immediately, so the one fact that distinguishes a REBUILT box from a box that never had + // off-site backups was thrown away on every cycle. nil → not recorded (older wiring, tests). + RecordPresence func(present bool) error + Logger *log.Logger mu sync.Mutex warnedHash string // last mismatched hub hash we warned about (dedupe; shared by both branches) @@ -76,6 +90,15 @@ func (c *EscrowAutoConfirmer) Reconcile(es *EscrowStatus) { if es == nil { return } + // FIRST, unconditionally — see RecordPresence. Every gate below is allowed to skip the + // auto-confirm; none of them may skip this, because an unconfigured box is exactly the case that + // needs the fact. A record failure is logged and does NOT stop the auto-confirm: the two are + // independent, and swallowing it silently is the shape this project keeps removing. + if c.RecordPresence != nil { + if err := c.RecordPresence(es.IdentityBlobPresent); err != nil { + c.logf("[WARN] [escrow-confirm] could not record the hub's identity-blob presence (present=%v): %v", es.IdentityBlobPresent, err) + } + } if !c.Pending() { // Scenario F (v0.127.0): an ESCROWED box re-checks the hash on every ACK — a superseding // blob that does not cover the current password must be surfaced (warn + card flag), while diff --git a/controller/internal/report/escrow_presence_test.go b/controller/internal/report/escrow_presence_test.go new file mode 100644 index 0000000..974929c --- /dev/null +++ b/controller/internal/report/escrow_presence_test.go @@ -0,0 +1,81 @@ +package report + +import ( + "io" + "log" + "testing" +) + +// R-204 item 4 / R-193 — the ACK's `identity_blob_present` must be recorded on EVERY ACK, including +// (especially) on a box with no off-site target. +// +// THE DEFECT THIS PINS: Reconcile returns early when the box is neither pending nor escrowed, which +// is exactly a REBUILT box's state — so the one fact that distinguishes it from a box that never had +// off-site backups was discarded on every cycle. The recorder therefore runs BEFORE every gate, and +// the test drives Reconcile itself rather than calling the recorder, because the ordering IS the fix. + +func TestEscrowConfirm_RecordsPresenceEvenWhenOffboxUnconfigured(t *testing.T) { + var recorded []bool + c := &EscrowAutoConfirmer{ + // The unconfigured-box shape: neither pending nor escrowed. Every gate below will skip. + Pending: func() bool { return false }, + Escrowed: func() bool { return false }, + LocalHash: func() (string, bool) { return "", false }, + Flip: func() error { t.Fatal("an unconfigured box must never flip"); return nil }, + RecordPresence: func(p bool) error { recorded = append(recorded, p); return nil }, + Logger: log.New(io.Discard, "", 0), + } + + c.Reconcile(&EscrowStatus{IdentityBlobPresent: true, ResticPwSHA256: "SHA1"}) + if len(recorded) != 1 || !recorded[0] { + t.Fatalf("presence not recorded on an unconfigured box: %v — a rebuilt box cannot learn the hub holds its recovery package", recorded) + } + + // It must also record the NEGATIVE, so a customer RESET (the hub losing its escrow row) turns the + // box's declaration back off. A set-only flag would strand the declaration forever. + c.Reconcile(&EscrowStatus{IdentityBlobPresent: false}) + if len(recorded) != 2 || recorded[1] { + t.Fatalf("a false presence was not recorded: %v", recorded) + } +} + +// A nil ACK escrow object records nothing (an old hub, or no escrow row) — absence of a statement is +// not a statement of absence, and overwriting a known-true with false here would un-declare a genuinely +// stranded box every time an old hub answered. +func TestEscrowConfirm_NilAckRecordsNothing(t *testing.T) { + called := false + c := &EscrowAutoConfirmer{ + Pending: func() bool { return false }, + Escrowed: func() bool { return false }, + RecordPresence: func(bool) error { called = true; return nil }, + Logger: log.New(io.Discard, "", 0), + } + c.Reconcile(nil) + if called { + t.Fatal("a nil ACK escrow object must not record a presence") + } +} + +// A recorder FAILURE must be logged, not swallowed, and must not stop the auto-confirm — the two are +// independent concerns and a failed settings write must not also break escrow confirmation. +func TestEscrowConfirm_RecordFailureDoesNotBlockAutoConfirm(t *testing.T) { + flipped := false + c := &EscrowAutoConfirmer{ + Pending: func() bool { return true }, + Escrowed: func() bool { return false }, + LocalHash: func() (string, bool) { return "MATCH", true }, + Flip: func() error { flipped = true; return nil }, + RecordPresence: func(bool) error { return errRecord }, + Logger: log.New(io.Discard, "", 0), + } + c.Reconcile(&EscrowStatus{IdentityBlobPresent: true, ResticPwSHA256: "MATCH"}) + if !flipped { + t.Fatal("a presence-record failure blocked the escrow auto-confirm — they are independent") + } +} + +type recordErr struct{} + +func (recordErr) Error() string { return "record failed" } + +var errRecord = recordErr{} diff --git a/controller/internal/report/escrow_presence_wiring_test.go b/controller/internal/report/escrow_presence_wiring_test.go new file mode 100644 index 0000000..4631d9c --- /dev/null +++ b/controller/internal/report/escrow_presence_wiring_test.go @@ -0,0 +1,68 @@ +package report + +import ( + "go/ast" + "go/parser" + "go/token" + "testing" +) + +// TestMainWiresRecordPresence — the seam-discipline test (§9 rule 6). +// +// `RecordPresence` is a nil-able field: an unwired confirmer compiles, every test in this package +// passes, the fleet reports nothing new, and the whole of R-204 item 4 is inert. 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. +// +// It walks the AST of main.go rather than grepping the file, because a commented-out field still +// contains the string (the lesson from the lifecycle-gate wiring test next door), and it parses with +// comments DROPPED so a commented assignment cannot satisfy it. +func TestMainWiresRecordPresence(t *testing.T) { + const mainPath = "../../cmd/controller/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 wiring of RecordPresence is now unasserted", mainPath, err) + } + + found := false + sawConfirmerLiteral := false + ast.Inspect(f, func(n ast.Node) bool { + lit, ok := n.(*ast.CompositeLit) + if !ok { + return true + } + // Match `report.EscrowAutoConfirmer{...}` (and a bare `EscrowAutoConfirmer{...}`). + name := "" + switch t := lit.Type.(type) { + case *ast.SelectorExpr: + name = t.Sel.Name + case *ast.Ident: + name = t.Name + } + if name != "EscrowAutoConfirmer" { + return true + } + sawConfirmerLiteral = true + for _, el := range lit.Elts { + kv, ok := el.(*ast.KeyValueExpr) + if !ok { + continue + } + if k, ok := kv.Key.(*ast.Ident); ok && k.Name == "RecordPresence" { + found = true + } + } + return true + }) + + // Distinguish "the literal moved" from "the field was dropped" — otherwise a refactor that + // relocated the confirmer would read as a passing test over nothing (the §12 rule: an absent + // thing is not evidence). + if !sawConfirmerLiteral { + t.Fatalf("no EscrowAutoConfirmer composite literal found in %s — did the wiring move? This test can no longer see it", mainPath) + } + if !found { + t.Fatal("EscrowAutoConfirmer is constructed WITHOUT RecordPresence — the box will never learn the hub holds its recovery package, and R-204 item 4 ships inert") + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index 17a1f5d..f65a780 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -55,6 +55,21 @@ type Settings struct { // re-enabled it on every read — this replaces it). OffboxEnlargeNoticeSeeded bool `json:"offbox_enlarge_notice_seeded,omitempty"` + // HubEscrowIdentityPresent (v0.199.0, R-204 item 4 / R-193) caches the report ACK's + // `escrow.identity_blob_present` — whether the HUB is holding a sealed recovery package for this + // box. It is recorded on EVERY ACK that carries an escrow object, including when no off-site + // target is configured, which is the whole point: until now that field was read only by the + // auto-confirm, which returns early on an unconfigured box, so a REBUILT box threw away the one + // fact that distinguishes it from a box that never had off-site backups. + // + // It is half of the "I am rebuilt and stranded" predicate (see backup.OffboxReportStatus). The + // other half is local: no repository password on disk. **Freshness alone is a box that never had + // off-site backups; an escrow alone is a healthy box. Only both together mean rebuilt.** + // + // Cached, not derived: a rebuilt box has an empty settings.json, so this is re-learned from its + // first ACK — which is correct, because the hub is the authority on what the hub holds. + HubEscrowIdentityPresent bool `json:"hub_escrow_identity_present,omitempty"` + // Cached state DBValidations map[string]DBValidationCache `json:"db_validations,omitempty"` @@ -601,6 +616,30 @@ func (s *Settings) SetLauncherSharePasswordHash(hash string) error { return s.save() } +// ── Hub-held recovery package (v0.199.0, R-204 item 4) ───────────────────────── + +// GetHubEscrowIdentityPresent reports whether the hub is holding a sealed identity/recovery package +// for this box, as last stated by a report ACK. False when no ACK has carried an escrow object yet. +func (s *Settings) GetHubEscrowIdentityPresent() bool { + s.mu.RLock() + defer s.mu.RUnlock() + return s.HubEscrowIdentityPresent +} + +// SetHubEscrowIdentityPresent records the ACK's `escrow.identity_blob_present`. It is a plain +// last-write-wins mirror of the hub's statement — NOT set-only, because the hub losing an escrow row +// (a customer RESET) must be able to turn the box's declaration back off. Saves only on a change, so +// the ordinary 15-minute ACK does not rewrite settings.json every cycle. +func (s *Settings) SetHubEscrowIdentityPresent(present bool) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.HubEscrowIdentityPresent == present { + return nil + } + s.HubEscrowIdentityPresent = present + return s.save() +} + // ── Customer-claim arc (v0.122.0) ────────────────────────────────────────────── // GetClaimed reports whether this box has completed a claim (set-only).