fix: honor changed_files from every config source and stop failing silently - #98
Open
John-David Dalton (jdalton) wants to merge 1 commit into
Open
fix: honor changed_files from every config source and stop failing silently#98John-David Dalton (jdalton) wants to merge 1 commit into
John-David Dalton (jdalton) wants to merge 1 commit into
Conversation
Contributor
Author
|
bugbot run |
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Aug 3, 2026
… the whole workspace Trivy's filesystem vulnerability scan is the one scanner that builds its own path list instead of going through Config.get_scan_targets(). Declining the staged-file substitution was not enough on its own: when the requested scope resolved to no scannable paths, scan_paths stayed empty and the existing fallback assigned the whole workspace, so the scan expanded to the full repository instead of skipping. Caught by Cursor Bugbot on #98.
Contributor
Author
|
bugbot run |
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Aug 3, 2026
… the event-payload base fallback The event-payload fallback reads a top-level pull_request.base, which covers pull_request, pull_request_target, pull_request_review and pull_request_review_comment. It does not cover issue_comment: that payload carries issue.pull_request, a set of URLs with no base ref or sha, so the base cannot be worked out without a GitHub API call. The docstring and docs claimed otherwise, and issue_comment is the trigger the change was motivated by. Correct the claim, and give that shape its own warning telling the workflow author to look the base up and pass GITHUB_BASE_REF, so an unsupported trigger reports itself instead of looking like an empty diff. Caught by Cursor Bugbot on #98.
Contributor
Author
|
bugbot run |
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Aug 3, 2026
…t meaning scan everything The scan_all warning claimed the changed-files scope was simply ignored. Only the scanners that ask Config.get_scan_targets() for their paths widen -- SAST does, while TruffleHog and Trivy read changed_files off the config themselves and stay scoped. Both settings together produce a mixed run, so the warning now says that instead of sending someone looking for a full-repo secret scan that never happens. Also stop the new Trivy empty-scope skip from firing under scan_all. scan_all is an explicit request to scan everything, and turning it into scanning nothing was a regression in the previous commit on this branch. Caught by Cursor Bugbot on #98.
Contributor
Author
|
bugbot run |
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
Author
|
bugbot run |
Contributor
Author
|
bugbot run |
Contributor
Author
|
bugbot run |
Contributor
Author
|
bugbot run |
lelia
requested changes
Aug 5, 2026
lelia
left a comment
Contributor
There was a problem hiding this comment.
Requesting changes for two remaining changed-file scoping gaps. I reproduced both locally: Trivy scans a configured Dockerfile when the only changed file is unrelated, and TruffleHog is invoked with a deleted/nonexistent changed path even though the shared target resolver found nothing. The full test suite currently passes, so please add regression coverage for both cases.
John-David Dalton (jdalton)
force-pushed
the
fix/changed-files-scope-observability
branch
from
August 7, 2026 19:49
3d2b8ab to
f0c9446
Compare
John-David Dalton (jdalton)
force-pushed
the
fix/changed-files-scope-observability
branch
from
August 7, 2026 20:01
f0c9446 to
0e7a87f
Compare
John-David Dalton (jdalton)
force-pushed
the
fix/changed-files-scope-observability
branch
from
August 7, 2026 20:04
0e7a87f to
b0ce7e3
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.
Scoping a scan to changed files now works no matter how the config was built, and when a scope cannot be honored the run says so instead of quietly scanning nothing. Before this change, several common setups silently scanned zero files and reported a clean result, which is the worst way for a security scan to fail.
The failures this fixes
Each of these produced a green, empty scan with no warning.
create_config_from_args()INPUT_CHANGED_FILESwas missing from the environment loader entirely, so the whole repository was scanned.changed_files: "auto"from a JSON or dashboard configa,u,t, ando. Scope resolved to nothing.pull_requestGITHUB_BASE_REFis unset there, so no base could be found and the diff was empty.detected dubious ownership. Every diff failed.The container case was the one nothing in a workflow could fix, because setting
safe.directoryin a workflow step writes the runner's git config rather than the container's.How each one is fixed - one resolver, a real base lookup, and a targeted git ownership retry
One resolver for every source.
changed_filesused to be resolved only insidecreate_config_from_args(). Every config source now goes through the same resolver, so environment, JSON, and dashboard configs all behave identically. A raw string like"auto"is resolved against git rather than stored as characters.Pull request base lookup. When
GITHUB_BASE_REFis unset, the base now comes frompull_request.base.shaandpull_request.base.refin the GitHub event payload. That coverspull_request_reviewandpull_request_review_comment, whose payloads carry a top-levelpull_request.It deliberately does not cover
issue_comment. That payload only hasissue.pull_request, which is URLs with no base ref or sha, so there is no base to be had without a GitHub API call.config.pymakes no network requests today, and falling back to the default branch would diff against the wrong base for anything targeting a release or stacked branch. So that trigger now names itself in a warning and says to passGITHUB_BASE_REFfrom the workflow instead.Git ownership. When git refuses to read the workspace, the git reads retry with that one directory trusted, and log that they did. When git is not refusing, which is any ordinary local run, nothing is relaxed. The retry is scoped to the workspace path and never a wildcard.
Every unhonorable scope now explains itself - five specific warnings replacing silent empty lists
A shallow checkout names
fetch-depth: 0as the fix. A workspace that is not a git repository, git refusing to read the repository, and a missing PR base each get their own warning. A scope that resolves to zero files warns that the scanners are being skipped.All five previously returned an empty list in complete silence.
scan_allstill overrideschanged_files, but now logs a warning naming the scope it discarded, because that setting can come from a dashboard config rather than the workflow. The warning is explicit that the override is partial: only scanners that askget_scan_targets()for their paths widen, while the secret and container scanners readchanged_filesdirectly and stay scoped, so setting both produces a mixed run.Scanners no longer substitute their own scope - three separate ways a narrow scope used to widen or misfire
TruffleHog and Trivy used to fall back to their own staged-file scope when an explicit
changed_filesrequest resolved to nothing, which quietly replaced the scope the user asked for.Trivy's filesystem vulnerability scan also widened an empty scope back out to the whole workspace. It is the one scanner that builds its own path list instead of going through
get_scan_targets(), so it needed the check in two places.Two more came out of review. Trivy's Dockerfile scan kept scanning the configured Dockerfiles when the changed set contained none, so a Python-only change still triggered a Dockerfile scan. And TruffleHog was handed changed paths that no longer exist on disk, producing
trufflehog filesystem <workspace>/gone.pyfor a deleted file. Both now skip cleanly.scan_allkeeps working the same way throughout: it does not mean "ignore the scope", it means "when the scope resolves to nothing, scan everything rather than nothing".Testing
267 tests pass. The new coverage pins both directions for each fix: that a scope narrows when it should, and that it does not widen when it should not.
Worth noting for reviewers, two existing tests in
test_trufflehog_excludes.pywere encoding the old bug. They named changed files that were never created on disk and asserted those paths reached TruffleHog. They now get real files, with their assertions untouched.