fix: prevent HTTP response body leaks in CLI API methods#336
Open
1Ckpwee wants to merge 1 commit into
Open
Conversation
Add defer resp.Body.Close() immediately after receiving HTTP responses in TellPlan, BuildPlan, RespondMissingFile, and ConnectPlan. Move the defer in GetCliTrialSession before the early 404 return to prevent a leak on that path. Remove redundant explicit resp.Body.Close() calls that are now covered by defer.
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.
Summary
Several API methods in
app/cli/api/methods.gohad HTTP response body leak issues:GetCliTrialSession:defer resp.Body.Close()was placed after an early return on 404, so the body was never closed on that pathTellPlan: Nodefer resp.Body.Close()at all — body only closed in the non-streaming branch, leaked on error pathsBuildPlan: Same pattern as TellPlanRespondMissingFile: Nodefer resp.Body.Close()anywhereConnectPlan: Nodefer resp.Body.Close()— body leaked on error pathsThese leaks can cause resource exhaustion in long-running CLI sessions, especially when many API calls are made in succession.
Changes
defer resp.Body.Close()immediately after the response is received in all affected methodsresp.Body.Close()calls that are now covered by deferTest plan
plandex tell,plandex build, andplandex connectcommands