carlos / platform Public

console: enabling instances is a closed door, and CLIAppToken is gone

Three rulings on task 4's review (C25-C27).

C25. PUT .../instances-manifest is registered with the zero Set. It was
bound Allow(instance) because the legacy-reach list was built from the
route registrations filed under "instances" rather than from each
handler's gate — but handleAPIInstanceEnable has always gated on
apiAccountApp, which refused a non-person at cliUser, so no app token has
ever reached it. The door writes the MANIFEST, which is standing policy
about which domain, pool, regions and channel the app may ever be brought
up on, and a credential the app itself holds must not rewrite the bounds
it runs inside. Closing it takes no authority away and moves the refusal
out to the wrapper, where it is structural: the answer moves from 401 to
403, as it did at the mint door. Dropping the door from the legacy pin
CORRECTS that pin rather than breaking it, and its comment now says so.

apiPerson stays at both doors. The registration is the load-bearing wall;
this is the second one, and it is not redundancy — before this branch the
refusal was implicit in cliUser, and collapsing that resolution into the
principal-aware path destroyed it. A handler whose semantics are
person-only should not depend solely on a line in another file. Since no
request can now deliver a service principal to it through the mux, it is
tested directly.

C26. The trashed-app narrowing stays, and its test now says in words that
it is deliberate, so nobody restores the old behaviour as a fix.

C27. CLIAppToken is deleted. An unused token-resolution function in an
auth package is what gets wired back up by someone who does not know it
was superseded. None of its four tests covered only it, so their app-gate
assertions are re-expressed against CLIPrincipal property for property —
the binding, the real expiry and its end, the kind check standing alone,
the four no-answers, the purge, the revoke, the reopen. Twenty tests
across six files still mint a kind='app' row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016UimU7fAfeAoBMm6HKCPwQ

Paul Campbell 2 months ago 1bd5a204bba460aaa02b402a7aba66aecb71f978 parent 0f1907e

8 files changed, +164 −101

internal/console/apicli.go +17 −8
internal/console/apiinstances.go +14 −16
internal/console/apiinstancesenable_test.go +11 −6
internal/console/authcli.go +12 −1
internal/console/authstore.go +0 −43
internal/console/authstore_test.go +30 −18
internal/console/svcroutes_test.go +17 −1
internal/console/svcscope_test.go +63 −8

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

index b915270a0..e652d3bac 100644

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

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

@@ -245,18 +245,27 @@ func (s *Server) apiAccount(w http.ResponseWriter, r *http.Request) (string, *Ac

// the answer stays a constant function of the credential rather than telling

// a service whether the path it named is its own.

-// It is the inner of two walls at the doors whose subject is a human act:

+// It is the INNER of two walls at the doors whose subject is a human act:

// minting a credential (a credential may never mint a credential) and

// enabling instances (the manifest is standing policy about what may be

-// brought into existence). cliRoute is the outer wall at the mint doors,

-// which are registered with the zero Set and so are closed to every role.

-// The inner wall used to be a side effect of those handlers resolving their

-// caller through cliUser; folding that resolution into the shared

-// principal-aware path removed it, so it is stated here deliberately instead

-// of being inherited by accident.

+// brought into existence). Both are registered with the zero Set, so no role

+// reaches either — and THAT registration is the load-bearing wall. The role

+// table is the single source of truth for what a service credential may

+// reach, and a caller refused there never arrives here at all.

+//

+// This second wall is not redundancy. Before service principals, those

+// handlers refused a non-person IMPLICITLY: they resolved through

+// apiAccountApp, which resolved through cliUser, which answers for nobody but

+// a person. Collapsing that resolution into the principal-aware path removed

+// the refusal, and an invariant a handler relies on should not be left

+// depending solely on a line in another file staying right. So the

+// person-only semantics are stated where the handler is, not only where it

+// is registered.

// It reads the principal cliRoute already resolved, so it costs no second

-// lookup. Pinned by TestAPIInstanceEnableRefusals and TestEnableStaysAHumanAct.

+// lookup. Pinned directly by TestApiPersonIsTheInnerWall, because the closed

+// registrations mean no request can ever deliver a service principal to it

+// through the mux.

func (s *Server) apiPerson(w http.ResponseWriter, r *http.Request) bool {

p, ok := principalFrom(r.Context())

if !ok || p.Kind != PrincipalUser {

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

index 4aa5a5ed7..8812a5a79 100644

--- a/internal/console/apiinstances.go

+++ b/internal/console/apiinstances.go

@@ -840,12 +840,12 @@ func (s *Server) handleAPIInstanceDelete(w http.ResponseWriter, r *http.Request)

// Budgeted on the address-keyed write limiter for the same reason: the

// principal a mint belongs to is a person.

-// That 401 is the INNER of two walls. The outer is cliRoute: this door is

-// registered with the zero Set, so no role — admin included — reaches it at

-// all (TestNoRoleReachesAMintOrFleetDoor, TestAdminIsRefusedAtAClosedDoor).

-// Both are kept, because the inner one used to be a side effect of resolving

-// through cliUser and would otherwise have vanished the day that resolution

-// became principal-aware.

+// That 401 is the INNER of two walls, and it is the one nothing can reach:

+// the outer is cliRoute, where this door's zero Set refuses every role,

+// admin included (TestNoRoleReachesAMintOrFleetDoor,

+// TestAdminIsRefusedAtAClosedDoor). Both are kept, because the inner one

+// used to be a side effect of resolving through cliUser and would otherwise

+// have vanished the day that resolution became principal-aware.

func (s *Server) handleAPIInstanceToken(w http.ResponseWriter, r *http.Request) {

if !s.apiPerson(w, r) {

return

@@ -916,16 +916,14 @@ const (

// to rewrite the bounds it runs inside. Budgeted on the address-keyed write

// limiter for the same reason.

-// The role table DOES list this door under `instance` (spec §5, and

-// TestInstanceRoleMatchesTodaysAppTokenReach pins it), so the role gate

-// passes an app token through to here. That is not a contradiction: role is

-// the outer wall and person-only is the inner one, and this handler is the

-// only place the inner one can be stated, because it is a fact about the

-// SUBJECT of the door rather than about the caller's role. Before service

-// principals it was a side effect of resolving through cliUser; making the

-// resolution principal-aware turned it into something that has to be said

-// out loud. Pinned by TestAPIInstanceEnableRefusals and

-// TestEnableStaysAHumanAct.

+// The door is registered with the zero Set (authcli.go), so no role reaches

+// it and a service credential is refused out at cliRoute with the same 403

+// the mint door gives. That registration is the load-bearing wall; apiPerson

+// is the second one, stated here because the person-only rule is a fact

+// about the SUBJECT of this door rather than about any caller's role, and

+// because it used to hold only as a side effect of resolving through

+// cliUser. Pinned by TestAPIInstanceEnableRefusals, TestEnableStaysAHumanAct

+// and TestApiPersonIsTheInnerWall.

func (s *Server) handleAPIInstanceEnable(w http.ResponseWriter, r *http.Request) {

if !s.apiPerson(w, r) {

return

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

index 82896501b..7271aee5b 100644

--- a/internal/console/apiinstancesenable_test.go

+++ b/internal/console/apiinstancesenable_test.go

@@ -221,8 +221,8 @@ func TestAPIInstanceCreateSiblingApexRefused(t *testing.T) {

// TestAPIInstanceEnableRefusals: every way of not being allowed to enable,

// and the order the answers keep secrets in. Before membership is proven the

-// answer is the family's byte-identical 404; an app token meets cliUser's

-// 401 (enabling is a human act); only after the gate do causes get names —

+// answer is the family's byte-identical 404; a service credential meets the

+// closed door's 403 (enabling is a human act); only after the gate do causes get names —

// including the one refusal that guards somebody else's standing policy,

// the foreign-domain 409.

func TestAPIInstanceEnableRefusals(t *testing.T) {

@@ -246,10 +246,15 @@ func TestAPIInstanceEnableRefusals(t *testing.T) {

assertNotFound(t, "enable: "+c.what, want, status, h, out)

}

- // An app token cannot enable: CLITokenAddress answers only user tokens,

- // so the binding that authorizes record writes meets a plain 401 here.

- if status, _, out := rawReq(t, http.MethodPut, enableURL(fx, "bab", "titogo"), appTok, `{}`); status != http.StatusUnauthorized {

- t.Fatalf("app token enable = %d %s, want 401", status, out)

+ // An app token cannot enable. The containment is unchanged and always

+ // was — no app token has ever reached this handler — but the ANSWER

+ // moved from 401 to 403 when the door was registered with the zero Set

+ // (service-principals ruling C25). Enabling writes the manifest that

+ // bounds what the app may ever be brought up as, so it takes the same

+ // escalation cut the mint door takes, and the refusal now comes from

+ // cliRoute before the handler rather than from cliUser inside it.

+ if status, _, out := rawReq(t, http.MethodPut, enableURL(fx, "bab", "titogo"), appTok, `{}`); status != http.StatusForbidden {

+ t.Fatalf("app token enable = %d %s, want 403", status, out)

}

// Cheap validation refusals, after the gate.

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

index 2b8094c99..2a231b7d2 100644

--- a/internal/console/authcli.go

+++ b/internal/console/authcli.go

@@ -348,7 +348,18 @@ func (s *Server) registerCLIRoutes() {

// `instances-manifest` is the member-facing half of `carlos instances

// enable` (issue #66): a literal sibling of instances/, never under it,

// so a host named "instances-manifest" could still never reach it.

- s.cliRoute("PUT /api/cli/apps/{account}/{app}/instances-manifest", svcrole.Allow(svcrole.Instance), s.handleAPIInstanceEnable)

+ //

+ // The zero Set, for the same escalation cut the mint door takes. This

+ // door writes the MANIFEST — standing policy about which domain, pool,

+ // regions and channel the app may ever be brought up on — and a

+ // credential the app itself holds must not be able to rewrite the

+ // bounds it runs inside. It takes no authority away: handleAPIInstanceEnable

+ // has always gated on apiAccountApp, which could not see a non-person at

+ // all, so no app token has ever reached it. The registration is where

+ // that is now said, because a registration reading "instance may pass"

+ // while the handler refuses is a registration that lies. As at the mint

+ // door, the ANSWER a credential gets here moves from 401 to 403.

+ s.cliRoute("PUT /api/cli/apps/{account}/{app}/instances-manifest", svcrole.Set{}, s.handleAPIInstanceEnable)

s.cliRoute("GET /api/cli/apps/{account}/{app}/instances", svcrole.Allow(svcrole.Instance), s.handleAPIInstanceList)

// The logs read (`carlos logs`, apilogs.go) — same gate, same 404

// discipline, and an app token may read its own app's logs.

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

index 95fd49444..3c204fa54 100644

--- a/internal/console/authstore.go

+++ b/internal/console/authstore.go

@@ -489,49 +489,6 @@ VALUES (?, ?, ?, '', ?, 0, ?, ?, ?)`,

return raw, nil

}

-// CLIAppToken resolves a bearer token to the account and app it is bound to;

-// ok is false for unknown, revoked and expired tokens — and for every USER

-// token, which is the containment property read from this side: the app gate

-// must not accept a person's credential just because it is valid.

-//

-// NO PRODUCTION CALLER since apiInstancePrincipal collapsed into the shared

-// principal path (spec 2026-08-18 §5): CLIPrincipal reads a kind='app' row

-// now, and reads it as a service principal holding role `instance`. This is

-// kept as the direct pin on the LEGACY read path — authstore_test.go

-// exercises expiry, the user-token refusal and the after-reopen read against

-// this function without an HTTP server in the way — so that the row shape

-// those tests describe stays described even as the gate above it moves.

-// Delete it when that coverage moves to CLIPrincipal, not before.

-func (s *Store) CLIAppToken(rawToken string, now time.Time) (int64, string, bool) {

- if rawToken == "" {

- return 0, "", false

- }

- hash := hashToken(rawToken)

- var storedHash, kind, app string

- var accountID, expires int64

- err := s.db.QueryRow(`SELECT token_hash, kind, account_id, app, expires_at

-FROM cli_tokens WHERE token_hash = ?`, hash).

- Scan(&storedHash, &kind, &accountID, &app, &expires)

- if err != nil {

- return 0, "", false

- }

- if subtle.ConstantTimeCompare([]byte(storedHash), []byte(hash)) != 1 {

- return 0, "", false

- }

- if kind != cliKindApp {

- return 0, "", false

- }

- // 0 is "never" (see NewAppToken); any other value is still honoured, so a

- // bounded app token remains expressible without a second code path.

- if !(expires == 0 || now.Unix() <= expires) {

- return 0, "", false

- }

- // Best-effort: "last used" is for the owner's benefit on the tokens page,

- // and a failed write must not fail the request it is annotating.

- s.db.Exec(`UPDATE cli_tokens SET last_used = ? WHERE token_hash = ?`, now.Unix(), hash)

- return accountID, app, true

// CLITokenAddress resolves a bearer token to the address that approved it; ok

// is false for unknown, revoked and expired tokens. This is the whole

// authorization decision for every CLI-facing endpoint.

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

index 91ba3a077..68b083351 100644

--- a/internal/console/authstore_test.go

+++ b/internal/console/authstore_test.go

@@ -230,7 +230,15 @@ func mintUserToken(t *testing.T, s *Store, address string, now time.Time) string

}

// The containment property, both directions: an app token is invisible to

-// every user-token door, and a user token is invisible to the app-token gate.

+// every user-token door, and a user token never resolves as a service.

+//

+// The app-token side is asserted through CLIPrincipal, which is the resolver

+// that exists — CLIAppToken was deleted once apiInstancePrincipal collapsed

+// into the shared principal path (service-principals ruling C27). Every

+// property it used to prove is proved here against its successor: the

+// binding it reads back, the kind check that stands alone from expiry, a

+// real expiry still honoured, and the four no-answers (a user token, a

+// made-up one, an empty one, a revoked one).

func TestAppTokenContainment(t *testing.T) {

s := openTestStore(t)

now := time.Now()

@@ -242,9 +250,9 @@ func TestAppTokenContainment(t *testing.T) {

if app == "" || len(app) < 40 {

t.Fatalf("app token %q: want a high-entropy secret", app)

}

- accountID, appName, ok := s.CLIAppToken(app, now)

- if !ok || accountID != 7 || appName != "tito" {

- t.Fatalf("CLIAppToken = %d %q %v, want 7 tito true", accountID, appName, ok)

+ p, ok := s.CLIPrincipal(app, now)

+ if !ok || p.Kind != PrincipalService || p.AccountID != 7 || p.App != "tito" {

+ t.Fatalf("CLIPrincipal = %+v %v, want the service principal bound to 7/tito", p, ok)

}

// The whole point: presented at a user door it must look like nothing.

if addr, ok := s.CLITokenAddress(app, now); ok {

@@ -266,10 +274,10 @@ VALUES (?, 'paul@keymail.dev', 'dated', '', ?, ?, 'app', 9, 'tito')`,

if addr, ok := s.CLITokenAddress(dated, now); ok {

t.Fatalf("an unexpired app token authenticated at a user door as %q", addr)

}

- if id, name, ok := s.CLIAppToken(dated, now); !ok || id != 9 || name != "tito" {

- t.Fatalf("CLIAppToken(dated) = %d %q %v, want 9 tito true", id, name, ok)

+ if p, ok := s.CLIPrincipal(dated, now); !ok || p.Kind != PrincipalService || p.AccountID != 9 || p.App != "tito" {

+ t.Fatalf("CLIPrincipal(dated) = %+v %v, want the service principal bound to 9/tito", p, ok)

}

- if _, _, ok := s.CLIAppToken(dated, now.Add(CLITokenTTL+time.Second)); ok {

+ if _, ok := s.CLIPrincipal(dated, now.Add(CLITokenTTL+time.Second)); ok {

t.Fatal("an app token with a real expiry outlived it")

}

@@ -277,16 +285,20 @@ VALUES (?, 'paul@keymail.dev', 'dated', '', ?, ?, 'app', 9, 'tito')`,

if addr, ok := s.CLITokenAddress(user, now); !ok || addr != "paul@keymail.dev" {

t.Fatalf("CLITokenAddress = %q %v, want the approver", addr, ok)

}

- if id, name, ok := s.CLIAppToken(user, now); ok {

- t.Fatalf("a user token passed the app gate as %d %q", id, name)

+ // The other direction. CLIPrincipal answers for a user token — it is the

+ // one resolver for every kind now — so the containment it holds is that

+ // a person never reads back as a SERVICE, which is the property the app

+ // gate used to hold by refusing them outright.

+ if p, ok := s.CLIPrincipal(user, now); !ok || p.Kind != PrincipalUser || p.Address != "paul@keymail.dev" {

+ t.Fatalf("CLIPrincipal(user) = %+v %v, want paul as a person", p, ok)

}

// Unknown and empty tokens answer no at both doors.

- if _, _, ok := s.CLIAppToken("not-a-token", now); ok {

- t.Fatal("a made-up token passed the app gate")

+ if _, ok := s.CLIPrincipal("not-a-token", now); ok {

+ t.Fatal("a made-up token resolved to a principal")

}

- if _, _, ok := s.CLIAppToken("", now); ok {

- t.Fatal("an empty token passed the app gate")

+ if _, ok := s.CLIPrincipal("", now); ok {

+ t.Fatal("an empty token resolved to a principal")

}

@@ -302,7 +314,7 @@ func TestAppTokenNeverExpires(t *testing.T) {

user := mintUserToken(t, s, "paul@keymail.dev", now)

decade := now.Add(10 * 365 * 24 * time.Hour)

- if _, _, ok := s.CLIAppToken(app, decade); !ok {

+ if _, ok := s.CLIPrincipal(app, decade); !ok {

t.Fatal("an app token expired")

}

if _, ok := s.CLITokenAddress(user, now.Add(CLITokenTTL+time.Second)); ok {

@@ -314,7 +326,7 @@ func TestAppTokenNeverExpires(t *testing.T) {

lastCLIPurge.Store(0)

defer lastCLIPurge.Store(0)

s.PurgeCLIAuth(decade)

- if _, _, ok := s.CLIAppToken(app, decade); !ok {

+ if _, ok := s.CLIPrincipal(app, decade); !ok {

t.Fatal("the purge swept an app token that never expires")

}

@@ -344,7 +356,7 @@ func TestAppTokenRevocation(t *testing.T) {

if err := s.RevokeCLIToken("paul@keymail.dev", toks[0].ID); err != nil {

t.Fatal(err)

}

- if _, _, ok := s.CLIAppToken(app, now); ok {

+ if _, ok := s.CLIPrincipal(app, now); ok {

t.Fatal("a revoked app token still authenticates")

}

@@ -377,8 +389,8 @@ func TestCLITokenMigrationReopens(t *testing.T) {

if addr, ok := second.CLITokenAddress(user, now); !ok || addr != "paul@keymail.dev" {

t.Fatalf("after reopen CLITokenAddress = %q %v, want the approver", addr, ok)

}

- if id, name, ok := second.CLIAppToken(app, now); !ok || id != 7 || name != "tito" {

- t.Fatalf("after reopen CLIAppToken = %d %q %v, want 7 tito true", id, name, ok)

+ if p, ok := second.CLIPrincipal(app, now); !ok || p.Kind != PrincipalService || p.AccountID != 7 || p.App != "tito" {

+ t.Fatalf("after reopen CLIPrincipal = %+v %v, want the service principal bound to 7/tito", p, ok)

}

if _, ok := second.CLITokenAddress(app, now); ok {

t.Fatal("after reopen an app token authenticated at a user door")

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

index 289e65332..6256f8b12 100644

--- a/internal/console/svcroutes_test.go

+++ b/internal/console/svcroutes_test.go

@@ -54,6 +54,13 @@ var closedDoors = []string{

// A credential may never mint a credential, including itself. This is

// the escalation cut of spec §3.

"POST /api/cli/apps/{account}/{app}/instance-token",

+ // Enabling instances writes the MANIFEST — standing policy about which

+ // domain, pool, regions and channel the app may ever be brought up on.

+ // A credential the app itself holds must not rewrite the bounds it runs

+ // inside, and none ever has: handleAPIInstanceEnable gates on

+ // apiAccountApp, which could not see a non-person at all. Closing it at

+ // the registration says so where the role table can be read.

+ "PUT /api/cli/apps/{account}/{app}/instances-manifest",

// Enrolling a BOX into a fleet, and rotating or detaching one, is a

// human act with a machine's credentials as its output. Same cut.

"POST /api/cli/fleets/{account}/new",

@@ -142,11 +149,20 @@ var exemptFromLegacyPin = map[string]bool{

// CARLOS_INSTANCE_TOKEN. A tidy `instance` role meaning "the instance verbs"

// would 403 that router on the next console deploy.

+// PUT .../instances-manifest IS ABSENT ON PURPOSE, and its absence is a

+// correction rather than an oversight. The first draft of this list was built

+// from the route registrations filed under "instances" instead of from each

+// handler's actual gate, and it wrongly included the enable door.

+// handleAPIInstanceEnable has always gated on apiAccountApp, which refused a

+// non-person at cliUser with a 401 — no app token has ever reached it. So it

+// is not legacy reach, it is a door that is closed (svcrole.Set{}, see

+// closedDoors) and always was. Do not add it back without first making an app

+// token reach it on a pre-service-principals build.

+//

// 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,

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

index 30c0bd423..a5b94e9c7 100644

--- a/internal/console/svcscope_test.go

+++ b/internal/console/svcscope_test.go

@@ -12,8 +12,10 @@ package console

// tell the machine from the human who minted its token.

import (

+ "context"

"encoding/json"

"net/http"

+ "net/http/httptest"

"sort"

"testing"

"time"

@@ -297,11 +299,9 @@ func TestServiceBoundToAVanishedAccountReachesNothing(t *testing.T) {

// TestEnableStaysAHumanAct: enabling instances is policy — it writes the

// manifest that says which domain, pool, regions and channel an app may ever

-// be brought up on. The role gate lets an `instance` credential as far as the

-// door (the legacy pin, TestInstanceRoleMatchesTodaysAppTokenReach), and the

-// handler's own person check is what refuses it, with the same 401 a

-// fabricated token meets. Collapsing apiInstancePrincipal removed the wall

-// that used to do this by accident; this pins the deliberate one.

+// be brought up on — so the door is registered with the zero Set and answers

+// every service credential, of either kind, with the closed-door 403. This

+// is the OUTER and load-bearing wall (ruling C25).

func TestEnableStaysAHumanAct(t *testing.T) {

fx, _ := instanceFixture(t, "code-scope-enable-human")

fx.srv.cfg.AppsDomain = "oncarlos.test"

@@ -314,10 +314,64 @@ func TestEnableStaysAHumanAct(t *testing.T) {

{"named service credential", named},

{"legacy app token", legacy},

} {

- if status, _, body := rawReq(t, http.MethodPut, enableURL(fx, "bab", "titogo"), c.tok, `{}`); status != http.StatusUnauthorized {

- t.Errorf("%s enabling instances = %d %s, want 401", c.what, status, body)

+ status, _, body := rawReq(t, http.MethodPut, enableURL(fx, "bab", "titogo"), c.tok, `{}`)

+ if status != http.StatusForbidden {

+ t.Errorf("%s enabling instances = %d %s, want 403", c.what, status, body)

}

+ // A member still enables, which is the point of the door.

+ member := cliToken(t, fx, "alice@keymail.dev", "code-scope-enable-member")

+ if status, _, body := rawReq(t, http.MethodPut, enableURL(fx, "bab", "titogo"), member, `{}`); status != http.StatusOK {

+ t.Errorf("member enabling instances = %d %s, want 200", status, body)

+ }

+// TestApiPersonIsTheInnerWall tests apiPerson DIRECTLY, because the two doors

+// that use it are both registered with the zero Set and so can never deliver

+// a service principal to it through the mux.

+//

+// That is exactly why it is worth testing rather than deleting. The

+// registration is the load-bearing wall; this is the second one, and it

+// exists because a handler whose semantics are person-only should not depend

+// solely on a line in another file staying right. Before this branch the

+// refusal was implicit — apiAccountApp resolved through cliUser, which

+// answers for nobody but a person — and collapsing that resolution into the

+// principal-aware path destroyed the invariant. apiPerson restores it as

+// something stated rather than inherited.

+func TestApiPersonIsTheInnerWall(t *testing.T) {

+ fx := newConsoleFixture(t)

+ for _, c := range []struct {

+ what string

+ p Principal

+ want bool

+ }{

+ {"a person", Principal{Kind: PrincipalUser, Address: "paul@keymail.dev"}, true},

+ {"a named service credential",

+ Principal{Kind: PrincipalService, Address: serviceMinter, AccountID: OpsAccountID,

+ Name: "hub", Role: svcrole.Admin}, false},

+ {"a legacy app token",

+ Principal{Kind: PrincipalService, Address: serviceMinter, AccountID: OpsAccountID,

+ App: "kass", Role: svcrole.Instance}, false},

+ } {

+ r := httptest.NewRequest(http.MethodPut, "/api/cli/apps/bab/kass/instances-manifest", nil)

+ r = r.WithContext(context.WithValue(r.Context(), cliRouteKey{}, c.p))

+ rec := httptest.NewRecorder()

+ if got := fx.srv.apiPerson(rec, r); got != c.want {

+ t.Errorf("apiPerson(%s) = %v, want %v", c.what, got, c.want)

+ }

+ if c.want {

+ continue

+ }

+ if rec.Code != http.StatusUnauthorized {

+ t.Errorf("apiPerson(%s) wrote %d, want the 401 cliUser gives", c.what, rec.Code)

+ }

+ // A request that never went through cliRoute carries no principal, and

+ // must not read as a zero-valued user principal with no address.

+ rec := httptest.NewRecorder()

+ if fx.srv.apiPerson(rec, httptest.NewRequest(http.MethodPut, "/x", nil)) {

+ t.Error("apiPerson passed a request with no principal on it")

+ }

}

// TestServiceBindingDoesNotOutliveTheApp pins a deliberate NARROWING of the

@@ -329,7 +383,8 @@ func TestEnableStaysAHumanAct(t *testing.T) {

// HasAppIn a person's request has always been held to — so the credential

// now stops where the app does. That is the right answer (a trashed app is

// not a thing to provision into) and it is a change, so it gets a test

-// rather than a discovery.

+// rather than a discovery. It is a NARROWING and it is meant: do not "fix"

+// it back by dropping the HasAppIn call for service principals.

func TestServiceBindingDoesNotOutliveTheApp(t *testing.T) {

fx := newConsoleFixture(t)

now := time.Now()