amadan

carlos / platform Public

deploy credential: final review fixes (#229)

F-1: carlosTestEnv (both copies) drops an inherited CARLOS_SERVICE_TOKEN so subprocess tests do not hit the token-plus-store refusal.
F-2: handleAPIRollback gives a deploy credential the D22 refusal for stable instead of the legacy passkey text.
F-3: test pinning that the promote door ignores fast_track/fastTrack from a deploy credential.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
claude 93d75f8704ae2f463943f3ccce5ac050dd7cc947 parent e84365b
6 files changed, +129 −2
  • .superpowers/sdd/2026-10-05-deploy-credential/final-fix-report.md +32 −0
  • e2e_test.go +11 −1
  • internal/console/apicli.go +11 −0
  • internal/console/deployscope_test.go +39 −0
  • internal/console/deploysign_test.go +25 −0
  • scopedkeys_e2e_test.go +11 −1
diff --git a/.superpowers/sdd/2026-10-05-deploy-credential/final-fix-report.md b/.superpowers/sdd/2026-10-05-deploy-credential/final-fix-report.md
new file mode 100644
index 000000000..64cc061fc
--- /dev/null
+++ b/.superpowers/sdd/2026-10-05-deploy-credential/final-fix-report.md
@@ -0,0 +1,32 @@
+# Final whole-branch review fixes (#229)
+
+## F-1 subprocess tests inherit CARLOS_SERVICE_TOKEN
+Both `carlosTestEnv` helpers (e2e_test.go, scopedkeys_e2e_test.go) now build the child env by filtering any
+`CARLOS_SERVICE_TOKEN=` entry out of `os.Environ()` before appending the test overrides. A token a test passes
+in `extra` still wins. No production code changed.
+Covering: every subprocess test using carlosTestEnv, notably TestScopedKeysCLI and TestRestartSelectorRefusals
+(sets CARLOS_DEPLOYMENT_DIR).
+Command: `CARLOS_SERVICE_TOKEN=dummy go test . -run 'TestScopedKeysCLI|TestRestartSelectorRefusals|TestE2E|TestAuth' -count=1`
+Output: `ok github.com/carlosframework/platform 30.068s`
+Note: the whole root package with the token exported still fails, in TestRunDomainsAttachPrintsAttached, an
+in-process run() test (not a subprocess helper) that exits with "CARLOS_SERVICE_TOKEN is set but no console is
+named". Not touched, outside this finding. Without the token the whole package passes.
+
+## F-2 deploy rollback of stable got the legacy refusal
+internal/console/apicli.go handleAPIRollback: immediately before the legacy stable block, when the channel is
+stable, `deployScopeAllows(p, false, channel, false)` runs and a refusal goes through `writeDeployRefusal`.
+It returns nil for every other principal, so people and publish credentials get the legacy 403 unchanged. The
+later full check after the pipeline read is unchanged.
+Test: TestDeployRollbackOfStableIsD22 (deployscope_test.go): deploy gets 403 with
+"deploy credential ci may move canary/<name>, edge only, and stable is not one of them." (no console_url),
+stable pointer still absent; person gets the legacy text with console_url.
+
+## F-3 fast-track regression
+TestDeployPromoteCannotFastTrack (deploysign_test.go): promote with `fast_track:true` and `fastTrack:true` as a
+deploy credential; both promote (200) and the verified edge pointer payload has FastTrack=false.
+
+## Runs (TMPDIR=/tmp/claude/fx, GOCACHE from original TMPDIR)
+- focused console tests: ok
+- `go test ./internal/console/ -count=1`: ok .../internal/console 50.510s
+- `go test . -count=1`: ok .../platform 225.343s
+- gofmt -l: clean
diff --git a/e2e_test.go b/e2e_test.go
index 268f42903..977a38e5f 100644
--- a/e2e_test.go
+++ b/e2e_test.go
@@ -82,7 +82,17 @@ func e2eGet(t *testing.T, base, host, path string) (int, string) {
func carlosTestEnv(t *testing.T, extra ...string) []string {
t.Helper()
scratch := t.TempDir()
- env := append(os.Environ(),
+ // An inherited CARLOS_SERVICE_TOKEN (a CI shell that exports one) would
+ // make any child that also sets CARLOS_DEPLOYMENT_DIR fail resolveMode's
+ // token-plus-store refusal, so it is dropped here. It is filtered rather
+ // than blanked: a test that passes its own token in extra still wins.
+ var env []string
+ for _, kv := range os.Environ() {
+ if !strings.HasPrefix(kv, "CARLOS_SERVICE_TOKEN=") {
+ env = append(env, kv)
+ }
+ }
+ env = append(env,
"CARLOS_CREDENTIALS="+filepath.Join(scratch, "credentials"),
"HOME="+scratch,
"CARLOS_CONSOLE=",
diff --git a/internal/console/apicli.go b/internal/console/apicli.go
index 17021d17d..5dba6cde8 100644
--- a/internal/console/apicli.go
+++ b/internal/console/apicli.go
@@ -1029,6 +1029,17 @@ func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) {
jsonError(w, http.StatusBadRequest, "invalid channel name")
return
}
+ // A deploy credential never gets the legacy stable text below: it is not
+ // a passkey it lacks, it is a rung it does not hold (spec §7.3, D22).
+ // This early call only ever produces a refusal; for stable
+ // deployScopeAllows refuses whatever the pipeline says, and the full check
+ // after the pipeline read stays as it is. Every other principal gets nil.
+ if rung, _ := release.Rung(channel); rung == release.RungStable {
+ if err := deployScopeAllows(p, false, channel, false); err != nil {
+ s.writeDeployRefusal(w, err)
+ return
+ }
+ }
if rung, _ := release.Rung(channel); rung == release.RungStable {
writeJSON(w, http.StatusForbidden, map[string]string{
"error": "stable needs a passkey — finish in the console",
diff --git a/internal/console/deployscope_test.go b/internal/console/deployscope_test.go
index 292710d12..298a5d443 100644
--- a/internal/console/deployscope_test.go
+++ b/internal/console/deployscope_test.go
@@ -140,3 +140,42 @@ func TestDeployScopeRefusalsAtTheDoors(t *testing.T) {
t.Errorf("rollback beta: channel moved %q -> %q", before, after)
}
}
+
+// TestDeployRollbackOfStableIsD22: a deploy credential rolling back stable
+// gets the D22 refusal naming it and its rungs, not the legacy passkey text,
+// and stable is left alone. A person's rollback of stable keeps the legacy
+// answer (the early deploy check returns nil for every other principal).
+func TestDeployRollbackOfStableIsD22(t *testing.T) {
+ fx, person, deploy := deployFixture(t)
+ stableAbsent := func() {
+ t.Helper()
+ if _, _, err := release.GetQualified(context.Background(), fx.rel, release.ChannelKey("bab", "hello", "stable")); err == nil {
+ t.Fatal("stable pointer exists, want it untouched (absent)")
+ }
+ }
+
+ status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/hello/rollback", map[string]any{"channel": "stable"}, deploy)
+ want := "deploy credential ci may move " + deployHeldWords([]string{"canary", "edge"}) + " only, and stable is not one of them."
+ if status != http.StatusForbidden {
+ t.Fatalf("deploy rollback stable = %d %v, want 403", status, out)
+ }
+ if got, _ := out["error"].(string); got != want {
+ t.Errorf("deploy rollback stable error = %q, want %q", got, want)
+ }
+ if _, has := out["console_url"]; has {
+ t.Errorf("deploy refusal carries the legacy console_url: %v", out)
+ }
+ stableAbsent()
+
+ status, out = postJSON(t, fx.ts.URL+"/api/cli/apps/bab/hello/rollback", map[string]any{"channel": "stable"}, person)
+ if status != http.StatusForbidden {
+ t.Fatalf("person rollback stable = %d %v, want 403", status, out)
+ }
+ if got, _ := out["error"].(string); got != "stable needs a passkey — finish in the console" {
+ t.Errorf("person rollback stable error = %q, want the legacy text", got)
+ }
+ if _, has := out["console_url"]; !has {
+ t.Errorf("person refusal lost its console_url: %v", out)
+ }
+ stableAbsent()
+}
diff --git a/internal/console/deploysign_test.go b/internal/console/deploysign_test.go
index 5ffa6df1d..94472451e 100644
--- a/internal/console/deploysign_test.go
+++ b/internal/console/deploysign_test.go
@@ -224,3 +224,28 @@ func TestDeployRollback(t *testing.T) {
t.Fatalf("rolled-back pointer grant = %v, want name deploy.ci", g)
}
}
+
+// TestDeployPromoteCannotFastTrack pins spec §12: the console's promote door
+// has no fast-track input, so a deploy credential cannot waive the bake
+// window by sending one, under either spelling. Whatever status the request
+// gets, a pointer it wrote must not carry fast_track.
+func TestDeployPromoteCannotFastTrack(t *testing.T) {
+ for _, key := range []string{"fast_track", "fastTrack"} {
+ t.Run(key, func(t *testing.T) {
+ fx, _, deploy := deployFixture(t)
+ status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/hello/promote",
+ map[string]any{"version": "v1", "channel": "edge", key: true}, deploy)
+ t.Logf("promote with %s = %d %v", key, status, out)
+ cp, err := release.Channel(context.Background(), fx.rel, fx.pub, "bab", "hello", "edge")
+ if err != nil {
+ if status == http.StatusOK {
+ t.Fatalf("promote answered 200 but edge does not verify: %v", err)
+ }
+ return // refused and nothing written: no pointer to carry the flag
+ }
+ if cp.FastTrack {
+ t.Fatalf("edge pointer payload carries fast_track=true: %+v", cp)
+ }
+ })
+ }
+}
diff --git a/scopedkeys_e2e_test.go b/scopedkeys_e2e_test.go
index eb7df661b..cb9cdf358 100644
--- a/scopedkeys_e2e_test.go
+++ b/scopedkeys_e2e_test.go
@@ -29,7 +29,17 @@ import (
func carlosTestEnv(t *testing.T, extra ...string) []string {
t.Helper()
scratch := t.TempDir()
- env := append(os.Environ(),
+ // An inherited CARLOS_SERVICE_TOKEN (a CI shell that exports one) would
+ // make any child that also sets CARLOS_DEPLOYMENT_DIR fail resolveMode's
+ // token-plus-store refusal, so it is dropped here. It is filtered rather
+ // than blanked: a test that passes its own token in extra still wins.
+ var env []string
+ for _, kv := range os.Environ() {
+ if !strings.HasPrefix(kv, "CARLOS_SERVICE_TOKEN=") {
+ env = append(env, kv)
+ }
+ }
+ env = append(env,
"CARLOS_CREDENTIALS="+filepath.Join(scratch, "credentials"),
"HOME="+scratch,
"CARLOS_CONSOLE=",