fix: honor changed_files from every config source and stop failing silently - #98
fix: honor changed_files from every config source and stop failing silently#98John-David Dalton (jdalton) wants to merge 9 commits into
Conversation
changed_files was resolved in exactly one place, create_config_from_args, and INPUT_CHANGED_FILES was missing from the environment loader entirely (unlike INPUT_SCAN_ALL and INPUT_SCAN_FILES). A Config built any other way silently scanned the whole repository, and a raw string value such as 'auto' from a --config JSON file or a Socket dashboard config was never git-resolved -- _resolve_file_targets iterated the string and looked for files named a, u, t and o. Every source now goes through one resolver, called from Config, so the action input, the CLI flag, the env var, a JSON config and a dashboard config are all honored identically. Then make the failures visible. The PR base is now also read from pull_request.base.sha/ref in the GitHub event payload, since GITHUB_BASE_REF is only set on pull_request triggers. When no base can be resolved the run names what it tried and why -- shallow checkout (pointing at fetch-depth: 0), workspace is not a git repository, git refusing to read the repository, or no PR base at all. A scope that resolves to zero files warns that the scanners are being skipped, and scan_all now warns when it discards a requested scope instead of overriding it in silence. Every one of those previously returned an empty list with no log output. TruffleHog and Trivy no longer substitute their own staged-file scope when an explicit request resolved to nothing. Refs: SURF-1452
PR base resolution now consults the GitHub event payload, so when these tests run inside a pull request the ambient payload names a real base ref and the fixture's temp repo happens to have a branch by that name. test_auto_falls_back_to_staged_without_base_ref then diffed against it instead of falling back to staged changes. Passed locally, failed in CI, which is exactly the leak.
|
bugbot run |
… 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.
|
bugbot run |
… 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.
|
bugbot run |
…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.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ee741d7. Configure here.
The action ships as a Docker container action, so the scan runs as root over a workspace owned by the runner user. git refuses that with "detected dubious ownership", which failed every diff and made changed_files: 'auto' and 'pr' resolve to nothing -- the same symptom as a pull request that changed no files. No workflow could fix it. Running `git config --global --add safe.directory` before the scan step writes the runner's git config, and the container has its own, so the advice the previous warning gave could not work. The git reads now declare the trust themselves, for the one directory the workflow asked us to scan and nothing else. Five tests drive real repositories through git's own GIT_TEST_ASSUME_DIFFERENT_OWNER switch, including one asserting the workspace is the only path trusted.
|
[agent] One more way the scope was being thrown away, and it is the one that mattered most: the container could not read the repository at all. This ships as a Docker container action. GitHub runs the container as root and mounts a workspace that belongs to the runner user, and git refuses to read a repository in that state — The warning this branch added made it visible but told the reader to do something that cannot work: Fixed in 946cac4. The git reads now declare the trust themselves, scoped to the one directory the workflow already asked us to scan: When ownership already matches this changes nothing, and it is never I also verified this the other way round, by running the same 14-scenario reproduction against
Separately, and worth checking on its own: the
|
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 946cac4. Configure here.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2be2e9e. Configure here.
|
bugbot run |
When both the plain and the trusting probe fail, _git_command_for falls back to plain git, so the warning claiming the directory is trusted was wrong for that path. It now says the retry was attempted and that the problem is more than an ownership mismatch, which is true for the only way that branch is now reachable. Splits the trusting command out of the decision so the "workspace only, never a wildcard" property can be asserted directly. It could not be before: a global safe.directory entry makes git ignore GIT_TEST_ASSUME_DIFFERENT_OWNER, the hosted runners have one, and the test asserting a trusting command was therefore red in CI and green locally. The tests that need git to actually be refusing now skip with a reason instead of passing vacuously.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6451e30. Configure here.
lelia
left a comment
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.
| changed_files = self.config.get('changed_files', []) if hasattr(self.config, '_config') else [] | ||
| # Fallback: attempt to detect staged changed files if none present | ||
| if not changed_files: | ||
| if not changed_files and not self._changed_files_scope_requested(): |
There was a problem hiding this comment.
Suppressing the staged fallback here is not enough when changed_files is non-empty but contains no Dockerfile. In that case changed_dockerfiles later remains empty, the configured dockerfiles list is left unchanged, and Trivy scans those unchanged files. I reproduced this with changed_files=['app.py'] and dockerfiles='Dockerfile'. When a changed-files scope is active, replace/intersect dockerfiles unconditionally and skip if no Dockerfiles remain; please add a regression test for this case.
| # substitute a different scope -- honor the empty result and skip. | ||
| changed_files = self.config.get('changed_files', []) if hasattr(self.config, '_config') else [] | ||
| if not changed_files: | ||
| if not changed_files and not self._changed_files_scope_requested(): |
There was a problem hiding this comment.
This guard prevents staged-file substitution when the resolved list is empty, but a non-empty list containing deleted or missing paths still bypasses the already-resolved targets below and passes the raw nonexistent path to TruffleHog. I reproduced trufflehog filesystem .../gone.py with changed_files=['gone.py']. Filter changed-file candidates for existence or reuse a changed-file-specific resolved target list so this skips cleanly, and add regression coverage.
LLM Description written by CLAUDE_CODE:claude-opus-5
Summary
A customer reported that
changed_filesdoes not work: they triedautoand they triedpr, and in every case the action scanned the whole repository and commented on files their PR never touched. Addingfetch-depth: 0to their checkout made no difference. Diff-only scoping shipped in v2.1.0 (#77), so this was supposed to already work.I reproduced the full pipeline against a real PR-shaped git checkout. The happy path in #77 is correct on a plain checkout — with
fetch-depth: 0andGITHUB_BASE_REFset,changed_files: 'auto'really does scope the scan to one file. What #77 missed is everything around it, and every one of the misses is completely silent.The biggest one is that it never worked in the action at all. The scan ships as a Docker container action, so it runs as root over a workspace owned by the runner user, and git refuses to read a repository in that state. Every diff failed, the scope came back empty, and an empty scope is indistinguishable from a pull request that changed nothing. No workflow could fix it.
On top of that, the scope request only reached one of the ways a config gets built, a raw string like
"auto"was iterated character by character, andscan_allthrew a correctly-resolved scope away without a word.This PR makes the git reads work inside the container, routes every source of
changed_filesthrough one resolver, and makes each remaining failure say what went wrong and what to do about it.Nothing reaches a workflow until a release — and the reporting customer needs a newer tag no matter what. This action is consumed by tag, and the tag their mirror pins ships an image that predates the
changed_filesinput entirely, so it accepts the setting and silently drops it. Merging this is safe on its own; after it merges, a release plus a re-mirror on their side is what actually changes their scans. See The tag a lot of people are pinned to, the first fold below.Release & rollout
The tag a lot of people are pinned to runs a much older image
Worth checking separately from this PR, because it explains the same symptom
without any code being wrong.
action.ymlpins the container image, and the release script keeps that pin insync with
pyproject.toml. Thev2.1.0tag was cut before either was bumped:action.ymldeclareschanged_files?v2.0.32.0.3v2.1.02.0.3v2.2.02.2.0v2.2.12.2.1So on
@v2.1.0the input is declared, GitHub passesINPUT_CHANGED_FILESto thecontainer without complaint, and the
2.0.3image never reads that variable —it has no
INPUT_CHANGED_FILEShandling at all, only the--changed-filesCLIflag, and the container entrypoint passes no arguments. The request is accepted
and dropped, the whole repository is scanned, and
fetch-depth: 0makes nodifference.
auto,prand an explicit file list all behave identically,because none of them are read.
Anyone pinned to
@v2.1.0, including through an internal mirror of it, needs tomove to
@v2.2.1or later; no code change reaches them until they do.scripts/sync_release_version.py --checkruns in CI and would catch anaction.ymlpin that disagrees withpyproject.tomltoday. It cannot catch thisone, because
v2.1.0'spyproject.tomlalso still said2.0.3— the tag nameis the only thing that disagreed, and nothing compares a tag to the version it
claims to be. That gap is worth closing on its own.
Is the customer's mirrored copy the problem?
Partly, possibly — but not entirely, and it does not need to be settled to merge this.
The customer runs internally mirrored copies of the actions rather than upstream tags. A mirror pinned to a pre-#77 image would ignore
changed_filescompletely and scan the whole repo, which matches their report exactly. That is worth checking on their side.But it is not the only explanation, and the other two are in our code, not theirs.
scan_allfrom an enterprise dashboard config produces precisely the reported symptom — full-repo scanning, unchanged acrossauto,prand an explicit list — and produces it on currentmain. So does any config path that does not go throughcreate_config_from_args. Both are fixed here.The part that matters most either way is the observability. Every one of these states used to be indistinguishable from "it worked and found nothing". After this PR, the customer's next run tells them which one they are in from the log, without another round trip.
Diagnosis
What I found: three ways the scope is discarded, and they all happen without a single log line
I built a script that stands up an upstream repo, a PR merge ref, and a CI-style checkout, then drives the real
Configcode with the environment a Docker container action actually sees. Results:fetch-depth: 0+GITHUB_BASE_REF+autofetch-depth: 0+GITHUB_BASE_REF+prfetch-depth: 0)GITHUB_BASE_REFabsent (non-pull_requesttrigger)changed_filesnever reaches the config layerscan_allalso setchanged_files: "auto"from a JSON/dashboard config1. The input only reached one config path.
changed_fileswas resolved in exactly one place,create_config_from_args().load_config_from_env()handlesINPUT_SCAN_ALLandINPUT_SCAN_FILESbut had nochanged_filesentry at all, and neither didload_explicit_env_config(). So aConfigbuilt any other way — the library entry point, or anything that constructsConfig()from the environment — never saw the request and scanned the whole repository.2. A raw string value was never resolved, and then iterated character by character. A
--configJSON file or a Socket dashboard config can carry"changed_files": "auto". That string went straight into the config, andget_scan_targets()handed it to_resolve_file_targets(), which iterates its argument. Iterating the string"auto"yields'a','u','t','o', so it looked for four one-character filenames, found none, and scoped the scan to nothing — logging four "Scan target does not exist" warnings naming<workspace>/a,<workspace>/uand so on.3.
scan_alldiscarded a correctly-resolved scope without a word.get_scan_targets()checksscan_allfirst and returns the whole workspace. I confirmed the sequence: the diff resolves to one file,changed_filesis['app.py'], and then the whole workspace is returned anyway with no output.scan_allcan be set in a Socket dashboard config or a shared workflow template rather than in the workflow that asked for diff-only scoping, so the person who set it and the person debugging it are often different people. This is the mechanism that best matches "we tried three configurations and nothing changed" — the scope is computed correctly every time and thrown away every time.And the base resolution only ever looked at
GITHUB_BASE_REF. That variable is set only onpull_requestandpull_request_targettriggers. On any other trigger_diff_against_base('')returnedNoneimmediately and the scope resolved to nothing. The customer's workflow setsGITHUB_PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }}, which says they also run this onissue_commentevents, so they land in that hole.The fix
The fix: one resolver, and no failure mode that stays quiet
One resolver. A new
resolve_changed_files_request(request, workspace)handlesauto,pr,current-commit, a commit hash, and a comma-separated list.Config.__init__calls it, so the action input, the--changed-filesCLI flag,INPUT_CHANGED_FILES, a--configJSON file and a Socket dashboard config all get the same treatment.create_config_from_args()now just records the raw CLI value and letsConfigresolve it, which deleted about 50 lines of duplicated per-mode branching. A value that is already a resolved list passes through untouched, so nothing is resolved twice.More ways to find the PR base.
autoandprnow try, in order:GITHUB_BASE_REF, thenpull_request.base.shafrom the event payload atGITHUB_EVENT_PATH, thenpull_request.base.ref.base.shais the best of the three because it is an exact commit and does not need a remote-tracking branch to exist. Each candidate is tried asorigin/<ref>and then bare, exactly as before.The payload steps cover the triggers whose payload has a top-level
pull_request:pull_request,pull_request_target,pull_request_reviewandpull_request_review_comment. They do not coverissue_comment, which Bugbot caught after the first version of this description claimed otherwise. That payload hasissue.pull_requestinstead — a set of URLs with no base ref or sha in it — so the base cannot be worked out without a GitHub API call, andconfig.pymakes no network calls at all. Rather than guess a base (diffing against the wrong one silently is the failure this PR exists to kill), that shape now gets a warning that names the trigger and tells the workflow author to look the base up and passGITHUB_BASE_REFin.docs/parameters.mdcarries the two-step workflow snippet.Every failure names itself. These all used to be
return []:none of the candidate PR bases (main) could be resolved ... The checkout is shallow, so the base branch is not in it -- set fetch-depth: 0 on actions/checkout.no pull request base was found. GITHUB_BASE_REF is unset and the GitHub event payload has no pull_request.base ... Use changed_files: 'current-commit', or pass an explicit file list.is not a git repository. Check out the repository (actions/checkout) before running the scangit refused to read ... usually a repository-ownership mismatch inside a container ('detected dubious ownership'). Run git config --global --add safe.directory ...resolved to zero files. The scanners will be SKIPPED rather than scanning the whole repository.scan_alloverrides the scopescan_all is enabled, so the whole workspace will be scanned and the requested changed-files scope (1 file(s)) is being ignored.The success path is loud too, so you can confirm the scope took effect:
Resolved PR diff base to 'origin/main', thenresolved 12 changed file(s), thenDiff-only scan scoping active: 12 scan target(s).Connectors stop substituting their own scope. TruffleHog and Trivy each re-derive a changed-file list with
mode='staged'when the config has none. When the user explicitly asked for a scope and it resolved to nothing, that fallback replaced "what the PR changed" with "whatever happens to be staged" — a different set of files, and never the one that was asked for. It now only runs when no scope was requested, so the existing default behavior is unchanged.The container could not read the repository at all, so none of this worked in the action
This action ships as a Docker container action. GitHub runs the container as
root and mounts the workspace, which belongs to the runner user. That ownership
mismatch is exactly what git refuses with
detected dubious ownership, so everygit diffin the scan failed andchanged_files: 'auto'or'pr'resolved tonothing — indistinguishable, before this PR, from a pull request that changed
no files.
I confirmed it by driving the real
Configcode over a real PR-shaped checkoutwith
GIT_TEST_ASSUME_DIFFERENT_OWNER=1, which is git's own switch for thatstate. On
mainand on the first version of this branch the scope came backempty; the only difference the branch made was that it explained itself.
The explanation it gave could not be acted on, either. It told the reader to run
git config --global --add safe.directory "$GITHUB_WORKSPACE"before the scanstep, and that writes the runner's git config — the container has its own,
so the advice does nothing. Every workflow hits this and no workflow can get out
of it.
So the git reads now declare the trust themselves, and only when git is actually
refusing:
The decision is a probe. If plain
gitcan read the workspace — any ordinarylocal run — that is what runs and nothing is relaxed at all. Only when it cannot
does the scan retry with that one directory trusted, and it logs that it did.
safe.directoryis a real protection against picking up a repository config youdo not control, and giving it up where it was not in the way would buy nothing.
It is never
safe.directory=*.Eight tests cover it: the "workspace only, never a wildcard" property against
the command builder, and the behaviour with git actually refusing. The second
group needs git to refuse for real, and
GIT_TEST_ASSUME_DIFFERENT_OWNERonlyforces git to consult
safe.directoryrather than forcing a refusal — a globalsafe.directory = *makes it inert, and the hosted runners have one. Thosetests skip with a reason there instead of passing vacuously.
Scope
What I deliberately did not change
scan_allstill wins overchanged_files. That precedence is documented inget_scan_targets()and other people rely on it. Flipping it would be a silent behavior change for everyone. It now warns instead. Note the precedence is only partial and always has been: SAST widens because it asksget_scan_targets()for its paths, while TruffleHog and Trivy readchanged_filesdirectly and stay scoped. Makingscan_allreach those two would change what a lot of existing runs scan, so the warning says the run will be a mix rather than pretending the override is clean.autostill falls back to staged changes when there is no PR base, which is what makes it useful for pre-commit hooks. It now warns first and logs how many staged files it found, so a CI run that lands there is obvious.current-commitand commit-hash modes still include deletions. Onlypr/autouse--diff-filter=ACMR. The deleted paths get dropped when targets are resolved, so the behavior is right; the asymmetry is pre-existing and out of scope here. There is a test pinning it.action.ymlstill points at the released2.2.1image.Testing & review
Ran — exit codes read directly from the harness, not through a pipe.
uv run --no-sync pytest -q tests/main; 45 new)python scripts/sync_release_version.py --checkGIT_TEST_ASSUME_DIFFERENT_OWNER=1mainand empty before the ownership fixHOMEpointed at a config holdingsafe.directory = *action.ymlNew tests in
tests/test_changed_files_scope.py:TestScopeRequestReachesEveryConfigPath(8) — env loader, raw"auto"string, raw comma list, already-resolved list, empty value, CLI-over-env precedenceTestPrBaseResolution(7) —base.shaandbase.reffrom the event payload, a payload with no pull request, an unreadable payload, a deep checkout resolvingorigin/<ref>, anissue_commentpayload getting its own named warning, and a plain issue comment getting the generic oneTestTrivyVulnScanHonorsTheResolvedScope(5) — Trivy's filesystem vulnerability scan is the one scanner that does not go throughget_scan_targets(), so it needed the empty-scope check of its own, and that check has to yield toscan_allTestScopeFailuresAreLoud(5) — shallow checkout, missing base, non-git workspace, zero resolution, and a success case asserting no warningTestScanAllOverrideIsLoud(5) — the override still wins, warns when it discards a scope, stays quiet when no scope was requested, and the warning describes the mixed run rather than a clean overrideTestResolveChangedFilesRequest(5) andTestConnectorsHonorTheResolvedScope(2)Mutation checks: every fix was broken on purpose and a named test went red
Each mutation was applied, the full suite was run, the mutation was reverted, and the suite was re-run green.
INPUT_CHANGED_FILES(the original gap)TestScopeRequestReachesEveryConfigPath::test_env_only_config_honors_input_changed_files,::test_env_value_is_used_when_no_cli_valuetest_raw_auto_string_is_resolved_not_iterated,test_raw_comma_list_string_is_split,test_env_only_config_honors_input_changed_files,test_cli_value_overrides_env_value,test_env_value_is_used_when_no_cli_value,TestDetectGitChangedFiles::test_delete_only_pr_config_creation_keeps_empty_scope,TestResolveChangedFilesRequest::test_current_commit_drops_deleted_paths_from_targetsTestPrBaseResolution::test_uses_base_sha_from_event_payload,::test_uses_base_ref_from_event_payloadissue_commentno longer gets its own warningTestPrBaseResolution::test_issue_comment_payload_yields_no_base_and_says_whyissuepayload treated as a PR commentTestPrBaseResolution::test_plain_issue_comment_gets_the_generic_warningTestTrivyVulnScanHonorsTheResolvedScope::test_unresolvable_scope_does_not_widen_to_the_whole_workspace,::test_scope_whose_paths_all_vanished_does_not_widen_eitherscan_allwarning goes back to claiming a clean overrideTestScanAllOverrideIsLoud::test_warning_says_the_run_will_be_a_mix_not_a_clean_overridescan_allTestTrivyVulnScanHonorsTheResolvedScope::test_scan_all_still_gets_the_whole_workspacescan_alldiscards the scope silently againTestScanAllOverrideIsLoud::test_scan_all_warns_when_it_discards_a_scope_request,::test_scan_all_warns_for_a_scope_that_resolved_to_nothingTestScopeFailuresAreLoud::test_shallow_checkout_warns_and_names_fetch_depth,::test_missing_pr_base_warnsTestScopeFailuresAreLoud::test_zero_resolution_warns_that_scanners_will_be_skippedTestConnectorsHonorTheResolvedScope::test_scope_resolved_to_nothing_is_not_replaced_by_stagedAfter restoring all seven: exit 0, 245 passed.
Review feedback addressed: two more places where the scope was still being thrown away
Bugbot found both.
Trivy's filesystem vulnerability scan widened an empty scope back out to the whole workspace. Every other scanner inherits the empty-scope behaviour from
Config.get_scan_targets(); this one builds its own path list fromchanged_filesand had aif not scan_paths: scan_paths = [workspace_path]fallback right after it. Declining the staged-file substitution left that fallback in charge, so an unresolvable scope still produced a full-repository scan — and it was a widening this PR introduced, because before it the staged fallback would at least have narrowed to the staged directories. Fixed in 5a7fe28 with four tests.The
scan_allwarning overstated what it does.scan_allonly reaches the scanners that askget_scan_targets()for their paths, so saying the changed-files scope is "ignored" would send someone looking for a full-repo secret scan that never happens. Fixed in ee741d7: the warning now says the run will be a mix and names which side does which. Chasing that also turned up a regression from 5a7fe28 -- withscan_allon and a scope that resolved to nothing, the new skip made Trivy's vulnerability scan do nothing at all, turning an explicit "scan everything" into scanning nothing. The skip now yields toscan_all.The event-payload base fallback does not cover
issue_comment, and this description said it did. It reads a top-levelpull_request.base;issue_commentpayloads haveissue.pull_request, which is URLs only. Fixed in 8fa4908 by correcting the claim in the docstring, the docs and above, and by giving that shape a warning that names the trigger and says how to supply the base. Two tests, including one making sure a comment on a real issue still gets the generic message.CI caught a test-isolation leak that the new event-payload fallback created
The first CI run failed one pre-existing test,
test_auto_falls_back_to_staged_without_base_ref, which passes locally. The cause is a genuine consequence of this change: PR base resolution now readsGITHUB_EVENT_PATH, and when the suite runs inside a pull request that variable points at a real event payload naming a real base ref —main— which the fixture's throwaway repo happens to have a branch for. So the test diffed against it instead of falling back to staged changes, which is what it was written to check.The fix is to clear
GITHUB_EVENT_PATHin thepr_repofixture alongsideGITHUB_WORKSPACEandGITHUB_BASE_REF, which it already cleared for the same reason.I reproduced the CI condition locally afterwards by running the suite with
GITHUB_EVENT_PATH,GITHUB_BASE_REFandCIset the way Actions sets them: with the fixture fix reverted the same single test fails (exit 1), and with it in place the suite passes (exit 0, 245 passed). Worth noting because a test that only fails inside a pull request is the kind that comes back.Did not run
Configand_detect_git_changed_filescode against realgitrepositories built to look like anactions/checkoutPR checkout (upstream remote,refs/pull/N/merge, detached HEAD, shallow and deep variants), with the container'sGITHUB_*andINPUT_*environment set. It does not exercise the GitHub runner itself.GIT_TEST_ASSUME_DIFFERENT_OWNERswitch, not by running git as a second real user. The generic "git refused" branch is exercised by the non-git-workspace test.Note
Medium Risk
Changes scan targeting and git invocation for all GitHub Action runs using
changed_files; behavior is more correct but mixedscan_all+ diff-only runs and trigger-specific base resolution need careful rollout.Overview
Diff-only
changed_filesscoping is fixed end-to-end so PR scans stop silently scanning the whole repo or nothing.Confignow resolves everychanged_filesvalue through oneresolve_changed_files_request()path (action input,INPUT_CHANGED_FILES, CLI, JSON, dashboard). Raw strings like"auto"are no longer iterated character-by-character.scan_allstill wins for SAST viaget_scan_targets()but logs a warning that secret/container scanners may stay scoped.PR base detection adds
pull_request.base.sha/base.reffromGITHUB_EVENT_PATHwhenGITHUB_BASE_REFis missing, with targeted warnings for shallow checkouts, non-git workspaces,issue_comment, and zero-file scopes. Git inside the Docker action uses per-workspacesafe.directoryonly when ownership blocks reads.TruffleHog and Trivy stop falling back to staged files or the full workspace when an explicit scope resolved empty; Trivy’s filesystem vuln scan gets the same guard. Docs and
action.ymldescribe the new logging and config sources; tests cover the new behavior.Reviewed by Cursor Bugbot for commit 93fe13b. Configure here.