[dborup]

Relates to #334

Summary

POST /api/decode and POST /api/packets/observations handed r.Body straight to a json.Decoder with no byte cap. The batch endpoint's 200-hash limit is a cardinality check that runs after parsing, so neither endpoint bounded how much a single request could make the server allocate.

Both handlers now go through one new helper, decodeLimitedJSONBody (cmd/server/request_body_limits.go):

r.Body = http.MaxBytesReader(w, r.Body, limit)
raw, err := io.ReadAll(r.Body)      // bounded buffer, before any parsing
// *http.MaxBytesError -> 413 ; any other read error / bad JSON -> 400
json.Unmarshal(raw, v)

Reading the bounded body first, rather than letting a json.Decoder stream it, is what makes this a byte cap: the verdict depends only on how many bytes arrived, never on where in the JSON the decoder happened to stop. A streaming decoder would let an oversized body through whenever the value it wants happens to complete inside the first limit bytes (trailing padding, ignored fields). Exactly limit bytes is accepted; limit+1 is 413.

Over the cap answers 413 via the existing writeError shape (#266) — {"error":"request body too large (max N bytes)"}, application/json — so there is one error contract across endpoints. Malformed JSON within the cap keeps its existing 400. No new map[string]interface{}; cmd/server stays read-only (no SQL touched).

Why these limits (upstream's 4 KiB / 64 KiB, checked against our clients)

Upstream's numbers hold for us, and both are now asserted by tests rather than asserted in prose:

Endpoint	Cap	Largest legitimate body	Headroom
POST /api/decode	4096 B	~520 B	~7.9x
POST /api/packets/observations	65536 B	3812 B	~17.2x

• decode. The only client is the BYOP modal in public/packets.js, which posts {"hex": <stripped hex>}. The largest frame DecodePacket can accept at all is 1 header + 4 transport codes + 1 path-length byte + MAX_PATH_SIZE (64) path bytes + MAX_PACKET_PAYLOAD (184) payload bytes = 254 bytes → 508 hex chars → a ~520-byte body. Anything bigger is a frame the decoder rejects anyway. 4 KiB also matches the limit already on POST /api/paths/inspect (inspectBodyLimit).
• batch observations. The only client is the packets table's obs-sort path, which posts {"hashes": [...]} of 16-hex content hashes. A full 200-hash request measures 3812 bytes on the wire ({"hashes":[ + 200x18 + 199 commas + ]}; a 201-hash request is 3831 bytes). That is the reason 64 KiB is the right number and 4 KiB would not be: a 4 KiB cap would leave only 284 bytes of slack on a request the handler is supposed to accept, so the byte cap would start masking the 200-hash limit. At 64 KiB every request inside the 200-hash limit passes and a 201-hash request still gets the established 400 too many hashes, not a 413.

TestDecodeBodyLimitFitsLargestDecodableFrame and TestBatchObservationsHashLimitPreserved assert the headroom on both sides (≥4x so no legitimate client is cut off, ≤32x / ≤64x so a cap loosened to the point of bounding nothing fails CI).

Deliberate behaviour change: trailing data is now rejected

decodeLimitedJSONBody finishes with json.Unmarshal, which rejects anything but whitespace after the JSON value. The json.Decoder.Decode these two handlers used before silently ignored it, so three bodies that were 200 on master are 400 on this branch:

POST /api/decode                {"hex":"0200"} trailing      200 -> 400
POST /api/decode                {"hex":"0200"}{"hex":"zz"}   200 -> 400
POST /api/packets/observations  {"hashes":[]} junk           200 -> 400

This is intentional, not a side effect: it matches decodeSingleJSONObject on POST /api/channel-proposals, and our only clients build their bodies with JSON.stringify, which cannot produce trailing data. Trailing whitespace is valid JSON and is still accepted — the boundary tests pad with it. TestBodyLimitsRejectTrailingData pins all four rejections plus the whitespace case.

Preserved behaviour

• 200-hash limit unchanged, and still what a client meets first (TestBatchObservationsHashLimitPreserved, plus the pre-existing TestHandleBatchObservations).
• decode behaviour unchanged: {"hex":"0200"} still 200 with decoded.{header,path,payload}; empty hex still 400 hex is required; undecodable hex still 400 (TestDecodeNormalRequestUnchanged).
• Empty hashes still {"results":{}}; malformed JSON still 400 invalid JSON body.

Not a proxy concern

The caps are enforced in the application. TestBodyLimitsOverRealChunkedRequests drives a real httptest.Server over a real connection with a body of unknown length, and TestChunkedRequestHasNoContentLength proves that request really arrives as Transfer-Encoding: chunked with ContentLength == -1. A separate case feeds a Content-Length that understates the real body and still gets 413. Verified again with curl against a built binary (below).

Perf

No hot path. One io.ReadAll into a buffer that is now bounded where it previously was not (the json.Decoder allocated from the same unbounded stream), so peak per-request allocation on these two endpoints goes from unbounded to ≤4 KiB / ≤64 KiB. Strictly a reduction in worst case; no change in the normal case.

Docs

• cmd/server/openapi.go: routeMeta gains a typed Errors []routeError, emitted as a real 413 response object (typed openAPIResponse/openAPIMediaType, per the refactor: drive map[string]interface{} out of API response layer + internal data (694 occurrences in cmd/) — typed structs, codegen for frontend Kpa-clawbot/CoreScope#1383 no-new-untyped-map-literals convention) referencing a new ApiError component schema. Both route descriptions and info.description state the caps. Verified on the served /api/spec.
• docs/api-spec.md: new Request-body byte caps section under Error Responses (table of all four body-capped endpoints, and that the cap is on bytes received, not on the parsed value or on Content-Length), 413 added to the shared error list and to POST /api/decode, and a new POST /api/packets/observations section — that endpoint was undocumented.
• TestAPISpecDocumentsBodyLimits pins the documented numbers to the constants, so the two cannot drift.

Config follow-up (AGENTS.md rule 8)

Both limits are hardcoded constants for now, with a TODO(#334, AGENTS.md rule 8) in cmd/server/request_body_limits.go: they should become operator-configurable (config.json, and the customizer where it makes sense) in a later milestone. Not done here to keep this change to the security fix.

Tests

New cmd/server/request_body_limits_test.go (16 top-level tests, 31 cases):

Area	Tests
Exact boundaries ±1	TestDecodeBodyLimitBoundaries, TestBatchObservationsBodyLimitBoundaries (limit−1 / limit / limit+1)
Oversized unknown fields	TestDecodeBodyLimitOversizedUnknownField, TestBatchObservationsBodyLimitOversizedUnknownField
Chunked / unknown length	TestBodyLimitsOverRealChunkedRequests (real connection), TestChunkedRequestHasNoContentLength, TestDecodeBodyLimitUnknownLength (incl. an understated Content-Length), TestBatchObservationsBodyLimitUnknownLength
Malformed JSON	TestDecodeBodyLimitMalformedJSON, TestBatchObservationsBodyLimitMalformedJSON (under the cap → 400; over it → 413, bytes before syntax; empty body → 400)
Normal requests	TestDecodeNormalRequestUnchanged, TestBatchObservationsNormalRequestUnchanged
Limits vs. real clients	TestDecodeBodyLimitFitsLargestDecodableFrame, TestBatchObservationsHashLimitPreserved
Documented 413	TestOpenAPIDocuments413OnBodyLimitedRoutes, TestAPISpecDocumentsBodyLimits

Every 413 assertion also checks the body is the shared {"error": ...} shape with Content-Type: application/json.

Red before, green after

Captured on this branch's origin/master starting point with only the two constants stubbed in (so the tests compile and fail on behaviour, not on a missing symbol) — 12 subtests red:

--- FAIL: TestDecodeBodyLimitBoundaries/one_byte_over_the_limit_is_413
        status: want 413, got 200
--- FAIL: TestDecodeBodyLimitOversizedUnknownField
        status: want 413, got 200
--- FAIL: TestDecodeBodyLimitUnknownLength/oversized_with_no_Content-Length_is_413
--- FAIL: TestDecodeBodyLimitUnknownLength/a_lying_Content-Length_cannot_raise_the_cap
--- FAIL: TestDecodeBodyLimitMalformedJSON/malformed_over_the_cap_is_413,_not_400
        status: want 413, got 400
--- FAIL: TestBatchObservationsBodyLimitBoundaries/one_byte_over_the_limit_is_413
--- FAIL: TestBatchObservationsBodyLimitOversizedUnknownField
--- FAIL: TestBatchObservationsBodyLimitUnknownLength/oversized_with_no_Content-Length_is_413
--- FAIL: TestBatchObservationsBodyLimitMalformedJSON/malformed_over_the_cap_is_413,_not_400
--- FAIL: TestBodyLimitsOverRealChunkedRequests/decode_over_the_cap
--- FAIL: TestBodyLimitsOverRealChunkedRequests/batch_over_the_cap

TestOpenAPIDocuments413OnBodyLimitedRoutes and TestAPISpecDocumentsBodyLimits were red on the same baseline (no 413 in the spec, no caps section in the doc). All green after.

Mutants (7, each reverted after confirming red)

#	Mutant	Killed by
1	http.MaxBytesReader wrapper dropped from the helper	12 subtests across both endpoints
2	413 downgraded to http.StatusBadRequest	the same 12
3	Off-by-one: MaxBytesReader(..., limit+1)	7 subtests (every exact-boundary and unknown-length case)
4	Batch handler left on the old unbounded json.NewDecoder	5 batch subtests; decode cases stayed green (correctly scoped)
5	maxDecodeBodyBytes raised to 1 MiB	TestDecodeBodyLimitFitsLargestDecodableFrame (>32x headroom), TestAPISpecDocumentsBodyLimits
6	maxBatchObservationsBodyBytes lowered to 1 KiB	TestBatchObservationsHashLimitPreserved (both cases), the pre-existing TestHandleBatchObservations/too_many_hashes_returns_400, TestAPISpecDocumentsBodyLimits
7	413 removed from the decode route's OpenAPI metadata	TestOpenAPIDocuments413OnBodyLimitedRoutes

Mutant 6 is the one that matters for "preserve the 200-hash limit": a too-tight byte cap masks the cardinality limit, and the suite says so.

Live verification against a built binary

cmd/server built and started on port 13701 against a copy of test-fixtures/e2e-fixture.db freshened, migrated and seeded exactly as the deploy workflow does.

# decode
POST /api/decode {"hex":"0200"}                      -> 200  {"decoded":{"header":{...
POST /api/decode  4096-byte body                     -> 200
POST /api/decode  4097-byte body                     -> 413  {"error":"request body too large (max 4096 bytes)"}
POST /api/decode  4097 bytes, Transfer-Encoding: chunked (no Content-Length)
                                                     -> 413  {"error":"request body too large (max 4096 bytes)"}
POST /api/decode  {"hex":"0200","unknownField":"A*5000"}
                                                     -> 413
POST /api/decode  {"hex":                            -> 400  {"error":"invalid JSON body"}
POST /api/decode  malformed + over the cap           -> 413

# batch observations
POST /api/packets/observations  3 real fixture hashes -> 200  {"results":{"013c8e48966eb4f7":[{...
POST /api/packets/observations  200 hashes (3812 B)   -> 200
POST /api/packets/observations  201 hashes (4033 B)   -> 400  {"error":"too many hashes (max 200)"}
POST /api/packets/observations  65536-byte body       -> 200
POST /api/packets/observations  65537-byte body       -> 413  {"error":"request body too large (max 65536 bytes)"}
POST /api/packets/observations  65537 B, chunked      -> 413
POST /api/packets/observations  oversized unknown field -> 413
POST /api/packets/observations  'not json'            -> 400
POST /api/packets/observations  {"hashes":[]}         -> 200  {"results":{}}

The chunked cases were confirmed to send no Content-Length at all (curl -v request headers: Transfer-Encoding: chunked, no length header).

Served spec on the same binary:

/api/decode                -> responses ['200', '413']
/api/packets/observations  -> responses ['200', '413']
components/schemas/ApiError present; info.description names the 413

CI (per job, run locally in this worktree)

• go build ./..., go vet ./... in cmd/server and cmd/ingestor — clean.
• gofmt -l on every touched/new file (request_body_limits.go, request_body_limits_test.go, routes.go, openapi.go) — no output. (Many pre-existing files in cmd/server are not gofmt-clean; none of them are touched here.)
• cd cmd/server && go test ./... — ok, 54.7s, no regressions.
• cd cmd/ingestor && go test ./... — ok, 112.6s. (No internal/* module touched.)
• sh test-all.sh — 233/233 files passed.
• node test-frontend-helpers.js — 709 passed, 0 failed.
• scripts/check-xss-sinks.sh --diff origin/master — clean (no public/** changes to scan).
• E2E against the live build on port 13701: test-e2e-playwright.js 132/135 passed, 3 skipped (the 3 skips are pre-existing and baked into the test), test-issue-1487-byop-modal-layout-e2e.js 4/4, test-map-modal-fluid-e2e.js 10/10 (includes the BYOP decode modal), test-issue-1648-m2-icons-e2e.js 21/21, test-issue-147-packets-url-obs-e2e.js 12/12, test-packet-detail-sender-hash-size-obs-e2e.js 3/3, test-packets.js 161/161, test-issue-1415-packets-layout.js 26/26.
• Fork guards unchanged: deploy.yml 9, release-fast-path.yml 1 — neither file touched.

GitHub Actions on this PR is checked after push; see the report comment.

Rester (leftovers / out of scope)

• [K] public/packets.js can build a batch request larger than the handler accepts. The obs-sort path does packets.filter(...).map(p => p.hash) with no cap, and PACKET_LIMIT is 50000 on desktop, so a page with more than 200 multi-observation groups posts more than 200 hashes. That request already fails today with 400 too many hashes (the response is swallowed by if (resp.ok)), so this PR does not introduce the problem — but above ~3400 hashes the answer changes from that 400 to a 413. Chunking the client into 200-hash batches is the real fix; it is new client logic with its own tests and belongs in its own issue, not in a server-side body-limit change.
• [K] The two caps are hardcoded, see the config follow-up above (AGENTS.md rule 8).
• [K] The other body-taking endpoints are bounded on the JSON decoder, not on bytes, and answer 400. POST /api/paths/inspect (4096 B), POST /api/channel-proposals (1024 B) and PUT /api/config/geo-filter (1 MiB) wrap the body in http.MaxBytesReader and hand it to a streaming decoder, so the limit only bites when the JSON value runs past it — a 6019-byte /api/paths/inspect body whose object ends at byte 19 answers 200 [A], where the same shape is 413 on /api/decode. POST /api/admin/prune-geo-filter?confirm=true (API key) has no limit at all. docs/api-spec.md now states each endpoint's limit, how it is enforced, and its over-limit status, and lists all three as known gaps. Tightening them is API-visible, touches handlers this issue does not, and is deliberately not bundled here.
• Node-list cardinality and Reach concurrency limits are explicitly out of scope per the issue.

[dborup]

Rapport — CS-Minimax PR#359 #334 — head e030447

Status: Complete — every acceptance point of #334 implemented, each with a red-before/green-after test and at least one mutant; CI green on all jobs, first run, no reruns needed.

Evidence tags: [T] automated test, [A] automated check against a built binary / live server, [K] known accepted gap.

Acceptance criteria

#	Criterion (#334)	Implementation	Test (red before → green after)	Mutant
1	Body limits applied before parsing on both endpoints	decodeLimitedJSONBody in new cmd/server/request_body_limits.go: http.MaxBytesReader → bounded io.ReadAll → json.Unmarshal. Wired into handleDecode and handleBatchObservations. A streaming decoder was deliberately not used: it lets an oversized body through whenever the value completes inside the first limit bytes.	[T] TestDecodeBodyLimitBoundaries, TestBatchObservationsBodyLimitBoundaries	M1 MaxBytesReader wrapper dropped → 12 subtests red. M4 batch handler left on the old unbounded decoder → 5 batch subtests red, decode cases correctly stayed green.
2	Exact boundaries, ±1 byte	Exactly limit accepted, limit+1 rejected, independent of JSON shape	[T] both boundary tests cover limit−1 / limit / limit+1. [A] curl: 4096 → 200, 4097 → 413; 65536 → 200, 65537 → 413	M3 MaxBytesReader(..., limit+1) → 7 subtests red (every exact-boundary and unknown-length case)
3	Oversized unknown fields	Cap is on bytes received, so bulk in an unread field cannot buy room	[T] TestDecodeBodyLimitOversizedUnknownField, TestBatchObservationsBodyLimitOversizedUnknownField. [A] curl with "unknownField":"A"*5000 → 413	M1, M2 (both kill these)
4	Chunked / unknown-length requests	No Content-Length precheck at all — one path, so a chunked or understated-length request is cut at the same byte count	[T] TestBodyLimitsOverRealChunkedRequests (real httptest.Server, real connection), TestChunkedRequestHasNoContentLength (proves the request arrives as Transfer-Encoding: chunked, ContentLength == -1), TestDecodeBodyLimitUnknownLength incl. an understated Content-Length, TestBatchObservationsBodyLimitUnknownLength. [A] curl -H 'Transfer-Encoding: chunked' → 413 on both endpoints; curl -v confirms no length header was sent	M1, M2, M3
5	Malformed JSON	Under the cap → existing 400; over the cap → 413 (bytes win over syntax); empty body → 400	[T] TestDecodeBodyLimitMalformedJSON, TestBatchObservationsBodyLimitMalformedJSON. [A] curl {"hex": → 400, truncated-and-oversized → 413	M2 413 downgraded to 400 → 12 subtests red
6	Normal requests unaffected	Unchanged happy paths and unchanged existing 400s	[T] TestDecodeNormalRequestUnchanged (valid frame → 200 with decoded.{header,path,payload}; empty hex → 400 hex is required; bad hex → 400), TestBatchObservationsNormalRequestUnchanged, plus the pre-existing TestHandleBatchObservations and the decode cases in routes_test.go. [A] curl against the fixture DB returned real observations for 3 fixture hashes	M6 (below) also trips the pre-existing TestHandleBatchObservations
7	Documented 413 with a consistent JSON error (same writeError shape as #266)	413 via writeError → {"error":"request body too large (max N bytes)"}, application/json. openapi.go: typed routeMeta.Errors []routeError emitted as a real 413 response object referencing a new ApiError component schema; both route descriptions and info.description state the caps. docs/api-spec.md: new Request-body byte caps section, 413 in the shared error list, 413 on POST /api/decode, and a new POST /api/packets/observations section (it was undocumented).	[T] TestOpenAPIDocuments413OnBodyLimitedRoutes (413 present on both routes, with an ApiError ref), TestAPISpecDocumentsBodyLimits (the doc's numbers are pinned to the constants). Every 413 assertion in the suite also checks the shared {"error": ...} shape + content type. [A] served /api/spec on the built binary: both routes → ['200','413'], ApiError present	M7 413 removed from the decode route metadata → TestOpenAPIDocuments413OnBodyLimitedRoutes red
8	4 KiB / 64 KiB evaluated against our legitimate clients	Upstream's numbers hold. decode: only client is the BYOP modal in public/packets.js; the largest frame DecodePacket can accept at all is 254 bytes (1 header + 4 transport codes + 1 path-length byte + MAX_PATH_SIZE 64 + MAX_PACKET_PAYLOAD 184) → a ~520-byte body → ~7.9x headroom. batch: only client is the packets obs-sort path; a full 200-hash request measures 4013 bytes → ~16.3x headroom. That measurement is also why 4 KiB would be wrong for the batch endpoint: it would leave 83 bytes of slack on a request the handler must accept.	[T] TestDecodeBodyLimitFitsLargestDecodableFrame and TestBatchObservationsHashLimitPreserved assert headroom on both sides — ≥4x so no legitimate client is cut off, ≤32x / ≤64x so a cap loosened past usefulness fails. [A] measured on the live server: 200-hash body = 4013 B → 200	M5 decode cap raised to 1 MiB → TestDecodeBodyLimitFitsLargestDecodableFrame + TestAPISpecDocumentsBodyLimits red
9	200-hash limit and decode behaviour preserved	maxHashes = 200 untouched and still what a client meets first; decode logic untouched below the body read	[T] TestBatchObservationsHashLimitPreserved (200 hashes → 200 and fit inside the cap; 201 hashes → 400 too many hashes, not 413). [A] curl: 200 hashes (4013 B) → 200; 201 hashes (4033 B) → 400 too many hashes (max 200)	M6 batch cap lowered to 1 KiB → both TestBatchObservationsHashLimitPreserved cases, the pre-existing TestHandleBatchObservations/too_many_hashes_returns_400, and TestAPISpecDocumentsBodyLimits red. This is the mutant that proves a too-tight cap would mask the cardinality limit.
10	Do not rely on proxy limits — verify the application directly	Enforced in the handlers	[A] cmd/server built and run on a local port against a copy of test-fixtures/e2e-fixture.db freshened / migrated / seeded exactly as the deploy workflow does; all 16 request shapes above exercised with curl straight at the Go server, no proxy in front. [T] TestBodyLimitsOverRealChunkedRequests does the same over a real connection in-suite.	M1–M4

Red baseline

Captured on this branch's origin/master starting point with only the two constants stubbed in, so the tests compile and fail on behaviour rather than on a missing symbol — 12 subtests red, e.g.:

--- FAIL: TestDecodeBodyLimitBoundaries/one_byte_over_the_limit_is_413   status: want 413, got 200
--- FAIL: TestDecodeBodyLimitUnknownLength/a_lying_Content-Length_cannot_raise_the_cap
--- FAIL: TestDecodeBodyLimitMalformedJSON/malformed_over_the_cap_is_413,_not_400   status: want 413, got 400
--- FAIL: TestBatchObservationsBodyLimitOversizedUnknownField   status: want 413, got 200
--- FAIL: TestBodyLimitsOverRealChunkedRequests/decode_over_the_cap
--- FAIL: TestBodyLimitsOverRealChunkedRequests/batch_over_the_cap

The two documentation tests were red on the same baseline (no 413 in the spec, no caps section in the doc). All green after. 7 mutants run in total, each reverted after confirming red; the working tree was verified clean afterwards.

CI (per job)

Job	Result
✅ Go Build & Test	pass, 22m46s — every step green, incl. go test for server + ingestor, the ingestor race-check, internal/anomaly with -race, CSS-var lint, test-all.sh, the preflight XSS gate (--diff), eslint, proto syntax
🎭 Playwright E2E Tests	pass, 27m36s — full fail-fast E2E run plus the Kpa-clawbot#1616 slide-over flake-gate (--repeat-each=3)
🏗️ Build & Publish Docker Image	pass, 57s
📦 Release Artifacts	skipping (not a tag)
🚀 Deploy Staging	skipping
📝 Publish Badges & Summary	skipping

No reruns were needed — green on the first run. The known-unstable test tracked in #271 did not fail.

Run locally in this worktree before pushing: go build/go vet clean in cmd/server and cmd/ingestor; cd cmd/server && go test ./... ok (54.7s); cd cmd/ingestor && go test ./... ok (112.6s); sh test-all.sh 233/233 files passed; node test-frontend-helpers.js 709 passed / 0 failed; scripts/check-xss-sinks.sh --diff origin/master clean; gofmt -l empty on all four touched files. E2E against the live local build: test-e2e-playwright.js 132/135 passed + 3 pre-existing skips, test-map-modal-fluid-e2e.js 10/10 (covers the BYOP decode modal), test-issue-1487-byop-modal-layout-e2e.js 4/4, test-issue-147-packets-url-obs-e2e.js 12/12, test-packet-detail-sender-hash-size-obs-e2e.js 3/3, test-issue-1648-m2-icons-e2e.js 21/21, test-packets.js 161/161, test-issue-1415-packets-layout.js 26/26.

Invariants

• [A] cmd/server read-only: no SQL added, no write-side helper; readonly_invariant_test.go green.
• [A] No new map[string]interface{} outside tests — the new helper is plain typed code, and the OpenAPI additions use typed openAPIResponse / openAPIMediaType / openAPISchema per the refactor: drive map[string]interface{} out of API response layer + internal data (694 occurrences in cmd/) — typed structs, codegen for frontend Kpa-clawbot/CoreScope#1383 convention.
• [A] No hardcoded colours; no public/** changes at all, so the XSS gate had nothing to scan and reported clean.
• [A] Fork guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml. Neither workflow file is touched by this branch.
• [A] Two commits, no rebase, amend or force-push; only explicit git add.

Rester (leftovers / out of scope)

• [K] public/packets.js can build a batch request larger than the handler accepts. The obs-sort path does packets.filter(...).map(p => p.hash) with no cap, and the page limit is 50000 on desktop, so a page with more than 200 multi-observation groups posts more than 200 hashes. That request already fails today with 400 too many hashes and the response is swallowed by if (resp.ok), so this PR does not introduce the problem — but above roughly 3400 hashes the answer changes from that 400 to a 413. The real fix is chunking the client into 200-hash batches; that is new client logic with its own tests and belongs in its own issue.
• [K] Both caps are hardcoded constants, with a TODO(#334, AGENTS.md rule 8) in the source: they should become operator-configurable (config.json, and the customizer where it makes sense) in a later milestone. Kept out of this change to keep it to the security fix.
• [K] POST /api/path-inspect and POST /api/channel-proposals already cap their bodies but answer 400, not 413, when the cap is hit. They are now listed as capped in docs/api-spec.md; aligning their status code is an API-visible change and is deliberately not bundled here.
• Node-list cardinality and Reach concurrency limits are explicitly out of scope per the issue.
• The PR is left as a draft; nothing merged, no issue closed.

[adminopenclaw8-sketch]

Rapport — CS-Macmini PR#359 runde-2 — head 1e3eccf

Review feedback addressed (commit 1e3eccf2).

Reviewed against "Review — CS-pve-agent1 PR#359" (REQUEST CHANGES, head e030447a). Three commits on top of a merge of origin/master (195ec4c3, which already contained ff9a501c): 196a368a (the five findings), dfb9a515 (a correction to F2 found while verifying it) and 1e3eccf2 (the same section's lead sentence). No rebase, amend or force-push; only explicit git add. PR stays a draft.

Evidence tags: [T] automated test, [A] automated/manual check against a built binary or live server, [K] known accepted gap.

Findings

1. F1 (blocking) — no test distinguished "cap before parsing" from "size check after an unbounded read". Agreed, and the fix is exactly the property the reviewer named: assert on the bytes the handler pulls off the body, not on the status it answers. Two new tests in cmd/server/request_body_limits_test.go:

• TestBodyLimitsStopReadingAtTheCap feeds each endpoint a countingEndlessBody — one '{' then spaces forever, counting every byte handed out, with an 8 MiB hard stop so a runaway handler fails instead of hanging — with ContentLength == -1, and asserts both the 413 and read <= limit + 4096.
• TestBodyLimitsDoNotDrainAnOversizedBody does the same on a finite body 16x over the cap through a countingReader, so a handler that buffers the whole request before measuring it is caught even without an endless body.

The 4096-byte slack is justified in the test: http.MaxBytesReader trims each Read to the remaining budget plus one byte, so it reads at most limit+1 from the underlying body. One page of slack is generous and still ~2000x below the hard stop.

Mutant A (the reviewer's) red → green [T]:

# Mutant A applied: MaxBytesReader dropped, unbounded io.ReadAll, size checked afterwards
--- FAIL: TestBodyLimitsStopReadingAtTheCap//api/decode
    handler read 8388608 bytes of an endless body; the cap is 4096 (allowed at most 8192).
    status: want 413, got 400
--- FAIL: TestBodyLimitsStopReadingAtTheCap//api/packets/observations
    handler read 8388608 bytes of an endless body; the cap is 65536 (allowed at most 69632).
    status: want 413, got 400
--- FAIL: TestBodyLimitsDoNotDrainAnOversizedBody//api/decode
    handler read 65544 bytes of a 65544-byte body; the cap is 4096 (allowed at most 8192)
--- FAIL: TestBodyLimitsDoNotDrainAnOversizedBody//api/packets/observations
    handler read 1048584 bytes of a 1048584-byte body; the cap is 65536 (allowed at most 69632)
FAIL

# Mutant reverted (diff clean):
ok  github.com/corescope/server

Four subtests red on the mutant that survived the entire previous suite. Note the second assertion line: with no bound, the hard stop also turns the answer into a 400, so the mutant is wrong twice over.

2. F2 (should fix) — docs/api-spec.md and info.description overclaimed. Agreed on every point, and verifying it turned up one more. Fixed rather than scoped away, because the table is the useful part:

• the route is POST /api/paths/inspect; POST /api/path-inspect 404s [A]. Corrected in the doc, in the maxDecodeBodyBytes comment, and guarded by a test that fails if the non-existent path reappears;
• PUT /api/config/geo-filter is now listed with its 1048576-byte (1 MiB) limit. Its inline 1<<20 became a named geoFilterBodyLimit so the doc row is pinned to the value the handler enforces, as the other rows already were;
• per-endpoint status, not a blanket claim. The table now carries the exact over-limit status and body of each row: 413 + ApiError for the two #334 endpoints, 400 {"error":"invalid JSON"} as text/plain for /api/paths/inspect, 400 {"error":"invalid request body"} for /api/channel-proposals, 400 {"error":"invalid JSON"} for PUT /api/config/geo-filter;
• POST /api/admin/prune-geo-filter?confirm=true (API key) decodes a JSON body with no limit at all — the fourth counterexample to "every endpoint", found while auditing the route table. Listed as a known gap;
• info.description is scoped the same way and names docs/api-spec.md for the detail.

The extra correction (commit dfb9a515). Writing "POST /api/paths/inspect | 4096 bytes" would still have misled a client, because the three pre-#334 limits are not byte caps. They wrap the body and hand it to a streaming json.Decoder, so the limit only bites when the JSON value runs past it. Measured against the live binary [A]:

POST /api/paths/inspect   {"prefixes":["aa"]} + 6000 B of trailing space (6019 B)  -> 200
POST /api/decode          {"hex":"0200"}      + 6000 B of trailing space (6019 B)  -> 413
POST /api/paths/inspect   {"prefixes":["aa"],"pad":"A"*5000} (5028 B)              -> 400, text/plain

So the table gained an "Enforced on" column (commit dfb9a515) (bytes received, before parsing vs the streaming decoder), both models are spelled out with that measurement as the worked example, and the weaker enforcement is a known gap in its own right next to the status-code mismatch. This is also the clearest statement yet of why this PR reads into a bounded buffer instead of streaming — the reviewer's mutant B measured the same weakness from the other side.

I did not align the three 400 endpoints on 413: it changes three handlers this issue does not own, two of them behind an API key, and it is API-visible for any client that distinguishes the codes. Documented honestly instead, and listed under Rester.

3. F3 (nit) — trailing data. Agreed it should be a decision. TestBodyLimitsRejectTrailingData covers junk after the object and a second JSON object, on both endpoints (4 cases), plus trailing whitespace still returning 200. The PR description now has a "Deliberate behaviour change: trailing data is now rejected" section with the three 200→400 bodies the reviewer measured and the reason (matches decodeSingleJSONObject; JSON.stringify cannot produce trailing data).

4. F4 (nit) — map[string]interface{} occurrence. Fixed as suggested: the "responses" literal is hoisted into a local responses variable, routeMeta.Errors is applied to it before the operation map is built, and the type assertion is gone. Counts [A]: cmd/server/openapi.go 282 → 281, matching master; non-test total across cmd/ + internal/ 746 → 745, matching master exactly.

5. F5 (nit) — numbers and summary. Agreed; 3812 B is right and my 4013 B was wrong. Corrected throughout the PR description: the headroom table (3812 B, ~17.2x), the justification prose (284 bytes of slack under a 4 KiB cap, not 83), and the curl log. Recomputed independently [A]: {"hashes":[ (11) + 200x18 + 199 commas + ]} (2) = 3812; 201 hashes = 3831; 4096 − 3812 = 284; 65536 / 3812 = 17.2x. The OpenAPI summary for POST /api/packets/observations is now "Get observations for several packets" (was "Batch submit observations" next to a description saying it returns them), pinned by a test.

Mutants

Each applied to a clean tree, run, then reverted and the tree verified clean with git diff.

#	Finding	Mutant	Result
A	F1	MaxBytesReader dropped; unbounded io.ReadAll + json.Unmarshal, size checked afterwards (the reviewer's surviving mutant)	Killed — 4 subtests across both new tests; read 8388608 B of an endless body against a 4096 B cap
B	F2	Caps table reverted to POST /api/path-inspect / blanket 413	Killed — TestAPISpecDocumentsBodyLimits + TestAPISpecCapsTableMatchesRealBehaviour, incl. the "non-existent route" guard
C	F2	info.description restored to "Endpoints that take a request body … answer 413"	Killed — TestOpenAPIDescriptionDoesNotOverclaim413, 3 assertions
D	F5	Summary restored to "Batch submit observations"	Killed — TestBatchObservationsSummaryMatchesBehaviour
E	F3	json.Unmarshal swapped back for a json.Decoder that ignores trailing data	Killed — all 4 trailing-data subtests, 200 instead of 400
F	F4	responses[e.Status] = jsonErrorResponse(...) loop removed after the hoist	Killed — TestOpenAPIDocuments413OnBodyLimitedRoutes, both routes down to [200]

Mutant F also confirms the hoist did not quietly drop the 413 metadata it refactors.

Tests

Local, in this worktree, on the merged tree:

• cd cmd/server && go test ./... -count=1 -timeout 45m — ok, 40.2 s [T]
• cd cmd/ingestor && go test ./... -count=1 -timeout 45m — ok, 105.1 s [T] (untouched by this PR; run because of the master merge)
• sh test-all.sh — 234 passed, 0 failed (234 files) [T]
• node test-frontend-helpers.js — 712 passed, 0 failed [T]
• scripts/check-xss-sinks.sh --diff origin/master — clean, no public/** changes to scan [A]
• gofmt -l on the five touched files (request_body_limits.go, request_body_limits_test.go, openapi.go, routes.go + the doc) — empty. Many pre-existing cmd/server files are not gofmt-clean; none of them are touched here. go vet ./... clean [A]

E2E. A local Go server on port 13801 against test-fixtures/e2e-fixture.db prepared exactly as CI does: tools/freshen-fixture.sh, the #1486/#1791 seed SQL block, corescope-migrate, then seeds 2073, 199 and 245, with scripts/instrument-frontend.sh and -public public-instrumented. Stopped afterwards; the fixture DB was restored with git checkout and the build artifacts removed, so the tree is clean.

• test-e2e-playwright.js — 132/135 passed, 3 skipped [T] (the 3 skips are pre-existing)
• test-issue-1487-byop-modal-layout-e2e.js — 4/4 [T]
• test-map-modal-fluid-e2e.js — 10/10 [T] (covers the BYOP decode modal, the only /api/decode client)
• test-issue-147-packets-url-obs-e2e.js — 12/12 [T]
• test-packet-detail-sender-hash-size-obs-e2e.js — 3/3 [T]

Live checks against the built binary [A], all on the same server:

POST /api/decode  4096 B                     -> 200
POST /api/decode  4097 B                     -> 413 application/json
                                                {"error":"request body too large (max 4096 bytes)"}
POST /api/decode  {"hex":"0200"} trailing    -> 400
POST /api/decode  {"hex":"0200"}{"hex":"zz"} -> 400
POST /api/packets/observations {"hashes":[]} junk -> 400
POST /api/path-inspect                       -> 404   (the route the old doc named)
POST /api/paths/inspect                      -> 200
GET  /api/spec  summary   = "Get observations for several packets"
                responses = ['200','413'] on both routes; ApiError present
                info.description scoped to the two #334 endpoints

CI (per job)

Run 37743709836, attempt 1, head 1e3eccf2. Green on the first attempt; no reruns.

Job	Result
✅ Go Build & Test	success, 23m18s — go test for server + ingestor, the ingestor race-check, internal/anomaly with -race, CSS-var lint, test-all.sh, the preflight XSS gate (--diff), eslint, proto syntax
🎭 Playwright E2E Tests	success, 28m28s — full fail-fast E2E run plus the Kpa-clawbot#1616 slide-over flake-gate
🏗️ Build & Publish Docker Image	success, 54s
📦 Release Artifacts	skipped (not a tag)
🚀 Deploy Staging	skipped
📝 Publish Badges & Summary	skipped

Neither known-unstable test (#271, #301) failed, so no job needed a second run. The intermediate run on dfb9a515 (37742364857) shows as cancelled: the 1e3eccf2 push superseded it mid-flight via the workflow's concurrency group, not a failure.

Rester

• [K] The three streaming-bounded endpoints are not tightened. POST /api/paths/inspect, POST /api/channel-proposals and PUT /api/config/geo-filter are bounded on the decoder, not on bytes, and answer 400 rather than 413 + ApiError. Both gaps are now stated in docs/api-spec.md with the measurement above. Converting them to decodeLimitedJSONBody is a small code change but an API-visible one across three handlers this issue does not own, so it belongs in its own change with its own clients checked.
• [K] POST /api/admin/prune-geo-filter?confirm=true (API key) reads a JSON body with no limit at all. Found while auditing the route table for F2. Documented as a gap; capping it is the same separate change.
• [K] PUT /api/config/geo-filter's over-limit 400 was verified by reading the handler, not against a live request — it is behind requireAPIKey and I did not use an API key. The limit value itself is pinned to geoFilterBodyLimit by a test, and POST /api/channel-proposals's over-limit 400 is covered by the pre-existing TestChannelProposalSubmitValidatesAndQueues [T].
• [K] public/packets.js can build a batch request larger than the handler accepts — unchanged from round 1 and from the reviewer's own assessment: pre-existing, already a swallowed 400 too many hashes, and the real fix is client-side chunking in its own issue.
• [K] Both #334 caps are still hardcoded constants with a TODO(#334, AGENTS.md rule 8); making them operator-configurable is a later milestone.
• [K] Not verified: behaviour behind a reverse proxy, or keep-alive reuse after a 413 — same gap the reviewer noted. No staging or prod access was used; no API key was used.