ci(security): raise an issue for what the nightly scan reports, and run in the merge queue - #2390
Conversation
…un in the merge queue The nightly jobs never fail a run, so a finding outside the baseline was a line in a summary nobody opens: it surfaced once, in the pull request that introduced it, and never again — the diff job only ever sees a pull request's own commits. Now each such finding gets an issue assigned to the author of the commit it arrived with, and an open one suppresses the next night's duplicate. The blocking job also runs on merge_group, which is what a required status check needs: inside the queue there is no pull_request event, and the range comes from the event rather than from HEAD^1. The two nightly jobs are restricted to schedule and manual runs so the queue does not trigger a full history scan. Refs #2301 Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe TruffleHog workflow now scans merge groups, exports redacted findings, and creates GitHub issues for new findings. Scheduled and manual history/API scans use restricted triggers. Notifications resolve commit authors and support dry-run mode. ChangesTruffleHog notifications
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TruffleHog
participant trufflehog_gate.py
participant trufflehog_notify.py
participant GitHub
TruffleHog->>trufflehog_gate.py: Scan findings
trufflehog_gate.py->>trufflehog_notify.py: Write redacted new-findings.json
trufflehog_notify.py->>GitHub: Check fingerprints and resolve authors
trufflehog_notify.py->>GitHub: Create or preview assigned issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/trufflehog_gate.py:
- Around line 90-92: Update the JSON export flow in trufflehog_gate.py to
preserve the full f["commit"] identifier in new before any formatting; apply
[:8] only when rendering the Markdown table or issue body, so
trufflehog_notify.py receives the complete SHA for GitHub API resolution.
In @.github/scripts/trufflehog_notify.py:
- Around line 84-85: Update the issue-creation flow around gh() so an empty
result from gh issue create is treated as a failure: report the failure without
incrementing created, and track failed creations separately for final status
reporting.
- Around line 38-51: The fallback in body_for must not publish Git author email
addresses; replace the f.get("email") branch with neutral wording such as
“unlinked Git author; inspect commit metadata,” while preserving linked-login
and commit-author fallbacks. In .github/scripts/trufflehog_gate.py lines 75-77,
stop adding email to NEW.json unless the required privacy basis is documented
elsewhere.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ec4c4181-299e-4626-98ea-52e07f5634c2
📒 Files selected for processing (3)
.github/scripts/trufflehog_gate.py.github/scripts/trufflehog_notify.py.github/workflows/trufflehog.yml
| print(gh(*args) or f"failed to open an issue for {f['fp']}") | ||
| created += 1 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Do not count a failed issue creation as opened.
gh() returns an empty string when gh issue create fails. Line 84 reports that failure, but Line 85 still increments created. The final status can therefore report issues as opened when no issue exists. Track failed creations separately.
Proposed fix
- created = skipped = 0
+ created = skipped = failed = 0
...
- print(gh(*args) or f"failed to open an issue for {f['fp']}")
- created += 1
+ url = gh(*args)
+ if url:
+ print(url)
+ created += 1
+ else:
+ print(f"failed to open an issue for {f['fp']}")
+ failed += 1
...
- print(f"{created} issue(s) {'would be ' if dry else ''}opened, {skipped} already tracked")
+ print(
+ f"{created} issue(s) {'would be ' if dry else ''}opened, "
+ f"{skipped} already tracked, {failed} failed"
+ )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/trufflehog_notify.py around lines 84 - 85, Update the
issue-creation flow around gh() so an empty result from gh issue create is
treated as a failure: report the failure without incrementing created, and track
failed creations separately for final status reporting.
…mit id whole Fixes a problem found by CodeRabbit on this pull request: the commit was truncated to eight characters at parse time and that prefix was then used as the API identifier. It stays whole now and is shortened only where rendered. Assignment used to rely on the commit author alone, which GitHub can only attribute when the address in the commit is attached to an account — a work address usually is not, and a quarter of the corpus resolved to nobody. The pull request the commit arrived with names an account either way, and .github/trufflehog-authors.txt covers the rest until those addresses are attached to their accounts. Over the 53 distinct commits behind the current findings: 35 resolve by commit author, 10 by pull request author, 7 by the map, 1 by nothing at all. Refs #2301 Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
cyberantonz
left a comment
There was a problem hiding this comment.
You provide real email addresses of real people. I think it is not the right way
|
|
||
|
|
||
| def main() -> int: | ||
| new_json, repo, author_map = sys.argv[1], sys.argv[2], sys.argv[3] |
There was a problem hiding this comment.
There is argparse library for this
| print(gh(*args) or f"failed to open an issue for {f['fp']}") | ||
| created += 1 | ||
|
|
||
| print(f"{created} issue(s) {'would be ' if dry else ''}opened, {skipped} already tracked") |
There was a problem hiding this comment.
No pre-commit installed?
The map held live work addresses in a public repository. They are already in the commit history, but a single harvestable list is a different thing, and it bought only historical commits: a finding from a new commit resolves through the pull request it arrived with. The issue body now links the GitHub profile and the commit rather than naming an address, and the gate no longer carries the author's address at all. An address that resolves to nobody is fixed at the source, by adding it under Settings → Emails, which reattributes past commits too. Refs #2301 Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
|
Emails are removed, now attributions of issues are based only on emails registered in the accounts |
… linters Both from review. argparse gives the script a usage line and rejects a mistyped flag, which the manual sys.argv walk silently read as "not --dry-run"; it also matches discover-image-matrix.py, the other script in this repository that takes arguments. The rest is what pre-commit would have caught before the review did: ruff at the pinned v0.15.21 reported eleven violations across the two scripts and ruff-format rewrote both. Prints stay, marked as the sibling scripts mark them — they are how a CI script reports. Refs #2301 Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
Refs #2301. Builds on #2351, now merged.
A finding that reaches
mainis announced exactly once, in the pull request that introduced it, and never again: the blocking job only ever scans a pull request's own commits, so later pull requests never see it. The nightly job does see it, every night, for as long as it is there — but it reports and exits zero, and nobody opens a run summary. A missed secret therefore looks the same as a clean repository.Two changes.
An issue per finding, assigned to whoever brought it
trufflehog_gate.pyalready computes the findings that are not in the baseline; it now also writes them to a JSON file when a sixth argument is given. Values are not included — the file carries the detector, the path, the line, the commit and the fingerprint.trufflehog_notify.pyreads that file, resolves each commit to a GitHub login through the commits API, and opens one issue per fingerprint, assigned to that person. One issue per fingerprint rather than a digest, because they close independently and each belongs to a different author; the fingerprint sits in the title, so an already-open issue suppresses the next night's duplicate.The assignee is resolved in two steps. GitHub can name a commit's author only when the address inside the commit is attached to an account, and a work address often is not — a quarter of this repository's commits resolve to nobody that way. The pull request a commit arrived with names an account either way, so it is the second route, and it covers every commit that reaches
mainthrough review.Neither the issue nor the scripts name an address. The body links the GitHub profile of whoever is resolved and the commit itself; when nothing resolves — a commit pushed straight to a branch — it says so and links the commit, which is where the author is visible anyway. An earlier revision carried a file mapping work addresses to logins; it is gone, because a single harvestable list of live addresses in a public repository is worth more than the historical commits it covered.
The fix for an address that resolves to nobody belongs with its owner: adding it under Settings → Emails attributes it, past commits included, and needs no table here. Attribution works from any verified address on an account, not only the primary one.
The blocking job runs in the merge queue
A required status check has to produce a result inside the merge queue, where there is no
pull_requestevent. The job now also triggers onmerge_group, and takes its scan range fromgithub.event.merge_group.base_sharather thanHEAD^1— on a queue branch the first parent is the previous entry, not the base.The two nightly jobs are pinned to
scheduleandworkflow_dispatch. They used to run on anything that was not a pull request, which would have meant a full-history scan and a GitHub API sweep on every entry the queue builds.This does not make the check required — that is a ruleset change, and it belongs after this has run in the queue at least once.
Test plan
detector, verified, commit, file, line, email, fptrufflehog_notify.py --dry-runover real findings assigns through both routes:aleksdotbarby commit author,ktursunovby pull request author, and a commit with neither is left unassignedpython -m py_compileon both scripts; the workflow parses and keeps its five, four and two steps per jobactionlint1.7.12 on this workflow and on every workflow in the repository — exit 0secrets (diff)green on this pull requestmerge_groupevent — only observable once this is on the default branchSummary by CodeRabbit
New Features
Bug Fixes