feat(enrichment): secrets-in-logs & PII-egress scanner

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

@@ -0,0 +1,144 @@
+// Secrets-in-logs / PII-egress analyzer (#1507). Flags added lines that pass sensitive data INTO a logging or
+// stdout sink — `console.log(req.headers.authorization)`, `logger.info(`token=${apiKey}`)`, `console.log(req)` —
+// distinct from the hardcoded-secret scan (which inspects literal VALUES; this inspects the data FLOW into a
+// sink). Pure compute, no network. Precision-first: string-literal *messages* are stripped before matching, so
+// `console.log("password reset")` is NOT flagged — a hit requires a sensitive name used as CODE (property access,
+// a `${…}` interpolation, or a dumped request object). Line-cited via hunk headers, mirroring the other analyzers.
+import type { EnrichRequest, SecretLogFinding } from "../types.js";
+
+const MAX_FINDINGS = 25; // keep the brief bounded
+const MAX_LINE_CHARS = 2000; // skip pathologically long lines (defensive)
+
+// All matchers below are FLAT alternations (no group is itself quantified), so each is linear-time — the analyzer
+// can never be the DoS class it sits beside (#1503). Logging / stdout sinks:
+const SINK_RE =
+  /\b(?:console|logger|log|winston|pino|bunyan)\s*\.\s*(?:log|info|warn|error|debug|trace|fatal|verbose|silly)\s*\(|\bprocess\s*\.\s*std(?:out|err)\s*\.\s*write\s*\(/;
+
+// Sensitive names, matched only against CODE (after string messages are stripped) so a hit means the value is
+// actually referenced, not merely named in a log message.
+const SECRET_RE =
+  /\b(?:passwords?|passwd|secret|api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|authorization|auth[_-]?token|bearer[_-]?token|private[_-]?key|credentials?|session[_-]?token|set[_-]?cookie)\b/i;
+const PII_RE =
+  /\b(?:ssn|social[_-]?security(?:[_-]?number)?|credit[_-]?card(?:[_-]?number)?|card[_-]?number|cvv|cvc|passport[_-]?(?:no|number)?|tax[_-]?id|national[_-]?id|date[_-]?of[_-]?birth)\b/i;
+// A whole request/session object — or one of its sensitive sub-objects (headers/body/cookies/session/auth) —
+// dumped into the sink. Innocuous scalar fields (`req.method`, `req.url`, `req.path`) are deliberately excluded to
+// keep the signal high, and the object must be referenced as code so `console.log("request received")` is not a hit.
+const REQUEST_OBJECT_RE =
+  /\b(?:req|request)\s*(?:\)|\.\s*(?:headers|body|cookies|session|auth|rawheaders)\b)|\b(?:headers|session|cookies)\s*(?:\)|\.\s*[\w$])/i;
+
+/** Blank out string-literal MESSAGE content (keeping `${…}` interpolation bodies, which are real code) in a single
+ *  linear pass — no regex, so it can never backtrack. Lets the matchers above run against code, not log prose. */
+export function codeOnly(s: string): string {
+  let out = "";
+  let i = 0;
+  const n = s.length;
+  while (i < n) {
+    const c = s[i]!;
+    if (c === '"' || c === "'") {
+      i++;
+      while (i < n && s[i] !== c) {
+        if (s[i] === "\\") i++;
+        i++;
+      }
+      i++; // closing quote
+      out += " ";
+      continue;
+    }
+    if (c === "`") {
+      i++;
+      while (i < n && s[i] !== "`") {
+        if (s[i] === "\\") {
+          i += 2;
+          continue;
+        }
+        if (s[i] === "$" && s[i + 1] === "{") {
+          i += 2;
+          let depth = 1;
+          while (i < n && depth > 0) {
+            if (s[i] === "{") depth++;
+            else if (s[i] === "}") depth--;
+            if (depth > 0) out += s[i];
+            i++;
+          }
+          continue;
+        }
+        i++; // ordinary template-literal char — drop it
+      }
+      i++; // closing backtick
+      out += " ";
+      continue;
+    }
+    out += c;
+    i++;
+  }
+  return out;
+}
+
+function sinkLabel(match: string): string {
+  return match.replace(/\s+/g, "").replace(/\($/, "");
+}
+
+/** Classify one line: does it pass sensitive data into a log/stdout sink? Returns the sink + category, or null. */
+export function detectSecretLog(
+  line: string,
+): { sink: string; category: SecretLogFinding["category"] } | null {
+  const m = SINK_RE.exec(line);
+  if (!m) return null;
+  const code = codeOnly(line.slice(m.index + m[0].length));
+  if (SECRET_RE.test(code))
+    return { sink: sinkLabel(m[0]), category: "secret" };
+  if (PII_RE.test(code)) return { sink: sinkLabel(m[0]), category: "pii" };
+  if (REQUEST_OBJECT_RE.test(code))
+    return { sink: sinkLabel(m[0]), category: "request-object" };
+  return null;
+}
+
+/** Scan one file patch's added lines for sensitive-data-into-a-sink, line-cited via hunk headers. Pure. */
+export function scanPatchForSecretLog(
+  path: string,
+  patch: string,
+): SecretLogFinding[] {
+  const findings: SecretLogFinding[] = [];
+  let newLine = 0;
+  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]);
+      continue;
+    }
+    if (line.startsWith("+")) {
+      const body = line.slice(1);
+      if (body.length <= MAX_LINE_CHARS) {
+        const hit = detectSecretLog(body);
+        if (hit) {
+          findings.push({
+            file: path,
+            line: newLine,
+            sink: hit.sink,
+            category: hit.category,
+          });
+        }
+      }
+      newLine++;
+    } else if (!line.startsWith("-")) {
+      newLine++;
+    }
+  }
+  return findings;
+}
+
+/** Analyzer entrypoint: scan every changed file's added lines for secrets/PII reaching a log or stdout sink. */
+export async function scanSecretLog(
+  req: EnrichRequest,
+): Promise<SecretLogFinding[]> {
+  const findings: SecretLogFinding[] = [];
+  for (const file of req.files ?? []) {
+    if (!file.patch) continue;
+    for (const finding of scanPatchForSecretLog(file.path, file.patch)) {
+      findings.push(finding);
+      if (findings.length >= MAX_FINDINGS) return findings;
+    }
+  }
+  return findings;
+}

review-enrichment/src/brief.ts

@@ -15,6 +15,7 @@ import { scanActionPins } from "./analyzers/actions-pin.js";
 import { scanEol } from "./analyzers/eol-check.js";
 import { scanRedos } from "./analyzers/redos.js";
 import { scanCodeowners } from "./analyzers/codeowners.js";
+import { scanSecretLog } from "./analyzers/secret-log.js";
 import { renderBrief } from "./render.js";
 
 type AnalyzerFn = (req: EnrichRequest, signal: AbortSignal) => Promise<unknown>;
@@ -29,6 +30,7 @@ const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
   eol: (req) => scanEol(req),
   redos: (req) => scanRedos(req),
   codeowners: (req, signal) => scanCodeowners(req, fetch, { signal }),
+  secretLog: (req) => scanSecretLog(req),
 };
 
 function runWithTimeout<T>(

review-enrichment/src/render.ts

@@ -142,6 +142,24 @@ export function renderBrief(
     }
   }
 
+  const secretLogs = findings.secretLog ?? [];
+  if (secretLogs.length) {
+    lines.push(
+      "### Secrets / PII reaching a log or stdout sink (redact before merging)",
+    );
+    for (const item of secretLogs) {
+      const what =
+        item.category === "secret"
+          ? "a secret/credential"
+          : item.category === "pii"
+            ? "PII"
+            : "a full request/session object";
+      lines.push(
+        `- ${safeCodeSpan(`${item.file}:${item.line}`)} — ${safeCodeSpan(item.sink)} writes ${what} to a log/stdout sink; redact or remove`,
+      );
+    }
+  }
+
   if (!lines.length) return { promptSection: "", systemSuffix: "" };
 
   const header =

review-enrichment/src/types.ts

@@ -100,6 +100,15 @@ export interface CodeownersFinding {
   owners: string[]; // sorted owners from the last-matching CODEOWNERS rule; always non-empty
 }
 
+/** An added line that passes sensitive data into a logging/stdout sink (a secret, PII, or a dumped request
+ *  object). Reports the location + sink + category only — never the logged value. */
+export interface SecretLogFinding {
+  file: string;
+  line: number;
+  sink: string;
+  category: "secret" | "pii" | "request-object";
+}
+
 /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */
 export interface BriefFindings {
   dependency?: DependencyFinding[];
@@ -110,6 +119,7 @@ export interface BriefFindings {
   eol?: EolFinding[];
   redos?: RedosFinding[];
   codeowners?: CodeownersFinding[];
+  secretLog?: SecretLogFinding[];
 }
 
 export type AnalyzerStatus = "ok" | "degraded" | "skipped";

review-enrichment/test/enrichment.test.ts

@@ -21,6 +21,12 @@ import {
   scanPatchForRedos,
   scanRedos,
 } from "../dist/analyzers/redos.js";
+import {
+  codeOnly,
+  detectSecretLog,
+  scanPatchForSecretLog,
+  scanSecretLog,
+} from "../dist/analyzers/secret-log.js";
 
 const NOW = new Date("2026-06-26").getTime();
 const eolFetch =
@@ -937,3 +943,116 @@ test("buildBrief: eol analyzer runs (real now, 2023 cycle is past)", async () =>
     globalThis.fetch = realFetch;
   }
 });
+
+test("codeOnly: blanks string messages, keeps ${...} interpolation bodies", () => {
+  assert.equal(codeOnly('"a secret here"'), " ");
+  assert.equal(codeOnly("'plain'"), " ");
+  assert.ok(codeOnly("`x=${apiKey}`").includes("apiKey"));
+  assert.ok(!codeOnly("`logging the password now`").includes("password"));
+  assert.equal(
+    codeOnly("req.headers.authorization"),
+    "req.headers.authorization",
+  );
+});
+
+test("detectSecretLog: flags sensitive data into a sink as CODE, not string messages", () => {
+  assert.equal(
+    detectSecretLog("console.log(req.headers.authorization);")?.category,
+    "secret",
+  );
+  assert.equal(
+    detectSecretLog("logger.info(`token=${apiKey}`);")?.category,
+    "secret",
+  );
+  assert.equal(detectSecretLog("log.error(user.password);")?.category, "secret");
+  assert.equal(detectSecretLog("console.debug(account.ssn);")?.category, "pii");
+  assert.equal(detectSecretLog("console.log(req);")?.category, "request-object");
+  assert.equal(
+    detectSecretLog("process.stdout.write(session.cookie);")?.sink,
+    "process.stdout.write",
+  );
+  // NOT flagged — sensitive word only in a string message, no sink, or a benign interpolation:
+  assert.equal(
+    detectSecretLog('console.log("password reset email sent");'),
+    null,
+  );
+  assert.equal(detectSecretLog('logger.info("request received");'), null);
+  assert.equal(detectSecretLog("const token = readToken();"), null);
+  assert.equal(detectSecretLog("logger.info(`user ${id} signed in`);"), null);
+  assert.equal(detectSecretLog("console.error(error);"), null);
+  // innocuous request scalars are NOT dumps:
+  assert.equal(detectSecretLog("console.log(req.method, req.url);"), null);
+  assert.equal(detectSecretLog("console.log(req.path);"), null);
+  // but a whole request or a sensitive sub-object IS:
+  assert.equal(
+    detectSecretLog("console.log(req.body);")?.category,
+    "request-object",
+  );
+});
+
+test("scanPatchForSecretLog: line-cited via hunk header; ignores context + safe lines", () => {
+  const patch = [
+    "@@ -1,1 +1,4 @@",
+    " const ok = true;",
+    "+console.log(req.headers.authorization);",
+    '+console.log("user signed in");',
+    "+logger.info(`ssn=${user.ssn}`);",
+  ].join("\n");
+  const findings = scanPatchForSecretLog("src/a.ts", patch);
+  assert.deepEqual(
+    findings.map(({ file, line, category }) => ({ file, line, category })),
+    [
+      { file: "src/a.ts", line: 2, category: "secret" },
+      { file: "src/a.ts", line: 4, category: "pii" },
+    ],
+  );
+  assert.equal(findings[0].sink, "console.log");
+});
+
+test("scanSecretLog: scans every changed file's added lines, caps to its budget", async () => {
+  const findings = await scanSecretLog({
+    repoFullName: "o/r",
+    prNumber: 1,
+    files: [
+      { path: "a.ts", patch: "@@ -1,0 +1,1 @@\n+console.log(user.password);" },
+      { path: "b.ts", patch: "@@ -1,0 +1,1 @@\n+console.log('hello world');" },
+      { path: "c.md", patch: undefined },
+    ],
+  });
+  assert.equal(findings.length, 1);
+  assert.equal(findings[0].file, "a.ts");
+});
+
+test("renderBrief: renders the secret-log block, code-spanning + sanitizing", () => {
+  const r = renderBrief({
+    secretLog: [
+      { file: "src/a.ts", line: 9, sink: "console.log", category: "secret" },
+    ],
+  });
+  assert.match(r.promptSection, /Secrets \/ PII reaching a log/);
+  assert.match(r.promptSection, /`src\/a\.ts:9`/);
+  assert.match(r.promptSection, /`console\.log`/);
+  assert.match(r.promptSection, /a secret\/credential/);
+});
+
+test("buildBrief: secret-log analyzer runs (pure, no network)", async () => {
+  const realFetch = globalThis.fetch;
+  globalThis.fetch = async () => ({ ok: true, json: async () => ({}) });
+  try {
+    const brief = await buildBrief({
+      repoFullName: "o/r",
+      prNumber: 1,
+      files: [
+        {
+          path: "src/a.ts",
+          patch: "@@ -1,0 +1,1 @@\n+console.log(req.headers.authorization);",
+        },
+      ],
+    });
+    assert.equal(brief.analyzerStatus.secretLog, "ok");
+    assert.equal(brief.findings.secretLog.length, 1);
+    assert.match(brief.promptSection, /Secrets \/ PII reaching a log/);
+  } finally {
+    globalThis.fetch = realFetch;
+  }
+});