JSONbored · JSONbored · Jul 4, 2026 · Jul 4, 2026
@@ -7,7 +7,7 @@
 
 import type { AdvisoryFinding } from "../types";
 import { neutralizePromptInjection, safeReviewTitle } from "./prompt-injection";
-import { scanForSecrets } from "./secrets-scan";
+import { scanDiffForSecretsWithLocations } from "./secrets-scan";
 
 // Concrete credential formats only — NOT the weak heuristics (`seed_or_mnemonic` / `bittensor_key`) that
 // false-positive on legitimate config/workflow content. A `coldkey:` / `hotkey =` line or the word
@@ -85,6 +85,10 @@ export function defangReviewInput(input: SafetyReviewInput): {
   return { title, body, diff, changedFiles };
 }
 
+// #3041: cap the number of locations listed in a finding's `detail` so a single PR with dozens of hits still
+// produces a readable comment; anything past the cap is summarized as an omitted count instead of listed.
+const MAX_REPORTED_SECRET_LOCATIONS = 5;
+
 /**
  * Scan the PR diff for leaked secrets and, on a hit, return ONE critical `secret_leak` advisory finding (else
  * null). Mapped to gittensory's {@link AdvisoryFinding} shape. The gate treats this code as a hard blocker
@@ -94,34 +98,37 @@ export function defangReviewInput(input: SafetyReviewInput): {
  * 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).
+ *
+ * #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
+ * added-only-text behavior) while also tracking each hit's file:line, so the finding can point a maintainer
+ * straight at the flagged content instead of forcing them to re-derive it from the whole diff.
  */
 export function secretLeakFinding(diff: string): AdvisoryFinding | null {
-  // Scan ONLY additions — the secrets THIS change introduces. A token on a removed/context line is not being
-  // committed by the PR, so flagging it would wrongly block a change that merely REMOVES or refactors a
-  // secret-shaped string (e.g. deleting/defanging a test fixture, or rotating a credential out). Added/renamed
-  // file paths are also committed PR state, but buildSecretScanDiff carries them only in `### path (status)`
-  // headers, so keep those metadata lines while still dropping modified/removed headers and `+++` patch headers.
-  const added = diff
-    .split("\n")
-    .filter(
-      (line) =>
-        (line.startsWith("+") && !line.startsWith("+++")) ||
-        /^### .+ \((?:added|renamed)\) /.test(line),
-    )
-    .join("\n");
   // 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 `secret_leak`
   // blocker. A real token (github_token, aws_access_key, …) still blocks regardless of which file it is in.
-  const kinds = scanForSecrets(added).kinds.filter((kind) =>
-    HARD_SECRET_KINDS.has(kind),
+  const hits = scanDiffForSecretsWithLocations(diff).filter((match) =>
+    HARD_SECRET_KINDS.has(match.kind),
+  );
+  if (hits.length === 0) return null;
+  const kinds = [...new Set(hits.map((hit) => hit.kind))].sort();
+  const locations = hits.map((hit) =>
+    hit.line === 0 ? `${hit.path} (filename)` : `${hit.path}:${hit.line}`,
   );
-  if (kinds.length === 0) return null;
+  const uniqueLocations = [...new Set(locations)];
+  const shown = uniqueLocations.slice(0, MAX_REPORTED_SECRET_LOCATIONS);
+  const omitted = uniqueLocations.length - shown.length;
+  const locationSummary =
+    omitted > 0
+      ? `Found at: ${shown.join(", ")} (+${omitted} more location${omitted === 1 ? "" : "s"})`
+      : `Found at: ${shown.join(", ")}`;
   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(", ")}. A committed credential must be rotated and removed from the change before merge.`,
+    detail: `The PR diff matches secret pattern(s): ${kinds.join(", ")}. ${locationSummary}. 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.",
   };

@@ -63,12 +63,22 @@ function hasLongSequentialRun(value: string): boolean {
   return false;
 }
 
+// #3041: a value made ENTIRELY of lowercase words joined by hyphens (2+ segments, e.g. the test-fixture
+// literal "installation-token" used 351+ times across this repo's own test suite as a mock fetch-response
+// token) reads as an ordinary English-word compound identifier -- a mock/fixture name -- not a generated
+// credential. Real secrets/tokens are essentially always alphanumeric, mixed-case, or base64/hex; they are
+// never a pure lowercase-hyphenated phrase. Require at least one hyphen (2+ segments) so this stays narrow
+// and doesn't broaden into excluding arbitrary single lowercase words that could plausibly be real secrets.
+const LOWERCASE_HYPHENATED_COMPOUND_PATTERN = /^[a-z]+(-[a-z]+)+$/;
+
 /** True for an obvious non-secret filler value: a known placeholder phrase, a string built from at most 2
- *  distinct characters (e.g. "xxxxxxxxxxxxxxxx", "----------------"), or a long monotonic character-code run
- *  (e.g. "abcdefghijklmnop123") — real high-entropy secrets never look like any of these. */
+ *  distinct characters (e.g. "xxxxxxxxxxxxxxxx", "----------------"), a long monotonic character-code run
+ *  (e.g. "abcdefghijklmnop123"), or a lowercase-hyphenated word compound (e.g. "installation-token") — real
+ *  high-entropy secrets never look like any of these. */
 function isPlaceholderSecretValue(value: string): boolean {
   if (PLACEHOLDER_VALUE_PATTERN.test(value)) return true;
   if (new Set(value.toLowerCase()).size <= 2) return true;
+  if (LOWERCASE_HYPHENATED_COMPOUND_PATTERN.test(value)) return true;
   return hasLongSequentialRun(value);
 }
 
@@ -85,14 +95,82 @@ function hasGenericSecretAssignment(text: string): boolean {
   return false;
 }
 
+// #3041: the one place the pattern list (format-specific SECRET_PATTERNS + the generic keyword-assignment
+// heuristic) is applied to a string. Both `scanForSecrets` (whole-text scan) and
+// `scanDiffForSecretsWithLocations` (per-line diff scan, for file:line attribution) delegate here so there is
+// exactly one implementation of "does this text contain secret-shaped content" to keep in sync.
+function matchedKindsIn(text: string): string[] {
+  if (!text) return [];
+  const kinds = SECRET_PATTERNS.filter((pattern) => pattern.re.test(text)).map((pattern) => pattern.name);
+  if (hasGenericSecretAssignment(text)) kinds.push("generic_secret_assignment");
+  return kinds;
+}
+
 export interface SecretScanResult {
   found: boolean;
   kinds: string[];
 }
 
 export function scanForSecrets(text: string): SecretScanResult {
-  if (!text) return { found: false, kinds: [] };
-  const kinds = SECRET_PATTERNS.filter((pattern) => pattern.re.test(text)).map((pattern) => pattern.name);
-  if (hasGenericSecretAssignment(text)) kinds.push("generic_secret_assignment");
+  const kinds = matchedKindsIn(text);
   return { found: kinds.length > 0, kinds };
 }
+
+/** One secret-pattern hit at a specific location in a diff, for surfacing file:line in a finding (#3041). A
+ *  `line` of `0` means the match came from a file-header PATH itself (an added/renamed filename), not from
+ *  diff content — there is no line number for that case. */
+export interface SecretScanLocationMatch {
+  kind: string;
+  path: string;
+  line: number;
+}
+
+const DIFF_FILE_HEADER_PATTERN = /^### (.+) \(([a-z]+)\) \+\d+\/-\d+$/;
+const DIFF_HUNK_HEADER_PATTERN = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/;
+
+/**
+ * Walk a `buildSecretScanDiff`-shaped diff (see src/queue/processors.ts) line by line, scanning only content
+ * this PR is INTRODUCING — added (`+`) lines and, for an added/renamed file, the path in its own section
+ * header — and return every pattern hit with its file path and 1-based line number in the new/post-change
+ * file. Context (` `) and removed (`-`) lines are tracked for line-number bookkeeping but never scanned: a
+ * removed or unchanged line is not something this PR is committing. This mirrors the added-only scanning
+ * `secretLeakFinding` used to do via string filtering, but keeps enough diff structure to report WHERE a hit
+ * lives instead of collapsing everything to a flat blob.
+ */
+export function scanDiffForSecretsWithLocations(diff: string): SecretScanLocationMatch[] {
+  const matches: SecretScanLocationMatch[] = [];
+  let currentPath = "";
+  let currentNewLine = 0;
+  for (const line of diff.split("\n")) {
+    const fileHeader = DIFF_FILE_HEADER_PATTERN.exec(line);
+    if (fileHeader) {
+      currentPath = fileHeader[1]!;
+      currentNewLine = 0;
+      const status = fileHeader[2]!;
+      if (status === "added" || status === "renamed") {
+        for (const kind of matchedKindsIn(currentPath)) {
+          matches.push({ kind, path: currentPath, line: 0 });
+        }
+      }
+      continue;
+    }
+    const hunkHeader = DIFF_HUNK_HEADER_PATTERN.exec(line);
+    if (hunkHeader) {
+      currentNewLine = Number(hunkHeader[1]) - 1;
+      continue;
+    }
+    if (line.startsWith("+") && !line.startsWith("+++")) {
+      currentNewLine += 1;
+      const content = line.slice(1);
+      for (const kind of matchedKindsIn(content)) {
+        matches.push({ kind, path: currentPath, line: currentNewLine });
+      }
+      continue;
+    }
+    if (line.startsWith("-")) continue;
+    // Context line (single leading space) or a blank separator between file sections -- either way it isn't
+    // new content this PR introduces, but a genuine context line still occupies a line in the new file.
+    currentNewLine += 1;
+  }
+  return matches;
+}
@@ -436,3 +436,78 @@ describe("secretLeakFinding scans only ADDED lines", () => {
     expect(secretLeakFinding(diff)).toBeNull();
   });
 });
+
+// #3041: the finding's `detail` must surface the exact file:line so a maintainer can jump straight to the
+// flagged content instead of re-deriving it from the whole diff.
+describe("secretLeakFinding surfaces file:line locations (#3041)", () => {
+  // Assembled at runtime so THIS test file's source carries no contiguous scannable token.
+  const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+
+  it("reports the exact file path and line number for a planted secret in a multi-file diff", () => {
+    const diff = [
+      "### src/unrelated.ts (modified) +2/-0",
+      "@@ -1,2 +1,4 @@",
+      " const a = 1;",
+      "+const b = 2;",
+      "+const c = 3;",
+      "",
+      "### test/fixture.ts (modified) +3/-0",
+      "@@ -10,3 +10,6 @@",
+      " const before = true;",
+      "+const filler = 0;",
+      `+const token = "${fakeToken}";`,
+      "+const after = 1;",
+    ].join("\n");
+    const finding = secretLeakFinding(diff);
+    expect(finding?.code).toBe("secret_leak");
+    // Hunk starts at new-file line 10; " const before" is line 10 (context), "+const filler" is line 11,
+    // and the secret line is line 12.
+    expect(finding?.detail).toContain("test/fixture.ts:12");
+    expect(finding?.detail).not.toContain("src/unrelated.ts");
+  });
+
+  it("caps the reported locations at 5 and notes how many more were omitted (singular)", () => {
+    const lines = ["### src/many.ts (modified) +6/-0", "@@ -1,0 +1,6 @@"];
+    for (let i = 0; i < 6; i += 1) {
+      lines.push(`+const secret${i} = "${fakeToken}";`);
+    }
+    const diff = lines.join("\n");
+    const finding = secretLeakFinding(diff);
+    expect(finding?.code).toBe("secret_leak");
+    // 6 distinct locations (lines 1-6) -> 5 shown + "1 more location" (singular) omitted note.
+    for (let line = 1; line <= 5; line += 1) {
+      expect(finding?.detail).toContain(`src/many.ts:${line}`);
+    }
+    expect(finding?.detail).not.toContain("src/many.ts:6");
+    expect(finding?.detail).toContain("+1 more location)");
+    expect(finding?.detail).not.toContain("+1 more locations)");
+  });
+
+  it("caps the reported locations at 5 and notes how many more were omitted (plural)", () => {
+    const lines = ["### src/many.ts (modified) +8/-0", "@@ -1,0 +1,8 @@"];
+    for (let i = 0; i < 8; i += 1) {
+      lines.push(`+const secret${i} = "${fakeToken}";`);
+    }
+    const diff = lines.join("\n");
+    const finding = secretLeakFinding(diff);
+    expect(finding?.code).toBe("secret_leak");
+    // 8 distinct locations (lines 1-8) -> 5 shown + "3 more locations" (plural) omitted note.
+    for (let line = 1; line <= 5; line += 1) {
+      expect(finding?.detail).toContain(`src/many.ts:${line}`);
+    }
+    expect(finding?.detail).not.toContain("src/many.ts:6");
+    expect(finding?.detail).toContain("+3 more locations)");
+  });
+
+  it("a removed line's secret-shaped content does not appear in the finding at all", () => {
+    const diff = `### src/config.ts (modified) +0/-1\n@@ -5,1 +5,0 @@\n-const token = "${fakeToken}";`;
+    expect(secretLeakFinding(diff)).toBeNull();
+  });
+
+  it("an added file's secret-shaped filename is reported as a filename-level (line 0) location", () => {
+    const diff = `### fixtures/${fakeToken}.txt (added) +1/-0\n@@ -0,0 +1,1 @@\n+benign fixture content`;
+    const finding = secretLeakFinding(diff);
+    expect(finding?.code).toBe("secret_leak");
+    expect(finding?.detail).toContain(`fixtures/${fakeToken}.txt (filename)`);
+  });
+});
@@ -128,4 +128,31 @@ describe("scanForSecrets — deterministic secret-pattern scanner", () => {
   it("does NOT flag a short value under the 16-character floor", () => {
     expect(scanForSecrets('token = "short12345"').kinds).not.toContain("generic_secret_assignment");
   });
+
+  // #3041: PR #3036 was wrongly hard-blocked by the test-fixture literal "installation-token" (used 351+
+  // times across this repo's own test suite as a mock fetch-response token) — a lowercase-hyphenated word
+  // compound reads as a fixture/mock name, not a generated credential.
+  it.each([
+    ["installation-token", 'token: "installation-token"'],
+    ["access-token", 'token = "access-token"'],
+    ["some-mock-secret-value", 'secret: "some-mock-secret-value"'],
+  ])("does NOT flag a lowercase-hyphenated word compound: %s (#3041)", (_name, snippet) => {
+    expect(scanForSecrets(snippet).kinds).not.toContain("generic_secret_assignment");
+  });
+
+  it("still flags a real-looking generic secret with digits and mixed case (regression guard for #3041)", () => {
+    // Same fixture as the "high-entropy value" test above — proves the new lowercase-hyphenated exclusion
+    // doesn't broaden past its intended narrow shape: this value has digits + mixed case, not a pure
+    // lowercase-hyphenated phrase, so it must still be flagged.
+    const fakeSecret = "sk_live_" + "aK9xQ2mZw7Ln4Rv8Pt3Bh6";
+    expect(scanForSecrets(`fakeSecret = "${fakeSecret}"`).kinds).toContain("generic_secret_assignment");
+  });
+
+  it("a single lowercase word with no hyphen is unaffected by the new hyphenated-compound exclusion (#3041)", () => {
+    // 20 lowercase letters, no repeats and no sequential run, so it isn't already caught by the entropy/
+    // placeholder checks either -- proves LOWERCASE_HYPHENATED_COMPOUND_PATTERN specifically requires a
+    // hyphen (2+ segments) and does not accidentally match a single unhyphenated word.
+    const singleWord = "qwzxvbnmalskdjfhgpoiu";
+    expect(scanForSecrets(`token = "${singleWord}"`).kinds).toContain("generic_secret_assignment");
+  });
 });