sec(scripts): anchor .gitallowed placeholder entries

[cristim]

Summary

• Removes the 18 bare placeholder entries (12-digit account IDs, the UUID) from .gitallowed. git-secrets matches an allowed regex against the whole path:line:content line and exempts the line from every detector, so a key or password beside one of them scanned clean.
• Adds one entry anchored on path and whole line for the only fixture the account-ID detector flags: tools/purchase_test.go Account: "123456789012",.
• Rewrites the .gitallowed header, adds a CHANGELOG line.

Closes #24 (part 1). The DSN detector audit (part 2) is a separate issue.

Evidence

• Pre-fix false negatives (exit 0, should be 1): AKIA-shaped fixture key beside a placeholder account ID; password = "..." beside one; a GCP-shaped key beside the UUID.
• Scanning every tracked file with only the --register-aws patterns (CI's setup) and .gitallowed emptied: the only placeholder-related hit is tools/purchase_test.go:138. No other bare entry matches a scanner hit, including the Archera tool (feat(tools): add read-only Archera comparison tool #55) fixtures and env names (ARCHERA_API_KEY, test-archera-key-0000, 222222222222 in testdata); nothing is newly allow-listed.
• bash scripts/test-git-secrets-allowlist.sh: 50 PASS, 0 FAIL. Against the old .gitallowed the new must-catch cases fail (8 FAIL lines).
• pre-commit run git-secrets --all-files passes with the new file.
• CI runs the self-test: .github/workflows/pre-commit.yml "Run git-secrets allowlist self-test" step.
• Local evidence is the real scripts and git-secrets on macOS; Linux is covered by CI.

Sibling repos (go, platform, cli) carry the same bare entries; follow-ups are tracked separately.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Security
  • Placeholder account IDs and UUIDs no longer prevent other credentials on the same line from being detected.
  • The permitted test-fixture account ID is limited to its specific context; adding a key to that line is still flagged.
  • Expanded checks confirm that nearby AWS and GCP keys, passwords, and non-placeholder account IDs remain detectable.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

• 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 46 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 63 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: 94978660-c1bb-47aa-8ea3-4421e7d9161b

---

📥 Commits

Reviewing files that changed from the base of the PR and between ecd085c and f625d67.

---

📒 Files selected for processing (1)

• scripts/test-git-secrets-allowlist.sh

---

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-mcp/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 90831456-d3cf-4b68-8a52-efda626547ce

---

📥 Commits

Reviewing files that changed from the base of the PR and between d0f8f08 and ecd085c.

---

📒 Files selected for processing (3)

• .gitallowed
• CHANGELOG.md
• scripts/test-git-secrets-allowlist.sh

---

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

---

📝 Walkthrough

📝 Walkthrough

Walkthrough

The change removes broad placeholder account-ID and UUID entries from .gitallowed, adds a path- and line-anchored exception for one account-ID fixture, and extends scanner self-tests to check that placeholder matches do not suppress other detector findings.

Changes

Placeholder allowlist hardening

Layer / File(s)	Summary
Document and scope placeholder exceptions .gitallowed	The guidance describes line-wide detector exemptions and requires path-anchored entries. A single account-ID fixture exception is anchored to its path and matching line.
Test scanner behavior for placeholder matches scripts/test-git-secrets-allowlist.sh, CHANGELOG.md	Self-tests check placeholder IDs and UUIDs alongside other detector findings, and verify that adding a key to the scoped fixture is still caught. The changelog records the allowlist and test changes.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

---

Merge Risk: ⚪ Minimal · up to ecd08

The fixture remains scannable without broadly exempting lines containing placeholder IDs. No change is needed before merging.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check		The PR satisfies the placeholder-entry requirement in #24. .gitallowed removes the bare entries and adds one path- and whole-line-anchored exception for tools/purchase_test.go. The self-test cover…	Complete and document the PostgreSQL, MySQL, and MongoDB DSN detector audit required by #24. File separate follow-up fixes when the audit finds coverage gaps.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: anchoring placeholder entries in .gitallowed.
Out of Scope Changes check		The reviewed changes stay within #24. The .gitallowed update implements the placeholder decision. The shell-script cases verify the detector behavior. The changelog entry documents the security chan…
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…

---

Full details: Linked Issues check

Explanation

The PR satisfies the placeholder-entry requirement in #24. .gitallowed removes the bare entries and adds one path- and whole-line-anchored exception for tools/purchase_test.go. The self-test covers adjacent AWS, GCP, password, and account-ID detections. However, #24 also requires an audit of the PostgreSQL, MySQL, and MongoDB DSN detectors. The PR description states that this audit remains separate, and the reviewed changes provide no audit result.

---

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

---

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[cristim]

Gate finding 1 (blocking): self-test does not guard against restoring the other 16 bare entries

Mutation run at ecd085c52fbbf18c8e31659422629e11006ea6c5, scripts/test-git-secrets-allowlist.sh with a mutated .gitallowed (macOS, git-secrets 1.3.0, throwaway repo, fake AKIA/AIza fixtures only):

mutation	self-test
restore bare 123456789012	44 PASS, 6 FAIL (caught)
restore bare UUID	48 PASS, 2 FAIL (caught)
drop trailing $ on new entry	48 PASS, 2 FAIL (caught)
restore bare 222222222222	50 PASS, 0 FAIL (survives)

The new must-catch cases (scripts/test-git-secrets-allowlist.sh:129-140) only use 123456789012 and the UUID, so re-adding any of the other 16 removed placeholders (000000000000...999999999999, 210987654321, 111222333444, 999988887777, ...) reopens #24 with CI green. Direct probe: with 222222222222 restored, a line <AIza key> 222222222222 exits 0.

Suggested fix: one structural case instead of more literals, e.g. assert every non-comment, non-blank line of .gitallowed starts with ^ (which is the rule the new header states), or loop the must-catch case over the 18 removed values.

Non-blocking, same area:

• scripts/test-git-secrets-allowlist.sh:182 "the one placeholder account ID fixture scans clean" is vacuous: the harness scans via --cached/hook, where the account-ID detector never fires on that line (exits 0 even with the anchored entry deleted, and for the same line in another file). Mutation "path widened to ^[^:]+:" also survives (50/0). Low risk, since the entry still pins the whole line, but the test does not exercise the entry. A file-mode case (git secrets --scan <path>, the path pre-commit --all-files uses) would.

Everything else verified so far: fail-before (old .gitallowed: 8 FAIL lines; new: 50/0); anchoring holds in file mode (other file, prefixed path, .goX, changed ID, fixture+trailing key, key+fixture all exit 1); whole tree with CI's --register-aws set: only hit with empty allowlist is tools/purchase_test.go:138, clean with the new file; pre-commit run git-secrets --all-files Passed. Labels mirror #24.

[cristim]

Gate verdict at ecd085c52fbbf18c8e31659422629e11006ea6c5: CHANGES REQUESTED (not merged)

Mutation run finished. Additional survivors: restore bare 999999999999 and 111122223333 (50 PASS, 0 FAIL); delete the new anchored entry entirely: 50 PASS, 0 FAIL, confirming the "fixture scans clean" case does not exercise the entry. Blocker is finding 1 above (self-test does not fail when an old bare entry is restored). The .gitallowed change itself is correct and the anchoring is tight; only the regression guard needs the structural check. CI run 38250233761 was still in progress at review time; mergeStateStatus UNSTABLE.

Pre-existing, not from this PR (same on origin/main with the full setup-git-secrets.sh pattern set, not CI's --register-aws set): markdown table rows in .github/workflows/README.md:8, SECURITY.md:31 and scripts/test-git-secrets-allowlist.sh:123 flag locally.

[cristim]

Note: on origin/main, the full setup-script pattern set also flags README.md:8, SECURITY.md:31 and test-script line 123 under git secrets --scan -r. Pre-existing and not touched here; tracked in #58.

[cristim]

Gate verdict at f625d67bba3487a2a8752e31d4ed872999578c21: APPROVED, merging

Delta since ecd085c5: only scripts/test-git-secrets-allowlist.sh (+53: structural anchoring guard plus file-mode cases). Finding 1 and the vacuous-case note are resolved.

Local evidence (macOS, git-secrets 1.3.0, throwaway repos, fake AKIA/AIza fixtures only, git archive of the head):

• Self-test baseline: 73 PASS, 0 FAIL.
• Mutation matrix, every mutant killed (22/22): each of the 18 old bare entries restored one at a time fails (1 FAIL each; 3 for 111111111111/UUID, 9 for 123456789012); deleting the anchored entry: 2 FAIL; widening its path to ^[^:]+:: 2 FAIL; dropping its $: 3 FAIL.
• From the earlier pass, still valid (.gitallowed unchanged): old .gitallowed gives 8 FAIL on the must-catch cases; the anchored entry exempts only the exact line in tools/purchase_test.go; a whole-tree scan with CI's --register-aws set is clean.
• pre-commit run git-secrets --all-files: Passed. go test ./... (GOTOOLCHAIN=go1.26.9): all 3 packages ok.
• CI green on this SHA (Unit Tests re-run SUCCESS), mergeStateStatus CLEAN.

Non-blocking: the structural guard (^\^.*:\[0-9\]\+:.*\$$) would still accept a deliberately wildcarded entry such as ^[^:]+:[0-9]+:.*<literal>.*$. That is acceptable as a heuristic; the header comment covers the intent.