carlos / platform Public

console: the flag guards store destruction too, and its refusals point somewhere

Review fix round 1.

handleStoreDestroy was a sixth destructive door the flag did not guard: owner,
CSRF, the typed BUCKET name and a passkey — the exact ceremony spec §6 rules
insufficient for the control plane — over an estate its own doc says has no
trash and no undo. Refused before the typed word and before the passkey. The
refusal is on DESTROYING a store, not on having one: a control-plane app may
hold one like any other app, and forbidding that would be an invented
constraint.

Four smaller ones, all in files this task already touches:

- The fail-closed branch is pinned. "A lookup failure must REFUSE" is a named
  project constraint and reads like harmless tidying to a later simplifier, so
  a test drops the control_plane COLUMN — surgical enough that auth and the app
  404 still work and only AppFlags fails — and asserts 503 on the API door and
  the flash on the browser one. Inverting the branch makes it fail with a 200.

- The two member tests no longer pass vacuously: an unregistered route answers
  a member exactly as the owner gate does, so both now prove an OWNER reaches
  the handler on the same URL. Deleting the routes fails them.

- The refusals name the card that can clear the flag, absolutely, because a CLI
  caller has no page in front of them. controlPlaneRefusal takes *Account for
  it; every call site already had one.

- The account refusal's plural branch is covered: two flagged apps, both named.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 2 months ago 5a1538ca6dcc6a47a1bac17b9031d6968356aa0f parent e00b64e

5 files changed, +174 −19

internal/console/apicli.go +1 −1
internal/console/appsettings.go +20 −3
internal/console/controlplane.go +19 −6
internal/console/controlplane_test.go +133 −8
internal/console/ownership.go +1 −1

diff --git a/internal/console/apicli.go b/internal/console/apicli.go

index 1989a5ddb..7b68e2b38 100644

--- a/internal/console/apicli.go

+++ b/internal/console/apicli.go

@@ -964,7 +964,7 @@ func (s *Server) handleAPITrashApp(w http.ResponseWriter, r *http.Request) {

// The control plane is not trashable at all (controlplane.go): after the

// byte-identical 404 above, so a stranger never learns the app exists,

// and before anything is written.

- if s.refuseControlPlane(w, acct.ID, app, "trashed") {

+ if s.refuseControlPlane(w, acct, app, "trashed") {

return

}

if s.cfg.Releases == nil {

diff --git a/internal/console/appsettings.go b/internal/console/appsettings.go

index bb27f9054..f0fa593ca 100644

--- a/internal/console/appsettings.go

+++ b/internal/console/appsettings.go

@@ -334,7 +334,7 @@ func (s *Server) handleAppRename(w http.ResponseWriter, r *http.Request) {

// keys, its channels and the units that exec them are all written in it.

// Refused here, straight after the owner gate and before the confirm card

// is ever rendered — a card that cannot be confirmed is worse than no card.

- if msg, _ := s.controlPlaneRefusal(acct.ID, app, "renamed"); msg != "" {

+ if msg, _ := s.controlPlaneRefusal(acct, app, "renamed"); msg != "" {

back("err", msg)

return

}

@@ -470,7 +470,7 @@ func (s *Server) handleAppDelete(w http.ResponseWriter, r *http.Request) {

}

// Outright, before the typed name and long before the passkey: the flag

// is the guard, and no gesture answers it (controlplane.go).

- if msg, _ := s.controlPlaneRefusal(acct.ID, app, "trashed"); msg != "" {

+ if msg, _ := s.controlPlaneRefusal(acct, app, "trashed"); msg != "" {

back("err", msg)

return

}

@@ -555,6 +555,9 @@ func (s *Server) handleAppDelete(w http.ResponseWriter, r *http.Request) {

// owner — a member probing this gets a stranger's answer.

// CSRF — constant-time, before any pending action can be parked.

+// control plane — refused outright, before the typed word and before the

+// passkey: this ceremony is exactly the one spec §6 rules

+// insufficient for the app that IS the platform.

// type the BUCKET — not the app's name: the bucket is the thing that stops

// existing, and its objects are what cannot be brought

// back. A declared-only record has no bucket, so there the

@@ -589,6 +592,20 @@ func (s *Server) handleStoreDestroy(w http.ResponseWriter, r *http.Request) {

s.setFlash(w, "err", msg)

http.Redirect(w, r, "/a/"+url.PathEscape(acct.Sqid)+"/apps/"+url.PathEscape(app)+"/settings", http.StatusSeeOther)

}

+ // The sixth destructive door, and the one whose ceremony is closest to

+ // enough without being it: owner, CSRF, the typed BUCKET name and a

+ // passkey — exactly the ceremony spec §6 rules insufficient for the

+ // control plane, over an estate whose own doc says there is no trash and

+ // no undo. A guard the neighbouring door walks around reads as protection

+ // without being it (controlplane.go).

+ //

+ // The refusal is on DESTROYING the store, not on having one: a

+ // control-plane app may hold an object store like any other app, and

+ // forbidding that would be an invented constraint the spec does not make.

+ if msg, _ := s.controlPlaneRefusal(acct, app, "destroyed"); msg != "" {

+ fail(msg)

+ return

+ }

if s.cfg.Releases == nil {

fail("No deployment bucket on this host, so this console cannot read the store record — nothing was destroyed.")

return

@@ -697,7 +714,7 @@ func (s *Server) handleAppMoveAccount(w http.ResponseWriter, r *http.Request) {

}

// The control plane does not change accounts: its releases are read from

// one account's prefix by every box in the fleet (controlplane.go).

- if msg, _ := s.controlPlaneRefusal(acct.ID, app, "moved to another account"); msg != "" {

+ if msg, _ := s.controlPlaneRefusal(acct, app, "moved to another account"); msg != "" {

back("err", msg)

return

}

diff --git a/internal/console/controlplane.go b/internal/console/controlplane.go

index a9c545768..fbb0d8bb8 100644

--- a/internal/console/controlplane.go

+++ b/internal/console/controlplane.go

@@ -39,6 +39,11 @@ import (

// otherwise msg is the sentence to show and status the code a JSON door should

// send with it.

+// The sentence NAMES THE PAGE that can clear the flag. An operator meeting one

+// of these refusals is by definition someone the platform has just stopped,

+// and "clear the control-plane flag first" without saying where is the same

+// stranded-operator problem the clear door was built to end.

+//

// Callers MUST be past their own membership gate first — the byte-identical

// 404 protects account membership, and consulting the flag before it would

// turn this refusal into an oracle for which accounts hold a control-plane

@@ -48,10 +53,10 @@ import (

// — treating an unreadable row as "not the control plane" — makes a database

// blip the way past the guard. The error itself is logged, never returned:

// the caller gets one fixed sentence, as with every other store failure here.

-func (s *Server) controlPlaneRefusal(accountID int64, app, verb string) (msg string, status int) {

- cp, _, err := s.store.AppFlags(accountID, app)

+func (s *Server) controlPlaneRefusal(acct *Account, app, verb string) (msg string, status int) {

+ cp, _, err := s.store.AppFlags(acct.ID, app)

if err != nil {

- log.Printf("console: control-plane check %s in account %d: %v", app, accountID, err)

+ log.Printf("console: control-plane check %s in account %s: %v", app, acct.Sqid, err)

return "Could not check whether " + app + " is the control plane — nothing was changed. Try again.",

http.StatusServiceUnavailable

}

@@ -59,15 +64,23 @@ func (s *Server) controlPlaneRefusal(accountID int64, app, verb string) (msg str

return "", 0

}

return app + " is marked as the control plane, so it cannot be " + verb +

- " — clear the control-plane flag first if you really mean to.", http.StatusForbidden

+ " — clear the flag first on the app's Settings tab (" + s.appSettingsURL(acct, app) +

+ ") if you really mean to.", http.StatusForbidden

+// appSettingsURL is where both halves of the flag live, said in full: a CLI

+// caller has no page to be already looking at, and a browser flash pointing at

+// an absolute URL costs nothing.

+func (s *Server) appSettingsURL(acct *Account, app string) string {

+ return s.cfg.Origin + "/a/" + url.PathEscape(acct.Sqid) + "/apps/" + url.PathEscape(app) + "/settings"

}

// refuseControlPlane is controlPlaneRefusal for the JSON doors: it writes the

// refusal and returns true when the caller must stop. Browser doors render

// pages and set flashes, so they call controlPlaneRefusal directly and hand

// msg to their own back()/fail() closure.

-func (s *Server) refuseControlPlane(w http.ResponseWriter, accountID int64, app, verb string) bool {

- msg, status := s.controlPlaneRefusal(accountID, app, verb)

+func (s *Server) refuseControlPlane(w http.ResponseWriter, acct *Account, app, verb string) bool {

+ msg, status := s.controlPlaneRefusal(acct, app, verb)

if msg == "" {

return false

}

diff --git a/internal/console/controlplane_test.go b/internal/console/controlplane_test.go

index f6d5ca05b..ff38a7cd9 100644

--- a/internal/console/controlplane_test.go

+++ b/internal/console/controlplane_test.go

@@ -7,6 +7,7 @@ import (

"testing"

"time"

+ "github.com/carlosframework/platform/internal/objstore"

"github.com/carlosframework/platform/internal/release"

"github.com/carlosframework/platform/internal/svcrole"

)

@@ -40,9 +41,16 @@ func TestControlPlaneAppRefusesTrash(t *testing.T) {

if status != http.StatusForbidden {

t.Fatalf("trash a control-plane app = %d %v, want 403", status, out)

}

- if msg := str(t, out, "error"); !strings.Contains(msg, "control plane") {

+ msg := str(t, out, "error")

+ if !strings.Contains(msg, "control plane") {

t.Fatalf("refusal %q does not name the flag the operator has to clear", msg)

}

+ // And WHERE to clear it. A CLI caller has no page in front of them, so a

+ // refusal that names the flag without naming the card strands exactly the

+ // operator it just stopped.

+ if !strings.Contains(msg, "/a/bab/apps/console/settings") {

+ t.Fatalf("refusal %q does not point at the settings card that can clear the flag", msg)

+ }

present, err := fx.st.HasAppIn(OpsAccountID, "console")

if err != nil || !present {

t.Fatalf("app row after a refused trash: HasAppIn = %v %v, want true (untouched)", present, err)

@@ -210,11 +218,17 @@ func TestControlPlaneAppRefusesAccountDeletion(t *testing.T) {

if err != nil || acct == nil {

t.Fatalf("AccountBySqid(%q) = %+v %v", sqid, acct, err)

}

- if _, err := fx.st.CreateAppIn(acct.ID, "console", "paul@keymail.dev", time.Now()); err != nil {

- t.Fatal(err)

- }

- if err := fx.st.SetAppControlPlane(acct.ID, "console", true); err != nil {

- t.Fatal(err)

+ // TWO flagged apps, because the refusal LOOPS: a guard that stopped at the

+ // first row would name one app and leave the operator to discover the

+ // other by trying again (coordinator ruling 7 — an account may hold more

+ // than one).

+ for _, app := range []string{"console", "keymail"} {

+ if _, err := fx.st.CreateAppIn(acct.ID, app, "paul@keymail.dev", time.Now()); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.SetAppControlPlane(acct.ID, app, true); err != nil {

+ t.Fatal(err)

+ }

}

status, loc, body := postForm(t, c, fx.ts.URL+"/a/"+sqid+"/delete",

url.Values{"csrf": {csrf}, "confirm": {"Testco"}})

@@ -229,8 +243,13 @@ func TestControlPlaneAppRefusesAccountDeletion(t *testing.T) {

}

// The flash must name the app and the flag, or the operator cannot act.

_, _, page := get(t, c, fx.ts.URL+"/a/"+sqid+"/settings")

- if !strings.Contains(page, "control plane") || !strings.Contains(page, "console") {

- t.Fatalf("refusal flash names neither the flag nor the app:\n%s", page)

+ if !strings.Contains(page, "control plane") {

+ t.Fatalf("refusal flash does not name the flag:\n%s", page)

+ }

+ for _, app := range []string{"console", "keymail"} {

+ if !strings.Contains(page, app) {

+ t.Fatalf("refusal flash does not name %q — the loop stopped early:\n%s", app, page)

+ }

}

@@ -380,6 +399,17 @@ func TestMemberCannotClearTheControlPlaneFlag(t *testing.T) {

if cp, _, err := fx.st.AppFlags(OpsAccountID, "console"); err != nil || !cp {

t.Fatalf("AppFlags after a member's attempt = %v %v, want true (still flagged)", cp, err)

}

+ // The 404 above has to be the OWNER GATE's, not a missing route: an

+ // unregistered path answers a member exactly the same way, so without this

+ // the test would still pass with the door deleted.

+ fx.setUser("paul@keymail.dev")

+ owner := newBrowser(t)

+ signUp(t, fx, owner, "code-cp-clear-owner-probe")

+ status, loc, _ = postForm(t, owner, fx.u("/apps/console/control-plane/clear"),

+ url.Values{"csrf": {sessionToken(t, fx, owner)}})

+ if status != http.StatusSeeOther || !strings.HasPrefix(loc, fx.fk.srv.URL) {

+ t.Fatalf("the OWNER on the same URL = %d -> %q, want the handler's step-up — the member's 404 may just be a missing route", status, loc)

+ }

}

// TestOwnerSetsTheControlPlaneFlagWithoutAPasskey is the other half of the

@@ -444,4 +474,99 @@ func TestMemberCannotSetTheControlPlaneFlag(t *testing.T) {

if cp, _, err := fx.st.AppFlags(OpsAccountID, "titogo"); err != nil || cp {

t.Fatalf("AppFlags after a member's attempt = %v %v, want false", cp, err)

}

+ // Same proof as the clear door's: an owner must REACH the handler on this

+ // URL, or the member's 404 is indistinguishable from a route that is not

+ // registered at all.

+ fx.setUser("paul@keymail.dev")

+ owner := newBrowser(t)

+ signUp(t, fx, owner, "code-cp-set-owner-probe")

+ status, loc, _ = postForm(t, owner, fx.u("/apps/titogo/control-plane/set"),

+ url.Values{"csrf": {sessionToken(t, fx, owner)}})

+ if status != http.StatusSeeOther || loc != fx.path("/apps/titogo/settings") {

+ t.Fatalf("the OWNER on the same URL = %d -> %q, want the handler's redirect — the member's 404 may just be a missing route", status, loc)

+ }

+ if cp, _, err := fx.st.AppFlags(OpsAccountID, "titogo"); err != nil || !cp {

+ t.Fatalf("AppFlags after the owner's probe = %v %v, want true", cp, err)

+ }

+// TestControlPlaneAppRefusesStoreDestroy: the sixth destructive door. Its

+// ceremony — owner, CSRF, the typed BUCKET name, a passkey — is exactly the

+// ceremony spec §6 rules insufficient for the control plane, and what it

+// destroys (bucket, objects, IAM principal, credentials, record) has, by its

+// own doc, no trash and no undo. Refused before the typed word and before the

+// passkey, so the answer is the settings tab rather than a Keymail round trip.

+func TestControlPlaneAppRefusesStoreDestroy(t *testing.T) {

+ fx, _ := controlPlaneFixture(t, "code-cp-store")

+ putRecord(t, fx, "console", &objstore.Record{

+ Declared: &objstore.Declared{EnvPrefix: objstore.DefaultEnvPrefix, At: time.Now(), By: "paul@keymail.dev"},

+ })

+ c := signInPaul(t, fx)

+ csrf := sessionToken(t, fx, c)

+ status, loc, body := postForm(t, c, fx.u("/apps/console/store/destroy"),

+ url.Values{"csrf": {csrf}, "confirm": {"console"}})

+ if status != http.StatusSeeOther {

+ t.Fatalf("destroy a control-plane app's store = %d -> %q:\n%s", status, loc, body)

+ }

+ if loc != fx.path("/apps/console/settings") {

+ t.Fatalf("refusal redirected to %q, want the settings tab — a Keymail redirect means the guard ran too late", loc)

+ }

+ if rec := storedRecord(t, fx, "console"); rec == nil {

+ t.Fatal("the store record was destroyed despite the control-plane flag")

+ }

+ // Cleared, the same door works — the refusal is the flag's, not the store's.

+ if err := fx.st.SetAppControlPlane(OpsAccountID, "console", false); err != nil {

+ t.Fatal(err)

+ }

+ status, loc, body = postGated(t, fx, c, fx.u("/apps/console/store/destroy"),

+ url.Values{"csrf": {csrf}, "confirm": {"console"}})

+ if status != http.StatusSeeOther || loc != fx.path("/apps/console/settings") {

+ t.Fatalf("destroy after clearing the flag = %d -> %q:\n%s", status, loc, body)

+ }

+ if rec := storedRecord(t, fx, "console"); rec != nil {

+ t.Fatalf("the declaration survived a destroy that should have worked: %+v", rec)

+ }

+// TestControlPlaneLookupFailureRefuses pins the named project constraint: "a

+// lookup failure must REFUSE, not fall through". Treating an unreadable row as

+// "not the control plane" would make a database blip the way past every guard

+// in this file — and `cp, _, _ := s.store.AppFlags(...)` reads like harmless

+// tidying, so this is the branch a later simplification inverts.

+//

+// The column is dropped rather than the database closed, deliberately: closing

+// it would break the token lookup and the app 404 first, and the request would

+// never reach the flag at all. Dropping ONE column leaves every other reader

+// working and fails exactly AppFlags.

+func TestControlPlaneLookupFailureRefuses(t *testing.T) {

+ fx, token := controlPlaneFixture(t, "code-cp-blip")

+ c := signInPaul(t, fx)

+ csrf := sessionToken(t, fx, c)

+ if _, err := fx.st.db.Exec(`ALTER TABLE apps DROP COLUMN control_plane`); err != nil {

+ t.Fatalf("drop the flag column: %v", err)

+ }

+ // The JSON door: 503, not the 200 a fall-through would give.

+ status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/console/delete", map[string]string{}, token)

+ if status != http.StatusServiceUnavailable {

+ t.Fatalf("trash with an unreadable flag = %d %v, want 503", status, out)

+ }

+ if msg := str(t, out, "error"); !strings.Contains(msg, "Could not check") {

+ t.Fatalf("refusal %q does not say the check itself failed", msg)

+ }

+ if present, err := fx.st.HasAppIn(OpsAccountID, "console"); err != nil || !present {

+ t.Fatalf("app row after a refused trash: HasAppIn = %v %v, want true (untouched)", present, err)

+ }

+ // The browser door: the flash, the trash page, and no passkey demanded —

+ // postForm, so a step-up redirect would show rather than be walked.

+ status, loc, body := postForm(t, c, fx.u("/trash/delete"),

+ url.Values{"csrf": {csrf}, "app": {"console"}, "confirm": {"console"}})

+ if status != http.StatusSeeOther || loc != fx.path("/trash") {

+ t.Fatalf("purge with an unreadable flag = %d -> %q, want the trash page's flash:\n%s", status, loc, body)

+ }

+ if _, _, page := get(t, c, fx.ts.URL+fx.path("/trash")); !strings.Contains(page, "Could not check") {

+ t.Fatalf("purge refusal flash does not say the check failed:\n%s", page)

+ }

}

diff --git a/internal/console/ownership.go b/internal/console/ownership.go

index f8185c86d..05b3bc612 100644

--- a/internal/console/ownership.go

+++ b/internal/console/ownership.go

@@ -331,7 +331,7 @@ func (s *Server) handlePurgeApp(w http.ResponseWriter, r *http.Request) {

// prefix holds the release history the fleet is RUNNING. Refused before

// the typed name and before the passkey — neither gesture can answer the

// flag, and asking for one would only spend it (controlplane.go).

- if msg, _ := s.controlPlaneRefusal(acct.ID, app, "deleted"); msg != "" {

+ if msg, _ := s.controlPlaneRefusal(acct, app, "deleted"); msg != "" {

fail(msg)

return

}