fix(sandbox): cap the Go daemon's inline dispatch request body size - #6447
Merged
Conversation
The /dispatch route buffered r.Body with an unbounded io.ReadAll while its sibling offload path (FetchOffloadedMessages) already caps at maxOffloadBytes (32MB). A large inline dispatch body had no such limit, letting one request park up to the full body size in memory on the pod. Wraps the read in http.MaxBytesReader at maxOffloadBytes and answers 413 on overflow instead of buffering past the cap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Source: bounded resource fix (C3), same DoS-bound category as the already-merged #6215/#6039 fs caps and the open #6252 config-body cap — this is the
/dispatchentry point, which none of those touch.The gap:
HandleDispatchinpackages/sandbox/daemon-go/internal/dispatch/dispatch.goread the request body with a plainio.ReadAll(r.Body)— no size limit. Its sibling path,handleOffloadDispatch→FetchOffloadedMessages(offload.go), already caps external message fetches atmaxOffloadBytes(32MB) with aContent-Length/read-limit check specifically because a large payload is expected to go through that offload path, not inline. The inline path had no equivalent bound, so one oversized POST to/dispatchcould buffer arbitrarily large bytes into the pod's memory before validation even runs.Fix: wrap the read in
http.MaxBytesReader(w, r.Body, maxDispatchBodyBytes)(aliased to the existingmaxOffloadBytesconstant, so the two caps stay in sync) and answer413 body_too_largeviaerrors.Ason*http.MaxBytesErrorinstead of buffering past the cap.Verification:
TestDispatchRejectsOversizedBodysends amaxDispatchBodyBytes+1-byte body throughHandleDispatchand asserts a 413. Rango build ./...,go vet ./internal/dispatch/...,gofmt -l(clean), andgo test ./internal/dispatch/...(full package, all green) locally — CI runs the rest.A reviewer can confirm with:
cd packages/sandbox/daemon-go && go test ./internal/dispatch/... -run TestDispatchRejectsOversizedBody -v.Summary by cubic
Caps the Go sandbox daemon’s inline
/dispatchrequest body to prevent unbounded memory use. Previouslyio.ReadAllbuffered the full body; now the read is limited tomaxOffloadBytes(32MB) and oversized bodies return 413.http.MaxBytesReaderinHandleDispatch; on overflow returns 413body_too_largebefore buffering. Other read/JSON errors still return 400bad_json.maxOffloadBytes.TestDispatchRejectsOversizedBodyto assert the 413 response.Written for commit 1572591. Summary will update on new commits.