appview: refuse /v2/_catalog with UNSUPPORTED, and send Allow on every 405

/v2/_catalog answered a bare request with 200 {"repositories":[]} but 400ed
any n, because buildDistributionConfig leaves Catalog.MaxEntries at 0 and the
library rejects n > max. crane catalog sends n=1000, so it always failed. The
same endpoint both worked and rejected a legal parameter.

Setting MaxEntries was the obvious fix and is the wrong one. This registry has
no global catalog and will not grow one: repositories live in per-user ATProto
PDS namespaces, and buildStorageConfig hands the library a placeholder
inmemory driver, so its enumeration is empty by construction. An empty 200
asserts that this registry contains no repositories, which is false and
silently so. UNSUPPORTED says the true thing, and matches the vocabulary
DomainRoutingMiddleware already uses to refuse /v2/* on the UI domain.

There is no conformance cost: _catalog is not in the OCI distribution spec at
all. It is a Docker Registry HTTP API V2 extension, and the spec places
repository discovery out of scope. Docker Hub and GHCR both refuse it outright
and Quay returns an unconditional empty list; none of them 400s a legal n.

The path had three distinct behaviours, not two, and all three now collapse to
one: GET varied by parameter, HEAD was answered by gorilla's MethodHandler
with a bare 405, and the trailing-slash form 301-redirected because
distribution sets StrictSlash(true). Both path forms are registered, and chi
prefers a static pattern over the /v2/* wildcard regardless of declaration
order (verified against the pinned chi version, and pinned by a test that
fails if a request reaches the distribution stand-in).

Also adds the Allow header that RFC 9110 requires on any 405 — a MUST in both
15.5.6 and 10.2.1, not a SHOULD. The value is empty, which 10.2.1 defines as
"the resource allows no methods": true for the catalog, and true for /v2/* on
the UI domain, so the pre-existing gap in DomainRoutingMiddleware is closed
too. Naming a method there would advertise something that does not work.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9

pkg/appview/catalog_test.go

@@ -0,0 +1,174 @@
+package appview
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/go-chi/chi/v5"
+)
+
+// catalogRouter wires the production registration, mountCatalogRefusal, over a
+// stand-in for the distribution app, so the test can tell an intercepted request
+// from one that reached the library. Calling the real helper rather than
+// restating its routes keeps the test from drifting away from NewServer.
+func catalogRouter() (chi.Router, *bool) {
+	reachedDistribution := false
+	r := chi.NewRouter()
+	// Registered first on purpose: chi must prefer the static patterns below
+	// even when the wildcard is declared ahead of them.
+	r.Handle("/v2/*", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		reachedDistribution = true
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"repositories":[]}`))
+	}))
+	mountCatalogRefusal(r)
+	return r, &reachedDistribution
+}
+
+// TestCatalogUnsupported pins the contract: /v2/_catalog answers UNSUPPORTED
+// with 405 for every parameter shape, so no query string can change the answer.
+func TestCatalogUnsupported(t *testing.T) {
+	paths := []string{
+		"/v2/_catalog",
+		"/v2/_catalog?n=1",
+		"/v2/_catalog?n=1000",
+		"/v2/_catalog?n=0",
+		"/v2/_catalog?n=-1",
+		"/v2/_catalog?n=abc",
+		"/v2/_catalog?last=foo",
+		"/v2/_catalog?n=1000&last=foo",
+		"/v2/_catalog/",
+		"/v2/_catalog/?n=1000",
+	}
+	methods := []string{http.MethodGet, http.MethodHead, http.MethodPost, http.MethodDelete}
+
+	for _, path := range paths {
+		for _, method := range methods {
+			t.Run(method+" "+path, func(t *testing.T) {
+				router, reached := catalogRouter()
+				rec := httptest.NewRecorder()
+				router.ServeHTTP(rec, httptest.NewRequest(method, path, nil))
+
+				if *reached {
+					t.Fatalf("request reached the distribution handler, chi routed %q to the wildcard", path)
+				}
+				if rec.Code != http.StatusMethodNotAllowed {
+					t.Errorf("status = %d, want %d", rec.Code, http.StatusMethodNotAllowed)
+				}
+				if ct := rec.Header().Get("Content-Type"); ct != "application/json" {
+					t.Errorf("Content-Type = %q, want application/json", ct)
+				}
+
+				var body struct {
+					Errors []struct {
+						Code    string `json:"code"`
+						Message string `json:"message"`
+					} `json:"errors"`
+				}
+				if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
+					t.Fatalf("decode body %q: %v", rec.Body.String(), err)
+				}
+				if len(body.Errors) != 1 {
+					t.Fatalf("got %d errors, want 1: %s", len(body.Errors), rec.Body.String())
+				}
+				if body.Errors[0].Code != "UNSUPPORTED" {
+					t.Errorf("code = %q, want UNSUPPORTED", body.Errors[0].Code)
+				}
+				if body.Errors[0].Message == "" {
+					t.Error("message is empty, it should explain why there is no catalog")
+				}
+			})
+		}
+	}
+}
+
+// TestCatalogResponseIsIdentical asserts the stronger property: not merely that
+// every form is UNSUPPORTED, but that every form produces byte-identical output,
+// so no parameter is left able to influence the answer.
+func TestCatalogResponseIsIdentical(t *testing.T) {
+	paths := []string{
+		"/v2/_catalog",
+		"/v2/_catalog?n=1",
+		"/v2/_catalog?n=1000",
+		"/v2/_catalog?n=0",
+		"/v2/_catalog?n=-1",
+		"/v2/_catalog?n=abc",
+		"/v2/_catalog?last=foo",
+		"/v2/_catalog/",
+	}
+
+	var wantCode int
+	var wantBody string
+	for i, path := range paths {
+		router, _ := catalogRouter()
+		rec := httptest.NewRecorder()
+		router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
+		if i == 0 {
+			wantCode, wantBody = rec.Code, rec.Body.String()
+			continue
+		}
+		if rec.Code != wantCode || rec.Body.String() != wantBody {
+			t.Errorf("%s returned %d %q, want %d %q", path, rec.Code, rec.Body.String(), wantCode, wantBody)
+		}
+	}
+}
+
+// TestCatalogDoesNotShadowRepositories guards the interception's blast radius:
+// real repository paths must still reach the distribution handler.
+func TestCatalogDoesNotShadowRepositories(t *testing.T) {
+	paths := []string{
+		"/v2/",
+		"/v2/alice.example.com/nginx/tags/list",
+		"/v2/alice.example.com/_catalog/manifests/latest",
+		"/v2/_catalogue/nginx/tags/list",
+	}
+	for _, path := range paths {
+		t.Run(path, func(t *testing.T) {
+			router, reached := catalogRouter()
+			rec := httptest.NewRecorder()
+			router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
+			if !*reached {
+				t.Errorf("%s was intercepted, it should reach the distribution handler (got %d %q)", path, rec.Code, rec.Body.String())
+			}
+		})
+	}
+}
+
+// TestCatalogSends405Allow pins the Allow header RFC 9110 requires on a 405.
+//
+// 15.5.6 and 10.2.1 both state it as a MUST: "An origin server MUST generate an
+// Allow header field in a 405 (Method Not Allowed) response." UNSUPPORTED maps
+// to 405, so the header has to be there. The value is empty on purpose —
+// 10.2.1: "An empty Allow field value indicates that the resource allows no
+// methods" — which is the truth for a catalog that does not exist.
+//
+// Asserted via the header map rather than the raw wire because Go writes an
+// empty field value as "Allow: " and a client reads it back as present with an
+// empty string, which is the distinction that matters: present-and-empty is
+// compliant, absent is not.
+func TestCatalogSends405Allow(t *testing.T) {
+	r, _ := catalogRouter()
+
+	for _, method := range []string{http.MethodGet, http.MethodHead, http.MethodPost} {
+		for _, path := range []string{"/v2/_catalog", "/v2/_catalog/", "/v2/_catalog?n=1000"} {
+			req := httptest.NewRequest(method, path, nil)
+			rec := httptest.NewRecorder()
+			r.ServeHTTP(rec, req)
+
+			if rec.Code != http.StatusMethodNotAllowed {
+				t.Errorf("%s %s: status = %d, want %d", method, path, rec.Code, http.StatusMethodNotAllowed)
+			}
+			values, present := rec.Result().Header["Allow"]
+			if !present {
+				t.Errorf("%s %s: no Allow header on a 405; RFC 9110 15.5.6 makes it a MUST", method, path)
+				continue
+			}
+			if len(values) != 1 || values[0] != "" {
+				t.Errorf("%s %s: Allow = %q, want exactly one empty value (RFC 9110 10.2.1: the resource allows no methods)",
+					method, path, values)
+			}
+		}
+	}
+}

pkg/appview/server.go

@@ -559,6 +559,9 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer,
 	wrappedApp := middleware.BearerChallenge(cfg.Server.BaseURL+"/auth/token", cfg.Auth.Services)(
 		middleware.RetryAfterMiddleware(middleware.ExtractAuthMethod(app)))
 
+	// Refuse the global catalog before the distribution handler can see it.
+	mountCatalogRefusal(mainRouter)
+
 	// Mount registry at /v2/
 	mainRouter.Handle("/v2/*", wrappedApp)
 
@@ -846,6 +849,7 @@ func DomainRoutingMiddleware(registryDomains []string, uiBaseURL string) func(ht
 			case host == uiHost:
 				// UI domain: block /v2/*, serve everything else
 				if isV2 {
+					writeEmptyAllow(w)
 					if err := errcode.ServeJSON(w, errcode.ErrorCodeUnsupported.WithMessage(
 						fmt.Sprintf("registry API is not available on this domain, use %s", primaryReg),
 					)); err != nil {
@@ -1150,6 +1154,69 @@ var gzipUI = func() func(http.Handler) http.HandlerFunc {
 	return wrapper
 }()
 
+// mountCatalogRefusal registers the /v2/_catalog refusal on r. It must be
+// called on the same router that mounts "/v2/*": chi prefers a static pattern
+// over a wildcard, so these two registrations win regardless of declaration
+// order. The trailing-slash form is spelled out because the distribution router
+// runs with StrictSlash(true) and would otherwise answer it with a 301 to the
+// bare path, costing a round trip to reach the same refusal.
+//
+// Handle registers every method, so HEAD and anything else land here too.
+func mountCatalogRefusal(r chi.Router) {
+	r.Handle("/v2/_catalog", http.HandlerFunc(handleCatalogUnsupported))
+	r.Handle("/v2/_catalog/", http.HandlerFunc(handleCatalogUnsupported))
+}
+
+// handleCatalogUnsupported answers GET /v2/_catalog (and every other method and
+// query string on that path) with an OCI UNSUPPORTED error, HTTP 405.
+//
+// This registry has no global catalog and will not grow one. Repositories live
+// in per-user ATProto PDS namespaces, not in a single enumerable store, so
+// there is nothing for the distribution library to walk: buildStorageConfig()
+// hands it a placeholder inmemory driver, which makes its enumeration empty by
+// construction. Left to itself the library answers a bare request with
+// 200 {"repositories":[]} and 400s any n (Catalog.MaxEntries is 0), so the same
+// endpoint both "works" and rejects a legal parameter. Worse, the empty 200
+// asserts that this registry contains no repositories, which is false and
+// silently so. UNSUPPORTED says the true thing.
+//
+// There is no conformance cost. _catalog is not in the OCI distribution spec at
+// all: it is a Docker Registry HTTP API V2 extension, and the OCI spec places
+// repository discovery out of scope. Docker Hub and GHCR refuse _catalog
+// outright; Quay returns an unconditional empty list. None of them 400s a legal
+// n. Fixing this by setting Catalog.MaxEntries was considered and rejected: it
+// would make the lie coherent rather than remove it.
+//
+// HEAD gets the same status and headers, with the body suppressed by net/http.
+func handleCatalogUnsupported(w http.ResponseWriter, _ *http.Request) {
+	writeEmptyAllow(w)
+	if err := errcode.ServeJSON(w, errcode.ErrorCodeUnsupported.WithMessage(
+		"this registry does not offer a global catalog, repositories are namespaced per user: list tags for a known repository at /v2/{handle-or-did}/{image}/tags/list",
+	)); err != nil {
+		slog.Error("failed to write OCI error response", "error", err)
+	}
+}
+
+// writeEmptyAllow sets the Allow header required on a 405 response.
+//
+// RFC 9110 makes this a MUST, in both 15.5.6 and 10.2.1: "An origin server MUST
+// generate an Allow header field in a 405 (Method Not Allowed) response." The
+// OCI UNSUPPORTED error code maps to 405 (its descriptor in the distribution
+// library sets HTTPStatusCode: http.StatusMethodNotAllowed), so every place we
+// serve UNSUPPORTED owes the header.
+//
+// The value is deliberately empty. 10.2.1: "An empty Allow field value
+// indicates that the resource allows no methods." That is precisely the case at
+// both call sites — the global catalog does not exist and never will, and the
+// registry API is not served on the UI domain under any method — so naming a
+// method here would advertise something that does not work. Go writes the field
+// as "Allow: " and clients read it back as present-and-empty.
+//
+// Must be called before ServeJSON, which commits the status line.
+func writeEmptyAllow(w http.ResponseWriter) {
+	w.Header().Set("Allow", "")
+}
+
 // compressUIResponses gzips UI, static and JSON responses and leaves the OCI
 // registry API at /v2/* alone.
 //