Skip to content

fix: honor changed_files from every config source and stop failing silently - #98

Open
John-David Dalton (jdalton) wants to merge 1 commit into
mainfrom
fix/changed-files-scope-observability
Open

fix: honor changed_files from every config source and stop failing silently#98
John-David Dalton (jdalton) wants to merge 1 commit into
mainfrom
fix/changed-files-scope-observability

Conversation

@jdalton

@jdalton John-David Dalton (jdalton) commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

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.

Setup What happened
Config built outside create_config_from_args() INPUT_CHANGED_FILES was missing from the environment loader entirely, so the whole repository was scanned.
changed_files: "auto" from a JSON or dashboard config The string was stored verbatim, then iterated character by character looking for files named a, u, t, and o. Scope resolved to nothing.
Any trigger other than pull_request GITHUB_BASE_REF is unset there, so no base could be found and the diff was empty.
Running inside the action's container The scan runs as root over a workspace owned by the runner user, and git refuses that with detected dubious ownership. Every diff failed.
Shallow checkout No history to diff against, so the scope came back empty.

The container case was the one nothing in a workflow could fix, because setting safe.directory in 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_files used to be resolved only inside create_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_REF is unset, the base now comes from pull_request.base.sha and pull_request.base.ref in the GitHub event payload. That covers pull_request_review and pull_request_review_comment, whose payloads carry a top-level pull_request.

It deliberately does not cover issue_comment. That payload only has issue.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.py makes 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 pass GITHUB_BASE_REF from 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: 0 as 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_all still overrides changed_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 ask get_scan_targets() for their paths widen, while the secret and container scanners read changed_files directly 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_files request 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.py for a deleted file. Both now skip cleanly.

scan_all keeps 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.py were 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.

@jdalton
John-David Dalton (jdalton) requested a review from a team as a code owner August 3, 2026 19:03
@jdalton

Copy link
Copy Markdown
Contributor Author

bugbot run

cursor[bot]

This comment was marked as resolved.

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.
@jdalton

Copy link
Copy Markdown
Contributor Author

bugbot run

cursor[bot]

This comment was marked as resolved.

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.
@jdalton

Copy link
Copy Markdown
Contributor Author

bugbot run

cursor[bot]

This comment was marked as resolved.

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.
@jdalton

Copy link
Copy Markdown
Contributor Author

bugbot run

cursor[bot]

This comment was marked as resolved.

@jdalton

This comment was marked as outdated.

@jdalton

Copy link
Copy Markdown
Contributor Author

bugbot run

cursor[bot]

This comment was marked as resolved.

@jdalton

Copy link
Copy Markdown
Contributor Author

bugbot run

cursor[bot]

This comment was marked as resolved.

@jdalton

Copy link
Copy Markdown
Contributor Author

bugbot run

cursor[bot]

This comment was marked as resolved.

@jdalton

Copy link
Copy Markdown
Contributor Author

bugbot run

cursor[bot]

This comment was marked as resolved.

@lelia lelia self-assigned this Aug 5, 2026

@lelia lelia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread socket_basics/core/connector/trivy/trivy.py Outdated
Comment thread socket_basics/core/connector/trufflehog/__init__.py
@jdalton
John-David Dalton (jdalton) force-pushed the fix/changed-files-scope-observability branch from 3d2b8ab to f0c9446 Compare August 7, 2026 19:49
@jdalton
John-David Dalton (jdalton) force-pushed the fix/changed-files-scope-observability branch from f0c9446 to 0e7a87f Compare August 7, 2026 20:01
@jdalton
John-David Dalton (jdalton) force-pushed the fix/changed-files-scope-observability branch from 0e7a87f to b0ce7e3 Compare August 7, 2026 20:04
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.

2 participants