amadan
console: the control-plane flag also guards the account, and CI's retention
Three coordinator rulings on the flag's first cut: 1. handleDeleteAccount refuses while the account holds a control-plane app, naming the app and the flag. Account deletion is the same destruction by another door, and a guard with a bypass reads as protection without being it. AppCount already refused any account holding an app row, so nothing was reachable today — but that guard is about cascading deletes, is free to change for its own reasons, names neither the app nor the flag, and sits BELOW the passkey. This one is about the flag and refuses before a gesture is spent. The negative case is pinned too: an ordinary empty account still deletes. 2. POST .../retention wears the promote pair's service refusal. Retention is how the rollback depth behind the console gets shortened, and a CI credential must not reach it; a PERSON still may, since history depth is an operator's decision and PruneEligible already protects the running release. 3. The service refusal takes a whole clause, not a participle — "cannot roll back it" was broken English in user-facing text. promote it / roll it back / change its release retention.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 files changed,
+177
−9
internal/console/apicli.go+2 −2internal/console/apiretention.go+13 −0internal/console/controlplane.go+70 −7internal/console/controlplane_test.go+77 −0internal/console/ownership.go+15 −0
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 32ebb663d..1989a5ddb 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -684,7 +684,7 @@ func (s *Server) handleAPIPromote(w http.ResponseWriter, r *http.Request) { |
| // credential may not. This is the flag's promote-side job — the whole of |
| // it. Gating a PERSON here would break `carlos deploy -app console`, the |
| // command this exists to make ordinary (controlplane.go). |
| - if s.refuseServiceControlPlane(w, p, acct.ID, app, "promote") { |
| + if s.refuseServiceControlPlane(w, p, acct.ID, app, "promote it") { |
| return |
| } |
| res, err := release.PromoteOnLadder(ctx, s.cfg.Releases, s.cfg.Key, acct.Sqid, app, channel, version, |
| @@ -809,7 +809,7 @@ func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) { |
| // Rollback moves the same pointer promote moves, so it wears the same |
| // service refusal — a credential barred from promoting the control plane |
| // must not reach the identical effect through the other verb. |
| - if s.refuseServiceControlPlane(w, p, acct.ID, app, "roll back") { |
| + if s.refuseServiceControlPlane(w, p, acct.ID, app, "roll it back") { |
| return |
| } |
| if err := release.Rollback(ctx, s.cfg.Releases, s.cfg.Key, acct.Sqid, app, channel); err != nil { |
diff --git a/internal/console/apiretention.go b/internal/console/apiretention.go| index e13d064f2..4deb47ba6 100644 |
| --- a/internal/console/apiretention.go |
| +++ b/internal/console/apiretention.go |
| @@ -104,6 +104,19 @@ func (s *Server) handleAPIRetentionSet(w http.ResponseWriter, r *http.Request) { |
| "no deployment bucket on this host — this console cannot write retention") |
| return |
| } |
| + // Retention is how the rollback DEPTH behind a control-plane app gets |
| + // shortened, so it wears the promote pair's service refusal: a CI |
| + // credential must not quietly shrink the console's own history. A PERSON |
| + // still may — how much history to keep is an operator's decision, and |
| + // PruneEligible never touches anything promoted, tagged or recent. |
| + // |
| + // The whole door, dry run included: this is a POST whose default branch is |
| + // the write, and a credential that cannot set retention has no use for a |
| + // preview of setting it. Reading the current policy stays open — that is |
| + // the GET (controlplane.go). |
| + if s.refuseServiceControlPlane(w, p, acct.ID, app, "change its release retention") { |
| + return |
| + } |
| var in struct { |
| Keep int `json:"keep"` |
| Off bool `json:"off"` |
diff --git a/internal/console/controlplane.go b/internal/console/controlplane.go| index d85ea4d7b..8f5696a10 100644 |
| --- a/internal/console/controlplane.go |
| +++ b/internal/console/controlplane.go |
| @@ -23,6 +23,7 @@ package console |
| import ( |
| "log" |
| "net/http" |
| + "strings" |
| ) |
| // controlPlaneRefusal answers "may this door proceed?" for a destructive verb. |
| @@ -66,15 +67,20 @@ func (s *Server) refuseControlPlane(w http.ResponseWriter, accountID int64, app, |
| return true |
| } |
| -// refuseServiceControlPlane is the promote pair's gate, and the whole of what |
| -// the flag does on the promote path: a service credential cannot move a |
| -// control-plane app's channel pointer; a PERSON with ops membership can, and |
| -// must be able to — gating a person here would break `carlos deploy -app |
| -// console`, the command this work exists to make ordinary. |
| +// refuseServiceControlPlane is the release-authority gate: a service |
| +// credential cannot move a control-plane app's channel pointer, nor shorten |
| +// the history behind it; a PERSON with ops membership can, and must be able |
| +// to — gating a person on promote would break `carlos deploy -app console`, |
| +// the command this work exists to make ordinary. |
| +// |
| +// action is a whole CLAUSE, not a participle ("promote it", "roll it back", |
| +// "change its release retention"), because the sentence it lands in has to |
| +// read as English for three different verbs and "cannot roll back it" is not |
| +// English. |
| // |
| // The flag is only read for a service, so a person's promote does not grow a |
| // database round trip it has no use for. |
| -func (s *Server) refuseServiceControlPlane(w http.ResponseWriter, p Principal, accountID int64, app, verb string) bool { |
| +func (s *Server) refuseServiceControlPlane(w http.ResponseWriter, p Principal, accountID int64, app, action string) bool { |
| if p.Kind == PrincipalUser { |
| return false |
| } |
| @@ -89,6 +95,63 @@ func (s *Server) refuseServiceControlPlane(w http.ResponseWriter, p Principal, a |
| return false |
| } |
| jsonError(w, http.StatusForbidden, |
| - app+" is the control plane — a service credential cannot "+verb+" it; a person must") |
| + app+" is the control plane — a service credential cannot "+action+"; a person must") |
| return true |
| } |
| + |
| +// controlPlaneAppsIn lists the apps in an account carrying the control-plane |
| +// flag, TRASHED ROWS INCLUDED: a trashed app still owns its prefix in the |
| +// bucket — the release history the fleet is running is exactly what a trash |
| +// marker does not destroy — so it is still a reason to refuse the account's |
| +// retirement. (AppCount, handleDeleteAccount's older guard, counts the same |
| +// way for the same reason.) |
| +func (s *Store) controlPlaneAppsIn(accountID int64) ([]string, error) { |
| + rows, err := s.db.Query(`SELECT app FROM apps WHERE account_id = ? AND control_plane = 1 ORDER BY app`, accountID) |
| + if err != nil { |
| + return nil, err |
| + } |
| + defer rows.Close() |
| + var out []string |
| + for rows.Next() { |
| + var app string |
| + if err := rows.Scan(&app); err != nil { |
| + return nil, err |
| + } |
| + out = append(out, app) |
| + } |
| + return out, rows.Err() |
| +} |
| + |
| +// controlPlaneAccountRefusal is the same guard one level up: an account |
| +// holding a control-plane app cannot be RETIRED either. Refusing the app's own |
| +// destructive doors while the account above it could still be deleted would be |
| +// a guard with a published bypass, which reads as protection and is not. |
| +// |
| +// msg is "" when the account is free to go. A lookup failure refuses, on the |
| +// same reasoning as controlPlaneRefusal: a database blip must not be the way |
| +// past the guard. |
| +// |
| +// In practice handleDeleteAccount's AppCount refusal already catches every |
| +// account this could fire on, since an account holding ANY app row is refused |
| +// — but that guard is about cascading deletes, is written to change for its |
| +// own reasons, and sits BELOW the passkey. This one is about the flag, names |
| +// the app, and refuses before a gesture is spent. |
| +func (s *Server) controlPlaneAccountRefusal(accountID int64) string { |
| + apps, err := s.store.controlPlaneAppsIn(accountID) |
| + if err != nil { |
| + log.Printf("console: control-plane check for account %d: %v", accountID, err) |
| + return "Could not check whether this account holds a control-plane app. Nothing was deleted." |
| + } |
| + if len(apps) == 0 { |
| + return "" |
| + } |
| + return "“" + strings.Join(apps, "”, “") + "” " + isare(len(apps)) + |
| + " marked as the control plane, so this account cannot be deleted — clear the control-plane flag first if you really mean to." |
| +} |
| + |
| +func isare(n int) string { |
| + if n == 1 { |
| + return "is" |
| + } |
| + return "are" |
| +} |
diff --git a/internal/console/controlplane_test.go b/internal/console/controlplane_test.go| index de92defdd..64a18172f 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/release" |
| "github.com/carlosframework/platform/internal/svcrole" |
| ) |
| @@ -192,3 +193,79 @@ func TestControlPlaneRefusalIsNotAMembershipOracle(t *testing.T) { |
| status, body, otherStatus, otherBody) |
| } |
| } |
| + |
| +// TestControlPlaneAppRefusesAccountDeletion closes the door one level up. |
| +// Refusing trash, purge, rename and transfer on the app while the ACCOUNT that |
| +// holds it can be retired would be a guard with a documented bypass — worse |
| +// than no guard, because it reads as protection. |
| +// |
| +// Deliberately asserted with postForm, not postGated: the refusal must arrive |
| +// BEFORE requireFreshAuth, so the answer is the settings tab and never a |
| +// redirect into Keymail for a passkey that cannot help. (The pre-existing |
| +// "account still holds apps" refusal sits BELOW step-up, so a passkey demand |
| +// here would mean the control-plane guard never fired.) |
| +func TestControlPlaneAppRefusesAccountDeletion(t *testing.T) { |
| + fx, c, csrf, sqid := deletableFixture(t) |
| + acct, err := fx.st.AccountBySqid(sqid) |
| + 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) |
| + } |
| + status, loc, body := postForm(t, c, fx.ts.URL+"/a/"+sqid+"/delete", |
| + url.Values{"csrf": {csrf}, "confirm": {"Testco"}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("delete an account holding the control plane = %d, want the flash redirect:\n%s", status, body) |
| + } |
| + if loc != "/a/"+sqid+"/settings" { |
| + t.Fatalf("refusal redirected to %q, want the settings tab — a Keymail redirect means the guard ran too late", loc) |
| + } |
| + if a, _ := fx.st.AccountBySqid(sqid); a == nil { |
| + t.Fatal("the account was deleted despite holding a control-plane app") |
| + } |
| + // 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) |
| + } |
| +} |
| + |
| +// TestAccountWithNoControlPlaneAppStillDeletes is the negative half, and it |
| +// is the one that keeps the guard honest: a lookup that fell back to "refuse" |
| +// for every account would turn one refusal into "no account may ever be |
| +// deleted", and nothing else in the suite would notice. |
| +func TestAccountWithNoControlPlaneAppStillDeletes(t *testing.T) { |
| + fx, c, csrf, sqid := deletableFixture(t) |
| + status, _, body := postGated(t, fx, c, fx.ts.URL+"/a/"+sqid+"/delete", |
| + url.Values{"csrf": {csrf}, "confirm": {"Testco"}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("delete an ordinary empty account = %d, want it to work as before:\n%s", status, body) |
| + } |
| + if a, _ := fx.st.AccountBySqid(sqid); a != nil { |
| + t.Fatal("an account with no control-plane app survived its own deletion") |
| + } |
| +} |
| + |
| +// TestControlPlaneRetentionRefusesAServiceButNotAPerson: retention is how the |
| +// console's rollback DEPTH gets shortened, so a CI credential must not reach |
| +// it on a control-plane app. A person still may — how much history to keep is |
| +// an operator's decision, and PruneEligible already refuses to touch anything |
| +// promoted, tagged or recent. |
| +func TestControlPlaneRetentionRefusesAServiceButNotAPerson(t *testing.T) { |
| + fx, token := controlPlaneFixture(t, "code-cp-retention") |
| + svc := serviceToken(t, fx.st, OpsAccountID, "ci", "console", svcrole.Publish) |
| + body := map[string]any{"keep": release.KeepFloor} |
| + |
| + status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/console/retention", body, svc) |
| + if status != http.StatusForbidden { |
| + t.Fatalf("a SERVICE setting retention on the control plane = %d %v, want 403", status, out) |
| + } |
| + status, out = postJSON(t, fx.ts.URL+"/api/cli/apps/bab/console/retention", body, token) |
| + if status != http.StatusOK { |
| + t.Fatalf("a PERSON setting retention on the control plane = %d %v, want 200 — only the service is barred", status, out) |
| + } |
| +} |
diff --git a/internal/console/ownership.go b/internal/console/ownership.go| index c98236c8f..f8185c86d 100644 |
| --- a/internal/console/ownership.go |
| +++ b/internal/console/ownership.go |
| @@ -84,6 +84,11 @@ func (s *Server) handleTransferOwnership(w http.ResponseWriter, r *http.Request) |
| // destroy and undoing that is not a thing anyone can do. The useful case — a |
| // mistyped or abandoned account — is fully served, and the dangerous case is |
| // left to a deliberate migration. |
| +// |
| +// A control-plane app is refused by NAME on top of that, before the passkey: |
| +// the flag's whole job is to stop the platform deleting itself, and a guard on |
| +// the app's doors that an account deletion walks around would read as |
| +// protection without being it (controlplane.go). |
| func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) { |
| addr, raw, acct, ok := s.owner(w, r) |
| if !ok { |
| @@ -93,6 +98,16 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) { |
| s.renderMessage(w, r, http.StatusForbidden, "Refused", "Cross-site request blocked.") |
| return |
| } |
| + // An account holding a control-plane app is not retirable: deleting the |
| + // account is the same destruction the app's own doors refuse, reached one |
| + // level up (controlplane.go). Refused here — after the owner gate, before |
| + // the typed name and before requireFreshAuth — because no gesture answers |
| + // the flag, and the refusal must not cost a passkey touch. |
| + if msg := s.controlPlaneAccountRefusal(acct.ID); msg != "" { |
| + s.setFlash(w, "err", msg) |
| + http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther) |
| + return |
| + } |
| // Typing the name is not a security control — it is a "did you mean this |
| // account?" control, and it is checked before step-up so a slip does not |
| // cost a passkey touch. |