From 205539cde737b75f326345abfa55b508317f2f33 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 07:26:50 +0000 Subject: [PATCH 1/2] Restrict /s/* to GET and HEAD (closes #169) The static file server was attached with Mount, which registers every method, so POST, PUT and DELETE on an asset were answered 200 with the file. It is now registered for GET and HEAD only, inside a /s group whose method-not-allowed handler answers 405 with Allow: GET, HEAD (chi's default 405 sends no Allow header). TestStaticServesEveryMethod is inverted and renamed TestStaticServesOnlyGetAndHead, and the README route table row for /s/* now says the same. Model: opus-5-5 --- README.md | 2 +- internal/server/routes.go | 20 ++++++++++++++--- internal/server/routes_test.go | 40 ++++++++++++++++++++-------------- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 5729606..2c657cc 100644 --- a/README.md +++ b/README.md @@ -2721,7 +2721,7 @@ abuse limit later; they are tracked as future work. | ------ | --------------------------- | ----------- | | `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) | | `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) | -| any | `/s/*` | Static file serving (embedded CSS, JS). Mounted for every method, not just `GET`/`HEAD`: chi's `Mount` registers all methods and `http.FileServer` special-cases only `HEAD` (by omitting the body), so a `POST` or `DELETE` to an asset is answered `200` with the file. Pinned by `TestStaticServesEveryMethod` | +| `GET`, `HEAD` | `/s/*` | Static file serving (embedded CSS, JS). `GET` and `HEAD` only — every other method is answered `405 Method Not Allowed` with `Allow: GET, HEAD`. Pinned by `TestStaticServesOnlyGetAndHead` | | `POST` | `/webhook/{uuid}` | Webhook receiver endpoint. `POST` only — every other method is answered `405 Method Not Allowed` with `Allow: POST`. Rate limited (see [Rate Limiting](#rate-limiting)) | #### Authentication Endpoints diff --git a/internal/server/routes.go b/internal/server/routes.go index 05872f8..7f3699a 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -92,11 +92,25 @@ func (s *Server) setupGlobalMiddleware() { func (s *Server) setupRoutes() { s.router.Get("/", s.h.HandleIndex()) - s.router.Mount( - "/s", - http.StripPrefix("/s", http.FileServer(http.FS(static.Static))), + // Static assets answer GET and HEAD only. chi's default 405 + // carries no Allow header, so this group supplies its own. + staticFiles := http.StripPrefix( + "/s", http.FileServer(http.FS(static.Static)), ) + s.router.Route("/s", func(r chi.Router) { + r.MethodNotAllowed(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Allow", "GET, HEAD") + http.Error( + w, + "Method Not Allowed", + http.StatusMethodNotAllowed, + ) + }) + r.Method(http.MethodGet, "/*", staticFiles) + r.Method(http.MethodHead, "/*", staticFiles) + }) + s.router.Route("/api/v1", func(_ chi.Router) { // API routes will be added here. }) diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index 97937ee..081fdef 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -396,13 +396,12 @@ func (e *testEnv) storedHash(t *testing.T, username string) string { // --- /s static group --- -// TestStaticServesEveryMethod pins what the static mount actually -// answers. chi's Mount registers the handler for all methods and -// http.FileServer only special-cases HEAD (by suppressing the body), -// so a POST or a DELETE to an asset is served the file rather than -// refused. The README documents this; the test is what keeps the two -// from drifting. -func TestStaticServesEveryMethod(t *testing.T) { +// TestStaticServesOnlyGetAndHead pins the methods the static group +// answers: GET and HEAD are served the asset, and any other method +// is refused with 405 and an Allow header naming those two. The +// README documents this; the test is what keeps the two from +// drifting. +func TestStaticServesOnlyGetAndHead(t *testing.T) { t.Parallel() env := newTestEnv(t) @@ -428,18 +427,27 @@ func TestStaticServesEveryMethod(t *testing.T) { w := httptest.NewRecorder() env.router.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code, - "static mount answers every method") - - if method == http.MethodHead { + switch method { + case http.MethodGet: + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, body, w.Body.Bytes(), + "the asset itself is returned") + case http.MethodHead: + assert.Equal(t, http.StatusOK, w.Code) assert.Empty(t, w.Body.Bytes(), "HEAD must not carry a body") - - return + default: + assert.Equal( + t, http.StatusMethodNotAllowed, w.Code, + ) + assert.Equal( + t, "GET, HEAD", w.Header().Get("Allow"), + ) + assert.NotContains( + t, w.Body.String(), string(body), + "a refused method must not get the asset", + ) } - - assert.Equal(t, body, w.Body.Bytes(), - "the asset itself is returned") }) } } -- 2.54.0 From 3a4f3625e8eda9a554fad026a1576f7fd52676de Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 08:23:34 +0000 Subject: [PATCH 2/2] Document and test that unrouted methods get 405 without Allow on /s/* A method chi does not route, such as PROPFIND, is refused by the top-level router before it reaches the /s group, so it gets 405 without an Allow header. The README row and the test's doc comment now say so, and TestStaticServesOnlyGetAndHead checks PROPFIND. Model: opus-5-5 --- README.md | 2 +- internal/server/routes_test.go | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 2c657cc..8a3ad14 100644 --- a/README.md +++ b/README.md @@ -2721,7 +2721,7 @@ abuse limit later; they are tracked as future work. | ------ | --------------------------- | ----------- | | `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) | | `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) | -| `GET`, `HEAD` | `/s/*` | Static file serving (embedded CSS, JS). `GET` and `HEAD` only — every other method is answered `405 Method Not Allowed` with `Allow: GET, HEAD`. Pinned by `TestStaticServesOnlyGetAndHead` | +| `GET`, `HEAD` | `/s/*` | Static file serving (embedded CSS, JS). `GET` and `HEAD` only — `POST`, `PUT`, `PATCH`, `DELETE`, `OPTIONS`, `TRACE` and `CONNECT` are answered `405 Method Not Allowed` with `Allow: GET, HEAD`. Any other method (such as `PROPFIND`) is refused by chi before it reaches this route, and gets `405` without an `Allow` header. Pinned by `TestStaticServesOnlyGetAndHead` | | `POST` | `/webhook/{uuid}` | Webhook receiver endpoint. `POST` only — every other method is answered `405 Method Not Allowed` with `Allow: POST`. Rate limited (see [Rate Limiting](#rate-limiting)) | #### Authentication Endpoints diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index 081fdef..429a7f7 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -397,9 +397,12 @@ func (e *testEnv) storedHash(t *testing.T, username string) string { // --- /s static group --- // TestStaticServesOnlyGetAndHead pins the methods the static group -// answers: GET and HEAD are served the asset, and any other method -// is refused with 405 and an Allow header naming those two. The -// README documents this; the test is what keeps the two from +// answers: GET and HEAD are served the asset, and the other methods +// chi routes (POST, PUT, DELETE and the rest) are refused with 405 +// and an Allow header naming those two. A method chi does not route, +// such as PROPFIND, is refused with 405 by the top-level router +// before it reaches the static group, so it gets no Allow header. +// The README documents this; the test is what keeps the two from // drifting. func TestStaticServesOnlyGetAndHead(t *testing.T) { t.Parallel() @@ -416,6 +419,7 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) { http.MethodPost, http.MethodPut, http.MethodDelete, + "PROPFIND", } { t.Run(method, func(t *testing.T) { t.Parallel() @@ -436,6 +440,17 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) { assert.Equal(t, http.StatusOK, w.Code) assert.Empty(t, w.Body.Bytes(), "HEAD must not carry a body") + case "PROPFIND": + assert.Equal( + t, http.StatusMethodNotAllowed, w.Code, + ) + assert.Empty(t, w.Header().Get("Allow"), + "chi refuses a method it does not route "+ + "before the static group runs") + assert.NotContains( + t, w.Body.String(), string(body), + "a refused method must not get the asset", + ) default: assert.Equal( t, http.StatusMethodNotAllowed, w.Code, -- 2.54.0