amadan

carlos / platform Public

console: command words refuse whitespace, commas and control characters, and credential ids are unique (PRO-6214)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 58df3a09fa74f6ad7639c44751dcad4a29f32d1d parent bede319
8 files changed, +329 −7
  • apimode.go +3 −0
  • internal/console/apiservices.go +15 −4
  • internal/console/authstore.go +31 −2
  • internal/console/svcprincipal_run_test.go +156 −0
  • internal/runs/commands.go +28 −0
  • internal/runs/commands_test.go +57 −0
  • services.go +4 −1
  • services_cli_test.go +35 −0
diff --git a/apimode.go b/apimode.go
index 621a71dd2..87244c546 100644
--- a/apimode.go
+++ b/apimode.go
@@ -4853,6 +4853,9 @@ type apiServiceView struct {
// Commands is a run credential's granted command prefixes, each a list
// of words; empty for every other role.
Commands [][]string `json:"commands"`
+ // CommandsUnreadable is a run row whose stored list the console cannot
+ // read; it is refused at sign-in.
+ CommandsUnreadable bool `json:"commands_unreadable"`
}
// createService mints one account-owned service credential, optionally
diff --git a/internal/console/apiservices.go b/internal/console/apiservices.go
index 9c9b42c80..80603f5b4 100644
--- a/internal/console/apiservices.go
+++ b/internal/console/apiservices.go
@@ -71,6 +71,10 @@ type serviceView struct {
// of words, in the order minted. An empty array, never null, for every
// other row.
Commands [][]string `json:"commands"`
+ // CommandsUnreadable is true for a run row whose stored command list
+ // cannot be read, which CLIPrincipal refuses at sign-in. Omitted
+ // otherwise. Commands is then [].
+ CommandsUnreadable bool `json:"commands_unreadable,omitempty"`
// Created is when the credential was GRANTED; Rotated is when its
// current secret began. The two answer different questions — how long
// has this existed, and how old is the secret in use right now — and a
@@ -326,6 +330,8 @@ func runMintRefusal(err error) (msg string, ok bool) {
return "a command prefix holds an empty word", true
case errors.Is(err, runs.ErrCommandWordTooLong):
return fmt.Sprintf("a command word is over %d bytes", runs.MaxCommandWordBytes), true
+ case errors.Is(err, runs.ErrCommandWordChars):
+ return "a command word holds a space, a comma or a character that cannot be printed", true
}
return "", false
}
@@ -377,10 +383,15 @@ func (s *Server) handleAPIServiceList(w http.ResponseWriter, r *http.Request) {
App: t.App,
Rungs: rungs,
Commands: commands,
- Created: serviceStamp(t.CreatedAt),
- Rotated: serviceStamp(t.RotatedAt),
- LastUsed: serviceStamp(t.LastUsed),
- MintedBy: t.Address,
+ // Set only for a run row whose stored list cannot be read:
+ // CLIPrincipal refuses it, and an empty list would read as
+ // a credential that may run nothing rather than one that
+ // cannot sign in at all.
+ CommandsUnreadable: t.CommandsUnreadable(),
+ Created: serviceStamp(t.CreatedAt),
+ Rotated: serviceStamp(t.RotatedAt),
+ LastUsed: serviceStamp(t.LastUsed),
+ MintedBy: t.Address,
})
}
writeJSON(w, http.StatusOK, map[string]any{"services": out})
diff --git a/internal/console/authstore.go b/internal/console/authstore.go
index 36d595fdb..942ecbecb 100644
--- a/internal/console/authstore.go
+++ b/internal/console/authstore.go
@@ -147,8 +147,28 @@ func (t CLIToken) CommandPrefixes() [][]string {
return p
}
-// CommandList is Commands for a person to read: "import classic, import sync".
-func (t CLIToken) CommandList() string { return runs.FormatCommandList(t.CommandPrefixes()) }
+// CommandsUnreadable reports a run credential whose stored command list
+// cannot be read. CLIPrincipal refuses such a row, so a listing must say
+// so rather than show it as having no commands.
+func (t CLIToken) CommandsUnreadable() bool {
+ if t.Kind != cliKindService || t.Role != string(svcrole.Run) {
+ return false
+ }
+ _, ok := parseStoredCommands(t.Commands)
+ return !ok
+}
+
+// UnreadableCommands is what a listing shows for CommandsUnreadable.
+const UnreadableCommands = "(unreadable — refused at sign-in)"
+
+// CommandList is Commands for a person to read: "import classic, import
+// sync", or UnreadableCommands for a run row whose list cannot be read.
+func (t CLIToken) CommandList() string {
+ if t.CommandsUnreadable() {
+ return UnreadableCommands
+ }
+ return runs.FormatCommandList(t.CommandPrefixes())
+}
// NeverExpires reports whether this credential has no expiry — true only
// for app and service tokens, whose expires_at is 0 by design (spec §8). The
@@ -254,6 +274,15 @@ func (s *Store) migrateCLIAuth() error {
return err
}
}
+ // credential_id is unique wherever it is set. It is 128 random bits, so
+ // this never fires on a real mint; it makes "one id, one credential"
+ // structural, which is what a run's ownership check (Task 3) leans on.
+ // A person's token keeps '' and any number of those coexist. Like the
+ // index below, it must follow the ALTER loop that adds the column.
+ if _, err := s.db.Exec(`CREATE UNIQUE INDEX IF NOT EXISTS cli_tokens_credential_id
+ON cli_tokens (credential_id) WHERE credential_id != '';`); err != nil {
+ return err
+ }
// This index needs `name`, which the ALTER loop above just added (or
// already had, on a reopened database). It cannot live in the
// CREATE INDEX IF NOT EXISTS block above: that block runs before the
diff --git a/internal/console/svcprincipal_run_test.go b/internal/console/svcprincipal_run_test.go
index ca381dc69..09486f4b4 100644
--- a/internal/console/svcprincipal_run_test.go
+++ b/internal/console/svcprincipal_run_test.go
@@ -414,3 +414,159 @@ func TestFormMintRun(t *testing.T) {
t.Fatalf("the credential listing does not show the commands:\n%s", page)
}
}
+
+// badCommandWords are words neither mint door may store. Through the JSON
+// API each reaches the store as one word; "\xff" arrives as U+FFFD, which
+// is how the decoder replaces invalid UTF-8, and is refused as such.
+var badCommandWords = map[string]string{
+ "a space": "import classic",
+ "a comma": "import,classic",
+ "ESC": "\x1b[8mclassic",
+ "invalid UTF-8": "\xff",
+ "a tab": "import\tclassic",
+}
+
+func TestAPIMintRunRefusesUnprintableWords(t *testing.T) {
+ fx := newConsoleFixture(t)
+ if _, err := fx.st.CreateAppIn(OpsAccountID, "hello", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ token := cliToken(t, fx, "paul@keymail.dev", "code-run-words")
+ u := fx.ts.URL + "/api/cli/services/bab"
+ for why, w := range badCommandWords {
+ status, out := postJSON(t, u, map[string]any{"name": "imp", "role": "run", "app": "hello",
+ "commands": [][]string{{"import", w}}}, token)
+ if status != http.StatusBadRequest || !strings.Contains(str(t, out, "error"), "cannot be printed") {
+ t.Errorf("a word with %s = %d %v; want 400 naming the character rule", why, status, out)
+ }
+ }
+ if rows, err := fx.st.ServicePrincipals(OpsAccountID); err != nil || len(rows) != 0 {
+ t.Fatalf("ServicePrincipals = %d rows %v, want nothing minted", len(rows), err)
+ }
+}
+
+// The form's field is text, where a space, a comma or a tab separates
+// words and prefixes, so those three can never reach a word through it:
+// they split instead (pinned below). ESC, invalid UTF-8 and a zero-width
+// space do reach a word, and are refused before the passkey.
+func TestFormMintRunRefusesUnprintableWords(t *testing.T) {
+ fx := newConsoleFixture(t)
+ c := signInPaul(t, fx)
+ csrf := sessionToken(t, fx, c)
+ for why, field := range map[string]string{
+ "ESC": "import \x1b[8mclassic",
+ "invalid UTF-8": "import \xff",
+ "a zero-width space": "import\u200bclassic",
+ } {
+ status, loc, body := postForm(t, c, fx.u("/services/new"),
+ url.Values{"csrf": {csrf}, "name": {"imp"}, "role": {"run"}, "app": {"kass"}, "commands": {field}})
+ if strings.HasPrefix(loc, fx.fk.srv.URL) {
+ t.Errorf("%s cost a passkey trip (-> %q)", why, loc)
+ }
+ if status != http.StatusSeeOther || loc != fx.path("/settings") {
+ t.Errorf("%s = %d -> %q, want 303 back to the page:\n%s", why, status, loc, body)
+ continue
+ }
+ _, _, page := get(t, c, fx.u("/settings"))
+ if !strings.Contains(page, "A command word holds a space, a comma or a character that cannot be printed. Nothing was minted.") {
+ t.Errorf("%s: the settings page does not give the character rule:\n%s", why, page)
+ }
+ }
+ if rows, err := fx.st.ServicePrincipals(OpsAccountID); err != nil || len(rows) != 0 {
+ t.Fatalf("ServicePrincipals = %d rows %v, want nothing minted", len(rows), err)
+ }
+ // A space, comma or tab in the field separates; it never lands in a word.
+ status, _, body := postGated(t, fx, c, fx.u("/services/new"),
+ url.Values{"csrf": {csrf}, "name": {"imp"}, "role": {"run"}, "app": {"kass"}, "commands": {"import\tclassic,import sync"}})
+ if status != http.StatusOK {
+ t.Fatalf("run mint = %d:\n%s", status, body)
+ }
+ p, ok := fx.st.CLIPrincipal(paneSecret(t, body), time.Now())
+ if !ok || !reflect.DeepEqual(p.Commands, [][]string{{"import", "classic"}, {"import", "sync"}}) {
+ t.Fatalf("minted credential resolves to %+v, %v", p, ok)
+ }
+}
+
+// TestStoredUnprintableWordIsRefusedAtSignIn: the read side runs the same
+// word check, so a row holding such a word never resolves.
+func TestStoredUnprintableWordIsRefusedAtSignIn(t *testing.T) {
+ st := newDeployStore(t)
+ now := time.Now()
+ raw := mintRun(t, st, "imp", importClassic, now)
+ if _, err := st.db.Exec(`UPDATE cli_tokens SET commands = ? WHERE token_hash = ?`, `[["import","a\u001bb"]]`, hashToken(raw)); err != nil {
+ t.Fatal(err)
+ }
+ if p, ok := st.CLIPrincipal(raw, now); ok {
+ t.Fatalf("a stored word with ESC resolved to %+v; want refused", p)
+ }
+}
+
+func TestCredentialIDIsUnique(t *testing.T) {
+ st := newDeployStore(t)
+ now := time.Now()
+ a := mintRun(t, st, "a", importClassic, now)
+ b, err := st.NewServicePrincipalScoped(1, "b", "hello", svcrole.Publish, nil, nil, "paul@example.test", now)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := st.db.Exec(`UPDATE cli_tokens SET credential_id = ? WHERE token_hash = ?`,
+ credentialIDOf(t, st, a), hashToken(b)); err == nil {
+ t.Fatal("two rows were given one credential id; want the unique index to refuse it")
+ }
+ // Any number of people's tokens keep the empty id side by side.
+ for i := 0; i < 3; i++ {
+ mintUserToken(t, st, "paul@example.test", now)
+ }
+ var empty int
+ if err := st.db.QueryRow(`SELECT COUNT(*) FROM cli_tokens WHERE credential_id = ''`).Scan(&empty); err != nil || empty != 3 {
+ t.Fatalf("rows with an empty id = %d, %v; want 3", empty, err)
+ }
+}
+
+// TestUnreadableRunCommandsAreSaidSo: a run row whose stored list cannot
+// be read is refused at sign-in, so the listing says that rather than
+// showing it as a credential with no commands.
+func TestUnreadableRunCommandsAreSaidSo(t *testing.T) {
+ fx := newConsoleFixture(t)
+ if _, err := fx.st.CreateAppIn(OpsAccountID, "hello", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.NewServicePrincipalScoped(OpsAccountID, "mangled", "hello", svcrole.Run, nil, importClassic, "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.NewServicePrincipalScoped(OpsAccountID, "pub", "hello", svcrole.Publish, nil, nil, "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.db.Exec(`UPDATE cli_tokens SET commands = '[[]]' WHERE name = 'mangled'`); err != nil {
+ t.Fatal(err)
+ }
+ token := cliToken(t, fx, "paul@keymail.dev", "code-run-unreadable")
+ status, out := getJSON(t, fx.ts.URL+"/api/cli/services/bab", token)
+ if status != http.StatusOK {
+ t.Fatalf("list = %d %v", status, out)
+ }
+ for _, r := range out["services"].([]any) {
+ row := r.(map[string]any)
+ switch row["name"] {
+ case "mangled":
+ if row["commands_unreadable"] != true {
+ t.Errorf("mangled row = %v; want commands_unreadable true", row)
+ }
+ if c, ok := row["commands"].([]any); !ok || len(c) != 0 {
+ t.Errorf("mangled commands = %#v; want []", row["commands"])
+ }
+ case "pub":
+ if _, ok := row["commands_unreadable"]; ok {
+ t.Errorf("pub row carries commands_unreadable: %v", row)
+ }
+ }
+ }
+ c := signInPaul(t, fx)
+ _, _, page := get(t, c, fx.u("/settings"))
+ if !strings.Contains(page, "(unreadable — refused at sign-in)") {
+ t.Fatalf("the settings listing does not say the commands are unreadable:\n%s", page)
+ }
+ if n := strings.Count(page, "unreadable — refused"); n != 1 {
+ t.Fatalf("the unreadable note appears %d times, want once (only the run row)", n)
+ }
+}
diff --git a/internal/runs/commands.go b/internal/runs/commands.go
index 6ba9d272e..a9941da2f 100644
--- a/internal/runs/commands.go
+++ b/internal/runs/commands.go
@@ -3,6 +3,8 @@ package runs
import (
"errors"
"strings"
+ "unicode"
+ "unicode/utf8"
)
// A `run` service credential carries the command prefixes it may run
@@ -21,12 +23,19 @@ var (
ErrEmptyCommand = errors.New("runs: a command prefix has no words")
ErrEmptyCommandWord = errors.New("runs: a command prefix holds an empty word")
ErrCommandWordTooLong = errors.New("runs: a command word over 128 bytes")
+ ErrCommandWordChars = errors.New("runs: a command word holds a space, a comma or a character that cannot be printed")
)
// CheckCommandPrefixes is the shape every stored command list has: at least
// one prefix and at most MaxCommandPrefixes, each with at least one word,
// each word 1 to MaxCommandWordBytes bytes. A zero-word prefix is refused
// because it would match every argv.
+//
+// A word is also valid UTF-8 made only of printable characters, with no
+// whitespace and no comma (validCommandWord). Whitespace and commas are the
+// text form's separators, so a word holding one would come back as a
+// different grant when a listing is copied into a new mint; a control
+// character could hide a prefix in a terminal listing.
func CheckCommandPrefixes(prefixes [][]string) error {
if len(prefixes) == 0 {
return ErrNoCommands
@@ -45,11 +54,30 @@ func CheckCommandPrefixes(prefixes [][]string) error {
if len(w) > MaxCommandWordBytes {
return ErrCommandWordTooLong
}
+ if !validCommandWord(w) {
+ return ErrCommandWordChars
+ }
}
}
return nil
}
+// validCommandWord: valid UTF-8, every rune printable, none a space or a
+// comma. U+FFFD is refused too: it is what invalid UTF-8 becomes once a
+// JSON decoder has replaced it, so refusing it makes a mangled word fail
+// the same way at the API door as at the form.
+func validCommandWord(w string) bool {
+ if !utf8.ValidString(w) {
+ return false
+ }
+ for _, r := range w {
+ if r == ',' || r == utf8.RuneError || unicode.IsSpace(r) || !unicode.IsPrint(r) {
+ return false
+ }
+ }
+ return true
+}
+
// CommandGranted reports whether argv starts with every word of at least one
// prefix, compared word for word and exactly: no globbing, no case folding,
// no substring. It fails closed: a zero-word prefix matches nothing, and so
diff --git a/internal/runs/commands_test.go b/internal/runs/commands_test.go
index 7621cfde9..1a99c10e4 100644
--- a/internal/runs/commands_test.go
+++ b/internal/runs/commands_test.go
@@ -62,6 +62,16 @@ func TestCheckCommandPrefixes(t *testing.T) {
"129-byte word": {{"import", word128 + "a"}},
"33 prefixes": many(MaxCommandPrefixes + 1),
"zero-word prefix only": {{}},
+ "a word with a space": {{"import classic"}},
+ "a word with a comma": {{"import,classic"}},
+ "a word with a tab": {{"import\tclassic"}},
+ "a word with ESC": {{"import", "\x1b[8mclassic"}},
+ "a word with NUL": {{"import", "a\x00"}},
+ "invalid UTF-8": {{"import", "\xff"}},
+ "U+FFFD": {{"import", "\ufffd"}},
+ "a no-break space": {{"import\u00a0classic"}},
+ "a zero-width space": {{"import\u200bclassic"}},
+ "a newline": {{"import\n"}},
} {
if err := CheckCommandPrefixes(bad); err == nil {
t.Errorf("%s: accepted; want refused", name)
@@ -73,6 +83,12 @@ func TestCheckCommandPrefixes(t *testing.T) {
if err := CheckCommandPrefixes([][]string{{"import", "classic"}, {"import", "sync"}}); err != nil {
t.Errorf("two prefixes: %v", err)
}
+ if err := CheckCommandPrefixes([][]string{{"import", "--slug=café", "über/ü", "a:b;c"}}); err != nil {
+ t.Errorf("printable non-ASCII and punctuation: %v; want accepted", err)
+ }
+ if err := CheckCommandPrefixes([][]string{{"import", "\x1b"}}); !errors.Is(err, ErrCommandWordChars) {
+ t.Errorf("ESC: err = %v, want ErrCommandWordChars", err)
+ }
}
// TestParseCommandList is the one text form both the CLI flag and the
@@ -97,3 +113,44 @@ func TestParseCommandList(t *testing.T) {
t.Errorf("FormatCommandList = %q", got)
}
}
+
+// TestCommandListRoundTrips: every accepted list survives being written out
+// as text and read back, so a listing copied into a new mint grants exactly
+// what it showed.
+func TestCommandListRoundTrips(t *testing.T) {
+ word128 := strings.Repeat("ü", MaxCommandWordBytes/2)
+ max := make([][]string, MaxCommandPrefixes)
+ for i := range max {
+ max[i] = []string{word128, "x"}
+ }
+ for _, p := range [][][]string{
+ {{"import"}},
+ {{"import", "classic"}, {"import", "sync", "--all"}},
+ {{"--slug=café"}, {"a:b;c", "[x]", "\"q\""}},
+ max,
+ } {
+ if err := CheckCommandPrefixes(p); err != nil {
+ t.Fatalf("fixture %q is not accepted: %v", p, err)
+ }
+ if got := ParseCommandList(FormatCommandList(p)); !reflect.DeepEqual(got, p) {
+ t.Errorf("round trip of %q gave %q", p, got)
+ }
+ }
+}
+
+// FuzzCommandListRoundTrip: whatever text is typed, if the list it parses
+// to is accepted, formatting and parsing it again gives the same list.
+func FuzzCommandListRoundTrip(f *testing.F) {
+ for _, s := range []string{"import classic,import sync", " a ,, b c ", "a\u00a0b", "x\x1b", "\xff", "café, über"} {
+ f.Add(s)
+ }
+ f.Fuzz(func(t *testing.T, s string) {
+ p := ParseCommandList(s)
+ if CheckCommandPrefixes(p) != nil {
+ return
+ }
+ if got := ParseCommandList(FormatCommandList(p)); !reflect.DeepEqual(got, p) {
+ t.Fatalf("round trip of %q (from %q) gave %q", p, s, got)
+ }
+ })
+}
diff --git a/services.go b/services.go
index 446a8973e..d87147027 100644
--- a/services.go
+++ b/services.go
@@ -342,7 +342,10 @@ func runServicesList(args []string) {
commands[i] = strings.Join(c, " ")
}
cmds := strings.Join(commands, ",")
- if cmds == "" {
+ switch {
+ case r.CommandsUnreadable:
+ cmds = "(unreadable — refused at sign-in)"
+ case cmds == "":
cmds = "-"
}
created, rotated, lastUsed := r.Created, r.Rotated, r.LastUsed
diff --git a/services_cli_test.go b/services_cli_test.go
index 2b681b31e..5017d608b 100644
--- a/services_cli_test.go
+++ b/services_cli_test.go
@@ -311,6 +311,12 @@ func newFakeServicesConsole(t *testing.T) *fakeServicesConsole {
{"id": strings.Repeat("b", 64), "name": "", "role": "instance", "app": "router",
"label": "router", "created": "2026-08-01T00:00:00Z", "rotated": "",
"last_used": "", "minted_by": "paul@keymail.dev"},
+ {"id": strings.Repeat("c", 64), "name": "importer", "role": "run", "app": "titogo",
+ "commands": [][]string{{"import", "classic"}, {"import", "sync"}},
+ "created": "2026-10-24T00:00:00Z", "minted_by": "paul@keymail.dev"},
+ {"id": strings.Repeat("d", 64), "name": "mangled", "role": "run", "app": "titogo",
+ "commands": [][]string{}, "commands_unreadable": true,
+ "created": "2026-10-24T00:00:00Z", "minted_by": "paul@keymail.dev"},
},
})
})
@@ -939,3 +945,32 @@ func TestServicesCreatePrintsTheCommandsForARunCredential(t *testing.T) {
t.Fatalf("first line = %q, want %q", first, want)
}
}
+
+// TestRunServicesListShowsCommands: a run row's commands print in the
+// --commands form, so they can be copied into another mint, and a run row
+// whose stored list the console cannot read says so instead of "-".
+func TestRunServicesListShowsCommands(t *testing.T) {
+ servicesCLIFixture(t)
+ buf := captureStdout(t)
+ RunServices([]string{"ls", "-account", "bab"})
+ out := buf.String()
+ for _, line := range strings.Split(out, "\n") {
+ switch {
+ case strings.HasPrefix(line, "importer "):
+ if !strings.Contains(line, "import classic,import sync") {
+ t.Errorf("importer row does not show its commands: %q", line)
+ }
+ case strings.HasPrefix(line, "mangled "):
+ if !strings.Contains(line, "(unreadable — refused at sign-in)") {
+ t.Errorf("mangled row does not say its commands are unreadable: %q", line)
+ }
+ case strings.HasPrefix(line, "amadan-hub "):
+ if strings.Contains(line, "unreadable") {
+ t.Errorf("a publish row says unreadable: %q", line)
+ }
+ }
+ }
+ if !strings.Contains(out, "importer ") || !strings.Contains(out, "mangled ") {
+ t.Fatalf("services ls is missing a run row:\n%s", out)
+ }
+}