M README.md => README.md +4 -2
@@ 51,8 51,10 @@ authorizer spares git.sr.ht a GraphQL call on every page load.
the machinery around them is not here but in [sr-ht-ecore], wired up in
`web/server.go`: `chrome` for the page frame, `pages` for template discovery
and the shared error page, `assets` for the hashed artefacts and their cache
- policy, `csrf` and `middleware` for the router's guards. What is left is this
- service's own: the routes, the handlers, and the templates they render.
+ policy, `csrf` and `middleware` for the router's guards, `chimw` for the
+ GET+HEAD route pair, the rendered 404/405 and the request log record. What is
+ left is this service's own: the routes, the handlers, and the templates they
+ render.
- `frontend/` + `scss/` — build-time TypeScript diff bundle and the SCSS entry.
- `cmd/comparesrht/` — the daemon entry point and startup validation.
- `contrib/` — nginx server block, systemd unit, and a dev GraphQL stub.
M cmd/comparesrht/main.go => cmd/comparesrht/main.go +20 -7
@@ 32,11 32,12 @@ import (
"time"
"github.com/go-chi/chi/v5"
- "github.com/go-chi/chi/v5/middleware"
+ chimiddleware "github.com/go-chi/chi/v5/middleware"
"github.com/vaughan0/go-ini"
"go.bigb.es/auxilia/scribe"
"sourcecraft.dev/bigbes/sr-ht-core/config"
coreserver "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-ecore/logging"
"sourcecraft.dev/bigbes/sr-ht-ecore/login"
@@ 135,13 136,25 @@ func main() {
// Register installs three more of its own inside a nested group: the
// private cache policy, panic recovery through the service's error page,
// and the same-origin guard. chi's Recoverer stays here as the outer net
- // for a panic in the two middlewares above, which are outside that group —
- // it re-panics http.ErrAbortHandler, which is what the inner one raises for
- // a panic arriving after the response has already started.
+ // for a panic in the middlewares above, which are outside that group — it
+ // re-panics http.ErrAbortHandler, which is what the inner one raises for a
+ // panic arriving after the response has already started.
+ //
+ // The request line is chimw's and no longer chi's. chi's Logger writes an
+ // unstructured, colourised line to *stdout*, which on this daemon is the
+ // highest-volume record it emits and the only one not beside the rest in
+ // the journal: an operator grepping stderr for a path finds every panic and
+ // none of the requests that caused them. RequestID goes above it so the
+ // request line and the panic report carry the same id, and Recoverer below
+ // it so its own report goes through the log entry rather than to stdout.
+ // /healthz is skipped: it is a probe every second and says nothing.
srv.AnonRouter().Group(func(r chi.Router) {
- r.Use(middleware.RealIP)
- r.Use(middleware.Recoverer)
- r.Use(middleware.Logger)
+ r.Use(chimiddleware.RequestID)
+ r.Use(chimiddleware.RealIP)
+ r.Use(chimw.RequestLogger(chimw.SlogFormatter{
+ Skip: chimw.SkipPaths("/healthz"),
+ }))
+ r.Use(chimiddleware.Recoverer)
r.Use(config.Middleware(conf, service))
// The instance's one cookie decode, with the default validator: a name
// this service narrowed further would be an account logged out of
M web/router.go => web/router.go +17 -9
@@ 6,6 6,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/csrf"
"sourcecraft.dev/bigbes/sr-ht-ecore/middleware"
)
@@ 30,8 31,15 @@ import (
// back on — the session cookie is meta.sr.ht's, set on the parent domain,
// with a SameSite no individual service can choose.
//
-// The router's own 404 is set here too, so a mistyped URL lands on a page with a
-// nav rather than on chi's plain-text dead end.
+// Both of chi's routing failures are pointed at this service's error page here,
+// so a mistyped URL and a method this router does not serve land on a page with
+// a nav rather than on chi's plain-text dead end. The 405 is new: until ecore
+// grew chimw.RenderRefusals this service installed the 404 alone and left the
+// other refusal to net/http.
+//
+// Every read route is registered under GET and HEAD both. chi's Get registers
+// GET alone, so `curl -I` and every uptime probe used to be answered 405 by
+// pages whose whole job is to be cheap to ask about.
func (s *Server) Register(r chi.Router) {
r.Group(func(r chi.Router) {
r.Use(middleware.PrivateCache)
@@ 40,18 48,18 @@ func (s *Server) Register(r chi.Router) {
}))
r.Use(csrf.Require(s.chromeSvc.SelfOrigin(), nil))
- r.NotFound(s.handleNotFound)
+ chimw.RenderRefusals(r, s.renderError)
- r.Get("/", s.handleIndex)
- r.Get("/jump", s.handleJump)
- r.Get("/healthz", s.handleHealthz)
+ chimw.GetHead(r, "/", s.handleIndex)
+ chimw.GetHead(r, "/jump", s.handleJump)
+ chimw.GetHead(r, "/healthz", s.handleHealthz)
r.Handle(assets.DefaultPrefix+"*", s.static)
- r.Get("/~{owner}/{repo}", s.handleRepo)
+ chimw.GetHead(r, "/~{owner}/{repo}", s.handleRepo)
// A single wildcard route serves both the form target (empty wildcard ⇒
// redirect to the canonical URL) and the compare view itself.
- r.Get("/~{owner}/{repo}/compare/*", s.handleCompare)
- r.Get("/~{owner}/{repo}/commit/{rev}", s.handleCommit)
+ chimw.GetHead(r, "/~{owner}/{repo}/compare/*", s.handleCompare)
+ chimw.GetHead(r, "/~{owner}/{repo}/commit/{rev}", s.handleCommit)
})
}
M web/server.go => web/server.go +2 -1
@@ 34,9 34,10 @@
// guard — and assumes the following is already applied to the router it is
// handed, in this order (outermost first):
//
+// chi middleware.RequestID
// chi middleware.RealIP
+// chimw.RequestLogger(...) // the request line, as a slog record
// chi middleware.Recoverer
-// chi middleware.Logger (optional, but recommended)
// config.Middleware(conf, "compare.sr.ht") // required: authz + gitx read it
// login.Optional() // required: never 401s; sets the viewer
//
M web/web_test.go => web/web_test.go +60 -6
@@ 379,6 379,60 @@ func TestHealthz(t *testing.T) {
assert.Contains(t, rec.Body.String(), "ok")
}
+// TestHeadIsServedWhereGetIs pins what chimw.GetHead registers. chi's Get alone
+// left every read route answering `curl -I` — and every uptime probe, and every
+// cache revalidating what it holds — with a 405 plus a rendered error page from
+// a path whose whole job is to be cheap to ask about.
+//
+// The status has to be the GET's, on every route: a HEAD that answered 200 where
+// the GET answers 404 would be the private-repo leak the 404 exists to prevent.
+func TestHeadIsServedWhereGetIs(t *testing.T) {
+ root, mainSHA := gitFixture(t)
+ h := testServer(t, root, demoAuthorizer())
+
+ for _, target := range []string{
+ "/",
+ "/healthz",
+ "/~alice/demo",
+ "/~alice/demo/compare/main...feature",
+ "/~alice/demo/commit/" + mainSHA,
+ } {
+ req := httptest.NewRequest(http.MethodHead, target, nil)
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ assert.Equalf(t, http.StatusOK, rec.Code, "HEAD %s", target)
+ }
+
+ // And a hidden repository is a 404 to HEAD exactly as it is to GET.
+ req := httptest.NewRequest(http.MethodHead, "/~alice/nope", nil)
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ assert.Equal(t, http.StatusNotFound, rec.Code, "HEAD on a hidden repo")
+}
+
+// TestMethodNotAllowedIsAPage covers the refusal this service did not render
+// before chimw.RenderRefusals: it installed chi's 404 alone, so a method the
+// router does not serve produced net/http's plain-text 405 — the one answer on
+// the instance that did not look like the service it came from.
+//
+// The probe is OPTIONS rather than a write method, and the reason is worth
+// keeping: the group's middleware does run before chi answers a 405, so an
+// unsafe method that carries no Origin is refused 403 by the same-origin guard
+// and never reaches this handler at all. OPTIONS is safe, passes the guard, and
+// is registered nowhere.
+func TestMethodNotAllowedIsAPage(t *testing.T) {
+ root, _ := gitFixture(t)
+ h := testServer(t, root, demoAuthorizer())
+
+ req := httptest.NewRequest(http.MethodOptions, "/healthz", nil)
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+
+ require.Equal(t, http.StatusMethodNotAllowed, rec.Code)
+ assert.Contains(t, rec.Body.String(), pages.MethodMessage)
+ assert.Contains(t, rec.Body.String(), "navbar-brand", "the 405 has no chrome")
+}
+
func TestStaticBundleAndCSS(t *testing.T) {
root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())
@@ 509,12 563,12 @@ func TestBadSpecKeepsItsOwnMessage(t *testing.T) {
// today, so what this asserts is that the guard is installed at all — the day a
// form arrives it is already covered.
//
-// The probe is a path with no route, because that is the one unsafe request
-// this router hands to the guard: chi resolves a method before a group's
-// middleware runs, so a POST to a GET-only path is its own 405, while a POST to
-// nothing at all reaches the not-found handler through the whole chain. The
-// consequence to know about is in the first arm — such a request is now refused
-// 403 rather than answered 404.
+// The probe is a path with no route, but any path would do: the group's
+// middleware runs before chi answers either of its routing failures, so a POST
+// to a GET-only path is refused 403 by the guard exactly as a POST to nothing at
+// all is, and neither reaches the 405 or the 404 behind it. The consequence to
+// know about is in the first arm — such a request is refused 403 rather than
+// answered 404.
func TestUnsafeMethodRefused(t *testing.T) {
root, _ := gitFixture(t)
h := testServer(t, root, demoAuthorizer())