Repository navigation
Security: Handle Close() errors on writable files in zip extraction - #9332
Merged
Merged
Conversation
…mprehensive tests Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix unhandled writable file close error in extractZipFile function
Security: Handle Close() errors on writable files in zip extraction
Jan 8, 2026
pelikhan
marked this pull request as ready for review
January 8, 2026 12:19
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.
Security Fix: CWE-252 (Unchecked Return Value)
The
extractZipFilefunction opened files for writing but ignored Close() errors. Since buffered writes may only flush on close, this could cause silent data loss if Close() failed (disk full, I/O errors).Changes
(extractErr error)to enable error capture in deferred closuredefer destFile.Close()with deferred function that captures and propagates Close() errorsTest Coverage
Added comprehensive test suite covering:
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/user/usr/bin/gh gh api user --jq .login 7748965/.github/workflows -tests /usr/bin/infocmp -json GO111MODULE 64/bin/go infocmp -1 /ref/tags/v8 go /usr/bin/infocmp 100351/run-1 GO111MODULE x_amd64/link infocmp(http block)/usr/bin/gh gh api user --jq .login ithub-script/git/ref/tags/v8 rev-parse(http block)/usr/bin/gh gh api user --jq .login /run/containerd/io.containerd.runtime.v2.task/moby/54dbac9f144934f7a4a699d3772c740a82c13b79bd7a0c256529e6deff339c00/init.pid 54dbac9f144934f7a4a699d3772c740a82c13b79bd7a0c256529e6deff339c00 /usr/bin/git 40a82c13b79bd7a0c256529e6deff339c00/log.json GO111MODULE ache/go/1.25.0/x/repos/actions/github-script/git/ref/tags/v8 git -C ithub/workflows rev-parse(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
This section details on the original issue you should resolve
<issue_title>Security Fix: Handle errors from closing writable file (Alert #501)</issue_title>
<issue_description>## Security Fix: Unhandled Writable File Close Error
Alert Number: #501
Severity: Warning
Rule: go/unhandled-writable-file-close (CWE-252)
File: pkg/cli/logs_download.go:397
Vulnerability Description
The
extractZipFilefunction opens a file for writing usingos.OpenFilewithos.O_WRONLYflag, but does not handle errors that may occur when closing the file. Data written to a file may be cached in memory and only flushed when the file is closed. IfClose()fails and the error is ignored, data loss can occur silently without the program being aware of it.The CodeQL scanner flagged this as: "File handle may be writable as a result of data flow from a call to OpenFile and closing it may result in data loss upon failure, which is not handled explicitly."
Fix Applied
Applied the recommended pattern from the CodeQL documentation:
Changed function signature to use a named return value:
func extractZipFile(...) (extractErr error)Replaced
defer destFile.Close()with a deferred anonymous function that:destFile.Close()and captures any errorUpdated the
io.Copyerror handling to use the named return value for consistencyCode Changes
Before:
After:
Security Best Practices Applied
Testing Considerations
References
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
To apply the patch locally:
Show patch preview (58 of 58 lines)