amadan
fix wave: owner exemption as an allowlist + three comment/doc corrections
Final review returned zero blockers; these are its "should fix before merge" items. 1. claimApp's owner exemption was a DENYLIST — `prefix != LegacyAppPrefix(name)` — so any prefix ever appended to the slice would be exempted for an owner by default: fail-open in a guard whose whole job is refusing. It is now an allowlist naming the one prefix it covers (the account's own), via a testable `ownerExempt` seam. Identical over today's two prefixes; the difference is what happens to a third one added later, which is now refused until someone deliberately names it. TestOwnerExemptionRefusesAnUnknownPrefix pins that default direction; the six existing claim tests are unchanged. 2. Runbook R4 now tells the operator to EXPECT the default one-channel edge pipeline #254 stamps on every fresh claim, and why it gates nothing here: canary/* sits outside every pipeline, and stable keeps its passkey 403 either way. 3. apimode.go's createApp comment claimed the claim-time flags leave "no window". Server-side it is two writes inside one handler; the client comment now says what it actually buys (no second round trip) and points at the handler's own honest account. 4. handleAPIRollback's service refusal sits later in the handler than promote's, because `target` only exists once the pointer is read. Now says so, and why it is safe: nothing above it writes, the byte-identical membership 404 is far above, and the facts the extra reads disclose are already readable through handleAPIChannels. make test: exit 0, gofmt clean tree-wide, 33 ok. GOOS=darwin build passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 files changed,
+80
−9
apimode.go+9 −3docs/superpowers/plans/2026-08-23-console-as-an-app.md+4 −0internal/console/apicli.go+18 −0internal/console/apps_claim_test.go+26 −0internal/console/server.go+23 −6
diff --git a/apimode.go b/apimode.go| index 6ba4910fe..7f31ea400 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -938,9 +938,15 @@ type apiCreateAppResult struct { |
| // creating an unplaced app. |
| // |
| // controlPlane and buildProbe are the claim-time flags Task 3 adds |
| -// (-control-plane, -build-probe): set in the SAME call that claims the |
| -// name, so there is no window in which the freshly-claimed row is an |
| -// ordinary app before it is flagged. controlPlane costs an owner-only 403 |
| +// (-control-plane, -build-probe): carried in the SAME request that claims |
| +// the name, so no SECOND round trip is needed to flag the row. That is not |
| +// the same as atomicity — server-side (handleAPICreateApp, |
| +// internal/console/apicli.go) it is still two writes inside one handler, |
| +// the claim then the stamps, and that handler's own comment is the honest |
| +// account of the window that leaves. The client's job is only to make sure |
| +// the flags travel with the claim rather than after it. |
| +// |
| +// controlPlane costs an owner-only 403 |
| // (server-side; there is no client-side role check possible here) and |
| // buildProbe is grammar-checked client-side before this is ever called |
| // (validBuildProbe, apps.go), same "a shape the server is certain to |
diff --git a/docs/superpowers/plans/2026-08-23-console-as-an-app.md b/docs/superpowers/plans/2026-08-23-console-as-an-app.md| index 2d97cc537..7661f6014 100644 |
| --- a/docs/superpowers/plans/2026-08-23-console-as-an-app.md |
| +++ b/docs/superpowers/plans/2026-08-23-console-as-an-app.md |
| @@ -1946,6 +1946,10 @@ then the same against `https://platform.carlosframework.com`. One step, not two: |
| Verify: `carlos channels -app console -account ops -console <origin>` lists the existing history — proof the claim adopted the objects rather than creating an empty app. |
| +**Expect a pipeline object to appear.** Since #254, every fresh claim stamps a default one-channel `edge` pipeline, so this claim writes `apps/bab/console/…` pipeline state the earlier steps never mention. That is the normal shape of a claim now, not a sign the adoption went wrong — do not investigate it mid-runbook. |
| + |
| +It does not gate anything this runbook does: `canary/*` sits outside every pipeline by design, so `canary/console` promotes and the adoptions in R5–R7 are ungated by it, and `stable` keeps its passkey 403 with or without the pipeline. |
| + |
| ### R5 — First `carlos deploy -app console`, on tito |
| Build and deploy in the recipe the spec's failure-mode table demands — **`cmd/console`, not the root binary** (the 8-minute outage of 2026-08-08): |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index c29caa2b2..11d85ff63 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -979,6 +979,24 @@ func (s *Server) handleAPIRollback(w http.ResponseWriter, r *http.Request) { |
| // Rollback moves the same pointer promote moves, so it wears the same |
| // service refusal — a credential barred from promoting the control plane |
| // must not reach the identical effect through the other verb. |
| + // |
| + // It sits LATER in the handler than handleAPIPromote's does, and that |
| + // asymmetry is deliberate rather than drift. Promote's runs as soon as |
| + // the pipeline is in hand; this one runs after the pointer read and the |
| + // empty-history 409 as well, because rollback has no request-supplied |
| + // version — `target` only exists once the pointer has been read, and the |
| + // rules switch below has to pin it. |
| + // |
| + // Safe, on three counts. Nothing above this line WRITES: GetPipeline and |
| + // Channel are reads, so a refusal here still leaves the request with no |
| + // effect, which is the only property the gate owes. Nothing above it |
| + // leaks across a boundary either: the byte-identical membership 404 lives |
| + // far above in apiAccountApp, so a non-member never reaches any of this. |
| + // And the two facts the extra reads can disclose to a barred service — |
| + // that the channel resolves, and that it has history — are ones the same |
| + // credential can already read straight out of handleAPIChannels, which |
| + // carries no control-plane refusal at all because listing pointers is not |
| + // release authority. |
| if s.refuseServiceControlPlane(w, p, acct.ID, app, "roll it back") { |
| return |
| } |
diff --git a/internal/console/apps_claim_test.go b/internal/console/apps_claim_test.go| index 880ee4694..3d55f3513 100644 |
| --- a/internal/console/apps_claim_test.go |
| +++ b/internal/console/apps_claim_test.go |
| @@ -9,6 +9,7 @@ import ( |
| "testing" |
| "time" |
| + "github.com/carlosframework/platform/internal/release" |
| "github.com/carlosframework/platform/internal/svcrole" |
| ) |
| @@ -234,3 +235,28 @@ func TestBuildProbeMustBeAnAbsolutePath(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// TestOwnerExemptionRefusesAnUnknownPrefix pins the DEFAULT DIRECTION of the |
| +// #79 owner exemption rather than either of the two prefixes it covers today. |
| +// The decision is an allowlist — it names the account's own prefix — so a |
| +// hypothetical THIRD prefix appended to claimApp's list (a future migration |
| +// namespace, a shared-tenant prefix, anything) is refused for an owner until |
| +// someone deliberately adds it here. Written as the exclusion this replaced |
| +// (`prefix != LegacyAppPrefix(name)`), that same third prefix would have been |
| +// exempted silently, fail-open in a guard whose whole job is refusing. |
| +func TestOwnerExemptionRefusesAnUnknownPrefix(t *testing.T) { |
| + own := release.AppPrefix("bab", "console") |
| + if !ownerExempt(own, own) { |
| + t.Fatalf("ownerExempt(own, own) = false, want true: the account's own prefix IS the exemption") |
| + } |
| + for _, other := range []string{ |
| + release.LegacyAppPrefix("console"), // today's second prefix |
| + "apps/migrating/bab/console/", // a hypothetical third |
| + release.AppPrefix("bdk", "console"), // another account's |
| + strings.TrimSuffix(own, "/"), // a near-miss spelling |
| + } { |
| + if ownerExempt(other, own) { |
| + t.Fatalf("ownerExempt(%q, %q) = true, want false: only the account's own prefix is exempt", other, own) |
| + } |
| + } |
| +} |
diff --git a/internal/console/server.go b/internal/console/server.go| index ac00a53bc..c763d28fa 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -2608,6 +2608,13 @@ func validBuildProbe(p string) bool { |
| return true |
| } |
| +// ownerExempt reports whether the #79 owner exemption covers prefix, given |
| +// the account's own app prefix. It is an allowlist of exactly one prefix: a |
| +// prefix it has never been told about is NOT exempt, so a third prefix added |
| +// to claimApp's list is refused for an owner until someone deliberately names |
| +// it here. See the call site for why that default matters. |
| +func ownerExempt(prefix, ownPrefix string) bool { return prefix == ownPrefix } |
| + |
| // claimApp claims an app name for acct — once, ever. A name is refused when |
| // the apps table has it, when ANY owner exists for it (CLI-granted apps stay |
| // claim-proof), or when the release store already holds anything under |
| @@ -2687,7 +2694,8 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem |
| // of the `accounts migrate -from legacy` that is still owed to those |
| // objects. For a non-ops account the legacy prefix belongs to somebody |
| // else's app and must NOT block the name. |
| - prefixes := []string{release.AppPrefix(acct.Sqid, name)} |
| + ownPrefix := release.AppPrefix(acct.Sqid, name) |
| + prefixes := []string{ownPrefix} |
| if acct.ID == OpsAccountID { |
| prefixes = append(prefixes, release.LegacyAppPrefix(name)) |
| } |
| @@ -2704,11 +2712,20 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem |
| if len(keys) == 0 { |
| continue |
| } |
| - // The owner exemption covers the account's OWN prefix only. The |
| - // legacy unqualified one (ops only) still refuses whoever is asking: |
| - // those objects are owed an `accounts migrate -from legacy`, and a |
| - // name handed out over them puts an app in that migration's way. |
| - if ownerClaim && prefix != release.LegacyAppPrefix(name) { |
| + // The owner exemption is an ALLOWLIST: it NAMES the one prefix it |
| + // covers — the account's own, apps/<sqid>/<app>/ — rather than |
| + // excluding the one it doesn't. The two forms are identical over |
| + // today's two prefixes; they differ in what happens to a THIRD one |
| + // added later. Written as an exclusion, any prefix appended to the |
| + // slice above would be exempted for an owner by default, fail-open |
| + // in a guard whose whole job is refusing. Written this way a new |
| + // prefix is refused until someone deliberately adds it here. |
| + // |
| + // The legacy unqualified prefix (ops only) is therefore outside the |
| + // exemption and still refuses whoever is asking: those objects are |
| + // owed an `accounts migrate -from legacy`, and a name handed out |
| + // over them puts an app in that migration's way. |
| + if ownerClaim && ownerExempt(prefix, ownPrefix) { |
| continue |
| } |
| return claimTaken, "" |