fix: changed_files resolved to zero files in the Docker container action (git dubious ownership)

[David Larsen (dc-larsen)]

Problem

changed_files diff-only mode resolves to zero files on every PR when Socket Basics runs as the pre-built Docker container action. Each run logs No scan targets to analyze (scoped scan matched no existing files); skipping OpenGrep and exits green with zero alerts. The scan looks healthy while scanning nothing. Full-repo mode on the same checkout works. A customer hit this rolling the action out org-wide on v2.2.1 (eng-support thread: https://socketdev.slack.com/archives/C05TT2Q3FEZ/p1786394320795579), and it reproduces identically on 3.0.0.

Root cause

GitHub runs Docker container actions as root, while the checkout mounted at /github/workspace is owned by the runner user (uid 1001). Git 2.35.2+ refuses to read a repository owned by a different user. Every git subprocess in _detect_git_changed_files fails, the helper catches the CalledProcessError and returns [], and the empty scope makes every scanner skip.

actions/checkout does not cover this case: its safe.directory entry is written to /home/runner/.gitconfig, but container actions run with HOME=/github/home. A prior workflow step running git config --global --add safe.directory misses the container for the same reason.

The same mismatch breaks the git fallbacks in _discover_repository, _discover_branch, _discover_commit_hash, and _discover_is_default_branch. That surface shows up in local Docker runs, where no GITHUB_* env vars mask it.

Fix

A _git_env() helper injects safe.directory=<workspace> into the environment of each git subprocess via command-scope GIT_CONFIG_COUNT / GIT_CONFIG_KEY_n / GIT_CONFIG_VALUE_n entries. Git honors command-scope safe.directory since 2.38, and the image ships 2.47.

• No git config files are written, and nothing outside these subprocesses changes.
• Caller-provided GIT_CONFIG_* entries are preserved, with ours appended after them, so users who already deployed the env-var workaround are unaffected.
• The workspace is an explicit scan target, not an incidentally discovered repository, so marking it safe matches what the user asked the tool to do.

Testing

Unit (tests/test_changed_files_scope.py)

• TestDubiousOwnership drives the real git ownership check end to end using GIT_TEST_ASSUME_DIFFERENT_OWNER, git's own test knob for this code path. A control probe skips the tests on a git build without the knob. Both tests fail on the unpatched code and pass with the fix. I confirmed that by disabling the injection and re-running.
• TestGitEnv covers appending after caller entries, a malformed GIT_CONFIG_COUNT, and the GITHUB_WORKSPACE default.
• Full suite: 222 passed.

Published container images

Repo owned by uid 1001, process as root, GITHUB_BASE_REF=main, comment-only Go change:

Case	Image	Result
Unpatched, flags given	3.0.0	skip, Total alerts: 0 (bug)
Unpatched, no --repo/--branch	3.0.0	repository discovery fails (bug, second surface)
Patched	3.0.0	diff resolves, seeded finding reported, Total alerts: 1
Patched, no --repo/--branch	3.0.0	git discovery works, Total alerts: 1
Patched	2.2.1	Total alerts: 1
Patched + user GIT_CONFIG_* block already set	3.0.0	Total alerts: 1 (workaround coexists)
Patched, root-owned repo (ownership matches)	3.0.0	Total alerts: 1 (no regression)
Patched, delete-only PR	3.0.0	skip, Total alerts: 0 (empty-diff semantics preserved)

Real GitHub Actions A/B

One workflow, two jobs on a comment-only Go-file PR in a scratch repo. Each job builds this action from source at a different ref. The rig branches on my fork swap image: to Dockerfile and the trivy base to the public upstream, and are otherwise identical to base and fix.

• baseline (unpatched main): No scan targets to analyze (scoped scan matched no existing files); skipping OpenGrep, Total alerts: 0, job passed green. That is the customer symptom on a real runner.
• fix (this branch): the diff resolved to svc/main.go, OpenGrep ran on it, and the job failed the check with Total alerts: 1 and Found 1 high/critical severity issues. Failing the check is the intended blocking behavior for a seeded critical finding.

Run: https://github.com/dc-larsen/sb-changed-files-test/actions/runs/31432924728 (private scratch repo, log lines quoted verbatim above). The rig branches test-ownership-baseline and test-ownership-fix on dc-larsen/socket-basics reproduce this against any scratch repo.

Notes for review

• fix/changed-files-scope-observability is complementary: it makes this failure loud instead of silent. One interaction: its troubleshooting doc recommends git config --global --add safe.directory in a prior step, which cannot reach the container action. After this lands, that row can be dropped or swapped for the env-var form.
• CHANGELOG entry added under Unreleased.

Fixes CE-440

---

Note

Medium Risk
Changes CI failure modes for changed_files (fail-closed by default) and scan-target precedence across all scanners; fixes a high-impact silent skip in container actions but may surface new errors on misconfigured checkouts until fetch-depth: 0 or scan_all is set.

Overview
Fixes changed_files always resolving to zero files in the pre-built Docker GitHub Action by marking the scan workspace as safe.directory on git subprocesses via command-scope GIT_CONFIG_* (no config files; existing user GIT_CONFIG_* entries are preserved). The same env is applied to repo/branch/commit/default-branch discovery.

Diff scoping behavior changes: git failures are logged and distinguished from a genuinely empty diff (None vs []). When a requested scope cannot be resolved, the run now exits with a configuration error instead of a green job that scanned nothing; shallow checkouts get a fetch-depth: 0 hint. Resolved scope is logged (count at INFO, paths at DEBUG).

Adds scan_all as a GitHub Action input: optional fail-open widen to full-repo scan when resolution fails (with warning). get_scan_targets precedence flips so a successful changed_files scope wins over scan_all; empty successful scopes still skip scanners. TruffleHog and Trivy honor changed_files_scope_requested so they do not fall back to staged files or widen past an intentional empty scope.

^{Reviewed by Cursor Bugbot for commit f96145c. Configure here.}

[lelia]

bugbot run

[lelia]

bugbot run

[lelia]

David Larsen (@dc-larsen) Heads up — I pushed 3e836bd onto your branch so the observability half from the support report ships in this same PR/release. Built directly on your work — the root-cause fix and the GIT_TEST_ASSUME_DIFFERENT_OWNER test approach are great.

What it adds: detection now distinguishes failed resolution (None) from a genuinely empty diff ([]); git stderr is captured and logged instead of discarded; ref-not-found stays a soft miss (your candidate-loop and delete-only behavior are preserved, tests still green) while unreadable-repo errors fail fast. On failure, the config layer falls back to a full-repo scan with a prominent warning instead of silently skipping every scanner — that also now applies to an unresolvable base ref in a PR context (e.g. shallow fetch), which previously fell through to the usually-empty staged diff. Resolved scope is logged every run (count at INFO, list at DEBUG) — the direct ask. Four connector-internal staged callers got or [] for the new contract. 7 new tests, full suite 231 passing, CHANGELOG extended.

One thing worth your eyes specifically: the full-scan-fallback and shallow-fetch-base cases are deliberate semantic changes from "skip silently" — flag if you know a customer scenario where that's the wrong call.

[cursor]

✅ 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 3e836bd. Configure here.}

[David Larsen (dc-larsen)]

Verified 3e836bd end to end on the container rig: the ownership case still resolves and alerts, delete-only PRs still skip, and the missing-base case warns with the real git error before full-scanning. 231 tests pass locally, and the stderr capture with the dubious-ownership hint is exactly what support needed.

On your question, two customer scenarios worth weighing, both from the account that hit the original bug:

1. Very large repos choose diff-only mode partly because full-repo is the documented OOM path. Their main monorepo is 3.8 GB. A missing fetch-depth: 0 is deterministic, so with the fallback every PR in a misconfigured repo runs a full scan, and on a repo that size that's an OOM or timeout red check rather than a clean signal about the real problem.

2. Their full-repo run reports 851 findings, 28 high/critical. In PR context the fallback exits non-zero on pre-existing findings, so one org-wide workflow-template mistake blocks every PR with a very large comment, and the red check reads as "you have hundreds of findings" rather than "add fetch-depth: 0".

Both are loud and discoverable, which beats the silent skip. If you want to blunt them, the deterministic base-unresolvable case could fail fast with a config error naming the one-line fix, keeping the full-scan fallback for the non-deterministic failures. Fail-toward-scanning is a defensible default for a security tool and the warning names the cause, so I'm fine shipping it as is.

The shallow-fetch change itself looks strictly better. The staged fallthrough in CI was almost always an empty diff, and local pre-commit runs have no GITHUB_BASE_REF so they keep the staged path.

[lelia]

David Larsen (@dc-larsen) thanks for running it end-to-end - and for raising the two real-world customer scenarios.

I went one better than documenting: your fail-fast suggestion is now in the PR (b7c757a). Shallow checkout + unresolvable base → configuration error naming the fetch-depth: 0 fix (via git rev-parse --is-shallow-repository, matching the existing SystemExit convention the repo/branch discovery paths use); everything non-deterministic keeps the full-scan fallback. Shallow-with-resolvable-base still diffs normally, delete-only still skips — tests cover all four quadrants, full suite at 236.

The tradeoffs are documented, too (a204ae7): both your scenarios in github-action.md + parameters.md, with the shallow carve-out noted, plus how to tell an empty diff from a failed lookup in the logs.

So the final behavior matrix: deterministic misconfig → precise error with the fix; unknown failure → loud full scan; empty diff → skip; and nothing, ever, silently scans zero files.

[lelia]

bugbot run

[lelia]

bugbot run

[cursor]

✅ 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 f96145c. Configure here.}