Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 43 additions & 3 deletions cmd/levelrail/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -1725,6 +1725,7 @@
api.WithNotificationDeliveries(alertingDB),
api.WithSessionTTL(sessionTTL(logger)),
api.WithAutoPlacement(autoPlacementEnabled(logger)),
api.WithHSTS(hstsEnabled(logger)),
api.WithAPIRateLimit(apiRateLimitReadRPM(logger), apiRateLimitWriteRPM(logger)),
api.WithDataDir(dataDir),
api.WithDockerPinger(client),
Expand Down Expand Up @@ -1922,13 +1923,33 @@
}

rt := api.NewRouter(logger, b, db, opts...)
return composeMux(rt.Handler(), webhookHandler, web.Handler()), rt

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security CSP Bypasses Dashboard

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.

}

// composeMux wires the three top-level handlers rootHandler serves
// behind one *http.Server into a single mux. Pulled out of rootHandler
// so the routing precedence itself, not just rootHandler's much larger
// dependency graph, is directly unit-testable.
//
// "/healthz" is registered ahead of "/api/" and "/" as its own
// exact-path pattern so a plain GET /healthz (what a systemd unit,
// container orchestrator, or load balancer actually probes, see
// handleHealthz's own doc comment) reaches apiHandler's own "GET
// /healthz" route. Without this explicit entry, "/healthz" has no
// "/api/" prefix, so it would fall through to the "/" SPA fallback and
// get back a 200 with the dashboard's index.html body instead of the
// {"status":"ok"} JSON a prober actually expects. webhookHandler is
// nil-able: a control plane started without git-webhook config just
// serves no POST /webhook route.
func composeMux(apiHandler http.Handler, webhookHandler http.Handler, webHandler http.Handler) *http.ServeMux {

Check warning on line 1944 in cmd/levelrail/main.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Group together these consecutive parameters of the same type.

See more on https://sonarcloud.io/project/issues?id=glincker_levelrail&issues=AaCd9cEtEG_HJOnAOobi&open=AaCd9cEtEG_HJOnAOobi&pullRequest=449
mux := http.NewServeMux()
mux.Handle("/api/", rt.Handler())
mux.Handle("/healthz", apiHandler)
mux.Handle("/api/", apiHandler)
if webhookHandler != nil {
mux.Handle("POST /webhook", webhookHandler)
}
mux.Handle("/", web.Handler())
return mux, rt
mux.Handle("/", webHandler)
return mux
}

// backupSchedulerInterval reads APP_BACKUP_SCHEDULER_INTERVAL as a Go
Expand Down Expand Up @@ -2042,6 +2063,25 @@
return v
}

// hstsEnabled reads APP_ENABLE_HSTS as a bool, api.WithHSTS's own
// enabled param. Defaults to false (unlike autoPlacementEnabled above):
// Strict-Transport-Security is safe only once an operator has real,
// browser-trusted certificates (APP_PUBLIC_HOST plus ACMEEnabled, see
// Router.hstsEnabled's own doc comment), which this process has no way
// to confirm on its own, so it's opt-in rather than assumed.
func hstsEnabled(logger *slog.Logger) bool {
raw := os.Getenv("APP_ENABLE_HSTS")
if raw == "" {
return false
}
v, err := strconv.ParseBool(raw)
if err != nil {
logger.Warn("invalid APP_ENABLE_HSTS, defaulting to disabled", slog.String("value", raw), slog.String("error", err.Error()))
return false
}
return v
}

// certExpiryWarningWindow reads APP_CERT_EXPIRY_WARNING_WINDOW as a Go
// duration string, the same env-var-with-default shape sessionTTL above
// already uses for api.WithSessionTTL, applied here to both
Expand Down
118 changes: 118 additions & 0 deletions cmd/levelrail/routing_test.go
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")
}
}
11 changes: 11 additions & 0 deletions docs/domains-and-ingress.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,17 @@ Be clear-eyed about where this stands, because it's easy to overstate:
by step. Don't take "toggle exists" as "proven to work at internet
scale" until that runbook (or your own experience) confirms it.

- **HTTP Strict Transport Security (HSTS): opt-in, once you're on real
certificates.** Set `APP_ENABLE_HSTS=true` on the control plane to send
`Strict-Transport-Security` on every response. It defaults to off on
purpose: HSTS tells a browser to refuse plain HTTP and refuse to let a
visitor click through a certificate warning on this host for the next
180 days, so turning it on before `ACMEEnabled` is true (i.e. while
you're still on Caddy's self-signed internal issuer above) can lock
you out of your own dashboard the next time that self-signed cert
looks untrusted. Only enable it once real, browser-trusted certificates
are actually issuing.

- **Bring your own certificate.** If ACME can't reach a domain (an
internal-only host, an externally issued wildcard, a cert already
provisioned before DNS cuts over), you can upload your own
Expand Down
63 changes: 50 additions & 13 deletions internal/api/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 HSTS Covers Preview Subdomains

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.

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
Expand Down
31 changes: 27 additions & 4 deletions internal/api/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"
)

Expand Down Expand Up @@ -91,18 +92,40 @@ func TestSecurityHeadersMiddleware_SetsExpectedHeaders(t *testing.T) {

rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, "/whatever", nil)
securityHeadersMiddleware(inner).ServeHTTP(rec, req)
securityHeadersMiddleware(false)(inner).ServeHTTP(rec, req)

tests := map[string]string{
"X-Content-Type-Options": "nosniff",
"X-Frame-Options": "DENY",
"Referrer-Policy": "strict-origin-when-cross-origin",
"X-Content-Type-Options": "nosniff",
"X-Frame-Options": "DENY",
"Referrer-Policy": "strict-origin-when-cross-origin",
"Content-Security-Policy": contentSecurityPolicy,
}
for header, want := range tests {
if got := rec.Header().Get(header); got != want {
t.Errorf("%s = %q, want %q", header, got, want)
}
}
if got := rec.Header().Get("Strict-Transport-Security"); got != "" {
t.Errorf("Strict-Transport-Security = %q, want unset when hstsEnabled is false", got)
}
}

func TestSecurityHeadersMiddleware_HSTSEnabled_SetsHeader(t *testing.T) {
inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})

rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, "/whatever", nil)
securityHeadersMiddleware(true)(inner).ServeHTTP(rec, req)

got := rec.Header().Get("Strict-Transport-Security")
if got == "" {
t.Fatal("Strict-Transport-Security is unset, want it set when hstsEnabled is true")
}
if !strings.Contains(got, "includeSubDomains") {
t.Errorf("Strict-Transport-Security = %q, want includeSubDomains", got)
}
}

func TestHandleHealthz_OKWithNoAuth(t *testing.T) {
Expand Down
11 changes: 11 additions & 0 deletions internal/api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,17 @@ type Router struct {
// handleCheckDomain (domain_check.go) falls back to the request's own
// Host header in that case, see advertisedHost's own doc comment.
publicHost string
// hstsEnabled sends Strict-Transport-Security when true. Defaults to
// false: this control plane's own HTTP server never terminates TLS
// itself (the embedded Caddy ingress does, see WithDashboardDial in
// internal/reconcile/ingress), so it has no way to know from a
// request alone whether the certificate a browser actually saw was a
// trusted ACME one or the self-signed/internal-issuer default. HSTS
// on a self-signed deployment removes the browser's "proceed anyway"
// escape hatch on the next visit, turning a certificate warning into
// a hard lockout, so this stays opt-in (APP_ENABLE_HSTS) rather than
// inferred. Set via WithHSTS.
hstsEnabled bool
// lookupHost resolves a hostname's A/AAAA addresses for
// handleCheckDomain; always non-nil, defaulted to defaultLookupHost
// (a thin net.DefaultResolver.LookupHost wrapper) in NewRouter,
Expand Down
7 changes: 7 additions & 0 deletions internal/api/router_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,13 @@ func WithReconcileNudger(n ReconcileNudger) Option {
return func(rt *Router) { rt.reconcileNudger = n }
}

// WithHSTS enables the Strict-Transport-Security response header.
// Without it (the default), Strict-Transport-Security is never sent, see
// Router.hstsEnabled's own doc comment for why that's the safe default.
func WithHSTS(enabled bool) Option {
return func(rt *Router) { rt.hstsEnabled = enabled }
}

// WithAlertRules enables POST/GET /api/v1/apps/{name}/alerts and DELETE
// /api/v1/apps/{name}/alerts/{id}. Without one
// configured (the default), all three routes return 501, the same
Expand Down
2 changes: 1 addition & 1 deletion internal/api/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func (rt *Router) Handler() http.Handler {
rt.registerPlatformRoutes(mux)

var h http.Handler = mux
h = securityHeadersMiddleware(h)
h = securityHeadersMiddleware(rt.hstsEnabled)(h)
h = panicRecoveryMiddleware(rt.logger)(h)
h = requestIDMiddleware(h)
return h
Expand Down
Loading
Loading