fix(standards): rename GITLEAKS_SHA256→GITLEAKS_CHECKSUM, add .gitleaks.toml template#167
fix(standards): rename GITLEAKS_SHA256→GITLEAKS_CHECKSUM, add .gitleaks.toml template#167don-petry wants to merge 52 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 9 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a standardized gitleaks secret-scan: requires a repository-root ChangesGitleaks Secret-Scan Standardization
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (repo)
participant CI as GitHub Actions Runner
participant GL as Gitleaks Binary
participant Script as push-protection.sh
participant Org as Org Secrets Store
Dev->>CI: Push/PR triggers workflow (.github/workflows/ci.yml)
CI->>Org: read GITLEAKS_CHECKSUM & GITLEAKS_LICENSE
CI->>GL: download gitleaks@GITLEAKS_VERSION
CI->>GL: verify tarball using GITLEAKS_CHECKSUM
CI->>GL: install and add to PATH
CI->>GL: run gitleaks detect --config .gitleaks.toml --redact --exit-code 1
GL-->>CI: report findings / exit code
CI->>Script: run audit check secret_scan_ci_job_present (pattern match)
Script-->>CI: pass/fail enforcement result
CI-->>Dev: report status with remediation guidance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
Pull request overview
Updates the security standards around gitleaks-based secret scanning by switching the canonical CI job from the gitleaks/gitleaks-action to a manual binary install pattern, standardizing the checksum env var name, and introducing a required .gitleaks.toml template to suppress known false positives (notably BMAD paths).
Changes:
- Replace the Layer 3 canonical
secret-scanjob to download/verify/install the gitleaks binary and rungitleaks detect --config .gitleaks.toml. - Add a canonical
standards/gitleaks.tomltemplate and document the repo-root.gitleaks.tomlrequirement. - Update CI standards docs to reflect
.gitleaks.tomlrequirements, failure modes, and onboarding checklist updates.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
standards/push-protection.md |
Updates the canonical secret-scan job and adds a .gitleaks.toml template section. |
standards/gitleaks.toml |
Adds a new template config intended to be copied to repo root as .gitleaks.toml. |
standards/ci-standards.md |
Documents the required .gitleaks.toml artifact, checksum env var naming, updates failure table, and onboarding checklist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@standards/ci-standards.md`:
- Line 302: Update the guidance for the "Secrets found" table entry to stop
implying allowlisting is an acceptable default: change the cell that currently
reads "Legitimate secrets in the code | Add the path to `.gitleaks.toml`
`allowlist.paths`, or remove the secret" to instruct that real secrets must be
removed and rotated, and that adding paths to `.gitleaks.toml` `allowlist.paths`
is only allowed for confirmed false positives after documented justification and
approval; reference the "Secrets found" table header and the `.gitleaks.toml`
`allowlist.paths` symbol when making the change so reviewers can locate and
tighten the remediation text accordingly.
In `@standards/push-protection.md`:
- Around line 258-282: The document currently conflicts on which gitleaks
artifact to use: it mandates `.gitleaks.toml` in the “.gitleaks.toml template”
section but still instructs using `.gitleaksignore` in the “Writing tests and
fixtures” section; update the “Writing tests and fixtures” text to reference
`.gitleaks.toml` (and any example commands or file names there) so all guidance
consistently uses `.gitleaks.toml` and remove or replace any mention of
`.gitleaksignore`.
- Around line 227-241: Update the audit function secret_scan_ci_job_present in
scripts/lib/push-protection.sh (current logic at ~180-206 and similar check at
~250-255) so it no longer only greps for "uses: gitleaks/.*-action" but also
recognizes the new canonical job pattern by looking for the job name "name:
Install gitleaks" combined with the CLI invocation "gitleaks detect --config
.gitleaks.toml" (or the presence of both the install step and the detect
command); also update the compliance table description string that refers to the
old action to mention the new standard (Install gitleaks + gitleaks detect
--config .gitleaks.toml) so compliant repos are correctly classified.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 94305ad7-e024-461f-97cf-4874528512bd
📒 Files selected for processing (3)
standards/ci-standards.mdstandards/gitleaks.tomlstandards/push-protection.md
There was a problem hiding this comment.
♻️ Duplicate comments (1)
standards/ci-standards.md (1)
302-302:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTighten secret-remediation guidance to avoid allowlisting real leaks.
Line 302 currently implies allowlisting is acceptable for any "Secrets found" failure. This weakens incident response: real secrets should be removed/rotated, and allowlist should be limited to confirmed false positives.
The guidance should clarify that adding paths to
.gitleaks.tomlallowlist.pathsis only permitted for verified false positives after proper review, while actual secrets must always be removed and rotated immediately.🔒 Suggested documentation fix
-| Secrets found | Legitimate secrets in the code | Add the path to `.gitleaks.toml` `allowlist.paths`, or remove the secret | +| Secrets found | Secret detection triggered | Remove and rotate the secret immediately; use `.gitleaks.toml` `allowlist.paths` only for confirmed false positives |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@standards/ci-standards.md` at line 302, Update the guidance for the "Secrets found" row so it no longer implies blanket allowlisting: specify that adding paths to `.gitleaks.toml` `allowlist.paths` is only allowed for verified false positives after a documented review/approval, and that any actual secrets discovered must be removed and rotated immediately; reference `.gitleaks.toml` and `allowlist.paths` explicitly and add a short required checklist (verify false positive, obtain approval, document decision) to accompany any allowlist entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@standards/ci-standards.md`:
- Line 302: Update the guidance for the "Secrets found" row so it no longer
implies blanket allowlisting: specify that adding paths to `.gitleaks.toml`
`allowlist.paths` is only allowed for verified false positives after a
documented review/approval, and that any actual secrets discovered must be
removed and rotated immediately; reference `.gitleaks.toml` and
`allowlist.paths` explicitly and add a short required checklist (verify false
positive, obtain approval, document decision) to accompany any allowlist entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 57784b43-e376-4a35-b6e6-81879df5f308
📒 Files selected for processing (1)
standards/ci-standards.md
|
@claude Please address all open review comments on this PR from CodeRabbit and Copilot. |
|
Claude finished @don-petry's task in 5m 14s —— View job Addressed All Open Review CommentsAll 5 review comments from CodeRabbit and Copilot have been fixed in commit Tasks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/lib/push-protection.sh`:
- Around line 207-209: The has_binary check only matches when `gitleaks detect`
and `--config .gitleaks.toml` are on the same line; update the detection to
normalize newlines in `ci_content` (e.g., collapse newlines to spaces into a
temp variable) and run the grep against that collapsed content so multiline
`run:` blocks are matched; keep the same pattern
(`gitleaks[[:space:]]+detect.*--config.*\.gitleaks\.toml`) and set `has_binary`
from that grep result (refer to variables `ci_content` and `has_binary` in the
diff).
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f579dea6-36ee-4c71-8638-a0dd0f2b912e
📒 Files selected for processing (3)
scripts/lib/push-protection.shstandards/ci-standards.mdstandards/push-protection.md
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: bc94343161ba06958be139fbea7ef4e76e239ebc
Cascade: triage → deep (triage: haiku 4.5 → deep: sonnet 4.6 + duck: gpt-5.4 → audit: opus 4.6)
Summary
PR migrates gitleaks secret-scanning from a GitHub Action to a pinned binary install with SHA-256 checksum verification, renames the env var to avoid SonarCloud false positives, and adds a required .gitleaks.toml template. All CI gates pass (SonarCloud 0 issues/0 hotspots, CodeQL, Lint, ShellCheck all green). The one remaining CodeRabbit concern — the compliance audit script's has_binary grep failing on multiline YAML run blocks — is a false-negative audit accuracy issue, not a security vulnerability; the actual CI job in adopting repos is correctly formed and enforces secret scanning. Approving as MEDIUM-risk with the audit script limitation noted as a minor finding to address in follow-up.
Findings
- MINOR: The
has_binarygrep inpp_check_secret_scan_ci_jobuses a single-line match (grep -cE 'gitleaks[[:space:]]+detect[[:space:]].*--config[[:space:]].*\.gitleaks\.toml'). If a repo'sci.ymlsplitsgitleaks detectand--config .gitleaks.tomlacross multiple lines in a YAMLrun:block, the compliance check will return a false-negative (non-compliant) even though the job is valid. CodeRabbit flagged this in the latest review cycle and it remains unresolved. The actual security enforcement (the CI job itself) is unaffected — only the audit tool has a detection gap. - INFO: Hardcoded gitleaks binary checksum (
551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470ebfor v8.30.1) is embedded in the canonical standard. This must be manually updated when the version is bumped. Consider adding an inline comment referencing the official release checksums page for maintainability. - INFO: Removing
security-events: writepermission means gitleaks findings are no longer uploaded to the GitHub Security tab as SARIF. The old action uploaded SARIF automatically. This is intentional and documented, but reduces centralized finding visibility in the GitHub Security dashboard. - INFO: The canonical job now passes
--config .gitleaks.tomlunconditionally; omitting the file is a hard failure. Repos already using the oldgitleaks/gitleaks-actionpattern are not affected (both patterns are accepted by the audit script), but any repo adopting the new pattern without first adding.gitleaks.tomlwill see immediate CI failures. Migration guidance is clearly documented. - INFO: Binary install approach with pinned version + checksum verification is equivalent in supply-chain integrity to pinning a GitHub Action by commit SHA, and removes the need for a GITLEAKS_LICENSE secret for org repos. The
$RUNNER_TEMPinstall directory (not/usr/local/bin) correctly avoids the need for root/sudo on GitHub-hosted runners.
Reviewed by the don-petry PR-review cascade (triage: haiku 4.5 → deep: sonnet 4.6 + duck: gpt-5.4 → audit: opus 4.6). Reply with @don-petry if you need a human.
|
@claude Please address all open review comments on this PR from CodeRabbit and Copilot. |
|
Claude finished @don-petry's task in 1m 25s —— View job Addressed Open Review Comment
What was fixed
The previous # This valid pattern was previously flagged as non-compliant:
run: |
gitleaks detect \
--config .gitleaks.toml \
--source . --redactThe fix replaces the single-line grep with two independent greps (one for - has_binary=$(echo "$ci_content" | grep -cE 'gitleaks[[:space:]]+detect[[:space:]].*--config[[:space:]].*\.gitleaks\.toml' || true)
+ if echo "$ci_content" | grep -qE 'gitleaks[[:space:]]+detect' \
+ && echo "$ci_content" | grep -qE -- '--config([[:space:]]|=).*\.gitleaks\.toml'; then
+ has_binary=1
+ else
+ has_binary=0
+ fi| Branch |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/lib/push-protection.sh`:
- Around line 207-212: The current dual independent greps on ci_content can
produce false negatives; collapse newlines in ci_content into a single-line
buffer (e.g., replace newlines with spaces) and run a single anchored regex
search that requires "gitleaks detect" and the "--config.*.gitleaks.toml" token
to appear in the same contiguous run, then set has_binary accordingly; update
the if-condition that uses ci_content and sets has_binary so it tests the
collapsed buffer with one grep/regex (e.g., matching
gitleaks[[:space:]]+detect.*--config([[:space:]]|=).*\.gitleaks\.toml) instead
of the two independent grep -qE calls.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 779c5bf5-24eb-44f2-8262-36c789094704
📒 Files selected for processing (1)
scripts/lib/push-protection.sh
|
@claude This PR has 3 unresolved review threads. The two Copilot threads from 2026-04-28 predate the
Push the changes and resolve the threads. Generated by Claude Code |
|
Claude finished @don-petry's task in 2m 7s —— View job Addressed 3 Unresolved Review Threads
What changed in
|
Superseded by automated re-review at
|
|
@coderabbitai resolve |
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
✅ Action performedComments resolved and changes approved. |
Dev-Lead Fix CI — failedPR: #167 | SHA: |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Summary
standards/push-protection.md— Replaces thegitleaks/gitleaks-actionbased canonical job (Layer 3) with the manual-install pattern usingGITLEAKS_CHECKSUM(notGITLEAKS_SHA256). Adds a new### .gitleaks.toml templatesubsection explaining why the config file is required and what to put in it. Drops the now-unusedsecurity-events: writepermission from the job.standards/gitleaks.toml— New canonical template. Copy to repo root; extendallowlist.pathsfor repo-specific false positives. Ships with'''_bmad/'''pre-populated.standards/ci-standards.md— Adds a "Required repo artifact" callout under §4 Secret Scanning explaining the.gitleaks.tomlrequirement and theGITLEAKS_CHECKSUMnaming rule. Updates the CI failure table (addsconfig file not foundrow, fixes the.gitleaksignore→.gitleaks.tomlreference). Adds step 3 to the "Applying CI to a New Repository" checklist.Why these changes
GITLEAKS_CHECKSUMnotGITLEAKS_SHA256: SonarCloud's security gate flags env var names matching*SHA256*that contain hex strings as Security Hotspots (hardcoded credential false positive). Renaming toGITLEAKS_CHECKSUMsuppresses the false positive without any functional change —sha256sum -creads the value regardless of the variable name.Required
.gitleaks.toml: Thegeneric-api-keyrule in gitleaks fires on BMAD knowledge file paths (e.g.api-request.md,auth-session.mdin_bmad/directories) because their names contain substrings gitleaks treats as API-key indicators. Every adopting repo needs a config file that allowlists these paths;--config .gitleaks.tomlis now part of the canonical Run step so omitting the file is a hard failure rather than a silent miss.Test plan
push-protection.mdLayer 3 canonical job YAML is well-formed (correct step indentation, no stray nesting)standards/gitleaks.tomlis valid TOML (tomlq . standards/gitleaks.tomlor equivalent)standards/gitleaks.toml→.gitleaks.toml, add thesecret-scanjob withGITLEAKS_CHECKSUM, confirm SonarCloud reports no Security Hotspot for the env vargitleaks detect --config .gitleaks.tomldoes not fire on_bmad/path fixtures🤖 Generated with Claude Code
Summary by CodeRabbit