carlos / platform Public

console: service-principal review fixes (Risk A/B, F1-F11)

Fix round on d4eebbe per the coordinator's review rulings.

Risk A: revoked_at is a tombstone, not history. Struck the
last-used-history rationale from RevokeServicePrincipal's comment; it is
kept so CLIPrincipal keeps refusing the secret and a re-mint of the same
name is a new row, not a resurrection. ServicePrincipals already filtered
revoked_at = 0; that stands.

Risk B: CLITokens gains AND revoked_at = 0, matching its own doc comment's
claim to list "live" credentials -- a soft-revoked service principal no
longer renders there as an unbadged, still-revocable row.

F3b/C14: RevokeAccountToken is now a soft UPDATE (same shape as
RevokeServicePrincipal), takes now time.Time, and stays broad -- no kind
filter -- because a person's cli_tokens row always carries account_id = 0,
so it can never reach a human's terminal token. It is the account pane's
one universal revoke handle.

F7/C15: RotateServicePrincipal no longer overwrites created_at; only
token_hash and last_used move, so a year-old credential does not read as
freshly minted on the page an owner uses to judge staleness.

F1: MoveApp and RenameAppIn widened to kind IN (?, ?) with cliKindApp and
cliKindService, in both the stale-token guard and the moving UPDATE -- an
app-scoped service principal now follows a rename/move instead of becoming
an orphaned credential a later same-named app inherits unminted. Covered
by TestRenameAppMovesServicePrincipalToo and
TestMoveAppMovesServicePrincipalToo.

F2: NewServicePrincipal rejects name == "" -- an empty-named row would be
addressable by neither Rotate nor Revoke (both guard on name != '').

F4: added the ErrNoServicePrincipal sentinel; RotateServicePrincipal,
RevokeServicePrincipal and RevokeAccountToken all return it instead of an
anonymous errors.New, so a door can render 404 vs 500 without
string-matching.

F5: added tests for NeverExpires() and IsService() on a kind='service' row
(C8 had shipped with no test).

F6: extracted scanCLITokenRows, shared by CLITokens and ServicePrincipals,
so the legacy-role normalization comment's "one place" claim is true.

F8: RevokeServicePrincipal takes now time.Time instead of calling
time.Now() internally, matching every neighbouring Store method's
clock-injection convention.

F9: added a UNIQUE partial index (cli_tokens_service_name, WHERE
kind='service' AND revoked_at=0) alongside the plain lookup index, so name
uniqueness among live service principals is structural rather than resting
on SetMaxOpenConns(1) two files away. The transaction's COUNT-then-INSERT
stays, so a collision still surfaces as the friendly ErrServiceNameTaken.

F10: added a test that stamps revoked_at on a user row via raw SQL (the
Go API cannot produce that state) and asserts CLITokenAddress refuses it.

F11: corrected the report's test count (brief has 7 tests, not 11; the
file holds 7 + 4 = 11).

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

Paul Campbell 2 months ago 6d648a87ef274b90993f0fe6dd2abd072f144912 parent 550a889

4 files changed, +284 −86

internal/console/authstore.go +40 −10
internal/console/store.go +27 −21
internal/console/svcprincipal.go +73 −51
internal/console/svcprincipal_test.go +144 −4

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

index 25119d92d..dfb7bac77 100644

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

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

@@ -197,6 +197,21 @@ func (s *Store) migrateCLIAuth() error {

if _, err := s.db.Exec(`CREATE INDEX IF NOT EXISTS cli_tokens_account_name ON cli_tokens (account_id, name);`); err != nil {

return err

}

+ // Name uniqueness among live service principals is enforced by

+ // NewServicePrincipal's own COUNT-then-INSERT inside a transaction, which

+ // is race-free only because SetMaxOpenConns(1) (store.go) gives that

+ // transaction the sole connection — a fact this file cannot see. This

+ // UNIQUE partial index makes the invariant structural rather than

+ // resting on a pool setting two files away: it is scoped to

+ // kind='service' AND revoked_at=0 so it composes with the soft revoke —

+ // a revoked `hub` does not block a fresh `hub` of the same name. The

+ // transaction stays too, belt and braces, and is what turns a would-be

+ // constraint violation into the friendly ErrServiceNameTaken.

+ if _, err := s.db.Exec(`CREATE UNIQUE INDEX IF NOT EXISTS cli_tokens_service_name

+ON cli_tokens (account_id, name)

+WHERE kind = 'service' AND revoked_at = 0;`); err != nil {

+ return err

+ }

return nil

}

@@ -537,16 +552,14 @@ func (s *Store) CLITokenAddress(rawToken string, now time.Time) (string, bool) {

return addr, true

}

-// CLITokens lists one address's live CLI credentials, newest first — the

-// listing the tokens page renders so an owner can see, and kill, every

-// terminal that speaks as them.

-func (s *Store) CLITokens(address string) ([]CLIToken, error) {

- rows, err := s.db.Query(`SELECT token_hash, address, label, remote_addr, kind, account_id, app,

-role, name, created_at, expires_at, last_used, revoked_at

-FROM cli_tokens WHERE address = ? ORDER BY created_at DESC, token_hash`, address)

- if err != nil {

- return nil, err

- }

+// scanCLITokenRows drains a `cli_tokens` listing query into CLIToken values.

+// CLITokens (by address) and ServicePrincipals (by account) select the same

+// thirteen columns in the same order and need the same legacy-row

+// normalization — a kind='app' row's `role` column is ” in the database but

+// IS the instance role (spec §5). This is the one place that normalization

+// happens, so neither caller, nor any reader downstream of them, has to

+// re-derive it.

+func scanCLITokenRows(rows *sql.Rows) ([]CLIToken, error) {

defer rows.Close()

var out []CLIToken

for rows.Next() {

@@ -572,6 +585,23 @@ FROM cli_tokens WHERE address = ? ORDER BY created_at DESC, token_hash`, address

return out, rows.Err()

}

+// CLITokens lists one address's live CLI credentials, newest first — the

+// listing the tokens page renders so an owner can see, and kill, every

+// terminal that speaks as them. revoked_at = 0: a soft-revoked service

+// principal minted under this address (see RevokeServicePrincipal) must not

+// render here as an unbadged, still-live, still-revocable row — the comment

+// above already promised "live", and until this filter the query did not

+// keep that promise.

+func (s *Store) CLITokens(address string) ([]CLIToken, error) {

+ rows, err := s.db.Query(`SELECT token_hash, address, label, remote_addr, kind, account_id, app,

+role, name, created_at, expires_at, last_used, revoked_at

+FROM cli_tokens WHERE address = ? AND revoked_at = 0 ORDER BY created_at DESC, token_hash`, address)

+ if err != nil {

+ return nil, err

+ }

+ return scanCLITokenRows(rows)

// RevokeCLIToken kills one token by its listing id, scoped to its owner: an

// id is not a secret, so the address is what makes this safe.

func (s *Store) RevokeCLIToken(address, id string) error {

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

index daa7b4463..babaa4f9a 100644

--- a/internal/console/store.go

+++ b/internal/console/store.go

@@ -1354,15 +1354,20 @@ func (s *Store) MoveApp(app string, from, to int64) error {

if atSrc == 0 {

return fmt.Errorf("console: account %d does not claim %q", from, app)

}

- // App-kind CLI tokens are bound (account, name), so they must follow the

- // app or the running deployment's credential 404s while retaining

- // authority over the name in the SOURCE account. Tokens already at the

- // destination under this name are a purged predecessor's leftovers —

- // re-homing onto them would hand this app a credential nobody minted for

- // it, so that refuses, exactly as RenameAppIn refuses.

+ // App-scoped CLI tokens — legacy kind='app' AND kind='service' — are

+ // bound (account, name), so they must follow the app or the running

+ // deployment's credential 404s while retaining authority over the name

+ // in the SOURCE account. Tokens already at the destination under this

+ // name are a purged predecessor's leftovers — re-homing onto them would

+ // hand this app a credential nobody minted for it, so that refuses,

+ // exactly as RenameAppIn refuses. Both kinds must be counted here: a

+ // service principal the guard cannot see is an orphaned credential a

+ // LATER app of the same name inherits unminted (see

+ // TestRenameAppMovesServicePrincipalToo for the RenameAppIn twin of this

+ // hazard).

var staleTokens int

- if err := tx.QueryRow(`SELECT COUNT(*) FROM cli_tokens WHERE kind = ? AND account_id = ? AND app = ?`,

- cliKindApp, to, app).Scan(&staleTokens); err != nil {

+ if err := tx.QueryRow(`SELECT COUNT(*) FROM cli_tokens WHERE kind IN (?, ?) AND account_id = ? AND app = ?`,

+ cliKindApp, cliKindService, to, app).Scan(&staleTokens); err != nil {

return err

}

if staleTokens > 0 {

@@ -1372,8 +1377,8 @@ func (s *Store) MoveApp(app string, from, to int64) error {

to, app, from); err != nil {

return err

}

- if _, err := tx.Exec(`UPDATE cli_tokens SET account_id = ? WHERE kind = ? AND account_id = ? AND app = ?`,

- to, cliKindApp, from, app); err != nil {

+ if _, err := tx.Exec(`UPDATE cli_tokens SET account_id = ? WHERE kind IN (?, ?) AND account_id = ? AND app = ?`,

+ to, cliKindApp, cliKindService, from, app); err != nil {

return err

}

return tx.Commit()

@@ -1391,16 +1396,17 @@ func (s *Store) MoveApp(app string, from, to int64) error {

// keyed by bare name across ALL accounts, so rewriting them could hand

// one account's grant to another, and renaming ONTO one would let the new

// name inherit a grant nobody issued it. Claim-proof stays claim-proof.

-// - The new name must carry no app-kind CLI tokens. A purged app's tokens

-// outlive its row, and a credential that suddenly binds to a NEW app is

-// authority nobody minted.

+// - The new name must carry no app-scoped CLI tokens — legacy kind='app'

+// OR kind='service'. A purged app's tokens outlive its row, and a

+// credential that suddenly binds to a NEW app is authority nobody

+// minted.

// What follows the app: its audit rows — but only those written since THIS

// app's row was created, because a purged predecessor of the same name left

-// its rows behind and its history is not this app's; and its app-kind CLI

-// tokens, which are bound (account, name) and would otherwise 404 the

-// running app while retaining authority over the freed name. The rename

-// itself is the caller's audit line.

+// its rows behind and its history is not this app's; and its app-scoped CLI

+// tokens (both kinds), which are bound (account, name) and would otherwise

+// 404 the running app while retaining authority over the freed name. The

+// rename itself is the caller's audit line.

func (s *Store) RenameAppIn(accountID int64, from, to string) error {

tx, err := s.db.Begin()

if err != nil {

@@ -1430,8 +1436,8 @@ func (s *Store) RenameAppIn(accountID int64, from, to string) error {

return fmt.Errorf("console: this account already has an app (or a trashed app) named %q", to)

}

var staleTokens int

- if err := tx.QueryRow(`SELECT COUNT(*) FROM cli_tokens WHERE kind = ? AND account_id = ? AND app = ?`,

- cliKindApp, accountID, to).Scan(&staleTokens); err != nil {

+ if err := tx.QueryRow(`SELECT COUNT(*) FROM cli_tokens WHERE kind IN (?, ?) AND account_id = ? AND app = ?`,

+ cliKindApp, cliKindService, accountID, to).Scan(&staleTokens); err != nil {

return err

}

if staleTokens > 0 {

@@ -1445,8 +1451,8 @@ func (s *Store) RenameAppIn(accountID int64, from, to string) error {

to, accountID, from, createdAt); err != nil {

return err

}

- if _, err := tx.Exec(`UPDATE cli_tokens SET app = ? WHERE kind = ? AND account_id = ? AND app = ?`,

- to, cliKindApp, accountID, from); err != nil {

+ if _, err := tx.Exec(`UPDATE cli_tokens SET app = ? WHERE kind IN (?, ?) AND account_id = ? AND app = ?`,

+ to, cliKindApp, cliKindService, accountID, from); err != nil {

return err

}

return tx.Commit()

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

index 4d7d3d1ac..6fa759179 100644

--- a/internal/console/svcprincipal.go

+++ b/internal/console/svcprincipal.go

@@ -56,6 +56,13 @@ type Principal struct {

// already holds a live credential of that name.

var ErrServiceNameTaken = errors.New("console: a service credential of that name already exists")

+// ErrNoServicePrincipal is returned by RotateServicePrincipal,

+// RevokeServicePrincipal and RevokeAccountToken when nothing matched —

+// zero rows affected, not a database failure. A door needs to tell the two

+// apart (404 vs 500) without string-matching an error, which an anonymous

+// errors.New at each call site would force it to do.

+var ErrNoServicePrincipal = errors.New("console: no such service credential")

// CLIPrincipal resolves a bearer token to whichever kind of caller minted it.

// ok is false for unknown, revoked, expired tokens, and — for a kind='service'

// row — for a role name this binary does not recognise: a caller that reads

@@ -117,11 +124,25 @@ FROM cli_tokens WHERE token_hash = ?`, hash).

// reason — the credential speaks for a service, but a human stays

// answerable for it.

+// name must be non-empty. ” is not a name a caller chose — it is the value

+// every legacy kind='app' row already carries by default, and both

+// RotateServicePrincipal and RevokeServicePrincipal refuse to match it for

+// exactly that reason (a call with an empty name must never reach a legacy

+// row it did not name). Minting a kind='service' row with name=” would

+// create a credential neither of those doors could ever address again except

+// by RevokeAccountToken's raw id, so the store refuses it at the door

+// instead of producing an unaddressable row later. Grammar validation for

+// what a GOOD name looks like stays at the console API (Task 6); this is

+// only "not empty".

+//

// The token never expires (expires_at = 0), the same deliberate trade

// NewAppToken documents: a production 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.

func (s *Store) NewServicePrincipal(accountID int64, name, app string, role svcrole.Role, mintedBy string, now time.Time) (string, error) {

+ if name == "" {

+ return "", errors.New("console: a service credential needs a name")

+ }

if _, ok := svcrole.Parse(string(role)); !ok {

return "", fmt.Errorf("console: unknown service role %q", role)

}

@@ -155,11 +176,14 @@ VALUES (?, ?, '', '', ?, 0, ?, ?, ?, ?, ?)`,

}

// ServicePrincipals lists one account's live service credentials, newest

-// first: kind='service' rows AND legacy kind='app' rows, normalized here to

-// the instance role they have always held. Absorbing the legacy kind into

-// this model must not make it invisible to the account it is bound to — this

-// is the one place that normalization happens, so no reader downstream has

-// to re-derive it.

+// first: kind='service' rows AND legacy kind='app' rows, normalized by

+// scanCLITokenRows to the instance role they have always held. Absorbing the

+// legacy kind into this model must not make it invisible to the account it

+// is bound to.

+//

+// revoked_at = 0: a revoked row is a tombstone (see RevokeServicePrincipal),

+// not history to display — it stays out of this listing the same way it

+// stays out of CLITokens.

func (s *Store) ServicePrincipals(accountID int64) ([]CLIToken, error) {

rows, err := s.db.Query(`SELECT token_hash, address, label, remote_addr, kind, account_id, app,

role, name, created_at, expires_at, last_used, revoked_at

@@ -168,48 +192,30 @@ ORDER BY created_at DESC, token_hash`, accountID, cliKindService, cliKindApp)

if err != nil {

return nil, err

}

- defer rows.Close()

- var out []CLIToken

- for rows.Next() {

- var t CLIToken

- var created, expires, used, revoked int64

- if err := rows.Scan(&t.ID, &t.Address, &t.Label, &t.RemoteAddr, &t.Kind, &t.AccountID, &t.App,

- &t.Role, &t.Name, &created, &expires, &used, &revoked); err != nil {

- return nil, err

- }

- t.CreatedAt = time.Unix(created, 0).UTC()

- t.ExpiresAt = time.Unix(expires, 0).UTC()

- if used != 0 {

- t.LastUsed = time.Unix(used, 0).UTC()

- }

- if revoked != 0 {

- t.RevokedAt = time.Unix(revoked, 0).UTC()

- }

- if t.Kind == cliKindApp {

- t.Role = string(svcrole.Instance)

- }

- out = append(out, t)

- }

- return out, rows.Err()

+ return scanCLITokenRows(rows)

}

// RotateServicePrincipal mints a fresh secret for a live, NAMED credential

// and retires the old one in the same statement: same row, same name, same

-// role, only the secret moves. Zero rows affected — including a call with

-// name="" — is an error the door renders as 404.

+// role, same age — only the secret moves and last_used resets, because a

+// fresh secret has not been used yet. created_at is deliberately left alone:

+// overwriting it would make a credential granted a year ago read as newly

+// minted on the very page an owner uses to decide what is stale. Zero rows

+// affected — including a call with name="" — is ErrNoServicePrincipal, which

+// the door renders as 404.

// name != ” guards against a legacy kind='app' row, whose name column is

// always ”: without it, RotateServicePrincipal(acctID, "") would match that

-// row's own nameless twin (or, if the account somehow held more than one, all

-// of them) and rotate a credential the caller never named.

+// row (or, if the account somehow held more than one, all of them) and

+// rotate a credential the caller never named.

func (s *Store) RotateServicePrincipal(accountID int64, name string, now time.Time) (string, error) {

raw, err := randomToken()

if err != nil {

return "", err

}

- res, err := s.db.Exec(`UPDATE cli_tokens SET token_hash = ?, created_at = ?, last_used = 0

+ res, err := s.db.Exec(`UPDATE cli_tokens SET token_hash = ?, last_used = 0

WHERE kind = ? AND account_id = ? AND name = ? AND name != '' AND revoked_at = 0`,

- hashToken(raw), now.Unix(), cliKindService, accountID, name)

+ hashToken(raw), cliKindService, accountID, name)

if err != nil {

return "", err

}

@@ -218,19 +224,25 @@ WHERE kind = ? AND account_id = ? AND name = ? AND name != '' AND revoked_at = 0

return "", err

}

if n == 0 {

- return "", errors.New("console: no such service credential")

+ return "", ErrNoServicePrincipal

}

return raw, nil

}

-// RevokeServicePrincipal soft-revokes a live, NAMED credential — UPDATE, not

-// DELETE, so ServicePrincipals can still show a revoked credential's last-used

-// time to whoever is working out what broke. name != ” guards against a

-// legacy kind='app' row for the same reason RotateServicePrincipal needs it.

-func (s *Store) RevokeServicePrincipal(accountID int64, name string) error {

+// RevokeServicePrincipal soft-revokes a live, NAMED credential: UPDATE, not

+// DELETE. The row is kept as a TOMBSTONE, not as history for a listing to

+// show — nothing lists a revoked row (ServicePrincipals and CLITokens both

+// filter revoked_at = 0). It is kept for two reasons: so CLIPrincipal goes on

+// refusing this exact secret rather than a gap in the table quietly

+// resolving it again, and so a later NewServicePrincipal minting the same

+// name creates a NEW row rather than resurrecting this one's identity —

+// the unique index (cli_tokens_service_name) is scoped to live rows for

+// exactly that reason. name != ” guards against a legacy kind='app' row for

+// the same reason RotateServicePrincipal needs it.

+func (s *Store) RevokeServicePrincipal(accountID int64, name string, now time.Time) error {

res, err := s.db.Exec(`UPDATE cli_tokens SET revoked_at = ?

WHERE kind = ? AND account_id = ? AND name = ? AND name != '' AND revoked_at = 0`,

- time.Now().Unix(), cliKindService, accountID, name)

+ now.Unix(), cliKindService, accountID, name)

if err != nil {

return err

}

@@ -239,19 +251,29 @@ WHERE kind = ? AND account_id = ? AND name = ? AND name != '' AND revoked_at = 0

return err

}

if n == 0 {

- return errors.New("console: no such service credential")

+ return ErrNoServicePrincipal

}

return nil

}

-// RevokeAccountToken kills one credential by its listing id (the token hash),

-// scoped to the ACCOUNT rather than to the address that minted it.

-// RevokeCLIToken scopes by address, which is right for a person killing their

-// own terminal and wrong for an owner revoking a credential a colleague

-// minted — and legacy kind='app' rows have no name to address them by, so

-// this is the only handle they have.

-func (s *Store) RevokeAccountToken(accountID int64, id string) error {

- res, err := s.db.Exec(`DELETE FROM cli_tokens WHERE token_hash = ? AND account_id = ?`, id, accountID)

+// RevokeAccountToken soft-revokes one credential by its listing id (the

+// token hash), scoped to the ACCOUNT rather than to the address that minted

+// it — the universal handle: it is the only path that works for every row

+// the account pane lists, service-named or legacy-nameless alike, so that

+// pane needs exactly one Revoke button rather than one per kind.

+// RevokeCLIToken scopes by address instead, which is right for a person

+// killing their own terminal — but a legacy kind='app' row has no name to

+// address by RevokeServicePrincipal, so this is the only handle it has, and

+// an owner revoking a credential a colleague minted needs the account, not

+// the minter's address, as the scope.

+//

+// The broad query (no kind filter) is not a hazard: a person's cli_tokens

+// row always carries account_id = 0 (see CLIToken), so an account-scoped

+// revoke can never reach a human's terminal token — there is no account id

+// for it to match.

+func (s *Store) RevokeAccountToken(accountID int64, id string, now time.Time) error {

+ res, err := s.db.Exec(`UPDATE cli_tokens SET revoked_at = ?

+WHERE token_hash = ? AND account_id = ? AND revoked_at = 0`, now.Unix(), id, accountID)

if err != nil {

return err

}

@@ -260,7 +282,7 @@ func (s *Store) RevokeAccountToken(accountID int64, id string) error {

return err

}

if n == 0 {

- return errors.New("console: no such token")

+ return ErrNoServicePrincipal

}

return nil

}

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

index 834db70d2..d619049ce 100644

--- a/internal/console/svcprincipal_test.go

+++ b/internal/console/svcprincipal_test.go

@@ -116,7 +116,7 @@ func TestRevokedServicePrincipalStopsResolving(t *testing.T) {

if err != nil {

t.Fatalf("NewServicePrincipal: %v", err)

}

- if err := s.RevokeServicePrincipal(acct.ID, "hub"); err != nil {

+ if err := s.RevokeServicePrincipal(acct.ID, "hub", now); err != nil {

t.Fatalf("RevokeServicePrincipal: %v", err)

}

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

@@ -250,7 +250,7 @@ func TestRotateAndRevokeIgnoreLegacyNamelessRows(t *testing.T) {

if _, err := s.RotateServicePrincipal(acct.ID, "", now); err == nil {

t.Fatal("RotateServicePrincipal(\"\") succeeded against a legacy row; want a refusal")

}

- if err := s.RevokeServicePrincipal(acct.ID, ""); err == nil {

+ if err := s.RevokeServicePrincipal(acct.ID, "", now); err == nil {

t.Fatal("RevokeServicePrincipal(\"\") succeeded against a legacy row; want a refusal")

}

// The legacy app token must still work: neither call touched it.

@@ -272,16 +272,156 @@ func TestRevokeAccountTokenScopesByAccountNotAddress(t *testing.T) {

t.Fatalf("NewAppToken: %v", err)

}

id := hashToken(app)

- if err := s.RevokeAccountToken(other.ID, id); err == nil {

+ if err := s.RevokeAccountToken(other.ID, id, now); err == nil {

t.Fatal("a different account revoked this one's token")

}

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

t.Fatal("token stopped working after a refused cross-account revoke")

}

- if err := s.RevokeAccountToken(acct.ID, id); err != nil {

+ if err := s.RevokeAccountToken(acct.ID, id, now); err != nil {

t.Fatalf("RevokeAccountToken: %v", err)

}

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

t.Error("token still resolves after RevokeAccountToken")

}

+// TestNewServicePrincipalRejectsEmptyName: ” is not a name a caller chose —

+// it is the default every legacy kind='app' row already carries — and both

+// RotateServicePrincipal and RevokeServicePrincipal refuse to match it, so a

+// kind='service' row minted with name=” would be unaddressable except by

+// RevokeAccountToken's raw id. The store refuses it at the door instead.

+func TestNewServicePrincipalRejectsEmptyName(t *testing.T) {

+ s := openTestStore(t)

+ now := time.Now()

+ acct := mustAccount(t, s, "amadan")

+ if _, err := s.NewServicePrincipal(acct.ID, "", "", svcrole.Publish, "paul@example.test", now); err == nil {

+ t.Fatal("NewServicePrincipal(name=\"\") succeeded; want a refusal")

+ }

+// TestServiceKindNeverExpiresAndIsService is C8's missing test: a

+// kind='service' row must report NeverExpires() == true (expires_at is 0 by

+// design) and IsService() == true — otherwise templates/settings.html would

+// render every service credential as expiring 1 Jan 1970, the exact failure

+// C8 exists to prevent.

+func TestServiceKindNeverExpiresAndIsService(t *testing.T) {

+ s := openTestStore(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("NewServicePrincipal: %v", err)

+ }

+ toks, err := s.ServicePrincipals(acct.ID)

+ if err != nil {

+ t.Fatal(err)

+ }

+ if len(toks) != 1 {

+ t.Fatalf("ServicePrincipals = %d rows, want 1", len(toks))

+ }

+ tok := toks[0]

+ if !tok.IsService() {

+ t.Error("IsService() = false for a kind='service' row")

+ }

+ if tok.IsApp() {

+ t.Error("IsApp() = true for a kind='service' row")

+ }

+ if !tok.NeverExpires() {

+ t.Error("NeverExpires() = false for a service principal; would render as expiring 1 Jan 1970")

+ }

+// TestCLITokenAddressRefusesRevokedUserRow exercises a state the Go API

+// cannot otherwise produce (RevokeCLIToken deletes rather than soft-revokes

+// a user row): same raw-SQL technique as

+// TestUnparseableRoleRefusesRatherThanFallingThrough, proving the

+// revoked_at != 0 branch in CLITokenAddress is live code, not dead code.

+func TestCLITokenAddressRefusesRevokedUserRow(t *testing.T) {

+ s := openTestStore(t)

+ now := time.Now()

+ raw := mintUserToken(t, s, "paul@example.test", now)

+ if _, err := s.db.Exec(`UPDATE cli_tokens SET revoked_at = ? WHERE token_hash = ?`, now.Unix(), hashToken(raw)); err != nil {

+ t.Fatal(err)

+ }

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

+ t.Errorf("CLITokenAddress(revoked user row) = %q,true; want \"\",false", addr)

+ }

+// TestRevokedNameIsFreeForAFreshMint: RevokeServicePrincipal tombstones the

+// row rather than deleting it (Risk A), and the unique index

+// (cli_tokens_service_name) is scoped to live rows — a revoked `hub` must

+// not permanently squat the name.

+func TestRevokedNameIsFreeForAFreshMint(t *testing.T) {

+ s := openTestStore(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.RevokeServicePrincipal(acct.ID, "hub", now); err != nil {

+ t.Fatalf("RevokeServicePrincipal: %v", err)

+ }

+ if _, err := s.NewServicePrincipal(acct.ID, "hub", "", svcrole.Operate, "paul@example.test", now); err != nil {

+ t.Fatalf("re-mint after revoke: %v", err)

+ }

+// TestRenameAppMovesServicePrincipalToo is F1's authority bug, made

+// concrete: an app-scoped service principal that does not follow a rename

+// becomes an orphaned credential a LATER app of the same (freed) name

+// inherits — authority nobody minted it for.

+func TestRenameAppMovesServicePrincipalToo(t *testing.T) {

+ s := openTestStore(t)

+ now := time.Now()

+ acct := mustAccount(t, s, "amadan")

+ if _, err := s.CreateAppIn(acct.ID, "titogo", "paul@example.test", now); err != nil {

+ t.Fatalf("CreateAppIn: %v", err)

+ }

+ raw, err := s.NewServicePrincipal(acct.ID, "hub", "titogo", svcrole.Publish, "paul@example.test", now)

+ if err != nil {

+ t.Fatalf("NewServicePrincipal: %v", err)

+ }

+ if err := s.RenameAppIn(acct.ID, "titogo", "tito"); err != nil {

+ t.Fatalf("RenameAppIn: %v", err)

+ }

+ // The freed name is claimable again — by a brand new app nobody minted a

+ // credential for.

+ if _, err := s.CreateAppIn(acct.ID, "titogo", "paul@example.test", now); err != nil {

+ t.Fatalf("CreateAppIn (fresh titogo): %v", err)

+ }

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

+ if !ok {

+ t.Fatal("CLIPrincipal(hub) = !ok after rename; want ok")

+ }

+ if p.App != "tito" {

+ t.Errorf("App = %q after rename; want the RENAMED app (tito) — not the freed name a new, different app now holds", p.App)

+ }

+// TestMoveAppMovesServicePrincipalToo is the MoveApp twin of the rename

+// guard fix: an app-scoped service principal must follow its app across

+// accounts, the same way a legacy app-kind token already does.

+func TestMoveAppMovesServicePrincipalToo(t *testing.T) {

+ s := openTestStore(t)

+ now := time.Now()

+ from := mustAccount(t, s, "amadan")

+ to := mustAccount(t, s, "other")

+ if _, err := s.CreateAppIn(from.ID, "titogo", "paul@example.test", now); err != nil {

+ t.Fatalf("CreateAppIn: %v", err)

+ }

+ raw, err := s.NewServicePrincipal(from.ID, "hub", "titogo", svcrole.Publish, "paul@example.test", now)

+ if err != nil {

+ t.Fatalf("NewServicePrincipal: %v", err)

+ }

+ if err := s.MoveApp("titogo", from.ID, to.ID); err != nil {

+ t.Fatalf("MoveApp: %v", err)

+ }

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

+ if !ok {

+ t.Fatal("CLIPrincipal(hub) = !ok after move; want ok")

+ }

+ if p.AccountID != to.ID {

+ t.Errorf("AccountID = %d after move; want %d (the destination account)", p.AccountID, to.ID)

+ }