From 7cd6fbf571fd808e0d4e3843d4feccb2ebbefe9b Mon Sep 17 00:00:00 2001 From: Niklas Ye Date: Wed, 7 Oct 2026 21:57:00 +0200 Subject: [PATCH] Cap request body size and add baseline security headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of a security-hardening pass (see wiki for the full backlog). decodeJSON had no size limit at all, so every JSON endpoint -- including the two unauthenticated ones (bootstrap, the Alertmanager webhook) -- would buffer an attacker-supplied body of unbounded size before it was even validated. decodeJSON now wraps the body in http.MaxBytesReader at a 1 MiB default; the webhook gets its own 8 MiB cap via decodeJSONLimit, since a real Alertmanager batch can be bigger than an ordinary API body. Also adds a securityHeaders middleware, applied globally: nosniff on every response (previously only the static site got it), and HSTS (180-day max-age, conservative on purpose) whenever cookieSecure's signal says the browser is on HTTPS. Checked the chart/gateway config first -- neither sets HSTS anywhere, so this was a real gap. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/alertmanager.go | 8 +- internal/api/helpers.go | 17 ++++ internal/api/middleware.go | 22 +++++ internal/api/router.go | 1 + internal/api/security_headers_test.go | 115 ++++++++++++++++++++++++++ 5 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 internal/api/security_headers_test.go diff --git a/internal/api/alertmanager.go b/internal/api/alertmanager.go index 0a41784..071684b 100644 --- a/internal/api/alertmanager.go +++ b/internal/api/alertmanager.go @@ -94,10 +94,16 @@ func handleIntegrationWebhook(db *sql.DB, notify NotifyConfig) http.HandlerFunc } } +// maxWebhookBodyBytes is larger than maxBodyBytes: a real Alertmanager batch +// can carry many alerts, each with several labels and annotations, and the +// sender is a trusted piece of infrastructure rather than an arbitrary +// caller. +const maxWebhookBodyBytes = 8 << 20 + func receiveWebhook(w http.ResponseWriter, r *http.Request, db *sql.DB, notify NotifyConfig, src alertSource) { teamID := src.teamID var payload amPayload - if err := decodeJSON(r, &payload); err != nil { + if err := decodeJSONLimit(r, &payload, maxWebhookBodyBytes); err != nil { respond(w, http.StatusBadRequest, errResp("invalid payload")) return } diff --git a/internal/api/helpers.go b/internal/api/helpers.go index b566a40..4ea7dcd 100644 --- a/internal/api/helpers.go +++ b/internal/api/helpers.go @@ -72,8 +72,25 @@ func respond(w http.ResponseWriter, status int, v any) { json.NewEncoder(w).Encode(v) } +// maxBodyBytes caps an ordinary JSON request body. 1 MiB is far more than any +// endpoint below needs — it exists so an unauthenticated caller (signup, +// login, bootstrap) can't make the server buffer an arbitrarily large body +// before the request is even validated. +const maxBodyBytes = 1 << 20 + func decodeJSON(r *http.Request, v any) error { + return decodeJSONLimit(r, v, maxBodyBytes) +} + +// decodeJSONLimit is decodeJSON with an explicit cap, for the one endpoint +// (the Alertmanager webhook, see maxWebhookBodyBytes) whose real payloads can +// legitimately be larger than maxBodyBytes. +func decodeJSONLimit(r *http.Request, v any, limit int64) error { defer r.Body.Close() + // w is nil: there is no ResponseWriter here to disable keep-alive with, + // which net/http documents as fine — the limit is still enforced, the + // connection just isn't closed early on a request that blows past it. + r.Body = http.MaxBytesReader(nil, r.Body, limit) return json.NewDecoder(r.Body).Decode(v) } diff --git a/internal/api/middleware.go b/internal/api/middleware.go index 0aa314a..efae813 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -79,6 +79,28 @@ func AuthMiddleware(db *sql.DB) func(http.Handler) http.Handler { } } +// securityHeaders sets headers that cost nothing to send on every response, +// API or static site alike. nosniff is unconditional; HSTS only fires once +// cookieSecure's signal says the browser is actually looking at this server +// over HTTPS — TLS terminates at the gateway, which (as of this writing) sets +// neither header itself. +// +// max-age is 180 days rather than the usual year-plus: short enough that if +// HTTPS here ever broke for real, the header would age out of a browser's +// cache well within a release cycle instead of locking anyone out of a +// working server. Raise it once this has run clean for a while. +func securityHeaders(publicURL string) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Content-Type-Options", "nosniff") + if cookieSecure(publicURL, r) { + w.Header().Set("Strict-Transport-Security", "max-age=15552000; includeSubDomains") + } + next.ServeHTTP(w, r) + }) + } +} + // AdminOnly rejects a caller who is not a system administrator. It runs inside // AuthMiddleware's group, so by the time it sees a request the caller is known. // diff --git a/internal/api/router.go b/internal/api/router.go index a484bb9..3540795 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -30,6 +30,7 @@ func NewRouter(db *sql.DB, notify NotifyConfig, cfg config.Config, version strin r := chi.NewRouter() r.Use(middleware.Logger) r.Use(middleware.Recoverer) + r.Use(securityHeaders(notify.PublicURL)) r.Get("/healthz", func(w http.ResponseWriter, r *http.Request) { respond(w, http.StatusOK, map[string]string{"status": "ok"}) diff --git a/internal/api/security_headers_test.go b/internal/api/security_headers_test.go new file mode 100644 index 0000000..0f07421 --- /dev/null +++ b/internal/api/security_headers_test.go @@ -0,0 +1,115 @@ +package api_test + +import ( + "bytes" + "net/http" + "strings" + "testing" + + "git.ryuvia.com/niklas/terdut-server/internal/api" +) + +// --------------------------------------------------------------------------- +// Security headers +// --------------------------------------------------------------------------- + +func TestSecurityHeaders_NosniffAlwaysSet(t *testing.T) { + s := newTS(t) // no PublicURL: the HTTPS signal is off + resp := s.req(t, http.MethodGet, "/api/me", nil) + defer resp.Body.Close() + + if got := resp.Header.Get("X-Content-Type-Options"); got != "nosniff" { + t.Errorf("X-Content-Type-Options = %q, want nosniff", got) + } + if got := resp.Header.Get("Strict-Transport-Security"); got != "" { + t.Errorf("Strict-Transport-Security = %q, want unset without an https PublicURL", got) + } +} + +func TestSecurityHeaders_HSTSWhenPublicURLIsHTTPS(t *testing.T) { + s := newTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"}) + resp := s.req(t, http.MethodGet, "/api/me", nil) + defer resp.Body.Close() + + got := resp.Header.Get("Strict-Transport-Security") + if !strings.HasPrefix(got, "max-age=") || !strings.Contains(got, "includeSubDomains") { + t.Errorf("Strict-Transport-Security = %q, want a max-age with includeSubDomains", got) + } +} + +// --------------------------------------------------------------------------- +// Request body size limits +// --------------------------------------------------------------------------- + +// TestBodySizeLimit_OrdinaryEndpointRejectsOversizedBody confirms an +// unauthenticated endpoint can't be made to buffer an arbitrarily large body: +// past maxBodyBytes, decodeJSON fails exactly as it would on any other +// malformed body, rather than the server reading the whole thing first. +func TestBodySizeLimit_OrdinaryEndpointRejectsOversizedBody(t *testing.T) { + s := newTS(t) + + huge := bytes.Repeat([]byte("a"), 2<<20) // 2 MiB, past the 1 MiB default + body := []byte(`{"username":"` + string(huge) + `","password":"x"}`) + + resp, err := http.Post(s.URL+"/api/login", "application/json", bytes.NewReader(body)) + if err != nil { + t.Fatalf("POST /api/login: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("status = %d, want %d (oversized body treated as invalid)", resp.StatusCode, http.StatusBadRequest) + } +} + +// TestBodySizeLimit_WebhookAllowsLargerBodyThanDefault confirms the +// Alertmanager webhook's separate, larger cap actually takes effect: a body +// bigger than the ordinary default but within maxWebhookBodyBytes is still +// accepted, not rejected by the smaller limit every other endpoint gets. +func TestBodySizeLimit_WebhookAllowsLargerBodyThanDefault(t *testing.T) { + s := newTS(t) + + // Padding kept inside one alert's annotation, comfortably past the 1 MiB + // default and still well under the webhook's 8 MiB cap. + padding := strings.Repeat("a", 3<<20) // 3 MiB + payload := `{"version":"4","status":"firing","groupKey":"big-group",` + + `"groupLabels":{"alertname":"BigAlert"},"alerts":[{"status":"firing",` + + `"labels":{"alertname":"BigAlert"},"annotations":{"note":"` + padding + `"},` + + `"startsAt":"2026-05-20T10:00:00Z","endsAt":"0001-01-01T00:00:00Z",` + + `"fingerprint":"fp-big"}]}` + + resp, err := http.Post(s.URL+"/api/integrations/"+s.ingestKey+"/alertmanager", + "application/json", strings.NewReader(payload)) + if err != nil { + t.Fatalf("POST webhook: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want %d (body under the webhook's own cap)", resp.StatusCode, http.StatusOK) + } +} + +// TestBodySizeLimit_WebhookRejectsPastItsOwnCap confirms the webhook's larger +// cap is still a cap, not an exemption from one. +func TestBodySizeLimit_WebhookRejectsPastItsOwnCap(t *testing.T) { + s := newTS(t) + + huge := strings.Repeat("a", 9<<20) // 9 MiB, past the 8 MiB webhook cap + payload := `{"version":"4","status":"firing","groupKey":"huge-group",` + + `"groupLabels":{"alertname":"HugeAlert"},"alerts":[{"status":"firing",` + + `"labels":{"alertname":"HugeAlert"},"annotations":{"note":"` + huge + `"},` + + `"startsAt":"2026-05-20T10:00:00Z","endsAt":"0001-01-01T00:00:00Z",` + + `"fingerprint":"fp-huge"}]}` + + resp, err := http.Post(s.URL+"/api/integrations/"+s.ingestKey+"/alertmanager", + "application/json", strings.NewReader(payload)) + if err != nil { + t.Fatalf("POST webhook: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("status = %d, want %d (body past the webhook's own cap)", resp.StatusCode, http.StatusBadRequest) + } +}