-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add Content-Security-Policy and opt-in HSTS, fix healthz routing #449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestComposeMux_HealthzReachesAPIHandlerNotSPAFallback(t *testing.T) { | ||
| apiHit := false | ||
| apiHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| apiHit = true | ||
| if r.URL.Path != "/healthz" { | ||
| t.Errorf("apiHandler saw path %q, want /healthz unchanged", r.URL.Path) | ||
| } | ||
| w.WriteHeader(http.StatusOK) | ||
| _, _ = w.Write([]byte(`{"status":"ok"}`)) | ||
| }) | ||
| webHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| _, _ = w.Write([]byte("<html>spa shell</html>")) | ||
| }) | ||
|
|
||
| mux := composeMux(apiHandler, nil, webHandler) | ||
|
|
||
| rec := httptest.NewRecorder() | ||
| req := httptest.NewRequest(http.MethodGet, "/healthz", nil) | ||
| mux.ServeHTTP(rec, req) | ||
|
|
||
| if !apiHit { | ||
| t.Fatal("GET /healthz did not reach apiHandler, want it exact-matched ahead of the \"/\" SPA fallback") | ||
| } | ||
| if body := rec.Body.String(); body != `{"status":"ok"}` { | ||
| t.Errorf("body = %q, want the API handler's JSON, not the SPA shell", body) | ||
| } | ||
| } | ||
|
|
||
| func TestComposeMux_APIPrefixReachesAPIHandlerWithFullPath(t *testing.T) { | ||
| var gotPath string | ||
| apiHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| gotPath = r.URL.Path | ||
| w.WriteHeader(http.StatusOK) | ||
| }) | ||
| webHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| }) | ||
|
|
||
| mux := composeMux(apiHandler, nil, webHandler) | ||
|
|
||
| rec := httptest.NewRecorder() | ||
| req := httptest.NewRequest(http.MethodGet, "/api/v1/brand", nil) | ||
| mux.ServeHTTP(rec, req) | ||
|
|
||
| if gotPath != "/api/v1/brand" { | ||
| t.Errorf("apiHandler saw path %q, want the full original path unchanged", gotPath) | ||
| } | ||
| } | ||
|
|
||
| func TestComposeMux_UnmatchedPathFallsBackToWebHandler(t *testing.T) { | ||
| apiHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| _, _ = w.Write([]byte("api")) | ||
| }) | ||
| webHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| _, _ = w.Write([]byte("spa")) | ||
| }) | ||
|
|
||
| mux := composeMux(apiHandler, nil, webHandler) | ||
|
|
||
| rec := httptest.NewRecorder() | ||
| req := httptest.NewRequest(http.MethodGet, "/apps/my-app", nil) | ||
| mux.ServeHTTP(rec, req) | ||
|
|
||
| if got := rec.Body.String(); got != "spa" { | ||
| t.Errorf("body = %q, want the SPA fallback to serve an unmatched client-side route", got) | ||
| } | ||
| } | ||
|
|
||
| func TestComposeMux_NilWebhookHandler_NoWebhookRoute(t *testing.T) { | ||
| apiHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| }) | ||
| webHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| _, _ = w.Write([]byte("spa")) | ||
| }) | ||
|
|
||
| mux := composeMux(apiHandler, nil, webHandler) | ||
|
|
||
| rec := httptest.NewRecorder() | ||
| req := httptest.NewRequest(http.MethodPost, "/webhook", nil) | ||
| mux.ServeHTTP(rec, req) | ||
|
|
||
| if got := rec.Body.String(); got != "spa" { | ||
| t.Errorf("POST /webhook body = %q, want the SPA fallback since no webhookHandler was configured", got) | ||
| } | ||
| } | ||
|
|
||
| func TestComposeMux_WebhookHandlerConfigured_ReceivesPost(t *testing.T) { | ||
| webhookHit := false | ||
| webhookHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| webhookHit = true | ||
| w.WriteHeader(http.StatusOK) | ||
| }) | ||
| apiHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) | ||
| webHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) | ||
|
|
||
| mux := composeMux(apiHandler, webhookHandler, webHandler) | ||
|
|
||
| rec := httptest.NewRecorder() | ||
| req := httptest.NewRequest(http.MethodPost, "/webhook", nil) | ||
| mux.ServeHTTP(rec, req) | ||
|
|
||
| if !webhookHit { | ||
| t.Error("POST /webhook did not reach webhookHandler") | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,11 @@ import ( | |
| "context" | ||
| "crypto/rand" | ||
| "encoding/base64" | ||
| "fmt" | ||
| "log/slog" | ||
| "net/http" | ||
| "runtime/debug" | ||
| "time" | ||
| ) | ||
|
|
||
| // requestIDContextKey is unexported, the standard "don't collide with | ||
|
|
@@ -92,22 +94,57 @@ func panicRecoveryMiddleware(logger *slog.Logger) func(http.Handler) http.Handle | |
| } | ||
| } | ||
|
|
||
| // contentSecurityPolicy locks script execution down to the app's own | ||
| // same-origin bundle: no inline script survives web/index.html's own | ||
| // theme-init.js extraction, so script-src needs no 'unsafe-inline' or | ||
| // 'unsafe-eval'. style-src keeps 'unsafe-inline' because @xterm/xterm | ||
| // (AppTerminal.tsx's exec terminal) injects its own <style> element at | ||
| // runtime for cursor rendering; inline style is a far smaller blast | ||
| // radius than inline script (no code execution) and locking it down | ||
| // would need per-request nonce plumbing through the embedded, otherwise | ||
| // static web/dist/index.html, not worth it for the risk it removes. | ||
| // connect-src 'self' already covers the exec terminal's WebSocket and | ||
| // the log/deploy-log SSE streams: CSP3 upgrades a 'self' connect-src | ||
| // match to ws/wss automatically for a same-origin WebSocket, no explicit | ||
| // ws:/wss: entry needed. | ||
| const contentSecurityPolicy = "default-src 'self'; " + | ||
| "script-src 'self'; " + | ||
| "style-src 'self' 'unsafe-inline'; " + | ||
| "img-src 'self' data:; " + | ||
| "font-src 'self' data:; " + | ||
| "connect-src 'self'; " + | ||
| "object-src 'none'; " + | ||
| "base-uri 'self'; " + | ||
| "form-action 'self'; " + | ||
| "frame-ancestors 'none'" | ||
|
|
||
| // hstsMaxAge is 180 days: long enough to be meaningful, short enough | ||
| // that disabling APP_ENABLE_HSTS again (see Router.hstsEnabled) doesn't | ||
| // leave a browser enforcing HTTPS-only against this host indefinitely. | ||
| const hstsMaxAge = 180 * 24 * time.Hour | ||
|
|
||
| // securityHeadersMiddleware sets the response headers that cost nothing | ||
| // to get right and directly reduce the blast radius of an XSS or | ||
| // clickjacking attempt against a dashboard with root-level actions | ||
| // (deploys, secrets, rollbacks) behind it. Deliberately narrow: a | ||
| // Content-Security-Policy or Strict-Transport-Security header needs | ||
| // verifying against the actual frontend bundle and the embedded-Caddy | ||
| // TLS story respectively before shipping, so both are left for a | ||
| // follow-up rather than guessed at here. | ||
| func securityHeadersMiddleware(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| h := w.Header() | ||
| h.Set("X-Content-Type-Options", "nosniff") | ||
| h.Set("X-Frame-Options", "DENY") | ||
| h.Set("Referrer-Policy", "strict-origin-when-cross-origin") | ||
| next.ServeHTTP(w, r) | ||
| }) | ||
| // (deploys, secrets, rollbacks) behind it. Strict-Transport-Security is | ||
| // gated on hstsEnabled (Router.hstsEnabled's own doc comment explains | ||
| // why it isn't inferred automatically); everything else, including | ||
| // Content-Security-Policy, is unconditional. | ||
| func securityHeadersMiddleware(hstsEnabled bool) func(http.Handler) http.Handler { | ||
| hsts := fmt.Sprintf("max-age=%d; includeSubDomains", int(hstsMaxAge.Seconds())) | ||
| return func(next http.Handler) http.Handler { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Enabling HSTS also sends |
||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| h := w.Header() | ||
| h.Set("X-Content-Type-Options", "nosniff") | ||
| h.Set("X-Frame-Options", "DENY") | ||
| h.Set("Referrer-Policy", "strict-origin-when-cross-origin") | ||
| h.Set("Content-Security-Policy", contentSecurityPolicy) | ||
| if hstsEnabled { | ||
| h.Set("Strict-Transport-Security", hsts) | ||
| } | ||
| next.ServeHTTP(w, r) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // handleHealthz handles GET /healthz: an unauthenticated liveness check | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
composeMuxsends/healthzand/api/throughrt.Handler(), where the security middleware is installed, but sends/and static assets directly throughweb.Handler(). As a result, the dashboard HTML does not receive the new Content-Security-Policy, so the policy does not restrict script execution on the page it is meant to protect.How this was verified:
GET /is routed directly to the unwrapped web handler, whereas the CSP middleware exists only inside the API handler.