merge: account names as addresses — /a/amadan resolves, ambiguity is loud, -account-id never guesses (#46)
Symmetric membership-scoped resolution in console, API and CLI; picker on ambiguity; names validated at the three human creation edges; test credential isolation after the Kass-accounts incident.
31 files changed,
+2474
−282
CLAUDE.md+4 −0accounts.go+87 −36accounts_resolve_test.go+115 −0apimode.go+52 −21apimode_test.go+87 −42appconfig_cli.go+8 −8auth_e2e_test.go+72 −6cli.go+80 −53docs/superpowers/plans/2026-08-04-account-slugs.md+916 −0docs/superpowers/specs/2026-08-04-account-slugs-design.md+187 −0e2e_test.go+128 −27internal/console/apicli.go+36 −30internal/console/apicli_test.go+83 −11internal/console/authcli.go+5 −2internal/console/authcli_test.go+8 −2internal/console/authowner_test.go+1 −1internal/console/resolve.go+47 −0internal/console/resolve_test.go+92 −0internal/console/server.go+66 −21internal/console/server_account_test.go+208 −8internal/console/server_test.go+5 −3internal/console/store.go+4 −0internal/console/templates/apps.html+1 −1internal/console/templates/layout.html+2 −2internal/console/templates/newaccount.html+8 −6internal/console/templates/picker.html+22 −0internal/domains/slugname.go+16 −0internal/domains/slugname_test.go+23 −0internal/domains/sqid.go+36 −0internal/domains/sqid_test.go+56 −0scopedkeys_e2e_test.go+19 −2
diff --git a/CLAUDE.md b/CLAUDE.md| index b1797c72e..c1c195707 100644 |
| --- a/CLAUDE.md |
| +++ b/CLAUDE.md |
| @@ -50,6 +50,10 @@ capable model — exact values, file paths and interfaces go in the brief. |
| "verified" reports in one day. |
| - Icons are vendored Lucide (in-code SVG constants, never CDN); emoji is |
| content, icons are chrome. |
| +- **A test that shells out to the carlos binary must set `CARLOS_CREDENTIALS` |
| + into its tempdir.** The fallback is the operator's real login; a logged-in |
| + laptop running `go test ./...` minted 40+ real accounts on the production |
| + console before this was isolated (`carlosTestEnv`). |
| ## Deploy reality (2026-08-02) |
diff --git a/accounts.go b/accounts.go| index d7e46b3c3..3b3d9911f 100644 |
| --- a/accounts.go |
| +++ b/accounts.go |
| @@ -79,14 +79,20 @@ func RunAccounts(args []string) { |
| func runAccountsCreate(args []string) { |
| fs := flag.NewFlagSet("accounts create", flag.ExitOnError) |
| regPath := fs.String("registry", defaultRegistry(), "registry database") |
| - name := fs.String("name", "", "account name (free text, need not be unique)") |
| + name := fs.String("name", "", "account name (a-z, 0-9, hyphen; need not be unique)") |
| owner := fs.String("owner", "", "keymail address that owns the account — the box-side/headless path, "+ |
| "and what to use when this terminal is not logged in") |
| console := fs.String("console", "", "which logged-in console to create it through (only needed with several)") |
| fs.Parse(args) |
| - if strings.TrimSpace(*name) == "" { |
| + *name = strings.TrimSpace(*name) |
| + if *name == "" { |
| log.Fatal("usage: carlos accounts create -name <name> [-owner <address>]") |
| } |
| + if !domains.ValidAccountName(*name) { |
| + log.Fatalf("accounts create: %q is not a usable account name — 1-63 characters of a-z, 0-9\n"+ |
| + "and hyphen, no leading or trailing hyphen. The name appears in console URLs and\n"+ |
| + "resolves -account.", *name) |
| + } |
| // An ownerless account is the failure this whole path exists to prevent: |
| // it materializes into the console with no members, which means nobody can |
| @@ -619,11 +625,81 @@ func verifyMigratedPointers(ctx context.Context, store release.Store, app, srcAc |
| } |
| } |
| -// resolveAccount turns a -account value into an account sqid: an exact sqid |
| -// match wins, then a unique name match; an ambiguous name is an error that |
| -// lists the candidates rather than picking one. Friendly without being |
| -// guessy — picking the wrong account here would ship to the wrong tenant. |
| -func resolveAccount(regPath, want string) string { |
| +// pickAccount is resolveAccount's core, symmetric over the two identity |
| +// spaces: want matches an account by sqid OR by name (case-insensitively), |
| +// with NO precedence between them. Precedence is how a script's -account |
| +// silently changes target the day a colliding account materializes; a |
| +// conflict here is loud instead, and names -account-id, the reference that |
| +// never guesses. A sqid-shaped want that matches nothing passes through (a |
| +// fresh box may not have reconciled; CI has no registry at all). |
| +func pickAccount(accts []registry.Account, want string) (string, error) { |
| + var candidates []registry.Account |
| + for _, a := range accts { |
| + if a.Sqid == want || strings.EqualFold(a.Name, want) { |
| + candidates = append(candidates, a) |
| + } |
| + } |
| + switch len(candidates) { |
| + case 1: |
| + return candidates[0].Sqid, nil |
| + case 0: |
| + // A sqid this registry hasn't materialized yet is still usable: the |
| + // bucket is the control plane, and a fresh box may simply not have |
| + // reconciled. A NAME, though, can only ever be resolved here. |
| + if release.ValidAccount(want) { |
| + return want, nil |
| + } |
| + return "", fmt.Errorf("no account %q here — `carlos accounts list` shows what this box knows", want) |
| + default: |
| + alts := make([]string, len(candidates)) |
| + for i, a := range candidates { |
| + alts[i] = fmt.Sprintf("%s (%q)", a.Sqid, a.Name) |
| + } |
| + return "", fmt.Errorf("account %q is ambiguous — pass -account-id with one of: %s", |
| + want, strings.Join(alts, ", ")) |
| + } |
| +} |
| + |
| +// checkAccountID is the ONE gate -account-id passes through, shared by |
| +// resolveAccount (direct-bucket mode) and apiAccount (API mode) so the two |
| +// modes can never disagree about what a sqid is. It returns the trimmed id, |
| +// or "" when the flag was not given. |
| +// |
| +// The check is domains.ValidSqid, NOT release.ValidAccount. ValidAccount |
| +// accepts any lowercase word because it gates bucket path segments, and |
| +// every account NAME is one — so it accepts "ops", which the frozen encoder |
| +// (no i, l, o) can never emit for any id. Waved through here, `-account-id |
| +// ops` would have shipped to apps/ops/..., exited 0, and served nothing: the |
| +// real ops account is "bab". A name in -account-id is a mistake with no |
| +// honest reading, so it is refused by shape before anything else happens. |
| +func checkAccountID(wantID string) string { |
| + wantID = strings.TrimSpace(wantID) |
| + if wantID == "" || domains.ValidSqid(wantID) { |
| + return wantID |
| + } |
| + // "ops" is THE trap: it is the default -account value, the one account |
| + // name every operator types, and it is name-shaped in a way that reads |
| + // exactly like a short sqid. Name its actual sqid rather than making |
| + // someone go look it up. |
| + if wantID == "ops" { |
| + log.Fatalf(`-account-id takes a sqid; "ops" is a name — use -account ops, or -account-id %s`, release.OpsAccount) |
| + } |
| + log.Fatalf("-account-id %q is not a sqid (sqids use only the letters %s — no i, l, o) — "+ |
| + "it looks like a NAME; names go in -account", wantID, domains.SqidAlphabet) |
| + return "" |
| +} |
| + |
| +// resolveAccount turns a -account/-account-id pair into an account sqid. |
| +// -account-id supersedes -account outright: it is a sqid, matched as a sqid |
| +// only, never consulted against the registry at all — the reference that |
| +// cannot be shadowed by a name. Otherwise -account resolves through |
| +// pickAccount, symmetrically over sqids and names, and an ambiguity is fatal |
| +// rather than guessed: picking the wrong account here would ship to the |
| +// wrong tenant. |
| +func resolveAccount(regPath, want, wantID string) string { |
| + if wantID = checkAccountID(wantID); wantID != "" { |
| + return wantID |
| + } |
| want = strings.TrimSpace(want) |
| if want == "" || want == "ops" { |
| return release.OpsAccount |
| @@ -644,34 +720,9 @@ func resolveAccount(regPath, want string) string { |
| if err != nil { |
| log.Fatal(err) |
| } |
| - for _, a := range accts { |
| - if a.Sqid == want { |
| - return a.Sqid |
| - } |
| - } |
| - var matches []registry.Account |
| - for _, a := range accts { |
| - if strings.EqualFold(a.Name, want) { |
| - matches = append(matches, a) |
| - } |
| - } |
| - switch len(matches) { |
| - case 1: |
| - return matches[0].Sqid |
| - case 0: |
| - // A sqid this registry hasn't materialized yet is still usable: the |
| - // bucket is the control plane, and a fresh box may simply not have |
| - // reconciled. A NAME, though, can only ever be resolved here. |
| - if release.ValidAccount(want) { |
| - return want |
| - } |
| - log.Fatalf("no account %q here — `carlos accounts list` shows what this box knows", want) |
| - default: |
| - var names []string |
| - for _, a := range matches { |
| - names = append(names, fmt.Sprintf("%s (%s)", a.Sqid, a.Name)) |
| - } |
| - log.Fatalf("account %q is ambiguous — use a sqid: %s", want, strings.Join(names, ", ")) |
| + sqid, err := pickAccount(accts, want) |
| + if err != nil { |
| + log.Fatal(err) |
| } |
| - return release.OpsAccount |
| + return sqid |
| } |
diff --git a/accounts_resolve_test.go b/accounts_resolve_test.go| new file mode 100644 |
| index 000000000..4464126ab |
| --- /dev/null |
| +++ b/accounts_resolve_test.go |
| @@ -0,0 +1,115 @@ |
| +package main |
| + |
| +import ( |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/domains" |
| + "github.com/carlosframework/platform/internal/registry" |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +func TestPickAccount(t *testing.T) { |
| + accts := []registry.Account{ |
| + {ID: 1, Sqid: "bab", Name: "ops"}, |
| + {ID: 2, Sqid: "bah", Name: "amadan"}, |
| + {ID: 3, Sqid: "bad", Name: "amadan"}, |
| + {ID: 4, Sqid: "baf", Name: "bah"}, // name collides with #2's sqid |
| + } |
| + if got, err := pickAccount(accts, "bab"); err != nil || got != "bab" { |
| + t.Errorf("sqid: %q, %v", got, err) |
| + } |
| + if got, err := pickAccount(accts, "ops"); err != nil || got != "bab" { |
| + t.Errorf("unique name: %q, %v", got, err) |
| + } |
| + if got, err := pickAccount(accts, "OPS"); err != nil || got != "bab" { |
| + t.Errorf("case-insensitive name: %q, %v", got, err) |
| + } |
| + // A sqid nobody has materialized passes through — a fresh box may not |
| + // have reconciled, and CI has no registry at all. |
| + if got, err := pickAccount(accts, "zzz"); err != nil || got != "zzz" { |
| + t.Errorf("passthrough sqid: %q, %v", got, err) |
| + } |
| + if _, err := pickAccount(accts, "not-a-sqid-or-name"); err == nil { |
| + t.Error("unresolvable non-sqid name must error") |
| + } |
| + // The two ambiguity shapes: a shared name, and a name/sqid collision. |
| + // Both must refuse, and the fix-up named must be -account-id. |
| + for _, want := range []string{"amadan", "bah"} { |
| + _, err := pickAccount(accts, want) |
| + if err == nil || !strings.Contains(err.Error(), "-account-id") { |
| + t.Errorf("pickAccount(%q) = %v, want ambiguity error naming -account-id", want, err) |
| + } |
| + } |
| + if _, err := pickAccount(accts, "bah"); err == nil || |
| + !strings.Contains(err.Error(), "bah") || !strings.Contains(err.Error(), "baf") { |
| + t.Error("name/sqid conflict error must list both candidates") |
| + } |
| +} |
| + |
| +// TestAccountIDShape pins the gate BOTH modes share (checkAccountID, which |
| +// resolveAccount and apiAccount each call). The values are the ones that |
| +// matter: "ops" and "oil" pass release.ValidAccount (^[a-z]{3,16}$) and must |
| +// still be refused, because the frozen encoder has no i, l or o and so can |
| +// never emit them for any id — a phantom account, written to and served by |
| +// nobody. "bab" is real; "zzz" is unminted-but-shaped and must pass, since |
| +// -account-id is the flag you reach for on a box that hasn't reconciled. |
| +// |
| +// The refusals themselves are log.Fatal (os.Exit, unobservable in-process), |
| +// so they are pinned two ways instead: the predicate directly here, and the |
| +// whole `carlos add -account-id ops` invocation end to end in |
| +// TestAddAccountIDRefusesAName (e2e_test.go), which asserts the non-zero |
| +// exit and that no row was written. |
| +func TestAccountIDShape(t *testing.T) { |
| + for _, want := range []string{"bab", "zzz", "baf"} { |
| + if !domains.ValidSqid(want) { |
| + t.Errorf("-account-id %q must be accepted — it is shaped like something Sqid emits", want) |
| + } |
| + // The accept path IS reachable in-process: only a refusal exits. |
| + empty, id := "", want |
| + if a := apiAccount(&empty, &id); a.Seg != want || !a.Exact { |
| + t.Errorf("apiAccount(-account-id %q) = %+v, want {%s true}", want, a, want) |
| + } |
| + } |
| + for _, bad := range []string{"ops", "oil", "ill", "ba", "BAB", "ba1", ""} { |
| + if domains.ValidSqid(bad) { |
| + t.Errorf("-account-id %q must be refused — it is a name, not a sqid", bad) |
| + } |
| + } |
| + // The specific trap the reviewer caught: release.ValidAccount would have |
| + // waved "ops" straight through. If this ever stops being true, the two |
| + // predicates have converged and the gate is doing nothing. |
| + if !release.ValidAccount("ops") { |
| + t.Error("release.ValidAccount no longer accepts \"ops\" — this gate's whole reason to exist has moved") |
| + } |
| +} |
| + |
| +func TestApiAccountExact(t *testing.T) { |
| + acct, id := "ops", "bah" |
| + a := apiAccount(&acct, &id) |
| + if a.Seg != "bah" || !a.Exact { |
| + t.Errorf("apiAccount with -account-id = %+v, want {bah true}", a) |
| + } |
| + id = "" |
| + a = apiAccount(&acct, &id) |
| + if a.Seg != "ops" || a.Exact { |
| + t.Errorf("apiAccount without -account-id = %+v, want {ops false}", a) |
| + } |
| + // Both flags are trimmed the way resolveAccount trims them, so a value |
| + // that picked up whitespace from a shell means the same thing in either |
| + // mode — API mode used to send " ops " to the console verbatim. |
| + padded, id := " ops ", " bah " |
| + if a := apiAccount(&padded, &id); a.Seg != "bah" || !a.Exact { |
| + t.Errorf("apiAccount(padded -account-id) = %+v, want {bah true}", a) |
| + } |
| + id = " " |
| + if a := apiAccount(&padded, &id); a.Seg != "ops" || a.Exact { |
| + t.Errorf("apiAccount(padded -account, blank -account-id) = %+v, want {ops false}", a) |
| + } |
| + if p := apiPath(apiAcct{Seg: "bah", Exact: true}, "web", "/channels"); !strings.HasSuffix(p, "?exact=sqid") { |
| + t.Errorf("apiPath exact = %q, want ?exact=sqid suffix", p) |
| + } |
| + if p := apiPath(apiAcct{Seg: "ops"}, "web", "/channels"); strings.Contains(p, "exact") { |
| + t.Errorf("apiPath non-exact = %q, must carry no exact param", p) |
| + } |
| +} |
diff --git a/apimode.go b/apimode.go| index 0f22b2679..f50e08cd7 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -28,6 +28,7 @@ import ( |
| "os" |
| "path/filepath" |
| "sort" |
| + "strings" |
| "time" |
| "github.com/carlosframework/platform/internal/ledger" |
| @@ -166,12 +167,42 @@ type apiShipResult struct { |
| NoOp bool |
| } |
| -// apiPath builds one /api/cli/apps/{account}/{app}/... path. account is |
| +// apiAcct is one resolved -account/-account-id pair, API-mode: the segment |
| +// the console will resolve, and whether it must match as a sqid only. |
| +type apiAcct struct { |
| + Seg string |
| + Exact bool |
| +} |
| + |
| +// apiAccount is resolveAccount's API-mode counterpart. Names resolve |
| +// SERVER-side against the caller's memberships, so -account travels |
| +// verbatim; -account-id travels as the segment plus exact=sqid, so the |
| +// console never name-matches it. Both flags are trimmed exactly as |
| +// resolveAccount trims them, so `-account " ops "` means the same thing in |
| +// either mode, and -account-id passes the same checkAccountID gate — a name |
| +// there is refused on this side of the wire, not turned into a 404 the |
| +// console cannot explain. |
| +func apiAccount(raw, rawID *string) apiAcct { |
| + if id := checkAccountID(*rawID); id != "" { |
| + return apiAcct{Seg: id, Exact: true} |
| + } |
| + return apiAcct{Seg: strings.TrimSpace(*raw)} |
| +} |
| + |
| +// apiPath builds one /api/cli/apps/{account}/{app}/... path. The segment is |
| // passed through verbatim (sqid or name, exactly what -account carried, |
| // default "ops") — the console resolves it against the caller's own |
| // memberships; this client must never resolve it against a local registry. |
| -func apiPath(account, app, tail string) string { |
| - return "/api/cli/apps/" + url.PathEscape(account) + "/" + url.PathEscape(app) + tail |
| +// -account-id adds ?exact=sqid, the console-side switch that makes the |
| +// segment match as a sqid ONLY (internal/console/apicli.go). Plain string |
| +// concatenation is safe here because no tail this package builds carries a |
| +// query string of its own — every one is a bare path. |
| +func apiPath(a apiAcct, app, tail string) string { |
| + p := "/api/cli/apps/" + url.PathEscape(a.Seg) + "/" + url.PathEscape(app) + tail |
| + if a.Exact { |
| + p += "?exact=sqid" |
| + } |
| + return p |
| } |
| // notFoundErr is what every call here returns for a 404 whose body carries |
| @@ -286,12 +317,12 @@ func stableRefusal(status int, body []byte) (error, bool) { |
| // carries (RUNG names in From/To) is not what the caller prints: the CLI |
| // already knows the version/channel it asked for and prints those, never |
| // res.From/res.To. |
| -func (s *apiSession) promote(ctx context.Context, account, app, version, channel string) (apiPromoteResult, error) { |
| +func (s *apiSession) promote(ctx context.Context, a apiAcct, app, version, channel string) (apiPromoteResult, error) { |
| in := struct { |
| Version string `json:"version"` |
| Channel string `json:"channel"` |
| }{version, channel} |
| - status, body, err := s.postJSON(ctx, apiPath(account, app, "/promote"), in) |
| + status, body, err := s.postJSON(ctx, apiPath(a, app, "/promote"), in) |
| if err != nil { |
| return apiPromoteResult{}, err |
| } |
| @@ -313,11 +344,11 @@ func (s *apiSession) promote(ctx context.Context, account, app, version, channel |
| // rollback asks the console to roll channel back to its previous version. |
| // There is no noop case for a rollback — it either has a previous version to |
| // land on (409 if not) or it doesn't — so this stays error-only. |
| -func (s *apiSession) rollback(ctx context.Context, account, app, channel string) error { |
| +func (s *apiSession) rollback(ctx context.Context, a apiAcct, app, channel string) error { |
| in := struct { |
| Channel string `json:"channel"` |
| }{channel} |
| - status, body, err := s.postJSON(ctx, apiPath(account, app, "/rollback"), in) |
| + status, body, err := s.postJSON(ctx, apiPath(a, app, "/rollback"), in) |
| if err != nil { |
| return err |
| } |
| @@ -331,8 +362,8 @@ func (s *apiSession) rollback(ctx context.Context, account, app, channel string) |
| } |
| // channels reads one app's channel ladder through the console. |
| -func (s *apiSession) channels(ctx context.Context, account, app string) ([]apiChannel, error) { |
| - status, body, err := s.getJSON(ctx, apiPath(account, app, "/channels")) |
| +func (s *apiSession) channels(ctx context.Context, a apiAcct, app string) ([]apiChannel, error) { |
| + status, body, err := s.getJSON(ctx, apiPath(a, app, "/channels")) |
| if err != nil { |
| return nil, err |
| } |
| @@ -368,9 +399,9 @@ func (s *apiSession) channels(ctx context.Context, account, app string) ([]apiCh |
| // which is exactly what apiErrorFromBody's json-first decode surfaces, since |
| // ledgerStoreError answers it with a real "{"error": "..."}" body, not a bare |
| // status. |
| -func (s *apiSession) ledgerAppend(ctx context.Context, account, app, name string, payload json.RawMessage) (ledger.Appended, error) { |
| +func (s *apiSession) ledgerAppend(ctx context.Context, a apiAcct, app, name string, payload json.RawMessage) (ledger.Appended, error) { |
| var out ledger.Appended |
| - status, body, err := s.postJSON(ctx, apiPath(account, app, "/ledgers/"+url.PathEscape(name)+"/append"), |
| + status, body, err := s.postJSON(ctx, apiPath(a, app, "/ledgers/"+url.PathEscape(name)+"/append"), |
| map[string]json.RawMessage{"payload": payload}) |
| if err != nil { |
| return out, err |
| @@ -386,9 +417,9 @@ func (s *apiSession) ledgerAppend(ctx context.Context, account, app, name string |
| // text: an unknown LEDGER under an app the caller does belong to is a |
| // different, much more common case ("nothing appended yet") than "check |
| // -account and -app", and deserves to say so. |
| -func (s *apiSession) ledgerHead(ctx context.Context, account, app, name string) (ledger.Head, error) { |
| +func (s *apiSession) ledgerHead(ctx context.Context, a apiAcct, app, name string) (ledger.Head, error) { |
| var out ledger.Head |
| - status, body, err := s.getJSON(ctx, apiPath(account, app, "/ledgers/"+url.PathEscape(name)+"/head")) |
| + status, body, err := s.getJSON(ctx, apiPath(a, app, "/ledgers/"+url.PathEscape(name)+"/head")) |
| if err != nil { |
| return out, err |
| } |
| @@ -407,8 +438,8 @@ func (s *apiSession) ledgerHead(ctx context.Context, account, app, name string) |
| // interpret (a null manifest means "never published" — a caller-side zero |
| // value would erase that distinction), and the CLI print path decodes it |
| // itself right where it decides what to show a human. |
| -func (s *apiSession) ledgerList(ctx context.Context, account, app string) ([]byte, error) { |
| - status, body, err := s.getJSON(ctx, apiPath(account, app, "/ledgers")) |
| +func (s *apiSession) ledgerList(ctx context.Context, a apiAcct, app string) ([]byte, error) { |
| + status, body, err := s.getJSON(ctx, apiPath(a, app, "/ledgers")) |
| if err != nil { |
| return nil, err |
| } |
| @@ -426,8 +457,8 @@ func (s *apiSession) ledgerList(ctx context.Context, account, app string) ([]byt |
| // it the CLI could only echo the caller's own argument back at them, which |
| // is exactly how someone comes to believe they unpublished a chain they |
| // didn't. |
| -func (s *apiSession) ledgerPublish(ctx context.Context, account, app string, m ledger.Manifest) (ledger.Manifest, error) { |
| - status, body, err := s.postJSON(ctx, apiPath(account, app, "/ledgers/publish"), m) |
| +func (s *apiSession) ledgerPublish(ctx context.Context, a apiAcct, app string, m ledger.Manifest) (ledger.Manifest, error) { |
| + status, body, err := s.postJSON(ctx, apiPath(a, app, "/ledgers/publish"), m) |
| if err != nil { |
| return ledger.Manifest{}, err |
| } |
| @@ -448,9 +479,9 @@ func (s *apiSession) ledgerPublish(ctx context.Context, account, app string, m l |
| // header (apimode.go:212-230) — but with http.MethodPost and a |
| // bytes.Reader body in place of GET/nil, since the wire body here is the |
| // blob's raw bytes, never JSON. |
| -func (s *apiSession) ledgerBlob(ctx context.Context, account, app, name string, body []byte) (string, error) { |
| +func (s *apiSession) ledgerBlob(ctx context.Context, a apiAcct, app, name string, body []byte) (string, error) { |
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, |
| - s.c.base+apiPath(account, app, "/ledgers/"+url.PathEscape(name)+"/blobs"), bytes.NewReader(body)) |
| + s.c.base+apiPath(a, app, "/ledgers/"+url.PathEscape(name)+"/blobs"), bytes.NewReader(body)) |
| if err != nil { |
| return "", err |
| } |
| @@ -490,7 +521,7 @@ const shipTimeout = 15 * time.Minute |
| // ({"version","kind"}) first, then one file part per entry in files (partName |
| // -> local path), in map-key-sorted order — sorted so the wire bytes are |
| // deterministic, since Go's map iteration order is not. |
| -func (s *apiSession) ship(ctx context.Context, account, app, version, kind string, files map[string]string) (apiShipResult, error) { |
| +func (s *apiSession) ship(ctx context.Context, a apiAcct, app, version, kind string, files map[string]string) (apiShipResult, error) { |
| ctx, cancel := context.WithTimeout(ctx, shipTimeout) |
| defer cancel() |
| @@ -500,7 +531,7 @@ func (s *apiSession) ship(ctx context.Context, account, app, version, kind strin |
| } |
| sort.Strings(names) |
| - status, body, err := s.c.doMultipart(ctx, apiPath(account, app, "/releases"), func(mw *multipart.Writer) error { |
| + status, body, err := s.c.doMultipart(ctx, apiPath(a, app, "/releases"), func(mw *multipart.Writer) error { |
| metaJSON, err := json.Marshal(struct { |
| Version string `json:"version"` |
| Kind string `json:"kind"` |
diff --git a/apimode_test.go b/apimode_test.go| index 8a05cecfe..6365f8fe8 100644 |
| --- a/apimode_test.go |
| +++ b/apimode_test.go |
| @@ -259,7 +259,7 @@ func TestAPISessionShipMultipart(t *testing.T) { |
| } |
| s := newTestSession(ts.URL) |
| - res, err := s.ship(context.Background(), "acct", "app", "1.2.3", "binary", map[string]string{ |
| + res, err := s.ship(context.Background(), apiAcct{Seg: "acct"}, "app", "1.2.3", "binary", map[string]string{ |
| "zeta-amd64": fileB, |
| "alpha-amd64": fileA, |
| }) |
| @@ -322,7 +322,7 @@ func TestAPISessionShipNoOp(t *testing.T) { |
| t.Fatal(err) |
| } |
| s := newTestSession(ts.URL) |
| - res, err := s.ship(context.Background(), "acct", "app", "1.2.3", "binary", map[string]string{"alpha-amd64": file}) |
| + res, err := s.ship(context.Background(), apiAcct{Seg: "acct"}, "app", "1.2.3", "binary", map[string]string{"alpha-amd64": file}) |
| if err != nil { |
| t.Fatalf("ship: %v", err) |
| } |
| @@ -346,7 +346,7 @@ func TestAPISessionShipErrorSurfacesServerMessage(t *testing.T) { |
| t.Fatal(err) |
| } |
| s := newTestSession(ts.URL) |
| - _, err := s.ship(context.Background(), "acct", "app", "1.2.3", "binary", map[string]string{"alpha-amd64": file}) |
| + _, err := s.ship(context.Background(), apiAcct{Seg: "acct"}, "app", "1.2.3", "binary", map[string]string{"alpha-amd64": file}) |
| if err == nil || !strings.Contains(err.Error(), "already shipped with different content") { |
| t.Fatalf("ship error = %v, want the server's conflict message", err) |
| } |
| @@ -388,7 +388,7 @@ func TestAPISessionShipSurvivesSlowServer(t *testing.T) { |
| ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) |
| defer cancel() |
| - _, err := s.ship(ctx, "acct", "app", "1.2.3", "binary", map[string]string{"alpha-amd64": file}) |
| + _, err := s.ship(ctx, apiAcct{Seg: "acct"}, "app", "1.2.3", "binary", map[string]string{"alpha-amd64": file}) |
| if err == nil { |
| t.Fatal("ship with a tiny absolute client Timeout against a slow-to-answer server must fail") |
| } |
| @@ -412,7 +412,7 @@ func TestAPISessionShipSurvivesSlowServer(t *testing.T) { |
| // shipHTTPClientTimeout is back at its zero production default here |
| // (the sibling subtest above restored it) — only the context bounds |
| // this call. |
| - res, err := s.ship(context.Background(), "acct", "app", "1.2.3", "binary", map[string]string{"alpha-amd64": file}) |
| + res, err := s.ship(context.Background(), apiAcct{Seg: "acct"}, "app", "1.2.3", "binary", map[string]string{"alpha-amd64": file}) |
| if err != nil { |
| t.Fatalf("ship against a slow-but-healthy server = %v, want success", err) |
| } |
| @@ -437,7 +437,7 @@ func TestAPISessionStableRefusal(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - _, err := s.promote(context.Background(), "acct", "app", "1.2.3", "canary") |
| + _, err := s.promote(context.Background(), apiAcct{Seg: "acct"}, "app", "1.2.3", "canary") |
| if err == nil { |
| t.Fatal("promote against a stable refusal must error") |
| } |
| @@ -460,7 +460,7 @@ func TestAPISessionRollbackStableRefusal(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - err := s.rollback(context.Background(), "acct", "app", "stable") |
| + err := s.rollback(context.Background(), apiAcct{Seg: "acct"}, "app", "stable") |
| if err == nil || !strings.Contains(err.Error(), "http://x/a/b") { |
| t.Fatalf("rollback error = %v, want it to contain the console_url", err) |
| } |
| @@ -484,7 +484,7 @@ func TestAPISessionPromoteSuccess(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - res, err := s.promote(context.Background(), "acct", "app", "1.2.3", "canary/a") |
| + res, err := s.promote(context.Background(), apiAcct{Seg: "acct"}, "app", "1.2.3", "canary/a") |
| if err != nil { |
| t.Fatalf("promote: %v", err) |
| } |
| @@ -505,7 +505,7 @@ func TestAPISessionPromoteNoOp(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - res, err := s.promote(context.Background(), "acct", "app", "1.2.3", "canary/a") |
| + res, err := s.promote(context.Background(), apiAcct{Seg: "acct"}, "app", "1.2.3", "canary/a") |
| if err != nil { |
| t.Fatalf("promote: %v", err) |
| } |
| @@ -529,7 +529,7 @@ func TestAPISessionPromoteUnshippedVersionSurfacesServerText(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - _, err := s.promote(context.Background(), "acct", "app", "9.9.9", "canary/a") |
| + _, err := s.promote(context.Background(), apiAcct{Seg: "acct"}, "app", "9.9.9", "canary/a") |
| if err == nil || !strings.Contains(err.Error(), "not a shipped release") { |
| t.Fatalf("promote 404 with a JSON diagnosis = %v, want the server's own text", err) |
| } |
| @@ -549,7 +549,7 @@ func TestAPISessionPromoteAccountGateNotFound(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - _, err := s.promote(context.Background(), "nope", "app", "1.2.3", "canary/a") |
| + _, err := s.promote(context.Background(), apiAcct{Seg: "nope"}, "app", "1.2.3", "canary/a") |
| if err == nil || !strings.Contains(err.Error(), "not found") || !strings.Contains(err.Error(), "-account") { |
| t.Fatalf("promote against the account/app gate 404 = %v, want the membership message", err) |
| } |
| @@ -567,7 +567,7 @@ func TestAPISessionChannelsNotFound(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - _, err := s.channels(context.Background(), "nope", "app") |
| + _, err := s.channels(context.Background(), apiAcct{Seg: "nope"}, "app") |
| if err == nil { |
| t.Fatal("channels against a 404 must error") |
| } |
| @@ -588,7 +588,7 @@ func TestAPISessionChannels(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - chans, err := s.channels(context.Background(), "acct", "app") |
| + chans, err := s.channels(context.Background(), apiAcct{Seg: "acct"}, "app") |
| if err != nil { |
| t.Fatalf("channels: %v", err) |
| } |
| @@ -654,22 +654,22 @@ func requireNoRegistryTouch(t *testing.T) func() { |
| } |
| } |
| -// newShipFlagSet builds the -account/-registry flags runShipAPI's callers |
| -// register in production (accountFlags), parses account plus the given |
| -// positional file/dir args exactly as fs.Parse(os.Args) would, and returns |
| -// the FlagSet and the raw -account pointer runShipAPI expects — letting a |
| -// test drive runShipAPI directly (bypassing runShip's own flag set and, |
| -// crucially, its log.Fatal on error) so a refusal can be asserted as a |
| -// returned error instead of a process exit. |
| -func newShipFlagSet(t *testing.T, account string, args ...string) (*flag.FlagSet, *string) { |
| +// newShipFlagSet builds the -account/-account-id/-registry flags |
| +// runShipAPI's callers register in production (accountFlags), parses account |
| +// plus the given positional file/dir args exactly as fs.Parse(os.Args) |
| +// would, and returns the FlagSet and the two raw account pointers runShipAPI |
| +// expects — letting a test drive runShipAPI directly (bypassing runShip's |
| +// own flag set and, crucially, its log.Fatal on error) so a refusal can be |
| +// asserted as a returned error instead of a process exit. |
| +func newShipFlagSet(t *testing.T, account string, args ...string) (*flag.FlagSet, *string, *string) { |
| t.Helper() |
| fs := flag.NewFlagSet("ship", flag.ContinueOnError) |
| - acctRaw, _ := accountFlags(fs) |
| + acctRaw, acctIDRaw, _ := accountFlags(fs) |
| full := append([]string{"-account", account}, args...) |
| if err := fs.Parse(full); err != nil { |
| t.Fatalf("parse: %v", err) |
| } |
| - return fs, acctRaw |
| + return fs, acctRaw, acctIDRaw |
| } |
| // TestShipVerbAPIMode pins runShip's API-mode wiring end to end: with a live |
| @@ -827,6 +827,45 @@ func TestPromoteVerbAPIMode(t *testing.T) { |
| checkNoRegistry() |
| } |
| +// TestPromoteVerbAPIModeAccountID pins -account-id's transport on a POST |
| +// route end to end (the GET side is covered server-side by apicli_test.go, |
| +// which drives only GETs): the sqid must land in the path segment AND the |
| +// request must carry ?exact=sqid, the console-side switch that stops the |
| +// segment from ever being name-matched. -account is set to a colliding |
| +// value at the same time to pin the supersede rule — the sqid wins, and the |
| +// name never reaches the wire. |
| +func TestPromoteVerbAPIModeAccountID(t *testing.T) { |
| + useTempCredentials(t) |
| + clearDeploymentEnv(t) |
| + checkNoRegistry := requireNoRegistryTouch(t) |
| + |
| + var reqCount int32 |
| + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + atomic.AddInt32(&reqCount, 1) |
| + if r.URL.Path != "/api/cli/apps/bah/titogo/promote" { |
| + t.Errorf("path = %s, want the -account-id sqid as the segment", r.URL.Path) |
| + } |
| + if got := r.URL.Query().Get("exact"); got != "sqid" { |
| + t.Errorf("exact = %q, want %q — -account-id must never be name-matched", got, "sqid") |
| + } |
| + json.NewEncoder(w).Encode(map[string]any{"result": "promoted"}) |
| + })) |
| + defer ts.Close() |
| + |
| + loginTo(t, ts.URL, "vicky@keymail.dev") |
| + |
| + buf := captureStdout(t) |
| + runPromote([]string{"-app", "titogo", "-account", apiTestAccount, "-account-id", "bah", "abc1234", "canary/x"}) |
| + |
| + if want := "promoted titogo abc1234 -> canary/x"; !strings.Contains(buf.String(), want) { |
| + t.Fatalf("stdout = %q, want %q", buf.String(), want) |
| + } |
| + if got := atomic.LoadInt32(&reqCount); got != 1 { |
| + t.Fatalf("server got %d requests, want 1", got) |
| + } |
| + checkNoRegistry() |
| +} |
| + |
| // TestPromoteVerbAPIModeNoOp pins the noop-parity amendment itself: API-mode |
| // promote must print the byte-identical "already on X — nothing to do" shape |
| // direct mode prints (runPromote's res.NoOp branch), not a differently-worded |
| @@ -1042,8 +1081,8 @@ func TestShipAPIStaticNamesSiteTarGz(t *testing.T) { |
| defer ts.Close() |
| sess := newTestSession(ts.URL) |
| - fs, acctRaw := newShipFlagSet(t, apiTestAccount, siteDir) |
| - if err := runShipAPI(sess, acctRaw, fs, "titesite", "abc1234", "static", ""); err != nil { |
| + fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, siteDir) |
| + if err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titesite", "abc1234", "static", ""); err != nil { |
| t.Fatalf("runShipAPI: %v", err) |
| } |
| @@ -1089,8 +1128,8 @@ func TestShipAPIStaticCleansUpTempFileOnServerError(t *testing.T) { |
| } |
| sess := newTestSession(ts.URL) |
| - fs, acctRaw := newShipFlagSet(t, apiTestAccount, siteDir) |
| - if err := runShipAPI(sess, acctRaw, fs, "titesite", "abc1234", "static", ""); err == nil { |
| + fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, siteDir) |
| + if err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titesite", "abc1234", "static", ""); err == nil { |
| t.Fatal("runShipAPI against a failing server must error") |
| } |
| @@ -1155,8 +1194,8 @@ func TestShipAPIMultiFileGoodBasenames(t *testing.T) { |
| defer ts.Close() |
| sess := newTestSession(ts.URL) |
| - fs, acctRaw := newShipFlagSet(t, apiTestAccount, fileArm, fileAmd) |
| - if err := runShipAPI(sess, acctRaw, fs, "titogo", "abc1234", "binary", "linux-arm64"); err != nil { |
| + fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, fileArm, fileAmd) |
| + if err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titogo", "abc1234", "binary", "linux-arm64"); err != nil { |
| t.Fatalf("runShipAPI: %v", err) |
| } |
| @@ -1197,8 +1236,8 @@ func TestShipAPIMultiFileBadBasenameRefused(t *testing.T) { |
| defer ts.Close() |
| sess := newTestSession(ts.URL) |
| - fs, acctRaw := newShipFlagSet(t, apiTestAccount, fileGood, fileBad) |
| - err := runShipAPI(sess, acctRaw, fs, "titogo", "abc1234", "binary", "linux-arm64") |
| + fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, fileGood, fileBad) |
| + err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titogo", "abc1234", "binary", "linux-arm64") |
| if err == nil { |
| t.Fatal("runShipAPI with a bad basename must error") |
| } |
| @@ -1232,8 +1271,8 @@ func TestShipAPIMissingArtifactRefusedBeforeDialing(t *testing.T) { |
| defer ts.Close() |
| sess := newTestSession(ts.URL) |
| - fs, acctRaw := newShipFlagSet(t, apiTestAccount, missing) |
| - err := runShipAPI(sess, acctRaw, fs, "titogo", "abc1234", "binary", "linux-arm64") |
| + fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, missing) |
| + err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titogo", "abc1234", "binary", "linux-arm64") |
| if err == nil { |
| t.Fatal("runShipAPI with a missing artifact path must error") |
| } |
| @@ -1260,7 +1299,7 @@ func TestAPISessionLedgerAppend(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - a, err := s.ledgerAppend(context.Background(), "acct", "app", "carbon", json.RawMessage(`{"n":1}`)) |
| + a, err := s.ledgerAppend(context.Background(), apiAcct{Seg: "acct"}, "app", "carbon", json.RawMessage(`{"n":1}`)) |
| if err != nil { |
| t.Fatalf("ledgerAppend: %v", err) |
| } |
| @@ -1289,7 +1328,7 @@ func TestAPISessionLedgerAppendBusy(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - _, err := s.ledgerAppend(context.Background(), "acct", "app", "carbon", json.RawMessage(`{"n":1}`)) |
| + _, err := s.ledgerAppend(context.Background(), apiAcct{Seg: "acct"}, "app", "carbon", json.RawMessage(`{"n":1}`)) |
| if err == nil || !strings.Contains(err.Error(), "busy") { |
| t.Fatalf("ledgerAppend 503 error = %v, want it to surface the busy message", err) |
| } |
| @@ -1306,7 +1345,7 @@ func TestAPISessionLedgerHeadNotFound(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - _, err := s.ledgerHead(context.Background(), "acct", "app", "carbon") |
| + _, err := s.ledgerHead(context.Background(), apiAcct{Seg: "acct"}, "app", "carbon") |
| if err == nil { |
| t.Fatal("ledgerHead against a 404 must error") |
| } |
| @@ -1325,7 +1364,7 @@ func TestAPISessionLedgerHead(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - h, err := s.ledgerHead(context.Background(), "acct", "app", "carbon") |
| + h, err := s.ledgerHead(context.Background(), apiAcct{Seg: "acct"}, "app", "carbon") |
| if err != nil { |
| t.Fatalf("ledgerHead: %v", err) |
| } |
| @@ -1345,7 +1384,7 @@ func TestAPISessionLedgerList(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - body, err := s.ledgerList(context.Background(), "acct", "app") |
| + body, err := s.ledgerList(context.Background(), apiAcct{Seg: "acct"}, "app") |
| if err != nil { |
| t.Fatalf("ledgerList: %v", err) |
| } |
| @@ -1380,7 +1419,7 @@ func TestAPISessionLedgerPublish(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - m, err := s.ledgerPublish(context.Background(), "acct", "app", |
| + m, err := s.ledgerPublish(context.Background(), apiAcct{Seg: "acct"}, "app", |
| ledger.Manifest{Public: true, Ledgers: []string{"carbon"}, Title: "T"}) |
| if err != nil { |
| t.Fatalf("ledgerPublish: %v", err) |
| @@ -1413,7 +1452,7 @@ func TestAPISessionLedgerBlob(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - sha, err := s.ledgerBlob(context.Background(), "acct", "app", "carbon", []byte("corpus")) |
| + sha, err := s.ledgerBlob(context.Background(), apiAcct{Seg: "acct"}, "app", "carbon", []byte("corpus")) |
| if err != nil { |
| t.Fatalf("ledgerBlob: %v", err) |
| } |
| @@ -1442,7 +1481,7 @@ func TestAPISessionLedgerBlobError(t *testing.T) { |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - _, err := s.ledgerBlob(context.Background(), "acct", "app", "carbon", []byte("x")) |
| + _, err := s.ledgerBlob(context.Background(), apiAcct{Seg: "acct"}, "app", "carbon", []byte("x")) |
| if err == nil || !strings.Contains(err.Error(), "4 MiB cap") { |
| t.Fatalf("ledgerBlob error = %v, want the server's 413 message surfaced", err) |
| } |
| @@ -1457,10 +1496,16 @@ func TestAPISessionLedgerBlobError(t *testing.T) { |
| // half of that proof). |
| func TestLedgerVerbsOfferNoRegistryFlag(t *testing.T) { |
| fs := flag.NewFlagSet("ledger", flag.ContinueOnError) |
| - ledgerAccountFlag(fs) |
| + ledgerAccountFlags(fs) |
| if fs.Lookup("account") == nil { |
| t.Fatal("ledger verbs must still take -account") |
| } |
| + // -account-id is the way out of an ambiguity the console reports (409), |
| + // so a ledger verb that offered -account without it would name a fix-up |
| + // flag its own caller cannot type. |
| + if fs.Lookup("account-id") == nil { |
| + t.Fatal("ledger verbs must take -account-id — it is what the console's ambiguity 409 tells the caller to use") |
| + } |
| if f := fs.Lookup("registry"); f != nil { |
| t.Fatalf("ledger verbs register -registry (%q) — direct-bucket mode is refused, not configured", f.Usage) |
| } |
diff --git a/appconfig_cli.go b/appconfig_cli.go| index 749f32fc7..599ab0b8a 100644 |
| --- a/appconfig_cli.go |
| +++ b/appconfig_cli.go |
| @@ -177,7 +177,7 @@ func runEnvSet(args []string) { |
| fs := flag.NewFlagSet("env set", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| environment := fs.String("environment", "", "named environment bundle to set in (omit for the default bundle)") |
| - _, acctOf := accountFlags(fs) |
| + _, _, acctOf := accountFlags(fs) |
| fs.Parse(args) |
| if *app == "" { |
| log.Fatal("usage: carlos env set -app <a> [-environment <name>] KEY=value [KEY2=value2 ...]") |
| @@ -197,7 +197,7 @@ func runEnvUnset(args []string) { |
| fs := flag.NewFlagSet("env unset", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| environment := fs.String("environment", "", "named environment bundle to remove from (omit for the default bundle)") |
| - _, acctOf := accountFlags(fs) |
| + _, _, acctOf := accountFlags(fs) |
| fs.Parse(args) |
| if *app == "" || fs.NArg() == 0 { |
| log.Fatal("usage: carlos env unset -app <a> [-environment <name>] KEY [KEY2 ...]") |
| @@ -214,7 +214,7 @@ func runEnvList(args []string) { |
| fs := flag.NewFlagSet("env list", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| environment := fs.String("environment", "", "named environment bundle to list (omit for the default bundle)") |
| - _, acctOf := accountFlags(fs) |
| + _, _, acctOf := accountFlags(fs) |
| fs.Parse(args) |
| if *app == "" { |
| log.Fatal("usage: carlos env list -app <a> [-environment <name>]") |
| @@ -264,7 +264,7 @@ func runEnvExecDelivery(args []string) { |
| // below. There is no environment-scoped variant to route it to: |
| // appconfig.SetExecDelivery has no environment parameter at all. |
| environment := fs.String("environment", "", "refused: exec_delivery is app-level only (default bundle), never a named environment") |
| - _, acctOf := accountFlags(fs) |
| + _, _, acctOf := accountFlags(fs) |
| fs.Parse(args) |
| if *app == "" || fs.NArg() != 1 { |
| log.Fatal("usage: carlos env exec-delivery -app <a> file|off") |
| @@ -299,7 +299,7 @@ func runEnvExecDelivery(args []string) { |
| func runEnvEnvironments(args []string) { |
| fs := flag.NewFlagSet("env environments", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - _, acctOf := accountFlags(fs) |
| + _, _, acctOf := accountFlags(fs) |
| fs.Parse(args) |
| if *app == "" { |
| log.Fatal("usage: carlos env environments -app <a>") |
| @@ -362,7 +362,7 @@ func runSecretsSet(args []string) { |
| fs := flag.NewFlagSet("secrets set", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| environment := fs.String("environment", "", "named environment bundle to set in (omit for the default bundle)") |
| - _, acctOf := accountFlags(fs) |
| + _, _, acctOf := accountFlags(fs) |
| fs.Parse(args) |
| if *app == "" { |
| log.Fatal("usage: carlos secrets set -app <a> [-environment <name>] KEY=value [KEY2=value2 ...]") |
| @@ -399,7 +399,7 @@ func runSecretsUnset(args []string) { |
| fs := flag.NewFlagSet("secrets unset", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| environment := fs.String("environment", "", "named environment bundle to remove from (omit for the default bundle)") |
| - _, acctOf := accountFlags(fs) |
| + _, _, acctOf := accountFlags(fs) |
| fs.Parse(args) |
| if *app == "" || fs.NArg() == 0 { |
| log.Fatal("usage: carlos secrets unset -app <a> [-environment <name>] KEY [KEY2 ...]") |
| @@ -416,7 +416,7 @@ func runSecretsList(args []string) { |
| fs := flag.NewFlagSet("secrets list", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| environment := fs.String("environment", "", "named environment bundle to list (omit for the default bundle)") |
| - _, acctOf := accountFlags(fs) |
| + _, _, acctOf := accountFlags(fs) |
| fs.Parse(args) |
| if *app == "" { |
| log.Fatal("usage: carlos secrets list -app <a> [-environment <name>]") |
diff --git a/auth_e2e_test.go b/auth_e2e_test.go| index 967e34ef3..c1d0bb82e 100644 |
| --- a/auth_e2e_test.go |
| +++ b/auth_e2e_test.go |
| @@ -117,7 +117,7 @@ func TestAccountsCreateNeedsAnOwner(t *testing.T) { |
| "CARLOS_CREDENTIALS="+filepath.Join(tmp, "credentials"), |
| "CARLOS_DATA_DIR="+tmp, |
| ) |
| - cmd := exec.Command(bin, "accounts", "create", "-name", "Orphan") |
| + cmd := exec.Command(bin, "accounts", "create", "-name", "orphan") |
| cmd.Env = env |
| out, err := cmd.CombinedOutput() |
| if err == nil { |
| @@ -134,13 +134,42 @@ func TestAccountsCreateNeedsAnOwner(t *testing.T) { |
| } |
| } |
| +// A non-slug name is refused before anything is minted — this is the third |
| +// (and only bucket-writing) door, so a shape mistake here must not reach |
| +// domains.MintAccount at all. -owner is supplied so the refusal is |
| +// unambiguously about the NAME, not the ownerless-account path above. |
| +func TestAccountsCreateRefusesANonSlugName(t *testing.T) { |
| + bin := buildCarlos(t) |
| + tmp := t.TempDir() |
| + storeDir := filepath.Join(tmp, "store") |
| + env := append(os.Environ(), |
| + "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| + "CARLOS_CREDENTIALS="+filepath.Join(tmp, "credentials"), |
| + "CARLOS_DATA_DIR="+tmp, |
| + ) |
| + cmd := exec.Command(bin, "accounts", "create", "-name", "Paul's Stuff!", "-owner", "paul@keymail.dev") |
| + cmd.Env = env |
| + out, err := cmd.CombinedOutput() |
| + if err == nil { |
| + t.Fatalf("non-slug accounts create exited 0:\n%s", out) |
| + } |
| + if !strings.Contains(string(out), "not a usable account name") { |
| + t.Fatalf("refusal does not name the shape rule:\n%s", out) |
| + } |
| + // Nothing was minted: a rejected name must not burn an id or leave a |
| + // half-made account in the bucket. |
| + if keys, err := release.NewDirStore(storeDir).List(t.Context(), "accounts/"); err != nil || len(keys) != 0 { |
| + t.Fatalf("a refused create left %v behind (%v)", keys, err) |
| + } |
| +} |
| + |
| // The headless path: -owner names the owner outright, no login, no browser, |
| // and the address lands in the manifest every box materializes identity from. |
| func TestAccountsCreateWithOwnerFlag(t *testing.T) { |
| bin := buildCarlos(t) |
| tmp := t.TempDir() |
| storeDir := filepath.Join(tmp, "store") |
| - cmd := exec.Command(bin, "accounts", "create", "-name", "Kass", "-owner", "paul@keymail.dev") |
| + cmd := exec.Command(bin, "accounts", "create", "-name", "kass", "-owner", "paul@keymail.dev") |
| cmd.Env = append(os.Environ(), |
| "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| "CARLOS_CREDENTIALS="+filepath.Join(tmp, "credentials"), |
| @@ -166,8 +195,45 @@ func TestAccountsCreateWithOwnerFlag(t *testing.T) { |
| if err := json.Unmarshal(body, &m); err != nil { |
| t.Fatal(err) |
| } |
| - if m.Owner != "paul@keymail.dev" || m.Name != "Kass" { |
| - t.Fatalf("manifest = %+v, want Kass owned by paul", m) |
| + if m.Owner != "paul@keymail.dev" || m.Name != "kass" { |
| + t.Fatalf("manifest = %+v, want kass owned by paul", m) |
| + } |
| +} |
| + |
| +// A padded name is trimmed BEFORE it is validated or minted — the gate and |
| +// the write must agree on what "the name" is. Before this fix, -name " |
| +// kass " passed ValidAccountName on the trimmed copy but handed the |
| +// untrimmed original to MintAccount, landing padding in the manifest |
| +// through the one edge that writes straight to the bucket. |
| +func TestAccountsCreateTrimsTheNameBeforeMinting(t *testing.T) { |
| + bin := buildCarlos(t) |
| + tmp := t.TempDir() |
| + storeDir := filepath.Join(tmp, "store") |
| + cmd := exec.Command(bin, "accounts", "create", "-name", " kass ", "-owner", "paul@keymail.dev") |
| + cmd.Env = append(os.Environ(), |
| + "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| + "CARLOS_CREDENTIALS="+filepath.Join(tmp, "credentials"), |
| + "CARLOS_DATA_DIR="+tmp, |
| + ) |
| + out, err := cmd.CombinedOutput() |
| + if err != nil { |
| + t.Fatalf("carlos accounts create -name ' kass ': %v\n%s", err, out) |
| + } |
| + store := release.NewDirStore(storeDir) |
| + keys, err := store.List(t.Context(), "accounts/") |
| + if err != nil || len(keys) != 1 { |
| + t.Fatalf("account manifests = %v (%v), want exactly one", keys, err) |
| + } |
| + body, _, err := store.Get(t.Context(), keys[0]) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var m domains.AccountManifest |
| + if err := json.Unmarshal(body, &m); err != nil { |
| + t.Fatal(err) |
| + } |
| + if m.Name != "kass" { |
| + t.Fatalf("manifest name = %q, want the trimmed %q", m.Name, "kass") |
| } |
| } |
| @@ -189,7 +255,7 @@ func TestAccountsCreateUsesTheLoggedInIdentity(t *testing.T) { |
| if err := os.WriteFile(creds, raw, 0o600); err != nil { |
| t.Fatal(err) |
| } |
| - cmd := exec.Command(bin, "accounts", "create", "-name", "Kass") |
| + cmd := exec.Command(bin, "accounts", "create", "-name", "kass") |
| cmd.Env = append(os.Environ(), |
| "CARLOS_CREDENTIALS="+creds, |
| "CARLOS_DATA_DIR="+tmp, |
| @@ -455,7 +521,7 @@ func TestE2ECLIThroughConsole(t *testing.T) { |
| // apiSession level instead: the exact call runPromote's API-mode branch |
| // makes, against the same real server, over the same credential. |
| sess := &apiSession{c: newConsoleClient(ts.URL, token), console: ts.URL, address: addr} |
| - if _, err := sess.promote(context.Background(), "ops", "titogo", "abc1234", "stable"); err == nil { |
| + if _, err := sess.promote(context.Background(), apiAcct{Seg: "ops"}, "titogo", "abc1234", "stable"); err == nil { |
| t.Fatal("promote to stable must refuse") |
| } else { |
| if !strings.Contains(err.Error(), ts.URL) { |
diff --git a/cli.go b/cli.go| index 3594d88b7..a95989041 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -67,6 +67,7 @@ func runAdd(args []string) { |
| reg := fs.String("registry", defaultRegistry(), "registry database") |
| app := fs.String("app", "", "app whose releases this route runs") |
| acct := fs.String("account", "ops", "account (sqid or name) the app belongs to") |
| + acctID := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account") |
| slug := fs.String("slug", "", "root-domain short name (optional)") |
| kind := fs.String("kind", "instance", "instance | service | static") |
| socket := fs.String("socket", "", "unix socket upstream") |
| @@ -127,7 +128,7 @@ func runAdd(args []string) { |
| // unqualified keys, so an unmigrated app added here keeps working. |
| if err := r.Upsert(registry.Route{ |
| Host: host, Slug: *slug, Kind: *kind, Socket: *socket, Addr: *addr, |
| - App: *app, Account: resolveAccount(*reg, *acct), |
| + App: *app, Account: resolveAccount(*reg, *acct, *acctID), |
| Channel: *channel, Hibernate: *hibernate, StatusNamed: *statusNamed, Backing: *backing, |
| DBPath: *dbPath, Status: "ready", |
| }); err != nil { |
| @@ -265,6 +266,7 @@ func runRoute(args []string) { |
| host := fs.String("host", "", "the one route to repoint") |
| app := fs.String("app", "", "repoint every non-platform route of this app") |
| acctFilter := fs.String("account", "ops", `account (sqid or name) to filter -app routes by; "all" for every account`) |
| + acctIDFilter := fs.String("account-id", "", "account sqid to filter -app routes by, matched as a sqid ONLY — supersedes -account") |
| channel := fs.String("channel", "", "release channel to subscribe the route(s) to") |
| environment := fs.String("environment", "", |
| `named config environment to bind the route(s) to (must already exist in the bucket); "-" clears to default`) |
| @@ -275,7 +277,8 @@ func runRoute(args []string) { |
| fs.Parse(args) |
| const usage = "usage: carlos route -host <host> [-channel <channel>] [-environment <name>] [-registry <db>] [-allow-version-change] [-dry-run]\n" + |
| - " or: carlos route -app <app> [-channel <channel>] [-environment <name>] [-account <sqid|name|all>] [-registry <db>] [-allow-version-change] [-dry-run]" |
| + " or: carlos route -app <app> [-channel <channel>] [-environment <name>] [-account <sqid|name|all>] [-account-id <sqid>] [-registry <db>] [-allow-version-change] [-dry-run]\n" + |
| + " (-account-id names ONE account by sqid and supersedes -account, \"all\" included)" |
| if *host == "" && *app == "" { |
| log.Fatal(usage) |
| } |
| @@ -340,9 +343,15 @@ func runRoute(args []string) { |
| if err != nil { |
| log.Fatal(err) |
| } |
| + // -account-id supersedes -account here as everywhere, including |
| + // over the "all" escape hatch: someone who names one account by |
| + // its sqid means that account, not every account, whatever -account |
| + // happens to still hold from a script's defaults. |
| var wantAcct string |
| - if !strings.EqualFold(*acctFilter, "all") { |
| - wantAcct = resolveAccount(*regPath, *acctFilter) |
| + if strings.TrimSpace(*acctIDFilter) != "" { |
| + wantAcct = resolveAccount(*regPath, "", *acctIDFilter) |
| + } else if !strings.EqualFold(*acctFilter, "all") { |
| + wantAcct = resolveAccount(*regPath, *acctFilter, "") |
| } |
| for _, rt := range routes { |
| if rt.App != *app { |
| @@ -719,13 +728,17 @@ func routeVersionCheck(ctx context.Context, store release.Store, pub ed25519.Pub |
| current.Version, target.Version, fromChannel, current.Version, toChannel, target.Version), false, true |
| } |
| -// accountFlags registers the -account (and its -registry seam) every app verb |
| -// carries. -account takes a sqid OR a name, resolved against the registry's |
| -// materialized accounts; it defaults to the built-in ops account, so a |
| -// single-tenant deployment never types it. Reads fall back to the legacy |
| -// unqualified keys either way, so an unmigrated app keeps working. |
| +// accountFlags registers the -account/-account-id pair (and the -registry |
| +// seam) every app verb carries. -account takes a sqid OR a name, resolved |
| +// symmetrically against the registry's materialized accounts — neither space |
| +// wins, so a name that collides with some other account's sqid is refused, |
| +// not silently redirected. -account-id is the way out of that refusal, and |
| +// the reference a script should carry: a sqid, matched as a sqid only, never |
| +// looked up as a name. -account defaults to the built-in ops account, so a |
| +// single-tenant deployment never types either flag. Reads fall back to the |
| +// legacy unqualified keys either way, so an unmigrated app keeps working. |
| // |
| -// Returns the raw flag value alongside a resolver thunk, rather than the |
| +// Returns the raw flag values alongside a resolver thunk, rather than the |
| // resolved value alone, because resolution has to happen AFTER fs.Parse — |
| // and it opens the registry, which a verb that never uses the account should |
| // not pay for. The raw pointer exists for API mode (cli.go's four |
| @@ -735,10 +748,11 @@ func routeVersionCheck(ctx context.Context, store release.Store, pub ed25519.Pub |
| // reading it straight off this return value keeps that path from being |
| // string-coupled to this function's internal flag name the way |
| // fs.Lookup("account") would be. |
| -func accountFlags(fs *flag.FlagSet) (raw *string, resolve func() string) { |
| +func accountFlags(fs *flag.FlagSet) (raw, rawID *string, resolve func() string) { |
| acct := fs.String("account", "ops", "account (sqid or name) the app belongs to") |
| + acctID := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account; this is the flag an ambiguity error tells you to use") |
| regPath := fs.String("registry", defaultRegistry(), "registry database (resolves -account)") |
| - return acct, func() string { return resolveAccount(*regPath, *acct) } |
| + return acct, acctID, func() string { return resolveAccount(*regPath, *acct, *acctID) } |
| } |
| func openStore() release.Store { |
| @@ -813,14 +827,14 @@ func publicKey() ed25519.PublicKey { |
| func runShip(args []string) { |
| fs := flag.NewFlagSet("ship", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw, acctOf := accountFlags(fs) |
| + acctRaw, acctIDRaw, acctOf := accountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to ship through (API mode only; default: the only logged-in one)") |
| kind := fs.String("kind", "binary", "binary | static") |
| ver := fs.String("version", "", "version (git short sha)") |
| target := fs.String("target", "linux-arm64", "GOOS-GOARCH the binary was built for (binary kind, single file)") |
| fs.Parse(args) |
| if *app == "" || *ver == "" || fs.NArg() == 0 { |
| - log.Fatal("usage: carlos ship -app <a> -version <sha> [-kind binary|static] [-console <origin>] <files-or-site-dir>") |
| + log.Fatal("usage: carlos ship -app <a> -version <sha> [-kind binary|static] [-account <sqid-or-name>] [-account-id <sqid>] [-console <origin>] <files-or-site-dir>") |
| } |
| sess, err := resolveMode(*consoleFlag) |
| @@ -828,7 +842,13 @@ func runShip(args []string) { |
| log.Fatal(err) |
| } |
| if sess != nil { |
| - if err := runShipAPI(sess, acctRaw, fs, *app, *ver, *kind, *target); err != nil { |
| + // apiAccount is resolved HERE, not inside runShipAPI: it fatals on a |
| + // name in -account-id, and log.Fatal skips every deferred function — |
| + // which is the one thing runShipAPI exists to avoid (its static |
| + // branch defers the cleanup of a scratch tarball up to 256MiB). |
| + // Evaluated as an argument, the refusal lands before runShipAPI's |
| + // body has registered a defer at all. |
| + if err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, *app, *ver, *kind, *target); err != nil { |
| log.Fatal(err) |
| } |
| return |
| @@ -889,9 +909,12 @@ func runShip(args []string) { |
| // than after paying for the whole upload and hitting the server's own 400. |
| var shipTargetRe = regexp.MustCompile(`^[a-z0-9]+-[a-z0-9]+$`) |
| -// runShipAPI is runShip's API-mode body: the raw -account flag value goes |
| -// straight to the console (never acctOf/resolveAccount — there is no local |
| -// registry to resolve against on a developer laptop). Unlike the direct-mode |
| +// runShipAPI is runShip's API-mode body: acct is the caller's -account / |
| +// -account-id as apiAccount read them, and goes straight to the console |
| +// (never acctOf/resolveAccount — there is no local registry to resolve |
| +// against on a developer laptop). It arrives already resolved because |
| +// apiAccount can fatal, and nothing that fatals may run inside this |
| +// function — see the deferred-cleanup note below. Unlike the direct-mode |
| // body above, artifacts are streamed from disk by path (apiSession.ship |
| // reads them lazily) rather than buffered into memory here first — for a |
| // static release, whose bytes only exist in memory (edge.PackDir's tarball), |
| @@ -903,8 +926,7 @@ var shipTargetRe = regexp.MustCompile(`^[a-z0-9]+-[a-z0-9]+$`) |
| // function — including the static kind's scratch temp file cleanup below, |
| // up to 256MiB per invocation. Returning lets that defer actually run on |
| // every exit path, fatal or not. |
| -func runShipAPI(sess *apiSession, acctRaw *string, fs *flag.FlagSet, app, ver, kind, target string) error { |
| - acct := *acctRaw |
| +func runShipAPI(sess *apiSession, acct apiAcct, fs *flag.FlagSet, app, ver, kind, target string) error { |
| files := map[string]string{} |
| if kind == "static" { |
| if fs.NArg() != 1 { |
| @@ -981,11 +1003,11 @@ func runPromote(args []string) { |
| fs := flag.NewFlagSet("promote", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| hotfix := fs.Bool("hotfix", false, "bypass the channel ladder (recorded as a hotfix)") |
| - acctRaw, acctOf := accountFlags(fs) |
| + acctRaw, acctIDRaw, acctOf := accountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to promote through (API mode only; default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() != 2 { |
| - log.Fatal("usage: carlos promote -app <a> [-hotfix] [-console <origin>] <version> <channel>") |
| + log.Fatal("usage: carlos promote -app <a> [-hotfix] [-account <sqid-or-name>] [-account-id <sqid>] [-console <origin>] <version> <channel>") |
| } |
| version, channel := fs.Arg(0), fs.Arg(1) |
| @@ -1003,7 +1025,7 @@ func runPromote(args []string) { |
| // flag silently do nothing. |
| log.Fatal("hotfix is an operator act — use the direct path (CARLOS_DEPLOYMENT_BUCKET) or the console") |
| } |
| - acct := *acctRaw |
| + acct := apiAccount(acctRaw, acctIDRaw) |
| fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address) |
| res, err := sess.promote(context.Background(), acct, *app, version, channel) |
| if err != nil { |
| @@ -1049,11 +1071,11 @@ func promoteActor() string { |
| func runChannels(args []string) { |
| fs := flag.NewFlagSet("channels", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw, acctOf := accountFlags(fs) |
| + acctRaw, acctIDRaw, acctOf := accountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to read through (API mode only; default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" { |
| - log.Fatal("usage: carlos channels -app <a> [-account <sqid-or-name>] [-console <origin>]") |
| + log.Fatal("usage: carlos channels -app <a> [-account <sqid-or-name>] [-account-id <sqid>] [-console <origin>]") |
| } |
| sess, err := resolveMode(*consoleFlag) |
| @@ -1061,7 +1083,7 @@ func runChannels(args []string) { |
| log.Fatal(err) |
| } |
| if sess != nil { |
| - acct := *acctRaw |
| + acct := apiAccount(acctRaw, acctIDRaw) |
| fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address) |
| chans, err := sess.channels(context.Background(), acct, *app) |
| if err != nil { |
| @@ -1105,11 +1127,11 @@ func runChannels(args []string) { |
| func runRollback(args []string) { |
| fs := flag.NewFlagSet("rollback", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw, acctOf := accountFlags(fs) |
| + acctRaw, acctIDRaw, acctOf := accountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to roll back through (API mode only; default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() != 1 { |
| - log.Fatal("usage: carlos rollback -app <a> [-console <origin>] <channel>") |
| + log.Fatal("usage: carlos rollback -app <a> [-account <sqid-or-name>] [-account-id <sqid>] [-console <origin>] <channel>") |
| } |
| sess, err := resolveMode(*consoleFlag) |
| @@ -1117,7 +1139,7 @@ func runRollback(args []string) { |
| log.Fatal(err) |
| } |
| if sess != nil { |
| - acct := *acctRaw |
| + acct := apiAccount(acctRaw, acctIDRaw) |
| fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address) |
| if err := sess.rollback(context.Background(), acct, *app, fs.Arg(0)); err != nil { |
| log.Fatal(err) |
| @@ -1141,17 +1163,17 @@ func runRollback(args []string) { |
| const ledgerUsage = `usage: carlos ledger <command> |
| commands: |
| - append -app <a> [-account <acct>] <ledger> <entry.json|-> |
| + append -app <a> [-account <acct>] [-account-id <sqid>] <ledger> <entry.json|-> |
| append one JSON entry to the chain — |
| reads the file, or stdin with "-"; |
| validated as JSON before it is sent |
| - head -app <a> [-account <acct>] <ledger> |
| + head -app <a> [-account <acct>] [-account-id <sqid>] <ledger> |
| print the chain's HEAD.json verbatim |
| - list -app <a> [-account <acct>] |
| + list -app <a> [-account <acct>] [-account-id <sqid>] |
| list this app's ledger names and its |
| publish manifest (what's public, if |
| anything) |
| - publish -app <a> [-account <acct>] [-public=true|false] [-title <t>] <ledger>... |
| + publish -app <a> [-account <acct>] [-account-id <sqid>] [-public=true|false] [-title <t>] <ledger>... |
| add these ledgers to what this app |
| serves publicly (additive: the ones |
| already public stay public, and an |
| @@ -1159,7 +1181,7 @@ commands: |
| -public=false removes just these, |
| taking the app private only when |
| nothing is left |
| - blob -app <a> [-account <acct>] <ledger> <file>... |
| + blob -app <a> [-account <acct>] [-account-id <sqid>] <ledger> <file>... |
| upload one or more content-addressed |
| blobs (create-only; re-upload of |
| identical bytes is a no-op) |
| @@ -1224,7 +1246,7 @@ func ledgerAPISession(consoleFlag string) *apiSession { |
| return sess |
| } |
| -// ledgerAccountFlag registers the ONE account flag a ledger verb has, in |
| +// ledgerAccountFlags registers the account flags a ledger verb has, in |
| // place of accountFlags. accountFlags also registers -registry, the local |
| // registry database that resolves an account name in DIRECT-bucket mode — |
| // and ledgerAPISession refuses direct mode outright (spec §2.3: no bucket |
| @@ -1232,19 +1254,24 @@ func ledgerAPISession(consoleFlag string) *apiSession { |
| // dead: nothing here ever opens a registry, and offering the flag advertises |
| // a mode that does not exist. The account is resolved server-side, by the |
| // console, against the caller's own memberships — which is why -account |
| -// takes a sqid OR a name here and needs no local database to do it. |
| -func ledgerAccountFlag(fs *flag.FlagSet) *string { |
| - return fs.String("account", "ops", "account (sqid or name) the app belongs to — resolved by the console") |
| +// takes a sqid OR a name here and needs no local database to do it — |
| +// and why -account-id needs no local database either: it rides the wire as |
| +// ?exact=sqid, the console-side switch that makes the segment match as a |
| +// sqid only, so the console's own symmetric resolution can never name-match |
| +// it. |
| +func ledgerAccountFlags(fs *flag.FlagSet) (raw, rawID *string) { |
| + return fs.String("account", "ops", "account (sqid or name) the app belongs to — resolved by the console"), |
| + fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account") |
| } |
| func runLedgerAppend(args []string) { |
| fs := flag.NewFlagSet("ledger append", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw := ledgerAccountFlag(fs) |
| + acctRaw, acctIDRaw := ledgerAccountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to append through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() != 2 { |
| - log.Fatal("usage: carlos ledger append -app <a> [-account <acct>] <ledger> <entry.json|->") |
| + log.Fatal("usage: carlos ledger append -app <a> [-account <acct>] [-account-id <sqid>] <ledger> <entry.json|->") |
| } |
| name, src := fs.Arg(0), fs.Arg(1) |
| if !ledger.ValidName(name) { |
| @@ -1270,7 +1297,7 @@ func runLedgerAppend(args []string) { |
| sess := ledgerAPISession(*consoleFlag) |
| fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address) |
| - a, err := sess.ledgerAppend(context.Background(), *acctRaw, *app, name, json.RawMessage(raw)) |
| + a, err := sess.ledgerAppend(context.Background(), apiAccount(acctRaw, acctIDRaw), *app, name, json.RawMessage(raw)) |
| if err != nil { |
| log.Fatal(err) |
| } |
| @@ -1284,11 +1311,11 @@ func runLedgerAppend(args []string) { |
| func runLedgerHead(args []string) { |
| fs := flag.NewFlagSet("ledger head", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw := ledgerAccountFlag(fs) |
| + acctRaw, acctIDRaw := ledgerAccountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to read through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() != 1 { |
| - log.Fatal("usage: carlos ledger head -app <a> [-account <acct>] <ledger>") |
| + log.Fatal("usage: carlos ledger head -app <a> [-account <acct>] [-account-id <sqid>] <ledger>") |
| } |
| name := fs.Arg(0) |
| if !ledger.ValidName(name) { |
| @@ -1296,7 +1323,7 @@ func runLedgerHead(args []string) { |
| } |
| sess := ledgerAPISession(*consoleFlag) |
| - h, err := sess.ledgerHead(context.Background(), *acctRaw, *app, name) |
| + h, err := sess.ledgerHead(context.Background(), apiAccount(acctRaw, acctIDRaw), *app, name) |
| if err != nil { |
| log.Fatal(err) |
| } |
| @@ -1310,16 +1337,16 @@ func runLedgerHead(args []string) { |
| func runLedgerList(args []string) { |
| fs := flag.NewFlagSet("ledger list", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw := ledgerAccountFlag(fs) |
| + acctRaw, acctIDRaw := ledgerAccountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to read through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" { |
| - log.Fatal("usage: carlos ledger list -app <a> [-account <acct>]") |
| + log.Fatal("usage: carlos ledger list -app <a> [-account <acct>] [-account-id <sqid>]") |
| } |
| sess := ledgerAPISession(*consoleFlag) |
| fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address) |
| - body, err := sess.ledgerList(context.Background(), *acctRaw, *app) |
| + body, err := sess.ledgerList(context.Background(), apiAccount(acctRaw, acctIDRaw), *app) |
| if err != nil { |
| log.Fatal(err) |
| } |
| @@ -1358,13 +1385,13 @@ func runLedgerList(args []string) { |
| func runLedgerPublish(args []string) { |
| fs := flag.NewFlagSet("ledger publish", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw := ledgerAccountFlag(fs) |
| + acctRaw, acctIDRaw := ledgerAccountFlags(fs) |
| public := fs.Bool("public", true, "serve these ledgers publicly (-public=false unpublishes)") |
| title := fs.String("title", "", "display title for the published set") |
| consoleFlag := fs.String("console", "", "console origin to publish through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() == 0 { |
| - log.Fatal("usage: carlos ledger publish -app <a> [-account <acct>] [-public=true|false] [-title <t>] <ledger>...") |
| + log.Fatal("usage: carlos ledger publish -app <a> [-account <acct>] [-account-id <sqid>] [-public=true|false] [-title <t>] <ledger>...") |
| } |
| names := fs.Args() |
| for _, n := range names { |
| @@ -1376,7 +1403,7 @@ func runLedgerPublish(args []string) { |
| sess := ledgerAPISession(*consoleFlag) |
| fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address) |
| m := ledger.Manifest{Public: *public, Ledgers: names, Title: *title} |
| - got, err := sess.ledgerPublish(context.Background(), *acctRaw, *app, m) |
| + got, err := sess.ledgerPublish(context.Background(), apiAccount(acctRaw, acctIDRaw), *app, m) |
| if err != nil { |
| log.Fatal(err) |
| } |
| @@ -1400,11 +1427,11 @@ func runLedgerPublish(args []string) { |
| func runLedgerBlob(args []string) { |
| fs := flag.NewFlagSet("ledger blob", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw := ledgerAccountFlag(fs) |
| + acctRaw, acctIDRaw := ledgerAccountFlags(fs) |
| consoleFlag := fs.String("console", "", "console origin to upload through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() < 2 { |
| - log.Fatal("usage: carlos ledger blob -app <a> [-account <acct>] <ledger> <file>...") |
| + log.Fatal("usage: carlos ledger blob -app <a> [-account <acct>] [-account-id <sqid>] <ledger> <file>...") |
| } |
| name := fs.Arg(0) |
| if !ledger.ValidName(name) { |
| @@ -1422,7 +1449,7 @@ func runLedgerBlob(args []string) { |
| if len(data) > ledger.MaxBlob { |
| log.Fatalf("ledger blob: %s is %d bytes, exceeds the %d byte cap", path, len(data), ledger.MaxBlob) |
| } |
| - sha, err := sess.ledgerBlob(context.Background(), *acctRaw, *app, name, data) |
| + sha, err := sess.ledgerBlob(context.Background(), apiAccount(acctRaw, acctIDRaw), *app, name, data) |
| if err != nil { |
| log.Fatal(err) |
| } |
diff --git a/docs/superpowers/plans/2026-08-04-account-slugs.md b/docs/superpowers/plans/2026-08-04-account-slugs.md| new file mode 100644 |
| index 000000000..ebb937c49 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/2026-08-04-account-slugs.md |
| @@ -0,0 +1,916 @@ |
| +# Account Slugs 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:** Make account names resolve wherever sqids do — `/a/amadan/` in the console, `-account amadan` in the CLI — via symmetric membership-scoped resolution, with ambiguity surfaced (picker page / error naming `-account-id`), per `docs/superpowers/specs/2026-08-04-account-slugs-design.md`. |
| + |
| +**Architecture:** No storage change. A name validator in `internal/domains`; symmetric resolution in the console's `s.account` gate and `apiAccountApp`; a pure `pickAccount` core in the CLI's `resolveAccount`; a `preferredSeg` helper so generated links are pretty while form actions stay on sqid paths. |
| + |
| +**Tech Stack:** Go stdlib only (net/http, html/template, database/sql); no new dependencies. |
| + |
| +## Global Constraints |
| + |
| +- Build/test/vet: `go build ./... && go vet ./... && go test ./... -count=1` — all 14 packages, from the repo root, before every commit. |
| +- Sandbox: prefix go commands with `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off`; "read-only file system" from the toolchain means run with sandbox disabled. |
| +- `gofmt -l` must be clean for every file you touch. |
| +- **Wire shapes are additive** — no manifest/registry/console schema change is permitted by this plan; if a task seems to need one, stop and say so. |
| +- **Console security-gate order is fixed**: session → CSRF → member/owner (byte-identical 404) → `requireFreshAuth` LAST. A non-member must get the byte-identical `http.NotFound` body for names exactly as for sqids. |
| +- **Production channel pins** (`internal/release/laddercheck_test.go`) must stay green and untouched. |
| +- Commit after each task with the trailer: |
| + `Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>` and |
| + `Claude-Session: https://claude.ai/code/session_01QGdTfzwT492ysLPDaWfpvJ` |
| +- Model policy (dispatcher): Task 1 → **Haiku**; Tasks 4, 6 → **Sonnet**; Tasks 2, 3, 5 → **Opus** (they touch the 404 gate and the deploy-target chooser). **Opus reviews every task.** |
| + |
| +--- |
| + |
| +### Task 1: `domains.ValidAccountName` (model: Haiku) |
| + |
| +**Files:** |
| +- Create: `internal/domains/slugname.go` |
| +- Test: `internal/domains/slugname_test.go` |
| + |
| +**Interfaces:** |
| +- Produces: `func ValidAccountName(name string) bool` in package `domains` — later tasks (console picker/preferred-segment, creation-edge validation) call exactly this. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +`internal/domains/slugname_test.go`: |
| + |
| +```go |
| +package domains |
| + |
| +import "testing" |
| + |
| +func TestValidAccountName(t *testing.T) { |
| + valid := []string{"amadan", "a", "kass-2", "ops", "a-b-c", "0day", "x9"} |
| + invalid := []string{ |
| + "", "Amadan", "-lead", "trail-", "two words", "paul@keymail.dev", |
| + "dots.no", "unicode-é", "UPPER", |
| + // 64 chars — one past the limit. |
| + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", |
| + } |
| + for _, name := range valid { |
| + if !ValidAccountName(name) { |
| + t.Errorf("ValidAccountName(%q) = false, want true", name) |
| + } |
| + } |
| + for _, name := range invalid { |
| + if ValidAccountName(name) { |
| + t.Errorf("ValidAccountName(%q) = true, want false", name) |
| + } |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run it, expect FAIL** — `go test ./internal/domains/ -run TestValidAccountName -count=1` fails with "undefined: ValidAccountName". |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +`internal/domains/slugname.go`: |
| + |
| +```go |
| +package domains |
| + |
| +import "regexp" |
| + |
| +// accountNameRe pins what an account NAME may be so it can live in a URL |
| +// path segment and a CLI flag: 1–63 characters of lowercase a-z, 0-9 and |
| +// hyphen, no leading or trailing hyphen. Names are still deliberately |
| +// non-unique (identity is the sqid); this constrains shape, not ownership. |
| +var accountNameRe = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`) |
| + |
| +// ValidAccountName reports whether name is usable as an account name. |
| +// Applied at the edges where a human TYPES a name (console new-account |
| +// form, POST /auth/cli/accounts, `carlos accounts create`) — never at |
| +// auto-mint, where the personal account is named by keymail address and a |
| +// refusal would strand a signed-in user. |
| +func ValidAccountName(name string) bool { return accountNameRe.MatchString(name) } |
| +``` |
| + |
| +- [ ] **Step 4: Run it, expect PASS**, then the full suite. |
| +- [ ] **Step 5: Commit** — `feat(domains): ValidAccountName pins the slug shape account names must take` |
| + |
| +--- |
| + |
| +### Task 2: Console symmetric resolution + picker page (model: Opus) |
| + |
| +**Files:** |
| +- Modify: `internal/console/server.go` (`s.account`, ~line 525–560; `page` struct ~line 281) |
| +- Create: `internal/console/templates/picker.html` |
| +- Create: `internal/console/resolve.go` (`matchAccounts`) |
| +- Test: `internal/console/server_account_test.go` (extend), `internal/console/resolve_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `store.AccountsFor(addr) ([]Account, error)` (fills `Role`), `page` fields, `s.render`. |
| +- Produces: `func matchAccounts(accts []Account, seg string) []*Account` — Task 3 mirrors its semantics; Task 4 relies on `s.account`'s resolved account being membership-scoped. New `page` fields `PickerSeg string`, `PickerAccounts []Account`. |
| + |
| +**Semantics to implement** (from spec §2): `/a/{seg}` resolves against the viewer's memberships, matching `seg` as sqid OR name (case-insensitive). One match → proceed as today. Zero → byte-identical `http.NotFound`. Several → GET renders the picker (200); any other method gets `http.NotFound` (forms post to sqid paths, so an ambiguous non-GET has no legitimate author). Signed-out still redirects to `/`. |
| + |
| +- [ ] **Step 1: Write failing tests** |
| + |
| +`internal/console/resolve_test.go`: |
| + |
| +```go |
| +package console |
| + |
| +import "testing" |
| + |
| +func TestMatchAccounts(t *testing.T) { |
| + accts := []Account{ |
| + {ID: 1, Sqid: "bab", Name: "ops"}, |
| + {ID: 2, Sqid: "bah", Name: "amadan"}, |
| + {ID: 3, Sqid: "bad", Name: "amadan"}, |
| + {ID: 4, Sqid: "baf", Name: "bah"}, // name colliding with #2's sqid |
| + {ID: 5, Sqid: "bag", Name: "Mixed"}, |
| + } |
| + cases := []struct { |
| + seg string |
| + want []int64 // matched account IDs, in accts order |
| + }{ |
| + {"bab", []int64{1}}, // sqid only |
| + {"ops", []int64{1}}, // name only |
| + {"amadan", []int64{2, 3}}, // two accounts share the name |
| + {"bah", []int64{2, 4}}, // sqid of one, name of another |
| + {"mixed", []int64{5}}, // names match case-insensitively |
| + {"zzz", nil}, // nothing |
| + } |
| + for _, c := range cases { |
| + got := matchAccounts(accts, c.seg) |
| + var ids []int64 |
| + for _, a := range got { |
| + ids = append(ids, a.ID) |
| + } |
| + if len(ids) != len(c.want) { |
| + t.Errorf("matchAccounts(%q) = %v, want %v", c.seg, ids, c.want) |
| + continue |
| + } |
| + for i := range ids { |
| + if ids[i] != c.want[i] { |
| + t.Errorf("matchAccounts(%q) = %v, want %v", c.seg, ids, c.want) |
| + break |
| + } |
| + } |
| + } |
| +} |
| +``` |
| + |
| +Extend `internal/console/server_account_test.go` (use the existing fixture helpers — `newConsoleFixture`, `newBrowser`, `signUp`, `get`, `postForm`, `testAccountID`, `accountSqid`; read the fixture first to learn the fixture account's name): |
| + |
| +```go |
| +// A member can address an account by its name; the page is the same page |
| +// its sqid serves. |
| +func TestAccountNameResolvesForMember(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + id := testAccountID() |
| + sqid := accountSqid(id) |
| + if err := fx.st.CreateAccount(id, sqid, "amadan", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + st, _, byName := get(t, c, fx.ts.URL+"/a/amadan/") |
| + if st != http.StatusOK { |
| + t.Fatalf("GET /a/amadan/ = %d, want 200", st) |
| + } |
| + if !strings.Contains(byName, "amadan") { |
| + t.Fatalf("name-resolved page does not mention the account") |
| + } |
| +} |
| + |
| +// A name the viewer cannot resolve — someone ELSE's account — answers with |
| +// the byte-identical 404 an unknown page gets, exactly as sqids do. |
| +func TestAccountNameNotProbeable(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + id := testAccountID() |
| + if err := fx.st.CreateAccount(id, accountSqid(id), "secretco", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + fx.setUser("bob@keymail.dev") |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-bob") |
| + st, _, body := get(t, c, fx.ts.URL+"/a/secretco/") |
| + _, _, ghost := get(t, c, fx.ts.URL+"/definitely-not-a-page") |
| + if st != http.StatusNotFound || body != ghost { |
| + t.Fatalf("non-member name lookup: status %d, body match %v — want the byte-identical 404", st, body == ghost) |
| + } |
| +} |
| + |
| +// Two memberships sharing a name: GET renders the picker naming both sqids; |
| +// a POST to the ambiguous segment is refused with the same 404. |
| +func TestAmbiguousNameRendersPicker(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + idA, idB := testAccountID(), testAccountID() |
| + sqidA, sqidB := accountSqid(idA), accountSqid(idB) |
| + for _, x := range []struct { |
| + id int64 |
| + sqid string |
| + }{{idA, sqidA}, {idB, sqidB}} { |
| + if err := fx.st.CreateAccount(x.id, x.sqid, "amadan", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + st, _, body := get(t, c, fx.ts.URL+"/a/amadan/") |
| + if st != http.StatusOK { |
| + t.Fatalf("ambiguous GET = %d, want 200 picker", st) |
| + } |
| + if !strings.Contains(body, "/a/"+sqidA+"/") || !strings.Contains(body, "/a/"+sqidB+"/") { |
| + t.Fatalf("picker must link both candidates by sqid:\n%s", body) |
| + } |
| + if st, _, _ := postForm(t, c, fx.ts.URL+"/a/amadan/invite", url.Values{"address": {"x@keymail.dev"}}); st != http.StatusNotFound { |
| + t.Errorf("POST to ambiguous segment = %d, want 404", st) |
| + } |
| +} |
| + |
| +// A name that equals another membership's SQID is ambiguous the same way — |
| +// symmetric resolution never silently prefers one space over the other. |
| +func TestNameSqidCollisionRendersPicker(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + idA := testAccountID() |
| + sqidA := accountSqid(idA) |
| + if err := fx.st.CreateAccount(idA, sqidA, "amadan", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + idB := testAccountID() |
| + if err := fx.st.CreateAccount(idB, accountSqid(idB), sqidA, "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + st, _, body := get(t, c, fx.ts.URL+"/a/"+sqidA+"/") |
| + if st != http.StatusOK || !strings.Contains(body, "/a/"+accountSqid(idB)+"/") { |
| + t.Fatalf("sqid-vs-name collision: status %d — want the picker listing both", st) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run, expect FAIL** — `matchAccounts` undefined; name URLs 404. |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +`internal/console/resolve.go`: |
| + |
| +```go |
| +package console |
| + |
| +import "strings" |
| + |
| +// matchAccounts resolves one path/flag segment against the accounts the |
| +// VIEWER belongs to, matching it as a sqid or a name — symmetrically, with |
| +// no precedence between the two spaces. Precedence is how a reference |
| +// silently changes meaning the day a colliding account appears; a multi- |
| +// match is returned whole so the caller can be loud instead (picker page, |
| +// CLI error). Names match case-insensitively; an account whose name equals |
| +// its own sqid still counts once. |
| +func matchAccounts(accts []Account, seg string) []*Account { |
| + var out []*Account |
| + for i := range accts { |
| + if accts[i].Sqid == seg || strings.EqualFold(accts[i].Name, seg) { |
| + out = append(out, &accts[i]) |
| + } |
| + } |
| + return out |
| +} |
| +``` |
| + |
| +Rewrite `s.account` in `internal/console/server.go` (keeping its comment's contract, updated): |
| + |
| +```go |
| +// account is the gate on every /a/{seg}/… route: a session, and a segment |
| +// that resolves — as a sqid OR a name, symmetrically — to an account the |
| +// viewer belongs to. Resolution and membership proof are deliberately the |
| +// same lookup (the membership set IS the resolution scope), so an unknown |
| +// account, someone else's account, and a name that resolves to nothing all |
| +// answer with the BYTE-IDENTICAL http.NotFound an unrouted URL gets. |
| +// Several matches (a shared name, or a name colliding with a sqid) render |
| +// the picker on GET and the same 404 otherwise — forms always post to sqid |
| +// paths, so an ambiguous mutation has no legitimate author. |
| +func (s *Server) account(w http.ResponseWriter, r *http.Request) (addr, raw string, acct *Account, role string, ok bool) { |
| + addr, raw, sok := s.session(r) |
| + if !sok { |
| + http.Redirect(w, r, "/", http.StatusFound) |
| + return "", "", nil, "", false |
| + } |
| + accts, err := s.store.AccountsFor(addr) |
| + if err != nil { |
| + log.Printf("console: account lookup: %v", err) |
| + http.NotFound(w, r) |
| + return "", "", nil, "", false |
| + } |
| + seg := r.PathValue("sqid") |
| + matches := matchAccounts(accts, seg) |
| + switch len(matches) { |
| + case 1: |
| + a := matches[0] |
| + return addr, raw, a, a.Role, true |
| + case 0: |
| + http.NotFound(w, r) |
| + return "", "", nil, "", false |
| + default: |
| + if r.Method != http.MethodGet { |
| + http.NotFound(w, r) |
| + return "", "", nil, "", false |
| + } |
| + p := s.basePage(r) |
| + p.Title = "Which " + seg + "?" |
| + p.PickerSeg = seg |
| + for _, m := range matches { |
| + p.PickerAccounts = append(p.PickerAccounts, *m) |
| + } |
| + s.render(w, http.StatusOK, "picker", p) |
| + return "", "", nil, "", false |
| + } |
| +} |
| +``` |
| + |
| +Note: `role` now comes from `AccountsFor`'s `m.Role` — verify `AccountsFor` fills it (it selects `m.role`; it does). The old `AccountBySqid`+`Role` pair is no longer called here; leave those store methods alone (other callers exist). |
| + |
| +Add to `page` struct (near the account chrome block): |
| + |
| +```go |
| + PickerSeg string // the ambiguous segment the picker is disambiguating |
| + PickerAccounts []Account // every account of the viewer's that segment matches |
| +``` |
| + |
| +`internal/console/templates/picker.html` (match the house template style — `{{define}}`, `top`/`bottom`): |
| + |
| +```html |
| +{{define "picker"}}{{template "top" .}} |
| +<h1>Which “{{.PickerSeg}}”?</h1> |
| +<p class="lede">More than one of your accounts answers to |
| +<code>{{.PickerSeg}}</code>. Pick the one you meant — a sqid link is |
| +permanent and never asks this question.</p> |
| +<table> |
| + <thead><tr><th>Name</th><th>Sqid</th><th>Your role</th><th>Created</th></tr></thead> |
| + <tbody> |
| + {{range .PickerAccounts}} |
| + <tr> |
| + <td><a href="/a/{{.Sqid}}/">{{.Name}}</a></td> |
| + <td><code>/a/{{.Sqid}}/</code></td> |
| + <td>{{.Role}}</td> |
| + <td>{{.CreatedAt.Format "2006-01-02"}}</td> |
| + </tr> |
| + {{end}} |
| + </tbody> |
| +</table> |
| +{{template "bottom" .}}{{end}} |
| +``` |
| + |
| +- [ ] **Step 4: Run the new tests, then the whole console package, then the full suite.** `TestNonMemberGetsIdenticalNotFound` and every existing `/a/<sqid>` test must pass unchanged. |
| +- [ ] **Step 5: Commit** — `feat(console): /a/{seg} resolves names symmetrically; ambiguity renders a picker, never a guess` |
| + |
| +--- |
| + |
| +### Task 3: Console API symmetric resolution + `exact=sqid` (model: Opus) |
| + |
| +**Files:** |
| +- Modify: `internal/console/apicli.go` (`apiAccountApp`, lines ~30–100) |
| +- Test: alongside the existing API-CLI tests (grep `apiAccountApp\|/api/cli` in `internal/console/*_test.go` and extend that file; if none exists, add `internal/console/apicli_test.go` using the console fixture + a CLI token the way the existing authcli tests mint one) |
| + |
| +**Interfaces:** |
| +- Consumes: `store.AccountsFor`, `jsonError`, `s.cliUser`. |
| +- Produces: the query contract Task 5's client relies on — `?exact=sqid` on any `/api/cli/apps/{account}/{app}/...` URL makes the `{account}` segment match as a sqid only; the ambiguity refusal is HTTP 409 whose message contains `-account-id`. |
| + |
| +- [ ] **Step 1: Write failing tests** — through the HTTP surface (`GET /api/cli/apps/{account}/{app}/channels` is the simplest authed route): |
| + - caller with two accounts named `amadan` (one holding app `web`): request with segment `amadan` → **409**, body mentions both sqids and the string `-account-id`. |
| + - same setup, segment `<sqidA>?exact=sqid` → 200 (or the route's normal answer for an app that exists there). |
| + - caller whose account named `bah` collides with another membership's sqid `bah`: segment `bah` → **409** (symmetric — today's code would silently pick the sqid match). |
| + - `?exact=sqid` with a segment that is a NAME (`amadan`) → 404 (never name-matches). |
| + - non-member account segment (name or sqid) → 404, `http.NotFound` shape. |
| + |
| +- [ ] **Step 2: Run, expect FAIL** (the name/sqid collision silently resolves today; `exact` is ignored). |
| + |
| +- [ ] **Step 3: Implement** — replace the resolution block of `apiAccountApp`: |
| + |
| +```go |
| + seg := r.PathValue("account") |
| + app := r.PathValue("app") |
| + accts, err := s.store.AccountsFor(addr) |
| + if err != nil { |
| + jsonError(w, http.StatusInternalServerError, "could not load accounts") |
| + return "", nil, "", false |
| + } |
| + var matches []*Account |
| + if r.URL.Query().Get("exact") == "sqid" { |
| + // -account-id: the reference that never guesses. Match the segment |
| + // as a sqid only — a name, however unique, does not answer. |
| + for i := range accts { |
| + if accts[i].Sqid == seg { |
| + matches = append(matches, &accts[i]) |
| + break |
| + } |
| + } |
| + } else { |
| + matches = matchAccounts(accts, seg) |
| + } |
| + var acct *Account |
| + switch len(matches) { |
| + case 1: |
| + acct = matches[0] |
| + case 0: |
| + // acct stays nil — the same 404 an unknown or someone-else's |
| + // account gets, below. |
| + default: |
| + alts := make([]string, len(matches)) |
| + for i, m := range matches { |
| + alts[i] = fmt.Sprintf("%s (%q)", m.Sqid, m.Name) |
| + } |
| + jsonError(w, http.StatusConflict, |
| + fmt.Sprintf("account %q is ambiguous — pass -account-id with one of: %s", seg, strings.Join(alts, ", "))) |
| + return "", nil, "", false |
| + } |
| +``` |
| + |
| +Update the function's doc comment: resolution is now symmetric (no sqid precedence), and `exact=sqid` is the `-account-id` transport. Keep everything from the `if acct == nil` 404 onward unchanged. |
| + |
| +- [ ] **Step 4: Run new tests + console package + full suite.** |
| +- [ ] **Step 5: Commit** — `feat(console): API account resolution goes symmetric; exact=sqid carries -account-id` |
| + |
| +--- |
| + |
| +### Task 4: Preferred segments — links pretty, actions canonical (model: Sonnet) |
| + |
| +**Files:** |
| +- Modify: `internal/console/server.go` (`accountPage` ~1068, `redirectToCurrentAccount` ~430, `page` struct — add `Nav`; every `Base:` assignment gets a `Nav:` sibling — grep `Base:`) |
| +- Modify: `internal/console/store.go` (`Account` struct — add view-only `Seg` field) |
| +- Modify: `internal/console/resolve.go` (add `preferredSeg`) |
| +- Modify templates: `layout.html` (lines 229, 241), `apps.html` (line 12), `newaccount.html` (line 17) |
| +- Test: `internal/console/resolve_test.go`, `internal/console/server_account_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `matchAccounts` (Task 2), `domains.ValidAccountName` (Task 1). |
| +- Produces: `func preferredSeg(accts []Account, a *Account) string`; `page.Nav string` (link base, pretty); `Account.Seg string` (switcher hrefs). **Form actions keep using `page.Base` (sqid) — do not touch any `action=` attribute.** |
| + |
| +- [ ] **Step 1: Write failing tests** |
| + |
| +`internal/console/resolve_test.go`: |
| + |
| +```go |
| +func TestPreferredSeg(t *testing.T) { |
| + accts := []Account{ |
| + {ID: 1, Sqid: "bab", Name: "ops"}, |
| + {ID: 2, Sqid: "bah", Name: "amadan"}, |
| + {ID: 3, Sqid: "bad", Name: "amadan"}, |
| + {ID: 4, Sqid: "baf", Name: "paul@keymail.dev"}, |
| + {ID: 5, Sqid: "bag", Name: "bah"}, // collides with #2's sqid |
| + } |
| + cases := []struct { |
| + id int64 |
| + want string |
| + }{ |
| + {1, "ops"}, // unique slug-shaped name → the name |
| + {2, "bah"}, // name shared with #3 → sqid |
| + {3, "bad"}, // same |
| + {4, "baf"}, // address-named (auto-mint) → sqid |
| + {5, "bag"}, // name equals #2's sqid → sqid |
| + } |
| + for _, c := range cases { |
| + var a *Account |
| + for i := range accts { |
| + if accts[i].ID == c.id { |
| + a = &accts[i] |
| + } |
| + } |
| + if got := preferredSeg(accts, a); got != c.want { |
| + t.Errorf("preferredSeg(#%d) = %q, want %q", c.id, got, c.want) |
| + } |
| + } |
| +} |
| +``` |
| + |
| +`server_account_test.go` — landing and chrome: |
| + |
| +```go |
| +// Landing prefers the readable URL: a viewer whose account has a unique |
| +// slug-shaped name lands on /a/<name>/, and the page's nav links use it, |
| +// while its forms keep posting to the sqid path. |
| +func TestLandingPrefersNameSegment(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + fx.setUser("solo@keymail.dev") |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-solo") |
| + id := testAccountID() |
| + sqid := accountSqid(id) |
| + if err := fx.st.CreateAccount(id, sqid, "amadan", "solo@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st, _, _ := get(t, c, fx.ts.URL+"/a/"+sqid+"/"); st != http.StatusOK { |
| + t.Fatal("visit to set MRU") |
| + } |
| + st, loc, _ := get(t, c, fx.ts.URL+"/") |
| + if st != http.StatusFound || loc != "/a/amadan/" { |
| + t.Fatalf("landing = %d -> %q, want 302 -> /a/amadan/", st, loc) |
| + } |
| + _, _, body := get(t, c, fx.ts.URL+"/a/amadan/") |
| + if !strings.Contains(body, `href="/a/amadan/"`) { |
| + t.Errorf("nav links should use the name segment") |
| + } |
| + if !strings.Contains(body, `action="/a/`+sqid+`/`) { |
| + t.Errorf("form actions must stay on the sqid path") |
| + } |
| +} |
| +``` |
| + |
| +(If the fixture's own account name is slug-shaped and unique for `paul@keymail.dev`, existing assertions like `loc != "/a/"+sqid+"/"` in `TestLandingRedirectsToLastAccount` may now see the name — run the suite and update any such assertion to the preferred segment, stating why in the diff.) |
| + |
| +- [ ] **Step 2: Run, expect FAIL.** |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +`internal/console/resolve.go`: |
| + |
| +```go |
| +// preferredSeg is the path segment the console GENERATES for an account: |
| +// its name, when the name is slug-shaped and resolves uniquely among the |
| +// viewer's own accounts; the sqid otherwise. Links and redirects use it — |
| +// forms always post to the sqid path (page.Base), because a mutation must |
| +// name its target exactly and prettiness is for links. |
| +func preferredSeg(accts []Account, a *Account) string { |
| + if !domains.ValidAccountName(a.Name) { |
| + return a.Sqid |
| + } |
| + for i := range accts { |
| + if accts[i].ID == a.ID { |
| + continue |
| + } |
| + if strings.EqualFold(accts[i].Name, a.Name) || accts[i].Sqid == a.Name { |
| + return a.Sqid |
| + } |
| + } |
| + return a.Name |
| +} |
| +``` |
| + |
| +(import `github.com/carlosframework/platform/internal/domains` — the console already depends on it.) |
| + |
| +`store.go` `Account` struct — add: |
| + |
| +```go |
| + // Seg is the path segment the console generates for this account in the |
| + // viewer's chrome (preferredSeg) — display state, never stored. |
| + Seg string |
| +``` |
| + |
| +`accountPage` — after the existing `p.Accounts` fallback block: |
| + |
| +```go |
| + for i := range p.Accounts { |
| + p.Accounts[i].Seg = preferredSeg(p.Accounts, &p.Accounts[i]) |
| + } |
| + p.Nav = "/a/" + url.PathEscape(preferredSeg(p.Accounts, acct)) |
| +``` |
| + |
| +`page` struct — next to `Base`: |
| + |
| +```go |
| + Nav string // "/a/<name-or-sqid>" — what LINKS use; forms keep posting to Base |
| +``` |
| + |
| +Every other `Base:` assignment (grep — e.g. the `/accounts/new` page builder) gets `Nav` set the same way; where the accounts list isn't loaded, set `Nav` equal to `Base` rather than loading it. |
| + |
| +`redirectToCurrentAccount` — before the redirect: |
| + |
| +```go |
| + seg := sqid |
| + if accts, err := s.store.AccountsFor(addr); err == nil { |
| + for i := range accts { |
| + if accts[i].Sqid == sqid { |
| + seg = preferredSeg(accts, &accts[i]) |
| + break |
| + } |
| + } |
| + } |
| + http.Redirect(w, r, "/a/"+url.PathEscape(seg)+"/", code) |
| +``` |
| + |
| +Templates — hrefs only: |
| +- `layout.html:229`: `<a href="/a/{{.Seg}}/">…{{.Name}}</a>` |
| +- `layout.html:241`: `<a href="{{.Nav}}/">Apps</a>` |
| +- `apps.html:12`: `<a href="{{$.Nav}}/apps/{{.App}}">{{.App}}</a>` |
| +- `newaccount.html:17`: `<a href="{{.Nav}}/">Back to {{.Account.Name}}</a>` |
| + |
| +Redirects **after POST** (ownership.go's `/a/"+acct.Sqid+"/`, `handleNewAccount`'s) stay on sqid paths — do not change them. |
| + |
| +- [ ] **Step 4: Run console package, then full suite; fix any landing assertions per Step 1's note.** |
| +- [ ] **Step 5: Commit** — `feat(console): links and landings prefer the account name; forms stay on sqid paths` |
| + |
| +--- |
| + |
| +### Task 5: CLI — symmetric `resolveAccount`, `-account-id`, API-mode threading (model: Opus) |
| + |
| +**Files:** |
| +- Modify: `accounts.go` (`resolveAccount` ~line 622 — extract pure `pickAccount`) |
| +- Modify: `cli.go` (`accountFlags` ~738 and its call sites ~816, 984, 1052, 1108; the manual `-account` registrations at ~69/130 and ~267/345; `ledgerAccountFlag` ~1237 and its call sites) |
| +- Modify: `apimode.go` (`apiPath` ~169; the `apiSession` methods that take an `account string`) |
| +- Test: `accounts_resolve_test.go` (new, package main) |
| + |
| +**Interfaces:** |
| +- Consumes: `registry.Account{ID, Sqid, Name}`, `release.ValidAccount`, the `exact=sqid` query contract from Task 3. |
| +- Produces: |
| + - `func pickAccount(accts []registry.Account, want string) (string, error)` — pure resolution core. |
| + - `func resolveAccount(regPath, want, wantID string) string` — new third parameter. |
| + - `func accountFlags(fs *flag.FlagSet) (raw, rawID *string, resolve func() string)`. |
| + - `func apiAccount(raw, rawID *string) apiAcct` and `type apiAcct struct { Seg string; Exact bool }` — every `apiSession` method that took `account string` takes `apiAcct`; `apiPath(a apiAcct, app, tail string)` appends `?exact=sqid` when `a.Exact` (verify no existing `tail` carries a query string first). |
| + |
| +- [ ] **Step 1: Write failing tests** |
| + |
| +`accounts_resolve_test.go`: |
| + |
| +```go |
| +package main |
| + |
| +import ( |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +func TestPickAccount(t *testing.T) { |
| + accts := []registry.Account{ |
| + {ID: 1, Sqid: "bab", Name: "ops"}, |
| + {ID: 2, Sqid: "bah", Name: "amadan"}, |
| + {ID: 3, Sqid: "bad", Name: "amadan"}, |
| + {ID: 4, Sqid: "baf", Name: "bah"}, // name collides with #2's sqid |
| + } |
| + if got, err := pickAccount(accts, "bab"); err != nil || got != "bab" { |
| + t.Errorf("sqid: %q, %v", got, err) |
| + } |
| + if got, err := pickAccount(accts, "ops"); err != nil || got != "bab" { |
| + t.Errorf("unique name: %q, %v", got, err) |
| + } |
| + if got, err := pickAccount(accts, "OPS"); err != nil || got != "bab" { |
| + t.Errorf("case-insensitive name: %q, %v", got, err) |
| + } |
| + // A sqid nobody has materialized passes through — a fresh box may not |
| + // have reconciled, and CI has no registry at all. |
| + if got, err := pickAccount(accts, "zzz"); err != nil || got != "zzz" { |
| + t.Errorf("passthrough sqid: %q, %v", got, err) |
| + } |
| + if _, err := pickAccount(accts, "not-a-sqid-or-name"); err == nil { |
| + t.Error("unresolvable non-sqid name must error") |
| + } |
| + // The two ambiguity shapes: a shared name, and a name/sqid collision. |
| + // Both must refuse, and the fix-up named must be -account-id. |
| + for _, want := range []string{"amadan", "bah"} { |
| + _, err := pickAccount(accts, want) |
| + if err == nil || !strings.Contains(err.Error(), "-account-id") { |
| + t.Errorf("pickAccount(%q) = %v, want ambiguity error naming -account-id", want, err) |
| + } |
| + } |
| + if _, err := pickAccount(accts, "bah"); err == nil || |
| + !strings.Contains(err.Error(), "bah") || !strings.Contains(err.Error(), "baf") { |
| + t.Error("name/sqid conflict error must list both candidates") |
| + } |
| +} |
| + |
| +func TestApiAccountExact(t *testing.T) { |
| + acct, id := "ops", "bah" |
| + a := apiAccount(&acct, &id) |
| + if a.Seg != "bah" || !a.Exact { |
| + t.Errorf("apiAccount with -account-id = %+v, want {bah true}", a) |
| + } |
| + id = "" |
| + a = apiAccount(&acct, &id) |
| + if a.Seg != "ops" || a.Exact { |
| + t.Errorf("apiAccount without -account-id = %+v, want {ops false}", a) |
| + } |
| + if p := apiPath(apiAcct{Seg: "bah", Exact: true}, "web", "/channels"); !strings.HasSuffix(p, "?exact=sqid") { |
| + t.Errorf("apiPath exact = %q, want ?exact=sqid suffix", p) |
| + } |
| + if p := apiPath(apiAcct{Seg: "ops"}, "web", "/channels"); strings.Contains(p, "exact") { |
| + t.Errorf("apiPath non-exact = %q, must carry no exact param", p) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run, expect FAIL** (`pickAccount`, `apiAccount`, `apiAcct` undefined). |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +`accounts.go` — split `resolveAccount`: |
| + |
| +```go |
| +// pickAccount is resolveAccount's core, symmetric over the two identity |
| +// spaces: want matches an account by sqid OR by name (case-insensitively), |
| +// with NO precedence between them. Precedence is how a script's -account |
| +// silently changes target the day a colliding account materializes; a |
| +// conflict here is loud instead, and names -account-id, the reference that |
| +// never guesses. A sqid-shaped want that matches nothing passes through (a |
| +// fresh box may not have reconciled; CI has no registry at all). |
| +func pickAccount(accts []registry.Account, want string) (string, error) { |
| + var candidates []registry.Account |
| + for _, a := range accts { |
| + if a.Sqid == want || strings.EqualFold(a.Name, want) { |
| + candidates = append(candidates, a) |
| + } |
| + } |
| + switch len(candidates) { |
| + case 1: |
| + return candidates[0].Sqid, nil |
| + case 0: |
| + if release.ValidAccount(want) { |
| + return want, nil |
| + } |
| + return "", fmt.Errorf("no account %q here — `carlos accounts list` shows what this box knows", want) |
| + default: |
| + alts := make([]string, len(candidates)) |
| + for i, a := range candidates { |
| + alts[i] = fmt.Sprintf("%s (%q)", a.Sqid, a.Name) |
| + } |
| + return "", fmt.Errorf("account %q is ambiguous — pass -account-id with one of: %s", |
| + want, strings.Join(alts, ", ")) |
| + } |
| +} |
| +``` |
| + |
| +`resolveAccount` keeps its shell (empty/"ops" shortcut, registry-open with its existing no-registry fallback and messages) and gains the id short-circuit; the matching loops are replaced by one `pickAccount` call whose error goes to `log.Fatalf`: |
| + |
| +```go |
| +func resolveAccount(regPath, want, wantID string) string { |
| + if wantID = strings.TrimSpace(wantID); wantID != "" { |
| + if !release.ValidAccount(wantID) { |
| + log.Fatalf("-account-id %q is not a sqid — it is matched exactly, never as a name", wantID) |
| + } |
| + return wantID |
| + } |
| + // ... existing empty/"ops" shortcut and registry-open fallback unchanged ... |
| + sqid, err := pickAccount(accts, want) |
| + if err != nil { |
| + log.Fatal(err) |
| + } |
| + return sqid |
| +} |
| +``` |
| + |
| +`cli.go` `accountFlags`: |
| + |
| +```go |
| +func accountFlags(fs *flag.FlagSet) (raw, rawID *string, resolve func() string) { |
| + acct := fs.String("account", "ops", "account (sqid or name) the app belongs to") |
| + acctID := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account; the flag ambiguity errors tell you to use") |
| + regPath := fs.String("registry", defaultRegistry(), "registry database (resolves -account)") |
| + return acct, acctID, func() string { return resolveAccount(*regPath, *acct, *acctID) } |
| +} |
| +``` |
| + |
| +Every `accountFlags` call site becomes `acctRaw, acctIDRaw, acctOf := accountFlags(fs)`; API-mode branches build `apiAccount(acctRaw, acctIDRaw)` instead of `*acctRaw` (this changes `runShipAPI`'s signature — pass both pointers). The two manual `-account` registrations (`cli.go` ~69 and ~267) each gain their own `fs.String("account-id", "", …)` threaded into their `resolveAccount` calls (the ~267 route-filter keeps its `"all"` semantics: `all` never reaches `resolveAccount`, so nothing changes there). `ledgerAccountFlag` grows into: |
| + |
| +```go |
| +func ledgerAccountFlags(fs *flag.FlagSet) (raw, rawID *string) { |
| + return fs.String("account", "ops", "account (sqid or name) the app belongs to — resolved by the console"), |
| + fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account") |
| +} |
| +``` |
| + |
| +with its call sites building `apiAccount(raw, rawID)`. |
| + |
| +`apimode.go`: |
| + |
| +```go |
| +// apiAcct is one resolved -account/-account-id pair, API-mode: the segment |
| +// the console will resolve, and whether it must match as a sqid only. |
| +type apiAcct struct { |
| + Seg string |
| + Exact bool |
| +} |
| + |
| +// apiAccount is resolveAccount's API-mode counterpart. Names resolve |
| +// SERVER-side against the caller's memberships, so -account travels |
| +// verbatim; -account-id travels as the segment plus exact=sqid, so the |
| +// console never name-matches it. |
| +func apiAccount(raw, rawID *string) apiAcct { |
| + if id := strings.TrimSpace(*rawID); id != "" { |
| + if !release.ValidAccount(id) { |
| + log.Fatalf("-account-id %q is not a sqid — it is matched exactly, never as a name", id) |
| + } |
| + return apiAcct{Seg: id, Exact: true} |
| + } |
| + return apiAcct{Seg: *raw} |
| +} |
| +``` |
| + |
| +`apiPath` takes the pair (update its comment: account still resolves server-side): |
| + |
| +```go |
| +func apiPath(a apiAcct, app, tail string) string { |
| + p := "/api/cli/apps/" + url.PathEscape(a.Seg) + "/" + url.PathEscape(app) + tail |
| + if a.Exact { |
| + p += "?exact=sqid" |
| + } |
| + return p |
| +} |
| +``` |
| + |
| +Change each `apiSession` method (`promote`, `rollback`, `channels`, `ship`, the five `ledger*`) from `account string` to `a apiAcct`, passing it through to `apiPath`. First verify no call passes a `tail` containing `?`; if one does, switch `apiPath` to `url.Values` instead of string concat and say so in the commit message. |
| + |
| +- [ ] **Step 4: Run new tests, then the full suite** — `apimode_test.go` exercises this client surface end-to-end and must stay green (it will need its call sites updated for the new signatures; behavior assertions stay). |
| +- [ ] **Step 5: Commit** — `cli: -account resolves symmetrically and loudly; -account-id is the reference that never guesses` |
| + |
| +--- |
| + |
| +### Task 6: Validate names at the three creation edges (model: Sonnet) |
| + |
| +**Files:** |
| +- Modify: `internal/console/server.go` (`handleNewAccount` ~633; `mintAccount` comment ~500) |
| +- Modify: `internal/console/authcli.go` (`handleCLINewAccount` ~686) |
| +- Modify: `accounts.go` (`runAccountsCreate` ~79) |
| +- Modify: `internal/console/templates/newaccount.html` (the name input) |
| +- Test: `internal/console/server_account_test.go` (or the file housing existing new-account tests — grep `accounts/new`) |
| + |
| +**Interfaces:** |
| +- Consumes: `domains.ValidAccountName` (Task 1). |
| +- Produces: nothing new — this closes the front doors. |
| + |
| +- [ ] **Step 1: Write failing tests** |
| + |
| +```go |
| +// The three edges where a human types a name all refuse a non-slug; the |
| +// auto-mint path (address-named personal accounts) must NOT be validated — |
| +// a mint that can refuse strands a signed-in user. |
| +func TestNewAccountNameValidated(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + before, err := fx.st.AccountsFor("paul@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + st, loc, _ := postForm(t, c, fx.ts.URL+"/accounts/new", url.Values{"name": {"Paul's Stuff!"}}) |
| + if st != http.StatusSeeOther || loc != "/accounts/new" { |
| + t.Fatalf("bad name = %d -> %q, want 303 back to the form", st, loc) |
| + } |
| + after, err := fx.st.AccountsFor("paul@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(after) != len(before) { |
| + t.Fatal("a rejected name must not create an account") |
| + } |
| + if st, _, _ := postForm(t, c, fx.ts.URL+"/accounts/new", url.Values{"name": {"amadan"}}); st != http.StatusSeeOther { |
| + t.Fatalf("good name = %d, want 303 into the new account", st) |
| + } |
| +} |
| +``` |
| + |
| +(Adapt the CSRF plumbing to how existing `/accounts/new` tests post the form — copy their shape. Add the equivalent 400 assertion for `POST /auth/cli/accounts` in the file that already tests `handleCLINewAccount`, posting `{"name":"Paul's Stuff!"}` with a valid CLI token and expecting `http.StatusBadRequest`.) |
| + |
| +- [ ] **Step 2: Run, expect FAIL** (free-text names are accepted today). |
| + |
| +- [ ] **Step 3: Implement** |
| + |
| +`handleNewAccount` — replace the `name == "" || len(name) > 64` check: |
| + |
| +```go |
| + name := strings.TrimSpace(r.FormValue("name")) |
| + if !domains.ValidAccountName(name) { |
| + s.setFlash(w, "err", "Account names appear in URLs: 1–63 characters of a-z, 0-9 and hyphen, no leading or trailing hyphen.") |
| + http.Redirect(w, r, "/accounts/new", http.StatusSeeOther) |
| + return |
| + } |
| +``` |
| + |
| +`handleCLINewAccount` — same predicate: |
| + |
| +```go |
| + name := strings.TrimSpace(req.Name) |
| + if !domains.ValidAccountName(name) { |
| + jsonError(w, http.StatusBadRequest, |
| + "an account name appears in URLs and -account: 1-63 characters of a-z, 0-9 and hyphen, no leading or trailing hyphen") |
| + return |
| + } |
| +``` |
| + |
| +`runAccountsCreate` — after the existing empty-name usage check: |
| + |
| +```go |
| + if !domains.ValidAccountName(strings.TrimSpace(*name)) { |
| + log.Fatalf("accounts create: %q is not a usable account name — 1-63 characters of a-z, 0-9\n"+ |
| + "and hyphen, no leading or trailing hyphen. The name appears in console URLs and\n"+ |
| + "resolves -account.", *name) |
| + } |
| +``` |
| + |
| +Also update the `-name` flag's help text (`"account name (free text, need not be unique)"` → `"account name (a-z, 0-9, hyphen; need not be unique)"`). |
| + |
| +`mintAccount` — add one line to its comment: |
| + |
| +```go |
| +// The name is NOT validated here: auto-mint names personal accounts by |
| +// keymail address, and landing must never refuse to mint (spec §1). |
| +``` |
| + |
| +`newaccount.html` — on the name `<input>`, add `pattern="[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?"` and a hint line under it: `a-z, 0-9 and hyphens — it appears in your URLs`. Adjust any copy that says names are free text. |
| + |
| +- [ ] **Step 4: Run the console package and full suite** — `TestLandingMintsPersonalAccountWhenNoMemberships` (auto-mint) must stay green; update any existing test that created accounts through these handlers with non-slug names. |
| +- [ ] **Step 5: Commit** — `feat: account names are validated at the three doors a human types one through` |
| + |
| +--- |
| + |
| +### Task 7: Verification sweep (dispatcher, with Opus review of the whole diff) |
| + |
| +- [ ] `go build ./... && go vet ./... && go test ./... -count=1` — all packages, zero failures. |
| +- [ ] `gofmt -l .` — no touched file listed (the three pre-existing unformatted files elsewhere are left alone). |
| +- [ ] `git log --oneline main..HEAD` — one commit per task, plan and spec committed. |
| +- [ ] Opus reviews the full branch diff against the spec (superpowers:requesting-code-review), with explicit attention to: byte-identical 404 preserved for names; no `action=` attribute moved off `Base`; no storage shape changed; `laddercheck_test.go` untouched. |
diff --git a/docs/superpowers/specs/2026-08-04-account-slugs-design.md b/docs/superpowers/specs/2026-08-04-account-slugs-design.md| new file mode 100644 |
| index 000000000..312961682 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/2026-08-04-account-slugs-design.md |
| @@ -0,0 +1,187 @@ |
| +# Account names as addresses — slugs without a namespace |
| + |
| +`/a/bah/` works but nobody can read it. This spec makes the account **name** |
| +usable wherever the sqid is usable today — `/a/amadan/` in the console, |
| +`-account amadan` in the CLI — without creating a global name registry, |
| +changing any storage shape, or weakening the byte-identical-404 property. |
| + |
| +Extends `2026-08-02-multi-account-design.md` (§1, §3): names stay |
| +**deliberately non-unique** — identity is still the sqid, so there is still |
| +nothing to squat. What changes is that a name now *resolves*, per viewer, |
| +against the accounts that viewer is a member of. |
| + |
| +## 1. The name becomes the slug |
| + |
| +There is no new field. The existing `name` — already carried by the account |
| +manifest, the registry row and the console row — is the slug, constrained at |
| +the edges where a human types one: |
| + |
| +- **Charset**: `[a-z0-9-]`, 1–63 characters, no leading or trailing hyphen. |
| + Lowercase only; there is no case-folding ambiguity to litigate later. |
| +- **Validated at the three user-input edges**: the console's new-account |
| + form, `POST /auth/cli/accounts` (the console-mediated |
| + `carlos accounts create -console` path), and headless |
| + `carlos accounts create` writing straight to the bucket. One validator, |
| + `domains.ValidAccountName`, used by all three — the CLI and the console |
| + must not disagree about what a name is. |
| +- **Not validated at auto-mint.** The personal account minted on first |
| + sign-in is named by keymail address (`paul@keymail.dev`), which is not |
| + slug-shaped and must never fail: landing mints, and a mint that can refuse |
| + strands a signed-in user. Address-named accounts are simply not |
| + URL-addressable by name; their sqid works as it always has. |
| +- **Existing names are left alone.** No migration, no normalization pass. |
| + A stored name that happens to be slug-shaped (every real tenant account |
| + today: `ops`, `kass`, `keymail`, `bae`…) becomes addressable the day this |
| + ships; one that isn't, isn't. Name matching stays case-insensitive, as the |
| + CLI's resolver has always been. |
| + |
| +**Wire shape: unchanged.** No manifest field is added, removed or |
| +re-interpreted; anything written before this change parses exactly as |
| +before. Validation is a front-door policy, not a storage property. |
| + |
| +A free-text **display label** is named future work: if someone ever wants |
| +"Paul's Experiments 🧪" on the page while `amadan` addresses it, that is an |
| +additive `label` field on console row + manifest, defaulting to the name. |
| +Not built now. |
| + |
| +## 2. Resolution is symmetric and membership-scoped |
| + |
| +One rule, console and CLI alike: |
| + |
| +> A reference resolves against the viewer's own account set, matching as |
| +> **either** sqid or name. Exactly one account matches → that account. More |
| +> than one → surface the ambiguity, never pick. Zero → exactly what a |
| +> nonexistent account produces today. |
| + |
| +**Symmetric, not precedence-ordered — this is the collision answer.** The |
| +CLI today gives an exact sqid match silent precedence, which means a script |
| +targeting an account *named* `bah` silently changes target the day its owner |
| +is invited to an account whose *sqid* is `bah`. Collisions between the two |
| +spaces cannot be prevented: sqids are minted from sequential ids (nobody |
| +chooses them), names have no global registry to check (by design), and the |
| +sqid alphabet is 23 lowercase letters so most good names — `amadan` |
| +included — are valid future sqids. Prevention being off the table, the rule |
| +is that a collision is **loud, never wrong**: a reference matching two |
| +different accounts is an ambiguity in both surfaces, identical to two |
| +accounts sharing a name. |
| + |
| +### Console |
| + |
| +`/a/{seg}` resolves `seg` against the signed-in viewer's memberships: |
| + |
| +- **One match** → the account page, exactly as `/a/<sqid>` behaves today. |
| +- **Several matches** (same name twice, or a name/sqid collision) → a |
| + **picker page**: each candidate's name, sqid, the viewer's role and the |
| + creation date, linking to `/a/<sqid>/`. This is the disambiguation map — |
| + it lists only accounts the viewer is a member of, so it discloses nothing. |
| +- **No match** → the byte-identical 404 (`http.NotFound`), same bytes as an |
| + account that does not exist. The resolution scope *is* the membership |
| + set, so the invariant — a non-member cannot probe whether an account |
| + exists — holds by construction, for names as well as sqids. |
| +- **Mutations don't negotiate.** The picker renders for GET only. Forms are |
| + always generated against sqid paths (§3), so a non-GET arriving at an |
| + ambiguous segment has no legitimate author; it gets the 404. |
| + |
| +The security gate order is untouched: session → CSRF → membership/owner |
| +(byte-identical 404) → `requireFreshAuth` last. Name resolution happens |
| +inside the membership step, not before it. |
| + |
| +**One consequence, accepted and pinned**: `/a/<sqid>` for an account you are |
| +NOT a member of has always 404'd. Under membership-scoped resolution, if |
| +that same string happens to be the *name* of an account you are in, you land |
| +on your own account instead. Nothing leaks — the response is derived |
| +entirely from your own memberships — but a shared sqid link can now resolve |
| +differently for a recipient who owns that name. Mildly surprising, and |
| +strictly better than the alternative: resolving sqids outside the membership |
| +set is exactly what the 404 invariant forbids. |
| + |
| +### CLI |
| + |
| +`resolveAccount` becomes symmetric over the registry's materialized |
| +accounts: collect the sqid match and the case-insensitive name matches; if |
| +they disagree about which account is meant, fail listing every candidate |
| +and naming the fix (`-account-id`). The existing behaviors that are not |
| +precedence stay: `ops` short-circuits, a sqid-shaped string that matches |
| +nothing passes through verbatim (a fresh box may not have reconciled; CI |
| +has no registry at all), a non-sqid-shaped name that matches nothing is |
| +fatal. |
| + |
| +**`-account-id` — the reference that never guesses.** Registered alongside |
| +`-account` by `accountFlags`, so every account-scoped verb gets it. It |
| +accepts a sqid only (`release.ValidAccount` shape), never consults the |
| +registry, never matches a name. When both flags are given, `-account-id` |
| +wins — that is its purpose: the disambiguation error tells you to add it, |
| +and adding it must work without editing the rest of the command. Scripts |
| +that must never be surprised use it exclusively. |
| + |
| +## 3. URLs: links are pretty, actions are canonical |
| + |
| +No compound URL form. `/a/amadan-bah` was considered and rejected: hyphen is |
| +legal inside slugs, so the form is ambiguous grammar (an account literally |
| +named `amadan-bah` shadows it), and its only job — being a working link on |
| +the picker page — is already done by `/a/<sqid>`, which exists, is |
| +permanent, and is what the picker links to. A reserved separator |
| +(`amadan.bah`) would be unambiguous but is new grammar with no job. |
| + |
| +**Sqid URLs are permanent and canonical.** Every URL that must keep meaning |
| +the same thing forever — bookmarks that predate this change, links in |
| +invites, form actions — is a sqid URL. Nothing that works today stops |
| +working. |
| + |
| +**Name URLs are the human layer on top**, and the console prefers them |
| +where they are unambiguous *for this viewer*: |
| + |
| +- One helper — preferred segment: the account's name if it is slug-shaped |
| + and resolves uniquely within the viewer's memberships (no other |
| + membership shares the name or holds it as a sqid); otherwise the sqid. |
| +- `/` landing redirects and the account switcher use it — signing in lands |
| + on `/a/amadan/`, organically. |
| +- In-page **navigation links** use it, so the URL bar stays readable while |
| + browsing. |
| +- **Form actions and redirects-after-POST use the sqid path**, always. A |
| + mutation must name its target exactly; prettiness is for links. This is |
| + also why the picker never needs to handle a POST. |
| + |
| +A name URL can degrade to the picker later (you get invited to a colliding |
| +account) — degrade loudly, never resolve differently silently. |
| + |
| +## 4. Testing |
| + |
| +- **Validator**: table-driven — accepts `amadan`, `a`, `kass-2`; rejects |
| + empty, uppercase, `-lead`, `trail-`, 64 chars, `paul@keymail.dev`, inner |
| + spaces, unicode. |
| +- **Console resolution**: name resolves for a member; non-member gets the |
| + byte-identical 404 for a name exactly as for a sqid (bytes compared |
| + against the unknown-route 404, extending the existing test); two |
| + same-named memberships render the picker listing both sqids; name/sqid |
| + collision renders the picker; POST to an ambiguous segment 404s; sqid |
| + paths behave exactly as before throughout. |
| +- **Preferred segment**: address-named account falls back to sqid; unique |
| + slug name is chosen; collision within the viewer's set falls back to |
| + sqid; landing redirect and switcher hrefs agree with it. |
| +- **Auto-mint**: first sign-in still mints the address-named account — |
| + validation does not reach that path. |
| +- **CLI resolver**: existing cases stay green (ops, passthrough sqid, |
| + unique name, ambiguous name lists candidates); new: name/sqid conflict |
| + is fatal and lists both; `-account-id` bypasses name matching entirely |
| + and beats `-account` when both are set. |
| +- **Ladder/production pins** (`laddercheck_test.go`) untouched — nothing |
| + here goes near channels. |
| + |
| +## 5. Rollout |
| + |
| +Code-only; no data changes, no migration, no deploy ordering constraints |
| +beyond the ordinary ship → promote → adopt → restart (console is a |
| +long-running unit and needs the restart to serve name URLs). Existing |
| +sqid links keep working before, during and after. |
| + |
| +## 6. Future work (named, not built) |
| + |
| +- **Display label** (§1) — additive field, defaults to name. |
| +- **Rename** — already named owner-capability in the multi-account spec, |
| + still unimplemented; when built, the rename form applies |
| + `domains.ValidAccountName`, and renames simply change what resolves — |
| + sqid URLs never break, which is the point of keeping them canonical. |
| +- **Slug-shaped names for personal accounts** — derive `paul` from |
| + `paul@keymail.dev` at auto-mint, with a collision fallback. Deferred: it |
| + invents naming policy nobody asked for; rename covers it once built. |
diff --git a/e2e_test.go b/e2e_test.go| index ec7c817f9..d7dcc6277 100644 |
| --- a/e2e_test.go |
| +++ b/e2e_test.go |
| @@ -63,6 +63,23 @@ func e2eGet(t *testing.T, base, host, path string) (int, string) { |
| return resp.StatusCode, string(b) |
| } |
| +// carlosTestEnv is os.Environ() with the operator's real login masked: |
| +// CARLOS_CREDENTIALS points into the test's own tempdir, where no |
| +// credential exists. Every subprocess that runs the carlos binary MUST |
| +// build its env through this. A test that inherits the real |
| +// ~/.carlos/credentials is one `go test ./...` on a logged-in laptop away |
| +// from writing to a production console — which is exactly how 40+ test |
| +// accounts got minted on the flagship (2026-08-04). |
| +// |
| +// Duplicated in scopedkeys_e2e_test.go: this file is package main_test (an |
| +// external test package), that one is package main, and the two cannot |
| +// share an unexported helper across the package boundary. |
| +func carlosTestEnv(t *testing.T, extra ...string) []string { |
| + t.Helper() |
| + env := append(os.Environ(), "CARLOS_CREDENTIALS="+filepath.Join(t.TempDir(), "credentials")) |
| + return append(env, extra...) |
| +} |
| + |
| func e2eShipSite(t *testing.T, store release.Store, version string, files map[string]string) { |
| t.Helper() |
| dir := t.TempDir() |
| @@ -685,7 +702,7 @@ func TestAccountsCLI(t *testing.T) { |
| } |
| storeDir := filepath.Join(tmp, "store") |
| regPath := filepath.Join(tmp, "registry.db") |
| - env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| + env := carlosTestEnv(t, "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| run := func(args ...string) string { |
| t.Helper() |
| cmd := exec.Command(carlosBin, args...) |
| @@ -725,7 +742,7 @@ func TestAccountsCLI(t *testing.T) { |
| // membership is authorization, so the operator could not open either one in |
| // the console. -owner is required, and its absence has to name the flag |
| // rather than print a shrug. |
| - failed := exec.Command(carlosBin, "accounts", "create", "-registry", regPath, "-name", "Kass") |
| + failed := exec.Command(carlosBin, "accounts", "create", "-registry", regPath, "-name", "kass") |
| failed.Env = env |
| refusal, err := failed.CombinedOutput() |
| if err == nil { |
| @@ -742,14 +759,14 @@ func TestAccountsCLI(t *testing.T) { |
| // A typo'd address is refused with the same rule the console's sign-in form |
| // uses — an owner keymail could never authenticate is a door that does not |
| // open either. |
| - failed = exec.Command(carlosBin, "accounts", "create", "-registry", regPath, "-name", "Kass", "-owner", "paul") |
| + failed = exec.Command(carlosBin, "accounts", "create", "-registry", regPath, "-name", "kass", "-owner", "paul") |
| failed.Env = env |
| if refusal, err := failed.CombinedOutput(); err == nil { |
| t.Errorf("accounts create accepted the non-address owner %q:\n%s", "paul", refusal) |
| } |
| // create: a minted id, its frozen sqid printed for the operator to use. |
| - out := run("accounts", "create", "-registry", regPath, "-name", "Kass", "-owner", "Paul@Keymail.dev") |
| + out := run("accounts", "create", "-registry", regPath, "-name", "kass", "-owner", "Paul@Keymail.dev") |
| m := regexp.MustCompile(`(?m)^\s*sqid:\s*(\S+)`).FindStringSubmatch(out) |
| if m == nil { |
| t.Fatalf("accounts create printed no sqid:\n%s", out) |
| @@ -780,7 +797,7 @@ func TestAccountsCLI(t *testing.T) { |
| } |
| if out := run("accounts", "list", "-registry", regPath); !strings.Contains(out, sqid) || |
| - !strings.Contains(out, "Kass") || !strings.Contains(out, "ops") { |
| + !strings.Contains(out, "kass") || !strings.Contains(out, "ops") { |
| t.Fatalf("accounts list = %q, want ops and the new account", out) |
| } |
| @@ -892,7 +909,7 @@ func TestAccountsMigrateTakesTheLiveObjects(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - env := append(os.Environ(), |
| + env := carlosTestEnv(t, |
| "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| "CARLOS_RELEASE_PUBKEY="+pubB64) |
| run := func(args ...string) (string, error) { |
| @@ -964,7 +981,7 @@ func TestAccountsMigrateTakesTheLiveObjects(t *testing.T) { |
| } |
| reg.Close() |
| - out, err := run("accounts", "create", "-registry", regPath, "-name", "Kass", "-owner", "paul@keymail.dev") |
| + out, err := run("accounts", "create", "-registry", regPath, "-name", "kass", "-owner", "paul@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, out) |
| } |
| @@ -1046,7 +1063,7 @@ func TestAccountsMigrateConvergesADivergedPointer(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - env := append(os.Environ(), |
| + env := carlosTestEnv(t, |
| "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| "CARLOS_RELEASE_PUBKEY="+pubB64) |
| run := func(args ...string) (string, error) { |
| @@ -1073,7 +1090,7 @@ func TestAccountsMigrateConvergesADivergedPointer(t *testing.T) { |
| } |
| reg.Close() |
| - out, err := run("accounts", "create", "-registry", regPath, "-name", "Kass", "-owner", "paul@keymail.dev") |
| + out, err := run("accounts", "create", "-registry", regPath, "-name", "kass", "-owner", "paul@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, out) |
| } |
| @@ -1200,6 +1217,90 @@ func TestAddUnitBackingRequiresDBForHibernate(t *testing.T) { |
| } |
| } |
| +// TestAddAccountIDRefusesAName pins the whole -account-id gate end to end, |
| +// on the verb where getting it wrong is quietest. `carlos add -account-id |
| +// ops` used to succeed: "ops" passes release.ValidAccount (^[a-z]{3,16}$), |
| +// so a bare shape check waved it through and stamped Account="ops" onto the |
| +// registry row — a phantom account (the real ops is Sqid(1) = "bab") that |
| +// every subsequent adopt and edge lookup would key off and find nothing |
| +// under. Exit 0, no error, nothing serving. The frozen alphabet has no i, l |
| +// or o, so domains.ValidSqid can refuse it for free; this test is the proof |
| +// the CLI actually does. |
| +// |
| +// The in-process half of this coverage is TestAccountIDShape |
| +// (accounts_resolve_test.go); the refusal itself is log.Fatal, so only a |
| +// real subprocess can observe the exit status and the absent row. |
| +func TestAddAccountIDRefusesAName(t *testing.T) { |
| + goBin, err := exec.LookPath("go") |
| + if err != nil { |
| + t.Skip("go toolchain not found") |
| + } |
| + tmp := t.TempDir() |
| + carlosBin := filepath.Join(tmp, "carlos") |
| + build := exec.Command(goBin, "build", "-o", carlosBin, ".") |
| + build.Env = os.Environ() |
| + if out, err := build.CombinedOutput(); err != nil { |
| + t.Fatalf("build carlos: %v\n%s", err, out) |
| + } |
| + regPath := filepath.Join(tmp, "registry.db") |
| + |
| + out, err := exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass", |
| + "-account-id", "ops", "phantom.example.com").CombinedOutput() |
| + if err == nil { |
| + t.Fatalf("add -account-id ops: want non-zero exit, got 0\n%s", out) |
| + } |
| + // The message must send them to the right flag AND name the sqid they |
| + // actually wanted — "that's not a sqid" alone leaves them guessing. |
| + if !strings.Contains(string(out), "-account ops") || !strings.Contains(string(out), "bab") { |
| + t.Errorf("refusal = %q, want it to name -account ops and the real sqid bab", out) |
| + } |
| + |
| + // A name that is merely sqid-SHAPED is refused the same way when it |
| + // carries an i, l or o — the letters no id can ever encode to. |
| + out, err = exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass", |
| + "-account-id", "oil", "phantom2.example.com").CombinedOutput() |
| + if err == nil { |
| + t.Fatalf("add -account-id oil: want non-zero exit, got 0\n%s", out) |
| + } |
| + if !strings.Contains(string(out), "-account") { |
| + t.Errorf("refusal = %q, want it to point at -account", out) |
| + } |
| + |
| + // A real sqid is accepted and stamped verbatim — never resolved against |
| + // the registry, which is the point of the flag: this one has no accounts |
| + // in it at all. |
| + out, err = exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass", |
| + "-account-id", "bah", "real.example.com").CombinedOutput() |
| + if err != nil { |
| + t.Fatalf("add -account-id bah: %v\n%s", err, out) |
| + } |
| + |
| + reg, err := registry.Open(regPath) |
| + if err != nil { |
| + t.Fatalf("open registry: %v", err) |
| + } |
| + defer reg.Close() |
| + routes, err := reg.List() |
| + if err != nil { |
| + t.Fatalf("list routes: %v", err) |
| + } |
| + var seen int |
| + for _, rt := range routes { |
| + switch rt.Host { |
| + case "phantom.example.com", "phantom2.example.com": |
| + t.Errorf("refused add left a route behind (%s, account %q)", rt.Host, rt.Account) |
| + case "real.example.com": |
| + seen++ |
| + if rt.Account != "bah" { |
| + t.Errorf("route account = %q, want the -account-id sqid %q", rt.Account, "bah") |
| + } |
| + } |
| + } |
| + if seen != 1 { |
| + t.Errorf("found %d rows for real.example.com, want 1", seen) |
| + } |
| +} |
| + |
| // An app still on the pre-accounts keys can be moved INTO ops. Until `-from |
| // legacy` existed there was no way to say it: a bare move into ops is refused |
| // (ops is also the default source, and the verb will not guess a direction), |
| @@ -1235,7 +1336,7 @@ func TestAccountsMigrateLegacyIntoOps(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - env := append(os.Environ(), |
| + env := carlosTestEnv(t, |
| "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| "CARLOS_RELEASE_PUBKEY="+pubB64) |
| run := func(args ...string) (string, error) { |
| @@ -1348,7 +1449,7 @@ func TestEnvCLI(t *testing.T) { |
| } |
| storeDir := filepath.Join(tmp, "store") |
| regPath := filepath.Join(tmp, "registry.db") |
| - env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| + env := carlosTestEnv(t, "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| run := func(args ...string) string { |
| t.Helper() |
| cmd := exec.Command(carlosBin, args...) |
| @@ -1399,7 +1500,7 @@ func TestSecretsCLI(t *testing.T) { |
| } |
| storeDir := filepath.Join(tmp, "store") |
| regPath := filepath.Join(tmp, "registry.db") |
| - baseEnv := os.Environ() |
| + baseEnv := carlosTestEnv(t) |
| run := func(env []string, args ...string) string { |
| t.Helper() |
| cmd := exec.Command(carlosBin, args...) |
| @@ -1477,7 +1578,7 @@ func TestEnvironmentBundlesCLI(t *testing.T) { |
| t.Fatalf("build carlos: %v\n%s", err, out) |
| } |
| storeDir := filepath.Join(tmp, "store") |
| - env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| + env := carlosTestEnv(t, "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| run := func(args ...string) string { |
| t.Helper() |
| cmd := exec.Command(carlosBin, args...) |
| @@ -1624,7 +1725,7 @@ func TestExecDeliveryCLI(t *testing.T) { |
| t.Fatalf("build carlos: %v\n%s", err, out) |
| } |
| storeDir := filepath.Join(tmp, "store") |
| - env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| + env := carlosTestEnv(t, "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| run := func(args ...string) string { |
| t.Helper() |
| cmd := exec.Command(carlosBin, args...) |
| @@ -1730,7 +1831,7 @@ func TestEnvironmentMergeVisibleFromCLI(t *testing.T) { |
| t.Fatalf("build carlos: %v\n%s", err, out) |
| } |
| storeDir := filepath.Join(tmp, "store") |
| - env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| + env := carlosTestEnv(t, "CARLOS_DEPLOYMENT_DIR="+storeDir) |
| run := func(args ...string) { |
| t.Helper() |
| cmd := exec.Command(carlosBin, args...) |
| @@ -1806,7 +1907,7 @@ func TestRouteCLI(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - env := append(os.Environ(), |
| + env := carlosTestEnv(t, |
| "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| "CARLOS_RELEASE_PUBKEY="+pubB64) |
| run := func(args ...string) (string, error) { |
| @@ -2014,7 +2115,7 @@ func TestRouteCLI(t *testing.T) { |
| }) |
| t.Run("-app form optionally filtered by -account", func(t *testing.T) { |
| - accOut, err := run("accounts", "create", "-registry", regPath, "-name", "Second", "-owner", "second@keymail.dev") |
| + accOut, err := run("accounts", "create", "-registry", regPath, "-name", "second", "-owner", "second@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, accOut) |
| } |
| @@ -2124,7 +2225,7 @@ func (f *routeTestFixture) run(env []string, args ...string) (string, error) { |
| } |
| func (f *routeTestFixture) fullEnv() []string { |
| - return append(os.Environ(), |
| + return carlosTestEnv(f.t, |
| "CARLOS_DEPLOYMENT_DIR="+f.storeDir, |
| "CARLOS_RELEASE_PUBKEY="+f.pubB64) |
| } |
| @@ -2218,7 +2319,7 @@ func TestRouteRefusesWithoutPubkey(t *testing.T) { |
| // Mutation-proven gap: flipping the pub==nil branch back to |
| // overridable=true left the whole suite green until this flag was added |
| // here. |
| - env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+f.storeDir) // no CARLOS_RELEASE_PUBKEY |
| + env := carlosTestEnv(f.t, "CARLOS_DEPLOYMENT_DIR="+f.storeDir) // no CARLOS_RELEASE_PUBKEY |
| out, err := f.run(env, "route", "-registry", f.regPath, "-host", "demo.example.com", |
| "-channel", "stable", "-allow-version-change") |
| if err == nil { |
| @@ -2243,7 +2344,7 @@ func TestRouteRefusesWithoutStore(t *testing.T) { |
| f.addRoute(registry.Route{Host: "demo.example.com", Kind: "instance", |
| App: "demo", Channel: "canary/x", Version: "v1", Status: "ready"}) |
| - env := append(os.Environ(), "CARLOS_RELEASE_PUBKEY="+f.pubB64) // no store env at all |
| + env := carlosTestEnv(f.t, "CARLOS_RELEASE_PUBKEY="+f.pubB64) // no store env at all |
| out, err := f.run(env, "route", "-registry", f.regPath, "-host", "demo.example.com", "-channel", "stable") |
| if err == nil { |
| t.Fatalf("route with no deployment store configured was accepted:\n%s", out) |
| @@ -2275,7 +2376,7 @@ func TestRouteRefusesOnBadSignature(t *testing.T) { |
| // TestRouteRefusesWithoutPubkey (re-review NEW 2): a bad signature is a |
| // worse state than an unresolvable target (suspect, not merely unknown), |
| // and must refuse even with the override. |
| - env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+f.storeDir, "CARLOS_RELEASE_PUBKEY="+otherPub) |
| + env := carlosTestEnv(f.t, "CARLOS_DEPLOYMENT_DIR="+f.storeDir, "CARLOS_RELEASE_PUBKEY="+otherPub) |
| out, err := f.run(env, "route", "-registry", f.regPath, "-host", "demo.example.com", |
| "-channel", "stable", "-allow-version-change") |
| if err == nil { |
| @@ -2453,7 +2554,7 @@ func TestRoutePlatformCanaryRefusesMoveBetweenCanaryChannels(t *testing.T) { |
| // all opts into the cross-account sweep Tito's eighteen routes need. |
| func TestRouteAccountDefaultsToOpsAndAllOptsIn(t *testing.T) { |
| f := newRouteTestFixture(t) |
| - accOut, err := f.run(f.fullEnv(), "accounts", "create", "-registry", f.regPath, "-name", "Second", "-owner", "second@keymail.dev") |
| + accOut, err := f.run(f.fullEnv(), "accounts", "create", "-registry", f.regPath, "-name", "second", "-owner", "second@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, accOut) |
| } |
| @@ -2630,7 +2731,7 @@ func TestRouteEnvironmentInvalidNameRefused(t *testing.T) { |
| f.addRoute(registry.Route{Host: "demo.tito.example.com", Kind: "instance", App: "tito", Status: "ready"}) |
| for _, bad := range []string{"Has-Upper", "has_underscore", strings.Repeat("a", 33)} { |
| - out, err := f.run(os.Environ(), "route", "-registry", f.regPath, |
| + out, err := f.run(carlosTestEnv(f.t), "route", "-registry", f.regPath, |
| "-host", "demo.tito.example.com", "-environment", bad) |
| if err == nil { |
| t.Fatalf("invalid environment name %q was accepted:\n%s", bad, out) |
| @@ -2644,7 +2745,7 @@ func TestRouteEnvironmentInvalidNameRefused(t *testing.T) { |
| // keeps this pinned to the reservation: with the reservation removed, the |
| // existence rail would still refuse "default", for an unrelated reason, |
| // and an assertion on err alone would stay green. |
| - out, err := f.run(os.Environ(), "route", "-registry", f.regPath, |
| + out, err := f.run(carlosTestEnv(f.t), "route", "-registry", f.regPath, |
| "-host", "demo.tito.example.com", "-environment", "default") |
| if err == nil { |
| t.Fatalf("-environment default was accepted:\n%s", out) |
| @@ -2662,7 +2763,7 @@ func TestRouteEnvironmentAloneNeedsNoPubkey(t *testing.T) { |
| f.addRoute(registry.Route{Host: "demo.tito.example.com", Kind: "instance", App: "tito", Status: "ready"}) |
| f.saveEnvironment(release.OpsAccount, "tito", "demo") |
| - env := append(os.Environ(), "CARLOS_DEPLOYMENT_DIR="+f.storeDir) // no CARLOS_RELEASE_PUBKEY |
| + env := carlosTestEnv(f.t, "CARLOS_DEPLOYMENT_DIR="+f.storeDir) // no CARLOS_RELEASE_PUBKEY |
| out, err := f.run(env, "route", "-registry", f.regPath, |
| "-host", "demo.tito.example.com", "-environment", "demo") |
| if err != nil { |
| @@ -2704,7 +2805,7 @@ func TestRouteChannelAloneNeverTouchesEnvironment(t *testing.T) { |
| // whose bundle genuinely exists. |
| func TestRouteEnvironmentAppBulkAllOrNothingAcrossAccounts(t *testing.T) { |
| f := newRouteTestFixture(t) |
| - accOut, err := f.run(f.fullEnv(), "accounts", "create", "-registry", f.regPath, "-name", "Second", "-owner", "second@keymail.dev") |
| + accOut, err := f.run(f.fullEnv(), "accounts", "create", "-registry", f.regPath, "-name", "second", "-owner", "second@keymail.dev") |
| if err != nil { |
| t.Fatalf("accounts create: %v\n%s", err, accOut) |
| } |
| @@ -2966,7 +3067,7 @@ func TestAccountsMigrateLegacyKeepsNewerQualifiedPointer(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - env := append(os.Environ(), |
| + env := carlosTestEnv(t, |
| "CARLOS_DEPLOYMENT_DIR="+storeDir, |
| "CARLOS_RELEASE_PUBKEY="+pubB64) |
| run := func(args ...string) (string, error) { |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 9ab77f6da..9d5a4d479 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -32,12 +32,19 @@ import ( |
| // memberships — resolution and membership proof are deliberately the same |
| // lookup, so an unknown account and someone else's account are the same 404. |
| // |
| -// Resolution follows the same doctrine as the CLI's own resolveAccount |
| -// (accounts.go): an exact SQID match wins outright, and only when none |
| -// matches does a NAME match apply, case-insensitively (EqualFold) — and an |
| -// ambiguous name (two of the caller's own accounts sharing it) is refused |
| -// rather than guessed at, because picking the wrong account here would list |
| -// or promote against the wrong tenant. |
| +// Resolution is SYMMETRIC (matchAccounts, resolve.go): the segment matches |
| +// as a sqid or as a name — case-insensitively — with no precedence between |
| +// the two spaces, exactly as the browser's /a/{seg} resolves it. A sqid |
| +// match no longer wins outright over a name match, because precedence is how |
| +// a reference silently changes meaning the day a colliding account appears: |
| +// the caller who typed a name would start acting on someone else's tenant |
| +// without the wording of their command changing at all. Two candidates are |
| +// refused (409) rather than guessed at. |
| +// |
| +// `?exact=sqid` is the transport for the CLI's -account-id flag: it matches |
| +// the segment as a sqid ONLY, so the reference a person reaches for to |
| +// settle an ambiguity can never itself be ambiguous. It never widens access |
| +// — a non-member's probe meets the same 404 either way. |
| func (s *Server) apiAccountApp(w http.ResponseWriter, r *http.Request) (string, *Account, string, bool) { |
| addr, ok := s.cliUser(w, r) |
| if !ok { |
| @@ -50,35 +57,34 @@ func (s *Server) apiAccountApp(w http.ResponseWriter, r *http.Request) (string, |
| jsonError(w, http.StatusInternalServerError, "could not load accounts") |
| return "", nil, "", false |
| } |
| - var acct *Account |
| - for i := range accts { |
| - if accts[i].Sqid == seg { |
| - acct = &accts[i] |
| - break |
| - } |
| - } |
| - if acct == nil { |
| - var matches []*Account |
| + var matches []*Account |
| + if r.URL.Query().Get("exact") == "sqid" { |
| + // -account-id: the reference that never guesses. Match the segment |
| + // as a sqid only — a name, however unique, does not answer. |
| for i := range accts { |
| - if strings.EqualFold(accts[i].Name, seg) { |
| + if accts[i].Sqid == seg { |
| matches = append(matches, &accts[i]) |
| + break |
| } |
| } |
| - switch len(matches) { |
| - case 1: |
| - acct = matches[0] |
| - case 0: |
| - // acct stays nil — the same 404 an unknown or someone-else's |
| - // account gets, below. |
| - default: |
| - sqids := make([]string, len(matches)) |
| - for i, m := range matches { |
| - sqids[i] = m.Sqid |
| - } |
| - jsonError(w, http.StatusConflict, |
| - fmt.Sprintf("account name %q is ambiguous between %s — use a sqid", seg, strings.Join(sqids, ", "))) |
| - return "", nil, "", false |
| + } else { |
| + matches = matchAccounts(accts, seg) |
| + } |
| + var acct *Account |
| + switch len(matches) { |
| + case 1: |
| + acct = matches[0] |
| + case 0: |
| + // acct stays nil — the same 404 an unknown or someone-else's |
| + // account gets, below. |
| + default: |
| + alts := make([]string, len(matches)) |
| + for i, m := range matches { |
| + alts[i] = fmt.Sprintf("%s (%q)", m.Sqid, m.Name) |
| } |
| + jsonError(w, http.StatusConflict, |
| + fmt.Sprintf("account %q is ambiguous — pass -account-id with one of: %s", seg, strings.Join(alts, ", "))) |
| + return "", nil, "", false |
| } |
| if acct == nil { |
| http.NotFound(w, r) |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index 60afc5b60..464f0c472 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -289,13 +289,22 @@ func TestAPIChannelsRefusalMatrix(t *testing.T) { |
| if bodyA != "404 page not found\n" { |
| t.Fatalf("refusal body = %q, want the stdlib http.NotFound output", bodyA) |
| } |
| + |
| + // (c) the same probe carrying ?exact=sqid — the -account-id transport is |
| + // not a second, chattier door: a non-member meets the identical refusal |
| + // whether they name the account exactly or loosely. |
| + statusC, bodyC := getRaw(t, fx.ts.URL+"/api/cli/apps/bab/titogo/channels?exact=sqid", eve) |
| + if statusC != http.StatusNotFound || bodyC != bodyA { |
| + t.Fatalf("exact=sqid member-of-nothing = %d %q, want the identical 404 %q", statusC, bodyC, bodyA) |
| + } |
| } |
| -// TestAPIAccountSqidBeatsName: a SQID match wins outright, even when the |
| -// segment given also happens to equal ANOTHER of the caller's accounts' |
| -// NAME. Picking that other account here would list (or later, act on) the |
| -// wrong tenant. |
| -func TestAPIAccountSqidBeatsName(t *testing.T) { |
| +// TestAPIAccountSqidNameCollisionRefused: resolution is SYMMETRIC — a |
| +// segment matching one account's SQID and another's NAME is ambiguous, not a |
| +// sqid win. Precedence is how a reference silently changes meaning the day a |
| +// colliding account appears; here the API refuses and names the flag that |
| +// settles it, and -account-id (?exact=sqid) then answers unambiguously. |
| +func TestAPIAccountSqidNameCollisionRefused(t *testing.T) { |
| fx := newConsoleFixture(t) |
| now := time.Now() |
| // bac's NAME is literally "bab" — the sqid of the ops account. Alice |
| @@ -309,19 +318,28 @@ func TestAPIAccountSqidBeatsName(t *testing.T) { |
| if _, err := fx.st.CreateAppIn(OpsAccountID, "titogo", "paul@keymail.dev", now); err != nil { |
| t.Fatal(err) |
| } |
| - token := cliToken(t, fx, "alice@keymail.dev", "code-sqid-beats-name") |
| + token := cliToken(t, fx, "alice@keymail.dev", "code-sqid-name-collision") |
| - // "bab" as the account segment must resolve to the SQID match (the ops |
| - // account, which has titogo) and not the NAME match (bac, which doesn't). |
| status, out := getJSON(t, fx.ts.URL+"/api/cli/apps/bab/titogo/channels", token) |
| + if status != http.StatusConflict { |
| + t.Fatalf("collision channels = %d %v, want 409 (bab is both a sqid and a name here)", status, out) |
| + } |
| + msg := str(t, out, "error") |
| + if !strings.Contains(msg, "bab") || !strings.Contains(msg, "bac") || !strings.Contains(msg, "-account-id") { |
| + t.Fatalf("collision error = %q, want both candidates and -account-id named", msg) |
| + } |
| + |
| + // -account-id settles it: as a sqid, "bab" is the ops account alone. |
| + status, out = getJSON(t, fx.ts.URL+"/api/cli/apps/bab/titogo/channels?exact=sqid", token) |
| if status != http.StatusOK { |
| - t.Fatalf("channels = %d %v, want 200 (sqid match, not bac's name match)", status, out) |
| + t.Fatalf("exact=sqid channels = %d %v, want 200 (the ops account, which holds titogo)", status, out) |
| } |
| } |
| // TestAPIAccountAmbiguousNameRefused: two of the caller's own accounts |
| -// sharing a name is refused with 409 and both candidate sqids named, never |
| -// silently resolved to one of them. |
| +// sharing a name is refused with 409 and both candidates named — never |
| +// silently resolved to one of them — and the refusal names -account-id, |
| +// which is the only thing the caller can do about it. |
| func TestAPIAccountAmbiguousNameRefused(t *testing.T) { |
| fx := newConsoleFixture(t) |
| now := time.Now() |
| @@ -342,6 +360,60 @@ func TestAPIAccountAmbiguousNameRefused(t *testing.T) { |
| if !strings.Contains(msg, sqidA) || !strings.Contains(msg, sqidB) { |
| t.Fatalf("ambiguous-name error = %q, want both %q and %q named", msg, sqidA, sqidB) |
| } |
| + if !strings.Contains(msg, "-account-id") { |
| + t.Fatalf("ambiguous-name error = %q, want it to name the -account-id flag", msg) |
| + } |
| +} |
| + |
| +// TestAPIAccountExactSqid: the ?exact=sqid contract the CLI's -account-id |
| +// flag rides on. It matches the segment as a SQID only — a name, however |
| +// unique to this caller, never answers — so the reference a person reaches |
| +// for when the pretty one is ambiguous can never itself become ambiguous. |
| +func TestAPIAccountExactSqid(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + now := time.Now() |
| + sqidA, sqidB := domains.Sqid(2), domains.Sqid(3) |
| + // Two of alice's own accounts named "amadan"; only the first holds web. |
| + if err := fx.st.CreateAccount(2, sqidA, "amadan", "alice@keymail.dev", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := fx.st.CreateAccount(3, sqidB, "amadan", "alice@keymail.dev", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.st.CreateAppIn(2, "web", "alice@keymail.dev", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + token := cliToken(t, fx, "alice@keymail.dev", "code-exact-sqid") |
| + |
| + // The bare name is ambiguous — and says so, in the words a caller can act on. |
| + status, out := getJSON(t, fx.ts.URL+"/api/cli/apps/amadan/web/channels", token) |
| + if status != http.StatusConflict { |
| + t.Fatalf("ambiguous amadan = %d %v, want 409", status, out) |
| + } |
| + msg := str(t, out, "error") |
| + if !strings.Contains(msg, sqidA) || !strings.Contains(msg, sqidB) || !strings.Contains(msg, "-account-id") { |
| + t.Fatalf("ambiguous error = %q, want both sqids and -account-id named", msg) |
| + } |
| + |
| + // The sqid, exactly, resolves to the account that holds the app. |
| + status, out = getJSON(t, fx.ts.URL+"/api/cli/apps/"+sqidA+"/web/channels?exact=sqid", token) |
| + if status != http.StatusOK { |
| + t.Fatalf("exact=sqid %s = %d %v, want 200", sqidA, status, out) |
| + } |
| + |
| + // A NAME under exact=sqid does not match at all — the same 404 an unknown |
| + // account gets, byte-identical to http.NotFound's own output. |
| + st, body := getRaw(t, fx.ts.URL+"/api/cli/apps/amadan/web/channels?exact=sqid", token) |
| + if st != http.StatusNotFound || body != "404 page not found\n" { |
| + t.Fatalf("exact=sqid with a name = %d %q, want the stdlib 404", st, body) |
| + } |
| + |
| + // The caller's OTHER account, named exactly, simply hasn't claimed web: |
| + // the identical 404, so exact mode leaks nothing either. |
| + st, body2 := getRaw(t, fx.ts.URL+"/api/cli/apps/"+sqidB+"/web/channels?exact=sqid", token) |
| + if st != http.StatusNotFound || body2 != body { |
| + t.Fatalf("exact=sqid unclaimed app = %d %q, want the identical 404 %q", st, body2, body) |
| + } |
| } |
| // TestAPIAccountNameMatchIsCaseInsensitive: a unique name match applies |
diff --git a/internal/console/authcli.go b/internal/console/authcli.go| index 122cec00e..60db52c59 100644 |
| --- a/internal/console/authcli.go |
| +++ b/internal/console/authcli.go |
| @@ -33,6 +33,8 @@ import ( |
| "strings" |
| "sync" |
| "time" |
| + |
| + "github.com/carlosframework/platform/internal/domains" |
| ) |
| // Rate limits. Deliberately generous for humans and tight for scripts: the |
| @@ -699,8 +701,9 @@ func (s *Server) handleCLINewAccount(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| name := strings.TrimSpace(req.Name) |
| - if name == "" || len(name) > 64 { |
| - jsonError(w, http.StatusBadRequest, "an account needs a name (up to 64 characters)") |
| + if !domains.ValidAccountName(name) { |
| + jsonError(w, http.StatusBadRequest, |
| + "an account name appears in URLs and -account: 1-63 characters of a-z, 0-9 and hyphen, no leading or trailing hyphen") |
| return |
| } |
| ctx, cancel := context.WithTimeout(r.Context(), 20*time.Second) |
diff --git a/internal/console/authcli_test.go b/internal/console/authcli_test.go| index ee59bc508..b7be257e6 100644 |
| --- a/internal/console/authcli_test.go |
| +++ b/internal/console/authcli_test.go |
| @@ -412,12 +412,12 @@ func TestCLICreatesAccountOwnedByTheToken(t *testing.T) { |
| fx := newConsoleFixture(t) |
| token := cliToken(t, fx, "paul@keymail.dev", "code-acct") |
| - status, out := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": "Kass"}, token) |
| + status, out := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": "kass"}, token) |
| if status != http.StatusOK { |
| t.Fatalf("accounts create = %d %v", status, out) |
| } |
| sqid := str(t, out, "sqid") |
| - if sqid == "" || str(t, out, "name") != "Kass" || str(t, out, "owner") != "paul@keymail.dev" { |
| + if sqid == "" || str(t, out, "name") != "kass" || str(t, out, "owner") != "paul@keymail.dev" { |
| t.Fatalf("accounts create returned %v", out) |
| } |
| acct, err := fx.st.AccountBySqid(sqid) |
| @@ -440,6 +440,12 @@ func TestCLICreatesAccountOwnedByTheToken(t *testing.T) { |
| if status, _ := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": ""}, token); status != http.StatusBadRequest { |
| t.Fatalf("nameless accounts create = %d, want 400", status) |
| } |
| + // A non-slug name is refused the same way: it would appear in URLs and |
| + // -account exactly as typed. |
| + if status, _ := postJSON(t, fx.ts.URL+"/auth/cli/accounts", |
| + map[string]string{"name": "Paul's Stuff!"}, token); status != http.StatusBadRequest { |
| + t.Fatalf("non-slug accounts create = %d, want 400", status) |
| + } |
| } |
| // `carlos auth logout` retires the credential rather than just forgetting it. |
diff --git a/internal/console/authowner_test.go b/internal/console/authowner_test.go| index fc3a489ed..1fa7864ee 100644 |
| --- a/internal/console/authowner_test.go |
| +++ b/internal/console/authowner_test.go |
| @@ -117,7 +117,7 @@ func TestMaterializeRefusesAnUnusableOwner(t *testing.T) { |
| func TestConsoleMintStampsTheOwner(t *testing.T) { |
| fx := newConsoleFixture(t) |
| token := cliToken(t, fx, "paul@keymail.dev", "code-owner") |
| - _, out := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": "Kass"}, token) |
| + _, out := postJSON(t, fx.ts.URL+"/auth/cli/accounts", map[string]string{"name": "kass"}, token) |
| sqid := str(t, out, "sqid") |
| body, _, err := fx.rel.Get(context.Background(), AccountManifestKey(sqid)) |
| if err != nil { |
diff --git a/internal/console/resolve.go b/internal/console/resolve.go| new file mode 100644 |
| index 000000000..55201399a |
| --- /dev/null |
| +++ b/internal/console/resolve.go |
| @@ -0,0 +1,47 @@ |
| +package console |
| + |
| +import ( |
| + "strings" |
| + |
| + "github.com/carlosframework/platform/internal/domains" |
| +) |
| + |
| +// matchAccounts resolves one path/flag segment against the accounts the |
| +// VIEWER belongs to, matching it as a sqid or a name — symmetrically, with |
| +// no precedence between the two spaces. Precedence is how a reference |
| +// silently changes meaning the day a colliding account appears; a multi- |
| +// match is returned whole so the caller can be loud instead (picker page, |
| +// CLI error). Names match case-insensitively; an account whose name equals |
| +// its own sqid still counts once. |
| +// |
| +// The returned pointers address accts, so each match carries whatever |
| +// AccountsFor filled in — the viewer's Role included. |
| +func matchAccounts(accts []Account, seg string) []*Account { |
| + var out []*Account |
| + for i := range accts { |
| + if accts[i].Sqid == seg || strings.EqualFold(accts[i].Name, seg) { |
| + out = append(out, &accts[i]) |
| + } |
| + } |
| + return out |
| +} |
| + |
| +// preferredSeg is the path segment the console GENERATES for an account: |
| +// its name, when the name is slug-shaped and resolves uniquely among the |
| +// viewer's own accounts; the sqid otherwise. Links and redirects use it — |
| +// forms always post to the sqid path (page.Base), because a mutation must |
| +// name its target exactly and prettiness is for links. |
| +func preferredSeg(accts []Account, a *Account) string { |
| + if !domains.ValidAccountName(a.Name) { |
| + return a.Sqid |
| + } |
| + for i := range accts { |
| + if accts[i].ID == a.ID { |
| + continue |
| + } |
| + if strings.EqualFold(accts[i].Name, a.Name) || accts[i].Sqid == a.Name { |
| + return a.Sqid |
| + } |
| + } |
| + return a.Name |
| +} |
diff --git a/internal/console/resolve_test.go b/internal/console/resolve_test.go| new file mode 100644 |
| index 000000000..e1310f995 |
| --- /dev/null |
| +++ b/internal/console/resolve_test.go |
| @@ -0,0 +1,92 @@ |
| +package console |
| + |
| +import "testing" |
| + |
| +func TestMatchAccounts(t *testing.T) { |
| + accts := []Account{ |
| + {ID: 1, Sqid: "bab", Name: "ops"}, |
| + {ID: 2, Sqid: "bah", Name: "amadan"}, |
| + {ID: 3, Sqid: "bad", Name: "amadan"}, |
| + {ID: 4, Sqid: "baf", Name: "bah"}, // name colliding with #2's sqid |
| + {ID: 5, Sqid: "bag", Name: "Mixed"}, |
| + } |
| + cases := []struct { |
| + seg string |
| + want []int64 // matched account IDs, in accts order |
| + }{ |
| + {"bab", []int64{1}}, // sqid only |
| + {"ops", []int64{1}}, // name only |
| + {"amadan", []int64{2, 3}}, // two accounts share the name |
| + {"bah", []int64{2, 4}}, // sqid of one, name of another |
| + {"mixed", []int64{5}}, // names match case-insensitively |
| + {"zzz", nil}, // nothing |
| + } |
| + for _, c := range cases { |
| + got := matchAccounts(accts, c.seg) |
| + var ids []int64 |
| + for _, a := range got { |
| + ids = append(ids, a.ID) |
| + } |
| + if len(ids) != len(c.want) { |
| + t.Errorf("matchAccounts(%q) = %v, want %v", c.seg, ids, c.want) |
| + continue |
| + } |
| + for i := range ids { |
| + if ids[i] != c.want[i] { |
| + t.Errorf("matchAccounts(%q) = %v, want %v", c.seg, ids, c.want) |
| + break |
| + } |
| + } |
| + } |
| +} |
| + |
| +// An account whose NAME is its own sqid is one account, not two matches — |
| +// otherwise the picker would ask a viewer to disambiguate between an account |
| +// and itself. |
| +func TestMatchAccountsCountsSelfCollisionOnce(t *testing.T) { |
| + accts := []Account{{ID: 1, Sqid: "bab", Name: "bab"}} |
| + if got := matchAccounts(accts, "bab"); len(got) != 1 || got[0].ID != 1 { |
| + t.Errorf("matchAccounts(self-collision) = %+v, want the one account", got) |
| + } |
| +} |
| + |
| +// The returned pointers address the CALLER's slice, so a match carries the |
| +// Role AccountsFor filled in — s.account returns it as the viewer's role. |
| +func TestMatchAccountsKeepsRole(t *testing.T) { |
| + accts := []Account{{ID: 7, Sqid: "bab", Name: "ops", Role: RoleOwner}} |
| + got := matchAccounts(accts, "ops") |
| + if len(got) != 1 || got[0].Role != RoleOwner { |
| + t.Errorf("matchAccounts dropped the role: %+v", got) |
| + } |
| +} |
| + |
| +func TestPreferredSeg(t *testing.T) { |
| + accts := []Account{ |
| + {ID: 1, Sqid: "bab", Name: "ops"}, |
| + {ID: 2, Sqid: "bah", Name: "amadan"}, |
| + {ID: 3, Sqid: "bad", Name: "amadan"}, |
| + {ID: 4, Sqid: "baf", Name: "paul@keymail.dev"}, |
| + {ID: 5, Sqid: "bag", Name: "bah"}, // collides with #2's sqid |
| + } |
| + cases := []struct { |
| + id int64 |
| + want string |
| + }{ |
| + {1, "ops"}, // unique slug-shaped name → the name |
| + {2, "bah"}, // name shared with #3 → sqid |
| + {3, "bad"}, // same |
| + {4, "baf"}, // address-named (auto-mint) → sqid |
| + {5, "bag"}, // name equals #2's sqid → sqid |
| + } |
| + for _, c := range cases { |
| + var a *Account |
| + for i := range accts { |
| + if accts[i].ID == c.id { |
| + a = &accts[i] |
| + } |
| + } |
| + if got := preferredSeg(accts, a); got != c.want { |
| + t.Errorf("preferredSeg(#%d) = %q, want %q", c.id, got, c.want) |
| + } |
| + } |
| +} |
diff --git a/internal/console/server.go b/internal/console/server.go| index 5e44aa521..7f65c1a44 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -304,10 +304,16 @@ type page struct { |
| Account *Account // the account being viewed |
| Role string // the viewer's role in it: owner | member |
| Base string // "/a/<sqid>" — what every form on the page posts under |
| + Nav string // "/a/<name-or-sqid>" — what LINKS use; forms keep posting to Base |
| Accounts []Account // the switcher: every account the viewer belongs to |
| Members []Account // account home: memberships (Name carries the address) |
| Invites []Invite // account home, owners only: pending invitations |
| + // The picker: /a/{seg}/ where seg matches more than one of the viewer's |
| + // accounts, as a name, a sqid, or one of each. |
| + PickerSeg string // the ambiguous segment the picker is disambiguating |
| + PickerAccounts []Account // every account of the viewer's that segment matches |
| + |
| Fleet *status.Fleet // /fleet: nil means no snapshot published yet |
| // /fleet/metrics: the operator dashboard (Task 11), both from LoadMetrics |
| @@ -436,7 +442,16 @@ func (s *Server) redirectToCurrentAccount(w http.ResponseWriter, r *http.Request |
| s.renderMessage(w, r, http.StatusInternalServerError, "Error", "Could not open your account. Please try again.") |
| return |
| } |
| - http.Redirect(w, r, "/a/"+url.PathEscape(sqid)+"/", code) |
| + seg := sqid |
| + if accts, err := s.store.AccountsFor(addr); err == nil { |
| + for i := range accts { |
| + if accts[i].Sqid == sqid { |
| + seg = preferredSeg(accts, &accts[i]) |
| + break |
| + } |
| + } |
| + } |
| + http.Redirect(w, r, "/a/"+url.PathEscape(seg)+"/", code) |
| } |
| // currentAccount resolves the three landing cases in order: MRU history (only |
| @@ -497,6 +512,9 @@ func (s *Server) memberAccount(addr string) (*Account, error) { |
| // what stops two boxes minting the same integer at the same moment. A console |
| // row whose manifest never landed would be an account that vanishes on the |
| // next rebuild — worse than a "new account" button that reports the outage. |
| +// |
| +// The name is NOT validated here: auto-mint names personal accounts by |
| +// keymail address, and landing must never refuse to mint (spec §1). |
| func (s *Server) mintAccount(ctx context.Context, name, ownerAddr string) (*Account, error) { |
| now := time.Now() |
| id, sqid, err := domains.MintAccount(ctx, s.cfg.Releases, name, ownerAddr, now) |
| @@ -522,39 +540,54 @@ func (s *Server) mintAccount(ctx context.Context, name, ownerAddr string) (*Acco |
| return &Account{ID: id, Sqid: sqid, Name: name, Role: RoleOwner, CreatedAt: now}, nil |
| } |
| -// account is the gate on every /a/{sqid}/… route: a session, an account that |
| -// exists, and a membership in it. All three failures answer with the |
| -// BYTE-IDENTICAL http.NotFound an unrouted URL gets — account existence is |
| -// not probeable, matching /fleet's operator gate. Signed-out users are sent |
| -// to the front page instead, which reveals nothing either way because it |
| -// happens for every sqid, real or not. |
| +// account is the gate on every /a/{seg}/… route: a session, and a segment |
| +// that resolves — as a sqid OR a name, symmetrically — to an account the |
| +// viewer belongs to. Resolution and membership proof are deliberately the |
| +// same lookup (the membership set IS the resolution scope), so an unknown |
| +// account, someone else's account, and a name that resolves to nothing all |
| +// answer with the BYTE-IDENTICAL http.NotFound an unrouted URL gets — |
| +// account existence is not probeable, matching /fleet's operator gate. |
| +// Several matches (a shared name, or a name colliding with a sqid) render |
| +// the picker on GET and the same 404 otherwise — forms always post to sqid |
| +// paths, so an ambiguous mutation has no legitimate author. |
| +// |
| +// Signed-out users are sent to the front page instead, which reveals nothing |
| +// either way because it happens for every segment, real or not. |
| func (s *Server) account(w http.ResponseWriter, r *http.Request) (addr, raw string, acct *Account, role string, ok bool) { |
| addr, raw, sok := s.session(r) |
| if !sok { |
| http.Redirect(w, r, "/", http.StatusFound) |
| return "", "", nil, "", false |
| } |
| - a, err := s.store.AccountBySqid(r.PathValue("sqid")) |
| + accts, err := s.store.AccountsFor(addr) |
| if err != nil { |
| log.Printf("console: account lookup: %v", err) |
| http.NotFound(w, r) |
| return "", "", nil, "", false |
| } |
| - if a == nil { |
| + seg := r.PathValue("sqid") |
| + matches := matchAccounts(accts, seg) |
| + switch len(matches) { |
| + case 1: |
| + a := matches[0] |
| + return addr, raw, a, a.Role, true |
| + case 0: |
| http.NotFound(w, r) |
| return "", "", nil, "", false |
| - } |
| - role, err = s.store.Role(a.ID, addr) |
| - if err != nil { |
| - log.Printf("console: role lookup: %v", err) |
| - http.NotFound(w, r) |
| - return "", "", nil, "", false |
| - } |
| - if role == "" { |
| - http.NotFound(w, r) |
| + default: |
| + if r.Method != http.MethodGet { |
| + http.NotFound(w, r) |
| + return "", "", nil, "", false |
| + } |
| + p := s.basePage(r) |
| + p.Title = "Which " + seg + "?" |
| + p.PickerSeg = seg |
| + for _, m := range matches { |
| + p.PickerAccounts = append(p.PickerAccounts, *m) |
| + } |
| + s.render(w, http.StatusOK, "picker", p) |
| return "", "", nil, "", false |
| } |
| - return addr, raw, a, role, true |
| } |
| // owner is account plus the owner-only gate. A member who probes an |
| @@ -616,6 +649,9 @@ func (s *Server) handleNewAccountForm(w http.ResponseWriter, r *http.Request) { |
| } else { |
| p.Accounts = accts |
| } |
| + for i := range p.Accounts { |
| + p.Accounts[i].Seg = preferredSeg(p.Accounts, &p.Accounts[i]) |
| + } |
| // The header chrome wants an account to name in the switcher's trigger. |
| // memberAccount, not currentAccount: looking at this page must never mint. |
| // A user with no memberships at all simply gets the wordmark header. |
| @@ -625,6 +661,11 @@ func (s *Server) handleNewAccountForm(w http.ResponseWriter, r *http.Request) { |
| p.Account = a |
| p.Role = a.Role |
| p.Base = "/a/" + url.PathEscape(a.Sqid) |
| + if len(p.Accounts) == 0 { |
| + p.Nav = p.Base |
| + } else { |
| + p.Nav = "/a/" + url.PathEscape(preferredSeg(p.Accounts, a)) |
| + } |
| } |
| s.render(w, http.StatusOK, "newaccount", p) |
| } |
| @@ -641,10 +682,10 @@ func (s *Server) handleNewAccount(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| name := strings.TrimSpace(r.FormValue("name")) |
| - if name == "" || len(name) > 64 { |
| + if !domains.ValidAccountName(name) { |
| // Back to the screen the mistake was made on, not off to some other |
| // account: the form is a page now, and a page can show its own error. |
| - s.setFlash(w, "err", "An account needs a name (up to 64 characters).") |
| + s.setFlash(w, "err", "Account names appear in URLs: 1–63 characters of a-z, 0-9 and hyphen, no leading or trailing hyphen.") |
| http.Redirect(w, r, "/accounts/new", http.StatusSeeOther) |
| return |
| } |
| @@ -1090,6 +1131,10 @@ func (s *Server) accountPage(w http.ResponseWriter, r *http.Request, addr, raw s |
| if len(p.Accounts) == 0 { |
| p.Accounts = []Account{*acct} |
| } |
| + for i := range p.Accounts { |
| + p.Accounts[i].Seg = preferredSeg(p.Accounts, &p.Accounts[i]) |
| + } |
| + p.Nav = "/a/" + url.PathEscape(preferredSeg(p.Accounts, acct)) |
| return p |
| } |
diff --git a/internal/console/server_account_test.go b/internal/console/server_account_test.go| index f0969ea6c..a36eb87ae 100644 |
| --- a/internal/console/server_account_test.go |
| +++ b/internal/console/server_account_test.go |
| @@ -41,6 +41,26 @@ func TestNonMemberGetsIdenticalNotFound(t *testing.T) { |
| } |
| } |
| +// A member can address an account by its name; the page is the same page |
| +// its sqid serves. |
| +func TestAccountNameResolvesForMember(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + id := testAccountID() |
| + sqid := accountSqid(id) |
| + if err := fx.st.CreateAccount(id, sqid, "amadan", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + st, _, byName := get(t, c, fx.ts.URL+"/a/amadan/") |
| + if st != http.StatusOK { |
| + t.Fatalf("GET /a/amadan/ = %d, want 200", st) |
| + } |
| + if !strings.Contains(byName, "amadan") { |
| + t.Fatalf("name-resolved page does not mention the account") |
| + } |
| +} |
| + |
| func TestLandingRedirectsToLastAccount(t *testing.T) { |
| fx := newConsoleFixture(t) |
| c := newBrowser(t) |
| @@ -65,8 +85,160 @@ func TestLandingRedirectsToLastAccount(t *testing.T) { |
| } |
| } |
| +// Landing prefers the readable URL: a viewer whose account has a unique |
| +// slug-shaped name lands on /a/<name>/, and the page's nav links use it, |
| +// while its forms keep posting to the sqid path. |
| +func TestLandingPrefersNameSegment(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + fx.setUser("solo@keymail.dev") |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-solo") |
| + id := testAccountID() |
| + sqid := accountSqid(id) |
| + if err := fx.st.CreateAccount(id, sqid, "amadan", "solo@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st, _, _ := get(t, c, fx.ts.URL+"/a/"+sqid+"/"); st != http.StatusOK { |
| + t.Fatal("visit to set MRU") |
| + } |
| + st, loc, _ := get(t, c, fx.ts.URL+"/") |
| + if st != http.StatusFound || loc != "/a/amadan/" { |
| + t.Fatalf("landing = %d -> %q, want 302 -> /a/amadan/", st, loc) |
| + } |
| + _, _, body := get(t, c, fx.ts.URL+"/a/amadan/") |
| + if !strings.Contains(body, `href="/a/amadan/"`) { |
| + t.Errorf("nav links should use the name segment") |
| + } |
| + if !strings.Contains(body, `action="/a/`+sqid+`/`) { |
| + t.Errorf("form actions must stay on the sqid path") |
| + } |
| +} |
| + |
| +// The switcher's non-current rows must link somewhere real on every page that |
| +// renders it — not just the pages that happen to load Accounts through |
| +// accountPage. /accounts/new fills its own Accounts (for the switcher's |
| +// trigger, not chrome accountPage builds), and it must fill Seg on each row |
| +// too, or the link degenerates to href="/a//" for any account whose name |
| +// isn't its own preferred segment. |
| +func TestSwitcherLinksUsePreferredSegmentsEverywhere(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + fx.setUser("duo@keymail.dev") |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-duo") // mints a personal account named by address (not slug-shaped) |
| + |
| + accts, err := fx.st.AccountsFor("duo@keymail.dev") |
| + if err != nil || len(accts) != 1 { |
| + t.Fatalf("AccountsFor = %+v %v, want one minted account", accts, err) |
| + } |
| + personalSqid := accts[0].Sqid |
| + |
| + id := testAccountID() |
| + sqid := accountSqid(id) |
| + if err := fx.st.CreateAccount(id, sqid, "amadan", "duo@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // On the personal account's own page, "amadan" is the non-current row — |
| + // its href must be the name, not a dead "/a//". |
| + if st, _, _ := get(t, c, fx.ts.URL+"/a/"+personalSqid+"/"); st != http.StatusOK { |
| + t.Fatalf("personal account page = %d", st) |
| + } |
| + _, _, body := get(t, c, fx.ts.URL+"/a/"+personalSqid+"/") |
| + if !strings.Contains(body, `href="/a/amadan/"`) { |
| + t.Errorf("account page switcher: non-current row for the named account has no usable href:\n%s", body) |
| + } |
| + |
| + // /accounts/new builds its own Accounts list (not through accountPage). |
| + // The MRU is still the personal account, so "amadan" is again the |
| + // non-current row there too. |
| + _, _, body = get(t, c, fx.ts.URL+"/accounts/new") |
| + if !strings.Contains(body, `href="/a/amadan/"`) { |
| + t.Errorf("/accounts/new switcher: non-current row for the named account has no usable href:\n%s", body) |
| + } |
| + if strings.Contains(body, `href="/a//"`) { |
| + t.Errorf("/accounts/new switcher rendered a dead href:\n%s", body) |
| + } |
| +} |
| + |
| // A user with no memberships at all gets a personal account minted on the |
| // spot — with a bucket manifest, so every box materializes the same identity. |
| +// A name the viewer cannot resolve — someone ELSE's account — answers with |
| +// the byte-identical 404 an unknown page gets, exactly as sqids do. |
| +func TestAccountNameNotProbeable(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + id := testAccountID() |
| + if err := fx.st.CreateAccount(id, accountSqid(id), "secretco", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + fx.setUser("bob@keymail.dev") |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-bob") |
| + st, _, body := get(t, c, fx.ts.URL+"/a/secretco/") |
| + _, _, ghost := get(t, c, fx.ts.URL+"/definitely-not-a-page") |
| + if st != http.StatusNotFound || body != ghost { |
| + t.Fatalf("non-member name lookup: status %d, body match %v — want the byte-identical 404", st, body == ghost) |
| + } |
| +} |
| + |
| +// Two memberships sharing a name: GET renders the picker naming both sqids; |
| +// a POST to the ambiguous segment is refused with the same 404 — even with a |
| +// good CSRF token and an owner behind it, so the refusal is resolution's and |
| +// not some other gate's. |
| +func TestAmbiguousNameRendersPicker(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + csrf := sessionToken(t, fx, c) |
| + idA, idB := testAccountID(), testAccountID() |
| + sqidA, sqidB := accountSqid(idA), accountSqid(idB) |
| + for _, x := range []struct { |
| + id int64 |
| + sqid string |
| + }{{idA, sqidA}, {idB, sqidB}} { |
| + if err := fx.st.CreateAccount(x.id, x.sqid, "amadan", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + st, _, body := get(t, c, fx.ts.URL+"/a/amadan/") |
| + if st != http.StatusOK { |
| + t.Fatalf("ambiguous GET = %d, want 200 picker", st) |
| + } |
| + if !strings.Contains(body, "/a/"+sqidA+"/") || !strings.Contains(body, "/a/"+sqidB+"/") { |
| + t.Fatalf("picker must link both candidates by sqid:\n%s", body) |
| + } |
| + if st, _, _ := postForm(t, c, fx.ts.URL+"/a/amadan/invite", |
| + url.Values{"csrf": {csrf}, "address": {"x@keymail.dev"}}); st != http.StatusNotFound { |
| + t.Errorf("POST to ambiguous segment = %d, want 404", st) |
| + } |
| + // And the refused POST did nothing: no invite in either account. |
| + for _, id := range []int64{idA, idB} { |
| + if pending, _ := fx.st.PendingInvites(id); len(pending) != 0 { |
| + t.Errorf("ambiguous POST still invited into account %d: %+v", id, pending) |
| + } |
| + } |
| +} |
| + |
| +// A name that equals another membership's SQID is ambiguous the same way — |
| +// symmetric resolution never silently prefers one space over the other. |
| +func TestNameSqidCollisionRendersPicker(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + idA := testAccountID() |
| + sqidA := accountSqid(idA) |
| + if err := fx.st.CreateAccount(idA, sqidA, "amadan", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + idB := testAccountID() |
| + if err := fx.st.CreateAccount(idB, accountSqid(idB), sqidA, "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + st, _, body := get(t, c, fx.ts.URL+"/a/"+sqidA+"/") |
| + if st != http.StatusOK || !strings.Contains(body, "/a/"+accountSqid(idB)+"/") { |
| + t.Fatalf("sqid-vs-name collision: status %d — want the picker listing both", st) |
| + } |
| +} |
| + |
| func TestLandingMintsPersonalAccountWhenNoMemberships(t *testing.T) { |
| fx := newConsoleFixture(t) |
| fx.setUser("newbie@keymail.dev") |
| @@ -342,26 +514,26 @@ func TestNewAccountScreen(t *testing.T) { |
| // Creating from the screen lands you in the account it made. |
| csrf := sessionToken(t, fx, c) |
| st, loc, _ := postForm(t, c, fx.ts.URL+"/accounts/new", |
| - url.Values{"csrf": {csrf}, "name": {"Woodstar"}}) |
| + url.Values{"csrf": {csrf}, "name": {"woodstar"}}) |
| if st != http.StatusSeeOther { |
| t.Fatalf("create = %d -> %q, want 303", st, loc) |
| } |
| accts, _ := fx.st.AccountsFor("paul@keymail.dev") |
| var made *Account |
| for i := range accts { |
| - if accts[i].Name == "Woodstar" { |
| + if accts[i].Name == "woodstar" { |
| made = &accts[i] |
| } |
| } |
| if made == nil { |
| - t.Fatalf("AccountsFor = %+v, want a Woodstar", accts) |
| + t.Fatalf("AccountsFor = %+v, want a woodstar", accts) |
| } |
| if loc != "/a/"+made.Sqid+"/" { |
| t.Fatalf("landed at %q, want /a/%s/", loc, made.Sqid) |
| } |
| // And the account you landed in is the one the header now names. |
| _, _, body = get(t, c, fx.ts.URL+loc) |
| - if h := header(t, body); !strings.Contains(h, "Woodstar") { |
| + if h := header(t, body); !strings.Contains(h, "woodstar") { |
| t.Errorf("the header does not name the account just created:\n%s", h) |
| } |
| } |
| @@ -377,11 +549,39 @@ func TestNewAccountScreenRejectsAnEmptyNameInPlace(t *testing.T) { |
| if st != http.StatusSeeOther || loc != "/accounts/new" { |
| t.Fatalf("empty name = %d -> %q, want 303 -> /accounts/new", st, loc) |
| } |
| - if _, _, body := get(t, c, fx.ts.URL+"/accounts/new"); !strings.Contains(body, "needs a name") { |
| + if _, _, body := get(t, c, fx.ts.URL+"/accounts/new"); !strings.Contains(body, "Account names appear in URLs") { |
| t.Errorf("the screen does not say why it refused:\n%s", body) |
| } |
| } |
| +// The three edges where a human types a name all refuse a non-slug; the |
| +// auto-mint path (address-named personal accounts) must NOT be validated — |
| +// a mint that can refuse strands a signed-in user. |
| +func TestNewAccountNameValidated(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + c := newBrowser(t) |
| + signUp(t, fx, c, "code-1") |
| + csrf := sessionToken(t, fx, c) |
| + before, err := fx.st.AccountsFor("paul@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + st, loc, _ := postForm(t, c, fx.ts.URL+"/accounts/new", url.Values{"csrf": {csrf}, "name": {"Paul's Stuff!"}}) |
| + if st != http.StatusSeeOther || loc != "/accounts/new" { |
| + t.Fatalf("bad name = %d -> %q, want 303 back to the form", st, loc) |
| + } |
| + after, err := fx.st.AccountsFor("paul@keymail.dev") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(after) != len(before) { |
| + t.Fatal("a rejected name must not create an account") |
| + } |
| + if st, _, _ := postForm(t, c, fx.ts.URL+"/accounts/new", url.Values{"csrf": {csrf}, "name": {"amadan"}}); st != http.StatusSeeOther { |
| + t.Fatalf("good name = %d, want 303 into the new account", st) |
| + } |
| +} |
| + |
| // Visiting an account page updates the MRU; visiting /accounts/new does not |
| // — it is not an account you are "in", and moving the MRU there would send |
| // the next sign-in somewhere the user never chose. |
| @@ -503,19 +703,19 @@ func TestCreateAccountFromConsole(t *testing.T) { |
| csrf := sessionToken(t, fx, c) |
| st, loc, _ := postForm(t, c, fx.ts.URL+"/accounts/new", |
| - url.Values{"csrf": {csrf}, "name": {"Kass"}}) |
| + url.Values{"csrf": {csrf}, "name": {"kass"}}) |
| if st != http.StatusSeeOther || !strings.HasPrefix(loc, "/a/") { |
| t.Fatalf("create account = %d -> %q", st, loc) |
| } |
| accts, _ := fx.st.AccountsFor("paul@keymail.dev") |
| var made *Account |
| for i := range accts { |
| - if accts[i].Name == "Kass" { |
| + if accts[i].Name == "kass" { |
| made = &accts[i] |
| } |
| } |
| if made == nil || made.Role != RoleOwner { |
| - t.Fatalf("AccountsFor = %+v, want an owned Kass", accts) |
| + t.Fatalf("AccountsFor = %+v, want an owned kass", accts) |
| } |
| if _, _, err := fx.rel.Get(t.Context(), AccountManifestKey(made.Sqid)); err != nil { |
| t.Errorf("no manifest for the new account: %v", err) |
diff --git a/internal/console/server_test.go b/internal/console/server_test.go| index 74da16298..a2d5efe5a 100644 |
| --- a/internal/console/server_test.go |
| +++ b/internal/console/server_test.go |
| @@ -298,9 +298,11 @@ func TestReturningSignInViaConfirmationLink(t *testing.T) { |
| t.Fatalf("no confirmation link in sealed body: %q", sent.Body) |
| } |
| - // The link mints a session — once. |
| - if status, loc, _ := get(t, c, link); status != http.StatusSeeOther || loc != fx.path("/") { |
| - t.Fatalf("confirm = %d -> %q, want 303 -> /apps", status, loc) |
| + // The link mints a session — once. Landing prefers the readable segment: |
| + // the fixture's only account for paul is named "ops", slug-shaped and |
| + // unique for him, so preferredSeg picks the name over fx.sqid ("bab"). |
| + if status, loc, _ := get(t, c, link); status != http.StatusSeeOther || loc != "/a/ops/" { |
| + t.Fatalf("confirm = %d -> %q, want 303 -> /a/ops/", status, loc) |
| } |
| _, _, body = get(t, c, fx.u("/")) |
| if !strings.Contains(body, "kass") { |
diff --git a/internal/console/store.go b/internal/console/store.go| index 301e154e0..7c96ae37e 100644 |
| --- a/internal/console/store.go |
| +++ b/internal/console/store.go |
| @@ -510,6 +510,10 @@ type Account struct { |
| Name string |
| Role string |
| CreatedAt time.Time |
| + |
| + // Seg is the path segment the console generates for this account in the |
| + // viewer's chrome (preferredSeg) — display state, never stored. |
| + Seg string |
| } |
| // Invite is one pending invitation. |
diff --git a/internal/console/templates/apps.html b/internal/console/templates/apps.html| index ba7f6fd52..3f6c4017e 100644 |
| --- a/internal/console/templates/apps.html |
| +++ b/internal/console/templates/apps.html |
| @@ -9,7 +9,7 @@ |
| <tr><th>App</th><th>Hosts</th><th>Instances</th><th>Status</th><th>Stable</th><th>Canary</th></tr> |
| {{range .Apps}} |
| <tr> |
| - <td><a href="{{$.Base}}/apps/{{.App}}">{{.App}}</a></td> |
| + <td><a href="{{$.Nav}}/apps/{{.App}}">{{.App}}</a></td> |
| <td>{{.Hosts}}</td> |
| <td>{{.Instances}}</td> |
| <td>{{if .Statuses}}{{.Statuses}}{{else}}—{{end}}</td> |
diff --git a/internal/console/templates/layout.html b/internal/console/templates/layout.html| index cf142d4d1..f0a7974eb 100644 |
| --- a/internal/console/templates/layout.html |
| +++ b/internal/console/templates/layout.html |
| @@ -226,7 +226,7 @@ |
| {{range .Accounts}}<li>{{if eq .Sqid $.Account.Sqid}} |
| <span class="current" aria-current="true"><span class="mark">{{icon "check"}}</span>{{.Name}}</span> |
| {{else}} |
| - <a href="/a/{{.Sqid}}/"><span class="mark">{{icon "check"}}</span>{{.Name}}</a> |
| + <a href="/a/{{.Seg}}/"><span class="mark">{{icon "check"}}</span>{{.Name}}</a> |
| {{end}}</li> |
| {{end}} |
| </ul> |
| @@ -238,7 +238,7 @@ |
| <div class="brand"><a href="/">CARLOS console</a></div> |
| {{end}} |
| {{if .Address}}<nav> |
| - {{if .Account}}<a href="{{.Base}}/">Apps</a>{{end}} |
| + {{if .Account}}<a href="{{.Nav}}/">Apps</a>{{end}} |
| <span class="addr">{{.Address}}</span> |
| <form method="post" action="/auth/signout"> |
| <input type="hidden" name="csrf" value="{{.CSRF}}"> |
diff --git a/internal/console/templates/newaccount.html b/internal/console/templates/newaccount.html| index 8bcfe1d3a..ef42103ca 100644 |
| --- a/internal/console/templates/newaccount.html |
| +++ b/internal/console/templates/newaccount.html |
| @@ -7,12 +7,14 @@ and deploys independently of your other accounts.</p> |
| <form class="signin" method="post" action="/accounts/new"> |
| <input type="hidden" name="csrf" value="{{.CSRF}}"> |
| <label class="visually-hidden" for="account-name">Account name</label> |
| - <input id="account-name" type="text" name="name" placeholder="Kass" autofocus> |
| + <input id="account-name" type="text" name="name" placeholder="kass" autofocus |
| + pattern="[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?"> |
| + <p class="small">a-z, 0-9 and hyphens — it appears in your URLs</p> |
| <button type="submit">Create account</button> |
| </form> |
| -<p class="small">Names are free text, up to 64 characters, and deliberately |
| -not unique — two accounts may both be called “Blog”. Identity is the short |
| -id in the URL, so there is nothing to squat. You will be its first owner, |
| -and you can invite teammates from the account's page.</p> |
| -{{if .Account}}<p class="small"><a href="{{.Base}}/">Back to {{.Account.Name}}</a></p>{{end}} |
| +<p class="small">Names are deliberately not unique — two accounts may both be |
| +called “blog”. Identity is the short id in the URL, so there is nothing to |
| +squat. You will be its first owner, and you can invite teammates from the |
| +account's page.</p> |
| +{{if .Account}}<p class="small"><a href="{{.Nav}}/">Back to {{.Account.Name}}</a></p>{{end}} |
| {{template "bottom" .}}{{end}} |
diff --git a/internal/console/templates/picker.html b/internal/console/templates/picker.html| new file mode 100644 |
| index 000000000..5b019687b |
| --- /dev/null |
| +++ b/internal/console/templates/picker.html |
| @@ -0,0 +1,22 @@ |
| +{{define "picker"}}{{template "top" .}} |
| +<h1>Which “{{.PickerSeg}}”?</h1> |
| +<p class="lede">More than one of your accounts answers to |
| +<code>{{.PickerSeg}}</code>. Pick the one you meant — a sqid link is |
| +permanent and never asks this question.</p> |
| +<div class="tablewrap"> |
| +<table> |
| + <tr><th>Name</th><th>Sqid</th><th>Your role</th><th>Created</th></tr> |
| + {{range .PickerAccounts}} |
| + <tr> |
| + <td><a href="/a/{{.Sqid}}/">{{.Name}}</a></td> |
| + <td><code>/a/{{.Sqid}}/</code></td> |
| + <td>{{.Role}}</td> |
| + <td>{{.CreatedAt.Format "2006-01-02"}}</td> |
| + </tr> |
| + {{end}} |
| +</table> |
| +</div> |
| +<p class="small">Account names are deliberately not unique, so a name can mean |
| +more than one thing to you. The short id in the URL is the identity, and it |
| +only ever means one account.</p> |
| +{{template "bottom" .}}{{end}} |
diff --git a/internal/domains/slugname.go b/internal/domains/slugname.go| new file mode 100644 |
| index 000000000..59ea6ec10 |
| --- /dev/null |
| +++ b/internal/domains/slugname.go |
| @@ -0,0 +1,16 @@ |
| +package domains |
| + |
| +import "regexp" |
| + |
| +// accountNameRe pins what an account NAME may be so it can live in a URL |
| +// path segment and a CLI flag: 1–63 characters of lowercase a-z, 0-9 and |
| +// hyphen, no leading or trailing hyphen. Names are still deliberately |
| +// non-unique (identity is the sqid); this constrains shape, not ownership. |
| +var accountNameRe = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`) |
| + |
| +// ValidAccountName reports whether name is usable as an account name. |
| +// Applied at the edges where a human TYPES a name (console new-account |
| +// form, POST /auth/cli/accounts, `carlos accounts create`) — never at |
| +// auto-mint, where the personal account is named by keymail address and a |
| +// refusal would strand a signed-in user. |
| +func ValidAccountName(name string) bool { return accountNameRe.MatchString(name) } |
diff --git a/internal/domains/slugname_test.go b/internal/domains/slugname_test.go| new file mode 100644 |
| index 000000000..bed2b1663 |
| --- /dev/null |
| +++ b/internal/domains/slugname_test.go |
| @@ -0,0 +1,23 @@ |
| +package domains |
| + |
| +import "testing" |
| + |
| +func TestValidAccountName(t *testing.T) { |
| + valid := []string{"amadan", "a", "kass-2", "ops", "a-b-c", "0day", "x9"} |
| + invalid := []string{ |
| + "", "Amadan", "-lead", "trail-", "two words", "paul@keymail.dev", |
| + "dots.no", "unicode-é", "UPPER", |
| + // 64 chars — one past the limit. |
| + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", |
| + } |
| + for _, name := range valid { |
| + if !ValidAccountName(name) { |
| + t.Errorf("ValidAccountName(%q) = false, want true", name) |
| + } |
| + } |
| + for _, name := range invalid { |
| + if ValidAccountName(name) { |
| + t.Errorf("ValidAccountName(%q) = true, want false", name) |
| + } |
| + } |
| +} |
diff --git a/internal/domains/sqid.go b/internal/domains/sqid.go| index 978adda17..c22a7ca95 100644 |
| --- a/internal/domains/sqid.go |
| +++ b/internal/domains/sqid.go |
| @@ -4,6 +4,8 @@ |
| // registry. See docs/superpowers/specs/2026-08-02-custom-domains-design.md. |
| package domains |
| +import "strings" |
| + |
| // sqidAlphabet is FROZEN FOREVER: lowercase base-23 with the lookalikes |
| // i, l, o removed. Every platform domain ever minted embeds a sqid over this |
| // alphabet — changing it (or the offset below) re-homes every one of them. |
| @@ -12,6 +14,12 @@ package domains |
| // platform domain invite squatting; custom domains are the vanity mechanism. |
| const sqidAlphabet = "abcdefghjkmnpqrstuvwxyz" |
| +// SqidAlphabet exports the frozen alphabet for one purpose: an error message |
| +// that tells someone which letters a sqid may contain (the CLI's -account-id |
| +// refusal) should read those letters from the encoder itself, so it can |
| +// never drift from the truth it is describing. |
| +const SqidAlphabet = sqidAlphabet |
| + |
| // sqidOffset guarantees a minimum length of 3: encoding id+23² in plain |
| // base-23 means every id ≥ 1 needs at least three digits, with no padding |
| // case-split (a single strictly-monotonic bijection, so distinct ids can |
| @@ -32,3 +40,31 @@ func Sqid(id int64) string { |
| } |
| return string(buf[i:]) |
| } |
| + |
| +// ValidSqid reports whether s is shaped like something Sqid could have |
| +// emitted: 3 to 16 letters, every one of them in the frozen alphabet. |
| +// |
| +// This is STRICTER than release.ValidAccount (^[a-z]{3,16}$), and the |
| +// difference is the whole point. ValidAccount gates what may become a bucket |
| +// path segment, so it accepts any lowercase word — including account NAMES |
| +// like "ops" or "amadan". A name is not a sqid: the frozen alphabet omits i, |
| +// l and o, so "ops" is a string Sqid can never produce for any id. Anything |
| +// that means "this value IS an account's identity, matched as a sqid and |
| +// never as a name" — the CLI's -account-id, above all — must ask THIS |
| +// question, because accepting "ops" there resolves to an account that does |
| +// not exist, and writes under it without ever failing (the real ops account |
| +// is Sqid(1), "bab"). |
| +// |
| +// The 3 floor is sqidOffset's guarantee and the 16 ceiling is Sqid's buffer; |
| +// both are frozen with the alphabet. |
| +func ValidSqid(s string) bool { |
| + if len(s) < 3 || len(s) > 16 { |
| + return false |
| + } |
| + for i := 0; i < len(s); i++ { |
| + if !strings.ContainsRune(sqidAlphabet, rune(s[i])) { |
| + return false |
| + } |
| + } |
| + return true |
| +} |
diff --git a/internal/domains/sqid_test.go b/internal/domains/sqid_test.go| index 77a4b8f81..78b932514 100644 |
| --- a/internal/domains/sqid_test.go |
| +++ b/internal/domains/sqid_test.go |
| @@ -42,6 +42,62 @@ func TestSqidGolden(t *testing.T) { |
| } |
| } |
| +// TestValidSqid pins the predicate the CLI's -account-id gate rests on. The |
| +// case that matters is the i/l/o rows: release.ValidAccount (^[a-z]{3,16}$) |
| +// accepts every one of them, because it gates bucket path segments and every |
| +// account NAME is a legal segment. ValidSqid must not — "ops" passing here |
| +// is `carlos ship -account-id ops` writing to apps/ops/..., exiting 0, and |
| +// serving nothing, when the real ops account is Sqid(1) = "bab". |
| +func TestValidSqid(t *testing.T) { |
| + for _, tc := range []struct { |
| + s string |
| + want bool |
| + }{ |
| + {"bab", true}, // ops, the built-in account |
| + {"zzz", true}, // never minted here, but shaped like something Sqid emits |
| + {"baf", true}, // |
| + {"abc", true}, // the 3-char floor sqidOffset guarantees |
| + {"abcdefghjkmnpqrs", true}, // 16, Sqid's buffer ceiling |
| + |
| + // The lookalikes the alphabet drops. Each of these passes |
| + // release.ValidAccount and is therefore exactly the bug. |
| + {"ops", false}, // o |
| + {"oil", false}, // o, i, l |
| + {"ill", false}, // i, l |
| + {"kass", true}, // a real account NAME that IS sqid-shaped: see below |
| + {"amadan", true}, |
| + |
| + {"ab", false}, // under the 3 floor |
| + {"abcdefghjkmnpqrst", false}, // over the 16 ceiling |
| + {"", false}, // |
| + {"BAB", false}, // uppercase is not the alphabet |
| + {"ba1", false}, // digits are not the alphabet |
| + {"ba-b", false}, // nor is a hyphen |
| + {"ba b", false}, // nor a space (callers must trim first) |
| + } { |
| + if got := domains.ValidSqid(tc.s); got != tc.want { |
| + t.Errorf("ValidSqid(%q) = %v, want %v", tc.s, got, tc.want) |
| + } |
| + } |
| + |
| + // The honest limit of a shape check, stated so nobody mistakes it for |
| + // more: a NAME made only of alphabet letters ("kass", "amadan") is |
| + // indistinguishable from a sqid by shape alone, and passes. -account-id |
| + // is matched exactly against the account space, so such a value simply |
| + // resolves to nothing rather than to the wrong account — the failure is |
| + // a 404, not a silent write under a phantom account. It is the i/l/o |
| + // names, and only those, that a shape check can catch for free. |
| + |
| + // Every sqid the encoder can actually emit must satisfy the predicate: |
| + // the two are frozen together, and this is the direction that would |
| + // break first if the alphabet or the buffer ever changed. |
| + for id := int64(1); id < 20000; id++ { |
| + if s := domains.Sqid(id); !domains.ValidSqid(s) { |
| + t.Fatalf("Sqid(%d) = %q, which ValidSqid rejects", id, s) |
| + } |
| + } |
| +} |
| + |
| // The frozen alphabet omits i, l and o, so no id encodes to "legacy" — which is |
| // what lets release.LegacySource use that word as a sentinel for the |
| // account-less key space in `carlos accounts migrate -from legacy`. If the |
diff --git a/scopedkeys_e2e_test.go b/scopedkeys_e2e_test.go| index 044b96012..a6cb352ab 100644 |
| --- a/scopedkeys_e2e_test.go |
| +++ b/scopedkeys_e2e_test.go |
| @@ -10,6 +10,24 @@ import ( |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| +// carlosTestEnv is os.Environ() with the operator's real login masked: |
| +// CARLOS_CREDENTIALS points into the test's own tempdir, where no |
| +// credential exists. Every subprocess that runs the carlos binary MUST |
| +// build its env through this. A test that inherits the real |
| +// ~/.carlos/credentials is one `go test ./...` on a logged-in laptop away |
| +// from writing to a production console — which is exactly how 40+ test |
| +// accounts got minted on the flagship (2026-08-04). |
| +// |
| +// Duplicated from e2e_test.go rather than shared: that file is package |
| +// main_test (an external test package with no access to this package's |
| +// unexported symbols), this one is package main, and the two cannot share |
| +// an unexported helper across the package boundary. |
| +func carlosTestEnv(t *testing.T, extra ...string) []string { |
| + t.Helper() |
| + env := append(os.Environ(), "CARLOS_CREDENTIALS="+filepath.Join(t.TempDir(), "credentials")) |
| + return append(env, extra...) |
| +} |
| + |
| // The scoped signing key as an operator meets it: two modes of one verb, and |
| // a promote that signs with whatever CARLOS_RELEASE_KEY holds. Driven through |
| // the real binary, because the whole feature is about which secret is sitting |
| @@ -22,11 +40,10 @@ func TestScopedKeysCLI(t *testing.T) { |
| run := func(env []string, args ...string) (string, error) { |
| t.Helper() |
| cmd := exec.Command(bin, args...) |
| - cmd.Env = append(os.Environ(), |
| + cmd.Env = carlosTestEnv(t, |
| append([]string{ |
| "CARLOS_DEPLOYMENT_DIR=" + bucket, |
| "CARLOS_DATA_DIR=" + t.TempDir(), |
| - "CARLOS_CREDENTIALS=" + filepath.Join(t.TempDir(), "credentials"), |
| }, env...)...) |
| out, err := cmd.CombinedOutput() |
| return string(out), err |