hub R-435: each clean-up window explains one fall only; a window stuck open past its deadline explains nothing (security review)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS

hub/internal/monitor/offsite.go

@@ -46,6 +46,9 @@ type OffsiteChecker struct {
 	// R-435: when the hub RECEIVED the report that set lastCounts — the start of the interval whose
 	// clean-up windows may explain the next fall (pinned tiers only, see snapshotDropped).
 	lastCountAt map[string]time.Time
+	// R-435: clean-up windows already spent explaining a fall, per customer — each window explains one
+	// fall only, so the slack before the interval cannot let it excuse a second one (security review).
+	usedWindows map[string]map[int64]bool
 }
 
 const defaultOffsiteStaleAfter = 48 * time.Hour
@@ -84,6 +87,7 @@ func NewOffsiteChecker(s *store.Store, staleAfter time.Duration, onEvent EventNo
 		store: s, logger: logger, onEvent: onEvent, staleAfter: staleAfter, now: time.Now,
 		fillStates: make(map[string]string), staleStates: make(map[string]string),
 		dropStates: make(map[string]string), lastCounts: make(map[string]int), lastCountAt: make(map[string]time.Time),
+		usedWindows: make(map[string]map[int64]bool),
 	}
 	customers, err := s.GetCustomers()
 	if err != nil {
@@ -221,22 +225,22 @@ func (oc *OffsiteChecker) isStale(customerID string, off *offsiteReport) bool {
 // THE THRESHOLD, AND THE MEASUREMENT IT CAME FROM. Measured over the hub's own `reports` table on
 // 2026-09-01: 12 898 reports, 4 customers, 2026-06-05 → 2026-09-01.
 //
-//   * In the whole history there are NINE decreases, and EVERY ONE of them lands exactly on ZERO
+//   - In the whole history there are NINE decreases, and EVERY ONE of them lands exactly on ZERO
 //     (36→0, 18→0 ×2, 15→0, 12→0, 8→0, 3→0). There is not one gradual retention decrease anywhere.
-//   * Every one of those nine predates `stats_known`, i.e. they are the R-331 shape — a zero that
+//   - Every one of those nine predates `stats_known`, i.e. they are the R-331 shape — a zero that
 //     means "could not measure", not "nothing is there". Several carry a declared State
 //     (`needs_credential`, `awaiting_recovery_key`), which says so outright.
-//   * In the window where `stats_known` is TRUE (380 reports across both live boxes) there are ZERO
+//   - In the window where `stats_known` is TRUE (380 reports across both live boxes) there are ZERO
 //     decreases: demo-felhom sat flat at 10, demo-hp moved 67→68→69. Only rises.
 //
 // So observed retention churn gives NOTHING to calibrate against, and saying so is the honest answer
 // rather than inventing a number (R-401's lesson). The threshold is therefore reasoned from what
 // retention CAN do, not from what it was seen to do:
 //
-//   the box runs `forget --keep-daily 7 --keep-weekly 4 --keep-monthly 6 --group-by host,tags`
-//   over ~8 apps. On a boundary day several groups can expire at once, so a legitimate pass can
-//   plausibly remove low double digits. **What it can NEVER do is halve the total**: keeping 7 daily
-//   + 4 weekly + 6 monthly per group is a floor, and a mass deletion goes to ~0.
+//	the box runs `forget --keep-daily 7 --keep-weekly 4 --keep-monthly 6 --group-by host,tags`
+//	over ~8 apps. On a boundary day several groups can expire at once, so a legitimate pass can
+//	plausibly remove low double digits. **What it can NEVER do is halve the total**: keeping 7 daily
+//	+ 4 weekly + 6 monthly per group is a floor, and a mass deletion goes to ~0.
 //
 // Hence: **a fall of MORE THAN HALF the previous count, and at least 5 snapshots.** The 50% cannot be
 // reached by retention; the floor of 5 stops a tiny-count box alarming on ordinary ageing. It is
@@ -318,7 +322,19 @@ func (oc *OffsiteChecker) snapshotDropped(customerID string, off *offsiteReport,
 	drop := prev - off.SnapshotCount
 	if drop > 0 && oc.pinnedTier(customerID) {
 		from := oc.lastCountAt[customerID].Add(-windowSlack)
-		removed, unknown := oc.store.RemovedByWindowsBetween(customerID, from, reportAt)
+		credits, unknown := oc.store.WindowCreditsBetween(customerID, from, reportAt)
+		used := oc.usedWindows[customerID]
+		if used == nil {
+			used = make(map[int64]bool)
+			oc.usedWindows[customerID] = used
+		}
+		removed := 0
+		for _, c := range credits {
+			if !used[c.ID] {
+				removed += c.Explains
+				used[c.ID] = true // spent on this fall, explained or not
+			}
+		}
 		if !unknown {
 			if drop > removed {
 				return true, prev, off.SnapshotCount, true
@@ -449,6 +465,7 @@ func (oc *OffsiteChecker) Check() {
 			delete(oc.dropStates, c.CustomerID)
 			delete(oc.lastCounts, c.CustomerID) // no baseline survives a vanished object
 			delete(oc.lastCountAt, c.CustomerID)
+			delete(oc.usedWindows, c.CustomerID)
 			continue
 		}
 		if oc.store.IsCustomerBlocked(c.CustomerID) {
@@ -457,6 +474,7 @@ func (oc *OffsiteChecker) Check() {
 			delete(oc.dropStates, c.CustomerID)
 			delete(oc.lastCounts, c.CustomerID)
 			delete(oc.lastCountAt, c.CustomerID)
+			delete(oc.usedWindows, c.CustomerID)
 			continue
 		}
 
@@ -524,6 +542,7 @@ func (oc *OffsiteChecker) Check() {
 			delete(oc.dropStates, k)
 			delete(oc.lastCounts, k)
 			delete(oc.lastCountAt, k)
+			delete(oc.usedWindows, k)
 		}
 	}
 }

hub/internal/monitor/r435_pinned_drop_test.go

@@ -117,6 +117,38 @@ func TestR435_LyingWindowExplainsOnlyItsCap(t *testing.T) {
 	}
 }
 
+// Security review 2026-10-08: one window explains ONE fall. A window that removed 9 explains 69 → 60; a
+// second fall 60 → 55 in the next report (inside the 2-h slack, no new window) alarms.
+// RED-PROOF: stop marking windows as spent (usedWindows) → the second fall is explained again → 0 alarms → FAILS.
+func TestR435_OneWindowExplainsOneFall(t *testing.T) {
+	st := newDiskStore(t)
+	cust := "p10"
+	if err := st.RecordOffsiteKeyInstalled(cust, "fp-1"); err != nil {
+		t.Fatal(err)
+	}
+	if ok, err := st.RecordOffsiteKeyConfirmed(cust, "fp-1"); err != nil || !ok {
+		t.Fatalf("confirm: %v %v", ok, err)
+	}
+	var msgs []string
+	saveOffsiteReport(t, st, cust, dropJSON(69, true, "", "ok"))
+	oc := NewOffsiteChecker(st, 48*time.Hour, func(_, et, _, msg, _, _ string) {
+		if et == "offsite_snapshots_dropped" {
+			msgs = append(msgs, msg)
+		}
+	}, quietLog())
+	closedWindow(t, cust, 69, 60)(st)
+	saveOffsiteReport(t, st, cust, dropJSON(60, true, "", "ok"))
+	oc.Check()
+	if len(msgs) != 0 {
+		t.Fatalf("the window explains 69 -> 60; got %d alarm(s)", len(msgs))
+	}
+	saveOffsiteReport(t, st, cust, dropJSON(55, true, "", "ok"))
+	oc.Check()
+	if len(msgs) != 1 {
+		t.Fatalf("a spent window must not explain the next fall 60 -> 55; got %d alarm(s)", len(msgs))
+	}
+}
+
 // Controls: the half-rule still governs a non-pinned tier, and an installed-but-unconfirmed key.
 func TestR435_NotPinnedKeepsHalfRule(t *testing.T) {
 	if msgs := r435Run(t, "n1", r435Setup{next: dropJSON(60, true, "", "ok")}); len(msgs) != 0 {

hub/internal/store/offsite_keys.go

@@ -325,36 +325,44 @@ func (s *Store) ForceOffsiteAbandonDueForTest(id int64) error {
 	return err
 }
 
-// RemovedByWindowsBetween — R-435 (D7, `09` §3 decision 191). How many snapshots the clean-up windows
-// the hub opened for this customer EXPLAIN between two reports, over windows closed in (from, to] or
-// still open. On a pinned tier these windows are the only legitimate way the count can fall, so
-// anything beyond the sum is unexplained.
+// WindowCredit is how many snapshots one clean-up window may explain (R-435).
+type WindowCredit struct {
+	ID       int64
+	Explains int
+}
+
+// WindowCreditsBetween — R-435 (D7, `09` §3 decision 191). The clean-up windows the hub opened for this
+// customer that may EXPLAIN a fall between two reports: windows closed in (from, to], and windows still
+// open at `to` whose closes_by has not passed before `from` (a window stuck open past its deadline
+// explains nothing). On a pinned tier these windows are the only legitimate way the count can fall.
 //
 // Each window explains AT MOST its hub-set max_remove (security review 2026-10-08): count_after is the
 // box's own word, so a box that lies about it — or a window closed by timeout or still open, which has
 // no count_after — can never explain more than the cap the hub itself granted. A closed window with a
-// count_after explains min(count_before − count_after, max_remove); a window with no usable count_after
-// explains max_remove.
+// count_after explains min(count_before − count_after, max_remove); one with no usable count_after
+// explains max_remove. The CALLER spends each window once (OffsiteChecker.usedWindows), so the slack
+// before `from` cannot let one window explain several falls.
 //
 // unknown = true only when the store cannot answer (a query error, or a window with no usable cap). The
-// caller then falls back to the half-rule — never to „explained" (fail closed, the review's other
-// finding). Pinned by r435_windows_between_test.go and r435_pinned_drop_test.go.
-func (s *Store) RemovedByWindowsBetween(customerID string, from, to time.Time) (removed int, unknown bool) {
+// caller then falls back to the half-rule — never to „explained". Pinned by r435_windows_between_test.go
+// and r435_pinned_drop_test.go.
+func (s *Store) WindowCreditsBetween(customerID string, from, to time.Time) (credits []WindowCredit, unknown bool) {
 	const f = "2006-01-02 15:04:05"
 	rows, err := s.db.Query(`
-		SELECT count_before, count_after, max_remove, closed_at IS NULL FROM offsite_windows
+		SELECT id, count_before, count_after, max_remove, closed_at IS NULL FROM offsite_windows
 		WHERE customer_id = ?
-		  AND ((closed_at IS NULL AND opened_at <= ?) OR (closed_at > ? AND closed_at <= ?))`,
-		customerID, to.UTC().Format(f), from.UTC().Format(f), to.UTC().Format(f))
+		  AND ((closed_at IS NULL AND opened_at <= ? AND closes_by > ?) OR (closed_at > ? AND closed_at <= ?))`,
+		customerID, to.UTC().Format(f), from.UTC().Format(f), from.UTC().Format(f), to.UTC().Format(f))
 	if err != nil {
-		return 0, true
+		return nil, true
 	}
 	defer rows.Close()
 	for rows.Next() {
+		var id int64
 		var before, after, maxRemove sql.NullInt64
 		var open bool
-		if err := rows.Scan(&before, &after, &maxRemove, &open); err != nil {
-			return 0, true
+		if err := rows.Scan(&id, &before, &after, &maxRemove, &open); err != nil {
+			return nil, true
 		}
 		if !maxRemove.Valid || maxRemove.Int64 <= 0 {
 			unknown = true
@@ -367,11 +375,11 @@ func (s *Store) RemovedByWindowsBetween(customerID string, from, to time.Time) (
 			}
 		}
 		if explains > 0 {
-			removed += explains
+			credits = append(credits, WindowCredit{ID: id, Explains: explains})
 		}
 	}
 	if rows.Err() != nil {
-		return 0, true
+		return nil, true
 	}
-	return removed, unknown
+	return credits, unknown
 }

hub/internal/store/r435_windows_between_test.go

@@ -5,12 +5,20 @@ import (
 	"time"
 )
 
-// R-435: RemovedByWindowsBetween sums only windows closed inside the interval (or still open), and each
+// R-435: WindowCreditsBetween lists only windows closed inside the interval (or still open), and each
 // window explains at most its hub-set max_remove — a box's count_after cannot widen it, and a window with
 // no usable count_after explains exactly its cap. Only a window with no cap makes the answer unknown.
 // RED-PROOF (security review 2026-10-08): drop the max_remove cap → „lying box" returns 69, not 34 → FAILS.
 func TestR435_RemovedByWindowsBetween(t *testing.T) {
 	s := newTestStore(t)
+	sum := func(cust string, from, to time.Time) (int, bool) {
+		cs, unk := s.WindowCreditsBetween(cust, from, to)
+		n := 0
+		for _, c := range cs {
+			n += c.Explains
+		}
+		return n, unk
+	}
 	at := func(ts string) time.Time { v, _ := time.Parse("2006-01-02 15:04:05", ts); return v }
 	ins := func(cust, opened, closed string, before, after, maxRemove any) {
 		t.Helper()
@@ -18,8 +26,9 @@ func TestR435_RemovedByWindowsBetween(t *testing.T) {
 		if closed != "" {
 			c = closed
 		}
+		closesBy := "2026-10-01 23:00:00" // a window's deadline; an open one past it before `from` is stale
 		if _, err := s.db.Exec(`INSERT INTO offsite_windows (customer_id, opened_at, closes_by, closed_at, count_before, count_after, max_remove) VALUES (?, ?, ?, ?, ?, ?, ?)`,
-			cust, opened, opened, c, before, after, maxRemove); err != nil {
+			cust, opened, closesBy, c, before, after, maxRemove); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -29,30 +38,38 @@ func TestR435_RemovedByWindowsBetween(t *testing.T) {
 	ins("b", "2026-10-01 03:00:00", "2026-10-01 03:10:00", 50, 40, 25) // another customer
 
 	from, to := at("2026-09-30 00:00:00"), at("2026-10-02 00:00:00")
-	if n, unk := s.RemovedByWindowsBetween("a", from, to); n != 9 || unk {
+	if n, unk := sum("a", from, to); n != 9 || unk {
 		t.Fatalf("a: want 9 known, got %d unknown=%v", n, unk)
 	}
-	if n, unk := s.RemovedByWindowsBetween("c", from, to); n != 0 || unk {
+	if n, unk := sum("c", from, to); n != 0 || unk {
 		t.Fatalf("no window: want 0 known, got %d unknown=%v", n, unk)
 	}
 	// a box that claims it removed everything explains only the cap the hub granted
 	ins("l", "2026-10-01 03:00:00", "2026-10-01 03:10:00", 69, 0, 34)
-	if n, unk := s.RemovedByWindowsBetween("l", from, to); n != 34 || unk {
+	if n, unk := sum("l", from, to); n != 34 || unk {
 		t.Fatalf("lying box: want the cap 34, got %d unknown=%v", n, unk)
 	}
 	// a timeout close (count_after −1) inside the interval → explains its cap
 	ins("d", "2026-10-01 03:00:00", "2026-10-01 03:40:00", 69, -1, 10)
-	if n, unk := s.RemovedByWindowsBetween("d", from, to); n != 10 || unk {
+	if n, unk := sum("d", from, to); n != 10 || unk {
 		t.Fatalf("timeout-closed window: want its cap 10, got %d unknown=%v", n, unk)
 	}
 	// a window still open → explains its cap
 	ins("e", "2026-10-01 03:00:00", "", 69, nil, 10)
-	if n, unk := s.RemovedByWindowsBetween("e", from, to); n != 10 || unk {
+	if n, unk := sum("e", from, to); n != 10 || unk {
 		t.Fatalf("open window: want its cap 10, got %d unknown=%v", n, unk)
 	}
+	// a window stuck open past its deadline before the interval explains nothing
+	ins("g", "2026-09-01 03:00:00", "", 69, nil, 10)
+	if _, err := s.db.Exec(`UPDATE offsite_windows SET closes_by = '2026-09-01 04:00:00' WHERE customer_id = 'g'`); err != nil {
+		t.Fatal(err)
+	}
+	if n, unk := sum("g", from, to); n != 0 || unk {
+		t.Fatalf("stale open window: want 0, got %d unknown=%v", n, unk)
+	}
 	// a window with no cap → unknown (the caller falls back to the half-rule)
 	ins("f", "2026-10-01 03:00:00", "2026-10-01 03:10:00", 69, 60, nil)
-	if _, unk := s.RemovedByWindowsBetween("f", from, to); !unk {
+	if _, unk := sum("f", from, to); !unk {
 		t.Fatal("a window with no cap must make the interval unknown")
 	}
 }