amadan

carlos / platform Public

cli: -console refuses to mix with direct mode; ship hardens its API path

Review round (3 Important + 5 minor), same functions as the prior commit:

- resolveMode refuses outright when -console is given alongside either
  store env, instead of silently letting direct mode win with the flag
  ignored — the spec's two modes must never mix in one invocation
  (docs/superpowers/specs/2026-08-03-cli-console-api-design.md §1).
- runShipAPI now returns an error instead of calling log.Fatal itself, so
  its static-kind scratch temp file (up to 256MiB) is always cleaned up
  via defer — log.Fatal's os.Exit was skipping it on every failure path.
- runShipAPI validates multi-file binary artifact basenames client-side
  against the same GOOS-GOARCH shape the server enforces, and stats every
  artifact path before ever dialing the console, so a bad name or a
  typo'd path is refused before a byte moves.
- accountFlags now returns the raw -account flag alongside its resolver
  thunk, so the four API-mode verbs read it directly instead of through
  fs.Lookup("account") (which string-coupled them to accountFlags' own
  flag name).
- Tests: API-mode verb tests use a non-sqid -account value and assert no
  registry.db appears, pinning that API mode never calls
  resolveAccount/opens the local registry; new coverage for the static
  ship path (site.tar.gz naming and bytes), multi-file ship (good and
  bad basenames), the pre-flight stat, the temp-file cleanup, and the
  -console/store-env conflict (both env vars).
Paul Campbell c57b194c4c2344db09cbee11ac1df9e569dba2af parent 2bd3d9e
5 files changed, +475 −42
  • apimode.go +15 −1
  • apimode_test.go +375 −10
  • appconfig_cli.go +8 −8
  • cli.go +71 −21
  • docs/superpowers/specs/2026-08-03-cli-console-api-design.md +6 −2
diff --git a/apimode.go b/apimode.go
index 4c6e46ee9..6145af533 100644
--- a/apimode.go
+++ b/apimode.go
@@ -48,8 +48,22 @@ type apiSession struct {
//
// consoleFlag is the verb's -console value ("" meaning "the only one" —
// pick's ordinary disambiguation rule, unchanged here).
+//
+// An explicit -console alongside either store env is refused outright
+// (design ruling, review round 2026-08-03) rather than silently winning as
+// direct mode: the spec's "the two modes never mix in one invocation" intent
+// (§1) would otherwise be violated invisibly — someone who typed -console
+// meaning it gets bucket access instead, with no signal that their flag was
+// ignored. A BARE env with no -console still wins silently, unchanged: that
+// half of §1 (operators, CI, box-side scripts) is deliberately untouched.
func resolveMode(consoleFlag string) (*apiSession, error) {
- if os.Getenv("CARLOS_DEPLOYMENT_BUCKET") != "" || os.Getenv("CARLOS_DEPLOYMENT_DIR") != "" {
+ bucketSet := os.Getenv("CARLOS_DEPLOYMENT_BUCKET") != ""
+ dirSet := os.Getenv("CARLOS_DEPLOYMENT_DIR") != ""
+ if consoleFlag != "" && (bucketSet || dirSet) {
+ return nil, errors.New("-console conflicts with CARLOS_DEPLOYMENT_BUCKET/CARLOS_DEPLOYMENT_DIR " +
+ "— unset the env to use the console, or drop -console")
+ }
+ if bucketSet || dirSet {
return nil, nil
}
cred, err := loggedInCredential(consoleFlag)
diff --git a/apimode_test.go b/apimode_test.go
index 95fe4eaa8..88d5c7cb0 100644
--- a/apimode_test.go
+++ b/apimode_test.go
@@ -8,6 +8,7 @@ import (
"bytes"
"context"
"encoding/json"
+ "flag"
"io"
"net/http"
"net/http/httptest"
@@ -18,6 +19,7 @@ import (
"testing"
"time"
+ "github.com/carlosframework/platform/internal/edge"
"github.com/carlosframework/platform/internal/release"
)
@@ -159,6 +161,44 @@ func TestResolveModeExpiredCredentialOnly(t *testing.T) {
}
}
+// TestResolveModeConsoleFlagRefusedWithStoreEnv pins the design ruling from
+// the review round (2026-08-03): an explicit -console alongside a store env
+// is refused outright, rather than silently winning as direct mode with
+// -console quietly ignored — the spec's "the two modes never mix in one
+// invocation" intent (§1) would otherwise be violated invisibly. Both store
+// env vars are checked; a BARE env with no -console still wins silently,
+// unchanged (already pinned by TestResolveModeDirectWins/TestResolveModeDirWins,
+// both of which call resolveMode("")).
+func TestResolveModeConsoleFlagRefusedWithStoreEnv(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ envVar string
+ }{
+ {"bucket", "CARLOS_DEPLOYMENT_BUCKET"},
+ {"dir", "CARLOS_DEPLOYMENT_DIR"},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ useTempCredentials(t)
+ clearDeploymentEnv(t)
+ t.Setenv(tc.envVar, "something")
+
+ sess, err := resolveMode("https://a.example")
+ if sess != nil {
+ t.Fatalf("resolveMode with -console AND %s set = %+v, want nil session", tc.envVar, sess)
+ }
+ if err == nil {
+ t.Fatalf("resolveMode with -console AND %s set must error", tc.envVar)
+ }
+ if !strings.Contains(err.Error(), "-console") {
+ t.Fatalf("error %q does not mention -console", err)
+ }
+ if !strings.Contains(err.Error(), tc.envVar) {
+ t.Fatalf("error %q does not mention %s", err, tc.envVar)
+ }
+ })
+ }
+}
+
func newTestSession(base string) *apiSession {
return &apiSession{c: newConsoleClient(base, "cli-token"), console: base, address: "a@keymail.dev"}
}
@@ -578,6 +618,59 @@ func loginTo(t *testing.T, origin, address string) {
}
}
+// apiTestAccount is the -account value every API-mode verb test below uses.
+// Deliberately NOT sqid-shaped and not "ops": every prior round of these
+// tests happened to pass an -account value ("acct") that resolveAccount
+// would have resolved WITHOUT error had it wrongly been called (release.
+// ValidAccount("acct") happens to be true, so a stray resolveAccount call
+// would have silently "worked" and gone unnoticed). "vickys-team" fails
+// that same check, so a regression that starts calling acctOf()/
+// resolveAccount in API mode again is loud (log.Fatal, or at minimum the
+// registry.db file requireNoRegistryTouch below watches for) rather than
+// silently passing.
+const apiTestAccount = "vickys-team"
+
+// requireNoRegistryTouch points CARLOS_DATA_DIR at a fresh, empty temp
+// directory and returns a func the caller runs AFTER the verb under test:
+// it fails the test if registry.db appeared there. accountFlags' resolver
+// (resolveAccount) is the only thing in this file that ever opens the
+// registry, and registry.Open creates the file on first open even if
+// resolution goes on to fail — so its presence is direct, mechanical proof
+// that API mode reached into the local registry, which it must never do
+// (there is no registry on a developer laptop to reach into).
+func requireNoRegistryTouch(t *testing.T) func() {
+ t.Helper()
+ dir := t.TempDir()
+ t.Setenv("CARLOS_DATA_DIR", dir)
+ return func() {
+ t.Helper()
+ path := filepath.Join(dir, "registry.db")
+ if _, err := os.Stat(path); err == nil {
+ t.Fatalf("%s was created — API mode must never open the local registry", path)
+ } else if !os.IsNotExist(err) {
+ t.Fatalf("stat %s: %v", path, err)
+ }
+ }
+}
+
+// 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) {
+ t.Helper()
+ fs := flag.NewFlagSet("ship", flag.ContinueOnError)
+ acctRaw, _ := accountFlags(fs)
+ full := append([]string{"-account", account}, args...)
+ if err := fs.Parse(full); err != nil {
+ t.Fatalf("parse: %v", err)
+ }
+ return fs, acctRaw
+}
+
// TestShipVerbAPIMode pins runShip's API-mode wiring end to end: with a live
// credential and neither store env set, it must ship through the console
// (not openStore/release.Ship) and print the first-use "using <origin> as
@@ -587,10 +680,11 @@ func loginTo(t *testing.T, origin, address string) {
func TestShipVerbAPIMode(t *testing.T) {
useTempCredentials(t)
clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
var reqCount int32
mux := http.NewServeMux()
- mux.HandleFunc("POST /api/cli/apps/acct/titogo/releases", func(w http.ResponseWriter, r *http.Request) {
+ mux.HandleFunc("POST /api/cli/apps/"+apiTestAccount+"/titogo/releases", func(w http.ResponseWriter, r *http.Request) {
atomic.AddInt32(&reqCount, 1)
mr, err := r.MultipartReader()
if err != nil {
@@ -626,7 +720,7 @@ func TestShipVerbAPIMode(t *testing.T) {
}
buf := captureStdout(t)
- runShip([]string{"-app", "titogo", "-account", "acct", "-version", "abc1234", "-target", "linux-arm64", bin})
+ runShip([]string{"-app", "titogo", "-account", apiTestAccount, "-version", "abc1234", "-target", "linux-arm64", bin})
out := buf.String()
if want := "using " + ts.URL + " as vicky@keymail.dev"; !strings.Contains(out, want) {
@@ -638,6 +732,7 @@ func TestShipVerbAPIMode(t *testing.T) {
if got := atomic.LoadInt32(&reqCount); got != 1 {
t.Fatalf("server got %d requests, want 1", got)
}
+ checkNoRegistry()
}
// TestShipVerbAPIModeNoOp pins the ship side of the noop-parity amendment: a
@@ -647,6 +742,7 @@ func TestShipVerbAPIMode(t *testing.T) {
func TestShipVerbAPIModeNoOp(t *testing.T) {
useTempCredentials(t)
clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mr, err := r.MultipartReader()
@@ -676,7 +772,7 @@ func TestShipVerbAPIModeNoOp(t *testing.T) {
}
buf := captureStdout(t)
- runShip([]string{"-app", "titogo", "-account", "acct", "-version", "abc1234", "-target", "linux-arm64", bin})
+ runShip([]string{"-app", "titogo", "-account", apiTestAccount, "-version", "abc1234", "-target", "linux-arm64", bin})
out := buf.String()
if want := "already shipped titogo abc1234 — identical content, nothing to do"; !strings.Contains(out, want) {
@@ -685,6 +781,7 @@ func TestShipVerbAPIModeNoOp(t *testing.T) {
if strings.Contains(out, "shipped titogo abc1234\n") {
t.Fatalf("stdout = %q, must not ALSO print the ordinary shipped line", out)
}
+ checkNoRegistry()
}
// TestPromoteVerbAPIMode pins runPromote's API-mode wiring: it must promote
@@ -694,11 +791,12 @@ func TestShipVerbAPIModeNoOp(t *testing.T) {
func TestPromoteVerbAPIMode(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/acct/titogo/promote" {
+ if r.URL.Path != "/api/cli/apps/"+apiTestAccount+"/titogo/promote" {
t.Errorf("path = %s", r.URL.Path)
}
var req struct{ Version, Channel string }
@@ -713,7 +811,7 @@ func TestPromoteVerbAPIMode(t *testing.T) {
loginTo(t, ts.URL, "vicky@keymail.dev")
buf := captureStdout(t)
- runPromote([]string{"-app", "titogo", "-account", "acct", "abc1234", "canary/x"})
+ runPromote([]string{"-app", "titogo", "-account", apiTestAccount, "abc1234", "canary/x"})
out := buf.String()
if want := "using " + ts.URL + " as vicky@keymail.dev"; !strings.Contains(out, want) {
@@ -725,6 +823,7 @@ func TestPromoteVerbAPIMode(t *testing.T) {
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
@@ -734,6 +833,7 @@ func TestPromoteVerbAPIMode(t *testing.T) {
func TestPromoteVerbAPIModeNoOp(t *testing.T) {
useTempCredentials(t)
clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
json.NewEncoder(w).Encode(map[string]any{"result": "noop"})
@@ -743,12 +843,13 @@ func TestPromoteVerbAPIModeNoOp(t *testing.T) {
loginTo(t, ts.URL, "vicky@keymail.dev")
buf := captureStdout(t)
- runPromote([]string{"-app", "titogo", "-account", "acct", "abc1234", "canary/x"})
+ runPromote([]string{"-app", "titogo", "-account", apiTestAccount, "abc1234", "canary/x"})
out := buf.String()
if want := "titogo abc1234 is already on canary/x — nothing to do"; !strings.Contains(out, want) {
t.Fatalf("stdout = %q, want %q", out, want)
}
+ checkNoRegistry()
}
// TestChannelsVerbAPIMode pins runChannels' API-mode wiring: it must read the
@@ -758,9 +859,10 @@ func TestPromoteVerbAPIModeNoOp(t *testing.T) {
func TestChannelsVerbAPIMode(t *testing.T) {
useTempCredentials(t)
clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- if r.URL.Path != "/api/cli/apps/acct/titogo/channels" {
+ if r.URL.Path != "/api/cli/apps/"+apiTestAccount+"/titogo/channels" {
t.Errorf("path = %s", r.URL.Path)
}
json.NewEncoder(w).Encode(map[string]any{
@@ -775,7 +877,7 @@ func TestChannelsVerbAPIMode(t *testing.T) {
loginTo(t, ts.URL, "vicky@keymail.dev")
buf := captureStdout(t)
- runChannels([]string{"-app", "titogo", "-account", "acct"})
+ runChannels([]string{"-app", "titogo", "-account", apiTestAccount})
out := buf.String()
if want := "using " + ts.URL + " as vicky@keymail.dev"; !strings.Contains(out, want) {
@@ -790,17 +892,19 @@ func TestChannelsVerbAPIMode(t *testing.T) {
if !strings.Contains(out, "broken") || !strings.Contains(out, "ERROR: signature mismatch") {
t.Fatalf("stdout = %q, want the broken row's ERROR text", out)
}
+ checkNoRegistry()
}
// TestRollbackVerbAPIMode pins runRollback's API-mode wiring.
func TestRollbackVerbAPIMode(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/acct/titogo/rollback" {
+ if r.URL.Path != "/api/cli/apps/"+apiTestAccount+"/titogo/rollback" {
t.Errorf("path = %s", r.URL.Path)
}
var req struct{ Channel string }
@@ -815,7 +919,7 @@ func TestRollbackVerbAPIMode(t *testing.T) {
loginTo(t, ts.URL, "vicky@keymail.dev")
buf := captureStdout(t)
- runRollback([]string{"-app", "titogo", "-account", "acct", "canary/x"})
+ runRollback([]string{"-app", "titogo", "-account", apiTestAccount, "canary/x"})
out := buf.String()
if want := "using " + ts.URL + " as vicky@keymail.dev"; !strings.Contains(out, want) {
@@ -827,6 +931,7 @@ func TestRollbackVerbAPIMode(t *testing.T) {
if got := atomic.LoadInt32(&reqCount); got != 1 {
t.Fatalf("server got %d requests, want 1", got)
}
+ checkNoRegistry()
}
// TestPromoteVerbDirectUnchanged pins the other half of resolveMode's
@@ -869,6 +974,13 @@ func TestPromoteVerbDirectUnchanged(t *testing.T) {
runPromote([]string{"-app", "demoapp", "abc1234", "canary/x"})
out := buf.String()
+ // Pins ship's direct-mode print format exactly (review round, MINOR 8):
+ // unlike API mode's deliberately different "shipped <app> <version>"
+ // (no count), direct mode still names the artifact count — this must
+ // stay byte-identical to what it printed before Task 5 touched this verb.
+ if want := "shipped demoapp abc1234 (1 artifacts)"; !strings.Contains(out, want) {
+ t.Fatalf("stdout = %q, want the direct-mode shipped line", out)
+ }
if want := "promoted demoapp abc1234 -> canary/x"; !strings.Contains(out, want) {
t.Fatalf("stdout = %q, want the direct-mode promoted line", out)
}
@@ -876,3 +988,256 @@ func TestPromoteVerbDirectUnchanged(t *testing.T) {
t.Fatalf("server got %d requests, want 0 — direct mode must never dial the console", got)
}
}
+
+// ---- Task 5 review round: runShipAPI's static/multi-file/pre-flight paths -
+
+// TestShipAPIStaticNamesSiteTarGz pins runShipAPI's static branch end to end
+// (review round, IMPORTANT 2 — previously zero coverage): the packed tarball
+// must reach the server in a part named EXACTLY "site.tar.gz", carrying the
+// same bytes edge.PackDir itself produces for the same directory — proof the
+// scratch-temp-file plumbing (PackDir -> temp file -> multipart part) does
+// not mangle anything in between.
+func TestShipAPIStaticNamesSiteTarGz(t *testing.T) {
+ useTempCredentials(t)
+ clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
+
+ siteDir := t.TempDir()
+ if err := os.WriteFile(filepath.Join(siteDir, "index.html"), []byte("<h1>hi</h1>"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ wantBytes, err := edge.PackDir(siteDir)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ var gotName string
+ var gotBytes []byte
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ mr, err := r.MultipartReader()
+ if err != nil {
+ t.Fatalf("not multipart: %v", err)
+ }
+ for {
+ part, err := mr.NextPart()
+ if err == io.EOF {
+ break
+ }
+ if err != nil {
+ t.Fatalf("part: %v", err)
+ }
+ b, err := io.ReadAll(part)
+ if err != nil {
+ t.Fatalf("read part: %v", err)
+ }
+ if part.FormName() != "meta" {
+ gotName = part.FormName()
+ gotBytes = b
+ }
+ }
+ w.WriteHeader(http.StatusCreated)
+ json.NewEncoder(w).Encode(map[string]any{"result": "shipped", "version": "abc1234", "artifacts": map[string]any{}})
+ }))
+ defer ts.Close()
+
+ sess := newTestSession(ts.URL)
+ fs, acctRaw := newShipFlagSet(t, apiTestAccount, siteDir)
+ if err := runShipAPI(sess, acctRaw, fs, "titesite", "abc1234", "static", ""); err != nil {
+ t.Fatalf("runShipAPI: %v", err)
+ }
+
+ if gotName != "site.tar.gz" {
+ t.Fatalf("server received artifact part named %q, want %q", gotName, "site.tar.gz")
+ }
+ if !bytes.Equal(gotBytes, wantBytes) {
+ t.Fatalf("server received %d bytes, want the %d bytes edge.PackDir produced for the same directory", len(gotBytes), len(wantBytes))
+ }
+ checkNoRegistry()
+}
+
+// TestShipAPIStaticCleansUpTempFileOnServerError pins the temp-file-leak fix
+// (review round, MINOR 4): runShipAPI used to call log.Fatal on a failing
+// sess.ship, which calls os.Exit and skips every deferred function —
+// including the static kind's scratch temp file cleanup, up to 256MiB per
+// invocation. Returning an error instead (the caller log.Fatals) lets that
+// defer actually run.
+func TestShipAPIStaticCleansUpTempFileOnServerError(t *testing.T) {
+ useTempCredentials(t)
+ clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
+
+ siteDir := t.TempDir()
+ if err := os.WriteFile(filepath.Join(siteDir, "index.html"), []byte("hi"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ io.Copy(io.Discard, r.Body)
+ w.WriteHeader(http.StatusInternalServerError)
+ }))
+ defer ts.Close()
+
+ tmpParent := os.TempDir()
+ before, err := os.ReadDir(tmpParent)
+ if err != nil {
+ t.Fatal(err)
+ }
+ beforeNames := map[string]bool{}
+ for _, e := range before {
+ beforeNames[e.Name()] = true
+ }
+
+ sess := newTestSession(ts.URL)
+ fs, acctRaw := newShipFlagSet(t, apiTestAccount, siteDir)
+ if err := runShipAPI(sess, acctRaw, fs, "titesite", "abc1234", "static", ""); err == nil {
+ t.Fatal("runShipAPI against a failing server must error")
+ }
+
+ after, err := os.ReadDir(tmpParent)
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, e := range after {
+ if beforeNames[e.Name()] {
+ continue
+ }
+ if strings.HasPrefix(e.Name(), "carlos-ship-") {
+ t.Fatalf("temp file %s leaked after a failed ship", e.Name())
+ }
+ }
+ checkNoRegistry()
+}
+
+// TestShipAPIMultiFileGoodBasenames pins the multi-file binary ship path
+// (review round, IMPORTANT 2 + MINOR 7's good case): every basename shaped
+// like a GOOS-GOARCH target ships as its own part, named exactly its
+// basename.
+func TestShipAPIMultiFileGoodBasenames(t *testing.T) {
+ useTempCredentials(t)
+ clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
+
+ dir := t.TempDir()
+ fileArm := filepath.Join(dir, "linux-arm64")
+ fileAmd := filepath.Join(dir, "linux-amd64")
+ if err := os.WriteFile(fileArm, []byte("arm bytes"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(fileAmd, []byte("amd bytes"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+
+ var reqCount int32
+ gotParts := map[string]bool{}
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ atomic.AddInt32(&reqCount, 1)
+ mr, err := r.MultipartReader()
+ if err != nil {
+ t.Fatalf("not multipart: %v", err)
+ }
+ for {
+ part, err := mr.NextPart()
+ if err == io.EOF {
+ break
+ }
+ if err != nil {
+ t.Fatalf("part: %v", err)
+ }
+ io.Copy(io.Discard, part)
+ if part.FormName() != "meta" {
+ gotParts[part.FormName()] = true
+ }
+ }
+ w.WriteHeader(http.StatusCreated)
+ json.NewEncoder(w).Encode(map[string]any{"result": "shipped", "version": "abc1234", "artifacts": map[string]any{}})
+ }))
+ 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 {
+ t.Fatalf("runShipAPI: %v", err)
+ }
+
+ if !gotParts["linux-arm64"] || !gotParts["linux-amd64"] || len(gotParts) != 2 {
+ t.Fatalf("server received parts %v, want exactly linux-arm64 and linux-amd64", gotParts)
+ }
+ if got := atomic.LoadInt32(&reqCount); got != 1 {
+ t.Fatalf("server got %d requests, want 1", got)
+ }
+ checkNoRegistry()
+}
+
+// TestShipAPIMultiFileBadBasenameRefused pins the client-side name check
+// (review round, MINOR 7's bad case): a multi-file binary ship whose
+// basename doesn't fit the server's own GOOS-GOARCH shape (targetRe,
+// internal/console/apicli.go) must be refused before ever dialing the
+// console, naming the offending file.
+func TestShipAPIMultiFileBadBasenameRefused(t *testing.T) {
+ useTempCredentials(t)
+ clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
+
+ dir := t.TempDir()
+ fileGood := filepath.Join(dir, "linux-arm64")
+ fileBad := filepath.Join(dir, "output.bin")
+ if err := os.WriteFile(fileGood, []byte("arm bytes"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(fileBad, []byte("bad bytes"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+
+ var reqCount int32
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ atomic.AddInt32(&reqCount, 1)
+ w.WriteHeader(http.StatusInternalServerError)
+ }))
+ defer ts.Close()
+
+ sess := newTestSession(ts.URL)
+ fs, acctRaw := newShipFlagSet(t, apiTestAccount, fileGood, fileBad)
+ err := runShipAPI(sess, acctRaw, fs, "titogo", "abc1234", "binary", "linux-arm64")
+ if err == nil {
+ t.Fatal("runShipAPI with a bad basename must error")
+ }
+ if !strings.Contains(err.Error(), "output.bin") {
+ t.Fatalf("error %q does not name the offending file", err)
+ }
+ if !strings.Contains(err.Error(), "invalid artifact name") {
+ t.Fatalf("error %q does not explain the refusal", err)
+ }
+ if got := atomic.LoadInt32(&reqCount); got != 0 {
+ t.Fatalf("server got %d requests, want 0 — a bad basename must be refused before dialing the console", got)
+ }
+ checkNoRegistry()
+}
+
+// TestShipAPIMissingArtifactRefusedBeforeDialing pins the pre-flight stat
+// (review round, MINOR 6): a typo'd artifact path must fail clean before the
+// console is ever dialed, not after.
+func TestShipAPIMissingArtifactRefusedBeforeDialing(t *testing.T) {
+ useTempCredentials(t)
+ clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
+
+ missing := filepath.Join(t.TempDir(), "linux-arm64") // never created
+
+ var reqCount int32
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ atomic.AddInt32(&reqCount, 1)
+ w.WriteHeader(http.StatusInternalServerError)
+ }))
+ defer ts.Close()
+
+ sess := newTestSession(ts.URL)
+ fs, acctRaw := newShipFlagSet(t, apiTestAccount, missing)
+ err := runShipAPI(sess, acctRaw, fs, "titogo", "abc1234", "binary", "linux-arm64")
+ if err == nil {
+ t.Fatal("runShipAPI with a missing artifact path must error")
+ }
+ if got := atomic.LoadInt32(&reqCount); got != 0 {
+ t.Fatalf("server got %d requests, want 0 — a missing path must be refused before dialing the console", got)
+ }
+ checkNoRegistry()
+}
diff --git a/appconfig_cli.go b/appconfig_cli.go
index 1a4528ae1..749f32fc7 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/cli.go b/cli.go
index 7908d6114..5b7cf4eb3 100644
--- a/cli.go
+++ b/cli.go
@@ -9,12 +9,14 @@ import (
"crypto/ed25519"
"crypto/sha256"
"encoding/hex"
+ "errors"
"flag"
"fmt"
"io"
"log"
"os"
"path/filepath"
+ "regexp"
"strings"
"text/tabwriter"
"time"
@@ -720,13 +722,20 @@ func routeVersionCheck(ctx context.Context, store release.Store, pub ed25519.Pub
// single-tenant deployment never types it. Reads fall back to the legacy
// unqualified keys either way, so an unmigrated app keeps working.
//
-// Returns a thunk rather than the value 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.
-func accountFlags(fs *flag.FlagSet) func() string {
+// Returns the raw flag value 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
+// console-API verbs): they must send the -account value verbatim (a
+// developer laptop has no local registry to resolve it against, and the
+// console does its own resolution against the caller's memberships), and
+// 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) {
acct := fs.String("account", "ops", "account (sqid or name) the app belongs to")
regPath := fs.String("registry", defaultRegistry(), "registry database (resolves -account)")
- return func() string { return resolveAccount(*regPath, *acct) }
+ return acct, func() string { return resolveAccount(*regPath, *acct) }
}
func openStore() release.Store {
@@ -763,7 +772,7 @@ func publicKey() ed25519.PublicKey {
func runShip(args []string) {
fs := flag.NewFlagSet("ship", flag.ExitOnError)
app := fs.String("app", "", "app name")
- acctOf := accountFlags(fs)
+ acctRaw, 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)")
@@ -778,7 +787,9 @@ func runShip(args []string) {
log.Fatal(err)
}
if sess != nil {
- runShipAPI(sess, fs, *app, *ver, *kind, *target)
+ if err := runShipAPI(sess, acctRaw, fs, *app, *ver, *kind, *target); err != nil {
+ log.Fatal(err)
+ }
return
}
@@ -830,6 +841,13 @@ func runShip(args []string) {
fmt.Fprintf(stdout, "shipped %s %s (%d artifacts)\n", *app, *ver, len(files))
}
+// shipTargetRe validates a binary artifact's part name client-side, against
+// the exact shape the server enforces (targetRe, internal/console/apicli.go)
+// — checked here too so a malformed name (a build filename shipped without
+// -target, say) is refused before a single byte crosses the wire, rather
+// 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
@@ -838,47 +856,79 @@ func runShip(args []string) {
// static release, whose bytes only exist in memory (edge.PackDir's tarball),
// that means writing them to a scratch temp file first so there is a path to
// hand over at all.
-func runShipAPI(sess *apiSession, fs *flag.FlagSet, app, ver, kind, target string) {
- acct := fs.Lookup("account").Value.String()
+//
+// Returns an error rather than calling log.Fatal itself (the caller does
+// that): log.Fatal calls os.Exit immediately, which skips every deferred
+// 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
files := map[string]string{}
if kind == "static" {
if fs.NArg() != 1 {
- log.Fatal("static ship takes exactly one site directory")
+ return errors.New("static ship takes exactly one site directory")
}
data, err := edge.PackDir(fs.Arg(0))
if err != nil {
- log.Fatal(err)
+ return err
}
tmp, err := os.CreateTemp("", "carlos-ship-*.tar.gz")
if err != nil {
- log.Fatal(err)
+ return err
}
defer os.Remove(tmp.Name())
if _, err := tmp.Write(data); err != nil {
tmp.Close()
- log.Fatal(err)
+ return err
}
if err := tmp.Close(); err != nil {
- log.Fatal(err)
+ return err
}
files["site.tar.gz"] = tmp.Name()
} else if fs.NArg() == 1 {
files[target] = fs.Arg(0)
} else {
+ // Multiple files: pre-named per target (linux-arm64, linux-amd64,
+ // ...) by basename, exactly like the direct-mode body above.
for _, path := range fs.Args() {
files[filepath.Base(path)] = path
}
}
+
+ // Client-side name check, binary kind only (a static release's one part
+ // is always named "site.tar.gz" above, never a caller-supplied name) —
+ // same shape the server enforces, checked here so a bad basename is
+ // named and refused before anything is stat'd or dialed.
+ if kind != "static" {
+ for name, path := range files {
+ if !shipTargetRe.MatchString(name) {
+ return fmt.Errorf("ship: invalid artifact name %q (from %s) — want a GOOS-GOARCH shape like linux-arm64 "+
+ "(pass -target to name a single file explicitly)", name, path)
+ }
+ }
+ }
+
+ // Pre-flight: every artifact path must exist before the console is ever
+ // dialed or a byte moves — a typo'd path must fail clean here, not after
+ // the "using ..." line already printed.
+ for name, path := range files {
+ if _, err := os.Stat(path); err != nil {
+ return fmt.Errorf("ship: artifact %q: %w", name, err)
+ }
+ }
+
fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address)
res, err := sess.ship(context.Background(), acct, app, ver, kind, files)
if err != nil {
- log.Fatal(err)
+ return err
}
if res.NoOp {
fmt.Fprintf(stdout, "already shipped %s %s — identical content, nothing to do\n", app, ver)
} else {
fmt.Fprintf(stdout, "shipped %s %s\n", app, ver)
}
+ return nil
}
// runPromote points a channel at a shipped version, with the ladder enforced:
@@ -890,7 +940,7 @@ 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)")
- acctOf := accountFlags(fs)
+ acctRaw, 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 {
@@ -912,7 +962,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 := fs.Lookup("account").Value.String()
+ acct := *acctRaw
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 {
@@ -958,7 +1008,7 @@ func promoteActor() string {
func runChannels(args []string) {
fs := flag.NewFlagSet("channels", flag.ExitOnError)
app := fs.String("app", "", "app name")
- acctOf := accountFlags(fs)
+ acctRaw, 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 == "" {
@@ -970,7 +1020,7 @@ func runChannels(args []string) {
log.Fatal(err)
}
if sess != nil {
- acct := fs.Lookup("account").Value.String()
+ acct := *acctRaw
fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address)
chans, err := sess.channels(context.Background(), acct, *app)
if err != nil {
@@ -1014,7 +1064,7 @@ func runChannels(args []string) {
func runRollback(args []string) {
fs := flag.NewFlagSet("rollback", flag.ExitOnError)
app := fs.String("app", "", "app name")
- acctOf := accountFlags(fs)
+ acctRaw, 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 {
@@ -1026,7 +1076,7 @@ func runRollback(args []string) {
log.Fatal(err)
}
if sess != nil {
- acct := fs.Lookup("account").Value.String()
+ acct := *acctRaw
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)
diff --git a/docs/superpowers/specs/2026-08-03-cli-console-api-design.md b/docs/superpowers/specs/2026-08-03-cli-console-api-design.md
index 86ff1a4b3..ab7ac1e20 100644
--- a/docs/superpowers/specs/2026-08-03-cli-console-api-design.md
+++ b/docs/superpowers/specs/2026-08-03-cli-console-api-design.md
@@ -45,8 +45,12 @@ In `ship`, `promote`, `rollback`, `channels` (and nothing else this slice):
mention `carlos auth login`.
No silent fallback from API mode to direct mode or vice versa: a failed API
-call is a failed command. The `-account` flag keeps its meaning in both
-modes; in API mode the server additionally verifies membership.
+call is a failed command. An explicit `-console` given alongside either store
+env is refused outright (not a silent direct-mode win), for the same reason:
+the two modes must never mix in one invocation, so a `-console` that would
+otherwise be silently ignored fails closed instead. The `-account` flag keeps
+its meaning in both modes; in API mode the server additionally verifies
+membership.
## §2 API surface (server)