amadan
console: check purge preconditions before tearing the store down
release.PurgeReady extracts PurgeApp's own precondition block (trash marker, release key, production flag) so the console's purge door can ask the same question BEFORE it destroys an object store — a refusal that arrived after the teardown had already destroyed a live app's bucket, credentials and delivered config vars, with no serial bump behind it. PurgeApp calls the same function, so the two lists cannot drift. Teardown walks also get their own 60s ceiling (storeTeardownTimeout) in both doors: EmptyBucket is paginated, and storeTimeout's 15s is documented for a single round trip.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 files changed,
+208
−20
internal/console/apistore.go+11 −0internal/console/appsettings.go+6 −1internal/console/deletion_test.go+112 −0internal/console/ownership.go+26 −1internal/release/trash.go+53 −18
diff --git a/internal/console/apistore.go b/internal/console/apistore.go| index ddcee5d89..2581750f1 100644 |
| --- a/internal/console/apistore.go |
| +++ b/internal/console/apistore.go |
| @@ -342,6 +342,17 @@ const storeGrantTimeout = 60 * time.Second |
| // grant but still well past the 15s a single object write gets. |
| const storeRotateTimeout = 30 * time.Second |
| +// storeTeardownTimeout bounds a Teardown walk, wherever it is driven from (the |
| +// browser destroy door and the purge door, appsettings.go and ownership.go). |
| +// A grant's ceiling for a walk with a grant's shape and one step a grant does |
| +// not have: EmptyBucket is PAGINATED precisely because a real bucket does not |
| +// empty in one call, so a customer's store can be many round trips before the |
| +// bucket, the principal and the config write even begin. Under storeTimeout's |
| +// 15s — a ceiling documented for a SINGLE round trip — the deadline would land |
| +// mid-walk and be reported as a backend that could not be reached, which is |
| +// the wrong thing to tell someone whose store is simply large. |
| +const storeTeardownTimeout = 60 * time.Second |
| + |
| // storeOperator reports whether this address may drive the grant door. |
| // |
| // EXACT MATCH ON THE CANONICAL FORM, never a fold. Every address entry point |
diff --git a/internal/console/appsettings.go b/internal/console/appsettings.go| index b0b941bdb..8b0fe44b4 100644 |
| --- a/internal/console/appsettings.go |
| +++ b/internal/console/appsettings.go |
| @@ -472,7 +472,12 @@ func (s *Server) handleStoreDestroy(w http.ResponseWriter, r *http.Request) { |
| fail("No deployment bucket on this host, so this console cannot read the store record — nothing was destroyed.") |
| return |
| } |
| - ctx, cancel := context.WithTimeout(r.Context(), storeTimeout) |
| + // storeTeardownTimeout, not storeTimeout: the walk below is many round trips |
| + // (EmptyBucket is paginated for exactly the bucket too big to clear in one |
| + // call), and a 15s ceiling documented for a single round trip would cut a |
| + // large store's teardown off mid-walk and report it as an unreachable |
| + // backend. |
| + ctx, cancel := context.WithTimeout(r.Context(), storeTeardownTimeout) |
| defer cancel() |
| rec, _, err := objstore.Get(ctx, s.cfg.Releases, acct.Sqid, app) |
| if err != nil { |
diff --git a/internal/console/deletion_test.go b/internal/console/deletion_test.go| index 189619416..2bb37f40f 100644 |
| --- a/internal/console/deletion_test.go |
| +++ b/internal/console/deletion_test.go |
| @@ -10,6 +10,7 @@ import ( |
| "testing" |
| "time" |
| + "github.com/carlosframework/platform/internal/appconfig" |
| "github.com/carlosframework/platform/internal/domains" |
| "github.com/carlosframework/platform/internal/objstore" |
| "github.com/carlosframework/platform/internal/release" |
| @@ -498,11 +499,46 @@ func seedStore(t *testing.T, fx *consoleFixture, fake *objstore.MemProvisioner, |
| if !crashed { |
| rec.Provisioned = &objstore.Provisioned{Bucket: bucket, Region: "eu-west-1", IAMUser: user, |
| UserARN: arn, KeyID: keyID, At: now} |
| + // The delivered half: the coordinates a grant writes into the app's |
| + // config. Teardown unsets these, so their presence is what "nothing was |
| + // destroyed" means on the config side, and their absence is half of what |
| + // a completed teardown means. |
| + bucketName, regionName, _, _, _ := objstore.EnvNames(objstore.DefaultEnvPrefix) |
| + if _, err := appconfig.Apply(ctx, fx.rel, fx.sqid, app, map[string]appconfig.Var{ |
| + bucketName: {Kind: "env", Value: bucket}, |
| + regionName: {Kind: "env", Value: "eu-west-1"}, |
| + }, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| } |
| putRecord(t, fx, app, rec) |
| return bucket |
| } |
| +// storeVarsDelivered reports whether the app's config still holds the store |
| +// coordinates seedStore delivered. |
| +func storeVarsDelivered(t *testing.T, fx *consoleFixture, app string) bool { |
| + t.Helper() |
| + bucketName, _, _, _, _ := objstore.EnvNames(objstore.DefaultEnvPrefix) |
| + _, ok := appVars(t, fx, app)[bucketName] |
| + return ok |
| +} |
| + |
| +// noStoreDestroyAudit asserts nothing claimed to have destroyed a store — the |
| +// trail a refusal that ran too late would leave. |
| +func noStoreDestroyAudit(t *testing.T, fx *consoleFixture, app string) { |
| + t.Helper() |
| + events, err := fx.st.Events(OpsAccountID, app, 20) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, e := range events { |
| + if e.Action == "store-destroy" { |
| + t.Fatalf("a refused purge audited a store teardown: %+v", e) |
| + } |
| + } |
| +} |
| + |
| // estateStanding asserts the backend still holds everything seedStore built — |
| // the yardstick for "a refusal destroyed nothing". |
| func estateStanding(t *testing.T, fx *consoleFixture, fake *objstore.MemProvisioner, app, bucket, when string) { |
| @@ -610,6 +646,82 @@ func TestPurgeTearsDownACrashedGrant(t *testing.T) { |
| auditedBy(t, fx, "kass", "store-destroy", "paul@keymail.dev", "carlos-bab-kass") |
| } |
| +// The production flag outranks deletion all the way down — and that promise is |
| +// only kept if it is checked BEFORE the store comes down. A refusal that |
| +// arrives after the teardown has already destroyed a LIVE app's bucket, its |
| +// credentials and the variables its hosts are running on, and purge announces |
| +// nothing, so those hosts would keep serving dead credentials until the hourly |
| +// floor. |
| +func TestPurgeRefusesAProductionAppBeforeTouchingItsStore(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + ctx := context.Background() |
| + // Flagged BEFORE the trash, bucket-direct — the state only a flag set after |
| + // the fact can reach, which is exactly the one this ordering protects. |
| + if err := release.SetProduction(ctx, fx.rel, fx.priv, release.OpsAccount, "kass", true, "test"); err != nil { |
| + t.Fatal(err) |
| + } |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-purge-prod-store") |
| + trashKass(t, fx, "paul@keymail.dev", time.Now()) |
| + csrf := sessionToken(t, fx, c) |
| + fake := armStoreBackend(t, fx) |
| + bucket := seedStore(t, fx, fake, "kass", false) |
| + |
| + status, loc, body := postGated(t, fx, c, fx.u("/trash/delete"), |
| + url.Values{"csrf": {csrf}, "app": {"kass"}, "confirm": {"kass"}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("purge of a production app with a store = %d:\n%s", status, body) |
| + } |
| + estateStanding(t, fx, fake, "kass", bucket, "a purge refused for production") |
| + if rec := storedRecord(t, fx, "kass"); rec == nil || rec.Provisioned == nil { |
| + t.Fatalf("the store record was destroyed by a refused purge: %+v", rec) |
| + } |
| + if !storeVarsDelivered(t, fx, "kass") { |
| + t.Fatal("the delivered store variables were unset by a refused purge") |
| + } |
| + noStoreDestroyAudit(t, fx, "kass") |
| + if has, err := fx.st.HasTrashedAppIn(OpsAccountID, "kass"); err != nil || !has { |
| + t.Fatalf("HasTrashedAppIn = %v %v, want the row still trashed", has, err) |
| + } |
| + if _, _, page := get(t, c, fx.ts.URL+loc); !strings.Contains(page, "the off-switch on the app page") { |
| + t.Fatalf("the refusal does not name the off-switch:\n%s", page) |
| + } |
| +} |
| + |
| +// "A restore that landed while this form sat open wins" — handlePurgeApp's own |
| +// promise, which is worth nothing if the store is already gone by the time the |
| +// marker is re-read. The app here is LIVE again: destroying its store would be |
| +// destroying a running app's data. |
| +func TestPurgeRefusesARestoredAppBeforeTouchingItsStore(t *testing.T) { |
| + fx, c, csrf := trashPageFixture(t) |
| + fake := armStoreBackend(t, fx) |
| + bucket := seedStore(t, fx, fake, "kass", false) |
| + // The restore lands while the "Delete now" form sits open. |
| + if err := release.RestoreApp(context.Background(), fx.rel, fx.sqid, "kass"); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + status, loc, body := postGated(t, fx, c, fx.u("/trash/delete"), |
| + url.Values{"csrf": {csrf}, "app": {"kass"}, "confirm": {"kass"}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("purge of a restored app with a store = %d:\n%s", status, body) |
| + } |
| + estateStanding(t, fx, fake, "kass", bucket, "a purge refused for a restore") |
| + if rec := storedRecord(t, fx, "kass"); rec == nil || rec.Provisioned == nil { |
| + t.Fatalf("the store record was destroyed by a refused purge: %+v", rec) |
| + } |
| + if !storeVarsDelivered(t, fx, "kass") { |
| + t.Fatal("the delivered store variables were unset by a refused purge") |
| + } |
| + noStoreDestroyAudit(t, fx, "kass") |
| + if len(appKeys(t, fx, "kass")) == 0 { |
| + t.Fatal("the restored app's deployment objects were swept") |
| + } |
| + if _, _, page := get(t, c, fx.ts.URL+loc); !strings.Contains(page, "not in the trash") { |
| + t.Fatalf("the refusal does not say the app is out of the trash:\n%s", page) |
| + } |
| +} |
| + |
| // purgeAdvice must be honest in BOTH directions: the guard wraps |
| // ErrStoreProvisioned for an unreadable store.json too, so this sentence |
| // cannot claim a store certainly exists — and it must still name the way out. |
diff --git a/internal/console/ownership.go b/internal/console/ownership.go| index 19d0c6d83..353ad31e8 100644 |
| --- a/internal/console/ownership.go |
| +++ b/internal/console/ownership.go |
| @@ -319,6 +319,26 @@ func (s *Server) handlePurgeApp(w http.ResponseWriter, r *http.Request) { |
| fail("There is no deployment bucket on this host, so nothing can be deleted here.") |
| return |
| } |
| + // PurgeApp's OWN preconditions, asked BEFORE anything is destroyed. The |
| + // teardown below is irreversible and reaches outside the deployment bucket |
| + // — a customer's objects, their IAM principal, the credentials their live |
| + // hosts are holding — so it must not run in front of a purge that PurgeApp |
| + // would then refuse: an app restored while this form sat open, a production |
| + // flag set after trashing, an unreadable flag, a console with no release |
| + // key. Without this, every one of those refusals would arrive AFTER the |
| + // store was destroyed, on an app that is still alive and (purge deliberately |
| + // not bumping the serial) whose boxes would keep serving dead credentials |
| + // until the hourly floor. |
| + // |
| + // One list, not two: PurgeReady is the block PurgeApp itself runs, so the |
| + // two cannot drift. PurgeApp re-runs it immediately before destroying |
| + // anything, which keeps the residual race to the one round trip between |
| + // here and there — the design's stated tolerance. |
| + if err := release.PurgeReady(r.Context(), s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app); err != nil { |
| + log.Printf("console: purge preflight %s/%s: %v", acct.Sqid, app, err) |
| + fail(purgeAdvice(err, app)) |
| + return |
| + } |
| // A provisioned store is torn down BEFORE the prefix sweep — this door |
| // is the only purge path with backend authority (objstore spec §9; |
| // box-side auto-purge refuses via release.ErrStoreProvisioned). Failure |
| @@ -331,7 +351,12 @@ func (s *Server) handlePurgeApp(w http.ResponseWriter, r *http.Request) { |
| fail("“" + app + "” has a provisioned object store, and this console has no storage backend configured (CARLOS_STORE_BACKEND) — it cannot be deleted here until the store is torn down.") |
| return |
| } |
| - if terr := objstore.Teardown(r.Context(), s.cfg.Releases, s.cfg.StoreProvisioner, acct.Sqid, app); terr != nil { |
| + // The walk gets its own ceiling rather than the request's open-ended |
| + // context: a teardown is many round trips (EmptyBucket is paginated), |
| + // and it must not be able to hold this handler open indefinitely. |
| + tctx, cancel := context.WithTimeout(r.Context(), storeTeardownTimeout) |
| + defer cancel() |
| + if terr := objstore.Teardown(tctx, s.cfg.Releases, s.cfg.StoreProvisioner, acct.Sqid, app); terr != nil { |
| log.Printf("console: purge store teardown %s/%s: %v", acct.Sqid, app, terr) |
| fail("Could not tear down “" + app + "”'s object store — nothing was deleted; try again.") |
| return |
diff --git a/internal/release/trash.go b/internal/release/trash.go| index 0ba27cb34..b802099de 100644 |
| --- a/internal/release/trash.go |
| +++ b/internal/release/trash.go |
| @@ -110,26 +110,31 @@ var ErrNotTrashed = errors.New("release: app is not in the trash") |
| // PurgeApp for why a box is the wrong actor to decide otherwise. |
| var ErrStoreProvisioned = errors.New("release: app has a provisioned object store — tear it down via the console before purging") |
| -// PurgeApp irreversibly destroys a trashed app's bucket objects: every key |
| -// under apps/<acct>/<app>/, the trash marker LAST, so a purge that dies |
| -// half-way leaves a trashed app with fewer objects rather than a live app |
| -// with holes — the next pass finishes the job. |
| +// PurgeReady answers "would PurgeApp destroy this app right now?" — the |
| +// preconditions and nothing else: a valid trash marker, a release public key to |
| +// prove production with, and a production flag that is readable and off. It is |
| +// PurgeApp's OWN precondition block, extracted so a caller that must act BEFORE |
| +// PurgeApp can ask the same question from the same list. |
| // |
| -// This is the platform's first destructive automation surface, so every |
| -// guard fails toward keeping data: |
| -// - no valid marker → ErrNotTrashed (re-read HERE, immediately before |
| -// destruction — a restore that landed after the caller's listing wins); |
| -// - nil pub, production flag ON, or production flag UNREADABLE → refuse. |
| -// Flag ABSENT is fine: most apps never had one, and GetProduction says so |
| -// with (nil, nil). |
| +// The caller it exists for is the console's purge door (console/ownership.go), |
| +// which tears an object store down before re-entering PurgeApp. That teardown |
| +// destroys a bucket, an IAM principal and the app's delivered credentials — all |
| +// of it outside the deployment bucket, and none of it undoable — so it must not |
| +// happen in front of a purge that PurgeApp then refuses: a restore that landed |
| +// while the form sat open, a production flag set after trashing, an unreadable |
| +// flag, a console with no public key. Asking here first keeps those refusals |
| +// exactly as strong as they read in PurgeApp's doc comment. |
| // |
| -// The AGE of the marker is deliberately the caller's decision, not this |
| -// function's: the auto-purge pass enforces TrashCooloff, and the console's |
| -// "Delete now" purges early behind its own ceremony. now is therefore taken |
| -// and not read — it is here so that every caller passes its OWN clock seam |
| -// and no future guard added inside this function has to reach for |
| -// time.Now() behind the tests' backs. |
| -func PurgeApp(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app string, now time.Time) error { |
| +// The store guard and the sweep are deliberately NOT here: the store guard is |
| +// the box's refusal to act without backend authority (the console door's whole |
| +// purpose is to have already dealt with it), and the sweep is the destruction |
| +// itself. |
| +// |
| +// This narrows the window; it does not close it. PurgeApp re-checks all of it |
| +// immediately before destroying, so the residual race is the one round trip |
| +// between the two calls — the design's stated tolerance, and the same shape |
| +// objstore.Teardown's final re-read settles for. |
| +func PurgeReady(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app string) error { |
| tomb, err := ReadAppTombstone(ctx, s, acct, app) |
| if err != nil { |
| return err |
| @@ -152,6 +157,36 @@ func PurgeApp(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app str |
| if flag != nil && flag.Production { |
| return fmt.Errorf("release: %s/%s is flagged production — a trashed production app should be impossible; refusing to purge", acct, app) |
| } |
| + return nil |
| +} |
| + |
| +// PurgeApp irreversibly destroys a trashed app's bucket objects: every key |
| +// under apps/<acct>/<app>/, the trash marker LAST, so a purge that dies |
| +// half-way leaves a trashed app with fewer objects rather than a live app |
| +// with holes — the next pass finishes the job. |
| +// |
| +// This is the platform's first destructive automation surface, so every |
| +// guard fails toward keeping data: |
| +// - no valid marker → ErrNotTrashed (re-read HERE, immediately before |
| +// destruction — a restore that landed after the caller's listing wins); |
| +// - nil pub, production flag ON, or production flag UNREADABLE → refuse. |
| +// Flag ABSENT is fine: most apps never had one, and GetProduction says so |
| +// with (nil, nil). |
| +// |
| +// The AGE of the marker is deliberately the caller's decision, not this |
| +// function's: the auto-purge pass enforces TrashCooloff, and the console's |
| +// "Delete now" purges early behind its own ceremony. now is therefore taken |
| +// and not read — it is here so that every caller passes its OWN clock seam |
| +// and no future guard added inside this function has to reach for |
| +// time.Now() behind the tests' backs. |
| +func PurgeApp(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app string, now time.Time) error { |
| + // The preconditions live in PurgeReady so that the console's purge door, |
| + // which must satisfy them BEFORE it tears an object store down, checks the |
| + // same list this function does. One list, one place: a guard added there is |
| + // a guard both callers get, and neither can drift from the other. |
| + if err := PurgeReady(ctx, s, pub, acct, app); err != nil { |
| + return err |
| + } |
| prefix := appPrefix(acct, app) |
| // An object store with backend estate refuses the purge outright (objstore |
| // spec §9): auto-purge runs on BOXES, boxes hold no backend authority, and |