grafana / grafana/security-github-actions

reusable-trufflehog: two-dot git diff over-scopes PR scans, reporting findings against PRs that did not touch the file

Open
#217 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
8
Forks
14
Avg merge
4h 35m
Merged PRs (30d)
6

Description

## Summary

In `.github/workflows/reusable-trufflehog.yml`, the PR file list is built with a **two-dot** `git diff`, so on any branch that is behind its base the scan also covers everything merged into the base since the branch point. That list becomes `--include-paths` for the scan, so findings get reported against PRs that never touched the file.

On [grafana/grafana-pathfinder-app#1749](https://github.com/grafana/grafana-pathfinder-app/pull/1749) — a Renovate bump touching only `package-lock.json` — this scanned **833** files and reported two unverified `URI` findings in a telemetry test the PR does not touch.

The immediate effect is noise, but the part I think matters is desensitization: the value of this gate is that a hit means *you* introduced something. Once hits routinely come from other people's code, the rational response is to stop reading them, which is exactly when a real finding gets waved through. There is also a cost angle — on a stale branch this is a full-repo scan billed as a PR scan.

Same file and broadly the same failure mode as #208 (a defect that quietly degrades the scan rather than failing loudly).

## Cause

[`reusable-trufflehog.yml#L201`](https://github.com/grafana/security-github-actions/blob/7da08dc6773bde8f8f7dd0427fd42989a8367715/.github/workflows/reusable-trufflehog.yml#L201) (`pull_request`) and [`#L204`](https://github.com/grafana/security-github-actions/blob/7da08dc6773bde8f8f7dd0427fd42989a8367715/.github/workflows/reusable-trufflehog.yml#L204) (`merge_group`):

```yaml
git diff --name-only "${PR_BASE_SHA}" "${PR_HEAD_SHA}" > changed-files.txt
```

`git diff A B` compares two *tips*. `git diff A...B` compares `B` against `merge-base(A, B)` — the branch's own work. `PR_BASE_SHA` is the base branch tip at event time, so the two-dot form adds every file that changed on the base since the branch point.

Measured on the PR above:

| form | files in scope |
|---|---|
| `BASE HEAD` (current) | **833** |
| `BASE...HEAD` | **1** |

That list is then passed to `trufflehog filesystem .` via `--include-paths` at [`#L227`](https://github.com/grafana/security-github-actions/blob/7da08dc6773bde8f8f7dd0427fd42989a8367715/.github/workflows/reusable-trufflehog.yml#L227).

## Important caveat: three dots alone will not work

The job checks out with `fetch-depth: 1` ([`#L48`](https://github.com/grafana/security-github-actions/blob/7da08dc6773bde8f8f7dd0427fd42989a8367715/.github/workflows/reusable-trufflehog.yml#L48)) and fetches only the two SHAs with `git fetch --depth=1` ([`#L56`](https://github.com/grafana/security-github-actions/blob/7da08dc6773bde8f8f7dd0427fd42989a8367715/.github/workflows/reusable-trufflehog.yml#L56)). Depth-1 fetches carry no ancestors, so there is no merge base in the object store. Replicating that fetch exactly and asking for a three-dot diff:

```
$ git fetch --depth=1 origin $BASE $HEAD
$ git diff --name-only "$BASE...$HEAD"
fatal: 96850ea...0f52f92: no merge base
```

So the two-dot form is currently the only one that works — this is a design interaction, not a typo. Worth flagging because the scan step runs under `set +e` with `|| true`, so a naive dot change would likely produce an empty `changed-files.txt` and a scan that silently covers **nothing** — a worse bug than the one being fixed.

## Suggested fix

Enumerate the PR's files from the API rather than from git. It needs no history, is unaffected by branch staleness, and is exactly the intended set:

```
GET /repos/{owner}/{repo}/pulls/{number}/files
```

Deepening the fetch until a merge base exists also works, but costs clone time on long-lived branches and needs a fallback for when the deepening is still insufficient.

Whichever route, it would be worth failing loudly if `changed-files.txt` is empty while the PR is known to have changed files, so that a future regression here surfaces rather than silently reducing coverage.

## Secondary note on coverage (lower confidence, not observed)

Over-scoping is the main effect, but two-dot can also *under*-scope: a file whose content at `BASE` and `HEAD` is identical is omitted from the diff entirely. For a PR file that requires the same content to already be on the base branch, which the `push` full-filesystem scan at [`#L243`](https://github.com/grafana/security-github-actions/blob/7da08dc6773bde8f8f7dd0427fd42989a8367715/.github/workflows/reusable-trufflehog.yml#L243) should cover, so I do not think this is a live gap. Flagging it only so the coverage question gets evaluated alongside the noise one.

## Notes

Observed with TruffleHog `v3.95.9` as pinned in the workflow; I reproduced the scan-scoping behaviour locally with `v3.97.4` using the workflow's own command and `--include-paths` regex construction. The file-count numbers and the `no merge base` failure are measured. The `set +e` consequence is read off the script, not executed — treat that part as a prediction.

Happy to open a PR if that is useful. Not urgent and not a vulnerability — a scoping issue, but one that quietly erodes signal in every repo using the workflow.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in .github/workflows/reusable-trufflehog.yml, especially the pull_request and merge_group file-list steps around lines 201 and 204, then trace changed-files.txt into the TruffleHog command around line 227. Compare the workflow's shallow fetch behavior with the pull-files API approach described in the issue. Done means PR scans include only files changed by that PR, preserve coverage, and fail loudly when the file list is unexpectedly empty.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, github-actions
Domain
ci-cd, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.