Conversation
When os.Open fails for a scan target, Fragments logged a warning only for permission-denied and otherwise stayed completely silent, then returned nil regardless -- so a scan that silently missed a file still printed "no leaks found" and exited 0. The partial-scan machinery in cmd/root.go (the "no leaks found in partial scan" message and exit 1) already exists for exactly this situation but was unreachable, because nothing on this path ever produced a non-nil error. Adds an atomic counter to Files, incremented whenever a scan target can't be opened for any reason (not just permission-denied, which is now logged with the actual error instead of a generic message). After the scan completes, the count is folded into the returned error via the new withUnreadableFilesError helper, joined with any existing walk error, so the caller's partial-scan handling now actually fires. Tests: - sources/files_test.go: TestFiles_withUnreadableFilesError covers the helper directly (no unreadable files, unreadable files alone, and combined with an existing walk error) -- portable, no filesystem permissions involved. - detect/detect_test.go: TestDetectSkipsUnreadableFileAndReportsPartialScan is an end-to-end regression through the real DetectSource path: one readable file with a real secret, one chmod 0000 file in the same directory. Asserts the readable file's finding still surfaces (an unreadable sibling doesn't take down the whole scan) and the returned error mentions the skipped file. Skipped on Windows/root, following the same pattern already used by TestDetectWithSymlinks -- chmod-based permission denial isn't portable to either. Verified go build/go vet clean, sources package tests pass in full (including on Windows, where this was developed), and the Windows-only detect-level test correctly registers and skips rather than silently not running. Fixes gitleaks#2232
hamodywe
requested review from
bryanbeverly,
dustin-decker,
dxa4481 and
zricethezav
as code owners
August 8, 2026 17:39
This branch has not been deployed
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.
When os.Open fails for a scan target, Fragments logged a warning only for permission-denied and otherwise stayed completely silent, then returned nil regardless -- so a scan that silently missed a file still printed "no leaks found" and exited 0. The partial-scan machinery in cmd/root.go (the "no leaks found in partial scan" message and exit 1) already exists for exactly this situation but was unreachable, because nothing on this path ever produced a non-nil error.
Adds an atomic counter to Files, incremented whenever a scan target can't be opened for any reason (not just permission-denied, which is now logged with the actual error instead of a generic message). After the scan completes, the count is folded into the returned error via the new withUnreadableFilesError helper, joined with any existing walk error, so the caller's partial-scan handling now actually fires.
Tests:
Verified go build/go vet clean, sources package tests pass in full (including on Windows, where this was developed), and the Windows-only detect-level test correctly registers and skips rather than silently not running.
Fixes #2232
Description:
Explain the purpose of the PR.
Checklist: