Close the review gaps: CI lint, server timeouts, security headers
The initial scaffold left three holes the review caught. CI only checked gofmt and go vet, so golangci-lint and the pre-commit hooks were advisory rather than enforced. The HTTP server bounded only the header read, so a slow or stalled peer could hold a connection indefinitely. And the browser got no content-security policy at all, leaving the SPA's same-origin assumption unenforced. Add golangci-lint and pre-commit hook steps to the existing pre-commit workflow, mirroring the estate's images so the required context name stays ci/woodpecker/pr/pre-commit. Bound the server with ReadTimeout, WriteTimeout, and IdleTimeout, keeping the write budget generous enough for the poster proxy's streamed responses. Stamp Content-Security-Policy, X-Content-Type-Options, and Referrer-Policy onto every response from a single middleware wrapping the root handler. Assert the headers across the API, UI, assets, probes, and rejections. Guard the CSP's no-unsafe-inline assumption with a ui test that fails if a shipped asset grows an inline script, style block, or event handler. Extend the make pre-commit target to match the widened CI checks.
This commit is contained in:
@@ -62,7 +62,24 @@ func (s *Server) Handler() http.Handler {
|
||||
gated.HandleFunc("/", s.handleUI)
|
||||
|
||||
mux.Handle("/", s.gate.Wrap(gated))
|
||||
return mux
|
||||
return secureHeaders(mux)
|
||||
}
|
||||
|
||||
// cspPolicy locks the page to same-origin code. The SPA carries no inline
|
||||
// script or style, so no unsafe-inline escape hatch is needed; data: is in
|
||||
// img-src solely for the inline SVG favicon.
|
||||
const cspPolicy = "default-src 'self'; img-src 'self' data:; style-src 'self'; script-src 'self'"
|
||||
|
||||
// secureHeaders stamps the browser-facing hardening headers onto every
|
||||
// response — API, UI and probes alike — before the handler writes.
|
||||
func secureHeaders(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
h := w.Header()
|
||||
h.Set("Content-Security-Policy", cspPolicy)
|
||||
h.Set("X-Content-Type-Options", "nosniff")
|
||||
h.Set("Referrer-Policy", "no-referrer")
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
func (s *Server) readyz(w http.ResponseWriter, _ *http.Request) {
|
||||
|
||||
@@ -375,3 +375,33 @@ func TestSPAFallbackAndAssets(t *testing.T) {
|
||||
t.Errorf("unknown api route = %d %q", rec.Code, rec.Body)
|
||||
}
|
||||
}
|
||||
|
||||
// The hardening headers must ride on every response: the page shell, static
|
||||
// assets, the JSON API, the ungated probes, and rejections alike.
|
||||
func TestSecurityHeadersOnEveryResponse(t *testing.T) {
|
||||
e := newEnv(t, nil)
|
||||
want := map[string]string{
|
||||
"Content-Security-Policy": "default-src 'self'; img-src 'self' data:; style-src 'self'; script-src 'self'",
|
||||
"X-Content-Type-Options": "nosniff",
|
||||
"Referrer-Policy": "no-referrer",
|
||||
}
|
||||
check := func(what string, rec *httptest.ResponseRecorder) {
|
||||
t.Helper()
|
||||
for k, v := range want {
|
||||
if got := rec.Header().Get(k); got != v {
|
||||
t.Errorf("%s: %s = %q, want %q", what, k, got, v)
|
||||
}
|
||||
}
|
||||
}
|
||||
for _, p := range []string{"/", "/movies", "/app.js", "/api/library/movies", "/livez", "/readyz"} {
|
||||
check(p, e.do(t, http.MethodGet, p))
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
e.h.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("unauthorized = %d, want 403", rec.Code)
|
||||
}
|
||||
check("403", rec)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user