3 files changed, 105 insertions(+), 13 deletions(-)

M cmd/doltsrht/main.go
M web/router.go
M web/web_test.go

@@ 25,7 25,7 @@ import (
 	"os"
 
 	"github.com/go-chi/chi/v5"
-	chimw "github.com/go-chi/chi/v5/middleware"
+	chimiddleware "github.com/go-chi/chi/v5/middleware"
 	_ "github.com/lib/pq" // registers the "postgres" database/sql driver
 	"github.com/vaughan0/go-ini"
 


@@ 37,6 37,7 @@ import (
 	"sourcecraft.dev/bigbes/sr-ht-core/database"
 	"sourcecraft.dev/bigbes/sr-ht-core/server"
 
+	"sourcecraft.dev/bigbes/sr-ht-ecore/chimw"
 	"sourcecraft.dev/bigbes/sr-ht-ecore/instconf"
 
 	"sourcecraft.dev/bigbes/sr-ht-dolt/authn"


@@ 200,7 201,17 @@ func main() {
 	}
 
 	srv.AnonRouter().Group(func(r chi.Router) {
-		r.Use(chimw.RealIP, chimw.Recoverer)
+		// RequestID and RealIP first: the request line below carries the id and
+		// the viewer's address, and neither exists until these have run.
+		r.Use(chimiddleware.RequestID, chimiddleware.RealIP)
+		// The request line as a slog record rather than chi's colourised line on
+		// stdout — the one line this daemon emitted that was neither structured
+		// nor on stderr, so an operator grepping the journal for a request id
+		// found every panic and none of the requests. It goes outermost, above
+		// the panic guards, so that the status it reports is the one that
+		// actually went out.
+		r.Use(chimw.RequestLogger(chimw.SlogFormatter{}))
+		r.Use(chimiddleware.Recoverer)
 		r.Use(config.Middleware(conf, serviceName), database.Middleware(db))
 		r.Use(authn.OptionalCookieMiddleware()) // never 401s; anonymous stays anonymous
 

@@ 9,6 9,7 @@ import (
 	"github.com/go-chi/chi/v5"
 
 	"sourcecraft.dev/bigbes/sr-ht-ecore/assets"
+	"sourcecraft.dev/bigbes/sr-ht-ecore/chimw"
 	"sourcecraft.dev/bigbes/sr-ht-ecore/chrome"
 	"sourcecraft.dev/bigbes/sr-ht-ecore/csrf"
 	"sourcecraft.dev/bigbes/sr-ht-ecore/internalauth"


@@ 172,6 173,13 @@ func (a *app) mount(r chi.Router) {
 	r.With(internalauth.Guard(core.InternalClientID, core.InternalNodeID, nil)).
 		Post("/internal/repos", a.handleInternalCreate)
 
+	// A URL this router does not serve, and a method it does not allow, are
+	// answered by the same page every other refusal here is. chi's own pair is
+	// net/http's plain text — no chrome, no nav, and no way out for a viewer who
+	// mistyped an address. It is a registration on the tree rather than a link
+	// in a chain, so it is installed once and inherited by everything below.
+	chimw.RenderRefusals(r, a.fail)
+
 	// Everything a browser reaches. The same-origin guard is the group's, not
 	// each mutating handler's: a predicate spelled per handler is protection
 	// somebody has to remember, and the form added next year is the one that


@@ 179,21 187,29 @@ func (a *app) mount(r chi.Router) {
 	r.Group(func(r chi.Router) {
 		r.Use(csrf.Require(a.chrome.SelfOrigin(), a.denyCSRF))
 
-		r.Get("/", a.handleIndex)
-		r.Get("/create", a.handleCreateForm)
+		// Read routes are registered for GET and HEAD both. Nothing on this
+		// surface reads r.Method, so a HEAD is the same query answered by the
+		// same handler and can never say 200 where the GET says 404 — which on
+		// these pages would be the visibility leak the 404 exists to prevent.
+		// Registered rather than rewritten per request, so the routing tree
+		// stays the one record of what this service serves. The mutating half
+		// of a form page is registered beside it with r.Post: a HEAD that
+		// writes is not a HEAD.
+		chimw.GetHead(r, "/", a.handleIndex)
+		chimw.GetHead(r, "/create", a.handleCreateForm)
 		r.Post("/create", a.handleCreate)
 
-		r.Get("/settings/keys", a.handleKeys)
+		chimw.GetHead(r, "/settings/keys", a.handleKeys)
 		r.Post("/settings/keys", a.handleKeysPost)
 
-		r.Get("/~{user}", a.handleUser)
-		r.Get("/~{user}/{db}", a.handleOverview)
-		r.Get("/~{user}/{db}/log", a.handleLog)
-		r.Get("/~{user}/{db}/commit/{hash}", a.handleCommit)
-		r.Get("/~{user}/{db}/tree/{ref}", a.handleTree)
-		r.Get("/~{user}/{db}/table/{ref}/{table}", a.handleTable)
-		r.Get("/~{user}/{db}/view/{view}", a.handleView)
-		r.Get("/~{user}/{db}/settings", a.handleSettings)
+		chimw.GetHead(r, "/~{user}", a.handleUser)
+		chimw.GetHead(r, "/~{user}/{db}", a.handleOverview)
+		chimw.GetHead(r, "/~{user}/{db}/log", a.handleLog)
+		chimw.GetHead(r, "/~{user}/{db}/commit/{hash}", a.handleCommit)
+		chimw.GetHead(r, "/~{user}/{db}/tree/{ref}", a.handleTree)
+		chimw.GetHead(r, "/~{user}/{db}/table/{ref}/{table}", a.handleTable)
+		chimw.GetHead(r, "/~{user}/{db}/view/{view}", a.handleView)
+		chimw.GetHead(r, "/~{user}/{db}/settings", a.handleSettings)
 		r.Post("/~{user}/{db}/settings", a.handleSettingsPost)
 
 		// The static tree, with the cache policy the hashed names imply and no

@@ 712,6 712,71 @@ func TestRefusalsRenderTheSharedErrorPage(t *testing.T) {
 	assert.Contains(t, denied.Body.String(), "Only the owner may change database settings.")
 }
 
+// TestRoutingRefusalsRenderTheSharedErrorPage covers the two refusals that never
+// reach a handler at all — a path this router does not serve, and a method it
+// does not allow. Both used to fall through to chi's net/http default: plain
+// text, no chrome, no nav, and the only refusals on this instance that did not
+// look like the service they came from.
+func TestRoutingRefusalsRenderTheSharedErrorPage(t *testing.T) {
+	h := newHarness(t)
+
+	unrouted := h.do("GET", "/no/such/path", nil, nil)
+	require.Equal(t, http.StatusNotFound, unrouted.Code)
+	assert.Contains(t, unrouted.Body.String(), pages.NotFoundMessage)
+	assert.Contains(t, unrouted.Body.String(), `<span class="text-danger">dolt</span>`,
+		"an unrouted URL is answered through our chrome")
+
+	// POST to a read-only route: routed, but not for this method.
+	badMethod := h.do("POST", "/~alice/anything/log", nil, url.Values{})
+	require.Equal(t, http.StatusMethodNotAllowed, badMethod.Code)
+	assert.Contains(t, badMethod.Body.String(), pages.MethodMessage)
+}
+
+// TestReadRoutesAnswerHead walks the routing tree and requires every GET route
+// to be registered for HEAD as well.
+//
+// It asks the tree rather than issuing requests because the tree is the record
+// that matters: a middleware that rewrote the method per request would answer
+// HEAD while chi's own 405 handler, built out of the methods that were
+// registered, still said the route accepts GET alone. A read route that answers
+// `curl -I` with a 405 and a kilobyte of error page is a route no monitor and no
+// cache can revalidate cheaply.
+func TestReadRoutesAnswerHead(t *testing.T) {
+	h := newHarness(t)
+
+	methods := map[string]map[string]bool{}
+	require.NoError(t, chi.Walk(h.router,
+		func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error {
+			if methods[route] == nil {
+				methods[route] = map[string]bool{}
+			}
+			methods[route][method] = true
+			return nil
+		}))
+	require.NotEmpty(t, methods)
+
+	for route, served := range methods {
+		if served[http.MethodGet] {
+			assert.True(t, served[http.MethodHead], "%s serves GET but not HEAD", route)
+		}
+	}
+}
+
+// TestHeadOnAPrivateDatabaseIsStillNotFound: the HEAD twin shares the GET's
+// handler, so it cannot answer 200 where the GET answers 404. That equivalence
+// is what keeps HEAD from becoming a cheap existence oracle for somebody else's
+// private database (SPEC ch. 6.3).
+func TestHeadOnAPrivateDatabaseIsStillNotFound(t *testing.T) {
+	h := newHarness(t)
+	h.store.add(&core.Repo{Name: "sec", OwnerID: 1, OwnerName: "alice", Path: "/s", Visibility: core.VisibilityPrivate})
+	h.store.add(&core.Repo{Name: "pub", OwnerID: 1, OwnerName: "alice", Path: "/p", Visibility: core.VisibilityPublic})
+
+	assert.Equal(t, http.StatusNotFound, h.do("HEAD", "/~alice/sec", nil, nil).Code)
+	assert.Equal(t, http.StatusNotFound, h.do("HEAD", "/~alice/nosuch", nil, nil).Code,
+		"a private database and a missing one must be indistinguishable to HEAD too")
+	assert.Equal(t, http.StatusOK, h.do("HEAD", "/~alice/pub", nil, nil).Code)
+}
+
 // leakyView renders a page whose content block reads a field its envelope does
 // not carry, so executing it fails halfway. It is the shape of the bug the old
 // renderer turned into a disclosure.