amadan
cloud: reaper false-positive guards — content check + per-sweep cap (#443)
Two mitigations so the reaper can't reclaim a space that's actually in use:
- Content guard (the decisive one): stats now also returns has_content (any
message or thread, via O(1) EXISTS). A candidate is reaped only if it never
completed setup AND has <=1 member AND has zero content. The instant anyone
posts anything, the space is protected no matter what setup_complete says —
closing the sharp edge of a solo space whose owner uses it but never finished
the setup conversation.
- Per-sweep cap: LCHAT_CLOUD_REAP_MAX_PER_SWEEP (default 5) bounds how many one
sweep reclaims; the rest defer to the next. Caps the blast radius of any future
mis-classification to a handful of recoverable archives before it's noticed.
(These layer on the existing defenses: ledger-scoped so operator instances can
never be candidates; archive-not-destroy; skip-on-error; 14-day age.)
instanceStatus + GET /instances/{host}/stats now return has_content; reapOnce
gates on it and honours reapMax. Tests: keeps-a-space-with-content,
caps-per-sweep (5 candidates, cap 2 → 2 reaped, 3 deferred), instanceStatus
has_content true/false. go test ./... + vet green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
5 files changed,
+144
−45
cloud.go+3 −0controld.go+4 −4docs/cloud-provisioner.md+19 −8reaper.go+37 −21reaper_test.go+81 −12
diff --git a/cloud.go b/cloud.go| index 4a8d0b8..340e66d 100644 |
| --- a/cloud.go |
| +++ b/cloud.go |
| @@ -80,6 +80,7 @@ type cloudServer struct { |
| quota int // max spaces per (caller, requester) |
| reapAfter time.Duration // reclaim never-claimed spaces older than this (0 = reaper off) |
| reapDryRun bool // log what the reaper would reclaim, delete nothing |
| + reapMax int // max reclaims per sweep (blast-radius cap; 0 = unlimited) |
| } |
| func (s *cloudServer) handler() http.Handler { |
| @@ -367,6 +368,7 @@ func runCloud(args []string) { |
| reapAfterDays := fs.Int("reap-after-days", envInt("LCHAT_CLOUD_REAP_AFTER_DAYS", 0), "reclaim never-claimed (zero-member) spaces older than this many days; 0 disables the reaper") |
| reapIntervalHours := fs.Int("reap-interval-hours", envInt("LCHAT_CLOUD_REAP_INTERVAL_HOURS", 6), "how often the reaper sweeps") |
| reapDryRun := fs.Bool("reap-dry-run", os.Getenv("LCHAT_CLOUD_REAP_DRY_RUN") != "", "log what the reaper would reclaim without deleting anything") |
| + reapMax := fs.Int("reap-max-per-sweep", envInt("LCHAT_CLOUD_REAP_MAX_PER_SWEEP", 5), "cap reclaims per reaper sweep (blast-radius bound; 0 = unlimited)") |
| _ = fs.Parse(args) |
| if *baseDomain == "" || !validHost("x."+*baseDomain) { |
| @@ -394,6 +396,7 @@ func runCloud(args []string) { |
| quota: *quota, |
| reapAfter: time.Duration(*reapAfterDays) * 24 * time.Hour, |
| reapDryRun: *reapDryRun, |
| + reapMax: *reapMax, |
| } |
| go s.reapLoop(time.Duration(*reapIntervalHours) * time.Hour) |
| ln, where, err := listen(*addr, *socketPath) |
diff --git a/controld.go b/controld.go| index f4d812d..76e7a90 100644 |
| --- a/controld.go |
| +++ b/controld.go |
| @@ -89,8 +89,8 @@ func (c *controld) handler() http.Handler { |
| return mux |
| } |
| -// statsHandler reports whether a space was ever set up (setup_complete) plus its |
| -// member count — the reaper's "never claimed" signal. Read-only against the live |
| +// statsHandler reports the reaper's guards for a space — setup_complete, member |
| +// count, and has_content (any message or thread). Read-only against the live |
| // instance DB. validHost guards the DB path against traversal before it's joined |
| // with the data dir. |
| func (c *controld) statsHandler(w http.ResponseWriter, r *http.Request) { |
| @@ -99,7 +99,7 @@ func (c *controld) statsHandler(w http.ResponseWriter, r *http.Request) { |
| writeControlErr(w, http.StatusBadRequest, "invalid_host", "not a valid hostname") |
| return |
| } |
| - members, setupComplete, err := instanceStatus(host) |
| + members, setupComplete, hasContent, err := instanceStatus(host) |
| if err != nil { |
| if os.IsNotExist(err) { |
| writeControlErr(w, http.StatusNotFound, "unknown_host", "no such instance") |
| @@ -109,7 +109,7 @@ func (c *controld) statsHandler(w http.ResponseWriter, r *http.Request) { |
| writeControlErr(w, http.StatusInternalServerError, "stats_failed", err.Error()) |
| return |
| } |
| - writeJSON(w, http.StatusOK, map[string]any{"host": host, "members": members, "setup_complete": setupComplete}) |
| + writeJSON(w, http.StatusOK, map[string]any{"host": host, "members": members, "setup_complete": setupComplete, "has_content": hasContent}) |
| } |
| // deleteHandler deprovisions one instance (self-serve space deletion, reached |
diff --git a/docs/cloud-provisioner.md b/docs/cloud-provisioner.md| index ef43a07..c820d75 100644 |
| --- a/docs/cloud-provisioner.md |
| +++ b/docs/cloud-provisioner.md |
| @@ -206,17 +206,28 @@ would otherwise sit there forever: empty, but holding a slot and a disk. The |
| disables it**; a value comfortably past the 7-day invite expiry, e.g. 14, is |
| the intent). |
| - For each candidate it asks the control daemon `GET /instances/{host}/stats`, |
| - which reads the instance DB **read-only** and returns `setup_complete` + member |
| - count. The "never claimed" signal is **`setup_complete`, not member count**: |
| - `add-instance` pre-seeds the admin user so the setup invite has an identity, and |
| - claiming *becomes* that user — so a space has exactly one member whether or not |
| - it was ever set up. A candidate is reaped only if it **never completed setup** |
| - (`setup_complete` absent) AND no one beyond the seeded admin joined |
| - (`members <= 1`). **Anything set up, in use, or whose status can't be read is |
| - left untouched** — a stats error is always a skip, never a delete. |
| + which reads the instance DB **read-only** and returns `setup_complete`, member |
| + count, and `has_content`. A candidate is reaped only when **all three guards** |
| + agree it's an abandoned shell: |
| + 1. **`setup_complete` absent** — the admin never finished the setup conversation. |
| + (The signal is `setup_complete`, **not** member count: `add-instance` |
| + pre-seeds the admin user so the setup invite has an identity, and claiming |
| + *becomes* that user — so a space has exactly one member whether or not it was |
| + ever set up.) |
| + 2. **`members <= 1`** — nobody beyond the seeded admin joined. |
| + 3. **`has_content` false** — zero messages and zero threads. The instant anyone |
| + posts anything, the space is protected regardless of the other signals; this |
| + is the decisive guard against a false positive on a used-but-not-marked space. |
| + |
| + **Anything set up, joined, with content, or whose status can't be read is left |
| + untouched** — a stats error is always a skip, never a delete. |
| - A reaped candidate is deprovisioned through the normal controld delete, which |
| **archives** the DB/env to `/data/trash` (never destroys), so a false positive |
| is recoverable. Its ledger row is cleared, freeing the quota slot. |
| +- **Per-sweep cap:** `LCHAT_CLOUD_REAP_MAX_PER_SWEEP` (default 5) bounds how many a |
| + single sweep reclaims; the rest defer to the next sweep. This caps the blast |
| + radius of any future mis-classification to a handful of recoverable archives |
| + before an operator would notice. |
| - `LCHAT_CLOUD_REAP_DRY_RUN=1` logs exactly what it *would* reclaim without |
| deleting — run it once before arming the real sweep. |
diff --git a/reaper.go b/reaper.go| index 3a246eb..f03205e 100644 |
| --- a/reaper.go |
| +++ b/reaper.go |
| @@ -41,29 +41,37 @@ import ( |
| // The real signal is `setup_complete`: absent until the admin finishes the setup |
| // conversation (markSetupComplete), then "1". members is returned too, as a |
| // belt-and-suspenders guard (don't reap if anyone beyond the seeded admin joined). |
| -func instanceStatus(host string) (members int, setupComplete bool, err error) { |
| +func instanceStatus(host string) (members int, setupComplete, hasContent bool, err error) { |
| path := filepath.Join(instanceDataDir, host+".db") |
| if _, err := os.Stat(path); err != nil { |
| - return 0, false, err // not-exist propagates → the caller answers unknown_host |
| + return 0, false, false, err // not-exist propagates → the caller answers unknown_host |
| } |
| conn, err := sql.Open("sqlite", "file:"+path+"?mode=ro&_pragma=busy_timeout(3000)") |
| if err != nil { |
| - return 0, false, err |
| + return 0, false, false, err |
| } |
| defer conn.Close() |
| conn.SetMaxOpenConns(1) |
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| defer cancel() |
| if err := conn.QueryRowContext(ctx, "SELECT COUNT(*) FROM users").Scan(&members); err != nil { |
| - return 0, false, err |
| + return 0, false, false, err |
| } |
| var v string |
| switch err := conn.QueryRowContext(ctx, "SELECT value FROM config WHERE key='setup_complete'").Scan(&v); err { |
| case nil, sql.ErrNoRows: // absent = never set up |
| default: |
| - return 0, false, err |
| + return 0, false, false, err |
| } |
| - return members, v == "1", nil |
| + // hasContent guards against a false positive: any message or thread means the |
| + // space was actually used, so it's never reaped no matter what setup_complete |
| + // says. EXISTS short-circuits, so this stays O(1) even on a large DB. |
| + var hasMsg, hasThread int |
| + if err := conn.QueryRowContext(ctx, |
| + "SELECT EXISTS(SELECT 1 FROM messages), EXISTS(SELECT 1 FROM threads)").Scan(&hasMsg, &hasThread); err != nil { |
| + return 0, false, false, err |
| + } |
| + return members, v == "1", hasMsg == 1 || hasThread == 1, nil |
| } |
| // ---- trash purge (controld-side, root) ---------------------------------------- |
| @@ -129,30 +137,31 @@ func runTrashPurge(keep time.Duration) { |
| // ---- the reaper (cloud-side) --------------------------------------------------- |
| -// controlStatus asks the control daemon whether a space was ever set up (and its |
| -// member count, as a guard). |
| -func (s *cloudServer) controlStatus(host string) (members int, setupComplete bool, err error) { |
| +// controlStatus asks the control daemon whether a space was ever set up, plus |
| +// the member count and whether it has any content — the reaper's guards. |
| +func (s *cloudServer) controlStatus(host string) (members int, setupComplete, hasContent bool, err error) { |
| req, err := http.NewRequest("GET", "http://control/instances/"+host+"/stats", nil) |
| if err != nil { |
| - return 0, false, err |
| + return 0, false, false, err |
| } |
| req.Header.Set("Authorization", "Bearer "+s.controlToken) |
| resp, err := s.control.Do(req) |
| if err != nil { |
| - return 0, false, err |
| + return 0, false, false, err |
| } |
| defer resp.Body.Close() |
| if resp.StatusCode != http.StatusOK { |
| - return 0, false, fmt.Errorf("control stats %s: HTTP %d", host, resp.StatusCode) |
| + return 0, false, false, fmt.Errorf("control stats %s: HTTP %d", host, resp.StatusCode) |
| } |
| var body struct { |
| Members int `json:"members"` |
| SetupComplete bool `json:"setup_complete"` |
| + HasContent bool `json:"has_content"` |
| } |
| if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { |
| - return 0, false, err |
| + return 0, false, false, err |
| } |
| - return body.Members, body.SetupComplete, nil |
| + return body.Members, body.SetupComplete, body.HasContent, nil |
| } |
| // reapLoop sweeps every interval; enabled only when reapAfter>0. |
| @@ -172,10 +181,13 @@ func (s *cloudServer) reapLoop(interval time.Duration) { |
| } |
| // reapOnce reclaims never-claimed spaces older than reapAfter. It deletes only a |
| -// space the control daemon confirms was NEVER set up (setup_complete absent) AND |
| -// that nobody beyond the pre-seeded admin joined (members <= 1); anything set up, |
| -// in use, or whose status can't be read is left untouched (a stats error is a |
| -// skip, never a reap). Returns counts. |
| +// space the control daemon confirms was NEVER set up (setup_complete absent), that |
| +// nobody beyond the pre-seeded admin joined (members <= 1), AND that has zero |
| +// content (no messages or threads) — three independent guards, so a space with any |
| +// real use survives all of them. Anything set up / in use / with content / whose |
| +// status can't be read is left untouched (a stats error is a skip, never a reap). |
| +// A per-sweep cap (reapMax) bounds the blast radius of any future mis-classification |
| +// to a handful of recoverable archives before an operator would notice. Returns counts. |
| func (s *cloudServer) reapOnce(now time.Time) (reaped, kept int) { |
| cutoff := now.Add(-s.reapAfter).Unix() |
| cands, err := s.db.spacesOlderThan(cutoff) |
| @@ -184,20 +196,24 @@ func (s *cloudServer) reapOnce(now time.Time) (reaped, kept int) { |
| return 0, 0 |
| } |
| for _, c := range cands { |
| - members, setupComplete, err := s.controlStatus(c.Host) |
| + members, setupComplete, hasContent, err := s.controlStatus(c.Host) |
| if err != nil { |
| log.Printf("cloud: reaper: skip %s (stats unavailable): %v", c.Host, err) |
| continue |
| } |
| - if setupComplete || members > 1 { |
| + if setupComplete || members > 1 || hasContent { |
| kept++ |
| - continue // set up, or someone else joined — never reap |
| + continue // set up, someone else joined, or has content — never reap |
| } |
| created := time.Unix(c.CreatedAt, 0).UTC().Format(time.RFC3339) |
| if s.reapDryRun { |
| log.Printf("cloud: reaper: WOULD reclaim %s (id %d, never set up, created %s)", c.Host, c.ID, created) |
| continue |
| } |
| + if s.reapMax > 0 && reaped >= s.reapMax { |
| + log.Printf("cloud: reaper: per-sweep cap (%d) reached — deferring the rest to the next sweep", s.reapMax) |
| + break |
| + } |
| status, body, err := s.controlDelete(c.Host) |
| if err != nil { |
| log.Printf("cloud: reaper: delete %s failed: %v", c.Host, err) |
diff --git a/reaper_test.go b/reaper_test.go| index 6a41852..168c201 100644 |
| --- a/reaper_test.go |
| +++ b/reaper_test.go |
| @@ -102,6 +102,7 @@ func TestReapOnceReclaimsOnlyOldEmpties(t *testing.T) { |
| sock := reapControl(t, |
| map[string]int{"unclaimed.11ch.at": 1, "used.11ch.at": 1}, |
| map[string]bool{"used.11ch.at": true}, // only used.* completed setup |
| + nil, // neither has content |
| deleted, nil) |
| s := &cloudServer{db: db, control: controlClient(sock), controlToken: "t", reapAfter: 14 * 24 * time.Hour} |
| @@ -124,6 +125,57 @@ func TestReapOnceReclaimsOnlyOldEmpties(t *testing.T) { |
| } |
| } |
| +func TestReapOnceKeepsSpaceWithContent(t *testing.T) { |
| + db := openTestCloudDB(t) |
| + now := time.Unix(2_000_000_000, 0) |
| + old := now.Add(-20 * 24 * time.Hour).Unix() |
| + id := insertSpaceRow(t, db, "hascontent.11ch.at", old) // old + never set up + 1 member, BUT has content |
| + |
| + deleted := map[string]bool{} |
| + sock := reapControl(t, |
| + map[string]int{"hascontent.11ch.at": 1}, |
| + nil, // never set up |
| + map[string]bool{"hascontent.11ch.at": true}, // ...but has a message/thread |
| + deleted, nil) |
| + s := &cloudServer{db: db, control: controlClient(sock), controlToken: "t", reapAfter: 14 * 24 * time.Hour} |
| + |
| + reaped, kept := s.reapOnce(now) |
| + if reaped != 0 || kept != 1 { |
| + t.Fatalf("reaped=%d kept=%d, want 0/1 (content protects it)", reaped, kept) |
| + } |
| + if deleted["hascontent.11ch.at"] { |
| + t.Error("a space with content must NEVER be reaped") |
| + } |
| + if _, ok, _ := db.spaceOwnedByID(t, id); !ok { |
| + t.Error("a space with content should remain in the ledger") |
| + } |
| +} |
| + |
| +func TestReapOnceCapsPerSweep(t *testing.T) { |
| + db := openTestCloudDB(t) |
| + now := time.Unix(2_000_000_000, 0) |
| + old := now.Add(-20 * 24 * time.Hour).Unix() |
| + members := map[string]int{} |
| + for i := 0; i < 5; i++ { // 5 reapable candidates |
| + h := "dead" + strconv.Itoa(i) + ".11ch.at" |
| + insertSpaceRow(t, db, h, old) |
| + members[h] = 1 // all never-set-up, 1 member, no content |
| + } |
| + |
| + deleted := map[string]bool{} |
| + sock := reapControl(t, members, nil, nil, deleted, nil) |
| + s := &cloudServer{db: db, control: controlClient(sock), controlToken: "t", reapAfter: 14 * 24 * time.Hour, reapMax: 2} |
| + |
| + reaped, _ := s.reapOnce(now) |
| + if reaped != 2 || len(deleted) != 2 { |
| + t.Fatalf("reaped=%d deleted=%d, want 2/2 (per-sweep cap)", reaped, len(deleted)) |
| + } |
| + // the other 3 survive this sweep, to be handled by the next one |
| + if remaining, _ := db.spacesOlderThan(now.Unix()); len(remaining) != 3 { |
| + t.Errorf("remaining candidates = %d, want 3 (deferred, not lost)", len(remaining)) |
| + } |
| +} |
| + |
| func TestReapOnceDryRunDeletesNothing(t *testing.T) { |
| db := openTestCloudDB(t) |
| now := time.Unix(2_000_000_000, 0) |
| @@ -131,7 +183,7 @@ func TestReapOnceDryRunDeletesNothing(t *testing.T) { |
| insertSpaceRow(t, db, "unclaimed.11ch.at", old) |
| deleted := map[string]bool{} |
| - sock := reapControl(t, map[string]int{"unclaimed.11ch.at": 1}, nil, deleted, nil) |
| + sock := reapControl(t, map[string]int{"unclaimed.11ch.at": 1}, nil, nil, deleted, nil) |
| s := &cloudServer{db: db, control: controlClient(sock), controlToken: "t", reapAfter: 14 * 24 * time.Hour, reapDryRun: true} |
| reaped, _ := s.reapOnce(now) |
| @@ -151,7 +203,7 @@ func TestReapOnceSkipsWhenStatsUnavailable(t *testing.T) { |
| deleted := map[string]bool{} |
| // stats endpoint errors for this host → the reaper must skip, never delete. |
| - sock := reapControl(t, map[string]int{}, nil, deleted, map[string]bool{"flaky.11ch.at": true}) |
| + sock := reapControl(t, map[string]int{}, nil, nil, deleted, map[string]bool{"flaky.11ch.at": true}) |
| s := &cloudServer{db: db, control: controlClient(sock), controlToken: "t", reapAfter: 14 * 24 * time.Hour} |
| reaped, kept := s.reapOnce(now) |
| @@ -192,18 +244,34 @@ func TestInstanceStatusAndStatsHandler(t *testing.T) { |
| if _, err := fresh.sql.Exec(`INSERT INTO users(id, created_at) VALUES('admin',1)`); err != nil { |
| t.Fatal(err) |
| } |
| + // A never-set-up space that nonetheless HAS content (a stray thread) — the |
| + // content guard must protect it even though setup_complete is unset. |
| + withMsg, err := openDB(filepath.Join(dir, "hascontent.11ch.at.db")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer withMsg.sql.Close() |
| + if _, err := withMsg.sql.Exec(`INSERT INTO users(id, created_at) VALUES('admin',1)`); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := withMsg.sql.Exec(`INSERT INTO threads(id, kind, created_at) VALUES('t1','group',1)`); err != nil { |
| + t.Fatal(err) |
| + } |
| - if m, sc, err := instanceStatus("used.11ch.at"); err != nil || m != 1 || !sc { |
| - t.Errorf("instanceStatus(used) = (%d, %v, %v); want (1, true, nil)", m, sc, err) |
| + if m, sc, hc, err := instanceStatus("used.11ch.at"); err != nil || m != 1 || !sc || hc { |
| + t.Errorf("instanceStatus(used) = (%d, %v, %v, %v); want (1, true, false, nil)", m, sc, hc, err) |
| + } |
| + if m, sc, hc, err := instanceStatus("fresh.11ch.at"); err != nil || m != 1 || sc || hc { |
| + t.Errorf("instanceStatus(fresh) = (%d, %v, %v, %v); want (1, false, false, nil)", m, sc, hc, err) |
| } |
| - if m, sc, err := instanceStatus("fresh.11ch.at"); err != nil || m != 1 || sc { |
| - t.Errorf("instanceStatus(fresh) = (%d, %v, %v); want (1, false, nil)", m, sc, err) |
| + if m, sc, hc, err := instanceStatus("hascontent.11ch.at"); err != nil || m != 1 || sc || !hc { |
| + t.Errorf("instanceStatus(hascontent) = (%d, %v, %v, %v); want (1, false, true, nil)", m, sc, hc, err) |
| } |
| - if _, _, err := instanceStatus("missing.11ch.at"); !os.IsNotExist(err) { |
| + if _, _, _, err := instanceStatus("missing.11ch.at"); !os.IsNotExist(err) { |
| t.Errorf("missing instance should be not-exist, got %v", err) |
| } |
| - // The stats endpoint, routed + auth-gated, reports setup_complete. |
| + // The stats endpoint, routed + auth-gated, reports the signals. |
| c := &controld{tokenHash: hashToken("secret")} |
| srv := httptest.NewServer(c.handler()) |
| defer srv.Close() |
| @@ -220,10 +288,11 @@ func TestInstanceStatusAndStatsHandler(t *testing.T) { |
| var body struct { |
| Members int `json:"members"` |
| SetupComplete bool `json:"setup_complete"` |
| + HasContent bool `json:"has_content"` |
| } |
| _ = json.NewDecoder(resp.Body).Decode(&body) |
| - if body.Members != 1 || body.SetupComplete { |
| - t.Errorf("stats = %+v, want members 1, setup_complete false", body) |
| + if body.Members != 1 || body.SetupComplete || body.HasContent { |
| + t.Errorf("stats = %+v, want members 1, setup_complete false, has_content false", body) |
| } |
| // No token → 401. |
| @@ -275,7 +344,7 @@ func (db *cloudDB) spaceOwnedByID(t *testing.T, id int64) (string, bool, error) |
| // fakeControl helper) answering the reaper's stats + delete calls: GET stats |
| // returns member count + setup_complete (or 500 when statsErr[host]); DELETE |
| // records the host. |
| -func reapControl(t *testing.T, members map[string]int, setup, deleted, statsErr map[string]bool) string { |
| +func reapControl(t *testing.T, members map[string]int, setup, content, deleted, statsErr map[string]bool) string { |
| t.Helper() |
| mux := http.NewServeMux() |
| mux.HandleFunc("GET /instances/{host}/stats", func(w http.ResponseWriter, r *http.Request) { |
| @@ -284,7 +353,7 @@ func reapControl(t *testing.T, members map[string]int, setup, deleted, statsErr |
| w.WriteHeader(http.StatusInternalServerError) |
| return |
| } |
| - writeJSON(w, http.StatusOK, map[string]any{"members": members[h], "setup_complete": setup[h]}) |
| + writeJSON(w, http.StatusOK, map[string]any{"members": members[h], "setup_complete": setup[h], "has_content": content[h]}) |
| }) |
| mux.HandleFunc("DELETE /instances/{host}", func(w http.ResponseWriter, r *http.Request) { |
| deleted[r.PathValue("host")] = true |