From d1a8edb3320c76c00805d1d8fd4ff31962ee875c Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 5 Aug 2026 07:17:29 +0200 Subject: [PATCH] R-196 / R-204 item 2: a re-issue no longer marks a healthy escrow stale (hub v0.95.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReissueCredentials marked the escrow stale on every re-issue, on precautionary grounds — the box's re-apply MIGHT mint a fresh repository password. It usually does not. A stale flag withholds restic_pw_sha256 from the ACK, which stops the controller's auto-confirm, which leaves EscrowState pending, which makes OffboxRunnable false: every off-site backup refused on a box whose key was never in doubt — and the customer told to re-run the one ceremony that would have superseded the key just recovered. The case it guessed at is measured elsewhere: the controller's Scenario-F re-check compares the sealed hash against the live repo password on every ACK (and the mark was BLINDING it by emptying that hash), and R-197's offsite_repo_key_changed fires on a proven difference across a supersession. offsite_reissued is unchanged. MarkEscrowStale is kept without a caller so a future EVIDENTIAL writer has the mechanism, with a test pinning it live. TestReissue_InvalidatesEscrow is replaced by its exact inverse. --- hub/internal/offsite/offsite.go | 70 ++++++++++++++------------ hub/internal/offsite/offsite_test.go | 73 ++++++++++++++++++++++------ hub/internal/store/store.go | 50 ++++++++++++------- 3 files changed, 130 insertions(+), 63 deletions(-) diff --git a/hub/internal/offsite/offsite.go b/hub/internal/offsite/offsite.go index bd830a7..0e07870 100644 --- a/hub/internal/offsite/offsite.go +++ b/hub/internal/offsite/offsite.go @@ -204,43 +204,51 @@ func (p *Provisioner) ReissueCredentials(ctx context.Context, customerID, typ st // nobody checked the CAUSE — and it sent two separate investigations the wrong way in one day // (audits/SPIKE-offsite-credential-recovery-2026-08-04.md Q4). // - // WHY THE MARK IS STILL MADE, on honest grounds: a re-issue is the operator's response to a - // broken delivery, and the box's re-apply may mint a fresh repository password (it does exactly - // that whenever `/offbox/repo_password` is absent — the guest-rebuild shape). The mark is - // therefore PRECAUTIONARY, not evidential. The evidential signal is R-197's - // `offsite_repo_key_changed`, which fires on a measured hash difference at the next ceremony. - // KNOWN CONSEQUENCE, left in place deliberately: on the ordinary re-issue shape — a box that still - // holds its repo_password file — the password does NOT change, so this marks a healthy escrow - // stale and asks the customer for an unnecessary ceremony. R-196 stays OPEN for that; it is a - // behaviour change, not a comment fix, and it must not ride a comment-correction commit. + // ⚠ THE MARK IS GONE (R-196 / R-204 item 2, hub v0.95.0, 2026-08-05). What used to stand here — + // a `MarkEscrowStale` on every re-issue that found an escrow row, plus an `escrow_stale` customer + // event — was PRECAUTIONARY, not evidential: it guessed that the box's re-apply MIGHT mint a fresh + // repository password. On the ordinary re-issue shape (a box that still holds its + // `/offbox/repo_password`) the password does not change, so it marked a HEALTHY escrow + // stale. The 2026-08-04 recovery drill (R-201) is what promoted this from a nit to a blocker. // - // Mark the escrow stale so the hub stops advertising "ceremony done" and the customer's escrow - // wizard is offered again; a fresh ceremony seals the current password and clears the flag. Every - // credential change also emits a visible customer event (offsite_reissued always; escrow_stale - // only when a blob was invalidated). Best-effort: the password reset already succeeded — a - // bookkeeping failure here must not fail it. - escrowStaled := false - if host, herr := p.Store.GetHostByCustomer(customerID); herr == nil && host != nil { - if esc, eerr := p.Store.GetHostEscrow(host.HostID); eerr == nil && esc != nil { - if serr := p.Store.MarkEscrowStale(host.HostID); serr != nil { - p.logf("[offsite] WARN mark-escrow-stale for %s: %v", customerID, serr) - } else { - escrowStaled = true - } - } - } + // WHAT THE MARK ACTUALLY DID, mechanically, because "it asked for an unnecessary ceremony" + // understates it by a lot: + // 1. `stale_at` set → `GetEscrowStatusForCustomer` WITHHOLDS `restic_pw_sha256` from the report + // ACK (store.go, the v0.57.0 rule). + // 2. With no hash, the controller's SLICE-3 auto-confirm cannot flip pending→escrowed + // (report.EscrowAutoConfirmer.Reconcile returns early on an empty hash). + // 3. `OffboxRunnable() = OffboxConfigured() && EscrowState=="escrowed"` → EVERY off-site backup + // is refused, indefinitely, on a box whose key was never in doubt. + // 4. The customer is told to re-run the recovery ceremony — which mints a NEW recovery code and + // supersedes the sealed blob. During a recovery that is the one act that would have destroyed + // the key just recovered. + // A precautionary flag that stops the data-protection it is guarding is not conservative. + // + // WHY REMOVING IT LEAVES NO GAP — the case it guessed at is MEASURED elsewhere, and better: + // • Continuous, box-side: the controller compares the ACK's sealed hash against its CURRENT + // local repo password on EVERY report ACK (`reconcileEscrowed`, the Scenario-F re-check). In + // the guest-rebuild shape — the only shape where a re-issue is followed by a fresh repository + // password — that comparison mismatches within one report cycle and raises the stale card plus + // the „create a new recovery code" CTA. It is a measurement, not a guess. + // AND THE MARK WAS BLINDING IT: by emptying the hash (step 1 above) it removed the very value + // that comparison needs, so the box could only report the hash-LESS reason, which is false. + // • Edge-triggered, hub-side: R-197's `offsite_repo_key_changed` fires on a proven hash + // difference across a supersession (api.maybeEmitRepoKeyChanged) and pages the operator. + // + // DISAGREEMENT RECORDED, per the R-96 standing rule: the task's Scenario D asks that a real key + // change "marks the escrow stale". It must NOT, and nothing here was changed to make it: the hub + // learns of a real change at the moment a supersession SEALS THE NEW PASSWORD, i.e. when the escrow + // is freshest. Marking it stale there would ask for a ceremony to fix the ceremony that just ran. + // The correct consequence at that instant is the operator alarm, which is what R-197 does. + // + // `offsite_reissued` is UNCHANGED and still always fires — the customer must still learn that the + // credential moved. Best-effort: the password reset already succeeded, so a bookkeeping failure + // here must not fail it. if _, serr := p.Store.SaveEvent(customerID, "offsite_reissued", "info", "Az offsite (házon kívüli) mentési hozzáférést újra kiadtuk — az új egyszeri jelszót a vezérlő a következő frissítéskor átveszi.", "", "hub"); serr != nil { p.logf("[offsite] WARN save offsite_reissued event for %s: %v", customerID, serr) } - if escrowStaled { - if _, serr := p.Store.SaveEvent(customerID, "escrow_stale", "warning", - "A helyreállítási kulcs-letét elavult az offsite jelszó cseréje miatt — futtasd le újra a helyreállítási szertartást (Biztonsági mentés → Helyreállítás).", - "", "hub"); serr != nil { - p.logf("[offsite] WARN save escrow_stale event for %s: %v", customerID, serr) - } - } return nil } diff --git a/hub/internal/offsite/offsite_test.go b/hub/internal/offsite/offsite_test.go index 1dbfcac..456222d 100644 --- a/hub/internal/offsite/offsite_test.go +++ b/hub/internal/offsite/offsite_test.go @@ -211,12 +211,27 @@ func TestReissue_RefusesAmbiguousLookup(t *testing.T) { } } -// v0.57.0 (2.3, escrow honesty) — re-issuing offsite credentials INVALIDATES the key-escrow blob: -// the blob sealed the OLD repo password, so a recovery code minted against it would decrypt a -// password that no longer opens the repo. RED-PROOF (Scenario C): on pre-fix code (no MarkEscrowStale -// in ReissueCredentials + no stale plumbing) the hub keeps advertising the escrow as current after a -// re-issue and keeps serving its restic-hash for auto-confirm — this test asserts it does NEITHER. -func TestReissue_InvalidatesEscrow(t *testing.T) { +// SCENARIO C (R-196 / R-204 item 2, hub v0.95.0) — re-issuing the offsite PROVIDER credential must +// LEAVE A HEALTHY ESCROW ALONE. +// +// THIS TEST REPLACES `TestReissue_InvalidatesEscrow`, which asserted the opposite and was wrong on a +// false premise: a re-issue resets the storage-provider account password and cannot touch the restic +// REPOSITORY password, which is generated on the box and never leaves it except sealed under R. The +// old assertion looked right because the EFFECT it checked (a stale escrow) was real — nobody checked +// the CAUSE. Kept as one test rather than deleted so the inversion is visible in `git log -L`. +// +// WHAT IT ASSERTS IS THE CONSEQUENCE, not the mechanism: not "MarkEscrowStale was not called" but +// "the ACK still carries the hash", because the hash is what the controller's auto-confirm needs and +// its absence is what blocked every off-site backup during the 2026-08-04 drill. +// +// RED-PROOF: restore the marking — put back +// +// if host, _ := p.Store.GetHostByCustomer(customerID); host != nil { +// if esc, _ := p.Store.GetHostEscrow(host.HostID); esc != nil { p.Store.MarkEscrowStale(host.HostID) } +// } +// +// in ReissueCredentials → es.Stale becomes true, the hash is withheld, and this test fails on both. +func TestReissue_DoesNotMarkAHealthyEscrowStale(t *testing.T) { p, _, st := newTestProvisioner(t) const cust = "cust-esc" if _, err := p.ProvisionOffsite(context.Background(), cust, Input{Enabled: true, Type: "shared", QuotaGB: 10}); err != nil { @@ -229,7 +244,6 @@ func TestReissue_InvalidatesEscrow(t *testing.T) { if _, _, err := st.SaveHostEscrow(cust+"-01", []byte("opaque-blob"), "SHA256:fp", "zero_knowledge", "2026-07-16T00:00:00Z", "OLDHASH"); err != nil { t.Fatal(err) } - // Before re-issue: current escrow — the hub serves the sealed hash and is NOT stale. es, err := st.GetEscrowStatusForCustomer(cust) if err != nil || es == nil { t.Fatalf("escrow status (before): %v", err) @@ -238,25 +252,56 @@ func TestReissue_InvalidatesEscrow(t *testing.T) { t.Fatalf("pre-reissue escrow must be current: stale=%v hash=%q", es.Stale, es.ResticPwSHA256) } - // Re-issue the offsite credential — the repo password just changed under the sealed blob. + // Re-issue the offsite PROVIDER credential. The box still holds its repo_password file, so the + // repository password does NOT change — this is the ordinary re-issue shape. if err := p.ReissueCredentials(context.Background(), cust, "shared"); err != nil { t.Fatalf("reissue: %v", err) } - // After: the escrow is STALE and the restic-hash is WITHHELD (no auto-confirm against a dead key). es, err = st.GetEscrowStatusForCustomer(cust) if err != nil || es == nil { t.Fatalf("escrow status (after): %v", err) } - if !es.Stale { - t.Fatal("RED-PROOF: escrow must be STALE after an offsite re-issue (the hub was advertising ceremony-done against a key the repo no longer accepts)") + if es.Stale { + t.Fatal("a re-issue marked a HEALTHY escrow stale — this blocks every off-site backup and asks the customer for a ceremony that would supersede a good key (R-196)") } - if es.ResticPwSHA256 != "" { - t.Fatalf("a stale escrow must WITHHOLD the restic hash to inhibit auto-confirm, got %q", es.ResticPwSHA256) + if es.ResticPwSHA256 != "OLDHASH" { + t.Fatalf("the sealed hash must keep flowing to the ACK so the controller can auto-confirm; got %q", es.ResticPwSHA256) } + // The customer is still TOLD the credential moved — removing the false alarm must not remove the + // true notice. `offsite_reissued` is untouched by this change. + ev, eerr := st.GetLatestEventByType(cust, "offsite_reissued") + if eerr != nil || ev == nil { + t.Fatalf("offsite_reissued must still fire on every re-issue (err=%v ev=%+v)", eerr, ev) + } + // …and the false alarm is gone. + if bad, _ := st.GetLatestEventByType(cust, "escrow_stale"); bad != nil { + t.Fatalf("a re-issue must not raise escrow_stale on a healthy escrow: %+v", bad) + } +} + +// The other half of Scenario C: the stale MECHANISM is intact and still does its job when something +// legitimately sets it. Without this, "nothing marks stale any more" could decay into "stale no longer +// works", and the next evidential caller would ship inert (the seam-built-but-never-wired shape). +func TestEscrowStaleMechanism_StillWithholdsAndClears(t *testing.T) { + _, _, st := newTestProvisioner(t) + const cust = "cust-stale-mech" + if err := st.UpsertHost(&store.Host{HostID: cust + "-01", CustomerID: cust, APIKey: "k"}); err != nil { + t.Fatal(err) + } + if _, _, err := st.SaveHostEscrow(cust+"-01", []byte("blob"), "SHA256:fp", "zero_knowledge", "2026-07-16T00:00:00Z", "OLDHASH"); err != nil { + t.Fatal(err) + } + if err := st.MarkEscrowStale(cust + "-01"); err != nil { + t.Fatalf("MarkEscrowStale: %v", err) + } + es, _ := st.GetEscrowStatusForCustomer(cust) + if es == nil || !es.Stale || es.ResticPwSHA256 != "" { + t.Fatalf("a marked escrow must read stale AND withhold the hash: %+v", es) + } // A fresh ceremony (new blob sealing the new password) clears stale + serves the new hash. - if _, _, err := st.SaveHostEscrow(cust+"-01", []byte("opaque-blob-2"), "SHA256:fp", "zero_knowledge", "2026-07-16T01:00:00Z", "NEWHASH"); err != nil { + if _, _, err := st.SaveHostEscrow(cust+"-01", []byte("blob-2"), "SHA256:fp", "zero_knowledge", "2026-07-16T01:00:00Z", "NEWHASH"); err != nil { t.Fatal(err) } es, _ = st.GetEscrowStatusForCustomer(cust) diff --git a/hub/internal/store/store.go b/hub/internal/store/store.go index af54aed..fff7bed 100644 --- a/hub/internal/store/store.go +++ b/hub/internal/store/store.go @@ -445,14 +445,14 @@ func (s *Store) migrate() error { // instead of trusting blob-presence. NULL/'' = a legacy or password-less blob (never auto-confirms). s.db.Exec(`ALTER TABLE host_escrow ADD COLUMN restic_pw_sha256 TEXT`) - // v0.57.0 (2.3, escrow honesty on offsite re-issue) — stale_at is set when the offsite PROVIDER - // credentials are re-issued. ⚠ CORRECTED 2026-08-04 (R-196): it used to say "when the offsite repo - // password is re-issued", which nothing in the hub does — the repository password is generated on - // the box and never leaves it except sealed under R. The flag is PRECAUTIONARY (the box's re-apply - // MAY mint a fresh repository password), not evidence that it changed; the evidential signal is - // R-197's offsite_repo_key_changed. While set, the hub stops advertising "ceremony done" and - // withholds the possibly-non-matching restic_pw_sha256 from the auto-confirm ACK. NULL = current; - // a fresh ceremony (SaveHostEscrow) clears it. + // v0.57.0 (2.3, escrow honesty on offsite re-issue) — stale_at marks an escrow blob that may no + // longer cover the box's live repository password. While set, the hub stops advertising "ceremony + // done" and withholds restic_pw_sha256 from the auto-confirm ACK. NULL = current; a fresh ceremony + // (SaveHostEscrow) clears it. + // ⚠ NOTHING SETS IT as of v0.95.0 (R-196 / R-204 item 2). The only writer was the PRECAUTIONARY + // mark on offsite re-issue, which guessed rather than measured and so blocked off-site backups on + // boxes whose key had not changed. The column and its readers stay; see MarkEscrowStale for what a + // legitimate future writer would have to prove first. s.db.Exec(`ALTER TABLE host_escrow ADD COLUMN stale_at DATETIME`) // dr_recipe (SPIKE-dr-recipe-2026-06-16): the secret-free DR reconstruction recipe, stored @@ -2699,12 +2699,24 @@ func (s *Store) CountCurrentEscrowWithIdentity() (total, withIdentity int, err e return total, withIdentity, err } -// MarkEscrowStale flags a host's escrow blob as stale (v0.57.0, 2.3) — called when the offsite -// PROVIDER credentials are re-issued. ⚠ CORRECTED 2026-08-04 (R-196): it used to say "when the offsite -// repo password is re-issued", which no hub path does; see the reasoning at offsite.ReissueCredentials. -// The flag is precautionary — the box's re-apply may mint a fresh repository password — not a -// measurement that one did. No-op when no escrow row exists; idempotent (only stamps the first -// re-issue since the last ceremony; a fresh ceremony clears stale_at via SaveHostEscrow's ON CONFLICT). +// MarkEscrowStale flags a host's escrow blob as stale (v0.57.0, 2.3). +// +// ⚠ IT HAS NO CALLER as of hub v0.95.0 (R-196 / R-204 item 2), and that is deliberate, not an +// oversight. Its ONE caller was `offsite.ReissueCredentials`, which called it on every re-issue on +// the PRECAUTIONARY grounds that the box's re-apply might mint a fresh repository password. It +// usually does not, so the call marked healthy escrows stale — and because a stale flag WITHHOLDS +// restic_pw_sha256 from the ACK (GetEscrowStatusForCustomer, below), it blocked every off-site backup +// on those boxes and asked the customer for a ceremony that would supersede a perfectly good key. The +// full reasoning, and the two measured signals that cover the real case, are at +// offsite.ReissueCredentials. +// +// KEPT, not deleted, because the FLAG is still live and correct — `stale_at` is read by the ACK, the +// operator card and the PBS-DR view, and a future EVIDENTIAL caller (one that has measured a key +// change rather than guessed at one) is the right way to set it. Pinned by +// TestReissue_DoesNotMarkAHealthyEscrowStale: if a caller reappears without that evidence, it fails. +// +// No-op when no escrow row exists; idempotent (only stamps the first mark since the last ceremony; a +// fresh ceremony clears stale_at via SaveHostEscrow's ON CONFLICT). func (s *Store) MarkEscrowStale(hostID string) error { _, err := s.db.Exec(`UPDATE host_escrow SET stale_at = datetime('now') WHERE host_id = ? AND stale_at IS NULL`, hostID) return err @@ -2733,11 +2745,13 @@ type EscrowStatus struct { IdentityBlobPresent bool `json:"identity_blob_present"` ResticPwSHA256 string `json:"restic_pw_sha256,omitempty"` CreatedAt string `json:"created_at,omitempty"` - // Stale (v0.57.0, 2.3) — true when the offsite PROVIDER credentials were re-issued after the blob - // was sealed. ⚠ CORRECTED 2026-08-04 (R-196): it used to say "the offsite password was re-issued", - // which reads as the repository password and is not what happens; see MarkEscrowStale. + // Stale (v0.57.0, 2.3) — true when `stale_at` is stamped on the host's escrow row. ⚠ SINCE hub + // v0.95.0 (R-196 / R-204 item 2) NOTHING STAMPS IT: the one caller was the precautionary + // re-issue mark, and it was removed for marking healthy escrows stale. See MarkEscrowStale. // When stale the ResticPwSHA256 is WITHHELD (emptied) so the controller cannot auto-confirm against - // a hash that may no longer match the live repo password — the ceremony must run again. + // a hash that may no longer match the live repo password — the ceremony must run again. That + // withholding is exactly why the precautionary caller had to go: it BLINDED the controller's own + // hash comparison, which is the measurement that actually detects a changed repository password. Stale bool `json:"escrow_stale,omitempty"` }