diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index 1187d80..a34abaa 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -381,10 +381,46 @@ func (m *Manager) offboxKeyPath() string { return filepath.Join(m.offboxDir() func (m *Manager) offboxPwPath() string { return filepath.Join(m.offboxDir(), "repo_password") } func (m *Manager) offboxKnownHosts() string { return filepath.Join(m.offboxDir(), "known_hosts") } +// ErrOffboxSealedPackageHeld is the R-241 mint refusal: this box has no repository password and the +// hub is holding a sealed recovery package for it, so minting one would write a key the package does +// not cover — orphaning the very history the customer's recovery code protects. +// +// It is a SENTINEL, not a failure. `ApplyOffsiteTarget` catches it and still configures the transport +// (SSH key, known_hosts, host/user/path), because the transport is not the problem and having it is +// what lets the recovery screen bring the tier up the moment the key arrives (R-219). What it does +// NOT do is let the tier come up under a key nobody escrowed. +var ErrOffboxSealedPackageHeld = fmt.Errorf("offbox: the hub holds a sealed recovery package for this box — not minting a repository password over it") + +// ErrOffboxSealedPackageHeld reports whether err is the mint refusal (errors.Is-friendly for callers +// that wrap it). +func IsOffboxSealedPackageHeld(err error) bool { return errors.Is(err, ErrOffboxSealedPackageHeld) } + // WriteOffboxSecrets persists the SSH private key + (auto-generated if empty) repo password + the pinned // known-host line as 0600/0644 files in the data dir. The key is provided out-of-band by the operator -// (UI), never logged. Returns the repo password so the caller need not read the file. Idempotent: an empty -// sshKey/knownHosts leaves the existing file untouched (a re-save of just the target shouldn't wipe keys). +// (UI), never logged. Idempotent: an empty sshKey/knownHosts leaves the existing file untouched (a +// re-save of just the target shouldn't wipe keys). +// +// ⚠ R-241 (v0.206.0) — IT NO LONGER MINTS OVER A SEALED PACKAGE, AND THAT IS THE WHOLE FIX. +// +// Until now the auto-generate branch consulted **one** input: does the file exist. Not the settings, +// not the hub's ACK — nothing about whether anything already depended on a different key. Its two +// neighbours in this very file, `OffsiteRecoveryOffer` (:1412) and `needsOffsiteCredential` (:1377), +// BOTH consult `GetHubEscrowIdentityPresent()`. The same fact was available on three paths and used +// on two. +// +// WHAT THAT COST, measured on the final walk (2026-08-06/07, SPIKE-r241-recovery-offer-2026-08-07): +// a rebuilt box's credential self-heal reached here at 03:18:06Z and minted `9b4a9a9d…` over a hub +// package sealing `30ef574f…`. The recovery screen then looked, found a key present and no orphan +// recorded, and correctly said there was nothing to recover. **The screen was telling the truth; the +// lie happened thirty minutes earlier, here.** And the flag was not merely available at that moment — +// it was the PRECONDITION of the chain that reached this function: the credential retry only runs +// while `needsOffsiteCredential` is true, which requires this exact flag, and the venue logged it at +// 02:48:03Z, six ticks before the mint. +// +// THE GUARD IS DELIBERATELY NARROW — see the Scenario B test. It fires ONLY when a package is held AND +// no password exists. A box the hub holds nothing for mints exactly as before, which is every +// first-time box in the fleet; widening this to "never mint" would leave a new customer unable to +// start, waiting for a package that will never exist. func (m *Manager) WriteOffboxSecrets(sshKey, knownHosts string) error { if err := os.MkdirAll(m.offboxDir(), 0o700); err != nil { return fmt.Errorf("offbox dir: %w", err) @@ -407,8 +443,15 @@ func (m *Manager) WriteOffboxSecrets(sshKey, knownHosts string) error { return fmt.Errorf("offbox known_hosts: %w", err) } } - // Auto-generate the repo password once (0600), never log it. + // Auto-generate the repo password once (0600), never log it — UNLESS the hub is holding a sealed + // package for us (R-241). The transport files above are already written and that is deliberate. if _, err := os.Stat(m.offboxPwPath()); os.IsNotExist(err) { + if m.sealedPackageHeld() { + m.logger.Printf("[WARN] [offbox] NOT minting a repository password: the hub holds a sealed recovery package for this box, " + + "and a fresh key would orphan the history that package protects (R-241). The transport is configured; " + + "the tier stays down until the customer's recovery code places the escrowed key.") + return ErrOffboxSealedPackageHeld + } pw, gerr := generateOffboxPassword() if gerr != nil { return gerr @@ -420,6 +463,37 @@ func (m *Manager) WriteOffboxSecrets(sshKey, knownHosts string) error { return nil } +// sealedPackageHeld reports the ACK-cached fact that the hub is holding a sealed recovery package for +// this box. It is the SAME call `OffsiteRecoveryOffer` and `needsOffsiteCredential` already make — +// deliberately, so the three paths can never disagree about it. A nil settings store reads as "no +// package": the mint guard must never block a box whose settings could not be read, because that +// would turn a transient read failure into a tier that never comes up. +func (m *Manager) sealedPackageHeld() bool { + return m.settings != nil && m.settings.GetHubEscrowIdentityPresent() +} + +// OffboxAwaitingRecoveryKey reports the R-241 holding state: a transport target exists, but no +// repository password does, because the hub holds a sealed package and the mint was refused. +// +// DERIVED, NOT STORED, and that is the §2.1 ruling applied to this field too: a stored flag would be a +// second copy of a fact the three inputs already carry, and a second copy is a thing that can drift. +// The moment a recovery places the escrowed key, this goes false on its own with nothing to clear. +// +// ⚠ `t.Enabled` IS LOAD-BEARING, and it was missing in the first draft — caught by the existing +// TestOffsiteDeclare_DisabledTargetIsNotStranded rather than by review. A customer who switched +// off-site OFF is not awaiting anything, and a box that declares a holding state for a tier nobody +// asked for is the R-215 shape (a screen about data the customer did not ask to protect). This is the +// SAME Scenario-E carve-out `needsOffsiteCredential` makes two functions above; the two must agree, +// and now do. +func (m *Manager) OffboxAwaitingRecoveryKey() bool { + t := m.settings.GetOffboxTarget() + if t == nil || !t.Enabled || !m.sealedPackageHeld() { + return false + } + _, hasPw := m.OffboxRepoPasswordHash() + return !hasPw +} + // generateOffboxPassword returns a 256-bit hex repo password. func generateOffboxPassword() (string, error) { b := make([]byte, 32) @@ -489,8 +563,19 @@ var offboxRepoPwPattern = regexp.MustCompile(`^[0-9a-fA-F]{64}$`) // the repo password to the agent for escrow — the SAME fork-4 enable path a manual config takes. `stage` is // the agent escrow-stage push (nil skips it, e.g. when the agent is unreachable — the run gate still holds). func (m *Manager) ApplyOffsiteTarget(ctx context.Context, tgt *settings.OffboxTarget, sshKeyPEM, knownHosts string, stage func(ctx context.Context, pw string) error) error { + // R-241: the mint refusal is a HOLDING state, not a failure. The transport files were written + // before the refusal, so we still record the target — `OffboxConfigured()` stays false because the + // password file is absent, which is what keeps runs gated, and the recovery screen can bring the + // tier up the instant the escrowed key is placed (R-219's synchronous tier-up). + // + // Returning the error here instead would leave `needsOffsiteCredential` true forever, so the hub + // would re-stage a credential the box had already consumed, on every cycle, for ever. + awaitingKey := false if err := m.WriteOffboxSecrets(sshKeyPEM, knownHosts); err != nil { - return fmt.Errorf("apply offsite secrets: %w", err) + if !IsOffboxSealedPackageHeld(err) { + return fmt.Errorf("apply offsite secrets: %w", err) + } + awaitingKey = true } // Re-apply (v0.109.1 live finding): the bridge rebuilds the target from the descriptor, but the // EXISTING target's custody + runtime status must carry over — EscrowState tracks the REPO PASSWORD @@ -514,6 +599,14 @@ func (m *Manager) ApplyOffsiteTarget(ctx context.Context, tgt *settings.OffboxTa if err := m.settings.SetOffboxTarget(tgt); err != nil { return fmt.Errorf("apply offsite target: %w", err) } + // R-241: nothing to stage — there is no repository password, by design. Say so once, plainly, and + // return without touching the escrow. `PushOffboxPasswordForEscrow` would fail on the absent file + // anyway; naming the situation beats a misleading "agent unreachable?" warning. + if awaitingKey { + m.logger.Printf("[INFO] [offbox] apply-offsite: transport configured for %s@%s:%s, tier HELD awaiting the escrowed key "+ + "(the hub holds a sealed package; no key was minted — R-241)", tgt.User, tgt.Host, tgt.RepoPath) + return nil + } if stage != nil { // Best-effort: the offbox is configured + pending regardless. A stage-push failure (agent momentarily // unreachable) is logged, not fatal — the escrow can be (re-)staged later (operator ceremony / re-enable). @@ -1331,6 +1424,27 @@ type OffboxReportStatus struct { // That was the last of the four manual interventions the 2026-08-04 drill needed. const OffsiteStateNeedsCredential = "needs_credential" +// OffsiteStateAwaitingRecoveryKey (v0.206.0, R-241) is the declared HOLDING state: the transport is +// configured, but no repository password exists because the hub holds a sealed package and minting +// one would orphan the history it protects. The box is not stranded (it has its credential) and not +// healthy (it cannot run) — it is waiting for a person with a recovery code. +// +// WHY IT IS INERT TO EVERY EXISTING HUB READER, established from their code rather than assumed — +// the same discipline `OffsiteStateNeedsCredential`'s own note applies: +// +// - `offsiteheal` acts on EXACTLY ONE string, `needs_credential` ("Everything else … is a no-op"), +// so it will not re-stage a credential this box already has; +// - `monitor.OffsiteChecker.isStale` returns false unless `Enabled && EscrowState == "escrowed"`, +// and this object carries Enabled=false; +// - `monitor/offsite_delivery.go` keys on the delivery shape, which is `applied` here (the secret +// WAS consumed), and that branch is skipped; +// - an unknown `state` string is ignored by encoding/json on an older hub. +// +// So this needs NO hub change to be safe. It does mean a held box raises no alarm — which is R-243, +// filed and deliberately not widened here; the difference from R-241 is that this state is now +// VISIBLE to the customer instead of silent. +const OffsiteStateAwaitingRecoveryKey = "awaiting_recovery_key" + // 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: // @@ -1438,6 +1552,13 @@ func (m *Manager) OffsiteRecoveryOffer() bool { // never emitted a disabled object before. func (m *Manager) OffboxReportStatus() *OffboxReportStatus { t := m.settings.GetOffboxTarget() + // R-241: the HOLDING state is declared before the enabled/disabled split, because a held target IS + // enabled — the customer wants off-site backups; what is missing is the key. Reported with + // Enabled=false so every existing hub reader treats it exactly as the stranded declaration (see + // OffsiteStateAwaitingRecoveryKey), while the string names the difference for anything that looks. + if m.OffboxAwaitingRecoveryKey() { + return &OffboxReportStatus{Enabled: false, State: OffsiteStateAwaitingRecoveryKey, EscrowState: t.EscrowState} + } if t == nil || !t.Enabled { if m.needsOffsiteCredential(t) { return &OffboxReportStatus{Enabled: false, State: OffsiteStateNeedsCredential} diff --git a/controller/internal/backup/offbox_mintguard_r241_test.go b/controller/internal/backup/offbox_mintguard_r241_test.go new file mode 100644 index 0000000..22bfacf --- /dev/null +++ b/controller/internal/backup/offbox_mintguard_r241_test.go @@ -0,0 +1,192 @@ +package backup + +import ( + "context" + "log" + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-241 — THE MINT GUARD. This file is the session's headline test. +// +// The defect, measured on the final walk (SPIKE-r241-recovery-offer-2026-08-07): a rebuilt box's +// credential self-heal reached WriteOffboxSecrets at 03:18:06Z and minted a fresh repository password +// over a hub package sealing a DIFFERENT key. The recovery screen then correctly reported that there +// was nothing recoverable under the key the box held. The screen was honest; the minting was not. +// +// Scenario A asserts the key is NOT written. Scenario B asserts the guard is narrow enough that a +// first-time box still starts — the guard's own failure mode, and the one an over-broad fix produces. + +// mintGuardManager builds a Manager with NO offbox secrets written, so the mint branch is live. +// hubHoldsPackage sets the ACK-cached fact the guard consults. +func mintGuardManager(t *testing.T, hubHoldsPackage bool) (*Manager, *settings.Settings, string) { + t.Helper() + logger := log.New(os.Stderr, "", 0) + dataDir := t.TempDir() + sett, err := settings.Load(filepath.Join(dataDir, "settings.json"), logger) + if err != nil { + t.Fatal(err) + } + cfg := &config.Config{} + cfg.Paths.DataDir = dataDir + cfg.Paths.SystemDataPath = filepath.Join(dataDir, "sys") + m := NewManager(cfg, sett, logger) + if err := sett.SetHubEscrowIdentityPresent(hubHoldsPackage); err != nil { + t.Fatal(err) + } + return m, sett, filepath.Join(dataDir, "offbox", "repo_password") +} + +// ── SCENARIO A — the box does not mint over a sealed package ──────────────────────────────────── +// +// RED-PROOF: delete the `if m.sealedPackageHeld()` block in WriteOffboxSecrets. The password file +// then exists and this test fails on the first assertion — which is exactly the 03:18:06Z event. +func TestR241_ScenarioA_NoMintWhenHubHoldsSealedPackage(t *testing.T) { + m, _, pwPath := mintGuardManager(t, true) + + err := m.WriteOffboxSecrets("PRIVATE-KEY-MATERIAL", "nas.local ssh-ed25519 AAAAhostkey") + + if !IsOffboxSealedPackageHeld(err) { + t.Fatalf("want the sealed-package refusal sentinel, got %v", err) + } + // THE ASSERTION THAT IS THE WHOLE SESSION: no key on disk. + if _, serr := os.Stat(pwPath); !os.IsNotExist(serr) { + t.Fatalf("R-241 REGRESSION: a repository password was minted over the hub's sealed package (stat err=%v)", serr) + } + // The transport IS still written — the refusal is a holding state, not a failure. Without this the + // recovery screen could not bring the tier up when the key arrives (R-219). + for _, f := range []string{"ssh_key", "known_hosts"} { + if _, serr := os.Stat(filepath.Join(filepath.Dir(pwPath), f)); serr != nil { + t.Errorf("transport file %s should still be written on the refusal path: %v", f, serr) + } + } +} + +// Scenario A at the APPLY level — the path the self-heal actually takes. ApplyOffsiteTarget must +// swallow the sentinel, record the target, and NOT stage an escrow. +func TestR241_ScenarioA_ApplyOffsiteTargetHoldsInsteadOfMinting(t *testing.T) { + m, sett, pwPath := mintGuardManager(t, true) + + staged := 0 + stage := func(ctx context.Context, pw string) error { staged++; return nil } + + tgt := &settings.OffboxTarget{Enabled: true, Host: "box.example", Port: 23, User: "u1", RepoPath: "/home/felhom-repo"} + if err := m.ApplyOffsiteTarget(context.Background(), tgt, "KEYMATERIAL", "box.example ssh-ed25519 HOSTKEY", stage); err != nil { + t.Fatalf("apply should SUCCEED into the holding state, not fail: %v", err) + } + if _, serr := os.Stat(pwPath); !os.IsNotExist(serr) { + t.Fatalf("R-241 REGRESSION: apply minted a repository password over the sealed package") + } + if staged != 0 { + t.Errorf("nothing may be staged for escrow — there is no key to escrow; staged=%d", staged) + } + // The target is recorded, so the box stops declaring needs_credential and the hub stops re-staging. + if got := sett.GetOffboxTarget(); got == nil { + t.Fatal("the transport target must be recorded, or the hub re-stages a consumed credential forever") + } + // Runs stay gated: no password file ⇒ not configured. + if m.OffboxConfigured() { + t.Error("OffboxConfigured must be false while the key is awaited — runs must not proceed") + } + // And the box says so, in the state the hub reads. + if !m.OffboxAwaitingRecoveryKey() { + t.Error("OffboxAwaitingRecoveryKey should be true in the holding state") + } + st := m.OffboxReportStatus() + if st == nil || st.State != OffsiteStateAwaitingRecoveryKey { + t.Fatalf("want declared state %q, got %+v", OffsiteStateAwaitingRecoveryKey, st) + } + if st.Enabled { + t.Error("the declared holding object must carry Enabled=false so existing hub readers stay inert") + } +} + +// ── SCENARIO B — a box the hub holds nothing for still mints, exactly as today ─────────────────── +// +// RED-PROOF: widen the guard to `if true` (or drop the GetHubEscrowIdentityPresent() conjunct in +// sealedPackageHeld). A first-time box then cannot start, and this test fails — the failure mode an +// over-broad fix produces, which is why the guard is written as a conjunction. +func TestR241_ScenarioB_FirstTimeBoxStillMints(t *testing.T) { + m, _, pwPath := mintGuardManager(t, false) // the hub holds nothing for us + + if err := m.WriteOffboxSecrets("PRIVATE-KEY-MATERIAL", "nas.local ssh-ed25519 AAAAhostkey"); err != nil { + t.Fatalf("a first-time box must mint exactly as before, got %v", err) + } + pw, rerr := os.ReadFile(pwPath) + if rerr != nil { + t.Fatalf("a first-time box must get a repository password: %v", rerr) + } + if !offboxRepoPwPattern.Match(pw) { + t.Errorf("minted password is not the expected 64-hex shape") + } + if m.OffboxAwaitingRecoveryKey() { + t.Error("a box with no sealed package is not awaiting anything") + } +} + +// The guard must not fire once a key EXISTS — a healthy box re-applying its target (a quota bump, +// a hub re-push) must be untouched, package or no package. This is the idempotency half. +func TestR241_ExistingKeyIsNeverDisturbed(t *testing.T) { + m, _, pwPath := mintGuardManager(t, false) + if err := m.WriteOffboxSecrets("K", "kh"); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(pwPath) + if err != nil { + t.Fatal(err) + } + // Now the hub starts holding a package (the ceremony ran) and the target is re-applied. + if err := m.settings.SetHubEscrowIdentityPresent(true); err != nil { + t.Fatal(err) + } + if err := m.WriteOffboxSecrets("K2", "kh2"); err != nil { + t.Fatalf("a re-apply on a box that already has a key must not be refused: %v", err) + } + after, err := os.ReadFile(pwPath) + if err != nil { + t.Fatal(err) + } + if string(before) != string(after) { + t.Error("the existing repository password must never be rotated by an apply") + } + if m.OffboxAwaitingRecoveryKey() { + t.Error("a box holding its key is not awaiting one") + } +} + +// Fail-safe: an unreadable settings store must not block a tier. A transient read failure turning +// into a permanently-held tier is a worse defect than the one being fixed. +func TestR241_NilSettingsDoesNotBlockTheMint(t *testing.T) { + logger := log.New(os.Stderr, "", 0) + dataDir := t.TempDir() + cfg := &config.Config{} + cfg.Paths.DataDir = dataDir + m := NewManager(cfg, nil, logger) + if m.sealedPackageHeld() { + t.Fatal("a nil settings store must read as 'no package held' — fail toward letting the box work") + } +} + +// Scenario E's carve-out, pinned for the HOLDING state too. A customer who switched off-site off is +// not awaiting a recovery key, and must not declare one. The first draft of +// OffboxAwaitingRecoveryKey omitted `t.Enabled` and TestOffsiteDeclare_DisabledTargetIsNotStranded +// caught it; this test pins the same invariant from the new predicate's own side, so a future edit +// to THIS function fails here rather than in a neighbouring file. +func TestR241_DisabledTargetIsNotAwaitingAnything(t *testing.T) { + m, sett, _ := mintGuardManager(t, true) // the hub holds a package, and there is no key + if err := sett.SetOffboxTarget(&settings.OffboxTarget{ + Enabled: false, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo", + }); err != nil { + t.Fatal(err) + } + if m.OffboxAwaitingRecoveryKey() { + t.Fatal("a deliberately DISABLED target must never declare the holding state (Scenario E)") + } + if st := m.OffboxReportStatus(); st != nil { + t.Fatalf("a disabled target must stay silent in the report, got %+v", st) + } +} diff --git a/controller/internal/web/recovery_test.go b/controller/internal/web/recovery_test.go index f4f077a..2b14bbf 100644 --- a/controller/internal/web/recovery_test.go +++ b/controller/internal/web/recovery_test.go @@ -118,14 +118,16 @@ func newRecoveryFixture(t *testing.T) *recoveryFixture { t.Fatal(err) } mgr := backup.NewManager(cfg, sett, lg) - if err := mgr.WriteOffboxSecrets("PRIVATE-KEY-MATERIAL", "nas.local ssh-ed25519 AAAAhostkey"); err != nil { - t.Fatal(err) - } - // WriteOffboxSecrets auto-generates a repository password — remove it, because "this box cannot - // open the inherited history" is the whole precondition of the screen. - if err := os.Remove(filepath.Join(cfg.Paths.DataDir, "offbox", "repo_password")); err != nil { + // R-241 (v0.206.0): with a sealed package held, WriteOffboxSecrets now REFUSES to mint — which is + // precisely the state this fixture used to hand-construct by deleting the key afterwards. Accept + // the sentinel; it is the product doing what this test's precondition describes. + if err := mgr.WriteOffboxSecrets("PRIVATE-KEY-MATERIAL", "nas.local ssh-ed25519 AAAAhostkey"); err != nil && + !backup.IsOffboxSealedPackageHeld(err) { t.Fatal(err) } + // Belt and braces for any path that DID mint (a fixture variant with no package held): "this box + // cannot open the inherited history" is the whole precondition of the screen. + _ = os.Remove(filepath.Join(cfg.Paths.DataDir, "offbox", "repo_password")) rr := &recoveryRunner{statsSize: 4 << 20} mgr.SetOffboxRunner(rr.run)