fix(sandbox): cap the Go daemon's file-read path at 10MB - #6215
Open
pedrofrxncx wants to merge 1 commit into
Open
fix(sandbox): cap the Go daemon's file-read path at 10MB#6215pedrofrxncx wants to merge 1 commit into
pedrofrxncx wants to merge 1 commit into
Conversation
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.
Follows the same bug class as open PR #6039 (
/editfile-read cap), but on a different route:Read's text path (packages/sandbox/daemon-go/internal/routes/fs.go)./readalready caps its image path atmaxImageBytes(5MB), but for a text file it falls through to an unconditionalos.ReadFile(filePath)regardless of size. A large text file (a log, a generated artifact, an oversized decofile) gets read fully into memory with no bound, which can OOM the daemon and tear down the sandbox pod on the next missed health probe — the exact failure mode the file's owndecodeBodycomment already documents for the request-body side.Added a
maxTextReadBytes(10MB, matching the cap #6039 is adding to/edit) check right before the read, returning a 400 with the file size and cap instead of buffering the whole file.Failure scenario: an agent runs
/readagainst a >10MB text file in the sandbox → daemon buffers the entire file into memory → repeated/concurrent large reads can push the daemon over its memory limit and get the pod killed mid-session.Regression test:
TestReadRejectsOversizedTextFile(fixture file just over the cap, expects 400 with a "too large" message) andTestReadAllowsTextFileUnderCap(small file still reads fine) infs_test.go.Reviewer check:
cd packages/sandbox/daemon-go && go test ./internal/routes/...Locally ran:
go build ./...,go test ./internal/routes/...,gofmt -l(clean),go vet ./internal/routes/...(clean). Full CI validates the rest.Summary by cubic
Caps text-file reads in the sandbox Go daemon’s
/readroute at 10MB to prevent OOMs. Previously, the text path used unboundedos.ReadFile; now requests for files >10MB return 400 with the file size and cap. This aligns with the/edit10MB cap.Review notes
maxTextReadBytesand a pre-read size check inpackages/sandbox/daemon-go/internal/routes/fs.go.TestReadRejectsOversizedTextFileandTestReadAllowsTextFileUnderCapinpackages/sandbox/daemon-go/internal/routes/fs_test.go.cd packages/sandbox/daemon-go && go test ./internal/routes/...Written for commit 134f0f2. Summary will update on new commits.