amadan
console: rate-limit the CLI write doors, refuse stable rollback too
Review round on the promote/rollback CLI API: neither write door applied s.cli.write (design §3 says reuse the cliLimits family, keyed on the token hash like handleCLINewAccount already does) — both now check it right after apiAccountApp. Paul's ruling: a rollback that would land on stable carries the same blast radius as a stable promote, so it gets the identical 403 before release.Rollback is ever called, role never consulted. Also hardens TestAPIPromoteStableRefused to assert no stable pointer was written, and fixes TestAPIPromoteNoHotfix, which was exercising the ladder's "not a rung" arm instead of the ordering arm its comment claimed.
3 files changed,
+96
−12
docs/superpowers/specs/2026-08-03-cli-console-api-design.md+3 −1internal/console/apicli.go+32 −3internal/console/apicli_test.go+61 −8
diff --git a/docs/superpowers/specs/2026-08-03-cli-console-api-design.md b/docs/superpowers/specs/2026-08-03-cli-console-api-design.md| index 86f90115b..86ff1a4b3 100644 |
| --- a/docs/superpowers/specs/2026-08-03-cli-console-api-design.md |
| +++ b/docs/superpowers/specs/2026-08-03-cli-console-api-design.md |
| @@ -74,7 +74,9 @@ semantics — a hand-written request meets exactly the checks a button does: |
| this slice — a recorded ladder bypass is an operator act, and operators |
| have direct mode. |
| - `POST /api/cli/apps/{account}/{app}/rollback` — body {channel}. Mirrors |
| - the browser rollback door (rollback never bakes). |
| + the browser rollback door (rollback never bakes); stable is refused with |
| + the same 403 as promote (Paul's ruling 2026-08-03 — §8's no-stable blast |
| + radius governs). |
| ## §3 Authorization and refusal order |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 97d5da69a..accb23fa7 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -186,6 +186,13 @@ func (s *Server) handleAPIPromote(w http.ResponseWriter, r *http.Request) { |
| if !ok { |
| return |
| } |
| + // Keyed on the TOKEN, not the client IP (design §3): a token is the |
| + // natural principal for a write a bearer credential is already |
| + // authenticating, the same choice handleCLINewAccount makes. |
| + if !s.cli.write.allow(hashToken(bearer(r)), time.Now()) { |
| + jsonError(w, http.StatusTooManyRequests, "too many requests — wait a minute and try again") |
| + return |
| + } |
| if s.cfg.Key == nil { |
| jsonError(w, http.StatusServiceUnavailable, |
| "no signing key on this host — the console is running read-only") |
| @@ -248,14 +255,29 @@ func (s *Server) handleAPIPromote(w http.ResponseWriter, r *http.Request) { |
| // handleAPIRollback answers `carlos rollback`: the CLI's write door onto the |
| // same swap-to-previous-version primitive as the console's own rollback |
| -// button (handleRollback, server.go). No rung gate here, matching the |
| -// browser door — rolling back never climbs the ladder, so nothing about |
| -// maturity has a say over it. |
| +// button (handleRollback, server.go). Below stable there is no rung gate, |
| +// matching the browser door — rolling back never climbs the ladder, so |
| +// nothing about maturity has a say over it. |
| +// |
| +// Stable is the one exception, and it is Paul's ruling (2026-08-03): a |
| +// rollback that lands on stable puts a build in front of every user of the |
| +// app exactly as a promotion onto it would (§8's no-stable blast radius |
| +// governs regardless of which verb got there), so it is refused with the |
| +// SAME byte-identical 403 handleAPIPromote gives stable — before |
| +// release.Rollback is ever called, and before the channel's current pointer |
| +// is even read, so the refusal cannot vary with what that pointer contains. |
| func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) { |
| addr, acct, app, ok := s.apiAccountApp(w, r) |
| if !ok { |
| return |
| } |
| + // Keyed on the TOKEN, not the client IP (design §3): a token is the |
| + // natural principal for a write a bearer credential is already |
| + // authenticating, the same choice handleCLINewAccount makes. |
| + if !s.cli.write.allow(hashToken(bearer(r)), time.Now()) { |
| + jsonError(w, http.StatusTooManyRequests, "too many requests — wait a minute and try again") |
| + return |
| + } |
| if s.cfg.Key == nil { |
| jsonError(w, http.StatusServiceUnavailable, |
| "no signing key on this host — the console is running read-only") |
| @@ -272,6 +294,13 @@ func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) { |
| jsonError(w, http.StatusBadRequest, "invalid channel name") |
| return |
| } |
| + if rung, _ := release.Rung(channel); rung == release.RungStable { |
| + writeJSON(w, http.StatusForbidden, map[string]string{ |
| + "error": "stable needs a passkey — finish in the console", |
| + "console_url": s.cfg.Origin + "/a/" + url.PathEscape(acct.Sqid) + "/apps/" + url.PathEscape(app), |
| + }) |
| + return |
| + } |
| ctx, cancel := context.WithTimeout(r.Context(), 15*time.Second) |
| defer cancel() |
| cur, err := release.Channel(ctx, s.cfg.Releases, s.cfg.ReleasePub, acct.Sqid, app, channel) |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index 56baa749e..23d5b114d 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -440,6 +440,14 @@ func TestAPIPromoteStableRefused(t *testing.T) { |
| if !strings.Contains(outO, "passkey") { |
| t.Fatalf("stable refusal = %q, want it to name the passkey requirement", outO) |
| } |
| + // The refusal must be true, not just spoken: neither call may have moved |
| + // the rung gate to a spot where PromoteOnLadder ran first and the 403 was |
| + // merely decorative. If a future edit reordered the gate below |
| + // PromoteOnLadder, this is what would catch it — the test above would |
| + // still pass on status and body alone. |
| + if _, err := release.Channel(context.Background(), fx.rel, fx.pub, release.OpsAccount, "titogo", "stable"); err == nil { |
| + t.Fatalf("stable channel exists after two refused promotions, want it absent") |
| + } |
| } |
| // TestAPIPromoteNoKey: a read-only deployment (no signing key) refuses every |
| @@ -533,20 +541,21 @@ func TestAPIPromoteNoHotfix(t *testing.T) { |
| shipVersion(t, fx, "titogo", "abc1234") |
| token := cliToken(t, fx, "paul@keymail.dev", "code-promote-nohotfix") |
| - // "beta/x" is on no rung yet (nothing has promoted it there), so this is |
| - // the ladder's plain refusal, sent alongside a hotfix:true the handler |
| - // must never act on. |
| + // abc1234 is on no rung yet (nothing has promoted it there), so promoting |
| + // straight to "beta" is the ladder-ORDERING refusal (beta is entered from |
| + // edge only) — not the "not a rung" arm a nested non-canary segment like |
| + // "beta/x" would hit instead. hotfix:true rides along and must be inert. |
| status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/titogo/promote", |
| - map[string]any{"version": "abc1234", "channel": "beta/x", "hotfix": true}, token) |
| + map[string]any{"version": "abc1234", "channel": "beta", "hotfix": true}, token) |
| if status != http.StatusConflict { |
| t.Fatalf("ladder-skip promote with hotfix:true = %d %v, want 409 (hotfix must be inert)", status, out) |
| } |
| - if !strings.Contains(str(t, out, "error"), "rung") { |
| - t.Fatalf("ladder-skip error = %q, want the ladder's own refusal text", str(t, out, "error")) |
| + if !strings.Contains(str(t, out, "error"), "beta is entered from edge only") { |
| + t.Fatalf("ladder-skip error = %q, want the ladder-ordering refusal text", str(t, out, "error")) |
| } |
| // And the pointer must not exist — the promotion truly never happened. |
| - if _, err := release.Channel(context.Background(), fx.rel, fx.pub, release.OpsAccount, "titogo", "beta/x"); err == nil { |
| - t.Fatalf("beta/x channel exists after a refused promotion, want it absent") |
| + if _, err := release.Channel(context.Background(), fx.rel, fx.pub, release.OpsAccount, "titogo", "beta"); err == nil { |
| + t.Fatalf("beta channel exists after a refused promotion, want it absent") |
| } |
| } |
| @@ -601,3 +610,47 @@ func TestAPIRollbackHappy(t *testing.T) { |
| t.Fatalf("events = %v, want a rollback row whose detail names the CLI", events) |
| } |
| } |
| + |
| +// TestAPIRollbackStableRefused: Paul's ruling (2026-08-03) — a rollback that |
| +// would land on stable is refused with the SAME byte-identical 403 as a |
| +// stable promote, for a member token and an owner token alike, and the |
| +// pointer it would have touched is left exactly as it was. |
| +func TestAPIRollbackStableRefused(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + now := time.Now() |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "titogo", "paul@keymail.dev", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + shipVersion(t, fx, "titogo", "abc1234") |
| + shipVersion(t, fx, "titogo", "def5678") |
| + // A stable channel with a real history to roll back to — so a refusal |
| + // that let the rollback through would be visible in what "stable" points |
| + // at afterwards, not just in a channel that never existed. |
| + seedChannel(t, fx, "titogo", "stable", "abc1234") |
| + seedChannel(t, fx, "titogo", "stable", "def5678") |
| + if err := fx.st.AddMember(OpsAccountID, "alice@keymail.dev", RoleMember, now); err != nil { |
| + t.Fatal(err) |
| + } |
| + owner := cliToken(t, fx, "paul@keymail.dev", "code-rollback-stable-owner") |
| + member := cliToken(t, fx, "alice@keymail.dev", "code-rollback-stable-member") |
| + |
| + body := map[string]string{"channel": "stable"} |
| + statusO, outO := postRaw(t, fx.ts.URL+"/api/cli/apps/bab/titogo/rollback", body, owner) |
| + statusM, outM := postRaw(t, fx.ts.URL+"/api/cli/apps/bab/titogo/rollback", body, member) |
| + if statusO != http.StatusForbidden || statusM != http.StatusForbidden { |
| + t.Fatalf("stable rollback = owner %d, member %d, want both 403", statusO, statusM) |
| + } |
| + if outO != outM { |
| + t.Fatalf("stable rollback refusal not byte-identical:\nowner: %q\nmember: %q", outO, outM) |
| + } |
| + wantURL := fx.ts.URL + "/a/bab/apps/titogo" |
| + if !strings.Contains(outO, wantURL) { |
| + t.Fatalf("stable rollback refusal = %q, want console_url %q", outO, wantURL) |
| + } |
| + if !strings.Contains(outO, "passkey") { |
| + t.Fatalf("stable rollback refusal = %q, want it to name the passkey requirement", outO) |
| + } |
| + if cp := fx.channelVersion(t, "titogo", "stable"); cp.Version != "def5678" { |
| + t.Fatalf("stable channel after two refused rollbacks = %q, want def5678 (unchanged)", cp.Version) |
| + } |
| +} |