fix: bound HTTP response reads and use sync.Once for config init

[appleboy]

Summary

• H-1: Wrap all 5 io.ReadAll(resp.Body) calls with io.LimitReader(resp.Body, 1MB) to prevent memory exhaustion from malicious/compromised servers (DoS)
• H-2: Replace non-thread-safe configInitialized bool with sync.Once for safe concurrent config initialization

Test plan

• make test — all existing tests pass
• make lint — no warnings

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

❌ Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
main.go	33.33%	6 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR hardens the CLI’s HTTP handling and config initialization by bounding HTTP response body reads to mitigate memory-exhaustion DoS risks and making config initialization concurrency-safe.

Changes:

• Limit all HTTP io.ReadAll(resp.Body) usages to a 1MB maximum via io.LimitReader.
• Replace the non-thread-safe config initialization guard with sync.Once (via configOnce).

Comments suppressed due to low confidence (1)

main.go:449

• io.LimitReader will truncate the body at maxResponseBodySize without indicating that truncation happened. That can lead to confusing downstream errors (e.g., json.Unmarshal returning unexpected EOF) and makes it hard to distinguish "malicious/oversized response" from "invalid JSON". Consider reading maxResponseBodySize+1 bytes and explicitly erroring if the limit is exceeded (and optionally returning a dedicated "response too large" error) before attempting to parse/use the body.

	body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize))
	if err != nil {
		return nil, fmt.Errorf("failed to read response: %w", err)
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When reading the API response body, io.LimitReader prevents large allocations but doesn't indicate truncation. If the upstream returns >maxResponseBodySize, you'll operate on a truncated body. Consider detecting and returning a specific "response too large" error (e.g., read max+1 and check length).

[Copilot]

In the 401 retry path, the first resp.Body is only closed via the earlier deferred close, so it stays open while refreshAccessToken and the retry request run. That can temporarily hold onto connections/resources unnecessarily. Consider explicitly closing (and optionally draining) the original resp.Body immediately before starting the refresh/retry, and only deferring the close for the response you actually read from.

[Copilot]

This change introduces a new security boundary (maxResponseBodySize) but there are no tests exercising the oversized-response path (e.g., server returns >1MB, client should fail in a well-defined way). Since this file already has extensive HTTP/flow tests, consider adding a test case that returns a payload just over the limit and asserts the resulting error (ideally a dedicated "response too large" error if you add explicit truncation detection).

[Copilot]

Same pattern as elsewhere: io.LimitReader truncates silently at maxResponseBodySize, so an oversized response will be treated as valid input for later parsing/handling and may surface as misleading unexpected EOF/JSON errors. Consider reading maxResponseBodySize+1 and returning an explicit "response too large" error when exceeded.

Suggested change

	body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize))
	if err != nil {
	return nil, fmt.Errorf("failed to read response: %w", err)
	}
	// Read up to maxResponseBodySize+1 bytes so we can detect oversized responses
	body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize+1))
	if err != nil {
	return nil, fmt.Errorf("failed to read response: %w", err)
	}
	if len(body) > int(maxResponseBodySize) {
	return nil, fmt.Errorf("response too large")
	}

[Copilot]

io.ReadAll(io.LimitReader(...)) will not tell you whether the server sent more than maxResponseBodySize; it just stops at the limit. Consider detecting oversize bodies (read max+1, check length) so callers get a clear "response too large" error rather than parse/format errors from truncated content.

Suggested change

	body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize))
	if err != nil {
	return fmt.Errorf("failed to read response: %w", err)
	}
	limitedReader := io.LimitReader(resp.Body, int64(maxResponseBodySize)+1)
	body, err := io.ReadAll(limitedReader)
	if err != nil {
	return fmt.Errorf("failed to read response: %w", err)
	}
	if int64(len(body)) > int64(maxResponseBodySize) {
	return fmt.Errorf("failed to read response: response body too large")
	}

[Copilot]

Because io.LimitReader truncates without error, a response slightly over maxResponseBodySize will be truncated and then unmarshaled/printed, producing confusing errors and potentially incomplete diagnostics. Consider explicitly checking for truncation (read max+1 and error if exceeded).

Suggested change

	body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize))
	if err != nil {
	return credstore.Token{}, fmt.Errorf("failed to read response: %w", err)
	}
	// Read up to maxResponseBodySize+1 bytes so we can detect truncation explicitly.
	body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodySize+1))
	if err != nil {
	return credstore.Token{}, fmt.Errorf("failed to read response: %w", err)
	}
	if len(body) > maxResponseBodySize {
	return credstore.Token{}, fmt.Errorf("failed to read response: body exceeds maximum allowed size")
	}