fix(backend): report ingest correctness — propagate storage failure, 413 on oversize, global body cap (closes #23)
check / check (push) Failing after 1s
check / check (push) Failing after 1s
A buffer failure on POST /api/v1/reports now returns 500 instead of a
false {"status":"ok"}, so clients can retry. Decode errors are split:
an over-limit body returns 413 (via errors.As on *http.MaxBytesError),
malformed JSON stays 400. A new MaxBodyBytes middleware caps every
route (1 MiB default; rejects an oversized Content-Length up-front and
caps the read otherwise), replacing the per-route reader so the health
check and future routes are bounded too. The raw attacker-controlled
geo blob is no longer logged — only its byte length — and client_id and
timestamp are length-bounded before logging. A decodeJSON handler helper
is added per the HTTP server conventions. Panic recovery is now a local
middleware that routes the stack through slog as structured JSON rather
than chi's plain-text stderr. Error bodies still leak nothing internal.
Chosen failure code for a storage failure: 500, since a full buffer or
write error is server-side and retryable, not the client's fault.
Model: opus-4-8
This commit is contained in:
@@ -0,0 +1,10 @@
|
||||
package handlers
|
||||
|
||||
import "log/slog"
|
||||
|
||||
// NewForTest builds a Handlers around a report sink and logger,
|
||||
// bypassing the fx graph so handler behaviour (including the
|
||||
// storage failure path) is exercisable in unit tests.
|
||||
func NewForTest(buf reportAppender, log *slog.Logger) *Handlers {
|
||||
return &Handlers{buf: buf, log: log}
|
||||
}
|
||||
@@ -18,6 +18,13 @@ import (
|
||||
|
||||
const jsonContentType = "application/json; charset=utf-8"
|
||||
|
||||
// reportAppender is the subset of the report buffer the handlers
|
||||
// depend on. Defining it here keeps the storage failure path
|
||||
// exercisable with a stub in tests.
|
||||
type reportAppender interface {
|
||||
Append(v any) error
|
||||
}
|
||||
|
||||
// Params defines the dependencies for Handlers.
|
||||
type Params struct {
|
||||
fx.In
|
||||
@@ -30,7 +37,7 @@ type Params struct {
|
||||
|
||||
// Handlers provides HTTP handler factories for all endpoints.
|
||||
type Handlers struct {
|
||||
buf *reportbuf.Buffer
|
||||
buf reportAppender
|
||||
hc *healthcheck.Healthcheck
|
||||
log *slog.Logger
|
||||
params *Params
|
||||
@@ -72,3 +79,15 @@ func (s *Handlers) respondJSON(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// decodeJSON decodes the request body into v. The body is
|
||||
// expected to already be bounded by the body-size middleware, so
|
||||
// a caller can distinguish an over-limit body from malformed
|
||||
// JSON by testing the returned error for *http.MaxBytesError.
|
||||
func (s *Handlers) decodeJSON(
|
||||
_ http.ResponseWriter,
|
||||
r *http.Request,
|
||||
v any,
|
||||
) error {
|
||||
return json.NewDecoder(r.Body).Decode(v)
|
||||
}
|
||||
|
||||
@@ -2,10 +2,14 @@ package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
)
|
||||
|
||||
const maxReportBodyBytes = 1 << 20 // 1 MiB
|
||||
// maxLoggedFieldBytes bounds untrusted string fields before they
|
||||
// are logged, so a caller cannot inflate log volume with an
|
||||
// oversized value.
|
||||
const maxLoggedFieldBytes = 128
|
||||
|
||||
type reportSample struct {
|
||||
T int64 `json:"t"`
|
||||
@@ -35,48 +39,76 @@ func (s *Handlers) HandleReport() http.HandlerFunc {
|
||||
}
|
||||
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, maxReportBodyBytes,
|
||||
)
|
||||
|
||||
var rpt report
|
||||
|
||||
err := json.NewDecoder(r.Body).Decode(&rpt)
|
||||
err := s.decodeJSON(w, r, &rpt)
|
||||
if err != nil {
|
||||
s.log.Error("failed to decode report",
|
||||
"error", err,
|
||||
)
|
||||
s.respondJSON(w, r,
|
||||
&response{Status: "error"},
|
||||
http.StatusBadRequest,
|
||||
s.decodeErrorStatus(err),
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
totalSamples := 0
|
||||
for _, h := range rpt.Hosts {
|
||||
totalSamples += len(h.History)
|
||||
}
|
||||
s.logReportReceived(rpt)
|
||||
|
||||
s.log.Info("report received",
|
||||
"client_id", rpt.ClientID,
|
||||
"timestamp", rpt.Timestamp,
|
||||
"host_count", len(rpt.Hosts),
|
||||
"total_samples", totalSamples,
|
||||
"geo", string(rpt.Geo),
|
||||
)
|
||||
|
||||
bufErr := s.buf.Append(rpt)
|
||||
if bufErr != nil {
|
||||
s.log.Error("failed to buffer report",
|
||||
"error", bufErr,
|
||||
err = s.buf.Append(rpt)
|
||||
if err != nil {
|
||||
s.log.Error("failed to buffer report", "error", err)
|
||||
s.respondJSON(w, r,
|
||||
&response{Status: "error"},
|
||||
http.StatusInternalServerError,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
s.respondJSON(w, r,
|
||||
&response{Status: "ok"},
|
||||
http.StatusOK,
|
||||
)
|
||||
s.respondJSON(w, r, &response{Status: "ok"}, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
// decodeErrorStatus logs a report decode failure and returns the
|
||||
// status to send: 413 when the body exceeded the size limit,
|
||||
// otherwise 400 for malformed JSON.
|
||||
func (s *Handlers) decodeErrorStatus(err error) int {
|
||||
var tooLarge *http.MaxBytesError
|
||||
if errors.As(err, &tooLarge) {
|
||||
s.log.Warn("report body too large", "limit_bytes", tooLarge.Limit)
|
||||
|
||||
return http.StatusRequestEntityTooLarge
|
||||
}
|
||||
|
||||
s.log.Error("failed to decode report", "error", err)
|
||||
|
||||
return http.StatusBadRequest
|
||||
}
|
||||
|
||||
// logReportReceived logs an accepted report. Untrusted fields are
|
||||
// bounded (client_id, timestamp) or reduced to a length
|
||||
// (geo_bytes) so the raw attacker-controlled body never reaches
|
||||
// the log.
|
||||
func (s *Handlers) logReportReceived(rpt report) {
|
||||
totalSamples := 0
|
||||
for _, h := range rpt.Hosts {
|
||||
totalSamples += len(h.History)
|
||||
}
|
||||
|
||||
s.log.Info("report received",
|
||||
"client_id", boundedForLog(rpt.ClientID),
|
||||
"timestamp", boundedForLog(rpt.Timestamp),
|
||||
"host_count", len(rpt.Hosts),
|
||||
"total_samples", totalSamples,
|
||||
"geo_bytes", len(rpt.Geo),
|
||||
)
|
||||
}
|
||||
|
||||
// boundedForLog truncates an untrusted string to a fixed byte
|
||||
// bound so an attacker-controlled field cannot dominate the log.
|
||||
func boundedForLog(s string) string {
|
||||
if len(s) > maxLoggedFieldBytes {
|
||||
return s[:maxLoggedFieldBytes]
|
||||
}
|
||||
|
||||
return s
|
||||
}
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
package handlers_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/netwatch/internal/handlers"
|
||||
)
|
||||
|
||||
var errStorageFailed = errors.New("storage failed")
|
||||
|
||||
// stubAppender drives the storage success/failure path without a
|
||||
// real buffer or disk.
|
||||
type stubAppender struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (s stubAppender) Append(any) error { return s.err }
|
||||
|
||||
func newTestHandlers(buf stubAppender, out io.Writer) *handlers.Handlers {
|
||||
return handlers.NewForTest(buf, slog.New(slog.NewJSONHandler(out, nil)))
|
||||
}
|
||||
|
||||
func decodeStatus(t *testing.T, body []byte) string {
|
||||
t.Helper()
|
||||
|
||||
var resp struct {
|
||||
Status string `json:"status"`
|
||||
}
|
||||
|
||||
err := json.Unmarshal(body, &resp)
|
||||
if err != nil {
|
||||
t.Fatalf("response body not JSON: %v (%q)", err, body)
|
||||
}
|
||||
|
||||
return resp.Status
|
||||
}
|
||||
|
||||
func TestHandleReportStorageFailureIsNon2xx(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
h := newTestHandlers(stubAppender{err: errStorageFailed}, io.Discard)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{"clientId":"c1","hosts":[]}`),
|
||||
)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code < 500 {
|
||||
t.Fatalf("storage failure status = %d, want a 5xx", rec.Code)
|
||||
}
|
||||
|
||||
if got := decodeStatus(t, rec.Body.Bytes()); got != "error" {
|
||||
t.Fatalf("status field = %q, want %q", got, "error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleReportMalformedJSONIs400(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
h := newTestHandlers(stubAppender{}, io.Discard)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{not json`),
|
||||
)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("malformed status = %d, want %d",
|
||||
rec.Code, http.StatusBadRequest)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleReportOversizeIs413(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 32
|
||||
|
||||
h := newTestHandlers(stubAppender{}, io.Discard)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{"clientId":"`+strings.Repeat("x", 200)+`"}`),
|
||||
)
|
||||
// Emulate the body-size middleware capping the body so the
|
||||
// handler observes a *http.MaxBytesError while decoding.
|
||||
req.Body = http.MaxBytesReader(rec, req.Body, limit)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusRequestEntityTooLarge {
|
||||
t.Fatalf("oversize status = %d, want %d",
|
||||
rec.Code, http.StatusRequestEntityTooLarge)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleReportDoesNotLogRawGeo(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const sentinel = "SENSITIVE-GEO-BLOB"
|
||||
|
||||
var logbuf bytes.Buffer
|
||||
|
||||
h := newTestHandlers(stubAppender{}, &logbuf)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(
|
||||
`{"clientId":"c1","geo":{"raw":"`+sentinel+`"},"hosts":[]}`,
|
||||
),
|
||||
)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
|
||||
}
|
||||
|
||||
if strings.Contains(logbuf.String(), sentinel) {
|
||||
t.Fatal("raw geo bytes were written to the log")
|
||||
}
|
||||
|
||||
if !strings.Contains(logbuf.String(), "geo_bytes") {
|
||||
t.Fatal("expected a bounded geo_bytes field in the log")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user