amadan
cli: -account-id must be a SQID, not merely lowercase — "ops" was a phantom account
release.ValidAccount is ^[a-z]{3,16}$: it gates bucket path segments, so it
accepts every account NAME. The frozen sqid alphabet has no i, l or o. Under
the previous commit that gap was a silent write to nowhere — `carlos ship
-account-id ops` passed the shape check, wrote apps/ops/..., exited 0, and
served nothing, because the real ops account is Sqid(1) = "bab". `carlos add
-account-id ops` stamped the same phantom onto a registry row, where every
later adopt and edge lookup would key off it and find nothing.
domains.ValidSqid is the strict predicate, next to the frozen encoder it
describes (3-16 letters, all in the alphabet), with SqidAlphabet exported so
the CLI's "these letters only" message cannot drift from the truth.
release.ValidAccount is untouched — it answers a different question.
checkAccountID is the one gate both modes now share, so direct-bucket and API
mode cannot disagree about what a sqid is. It special-cases the trap by name:
"ops" is answered with `use -account ops, or -account-id bab` rather than a
bare "not a sqid" that leaves someone to go look the sqid up.
pickAccount's case-0 passthrough is deliberately unchanged: it reads -account,
where name-vs-sqid ambiguity is already settled by the candidate pool.
Also from the same review: -account-id reaches the hand-written usage strings
(runRoute's says in words that it supersedes "all"); apiAccount is called from
runShip rather than inside runShipAPI, which exists precisely so nothing
fatals past its 256MiB scratch-file defer; apiAccount trims -account the way
resolveAccount does, so `-account " ops "` agrees across modes; and the flag
help no longer reads as a cut-off sentence.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGdTfzwT492ysLPDaWfpvJ
8 files changed,
+299
−39
accounts.go+30 −4accounts_resolve_test.go+50 −0apimode.go+7 −8apimode_test.go+5 −5cli.go+31 −22e2e_test.go+84 −0internal/domains/sqid.go+36 −0internal/domains/sqid_test.go+56 −0
diff --git a/accounts.go b/accounts.go| index d01afcfcf..2eb55ba8b 100644 |
| --- a/accounts.go |
| +++ b/accounts.go |
| @@ -654,6 +654,35 @@ func pickAccount(accts []registry.Account, want string) (string, error) { |
| } |
| } |
| +// 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 |
| @@ -662,10 +691,7 @@ func pickAccount(accts []registry.Account, want string) (string, error) { |
| // rather than guessed: picking the wrong account here would ship to the |
| // wrong tenant. |
| 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) |
| - } |
| + if wantID = checkAccountID(wantID); wantID != "" { |
| return wantID |
| } |
| want = strings.TrimSpace(want) |
diff --git a/accounts_resolve_test.go b/accounts_resolve_test.go| index d11c728df..4464126ab 100644 |
| --- a/accounts_resolve_test.go |
| +++ b/accounts_resolve_test.go |
| @@ -4,7 +4,9 @@ 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) { |
| @@ -45,6 +47,43 @@ func TestPickAccount(t *testing.T) { |
| } |
| } |
| +// 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) |
| @@ -56,6 +95,17 @@ func TestApiAccountExact(t *testing.T) { |
| 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) |
| } |
diff --git a/apimode.go b/apimode.go| index c75aaea4f..f50e08cd7 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -22,7 +22,6 @@ import ( |
| "errors" |
| "fmt" |
| "io" |
| - "log" |
| "mime/multipart" |
| "net/http" |
| "net/url" |
| @@ -33,7 +32,6 @@ import ( |
| "time" |
| "github.com/carlosframework/platform/internal/ledger" |
| - "github.com/carlosframework/platform/internal/release" |
| ) |
| // apiSession is the resolved API mode: which console, as whom. console and |
| @@ -179,15 +177,16 @@ type apiAcct struct { |
| // 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. |
| +// 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 := 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) |
| - } |
| + if id := checkAccountID(*rawID); id != "" { |
| return apiAcct{Seg: id, Exact: true} |
| } |
| - return apiAcct{Seg: *raw} |
| + return apiAcct{Seg: strings.TrimSpace(*raw)} |
| } |
| // apiPath builds one /api/cli/apps/{account}/{app}/... path. The segment is |
diff --git a/apimode_test.go b/apimode_test.go| index de554ea5d..6365f8fe8 100644 |
| --- a/apimode_test.go |
| +++ b/apimode_test.go |
| @@ -1082,7 +1082,7 @@ func TestShipAPIStaticNamesSiteTarGz(t *testing.T) { |
| sess := newTestSession(ts.URL) |
| fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, siteDir) |
| - if err := runShipAPI(sess, acctRaw, acctIDRaw, fs, "titesite", "abc1234", "static", ""); err != nil { |
| + if err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titesite", "abc1234", "static", ""); err != nil { |
| t.Fatalf("runShipAPI: %v", err) |
| } |
| @@ -1129,7 +1129,7 @@ func TestShipAPIStaticCleansUpTempFileOnServerError(t *testing.T) { |
| sess := newTestSession(ts.URL) |
| fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, siteDir) |
| - if err := runShipAPI(sess, acctRaw, acctIDRaw, fs, "titesite", "abc1234", "static", ""); err == nil { |
| + if err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titesite", "abc1234", "static", ""); err == nil { |
| t.Fatal("runShipAPI against a failing server must error") |
| } |
| @@ -1195,7 +1195,7 @@ func TestShipAPIMultiFileGoodBasenames(t *testing.T) { |
| sess := newTestSession(ts.URL) |
| fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, fileArm, fileAmd) |
| - if err := runShipAPI(sess, acctRaw, acctIDRaw, fs, "titogo", "abc1234", "binary", "linux-arm64"); err != nil { |
| + if err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titogo", "abc1234", "binary", "linux-arm64"); err != nil { |
| t.Fatalf("runShipAPI: %v", err) |
| } |
| @@ -1237,7 +1237,7 @@ func TestShipAPIMultiFileBadBasenameRefused(t *testing.T) { |
| sess := newTestSession(ts.URL) |
| fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, fileGood, fileBad) |
| - err := runShipAPI(sess, acctRaw, acctIDRaw, fs, "titogo", "abc1234", "binary", "linux-arm64") |
| + err := runShipAPI(sess, apiAccount(acctRaw, acctIDRaw), fs, "titogo", "abc1234", "binary", "linux-arm64") |
| if err == nil { |
| t.Fatal("runShipAPI with a bad basename must error") |
| } |
| @@ -1272,7 +1272,7 @@ func TestShipAPIMissingArtifactRefusedBeforeDialing(t *testing.T) { |
| sess := newTestSession(ts.URL) |
| fs, acctRaw, acctIDRaw := newShipFlagSet(t, apiTestAccount, missing) |
| - err := runShipAPI(sess, acctRaw, acctIDRaw, fs, "titogo", "abc1234", "binary", "linux-arm64") |
| + 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") |
| } |
diff --git a/cli.go b/cli.go| index ff004dd54..a95989041 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -277,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) |
| } |
| @@ -749,7 +750,7 @@ func routeVersionCheck(ctx context.Context, store release.Store, pub ed25519.Pub |
| // fs.Lookup("account") would be. |
| 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") |
| + 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, acctID, func() string { return resolveAccount(*regPath, *acct, *acctID) } |
| } |
| @@ -833,7 +834,7 @@ func runShip(args []string) { |
| 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) |
| @@ -841,7 +842,13 @@ func runShip(args []string) { |
| log.Fatal(err) |
| } |
| if sess != nil { |
| - if err := runShipAPI(sess, acctRaw, acctIDRaw, 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 |
| @@ -902,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), |
| @@ -916,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, acctIDRaw *string, fs *flag.FlagSet, app, ver, kind, target string) error { |
| - acct := apiAccount(acctRaw, acctIDRaw) |
| +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 { |
| @@ -998,7 +1007,7 @@ func runPromote(args []string) { |
| 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) |
| @@ -1066,7 +1075,7 @@ func runChannels(args []string) { |
| 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) |
| @@ -1122,7 +1131,7 @@ func runRollback(args []string) { |
| 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) |
| @@ -1154,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 |
| @@ -1172,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) |
| @@ -1262,7 +1271,7 @@ func runLedgerAppend(args []string) { |
| 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) { |
| @@ -1306,7 +1315,7 @@ func runLedgerHead(args []string) { |
| 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) { |
| @@ -1332,7 +1341,7 @@ func runLedgerList(args []string) { |
| 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) |
| @@ -1382,7 +1391,7 @@ func runLedgerPublish(args []string) { |
| 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 { |
| @@ -1422,7 +1431,7 @@ func runLedgerBlob(args []string) { |
| 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) { |
diff --git a/e2e_test.go b/e2e_test.go| index ec7c817f9..a0b65ab73 100644 |
| --- a/e2e_test.go |
| +++ b/e2e_test.go |
| @@ -1200,6 +1200,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), |
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 |