carlos / platform Public

console: summarize account mismatches and scope account review

Paul Campbell pushed by paul@keymail.dev 9a9a07f0624c9b47ef9d8d7715516781b6119e07 parent dcf0cad
15 files changed, +532 −175
  • .gitignore +2 −0
  • internal/console/accountreview.go +99 −0
  • internal/console/accountreview_test.go +283 −0
  • internal/console/appsettings.go +32 −35
  • internal/console/appsettings_test.go +5 −8
  • internal/console/mismatchcopy.go +23 −0
  • internal/console/overview_test.go +5 −2
  • internal/console/server.go +29 −0
  • internal/console/server_account_test.go +5 −8
  • internal/console/templates/account_review.html +24 −0
  • internal/console/templates/app_overview.html +1 −22
  • internal/console/templates/app_releases.html +1 −22
  • internal/console/templates/app_settings.html +3 −0
  • internal/console/view.go +14 −66
  • internal/console/view_test.go +6 −12

CI passed — run details

diff --git a/.gitignore b/.gitignore
index 42d71f76e..183f87420 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,3 +9,5 @@ platform
# directory in the checkout is that step's cache — see the step for why.
.amadan-cache/
infra/modules/**/.terraform.lock.hcl
+
+writing/
diff --git a/internal/console/accountreview.go b/internal/console/accountreview.go
new file mode 100644
index 000000000..81af04eac
--- /dev/null
+++ b/internal/console/accountreview.go
@@ -0,0 +1,99 @@
+package console
+
+import (
+ "net/http"
+ "sort"
+ "strconv"
+ "strings"
+
+ "github.com/carlosframework/platform/internal/release"
+)
+
+type AccountReviewGroup struct {
+ Account Account
+ Hosts []string
+}
+
+type AccountReview struct {
+ Groups []AccountReviewGroup
+ Count int
+ NoInstances bool
+ Owned int
+ blocked bool
+}
+
+func (a AccountReview) SummaryText() string {
+ return strings.ReplaceAll(approvedMismatchCopy.Summary, "{count}", strconv.Itoa(a.Count))
+}
+
+// appAccountReview returns only candidates whose source account the viewer
+// can access. A source with its own same-named app is a legitimate sibling,
+// not evidence that the viewed app is half-migrated.
+func (s *Server) appAccountReview(address string, viewed *Account, app string) (AccountReview, bool, error) {
+ reg, err := openRegistry(s.cfg.Registry)
+ if err != nil {
+ return AccountReview{}, false, err
+ }
+ routes, err := reg.List()
+ if err != nil {
+ return AccountReview{}, false, err
+ }
+ accounts, err := s.store.AccountsFor(address)
+ if err != nil {
+ return AccountReview{}, false, err
+ }
+ accessible := make(map[string]Account, len(accounts))
+ for _, account := range accounts {
+ accessible[account.Sqid] = account
+ }
+ groups := make(map[string][]string)
+ owned, hidden, sibling := 0, false, false
+ for _, rt := range routes {
+ if rt.App != app || rt.Platform {
+ continue
+ }
+ source := release.RouteAccount(rt)
+ if source == viewed.Sqid {
+ owned++
+ continue
+ }
+ account, ok := accessible[source]
+ if !ok {
+ hidden = true
+ continue
+ }
+ has, err := s.store.HasAppIn(account.ID, app)
+ if err != nil {
+ return AccountReview{}, false, err
+ }
+ if !has {
+ has, err = s.store.HasTrashedAppIn(account.ID, app)
+ if err != nil {
+ return AccountReview{}, false, err
+ }
+ }
+ if has {
+ sibling = true
+ continue
+ }
+ groups[source] = append(groups[source], rt.Host)
+ }
+ var review AccountReview
+ review.Owned = owned
+ review.blocked = hidden || sibling
+ for source, hosts := range groups {
+ sort.Strings(hosts)
+ review.Groups = append(review.Groups, AccountReviewGroup{Account: accessible[source], Hosts: hosts})
+ review.Count += len(hosts)
+ }
+ sort.Slice(review.Groups, func(i, j int) bool { return review.Groups[i].Account.Sqid < review.Groups[j].Account.Sqid })
+ return review, review.Count > 0 && owned == 0 && !hidden && !sibling && len(review.Groups) == 1, nil
+}
+
+func (s *Server) handleAppAccountReview(w http.ResponseWriter, r *http.Request) {
+ p, _, ok := s.appPage(w, r, "overview")
+ if !ok {
+ return
+ }
+ s.render(w, http.StatusOK, "app_account_review", p)
+}
diff --git a/internal/console/accountreview_test.go b/internal/console/accountreview_test.go
new file mode 100644
index 000000000..6dc05deca
--- /dev/null
+++ b/internal/console/accountreview_test.go
@@ -0,0 +1,283 @@
+package console
+
+import (
+ "context"
+ "errors"
+ "net/http"
+ "net/url"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/registry"
+ "github.com/carlosframework/platform/internal/release"
+)
+
+func TestAccountReviewExcludesLegitimateSameNamedApp(t *testing.T) {
+ fx := newConsoleFixture(t)
+ const otherID = 4242
+ if err := fx.st.CreateAccount(otherID, "cdef", "Other", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.AddMember(otherID, "paul@keymail.dev", RoleOwner, time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.CreateAppIn(otherID, "kass", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ fx.addRoute(t, registry.Route{Host: "other.example", App: "kass", Account: "cdef", Kind: "instance"})
+ viewed, err := fx.st.AccountBySqid("bab")
+ if err != nil || viewed == nil {
+ t.Fatal(err)
+ }
+ review, all, err := fx.srv.appAccountReview("paul@keymail.dev", viewed, "kass")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if review.Count != 0 || all {
+ t.Fatalf("legitimate sibling became a repair candidate: %+v all=%v", review, all)
+ }
+}
+
+func TestAccountReviewSiblingPreventsUnrelatedMove(t *testing.T) {
+ fx := newConsoleFixture(t)
+ now := time.Now()
+ for _, tc := range []struct {
+ id int64
+ sqid string
+ name string
+ app bool
+ }{{4242, "cdef", "Sibling", true}, {4343, "cghi", "Candidate", false}} {
+ if err := fx.st.CreateAccount(tc.id, tc.sqid, tc.name, "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.AddMember(tc.id, "paul@keymail.dev", RoleOwner, now); err != nil {
+ t.Fatal(err)
+ }
+ if tc.app {
+ if _, err := fx.st.CreateAppIn(tc.id, "kass", "paul@keymail.dev", now); err != nil {
+ t.Fatal(err)
+ }
+ }
+ }
+ fx.addRoute(t, registry.Route{Host: "sibling.example", App: "kass", Account: "cdef", Kind: "instance"})
+ fx.addRoute(t, registry.Route{Host: "candidate.example", App: "kass", Account: "cghi", Kind: "instance"})
+ viewed, _ := fx.st.AccountBySqid("bab")
+ review, all, err := fx.srv.appAccountReview("paul@keymail.dev", viewed, "kass")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if review.Count != 1 || all {
+ t.Fatalf("review = %+v all=%v; sibling must prevent move authority", review, all)
+ }
+ if _, agreed, err := fx.srv.routeAccountFor("paul@keymail.dev", viewed, "kass"); !errors.Is(err, errMoveTargetUnavailable) || agreed {
+ t.Fatalf("routeAccountFor agreed=%v err=%v; same-named sibling must hide the move target", agreed, err)
+ }
+}
+
+func TestAccountReviewHidesInaccessibleSource(t *testing.T) {
+ fx := newConsoleFixture(t)
+ fx.addRoute(t, registry.Route{Host: "secret.example", App: "kass", Account: "unknown", Kind: "instance"})
+ viewed, _ := fx.st.AccountBySqid("bab")
+ review, all, err := fx.srv.appAccountReview("paul@keymail.dev", viewed, "kass")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if review.Count != 0 || all {
+ t.Fatalf("inaccessible source disclosed: %+v all=%v", review, all)
+ }
+ if _, agreed, err := fx.srv.routeAccountFor("paul@keymail.dev", viewed, "kass"); !errors.Is(err, errMoveTargetUnavailable) || agreed {
+ t.Fatalf("routeAccountFor agreed=%v err=%v; an inaccessible source must hide the move target", agreed, err)
+ }
+}
+
+func TestAccountReviewTreatsTrashedSameNamedAppAsSibling(t *testing.T) {
+ fx := newConsoleFixture(t)
+ const otherID = 4242
+ if err := fx.st.CreateAccount(otherID, "cdef", "Other", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.CreateAppIn(otherID, "kass", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.TrashAppRow(otherID, "kass", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ fx.addRoute(t, registry.Route{Host: "trashed-sibling.example", App: "kass", Account: "cdef", Kind: "instance"})
+ viewed, _ := fx.st.AccountBySqid("bab")
+ review, all, err := fx.srv.appAccountReview("paul@keymail.dev", viewed, "kass")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if review.Count != 0 || all {
+ t.Fatalf("trashed sibling became a move candidate: %+v all=%v", review, all)
+ }
+ if _, agreed, err := fx.srv.routeAccountFor("paul@keymail.dev", viewed, "kass"); !errors.Is(err, errMoveTargetUnavailable) || agreed {
+ t.Fatalf("routeAccountFor agreed=%v err=%v; trashed sibling must hide the move target", agreed, err)
+ }
+}
+
+func TestAccountReviewPageShowsOwnedCandidatesAndHidesFromNonMembers(t *testing.T) {
+ fx := newConsoleFixture(t)
+ const otherID = 4242
+ if err := fx.st.CreateAccount(otherID, "cdef", "Other", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.AddMember(otherID, "paul@keymail.dev", RoleOwner, time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ fx.addRoute(t, registry.Route{Host: "candidate.example", App: "kass", Account: "cdef", Kind: "instance"})
+
+ c := signInPaul(t, fx)
+ status, _, body := get(t, c, fx.u("/apps/kass/account-review"))
+ if status != http.StatusOK || !strings.Contains(body, "candidate.example") || !strings.Contains(body, "Other") ||
+ !strings.Contains(body, approvedMismatchCopy.Partial) {
+ t.Fatalf("account review = %d, missing candidate/account/partial copy:\n%s", status, body)
+ }
+ reg, err := registry.Open(fx.regPath)
+ if err != nil {
+ t.Fatal(err)
+ }
+ routes, err := reg.List()
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, route := range routes {
+ if route.App == "kass" && !route.Platform {
+ if err := reg.SetAccount(route.Host, "cdef"); err != nil {
+ t.Fatal(err)
+ }
+ }
+ }
+ reg.Close()
+ status, _, body = get(t, c, fx.u("/apps/kass/account-review"))
+ if status != http.StatusOK || !strings.Contains(body, approvedMismatchCopy.Repairable) ||
+ strings.Contains(body, approvedMismatchCopy.Partial) {
+ t.Fatalf("fully mismatched review = %d, missing repairable branch:\n%s", status, body)
+ }
+
+ fx.setUser("eve@keymail.dev")
+ stranger := newBrowser(t)
+ signUp(t, fx, stranger, "code-account-review-stranger")
+ strangerCSRF := sessionToken(t, fx, stranger)
+ strangerStatus, _, strangerBody := get(t, stranger, fx.u("/apps/kass/account-review"))
+ ghostStatus, _, ghostBody := get(t, stranger, fx.ts.URL+"/definitely-not-a-page")
+ if strangerStatus != http.StatusNotFound || strangerStatus != ghostStatus || strangerBody != ghostBody {
+ t.Fatalf("non-member review = %d %q; unrouted = %d %q", strangerStatus, strangerBody, ghostStatus, ghostBody)
+ }
+ moveStatus, _, moveBody := postForm(t, stranger, fx.u("/apps/kass/move"), url.Values{"csrf": {strangerCSRF}})
+ if moveStatus != http.StatusNotFound || moveBody != ghostBody {
+ t.Fatalf("non-member move = %d %q; unrouted = %d %q", moveStatus, moveBody, ghostStatus, ghostBody)
+ }
+}
+
+func TestMoveRefusesHiddenOrSiblingSources(t *testing.T) {
+ for _, tc := range []struct {
+ name, account string
+ sibling bool
+ }{
+ {name: "hidden", account: "unknown"},
+ {name: "sibling", account: "cdef", sibling: true},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ fx := newConsoleFixture(t)
+ if tc.sibling {
+ const otherID = 4242
+ if err := fx.st.CreateAccount(otherID, tc.account, "Other", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.CreateAppIn(otherID, "kass", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ }
+ fx.addRoute(t, registry.Route{Host: tc.name + ".example", App: "kass", Account: tc.account, Kind: "instance"})
+ c := signInPaul(t, fx)
+ csrf := sessionToken(t, fx, c)
+ status, _, body := postForm(t, c, fx.u("/apps/kass/move"), url.Values{"csrf": {csrf}})
+ ghostStatus, _, ghostBody := get(t, c, fx.ts.URL+"/definitely-not-a-page")
+ if status != http.StatusNotFound || status != ghostStatus || body != ghostBody {
+ t.Fatalf("unavailable move = %d %q; unrouted = %d %q", status, body, ghostStatus, ghostBody)
+ }
+ accounts, err := fx.st.AppAccounts("kass")
+ if err != nil || len(accounts) == 0 || accounts[0] != OpsAccountID {
+ t.Fatalf("AppAccounts after refusal = %v, %v", accounts, err)
+ }
+ })
+ }
+}
+
+func TestPublishedAppWithoutOwnedInstancesWarnsFromOwnedStateOnly(t *testing.T) {
+ fx := newConsoleFixture(t)
+ if _, err := fx.st.CreateAppIn(OpsAccountID, "empty", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ c := signInPaul(t, fx)
+
+ _, _, empty := get(t, c, fx.u("/apps/empty"))
+ if strings.Contains(empty, approvedMismatchCopy.NoInstances) {
+ t.Fatalf("an unshipped empty app shows the published-app warning:\n%s", empty)
+ }
+
+ manifest := release.Manifest{
+ Schema: 1, App: "empty", Version: "v1", Kind: "binary", BuiltAt: time.Now().UTC().Format(time.RFC3339),
+ Artifacts: map[string]release.Artifact{"linux-amd64": {File: "empty"}},
+ }
+ if err := release.Ship(context.Background(), fx.rel, "bab", manifest,
+ map[string][]byte{"linux-amd64": []byte("binary")}); err != nil {
+ t.Fatal(err)
+ }
+ seedChannel(t, fx, "empty", "stable", "v1")
+ _, _, absentForeign := get(t, c, fx.u("/apps/empty"))
+ if !strings.Contains(absentForeign, approvedMismatchCopy.NoInstances) ||
+ !strings.Contains(absentForeign, `/apps/empty/instances">Instances</a>`) {
+ t.Fatalf("published app with no owned instances lacks the warning and Instances link:\n%s", absentForeign)
+ }
+ _, _, settings := get(t, c, fx.u("/apps/empty/settings"))
+ if !strings.Contains(settings, approvedMismatchCopy.NoInstances) || strings.Contains(settings, "This app deploys from") {
+ t.Fatalf("settings reassures a published app with no owned instances:\n%s", settings)
+ }
+
+ fx.addRoute(t, registry.Route{Host: "hidden.example", App: "empty", Account: "unknown", Kind: "instance"})
+ _, _, hiddenForeign := get(t, c, fx.u("/apps/empty"))
+ if hiddenForeign != absentForeign {
+ t.Fatal("a hidden foreign route changed the owned app warning render")
+ }
+
+ fx.addRoute(t, registry.Route{Host: "static.example", App: "empty", Account: "bab", Kind: "static"})
+ _, _, ownedStatic := get(t, c, fx.u("/apps/empty"))
+ if strings.Contains(ownedStatic, approvedMismatchCopy.NoInstances) {
+ t.Fatalf("an owned static route still shows the no-instances warning:\n%s", ownedStatic)
+ }
+}
+
+func TestAccountReviewDoesNotClaimOwnedInstancesWhenABlockerLeavesNone(t *testing.T) {
+ fx := newConsoleFixture(t)
+ const candidateID = 4242
+ if err := fx.st.CreateAccount(candidateID, "cdef", "Candidate", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.AddMember(candidateID, "paul@keymail.dev", RoleOwner, time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ const siblingID = 4343
+ if err := fx.st.CreateAccount(siblingID, "cghi", "Sibling", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.CreateAppIn(siblingID, "empty", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.CreateAppIn(OpsAccountID, "empty", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ fx.addRoute(t, registry.Route{Host: "candidate.example", App: "empty", Account: "cdef", Kind: "instance"})
+ fx.addRoute(t, registry.Route{Host: "sibling.example", App: "empty", Account: "cghi", Kind: "instance"})
+
+ c := signInPaul(t, fx)
+ status, _, body := get(t, c, fx.u("/apps/empty/account-review"))
+ if status != http.StatusOK || !strings.Contains(body, "candidate.example") {
+ t.Fatalf("blocked review = %d, missing safe candidate:\n%s", status, body)
+ }
+ if strings.Contains(body, approvedMismatchCopy.Partial) || strings.Contains(body, "sibling.example") {
+ t.Fatalf("blocked review claims owned instances or exposes sibling:\n%s", body)
+ }
+}
diff --git a/internal/console/appsettings.go b/internal/console/appsettings.go
index c6af24e27..6e7d2c3d0 100644
--- a/internal/console/appsettings.go
+++ b/internal/console/appsettings.go
@@ -1047,7 +1047,11 @@ func (s *Server) handleAppMoveAccount(w http.ResponseWriter, r *http.Request) {
back("err", msg)
return
}
- target, agreed, err := s.routeAccountFor(app)
+ target, agreed, err := s.routeAccountFor(addr, acct, app)
+ if errors.Is(err, errMoveTargetUnavailable) {
+ http.NotFound(w, r)
+ return
+ }
if err != nil {
back("err", "Could not read the routes — nothing was moved. Try again.")
return
@@ -1129,45 +1133,38 @@ func moveAdvice(err error, destName string) string {
return "Try again."
}
-// routeAccountFor reads what the ROUTES say the app's account is, and whether
-// they say it with one voice: the first non-platform route's stamped account,
-// plus agreed=false the moment any other non-platform route names a different
-// one. Both are read fresh at action time, so the move never trusts a stale
-// page.
-//
-// The Settings card only offers the move when AllMismatched (assembleSettings),
-// and under AllMismatched every non-platform route disagrees with the viewed
-// account — so the first route is provably also the first MISMATCHED one, and
-// the target here is the same account Assemble's mismatchedByApp[app][0].Account
-// is. But a target alone cannot police the door: outside that gate the first
-// route may be the ONLY one disagreeing, and moving to it would re-home the
-// record away from where most of the app's hosts actually deploy from. agreed
-// is what the handler refuses on — an app whose routes contradict each other
-// has no single account to move to, and the routes are the repair, not the
-// record. An app with no non-platform routes at all agrees vacuously (target
-// ""), which the handler's own nothing-to-move refusal then catches.
-func (s *Server) routeAccountFor(app string) (target string, agreed bool, err error) {
- reg, err := openRegistry(s.cfg.Registry)
- if err != nil {
- return "", false, err
- }
- routes, err := reg.List()
+var errMoveTargetUnavailable = errors.New("move target unavailable to this viewer")
+
+// routeAccountFor recomputes the viewer-safe diagnostic at action time. It
+// grants a target only when every relevant route points to one accessible
+// account which does not already claim its own same-named app.
+func (s *Server) routeAccountFor(address string, source *Account, app string) (target string, agreed bool, err error) {
+ review, all, err := s.appAccountReview(address, source, app)
if err != nil {
return "", false, err
}
- var seen bool
- for _, rt := range routes {
- if rt.App != app || rt.Platform {
- continue
+ if all {
+ target := review.Groups[0].Account.Sqid
+ reg, err := openRegistry(s.cfg.Registry)
+ if err != nil {
+ return "", false, err
}
- acct := release.RouteAccount(rt)
- if !seen {
- target, seen = acct, true
- continue
+ routes, err := reg.List()
+ if err != nil {
+ return "", false, err
}
- if acct != target {
- return target, false, nil
+ for _, rt := range routes {
+ if rt.App == app && !rt.Platform && release.RouteAccount(rt) != target {
+ return "", false, nil
+ }
}
+ return target, true, nil
+ }
+ if review.blocked {
+ return "", false, errMoveTargetUnavailable
+ }
+ if review.Count == 0 && !review.blocked {
+ return source.Sqid, true, nil
}
- return target, true, nil
+ return "", false, nil
}
diff --git a/internal/console/appsettings_test.go b/internal/console/appsettings_test.go
index f4b6edde1..e314c2cb3 100644
--- a/internal/console/appsettings_test.go
+++ b/internal/console/appsettings_test.go
@@ -549,7 +549,7 @@ func TestAppMoveConfirmsThenMovesTheRecord(t *testing.T) {
}
// Moving the record into an account the viewer does not belong to would strand
-// the app for them — refused, with the operator path named instead.
+// the app for them. The refusal must not disclose that account as a move target.
func TestAppMoveRefusesANonMemberOfTheTarget(t *testing.T) {
fx := newConsoleFixture(t)
const destID = 4343
@@ -560,16 +560,13 @@ func TestAppMoveRefusesANonMemberOfTheTarget(t *testing.T) {
c := signInPaul(t, fx)
csrf := sessionToken(t, fx, c)
- status, loc, body := postGated(t, fx, c, fx.u("/apps/kass/move"),
+ status, _, body := postGated(t, fx, c, fx.u("/apps/kass/move"),
url.Values{"csrf": {csrf}, "confirmed": {"1"}})
- if status != http.StatusSeeOther || loc != fx.path("/apps/kass/settings") {
- t.Fatalf("move into a stranger account = %d -> %q:\n%s", status, loc, body)
+ ghostStatus, _, ghostBody := get(t, c, fx.ts.URL+"/definitely-not-a-page")
+ if status != http.StatusNotFound || status != ghostStatus || body != ghostBody {
+ t.Fatalf("move into a stranger account = %d %q; unrouted = %d %q", status, body, ghostStatus, ghostBody)
}
notACard(t, body)
- if _, _, page := get(t, c, fx.ts.URL+loc); !strings.Contains(page,
- "You are not a member of Third (cghi)") {
- t.Fatalf("no non-membership refusal flash:\n%s", page)
- }
if has, err := fx.st.HasAppIn(OpsAccountID, "kass"); err != nil || !has {
t.Fatalf("HasAppIn(ops) = %v %v, want the record where it was", has, err)
}
diff --git a/internal/console/mismatchcopy.go b/internal/console/mismatchcopy.go
new file mode 100644
index 000000000..dd9ee8385
--- /dev/null
+++ b/internal/console/mismatchcopy.go
@@ -0,0 +1,23 @@
+package console
+
+type MismatchCopy struct {
+ Summary string
+ Link string
+ Title string
+ Intro string
+ Partial string
+ Repairable string
+ Empty string
+ NoInstances string
+}
+
+var approvedMismatchCopy = MismatchCopy{
+ Summary: "{count} addresses need account review.",
+ Link: "Review account use",
+ Title: "Review account use",
+ Intro: "These addresses use another account’s releases. Review ownership before making changes.",
+ Partial: "This account also has its own instances. Review each address before changing its account.",
+ Repairable: "Review this app’s account in Settings before deciding whether to move it.",
+ Empty: "No addresses need account review.",
+ NoInstances: "No instances yet.",
+}
diff --git a/internal/console/overview_test.go b/internal/console/overview_test.go
index 02ab3834c..8e7ec1e3d 100644
--- a/internal/console/overview_test.go
+++ b/internal/console/overview_test.go
@@ -583,6 +583,9 @@ func TestAppPageShowsPartialMismatchNotFullAlarm(t *testing.T) {
if err := fx.st.CreateAccount(moved, sqid, "Kass", "paul@keymail.dev", time.Now()); err != nil {
t.Fatal(err)
}
+ if err := fx.st.AddMember(moved, "paul@keymail.dev", RoleOwner, time.Now()); err != nil {
+ t.Fatal(err)
+ }
// Only ONE of kass's two fixture routes moves (kass.carlosapp.dev);
// docs.kass.carlosapp.dev stays on ops — a partial mismatch, not a full
// one.
@@ -595,7 +598,7 @@ func TestAppPageShowsPartialMismatchNotFullAlarm(t *testing.T) {
if strings.Contains(body, "This app is not deploying") {
t.Errorf("a partial mismatch must not render the full-alarm copy:\n%s", body)
}
- if !strings.Contains(body, "kass.carlosapp.dev") || !strings.Contains(body, "(reads <code>"+sqid+"</code>)") {
- t.Errorf("the partial-mismatch banner does not name the disagreeing route and its account:\n%s", body)
+ if !strings.Contains(body, `/account-review`) {
+ t.Errorf("the partial-mismatch banner does not link to diagnostics:\n%s", body)
}
}
diff --git a/internal/console/server.go b/internal/console/server.go
index 449b0ab52..572259cc7 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -1105,6 +1105,7 @@ func NewServer(cfg Config, st *Store) (*Server, error) {
s.mux.HandleFunc("POST /a/{sqid}/apps/new", s.handleNewApp)
s.mux.HandleFunc("GET /a/{sqid}/apps/{app}", s.handleApp)
s.mux.HandleFunc("GET /a/{sqid}/apps/{app}/releases", s.handleAppReleases)
+ s.mux.HandleFunc("GET /a/{sqid}/apps/{app}/account-review", s.handleAppAccountReview)
s.mux.HandleFunc("GET /a/{sqid}/apps/{app}/instances", s.handleAppInstances)
s.mux.HandleFunc("GET /a/{sqid}/apps/{app}/logs", s.handleAppLogs)
// The Errors tab and its two setting doors (errorsview.go): the tab is
@@ -1294,6 +1295,9 @@ type page struct {
Error string
Welcome bool // one-shot "account created" banner
+ AccountReview AccountReview
+ MismatchCopy MismatchCopy
+
ActiveNav string // account nav section this page sits in: "apps" | "fleet" | "team" | "settings"
ActiveTab string // app pages: "overview" | "releases" | "instances" | "activity" | "settings"
@@ -1583,6 +1587,7 @@ type appRow struct {
}
func (s *Server) render(w http.ResponseWriter, status int, name string, p page) {
+ p.MismatchCopy = approvedMismatchCopy
w.Header().Set("Content-Type", "text/html; charset=utf-8")
w.WriteHeader(status)
if err := templates.ExecuteTemplate(w, name, p); err != nil {
@@ -2570,10 +2575,34 @@ func (s *Server) appPage(w http.ResponseWriter, r *http.Request, tab string) (pa
p.ActiveTab = tab
p.ActiveNav = "apps"
p.Detail = &views[0]
+ if review, all, err := s.appAccountReview(addr, acct, app); err != nil {
+ log.Printf("console: account review %s/%s: %v", acct.Sqid, app, err)
+ } else {
+ p.AccountReview = review
+ p.AccountReview.NoInstances = review.Owned == 0 && hasPublishedChannel(p.Detail.Channels)
+ for _, group := range review.Groups {
+ for _, host := range group.Hosts {
+ p.Detail.Mismatched = append(p.Detail.Mismatched, MismatchedRoute{Host: host, Account: group.Account.Sqid})
+ }
+ }
+ p.Detail.AllMismatched = all
+ if all {
+ p.Detail.RouteAccount = review.Groups[0].Account.Sqid
+ }
+ }
s.applyPipelineView(r.Context(), p.Detail, acct)
return p, acct, true
}
+func hasPublishedChannel(channels []ChannelView) bool {
+ for _, channel := range channels {
+ if channel.Err == "" && channel.Version != "" {
+ return true
+ }
+ }
+ return false
+}
+
// applyPipelineView is the one place a single app's view becomes
// pipeline-aware (release pipelines v2 §2, §5) — deliberately not inside
// Assemble (AppView.Pipeline's own doc comment: the account home page calls
diff --git a/internal/console/server_account_test.go b/internal/console/server_account_test.go
index 700346616..cac0c3935 100644
--- a/internal/console/server_account_test.go
+++ b/internal/console/server_account_test.go
@@ -684,6 +684,9 @@ func TestAppPageWarnsWhenAccountsDisagree(t *testing.T) {
if err := fx.st.CreateAccount(moved, sqid, "Kass", "paul@keymail.dev", time.Now()); err != nil {
t.Fatal(err)
}
+ if err := fx.st.AddMember(moved, "paul@keymail.dev", RoleOwner, time.Now()); err != nil {
+ t.Fatal(err)
+ }
for _, host := range []string{"kass.carlosapp.dev", "docs.kass.carlosapp.dev"} {
if err := reg.SetAccount(host, sqid); err != nil {
t.Fatal(err)
@@ -692,23 +695,17 @@ func TestAppPageWarnsWhenAccountsDisagree(t *testing.T) {
reg.Close()
_, _, body = get(t, c, fx.u("/apps/kass"))
- if !strings.Contains(body, "not deploying") {
+ if !strings.Contains(body, `/apps/kass/account-review`) {
t.Errorf("no warning on a half-migrated app:\n%s", body)
}
- if !strings.Contains(body, `/apps/kass/settings"`) {
- t.Errorf("the warning does not link to the Settings tab, where the fix now lives:\n%s", body)
- }
// The promote/rollback/canary forms live on the releases tab, not the
// overview — a deep link there must not promote blind. Same warning,
// same fix, on the page that actually hosts the mutations.
_, _, body = get(t, c, fx.u("/apps/kass/releases"))
- if !strings.Contains(body, "not deploying") {
+ if !strings.Contains(body, `/apps/kass/account-review`) {
t.Errorf("no warning on the releases tab of a half-migrated app:\n%s", body)
}
- if !strings.Contains(body, `/apps/kass/settings"`) {
- t.Errorf("the releases tab warning does not link to the Settings tab, where the fix now lives:\n%s", body)
- }
// Settings itself holds the actual repair. paul owns the target account
// here (CreateAccount above makes its creator the owner), so he can reach
diff --git a/internal/console/templates/account_review.html b/internal/console/templates/account_review.html
new file mode 100644
index 000000000..717f81702
--- /dev/null
+++ b/internal/console/templates/account_review.html
@@ -0,0 +1,24 @@
+{{define "accountreviewbanner"}}{{if .AccountReview.Count}}
+<p class="banner">{{.AccountReview.SummaryText}}
+ <a href="{{.Nav}}/apps/{{.AppName}}/account-review">{{.MismatchCopy.Link}}</a>
+</p>
+{{else if .AccountReview.NoInstances}}
+<p class="banner">{{.MismatchCopy.NoInstances}}
+ <a href="{{.Nav}}/apps/{{.AppName}}/instances">Instances</a>
+</p>
+{{end}}{{end}}
+
+{{define "app_account_review"}}{{template "top" .}}
+{{template "appnav" .}}
+<h2>{{.MismatchCopy.Title}}</h2>
+<p>{{.MismatchCopy.Intro}}</p>
+{{if .AccountReview.Groups}}
+ {{range .AccountReview.Groups}}
+ <h3>{{.Account.Name}} (<code>{{.Account.Sqid}}</code>)</h3>
+ <ul>{{range .Hosts}}<li><code>{{.}}</code></li>{{end}}</ul>
+ {{end}}
+ {{if .Detail.AllMismatched}}<p>{{.MismatchCopy.Repairable}}</p>{{else if .AccountReview.Owned}}<p>{{.MismatchCopy.Partial}}</p>{{end}}
+{{else}}
+<p>{{.MismatchCopy.Empty}}</p>
+{{end}}
+{{template "bottom" .}}{{end}}
diff --git a/internal/console/templates/app_overview.html b/internal/console/templates/app_overview.html
index 07a415522..8db7f22bb 100644
--- a/internal/console/templates/app_overview.html
+++ b/internal/console/templates/app_overview.html
@@ -2,28 +2,7 @@
{{if .Flash}}<p class="banner">{{.Flash}}</p>{{end}}
{{if .FlashErr}}<p class="banner error">{{.FlashErr}}</p>{{end}}
{{template "appnav" .}}
-{{if .Detail.AccountMismatch}}
-{{if .Detail.AllMismatched}}
-<p class="banner error">
- <strong>This app is not deploying.</strong> Every one of its addresses reads
- releases from another account, but this page writes them to
- <code>{{.Account.Sqid}}</code> — so a promote here succeeds and reaches
- nothing. Affected: {{range $i, $m := .Detail.Mismatched}}{{if $i}}, {{end}}<code>{{$m.Host}}</code> (reads <code>{{$m.Account}}</code>){{end}}.
- Finish the move on this app's
- <a href="{{.Nav}}/apps/{{.AppName}}/settings">Settings</a> page.
- <span class="small">If it is the routes that are in the wrong account, move
- those back instead — whichever of the two is right, they have to agree.</span>
-</p>
-{{else}}
-<p class="banner">
- {{len .Detail.Mismatched}} address{{if gt (len .Detail.Mismatched) 1}}es{{end}} on this app
- read{{if eq (len .Detail.Mismatched) 1}}s{{end}} releases from another account and won't
- follow promotes made here: {{range $i, $m := .Detail.Mismatched}}{{if $i}}, {{end}}<code>{{$m.Host}}</code> (reads <code>{{$m.Account}}</code>){{end}}.
- The rest deploy normally. Fix on
- <a href="{{.Nav}}/apps/{{.AppName}}/settings">Settings</a>.
-</p>
-{{end}}
-{{end}}
+{{template "accountreviewbanner" .}}
{{with .Overview}}
<section class="pagehead" aria-label="App status">
<p><span class="chip chip-health-{{.Verdict}}">{{.VerdictLabel}}</span></p>
diff --git a/internal/console/templates/app_releases.html b/internal/console/templates/app_releases.html
index 7e00f916e..1f73f349a 100644
--- a/internal/console/templates/app_releases.html
+++ b/internal/console/templates/app_releases.html
@@ -2,28 +2,7 @@
{{if .Flash}}<p class="banner">{{.Flash}}</p>{{end}}
{{if .FlashErr}}<p class="banner error">{{.FlashErr}}</p>{{end}}
{{template "appnav" .}}
-{{if .Detail.AccountMismatch}}
-{{if .Detail.AllMismatched}}
-<p class="banner error">
- <strong>This app is not deploying.</strong> Every one of its addresses reads
- releases from another account, but this page writes them to
- <code>{{.Account.Sqid}}</code> — so a promote here succeeds and reaches
- nothing. Affected: {{range $i, $m := .Detail.Mismatched}}{{if $i}}, {{end}}<code>{{$m.Host}}</code> (reads <code>{{$m.Account}}</code>){{end}}.
- Finish the move on this app's
- <a href="{{.Nav}}/apps/{{.AppName}}/settings">Settings</a> page.
- <span class="small">If it is the routes that are in the wrong account, move
- those back instead — whichever of the two is right, they have to agree.</span>
-</p>
-{{else}}
-<p class="banner">
- {{len .Detail.Mismatched}} address{{if gt (len .Detail.Mismatched) 1}}es{{end}} on this app
- read{{if eq (len .Detail.Mismatched) 1}}s{{end}} releases from another account and won't
- follow promotes made here: {{range $i, $m := .Detail.Mismatched}}{{if $i}}, {{end}}<code>{{$m.Host}}</code> (reads <code>{{$m.Account}}</code>){{end}}.
- The rest deploy normally. Fix on
- <a href="{{.Nav}}/apps/{{.AppName}}/settings">Settings</a>.
-</p>
-{{end}}
-{{end}}
+{{template "accountreviewbanner" .}}
{{with .ReleaseSummary}}
<section class="pagehead" aria-label="Release status">
diff --git a/internal/console/templates/app_settings.html b/internal/console/templates/app_settings.html
index f6cbb790b..9b03d89e9 100644
--- a/internal/console/templates/app_settings.html
+++ b/internal/console/templates/app_settings.html
@@ -140,6 +140,9 @@ console does not know{{else}}You are not a member of that account{{end}} —
the operator path is <code>console migrate-app --app {{.Detail.App}} --to
{{.Settings.MismatchTarget}}</code> on the console's box.</p>
{{end}}
+{{else if .AccountReview.NoInstances}}
+<h2>Account</h2>
+<p>{{.MismatchCopy.NoInstances}} <a href="{{.Nav}}/apps/{{.Detail.App}}/instances">Instances</a></p>
{{else}}{{if .IsOwner}}
<h2>Account</h2>
<p class="small">This app deploys from {{.Account.Name}}. Moving an app to a
diff --git a/internal/console/view.go b/internal/console/view.go
index ec65561ed..d96b7e768 100644
--- a/internal/console/view.go
+++ b/internal/console/view.go
@@ -106,19 +106,11 @@ func (v AppView) RollbackChannels() []ChannelView {
return out
}
-// MismatchedRoute is one route whose stamped account disagrees with the
-// account whose page is being viewed: host plus where it actually reads
-// releases from. The banner names these — a warning that says WHICH routes
-// is a diagnostic; one that says "this app" is an alarm, and on tito
-// (2026-08-10) it was a false one.
+// MismatchedRoute is a viewer-safe account-review candidate assembled by the
+// server after authorization. Assemble itself exposes owned routes only.
type MismatchedRoute struct{ Host, Account string }
-// AccountMismatch reports the half-migrated state that would otherwise be
-// silent: `carlos accounts migrate` re-stamped this app's routes onto another
-// account, and `console migrate-app` was never run, so the console promotes
-// into one account's bucket keys while every host reads another's. A promote
-// then flashes "Promoted" and deploys nothing, forever, with no error
-// anywhere. The page has to say so.
+// AccountMismatch reports whether the server found account-review candidates.
func (v AppView) AccountMismatch() bool { return len(v.Mismatched) > 0 }
// DomainView is one row of the app page's Domains table — the product
@@ -539,58 +531,22 @@ func Assemble(regPath string, s release.Store, pub ed25519.PublicKey, acct strin
if err != nil {
return nil, err
}
- // byApp is keyed on app NAME with no account filter, so two accounts that
- // happen to name an app the same way share these buckets — pre-existing
- // (Host/Version/Channel/Status already cross that boundary, which is why
- // the RouteAccount mismatch machinery below exists) and gated: the app
- // page runs session → membership → HasAppIn → byte-identical 404 before
- // reaching here, so this is not a non-member leak. Recorded because the
- // blast radius grew rather than the gate weakening (2026-08-02 review,
- // MINOR 8): the Environment chip now shows one tenant's bundle NAMES to
- // the other, and prodWiredCanaryHosts' stable-environment set reads
- // across the same boundary, so one tenant's stable route can make
- // another's canary badge prod-wired. Capabilities joined that list
- // (2026-08-03 review, I4): Caps is the ladder's "granted: host-role,
- // port:25" line, which says which privileged ports and which box
- // authority an operator has handed one tenant's route — the most
- // sensitive thing this bleed has carried so far. The fix is at this
- // source — filter by release.RouteAccount(rt) == acct — not further
- // downstream.
- //
- // That whole-view filter stays a separate change, because it also decides
- // what the half-migrated AccountMismatch banner reads from. Until then,
- // Caps ALONE is scoped here, at the one place a RouteView is built: a
- // route belonging to another account arrives with Caps blank, so the
- // ladder renders no granted line for it (ladder.go copies this field
- // verbatim, and `{{if .Caps}}` in ladder.html renders nothing for ""),
- // while every other cross-account field keeps behaving exactly as it did.
- // Scoping it HERE rather than in handleApp is not a style choice: Ladder
- // is assembled inside this function, so a post-Assemble blank in the
- // handler would leave every LadderRoute's copy untouched — which is the
- // only place Caps renders at all.
+ // Routes are scoped here, before any downstream view is assembled. App names
+ // are account-local, so grouping by name first would mix legitimate sibling
+ // apps and leak their host, environment, capability and health details into
+ // one another's pages. Account-review candidates are derived separately by
+ // the server, after it knows the viewer's memberships.
byApp := map[string][]RouteView{}
domainsByApp := map[string][]DomainView{}
- // What the ROUTES say each app's account is, at route granularity. A
- // disagreement with the account being viewed is the half-migrated state
- // (see AccountMismatch); platform-minted routes are excluded because
- // they are aliases that reconciliation re-stamps on its own schedule, so
- // a lagging one is noise rather than a stuck deploy. routableByApp is
- // the denominator AllMismatched needs: every non-platform route counted,
- // so "all of them mismatch" can be told apart from "some of them".
- mismatchedByApp := map[string][]MismatchedRoute{}
- routableByApp := map[string]int{}
for _, rt := range routes {
+ rtAcct := release.RouteAccount(rt)
+ if rtAcct != acct {
+ continue
+ }
ch := rt.Channel
if ch == "" {
ch = "stable" // registry contract: empty channel means stable
}
- if rt.App != "" && !rt.Platform {
- routableByApp[rt.App]++
- if a := release.RouteAccount(rt); a != acct {
- mismatchedByApp[rt.App] = append(mismatchedByApp[rt.App], MismatchedRoute{Host: rt.Host, Account: a})
- }
- }
- rtAcct := release.RouteAccount(rt)
// The scoped guard described above: grants render only on rows the
// viewing account owns. Fails closed — a route whose stamped account
// disagrees with the account being viewed (the half-migrated state)
@@ -625,11 +581,6 @@ func Assemble(regPath string, s release.Store, pub ed25519.PublicKey, acct strin
}
seen[app] = true
channels := assembleChannels(ctx, s, pub, acct, app, now)
- mm := mismatchedByApp[app]
- mismatch := ""
- if len(mm) > 0 {
- mismatch = mm[0].Account
- }
pv := assembleProduction(ctx, s, pub, acct, app, channels, byApp[app], now)
if pv.Err == "" && !pv.On {
// No production flag (or off): nothing bakes, so no estimate. An
@@ -654,11 +605,8 @@ func Assemble(regPath string, s release.Store, pub ed25519.PublicKey, acct strin
// AppView.Pipeline's doc comment — so appPage (server.go) is
// where a single app's view gets pipeline-aware, and rebuilds
// Ladder from the corrected Channels once it has one.
- Ladder: assembleLadder(channels, byApp[app], nil, now),
- Production: pv,
- RouteAccount: mismatch,
- Mismatched: mm,
- AllMismatched: len(mm) > 0 && len(mm) == routableByApp[app],
+ Ladder: assembleLadder(channels, byApp[app], nil, now),
+ Production: pv,
})
}
return out, nil
diff --git a/internal/console/view_test.go b/internal/console/view_test.go
index efad811c0..3465c9795 100644
--- a/internal/console/view_test.go
+++ b/internal/console/view_test.go
@@ -344,17 +344,14 @@ func TestMismatchListsRoutes(t *testing.T) {
t.Fatal(err)
}
v := views[0]
- if len(v.Mismatched) != 1 || v.Mismatched[0].Host != "one.mismatchlist.test" || v.Mismatched[0].Account != "otheracct" {
- t.Fatalf("Mismatched = %+v, want exactly [{one.mismatchlist.test otheracct}]", v.Mismatched)
+ if len(v.Routes) != 2 {
+ t.Fatalf("Routes = %+v, want only the viewed account's two routes", v.Routes)
}
if v.AllMismatched {
t.Fatalf("AllMismatched = true, want false (2 of 3 routes agree)")
}
- if !v.AccountMismatch() {
- t.Fatal("AccountMismatch() = false, want true")
- }
- if v.RouteAccount != "otheracct" {
- t.Fatalf("RouteAccount = %q, want otheracct (first mismatched account)", v.RouteAccount)
+ if v.AccountMismatch() || v.RouteAccount != "" {
+ t.Fatalf("Assemble leaked cross-account diagnostics: %+v", v)
}
}
@@ -384,11 +381,8 @@ func TestMismatchAllRoutes(t *testing.T) {
t.Fatal(err)
}
v := views[0]
- if len(v.Mismatched) != 3 {
- t.Fatalf("Mismatched = %+v, want all 3 routes", v.Mismatched)
- }
- if !v.AllMismatched {
- t.Fatal("AllMismatched = false, want true (every route disagrees)")
+ if len(v.Routes) != 0 || v.AccountMismatch() || v.AllMismatched {
+ t.Fatalf("foreign routes leaked into view: %+v", v)
}
}