Skip to content

chore(pre-commit): scope the private-key hook exclusion and drop the dead gitleaks ignore - #2218

Merged
cristim merged 1 commit into
mainfrom
chore/tighten-private-key-hook
Oct 16, 2026
Merged

cristim merged 1 commit into
mainfrom
chore/tighten-private-key-hook

Conversation

@cristim

@cristim cristim commented Oct 16, 2026

Copy link
Copy Markdown
Member

Closes #448

Summary

This PR applies the cloud-commitments-cli copy of the 2026-09-02 audit finding A14-036. It changes config only, with no Go code.

  • Deleted .gitleaksignore. Its only entry pointed at internal/secrets/aws_resolver_httptest_test.go, which doesn't exist in this repo. Nothing here runs gitleaks: a case-insensitive grep finds no reference in CI, the Makefile, scripts or docs. So the file read as an active suppression while doing nothing. This also covers chore: prune monorepo residue left after the CLI split (lint/gitleaks/pre-commit config, NOTICE, docs) #2097 item 2. Wiring gitleaks into CI is still a separate decision, as the issue says.
  • Narrowed the detect-private-key exclusion. It was (_test\.go|frontend/src/index\.html)$, which skipped every Go test file and a frontend/ path that no longer exists. It is now exactly ^(cmd/configure_test\.go|cmd/configure_gcp_test\.go)$. Those are the only two tracked files that embed PEM-shaped dummy keys.

Evidence (local, macOS; pre-commit 4.7.0)

  • Exact fixture set: I ran the hook with an empty exclude over all files. Separately, the reviewer ran git grep for all 10 of the hook's substring patterns (RSA, DSA, EC, OPENSSH, plain PRIVATE KEY, PuTTY, SSH2 ENCRYPTED, PGP, ENCRYPTED, OpenVPN). Both checks matched only these two files.
  • Mutation probes:
    • A staged scratch _test.go file with a PEM header is now caught ("Private key found").
    • A staged OPENSSH header in docs/probe.txt and a PGP header in a new _test.go file are caught too.
    • None of the probe files were committed.
  • Checks:
    • pre-commit run detect-private-key --all-files passes.
    • pre-commit run check-yaml --all-files passes.
    • pre-commit run --all-files passes.
    • scripts/test-git-secrets-allowlist.sh is 44/44.
    • CI (.github/workflows/pre-commit.yml:177) runs pre-commit run --all-files against the same config, so it behaves the same as the local run.
  • Review: two passes by an independent non-author reviewer (sonnet, high). Neither pass found anything actionable at 5b7283d. It noted unrelated residue in the check-yaml exclude, which is recorded on chore: prune monorepo residue left after the CLI split (lint/gitleaks/pre-commit config, NOTICE, docs) #2097 rather than widened here.

🤖 Generated with Claude Code

…dead gitleaks ignore

The detect-private-key hook skipped every *_test.go file and a frontend
path that does not exist in this repo, so a real key committed in any
test file went unchecked. Exclude only the two files that embed
PEM-shaped dummy keys (cmd/configure_test.go, cmd/configure_gcp_test.go),
found by running the hook with no exclude.

.gitleaksignore is removed: nothing here runs gitleaks and its only
entry points at a file that does not exist in this repo.

Closes #448

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

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 35 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 65 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-cli/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: fc7ad477-8c9f-454f-b28e-37e1032d006b
📥 Commits

Reviewing files that changed from the base of the PR and between a1ea97e and 5b7283d.

📒 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 added priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline impact/internal Team-internal only effort/xs Trivial / one-liner type/chore Maintenance / non-user-visible triaged Item has been triaged bug Something isn't working labels Oct 16, 2026
@cristim

cristim commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

Independent review verdict: CLEAN at 5b7283d

Reviewer: fresh Claude agent (Sonnet, high effort), not the author. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

What was attacked and how (local macOS):

  • pre-commit run detect-private-key --all-files at this head: Passed.
  • Hook still catches keys: a planted -----BEGIN RSA PRIVATE KEY----- in a new, non-excluded zz_test.go makes the hook fail ("Private key found"). The probe was removed.
  • Narrowed exclusion is exact: grep -rIn 'BEGIN .*PRIVATE KEY' hits only cmd/configure_test.go (:98, 106, 560) and cmd/configure_gcp_test.go:267. Apart from those, only the .gitallowed regexes for those same files match, plus the split header in scripts/test-git-secrets-allowlist.sh:104. frontend/src/index.html no longer exists.
  • gitleaks: it is not referenced anywhere on main (CI, Makefile, pre-commit, scripts), so deleting .gitleaksignore breaks no gate. An ad-hoc gitleaks detect --no-git reports nothing in internal/secrets/aws_resolver_httptest_test.go.
  • scripts/test-git-secrets-allowlist.sh: 44 PASS, 0 FAIL.

Non-blocking note: that ad-hoc gitleaks run flags 4 unrelated, ungated findings: .github/workflows/pre-commit.yml:96, cmd/archera_comparison_key_test.go:20, cmd/configure_test.go:98 and cmd/archera_comparison_test.go:33.

Gate:

@cristim
cristim merged commit dc81a9f into main Oct 16, 2026
12 checks passed
@cristim
cristim deleted the chore/tighten-private-key-hook branch October 16, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 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 fingerprints (suppression not effective)

1 participant