Skip to content

Fix file descriptor leak in GCP FGetObject on error paths#2053

Open
SebTardif wants to merge 1 commit into
fluxcd:mainfrom
SebTardif:fix/gcp-fgetobject-file-descriptor-leak
Open

Fix file descriptor leak in GCP FGetObject on error paths#2053
SebTardif wants to merge 1 commit into
fluxcd:mainfrom
SebTardif:fix/gcp-fgetobject-file-descriptor-leak

Conversation

@SebTardif
Copy link
Copy Markdown

@SebTardif SebTardif commented May 17, 2026

Summary

objectFile opened with os.OpenFile in GCSClient.FGetObject is only closed on the happy path. If NewReader or io.Copy fails, the file descriptor leaks.

Fix

Close objectFile on each error path with checked close errors, matching the pattern used by the Azure BlobClient.FGetObject implementation (blob.go:328-336).

The initial version used defer objectFile.Close(), which CodeQL flagged as discarding the close error on a writable file handle (code-scanning/6). The updated fix uses explicit close-on-error instead, consistent with the existing Azure pattern.

Error paths fixed

Error path Before After
NewReader fails fd leaks objectFile.Close() called, error logged
io.Copy fails fd leaks objectFile.Close() called, error logged
Happy path objectFile.Close() checked unchanged

Origin

The leak was introduced in #434 (2021-09-01) when GCS support was added.

Comment thread internal/bucket/gcp/gcp.go Fixed
objectFile opened with os.OpenFile is only closed on the happy path.
If NewReader or io.Copy fails, the file descriptor leaks. Close
objectFile on each error path with checked close errors, matching the
pattern used by the Azure BlobClient.FGetObject implementation.

The leak was introduced in fluxcd#434 (2021-09-01).

Assisted-by: Grok <noreply@x.ai>
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@SebTardif SebTardif force-pushed the fix/gcp-fgetobject-file-descriptor-leak branch from 2638e7b to 4ec284f Compare May 18, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants