Repository navigation
Conversation
This branch has not been deployed
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.
Description
downloadImageFromURL()callsDownloadBytes(), which currently uses an unboundedio.ReadAll. A remote image response can therefore consume memory beyond the configured file size limit, even when it has no Content-Length or understates its size.Reuse
GetMaxFileSize()(MAX_FILE_SIZE_MB, default 50 MiB). Reject oversized Content-Length before reading, then read at most the limit plus one byte to detect oversized streams. Return an explicit error without partial data and close the response body on every path.Add nine cases covering small/empty bodies, exact-limit bodies with and without Content-Length, oversized declared and streamed bodies, understated lengths, HTTP failures, read failures, and body closure. The three oversized cases fail on the original implementation. The shared SSRF client and validation remain in the download path.
Type of Change
Related Issue
None; found while reviewing the current remote-image download path.
Testing
Tested on Windows with Go 1.26.2. The following commands use the actual source files and existing SSRF tests, without replacement implementations or stubs for the SSRF policy:
go test internal/utils/httputil.go internal/utils/filesize.go internal/utils/security.go internal/utils/ssrf_outbound_cache.go internal/utils/shell_assignment.go internal/utils/httputil_test.go internal/utils/filesize_test.go internal/utils/security_test.go internal/utils/security_transport_test.go internal/utils/security_redirect_test.go internal/utils/security_whitelist_only_test.go internal/utils/ssrf_outbound_cache_test.go internal/utils/shell_assignment_test.go -count=1 go vet internal/utils/httputil.go internal/utils/filesize.go internal/utils/security.go internal/utils/ssrf_outbound_cache.go internal/utils/shell_assignment.go internal/utils/httputil_test.go gofmt -l internal/utils/httputil.go internal/utils/httputil_test.go git diff --checkAll above checks pass; gofmt reports no files. The new download test uses an in-memory HTTP transport and a counting response body, so it makes no network requests and checks that reads stop at the limit plus one byte.
Package-level validation was attempted with
go test ./internal/utils -run '^TestDownloadBytesSizeLimit$' -count=1. It cannot compile the unrelated SQL validator on this machine becauseCGO_ENABLED=0and no C compiler is available:internal/utils/inject.goreports undefinedpg_query.Parse/pg_query.Deparse. The initial run also reports a Go temporary-file cleanup permission error; source-scoped checks pass with GOTMPDIR inside the workspace. Full-repository checks and golangci-lint were not run; golangci-lint is not installed locally.Checklist
git diff --checkpassesRemote images exceeding the existing configured file limit now return an error. There are no API schema changes.