Skip to content

sec(pre-commit): scan Go test files for private keys - #70

Merged
cristim merged 1 commit into
mainfrom
sec/13-gitleaks-private-key-gate
Oct 17, 2026
Merged

cristim merged 1 commit into
mainfrom
sec/13-gitleaks-private-key-gate

Conversation

@cristim

@cristim cristim commented Oct 16, 2026

Copy link
Copy Markdown
Member

Summary

  • detect-private-key now scans Go test files. The hook excluded every _test.go file, plus a frontend/src/index.html this repo does not have. A PEM key pasted into a test fixture therefore passed both the local hook and CI, which runs pre-commit run --all-files. No tracked file contains a key marker, so the exclusion and its stale monorepo comment are dropped entirely, not narrowed. scripts/test-git-secrets-allowlist.sh builds its key fixture at runtime from split literals, so the hook does not match it.
  • .gitleaksignore is removed. Its only entry named internal/secrets/aws_resolver_httptest_test.go, which does not exist in this repository. Nothing in CI, the hooks, the Makefile or scripts runs gitleaks or reads the file.

Closes #13. The issue's third item, whether to wire gitleaks into CI, is an open question there and is not done here; it is left for the owner. Today the private-key hook and the git-secrets AWS scan cover this repo.

Verification (local, macOS, at d1d2809)

  • pre-commit run detect-private-key --all-files passes on the branch.
  • Before and after, with a PEM block planted in a scratch _test.go file:
    • With origin/main's config, the hook reports "(no files to check) Skipped": the key gets through.
    • With this branch's config, it reports "Failed - Private key found": the key is caught.
  • All pre-commit hooks pass on the commit.

Review

A non-author reviewer (Sonnet, high effort) did two passes and reproduced the planted-key result against both configs. It grepped the repo for every key-marker family and for any gitleaks or .gitleaksignore reference, and confirmed CI's pre-commit job runs this hook over the same tracked files. It agreed deleting the file is better than emptying it. No actionable findings. CodeRabbit is not pinged because the org's CR quota is exhausted.

🤖 Generated with Claude Code

detect-private-key excluded every _test.go file (and a frontend file
this repo does not have), so a PEM key pasted into a test fixture
passed the local and CI gate. No tracked file carries a key marker,
so the exclusion is dropped entirely. Verified: a key planted in a
_test.go file is skipped with the old config and caught with the new
one.

.gitleaksignore is removed: its only entry named
internal/secrets/aws_resolver_httptest_test.go, which does not exist
in this repository, and nothing here runs gitleaks.

Closes #13

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added impact/internal Team-internal only type/chore Maintenance / non-user-visible priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline effort/xs Trivial / one-liner triaged Item has been triaged labels Oct 16, 2026
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 64 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-mcp/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 1fd4f8a9-42b8-4ff4-85c4-6feb2b66c33e
📥 Commits

Reviewing files that changed from the base of the PR and between 02ea149 and d1d2809.

📒 Files selected for processing: 2
  • .gitleaksignore
  • .pre-commit-config.yaml
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

Merge-driver verdict: CLEAN at d1d2809e22cf74a088064093415718f4f2bd4fd2

An independent adversarial reviewer (Sonnet, high effort, cold brief, not the author) reviewed the full diff. It found no blocking findings.

Local verification (real, macOS, pre-commit 4.7.0):

  • Behaviour change (fixture key): I planted a private key in a _test.go file inside a throwaway repo and ran detect-private-key against it.
    • main's config skips it ("no files to check").
    • This PR's config catches it ("Private key found").
  • Current tree (real): pre-commit run detect-private-key --all-files passes on the PR tree.
  • .gitleaksignore deletion: its only entry was a bare path, internal/secrets/aws_resolver_httptest_test.go, which no longer exists in the tree, so the deletion unsuppresses nothing. No workflow, script or Makefile references gitleaks or the file.

Item 3 of #13 (gitleaks in CI): still an open owner decision. I filed it as #73, so closing #13 does not lose it.

Gate items:

@cristim

cristim commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

Gate record for head d1d2809e22cf74a088064093415718f4f2bd4fd2: CR waived: quota, adversarial review + local verification + green CI. The verdict and evidence are in the merge-driver comment above. This comment also requests a retrospective CodeRabbit pass; any actionable findings it raises will be resolved.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim

cristim commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

Automated review of d1d2809: no blocking findings and CI is green. Ready to merge.
Small, correct change. Dropping the _test.go exclusion makes detect-private-key scan Go tests. A grep finds no key markers in tracked files. The deleted .gitleaksignore pointed at a nonexistent path, and nothing references gitleaks. No bugs found.

  • (optional) .pre-commit-config.yaml:62: No regression test pins the hook scanning _test.go files; the PR relies on a manual planted-key check. Acceptable for a config change.

Posted by a read-only review sweep. It stays silent on this commit and re-reviews only after a new push.

@cristim
cristim merged commit e15bcd7 into main Oct 17, 2026
13 checks passed
@cristim
cristim deleted the sec/13-gitleaks-private-key-gate branch October 17, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/eventually No deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sec: gitleaks .gitleaksignore has invalid/dead fingerprints; pre-commit test-exclusion gap [mcp part]

1 participant