The build probe an existing app could never get, and a deploy that says
what it is watching
Discussion #13: `carlos deploy` printed "live" and exited 0 while the
previous binary was still serving. Every platform-side signal agreed,
because they all describe the ADOPTED release rather than the running
process — the edge stamps X-Carlos-Version from the route row the moment
adoption moves a symlink, whether or not anything restarted. The
instance had also silently missed an earlier adoption hours before, so
the box had been two releases stale with nothing reporting a problem.
The platform already had the answer. An app may declare a BUILD PROBE —
a path whose body names the build actually running — and the deploy
watch polls the app's own process instead of the header. That closes
exactly this gap, and #13's first suggestion is a description of it.
It could only be set at CLAIM time. SetAppBuildProbe had precisely one
caller, handleAPICreateApp, so an app created before the flag existed —
or one whose author did not think to pass it, which is most of them —
could never acquire a probe. The fix was unreachable by exactly the apps
that had the bug. That is the actual defect, and it is what this closes:
carlos apps build-probe --app docs /api/version
carlos apps build-probe --app docs - # back to the header
- POST /api/cli/apps/{account}/{app}/build-probe, registered with an
EMPTY role set and added to closedDoors AND policyDoors with the
reason: the probe is what the deploy watch trusts INSTEAD of the
header, so a credential able to rewrite it is a credential able to
make its own deploys report success over a process that never cycled —
the precise failure the probe exists to prevent, handed to the wrong
caller. A person sets it once.
- The stored value is read back and echoed, and the CLI compares it, the
same discipline `apps create` already applies to this field. Its whole
job is to be trusted.
- "-" clears, the sentinel `--unit` and `--environment` use, because ""
is a LEGAL value here (it means header-watching) and an empty argument
must not be mistaken for one nobody meant to pass.
- The shape is validated at the write door too, not only where deploy
reads it: the watch concatenates probe onto an origin, so
"@evil.example/x" would poll a machine of someone else's choosing and
report ITS build as this deploy's.
And the silence that made the bug survivable is gone. A deploy with no
probe now says, while it waits, that the header reports the release
adopted rather than the build running, and names the one command that
fixes it for that app. It already exits non-zero on a watch timeout;
what it could not do was tell you that a SUCCESS meant less than it
looked.
Also `carlos deploy --label/--notes`, filed in the same discussion: a
deploy is a ship plus a promote, the ship half has carried both since it
existed, and deploy passed "" — so `--label` failed with "flag provided
but not defined" and dumped usage. runShipAPI already took both
arguments; nothing below the flag had to change.
Not fixed here, and worth its own decision: the fire-and-forget restart
arms. adoptHost stamps Version after a restart callback returns nil, and
arms 2 and 3 (carlos-app@ unit tenants, exec children) swallow their
errors deliberately, so one tenant's failed cycle cannot poison a
release for the rest of the pass. That is the right trade for the pass
and the wrong one for the stamp, and the activator's 90s convergence
sweep is the only backstop — with two silent failure modes of its own
(a persistent symlink read error, or a backend with no LiveVersion,
both of which return "do not cycle" forever without a log line).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVvK41AUKXndpcy5KkUZRS
9 files changed,
+371
−10
apimode.go+32 −0apps.go+13 −1apps_buildprobe.go+97 −0deploy.go+23 −2docs/site/cli.md+25 −4internal/console/apibuildprobe.go+77 −0internal/console/apibuildprobe_test.go+88 −0internal/console/authcli.go+6 −0internal/console/svcroutes_test.go+10 −3
diff --git a/apimode.go b/apimode.go| index 0267d6b43..9650bccf4 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -3130,6 +3130,38 @@ func (s *apiSession) place(ctx context.Context, a apiAcct, app, target string) e |
| return nil |
| } |
| +// setBuildProbe stamps (or, with "", clears) an existing app's build-probe |
| +// path — the path `carlos deploy` polls INSTEAD of the X-Carlos-Version |
| +// header, which reports the adopted release rather than the running process. |
| +// |
| +// It returns what the console echoes back, which is what the store actually |
| +// holds: this is the field the deploy watch trusts, so a write that silently |
| +// failed has to be a loud error rather than a quiet lie. Same discipline the |
| +// claim door's own echo has. |
| +func (s *apiSession) setBuildProbe(ctx context.Context, a apiAcct, app, probe string) (string, error) { |
| + in := struct { |
| + BuildProbe string `json:"build_probe"` |
| + }{probe} |
| + status, body, err := s.postJSON(ctx, apiPath(a, app, "/build-probe"), in) |
| + if err != nil { |
| + return "", err |
| + } |
| + if status != http.StatusOK { |
| + if status == http.StatusNotFound { |
| + return "", fmt.Errorf("not found — either %s is not an app in this account, "+ |
| + "or this console predates `carlos apps build-probe` (deploy a console built from current main)", app) |
| + } |
| + return "", apiErrorFromBody(status, body) |
| + } |
| + var out struct { |
| + BuildProbe string `json:"build_probe"` |
| + } |
| + if err := json.Unmarshal(body, &out); err != nil { |
| + return "", fmt.Errorf("apps build-probe: unreadable response: %w", err) |
| + } |
| + return out.BuildProbe, nil |
| +} |
| + |
| // ---- service credentials (services.go's `carlos services` verbs) --------- |
| // servicePath builds one /api/cli/services/{account}... path — the service |
diff --git a/apps.go b/apps.go| index 6efc462d9..558e0f41e 100644 |
| --- a/apps.go |
| +++ b/apps.go |
| @@ -34,7 +34,7 @@ import ( |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| -const appsUsage = `usage: carlos apps <create|place|delete|restore> [flags] <app> |
| +const appsUsage = `usage: carlos apps <create|place|build-probe|delete|restore> [flags] <app> |
| create --app <name> [flags] claim a new app name in an account, through |
| the console API — the same claim the console's New app |
| @@ -73,6 +73,16 @@ const appsUsage = `usage: carlos apps <create|place|delete|restore> [flags] <app |
| silent clear (a fleet box retires an app's routes on its |
| next reconcile the moment placement reads empty). |
| + build-probe --app <name> </path|-> set (or clear, with "-") the path whose |
| + response body names the RUNNING build, on an app that |
| + already exists. "carlos deploy" polls it instead of the |
| + X-Carlos-Version header, which the edge stamps from the |
| + route row and which therefore reports the new build before |
| + the process has actually restarted — the whole reason a |
| + deploy can print "live" over an old binary. Settable only |
| + at claim time until now, which meant the fix was out of |
| + reach of every app that already had the problem. |
| + |
| delete [flags] <app> move an app to the Trash: every route drops |
| (custom domains included), its name stays reserved, and it |
| is restorable for 30 days. Refused while the app is |
| @@ -121,6 +131,8 @@ func RunApps(args []string) { |
| runAppsCreate(args[1:]) |
| case "place": |
| runAppsPlace(args[1:]) |
| + case "build-probe": |
| + runAppsBuildProbe(args[1:]) |
| case "delete": |
| runAppsDelete(args[1:]) |
| case "restore": |
diff --git a/apps_buildprobe.go b/apps_buildprobe.go| new file mode 100644 |
| index 000000000..3bd7f9215 |
| --- /dev/null |
| +++ b/apps_buildprobe.go |
| @@ -0,0 +1,97 @@ |
| +package main |
| + |
| +// `carlos apps build-probe` — set, on an app that already exists, the path |
| +// `carlos deploy` polls to learn which build is ACTUALLY running. |
| +// |
| +// The probe already existed and could only be stamped at claim time: |
| +// SetAppBuildProbe had exactly one caller, the create door. So every app made |
| +// before the flag existed, and every app whose author did not think to pass |
| +// it, was permanently stuck on header-watching — the X-Carlos-Version header |
| +// the edge stamps from the route row the moment adoption moves a symlink, |
| +// whether or not any process restarted. Which means the fix for "deploy said |
| +// live over the old binary" was out of reach of exactly the apps that had the |
| +// problem (discussion #13). |
| + |
| +import ( |
| + "context" |
| + "flag" |
| + "fmt" |
| + "log" |
| + "strings" |
| +) |
| + |
| +const buildProbeUsage = `usage: carlos apps build-probe --app <name> </path|-> [--account <sqid|name>] [--account-id <sqid>] [--console <origin>] |
| + |
| + </path> an absolute path, no query, whose response body names the running |
| + build — "/healthz" serving "ok abc1234" is the shape. carlos deploy |
| + polls it instead of the X-Carlos-Version header, which reports the |
| + release adoption INTENDED rather than the build actually serving. |
| + - clear the probe, back to watching the header.` |
| + |
| +// runAppsBuildProbe stamps the probe path, or clears it. |
| +// |
| +// "-" is the clear sentinel, the same one `carlos route --unit` and |
| +// `--environment` use and for the same reason: "" is a LEGAL value of this |
| +// field — it means header-watching — so an empty argument cannot be told from |
| +// an argument nobody meant to pass. The doors that clear say so out loud. |
| +// |
| +// The stored value is echoed back and compared rather than assumed, the same |
| +// discipline `apps create` applies to this exact field. It is what the deploy |
| +// watch trusts; a write that half-landed would leave a deploy quietly |
| +// believing a header again, which is the whole bug. |
| +func runAppsBuildProbe(args []string) { |
| + fs := flag.NewFlagSet("apps build-probe", flag.ExitOnError) |
| + app := appFlag(fs, "app whose build probe to set") |
| + acctRaw := fs.String("account", "", "account (sqid or name) the app belongs to (default: your only account, or the configured/chosen one)") |
| + acctIDRaw := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes --account") |
| + consoleFlag := fs.String("console", "", "console to act through (default: the only logged-in one)") |
| + fs.Usage = flagUsage(fs, buildProbeUsage) |
| + fs.Parse(args) |
| + if strings.TrimSpace(*app) == "" || fs.NArg() != 1 { |
| + log.Fatal(buildProbeUsage) |
| + } |
| + if err := validAppName(strings.TrimSpace(*app)); err != nil { |
| + log.Fatalf("apps build-probe: %v", err) |
| + } |
| + probe := strings.TrimSpace(fs.Arg(0)) |
| + clearProbe := probe == "-" |
| + if clearProbe { |
| + probe = "" |
| + } else { |
| + if probe == "" { |
| + log.Fatal(`apps build-probe: an empty probe is refused — pass "-" to clear it back to header-watching, or name the path`) |
| + } |
| + // Checked here, before the call, the same client-side grammar gate |
| + // `apps create` applies: a shape the server is certain to refuse |
| + // should cost no round trip and reach nothing. |
| + if err := validBuildProbe(probe); err != nil { |
| + log.Fatalf("apps build-probe: %v", err) |
| + } |
| + } |
| + |
| + sess, err := resolveMode(*consoleFlag) |
| + if err != nil { |
| + log.Fatal(err) |
| + } |
| + if sess == nil { |
| + log.Fatal("apps build-probe works through a console, and this shell is in direct bucket mode\n" + |
| + "(CARLOS_DEPLOYMENT_BUCKET/CARLOS_DEPLOYMENT_DIR is set). The probe is console-tracked\n" + |
| + "state on the app row: log in instead — carlos auth login --console <url>") |
| + } |
| + acct := resolvedAccount(sess, acctRaw, acctIDRaw) |
| + got, err := sess.setBuildProbe(context.Background(), acct, strings.TrimSpace(*app), probe) |
| + if err != nil { |
| + log.Fatalf("apps build-probe: %v", err) |
| + } |
| + if got != probe { |
| + log.Fatalf("apps build-probe: did not stick (console echoed %q, want %q) — the probe is what "+ |
| + "carlos deploy trusts, so a write that half-landed is worse than none", got, probe) |
| + } |
| + if clearProbe { |
| + fmt.Fprintf(stdout, "cleared the build probe for %s — carlos deploy watches the X-Carlos-Version header again\n", strings.TrimSpace(*app)) |
| + fmt.Fprintf(stdout, " that header is stamped by the edge from the route row, so it reports the release adoption intended, not the build serving\n") |
| + return |
| + } |
| + fmt.Fprintf(stdout, "build probe for %s: %s\n", strings.TrimSpace(*app), got) |
| + fmt.Fprintf(stdout, " carlos deploy now asks the app's own process which build it is running, instead of trusting the router's header\n") |
| +} |
diff --git a/deploy.go b/deploy.go| index 0f664742e..8c1d5902b 100644 |
| --- a/deploy.go |
| +++ b/deploy.go |
| @@ -45,11 +45,21 @@ func runDeploy(args []string) { |
| channelFlag := channelFlag(fs, "channel to promote to (default: resolved from the app's instances, or its one channel if it has no instances; error if ambiguous)") |
| hostFlag := fs.String("host", "", "serve on this hostname instead of the app's own platform host (also scopes channel resolution and the watch to it)") |
| timeout := fs.Duration("timeout", defaultDeployTimeout, "how long to watch the URL before failing") |
| + // A deploy IS a ship plus a promote, and the ship half has carried |
| + // --label and --notes since it existed — they are the line a person |
| + // doing a rollback at 2am reads. Deploy passed "" for both, so |
| + // `carlos deploy --label "..."` failed with "flag provided but not |
| + // defined" and dumped usage, and the only way to label a release was to |
| + // stop using the one-command form (discussion #13, filed alongside the |
| + // watch problem). Nothing had to change below: runShipAPI already takes |
| + // both. |
| + label := fs.String("label", "", "one line describing this release, for carlos releases and the console") |
| + notes := fs.String("notes", "", "longer release notes") |
| fs.Usage = flagUsage(fs, "") |
| fs.Parse(args) |
| usage := "usage: carlos deploy --app <a> [--version <sha>] [--kind binary|static] [--target linux-arm64] " + |
| "[--account <sqid-or-name>] [--account-id <sqid>] [--console <origin>] [--capabilities <file.json>] " + |
| - "[--channel <ch>] [--host <host>] [--timeout 120s] <files-or-site-dir>\n" + |
| + "[--channel <ch>] [--host <host>] [--label <text>] [--notes <text>] [--timeout 120s] <files-or-site-dir>\n" + |
| "or, in a project with saved defaults (or at a terminal, to create them): carlos deploy" |
| if *ver == "" { |
| *ver = gitShortHead() |
| @@ -235,7 +245,7 @@ func runDeploy(args []string) { |
| // printBanner=false: the "using ..." line already printed above, before |
| // the instance list — so a listing failure still names the console — |
| // and runShipAPI would otherwise print it a second time. |
| - shipRes, err := runShipAPI(sess, acct, fs, *app, *ver, *kind, *target, "", "", capsSet, false) |
| + shipRes, err := runShipAPI(sess, acct, fs, *app, *ver, *kind, *target, *label, *notes, capsSet, false) |
| if err != nil { |
| log.Fatalf("deploy: ship: %v", err) |
| } |
| @@ -314,6 +324,17 @@ func runDeploy(args []string) { |
| } |
| if probe != "" { |
| mutedf(stdout, "watching %s for the build %s reports itself\n", probe, *app) |
| + } else { |
| + // Said out loud, every time, because the silence is what the bug was |
| + // made of (discussion #13). With no probe this watch can only ask |
| + // the ROUTER what release was adopted; it cannot ask the process |
| + // what it is running. A deploy that goes green here has proved that |
| + // adoption happened, not that anything restarted — and an app whose |
| + // instance failed to cycle looks identical. It is a one-line fix |
| + // per app, so the line names it rather than describing the problem |
| + // and leaving the reader to find the verb. |
| + mutedf(stdout, "watching the X-Carlos-Version header: it reports the release ADOPTED, not the build running\n") |
| + mutedf(stdout, " a probe proves the process actually cycled — carlos apps build-probe --app %s /api/version\n", *app) |
| } |
| targets := make([]watchTarget, 0, len(hosts)) |
| for _, h := range hosts { |
diff --git a/docs/site/cli.md b/docs/site/cli.md| index 22561c166..7c2997622 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -84,6 +84,19 @@ the route row and which therefore reports the new build before the process has |
| actually restarted. Both are checked against what the console echoes back, so a |
| write that silently failed is an error rather than a quiet lie. |
| +`carlos apps build-probe` sets that same probe on an app that already exists, |
| +which is the common case: most apps were claimed before anyone thought about |
| +it. |
| + |
| +```sh |
| +carlos apps build-probe --app docs /api/version |
| +``` |
| + |
| +Pass `-` instead of a path to clear it and go back to watching the header. It |
| +is worth setting on anything you deploy often. Without it, a green deploy tells |
| +you the release was adopted, not that the process running the old one ever |
| +restarted — and those look identical from outside. |
| + |
| ### carlos ship |
| Publishes a release. The artifact is hashed, stored under its content |
| @@ -133,10 +146,18 @@ and the console refuses it by name, so the flag never quietly does nothing. |
| ### carlos deploy |
| Ship, promote, and then wait until the app's URL actually serves the new |
| -build. It watches the `X-Carlos-Version` header on the live host and does not |
| -report success until that header names what you shipped, over a 2xx. Reach |
| -for this one by default; ship and promote separately when you want the gap |
| -between them. |
| +build. Reach for this one by default; ship and promote separately when you |
| +want the gap between them. |
| + |
| +What it watches is worth knowing, because the two answers differ. If the app |
| +declares a build probe, deploy asks the app's own process which build it is |
| +running, and `live` means the process cycled. If it does not, deploy watches |
| +the `X-Carlos-Version` header — and the edge stamps that from the route row |
| +the moment adoption moves the symlink, whether or not anything restarted. So |
| +a probe-less deploy can print `live` over a process still running the old |
| +binary, and it says so while it waits. Set one with |
| +[carlos apps build-probe](/docs/cli#carlos-apps); it is a single command per |
| +app. |
| ```sh |
| carlos deploy --app hello ./hello |
diff --git a/internal/console/apibuildprobe.go b/internal/console/apibuildprobe.go| new file mode 100644 |
| index 000000000..b2bb42f54 |
| --- /dev/null |
| +++ b/internal/console/apibuildprobe.go |
| @@ -0,0 +1,77 @@ |
| +// The build probe, settable on an app that already exists (discussion #13). |
| +// |
| +// `carlos deploy` printed "live" and exited 0 while the previous binary was |
| +// still serving, and every platform-side signal agreed with it, because they |
| +// all describe the ADOPTED release rather than the running process: the edge |
| +// stamps X-Carlos-Version from the route row the moment adoption moves the |
| +// symlink, whether or not anything restarted. |
| +// |
| +// The platform already has the answer to that. An app may declare a BUILD |
| +// PROBE — a path whose response body names the build actually running — and |
| +// the deploy watch polls the app's own process instead of the header |
| +// (deploy.go's watchTarget.Probe). It closes exactly the gap #13 describes. |
| +// |
| +// It could only be set at CLAIM time. `SetAppBuildProbe` had precisely one |
| +// caller, handleAPICreateApp, so every app created before the feature existed |
| +// — or created without the flag, which is every app anyone forgot it for — |
| +// could never acquire one. The fix for the bug was unreachable by the apps |
| +// that had the bug. That is what this door is for. |
| +// |
| +// GATE: apiAccountApp proves membership, and the route is registered with an |
| +// EMPTY role set — no service credential of any role reaches it. This is a |
| +// policy door in the sense svcroutes_test.go means: the probe is what the |
| +// deploy watch TRUSTS, so a credential able to rewrite it is a credential |
| +// able to make its own deploys report success. A person sets it once. |
| +package console |
| + |
| +import ( |
| + "net/http" |
| + "strings" |
| + "time" |
| +) |
| + |
| +func (s *Server) handleAPIBuildProbe(w http.ResponseWriter, r *http.Request) { |
| + addr, acct, app, ok := s.apiAccountApp(w, r) |
| + if !ok { |
| + return |
| + } |
| + p, _ := principalFrom(r.Context()) |
| + if !s.allowWrite(p, time.Now()) { |
| + jsonError(w, http.StatusTooManyRequests, "too many requests — wait a minute and try again") |
| + return |
| + } |
| + var req struct { |
| + BuildProbe string `json:"build_probe"` |
| + } |
| + if !readJSON(w, r, &req) { |
| + return |
| + } |
| + // "" is a legal value: it clears the probe back to header-watching, the |
| + // same meaning SetAppBuildProbe's own doc comment gives it. The CLI's |
| + // clear sentinel is "-", resolved client-side, so nothing here has to |
| + // guess whether an empty body meant "clear" or "not mentioned". |
| + probe := strings.TrimSpace(req.BuildProbe) |
| + if probe != "" && !validBuildProbe(probe) { |
| + jsonError(w, http.StatusBadRequest, |
| + `build_probe must be an absolute URL path with no query, e.g. "/healthz"`) |
| + return |
| + } |
| + if err := s.store.SetAppBuildProbe(acct.ID, app, probe); err != nil { |
| + jsonError(w, http.StatusInternalServerError, "could not save the build probe — try again") |
| + return |
| + } |
| + // Read back and echo what was actually STORED, exactly as the claim door |
| + // does: a write that silently failed must be a loud error rather than a |
| + // quiet lie, and this is the field whose whole job is to be trusted. |
| + _, got, err := s.store.AppFlags(acct.ID, app) |
| + if err != nil { |
| + jsonError(w, http.StatusInternalServerError, "saved, but could not read the build probe back — check it with carlos apps") |
| + return |
| + } |
| + detail := "cleared the build probe for " + app + " (cli)" |
| + if got != "" { |
| + detail = "set the build probe for " + app + " to " + got + " (cli)" |
| + } |
| + s.audit(acct, app, addr, "build-probe", detail) |
| + writeJSON(w, http.StatusOK, map[string]any{"build_probe": got}) |
| +} |
diff --git a/internal/console/apibuildprobe_test.go b/internal/console/apibuildprobe_test.go| new file mode 100644 |
| index 000000000..2823bab85 |
| --- /dev/null |
| +++ b/internal/console/apibuildprobe_test.go |
| @@ -0,0 +1,88 @@ |
| +package console |
| + |
| +import ( |
| + "encoding/json" |
| + "net/http" |
| + "testing" |
| + "time" |
| +) |
| + |
| +// TestAPIBuildProbeSetsAndClearsOnAnExistingApp is the whole point of the |
| +// door. Until it existed, SetAppBuildProbe had exactly one caller — the claim |
| +// door — so an app created without the flag could never acquire a probe, and |
| +// the fix for "deploy reported live over the old binary" was unreachable by |
| +// every app that already had the problem (discussion #13). |
| +func TestAPIBuildProbeSetsAndClearsOnAnExistingApp(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-buildprobe") |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "docs", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatalf("create app: %v", err) |
| + } |
| + |
| + probeOf := func() string { |
| + t.Helper() |
| + _, probe, err := fx.st.AppFlags(OpsAccountID, "docs") |
| + if err != nil { |
| + t.Fatalf("AppFlags: %v", err) |
| + } |
| + return probe |
| + } |
| + if got := probeOf(); got != "" { |
| + t.Fatalf("a fresh app already carries a probe: %q", got) |
| + } |
| + |
| + u := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/docs/build-probe" |
| + status, _, body := rawReq(t, http.MethodPost, u, token, `{"build_probe":"/api/version"}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("set = %d %q", status, body) |
| + } |
| + var out struct { |
| + BuildProbe string `json:"build_probe"` |
| + } |
| + if err := json.Unmarshal([]byte(body), &out); err != nil { |
| + t.Fatalf("unreadable: %v: %s", err, body) |
| + } |
| + // The echo is the contract: this is the field the deploy watch trusts |
| + // instead of the header, so a write that silently failed has to be a loud |
| + // error rather than a quiet lie. Response and store both. |
| + if out.BuildProbe != "/api/version" { |
| + t.Errorf("echo = %q, want /api/version", out.BuildProbe) |
| + } |
| + if got := probeOf(); got != "/api/version" { |
| + t.Errorf("stored = %q, want /api/version", got) |
| + } |
| + |
| + // "" clears back to header-watching — a legal value of this field, not a |
| + // missing one. |
| + status, _, body = rawReq(t, http.MethodPost, u, token, `{"build_probe":""}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("clear = %d %q", status, body) |
| + } |
| + if got := probeOf(); got != "" { |
| + t.Errorf("after clear, stored = %q, want empty", got) |
| + } |
| +} |
| + |
| +// TestAPIBuildProbeRefusesABadShape: the watch builds "https://" + host + |
| +// probe, so a probe not starting with "/" turns the poll into a request to a |
| +// machine of somebody else's choosing, which would then report ITS build as |
| +// this deploy's. Refused at the write door, not only where it is read. |
| +func TestAPIBuildProbeRefusesABadShape(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-buildprobebad") |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "docs", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatalf("create app: %v", err) |
| + } |
| + |
| + u := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/docs/build-probe" |
| + for _, bad := range []string{"@evil.example/x", "api/version", "/x?y=1", "https://evil.example/x"} { |
| + payload, _ := json.Marshal(map[string]string{"build_probe": bad}) |
| + status, _, body := rawReq(t, http.MethodPost, u, token, string(payload)) |
| + if status != http.StatusBadRequest { |
| + t.Errorf("probe %q = %d %q, want 400", bad, status, body) |
| + } |
| + } |
| + if _, probe, _ := fx.st.AppFlags(OpsAccountID, "docs"); probe != "" { |
| + t.Errorf("a refused probe still landed: %q", probe) |
| + } |
| +} |
diff --git a/internal/console/authcli.go b/internal/console/authcli.go| index 232143bbb..2d1243302 100644 |
| --- a/internal/console/authcli.go |
| +++ b/internal/console/authcli.go |
| @@ -711,6 +711,12 @@ func (s *Server) registerCLIRoutes() { |
| // Placement lives under the apps family, not fleets: its subject is an |
| // APP, and a target fleet name is only ever the request body's data. |
| s.cliRoute("POST /api/cli/apps/{account}/{app}/place", svcrole.Allow(svcrole.Admin), s.handleAPIPlace) |
| + // The build probe on an EXISTING app (discussion #13). An empty role set: |
| + // the probe is what the deploy watch trusts instead of the header, so a |
| + // credential that could rewrite it could make its own deploys report |
| + // success over a process that never restarted — which is the exact bug |
| + // the probe exists to prevent, handed to the wrong caller. |
| + s.cliRoute("POST /api/cli/apps/{account}/{app}/build-probe", svcrole.Set{}, s.handleAPIBuildProbe) |
| // The object-storage API (apistore.go, issue #55) — declare a store, read |
| // what became of it, and drive the walks that make it real. Beside the |
| // config routes on purpose: a store's credentials are DELIVERED as config, |
diff --git a/internal/console/svcroutes_test.go b/internal/console/svcroutes_test.go| index 2c3fb99a4..0f3d30e82 100644 |
| --- a/internal/console/svcroutes_test.go |
| +++ b/internal/console/svcroutes_test.go |
| @@ -67,7 +67,7 @@ import ( |
| var ( |
| mintDoors = []string{"/instance-token", "/api/cli/services/", "/email/credentials"} |
| fleetDoors = []string{"/api/cli/fleets/"} |
| - policyDoors = []string{"/instances-manifest", "/{app}/routes", "/email/pause"} |
| + policyDoors = []string{"/instances-manifest", "/{app}/routes", "/email/pause", "/build-probe"} |
| ) |
| // closedDoors is the exact set of /api/cli/ patterns registered with the zero |
| @@ -130,6 +130,13 @@ var closedDoors = []string{ |
| // whose rules it never met. Shut for the same reason the manifest door |
| // is — an app may not rewrite the bounds it runs inside. |
| "POST /api/cli/apps/{account}/{app}/instances/{host}/channel", |
| + // The build probe is what `carlos deploy`'s watch TRUSTS in place of the |
| + // X-Carlos-Version header — the header being the thing that reports a |
| + // release before any process has restarted. A credential that could |
| + // repoint the probe could therefore make its own deploys report success |
| + // over a process that never cycled, which is the precise failure the |
| + // probe exists to prevent. Setting it is a person's one-time act. |
| + "POST /api/cli/apps/{account}/{app}/build-probe", |
| } |
| // TestEveryCLIDoorNamesItsRoles is the default-deny guarantee for the |
| @@ -686,8 +693,8 @@ func TestEveryClosedDoorRefusesAServiceCredential(t *testing.T) { |
| } |
| seen++ |
| } |
| - if seen != len(closedDoors) || seen != 17 { |
| - t.Errorf("walked %d closed doors, want all %d (and 17 today: instance-token, the routes door, instances-manifest, the route-channel move, six fleets, four services, two email credentials, email pause)", |
| + if seen != len(closedDoors) || seen != 18 { |
| + t.Errorf("walked %d closed doors, want all %d (and 18 today: instance-token, the routes door, instances-manifest, the route-channel move, six fleets, four services, two email credentials, email pause, the build probe)", |
| seen, len(closedDoors)) |
| } |
| } |