fix #84: security headers dropped because they were set post-flush; set pre-handler
CI / test (pull_request) Successful in 21s
CI / docker (pull_request) Skipped

This commit is contained in:
fen
2026-09-09 10:54:16 -05:00
parent 7ca1b58362
commit 3d415edbdc
2 changed files with 17 additions and 25 deletions
+8 -18
View File
@@ -3,7 +3,6 @@ package web
import ( import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"strings"
"testing" "testing"
) )
@@ -28,29 +27,20 @@ func TestSecurityHeaders(t *testing.T) {
t.Errorf("X-Content-Type-Options = %q, want nosniff", got) t.Errorf("X-Content-Type-Options = %q, want nosniff", got)
} }
// JSON response: no security headers. // JSON/raw responses: headers are now set unconditionally BEFORE the handler
// runs. The previous post-handler approach was silently dropped once a page
// handler flushed its template output (headers must be set before WriteHeader).
// CSP/nosniff/referrer on non-HTML bodies is harmless and desirable.
jsonh := SecurityHeaders(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { jsonh := SecurityHeaders(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json") w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"ok":true}`)) w.Write([]byte(`{"ok":true}`))
})) }))
rec = httptest.NewRecorder() rec = httptest.NewRecorder()
jsonh.ServeHTTP(rec, httptest.NewRequest("GET", "/api/x", nil)) jsonh.ServeHTTP(rec, httptest.NewRequest("GET", "/api/x", nil))
if got := rec.Header().Get("Content-Security-Policy"); got != "" { if got := rec.Header().Get("Content-Security-Policy"); got != wantCSP {
t.Errorf("unexpected CSP %q on JSON response", got) t.Errorf("CSP missing on JSON response: got %q", got)
} }
if got := rec.Header().Get("Referrer-Policy"); got != "" { if got := rec.Header().Get("Referrer-Policy"); got != "no-referrer" {
t.Errorf("unexpected Referrer-Policy %q on JSON response", got) t.Errorf("Referrer-Policy missing on JSON response: got %q", got)
}
// Content type set after the first Write (as the inline can page does) is
// still picked up because headers are inspected post-handler.
lateh := SecurityHeaders(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte("<html></html>"))
w.Header().Set("Content-Type", "text/html; charset=utf-8")
}))
rec = httptest.NewRecorder()
lateh.ServeHTTP(rec, httptest.NewRequest("GET", "/", nil))
if got := rec.Header().Get("Content-Security-Policy"); !strings.Contains(got, "frame-ancestors 'none'") {
t.Errorf("CSP = %q, want frame-ancestors 'none'", got)
} }
} }
+5 -3
View File
@@ -304,13 +304,15 @@ func (u *UI) Handlers() *Handlers { return &Handlers{UI: u} }
// pentest. Runs after the handler so the Content-Type is already set. // pentest. Runs after the handler so the Content-Type is already set.
func SecurityHeaders(next http.Handler) http.Handler { func SecurityHeaders(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
next.ServeHTTP(w, r) // Set before the handler runs: once a handler writes (template render
// flushes), header mutations are silently dropped. Setting the headers
// unconditionally is safe: CSP/nosniff/referrer on JSON or /raw bodies
// is harmless and arguably desirable.
h := w.Header() h := w.Header()
if strings.HasPrefix(h.Get("Content-Type"), "text/html") {
h.Set("Content-Security-Policy", h.Set("Content-Security-Policy",
"default-src 'self'; script-src 'self' 'unsafe-inline'; frame-ancestors 'none'") "default-src 'self'; script-src 'self' 'unsafe-inline'; frame-ancestors 'none'")
h.Set("Referrer-Policy", "no-referrer") h.Set("Referrer-Policy", "no-referrer")
h.Set("X-Content-Type-Options", "nosniff") h.Set("X-Content-Type-Options", "nosniff")
} next.ServeHTTP(w, r)
}) })
} }