fix: bound network response body reads - #770
Merged
Merged
Conversation
Limit allocation on Vendor API, login token, CMX metadata, checksum, and OCI manifest response bodies with generous ceilings that preserve normal use.
divolgin
reviewed
Jul 27, 2026
| return nil | ||
| } | ||
|
|
||
| func readAllLimited(r io.Reader, limit int64) ([]byte, error) { |
Member
There was a problem hiding this comment.
This seems unnecessary. This can potentially turn a successful API request into an error causing the client to retry. But also this is a CLI tool, not a long running service, so response size should not be a major issue.
Per review: this is a short-lived CLI talking to a trusted Vendor API. Hard caps can turn successful large responses into client errors without real security benefit. Keep limits only on small fixed-shape payloads.
divolgin
approved these changes
Jul 27, 2026
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
2 participants
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.
Why this matters
Some production paths buffered entire network response bodies with unbounded
io.ReadAll. For small, fixed-shape payloads (token JSON, CMX metadata, checksum text, OCI manifests), a modest ceiling is a reasonable guard without changing normal product behavior.Review feedback
Vendor API responses in
pkg/platformclientare not limited. Per review: this is a short-lived CLI against a trusted Vendor API; a hard cap can turn a successful large release payload into a client error without meaningful security benefit. That change was dropped.What remains bounded
Explicit non-goals
platformclient)Test plan
go test ./pkg/platformclient/ ./pkg/credentials/ ./pkg/tools/ ./cli/cmd/io.ReadAllagain (no size cap)