2 Commits
Author SHA1 Message Date
clawbot e683ac3b61 fix(backend): report ingest correctness — propagate storage failure, 413 on oversize, global body cap (closes #23)
check / check (push) Failing after 34s
A buffer failure on POST /api/v1/reports now returns 500 instead of a
false `ok`, so clients can retry. An over-limit body returns 413
(errors.As on `*http.MaxBytesError`); malformed JSON stays 400. A
MaxBodyBytes middleware (1 MiB) caps every route, rejecting an
oversized Content-Length up front and capping the read otherwise; a
route group can only lower that limit. The raw geo blob is no longer
logged, only its length; client_id, timestamp and decode error text
are length-bounded before logging. A decodeJSON handler helper is
added. Panic recovery routes the stack through slog as structured
JSON. Storage failure uses 500: a full buffer or write error is
server-side and retryable.

Model: opus-5-5
2026-09-28 17:27:43 +00:00
clawbot 7a1ee6e5a8 lint: adopt org-standard .golangci.yml and golangci-lint v2.12.2 (closes #14)
check / check (push) Failing after 1s
The old backend/.golangci.yml declared version "2" but used v1 schema
keys, so under v2 it never validated and its thresholds were inert: the
linter ran at defaults. Replace it verbatim with the org-standard file,
repin the Dockerfile.backend lint stage to golangci-lint v2.12.2, and
assert the config's sha256 as the first step of the backend lint target
so it cannot silently drift again -- a local hash check, no network.

The standard config surfaces findings only in the tests: the repeated
IP literals in middleware_test.go become named constants (goconst) and
its request switches to NewRequestWithContext (noctx). reportbuf.go's
gosec suppression gains a plain justification comment. The rest of the
backend, including the fx-based server lifecycle, is already clean.
TODO.md updated.

Model: opus-4-8
2026-09-21 19:30:06 +02:00
15 changed files with 668 additions and 67 deletions
+2 -2
View File
@@ -1,8 +1,8 @@
# Lint stage — fast feedback on formatting and lint issues. The # Lint stage — fast feedback on formatting and lint issues. The
# golangci/golangci-lint image ships Go, gofmt, make and the linter, so # golangci/golangci-lint image ships Go, gofmt, make and the linter, so
# nothing is installed here. # nothing is installed here.
# golangci/golangci-lint:v2.7.2 (2026-08-09) # golangci/golangci-lint:v2.12.2 (2026-08-10)
FROM golangci/golangci-lint@sha256:5d6d5c70a61f1356adfd9dd6316ce286799fefc9d743421356ff1b00842368ba AS lint FROM golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS lint
WORKDIR /src WORKDIR /src
COPY backend/go.mod backend/go.sum ./ COPY backend/go.mod backend/go.sum ./
+30 -7
View File
@@ -10,18 +10,27 @@
# Status # Status
pre-1.0. No git tags. Backend work in flight on feat/reportbuf-storage (dirty: pre-1.0. No git tags. `feat/reportbuf-storage` is merged; the backend, the CI
src/main.js). Frontend is functional; backend is new and unmerged. workflow, and the backend repo standard files are all on `main`. Frontend and
backend are both functional. Working toward the 1.0.0 milestone by closing the
remaining repo-compliance issues on the tracker.
# Next Step # Next Step
Land feat/reportbuf-storage: finish the in-progress src/main.js change, get make Confirm the `.gitea/workflows/check.yml` run is green (main always green
check green, and merge the branch to main. The branch adds the backend (buffered policy). The workflow file is already on `main`; what is unverified is that its
zstd-compressed report storage), the CI workflow, and backend repo standard latest run passes.
files, so merging it also closes most compliance gaps.
# Completed Steps # Completed Steps
- 2026-09-28: report ingest correctness (issue #23): a storage failure now
returns 500 instead of a false `ok`; oversize bodies return 413 (distinguished
from malformed JSON, which stays 400); a `MaxBodyBytes` middleware caps every
route, not just the report route; the raw attacker-controlled `geo` blob is no
longer logged (only its length), and `client_id`, `timestamp` and decode error
text are length-bounded before logging; a `decodeJSON` handler helper was
added; and panic recovery now routes the stack through slog instead of chi's
plain-text stderr
- 2026-09-21: shutdown lifecycle correctness. The process now shuts down through - 2026-09-21: shutdown lifecycle correctness. The process now shuts down through
fx instead of `os.Exit`, so every component's `OnStop` runs and buffered fx instead of `os.Exit`, so every component's `OnStop` runs and buffered
reports are flushed to disk on `SIGTERM` — previously a full flush window of reports are flushed to disk on `SIGTERM` — previously a full flush window of
@@ -36,6 +45,14 @@ files, so merging it also closes most compliance gaps.
frame/sniff/referrer/permissions headers) registered before CORS, and frame/sniff/referrer/permissions headers) registered before CORS, and
trusted-proxy client IP resolution honouring `X-Forwarded-For` / `X-Real-IP` trusted-proxy client IP resolution honouring `X-Forwarded-For` / `X-Real-IP`
only from a `TRUSTED_PROXIES` allowlist (loopback plus RFC1918 by default) only from a `TRUSTED_PROXIES` allowlist (loopback plus RFC1918 by default)
- 2026-08-10: adopted the org-standard `backend/.golangci.yml` verbatim and
moved the pinned golangci-lint from v2.7.2 to v2.12.2 (the `lint` stage of
`Dockerfile.backend` now pins the `golangci/golangci-lint:v2.12.2` image by
digest); the previous config declared `version: "2"` but used v1 schema keys,
so every threshold in it was inert and its green result was meaningless.
`backend/Makefile`'s `lint` target now asserts the config's sha256 against the
canonical file first, so drift from the org standard fails the build instead
of silently degrading to defaults
- 2026-08-10: every interactive control now meets the 44x44 CSS px minimum tap - 2026-08-10: every interactive control now meets the 44x44 CSS px minimum tap
target (`.pin-btn`, `#interval-select`, the debug-log label and, on narrow target (`.pin-btn`, `#interval-select`, the debug-log label and, on narrow
viewports, `#pause-btn`). The pin button's hit area grows via matching viewports, `#pause-btn`). The pin button's hit area grows via matching
@@ -65,7 +82,7 @@ files, so merging it also closes most compliance gaps.
shims, README Entrypoints section shims, README Entrypoints section
- 2026-02-27: backend with buffered zstd-compressed report storage; CI workflow - 2026-02-27: backend with buffered zstd-compressed report storage; CI workflow
and backend repo standard files; backend Dockerfile fixed (Go 1.25, and backend repo standard files; backend Dockerfile fixed (Go 1.25,
golangci-lint) and moved to repo root (feat/reportbuf-storage, unmerged) golangci-lint) and moved to repo root (feat/reportbuf-storage)
- 2026-02-26: host row layout redesigned with CSS grid; overflow and spacing - 2026-02-26: host row layout redesigned with CSS grid; overflow and spacing
fixes; nginx config extracted; port hardcoded to 8080 fixes; nginx config extracted; port hardcoded to 8080
- 2026-02-26: debug log panel, median stats, recovery probe, Docker build fix, - 2026-02-26: debug log panel, median stats, recovery probe, Docker build fix,
@@ -87,3 +104,9 @@ files, so merging it also closes most compliance gaps.
(main always green policy) (main always green policy)
- Decide what to do with untracked resume.sh: commit it, gitignore it, or delete - Decide what to do with untracked resume.sh: commit it, gitignore it, or delete
it it
- Upstream fix needed in `sneak/prompts`: the org-standard `.golangci.yml`
enables `gomodguard`, which golangci-lint v2.12.2 reports as deprecated since
v2.12.0 and replaced by `gomodguard_v2`, so every backend lint run prints a
deprecation warning. The file is standardized and must never be edited in this
repo, so nothing can be done here beyond tracking it — tracked at
<https://git.eeqj.de/sneak/netwatch/issues/41>
+14 -12
View File
@@ -1,5 +1,9 @@
version: "2" version: "2"
# Config schema uses the golangci-lint v2 layout (settings live under
# linters.settings, not top-level linters-settings) so that the
# thresholds below are actually applied by golangci-lint >= v2.
run: run:
timeout: 5m timeout: 5m
modules-download-mode: readonly modules-download-mode: readonly
@@ -14,19 +18,17 @@ linters:
- wsl # Deprecated, replaced by wsl_v5 - wsl # Deprecated, replaced by wsl_v5
- wrapcheck # Too verbose for internal packages - wrapcheck # Too verbose for internal packages
- varnamelen # Short names like db, id are idiomatic Go - varnamelen # Short names like db, id are idiomatic Go
settings:
linters-settings: lll:
lll: line-length: 88
line-length: 88 funlen:
funlen: lines: 80
lines: 80 statements: 50
statements: 50 cyclop:
cyclop: max-complexity: 15
max-complexity: 15 dupl:
dupl: threshold: 100
threshold: 100
issues: issues:
exclude-use-default: false
max-issues-per-linter: 0 max-issues-per-linter: 0
max-same-issues: 0 max-same-issues: 0
+19
View File
@@ -10,6 +10,17 @@ GOLDFLAGS += -s -w
GOLDFLAGS += -X main.Version=$(VERSION) GOLDFLAGS += -X main.Version=$(VERSION)
GOLDFLAGS += -X main.Buildarch=$(BUILDARCH) GOLDFLAGS += -X main.Buildarch=$(BUILDARCH)
# macOS ships shasum rather than sha256sum.
SHA256SUM := $(shell command -v sha256sum >/dev/null 2>&1 && echo sha256sum || echo shasum -a 256)
# .golangci.yml is standardized org-wide and must never be edited here
# (REPO_POLICIES.md). Its last silent drift replaced the v2 schema with
# v1 keys, which left every threshold in the file inert while the build
# stayed green. The lint target therefore asserts the file still matches
# the canonical copy byte for byte. The check is a local hash comparison:
# no network, no remote schema, nothing unpinned in the build path.
GOLANGCI_CONFIG_SHA256 := 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb
.PHONY: all build test lint fmt fmt-check check docker hooks run clean .PHONY: all build test lint fmt fmt-check check docker hooks run clean
all: build all: build
@@ -22,6 +33,14 @@ test:
timeout 30 go test ./... timeout 30 go test ./...
lint: lint:
@actual=$$($(SHA256SUM) .golangci.yml | cut -d' ' -f1); \
if [ "$$actual" != "$(GOLANGCI_CONFIG_SHA256)" ]; then \
echo ".golangci.yml has drifted from the org standard."; \
echo " expected $(GOLANGCI_CONFIG_SHA256)"; \
echo " actual $$actual"; \
echo "Restore it verbatim from sneak/prompts; do not edit it."; \
exit 1; \
fi
golangci-lint run ./... golangci-lint run ./...
fmt: fmt:
+13
View File
@@ -0,0 +1,13 @@
package handlers
import "log/slog"
// MaxLoggedFieldBytes exposes the log bound to the external tests.
const MaxLoggedFieldBytes = maxLoggedFieldBytes
// 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}
}
+20 -1
View File
@@ -18,6 +18,13 @@ import (
const jsonContentType = "application/json; charset=utf-8" 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. // Params defines the dependencies for Handlers.
type Params struct { type Params struct {
fx.In fx.In
@@ -30,7 +37,7 @@ type Params struct {
// Handlers provides HTTP handler factories for all endpoints. // Handlers provides HTTP handler factories for all endpoints.
type Handlers struct { type Handlers struct {
buf *reportbuf.Buffer buf reportAppender
hc *healthcheck.Healthcheck hc *healthcheck.Healthcheck
log *slog.Logger log *slog.Logger
params *Params 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)
}
+66 -30
View File
@@ -2,10 +2,14 @@ package handlers
import ( import (
"encoding/json" "encoding/json"
"errors"
"net/http" "net/http"
) )
const maxReportBodyBytes = 1 << 20 // 1 MiB // maxLoggedFieldBytes bounds untrusted text (string fields,
// decode error text) before it is logged, so a caller cannot
// inflate log volume with an oversized value.
const maxLoggedFieldBytes = 128
type reportSample struct { type reportSample struct {
T int64 `json:"t"` T int64 `json:"t"`
@@ -35,48 +39,80 @@ func (s *Handlers) HandleReport() http.HandlerFunc {
} }
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
r.Body = http.MaxBytesReader(
w, r.Body, maxReportBodyBytes,
)
var rpt report var rpt report
err := json.NewDecoder(r.Body).Decode(&rpt) err := s.decodeJSON(w, r, &rpt)
if err != nil { if err != nil {
s.log.Error("failed to decode report",
"error", err,
)
s.respondJSON(w, r, s.respondJSON(w, r,
&response{Status: "error"}, &response{Status: "error"},
http.StatusBadRequest, s.decodeErrorStatus(err),
) )
return return
} }
totalSamples := 0 s.logReportReceived(rpt)
for _, h := range rpt.Hosts {
totalSamples += len(h.History)
}
s.log.Info("report received", err = s.buf.Append(rpt)
"client_id", rpt.ClientID, if err != nil {
"timestamp", rpt.Timestamp, s.log.Error("failed to buffer report", "error", err)
"host_count", len(rpt.Hosts), s.respondJSON(w, r,
"total_samples", totalSamples, &response{Status: "error"},
"geo", string(rpt.Geo), http.StatusInternalServerError,
)
bufErr := s.buf.Append(rpt)
if bufErr != nil {
s.log.Error("failed to buffer report",
"error", bufErr,
) )
return
} }
s.respondJSON(w, r, s.respondJSON(w, r, &response{Status: "ok"}, http.StatusOK)
&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
}
// The decoder's error text can quote request bytes (a whole
// oversized number, for example), so it is bounded too.
s.log.Error("failed to decode report",
"error", boundedForLog(err.Error()),
)
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
}
+215
View File
@@ -0,0 +1,215 @@
package handlers_test
import (
"bytes"
"encoding/json"
"errors"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"sneak.berlin/go/netwatch/internal/handlers"
"sneak.berlin/go/netwatch/internal/middleware"
)
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.NewRequestWithContext(t.Context(),
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.NewRequestWithContext(t.Context(),
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)
handler := (&middleware.Middleware{}).MaxBodyBytes(limit)(
h.HandleReport(),
)
rec := httptest.NewRecorder()
req := httptest.NewRequestWithContext(t.Context(),
http.MethodPost, "/api/v1/reports",
strings.NewReader(`{"clientId":"`+strings.Repeat("x", 200)+`"}`),
)
// No declared length, so only the middleware's read cap can
// stop this body.
req.ContentLength = -1
handler.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.NewRequestWithContext(t.Context(),
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")
}
}
func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
t.Parallel()
long := strings.Repeat("c", 2*handlers.MaxLoggedFieldBytes)
var logbuf bytes.Buffer
h := newTestHandlers(stubAppender{}, &logbuf)
rec := httptest.NewRecorder()
req := httptest.NewRequestWithContext(t.Context(),
http.MethodPost, "/api/v1/reports",
strings.NewReader(
`{"clientId":"`+long+`","timestamp":"`+long+`","hosts":[]}`,
),
)
h.HandleReport().ServeHTTP(rec, req)
var logged struct {
ClientID string `json:"client_id"`
Timestamp string `json:"timestamp"`
}
err := json.Unmarshal(logbuf.Bytes(), &logged)
if err != nil {
t.Fatalf("log line not JSON: %v (%q)", err, logbuf.String())
}
want := long[:handlers.MaxLoggedFieldBytes]
if logged.ClientID != want {
t.Fatalf("logged client_id is %d bytes, want %d",
len(logged.ClientID), len(want))
}
if logged.Timestamp != want {
t.Fatalf("logged timestamp is %d bytes, want %d",
len(logged.Timestamp), len(want))
}
}
func TestHandleReportDecodeErrorLogIsBounded(t *testing.T) {
t.Parallel()
// A number too large for its int64 field makes the decoder's
// error text quote the whole number.
huge := strings.Repeat("9", 2*handlers.MaxLoggedFieldBytes)
var logbuf bytes.Buffer
h := newTestHandlers(stubAppender{}, &logbuf)
rec := httptest.NewRecorder()
req := httptest.NewRequestWithContext(t.Context(),
http.MethodPost, "/api/v1/reports",
strings.NewReader(`{"hosts":[{"history":[{"t":`+huge+`}]}]}`),
)
h.HandleReport().ServeHTTP(rec, req)
if rec.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if strings.Contains(logbuf.String(), huge) {
t.Fatal("the whole oversized number was written to the log")
}
}
@@ -1,6 +1,7 @@
package middleware package middleware
import ( import (
"log/slog"
"net/http" "net/http"
"net/netip" "net/netip"
) )
@@ -8,6 +9,12 @@ import (
// Test-only wrappers exposing unexported helpers to the // Test-only wrappers exposing unexported helpers to the
// external middleware_test package. // 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( func ClientIP(
remoteAddr string, remoteAddr string,
header http.Header, header http.Header,
+84
View File
@@ -3,11 +3,14 @@
package middleware package middleware
import ( import (
"errors"
"fmt" "fmt"
"io"
"log/slog" "log/slog"
"net" "net"
"net/http" "net/http"
"net/netip" "net/netip"
"runtime/debug"
"strings" "strings"
"time" "time"
@@ -22,6 +25,14 @@ import (
const corsMaxAgeSec = 300 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 // Security header values. The backend is a JSON API with no
// HTML surface, so the CSP forbids every resource type and // HTML surface, so the CSP forbids every resource type and
// framing outright. // 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. Mounted again on a route group, it can only lower
// the limit: a cap applied earlier in the chain still holds.
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. // CORS returns middleware that adds permissive CORS headers.
func (s *Middleware) CORS() func(http.Handler) http.Handler { func (s *Middleware) CORS() func(http.Handler) http.Handler {
return cors.Handler(cors.Options{ return cors.Handler(cors.Options{
+113 -12
View File
@@ -1,14 +1,26 @@
package middleware_test package middleware_test
import ( import (
"bytes"
"log/slog"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/netip" "net/netip"
"strings"
"testing" "testing"
"sneak.berlin/go/netwatch/internal/middleware" "sneak.berlin/go/netwatch/internal/middleware"
) )
const (
// loopbackPeer is a remote address inside the trusted-proxy allowlist.
loopbackPeer = "127.0.0.1:5000"
// forwardedIP is the client address presented via X-Forwarded-For.
forwardedIP = "203.0.113.7"
// realIP is the client address presented via X-Real-IP.
realIP = "203.0.113.9"
)
func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix { func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
t.Helper() t.Helper()
@@ -41,32 +53,32 @@ func clientIPCases() []clientIPCase {
return []clientIPCase{ return []clientIPCase{
{ {
name: "trusted proxy uses forwarded-for", name: "trusted proxy uses forwarded-for",
remoteAddr: "127.0.0.1:5000", remoteAddr: loopbackPeer,
xff: "203.0.113.7", xff: forwardedIP,
want: "203.0.113.7", want: forwardedIP,
}, },
{ {
name: "trusted proxy uses left-most of chain", name: "trusted proxy uses left-most of chain",
remoteAddr: "10.1.2.3:5000", remoteAddr: "10.1.2.3:5000",
xff: "203.0.113.7, 10.1.2.3", xff: forwardedIP + ", 10.1.2.3",
want: "203.0.113.7", want: forwardedIP,
}, },
{ {
name: "trusted proxy falls back to x-real-ip", name: "trusted proxy falls back to x-real-ip",
remoteAddr: "127.0.0.1:5000", remoteAddr: loopbackPeer,
xRealIP: "203.0.113.9", xRealIP: realIP,
want: "203.0.113.9", want: realIP,
}, },
{ {
name: "untrusted peer ignores forwarded-for", name: "untrusted peer ignores forwarded-for",
remoteAddr: "198.51.100.4:5000", remoteAddr: "198.51.100.4:5000",
xff: "203.0.113.7", xff: forwardedIP,
want: "198.51.100.4", want: "198.51.100.4",
}, },
{ {
name: "untrusted peer ignores x-real-ip", name: "untrusted peer ignores x-real-ip",
remoteAddr: "198.51.100.4:5000", remoteAddr: "198.51.100.4:5000",
xRealIP: "203.0.113.9", xRealIP: realIP,
want: "198.51.100.4", want: "198.51.100.4",
}, },
{ {
@@ -76,7 +88,7 @@ func clientIPCases() []clientIPCase {
}, },
{ {
name: "trusted proxy with garbage header uses peer", name: "trusted proxy with garbage header uses peer",
remoteAddr: "127.0.0.1:5000", remoteAddr: loopbackPeer,
xff: "not-an-ip", xff: "not-an-ip",
want: "127.0.0.1", want: "127.0.0.1",
}, },
@@ -119,7 +131,7 @@ func TestSecurityHeaders(t *testing.T) {
) )
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, "/", http.NoBody) req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", http.NoBody)
handler.ServeHTTP(rec, req) handler.ServeHTTP(rec, req)
want := map[string]string{ want := map[string]string{
@@ -137,3 +149,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.NewRequestWithContext(t.Context(),
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.NewRequestWithContext(t.Context(),
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.NewRequestWithContext(t.Context(), 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)
}
}
+3 -1
View File
@@ -169,7 +169,9 @@ func (b *Buffer) writeFile(data []byte) {
name := fmt.Sprintf("reports-%s.jsonl.zst", ts) name := fmt.Sprintf("reports-%s.jsonl.zst", ts)
path := filepath.Join(b.dataDir, name) path := filepath.Join(b.dataDir, name)
f, err := os.OpenFile( //nolint:gosec // path built from controlled dataDir + timestamp // path is built from the operator-supplied dataDir plus a
// generated timestamp, so it carries no external input.
f, err := os.OpenFile( //nolint:gosec // see comment above
path, path,
os.O_WRONLY|os.O_CREATE|os.O_EXCL, os.O_WRONLY|os.O_CREATE|os.O_EXCL,
filePerms, filePerms,
+5
View File
@@ -0,0 +1,5 @@
package server
// MaxRequestBodyBytes exposes the router-wide body limit to the
// external tests.
const MaxRequestBodyBytes = maxRequestBodyBytes
+10 -2
View File
@@ -7,18 +7,26 @@ import (
"github.com/go-chi/chi/v5/middleware" "github.com/go-chi/chi/v5/middleware"
) )
const requestTimeout = 60 * time.Second const (
requestTimeout = 60 * time.Second
// maxRequestBodyBytes caps every request body. A route group
// can mount s.mw.MaxBodyBytes with a smaller value to lower
// its bound, but cannot raise it: this cap runs first.
maxRequestBodyBytes int64 = 1 << 20 // 1 MiB
)
// SetupRoutes configures the chi router with middleware and // SetupRoutes configures the chi router with middleware and
// all application routes. // all application routes.
func (s *Server) SetupRoutes() { func (s *Server) SetupRoutes() {
s.router = chi.NewRouter() s.router = chi.NewRouter()
s.router.Use(middleware.Recoverer) s.router.Use(s.mw.Recoverer())
s.router.Use(middleware.RequestID) s.router.Use(middleware.RequestID)
s.router.Use(s.mw.Logging()) s.router.Use(s.mw.Logging())
s.router.Use(s.mw.SecurityHeaders()) s.router.Use(s.mw.SecurityHeaders())
s.router.Use(s.mw.CORS()) s.router.Use(s.mw.CORS())
s.router.Use(s.mw.MaxBodyBytes(maxRequestBodyBytes))
s.router.Use(middleware.Timeout(requestTimeout)) s.router.Use(middleware.Timeout(requestTimeout))
s.router.Get( s.router.Get(
+67
View File
@@ -0,0 +1,67 @@
package server_test
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"sneak.berlin/go/netwatch/internal/config"
"sneak.berlin/go/netwatch/internal/globals"
"sneak.berlin/go/netwatch/internal/handlers"
"sneak.berlin/go/netwatch/internal/healthcheck"
"sneak.berlin/go/netwatch/internal/logger"
"sneak.berlin/go/netwatch/internal/middleware"
"sneak.berlin/go/netwatch/internal/reportbuf"
"sneak.berlin/go/netwatch/internal/server"
"go.uber.org/fx"
"go.uber.org/fx/fxtest"
)
// TestHealthCheckRejectsOversizeBody sends the health check, which
// never reads its body, a body one byte over the limit. Only the
// router-wide body limit can reject it.
func TestHealthCheckRejectsOversizeBody(t *testing.T) {
t.Parallel()
var srv *server.Server
// The same constructors as main, never started: SetupRoutes is
// called directly, so nothing listens.
app := fxtest.New(t,
fx.Provide(
config.New,
globals.New,
handlers.New,
healthcheck.New,
logger.New,
middleware.New,
reportbuf.New,
server.New,
),
fx.Populate(&srv),
)
err := app.Err()
if err != nil {
t.Fatalf("build server: %v", err)
}
srv.SetupRoutes()
rec := httptest.NewRecorder()
req := httptest.NewRequestWithContext(t.Context(),
http.MethodGet, "/.well-known/healthcheck",
strings.NewReader(
strings.Repeat("x", int(server.MaxRequestBodyBytes)+1),
),
)
srv.ServeHTTP(rec, req)
if rec.Code != http.StatusRequestEntityTooLarge {
t.Fatalf("status = %d, want %d",
rec.Code, http.StatusRequestEntityTooLarge)
}
}