cmd/console: refuse a CARLOS_SIDECAR_ACCOUNTS that names no sqid
Review P2: "," or " , " parsed to nil and silently turned the sidecar pause off. Only unset or the empty string is now no gate; any other value that yields zero sqids stops serve at startup like a bad sqid. The parse test's "," case is now a refusal and the binary startup test covers it. The refusal sentence's marker records Paul's copy approval; the three approved texts are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 files changed,
+61
−43
cmd/console/main.go+16 −7cmd/console/sidecaraccounts_serve_test.go+38 −30cmd/console/sidecaraccounts_test.go+6 −5internal/console/sidecarsgate.go+1 −1
diff --git a/cmd/console/main.go b/cmd/console/main.go| index 1e53ead69..067f138c9 100644 |
| --- a/cmd/console/main.go |
| +++ b/cmd/console/main.go |
| @@ -56,8 +56,8 @@ |
| // creating one answer 403 sidecars-paused, while list, get, wake and delete |
| // keep working and existing sidecars keep running. Native sidecars run on |
| // the flat tier, outside the nested tenants' isolation, until discussion |
| -// #370 lands. Unset or empty is no gate. An entry that is not a sqid stops |
| -// the console at startup. |
| +// #370 lands. Unset or empty is no gate. An entry that is not a sqid, or a |
| +// value that names no sqid at all, stops the console at startup. |
| // |
| // Tenant email (task 17) reads CARLOS_MAIL_OPERATORS (comma-separated |
| // addresses; narrows the fleet-operator bit further for the pause/cap |
| @@ -901,13 +901,19 @@ func systemOperators() []string { |
| // name that matched nothing would pause the very account it meant to keep. |
| // An invalid entry is an error for serve to stop on. |
| // |
| -// Unset or empty is nil, and nil is no gate: every account may enable and |
| -// create sidecars, exactly as before the list existed. This is the opposite |
| -// direction to the operator lists above on purpose, so a deployment that |
| -// never sets it is unaffected. |
| +// Unset or the empty string is nil, and nil is no gate: every account may |
| +// enable and create sidecars, exactly as before the list existed. This is |
| +// the opposite direction to the operator lists above on purpose, so a |
| +// deployment that never sets it is unaffected. Any other value that names |
| +// no sqid at all ("," or " , ") is an error too: it reads as a list, and |
| +// parsing it to nil would quietly turn the gate off. |
| func sidecarAccounts(getenv func(string) string) ([]string, error) { |
| + raw := getenv("CARLOS_SIDECAR_ACCOUNTS") |
| + if raw == "" { |
| + return nil, nil |
| + } |
| var out []string |
| - for _, a := range strings.Split(getenv("CARLOS_SIDECAR_ACCOUNTS"), ",") { |
| + for _, a := range strings.Split(raw, ",") { |
| a = strings.ToLower(strings.TrimSpace(a)) |
| if a == "" { |
| continue |
| @@ -917,6 +923,9 @@ func sidecarAccounts(getenv func(string) string) ([]string, error) { |
| } |
| out = append(out, a) |
| } |
| + if len(out) == 0 { |
| + return nil, fmt.Errorf("CARLOS_SIDECAR_ACCOUNTS: %q names no account sqid; unset it for no gate, or list at least one sqid", raw) |
| + } |
| return out, nil |
| } |
diff --git a/cmd/console/sidecaraccounts_serve_test.go b/cmd/console/sidecaraccounts_serve_test.go| index 4f9c9b05b..066138fba 100644 |
| --- a/cmd/console/sidecaraccounts_serve_test.go |
| +++ b/cmd/console/sidecaraccounts_serve_test.go |
| @@ -11,37 +11,45 @@ import ( |
| ) |
| // TestServeStopsOnABadSidecarAccount: serve reads CARLOS_SIDECAR_ACCOUNTS |
| -// and an entry that is not a sqid stops the console before it opens |
| -// anything. The timeout is the failure mode it guards: a serve that ignored |
| -// the variable would carry on and listen. |
| +// and stops before it opens anything on an entry that is not a sqid, or on a |
| +// value set to a list that names no sqid at all ("," would otherwise parse |
| +// to nil and quietly turn the gate off). The timeout is the failure mode it |
| +// guards: a serve that ignored the variable would carry on and listen. |
| func TestServeStopsOnABadSidecarAccount(t *testing.T) { |
| bin := buildConsole(t) |
| - tmp := t.TempDir() |
| - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) |
| - defer cancel() |
| - cmd := exec.CommandContext(ctx, bin, "serve") |
| - cmd.Dir = tmp |
| - cmd.Env = append(os.Environ(), |
| - "CONSOLE_ORIGIN=http://127.0.0.1:1", |
| - "CONSOLE_ADDR=127.0.0.1:0", |
| - "CONSOLE_DB="+filepath.Join(tmp, "console.db"), |
| - // 32 zero bytes: a well-formed Ed25519 public key, so serve gets |
| - // past the key check to the variable under test. |
| - "CARLOS_RELEASE_PUBKEY=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=", |
| - "CARLOS_RELEASE_KEY=", |
| - "CARLOS_SIDECAR_ACCOUNTS=bab,ops", |
| - ) |
| - out, err := cmd.CombinedOutput() |
| - if ctx.Err() != nil { |
| - t.Fatalf("serve did not stop on a bad CARLOS_SIDECAR_ACCOUNTS: %s", out) |
| - } |
| - if err == nil { |
| - t.Fatalf("serve exited 0 with a bad CARLOS_SIDECAR_ACCOUNTS: %s", out) |
| - } |
| - if !strings.Contains(string(out), "CARLOS_SIDECAR_ACCOUNTS") || !strings.Contains(string(out), `"ops"`) { |
| - t.Fatalf("serve's refusal does not name the variable and the entry: %s", out) |
| - } |
| - if _, err := os.Stat(filepath.Join(tmp, "console.db")); err == nil { |
| - t.Fatalf("serve opened its database before refusing the variable") |
| + for _, c := range []struct{ val, want string }{ |
| + {"bab,ops", `"ops"`}, |
| + {",", "names no account sqid"}, |
| + {" , ", "names no account sqid"}, |
| + } { |
| + tmp := t.TempDir() |
| + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) |
| + cmd := exec.CommandContext(ctx, bin, "serve") |
| + cmd.Dir = tmp |
| + cmd.Env = append(os.Environ(), |
| + "CONSOLE_ORIGIN=http://127.0.0.1:1", |
| + "CONSOLE_ADDR=127.0.0.1:0", |
| + "CONSOLE_DB="+filepath.Join(tmp, "console.db"), |
| + // 32 zero bytes: a well-formed Ed25519 public key, so serve gets |
| + // past the key check to the variable under test. |
| + "CARLOS_RELEASE_PUBKEY=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=", |
| + "CARLOS_RELEASE_KEY=", |
| + "CARLOS_SIDECAR_ACCOUNTS="+c.val, |
| + ) |
| + out, err := cmd.CombinedOutput() |
| + timedOut := ctx.Err() != nil |
| + cancel() |
| + if timedOut { |
| + t.Fatalf("CARLOS_SIDECAR_ACCOUNTS=%q: serve did not stop: %s", c.val, out) |
| + } |
| + if err == nil { |
| + t.Fatalf("CARLOS_SIDECAR_ACCOUNTS=%q: serve exited 0: %s", c.val, out) |
| + } |
| + if !strings.Contains(string(out), "CARLOS_SIDECAR_ACCOUNTS") || !strings.Contains(string(out), c.want) { |
| + t.Fatalf("CARLOS_SIDECAR_ACCOUNTS=%q: the refusal does not name the variable and %s: %s", c.val, c.want, out) |
| + } |
| + if _, err := os.Stat(filepath.Join(tmp, "console.db")); err == nil { |
| + t.Fatalf("CARLOS_SIDECAR_ACCOUNTS=%q: serve opened its database before refusing", c.val) |
| + } |
| } |
| } |
diff --git a/cmd/console/sidecaraccounts_test.go b/cmd/console/sidecaraccounts_test.go| index a0dd819ae..cba72c2bd 100644 |
| --- a/cmd/console/sidecaraccounts_test.go |
| +++ b/cmd/console/sidecaraccounts_test.go |
| @@ -6,9 +6,9 @@ import ( |
| ) |
| // CARLOS_SIDECAR_ACCOUNTS reaches Config.SidecarAccounts through |
| -// sidecarAccounts: trimmed, lower-cased, each a sqid. Unset and empty are |
| -// nil, which is no gate; any entry that is not a sqid is an error serve |
| -// stops on (TestServeStopsOnABadSidecarAccount runs that through the binary). |
| +// sidecarAccounts: trimmed, lower-cased, each a sqid. Unset and the empty |
| +// string are nil, which is no gate; any entry that is not a sqid, and any |
| +// other value that names no sqid, is an error serve stops on (TestServeStopsOnABadSidecarAccount runs that through the binary). |
| func TestSidecarAccountsParsesTheList(t *testing.T) { |
| for _, tc := range []struct { |
| val string |
| @@ -16,8 +16,9 @@ func TestSidecarAccountsParsesTheList(t *testing.T) { |
| wantErr bool |
| }{ |
| {"", nil, false}, |
| - {" ", nil, false}, |
| - {",", nil, false}, |
| + {" ", nil, true}, // set, but names no sqid: refused, never "no gate" |
| + {",", nil, true}, // the same, and the review's case |
| + {" , ", nil, true}, // ditto |
| {"bab", []string{"bab"}, false}, |
| {" BAB , cdef ,", []string{"bab", "cdef"}, false}, |
| {"bab,ops", nil, true}, // a name, not a sqid: o is not in the alphabet |
diff --git a/internal/console/sidecarsgate.go b/internal/console/sidecarsgate.go| index 54354cc65..002d4c8e5 100644 |
| --- a/internal/console/sidecarsgate.go |
| +++ b/internal/console/sidecarsgate.go |
| @@ -35,7 +35,7 @@ import ( |
| ) |
| // sidecarsPausedMsg is the one sentence a refused account reads. |
| -// copy: pending review. |
| +// copy: approved by Paul 2026-10-24 (copy review). |
| const sidecarsPausedMsg = "sidecars are paused for this account. existing sidecars keep running, and you can still delete them" |
| // sidecarsPausedCode is the refusal's machine-readable code, beside the |