fix: Buffer request body for retry overrides, close intermediate response bodies

[danskmt]

Description

Fixes CLI-1591 and addresses several connection/resource management gaps in the retry middleware.

1. Always buffer request body for retry overrides

POST request bodies were not preserved across retry attempts when a 429 triggered the per-status-code retry override (3 attempts despite maxAttempts=1).

Root cause: Body buffering was gated on maxAttempts > 1, but the per-status-code override in getMaxRetryAttempts is only evaluated after the first response — by which point the body was already consumed and never buffered.

Fix: ensureGetBodyExists now always prepares a replayable body using three strategies in order of preference:

1. GetBody already set (e.g. http.NewRequest with *bytes.Reader) → use as-is, no copy.
2. Body implements io.ReadSeeker (e.g. *os.File) → wrap to suppress Close, rewind with Seek(0).
3. Fallback → io.ReadAll into memory buffer.

2. Close intermediate response bodies between retries

The middleware returned (response, retryErr) on retriable status codes, but backoff.Retry discarded intermediate responses without closing their bodies — leaking one TCP connection per retry. This matches the drain+close behavior in stdlib's Client.do.

drainAndClose is called eagerly when the retry decision is made (non-permanent errors). On permanent 503 where getErrorList replaces response.Body with a buffer copy, the orphaned original transport body is also closed.

3. Cleanup: remove dead code

Removed redundant noCloseSeekBody.Read method — the embedded io.ReadSeeker already promotes Read.

Ticket: CLI-1591

Test coverage

Scenario	Test
POST body preserved across 429 retries	TestRetryMiddleware_429_POST_BodyPreservedAcrossRetries
GetBody preset skips buffering	TestRetryMiddleware_GetBodyPreset_SkipsBuffering
NopCloser body falls back to ReadAll	TestRetryMiddleware_GetBodyNil_BuffersBody
Seekable body rewinds without copy	TestRetryMiddleware_SeekableBody_NoBuffer
Intermediate response body closed (1 retry)	TestRetryMiddleware_IntermediateResponseBodyClosed
All intermediate bodies closed (3 retries)	TestRetryMiddleware_MultipleIntermediateResponseBodiesClosed
Context cancellation still closes body	TestRetryMiddleware_ContextCancellation_BodyClosed
Permanent 503 closes orphaned transport body	TestRetryMiddleware_503Permanent_OriginalBodyClosed

Checklist

• Tests added and all succeed (make test)
• Regenerated mocks, etc. (make generate)
• Linted (make lint)
• Test your changes work for the CLI

1. Clone / pull the latest CLI main.
2. Run go get github.com/snyk/go-application-framework@YOUR_LATEST_GAF_COMMIT in the cliv2 directory.
3. Run go mod tidy in the cliv2 directory.
4. Run the CLI tests and do any required manual testing.
5. Open a PR in the CLI repo now with the go.mod and go.sum changes.

• Once this PR is merged, repeat these steps, but pointing to the latest GAF commit on main and update your CLI PR.

PR in CLI: snyk/cli#6912

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[PeterSchafer]

This can probably be removed, but please verify.

[snyk-pr-review-bot]

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Memory Regression / OOM Risk 🟠 [major] The removal of the maxAttempts > 1 guard, combined with the fallback to io.ReadAll in ensureGetBodyExists, means that all POST/PUT requests with non-seekable bodies (and no pre-set GetBody) will now be fully buffered in RAM. While this supports the new per-status-code retry overrides (like 429), it forces O(N) memory usage on requests that were previously streamed. For large payloads (e.g., container images or large archives) that exceed available system memory, the CLI will now crash with an Out-of-Memory (OOM) error before the first request attempt is made. bodyBytes, readErr := io.ReadAll(req.Body)

📚 Repository Context Analyzed This review considered 10 relevant code sections from 8 files (average relevance: 1.00) pkg/networking/middleware/response.go (lines 1-164) pkg/networking/middleware/netstack_error_handler.go (lines 1-170) pkg/networking/middleware/auth_header.go (lines 1-162) pkg/networking/networking.go (2 segments, lines 1-215) pkg/mocks/networking.go (lines 1-213) internal/api/fileupload/uploadrevision/compression.go (lines 1-68) pkg/networking/logging.go (lines 1-179) ... and 1 more file(s)

[robertolopezlopez]

can you maybe return struct, error instead of all those firstBody, newGetBody, error, error

[danskmt]

Yeah we can probably consider that. Btw, there is one scenario where it is not nil:
wrapped.RealClose

Just a little bit hard to read the code being like this

[robertolopezlopez]

yes I saw it after my initial message ;-)

[danskmt]

I'm thinking of considering this type of cleanup for followup task

[robertolopezlopez]

sounds good to me