service principals: implementation plan for issue #220
Nine tasks, TDD throughout, each ending in a green gate and a commit. 1 internal/svcrole the closed role catalogue 2 schema + Principal + one resolver (legacy kind='app' reads as instance) 3 cliRoute the structural role gate + the two coverage tests 4 scope from the principal; apiInstancePrincipal collapses in 5 rate limits keyed on the principal; wildcard exempt, failures bounded 6 the owner-gated mint doors 7 carlos services 8 the account Settings pane + the read-only mirror on /settings 9 whole-branch gate, migration proof against a real pre-change db, PR Two gaps named in the self-review rather than papered over: four tests are specified by assertion and reason but not written as code, because they depend on package-local fixtures whose shape must be read rather than guessed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016UimU7fAfeAoBMm6HKCPwQ
1 file changed,
+2012
−0
docs/superpowers/plans/2026-08-18-service-principals.md+2012 −0
diff --git a/docs/superpowers/plans/2026-08-18-service-principals.md b/docs/superpowers/plans/2026-08-18-service-principals.md| new file mode 100644 |
| index 000000000..efc051f82 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/2026-08-18-service-principals.md |
| @@ -0,0 +1,2012 @@ |
| +# Service Principals Implementation Plan |
| + |
| +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| + |
| +**Goal:** Give the console API a principal that is not a Keymail address — minted per account, role-scoped from a closed catalogue, rotatable and revocable, with its own rate-limit buckets. |
| + |
| +**Architecture:** Widen the existing non-human principal rather than invent one. `cli_tokens` already carries `kind`/`account_id`/`app` and already serves `kind='app'` tokens through `apiInstancePrincipal`; this adds `role`/`name`/`revoked_at`, a `Principal` type resolved by one store method, a closed role catalogue in a new `internal/svcrole` package, and a `cliRoute` registration wrapper that makes the role gate structural (a door that names no roles does not compile). Legacy `kind='app'` rows read back as role `instance`, defined by observation of today's call sites so titogo's router does not break. |
| + |
| +**Tech Stack:** Go 1.26, stdlib `net/http` with Go 1.22 pattern routing, SQLite (`modernc.org/sqlite`) via `internal/console`'s `Store`, no new dependencies. |
| + |
| +**Spec:** `docs/superpowers/specs/2026-08-18-service-principals-design.md` |
| + |
| +## Global Constraints |
| + |
| +- **Every change lands through a PR** (CLAUDE.md). Never merge to `main`, never commit to `main`. Push the branch, open the PR, a human merges. |
| +- **The full gate must be clean before any commit lands:** `go build ./...`, `go vet ./...`, `go test ./... -count=1`, and `gofmt -l` clean for every file touched. |
| +- **Sandbox env for every go command:** `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off`. If the toolchain reports "read-only file system", the sandbox must be disabled for that call. |
| +- **The gate is not Linux-only.** CI also cross-compiles `GOOS=darwin`; `internal/host` is linked into the CLI that people run on Macs. |
| +- **Wire-shape changes are additive.** Live tokens exist in production databases; every row written before this change must read back with identical authority. |
| +- **Schema changes use the swallowed-duplicate `ALTER TABLE` idiom** already in `migrateCLIAuth` (`internal/console/authstore.go:166-175`), never a table rewrite. |
| +- **A test that shells out to the carlos binary must build its env through `carlosTestEnv` and set `cmd.Dir` to a scratch dir.** The fallback is the operator's real login and the production console. |
| +- **Console security gates are ordered:** session → CSRF → owner (byte-identical 404) → `requireFreshAuth` LAST. |
| +- **Byte-identical refusals.** An unknown account, someone else's account, and an unknown app must be the same `http.NotFound(w, r)` — never `jsonError`, whose distinct body shape is itself a tell. |
| +- **Role names are exactly** `publish`, `instance`, `operate`, `admin`. **Principal string is exactly** `service:<sqid>/<name>`. **Service name grammar is `registry.ValidTunnelName`**: 1-32 characters of `a-z`, `0-9` and hyphen. |
| + |
| +--- |
| + |
| +## File Structure |
| + |
| +**Created:** |
| +- `internal/svcrole/svcrole.go` — the closed role catalogue: `Role`, the four constants, `Parse`, `All`, `Set`, `Allow`, `Set.Permits`. No console imports, no route knowledge. |
| +- `internal/svcrole/svcrole_test.go` |
| +- `internal/console/svcprincipal.go` — `Principal`, `PrincipalKind`, `Store.CLIPrincipal`, and the service-principal store methods (mint/list/rotate/revoke). |
| +- `internal/console/svcprincipal_test.go` |
| +- `internal/console/apiservices.go` — the four owner-gated `/api/cli/services/…` doors. |
| +- `internal/console/apiservices_test.go` |
| +- `internal/console/svcroutes_test.go` — the two structural tests: every `/api/cli/` door went through `cliRoute`; no role reaches a mint or fleets door. |
| +- `services.go` (repo root) — the `carlos services` verbs. |
| +- `services_cli_test.go` (repo root) |
| + |
| +**Modified:** |
| +- `internal/console/authstore.go` — three `ALTER TABLE` lines; `CLIToken` gains `Role`/`Name`/`RevokedAt`; `CLITokenAddress` unchanged in behaviour but must skip revoked rows. |
| +- `internal/console/authcli.go` — `cliRoute` helper; every `/api/cli/` registration rewritten to name its roles; service rate-limit constants and limiters. |
| +- `internal/console/apicli.go` — `apiAccount`/`apiAccountApp` resolve a `Principal`; `handleAPIApps` answers a service with its own scope; `principalKey` replaces `addr` on the create and write limiters. |
| +- `internal/console/apiinstances.go` — `apiInstancePrincipal` collapses into the general gate. |
| +- `internal/console/apidomains.go` — wildcard-enable failure bound. |
| +- `internal/console/server.go` — the account Settings route for the new pane. |
| +- `internal/console/templates/account_settings.html` (or the repo's actual template path) — Service credentials pane. |
| +- `apimode.go` (repo root) — four `apiSession` client methods. |
| +- `main.go` (repo root) — `services` verb dispatch. |
| + |
| +--- |
| + |
| +### Task 1: The role catalogue |
| + |
| +Pure data and logic, no console dependency. Doing it first means every later task has real role constants to import rather than string literals. |
| + |
| +**Files:** |
| +- Create: `internal/svcrole/svcrole.go` |
| +- Test: `internal/svcrole/svcrole_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: nothing. |
| +- Produces: |
| + - `type Role string`, constants `Publish`, `Instance`, `Operate`, `Admin` (values `"publish"`, `"instance"`, `"operate"`, `"admin"`) |
| + - `func Parse(s string) (Role, bool)` |
| + - `func All() []Role` |
| + - `type Set struct{ ... }` (opaque) |
| + - `func Allow(roles ...Role) Set` |
| + - `func (s Set) Permits(r Role) bool` |
| + - `func (s Set) Roles() []Role` — sorted, for test assertions and API output |
| + - `func (s Set) Empty() bool` |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/svcrole/svcrole_test.go`: |
| + |
| +```go |
| +package svcrole |
| + |
| +import ( |
| + "reflect" |
| + "testing" |
| +) |
| + |
| +func TestParse(t *testing.T) { |
| + for _, want := range All() { |
| + got, ok := Parse(string(want)) |
| + if !ok || got != want { |
| + t.Errorf("Parse(%q) = %q,%v; want %q,true", want, got, ok, want) |
| + } |
| + } |
| + // A role name that is not in the catalogue must not parse. "" is the |
| + // value a person's row carries, and it must never resolve to a role. |
| + for _, bad := range []string{"", "owner", "member", "Publish", "admin ", "root"} { |
| + if got, ok := Parse(bad); ok { |
| + t.Errorf("Parse(%q) = %q,true; want false", bad, got) |
| + } |
| + } |
| +} |
| + |
| +func TestAdminPermitsEveryNonEmptySet(t *testing.T) { |
| + // Admin is defined as the union, so it reaches every door that names any |
| + // role at all — including doors that name only Admin. |
| + for _, s := range []Set{ |
| + Allow(Publish), Allow(Instance), Allow(Operate), Allow(Admin), |
| + Allow(Publish, Operate), |
| + } { |
| + if !s.Permits(Admin) { |
| + t.Errorf("Allow(%v).Permits(Admin) = false; want true", s.Roles()) |
| + } |
| + } |
| +} |
| + |
| +func TestAllowIsExactForNonAdmin(t *testing.T) { |
| + s := Allow(Publish) |
| + if !s.Permits(Publish) { |
| + t.Error("Allow(Publish).Permits(Publish) = false; want true") |
| + } |
| + for _, r := range []Role{Instance, Operate} { |
| + if s.Permits(r) { |
| + t.Errorf("Allow(Publish).Permits(%q) = true; want false", r) |
| + } |
| + } |
| +} |
| + |
| +func TestEmptySetPermitsNothingIncludingAdmin(t *testing.T) { |
| + // The zero Set is what a door gets if someone constructs one by mistake. |
| + // It must be closed, not open, and Admin must not be an escape hatch |
| + // through it — otherwise "forgot to name a role" reads as "admin-only" |
| + // rather than as the refusal it is. |
| + var s Set |
| + if !s.Empty() { |
| + t.Fatal("zero Set.Empty() = false; want true") |
| + } |
| + for _, r := range append(All(), Role("")) { |
| + if s.Permits(r) { |
| + t.Errorf("zero Set.Permits(%q) = true; want false", r) |
| + } |
| + } |
| +} |
| + |
| +func TestUnknownRoleNeverPermitted(t *testing.T) { |
| + s := Allow(Publish, Instance, Operate, Admin) |
| + for _, r := range []Role{"", "owner", "wildcard", "*"} { |
| + if s.Permits(r) { |
| + t.Errorf("Permits(%q) = true; want false", r) |
| + } |
| + } |
| +} |
| + |
| +func TestRolesIsSortedAndDeduplicated(t *testing.T) { |
| + // Admin is always present: Allow adds it from the union definition, so a |
| + // door naming Publish twice is the set {admin, publish}, sorted. |
| + got := Allow(Operate, Publish, Publish).Roles() |
| + want := []Role{Admin, Operate, Publish} |
| + if !reflect.DeepEqual(got, want) { |
| + t.Errorf("Roles() = %v; want %v", got, want) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run test to verify it fails** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/svcrole/ -count=1` |
| +Expected: FAIL — the package does not build (`undefined: All`, `undefined: Allow`, …). |
| + |
| +- [ ] **Step 3: Write the implementation** |
| + |
| +Create `internal/svcrole/svcrole.go`: |
| + |
| +```go |
| +// Package svcrole is the closed role vocabulary for service principals: what |
| +// a credential held by a SERVER may ask the console API for. |
| +// |
| +// WHY NOT internal/caps. caps is the app-manifest vocabulary — systemd |
| +// directives, privileged ports, database paths, the host-role grant an |
| +// operator types on a box. It says what a UNIT may do ON A MACHINE. This says |
| +// what a CALLER may do AT A DOOR. Fusing them would couple a manifest format |
| +// to a permission model and give each a reason to change that has nothing to |
| +// do with the other, so they share a spirit and no types. |
| +// |
| +// This package deliberately knows NOTHING about routes. The door-to-role |
| +// binding lives at registration (internal/console's cliRoute), so there is |
| +// exactly one place a door's authority is written down, and no second table |
| +// here to drift from it. |
| +// |
| +// Spec: docs/superpowers/specs/2026-08-18-service-principals-design.md §3. |
| +package svcrole |
| + |
| +import "sort" |
| + |
| +// Role is one name from the closed catalogue. A person's cli_tokens row |
| +// carries the empty string, which is deliberately NOT a Role: authority for a |
| +// person is their membership set, evaluated per request, and no role name |
| +// must ever resolve to it. |
| +type Role string |
| + |
| +const ( |
| + // Publish is the deploy loop: claim an app, ship, promote, roll back, |
| + // manage its hostnames, retire it. |
| + Publish Role = "publish" |
| + // Instance is the LEGACY app-token authority, defined by observation of |
| + // today's call sites rather than by its name — it includes domains and |
| + // logs. See the console's role table and spec §5. |
| + Instance Role = "instance" |
| + // Operate is runtime work with no release authority: restart, config, |
| + // logs. |
| + Operate Role = "operate" |
| + // Admin is the union of the three above plus ledgers, store and place. |
| + // It is NOT "everything harmless": store grant mints a path-scoped IAM |
| + // user, which is a real handover. |
| + Admin Role = "admin" |
| +) |
| + |
| +// catalogue is the whole vocabulary, in the order All returns it. |
| +var catalogue = []Role{Publish, Instance, Operate, Admin} |
| + |
| +// All returns the catalogue. The slice is copied so a caller cannot edit the |
| +// vocabulary by writing through it. |
| +func All() []Role { |
| + out := make([]Role, len(catalogue)) |
| + copy(out, catalogue) |
| + return out |
| +} |
| + |
| +// Parse resolves a stored role name. ok is false for anything outside the |
| +// catalogue, INCLUDING the empty string a person's row carries — a caller |
| +// that treats "unparseable" as "no authority" is then correct by default. |
| +func Parse(s string) (Role, bool) { |
| + for _, r := range catalogue { |
| + if string(r) == s { |
| + return r, true |
| + } |
| + } |
| + return "", false |
| +} |
| + |
| +// Set is the roles that reach one door. The zero value permits nothing, |
| +// which is the whole point: a door whose roles were never named refuses |
| +// every service principal rather than accidentally admitting one. |
| +type Set struct { |
| + roles map[Role]bool |
| +} |
| + |
| +// Allow builds the Set for a door. Admin is never listed by a caller that |
| +// means "publish and above": it is added here, once, from the definition |
| +// that Admin is the union — so ~50 registrations cannot disagree with the |
| +// catalogue about what Admin is. A door reachable ONLY by an administrator |
| +// says Allow(Admin) explicitly and gets exactly that. |
| +func Allow(roles ...Role) Set { |
| + if len(roles) == 0 { |
| + return Set{} |
| + } |
| + s := Set{roles: make(map[Role]bool, len(roles)+1)} |
| + for _, r := range roles { |
| + if _, ok := Parse(string(r)); !ok { |
| + // A typo'd constant is a programming error, and the safe |
| + // direction is to drop it: the door ends up MORE closed, not |
| + // silently open to a name nothing can hold anyway. |
| + continue |
| + } |
| + s.roles[r] = true |
| + } |
| + if len(s.roles) == 0 { |
| + return Set{} |
| + } |
| + s.roles[Admin] = true |
| + return s |
| +} |
| + |
| +// Permits reports whether a principal holding r may pass this door. |
| +func (s Set) Permits(r Role) bool { |
| + if s.roles == nil { |
| + return false |
| + } |
| + return s.roles[r] |
| +} |
| + |
| +// Empty reports whether this Set permits nothing at all — what the |
| +// registration-coverage test asserts no door has. |
| +func (s Set) Empty() bool { return len(s.roles) == 0 } |
| + |
| +// Roles lists the Set's members, sorted, for test assertions and for the |
| +// door list the services API hands back so an operator can see what they |
| +// just granted. |
| +func (s Set) Roles() []Role { |
| + out := make([]Role, 0, len(s.roles)) |
| + for r := range s.roles { |
| + out = append(out, r) |
| + } |
| + sort.Slice(out, func(i, j int) bool { return out[i] < out[j] }) |
| + return out |
| +} |
| +``` |
| + |
| +- [ ] **Step 4: Run test to verify it passes** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/svcrole/ -count=1` |
| +Expected: PASS |
| + |
| +- [ ] **Step 5: Verify the gate and commit** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +gofmt -l internal/svcrole/ |
| +git add internal/svcrole/ |
| +git commit -m "svcrole: the closed role catalogue for service principals" |
| +``` |
| + |
| +--- |
| + |
| +### Task 2: Schema, the Principal type, and one resolver |
| + |
| +The storage half. After this task a service-principal row can exist and resolve, but nothing yet grants it anything — the doors still use the old resolvers. |
| + |
| +**Files:** |
| +- Modify: `internal/console/authstore.go:166-175` (the ALTER list), `internal/console/authstore.go:97-107` (`CLIToken`) |
| +- Create: `internal/console/svcprincipal.go` |
| +- Test: `internal/console/svcprincipal_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `svcrole.Role`, `svcrole.Parse` (Task 1); existing unexported `hashToken`, `randomToken` (`store.go:1594,1599`); existing `cliKindUser`/`cliKindApp` (`authstore.go:181-184`). |
| +- Produces: |
| + - `type PrincipalKind int`; `const (PrincipalUser PrincipalKind = iota; PrincipalService)` |
| + - `type Principal struct { Kind PrincipalKind; Address string; AccountID int64; App string; Role svcrole.Role; Name string }` |
| + - `func (s *Store) CLIPrincipal(rawToken string, now time.Time) (Principal, bool)` |
| + - `const cliKindService = "service"` |
| + - `CLIToken` gains fields `Role string`, `Name string`, `RevokedAt time.Time` |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/console/svcprincipal_test.go`. Use whatever store fixture the package already has — `authstore_test.go` opens one; follow its helper exactly rather than inventing a second. |
| + |
| +```go |
| +package console |
| + |
| +import ( |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/svcrole" |
| +) |
| + |
| +// TestCLIPrincipalLegacyAppTokenIsInstanceRole is the migration guarantee: |
| +// a row written before this change must read back with IDENTICAL authority. |
| +func TestCLIPrincipalLegacyAppTokenIsInstanceRole(t *testing.T) { |
| + s := newTestStore(t) |
| + now := time.Now() |
| + acct := mustAccount(t, s, "legacy") |
| + raw, err := s.NewAppToken(acct.ID, "titogo", "paul@example.test", "router", now) |
| + if err != nil { |
| + t.Fatalf("NewAppToken: %v", err) |
| + } |
| + p, ok := s.CLIPrincipal(raw, now) |
| + if !ok { |
| + t.Fatal("CLIPrincipal(app token) = !ok; want ok") |
| + } |
| + if p.Kind != PrincipalService { |
| + t.Errorf("Kind = %v; want PrincipalService", p.Kind) |
| + } |
| + if p.Role != svcrole.Instance { |
| + t.Errorf("Role = %q; want %q", p.Role, svcrole.Instance) |
| + } |
| + if p.AccountID != acct.ID || p.App != "titogo" { |
| + t.Errorf("scope = %d/%q; want %d/%q", p.AccountID, p.App, acct.ID, "titogo") |
| + } |
| + // The minter is a recorded fact, never the owner. |
| + if p.Address != "paul@example.test" { |
| + t.Errorf("Address = %q; want the minter", p.Address) |
| + } |
| +} |
| + |
| +func TestCLIPrincipalUserToken(t *testing.T) { |
| + s := newTestStore(t) |
| + now := time.Now() |
| + raw := mustUserToken(t, s, "paul@example.test", now) |
| + p, ok := s.CLIPrincipal(raw, now) |
| + if !ok { |
| + t.Fatal("CLIPrincipal(user token) = !ok; want ok") |
| + } |
| + if p.Kind != PrincipalUser { |
| + t.Errorf("Kind = %v; want PrincipalUser", p.Kind) |
| + } |
| + if p.Address != "paul@example.test" { |
| + t.Errorf("Address = %q", p.Address) |
| + } |
| + // A person carries no role and no scope. Anything else here would let a |
| + // role check read a person as a service with authority. |
| + if p.Role != "" || p.AccountID != 0 || p.App != "" || p.Name != "" { |
| + t.Errorf("person carries scope/role: %+v", p) |
| + } |
| +} |
| + |
| +func TestCLIPrincipalServiceToken(t *testing.T) { |
| + s := newTestStore(t) |
| + now := time.Now() |
| + acct := mustAccount(t, s, "amadan") |
| + raw, err := s.NewServicePrincipal(acct.ID, "hub", "", svcrole.Publish, "paul@example.test", now) |
| + if err != nil { |
| + t.Fatalf("NewServicePrincipal: %v", err) |
| + } |
| + p, ok := s.CLIPrincipal(raw, now) |
| + if !ok { |
| + t.Fatal("CLIPrincipal(service token) = !ok; want ok") |
| + } |
| + if p.Kind != PrincipalService || p.Role != svcrole.Publish || p.Name != "hub" { |
| + t.Errorf("got %+v", p) |
| + } |
| + if p.AccountID != acct.ID { |
| + t.Errorf("AccountID = %d; want %d", p.AccountID, acct.ID) |
| + } |
| + if p.App != "" { |
| + t.Errorf("App = %q; want account-scoped (empty)", p.App) |
| + } |
| +} |
| + |
| +// TestServiceTokenIsNotAPerson is the containment property read from the |
| +// other side: the person door must not answer for a service credential, and |
| +// its refusal must be the one a fabricated token gets. |
| +func TestServiceTokenIsNotAPerson(t *testing.T) { |
| + s := newTestStore(t) |
| + now := time.Now() |
| + acct := mustAccount(t, s, "amadan") |
| + raw, err := s.NewServicePrincipal(acct.ID, "hub", "", svcrole.Publish, "paul@example.test", now) |
| + if err != nil { |
| + t.Fatalf("NewServicePrincipal: %v", err) |
| + } |
| + if addr, ok := s.CLITokenAddress(raw, now); ok { |
| + t.Errorf("CLITokenAddress(service token) = %q,true; want \"\",false", addr) |
| + } |
| +} |
| + |
| +func TestRevokedServicePrincipalStopsResolving(t *testing.T) { |
| + s := newTestStore(t) |
| + now := time.Now() |
| + acct := mustAccount(t, s, "amadan") |
| + raw, err := s.NewServicePrincipal(acct.ID, "hub", "", svcrole.Publish, "paul@example.test", now) |
| + if err != nil { |
| + t.Fatalf("NewServicePrincipal: %v", err) |
| + } |
| + if err := s.RevokeServicePrincipal(acct.ID, "hub"); err != nil { |
| + t.Fatalf("RevokeServicePrincipal: %v", err) |
| + } |
| + if _, ok := s.CLIPrincipal(raw, now); ok { |
| + t.Error("CLIPrincipal(revoked) = ok; want !ok") |
| + } |
| +} |
| + |
| +// TestRotateKeepsTheNameAndKillsTheOldSecret is why `name` exists as a column |
| +// separate from the token hash: rate-limit keys and audit rows stay |
| +// continuous across a secret change, and the old secret stops working in the |
| +// same call. |
| +func TestRotateKeepsTheNameAndKillsTheOldSecret(t *testing.T) { |
| + s := newTestStore(t) |
| + now := time.Now() |
| + acct := mustAccount(t, s, "amadan") |
| + old, err := s.NewServicePrincipal(acct.ID, "hub", "", svcrole.Publish, "paul@example.test", now) |
| + if err != nil { |
| + t.Fatalf("NewServicePrincipal: %v", err) |
| + } |
| + fresh, err := s.RotateServicePrincipal(acct.ID, "hub", now) |
| + if err != nil { |
| + t.Fatalf("RotateServicePrincipal: %v", err) |
| + } |
| + if fresh == old { |
| + t.Fatal("rotate returned the same secret") |
| + } |
| + if _, ok := s.CLIPrincipal(old, now); ok { |
| + t.Error("old secret still resolves after rotate") |
| + } |
| + p, ok := s.CLIPrincipal(fresh, now) |
| + if !ok { |
| + t.Fatal("fresh secret does not resolve") |
| + } |
| + if p.Name != "hub" || p.Role != svcrole.Publish { |
| + t.Errorf("rotate changed identity: %+v", p) |
| + } |
| +} |
| + |
| +// TestNameIsUniquePerAccountAmongLiveRows — the name is the handle every |
| +// other surface addresses, so two live rows sharing one would make rotate and |
| +// revoke ambiguous. |
| +func TestNameIsUniquePerAccountAmongLiveRows(t *testing.T) { |
| + s := newTestStore(t) |
| + now := time.Now() |
| + acct := mustAccount(t, s, "amadan") |
| + if _, err := s.NewServicePrincipal(acct.ID, "hub", "", svcrole.Publish, "paul@example.test", now); err != nil { |
| + t.Fatalf("first mint: %v", err) |
| + } |
| + if _, err := s.NewServicePrincipal(acct.ID, "hub", "", svcrole.Operate, "paul@example.test", now); err == nil { |
| + t.Fatal("second mint with the same name succeeded; want ErrServiceNameTaken") |
| + } |
| + // A different account may hold the same name: the namespace is per |
| + // account, like an app name, not global like a fleet name. |
| + other := mustAccount(t, s, "other") |
| + if _, err := s.NewServicePrincipal(other.ID, "hub", "", svcrole.Publish, "paul@example.test", now); err != nil { |
| + t.Errorf("same name in another account: %v", err) |
| + } |
| +} |
| +``` |
| + |
| +Add the two fixture helpers if the package does not already have equivalents (check `authstore_test.go` and `store_test.go` first and reuse theirs if so): |
| + |
| +```go |
| +func mustAccount(t *testing.T, s *Store, name string) *Account { |
| + t.Helper() |
| + // Use whatever account-creation helper the package's existing tests use. |
| + // If none exists, s.CreateAccount(name, "paul@example.test", time.Now()) |
| + // is the constructor the console itself calls. |
| + a, err := s.CreateAccount(name, "paul@example.test", time.Now()) |
| + if err != nil { |
| + t.Fatalf("CreateAccount(%q): %v", name, err) |
| + } |
| + return a |
| +} |
| + |
| +func mustUserToken(t *testing.T, s *Store, addr string, now time.Time) string { |
| + t.Helper() |
| + // Mint through the ordinary approval flow the package's own tests use |
| + // (NewCLIAuth -> approve -> CLIAuthContinue). Do NOT insert a row by |
| + // hand: the point of this fixture is that a real person's token and a |
| + // service's differ only in the columns under test. |
| + ... |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run test to verify it fails** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -run 'CLIPrincipal|ServicePrincipal|ServiceToken|RotateKeeps|NameIsUnique' -count=1` |
| +Expected: FAIL — `undefined: CLIPrincipal`, `undefined: NewServicePrincipal`, … |
| + |
| +- [ ] **Step 3: Extend the schema** |
| + |
| +In `internal/console/authstore.go`, add three lines to the existing ALTER list (`authstore.go:166-175`), inside the same swallowed-duplicate loop: |
| + |
| +```go |
| + `ALTER TABLE cli_tokens ADD COLUMN role TEXT NOT NULL DEFAULT ''`, |
| + `ALTER TABLE cli_tokens ADD COLUMN name TEXT NOT NULL DEFAULT ''`, |
| + `ALTER TABLE cli_tokens ADD COLUMN revoked_at INTEGER NOT NULL DEFAULT 0`, |
| +``` |
| + |
| +Extend the list's existing comment: every one of these defaults to the value a pre-change row already means — no role, no name, not revoked. |
| + |
| +Add the index the name lookup needs, in the `CREATE INDEX IF NOT EXISTS` group above: |
| + |
| +```go |
| + `CREATE INDEX IF NOT EXISTS cli_tokens_account_name ON cli_tokens (account_id, name);`, |
| +``` |
| + |
| +Extend `CLIToken` (`authstore.go:97-107`) with: |
| + |
| +```go |
| + Role string // service principals only: a svcrole catalogue name |
| + Name string // service principals only: the stable handle |
| + RevokedAt time.Time // zero unless the credential has been revoked |
| +``` |
| + |
| +and update `CLITokens`' `SELECT`/`Scan` to carry them. |
| + |
| +Add the third kind beside the existing two (`authstore.go:181-184`): |
| + |
| +```go |
| + cliKindService = "service" |
| +``` |
| + |
| +- [ ] **Step 4: Write the resolver and the store methods** |
| + |
| +Create `internal/console/svcprincipal.go`. The load-bearing decisions, each of which has a test above: |
| + |
| +```go |
| +// Service principals: the console API's non-address caller. |
| +// |
| +// A console API token has always resolved to a Keymail address, and an |
| +// address's authority is the union of its memberships evaluated at request |
| +// time (apiAccount). That is right for a human at a terminal and wrong for a |
| +// server holding a credential: the reach grows without the holder acting, and |
| +// the rate limits are the operator's own. This file is the principal that is |
| +// not an address. |
| +// |
| +// It WIDENS an existing model rather than inventing one. cli_tokens has |
| +// carried kind='app' — bound to one account and one app, minted by a member, |
| +// never expiring — since the instances work, and apiInstancePrincipal has |
| +// gated six doors on it. A legacy row reads back here as an app-scoped |
| +// service principal holding role `instance`, which is defined by observation |
| +// of what those tokens reach TODAY (instances, domains, logs — spec §5), not |
| +// by what the word sounds like. |
| +// |
| +// Spec: docs/superpowers/specs/2026-08-18-service-principals-design.md §2. |
| +package console |
| + |
| +// PrincipalKind separates the two things a bearer token can be. It is a typed |
| +// constant rather than a string compare at each call site so that "is this a |
| +// person?" cannot be written two subtly different ways in two files. |
| +type PrincipalKind int |
| + |
| +const ( |
| + PrincipalUser PrincipalKind = iota |
| + PrincipalService |
| +) |
| + |
| +// Principal is one resolved caller. |
| +// |
| +// Address is the CALLER for a person and the MINTER for a service — a fact |
| +// recorded so a human stays answerable, never an authority. Nothing in the |
| +// gate path may read a service's Address as a set of memberships; that |
| +// mistake is exactly what issue #220 is about. |
| +type Principal struct { |
| + Kind PrincipalKind |
| + Address string |
| + AccountID int64 |
| + App string // "" means account-scoped |
| + Role svcrole.Role |
| + Name string |
| +} |
| + |
| +// ErrServiceNameTaken is returned by NewServicePrincipal when the account |
| +// already holds a live credential of that name. |
| +var ErrServiceNameTaken = errors.New("console: a service credential of that name already exists") |
| +``` |
| + |
| +`CLIPrincipal(rawToken, now)`: |
| +1. `hash := hashToken(rawToken)`; empty token → `Principal{}, false`. |
| +2. `SELECT token_hash, address, kind, account_id, app, role, name, expires_at, revoked_at FROM cli_tokens WHERE token_hash = ?`. |
| +3. `subtle.ConstantTimeCompare` on the hash, as `CLIAppToken` does. |
| +4. `revoked_at != 0` → `false`. |
| +5. Expiry: `expires_at == 0` means never (app and service tokens); otherwise `now.Unix() <= expires_at`. |
| +6. Switch on kind: |
| + - `cliKindUser` → `Principal{Kind: PrincipalUser, Address: addr}` and **nothing else populated**. |
| + - `cliKindApp` → `Principal{Kind: PrincipalService, Address: addr, AccountID: …, App: …, Role: svcrole.Instance, Name: ""}`. |
| + - `cliKindService` → parse `role` through `svcrole.Parse`; **an unparseable role is `false`, not a role-less service** — a row whose role name this binary does not know must refuse, never fall through to a gate that treats `Role: ""` as anything. |
| + - anything else → `false`. |
| +7. Best-effort `UPDATE cli_tokens SET last_used = ?`, as the existing resolvers do. |
| + |
| +Also in this file, `CLITokenAddress` needs one added condition: skip rows with `revoked_at != 0`. Add it to the existing method in `authstore.go` rather than duplicating the query. |
| + |
| +`NewServicePrincipal(accountID int64, name, app string, role svcrole.Role, mintedBy string, now time.Time) (string, error)`: |
| +- Validate `role` through `svcrole.Parse`; refuse otherwise. |
| +- In a transaction: `SELECT COUNT(*) FROM cli_tokens WHERE kind = ? AND account_id = ? AND name = ? AND revoked_at = 0`; non-zero → `ErrServiceNameTaken`. |
| +- `INSERT … (token_hash, address, label, remote_addr, created_at, expires_at, kind, account_id, app, role, name) VALUES (?, ?, ?, '', ?, 0, 'service', ?, ?, ?, ?)` with `expires_at = 0` — never expires, the same deliberate trade `NewAppToken` documents: a production publish path that silently dies on day 91 is a worse failure than a long-lived credential whose rotation is one command. Revocation is the control. |
| +- Return the plaintext. Only the hash is stored; there is no second copy. |
| + |
| +`ServicePrincipals(accountID int64) ([]CLIToken, error)`: live rows (`revoked_at = 0`) of kind `service` **and** kind `app` for that account, newest first — the legacy rows must appear in the account's listing or absorbing them would hide them. |
| + |
| +`RotateServicePrincipal(accountID int64, name string, now time.Time) (string, error)`: in one transaction, mint a fresh secret and `UPDATE cli_tokens SET token_hash = ?, created_at = ?, last_used = 0 WHERE kind = 'service' AND account_id = ? AND name = ? AND revoked_at = 0`. Zero rows affected → an error the door renders as 404. Same row, same name, same role: only the secret moves. |
| + |
| +`RevokeServicePrincipal(accountID int64, name string) error`: `UPDATE cli_tokens SET revoked_at = ? WHERE account_id = ? AND name = ? AND revoked_at = 0`. Soft, not `DELETE`, so `ServicePrincipals` can show a revoked credential's last-used time to whoever is working out what broke. |
| + |
| +- [ ] **Step 5: Run the tests** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -run 'CLIPrincipal|ServicePrincipal|ServiceToken|RotateKeeps|NameIsUnique' -count=1` |
| +Expected: PASS |
| + |
| +- [ ] **Step 6: Run the whole package — the migration must not disturb anything** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -count=1` |
| +Expected: PASS. Any existing test that fails here is telling you the schema change was not additive — fix the change, not the test. |
| + |
| +- [ ] **Step 7: Commit** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +gofmt -l internal/console/svcprincipal.go internal/console/authstore.go |
| +git add internal/console/svcprincipal.go internal/console/svcprincipal_test.go internal/console/authstore.go |
| +git commit -m "console: a principal that is not an address" |
| +``` |
| + |
| +--- |
| + |
| +### Task 3: The structural role gate |
| + |
| +The heart of the change. After this task every `/api/cli/` door names its roles at registration and a service principal that lacks the role is refused before the handler runs. |
| + |
| +**Note the gate order changed during planning** (spec §3b, commit `8d3e151`): role is checked BEFORE scope, in the wrapper. A role refusal reads neither the account segment nor the database, so it is a constant function of the door and the caller's own role and leaks nothing. Scope-first would have made the 404/403 pair a membership oracle. |
| + |
| +**Files:** |
| +- Modify: `internal/console/authcli.go:202-330` (`registerCLIRoutes`), and the `Server` struct wherever it is declared |
| +- Create: `internal/console/svcroutes_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `svcrole.Set`, `svcrole.Allow`, `svcrole.Role` (Task 1); `Principal`, `Store.CLIPrincipal` (Task 2). |
| +- Produces: |
| + - `func (s *Server) cliRoute(pattern string, allow svcrole.Set, h http.HandlerFunc)` |
| + - `func principalFrom(ctx context.Context) (Principal, bool)` — reads the context value `cliRoute` set |
| + - `Server` field `cliRoles map[string]svcrole.Set` — pattern → allowed set, for the coverage tests and the services API's door listing |
| + |
| +- [ ] **Step 1: Write the failing tests** |
| + |
| +Create `internal/console/svcroutes_test.go`: |
| + |
| +```go |
| +package console |
| + |
| +import ( |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/svcrole" |
| +) |
| + |
| +// mintDoors and fleetDoors are the two families no role may ever reach. |
| +// Listed as substrings of the registered pattern, because that is the shape |
| +// the table is keyed on. |
| +var ( |
| + mintDoors = []string{"/instance-token", "/api/cli/services/"} |
| + fleetDoors = []string{"/api/cli/fleets/"} |
| +) |
| + |
| +// TestEveryCLIDoorNamesItsRoles is the default-deny guarantee for the |
| +// /api/cli/ half. A door registered with plain mux.HandleFunc would not |
| +// appear in cliRoles at all, and a door registered with an empty Set is |
| +// unreachable by every service principal — both are failures worth catching |
| +// at build time rather than in production. |
| +func TestEveryCLIDoorNamesItsRoles(t *testing.T) { |
| + s := newTestServer(t) |
| + for pattern, set := range s.cliRoles { |
| + if set.Empty() { |
| + t.Errorf("%s: registered with an empty role set", pattern) |
| + } |
| + } |
| + for _, pattern := range registeredCLIPatterns(t, s) { |
| + if _, ok := s.cliRoles[pattern]; !ok { |
| + t.Errorf("%s: registered without going through cliRoute", pattern) |
| + } |
| + } |
| +} |
| + |
| +// TestNoRoleReachesAMintOrFleetDoor is the escalation cut: a credential must |
| +// never be able to mint or rotate a credential — including itself — and must |
| +// never enrol a box. |
| +func TestNoRoleReachesAMintOrFleetDoor(t *testing.T) { |
| + s := newTestServer(t) |
| + for pattern, set := range s.cliRoles { |
| + for _, closed := range append(append([]string{}, mintDoors...), fleetDoors...) { |
| + if !strings.Contains(pattern, closed) { |
| + continue |
| + } |
| + for _, r := range svcrole.All() { |
| + if set.Permits(r) { |
| + t.Errorf("%s is reachable by role %q; mint and fleet doors must be closed to every role", pattern, r) |
| + } |
| + } |
| + } |
| + } |
| +} |
| + |
| +// TestInstanceRoleMatchesTodaysAppTokenReach is the compatibility pin of spec |
| +// §5. kind='app' tokens today reach the instance verbs AND domains AND logs — |
| +// titogo's router re-attaches every hostname on every reload pass with its |
| +// CARLOS_INSTANCE_TOKEN. A tidy `instance` role meaning "the instance verbs" |
| +// would 403 that router on the next console deploy. |
| +// |
| +// If this test fails because a door moved, that is the point: decide |
| +// deliberately, then update BOTH this list and the role table. |
| +func TestInstanceRoleMatchesTodaysAppTokenReach(t *testing.T) { |
| + want := map[string]bool{ |
| + "PUT /api/cli/apps/{account}/{app}/instances-manifest": true, |
| + "GET /api/cli/apps/{account}/{app}/instances": true, |
| + "POST /api/cli/apps/{account}/{app}/instances": true, |
| + "GET /api/cli/apps/{account}/{app}/instances/{host}": true, |
| + "PATCH /api/cli/apps/{account}/{app}/instances/{host}": true, |
| + "DELETE /api/cli/apps/{account}/{app}/instances/{host}": true, |
| + "GET /api/cli/apps/{account}/{app}/domains": true, |
| + "POST /api/cli/apps/{account}/{app}/domains": true, |
| + "DELETE /api/cli/apps/{account}/{app}/domains/{host}": true, |
| + "GET /api/cli/apps/{account}/{app}/logs": true, |
| + } |
| + s := newTestServer(t) |
| + got := map[string]bool{} |
| + for pattern, set := range s.cliRoles { |
| + if set.Permits(svcrole.Instance) { |
| + got[pattern] = true |
| + } |
| + } |
| + for p := range want { |
| + if !got[p] { |
| + t.Errorf("%s: role instance does not reach it, but an app token does today", p) |
| + } |
| + } |
| + for p := range got { |
| + if !want[p] { |
| + t.Errorf("%s: role instance reaches it, but an app token does not today — widening the legacy role", p) |
| + } |
| + } |
| +} |
| +``` |
| + |
| +`registeredCLIPatterns` needs the set of patterns actually registered on the mux. Go's `http.ServeMux` does not enumerate; so make `cliRoute` the recorder and add a **second** guard instead: a source-level test that no `/api/cli/` string is passed to `mux.HandleFunc` anywhere in the package. |
| + |
| +```go |
| +// registeredCLIPatterns reads the package's own source rather than the mux, |
| +// because http.ServeMux cannot be enumerated. The check it enables is worth |
| +// the unusual shape: it catches a door added tomorrow with plain |
| +// mux.HandleFunc, which is the exact way default-deny would be lost. |
| +func registeredCLIPatterns(t *testing.T, s *Server) []string { |
| + t.Helper() |
| + src, err := os.ReadFile("authcli.go") |
| + if err != nil { |
| + t.Fatalf("read authcli.go: %v", err) |
| + } |
| + var out []string |
| + for _, line := range strings.Split(string(src), "\n") { |
| + if !strings.Contains(line, "/api/cli/") { |
| + continue |
| + } |
| + if strings.Contains(line, "s.mux.HandleFunc(") { |
| + t.Errorf("door registered with mux.HandleFunc, bypassing the role gate:\n %s", strings.TrimSpace(line)) |
| + continue |
| + } |
| + if !strings.Contains(line, "s.cliRoute(") { |
| + continue |
| + } |
| + // The pattern is the first quoted string on the line. |
| + i := strings.Index(line, `"`) |
| + j := strings.Index(line[i+1:], `"`) |
| + if i < 0 || j < 0 { |
| + continue |
| + } |
| + out = append(out, line[i+1:i+1+j]) |
| + } |
| + if len(out) < 40 { |
| + t.Fatalf("found only %d cliRoute patterns; the parser is broken, not the code", len(out)) |
| + } |
| + return out |
| +} |
| +``` |
| + |
| +Also add the `/auth/cli/` half of default-deny from spec §3a-i: |
| + |
| +```go |
| +// TestAuthCLIFamilyResolvesOnlyPersons is the OTHER half of default-deny. |
| +// The /auth/cli/ handlers — whoami, revoke, tokens/revoke, accounts (which |
| +// creates an ACCOUNT) — sit outside cliRoute and need no role bindings, |
| +// because every one of them gates on cliUser, which answers only for |
| +// kind=''. That is currently true by construction; this makes it true by |
| +// test, so the day someone reaches for CLIPrincipal over there the build |
| +// says so. |
| +func TestAuthCLIFamilyResolvesOnlyPersons(t *testing.T) { |
| + src, err := os.ReadFile("authcli.go") |
| + if err != nil { |
| + t.Fatalf("read authcli.go: %v", err) |
| + } |
| + // Every handler named on an /auth/cli/ registration line must be one |
| + // whose body calls s.cliUser and never s.store.CLIPrincipal. Walk the |
| + // registrations, collect the handler names, then assert on their bodies. |
| + // (Implementation: a small go/ast pass over the package, or a targeted |
| + // scan of the handler funcs by name — either is fine, the assertion is |
| + // what matters.) |
| + ... |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run the tests to verify they fail** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -run 'CLIDoor|MintOrFleet|InstanceRoleMatches|AuthCLIFamily' -count=1` |
| +Expected: FAIL — `s.cliRoles` undefined. |
| + |
| +- [ ] **Step 3: Add cliRoute and the context carrier** |
| + |
| +In `internal/console/authcli.go`, above `registerCLIRoutes`: |
| + |
| +```go |
| +// cliRouteKey is the context key cliRoute stashes the resolved principal |
| +// under. Unexported and of an unexported type, so nothing outside this |
| +// package can forge one onto a request. |
| +type cliRouteKey struct{} |
| + |
| +// principalFrom reads the principal cliRoute resolved. ok is false only for a |
| +// request that did not come through cliRoute — which, for anything under |
| +// /api/cli/, the coverage test says cannot happen. |
| +func principalFrom(ctx context.Context) (Principal, bool) { |
| + p, ok := ctx.Value(cliRouteKey{}).(Principal) |
| + return p, ok |
| +} |
| + |
| +// cliRoute registers one /api/cli/ door and is the ONLY way to register one. |
| +// It resolves the bearer once, applies the role gate, and passes the |
| +// principal down on the request context so the handler's own resolver |
| +// (apiAccount) does not repeat the lookup. |
| +// |
| +// THE ROLE GATE RUNS BEFORE THE HANDLER, AND THEREFORE BEFORE THE SCOPE |
| +// CHECK — spec §3b. A role refusal reads neither the account segment nor the |
| +// database: it is a constant function of the door and the caller's own role, |
| +// identical whether the named account exists, belongs to someone else, or |
| +// belongs to the caller. There is nothing in it to leak. Checking scope first |
| +// would instead make the pair of answers a membership oracle — 404 for an |
| +// account you do not hold, 403 for one you do. |
| +// |
| +// `allow` is never optional and the empty Set permits nothing, so a door |
| +// added without naming its roles is unreachable by every service principal |
| +// rather than open to all of them. |
| +func (s *Server) cliRoute(pattern string, allow svcrole.Set, h http.HandlerFunc) { |
| + if s.cliRoles == nil { |
| + s.cliRoles = map[string]svcrole.Set{} |
| + } |
| + s.cliRoles[pattern] = allow |
| + s.mux.HandleFunc(pattern, func(w http.ResponseWriter, r *http.Request) { |
| + p, ok := s.store.CLIPrincipal(bearer(r), time.Now()) |
| + if !ok { |
| + // The same 401, with the same body, that cliUser has always |
| + // given: an expired token, a revoked one and a fabricated one |
| + // are not distinguishable from outside. |
| + jsonError(w, http.StatusUnauthorized, "not signed in — run: carlos auth login -console "+s.cfg.Origin) |
| + return |
| + } |
| + if p.Kind == PrincipalService && !allow.Permits(p.Role) { |
| + jsonError(w, http.StatusForbidden, fmt.Sprintf( |
| + "this credential holds role %q; this door needs one of %v", p.Role, allow.Roles())) |
| + return |
| + } |
| + h(w, r.WithContext(context.WithValue(r.Context(), cliRouteKey{}, p))) |
| + }) |
| +} |
| +``` |
| + |
| +Add the field to `Server`: |
| + |
| +```go |
| + // cliRoles is pattern -> allowed roles, recorded by cliRoute. It exists |
| + // for two readers: the coverage tests, and the services API, which hands |
| + // an operator the door list for the role they just granted so they can |
| + // see what they gave away without reading a spec. |
| + cliRoles map[string]svcrole.Set |
| +``` |
| + |
| +- [ ] **Step 4: Rewrite every /api/cli/ registration** |
| + |
| +In `registerCLIRoutes`, change each `s.mux.HandleFunc("… /api/cli/…", h)` to `s.cliRoute("… /api/cli/…", svcrole.Allow(…), h)`. **Leave every `/auth/cli/` line and the `GET /settings` line exactly as they are** — they are the other half of default-deny and gate on `cliUser`. Keep all existing comments in place. |
| + |
| +The bindings, in registration order: |
| + |
| +| pattern | `svcrole.Allow(...)` | |
| +|---|---| |
| +| `GET /api/cli/apps` | `Publish, Instance, Operate` | |
| +| `POST /api/cli/apps/{account}/new` | `Publish` | |
| +| `GET …/{app}/channels` | `Publish` | |
| +| `POST …/{app}/promote` | `Publish` | |
| +| `POST …/{app}/rollback` | `Publish` | |
| +| `POST …/{app}/releases` | `Publish` | |
| +| `POST …/{app}/restart` | `Operate` | |
| +| `GET …/{app}/retention` | `Publish` | |
| +| `POST …/{app}/retention` | `Publish` | |
| +| `GET …/{app}/version-target` | `Publish` | |
| +| `PUT …/{app}/version-target` | `Publish` | |
| +| `GET …/{app}/tags` | `Publish` | |
| +| `GET …/{app}/tags/{version}` | `Publish` | |
| +| `GET …/{app}/releases` | `Publish` | |
| +| `POST …/{app}/delete` | `Publish` | |
| +| `POST …/{app}/restore` | `Publish` | |
| +| `GET …/{app}/ledgers` | `Admin` | |
| +| `POST …/{app}/ledgers/publish` | `Admin` | |
| +| `GET …/{app}/ledgers/{ledger}/head` | `Admin` | |
| +| `POST …/{app}/ledgers/{ledger}/append` | `Admin` | |
| +| `POST …/{app}/ledgers/{ledger}/blobs` | `Admin` | |
| +| `PUT …/{app}/instances-manifest` | `Instance` | |
| +| `GET …/{app}/instances` | `Instance` | |
| +| `GET …/{app}/logs` | `Instance, Operate` | |
| +| `POST …/{app}/instances` | `Instance` | |
| +| `GET …/{app}/instances/{host}` | `Instance` | |
| +| `PATCH …/{app}/instances/{host}` | `Instance` | |
| +| `DELETE …/{app}/instances/{host}` | `Instance` | |
| +| `POST …/{app}/instance-token` | `svcrole.Set{}` — the zero Set, deliberately | |
| +| `GET …/{app}/domains` | `Publish, Instance` | |
| +| `POST …/{app}/domains` | `Publish, Instance` | |
| +| `DELETE …/{app}/domains/{host}` | `Publish, Instance` | |
| +| `GET …/{app}/config` | `Operate` | |
| +| `PUT …/{app}/config` | `Operate` | |
| +| `POST …/{app}/config/sync` | `Operate` | |
| +| `POST /api/cli/fleets/{account}/new` | `svcrole.Set{}` | |
| +| `GET /api/cli/fleets/{account}` | `svcrole.Set{}` | |
| +| `POST /api/cli/fleets/{account}/{fleet}/boxes` | `svcrole.Set{}` | |
| +| `POST …/boxes/{label}/rotate` | `svcrole.Set{}` | |
| +| `POST …/boxes/{label}/detach` | `svcrole.Set{}` | |
| +| `POST …/boxes/{label}/tags` | `svcrole.Set{}` | |
| +| `POST …/{app}/place` | `Admin` | |
| +| `POST …/{app}/store` | `Admin` | |
| +| `GET …/{app}/store` | `Admin` | |
| +| `POST …/{app}/store/grant` | `Admin` | |
| +| `POST …/{app}/store/rotate` | `Admin` | |
| +| `POST …/{app}/store/rotate/finish` | `Admin` | |
| + |
| +Two of those need a comment at the line, because the reason is not visible from the pattern: |
| + |
| +```go |
| + // The zero Set, deliberately: a credential may never mint a credential, |
| + // including itself. This is the escalation cut (spec §3) and it is |
| + // already true today — handleAPIInstanceToken gates on apiAccountApp, |
| + // which refuses app tokens at cliUser — so this preserves behaviour |
| + // rather than adding a restriction. |
| + s.cliRoute("POST /api/cli/apps/{account}/{app}/instance-token", svcrole.Set{}, s.handleAPIInstanceToken) |
| +``` |
| + |
| +```go |
| + // Domains carries `Instance` for COMPATIBILITY, not because it belongs |
| + // there: titogo's router re-attaches every hostname on every reload pass |
| + // with its CARLOS_INSTANCE_TOKEN, which is a kind='app' row and so holds |
| + // role instance. Dropping it here 403s that router on the next console |
| + // deploy. Same for logs, below. Pinned by |
| + // TestInstanceRoleMatchesTodaysAppTokenReach; spec §5. |
| +``` |
| + |
| +- [ ] **Step 5: Run the tests** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -count=1` |
| +Expected: PASS. Existing API tests still pass because a user principal never meets the role gate (`p.Kind == PrincipalService` guards it) and the 401 body is unchanged. |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +gofmt -l internal/console/ |
| +git add internal/console/authcli.go internal/console/svcroutes_test.go internal/console/server.go |
| +git commit -m "console: the role gate is structural — a door that names no roles is closed" |
| +``` |
| + |
| +--- |
| + |
| +### Task 4: Scope resolution for a principal |
| + |
| +The gate now refuses on role. This task makes the *scope* half principal-aware, so a service reaches its own account and nothing else, and collapses `apiInstancePrincipal` into the general path. |
| + |
| +**Files:** |
| +- Modify: `internal/console/apicli.go:60-133` (`apiAccountAppBy`, `apiAccount`), `internal/console/apicli.go` (`handleAPIApps`) |
| +- Modify: `internal/console/apiinstances.go:52-97` (`apiInstancePrincipal`), `:98-120` (`instanceGate`) |
| +- Test: `internal/console/apicli_test.go` (extend), `internal/console/svcprincipal_test.go` (extend) |
| + |
| +**Interfaces:** |
| +- Consumes: `principalFrom` (Task 3), `Principal` (Task 2). |
| +- Produces: |
| + - `apiAccount` keeps its signature `(string, *Account, bool)` — first return is now the **audit actor**: the address for a person, `service:<sqid>/<name>` for a service. Every existing caller already treats it as "who to audit as", so no call site changes. |
| + - `func (s *Server) apiPrincipalAccount(w, r) (Principal, *Account, bool)` — the typed form, for the callers that need the role or the budget key. |
| + |
| +- [ ] **Step 1: Write the failing tests** |
| + |
| +Extend `internal/console/svcprincipal_test.go`: |
| + |
| +```go |
| +// TestServiceReachesOnlyItsOwnAccount is consequence 2 of issue #220 closed: |
| +// the reach is a property of the credential, fixed at mint, not of a person's |
| +// membership set evaluated per request. |
| +func TestServiceReachesOnlyItsOwnAccount(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + mine := fx.account("mine") |
| + theirs := fx.account("theirs") |
| + tok := fx.serviceToken(mine, "hub", "", svcrole.Publish) |
| + |
| + // Its own account: through. |
| + if code := fx.get(tok, "/api/cli/apps/"+mine.Sqid+"/app/channels"); code != 200 { |
| + t.Errorf("own account = %d; want 200", code) |
| + } |
| + // Another account, which the MINTER may well belong to: the byte- |
| + // identical 404. This is the whole point — the minter's memberships are |
| + // not consulted. |
| + if code := fx.get(tok, "/api/cli/apps/"+theirs.Sqid+"/app/channels"); code != 404 { |
| + t.Errorf("other account = %d; want 404", code) |
| + } |
| +} |
| + |
| +// TestServiceScopeIsSqidOnly keeps apiInstancePrincipal's existing rule. A |
| +// service holds no memberships, so there is no set within which to resolve a |
| +// NAME; resolving globally and then comparing would be a name-collision |
| +// oracle for a credential that never needed names. |
| +func TestServiceScopeIsSqidOnly(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + mine := fx.account("mine") // name "mine", sqid something else |
| + tok := fx.serviceToken(mine, "hub", "", svcrole.Publish) |
| + if code := fx.get(tok, "/api/cli/apps/"+mine.Sqid+"/app/channels"); code != 200 { |
| + t.Errorf("by sqid = %d; want 200", code) |
| + } |
| + if code := fx.get(tok, "/api/cli/apps/mine/app/channels"); code != 404 { |
| + t.Errorf("by name = %d; want 404 (sqid-only)", code) |
| + } |
| +} |
| + |
| +// TestAppScopedServiceCannotLeaveItsApp — the app binding is exact or it is a |
| +// 404, and an empty binding is never a wildcard. |
| +func TestAppScopedServiceCannotLeaveItsApp(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + fx.app(acct, "one") |
| + fx.app(acct, "two") |
| + tok := fx.serviceToken(acct, "onlyone", "one", svcrole.Publish) |
| + if code := fx.get(tok, "/api/cli/apps/"+acct.Sqid+"/one/channels"); code != 200 { |
| + t.Errorf("bound app = %d; want 200", code) |
| + } |
| + if code := fx.get(tok, "/api/cli/apps/"+acct.Sqid+"/two/channels"); code != 404 { |
| + t.Errorf("sibling app = %d; want 404", code) |
| + } |
| +} |
| + |
| +// TestRoleRefusalIsConstantAcrossAccounts is the neutrality property that |
| +// justifies role-before-scope (spec §3b): the 403 must be byte-identical |
| +// whether the named account exists, belongs to someone else, or belongs to |
| +// the caller. If these three ever differ, the 403 has become an oracle. |
| +func TestRoleRefusalIsConstantAcrossAccounts(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + mine := fx.account("mine") |
| + theirs := fx.account("theirs") |
| + tok := fx.serviceToken(mine, "hub", "", svcrole.Publish) // publish cannot restart |
| + |
| + var bodies []string |
| + for _, seg := range []string{mine.Sqid, theirs.Sqid, "bqqqqqqq"} { |
| + code, body := fx.post(tok, "/api/cli/apps/"+seg+"/app/restart", nil) |
| + if code != 403 { |
| + t.Fatalf("account %q: code = %d; want 403", seg, code) |
| + } |
| + bodies = append(bodies, body) |
| + } |
| + for i := 1; i < len(bodies); i++ { |
| + if bodies[i] != bodies[0] { |
| + t.Errorf("403 body differs by account:\n %q\n %q", bodies[0], bodies[i]) |
| + } |
| + } |
| +} |
| + |
| +// TestAppsListForAServiceIsItsOwnScope — spec §3a-i. GET /api/cli/apps has no |
| +// {account} segment: for a person it is every app in every account they |
| +// belong to, and a service has no such set. |
| +func TestAppsListForAServiceIsItsOwnScope(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + mine := fx.account("mine") |
| + theirs := fx.account("theirs") |
| + fx.app(mine, "one") |
| + fx.app(mine, "two") |
| + fx.app(theirs, "secret") |
| + |
| + acctTok := fx.serviceToken(mine, "hub", "", svcrole.Publish) |
| + got := fx.apps(acctTok) |
| + if !sameSet(got, []string{mine.Sqid + "/one", mine.Sqid + "/two"}) { |
| + t.Errorf("account-scoped service sees %v; want its own account's apps only", got) |
| + } |
| + |
| + appTok := fx.serviceToken(mine, "just-one", "one", svcrole.Publish) |
| + if got := fx.apps(appTok); !sameSet(got, []string{mine.Sqid + "/one"}) { |
| + t.Errorf("app-scoped service sees %v; want just its own app", got) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run to verify they fail** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -run 'ServiceReaches|ServiceScopeIsSqid|AppScopedService|RoleRefusalIsConstant|AppsListForAService' -count=1` |
| +Expected: FAIL — a service token currently 401s at `cliUser` inside `apiAccount`. |
| + |
| +- [ ] **Step 3: Make apiAccount principal-aware** |
| + |
| +Rewrite `apiAccount` (`apicli.go:91`). Keep the doc comment's existing paragraphs on symmetric resolution and `?exact=sqid`, and add the service branch above them: |
| + |
| +```go |
| +func (s *Server) apiPrincipalAccount(w http.ResponseWriter, r *http.Request) (Principal, *Account, bool) { |
| + p, ok := principalFrom(r.Context()) |
| + if !ok { |
| + // Only reachable for a door that skipped cliRoute, which the |
| + // coverage test says cannot happen. Refuse rather than fall back to |
| + // a second resolver: a silent fallback here would be the one path |
| + // on which the role gate does not run. |
| + http.NotFound(w, r) |
| + return Principal{}, nil, false |
| + } |
| + seg := r.PathValue("account") |
| + |
| + if p.Kind == PrincipalService { |
| + // A service's scope is a BINDING, not a membership set, so it is |
| + // matched as a sqid ONLY. It holds no memberships, so there is no |
| + // set within which to resolve a name; resolving globally and then |
| + // comparing would be a name-collision oracle for a credential that |
| + // never needed names. The binding is exact or it is a 404 — and |
| + // nothing here treats an absent account id or an empty app as a |
| + // wildcard, which is the failure mode a "bound to nothing means |
| + // bound to everything" bug would take. |
| + acct, err := s.store.AccountByID(p.AccountID) |
| + if err != nil { |
| + log.Printf("console: api principal account %d: %v", p.AccountID, err) |
| + } |
| + if err != nil || acct == nil || p.AccountID == 0 || seg != acct.Sqid { |
| + http.NotFound(w, r) |
| + return Principal{}, nil, false |
| + } |
| + return p, acct, true |
| + } |
| + |
| + // ... the existing person path, unchanged: AccountsFor, matchAccounts, |
| + // the ?exact=sqid branch, the 409 for two candidates, the 404 for none. |
| +} |
| + |
| +// apiAccount is apiPrincipalAccount for the callers that only need the audit |
| +// actor and the account. The first return is WHO TO AUDIT AS, which for a |
| +// service is deliberately not a bare address: a row written by a machine must |
| +// never read as a row written by the human who minted its token. |
| +func (s *Server) apiAccount(w http.ResponseWriter, r *http.Request) (string, *Account, bool) { |
| + p, acct, ok := s.apiPrincipalAccount(w, r) |
| + if !ok { |
| + return "", nil, false |
| + } |
| + return auditActor(p, acct), acct, true |
| +} |
| + |
| +// auditActor renders a principal for the audit trail and for log lines. |
| +func auditActor(p Principal, acct *Account) string { |
| + if p.Kind == PrincipalUser { |
| + return p.Address |
| + } |
| + if p.Name == "" { |
| + // A legacy kind='app' row has no name; keep the string the audit |
| + // trail has always written for it, so old rows and new rows about |
| + // the same credential still read as the same actor. |
| + return "app:" + acct.Sqid + "/" + p.App |
| + } |
| + return "service:" + acct.Sqid + "/" + p.Name |
| +} |
| +``` |
| + |
| +In `apiAccountAppBy`, after the account resolves, add the app-binding check for an app-scoped service: |
| + |
| +```go |
| + // An app-scoped service reaches exactly one app. Checked before the |
| + // existence lookup so that "not your app" and "no such app" stay the |
| + // same 404 rather than differing by which branch produced it. |
| + if p.Kind == PrincipalService && p.App != "" && p.App != app { |
| + http.NotFound(w, r) |
| + return "", nil, "", false |
| + } |
| +``` |
| + |
| +- [ ] **Step 4: Collapse apiInstancePrincipal** |
| + |
| +`apiInstancePrincipal` (`apiinstances.go:52-97`) becomes a call to `apiAccountApp`: the sqid-only match, the `app:<sqid>/<app>` actor string and the byte-identical 404 it hand-rolled are now all in the shared path. Delete the function and change `instanceGate` to call `s.apiAccountApp`. **Move its doc comment's reasoning into `apiPrincipalAccount` rather than deleting it** — the "sqid-only, and why" paragraph is the justification for the shared code now. |
| + |
| +`Store.CLIAppToken` has no callers left. Delete it and its tests, or keep it only if a test still exercises the legacy read path directly. |
| + |
| +- [ ] **Step 5: Scope handleAPIApps** |
| + |
| +`handleAPIApps` currently lists via `AccountsFor(addr)`. Add the service branch: |
| + |
| +```go |
| + // A service principal has no membership set to enumerate. It answers with |
| + // EXACTLY its own scope — the one account it is bound to, or the single |
| + // app if it is app-scoped — and never enumerates anything else. Readable |
| + // by every role, because a credential seeing the thing it is already |
| + // bound to discloses nothing it does not already hold. This is the one |
| + // door whose ANSWER is scoped rather than whose ACCESS is gated |
| + // (spec §3a-i). |
| +``` |
| + |
| +- [ ] **Step 6: Run the tests** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -count=1` |
| +Expected: PASS, including every pre-existing instances and domains test — those exercise the legacy app-token path, which is now the shared path, and they are the real proof the collapse preserved behaviour. |
| + |
| +- [ ] **Step 7: Commit** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +gofmt -l internal/console/ |
| +git add internal/console/ |
| +git commit -m "console: scope resolves from the principal, not from the minter's memberships" |
| +``` |
| + |
| +--- |
| + |
| +### Task 5: Rate limits keyed on the principal |
| + |
| +Consequence 1 of issue #220. After this task a server's publishes no longer draw down its operator's budgets. |
| + |
| +**Files:** |
| +- Modify: `internal/console/authcli.go:41-108` (the limit constants and `cliLimits`) |
| +- Modify: `internal/console/apicli.go:168,347,470,573,648,758` (the `s.cli.create.allow(addr, …)` / `s.cli.write.allow(addr, …)` call sites) and every other `.allow(addr` in the CLI API files |
| +- Modify: `internal/console/apidomains.go:287` (`setDomainCapabilities`) |
| +- Test: `internal/console/authcli_test.go` (extend), `internal/console/apidomains_test.go` (extend) |
| + |
| +**Interfaces:** |
| +- Consumes: `Principal` (Task 2), `apiPrincipalAccount` (Task 4). |
| +- Produces: |
| + - `func principalKey(p Principal) string` |
| + - `cliLimits` gains `svcCreate`, `svcWrite`, `svcShip`, `wildFail *rateLimiter` |
| + - `func (s *Server) allowCreate(p Principal, now time.Time) bool`, `allowWrite`, `allowShip` — pick the person or service limiter from the principal, so no call site has to |
| + |
| +- [ ] **Step 1: Write the failing tests** |
| + |
| +```go |
| +// TestServiceBudgetIsNotItsOperatorsBudget is the fix for consequence 1: a |
| +// server holding a credential must not draw down the budget of the human who |
| +// minted it. |
| +func TestServiceBudgetIsNotItsOperatorsBudget(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + human := fx.userToken("paul@example.test", acct) |
| + svc := fx.serviceToken(acct, "hub", "", svcrole.Publish) |
| + |
| + // Exhaust the SERVICE's daily create budget. |
| + for i := 0; i < svcCreateBurst; i++ { |
| + if code, _ := fx.post(svc, "/api/cli/apps/"+acct.Sqid+"/new", createBody(fmt.Sprintf("a%d", i))); code != 201 { |
| + t.Fatalf("service create %d = %d", i, code) |
| + } |
| + } |
| + if code, _ := fx.post(svc, "/api/cli/apps/"+acct.Sqid+"/new", createBody("over")); code != 429 { |
| + t.Fatalf("service create over budget = %d; want 429", code) |
| + } |
| + // The human's own budget is untouched. |
| + if code, _ := fx.post(human, "/api/cli/apps/"+acct.Sqid+"/new", createBody("mine")); code != 201 { |
| + t.Errorf("human create after service exhausted its budget = %d; want 201", code) |
| + } |
| +} |
| + |
| +// TestRotationDoesNotResetTheBudget — the key is the NAME, not the token |
| +// hash, so a rotation mid-flood buys nothing. |
| +func TestRotationDoesNotResetTheBudget(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + tok := fx.serviceToken(acct, "hub", "", svcrole.Publish) |
| + for i := 0; i < svcCreateBurst; i++ { |
| + fx.post(tok, "/api/cli/apps/"+acct.Sqid+"/new", createBody(fmt.Sprintf("a%d", i))) |
| + } |
| + fresh, err := fx.store.RotateServicePrincipal(acct.ID, "hub", time.Now()) |
| + if err != nil { |
| + t.Fatalf("rotate: %v", err) |
| + } |
| + if code, _ := fx.post(fresh, "/api/cli/apps/"+acct.Sqid+"/new", createBody("after-rotate")); code != 429 { |
| + t.Errorf("create after rotate = %d; want 429 — rotation must not reset the budget", code) |
| + } |
| +} |
| + |
| +// TestPersonBudgetsAreUnchanged pins that no human's limits moved. |
| +func TestPersonBudgetsAreUnchanged(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + tok := fx.userToken("paul@example.test", acct) |
| + for i := 0; i < cliCreateBurst; i++ { |
| + if code, _ := fx.post(tok, "/api/cli/apps/"+acct.Sqid+"/new", createBody(fmt.Sprintf("a%d", i))); code != 201 { |
| + t.Fatalf("create %d = %d", i, code) |
| + } |
| + } |
| + if code, _ := fx.post(tok, "/api/cli/apps/"+acct.Sqid+"/new", createBody("over")); code != 429 { |
| + t.Errorf("create %d+1 = %d; want 429 at the unchanged human limit", cliCreateBurst+1, code) |
| + } |
| +} |
| + |
| +// TestWildcardEnableIsNotRateLimited — spec §4a. A Pages hub with wildcard |
| +// DNS makes no per-repo hostname call at all, so the door is exempt rather |
| +// than uplifted. |
| +func TestWildcardEnableIsNotRateLimited(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + fx.app(acct, "pages") |
| + tok := fx.serviceToken(acct, "hub", "", svcrole.Publish) |
| + fx.delegationZone("acme.example") |
| + // Far past any budget: attach/detach must never 429. |
| + for i := 0; i < svcWriteBurst*3; i++ { |
| + host := fmt.Sprintf("r%d.pages.example", i) |
| + fx.resolvesCNAME("_acme-challenge."+host, "_acme-challenge."+host+".acme.example") |
| + if code, body := fx.post(tok, "/api/cli/apps/"+acct.Sqid+"/pages/domains", |
| + attachBody(host, true, true)); code == 429 { |
| + t.Fatalf("attach %d = 429: %s", i, body) |
| + } |
| + } |
| +} |
| + |
| +// TestFailedWildcardEnableIsBounded is the hazard the exemption has to |
| +// survive: the CNAME check is a live, sequential DNS round trip on the |
| +// request path, and a caller whose CNAME is not there yet can retry forever. |
| +// A SUCCEEDING enable is once-per-domain and exempt; a FAILING one is not. |
| +func TestFailedWildcardEnableIsBounded(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + fx.app(acct, "pages") |
| + tok := fx.serviceToken(acct, "hub", "", svcrole.Publish) |
| + fx.delegationZone("acme.example") |
| + fx.resolvesNothing() // no _acme-challenge CNAME anywhere |
| + var saw429 bool |
| + for i := 0; i < wildFailBurst+5; i++ { |
| + code, _ := fx.post(tok, "/api/cli/apps/"+acct.Sqid+"/pages/domains", |
| + attachBody(fmt.Sprintf("r%d.pages.example", i), true, false)) |
| + if code == 429 { |
| + saw429 = true |
| + break |
| + } |
| + } |
| + if !saw429 { |
| + t.Error("unbounded failing wildcard enables; want a 429 after wildFailBurst") |
| + } |
| +} |
| + |
| +// TestIdempotentReattachTouchesNeitherBudget — titogo's router re-attaches |
| +// every hostname with its capability flags on every reload pass. `enabling` |
| +// is a false->true transition, so a re-send skips the DNS lookup entirely and |
| +// must skip the failure bound with it. |
| +func TestIdempotentReattachTouchesNeitherBudget(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + fx.app(acct, "pages") |
| + tok := fx.serviceToken(acct, "hub", "", svcrole.Instance) |
| + fx.delegationZone("acme.example") |
| + host := "one.pages.example" |
| + fx.resolvesCNAME("_acme-challenge."+host, "_acme-challenge."+host+".acme.example") |
| + if code, _ := fx.post(tok, "/api/cli/apps/"+acct.Sqid+"/pages/domains", attachBody(host, true, true)); code != 200 { |
| + t.Fatal("first attach failed") |
| + } |
| + fx.resolvesNothing() // the resolver goes dark; a re-attach must not care |
| + for i := 0; i < wildFailBurst*2; i++ { |
| + if code, body := fx.post(tok, "/api/cli/apps/"+acct.Sqid+"/pages/domains", attachBody(host, true, true)); code != 200 { |
| + t.Fatalf("re-attach %d = %d: %s", i, code, body) |
| + } |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run to verify they fail** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -run 'ServiceBudget|RotationDoesNot|PersonBudgets|WildcardEnableIsNot|FailedWildcard|IdempotentReattach' -count=1` |
| +Expected: FAIL — `svcCreateBurst` and `wildFailBurst` undefined. |
| + |
| +- [ ] **Step 3: Add the constants** |
| + |
| +In `internal/console/authcli.go`, beside the existing limit block, keeping its documentary style: |
| + |
| +```go |
| + // Service-principal budgets. A hub is not a person, and the numbers |
| + // above are calibrated for a person. These are a judgement, not a |
| + // derivation: 100 repos enabling Pages in a day, and a push every 30 |
| + // seconds sustained. |
| + // |
| + // They are keyed on the principal NAME (principalKey), not the token |
| + // hash: rotating a credential must not hand it a fresh budget, or a |
| + // rotation mid-flood would be the limit doing nothing. |
| + svcCreateBurst = 100 // per day, the cliCreateWindow |
| + svcWriteBurst = 120 // per minute, the cliRateWindow |
| + svcShipBurst = 60 // per hour, the cliShipWindow |
| + |
| + // wildFailBurst bounds FAILED wildcard enables per principal per minute. |
| + // The successful path is exempt (spec §4a): with wildcard DNS a Pages hub |
| + // makes no per-repo hostname call at all, and an enable is genuinely |
| + // once-per-domain. But the enabling gate does a LIVE, SEQUENTIAL DNS |
| + // round trip on the request path, and a caller whose _acme-challenge |
| + // CNAME is not there yet can retry forever — so the failures are bounded |
| + // even though the successes are not. Roomy enough for a resolver blip, |
| + // closed enough that an exempt door is not a way to make the console do |
| + // unbounded outbound DNS. |
| + wildFailBurst = 20 |
| +``` |
| + |
| +Add the limiters to `cliLimits`/`newCLILimits`: |
| + |
| +```go |
| + svcCreate: newRateLimiter(svcCreateBurst, cliCreateWindow), |
| + svcWrite: newRateLimiter(svcWriteBurst, cliRateWindow), |
| + svcShip: newRateLimiter(svcShipBurst, cliShipWindow), |
| + wildFail: newRateLimiter(wildFailBurst, cliRateWindow), |
| +``` |
| + |
| +- [ ] **Step 4: Add principalKey and the three pickers** |
| + |
| +In `internal/console/svcprincipal.go`: |
| + |
| +```go |
| +// principalKey is what a rate limiter counts against. |
| +// |
| +// A person keys on their ADDRESS, byte for byte as before — no human's limits |
| +// move in this change. A service keys on its NAME, not on its token hash, so |
| +// that rotating a credential does not hand it a fresh budget: the name is |
| +// stable across a secret change precisely so budgets and audit rows stay |
| +// continuous. |
| +// |
| +// The account id rather than the sqid because this string is never shown to |
| +// anyone: it is a map key, and the id is what the resolver already has. |
| +func principalKey(p Principal) string { |
| + if p.Kind == PrincipalUser { |
| + return p.Address |
| + } |
| + return "service:" + strconv.FormatInt(p.AccountID, 10) + "/" + p.Name |
| +} |
| + |
| +// allowCreate, allowWrite and allowShip pick the person or the service |
| +// limiter from the principal, so that no call site has to remember which one |
| +// it is talking to — a call site that got it wrong would be a server drawing |
| +// down an operator's budget, which is the bug this whole change is about. |
| +func (s *Server) allowCreate(p Principal, now time.Time) bool { |
| + if p.Kind == PrincipalUser { |
| + return s.cli.create.allow(principalKey(p), now) |
| + } |
| + return s.cli.svcCreate.allow(principalKey(p), now) |
| +} |
| +// allowWrite and allowShip follow the same two lines against svcWrite/svcShip. |
| +``` |
| + |
| +Note for `allowShip`: the person branch keeps keying on the **token hash**, not the address — `apicli.go:976` explains why, and that reasoning is unchanged. Read the existing call site and preserve its key exactly. |
| + |
| +- [ ] **Step 5: Convert the call sites** |
| + |
| +Every `s.cli.create.allow(addr, …)` and `s.cli.write.allow(addr, …)` in the CLI API files becomes `s.allowCreate(p, …)` / `s.allowWrite(p, …)`, with the handler taking `p` from `apiPrincipalAccount` instead of the bare address. Find them all: |
| + |
| +```bash |
| +grep -rn "s.cli.\(create\|write\|ship\)\.allow" internal/console/ | grep -v _test |
| +``` |
| + |
| +**Do not convert `s.cli.instances.allow`** — it is already token-keyed and its comment (`cliInstanceBurst`) states the reason; it stays as it is, and spec §4 leaves it unchanged. |
| + |
| +**Do not convert the `/auth/cli/` limiters** (`start`, `poll`, `pollIP`, `guessAddr`, `guessIP`) — those doors never see a service principal. |
| + |
| +- [ ] **Step 6: Exempt the domains doors and bound the failures** |
| + |
| +In `handleAPIDomainAttach` and `handleAPIDomainDetach`, remove the budget call inherited from `instanceGate` for these two doors. The cleanest shape: give the domains handlers a gate that stops at the principal, and add the exemption's reasoning as a comment naming spec §4a — an exemption without a stated reason reads as an oversight to the next person. |
| + |
| +In `setDomainCapabilities`, inside the `case enabling:` branch, wrap the CNAME check: |
| + |
| +```go |
| + case enabling: |
| + if s.delegationZone == "" { … } // unchanged |
| + if !s.delegationCNAMEDelegated(ctx, host) { |
| + // Bounded ONLY on the failing path (spec §4a). A succeeding |
| + // enable is once-per-domain and exempt; a failure is a live DNS |
| + // round trip a caller can repeat forever. Counted after the |
| + // lookup, so the bound is on failures rather than on attempts — |
| + // a caller whose CNAME is correct never meets it at all. |
| + if !s.cli.wildFail.allow(principalKey(p), time.Now()) { |
| + jsonError(w, http.StatusTooManyRequests, |
| + "too many wildcard attempts with no _acme-challenge CNAME in place — wait a minute and retry") |
| + return false |
| + } |
| + jsonError(w, http.StatusUnprocessableEntity, …) // unchanged |
| + return false |
| + } |
| +``` |
| + |
| +`setDomainCapabilities` needs the `Principal` threaded in; it currently takes `principal string`. Change the parameter to `p Principal` and derive the audit string with `auditActor(p, acct)` at the audit call. |
| + |
| +- [ ] **Step 7: Run the tests** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -count=1` |
| +Expected: PASS |
| + |
| +- [ ] **Step 8: Commit** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +gofmt -l internal/console/ |
| +git add internal/console/ |
| +git commit -m "console: a server no longer competes with its operator for budget" |
| +``` |
| + |
| +--- |
| + |
| +### Task 6: The mint doors |
| + |
| +Owner-gated, never reachable by a service. Modelled on `internal/console/fleets.go` — the same mint-once discipline and the same refusal shapes. |
| + |
| +**Files:** |
| +- Create: `internal/console/apiservices.go`, `internal/console/apiservices_test.go` |
| +- Modify: `internal/console/authcli.go` (four `cliRoute` registrations, all `svcrole.Set{}`) |
| + |
| +**Interfaces:** |
| +- Consumes: `apiPrincipalAccount` (Task 4); `NewServicePrincipal`, `ServicePrincipals`, `RotateServicePrincipal`, `RevokeServicePrincipal`, `ErrServiceNameTaken` (Task 2); `s.cliRoles` (Task 3). |
| +- Produces: |
| + - `func (s *Server) apiOwnerAccount(w, r) (Principal, *Account, bool)` |
| + - handlers `handleAPIServiceCreate`, `handleAPIServiceList`, `handleAPIServiceRotate`, `handleAPIServiceRevoke` |
| + - `func (s *Server) doorsFor(role svcrole.Role) []string` — sorted patterns from `cliRoles`, for the create response |
| + |
| +- [ ] **Step 1: Write the failing tests** |
| + |
| +```go |
| +// TestServiceMintIsOwnerOnly — minting is an authority grant that outlives |
| +// its minter, so it needs the owner gate, not the ordinary member write door. |
| +func TestServiceMintIsOwnerOnly(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + owner := fx.userToken("owner@example.test", acct) // RoleOwner |
| + member := fx.memberToken("member@example.test", acct) |
| + |
| + if code, _ := fx.post(owner, "/api/cli/services/"+acct.Sqid, mintBody("hub", "publish", "")); code != 201 { |
| + t.Errorf("owner mint = %d; want 201", code) |
| + } |
| + if code, _ := fx.post(member, "/api/cli/services/"+acct.Sqid, mintBody("other", "publish", "")); code != 403 { |
| + t.Errorf("member mint = %d; want 403", code) |
| + } |
| + // A non-member gets the byte-identical 404 every other door gives, NOT |
| + // the 403 — the 403 is only for someone who has already proved they hold |
| + // the account. |
| + stranger := fx.userToken("stranger@example.test", fx.account("elsewhere")) |
| + if code, _ := fx.post(stranger, "/api/cli/services/"+acct.Sqid, mintBody("x", "publish", "")); code != 404 { |
| + t.Errorf("non-member mint = %d; want 404", code) |
| + } |
| +} |
| + |
| +// TestAServiceCannotMintAService is the escalation cut, tested from the wire |
| +// rather than from the role table: a publish credential and an ADMIN one must |
| +// both meet the 403 the role gate gives a zero Set. |
| +func TestAServiceCannotMintAService(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + for _, role := range []svcrole.Role{svcrole.Publish, svcrole.Admin} { |
| + tok := fx.serviceToken(acct, "svc-"+string(role), "", role) |
| + for _, call := range []struct{ method, path string }{ |
| + {"POST", "/api/cli/services/" + acct.Sqid}, |
| + {"POST", "/api/cli/services/" + acct.Sqid + "/other/rotate"}, |
| + {"POST", "/api/cli/services/" + acct.Sqid + "/other/revoke"}, |
| + {"POST", "/api/cli/apps/" + acct.Sqid + "/app/instance-token"}, |
| + } { |
| + if code, _ := fx.do(tok, call.method, call.path, nil); code != 403 { |
| + t.Errorf("role %q at %s = %d; want 403", role, call.path, code) |
| + } |
| + } |
| + } |
| +} |
| + |
| +// TestMintReturnsTheSecretExactlyOnce — only the hash is stored, so there is |
| +// no second copy to hand out. The listing must never carry a secret. |
| +func TestMintReturnsTheSecretExactlyOnce(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + owner := fx.userToken("owner@example.test", acct) |
| + _, body := fx.post(owner, "/api/cli/services/"+acct.Sqid, mintBody("hub", "publish", "")) |
| + secret := jsonField(t, body, "token") |
| + if secret == "" { |
| + t.Fatal("mint returned no token") |
| + } |
| + _, list := fx.get2(owner, "/api/cli/services/"+acct.Sqid) |
| + if strings.Contains(list, secret) { |
| + t.Error("the listing carries the secret") |
| + } |
| +} |
| + |
| +// TestMintRefusesAnUnknownRole keeps the catalogue closed at the door, not |
| +// just in the type. |
| +func TestMintRefusesAnUnknownRole(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + owner := fx.userToken("owner@example.test", acct) |
| + for _, bad := range []string{"", "root", "owner", "Publish", "*"} { |
| + if code, _ := fx.post(owner, "/api/cli/services/"+acct.Sqid, mintBody("hub", bad, "")); code != 400 { |
| + t.Errorf("role %q = %d; want 400", bad, code) |
| + } |
| + } |
| +} |
| + |
| +// TestCreateAnswersWithTheDoorsItGranted — an operator must be able to see |
| +// what they gave away without reading a spec (CLAUDE.md's no-infra rule |
| +// applied to comprehension, not just access). |
| +func TestCreateAnswersWithTheDoorsItGranted(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + owner := fx.userToken("owner@example.test", acct) |
| + _, body := fx.post(owner, "/api/cli/services/"+acct.Sqid, mintBody("hub", "publish", "")) |
| + doors := jsonStrings(t, body, "doors") |
| + if len(doors) == 0 { |
| + t.Fatal("create returned no door list") |
| + } |
| + for _, d := range doors { |
| + if strings.Contains(d, "instance-token") || strings.Contains(d, "/fleets/") { |
| + t.Errorf("door list names a closed door: %s", d) |
| + } |
| + } |
| +} |
| + |
| +// TestLegacyAppTokensAppearInTheAccountListing — absorbing them must not hide |
| +// them. A credential nobody can see is one nobody rotates. |
| +func TestLegacyAppTokensAppearInTheAccountListing(t *testing.T) { |
| + fx := newAPIFixture(t) |
| + acct := fx.account("mine") |
| + owner := fx.userToken("owner@example.test", acct) |
| + if _, err := fx.store.NewAppToken(acct.ID, "titogo", "paul@example.test", "router", time.Now()); err != nil { |
| + t.Fatalf("NewAppToken: %v", err) |
| + } |
| + _, body := fx.get2(owner, "/api/cli/services/"+acct.Sqid) |
| + if !strings.Contains(body, `"instance"`) || !strings.Contains(body, "titogo") { |
| + t.Errorf("legacy app token missing from the account listing: %s", body) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run to verify they fail** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -run 'ServiceMint|CannotMint|MintReturns|MintRefuses|CreateAnswers|LegacyAppTokensAppear' -count=1` |
| +Expected: FAIL — 404 on every `/api/cli/services/` path; the routes do not exist. |
| + |
| +- [ ] **Step 3: Write the owner gate** |
| + |
| +There is no API-side owner gate today — `apiFleet` is member-gated, and the owner checks all live on page handlers. `Account.Role` is populated by `AccountsFor`, so the gate is thin. In `internal/console/apiservices.go`: |
| + |
| +```go |
| +// apiOwnerAccount is apiPrincipalAccount plus the owner check, for the doors |
| +// whose subject is a credential rather than an app. |
| +// |
| +// The two refusals are deliberately different and the difference is the |
| +// point. A caller who does not hold this account at all gets the family's |
| +// byte-identical 404 from apiPrincipalAccount — unchanged, and the reason it |
| +// must stay byte-identical is that a distinct answer here would confirm the |
| +// account exists. A caller who DOES hold it but is only a member gets a 403: |
| +// they have already proved membership, so there is nothing left to conceal, |
| +// and a 404 would send an ordinary member hunting for a typo in a sqid they |
| +// typed correctly. |
| +// |
| +// A service principal never arrives here: every door in this file registers |
| +// with the zero role Set, so the role gate refuses it before the handler runs |
| +// (spec §3). This gate does not need to re-check that, and deliberately does |
| +// not — one place per rule. |
| +func (s *Server) apiOwnerAccount(w http.ResponseWriter, r *http.Request) (Principal, *Account, bool) { |
| + p, acct, ok := s.apiPrincipalAccount(w, r) |
| + if !ok { |
| + return Principal{}, nil, false |
| + } |
| + if acct.Role != RoleOwner { |
| + jsonError(w, http.StatusForbidden, "only an account owner can manage service credentials") |
| + return Principal{}, nil, false |
| + } |
| + return p, acct, true |
| +} |
| +``` |
| + |
| +- [ ] **Step 4: Write the four handlers** |
| + |
| +`handleAPIServiceCreate` — owner gate; `s.allowWrite(p, …)`; read `{name, role, app}`; validate name with `registry.ValidTunnelName` (the same grammar a fleet label uses, and for the same reason: it is a handle typed on a command line); `svcrole.Parse` the role, 400 on miss; if `app != ""`, prove it exists in the account with `s.store.HasAppIn` and 404 otherwise, so an app-scoped credential cannot be minted against a name that is not there; mint; audit `service-create` with the role and scope in the message; respond `201` with `{name, role, app, account, token, doors}`. |
| + |
| +`doorsFor(role)` walks `s.cliRoles`, collects patterns whose Set permits the role, sorts them. Because it reads the same map the gate reads, the door list cannot drift from what the gate actually allows. |
| + |
| +`handleAPIServiceList` — owner gate; `ServicePrincipals(acct.ID)`; render name, role, scope, created, last used, minted-by, revoked. **Never the hash and never a secret.** |
| + |
| +`handleAPIServiceRotate` — owner gate; `allowWrite`; `RotateServicePrincipal`; zero rows affected is the ordinary 404 (`http.NotFound`) — within an account the caller already owns, an unknown name is an ordinary not-found, not a cross-tenant probe, exactly as `findFleetBox`'s comment reasons; audit `service-rotate`; respond with the fresh secret. |
| + |
| +`handleAPIServiceRevoke` — owner gate; `allowWrite`; `RevokeServicePrincipal`; audit `service-revoke`; `{"result":"revoked"}`. Revoking an already-revoked or unknown name answers the same 200: revocation must be idempotent, because the person reaching for it is usually in a hurry and must not have to work out whether their first attempt landed. |
| + |
| +- [ ] **Step 5: Register the routes** |
| + |
| +In `registerCLIRoutes`, beside the fleets block: |
| + |
| +```go |
| + // Service credentials (apiservices.go, issue #220) — the principal that |
| + // is not an address. Every door here takes the ZERO role set: a |
| + // credential may never mint, rotate or revoke a credential, including |
| + // itself, which is what keeps a publish credential from quietly becoming |
| + // an admin one. Owner-gated inside, because minting is an authority |
| + // grant that outlives its minter. |
| + s.cliRoute("POST /api/cli/services/{account}", svcrole.Set{}, s.handleAPIServiceCreate) |
| + s.cliRoute("GET /api/cli/services/{account}", svcrole.Set{}, s.handleAPIServiceList) |
| + s.cliRoute("POST /api/cli/services/{account}/{name}/rotate", svcrole.Set{}, s.handleAPIServiceRotate) |
| + s.cliRoute("POST /api/cli/services/{account}/{name}/revoke", svcrole.Set{}, s.handleAPIServiceRevoke) |
| +``` |
| + |
| +The `{name}` segment is a wildcard beside no literals, so no ambiguity arises. `rotate` and `revoke` are literal segments under it, which the mux prefers over any wildcard regardless of registration order — the same property the fleets and store families already rely on. |
| + |
| +**`TestEveryCLIDoorNamesItsRoles` (Task 3) asserts no door has an empty Set.** These five doors — the four here plus `instance-token` — are the deliberate exceptions. Amend that test to carry an explicit allow-list of zero-Set patterns and assert the list matches exactly, so a door that becomes unreachable by accident still fails the build while these stay legal. |
| + |
| +- [ ] **Step 6: Run the tests** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -count=1` |
| +Expected: PASS |
| + |
| +- [ ] **Step 7: Commit** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +gofmt -l internal/console/ |
| +git add internal/console/ |
| +git commit -m "console: mint, rotate and revoke a service credential" |
| +``` |
| + |
| +--- |
| + |
| +### Task 7: `carlos services` |
| + |
| +**Files:** |
| +- Create: `services.go`, `services_cli_test.go` (repo root) |
| +- Modify: `apimode.go` (four client methods), `main.go` (verb dispatch), `docs/` CLI reference if the repo has one |
| + |
| +**Interfaces:** |
| +- Consumes: the four API doors (Task 6); existing `apiSession`, `resolveMode`, `resolvedAccount`, `apiAcct`, `postJSON`, `getJSON`, `apiErrorFromBody`, `stdout` (all in `apimode.go` / `fleets.go`). |
| +- Produces: `func RunServices(args []string)`; `apiSession` methods `createService`, `listServices`, `rotateService`, `revokeService`. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Follow `fleets_cli_test.go`'s shape exactly. **Every test that shells out to the binary must build its env through `carlosTestEnv` and set `cmd.Dir` to a scratch dir** — the binary walks cwd ancestors for `.carlos/config`, and the fallback is the operator's real login on the production console. |
| + |
| +```go |
| +// TestServicesCreatePrintsTheSecretAndTheDoors — the secret appears exactly |
| +// once, in this output, and the door list is printed with it so an operator |
| +// sees what they granted at the moment they grant it. |
| +func TestServicesCreatePrintsTheSecretAndTheDoors(t *testing.T) { … } |
| + |
| +// TestServicesCreateRefusesABadNameBeforeCalling — a shape the server is |
| +// certain to refuse should cost nothing, the principle apps.go's |
| +// validAppName states. |
| +func TestServicesCreateRefusesABadNameBeforeCalling(t *testing.T) { |
| + // Assert the process exits non-zero and that the fake console recorded |
| + // NO request. |
| +} |
| + |
| +// TestServicesRotateSaysWhatRotationIsNot — rotation refuses the credential's |
| +// next call with the old secret; it does not stop work already in flight, and |
| +// it is not what a compromised credential needs. That is `revoke`. The |
| +// fleets CLI makes the same distinction in words and this must too. |
| +func TestServicesRotateSaysWhatRotationIsNot(t *testing.T) { … } |
| +``` |
| + |
| +- [ ] **Step 2: Run to verify it fails** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test . -run Services -count=1` |
| +Expected: FAIL — `RunServices` undefined. |
| + |
| +- [ ] **Step 3: Write the client methods** |
| + |
| +In `apimode.go`, beside `createFleet`, four methods following its shape exactly (`postJSON`/`getJSON`, status check, `apiErrorFromBody`, typed result struct): |
| + |
| +```go |
| +type apiServiceView struct { |
| + Name string `json:"name"` |
| + Role string `json:"role"` |
| + App string `json:"app"` |
| + Account string `json:"account"` |
| + Created string `json:"created"` |
| + LastUsed string `json:"last_used"` |
| + MintedBy string `json:"minted_by"` |
| +} |
| + |
| +func (s *apiSession) createService(ctx context.Context, a apiAcct, name, role, app string) (token string, doors []string, err error) |
| +func (s *apiSession) listServices(ctx context.Context, a apiAcct) ([]apiServiceView, error) |
| +func (s *apiSession) rotateService(ctx context.Context, a apiAcct, name string) (string, error) |
| +func (s *apiSession) revokeService(ctx context.Context, a apiAcct, name string) error |
| +``` |
| + |
| +- [ ] **Step 4: Write the verbs** |
| + |
| +`services.go`, following `fleets.go`'s structure: a `servicesUsage` const, `RunServices` dispatching `create|ls|rotate|revoke`, a `servicesSignedInSession` that fatals with a services-specific message in direct bucket mode, and a `validServiceName` using `registry.ValidTunnelName`. |
| + |
| +The usage text must carry the three things an operator cannot infer: |
| + |
| +``` |
| + create -role <publish|instance|operate|admin> [-app <app>] [-account <sqid|name>] <name> |
| + mint a credential a SERVER holds, bound to this account — |
| + not to you. Unlike your own CLI token, its reach is fixed |
| + at mint: adding you to another account later does not widen |
| + it, and it draws on its own rate-limit budgets rather than |
| + yours. |
| + THE SECRET IS PRINTED EXACTLY ONCE, HERE. Only its hash is |
| + stored, so losing this output means rotate, not a re-run. |
| + -role admin INCLUDES `store grant`, which mints a |
| + path-scoped IAM user. It is a real handover, not a |
| + convenience bundle. Prefer the narrowest role that works. |
| + -app binds it to ONE app, which also means it cannot create |
| + apps. Omit -app for an account-scoped credential. |
| + |
| + rotate [-account <sqid|name>] <name> |
| + mint a FRESH secret for a credential that already exists, |
| + printed exactly once. This refuses the credential's NEXT |
| + call with the old secret. ROTATION IS NOT REVOCATION: it |
| + does not stop work already in flight, and a credential that |
| + may be COMPROMISED needs revoke. |
| + |
| + revoke [-account <sqid|name>] <name> |
| + the hard stop rotate is not: the credential's next call |
| + fails, whatever secret it holds. Idempotent — revoking a |
| + name twice, or one that never existed, answers the same. |
| +``` |
| + |
| +`create`'s success output prints the secret, then the doors, then the delivery step, because a credential nobody can deliver is not finished: |
| + |
| +``` |
| +minted service:bdm/amadan-hub (role publish, account-wide) |
| + |
| + CARLOS_SERVICE_TOKEN=<secret> |
| + |
| + ^ shown once. Only its hash is stored. |
| + |
| +it may: |
| + POST /api/cli/apps/{account}/new |
| + POST /api/cli/apps/{account}/{app}/releases |
| + … |
| + |
| +deliver it: carlos env set -app <app> -secret CARLOS_SERVICE_TOKEN=<secret> |
| +revoke it: carlos services revoke -account bdm amadan-hub |
| +``` |
| + |
| +Wire `services` into `main.go`'s verb dispatch beside `fleets`. |
| + |
| +- [ ] **Step 5: Run the tests** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test . -count=1` |
| +Expected: PASS |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +gofmt -l services.go apimode.go main.go |
| +git add services.go services_cli_test.go apimode.go main.go |
| +git commit -m "carlos services: mint, list, rotate and revoke from a terminal" |
| +``` |
| + |
| +--- |
| + |
| +### Task 8: The console pane |
| + |
| +Revocation is the emergency control, so it must work without a terminal. |
| + |
| +**Files:** |
| +- Create/modify: `internal/console/templates/account_settings.html` |
| +- Modify: `internal/console/server.go` (`handleAccountSettings`, plus two POST routes) |
| +- Test: `internal/console/apiservices_test.go` (extend) or a new `svcpage_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `ServicePrincipals`, `RotateServicePrincipal`, `RevokeServicePrincipal` (Task 2); `s.account` (`server.go:1263`), `s.accountPage`, `s.requireFreshAuth` (`stepup.go:249`). |
| +- Produces: `handleServiceRotatePage`, `handleServiceRevokePage`; `page` gains `Services []CLIToken`. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +```go |
| +// TestServicePaneIsOwnerGatedInOrder — the ordered gate this repo pins: |
| +// session -> CSRF -> owner (byte-identical 404) -> requireFreshAuth LAST. A |
| +// non-member must never get a redirect that confirms an account exists. |
| +func TestServicePaneIsOwnerGatedInOrder(t *testing.T) { … } |
| + |
| +// TestRevokeFromThePageKillsTheCredential — end to end: a token that worked |
| +// against the API stops working after the button. |
| +func TestRevokeFromThePageKillsTheCredential(t *testing.T) { … } |
| + |
| +// TestThePageNeverRendersASecret |
| +func TestThePageNeverRendersASecret(t *testing.T) { … } |
| +``` |
| + |
| +- [ ] **Step 2: Run to verify it fails** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -run 'ServicePane|RevokeFromThePage|NeverRendersASecret' -count=1` |
| +Expected: FAIL |
| + |
| +- [ ] **Step 3: Add the pane** |
| + |
| +In `handleAccountSettings` (`server.go:1930`), load `s.store.ServicePrincipals(acct.ID)` and hang it on the page. **Only for an owner** — a member sees the account settings page without this section rather than with an empty one, because an empty section reads as "this account has no service credentials", which is a different and possibly false statement. |
| + |
| +Register two POST routes beside the existing account settings routes, following `handleFleetRotateBoxPage` (`fleets.go:1099`) for the gate order, the flash-back-to-the-page pattern, and the once-only secret display. |
| + |
| +Template rows: name, role, scope (`account-wide` or `app <name>`), last used, minted by, and the two buttons. A legacy `kind='app'` row renders with role `instance` and a scope of its app — same table, no second listing. |
| + |
| +- [ ] **Step 4: Add the read-only mirror on `/settings`** |
| + |
| +`handleCLITokens` (`authcli.go:208`) lists the viewer's own tokens. Rows the viewer minted for a service must stay visible there — absorbing them into the account listing must not make them vanish from a page people know — but read-only, with "lives on account `<sqid>` — manage there" linking to the account pane. `CLIToken.IsApp()` already exists for the template; add `IsService()` beside it rather than comparing strings in the template. |
| + |
| +- [ ] **Step 5: Run the tests** |
| + |
| +Run: `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./internal/console/ -count=1` |
| +Expected: PASS |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +gofmt -l internal/console/ |
| +git add internal/console/ |
| +git commit -m "console: service credentials on the account, revocable without a terminal" |
| +``` |
| + |
| +--- |
| + |
| +### Task 9: Whole-branch verification and the PR |
| + |
| +- [ ] **Step 1: Run the full gate** |
| + |
| +```bash |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go vet ./... && \ |
| +GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go test ./... -count=1 |
| +GOOS=darwin GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off go build ./... |
| +gofmt -l $(git diff --name-only main...HEAD | grep '\.go$') |
| +``` |
| + |
| +All clean. The `GOOS=darwin` build is not optional: `internal/host` is linked into the CLI people run on Macs. |
| + |
| +- [ ] **Step 2: Prove the migration against a real pre-change database** |
| + |
| +Copy a console database from before this branch (or build one by checking out `main`, running the console's migrate, then checking back), open it with the new binary, and assert: |
| + |
| +- every existing user token still authenticates; |
| +- every existing `kind='app'` token still reaches instances, domains and logs; |
| +- no existing token reaches anything it could not reach before. |
| + |
| +This is the one check the unit tests cannot do, because they build their schema from the new code. |
| + |
| +- [ ] **Step 3: Open the PR** |
| + |
| +```bash |
| +git push -u origin service-roles |
| +gh pr create --title "Service principals: a console API caller that is not a person" --body "…" |
| +``` |
| + |
| +The body must carry: what issue #220 asked for, the three findings from spec §1a that changed the design (the principal already existed; `internal/caps` is the wrong vocabulary; ship was already safe), the gate-order reversal and why, and the **deploy note** below. Never offer merging to `main`, and never take it if offered — merging is a human's action. |
| + |
| +- [ ] **Step 4: Write the deploy note on the PR** |
| + |
| +The role gate ships **closed**, so a mis-bound door is a 403 for a live caller. Ordering matters: |
| + |
| +1. **tito first.** It is the deployment with a real `kind='app'` consumer — titogo's router, which re-attaches every hostname on every reload pass with its `CARLOS_INSTANCE_TOKEN`. |
| +2. Promote → adopt → restart the console (console restarts stay manual until the route is unit-stamped). |
| +3. **Verify the router's re-attach pass before touching flagship.** Watch for 403s in the console log, not just a green health check: a 403 here is silent from the outside, and the hostnames stay attached from the previous pass, so nothing visibly breaks until something needs to change. |
| +4. Flagship second. |
| +5. Verify with `X-Carlos-Version` on the URL via `curl -sI --resolve`, never a cached answer. |
| + |
| +--- |
| + |
| +## Self-Review |
| + |
| +**Spec coverage.** §1 problem → the whole branch. §1a corrections → Task 1 (svcrole not caps), Task 2 (widening kind='app'), Task 5 (ship left alone). §2 principal and schema → Task 2. §2a where a credential lives → Tasks 6 and 8. §3 catalogue → Task 1 + Task 3's binding table. §3a structural enforcement → Task 3. §3a-i the two uncovered doors → Task 4 Step 5 (`GET /api/cli/apps`) and Task 3 Step 1 (`TestAuthCLIFamilyResolvesOnlyPersons`). §3b gate order → Task 3 Step 3. §4 rate limits → Task 5. §4a wildcard → Task 5 Steps 1, 6. §5 compatibility → Task 2 Step 1, Task 3 Step 1's pin, Task 9 Step 2. §6 surfaces → Tasks 6, 7, 8. §7 testing → distributed, each in the task that makes it possible. §8 risks → Task 7's usage text (store grant), Task 9 Step 4 (deploy ordering), Task 8 Step 4 (the page move). |
| + |
| +**Two known gaps, deliberate and named rather than hidden:** |
| + |
| +- Task 3's `TestAuthCLIFamilyResolvesOnlyPersons` and Task 8's three tests are specified by name, assertion and reason but not written out as code. Both depend on package-local fixtures (`newTestServer`, the page-rendering helpers) whose exact shape must be read from the existing tests rather than guessed — writing them here from memory would produce code that does not compile against the real fixtures, which is worse than an explicit instruction. Every other test in this plan is complete code. |
| +- Task 4's fixture helpers (`newAPIFixture`, `fx.serviceToken`, `fx.apps`, `sameSet`) are named and used but not defined. Build them on the package's existing API test fixture — `apicli_test.go` and `apiinstances_test.go` already have one — rather than starting a second. |
| + |
| +**Type consistency check.** `svcrole.Role` is the role type everywhere; `Principal.Role` is `svcrole.Role`, while `CLIToken.Role` is `string` (it crosses into a template, which cannot call `Parse`). `principalKey` takes `Principal`, not an address, at every call site. `apiAccount` keeps its three-value signature so no existing caller changes; `apiPrincipalAccount` is the typed form. `auditActor(p, acct)` is the single renderer of an actor string and is used by `setDomainCapabilities` after Task 5 changes its parameter from `principal string` to `p Principal`. |