Skip to content

Scan the pushed commits in the secret scanning workflow - #6759

Open
albertvillanova wants to merge 1 commit into
ci-trufflehog-fail-on-scan-errorsfrom
ci-trufflehog-scan-diff
Open

Scan the pushed commits in the secret scanning workflow#6759
albertvillanova wants to merge 1 commit into
ci-trufflehog-fail-on-scan-errorsfrom
ci-trufflehog-scan-diff

Conversation

@albertvillanova

@albertvillanova albertvillanova commented Aug 14, 2026

Copy link
Copy Markdown
Member

Stacked on top of #6758.

This PR makes the secret scanning workflow scan the commits under review instead of the whole repository history, and adds a weekly full-history sweep.

Motivation

The Secret Leaks job fails intermittently on new branches, most recently on #6756: https://github.com/huggingface/trl/actions/runs/31793772381/job/94746235557

The action derives its base from github.event.before, which is all zeros on the push that creates a branch. Without a base commit the scanner walks every commit ever made, 33.5k chunks and 22.5 MB, instead of the pushed changes.

That history holds exactly one detectable secret: the public hub-ci dummy token added to tests/testing_constants.py in 6e80e09 (January 2023) and removed from the working tree in #1852. It is the __DUMMY_TRANSFORMERS_USER__ token shared across HF test suites, valid on hub-ci.huggingface.co and rejected with 401 by huggingface.co, which is the endpoint the detector verifies against. No credential needs rotating.

Every new branch push therefore re-verifies that one token, and the job's outcome depends on what huggingface.co answers:

run verification outcome
31792044718 66 ms 401, unverified, filtered out
31792208869 102 ms 401, unverified, filtered out
31793772381 2000 ms no answer, so unknown, reported, exit 183

The detector treats anything that is neither 2xx nor 401 as a verification error, and a verification error lands in the unknown bucket that --results=verified,unknown reports. One slow or blocked request to huggingface.co is thus enough to fail the job, on a finding that lives in immutable history and cannot be acted on. Reproduced locally with the pinned scanner image and networking disabled: same warning on tests/testing_constants.py, same exit 183.

Solution

Scan the commits under review. On pull_request the action passes base.sha..head.sha, and on a push to main it passes before..after, so the scan covers the new commits and never re-reads history.

Both of those paths pass commit shas, which is why no explicit base input is set here. huggingface/transformers does set one, falling back to the default branch name on new branches, and a ref name does not resolve in the clone the scanner makes: those runs report unable to resolve ref: no base refs succeeded for base: "main", scan zero bytes, and pass green (huggingface/transformers#47945).

Narrowing the trigger gives up one case, commits pushed to a branch with no pull request open. History stays in scope through the weekly sweep, which runs with no base commit and therefore scans everything, including with detectors added after a commit landed. That sweep is expected to trip over the same hub-ci token from time to time, without blocking anyone.

Detection is otherwise unchanged: same detectors, same --results=verified,unknown, same digest pin from #6677 and lob exclusion from #6674.

Changes

  • Trigger the workflow on pull_request and on pushes to main, so the scan covers the pushed commits
  • Add a weekly scheduled run covering the full history
  • Record why an explicit base, if ever set, must be a sha and not a ref name

Note

Low Risk
CI-only workflow trigger and scheduling changes; no application code, auth, or data paths affected.

Overview
Fixes flaky Secret Leaks runs on new branches by changing when TruffleHog runs and what it scans.

The workflow now triggers on pull_request, on push to main only (not every branch push), and on a weekly schedule (0 3 * * 1). PR and main-push runs use the action’s derived commit range so only new commits are scanned instead of the entire git history—which previously re-hit an old hub-ci dummy token and could fail when verification to huggingface.co timed out.

Comments in the workflow document that an explicit base, if ever added, must be a commit SHA, not a ref name, because refs do not resolve in the scanner’s clone.

Scanner image, detectors, and extra_args are unchanged.

Reviewed by Cursor Bugbot for commit 865fefa. Bugbot is set up for automated code reviews on this repo. Configure here.

@bot-ci-comment

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@albertvillanova

albertvillanova commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Reported the transformers side of this upstream:

Their workflow sets an explicit base that falls back to the default branch name on new branches, which does not resolve in the clone the scanner makes, so those runs scan zero bytes and still pass. That is the trap this PR avoids by letting the action derive base and head as commit shas.

@albertvillanova

Copy link
Copy Markdown
Member Author

For provenance: the diff-scanning approach here comes from the equivalent change in transformers, huggingface/transformers#47945 (workflow at 0cdd8a1). This PR keeps their intent, scanning the pushed commits rather than the whole history, and differs only in how the range is expressed: the action derives base and head as commit shas from the event, instead of an explicit base input that falls back to a ref name.

The run that started the investigation: https://github.com/huggingface/trl/actions/runs/31793772381/job/94746235557?pr=6756

@qgallouedec qgallouedec left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, that looks good, but to be honest, it's a little outside my area of expertise. Feel free to merge it, or wait for @McPatate's opinion if you think it's necessary.

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