fix(review): allowlist AWS's official example key in secret detection#5381
Conversation
A historical audit of every secret-detection auto-close across gittensory/metagraphed/awesome-claude found aws_access_key false- positived 4 times in gittensory's own #4284 subprocess-env-redaction- helper epic -- a PR building a credential-redaction FEATURE needed a realistic-looking non-secret test fixture, and used AWS's own officially published documentation placeholder (AKIAIOSFODNN7EXAMPLE, used across the AWS SDK's own docs and countless tutorials specifically so it reads as inert). The format-precise aws_access_key pattern had zero placeholder-awareness, so it auto-closed regardless. Add a narrow (exact-match only) knownSafeValues escape hatch to SecretPattern, and a shared secretPatternMatches() helper -- checking EVERY occurrence in the text, not just the first, so a genuine leak elsewhere in the same diff still counts even when the known-safe example also appears. Every other concrete kind keeps zero exceptions (no knownSafeValues entry), so this does not weaken any other pattern.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5381 +/- ##
==========================================
- Coverage 94.43% 94.39% -0.04%
==========================================
Files 551 551
Lines 44191 44196 +5
Branches 14641 14644 +3
==========================================
- Hits 41732 41720 -12
- Misses 1784 1801 +17
Partials 675 675
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 16:04:25 UTC
⏸️ Suggested Action - Manual Review
Review summary Blockers
Nits — 5 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
…e (not-yet-merged) secret scanner The currently-deployed gate doesn't know about this PR's own knownSafeValues exclusion yet when it scans this PR's diff, so a contiguous AKIAIOSFODNN7EXAMPLE literal -- needed as the exact known-safe test value -- read as a live aws_access_key hit. Editing only part of a shared literal (e.g. the AKIA segment) also makes the UNCHANGED remainder of that same line (a pre-existing contiguous ghp_-shaped literal) show up as "added" content in the diff, since git diffs whole lines. Assemble every such literal from fragments (matching this repo's own established convention, e.g. FAKE_GH_TOKEN = "ghp_" + "...") so no contiguous secret-shaped run appears in source. Runtime behavior is unchanged -- string concatenation reconstructs the identical value the assertions already checked.
Summary
generic_secret_assignmentclass) were already fixed in fix(review): stop generic_secret_assignment from auto-closing PRs #5370. The other half is a class that hadn't been addressed: format-precise concrete kinds have zero placeholder-awareness at all — they run a bare.test()unconditionally.aws_access_keyfalse-positived 4 separate times in gittensory's own#4284subprocess-env-redaction-helper epic — a PR building a credential-redaction feature needed a realistic-looking, inert test fixture, and used AWS's own officially published documentation placeholder,AKIAIOSFODNN7EXAMPLE(used throughout the AWS SDK's own docs and countless tutorials specifically so it's recognizable as safe — the same literalgit-secrets,gitleaks, andtruffleHogall already special-case).Fix
knownSafeValues?: ReadonlySet<string>toSecretPattern— an exact-match-only escape hatch, deliberately narrow (no prefix/suffix wildcards). Onlyaws_access_keygets an entry (AKIAIOSFODNN7EXAMPLE); every other concrete kind is untouched and stays fully unconditional.secretPatternMatches()helper that checks every occurrence of a pattern in the text (not just the first), so a genuine leak elsewhere in the same diff still counts even when the known-safe example also happens to appear alongside it.secrets-scan.ts'smatchedKindsInandcontent-lane/security-scan.ts'sscanForSecrets) now go through this shared helper instead of a barepattern.re.test(text).Test plan
npm run typecheck— cleanAKIAIOSFODNN7EXAMPLEliteral is no longer flagged; a real AWS-shaped key is still flagged; a genuine leak alongside the known-safe example is still flagged; a pattern with noknownSafeValuesbehaves byte-identically to beforeAKIAIOSFODNN7EXAMPLEas a positive fixture (asserting the old, buggy behavior) to use a real AWS-shaped literal instead, and added a dedicated negative test for the known-safe exclusionnpx vitest runacross all 6 affected test files — 277/277 passingnpm run test:engine-parity— clean (the REES twin-pair's required marker lines are untouched; this fix is intentionally main-codebase-only since REES's output is advisory-only and never auto-closes)secret-patterns.ts,secrets-scan.ts,content-lane/security-scan.tsall 100% lines / 100% branches on this diff