Restrict /s/* to GET and HEAD (closes #169)
check / check (push) Successful in 3m37s

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.
A method chi does not route at all, such as PROPFIND, still gets 405
from the top-level router, without Allow. The inverted test and the
README route table say the same.

Model: opus-5-5
This commit was merged in pull request #335.
This commit is contained in:
2026-09-29 11:10:27 +02:00
parent 9cf9cdd8eb
commit ab63b5f777
3 changed files with 57 additions and 20 deletions
+1 -1
View File
@@ -2721,7 +2721,7 @@ abuse limit later; they are tracked as future work.
| ------ | --------------------------- | ----------- | | ------ | --------------------------- | ----------- |
| `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) | | `GET` | `/` | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) |
| `GET` | `/.well-known/healthcheck` | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) | | `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 — `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)) | | `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 #### Authentication Endpoints
+17 -3
View File
@@ -92,11 +92,25 @@ func (s *Server) setupGlobalMiddleware() {
func (s *Server) setupRoutes() { func (s *Server) setupRoutes() {
s.router.Get("/", s.h.HandleIndex()) s.router.Get("/", s.h.HandleIndex())
s.router.Mount( // Static assets answer GET and HEAD only. chi's default 405
"/s", // carries no Allow header, so this group supplies its own.
http.StripPrefix("/s", http.FileServer(http.FS(static.Static))), 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) { s.router.Route("/api/v1", func(_ chi.Router) {
// API routes will be added here. // API routes will be added here.
}) })
+39 -16
View File
@@ -396,13 +396,15 @@ func (e *testEnv) storedHash(t *testing.T, username string) string {
// --- /s static group --- // --- /s static group ---
// TestStaticServesEveryMethod pins what the static mount actually // TestStaticServesOnlyGetAndHead pins the methods the static group
// answers. chi's Mount registers the handler for all methods and // answers: GET and HEAD are served the asset, and the other methods
// http.FileServer only special-cases HEAD (by suppressing the body), // chi routes (POST, PUT, DELETE and the rest) are refused with 405
// so a POST or a DELETE to an asset is served the file rather than // and an Allow header naming those two. A method chi does not route,
// refused. The README documents this; the test is what keeps the two // such as PROPFIND, is refused with 405 by the top-level router
// from drifting. // before it reaches the static group, so it gets no Allow header.
func TestStaticServesEveryMethod(t *testing.T) { // The README documents this; the test is what keeps the two from
// drifting.
func TestStaticServesOnlyGetAndHead(t *testing.T) {
t.Parallel() t.Parallel()
env := newTestEnv(t) env := newTestEnv(t)
@@ -417,6 +419,7 @@ func TestStaticServesEveryMethod(t *testing.T) {
http.MethodPost, http.MethodPost,
http.MethodPut, http.MethodPut,
http.MethodDelete, http.MethodDelete,
"PROPFIND",
} { } {
t.Run(method, func(t *testing.T) { t.Run(method, func(t *testing.T) {
t.Parallel() t.Parallel()
@@ -428,18 +431,38 @@ func TestStaticServesEveryMethod(t *testing.T) {
w := httptest.NewRecorder() w := httptest.NewRecorder()
env.router.ServeHTTP(w, req) env.router.ServeHTTP(w, req)
assert.Equal(t, http.StatusOK, w.Code, switch method {
"static mount answers every method") case http.MethodGet:
assert.Equal(t, http.StatusOK, w.Code)
if method == http.MethodHead { 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(), assert.Empty(t, w.Body.Bytes(),
"HEAD must not carry a body") "HEAD must not carry a body")
case "PROPFIND":
return 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,
)
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")
}) })
} }
} }