Skip to content

fix(security): enable gitleaks default rules in .gitleaks.toml template (#1395) - #1412

Merged
don-petry merged 3 commits into
mainfrom
fix/sec-gitleaks-default-rules
Oct 28, 2026
Merged

don-petry merged 3 commits into
mainfrom
fix/sec-gitleaks-default-rules

Conversation

@don-petry

@don-petry don-petry commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

.gitleaks.toml and the standards/gitleaks.toml template held only an [allowlist]. A file passed via --config replaces gitleaks' built-in ruleset, so the Secret scan (gitleaks) job loaded zero detection rules and passed on any history. This PR adds [extend] useDefault = true to both files, adds detection regression tests that run the real scanner, and documents the requirement.

Fixes #1395

How it was confirmed. Local gitleaks is v8.30.1, the same version ci.yml pins (.github/workflows/ci.yml secret-scan job; standards/workflows/ci.yml pins 8.30.1 too). I generated synthetic, obviously fake credentials at runtime (an AWS-access-key-shaped string and a ghp_-shaped string) and scanned them:

Config Result
.gitleaks.toml (origin/main) no leaks found, exit 0
standards/gitleaks.toml (origin/main) no leaks found, exit 0
same file + [extend] useDefault = true 3 findings (aws-access-token, github-pat, generic-api-key), exit 1

I got the same results with gitleaks dir, gitleaks detect --no-git, and git-history mode (gitleaks detect --source <repo>, the invocation CI uses).

Fix

  • .gitleaks.toml and standards/gitleaks.toml: add [extend] useDefault = true, with a comment saying it must not be removed. The _bmad/ path allowlist from [Fleet Monitor] petry-projects/.github — .github/workflows/ci.yml #996 is unchanged, so default rules plus that allowlist is the behavior [Fleet Monitor] petry-projects/.github — .github/workflows/ci.yml #996 originally intended.
  • .github/workflows/ci.yml: comment only. The Run gitleaks comment wrongly said --config exists to avoid the default ruleset. It now explains the allowlist and the mandatory extend block. The command is unchanged.
  • standards/push-protection.md: the template block now includes the extend block, plus a paragraph explaining why it is mandatory.
  • standards/workflows/ci.yml is unchanged, because the root cause is the config file and not the job.

History check with default rules enabled. I ran gitleaks detect --source . --config .gitleaks.toml --redact --exit-code 1 over this repo's full history (all refs, 1,401 commits, including this branch) with the new config: no leaks found. Turning the rules on does not red main.

Regression tests (test/workflows/ci/secret-scan-config.bats, TDD: written first; on origin/main tests 4–7 and 9 fail, and all pass with the fix)

  • Both configs set useDefault = true inside [extend] (static parse).
  • Both configs detect the synthetic secrets when gitleaks detect runs in git-history mode against a throwaway repo. Assertions are on exit 1 plus the aws-access-token and github-pat RuleIDs in the JSON report.
  • Control: an allowlist-only config detects nothing. This documents the failure mode and shows the tests can tell a working config from a zero-rule one.
  • ci-tests.yml installs the same gitleaks version and checksum as ci.yml, so the tests can't silently drift from the binary CI scans with.
  • Fixture values are put together at test time from split string pieces. No secret-shaped literal is committed, and the repo's own scan stays clean (confirmed by the history scan above).
  • If gitleaks is missing, the tests fail with a message and are never skipped (AGENTS.md).

CI wiring: .github/workflows/ci-tests.yml now installs gitleaks v8.30.1 exactly the way ci.yml does: gh release download with a bounded retry loop and sha256sum -c against the same pinned checksum. Its paths: filters now include .gitleaks.toml and standards/gitleaks.toml.

Test commands

  • bats test/workflows/ci/: 21/21 ok
  • actionlint .github/workflows/ci.yml .github/workflows/ci-tests.yml: clean
  • shellcheck -s bash test/workflows/ci/secret-scan-config.bats: clean
  • markdownlint-cli2 standards/push-protection.md: 0 errors
  • Full-history gitleaks detect with the new config: no leaks found

Follow-ups / rollout (needs a human decision)

  • Downstream repos are affected too. Every repo that copied the old standards/gitleaks.toml (seeded by seed-repo-template.sh, which lives outside this repo) is also scanning with zero rules. This PR fixes the template only. Existing copies need a standards-sync pass, and those repos' histories have never been scanned with real rules. Turning the rules on may red their Secret scan (gitleaks) required check until findings are triaged or rotated. I'd suggest running gitleaks git --redact per repo before or alongside the sync.
  • No compliance check currently validates .gitleaks.toml contents (scripts/lib/push-protection.sh only checks that ci.yml invokes gitleaks). A content check such as gitleaks_config_extends_defaults could catch regressions fleet-wide. It is left out here because it would flag every existing repo at once, so it should be planned together with the sync above.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

Problem

See the linked issue.

Risk

Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.

Test plan

Tests added/updated: test/workflows/ci/secret-scan-config.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.

…te (#1395)

A config passed via `--config` replaces gitleaks' built-in ruleset. The
allowlist-only .gitleaks.toml and standards/gitleaks.toml had no
`[extend] useDefault = true`, so the secret-scan job ran with zero rules
and reported "no leaks found" on any history. Add the extend block to
both files (the _bmad/ allowlist from #996 is kept).

Add detection regression tests to test/workflows/ci/secret-scan-config.bats
that run the pinned gitleaks against a throwaway repo with synthetic
credentials generated at test time, and install the same pinned,
checksum-verified gitleaks release in ci-tests.yml (asserted to stay in
lockstep with ci.yml). Document the mandatory extend block in
standards/push-protection.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@don-petry
don-petry requested a review from a team as a code owner October 28, 2026 03:06
@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

This comment has been minimized.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #1412
No changes were committed, but the PR still can't be marked done: required check SonarCloud is still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-10-28T03:37:26Z

@don-petry

Copy link
Copy Markdown
Contributor Author

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required check SonarCloud is still pending. I'll re-check automatically.
Next attempt after: 2026-10-28T03:37:26Z

@don-petry
don-petry enabled auto-merge (squash) October 28, 2026 03:07
@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-28T04:08:37Z.

@donpetry-bot

donpetry-bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry
don-petry disabled auto-merge October 28, 2026 05:27
@don-petry

Copy link
Copy Markdown
Contributor Author

Codex review failed to complete — this is a transient tooling issue, not a code defect. All CI checks pass. No code changes are needed.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
## Summary
**Bot:** chatgpt-codex-connector[bot]
**Issues addressed:** 0 (operational notice only)
**Finding:** Codex review failed to complete — transient tooling issue, not a code defect. All CI checks pass and no actionable code findings reported.
**Action taken:** Posted disposition reply with `informational` marker on the original bot comment (node id: `IC_kwDORyesfc8AAAABgQVMFw`). The harness will minimize the comment RESOLVED.
**Files changed:** None
The PR is ready — all meaningful CI checks pass, and the Codex review failure is a transient tool issue, not a code problem.

@don-petry
don-petry enabled auto-merge (squash) October 28, 2026 05:27
@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@don-petry
don-petry disabled auto-merge October 28, 2026 08:37
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) October 28, 2026 08:37
@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry
don-petry disabled auto-merge October 28, 2026 11:32
@don-petry

This comment has been minimized.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
## Summary
**Bot:** coderabbitai[bot]  
**Issues addressed:** 0  
**Disposition:** Informational — rate-limit notice  
The bot comment is a **rate-limit notice** indicating that CodeRabbit's free OSS review capacity was exhausted. The comment contains no actual code findings, no security concerns, and no actionable items — only an operational notice that the review was throttled.
- **Action taken:** Posted informational disposition on the original comment (node id: `IC_kwDORyesfc8AAAABgQVPXA`)
- **Code changes required:** None
- **Files changed:** None
- **CI status:** All checks passing
The harness will verify the disposition and resolve the comment, clearing the maintainer-comment gate.

@don-petry
don-petry enabled auto-merge (squash) October 28, 2026 11:33
@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-28T12:48:13Z.

@sonarqubecloud

Copy link
Copy Markdown

@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-28T16:41:43Z.

@don-petry
don-petry disabled auto-merge October 28, 2026 15:42
@don-petry

Copy link
Copy Markdown
Contributor Author

Re-read the current body of this CodeRabbit summary. It contains only the review-limit notice (free OSS reviews used, next review in ~59 minutes), run configuration, the list of 6 files selected, and the Autofix opt-in checkbox. There is no Security Architecture section, no actionable, outside-diff or nitpick findings. No action needed; no code change.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-reviews (no-changes)

Agent reasoning
Addressed 0 threads: there were no open review threads, and no CI check had failed. The `reviews` list was empty, so no review had requested changes.
- **Stale disposition:** I re-read the current body of CodeRabbit comment `IC_kwDORyesfc8AAAABgQVPXA`. It has only the review-limit notice, the run configuration, the list of 6 files selected and the Autofix checkbox. It has no Security Architecture section and no actionable, outside-diff or nitpick findings. I posted a fresh `informational` disposition reply: https://github.com/combsmith/.github/pull/1412#issuecomment-6473517662
- **Test verification:** not run, because I made no code changes.
- **Files changed:** none.

@don-petry
don-petry enabled auto-merge (squash) October 28, 2026 15:43

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 095ef5c190b484e90dbfdf754805659b1c46a653
Review mode: triage-approved (single reviewer)

Summary

Fixes a real security misconfiguration: .gitleaks.toml and the standards/gitleaks.toml template were allowlist-only. Because a --config file replaces gitleaks' built-in ruleset, the Secret scan (gitleaks) job ran with zero rules. The PR adds [extend] useDefault = true to both files, corrects the misleading comment in ci.yml (comment only, the command is unchanged), documents the requirement in standards/push-protection.md, and adds bats tests that run the pinned gitleaks binary against a throwaway repo holding synthetic credentials, plus a zero-rule control. ci-tests.yml installs gitleaks v8.30.1 with the same retry loop and SHA-256 checksum as ci.yml (job permissions contents: read). A test checks that the two workflows stay pinned to the same version. The change only tightens the security control, and this PR's own Secret scan (gitleaks) check passed with the default rules turned on, so enabling them does not red main. Triage's low-risk assessment holds. I rate it MEDIUM rather than LOW because it changes CI workflow behaviour and an org standards template.

Linked issue analysis

Closes #1395 (gitleaks secret scan runs with zero rules). The issue asks for default-rule inheritance and notes that the existing tests only checked that the file exists and that the command passes --config, not that a synthetic secret is detected. The PR does both: it enables useDefault in both configs and adds detection tests that assert the aws-access-token and github-pat RuleIDs, plus a control test. The issue is fully addressed, so Fixes #1395 is appropriate.

Findings

No blocking findings.

Non-blocking notes:

  • Downstream rollout (author-flagged). Repos that already copied the old template still scan with zero rules, and syncing the fix may red their required Secret scan (gitleaks) check until historical findings are triaged. That rollout is correctly left out of this PR. Consider opening a tracking issue for the standards-sync and the proposed gitleaks_config_extends_defaults compliance check so they are not lost when #1395 auto-closes.
  • Test brittleness (minor). The detection assertions grep for "RuleID": "..." in the JSON report, which depends on gitleaks' output formatting. That is acceptable while the version is pinned and the lockstep test exists, but expect to revisit it on a gitleaks upgrade.
  • Fixture hygiene. Synthetic credentials are assembled at runtime from split literals, so nothing secret-shaped is committed. The repo's own gitleaks scan passed on this head.
  • Secret scan (MCP). run_secret_scanning was not available in this environment, so it was skipped. The gitleaks CI check passed.

CI status

All checks completed without failures. Secret scan (gitleaks), bats (x2), Lint, ShellCheck, CodeQL / Analyze (actions, python), SonarCloud (Quality Gate passed), AgentShield, Agent Security Scan, duplicate-decl-gate and dependency-audit all succeeded. Skipped and neutral checks (dependabot, dev-lead relay jobs, the inapplicable audit ecosystems, and CodeRabbit, which was rate-limited) are not applicable. There are no unresolved review threads and no outstanding human questions.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

@donpetry-bot

Copy link
Copy Markdown
Contributor

pr-review approved on PARTIAL advisory evidence: 1/5 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).

@don-petry
don-petry merged commit 9ed4b30 into main Oct 28, 2026
37 checks passed
@don-petry
don-petry deleted the fix/sec-gitleaks-default-rules branch October 28, 2026 17:21
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.

gitleaks secret scan runs with zero rules: .gitleaks.toml and standards/gitleaks.toml define only an allowlist with no useDefault

2 participants