From 13a74265a8fd77f15605c65c8a3de26678ae3b39 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 09:24:31 +0000 Subject: [PATCH] Reports are taken only as application/json (closes #126) A page on another origin can POST text/plain or a form without a CORS preflight, so its visitors' browsers could store reports although the server sends no CORS headers. decodeJSON now refuses a request whose Content-Type is not application/json (parameters such as charset allowed), and the report handler answers it 415 with the usual error body, storing nothing. A JSON POST waits for the preflight, which only an origin in CORS_ALLOWED_ORIGINS passes; the page already sends application/json. Model: opus-5-5 --- TODO.md | 10 +++ backend/README.md | 8 ++- backend/internal/handlers/handlers.go | 25 +++++-- backend/internal/handlers/report.go | 12 +++- backend/internal/handlers/report_test.go | 91 ++++++++++++++++++++++++ backend/internal/server/routes_test.go | 1 + 6 files changed, 139 insertions(+), 8 deletions(-) diff --git a/TODO.md b/TODO.md index 53f31f5..ac60144 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,16 @@ Decide whether the repo moves to the layout `REPO_POLICIES.md` gives, with # Completed Steps +- 2026-10-14: `POST /api/v1/reports` takes only a report sent with + `Content-Type: application/json`, parameters such as `charset` allowed, and + refuses any other with 415 and the usual error body, storing nothing + ([#126](https://git.eeqj.de/sneak/netwatch/issues/126)). A page on another + origin can send a `text/plain` POST without a CORS preflight, so any site + could make its visitors' browsers store reports; a JSON POST waits for the + preflight, which only an origin in `CORS_ALLOWED_ORIGINS` passes. The page + already sent `application/json`. A test posts a report as `text/plain`, with + no `Content-Type`, and as `application/json` with and without a charset, and + checks which are stored - 2026-10-07: the files shared from `sneak/prompts` are its copies at commit `dd4027b` ([#113](https://git.eeqj.de/sneak/netwatch/issues/113)), with this repository's own entries after the shared content in `.gitignore`, diff --git a/backend/README.md b/backend/README.md index c6844b8..49bdec4 100644 --- a/backend/README.md +++ b/backend/README.md @@ -143,9 +143,15 @@ created with `O_EXCL` to prevent overwrites. ### Report limits `POST /api/v1/reports` takes reports from anyone who can reach it, without -credentials, so it is bounded instead. Both refusals below answer with the same +credentials, so it is bounded instead. Each refusal below answers with the same `{"status":"error"}` body as any other error. +- **JSON only.** A report must be sent with `Content-Type: application/json`, + parameters such as `charset` allowed, as the page sends it; any other is + refused with 415, and nothing of it is stored. A page on another origin can + send a `text/plain` POST, or a form, without a CORS preflight; refusing them + keeps any site from making its visitors' browsers send reports (see + [CORS](#cors)). - **Rate limit.** Each client address, resolved through `TRUSTED_PROXIES`, may send `REPORTS_PER_MINUTE` reports a minute; past that it gets 429 with `Retry-After: 60`. The minute slides: reports from the minute before still diff --git a/backend/internal/handlers/handlers.go b/backend/internal/handlers/handlers.go index db51059..f1b5432 100644 --- a/backend/internal/handlers/handlers.go +++ b/backend/internal/handlers/handlers.go @@ -5,7 +5,9 @@ package handlers import ( "context" "encoding/json" + "errors" "log/slog" + "mime" "net/http" "sneak.berlin/go/netwatch/internal/globals" @@ -18,6 +20,10 @@ import ( const jsonContentType = "application/json; charset=utf-8" +// errNotJSON is what decodeJSON returns for a request whose +// Content-Type is not application/json. +var errNotJSON = errors.New("content type is not application/json") + // 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. @@ -80,14 +86,25 @@ 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. +// decodeJSON decodes the request body into v. It returns errNotJSON, +// reading nothing, unless the request's Content-Type is +// application/json, with or without parameters such as charset. 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 { + // A page on another origin can POST text/plain or a form without + // a CORS preflight, so its visitors' browsers would send it + // whatever the CORS settings. A JSON POST waits for the + // preflight, so only JSON is taken. + mediaType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type")) + if err != nil || mediaType != "application/json" { + return errNotJSON + } + return json.NewDecoder(r.Body).Decode(v) } diff --git a/backend/internal/handlers/report.go b/backend/internal/handlers/report.go index 8aa5ad9..8656cfe 100644 --- a/backend/internal/handlers/report.go +++ b/backend/internal/handlers/report.go @@ -30,7 +30,7 @@ type report struct { } // HandleReport returns a handler that accepts telemetry -// reports from NetWatch clients. +// reports from NetWatch clients, sent as application/json. func (s *Handlers) HandleReport() http.HandlerFunc { type response struct { Status string `json:"status"` @@ -66,9 +66,15 @@ func (s *Handlers) HandleReport() http.HandlerFunc { } // 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. +// status to send: 415 when the report was not sent as JSON, 413 when +// the body exceeded the size limit, otherwise 400 for malformed JSON. func (s *Handlers) decodeErrorStatus(err error) int { + if errors.Is(err, errNotJSON) { + s.log.Warn("report refused: Content-Type is not application/json") + + return http.StatusUnsupportedMediaType + } + var tooLarge *http.MaxBytesError if errors.As(err, &tooLarge) { s.log.Warn("report body too large", "limit_bytes", tooLarge.Limit) diff --git a/backend/internal/handlers/report_test.go b/backend/internal/handlers/report_test.go index bd6345c..dfa4755 100644 --- a/backend/internal/handlers/report_test.go +++ b/backend/internal/handlers/report_test.go @@ -27,6 +27,17 @@ type stubAppender struct { func (s stubAppender) Append(any) error { return s.err } +// recordingAppender keeps every report the handler stores. +type recordingAppender struct { + reports []any +} + +func (a *recordingAppender) Append(v any) error { + a.reports = append(a.reports, v) + + return nil +} + func newTestHandlers(buf stubAppender, out io.Writer) *handlers.Handlers { return handlers.NewForTest(buf, slog.New(slog.NewJSONHandler(out, nil))) } @@ -83,6 +94,7 @@ func TestHandleReportAcceptsValidReports(t *testing.T) { http.MethodPost, "/api/v1/reports", strings.NewReader(tt.body), ) + req.Header.Set("Content-Type", "application/json") h.HandleReport().ServeHTTP(rec, req) @@ -103,6 +115,77 @@ func TestHandleReportAcceptsValidReports(t *testing.T) { } } +// TestHandleReportStoresOnlyJSON checks that a report is stored only +// when sent as application/json, as the page sends it. One sent as +// text/plain, which a page on another origin can send without a CORS +// preflight, is refused with 415. +func TestHandleReportStoresOnlyJSON(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + contentType string + wantStatus int + wantStored int + }{ + { + name: "text/plain", + contentType: "text/plain;charset=UTF-8", + wantStatus: http.StatusUnsupportedMediaType, + wantStored: 0, + }, + { + name: "no Content-Type", + contentType: "", + wantStatus: http.StatusUnsupportedMediaType, + wantStored: 0, + }, + { + name: "application/json", + contentType: "application/json", + wantStatus: http.StatusOK, + wantStored: 1, + }, + { + name: "application/json with a charset", + contentType: "application/json;charset=UTF-8", + wantStatus: http.StatusOK, + wantStored: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var stored recordingAppender + + h := handlers.NewForTest(&stored, slog.New(slog.DiscardHandler)) + + rec := httptest.NewRecorder() + req := httptest.NewRequestWithContext(t.Context(), + http.MethodPost, "/api/v1/reports", + strings.NewReader(`{"clientId":"c1","hosts":[]}`), + ) + + if tt.contentType != "" { + req.Header.Set("Content-Type", tt.contentType) + } + + h.HandleReport().ServeHTTP(rec, req) + + if rec.Code != tt.wantStatus { + t.Errorf("status = %d, want %d", rec.Code, tt.wantStatus) + } + + if len(stored.reports) != tt.wantStored { + t.Errorf("reports stored = %d, want %d", + len(stored.reports), tt.wantStored) + } + }) + } +} + func TestHandleReportStorageFailureIsNon2xx(t *testing.T) { t.Parallel() @@ -113,6 +196,7 @@ func TestHandleReportStorageFailureIsNon2xx(t *testing.T) { http.MethodPost, "/api/v1/reports", strings.NewReader(`{"clientId":"c1","hosts":[]}`), ) + req.Header.Set("Content-Type", "application/json") h.HandleReport().ServeHTTP(rec, req) @@ -138,6 +222,7 @@ func TestHandleReportFullIs507(t *testing.T) { http.MethodPost, "/api/v1/reports", strings.NewReader(`{"clientId":"c1","hosts":[]}`), ) + req.Header.Set("Content-Type", "application/json") h.HandleReport().ServeHTTP(rec, req) @@ -161,6 +246,7 @@ func TestHandleReportMalformedJSONIs400(t *testing.T) { http.MethodPost, "/api/v1/reports", strings.NewReader(`{not json`), ) + req.Header.Set("Content-Type", "application/json") h.HandleReport().ServeHTTP(rec, req) @@ -185,6 +271,8 @@ func TestHandleReportOversizeIs413(t *testing.T) { http.MethodPost, "/api/v1/reports", strings.NewReader(`{"clientId":"`+strings.Repeat("x", 200)+`"}`), ) + req.Header.Set("Content-Type", "application/json") + // No declared length, so only the middleware's read cap can // stop this body. req.ContentLength = -1 @@ -213,6 +301,7 @@ func TestHandleReportDoesNotLogRawGeo(t *testing.T) { `{"clientId":"c1","geo":{"raw":"`+sentinel+`"},"hosts":[]}`, ), ) + req.Header.Set("Content-Type", "application/json") h.HandleReport().ServeHTTP(rec, req) @@ -245,6 +334,7 @@ func TestHandleReportLogsClientIDCutToBound(t *testing.T) { `{"clientId":"`+long+`","timestamp":"`+long+`","hosts":[]}`, ), ) + req.Header.Set("Content-Type", "application/json") h.HandleReport().ServeHTTP(rec, req) @@ -284,6 +374,7 @@ func TestHandleReportDecodeErrorLogIsBounded(t *testing.T) { http.MethodPost, "/api/v1/reports", strings.NewReader(`{"hosts":[{"history":[{"t":`+huge+`}]}]}`), ) + req.Header.Set("Content-Type", "application/json") h.HandleReport().ServeHTTP(rec, req) diff --git a/backend/internal/server/routes_test.go b/backend/internal/server/routes_test.go index 8e076cf..fd805f3 100644 --- a/backend/internal/server/routes_test.go +++ b/backend/internal/server/routes_test.go @@ -66,6 +66,7 @@ func TestReportsAreRateLimited(t *testing.T) { http.MethodPost, "/api/v1/reports", strings.NewReader(`{"clientId":"c1","hosts":[]}`), ) + req.Header.Set("Content-Type", "application/json") srv.ServeHTTP(rec, req) return rec.Code