fix(review): hard-block generic secret assignments

src/review/content-lane/security-scan.ts

@@ -68,11 +68,9 @@ function firstLineMatching(text: string, re: RegExp): { n: number; text: string
 }
 
 function firstSecretLine(text: string): { n: number; kinds: string[] } | null {
-  // Per-line scan — O(n): catches every LINE-CONTAINED concrete kind (github_token, jwt, …) and cites its
+  // Per-line scan — O(n): catches every LINE-CONTAINED hard-blocking kind (github_token, jwt, generic_secret_assignment, …) and cites its
   // exact line. lines[i] is defined for an in-range index (split never yields holes) — assert past
-  // noUncheckedIndexedAccess. HARD_SECRET_KINDS no longer includes generic_secret_assignment (see
-  // ../secret-patterns.ts's doc comment) — that kind is handled separately by
-  // firstGenericSecretAssignmentLine below and routes to MANUAL, not this function's auto-close caller.
+  // noUncheckedIndexedAccess. Multiline generic assignments are handled by firstGenericSecretAssignmentLine below.
   const lines = text.split(/\r?\n/);
   for (let i = 0; i < lines.length; i += 1) {
     const hits = scanForSecrets(lines[i]!).kinds.filter((k) => HARD_SECRET_KINDS.has(k));
@@ -82,11 +80,9 @@ function firstSecretLine(text: string): { n: number; kinds: string[] } | null {
 }
 
 /**
- * generic_secret_assignment is a keyword-plus-quoted-value SHAPE heuristic, not a concrete credential format
- * (see ../secret-patterns.ts's HARD_SECRET_KINDS doc comment — split out post-gittensory-PR-#5346, which
- * auto-closed a legitimate contributor PR over two inert test-fixture strings). Per this file's own header
- * ("only ONE signal is unambiguous enough to hard-close... every other heuristic routes to MANUAL"), a hit
- * here routes to MANUAL, never scanSubmissionContent's auto-close. Its keyword-to-value span can wrap across
+ * generic_secret_assignment is a keyword-plus-quoted-value SHAPE heuristic whose captured value has already
+ * cleared placeholder filtering, so an in-submission generic-only hit is treated as an embedded secret. Its
+ * keyword-to-value span can wrap across
  * lines (`client_secret =\n"…"`), so this is a single whole-blob pass — LINEAR, not a quadratic prefix-rescan
  * — citing the line where the non-placeholder match COMPLETES.
  */
@@ -103,9 +99,8 @@ function firstGenericSecretAssignmentLine(text: string): number | null {
 
 /**
  * Deterministic security scan of the SUBMITTED content. Returns:
- *  - `close` (embedded_secret) on a concrete embedded credential — cited to a line; or
- *  - `manual` (possible_secret_assignment) on a secret-shaped-but-not-concrete-format assignment — cited to
- *    a line (see firstGenericSecretAssignmentLine's doc comment for why this is MANUAL, not close); or
+ *  - `close` (embedded_secret) on a concrete embedded credential or generic assignment that clears
+ *    placeholder filtering — cited to a line; or
  *  - `manual` (unsafe_install_pipeline) on a pipe-to-shell install in an executable category; or
  *  - null otherwise.
  * Prompt-injection / exfiltration prose is intentionally NOT matched here: it is indistinguishable
@@ -127,9 +122,9 @@ export function scanSubmissionContent(params: { content: string; category: strin
   const genericLine = firstGenericSecretAssignmentLine(content);
   if (genericLine !== null) {
     return {
-      verdict: "manual",
-      reasonCode: "possible_secret_assignment",
-      summary: `Submission contains a secret-shaped assignment (generic_secret_assignment) at line ${genericLine} that doesn't match a concrete credential format — routing to maintainer review to verify it isn't a real secret.`,
+      verdict: "close",
+      reasonCode: "embedded_secret",
+      summary: `Submission appears to expose a credential (generic_secret_assignment) at line ${genericLine}.`,
     };
   }
 
@@ -159,13 +154,6 @@ export function scanLinkedBodiesForSecrets(bodies: string[]): SecurityFinding |
         summary: `The linked source appears to expose a credential (${hits.join(", ")}) — routing to maintainer review.`,
       };
     }
-    if (hasGenericSecretAssignment(body)) {
-      return {
-        verdict: "manual",
-        reasonCode: "possible_secret_assignment",
-        summary: "The linked source contains a secret-shaped assignment (generic_secret_assignment) that doesn't match a concrete credential format — routing to maintainer review.",
-      };
-    }
   }
   return null;
 }

src/review/safety.ts

@@ -7,7 +7,7 @@
 
 import type { AdvisoryFinding } from "../types";
 import { neutralizePromptInjection, safeReviewTitle } from "./prompt-injection";
-import { ADVISORY_ONLY_SECRET_KINDS, HARD_SECRET_KINDS } from "./secret-patterns";
+import { HARD_SECRET_KINDS } from "./secret-patterns";
 import { scanDiffForSecretsWithLocations, type SecretScanLocationMatch } from "./secrets-scan";
 
 // Concrete credential formats only — NOT the weak heuristics (`seed_or_mnemonic` / `bittensor_key`) that
@@ -94,21 +94,12 @@ function locationSummaryFor(hits: SecretScanLocationMatch[]): string {
  * Scan the PR diff for leaked secrets and, on a hit, return ONE `AdvisoryFinding` (else null). Mapped to
  * gittensory's {@link AdvisoryFinding} shape.
  *
- * Only CONCRETE credential formats ({@link HARD_SECRET_KINDS}) produce the critical `secret_leak` code that
+ * Hard-blocking secret kinds ({@link HARD_SECRET_KINDS}) produce the critical `secret_leak` code that
  * `rules/advisory.ts`'s `isConfiguredGateBlocker` treats as an unconditional hard blocker — the weak
  * `seed_or_mnemonic` / `bittensor_key` heuristics are ignored entirely here because they false-positive on
  * legitimate config/workflow content (e.g. `coldkey:` / `hotkey =` lines in *.toml, .github/workflows/**, or
- * wrangler/workers config). This is UNCONDITIONAL (#audit-3.4): a concrete, real-format committed credential
- * is a leak on any repo, so the caller runs it regardless of the safety flag / review allowlist (unlike the
- * prompt-injection defang, which stays flag-gated).
- *
- * `ADVISORY_ONLY_SECRET_KINDS` (currently just `generic_secret_assignment`) is a keyword-plus-quoted-value
- * SHAPE heuristic, not a concrete format — see `secret-patterns.ts`'s `HARD_SECRET_KINDS` doc comment for why
- * it was split out (PR #5346 auto-closed a legitimate contributor PR on two inert test-fixture strings). A
- * hit on ONLY this kind (no concrete kind present) instead returns a warning-severity `possible_secret_
- * assignment` finding, which `isConfiguredGateBlocker` does not recognize as a blocker code — it surfaces in
- * the PR panel for a human/AI reviewer to verify, exactly as REES's own "medium confidence" rating for this
- * same signal already treats it, without risking another auto-close false positive.
+ * wrangler/workers config). This is UNCONDITIONAL (#audit-3.4): a committed credential is a leak on any repo,
+ * including unknown-format assigned credentials that clear the placeholder filter.
  *
  * #3041: scans the RAW diff directly — `scanDiffForSecretsWithLocations` does its own +/- line-type
  * distinguishing (only added lines and added/renamed file paths are scanned, matching the previous
@@ -117,31 +108,20 @@ function locationSummaryFor(hits: SecretScanLocationMatch[]): string {
  */
 export function secretLeakFinding(diff: string): AdvisoryFinding | null {
   const allHits = scanDiffForSecretsWithLocations(diff);
-  // Only CONCRETE credential formats hard-block. The raw scanner also returns the weak `seed_or_mnemonic` /
-  // `bittensor_key` heuristics, which false-positive on `coldkey:` / `hotkey =` / "mnemonic" lines in
-  // legitimate config/workflow files (RC6); those are filtered out here so they never produce a finding at
-  // all. A real token (github_token, aws_access_key, …) still blocks regardless of which file it is in.
-  const concreteHits = allHits.filter((match) => HARD_SECRET_KINDS.has(match.kind));
-  if (concreteHits.length > 0) {
-    const kinds = [...new Set(concreteHits.map((hit) => hit.kind))].sort();
+  // The raw scanner also returns the weak `seed_or_mnemonic` / `bittensor_key` heuristics, which false-positive
+  // on `coldkey:` / `hotkey =` / "mnemonic" lines in legitimate config/workflow files (RC6); those are filtered
+  // out here so they never produce a finding at all.
+  const hardHits = allHits.filter((match) => HARD_SECRET_KINDS.has(match.kind));
+  if (hardHits.length > 0) {
+    const kinds = [...new Set(hardHits.map((hit) => hit.kind))].sort();
     return {
       code: "secret_leak",
       severity: "critical",
       title: `Possible leaked secret in the diff (${kinds.join(", ")})`,
-      detail: `The PR diff matches secret pattern(s): ${kinds.join(", ")}. ${locationSummaryFor(concreteHits)}. A committed credential must be rotated and removed from the change before merge.`,
+      detail: `The PR diff matches secret pattern(s): ${kinds.join(", ")}. ${locationSummaryFor(hardHits)}. A committed credential must be rotated and removed from the change before merge.`,
       action:
         "Remove the secret from the diff, rotate the exposed credential, then re-run the gate.",
     };
   }
-  const advisoryHits = allHits.filter((match) => ADVISORY_ONLY_SECRET_KINDS.has(match.kind));
-  if (advisoryHits.length > 0) {
-    return {
-      code: "possible_secret_assignment",
-      severity: "warning",
-      title: "Possible secret-shaped assignment in the diff (generic_secret_assignment)",
-      detail: `The PR diff contains a keyword-plus-quoted-value assignment that resembles a credential but doesn't match a concrete credential format. ${locationSummaryFor(advisoryHits)}. This is a medium-confidence heuristic (it also matches inert test/fixture values) and does not block the gate on its own.`,
-      action: "Verify the value is not a real credential.",
-    };
-  }
   return null;
 }

src/review/secret-patterns.ts

@@ -180,26 +180,15 @@ export function secretPatternMatches(pattern: SecretPattern, text: string): bool
   return false;
 }
 
-// Concrete credential formats only -- NOT the weak heuristics (seed_or_mnemonic / bittensor_key) that would
+// Hard-blocking secret kinds -- NOT the weak heuristics (seed_or_mnemonic / bittensor_key) that would
 // false-positive on legitimate Bittensor content (a `coldkey:` / `hotkey =` line or the word "mnemonic" in a
 // .toml, .github/workflows/**, or wrangler/workers config is not a leaked credential; RC6: #1505/#1495/#1485).
 // #2553: google_api_key/jwt are as format-precise as the original five (near-zero false-positive risk), so
 // both are safe unconditional hard blockers. voyage_api_key/firecrawl_api_key (#4604) are equally
-// format-precise. Shared by both hard-block paths: src/review/safety.ts's secretLeakFinding (PR-diff) and
+// format-precise. generic_secret_assignment is also hard-blocking after placeholder filtering: unknown-format
+// assigned credentials (passwords/client secrets/passphrases) are common leaks and otherwise bypass the gate.
+// Shared by both hard-block paths: src/review/safety.ts's secretLeakFinding (PR-diff) and
 // src/review/content-lane/security-scan.ts's firstSecretLine/scanLinkedBodiesForSecrets (content-lane).
-//
-// generic_secret_assignment is deliberately NOT a member (post-PR-5346): unlike every kind above, it is a
-// keyword-plus-quoted-value SHAPE heuristic, not a concrete credential format, so isPlaceholderSecretValue's
-// closed escape-hatch keyword list can never keep pace with the open-ended ways a contributor phrases an
-// inert test value -- this exact gap closed a legitimate contributor PR twice in a row (#5341, then its
-// resubmission #5346, on two DIFFERENT non-placeholder-keyword fixture strings) after at least half a dozen
-// prior narrow-allowlist patches to this same heuristic (#4587, #3866, #3673, #3178, #2613, #4733) failed to
-// stop the pattern for good. REES's own copy of this rule (review-enrichment/src/analyzers/secret-scan.ts)
-// 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 underlying 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". See
-// ADVISORY_ONLY_SECRET_KINDS below for where it still surfaces.
 export const HARD_SECRET_KINDS = new Set([
   "github_token",
   "github_pat",
@@ -215,10 +204,5 @@ export const HARD_SECRET_KINDS = new Set([
   "voyage_api_key",
   "firecrawl_api_key",
   "jwt",
+  "generic_secret_assignment",
 ]);
-
-// The one kind excluded from HARD_SECRET_KINDS above: still detected and still worth a human's attention, but
-// never an unconditional auto-block/auto-close on its own -- see that constant's doc comment for why. Consumed
-// by src/review/safety.ts's secretLeakFinding and src/review/content-lane/security-scan.ts to route a hit here
-// to an advisory/manual-review signal instead of a hard blocker.
-export const ADVISORY_ONLY_SECRET_KINDS = new Set(["generic_secret_assignment"]);

test/unit/content-lane-security-scan.test.ts

@@ -233,16 +233,13 @@ describe("scanSubmissionContent", () => {
     }
   });
 
-  // #5346: generic_secret_assignment is a keyword-plus-quoted-value SHAPE heuristic, not a concrete format —
-  // it routes to MANUAL, never scanSubmissionContent's auto-close (this file's own header states the design
-  // principle: only a concrete credential is unambiguous enough to hard-close).
-  it("routes a generic_secret_assignment hit to MANUAL, never close (#5346)", () => {
+  it("hard-closes on a generic_secret_assignment hit after placeholder filtering", () => {
     const finding = scanSubmissionContent({
       content: `intro line\nclient_secret = "${GENERIC_VALUE}"`,
       category: "skills",
     });
-    expect(finding?.verdict).toBe("manual");
-    expect(finding?.reasonCode).toBe("possible_secret_assignment");
+    expect(finding?.verdict).toBe("close");
+    expect(finding?.reasonCode).toBe("embedded_secret");
     expect(finding?.summary).toContain("line 2");
   });
 
@@ -255,15 +252,14 @@ describe("scanSubmissionContent", () => {
     }
   });
 
-  it("routes a MULTILINE generic secret assignment whose value wraps to the next line to MANUAL (#5346)", () => {
+  it("hard-closes on a MULTILINE generic secret assignment whose value wraps to the next line", () => {
     // generic_secret_assignment's keyword-to-value span can wrap. scanForSecrets over the whole blob catches
-    // it; scanSubmissionContent must too, or a wrapped hit bypasses this signal entirely — but it still routes
-    // to MANUAL (never close), matching the single-line case above. Built from separate literals so this file
-    // embeds no contiguous secret.
+    // it; scanSubmissionContent must too, or a wrapped hit bypasses this signal entirely. Built from separate
+    // literals so this file embeds no contiguous secret.
     const content = `intro line\nclient_secret =\n"${GENERIC_VALUE}"`;
     const finding = scanSubmissionContent({ content, category: "guides" });
-    expect(finding?.verdict).toBe("manual");
-    expect(finding?.reasonCode).toBe("possible_secret_assignment");
+    expect(finding?.verdict).toBe("close");
+    expect(finding?.reasonCode).toBe("embedded_secret");
     expect(finding?.summary).toContain("line 3"); // cited where the wrapped match completes (the value line)
   });
 
@@ -282,10 +278,10 @@ describe("scanLinkedBodiesForSecrets", () => {
     expect(finding?.reasonCode).toBe("embedded_secret");
   });
 
-  it("flags a generic_secret_assignment-only hit in a LINKED body as MANUAL too (#5346)", () => {
+  it("flags a generic_secret_assignment-only hit in a LINKED body as an embedded secret for review", () => {
     const finding = scanLinkedBodiesForSecrets(["clean body", `client_secret = "${GENERIC_VALUE}"`]);
     expect(finding?.verdict).toBe("manual");
-    expect(finding?.reasonCode).toBe("possible_secret_assignment");
+    expect(finding?.reasonCode).toBe("embedded_secret");
   });
 
   it("returns null when no linked body leaks", () => {