amadan
Merge branch 'console-ux-04-diagnostics' into main
14 files changed,
+530
−175
internal/console/accountreview.go+99 −0internal/console/accountreview_test.go+283 −0internal/console/appsettings.go+32 −35internal/console/appsettings_test.go+5 −8internal/console/mismatchcopy.go+23 −0internal/console/overview_test.go+5 −2internal/console/server.go+29 −0internal/console/server_account_test.go+5 −8internal/console/templates/account_review.html+24 −0internal/console/templates/app_overview.html+1 −22internal/console/templates/app_releases.html+1 −22internal/console/templates/app_settings.html+3 −0internal/console/view.go+14 −66internal/console/view_test.go+6 −12
CI passed — run details
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 3ac7e6952..bdfd75b56 100644 |
| --- a/internal/console/server.go |
| +++ b/internal/console/server.go |
| @@ -1113,6 +1113,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}/instances/{host}", s.handleAppInstance) |
| s.mux.HandleFunc("GET /a/{sqid}/apps/{app}/logs", s.handleAppLogs) |
| @@ -1310,6 +1311,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" |
| @@ -1612,6 +1616,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 { |
| @@ -2599,10 +2604,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) |
| } |
| } |