From f8d9ad7fe9e85ddd0a1501eb0b8603104b832a7e Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Thu, 13 Aug 2026 20:32:35 -0500 Subject: [PATCH] appview: let a registry domain keep its port, and its /v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DomainRoutingMiddleware normalized the request Host to a bare hostname but matched server.registry_domains verbatim, so any configured domain carrying a port could never match. config-appview.example.yaml ships `registry_domains: [127.0.0.1:5000, atcr.io]`, which means that entry has been inert since it was written. It fails closed in the worst way. That same host is also the auto-detected UI host, and `host == uiHost` was evaluated first, so /v2/* was answered with "registry API is not available on this domain, use 127.0.0.1:5000" — naming the exact host the client had just used. The registry API is unreachable on the dev stack, and any single-host deployment hits the same wall: listing a host in registry_domains does nothing if it is also the UI host. Both sides are now normalized through hostWithoutPort, and a registry domain takes /v2/* even when it doubles as the UI host, which is a legitimate single-domain deployment. Everything else is unchanged: a UI-only host still refuses /v2/, registry domains still redirect non-/v2 traffic to the UI, and /auth/token and /auth/device/* are still served directly so a cross-host 307 cannot strip the Authorization header. hostWithoutPort uses net.SplitHostPort instead of the previous LastIndex(":") scan, which mangled bracketed IPv6 literals into "[::1" and could never match the "::1" that url.URL.Hostname() yields for the UI host. The middleware had no tests at all. The two failing cases are pinned first, and the four pre-existing behaviours are pinned alongside them so the reorder cannot quietly widen what /v2/ is served on. Pre-existing at efabb677 rather than introduced by this range, but it blocks every registry-facing batch in the stack, so it lands at the base. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/appview/domain_routing_test.go | 122 +++++++++++++++++++++++++++++ pkg/appview/server.go | 29 +++++-- 2 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 pkg/appview/domain_routing_test.go diff --git a/pkg/appview/domain_routing_test.go b/pkg/appview/domain_routing_test.go new file mode 100644 index 0000000..6e8fbb8 --- /dev/null +++ b/pkg/appview/domain_routing_test.go @@ -0,0 +1,122 @@ +package appview + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// DomainRoutingMiddleware had no coverage at all, which is how both defects +// below survived into a shipped example config. +// +// The middleware strips the port from the request Host before matching, but +// compares against server.registry_domains verbatim. config-appview.example.yaml +// ships `registry_domains: [127.0.0.1:5000, atcr.io]`, so the first entry can +// never match any request — and because it also equals the auto-detected UI +// host, /v2/* is answered with "registry API is not available on this domain, +// use 127.0.0.1:5000", naming the very host that was used. + +const ( + bodyRegistry = "NEXT" // the wrapped handler ran +) + +func routingProbe(t *testing.T, registryDomains []string, uiBaseURL, host, path string) *httptest.ResponseRecorder { + t.Helper() + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(bodyRegistry)) + }) + h := DomainRoutingMiddleware(registryDomains, uiBaseURL)(next) + + req := httptest.NewRequest(http.MethodGet, path, nil) + req.Host = host + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec +} + +// A port-bearing registry domain must match a request to that host:port. +// The shipped example config depends on this and it does not hold today. +func TestDomainRouting_PortBearingRegistryDomainMatches(t *testing.T) { + rec := routingProbe(t, + []string{"registry.example:5000"}, + "http://ui.example:5000", + "registry.example:5000", + "/v2/", + ) + if rec.Body.String() != bodyRegistry { + t.Fatalf("registry domain configured with a port did not serve /v2/: status=%d body=%q", + rec.Code, rec.Body.String()) + } +} + +// A single host serving both the UI and the registry is a legitimate +// deployment, and is exactly what the dev stack is. Listing that host in +// registry_domains should grant it /v2/*, but `host == uiHost` is evaluated +// first, so the registry entry is silently ignored. +func TestDomainRouting_SameHostServesBothUIAndRegistry(t *testing.T) { + const host = "127.0.0.1:5000" + + v2 := routingProbe(t, []string{host}, "http://127.0.0.1:5000", host, "/v2/") + if v2.Body.String() != bodyRegistry { + t.Errorf("/v2/ not served when the UI host is also a registry domain: status=%d body=%q", + v2.Code, v2.Body.String()) + } + + ui := routingProbe(t, []string{host}, "http://127.0.0.1:5000", host, "/settings") + if ui.Body.String() != bodyRegistry { + t.Errorf("UI path stopped working on a dual-role host: status=%d body=%q", + ui.Code, ui.Body.String()) + } +} + +// Guard the behaviour that must NOT change: a UI-only host still refuses /v2/. +func TestDomainRouting_UIOnlyHostStillBlocksV2(t *testing.T) { + rec := routingProbe(t, + []string{"registry.example"}, + "http://ui.example", + "ui.example", + "/v2/", + ) + if rec.Body.String() == bodyRegistry { + t.Fatal("/v2/ was served on a host that is not a registry domain") + } +} + +func TestDomainRouting_RegistryDomainRedirectsNonV2ToUI(t *testing.T) { + rec := routingProbe(t, + []string{"registry.example"}, + "http://ui.example", + "registry.example", + "/settings", + ) + if rec.Code != http.StatusTemporaryRedirect { + t.Fatalf("expected 307 to the UI, got %d", rec.Code) + } + if got := rec.Header().Get("Location"); got != "http://ui.example/settings" { + t.Fatalf("unexpected redirect target %q", got) + } +} + +// Auth endpoints stay on the registry domain: a cross-host 307 would strip the +// Authorization header. +func TestDomainRouting_AuthEndpointsServedOnRegistryDomain(t *testing.T) { + for _, path := range []string{"/auth/token", "/auth/device/code"} { + rec := routingProbe(t, []string{"registry.example"}, "http://ui.example", "registry.example", path) + if rec.Body.String() != bodyRegistry { + t.Errorf("%s was not served directly on the registry domain: status=%d", path, rec.Code) + } + } +} + +func TestDomainRouting_UnknownHostRedirectsExceptHealth(t *testing.T) { + rec := routingProbe(t, []string{"registry.example"}, "http://ui.example", "cdn.origin", "/") + if rec.Code != http.StatusTemporaryRedirect { + t.Errorf("unknown host should 307, got %d", rec.Code) + } + + health := routingProbe(t, []string{"registry.example"}, "http://ui.example", "cdn.origin", "/health") + if health.Body.String() != bodyRegistry { + t.Errorf("/health should be served on unknown hosts, got status=%d", health.Code) + } +} diff --git a/pkg/appview/server.go b/pkg/appview/server.go index 782db6f..afa07ee 100644 --- a/pkg/appview/server.go +++ b/pkg/appview/server.go @@ -809,9 +809,13 @@ func (s *AppViewServer) Shutdown(ctx context.Context) error { // 3. Unknown domains (CDN origins, IPs, etc.): redirects all requests to the // UI domain with 307, except /health for load balancer probes. func DomainRoutingMiddleware(registryDomains []string, uiBaseURL string) func(http.Handler) http.Handler { + // Request hosts are normalized to a bare hostname before matching, so the + // configured domains have to be normalized the same way. They frequently + // carry a port — config-appview.example.yaml ships "127.0.0.1:5000" — and + // matching those verbatim meant such an entry could never match anything. regDomains := make(map[string]bool, len(registryDomains)) for _, d := range registryDomains { - regDomains[d] = true + regDomains[hostWithoutPort(d)] = true } // Extract UI hostname from BaseURL (e.g., "https://seamark.dev" -> "seamark.dev") @@ -824,15 +828,19 @@ func DomainRoutingMiddleware(registryDomains []string, uiBaseURL string) func(ht return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - host := r.Host - if idx := strings.LastIndex(host, ":"); idx != -1 { - host = host[:idx] - } + host := hostWithoutPort(r.Host) path := r.URL.Path isV2 := path == "/v2" || path == "/v2/" || strings.HasPrefix(path, "/v2/") switch { + case regDomains[host] && isV2: + // A registry domain gets /v2/* even when it is also the UI host. + // One host serving both is a legitimate deployment (it is what + // the dev stack is); checking uiHost first meant listing that + // host in registry_domains was silently ignored. + next.ServeHTTP(w, r) + case host == uiHost: // UI domain: block /v2/*, serve everything else if isV2 { @@ -868,6 +876,17 @@ func DomainRoutingMiddleware(registryDomains []string, uiBaseURL string) func(ht } } +// hostWithoutPort strips a trailing :port from a host, leaving bare hostnames +// untouched. net.SplitHostPort is used rather than a LastIndex(":") scan so +// that bracketed IPv6 literals ("[::1]:5000") normalize to "::1" and match the +// form url.URL.Hostname() produces for the UI host. +func hostWithoutPort(h string) string { + if host, _, err := net.SplitHostPort(h); err == nil { + return host + } + return h +} + // primaryRegistryDomain returns the first registry domain, or empty string if none. func primaryRegistryDomain(domains []string) string { if len(domains) > 0 {