fix(middleware): admin JSON endpoints no longer serve unauthenticated callers

CHANGELOG.md

@@ -65,6 +65,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Security
 
+- **Unauthenticated read of `/admin/stats` and `/admin/metrics` is fixed.** `SoftSessionAuth`
+  decided between the login overlay and a hard 401 by testing the request for
+  `Accept: application/json`. A caller sending anything non-committal — `*/*`, or no
+  Accept header at all, which is curl's default — matched neither branch and fell
+  through to the handler, which returned its payload in full: Go version, goroutine
+  count, heap and GC statistics, uptime, and article/draft/tag counts. Present since
+  the endpoints moved under `/admin`; affects any deployment with `ADMIN_USERNAME` and
+  `ADMIN_PASSWORD` set. No article content, credentials, or session data was reachable,
+  and write paths were never exposed. Fixed twice over: soft-fail now requires an
+  explicit `text/html` Accept, and the two JSON-only endpoints moved to hard
+  `SessionAuth`, which has no fall-through path at all. Browser behaviour is unchanged.
 - Go toolchain 1.26.4 → 1.26.5 for GO-2026-5856.
 - `golang.org/x/crypto` 0.48.0 → 0.52.0, `golang.org/x/net` 0.51.0 → 0.55.0,
   `golang.org/x/sys` 0.41.0 → 0.45.0, `golang.org/x/text` 0.38.0 → 0.40.0.

internal/commands/serve/command.go

@@ -594,8 +594,19 @@ func setupRoutes(router *gin.Engine, h *handlers.Router, sessionStore *middlewar
 		registerGET(adminGroup, "/writing", h.Admin.Writing)
 		registerGET(adminGroup, "/drafts", h.Admin.Drafts)
 		adminGroup.POST("/cache/clear", h.ClearCache)
-		registerGET(adminGroup, "/stats", h.Admin.Stats)
-		registerGET(adminGroup, "/metrics", h.Admin.Metrics)
+		// JSON-only endpoints take hard auth, not soft. Soft auth exists so an HTML
+		// page can render the login overlay in place; a handler that always answers
+		// JSON has no overlay to render, so soft-failing it can only ever mean
+		// returning the data. Belt and braces with the Accept check in
+		// SoftSessionAuth — neither should be the only thing standing here.
+		adminJSONGroup := router.Group("/admin")
+		adminJSONGroup.Use(
+			middleware.RecoveryWithErrorHandler(logger),
+			middleware.SessionAuth(sessionStore),
+			middleware.NoCache(),
+		)
+		registerGET(adminJSONGroup, "/stats", h.Admin.Stats)
+		registerGET(adminJSONGroup, "/metrics", h.Admin.Metrics)
 		adminGroup.POST("/articles/reload", h.Admin.ReloadArticles)
 
 		// AMA moderation routes

internal/middleware/errors.go

@@ -26,6 +26,17 @@ func wantsJSON(c *gin.Context) bool {
 	return strings.Contains(accept, "application/json")
 }
 
+// acceptsHTML reports whether the caller explicitly asked for HTML.
+//
+// Deliberately not the negation of wantsJSON. Use this to decide whether a caller
+// can be shown an HTML affordance such as the login overlay: the answer must be no
+// for `Accept: */*`, an absent Accept header, or anything else non-committal, so
+// that a caller which cannot render the affordance is refused rather than handed
+// whatever the handler would have produced.
+func acceptsHTML(c *gin.Context) bool {
+	return strings.Contains(c.GetHeader("Accept"), "text/html")
+}
+
 // errorHTML returns a minimal self-contained HTML error page.
 // Does not use the template engine — middleware must be dependency-free.
 func errorHTML(status int, message string) string {

internal/middleware/session.go

@@ -197,8 +197,18 @@ func SoftSessionAuth(store *SessionStore, secureCookie bool) gin.HandlerFunc {
 		//                  fall-through would leak handler data (see GHSA / #42).
 		// Do NOT collapse this asymmetry without revisiting the bypass class.
 		if c.Request.Method == http.MethodGet || c.Request.Method == http.MethodHead {
-			if wantsJSON(c) {
-				slog.Warn("Unauthenticated JSON request", "method", c.Request.Method, "path", c.Request.URL.Path)
+			// Fail closed. Soft-fail hands the request to the handler so an HTML page
+			// can render the login overlay in place, so it is only safe for a caller
+			// that both prefers HTML and can render it. Testing wantsJSON alone left
+			// `Accept: */*` — curl's default, and most scripts — falling through to
+			// the handler, which for the JSON-only admin endpoints meant returning
+			// their data in full to an unauthenticated caller.
+			//
+			// Order matters: a client that lists both still gets 401 when it prefers
+			// JSON, so this tightens the non-committal case without loosening any
+			// case that already refused.
+			if wantsJSON(c) || !acceptsHTML(c) {
+				slog.Warn("Unauthenticated non-HTML request", "method", c.Request.Method, "path", c.Request.URL.Path)
 				abortWithError(c, http.StatusUnauthorized, "Authentication required")
 				return
 			}

internal/middleware/session_test.go

@@ -280,6 +280,9 @@ func TestSoftSessionAuth_NoSession_GET(t *testing.T) {
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 	c.Request = httptest.NewRequest(http.MethodGet, "/compose", http.NoBody)
+	// A browser is the only caller that can render the overlay, and a browser always
+	// sends this. Without it the request is non-committal and must be refused.
+	c.Request.Header.Set("Accept", "text/html,application/xhtml+xml")
 
 	handler := SoftSessionAuth(store, false)
 	handler(c)
@@ -324,6 +327,7 @@ func TestSoftSessionAuth_InvalidToken_GET(t *testing.T) {
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 	c.Request = httptest.NewRequest(http.MethodGet, "/admin", http.NoBody)
+	c.Request.Header.Set("Accept", "text/html,application/xhtml+xml")
 	c.Request.AddCookie(&http.Cookie{Name: "_session", Value: "bogus"})
 
 	handler := SoftSessionAuth(store, false)
@@ -345,6 +349,7 @@ func TestSoftSessionAuth_HEAD_Request(t *testing.T) {
 	w := httptest.NewRecorder()
 	c, _ := gin.CreateTestContext(w)
 	c.Request = httptest.NewRequest(http.MethodHead, "/compose", http.NoBody)
+	c.Request.Header.Set("Accept", "text/html,application/xhtml+xml")
 
 	handler := SoftSessionAuth(store, false)
 	handler(c)
@@ -467,7 +472,12 @@ func TestSoftSessionAuth_NoSession_JSON_AcceptVariants(t *testing.T) {
 		{"with charset", "application/json; charset=utf-8", true, http.StatusUnauthorized},
 		{"q-weighted multi", "text/html;q=0.9, application/json;q=1.0", true, http.StatusUnauthorized},
 		{"html only", "text/html", false, 0}, // falls through to soft-fail
-		{"empty", "", false, 0},              // falls through to soft-fail
+		// A caller that does not commit to HTML cannot render the overlay, so it is
+		// refused rather than handed whatever the handler produces. This case used to
+		// fall through, which is how an unauthenticated `curl /admin/metrics` — no
+		// Accept header at all — got a 200 with full runtime metrics.
+		{"empty", "", true, http.StatusUnauthorized},
+		{"wildcard only", "*/*", true, http.StatusUnauthorized},
 	}
 
 	for _, tc := range cases {
@@ -565,3 +575,47 @@ func TestShutdownRateLimiters_StopsAllCleanupGoroutines(t *testing.T) {
 	// Idempotency: second call should be a no-op.
 	assert.NotPanics(t, ShutdownRateLimiters)
 }
+
+// The bypass this pins is the one an attacker actually runs: plain `curl` against a
+// JSON admin endpoint. curl sends no Accept header, which was neither "wants JSON"
+// nor anything the old check refused, so the request fell through to the handler and
+// came back 200 with the payload. Exercised through a real router with a real
+// handler, because the leak was never in the middleware's own response — it was in
+// what ran after the middleware declined to stop it.
+func TestSoftSessionAuth_NonCommittalAccept_DoesNotReachHandler(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+
+	for _, tc := range []struct {
+		name   string
+		accept string
+	}{
+		{"no accept header at all (curl default)", ""},
+		{"wildcard", "*/*"},
+		{"unrelated type", "application/xml"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			store := NewSessionStore()
+			router := gin.New()
+			handlerRan := false
+			router.Use(SoftSessionAuth(store, false))
+			router.GET("/admin/metrics", func(c *gin.Context) {
+				handlerRan = true
+				c.JSON(http.StatusOK, gin.H{"goroutines": 25, "memory": gin.H{"heap_alloc": 51403288}})
+			})
+
+			req := httptest.NewRequest(http.MethodGet, "/admin/metrics", http.NoBody)
+			if tc.accept != "" {
+				req.Header.Set("Accept", tc.accept)
+			}
+			w := httptest.NewRecorder()
+			router.ServeHTTP(w, req)
+
+			assert.False(t, handlerRan, "handler must not run for an unauthenticated non-HTML caller")
+			assert.Equal(t, http.StatusUnauthorized, w.Code)
+			for _, leaked := range []string{"goroutines", "heap_alloc", "memory"} {
+				assert.NotContains(t, w.Body.String(), leaked,
+					"response must not carry admin payload key %q", leaked)
+			}
+		})
+	}
+}