amadan
console: an owner may claim a name whose own-account objects exist (#79)
apps/<sqid>/<app>/ is namespaced BY ACCOUNT, so objects under a sqid can only be that account's — an owner claiming there is adopting their own release history, not inheriting a stranger's. That is the whole of #79: an app first shipped direct-to-bucket was otherwise unclaimable forever, with no recovery but deleting its history. Owner, not member. Pinned FALSE for every service principal: an account-scoped one reaches claimApp and holds no account role, so without the pin every CI credential could adopt release history. The legacy unqualified prefix still refuses everyone — those objects are owed a migrate and the name must stay out of its way. TestCreateApp's "orphan" case asserted the old policy (own-prefix objects block an owner) and now asserts the new one; the two existing claimApp call sites in tests take the new argument.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 files changed,
+202
−16
internal/console/apicli.go+6 −1internal/console/apicli_test.go+1 −1internal/console/apps_claim_test.go+142 −0internal/console/server.go+37 −9internal/console/server_test.go+16 −5
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index f792f73dd..05fca6385 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -391,7 +391,12 @@ func (s *Server) handleAPICreateApp(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| } |
| - outcome, placed := s.claimApp(r.Context(), acct, name, addr, place) |
| + // ownerClaim: a PERSON who owns this account may adopt objects already |
| + // under its own prefix (#79). A service principal never may — it holds no |
| + // account role, and handing a CI credential the power to adopt release |
| + // history is exactly the widening svcrole exists to prevent. |
| + ownerClaim := p.Kind == PrincipalUser && acct.Role == RoleOwner |
| + outcome, placed := s.claimApp(r.Context(), acct, name, addr, place, ownerClaim) |
| switch outcome { |
| case claimInvalid: |
| jsonError(w, http.StatusBadRequest, |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index d8ce4723d..b5485e243 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -2332,7 +2332,7 @@ func TestClaimAppPlacementWriteFailureDoesNotUndoClaim(t *testing.T) { |
| } |
| acct := &Account{ID: OpsAccountID, Sqid: fx.sqid} |
| - outcome, placed := fx.srv.claimApp(context.Background(), acct, "titogo", "paul@keymail.dev", "fleet/pi") |
| + outcome, placed := fx.srv.claimApp(context.Background(), acct, "titogo", "paul@keymail.dev", "fleet/pi", false) |
| if outcome != claimOK { |
| t.Fatalf("claimApp outcome = %v, want claimOK — the claim itself must still succeed", outcome) |
| } |
diff --git a/internal/console/apps_claim_test.go b/internal/console/apps_claim_test.go| new file mode 100644 |
| index 000000000..b94eba071 |
| --- /dev/null |
| +++ b/internal/console/apps_claim_test.go |
| @@ -0,0 +1,142 @@ |
| +package console |
| + |
| +import ( |
| + "context" |
| + "net/http" |
| + "net/url" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/svcrole" |
| +) |
| + |
| +// seedBucketApp puts an object under an account's own app prefix WITHOUT a |
| +// row, which is exactly what a direct-bucket ship does (direct mode skips the |
| +// claim) and exactly the state that makes #79 unrecoverable: the objects |
| +// block the claim, and no pass can discover a row that does not exist. |
| +func seedBucketApp(t *testing.T, fx *consoleFixture, sqid, app string) { |
| + t.Helper() |
| + key := "apps/" + sqid + "/" + app + "/channels/canary-console.json" |
| + if _, err := fx.rel.Put(context.Background(), key, []byte(`{}`), ""); err != nil { |
| + t.Fatalf("seed %s: %v", key, err) |
| + } |
| +} |
| + |
| +// opsAccount is the ops account row the claim tests hand to claimApp. Role is |
| +// not read by claimApp — the ownerClaim argument carries that decision — so |
| +// the plain by-id accessor (which does not join a membership) is enough. |
| +func opsAccount(t *testing.T, fx *consoleFixture) *Account { |
| + t.Helper() |
| + acct, err := fx.st.AccountByID(OpsAccountID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if acct == nil { |
| + t.Fatal("ops account missing from the fixture") |
| + } |
| + return acct |
| +} |
| + |
| +// TestOwnerClaimsAppWhoseObjectsExist is #79's fix. Before it, an app first |
| +// shipped direct-to-bucket could never be claimed and so was permanently |
| +// invisible to the console, with no recovery but deleting its release history. |
| +func TestOwnerClaimsAppWhoseObjectsExist(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + seedBucketApp(t, fx, "bab", "console") |
| + acct := opsAccount(t, fx) |
| + outcome, _ := fx.srv.claimApp(context.Background(), acct, "console", "paul@keymail.dev", "", true) |
| + if outcome != claimOK { |
| + t.Fatalf("owner claim over existing objects = %v, want claimOK", outcome) |
| + } |
| + has, err := fx.st.HasAppIn(OpsAccountID, "console") |
| + if err != nil || !has { |
| + t.Fatalf("HasAppIn after claim = %v %v, want true", has, err) |
| + } |
| +} |
| + |
| +// TestNonOwnerStillRefusedOverExistingObjects: the exemption is OWNERSHIP, not |
| +// membership. Adopting existing release history is an ownership act. |
| +func TestNonOwnerStillRefusedOverExistingObjects(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + seedBucketApp(t, fx, "bab", "console") |
| + acct := opsAccount(t, fx) |
| + outcome, _ := fx.srv.claimApp(context.Background(), acct, "console", "member@keymail.dev", "", false) |
| + if outcome != claimTaken { |
| + t.Fatalf("non-owner claim over existing objects = %v, want claimTaken", outcome) |
| + } |
| +} |
| + |
| +// TestLegacyPrefixStillBlocksAnOwner: the exemption covers the ACCOUNT- |
| +// QUALIFIED prefix only. Legacy objects at apps/<app>/ predate account scoping |
| +// and are owed an `accounts migrate -from legacy`; handing the name out would |
| +// put an app in the way of that migration. |
| +func TestLegacyPrefixStillBlocksAnOwner(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + if _, err := fx.rel.Put(context.Background(), "apps/oldsite/channels/stable.json", []byte(`{}`), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + acct := opsAccount(t, fx) |
| + outcome, _ := fx.srv.claimApp(context.Background(), acct, "oldsite", "paul@keymail.dev", "", true) |
| + if outcome != claimTaken { |
| + t.Fatalf("owner claim over a LEGACY prefix = %v, want claimTaken — those objects are owed a migrate", outcome) |
| + } |
| +} |
| + |
| +// TestGrantStillBlocksAnOwner: a CLI-granted app in ops stays claim-proof. |
| +// Verified live on flagship 2026-08-23: derp, meep and platform-site hold |
| +// grants; console does not, which is why the exemption is sufficient for it. |
| +func TestGrantStillBlocksAnOwner(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + if err := fx.st.Grant("someone@keymail.dev", "derp"); err != nil { |
| + t.Fatal(err) |
| + } |
| + acct := opsAccount(t, fx) |
| + outcome, _ := fx.srv.claimApp(context.Background(), acct, "derp", "paul@keymail.dev", "", true) |
| + if outcome != claimTaken { |
| + t.Fatalf("owner claim over a GRANTED name = %v, want claimTaken", outcome) |
| + } |
| +} |
| + |
| +// TestServicePrincipalNeverClaimsOverObjects: a service has no role at all, so |
| +// ownerClaim must be pinned FALSE for it. An account-scoped PrincipalService |
| +// legitimately reaches claimApp (apicli.go blocks only app-scoped ones), so |
| +// without the pin the exemption would silently hand every CI credential the |
| +// power to adopt release history. |
| +func TestServicePrincipalNeverClaimsOverObjects(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + seedBucketApp(t, fx, "bab", "console") |
| + svc := serviceToken(t, fx.st, OpsAccountID, "hub", "", svcrole.Publish) |
| + status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/bab/new", map[string]string{"name": "console"}, svc) |
| + if status != http.StatusConflict { |
| + t.Fatalf("service claim over existing objects = %d %v, want 409 (taken)", status, out) |
| + } |
| + has, err := fx.st.HasAppIn(OpsAccountID, "console") |
| + if err != nil || has { |
| + t.Fatalf("HasAppIn after a service claim = %v %v, want false — a service must never adopt release history", has, err) |
| + } |
| +} |
| + |
| +// TestBrowserDoorMemberRefusedOverExistingObjects proves the browser door's |
| +// WIRING, not just claimApp's argument: handleNewApp now passes the session's |
| +// role through, and a plain member of the account must still be told the name |
| +// is taken. TestCreateApp covers the owner's side of the same door. |
| +func TestBrowserDoorMemberRefusedOverExistingObjects(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + seedBucketApp(t, fx, "bab", "console") |
| + if err := fx.st.AddMember(OpsAccountID, "sam@keymail.dev", RoleMember, time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + c, csrf := signInAs(t, fx, "sam@keymail.dev", "code-claim-member") |
| + |
| + status, loc, _ := postForm(t, c, fx.u("/apps/new"), url.Values{"csrf": {csrf}, "name": {"console"}}) |
| + if status != http.StatusSeeOther || loc != fx.path("/apps/new") { |
| + t.Fatalf("member claim over existing objects = %d -> %q, want 303 -> the create form", status, loc) |
| + } |
| + if _, _, body := get(t, c, fx.u("/apps/new")); !strings.Contains(body, "That name is taken.") { |
| + t.Fatal("member claim over existing objects: no taken flash") |
| + } |
| + if has, err := fx.st.HasAppIn(OpsAccountID, "console"); err != nil || has { |
| + t.Fatalf("HasAppIn after a member claim = %v %v, want false", has, err) |
| + } |
| +} |
diff --git a/internal/console/server.go b/internal/console/server.go| index fd581946d..b4f842c3e 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -2423,7 +2423,22 @@ func normalizeAppName(name string) string { |
| // OBSERVABLE instead of silently trusting the request — a caller that never |
| // checks the echo cannot tell "placed" from "claim succeeded, placement |
| // silently didn't". |
| -func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placement string) (outcome claimOutcome, placed string) { |
| +// |
| +// ownerClaim exempts the ACCOUNT-QUALIFIED prefix check below, and nothing |
| +// else. The guard exists so a stranger cannot inherit objects somebody else |
| +// shipped — but apps/<sqid>/<app>/ is namespaced BY ACCOUNT, so objects under |
| +// a given sqid can only be that account's. An OWNER of that account claiming |
| +// a name under its own prefix is adopting their own release history, which is |
| +// the whole of issue #79: an app first shipped direct-to-bucket (direct mode |
| +// skips the claim) could otherwise never be given a row, and so stayed |
| +// invisible to the console forever with no recovery but deleting its history. |
| +// |
| +// It is OWNER, not member: adopting existing release history is an ownership |
| +// act. It is FALSE for every service principal — a service has no role at |
| +// all, and an account-scoped one legitimately reaches this function, so |
| +// without an explicit pin the exemption would hand every CI credential the |
| +// same power. |
| +func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placement string, ownerClaim bool) (outcome claimOutcome, placed string) { |
| if !release.ValidApp(name) { |
| return claimInvalid, "" |
| } |
| @@ -2472,9 +2487,17 @@ func (s *Server) claimApp(ctx context.Context, acct *Account, name, addr, placem |
| if err != nil { |
| return claimStoreFailed, "" |
| } |
| - if len(keys) > 0 { |
| - return claimTaken, "" |
| + 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) { |
| + continue |
| + } |
| + return claimTaken, "" |
| } |
| created, err := s.store.CreateAppIn(acct.ID, name, addr, time.Now()) |
| if err != nil { |
| @@ -2516,12 +2539,17 @@ func (s *Server) handleNewAppForm(w http.ResponseWriter, r *http.Request) { |
| } |
| // handleNewApp is the browser door onto claimApp: the account page's New app |
| -// form. Any member may claim — creation grants nothing promotion doesn't |
| -// already gate — and unlike the CLI door it carries no rate limit: a session |
| -// behind CSRF is not the stolen-credential shape the API budget exists for, |
| -// and this form is the escape hatch when that budget is honestly spent. |
| +// form. Any member may claim a FREE name — creation grants nothing promotion |
| +// doesn't already gate — and unlike the CLI door it carries no rate limit: a |
| +// session behind CSRF is not the stolen-credential shape the API budget |
| +// exists for, and this form is the escape hatch when that budget is honestly |
| +// spent. |
| +// |
| +// An OWNER may additionally claim a name whose objects already exist under |
| +// this account's own prefix (#79, claimApp's ownerClaim). A plain member |
| +// cannot: adopting existing release history is an ownership act. |
| func (s *Server) handleNewApp(w http.ResponseWriter, r *http.Request) { |
| - addr, raw, acct, _, ok := s.account(w, r) |
| + addr, raw, acct, role, ok := s.account(w, r) |
| if !ok { |
| return |
| } |
| @@ -2539,7 +2567,7 @@ func (s *Server) handleNewApp(w http.ResponseWriter, r *http.Request) { |
| // The browser door never places at create time, so the second return |
| // (what actually stuck) is always "" here and there is nothing to echo |
| // it to — this form has no placement concept at all. |
| - outcome, _ := s.claimApp(r.Context(), acct, name, addr, "") |
| + outcome, _ := s.claimApp(r.Context(), acct, name, addr, "", role == RoleOwner) |
| switch outcome { |
| case claimInvalid: |
| fail("Invalid app name — lowercase letters, digits, and dashes, starting with a letter.") |
diff --git a/internal/console/server_test.go b/internal/console/server_test.go| index 3d57f1c67..e756ca8c3 100644 |
| --- a/internal/console/server_test.go |
| +++ b/internal/console/server_test.go |
| @@ -468,10 +468,14 @@ func TestCreateApp(t *testing.T) { |
| t.Fatalf("AppsIn = %v %v, want zephyr claimed in the account", apps, err) |
| } |
| - // Taken is taken WITHIN THE ACCOUNT: the name just claimed, a name the |
| - // pre-accounts grants hold, and a name whose releases already sit in the |
| - // bucket — all the same "taken". (Another ACCOUNT claiming the same name |
| - // is not a conflict; that's TestSameAppNameInTwoAccounts.) |
| + // Taken is taken WITHIN THE ACCOUNT: the name just claimed and a name the |
| + // pre-accounts grants hold are both "taken". (Another ACCOUNT claiming the |
| + // same name is not a conflict; that's TestSameAppNameInTwoAccounts.) |
| + // |
| + // "orphan" — releases already in the bucket under THIS account's own |
| + // prefix and no row — is no longer among them: since #79 an OWNER may |
| + // claim such a name, because those objects are the account's own release |
| + // history rather than a stranger's (apps_claim_test.go). |
| if err := release.Ship(context.Background(), fx.rel, release.OpsAccount, release.Manifest{ |
| Schema: 1, App: "orphan", Version: "v1", Kind: "binary", |
| Artifacts: map[string]release.Artifact{"linux-amd64": {File: "orphan"}}, |
| @@ -481,7 +485,7 @@ func TestCreateApp(t *testing.T) { |
| if err := fx.st.Grant("cli@keymail.dev", "cligranted"); err != nil { |
| t.Fatal(err) |
| } |
| - for _, name := range []string{"zephyr", "kass", "orphan", "cligranted"} { |
| + for _, name := range []string{"zephyr", "kass", "cligranted"} { |
| if status, loc, _ := postForm(t, c, fx.u("/apps/new"), url.Values{"csrf": {csrf}, "name": {name}}); status != http.StatusSeeOther || loc != fx.path("/apps/new") { |
| t.Fatalf("claim %s = %d -> %q, want 303 -> the create form", name, status, loc) |
| } |
| @@ -490,6 +494,13 @@ func TestCreateApp(t *testing.T) { |
| } |
| } |
| + // The #79 exemption, through the browser door: paul owns this account, so |
| + // the orphaned bucket objects under apps/bab/orphan/ are his to adopt and |
| + // the claim lands on the app page like any other. |
| + if status, loc, _ := postForm(t, c, fx.u("/apps/new"), url.Values{"csrf": {csrf}, "name": {"orphan"}}); status != http.StatusSeeOther || loc != fx.path("/apps/orphan") { |
| + t.Fatalf("owner claim of orphan = %d -> %q, want 303 -> /apps/orphan (#79)", status, loc) |
| + } |
| + |
| // Invalid names refused; CSRF enforced. |
| if status, _, _ := postForm(t, c, fx.u("/apps/new"), url.Values{"csrf": {csrf}, "name": {"Bad Name!"}}); status != http.StatusSeeOther { |
| t.Fatalf("invalid name = %d, want redirect with flash", status) |