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.

.woodpecker/pre-commit.yaml

@@ -17,3 +17,33 @@ steps:
           limits:
             memory: 2Gi
             cpu: 2
+
+  - name: lint
+    image: golangci/golangci-lint:latest
+    commands:
+      - golangci-lint run ./...
+    backend_options:
+      kubernetes:
+        serviceAccountName: mediamark-ci
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2
+
+  - name: hooks
+    image: git.unkin.net/unkin/almalinux9-gobuilder:20260606
+    commands:
+      - uvx pre-commit run --all-files
+    backend_options:
+      kubernetes:
+        serviceAccountName: mediamark-ci
+        resources:
+          requests:
+            memory: 512Mi
+            cpu: 1
+          limits:
+            memory: 2Gi
+            cpu: 2

Makefile

@@ -17,6 +17,8 @@ all: build
 pre-commit:
 	test -z "$$(gofmt -l .)"
 	go vet ./...
+	golangci-lint run ./...
+	uvx pre-commit run --all-files
 
 build:
 	@for b in $(BINARIES); do \

cmd/mediamark/main.go

@@ -42,6 +42,11 @@ func main() {
 		Addr:              cfg.Listen,
 		Handler:           server.New(cfg, lib, arrs, ui.Assets(), log).Handler(),
 		ReadHeaderTimeout: 10 * time.Second,
+		ReadTimeout:       30 * time.Second,
+		// The art proxy streams posters upstream-to-browser, so the write budget
+		// is the generous one.
+		WriteTimeout: 60 * time.Second,
+		IdleTimeout:  120 * time.Second,
 	}
 
 	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)

internal/server/server.go

@@ -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) {

internal/server/server_test.go

@@ -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)
+}

ui/embed_test.go

@@ -0,0 +1,66 @@
+package ui
+
+import (
+	"io/fs"
+	"regexp"
+	"strings"
+	"testing"
+)
+
+// The server ships a CSP of script-src 'self'; style-src 'self' with no
+// unsafe-inline. Assets that grow an inline script, an inline <style>, or a
+// style= attribute would silently stop rendering in the browser, so guard the
+// invariant here rather than discovering it in production.
+func TestShippedAssetsCarryNothingInline(t *testing.T) {
+	banned := []struct {
+		what string
+		re   *regexp.Regexp
+	}{
+		{"inline <style> block", regexp.MustCompile(`(?is)<style[\s>]`)},
+		{"style= attribute", regexp.MustCompile(`(?i)\sstyle\s*=`)},
+		{"inline event handler", regexp.MustCompile(`(?i)\son(?:click|load|error|submit|change|input|keydown)\s*=`)},
+	}
+	scriptBody := regexp.MustCompile(`(?is)<script[^>]*>(.*?)</script>`)
+
+	assets := Assets()
+	err := fs.WalkDir(assets, ".", func(p string, d fs.DirEntry, err error) error {
+		if err != nil || d.IsDir() || !strings.HasSuffix(p, ".html") {
+			return err
+		}
+		b, err := fs.ReadFile(assets, p)
+		if err != nil {
+			return err
+		}
+		for _, ban := range banned {
+			if ban.re.Match(b) {
+				t.Errorf("%s contains an %s, which the CSP forbids", p, ban.what)
+			}
+		}
+		for _, m := range scriptBody.FindAllSubmatch(b, -1) {
+			if len(strings.TrimSpace(string(m[1]))) > 0 {
+				t.Errorf("%s contains an inline <script> body, which the CSP forbids", p)
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+// The favicon is a data: URI, which is why img-src carries data:. If it ever
+// stops being one, img-src should tighten back to 'self'.
+func TestFaviconIsTheOnlyDataURI(t *testing.T) {
+	b, err := fs.ReadFile(Assets(), "index.html")
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, line := range strings.Split(string(b), "\n") {
+		if !strings.Contains(line, "data:") {
+			continue
+		}
+		if !strings.Contains(line, `rel="icon"`) {
+			t.Errorf("unexpected data: URI outside the favicon: %s", strings.TrimSpace(line))
+		}
+	}
+}