Skip to content

fix(server): wrap JSON body decoders with MaxBytesReader to prevent OOM (PILOT-134)#8

Closed
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-134-20260528-154142
Closed

fix(server): wrap JSON body decoders with MaxBytesReader to prevent OOM (PILOT-134)#8
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-134-20260528-154142

Conversation

@matthew-pilot
Copy link
Copy Markdown
Collaborator

What failed

POST /admin/recrawl, /admin/recrawl-by-domain, /feedback, /admin/reembed, and /contents (batch) all decoded r.Body with json.NewDecoder without a size limit. A large JSON array (e.g. 100M-element URL list) would OOM the server during parsing — the len(req.URLs) > N guard ran only AFTER decode.

Why this fix

Add maxRequestBodySize = 1 MiB and wrap all five decoder sites with http.MaxBytesReader(w, r.Body, maxRequestBodySize). Oversized bodies now fail at the transport layer before the JSON decoder allocates.

Verification

  • go build ./...
  • go vet ./... ✅ (pre-existing test warnings only)
  • go test ./... ✅ (9/9 packages pass)
  • New test: TestAdminRecrawlRejectsOversizedBody — sends 2 MiB body, confirms 400 rejection

Closes PILOT-134

…OM (PILOT-134)

POST /admin/recrawl, /admin/recrawl-by-domain, /feedback, /admin/reembed,
and /contents (batch) all decoded r.Body with json.NewDecoder without a
size limit. A large JSON array (e.g. 100M-element URL list) would OOM the
server during parsing — the len(req.URLs) > N guard ran only AFTER decode.

Add maxRequestBodySize = 1 MiB and wrap all five decoder sites with
http.MaxBytesReader(w, r.Body, maxRequestBodySize). Oversized bodies now
fail at the transport layer before the JSON decoder allocates.

TestAdminRecrawlRejectsOversizedBody: sends a 2 MiB body and confirms
400/413.

Closes PILOT-134
@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/server/http.go 80.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🦜 Matthew PR Check — #8 PILOT-134

Status

  • State: OPEN · MERGEABLE ✅
  • CI: 3/3 green (test ✅, codecov/patch ✅, security/snyk ✅)
  • Created: 2026-05-28 15:43 UTC
  • Files: 1 (+5 × MaxBytesReader wraps in http.go, +1 test in http_test.go)

Verdict

CLEAN — all CI green, mergeable, no conflicts. Ready for review.


🤖 matthew-pilot · PILOT-134

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🦜 Matthew Explains — #8 PILOT-134

What this does

Wraps all JSON body decoders in the cosift HTTP server with http.MaxBytesReader, capping request bodies at 1 MiB. This prevents a malicious or accidentally oversized JSON payload from causing OOM during json.NewDecoder(r.Body).Decode() — the stdlib decoder buffers the entire body before parsing.

Changes

  • internal/server/http.go: Added maxRequestBodySize = 1 << 20 constant. Wrapped 5 decoder sites: handleFeedback, handleAdminRecrawl, handleAdminRecrawlByDomain, handleAdminReembed, handleContentsBatch.
  • internal/server/http_test.go: Added TestAdminRecrawlRejectsOversizedBody — sends a 2 MiB valid JSON body and asserts HTTP 400 or 413.

Risk

Low. MaxBytesReader is a standard Go stdlib guard (net/http). The 1 MiB limit is generous for all existing cosift endpoints. No behavioral change for well-formed requests under the limit. The test validates both the limit and graceful error handling.

Jira

PILOT-134


🤖 matthew-pilot

@TeoSlayer
Copy link
Copy Markdown
Contributor

Superseded by #6 (PILOT-107) which added MaxBytesReader to handleFeedback, handleAdminRecrawl, and handleAdminRecrawlByDomain. Verified the same MaxBytesReader cap is present on those handlers at main HEAD.

@TeoSlayer TeoSlayer closed this May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants