review fixes: tag-pinned verified updates, offers that know their place
Opus findings applied: downloads come from the tag-pinned release path and the candidate binary must report the promised version before the rename (a floating 'latest' could install authentic-but-old bytes while claiming new); the moved-console offer runs once per process, never in quiet paths (whoami's bonus line), and only on 301/308 — 302/307 are SSO walls, not moves; the repoint prompt names the destination; ledgerBlob gains the missed hook; the update check negative-caches so a blackholed egress costs one command, not every command; -y no longer bypasses the terminal requirement; brew detection keys on /Cellar/ only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7 files changed,
+174
−43
apimode.go+32 −11apimode_test.go+68 −1auth.go+4 −1docs/superpowers/specs/2026-08-12-proactive-cli-design.md+3 −2main.go+2 −1update.go+57 −26update_test.go+8 −1
diff --git a/apimode.go b/apimode.go| index 1d66e0a67..3dffd2b6e 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -130,17 +130,18 @@ func (e *consoleMovedError) Error() string { |
| } |
| // consoleMoved inspects one response for a console-shaped redirect. Only a |
| -// 3xx whose Location names an ABSOLUTE origin normalizeConsole would accept |
| -// (https, or http strictly on-box) and that differs from the one dialed |
| -// counts — a same-origin or path-level redirect stays a plain unexpected |
| -// status, and a Location the CLI would refuse to log in to is never |
| -// surfaced as a destination. The Location is trusted for DISPLAY AND |
| -// PROMPTING only: nothing follows it, and no credential exists until the |
| -// human confirms the device flow against it. |
| +// PERMANENT redirect (301/308) whose Location names an ABSOLUTE origin |
| +// normalizeConsole would accept (https, or http strictly on-box) and that |
| +// differs from the one dialed counts — 302/307 are what SSO walls and |
| +// identity proxies answer with, and reading those as "the console moved" |
| +// would offer a login to accounts.google.com; a same-origin or path-level |
| +// redirect stays a plain unexpected status; and a Location the CLI would |
| +// refuse to log in to is never surfaced as a destination. The Location is |
| +// trusted for DISPLAY AND PROMPTING only: nothing follows it, and no |
| +// credential exists until the human confirms the device flow against it. |
| func consoleMoved(from string, status int, location string) *consoleMovedError { |
| switch status { |
| - case http.StatusMovedPermanently, http.StatusFound, |
| - http.StatusTemporaryRedirect, http.StatusPermanentRedirect: |
| + case http.StatusMovedPermanently, http.StatusPermanentRedirect: |
| default: |
| return nil |
| } |
| @@ -155,6 +156,19 @@ func consoleMoved(from string, status int, location string) *consoleMovedError { |
| return &consoleMovedError{From: from, To: origin} |
| } |
| +// consoleMovedOffered makes the offer once-per-process: polling verbs |
| +// (`logs -f`, authLogin's approval loop) hit the same 3xx every interval, |
| +// and prompting on each would eat the terminal's stdin; and the offer |
| +// itself runs authLogin, whose polls reach these same response sites — the |
| +// guard is also the re-entrancy stop for a redirect chain. After the one |
| +// offer (taken or declined), every further hit is the typed error. |
| +var consoleMovedOffered bool |
| + |
| +// quietConsoleOffers suppresses the offer entirely — set around calls |
| +// whose contract is to degrade silently (whoami's memberships bonus line), |
| +// where even one prompt would make a working command look broken. |
| +var quietConsoleOffers bool |
| + |
| // offerConsoleMoved is the terminal's version of consoleMovedError: explain |
| // the move, offer the standard device flow against the new host, and — when |
| // a .carlos/config names the old console — offer to repoint it (keeping |
| @@ -162,9 +176,10 @@ func consoleMoved(from string, status int, location string) *consoleMovedError { |
| // still ends nonzero: its request died against the old host mid-flight, and |
| // "logged in — re-run your command" is the honest state. |
| func offerConsoleMoved(e *consoleMovedError) error { |
| - if !interactiveTerminal() { |
| + if !interactiveTerminal() || quietConsoleOffers || consoleMovedOffered { |
| return e |
| } |
| + consoleMovedOffered = true |
| fmt.Fprintf(stdout, "this console has moved: %s -> %s\n", e.From, e.To) |
| if !promptYes(stdout, fmt.Sprintf("log in to %s now? [y/N] ", e.To)) { |
| return e |
| @@ -173,7 +188,10 @@ func offerConsoleMoved(e *consoleMovedError) error { |
| return err |
| } |
| if cfgConsole, src, err := configDefault(); err == nil && cfgConsole == e.From && src != "" { |
| - if promptYes(stdout, fmt.Sprintf("update %s to point there too? [y/N] ", src)) { |
| + // The destination is named in the prompt, not only the file: a y |
| + // here changes where future ships go, and that must never rest on |
| + // remembering what the previous prompt said. |
| + if promptYes(stdout, fmt.Sprintf("update %s to point at %s too? [y/N] ", src, e.To)) { |
| if err := saveConfigFile(src, e.To, 0o755); err != nil { |
| warnf(stdout, "not updated (%s): %v", src, err) |
| } else { |
| @@ -1272,6 +1290,9 @@ func (s *apiSession) ledgerBlob(ctx context.Context, a apiAcct, app, name string |
| return "", err |
| } |
| defer res.Body.Close() |
| + if merr := consoleMoved(s.c.base, res.StatusCode, res.Header.Get("Location")); merr != nil { |
| + return "", offerConsoleMoved(merr) |
| + } |
| respBody, err := io.ReadAll(io.LimitReader(res.Body, 1<<20)) |
| if err != nil { |
| return "", err |
diff --git a/apimode_test.go b/apimode_test.go| index af7b3f4ba..6faeab651 100644 |
| --- a/apimode_test.go |
| +++ b/apimode_test.go |
| @@ -3019,12 +3019,16 @@ func TestConsoleMovedDetection(t *testing.T) { |
| if e := consoleMoved(from, 301, "https://console.example/"); e == nil || e.Error() != want { |
| t.Fatalf("moved error = %v\nwant %q", e, want) |
| } |
| - // Everything that must NOT read as "moved". |
| + // Everything that must NOT read as "moved" — including TEMPORARY |
| + // redirects: 302/307 are what SSO walls answer with, and "log in to |
| + // accounts.google.com" is not a remedy. |
| for name, tc := range map[string]struct { |
| status int |
| location string |
| }{ |
| "non-redirect status": {404, "https://console.example"}, |
| + "302 sso wall": {302, "https://accounts.google.example"}, |
| + "307 temporary": {307, "https://console.example"}, |
| "no location": {308, ""}, |
| "relative location": {308, "/somewhere"}, |
| "same origin": {308, from + "/new-path"}, |
| @@ -3072,11 +3076,19 @@ func TestRedirectsAreNeverFollowed(t *testing.T) { |
| } |
| } |
| +// resetMovedOffer clears the once-per-process offer guard for one test. |
| +func resetMovedOffer(t *testing.T) { |
| + t.Helper() |
| + consoleMovedOffered = false |
| + t.Cleanup(func() { consoleMovedOffered = false }) |
| +} |
| + |
| func TestConsoleMovedOfferLogsInAndRepointsConfig(t *testing.T) { |
| useTempCredentials(t) |
| clearDeploymentEnv(t) |
| out := captureStdout(t) |
| forceTerminal(t, true) |
| + resetMovedOffer(t) |
| fc := newFakeConsole(t) // the console's NEW home — real device flow |
| old := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| w.Header().Set("Location", fc.ts.URL+r.URL.Path) |
| @@ -3106,6 +3118,61 @@ func TestConsoleMovedOfferLogsInAndRepointsConfig(t *testing.T) { |
| if !strings.Contains(out.String(), "this console has moved") { |
| t.Fatalf("the move must be explained:\n%s", out.String()) |
| } |
| + if !strings.Contains(out.String(), "to point at "+fc.ts.URL) { |
| + t.Fatalf("the repoint prompt must name the destination:\n%s", out.String()) |
| + } |
| + |
| + // The offer already ran once this process: a second hit — a polling |
| + // verb's next tick — gets the typed error and NO prompt, even with |
| + // answers queued up. |
| + out.Reset() |
| + chooserIn = strings.NewReader("y\ny\n") |
| + _, _, err = s.getJSON(context.Background(), "/api/cli/apps") |
| + if err == nil || !strings.Contains(err.Error(), "this console has moved") { |
| + t.Fatalf("second hit = %v, want the plain typed error", err) |
| + } |
| + if strings.Contains(out.String(), "[y/N]") { |
| + t.Fatalf("a second prompt fired:\n%s", out.String()) |
| + } |
| +} |
| + |
| +// whoami's memberships line degrades silently even through a MOVED console |
| +// — a bonus line must never turn into a prompt (the quietConsoleOffers |
| +// suppression). |
| +func TestWhoamiStaysSilentThroughAMovedConsole(t *testing.T) { |
| + useTempCredentials(t) |
| + clearDeploymentEnv(t) |
| + captureStdout(t) |
| + forceTerminal(t, true) |
| + resetMovedOffer(t) |
| + mux := http.NewServeMux() |
| + mux.HandleFunc("GET /auth/cli/whoami", func(w http.ResponseWriter, r *http.Request) { |
| + json.NewEncoder(w).Encode(map[string]any{"address": "tom@keymail.dev"}) |
| + }) |
| + mux.HandleFunc("GET /api/cli/apps", func(w http.ResponseWriter, r *http.Request) { |
| + w.Header().Set("Location", "https://console.elsewhere.example/api/cli/apps") |
| + w.WriteHeader(http.StatusPermanentRedirect) |
| + }) |
| + ts := httptest.NewServer(mux) |
| + t.Cleanup(ts.Close) |
| + f, _ := loadCredentials() |
| + f.put(credential{Console: ts.URL, Address: "tom@keymail.dev", Token: "tok", |
| + CreatedAt: time.Now(), ExpiresAt: time.Now().Add(time.Hour)}) |
| + if err := f.save(); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var out bytes.Buffer |
| + if err := authWhoami(&out, []string{"-console", ts.URL}); err != nil { |
| + t.Fatalf("whoami through a moved memberships endpoint: %v", err) |
| + } |
| + if strings.Contains(out.String(), "moved") || strings.Contains(out.String(), "[y/N]") || |
| + strings.Contains(out.String(), "accounts:") { |
| + t.Fatalf("whoami must degrade to identity-only silence:\n%s", out.String()) |
| + } |
| + if consoleMovedOffered { |
| + t.Fatal("whoami consumed the once-per-process offer") |
| + } |
| } |
| // resolveMode on a terminal offers to run the login it would otherwise |
diff --git a/auth.go b/auth.go| index c0b5b5c51..81c66d55f 100644 |
| --- a/auth.go |
| +++ b/auth.go |
| @@ -768,7 +768,10 @@ func authWhoami(out io.Writer, args []string) error { |
| // Memberships are a bonus line, not part of whoami's contract: identity |
| // already printed, and a console from before GET /api/cli/apps (or a |
| // blip fetching it) must not make whoami look broken — so ANY error |
| - // here degrades to silence rather than a message. |
| + // here degrades to silence rather than a message, and the moved-console |
| + // OFFER is suppressed too: a bonus line must never turn into a prompt. |
| + quietConsoleOffers = true |
| + defer func() { quietConsoleOffers = false }() |
| var acc struct { |
| Accounts []apiAccountEntry `json:"accounts"` |
| } |
diff --git a/docs/superpowers/specs/2026-08-12-proactive-cli-design.md b/docs/superpowers/specs/2026-08-12-proactive-cli-design.md| index 9d0b40645..0087b81f1 100644 |
| --- a/docs/superpowers/specs/2026-08-12-proactive-cli-design.md |
| +++ b/docs/superpowers/specs/2026-08-12-proactive-cli-design.md |
| @@ -100,8 +100,9 @@ ed25519 CLI-update keypair (NOT the release key: different blast radius, |
| different rotation story): private half at SSM `/carlos/cli-update-key`, |
| public half a constant in the binary. `SHA256SUMS.txt.sig` = |
| base64(ed25519.Sign(priv, file bytes)) — stdlib verify, no OpenPGP. The |
| -release recipe gains the signing step; v0.7.0's sums are signed |
| -retroactively so the first self-update has something to verify. |
| +release recipe gains the signing step from v0.8.0 onward — the first |
| +binary that verifies is v0.8.0, and it downloads the LATEST release's |
| +sums, so retro-signing v0.7.0's would protect nothing. |
| ## Testing |
diff --git a/main.go b/main.go| index 1c0141a77..57a13f569 100644 |
| --- a/main.go |
| +++ b/main.go |
| @@ -158,7 +158,8 @@ Your apps — ship and run software on a CARLOS deployment |
| ledger open ledgers: append | head | list | publish | blob | verify |
| vet check a release against the platform contract |
| update update this carlos binary to the latest release (or name |
| - the brew/apt command when a package manager owns it) |
| + the brew/apt command when a package manager owns it); |
| + -y skips the prompt, terminal still required |
| Your deployment — stand up and administer boxes, routes and accounts |
| bootstrap prepare a host: units, dirs, user |
diff --git a/update.go b/update.go| index b8fdc13f7..357b8e16f 100644 |
| --- a/update.go |
| +++ b/update.go |
| @@ -33,8 +33,14 @@ import ( |
| // Vars, not consts: the tests point them at httptest servers and a test |
| // keypair. Production never changes them. |
| var ( |
| - updateLatestURL = "https://apt.carlosframework.com/latest.txt" |
| - updateDownloadBase = "https://github.com/carlosframework/releases/releases/latest/download" |
| + updateLatestURL = "https://apt.carlosframework.com/latest.txt" |
| + // Tag-pinned, never "latest/download": the floating alias can serve an |
| + // older authentic release than the one latest.txt promised, and every |
| + // artifact of a self-replace must come from the ONE tag being claimed |
| + // (review 2026-08-12: an authentic-but-old sums+binary pair must not |
| + // be acceptable, or a caching layer can pin workstations to a |
| + // known-vulnerable release without a signature error). |
| + updateDownloadBase = "https://github.com/carlosframework/releases/releases/download" |
| // cliUpdatePubKey verifies SHA256SUMS.txt.sig on self-replace: the |
| // public half of the dedicated CLI-update keypair (private half at SSM |
| @@ -132,21 +138,31 @@ func maybeUpdateHint(cmd string) { |
| } |
| var cache updateCache |
| if raw, err := os.ReadFile(path); err == nil { |
| - if json.Unmarshal(raw, &cache) == nil && time.Since(cache.CheckedAt) < 24*time.Hour { |
| - return |
| + // A future checked_at (clock skew, corrupt write) must re-check, |
| + // not silence the hint until the clock catches up. |
| + if json.Unmarshal(raw, &cache) == nil { |
| + if since := time.Since(cache.CheckedAt); since >= 0 && since < 24*time.Hour { |
| + return |
| + } |
| + } |
| + } |
| + // The attempt is stamped BEFORE the fetch: a blackholed egress must |
| + // cost one command its 1.5s, not every command forever. Latest is |
| + // informational (for a human reading the file) — the hint only ever |
| + // speaks at check time, so a stale value never re-nags. |
| + writeCache := func(latest string) { |
| + if raw, err := json.Marshal(updateCache{CheckedAt: time.Now().UTC(), Latest: latest}); err == nil { |
| + if err := os.MkdirAll(filepath.Dir(path), 0o700); err == nil { |
| + os.WriteFile(path, raw, 0o600) |
| + } |
| } |
| } |
| + writeCache("") |
| latest, err := fetchLatest(1500 * time.Millisecond) |
| if err != nil { |
| return |
| } |
| - // The cache is written even when up to date — "checked, nothing new" |
| - // is exactly what should suppress tomorrow's re-check until due. |
| - if raw, err := json.Marshal(updateCache{CheckedAt: time.Now().UTC(), Latest: latest}); err == nil { |
| - if err := os.MkdirAll(filepath.Dir(path), 0o700); err == nil { |
| - os.WriteFile(path, raw, 0o600) |
| - } |
| - } |
| + writeCache(latest) |
| if newerVersion(latest, version.Build) { |
| mutedf(stdout, "carlos %s is available (you have %s) — run: carlos update", latest, version.Build) |
| } |
| @@ -187,7 +203,10 @@ func configUpdateCheckOff() (bool, error) { |
| // `carlos update` hands package-managed installs to their manager instead |
| // of fighting it. |
| func installMethod(exe string) string { |
| - if strings.Contains(exe, "/Cellar/") || strings.Contains(exe, "/homebrew/") || strings.Contains(exe, "/linuxbrew/") { |
| + // Both brews keep formula binaries under a Cellar (/opt/homebrew/Cellar, |
| + // ~/.linuxbrew/Cellar) — matching looser substrings ("/homebrew/") would |
| + // also match unrelated paths that merely contain the word. |
| + if strings.Contains(exe, "/Cellar/") { |
| return "brew" |
| } |
| if dpkg, err := exec.LookPath("dpkg"); err == nil { |
| @@ -238,13 +257,14 @@ func runUpdate(args []string) { |
| return |
| } |
| fmt.Fprintf(stdout, "carlos %s is available (you have %s); this binary: %s\n", latest, version.Build, exe) |
| - if !*yes { |
| - if !interactiveTerminal() { |
| - log.Fatal("not a terminal — pass -y to replace without asking") |
| - } |
| - if !promptYes(stdout, fmt.Sprintf("download %s and replace it? [y/N] ", latest)) { |
| - return |
| - } |
| + // Non-TTY never self-replaces, -y or not (spec): provisioning belongs |
| + // to package managers and image builds, not to a CLI quietly swapping |
| + // itself in a pipeline. -y only skips the question for a human. |
| + if !interactiveTerminal() { |
| + log.Fatal("carlos update replaces a binary only at a terminal — scripts should install the release directly") |
| + } |
| + if !*yes && !promptYes(stdout, fmt.Sprintf("download %s and replace it? [y/N] ", latest)) { |
| + return |
| } |
| if err := selfReplace(exe, latest); err != nil { |
| if errors.Is(err, os.ErrPermission) { |
| @@ -255,17 +275,19 @@ func runUpdate(args []string) { |
| fmt.Fprintf(stdout, "%s carlos %s -> %s (%s)\n", colorFor(stdout).ok("updated:"), version.Build, latest, exe) |
| } |
| -// selfReplace downloads the matching artifact for this OS/arch, proves it |
| -// against the SIGNED checksum list, and renames it over exe — temp file |
| +// selfReplace downloads the matching artifact for this OS/arch — from the |
| +// TAG-pINNED release path, so the bytes installed are the version claimed — |
| +// proves it against the SIGNED checksum list, runs the replacement's own |
| +// `version` as a final lie detector, and renames it over exe: temp file |
| // beside the target then rename, the same crash-safety rule as the config |
| // writes, so no failure can leave half a binary answering `carlos`. |
| func selfReplace(exe, latest string) error { |
| asset := fmt.Sprintf("carlos-%s-%s", runtime.GOOS, runtime.GOARCH) |
| - sums, err := fetchUpdateFile("SHA256SUMS.txt", 1<<20) |
| + sums, err := fetchUpdateFile(latest, "SHA256SUMS.txt", 1<<20) |
| if err != nil { |
| return err |
| } |
| - sig, err := fetchUpdateFile("SHA256SUMS.txt.sig", 4096) |
| + sig, err := fetchUpdateFile(latest, "SHA256SUMS.txt.sig", 4096) |
| if err != nil { |
| return fmt.Errorf("%v (a release without a signed checksum list predates carlos update)", err) |
| } |
| @@ -287,7 +309,7 @@ func selfReplace(exe, latest string) error { |
| if wantSum == "" { |
| return fmt.Errorf("no %s in the release's checksum list", asset) |
| } |
| - binData, err := fetchUpdateFile(asset, 256<<20) |
| + binData, err := fetchUpdateFile(latest, asset, 256<<20) |
| if err != nil { |
| return err |
| } |
| @@ -310,12 +332,21 @@ func selfReplace(exe, latest string) error { |
| if err := tmp.Close(); err != nil { |
| return err |
| } |
| + // The lie detector: before anything is replaced, the candidate itself |
| + // must report the version this whole exchange promised. Catches a |
| + // re-pointed tag, a mislabeled artifact, and "updated!" prints that |
| + // would otherwise claim a version that never arrived. |
| + if out, err := exec.Command(tmp.Name(), "version").Output(); err != nil || |
| + !strings.Contains(string(out), "carlos "+latest) { |
| + return fmt.Errorf("the downloaded binary reports %q, not %s — refusing to install it", |
| + strings.TrimSpace(string(out)), latest) |
| + } |
| return os.Rename(tmp.Name(), exe) |
| } |
| -func fetchUpdateFile(name string, limit int64) ([]byte, error) { |
| +func fetchUpdateFile(tag, name string, limit int64) ([]byte, error) { |
| client := &http.Client{Timeout: 5 * time.Minute} |
| - res, err := client.Get(updateDownloadBase + "/" + name) |
| + res, err := client.Get(updateDownloadBase + "/" + tag + "/" + name) |
| if err != nil { |
| return nil, err |
| } |
diff --git a/update_test.go b/update_test.go| index 47105e2c1..0a42356aa 100644 |
| --- a/update_test.go |
| +++ b/update_test.go |
| @@ -182,7 +182,14 @@ func releaseServer(t *testing.T, binary []byte, sums string, priv ed25519.Privat |
| } |
| asset := "carlos-" + goosArch() |
| ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| - switch strings.TrimPrefix(r.URL.Path, "/") { |
| + // Downloads are tag-pinned: /vX.Y.Z/<asset>, never a floating |
| + // "latest" alias — anything else here is a client bug. |
| + if !strings.HasPrefix(r.URL.Path, "/v0.8.0/") { |
| + t.Errorf("un-pinned download path %q", r.URL.Path) |
| + http.NotFound(w, r) |
| + return |
| + } |
| + switch strings.TrimPrefix(r.URL.Path, "/v0.8.0/") { |
| case "SHA256SUMS.txt": |
| w.Write([]byte(sums)) |
| case "SHA256SUMS.txt.sig": |