carlos / platform Public

fix(console): app convergence refuses an empty bucket — misconfiguration must not free names

Case (c) is the only branch that reasons from ABSENCE, and absence is
exactly what a mistyped CARLOS_DEPLOYMENT_BUCKET produces: a listing that
succeeds and returns nothing. Purging on that reads "everything was
purged everywhere" from what is really "I am looking in the wrong
place", and it is unrecoverable — the pass is row-driven, so once the
rows are gone the markers in the real bucket are invisible to it forever
and the names are free for anyone to claim.

So an apps/ listing with ZERO keys holds back every purge for the pass
and says so, naming the environment variable. Cases (a), (b) and (d) act
on presence and are unaffected. The accepted corner: a deployment whose
every app really was purged keeps its trashed names reserved until
anything ships again.

Also pins the resurrection refusal's log line. Deleting the corruption
arm entirely left the suite green — refusal fell through to "known
account, stop here" — silently losing both the explicit detection and
the only signal an operator gets that somebody rewrote a dead account's
manifest.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 938d638c83d96717045a933b8bf173b11673828e parent bfaf42d
2 files changed, +120 −0
  • internal/console/materialize.go +37 −0
  • internal/console/materialize_test.go +83 −0
diff --git a/internal/console/materialize.go b/internal/console/materialize.go
index a655f99dd..120adffbe 100644
--- a/internal/console/materialize.go
+++ b/internal/console/materialize.go
@@ -272,6 +272,10 @@ func materializeOwner(st *Store, key string, m accountManifest, created time.Tim
// whose sqid this console does not know contribute nothing to anything —
// another deployment's tenants are not this console's business.
//
+// Case (c) is the only one that reasons from absence, and it is held back
+// entirely when the apps/ listing comes back EMPTY — see the tripwire below,
+// which is what stops a mistyped bucket name from releasing every held name.
+//
// Best-effort like the rest of this file: a nil store or a failed listing is
// "nothing to catch up on", never a reason to stop the console.
func MaterializeApps(ctx context.Context, s release.Store, st *Store) error {
@@ -289,6 +293,36 @@ func MaterializeApps(ctx context.Context, s release.Store, st *Store) error {
if err != nil {
return err
}
+ // THE EMPTY-BUCKET TRIPWIRE. Case (c) reasons from ABSENCE, and absence is
+ // exactly what a mistyped CARLOS_DEPLOYMENT_BUCKET produces: a listing that
+ // succeeds and returns nothing. Without this, one wrong environment
+ // variable purges every trashed row on the first pass and frees every held
+ // name — unrecoverably, because this pass is row-driven and a marker with
+ // no row is invisible to it forever.
+ //
+ // So a completely empty apps/ listing is treated as no evidence rather
+ // than as evidence of destruction: cases (a), (b) and (d) act on PRESENCE
+ // and are unaffected, and only the purges are held back.
+ //
+ // The accepted corner: a deployment whose every app really was purged
+ // keeps its trashed names reserved until anything ships again. A
+ // reservation outliving its need is a small annoyance; names released
+ // because the console was pointed at the wrong bucket are gone.
+ purgesHeld := len(keys) == 0
+ if purgesHeld {
+ trashed := 0
+ for _, r := range rows {
+ if r.Deleted {
+ trashed++
+ }
+ }
+ if trashed == 0 {
+ purgesHeld = false // nothing to hold back; stay quiet
+ } else {
+ log.Printf("console: the apps/ listing is empty but %d trashed row(s) exist here — "+
+ "refusing to release their names; is CARLOS_DEPLOYMENT_BUCKET pointing at the right bucket?", trashed)
+ }
+ }
// One listing, indexed by app prefix. The prefixes come from the ROWS, so
// an unknown account's keys simply never find a home here.
type bucketState struct{ anyKeys, marker bool }
@@ -351,6 +385,9 @@ func MaterializeApps(ctx context.Context, s release.Store, st *Store) error {
// trashed here — so this frees the name rather than destroying
// anything. PurgeAppRow is guarded to refuse a live row even if
// this reasoning is ever wrong.
+ if purgesHeld {
+ continue // an empty listing is no evidence; see the tripwire above
+ }
if err := st.PurgeAppRow(r.AccountID, r.App); err != nil {
log.Printf("console: purge %s/%s: %v", r.Sqid, r.App, err)
continue
diff --git a/internal/console/materialize_test.go b/internal/console/materialize_test.go
index 845d9b3ff..223290ddb 100644
--- a/internal/console/materialize_test.go
+++ b/internal/console/materialize_test.go
@@ -1,7 +1,11 @@
package console
import (
+ "bytes"
"encoding/json"
+ "log"
+ "os"
+ "strings"
"testing"
"time"
@@ -483,6 +487,15 @@ func TestMaterializeRefusesResurrection(t *testing.T) {
// session — writes the manifest back, live, naming themselves.
rewriteManifest(t, rel, id, "eve@evil.example")
+ // The refusal must be EXPLICIT, not an accident of some later check. Delete
+ // the corruption arm and every assertion below still passes — refusal falls
+ // through to "known account, stop here" — while the operator loses the only
+ // signal that somebody rewrote a dead account's manifest. So the log line is
+ // part of the contract and is pinned here.
+ var logged bytes.Buffer
+ log.SetOutput(&logged)
+ t.Cleanup(func() { log.SetOutput(os.Stderr) })
+
// Every console start runs this pass, and so does every `console member`.
for i := 0; i < 2; i++ {
if n, err := MaterializeAccounts(ctx, rel, st); err != nil || n != 0 {
@@ -504,6 +517,11 @@ func TestMaterializeRefusesResurrection(t *testing.T) {
if members, _ := st.Members(id); len(members) != 0 {
t.Errorf("Members = %+v, want none", members)
}
+ if !strings.Contains(logged.String(), "refusing to resurrect") {
+ t.Errorf("nothing said a word about the rewritten manifest; log was:\n%s\n"+
+ "a manifest that came back to life over a tombstoned account is corruption, "+
+ "and an operator has to hear about it", logged.String())
+ }
}
// App-row convergence: the console's rows follow the bucket, because trash and
@@ -606,6 +624,71 @@ func TestMaterializeAppsConverges(t *testing.T) {
}
}
+// A MISTYPED BUCKET NAME MUST NOT FREE NAMES.
+//
+// Case (c) is the only branch that reasons from absence, and an empty listing
+// is exactly what CARLOS_DEPLOYMENT_BUCKET pointing one character wrong
+// produces: a listing that SUCCEEDS and returns nothing. Purging on that reads
+// "everything was purged everywhere" from what is really "I am looking in the
+// wrong place" — and it is unrecoverable, because this pass is row-driven: once
+// the rows are gone, the markers in the real bucket are invisible to it forever
+// and the names are free for anyone to claim.
+func TestMaterializeAppsRefusesAnEmptyBucket(t *testing.T) {
+ st := openTestStore(t)
+ ctx := t.Context()
+ now := time.Unix(1700000000, 0).UTC()
+
+ empty := release.NewDirStore(t.TempDir())
+ id, sqid, err := domains.MintAccount(ctx, empty, "Testco", "paul@keymail.dev", now)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if n, err := MaterializeAccounts(ctx, empty, st); err != nil || n != 1 {
+ t.Fatalf("materialized %d (%v), want the account", n, err)
+ }
+ if _, err := st.CreateAppIn(id, "gamma", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ if err := st.TrashAppRow(id, "gamma", now.Add(-48*time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+
+ // The wrong bucket: account manifests happen to be there, nothing under
+ // apps/ is. (A genuinely wrong bucket has neither; this store is the
+ // STRONGER test, since it still lists something.)
+ var logged bytes.Buffer
+ log.SetOutput(&logged)
+ t.Cleanup(func() { log.SetOutput(os.Stderr) })
+ if err := MaterializeApps(ctx, empty, st); err != nil {
+ t.Fatal(err)
+ }
+ if trashed, _ := st.HasTrashedAppIn(id, "gamma"); !trashed {
+ t.Error("an empty apps/ listing released a held name — one wrong environment variable " +
+ "is now enough to free every trashed app name in the deployment")
+ }
+ if accts, _ := st.AppAccounts("gamma"); len(accts) != 1 {
+ t.Errorf("AppAccounts(gamma) = %v, want the reservation still held", accts)
+ }
+ if !strings.Contains(logged.String(), "CARLOS_DEPLOYMENT_BUCKET") {
+ t.Errorf("the refusal was silent; log was:\n%s\nan operator has to be told why "+
+ "names are not being released", logged.String())
+ }
+
+ // And the tripwire is about EMPTINESS, not about this app: one unrelated
+ // key under apps/ is enough evidence that the bucket is the right bucket,
+ // and gamma — no marker, no keys of its own — purges as before.
+ if _, err := empty.Put(ctx, "apps/"+sqid+"/somethingelse/channels/stable.json", []byte(`{}`), ""); err != nil {
+ t.Fatal(err)
+ }
+ if err := MaterializeApps(ctx, empty, st); err != nil {
+ t.Fatal(err)
+ }
+ if accts, _ := st.AppAccounts("gamma"); len(accts) != 0 {
+ t.Errorf("AppAccounts(gamma) = %v after a non-empty listing, want the name freed — "+
+ "the tripwire must hold back purges only while there is no evidence at all", accts)
+ }
+}
+
// rewriteManifest overwrites a live account manifest with one naming owner —
// what a bucket-writer does, and the thing MintAccount's create-only Put does
// NOT prevent anyone else from doing. The etag is read and passed because the