amadan

carlos / platform Public

console: fail closed on retire-canary's follower check — a registry error must never read as "no hosts"

channelHosts folded any registry error into an empty host list; at
handleRetireCanary that let a transient registry hiccup delete a live
channel pointer. channelHosts now returns (hosts, error); create's
call site stays best-effort (cosmetic listing), retire's refuses with
a 500 and logs the underlying error.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 4ad337787a37c42464f14b80a3892255ae0d706c parent 687c54b
2 files changed, +54 −9
  • internal/console/server.go +25 −9
  • internal/console/server_test.go +29 −0
diff --git a/internal/console/server.go b/internal/console/server.go
index 2f1392633..d63ff66d6 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -749,16 +749,21 @@ type channelDeleter interface {
Delete(ctx context.Context, key string) error
}
-// channelHosts lists the registry hosts whose routes follow app's channel.
-// Read-only, best-effort: a registry hiccup reads as "no hosts".
-func (s *Server) channelHosts(app, channel string) []string {
+// channelHosts lists the registry hosts whose routes follow app's channel,
+// and reports whether the registry could be read at all. The two call sites
+// deliberately treat that error differently: handleNewCanary's use is
+// cosmetic (which URL to show in a success flash) and downgrades an error to
+// "no hosts", but handleRetireCanary's use is load-bearing — a canary with
+// followers must never be deleted — so it must fail CLOSED on a registry
+// hiccup rather than silently reading the error as "safe to retire".
+func (s *Server) channelHosts(app, channel string) ([]string, error) {
reg, err := openRegistry(s.cfg.Registry)
if err != nil {
- return nil
+ return nil, err
}
routes, err := reg.List()
if err != nil {
- return nil
+ return nil, err
}
var out []string
for _, rt := range routes {
@@ -766,7 +771,7 @@ func (s *Server) channelHosts(app, channel string) []string {
out = append(out, rt.Host)
}
}
- return out
+ return out, nil
}
// handleNewCanary creates a canary: a brand-new canary/<name> channel
@@ -822,7 +827,10 @@ func (s *Server) handleNewCanary(w http.ResponseWriter, r *http.Request) {
return
}
s.audit(app, addr, "canary-create", "created canary "+name+" at "+version)
- if hosts := s.channelHosts(app, channel); len(hosts) > 0 {
+ // Best-effort here: which URL to show in the success flash is cosmetic,
+ // so a registry hiccup just falls back to the generic "attach one" copy
+ // rather than failing the create that already succeeded.
+ if hosts, _ := s.channelHosts(app, channel); len(hosts) > 0 {
back("ok", "Created canary "+name+" — live at https://"+hosts[0]+" serving "+version+" (canaries never bake).")
return
}
@@ -856,8 +864,16 @@ func (s *Server) handleRetireCanary(w http.ResponseWriter, r *http.Request) {
}
channel := "canary/" + name
// A canary with followers is load-bearing: retiring its pointer would
- // strand every host on it.
- if hosts := s.channelHosts(app, channel); len(hosts) > 0 {
+ // strand every host on it. Fail CLOSED on a registry error — an "I
+ // couldn't check" must never be read as "nobody follows this channel".
+ hosts, err := s.channelHosts(app, channel)
+ if err != nil {
+ log.Printf("console: retire %s %s: check followers: %v", app, channel, err)
+ s.renderMessage(w, r, http.StatusInternalServerError, "Error",
+ "Could not verify no hosts follow this channel — try again.")
+ return
+ }
+ if len(hosts) > 0 {
back("err", "Canary "+name+" still has a host ("+hosts[0]+") following it — remove or repoint that route first.")
return
}
diff --git a/internal/console/server_test.go b/internal/console/server_test.go
index 4bd310957..c0cecb579 100644
--- a/internal/console/server_test.go
+++ b/internal/console/server_test.go
@@ -712,6 +712,35 @@ func TestRetireCanary(t *testing.T) {
}
}
+// TestRetireCanaryFailsClosedOnRegistryError pins the fail-closed contract: a
+// registry that can't be read must never be read as "no hosts follow this
+// channel" — that would let a transient hiccup delete a channel pointer live
+// hosts still follow. Deliberately does NOT touch /apps/kass before the
+// retire call (any earlier request would populate the console's process-wide
+// registry handle cache and hide the failure) — the retire POST is the very
+// first console-side access to this fixture's registry file, and by then the
+// file on disk has been turned into a directory so the open fails outright.
+func TestRetireCanaryFailsClosedOnRegistryError(t *testing.T) {
+ fx := newConsoleFixture(t)
+ c := signInPaul(t, fx)
+ csrf := sessionToken(t, fx, c)
+
+ if err := os.Remove(fx.regPath); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Mkdir(fx.regPath, 0o755); err != nil {
+ t.Fatal(err)
+ }
+
+ status, _, body := postForm(t, c, fx.ts.URL+"/apps/kass/canary/retire", url.Values{"csrf": {csrf}, "name": {"x"}})
+ if status != http.StatusInternalServerError {
+ t.Fatalf("retire with an unreadable registry = %d, want 500 (fail closed)", status)
+ }
+ if !strings.Contains(body, "Could not verify no hosts follow this channel") {
+ t.Fatalf("retire-with-broken-registry body = %q, want the fail-closed refusal", body)
+ }
+}
+
func TestRetireWithoutDeletePrimitive(t *testing.T) {
// A release store without Delete (today's release.Store exactly): the UI
// stays, retirement degrades to a polite refusal, nothing changes.