Reports are taken only as application/json (closes #126)
check / check (push) Waiting to run
check / check (push) Waiting to run
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
This commit is contained in:
@@ -231,9 +231,15 @@ Files are 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 store 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
|
||||
|
||||
@@ -21,6 +21,17 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
|
||||
|
||||
# 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`, as
|
||||
`application/x-www-form-urlencoded`, as `multipart/form-data`, with no
|
||||
`Content-Type`, and as `application/json` with and without a charset, and
|
||||
checks which are stored
|
||||
- 2026-10-14: changing the interval no longer drops the samples not yet reported
|
||||
to the backend ([#124](https://git.eeqj.de/sneak/netwatch/issues/124)). The
|
||||
interval menu resets each host with the new `HostState.reset`, which moves the
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,89 @@ 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 or as a form, 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: "a urlencoded form",
|
||||
contentType: "application/x-www-form-urlencoded",
|
||||
wantStatus: http.StatusUnsupportedMediaType,
|
||||
wantStored: 0,
|
||||
},
|
||||
{
|
||||
name: "a multipart form",
|
||||
contentType: "multipart/form-data; boundary=x",
|
||||
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 +208,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 +234,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 +258,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 +283,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 +313,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 +346,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 +386,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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user