fix(review): stop generic_secret_assignment from auto-closing PRs#5370
Conversation
PR #5346 (a resubmission of #5341) was auto-closed over two inert test-fixture strings that matched the generic_secret_assignment keyword-plus-quoted-value SHAPE but weren't real credentials -- the same heuristic has now caused at least eight prior false-positive incidents (#2613, #3178, #3673, #3866, #4587, #4733, plus several fixture-rewording commits), each patched by narrowing an allowlist rather than fixing the underlying design. REES's own copy of this rule already rates it "medium confidence" ("catches real keys but also the occasional long opaque non-secret"), and content-lane/security-scan.ts's own header states the design principle this violated: a gate that auto-closes with no human queue may only hard-close on a signal unambiguous enough that a false positive is essentially impossible. Split generic_secret_assignment out of HARD_SECRET_KINDS into a new ADVISORY_ONLY_SECRET_KINDS: it still surfaces (a warning-severity possible_secret_assignment finding / a "manual" content-lane verdict), but never auto-blocks or auto-closes on its own. Concrete credential formats (github_token, aws_access_key, private_key_block, ...) are unaffected and remain unconditional hard blockers. Also add a structural placeholder heuristic (looksLikeDescriptive PlaceholderPhrase, mirrored in REES): a value with 5+ lowercase-only hyphen/underscore segments containing an English function word reads as written prose describing the value, not a credential or a chosen passphrase -- this independently resolves both PR #5346 literals without weakening detection of a genuine human-chosen passphrase like "correct-horse-battery-secret" (no function words, by design).
|
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 #5370 +/- ##
==========================================
- Coverage 94.41% 94.37% -0.04%
==========================================
Files 550 550
Lines 44143 44160 +17
Branches 14632 14637 +5
==========================================
Hits 41677 41677
- Misses 1791 1808 +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:
|
loopover-orb
Bot
added
the
gittensor:bug
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 15:24:16 UTC
⏸️ Suggested Action - Manual Review
Review summary 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.
|
1 participant
Summary
GITHUB_TOKEN: "present-value-not-a-real-token"andconst secretToken = "test-value-should-never-appear-in-doctor-output"), not real credentials.generic_secret_assignmentis a keyword-plus-quoted-value SHAPE heuristic, not a concrete credential format — its placeholder escape hatch is a small fixed keyword list (fake/dummy/sample/etc.) that can never keep pace with the open-ended ways a contributor phrases an inert test value. Git history shows at least 8 prior false-positive incidents on this exact heuristic (fix(review): widen the gate's hard-blocking secret scan to match REES's richer ruleset #2613, fix(review): reduce secret-scan false positives and surface finding locations #3178, fix(review): tighten hyphenated secret placeholder scan #3673, fix(review): tighten mock placeholder carve-out to avoid secret-scan false negatives #3866, fix(review): stop generic_secret_assignment false-flagging self-naming fixture/enum values #4587, fix(review): tighten generic secret fixture allowlist #4733, plus several fixture-rewording-only commits), each patched by narrowing an allowlist rather than fixing the design.review-enrichment/src/analyzers/secret-scan.ts) already rates it "medium confidence" ("catches real keys but also the occasional long opaque non-secret... treats it as verify rather than block"), andcontent-lane/security-scan.ts's own header states the design principle this violated: a gate that auto-closes with no human queue may only hard-close on a signal unambiguous enough that a false positive is essentially impossible — "every other heuristic routes to MANUAL".generic_secret_assignmentwas folded into the unconditionalHARD_SECRET_KINDSbucket in fix(review): widen the gate's hard-blocking secret scan to match REES's richer ruleset #2553, contradicting both of those.Changes
src/review/secret-patterns.ts: splitgeneric_secret_assignmentout ofHARD_SECRET_KINDSinto a newADVISORY_ONLY_SECRET_KINDS. AddedlooksLikeDescriptivePlaceholderPhrase(+ENGLISH_FUNCTION_WORDS): a value with 5+ lowercase-only hyphen/underscore segments containing a low-entropy English function word ("not", "a", "should", "never", ...) reads as written prose describing the value, not a credential or a deliberately chosen passphrase — this independently resolves both PR feat(miner): add offline GitHub-token and coding-agent-credential presence checks to doctor (#5170) #5346 literals without weakening detection of a genuine human-chosen passphrase likecorrect-horse-battery-secret(diceware-style passphrases are chosen from high-entropy content words specifically because function words carry little entropy, so they essentially never contain one).review-enrichment/src/analyzers/secret-scan.ts(REES): mirrored the same heuristic, per this file's existing twin-pair drift-check design with the host copy.src/review/safety.ts:secretLeakFindingnow only produces the unconditional-blockingsecret_leak(critical) finding for concrete kinds; a generic-only hit produces a new non-blockingpossible_secret_assignment(warning) finding instead.src/review/content-lane/security-scan.ts: same demotion — a generic-only hit now routes tomanual(neverclose), matching this file's own documented auto-close design principle.scanLinkedBodiesForSecretsalso gained generic-hit detection (previously only reachable viaHARD_SECRET_KINDS, now needs an explicit check since that set narrowed).Test plan
npm run typecheck— cleannpm run test:coverage(full unsharded suite) — 758 passed | 2 skipped;secret-patterns.ts/safety.ts/content-lane/security-scan.tsall 100% lines + 100% branchesnpm run test:engine-parity/npm run test:driver-parity/npm run engine-parity:drift-check— all green (mirrored REES copy keeps the twin-pair check passing)npm run rees:test— 1320/1320 passing (REES's own build+drift+test gate)npm run docs:drift-check/npm run manifest:drift-check— cleannpm run test:ci(full local gate) — green aside from one confirmed-unrelated flaky test (miner-repo-clone.test.ts, a real git-clone integration test timing out under concurrent-session CPU load on this shared machine; passes in 22s in isolation, no relation to any file this PR touches)