feat: add Content-Security-Policy and opt-in HSTS, fix healthz routing - #449
Conversation
Ships the two security headers explicitly deferred from the earlier
production-readiness batch (securityHeadersMiddleware's own prior doc
comment):
- Content-Security-Policy is now sent unconditionally, script-src
locked to 'self' with no unsafe-inline/unsafe-eval. This required
extracting web/index.html's inline theme-flash-prevention script into
web/public/theme-init.js first, verified via a real npm run build
that the built dist/index.html has no inline script left.
- Strict-Transport-Security is opt-in via APP_ENABLE_HSTS (default
off): this control plane's own HTTP server never terminates TLS
itself, so it can't tell from a request alone whether the browser saw
a trusted ACME certificate or the self-signed internal-issuer
default, and HSTS on a self-signed deployment turns a certificate
warning into a hard lockout. Documented in domains-and-ingress.md
next to the ACME/self-signed TLS explanation.
Also fixes a real routing bug found while live-verifying this: the
control plane's top-level mux only forwarded the "/api/" prefix to the
API router, so a plain GET /healthz (what an orchestrator or load
balancer actually probes) fell through to the SPA catch-all and got a
200 with the dashboard's index.html body instead of {"status":"ok"}.
Extracted the mux composition into composeMux so this routing
precedence is directly unit-tested instead of only reachable through
rootHandler's full dependency graph.
Verified live: built with -tags embedweb, ran against an isolated
docker:dind daemon, confirmed GET /healthz, GET /, GET /theme-init.js,
and GET /api/v1/brand all return the expected body and headers.
What this doesn't do:
- No nonce/hash-based CSP: style-src keeps 'unsafe-inline' because
@xterm/xterm injects its own <style> element at runtime; locking that
down needs per-request nonce plumbing through the otherwise-static
embedded index.html, not worth it for the risk removed (inline style
can't execute script).
- Does not gate HSTS on the ingress settings' ACMEEnabled flag
automatically; that would need a DB read on every request for a
header most deployments won't enable yet, so it stays a manual
operator opt-in instead.
- Does not add a CSP report-uri/report-to endpoint to collect violation
reports from real browsers.
github-actions
Bot
added
size/l
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| rt := api.NewRouter(logger, b, db, opts...) | ||
| return composeMux(rt.Handler(), webhookHandler, web.Handler()), rt |
There was a problem hiding this comment.
composeMux sends /healthz and /api/ through rt.Handler(), where the security middleware is installed, but sends / and static assets directly through web.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.
| // 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.
Enabling HSTS also sends includeSubDomains, although the opt-in documentation describes enforcement only for the dashboard host. Preview environments use names such as pr-<number>.<app>.<primary-domain> and may still use Caddy's internal issuer when the dashboard itself has a trusted certificate. After an API response stores this policy, browsers will reject those preview certificates without allowing a bypass for 180 days. Either omit includeSubDomains or require and document trusted TLS for every subdomain before enabling HSTS.
1 participant
Summary
Ships the two security headers explicitly deferred from the earlier production-readiness batch (#445), plus a real routing bug found while live-verifying them.
script-src 'self'with nounsafe-inline/unsafe-eval. Required extractingweb/index.html's inline theme-flash-prevention script intoweb/public/theme-init.jsfirst (verified via a realnpm run buildthat the builtdist/index.htmlhas zero inline<script>content left, timing preserved: still a blocking, synchronous, non-module script).APP_ENABLE_HSTS(default off). This control plane's own HTTP server never terminates TLS itself (embedded Caddy does, proxying to it over loopback), so it has no way to tell from a request alone whether the browser saw a trusted ACME certificate or the self-signed internal-issuer default. HSTS on a self-signed deployment removes the browser's "proceed anyway" escape hatch, turning a cert warning into a hard lockout, so this stays a manual opt-in rather than inferred. Documented indocs/domains-and-ingress.mdnext to the existing ACME/self-signed TLS explanation./api/prefix to the API router, so a plainGET /healthz(what an orchestrator or load balancer actually probes) fell through to the SPA catch-all and returned a 200 with the dashboard'sindex.htmlbody instead of{"status":"ok"}. Extracted the mux composition into a smallcomposeMuxfunction so this routing precedence is directly unit-tested instead of only reachable throughrootHandler's full dependency graph.Verification
go build ./...,go vet ./...,golangci-lint runclean.go test ./internal/api/... -shortandgo test ./cmd/levelrail/... -shortpass, including new tests for both the header middleware and the mux routing precedence.-tags embedweb, ran the real binary against an isolateddocker:dinddaemon (never the shared host daemon), confirmedGET /healthz,GET /,GET /theme-init.js, andGET /api/v1/brandall return the expected body and full header set, then tore the isolated environment down.npx tsc -bclean on the frontend change.What this doesn't do
style-srckeeps'unsafe-inline'because@xterm/xterm(the exec terminal) injects its own<style>element at runtime for cursor rendering. Locking that down needs per-request nonce plumbing through the otherwise-static embeddedindex.html, not worth it for the risk removed (inline style can't execute script).ACMEEnabledflag automatically; that would need a DB read on every single request for a header most deployments won't enable yet, so it stays a manual operator opt-in instead.report-uri/report-toendpoint to collect violation reports from real browsers.