Fix APK handler spinning on closed temp file after context timeout#4870
Draft
dipto-truffle wants to merge 1 commit intomainfrom
Draft
Fix APK handler spinning on closed temp file after context timeout#4870dipto-truffle wants to merge 1 commit intomainfrom
dipto-truffle wants to merge 1 commit intomainfrom
Conversation
The APK handler's processAPK loop did not check for context cancellation, causing it to iterate through every file in a zip archive even after the 60s maxTimeout fires and the underlying temp file is closed. This produces thousands of "file already closed" error logs per APK and wastes scanner resources, starving co-located scan jobs. Add a common.IsDone(ctx) check at the top of the file iteration loop, matching the established pattern in archive.go, ar.go, and rpm.go. Made-with: Cursor
4e68573 to
f7c44b4
Compare
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
processAPKloop inpkg/handlers/apk.godid not check for context cancellation, causing it to iterate through every file in a zip archive even after the 60smaxTimeoutfires and the underlying temp file is closed. This produced ~2000 "file already closed" error log entries per APK and wasted scanner CPU/IO, contributing to slow scans for co-located jobs.common.IsDone(ctx)check at the top of the file iteration loop, matching the established pattern already used byarchive.go,ar.go, andrpm.goin the same package.processAPKexits promptly on both context cancellation and deadline exceeded.Vendors trufflesecurity/thog#6007
Test plan
go test ./pkg/handlers/ -run TestProcessAPK_ExitsOn -vpasses (context cancellation + deadline exceeded)Made with Cursor
Note
Low Risk
Low risk: adds an early-exit context check in the APK file iteration loop and corresponding unit tests, with minimal impact on normal processing aside from stopping work sooner on cancellation/timeouts.
Overview
Ensures APK scanning stops promptly when the processing context is cancelled or times out by adding a
common.IsDone(ctx)guard inapkHandler.processAPKbefore iterating each zip entry.Adds unit tests that build a minimal in-memory APK and assert
processAPKreturnscontext.Canceledandcontext.DeadlineExceededrather than continuing to process files after cancellation.Reviewed by Cursor Bugbot for commit f7c44b4. Bugbot is set up for automated code reviews on this repo. Configure here.