fix(enrichment): join adjacent added lines before secret-scan matching

review-enrichment/src/analyzers/secret-scan.ts

@@ -47,33 +47,65 @@ const RULES: Rule[] = [
   },
 ];
 
+/** Extract the inner text of every quoted string literal (single/double/backtick) on a line. Used to catch a
+ *  secret whose literal value is split across two adjacent added lines and joined at runtime (e.g.
+ *  `const a = "AKIA..."; const b = a + "REST";`) — pure per-line regex matching never sees the runtime-joined
+ *  value, only the two separate source literals either side of the `+`. */
+function extractStringLiteralContents(line: string): string[] {
+  const literals: string[] = [];
+  const re = /"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|`(?:[^`\\]|\\.)*`/g;
+  let match: RegExpExecArray | null;
+  while ((match = re.exec(line)) !== null) literals.push(match[0].slice(1, -1));
+  return literals;
+}
+
 /** Scan one file's unified-diff patch, tracking new-file line numbers via hunk headers. Pure. Value never captured. */
 export function scanPatch(path: string, patch: string): SecretFinding[] {
   const findings: SecretFinding[] = [];
   let newLine = 0;
+  // Last added line's extracted string-literal contents, for the cross-line join check below. Reset whenever a
+  // non-added line breaks the run — a secret is only plausibly split across CONSECUTIVE added lines.
+  let previousLiterals: string[] = [];
   for (const line of patch.split("\n")) {
     if (line.startsWith("+++") || line.startsWith("---")) continue;
     const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
     if (hunk) {
       newLine = Number(hunk[1]);
+      previousLiterals = [];
       continue;
     }
     if (line.startsWith("+")) {
       const content = line.slice(1);
+      let matched = false;
       for (const rule of RULES) {
         if (rule.re.test(content)) {
-          findings.push({
-            file: path,
-            line: newLine,
-            kind: rule.kind,
-            confidence: rule.confidence,
-          });
+          findings.push({ file: path, line: newLine, kind: rule.kind, confidence: rule.confidence });
+          matched = true;
           break; // one finding per line — first (most specific) rule wins
         }
       }
+      const currentLiterals = extractStringLiteralContents(content);
+      // Bounded: only the immediately-preceding line's LAST literal joined with this line's FIRST literal — the
+      // common "two sequential variable assignments" shape. Skipped once this line already matched on its own.
+      const lastPrevious = previousLiterals.at(-1);
+      const firstCurrent = currentLiterals[0];
+      if (!matched && lastPrevious !== undefined && firstCurrent !== undefined) {
+        const joined = lastPrevious + firstCurrent;
+        for (const rule of RULES) {
+          if (rule.re.test(joined)) {
+            // "medium" regardless of the rule's own confidence — a joined pair is a heuristic, not a direct match.
+            findings.push({ file: path, line: newLine, kind: rule.kind, confidence: "medium" });
+            break;
+          }
+        }
+      }
+      previousLiterals = currentLiterals;
       newLine++;
     } else if (!line.startsWith("-")) {
       newLine++; // context line advances the new-file counter; removed lines do not
+      previousLiterals = [];
+    } else {
+      previousLiterals = [];
     }
   }
   return findings;

review-enrichment/test/secret-scan.test.ts

@@ -0,0 +1,105 @@
+// Units for the secret-scan analyzer. Kept separate so analyzer PRs do not collide in one shared test file.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { scanPatch } from "../dist/analyzers/secret-scan.js";
+
+const hunk = (lines) => `@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`;
+
+// Built from fragments at test-run time (never a contiguous secret-shaped literal in the committed source) so
+// GitHub push protection does not flag this fixture file the way it would flag a real leaked credential — the
+// fragments below are only ever joined into one string inside the synthetic diff patch handed to scanPatch, or
+// (for the cross-line tests) deliberately kept as two SEPARATE short literals that mirror what a real split
+// secret looks like in source — neither fragment alone is a contiguous match for any RULES pattern.
+const awsKeyFragmentA = "AKIA" + "IOSFODNN7"; // 13 chars — too short alone to match \bAKIA[0-9A-Z]{16}\b
+const awsKeyFragmentB = "EXAMPLE"; // matches no RULES pattern alone
+const fakeAwsKey = awsKeyFragmentA + awsKeyFragmentB;
+const fakeStripeKey = ["sk_live_", "abcdefghijklmnop1234567890"].join("");
+
+test("scanPatch flags a single-line AWS access key with high confidence", () => {
+  const findings = scanPatch("src/config.ts", hunk([`const key = "${fakeAwsKey}";`]));
+  assert.equal(findings.length, 1);
+  assert.equal(findings[0].kind, "aws_access_key_id");
+  assert.equal(findings[0].confidence, "high");
+  assert.equal(findings[0].file, "src/config.ts");
+  assert.equal(findings[0].line, 1);
+});
+
+test("scanPatch flags a private key header", () => {
+  const findings = scanPatch("id_rsa", hunk(["-----BEGIN RSA PRIVATE KEY-----"]));
+  assert.equal(findings.length, 1);
+  assert.equal(findings[0].kind, "private_key");
+});
+
+test("scanPatch flags a generic secret assignment", () => {
+  const findings = scanPatch("src/config.ts", hunk([`const apiKey = "${fakeStripeKey}";`]));
+  assert.equal(findings.length, 1);
+  assert.equal(findings[0].kind, "generic_secret_assignment");
+  assert.equal(findings[0].confidence, "medium");
+});
+
+test("scanPatch reports nothing for clean code", () => {
+  const findings = scanPatch("src/app.ts", hunk(['const greeting = "hello world";', "export function run() {}"]));
+  assert.equal(findings.length, 0);
+});
+
+// Regression test for #2454: a real credential split across two added lines via string concatenation evaded
+// every RULES regex since neither line's own text contains the full contiguous pattern.
+test("scanPatch catches an AWS key split across two adjacent added lines via concatenation (#2454)", () => {
+  const findings = scanPatch(
+    "src/config.ts",
+    hunk([`const part1 = "${awsKeyFragmentA}";`, `const part2 = "${awsKeyFragmentB}";`, "const awsKey = part1 + part2;"]),
+  );
+  assert.equal(findings.length, 1);
+  assert.equal(findings[0].kind, "aws_access_key_id");
+  assert.equal(findings[0].confidence, "medium"); // downgraded — a joined pair is a heuristic, not a direct match
+  assert.equal(findings[0].line, 2); // attributed to the completing (second) line
+});
+
+test("scanPatch does not join literals across a context line that breaks the added-line run (#2454)", () => {
+  const patch = [
+    "@@ -1,1 +1,3 @@",
+    ' const unrelated = "context line";', // unchanged context line breaks the run
+    `+const part1 = "${awsKeyFragmentA}";`,
+    `+const part2 = "${awsKeyFragmentB}";`,
+  ].join("\n");
+  // part1 and part2 ARE still adjacent added lines here, so this should still match — this test instead
+  // verifies a context line BETWEEN the two secret halves prevents the join.
+  const findingsAdjacent = scanPatch("src/config.ts", patch);
+  assert.equal(findingsAdjacent.length, 1);
+
+  const patchWithGap = [
+    "@@ -1,1 +1,3 @@",
+    `+const part1 = "${awsKeyFragmentA}";`,
+    ' const unrelated = "context line";',
+    `+const part2 = "${awsKeyFragmentB}";`,
+  ].join("\n");
+  const findingsGapped = scanPatch("src/config.ts", patchWithGap);
+  assert.equal(findingsGapped.length, 0);
+});
+
+test("scanPatch does not join literals across a hunk boundary (#2454)", () => {
+  const patch = [
+    "@@ -1,0 +1,1 @@",
+    `+const part1 = "${awsKeyFragmentA}";`,
+    "@@ -10,0 +11,1 @@",
+    `+const part2 = "${awsKeyFragmentB}";`,
+  ].join("\n");
+  const findings = scanPatch("src/config.ts", patch);
+  assert.equal(findings.length, 0);
+});
+
+test("scanPatch does not double-report a line that already matched on its own", () => {
+  const findings = scanPatch(
+    "src/config.ts",
+    hunk([`const key = "${fakeAwsKey}";`, 'const other = "unrelated-string-value";']),
+  );
+  // The first line already matches directly; its literal must not ALSO be joined into a second, duplicate finding.
+  assert.equal(findings.length, 1);
+  assert.equal(findings[0].line, 1);
+});
+
+test("scanPatch does not join two unrelated short literals into a false positive", () => {
+  const findings = scanPatch("src/app.ts", hunk(['const a = "hello";', 'const b = "world";']));
+  assert.equal(findings.length, 0);
+});