fix(rules): add np.stripe.3 for publishable API keys (pk_live_/pk_test_)#215
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Claude ReviewCritical issuesNone. SecurityNo security concerns flagged. Rule correctly classifies Test coverageYAML rule change is covered by the existing rule loader/lint tests ( |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a new Stripe detection rule ✨ 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 |
Gemini ReviewThe PR correctly adds an informational rule for Stripe publishable keys to reduce false positives from generic API key rules. The regex and rule configuration are well-formed and effectively address the issue. Critical IssuesNone. SecurityNo security concerns flagged. Suggestions
Reviewed by Gemini (gemini-3.1-pro-preview) |
Codex ReviewCritical Issues
SecurityNo security concerns flagged. Suggestions
Reviewed by Codex (gpt-5.4) |
Stripe (and Moonpay) publishable keys are intentionally designed to be embedded in client-side code — they identify a merchant account for payment-form display only and cannot initiate charges or access user data. Flagging them as secrets produces false positives (issue #214). Add np.stripe.3 (base_score: 5, informational) with an explicit pattern that requires the pk_live_ or pk_test_ prefix. The rule includes the same keyword context (api_key, apikey, etc.) as np.generic.2 so that cross-rule deduplication clusters both matches and selects this more specific rule (longer pattern, named capture group) as the winner, correctly labelling the finding "Stripe Publishable API Key" rather than "Generic API Key". Also resolves the long-standing FIXME comment in stripe.yml that acknowledged this gap. Fixes #214
4a8f24e to
acf6be3
Compare
Root cause found and fixed — default ruleset was the missing pieceThe Codex feedback that Why the initial approach didn't work: How the cross-rule dedup mechanism works (and why it's the right approach): Verified empirically: This commit adds:
1133 tests pass. |
I needed to add np.stripe.3 to the default ruleset to make sure it wouldn't ALSO be triggered with the np.generic.2 rule. Now it's appropriately identified. |
Fixes #214
Problem
Stripe (and Moonpay) publishable keys (
pk_live_*,pk_test_*) were flagged bynp.generic.2(Generic API Key) when found in contexts likeapiKey=pk_live_.... These keys are intentionally public — designed to be embedded in browser/mobile code. They cannot initiate charges or access user data. Treating them as secrets generates noise.stripe.ymlalready had aFIXMEacknowledging this gap.Approach
Add explicit
np.stripe.3(base_score: 5 — info tier) rather than suppressing the pattern in the generic rule. Explicit beats implicit.The rule:
pk_live_/pk_test_prefix explicitlyapi_key,apikey, etc.) asnp.generic.2so both rules fire in the same scenariopatternLentiebreaker selects this rule, labelling the finding "Stripe Publishable API Key" rather than "Generic API Key"keycaptures the same bytes asnp.generic.2's positional capture → they're clustered for dedupResult
The finding description tells operators this is expected and normally not worth remediating unless found alongside a
sk_live_*key.Test plan
go test ./pkg/rule/...— 131 tests passmake test— all packages greentitus-score-lint— all rules valid