fix(backend): report ingest correctness — propagate storage failure, 413 on oversize, global body cap (closes #23)
check / check (push) Successful in 45s
check / check (push) Successful in 45s
A buffer failure on POST /api/v1/reports now returns 500 instead of a false `ok`, so clients can retry. Decode errors split: an over-limit body returns 413 (via errors.As on `*http.MaxBytesError`), malformed JSON stays 400. A new MaxBodyBytes middleware (1 MiB default) caps every route — rejecting an oversized Content-Length up front and capping the read otherwise — so the health check and future routes are bounded too. The raw attacker-controlled geo blob is no longer logged, only its length; client_id and timestamp are length-bounded before logging. A decodeJSON handler helper is added. Panic recovery is now a local middleware routing the stack through slog as structured JSON. Storage failure uses 500: a full buffer or write error is server-side and retryable. Model: opus-4-8
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
)
|
||||
@@ -8,6 +9,12 @@ import (
|
||||
// Test-only wrappers exposing unexported helpers to the
|
||||
// external middleware_test package.
|
||||
|
||||
// NewWithLogger builds a Middleware around a logger for tests
|
||||
// that exercise the logging paths without the fx graph.
|
||||
func NewWithLogger(log *slog.Logger) *Middleware {
|
||||
return &Middleware{log: log}
|
||||
}
|
||||
|
||||
func ClientIP(
|
||||
remoteAddr string,
|
||||
header http.Header,
|
||||
|
||||
@@ -3,11 +3,14 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"runtime/debug"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -22,6 +25,14 @@ import (
|
||||
|
||||
const corsMaxAgeSec = 300
|
||||
|
||||
// jsonErrorBody is the body written for errors raised inside
|
||||
// middleware, matching the {"status":"error"} shape the handlers
|
||||
// return so clients see one error contract across the API.
|
||||
const (
|
||||
jsonContentType = "application/json; charset=utf-8"
|
||||
jsonErrorBody = "{\"status\":\"error\"}\n"
|
||||
)
|
||||
|
||||
// Security header values. The backend is a JSON API with no
|
||||
// HTML surface, so the CSP forbids every resource type and
|
||||
// framing outright.
|
||||
@@ -236,6 +247,79 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
|
||||
}
|
||||
}
|
||||
|
||||
// writeJSONError writes the shared JSON error body with the
|
||||
// given status. Used where middleware must reject a request
|
||||
// before it reaches a handler.
|
||||
func writeJSONError(w http.ResponseWriter, status int) {
|
||||
w.Header().Set("Content-Type", jsonContentType)
|
||||
w.WriteHeader(status)
|
||||
_, _ = io.WriteString(w, jsonErrorBody)
|
||||
}
|
||||
|
||||
// MaxBodyBytes returns middleware that caps the request body at
|
||||
// limit bytes. A declared Content-Length over the limit is
|
||||
// rejected immediately with 413. Bodies without a declared
|
||||
// length (or that understate it) are capped as they are read, so
|
||||
// a handler that reads the body sees a *http.MaxBytesError it can
|
||||
// map to 413. Mount it with a different limit on a route group
|
||||
// that needs a different bound.
|
||||
func (s *Middleware) MaxBodyBytes(
|
||||
limit int64,
|
||||
) func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.ContentLength > limit {
|
||||
writeJSONError(
|
||||
w,
|
||||
http.StatusRequestEntityTooLarge,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
r.Body = http.MaxBytesReader(w, r.Body, limit)
|
||||
|
||||
next.ServeHTTP(w, r)
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Recoverer returns middleware that recovers from a panic in a
|
||||
// downstream handler, logs the panic and stack trace through
|
||||
// slog, and responds 500 with no body. http.ErrAbortHandler is
|
||||
// re-panicked so the server can abort the response as intended.
|
||||
func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
defer func() {
|
||||
rec := recover()
|
||||
if rec == nil {
|
||||
return
|
||||
}
|
||||
|
||||
err, ok := rec.(error)
|
||||
if ok && errors.Is(err, http.ErrAbortHandler) {
|
||||
panic(rec)
|
||||
}
|
||||
|
||||
s.log.ErrorContext(r.Context(),
|
||||
"panic recovered",
|
||||
"panic", fmt.Sprintf("%v", rec),
|
||||
"stack", string(debug.Stack()),
|
||||
)
|
||||
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
}()
|
||||
|
||||
next.ServeHTTP(w, r)
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// CORS returns middleware that adds permissive CORS headers.
|
||||
func (s *Middleware) CORS() func(http.Handler) http.Handler {
|
||||
return cors.Handler(cors.Options{
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
package middleware_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/netip"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/netwatch/internal/middleware"
|
||||
@@ -137,3 +140,92 @@ func TestSecurityHeaders(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestMaxBodyBytesRejectsOversizeOnNonReadingRoute confirms the
|
||||
// limit is enforced even for a handler that never reads the body
|
||||
// (for example the health check), via the Content-Length check.
|
||||
func TestMaxBodyBytesRejectsOversizeOnNonReadingRoute(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 16
|
||||
|
||||
called := false
|
||||
handler := (&middleware.Middleware{}).MaxBodyBytes(limit)(
|
||||
http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
|
||||
called = true
|
||||
}),
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPost, "/.well-known/healthcheck",
|
||||
strings.NewReader(strings.Repeat("x", limit+1)),
|
||||
)
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusRequestEntityTooLarge {
|
||||
t.Fatalf("status = %d, want %d",
|
||||
rec.Code, http.StatusRequestEntityTooLarge)
|
||||
}
|
||||
|
||||
if called {
|
||||
t.Fatal("handler ran despite oversize body")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMaxBodyBytesAllowsWithinLimit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 64
|
||||
|
||||
handler := (&middleware.Middleware{}).MaxBodyBytes(limit)(
|
||||
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}),
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{"clientId":"c1"}`),
|
||||
)
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRecovererReturns500AndLogsThroughSlog(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var logbuf bytes.Buffer
|
||||
|
||||
mw := middleware.NewWithLogger(
|
||||
slog.New(slog.NewJSONHandler(&logbuf, nil)),
|
||||
)
|
||||
|
||||
handler := mw.Recoverer()(
|
||||
http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
|
||||
panic("boom")
|
||||
}),
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/", http.NoBody)
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusInternalServerError {
|
||||
t.Fatalf("status = %d, want %d",
|
||||
rec.Code, http.StatusInternalServerError)
|
||||
}
|
||||
|
||||
out := logbuf.String()
|
||||
if !strings.Contains(out, "panic recovered") {
|
||||
t.Fatalf("panic was not logged through slog: %q", out)
|
||||
}
|
||||
|
||||
if !strings.Contains(out, `"level":"ERROR"`) {
|
||||
t.Fatalf("panic log was not structured JSON at error level: %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user