fix(api): bound decode and batch-observation request bodies before JSON parsing (#334)

cmd/server/openapi.go

@@ -22,6 +22,16 @@ type routeMeta struct {
 	// generic {"type":"object"} placeholder. Use schemaRef(...) to point
 	// at a named entry in components/schemas (see componentSchemas).
 	Response map[string]interface{} `json:"-"`
+	// Errors documents the non-200 status codes a route answers with, on top
+	// of the 400/404/405 the whole API shares (see info.description). Bodies
+	// are always the ApiError shape writeError produces (#266).
+	Errors []routeError `json:"-"`
+}
+
+// routeError is one documented non-200 response of a route (#334).
+type routeError struct {
+	Status      string // HTTP status code, e.g. "413"
+	Description string
 }
 
 type paramMeta struct {
@@ -67,12 +77,18 @@ func routeDescriptions() map[string]routeMeta {
 				{Name: "search", Description: "Full-text search", Type: "string"},
 				{Name: "groupByHash", Description: "Group duplicate packets by hash", Type: "boolean"},
 			}},
-		"GET /api/packets/{id}":          {Summary: "Get packet detail", Tag: "packets"},
-		"GET /api/packets/timestamps":    {Summary: "Get packet timestamp ranges", Tag: "packets"},
-		"POST /api/packets/observations": {Summary: "Batch submit observations", Description: "Submit multiple observer sightings for existing packets.", Tag: "packets"},
+		"GET /api/packets/{id}":       {Summary: "Get packet detail", Tag: "packets"},
+		"GET /api/packets/timestamps": {Summary: "Get packet timestamp ranges", Tag: "packets"},
+		"POST /api/packets/observations": {Summary: "Get observations for several packets", Description: "Returns the stored observations for up to 200 content hashes in one call: {\"hashes\": [...]} in, {\"results\": {\"<hash>\": [...]}} out. More than 200 hashes is 400. The request body is capped at 65536 bytes before it is parsed (#334) — a larger body is 413 regardless of its shape or of whether it declares a Content-Length, and the byte cap leaves roughly 17x headroom over a full 200-hash request, so the 200-hash limit is what a client actually meets first.", Tag: "packets",
+			Errors: []routeError{
+				{Status: "413", Description: "Request body over 65536 bytes (#334). Enforced before parsing, on bytes received, so it also applies to a chunked request with no Content-Length."},
+			}},
 
 		// Decode
-		"POST /api/decode": {Summary: "Decode a raw packet", Description: "Decodes a hex-encoded packet without storing it.", Tag: "packets"},
+		"POST /api/decode": {Summary: "Decode a raw packet", Description: "Decodes a hex-encoded packet without storing it. The request body is capped at 4096 bytes before it is parsed (#334) — a larger body is 413 regardless of its shape or of whether it declares a Content-Length. The largest frame the decoder can accept (header + transport codes + MAX_PATH_SIZE path bytes + MAX_PACKET_PAYLOAD payload bytes) is a ~520-byte body, so the cap leaves roughly 7x headroom.", Tag: "packets",
+			Errors: []routeError{
+				{Status: "413", Description: "Request body over 4096 bytes (#334). Enforced before parsing, on bytes received, so it also applies to a chunked request with no Content-Length."},
+			}},
 
 		// Nodes
 		"GET /api/nodes": {Summary: "List nodes", Description: "Returns all known mesh nodes with status and metadata. Repeater/room rows carry the issue #672 usefulness metrics (traffic_share_score, bridge_score, coverage_score, redundancy_score), the composite usefulness_score + usefulness_grade, and relay-activity counters. See the Node schema.", Tag: "nodes",
@@ -234,6 +250,28 @@ func openAPIRef(name string) *openAPISchema {
 	return &openAPISchema{Ref: "#/components/schemas/" + name}
 }
 
+// openAPIResponse / openAPIMediaType are typed OpenAPI 3.0 response objects.
+// They marshal to the same JSON as the map literals used for the shared 200
+// response; routeMeta.Errors uses them instead of new untyped map literals
+// (#1383).
+type openAPIResponse struct {
+	Description string                      `json:"description"`
+	Content     map[string]openAPIMediaType `json:"content,omitempty"`
+}
+
+type openAPIMediaType struct {
+	Schema *openAPISchema `json:"schema,omitempty"`
+}
+
+// jsonErrorResponse is an application/json response carrying the shared
+// ApiError body.
+func jsonErrorResponse(description string) openAPIResponse {
+	return openAPIResponse{
+		Description: description,
+		Content:     map[string]openAPIMediaType{"application/json": {Schema: openAPIRef("ApiError")}},
+	}
+}
+
 // nodeAdvertRouteSchemas documents the #2073 node-detail advert route fields
 // (port of upstream Kpa-clawbot/CoreScope#2073).
 func nodeAdvertRouteSchemas() map[string]*openAPISchema {
@@ -859,6 +897,11 @@ func componentSchemas() map[string]interface{} {
 			},
 		},
 	}
+	schemas["ApiError"] = &openAPISchema{
+		Type:        "object",
+		Description: "The shared error body every /api/* error answer uses (#266): a single human-readable message. Status codes carry the machine-readable meaning, not this string.",
+		Properties:  map[string]*openAPISchema{"error": {Type: "string"}},
+	}
 	for name, schema := range nodeAdvertRouteSchemas() {
 		schemas[name] = schema
 	}
@@ -925,6 +968,22 @@ func buildOpenAPISpec(router *mux.Router, version string) map[string]interface{}
 			respSchema = meta.Response
 		}
 
+		responses := map[string]interface{}{
+			"200": map[string]interface{}{
+				"description": "Success",
+				"content": map[string]interface{}{
+					"application/json": map[string]interface{}{
+						"schema": respSchema,
+					},
+				},
+			},
+		}
+		if hasMeta {
+			for _, e := range meta.Errors {
+				responses[e.Status] = jsonErrorResponse(e.Description)
+			}
+		}
+
 		// Build operation
 		op := map[string]interface{}{
 			"summary": func() string {
@@ -933,16 +992,7 @@ func buildOpenAPISpec(router *mux.Router, version string) map[string]interface{}
 				}
 				return ri.path
 			}(),
-			"responses": map[string]interface{}{
-				"200": map[string]interface{}{
-					"description": "Success",
-					"content": map[string]interface{}{
-						"application/json": map[string]interface{}{
-							"schema": respSchema,
-						},
-					},
-				},
-			},
+			"responses": responses,
 		}
 
 		if hasMeta {
@@ -1026,7 +1076,7 @@ func buildOpenAPISpec(router *mux.Router, version string) map[string]interface{}
 		"openapi": "3.0.3",
 		"info": map[string]interface{}{
 			"title":       "CoreScope API",
-			"description": "MeshCore network analyzer — packet capture, node tracking, and mesh analytics. An unrecognized /api or /api/* path returns 404; a documented path called with an unsupported method returns 405 with an Allow header. Both are JSON (#233). HEAD is served on every GET path.",
+			"description": "MeshCore network analyzer — packet capture, node tracking, and mesh analytics. An unrecognized /api or /api/* path returns 404; a documented path called with an unsupported method returns 405 with an Allow header. Both are JSON (#233). POST /api/decode and POST /api/packets/observations cap their request body in bytes before parsing it and answer 413 with the ApiError body when it is over the cap (#334); the other body-taking endpoints bound their body on the JSON decoder instead, which is weaker and reports an over-cap body as 400 — docs/api-spec.md lists every cap, how it is enforced, and its status. HEAD is served on every GET path.",
 			"version":     version,
 			"license": map[string]interface{}{
 				"name": "MIT",

cmd/server/request_body_limits.go

@@ -0,0 +1,89 @@
+package main
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+)
+
+// Issue #334 — request-body byte caps for the two public POST endpoints that
+// used to hand r.Body straight to a json.Decoder with no bound:
+//
+//   - POST /api/decode              (maxDecodeBodyBytes)
+//   - POST /api/packets/observations (maxBatchObservationsBodyBytes)
+//
+// Both caps run BEFORE any parsing, so an oversized body never gets decoded
+// into memory, whatever its shape (deeply nested, one giant string, or bulk
+// stuffed into a field the handler does not even know about). The existing
+// 200-hash limit on the batch endpoint is a cardinality limit that runs after
+// parsing, so it never bounded bytes; it is unchanged and still fires first for
+// every request a legitimate client sends (see the headroom notes below).
+//
+// Do not rely on a reverse proxy for this: the caps are enforced in the
+// application, and TestBodyLimitsOverRealChunkedRequests exercises them over a
+// real connection with no Content-Length at all.
+//
+// TODO(#334, AGENTS.md rule 8): both limits are hardcoded here for now and
+// should be exposed as operator-configurable values (config.json, and the
+// customizer where it makes sense) in a later milestone.
+const (
+	// maxDecodeBodyBytes caps POST /api/decode. The largest frame DecodePacket
+	// can possibly accept is 1 header + 4 transport codes + 1 path byte +
+	// MAX_PATH_SIZE (64) path bytes + MAX_PACKET_PAYLOAD (184) payload bytes =
+	// 254 bytes, i.e. 508 hex characters, i.e. a ~520-byte JSON body. 4 KiB
+	// leaves ~7x headroom over anything the decoder would not reject anyway.
+	// Matches the 4 KiB upstream uses, and the existing 4096-byte cap on
+	// POST /api/paths/inspect (inspectBodyLimit).
+	maxDecodeBodyBytes = 4 << 10 // 4 KiB
+
+	// maxBatchObservationsBodyBytes caps POST /api/packets/observations. The
+	// biggest request the handler accepts is 200 16-hex content hashes, which
+	// serializes to ~3.8 KiB, so 64 KiB leaves ~17x headroom: every request
+	// within the 200-hash limit passes, and a client that exceeds that limit
+	// still gets the established 400 "too many hashes" rather than a 413.
+	// Matches the 64 KiB upstream uses.
+	maxBatchObservationsBodyBytes = 64 << 10 // 64 KiB
+
+	// geoFilterBodyLimit caps PUT /api/config/geo-filter (API key). It predates
+	// #334 and is named here only so the caps table in docs/api-spec.md can be
+	// pinned to the value the handler actually enforces; the handler still
+	// reports an over-cap body as 400 "invalid JSON", not 413. A polygon may
+	// carry 1000 points, so the cap is generous on purpose.
+	geoFilterBodyLimit = 1 << 20 // 1 MiB
+)
+
+// decodeLimitedJSONBody reads at most limit bytes of r.Body and unmarshals them
+// into v. It answers the request itself and returns false when the body is over
+// the cap (413, naming the limit) or is not valid JSON (400); the caller returns
+// immediately in that case. Both answers use the shared writeError shape
+// ({"error": "..."} as application/json, #266), so clients get one error
+// contract across every endpoint.
+//
+// Reading the body to a bounded buffer first — rather than letting a
+// json.Decoder stream it — is what makes the cap a byte cap: the verdict
+// depends only on how many bytes arrived, never on where in the JSON the
+// decoder happened to stop. A body of exactly limit bytes is accepted; limit+1
+// is rejected.
+func decodeLimitedJSONBody(w http.ResponseWriter, r *http.Request, limit int64, v interface{}) bool {
+	r.Body = http.MaxBytesReader(w, r.Body, limit)
+	raw, err := io.ReadAll(r.Body)
+	if err != nil {
+		var tooLarge *http.MaxBytesError
+		if errors.As(err, &tooLarge) {
+			writeError(w, http.StatusRequestEntityTooLarge,
+				fmt.Sprintf("request body too large (max %d bytes)", limit))
+			return false
+		}
+		// A truncated or aborted upload is as unusable as malformed JSON, and
+		// json.Decoder answered the same 400 for it before this cap existed.
+		writeError(w, http.StatusBadRequest, "invalid JSON body")
+		return false
+	}
+	if err := json.Unmarshal(raw, v); err != nil {
+		writeError(w, http.StatusBadRequest, "invalid JSON body")
+		return false
+	}
+	return true
+}