diff --git a/docs/superpowers/plans/2026-04-23-block2-security-plan.md b/docs/superpowers/plans/2026-04-23-block2-security-plan.md new file mode 100644 index 0000000..f1032bb --- /dev/null +++ b/docs/superpowers/plans/2026-04-23-block2-security-plan.md @@ -0,0 +1,1110 @@ +# Block 2 — Security & Auth Hardening Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Harden the docsiq HTTP boundary and config loader — reject malformed config early, scrub secrets from debug logs, enforce browser-side CSP + baseline security headers, and thread the existing request ID into slog for operator-friendly log correlation. + +**Architecture:** Five independent server-side changes, four of them behind an existing middleware chain (`loggingMiddleware → recoveryMiddleware → bearerAuthMiddleware → projectMiddleware → mux`). Tasks 1–2 touch `internal/config`; Tasks 3–4 add a single new `securityHeadersMiddleware` placed as the outermost wrapper so every response (including 404s and errors) carries the headers; Task 5 adds a small `ContextLogger(ctx)` helper next to the existing `RequestIDFromContext` and updates a curated set of handler log sites. + +**Tech Stack:** Go 1.22+ standard `net/http`, `log/slog`, `reflect`, Viper + mapstructure. No new third-party dependencies. + +**Scope check:** Five items, one subsystem (HTTP + config). No sub-plan decomposition needed. + +**Self-contained:** Block 1 (PR #60) is a soft prerequisite only because Block 2 builds on the same middleware chain. None of Block 2 touches workq, the session cookie path, or the entity-fetch scoping from Block 1. + +--- + +## File Structure + +### Create +- `internal/api/security_headers.go` — `securityHeadersMiddleware(cfg *config.Config) func(http.Handler) http.Handler`; returns a handler that sets CSP + baseline headers on every response. +- `internal/api/security_headers_test.go` — header-presence assertions under `GET /health`, `OPTIONS /api/ping`, and authenticated `GET /api/stats`. +- `internal/api/log_context.go` — `ContextLogger(ctx context.Context) *slog.Logger` helper that enriches `slog.Default()` with `req_id` when the context carries one. +- `internal/api/log_context_test.go` — captures slog output via a `slog.NewTextHandler(&buf, nil)` default, asserts `req_id=xxxx` is present on emitted records. +- `internal/config/redact.go` — `(c *Config) Redact() *Config`; deep-copies and zeroes every string field tagged `secret:"true"` via `reflect`. +- `internal/config/redact_test.go` — round-trip test: load a config with known secrets, serialize the redacted copy to YAML, assert no secret substring appears. + +### Modify +- `internal/config/config.go:48-128` — add `secret:"true"` struct tags on API-key fields in `ServerConfig`, `OpenAIConfig`, `AzureConfig`, `AzureServiceConfig`. +- `internal/config/config.go:256` — swap `viper.Unmarshal(&cfg)` for `viper.UnmarshalExact(&cfg)`; add call to `validateLLM(&cfg)` before returning. +- `internal/config/config.go` (new function at end) — `validateLLM(cfg *Config) error` with provider switch. +- `internal/config/config.go:145-152` — add `HSTSEnabled bool `mapstructure:"hsts_enabled"`` to `ServerConfig` (no `secret` tag). +- `internal/config/config.go:238-241` — add `viper.BindEnv("server.hsts_enabled")`. +- `internal/config/config_test.go` (existing file; append) — tests for `validateLLM` + `UnmarshalExact` rejection of unknown keys. +- `internal/api/router.go:167-170` — wrap the existing middleware chain with `securityHeadersMiddleware(cfg)` as the new outermost layer. +- `internal/api/handlers.go` (representative sites) — replace 3–5 `slog.Warn` / `slog.Error` calls in request handlers with `ContextLogger(r.Context()).Warn(...)` equivalents. + +--- + +## Task 1: Strict config unmarshal + LLM consistency (2.4) + +**Files:** +- Modify: `internal/config/config.go:256` (Unmarshal → UnmarshalExact) +- Modify: `internal/config/config.go` — append `validateLLM(cfg *Config) error` +- Modify: `internal/config/config_test.go` — append new tests + +- [ ] **Step 1: Read current state** + +Confirm the current load pattern: + +```bash +sed -n '250,265p' internal/config/config.go +grep -n 'UnmarshalExact\|Unmarshal' internal/config/config.go +``` + +Expected: one existing `viper.Unmarshal(&cfg)` call at line 256 (give or take 3 lines if the file has drifted), no existing `UnmarshalExact`. + +- [ ] **Step 2: Write the failing tests (TDD red)** + +Append to `internal/config/config_test.go`: + +```go +func TestLoad_RejectsUnknownKey(t *testing.T) { + t.Parallel() + dir := t.TempDir() + f := filepath.Join(dir, "config.yaml") + must := func(err error) { t.Helper(); if err != nil { t.Fatal(err) } } + must(os.WriteFile(f, []byte("server:\n api_key: s3cret\n unknown_key: oops\n"), 0o600)) + + _, err := Load(f) + if err == nil { + t.Fatal("Load should reject unknown_key") + } + if !strings.Contains(err.Error(), "unknown_key") { + t.Fatalf("error should name the offending key; got %q", err) + } +} + +func TestLoad_ValidatesLLMProvider(t *testing.T) { + t.Parallel() + cases := []struct { + name string + yaml string + wantErr string + }{ + { + name: "unknown_provider", + yaml: "llm:\n provider: not_a_real_one\n", + wantErr: "provider", + }, + { + name: "azure_missing_endpoint", + yaml: "llm:\n provider: azure\n azure:\n api_key: k\n", + wantErr: "azure", + }, + { + name: "openai_missing_api_key", + yaml: "llm:\n provider: openai\n openai:\n base_url: https://api.openai.com/v1\n", + wantErr: "openai", + }, + { + name: "ollama_missing_base_url", + yaml: "llm:\n provider: ollama\n", + wantErr: "ollama", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + f := filepath.Join(dir, "config.yaml") + must := func(err error) { t.Helper(); if err != nil { t.Fatal(err) } } + must(os.WriteFile(f, []byte("server:\n api_key: s3cret\n"+tc.yaml), 0o600)) + + _, err := Load(f) + if err == nil { + t.Fatalf("Load should have rejected %s", tc.name) + } + if !strings.Contains(strings.ToLower(err.Error()), tc.wantErr) { + t.Fatalf("error should mention %q; got %q", tc.wantErr, err) + } + }) + } +} + +func TestLoad_AcceptsValidProviders(t *testing.T) { + t.Parallel() + cases := []struct { + name string + yaml string + }{ + {"azure", "llm:\n provider: azure\n azure:\n endpoint: https://x.openai.azure.com\n api_key: k\n api_version: 2024-02-15-preview\n chat:\n model: gpt-4o\n embed:\n model: text-embedding-3-small\n"}, + {"openai", "llm:\n provider: openai\n openai:\n api_key: k\n chat_model: gpt-4o\n embed_model: text-embedding-3-small\n"}, + {"ollama", "llm:\n provider: ollama\n ollama:\n base_url: http://127.0.0.1:11434\n chat_model: llama3\n embed_model: nomic-embed-text\n"}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + f := filepath.Join(dir, "config.yaml") + must := func(err error) { t.Helper(); if err != nil { t.Fatal(err) } } + must(os.WriteFile(f, []byte("server:\n api_key: s3cret\n"+tc.yaml), 0o600)) + + cfg, err := Load(f) + if err != nil { + t.Fatalf("valid %s config should load: %v", tc.name, err) + } + if cfg.LLM.Provider != tc.name { + t.Fatalf("provider not round-tripped: got %q", cfg.LLM.Provider) + } + }) + } +} +``` + +Ensure the test file's import block includes `"os"`, `"path/filepath"`, `"strings"`, `"testing"`. Add any missing. + +- [ ] **Step 3: Run the tests (red)** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 ./internal/config/ -run 'TestLoad_RejectsUnknownKey|TestLoad_ValidatesLLMProvider|TestLoad_AcceptsValidProviders' -v +``` + +Expected: `TestLoad_RejectsUnknownKey` FAILs (viper.Unmarshal accepts unknown keys silently). `TestLoad_ValidatesLLMProvider` subtests FAIL (no validator exists yet). Valid-provider tests should PASS already. + +- [ ] **Step 4: Swap to UnmarshalExact** + +Edit `internal/config/config.go` around line 256: + +```go +// OLD +if err := v.Unmarshal(&cfg); err != nil { + return nil, fmt.Errorf("unmarshal config: %w", err) +} + +// NEW +if err := v.UnmarshalExact(&cfg); err != nil { + return nil, fmt.Errorf("unmarshal config: %w", err) +} +``` + +- [ ] **Step 5: Add `validateLLM`** + +Append to the end of `internal/config/config.go`: + +```go +// validateLLM enforces that the selected LLM provider has the minimum +// fields needed to make any request. Called from Load after +// UnmarshalExact so the "unknown key" and "missing required field" +// errors land in a consistent spot. Error messages name the offending +// provider so an operator can grep logs → yaml key immediately. +func validateLLM(cfg *Config) error { + switch cfg.LLM.Provider { + case "": + // Provider unset is allowed — search paths that don't need an LLM + // (e.g. pure-FTS search) still work. Fail only if someone later + // tries to construct an LLM client with an empty provider string. + return nil + case "azure": + a := cfg.LLM.Azure + chatOK := a.Chat.Endpoint != "" || a.Endpoint != "" + chatOK = chatOK && (a.Chat.APIKey != "" || a.APIKey != "") + embedOK := a.Embed.Endpoint != "" || a.Endpoint != "" + embedOK = embedOK && (a.Embed.APIKey != "" || a.APIKey != "") + if !chatOK && !embedOK { + return fmt.Errorf("llm.azure: neither chat nor embed has a resolvable endpoint+api_key (set shared llm.azure.{endpoint,api_key} or per-service overrides)") + } + if a.APIVersion == "" && a.Chat.APIVersion == "" && a.Embed.APIVersion == "" { + return fmt.Errorf("llm.azure.api_version: required (shared or per-service)") + } + case "openai": + if cfg.LLM.OpenAI.APIKey == "" { + return fmt.Errorf("llm.openai.api_key: required when llm.provider=openai") + } + case "ollama": + if cfg.LLM.Ollama.BaseURL == "" { + return fmt.Errorf("llm.ollama.base_url: required when llm.provider=ollama") + } + default: + return fmt.Errorf("llm.provider: unknown value %q (valid: azure, openai, ollama)", cfg.LLM.Provider) + } + return nil +} +``` + +Now wire it into `Load`. Inside `Load`, after the `UnmarshalExact` call and before the final `return &cfg, nil`, add: + +```go +if err := validateLLM(&cfg); err != nil { + return nil, fmt.Errorf("config validation: %w", err) +} +``` + +- [ ] **Step 6: Run tests (green)** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 ./internal/config/ -v +``` + +Expected: all new tests PASS, existing config tests still PASS. + +If `TestLoad_RejectsUnknownKey` still fails: viper's `UnmarshalExact` may not catch nested unknown keys in all versions. Verify the error message content. If it rejects but doesn't name the offending key, the test's `strings.Contains(err.Error(), "unknown_key")` assertion needs to match what viper actually reports — inspect the error and adjust the substring. + +- [ ] **Step 7: Run full Go suite** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 -timeout 300s ./... +``` + +Expected: all pass. + +- [ ] **Step 8: Commit** + +```bash +git add internal/config/config.go internal/config/config_test.go +git commit -m "$(cat <<'EOF' +feat(config): reject unknown keys and invalid LLM provider configs + +- Load now uses viper.UnmarshalExact, surfacing typos and stale keys + at startup instead of at first-use. +- validateLLM enforces per-provider minimums (azure endpoint+api_key + and api_version; openai api_key; ollama base_url). Error messages + name the offending yaml key so an operator can grep → fix in one + hop. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 2: `Config.Redact()` + audit logged config (2.3) + +**Files:** +- Create: `internal/config/redact.go` +- Create: `internal/config/redact_test.go` +- Modify: `internal/config/config.go` — add `secret:"true"` tags on API-key fields +- Modify: `internal/config/config_test.go` — minor imports if needed (no new tests in this file) +- Modify: zero-to-three call sites in `internal/config` / `internal/llm` / `internal/api` that log config (audit step; see Step 7) + +- [ ] **Step 1: Tag the secret fields** + +Edit `internal/config/config.go`. Add `secret:"true"` to the following fields (keep existing `mapstructure` tags): + +- `ServerConfig.APIKey`: + ```go + APIKey string `mapstructure:"api_key" secret:"true"` + ``` +- `OpenAIConfig.APIKey`: + ```go + APIKey string `mapstructure:"api_key" secret:"true"` + ``` +- `AzureConfig.APIKey`: + ```go + APIKey string `mapstructure:"api_key" secret:"true"` + ``` +- `AzureServiceConfig.APIKey` (used by both `Chat` and `Embed`): + ```go + APIKey string `mapstructure:"api_key" secret:"true"` + ``` + +- [ ] **Step 2: Write the failing test** + +Create `internal/config/redact_test.go`: + +```go +package config + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestConfig_Redact_ZeroesSecrets(t *testing.T) { + t.Parallel() + in := &Config{} + in.Server.APIKey = "server-secret" + in.LLM.Provider = "azure" + in.LLM.Azure.APIKey = "azure-shared-secret" + in.LLM.Azure.Chat.APIKey = "azure-chat-secret" + in.LLM.Azure.Embed.APIKey = "azure-embed-secret" + in.LLM.OpenAI.APIKey = "openai-secret" + + redacted := in.Redact() + + b, err := json.Marshal(redacted) + if err != nil { + t.Fatal(err) + } + for _, s := range []string{ + "server-secret", + "azure-shared-secret", + "azure-chat-secret", + "azure-embed-secret", + "openai-secret", + } { + if strings.Contains(string(b), s) { + t.Fatalf("redacted output still contains %q:\n%s", s, b) + } + } + + // Original must be untouched. + if in.Server.APIKey != "server-secret" { + t.Fatalf("Redact mutated the original Config") + } +} + +func TestConfig_Redact_PreservesNonSecretFields(t *testing.T) { + t.Parallel() + in := &Config{} + in.Server.Host = "127.0.0.1" + in.Server.Port = 8080 + in.Server.APIKey = "s3cret" + in.LLM.Provider = "openai" + in.LLM.OpenAI.BaseURL = "https://api.openai.com/v1" + in.LLM.OpenAI.ChatModel = "gpt-4o" + + r := in.Redact() + if r.Server.Host != "127.0.0.1" { + t.Errorf("Host lost") + } + if r.Server.Port != 8080 { + t.Errorf("Port lost") + } + if r.LLM.Provider != "openai" { + t.Errorf("Provider lost") + } + if r.LLM.OpenAI.BaseURL != "https://api.openai.com/v1" { + t.Errorf("BaseURL lost") + } + if r.LLM.OpenAI.ChatModel != "gpt-4o" { + t.Errorf("ChatModel lost") + } + if r.Server.APIKey != "" { + t.Errorf("Server.APIKey should be zeroed; got %q", r.Server.APIKey) + } +} +``` + +- [ ] **Step 3: Run tests (red)** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 ./internal/config/ -run TestConfig_Redact -v +``` + +Expected: FAIL — `*Config has no method Redact`. + +- [ ] **Step 4: Implement `Redact`** + +Create `internal/config/redact.go`: + +```go +package config + +import "reflect" + +// Redact returns a deep copy of c with every string field tagged +// secret:"true" zeroed. The original c is not mutated. Safe for logging +// and for serializing config for introspection endpoints. +// +// Nested structs are walked recursively. Slices, maps, and pointers to +// structs are supported, though config.Config uses only direct struct +// nesting today — the broader coverage is cheap insurance. +func (c *Config) Redact() *Config { + if c == nil { + return nil + } + dup := *c + zeroSecrets(reflect.ValueOf(&dup).Elem()) + return &dup +} + +func zeroSecrets(v reflect.Value) { + switch v.Kind() { + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + f := v.Field(i) + tag := v.Type().Field(i).Tag.Get("secret") + if tag == "true" && f.Kind() == reflect.String && f.CanSet() { + f.SetString("") + continue + } + zeroSecrets(f) + } + case reflect.Ptr, reflect.Interface: + if !v.IsNil() { + zeroSecrets(v.Elem()) + } + case reflect.Slice, reflect.Array: + for i := 0; i < v.Len(); i++ { + zeroSecrets(v.Index(i)) + } + case reflect.Map: + for _, k := range v.MapKeys() { + // Maps in Go reflect are not addressable; copy out, zero, put back. + elem := reflect.New(v.Type().Elem()).Elem() + elem.Set(v.MapIndex(k)) + zeroSecrets(elem) + v.SetMapIndex(k, elem) + } + } +} +``` + +- [ ] **Step 5: Run tests (green)** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 ./internal/config/ -run TestConfig_Redact -v +``` + +Expected: both tests PASS. + +- [ ] **Step 6: Audit — find existing config log sites** + +``` +grep -rn 'slog\.\(Debug\|Info\|DebugContext\|InfoContext\).*cfg\b' internal/ || true +grep -rn 'slog\.\(Debug\|Info\|DebugContext\|InfoContext\).*config\b' internal/ || true +grep -rn 'fmt\.Printf.*cfg\.' internal/ || true +grep -rn 'fmt\.Println.*cfg\.' internal/ || true +``` + +For each hit that logs a `*Config` value OR the full `cfg` struct: +- Replace `slog.Info("loaded config", "cfg", cfg)` → `slog.Info("loaded config", "cfg", cfg.Redact())` +- Replace `slog.Debug("config", "value", cfg)` → `slog.Debug("config", "value", cfg.Redact())` + +If a site logs only specific non-secret fields (e.g., `slog.Info("host", "host", cfg.Server.Host)`), leave it alone. Only change sites that log the struct as a whole. + +If NO call sites are found, record that finding in the commit message and move on — the helper is still valuable as an available tool for future code. + +- [ ] **Step 7: Re-run full config + full Go suite** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 ./internal/config/... -v +CGO_ENABLED=1 go test -tags sqlite_fts5 -timeout 300s ./... +``` + +Expected: all pass. The `secret:"true"` tag addition is purely additive and should not affect any existing behavior. + +- [ ] **Step 8: Commit** + +```bash +git add internal/config/config.go internal/config/redact.go internal/config/redact_test.go +# Plus any audited log-site edits from Step 6: +# git add internal/config/<...>.go internal/api/<...>.go internal/llm/<...>.go +git commit -m "$(cat <<'EOF' +feat(config): redact helper zeroes fields tagged secret:"true" + +Config.Redact() returns a deep copy with api_key fields cleared. Uses +the new secret:"true" struct tag so future additions opt in simply by +tagging the field. Audited existing log sites that printed the full +Config; piped them through Redact() so debug logs no longer leak +keys. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 3: Content-Security-Policy middleware (2.1) + +**Files:** +- Create: `internal/api/security_headers.go` +- Create: `internal/api/security_headers_test.go` +- Modify: `internal/api/router.go:167-170` — add the wrapper + +- [ ] **Step 1: Write the failing test** + +Create `internal/api/security_headers_test.go`: + +```go +package api + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/RandomCodeSpace/docsiq/internal/config" +) + +func TestSecurityHeaders_CSPOnEveryResponse(t *testing.T) { + t.Parallel() + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + cfg := &config.Config{} + h := securityHeadersMiddleware(cfg)(next) + + req := httptest.NewRequest(http.MethodGet, "/health", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + csp := rr.Header().Get("Content-Security-Policy") + if csp == "" { + t.Fatal("CSP header missing") + } + for _, want := range []string{ + "default-src 'self'", + "script-src 'self' 'wasm-unsafe-eval'", + "style-src 'self' 'unsafe-inline'", + "connect-src 'self'", + "img-src 'self' data:", + "font-src 'self'", + "frame-ancestors 'none'", + "base-uri 'self'", + } { + if !strings.Contains(csp, want) { + t.Errorf("CSP missing directive %q: got %q", want, csp) + } + } +} + +func TestSecurityHeaders_SkipsOPTIONS(t *testing.T) { + t.Parallel() + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + }) + cfg := &config.Config{} + h := securityHeadersMiddleware(cfg)(next) + + req := httptest.NewRequest(http.MethodOptions, "/api/ping", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Header().Get("Content-Security-Policy") != "" { + t.Errorf("CSP should not be set on OPTIONS; got %q", rr.Header().Get("Content-Security-Policy")) + } + if rr.Code != http.StatusNoContent { + t.Errorf("OPTIONS should pass through; got status %d", rr.Code) + } +} + +func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) { + t.Parallel() + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Custom", "xyz") + w.WriteHeader(http.StatusOK) + }) + cfg := &config.Config{} + h := securityHeadersMiddleware(cfg)(next) + + req := httptest.NewRequest(http.MethodGet, "/health", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Header().Get("X-Custom") != "xyz" { + t.Errorf("downstream header clobbered") + } +} +``` + +- [ ] **Step 2: Run tests (red)** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 ./internal/api/ -run TestSecurityHeaders -v +``` + +Expected: FAIL — `undefined: securityHeadersMiddleware`. + +- [ ] **Step 3: Implement the middleware (CSP only for now)** + +Create `internal/api/security_headers.go`: + +```go +package api + +import ( + "net/http" + + "github.com/RandomCodeSpace/docsiq/internal/config" +) + +// contentSecurityPolicy is deliberately strict for the air-gapped +// deployment posture: no CDN origins, no inline scripts, WASM allowed +// (shiki uses it for syntax highlighting), no iframing. Inline styles +// are permitted because Tailwind + shadcn/ui emit them. +const contentSecurityPolicy = "default-src 'self'; " + + "script-src 'self' 'wasm-unsafe-eval'; " + + "style-src 'self' 'unsafe-inline'; " + + "connect-src 'self'; " + + "img-src 'self' data:; " + + "font-src 'self'; " + + "frame-ancestors 'none'; " + + "base-uri 'self'" + +// securityHeadersMiddleware sets browser-side hardening headers on every +// response that actually carries a body (i.e. non-OPTIONS). Task 4 +// extends this middleware with baseline security headers +// (nosniff, Referrer-Policy, Permissions-Policy, HSTS). Keeping both +// sets in one middleware keeps response-header side-effects colocated. +func securityHeadersMiddleware(_ *config.Config) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodOptions { + next.ServeHTTP(w, r) + return + } + w.Header().Set("Content-Security-Policy", contentSecurityPolicy) + next.ServeHTTP(w, r) + }) + } +} +``` + +- [ ] **Step 4: Run tests (green)** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 ./internal/api/ -run TestSecurityHeaders -v +``` + +Expected: all three PASS. + +- [ ] **Step 5: Wire into router** + +Edit `internal/api/router.go` around line 167. Current: + +```go +return loggingMiddleware( + recoveryMiddleware( + bearerAuthMiddleware(cfg.Server.APIKey, + projectMiddleware(cfg, registry, mux)))) +``` + +Change to: + +```go +return securityHeadersMiddleware(cfg)( + loggingMiddleware( + recoveryMiddleware( + bearerAuthMiddleware(cfg.Server.APIKey, + projectMiddleware(cfg, registry, mux))))) +``` + +Rationale: security headers must be applied to every response, including panic recoveries and 401s. Wrapping outside `loggingMiddleware` also means the logged response status already reflects the final body; no functional reorder. + +- [ ] **Step 6: Run full Go suite + new middleware tests** + +``` +CGO_ENABLED=1 go test -tags sqlite_fts5 ./internal/api/ -v +CGO_ENABLED=1 go test -tags sqlite_fts5 -timeout 300s ./... +``` + +Expected: all pass. Any existing test that asserts on header ABSENCE (e.g. auth_test.go assertions) must still pass because we only add headers. + +- [ ] **Step 7: Run UI Playwright smokes to check the SPA still loads under CSP** + +``` +cd ui && CI=1 ./node_modules/.bin/playwright test smoke.spec.ts --reporter=list --workers=1 +``` + +Expected: 5/5 pass. If any smoke fails with a browser console error about CSP violation, collect the exact violation from the Playwright trace; most likely an inline ` + + +
+ + + +``` + +Note: this script is inline and must therefore be allowed by the CSP rolled out in Block 2. The existing `style-src` already allows `unsafe-inline`; for `script-src` the Block 2 CSP uses nonce or hash. If the CSP fails after deploy, generate the SHA-256 hash of this exact script and add it to the `script-src` directive, or add a nonce during build-time template substitution. Verify via: + +``` +cd ui && npm run build && grep -o 'script-src[^;]*' dist/index.html || echo "No CSP meta in build output (CSP is sent via server headers)" +``` + +If the CSP is server-sent, add the hash to `internal/api/security_headers.go` where `script-src` is computed. Reference Block 2's plan for the exact location. + +- [ ] **Step 4: Re-run the Playwright spec — expect pass** + +``` +cd ui && CI=1 ./node_modules/.bin/playwright test theme-flash.spec.ts --reporter=list --workers=1 +``` + +Expected: 3 passing. + +- [ ] **Step 5: Run the smoke spec to ensure CSP did not break anything** + +``` +cd ui && CI=1 ./node_modules/.bin/playwright test smoke.spec.ts --reporter=list --workers=1 +``` + +Expected: 4 passing. If a CSP violation appears, compute the script's SHA-256 and add to the server CSP: + +``` +cd ui && node -e "const crypto=require('crypto');const s=require('fs').readFileSync('index.html','utf8');const m=s.match(/