fix(privacy): detect SSH endpoints, and stop scanning on import

scripts/privacy-scan.ts

@@ -175,6 +175,30 @@ function isAllowedBearerToken(file: string, token: string): boolean {
   return /^(?:access|stack|usage-debug)-token(?:-value)?-[A-Za-z0-9-]+$/.test(token);
 }
 
+/**
+ * Placeholder endpoints that are documentation, not infrastructure.
+ *
+ * Deliberately narrow: RFC 2606 reserved names, an obviously templated value, and
+ * SSH's own `%h`/`%p` tokens. Anything else naming a host or an account is treated
+ * as real, because the cost of a false positive here is one allowlist line and the
+ * cost of a false negative is a published endpoint.
+ */
+function isAllowedSshEndpoint(value: string): boolean {
+  const v = value.trim();
+  if (!v) return true;
+  // A bare substitution token is a template. A real command that merely CONTAINS
+  // `%h` is not — `ProxyCommand /opt/homebrew/bin/cloudflared access ssh --hostname %h`
+  // names the binary, the access method and the tunnel, which is the leak itself.
+  if (/^%[hpr]$/.test(v)) return true;
+  // `<host>`, `$HOST`, `{{ runner }}` — templated rather than literal.
+  if (/^[<{$]/.test(v)) return true;
+  // RFC 2606 / RFC 6761 reserved documentation names.
+  if (/(?:^|[.@\s])(?:example\.(?:com|net|org)|example|invalid|localhost|test)(?:$|[\s:/])/i.test(v)) return true;
+  // Generic account placeholders, matching the home-path allowlist's spirit.
+  if (/^(?:user|username|me|you|someone|root|ubuntu|runner)$/i.test(v)) return true;
+  return false;
+}
+
 function addFindingsForPattern(
   findings: Finding[],
   file: string,
@@ -197,9 +221,14 @@ function addFindingsForPattern(
 /**
  * Scan already-read text.
  *
- * Split out of `scanFile` so a test can exercise the REAL detectors. This module runs its
- * scan on import, so a test that cannot call a function ends up re-declaring the patterns
- * instead — and then stays green even if a detector here is deleted.
+ * Split out of `scanFile` so a test can exercise the REAL detectors rather than
+ * re-declaring the patterns — a copied regex stays green after the production
+ * detector is deleted, which is the failure this seam exists to prevent.
+ *
+ * Importing this module is side-effect free: the repo scan runs only under
+ * `import.meta.main` (see `runScan` below). It used to run at module scope, so
+ * importing `scanText` triggered a full scan and a failing one called
+ * `process.exit(1)` in the importing process.
  */
 export function scanText(file: string, text: string): Finding[] {
   const findings: Finding[] = [];
@@ -237,6 +266,45 @@ export function scanText(file: string, text: string): Finding[] {
     /\b(?:sk-[A-Za-z0-9_-]{20,}|ghp_[A-Za-z0-9_]{20,}|eyJ[A-Za-z0-9_-]{20,}\.[A-Za-z0-9_-]{20,})\b/g,
     match => isAllowedTokenLooking(file, match[0]),
   );
+  /*
+   * SSH config directives naming a real endpoint.
+   *
+   * `privacy-scan` knew about tokens, emails and home paths, but nothing about
+   * infrastructure — so a devlog could publish a working `Host` block and this
+   * scan passed. That is how a runner's hostname, login and Cloudflare
+   * `ProxyCommand` shipped in `260731_pr_merge_round/022`; #4623 removes them by
+   * hand. The values are deliberately not repeated here — this file is the fix,
+   * and restating them would outlive the cleanup.
+   *
+   * Anchored to the SSH config grammar — directive at the start of a line, with
+   * optional indent — because `User` is an ordinary English word and matching it
+   * in prose would make this unusable. `HostName`/`ProxyCommand` are distinctive
+   * enough on their own but are anchored the same way for consistency.
+   */
+  addFindingsForPattern(
+    findings,
+    file,
+    text,
+    "ssh-endpoint",
+    // `HostName` only, and the value must be the whole rest of the line.
+    //
+    // `User` is deliberately NOT matched. It is an ordinary English word, and
+    // anchoring it to the SSH grammar still fires on wrapped prose — "…the\nuser
+    // configuration." and "…the\nuser notice." both matched a line-anchored
+    // single-token form during development. The username alone is also the least
+    // sensitive part of a Host block, and `MAINTAINER_HOME_USERNAME` already
+    // covers the maintainer's account in path form.
+    /^[ \t]*HostName[ \t]+(\S+)[ \t]*$/gim,
+    match => isAllowedSshEndpoint(match[1] ?? ""),
+  );
+  addFindingsForPattern(
+    findings,
+    file,
+    text,
+    "ssh-proxy-command",
+    /^[ \t]*ProxyCommand[ \t]+(\S.*)$/gim,
+    match => isAllowedSshEndpoint(match[1] ?? ""),
+  );
   /*
    * Meta Model API keys. The pattern above does not match them: the measured shape is
    * `LLM|<16 digits>|<27 chars>`, verified against a real key's grammar (never its value).
@@ -263,26 +331,57 @@ function scanFile(file: string): Finding[] {
  * A home path or an email is context a reviewer needs in the failure message. A bearer
  * token or an API key is the very thing the scan exists to keep out of a readable
  * artifact, so the report names where it is instead of what it is.
+ *
+ * `ssh-proxy-command` is redacted for the same reason: the value carries the binary
+ * path, the access method and the tunnel options, and CI logs are far more widely
+ * readable than the diff it was caught in. `ssh-endpoint` (a bare `HostName`) is
+ * not — that one is the context a reviewer needs to find it.
+ */
+const REDACTED_FINDING_KINDS = new Set([
+  "bearer-token",
+  "token-looking",
+  "meta-api-key",
+  "ssh-proxy-command",
+  // Redacted for the same reason as the ProxyCommand: this scan runs in CI on a
+  // public repository, so printing the value would republish the endpoint into a
+  // public log — the scanner leaking what it was written to catch. `file:line`
+  // already locates it for whoever has to remove it.
+  "ssh-endpoint",
+]);
+
+/**
+ * Run the scan only when invoked as a script.
+ *
+ * Previously this ran at module scope, so `import { scanText }` executed a full
+ * repo scan as a side effect — and a failing scan called `process.exit(1)`,
+ * taking the importing test process with it. That coupling is invisible while
+ * the tree is clean and bites the moment a detector finds something: adding the
+ * `ssh-endpoint` rule below broke `privacy-scan-meta-key.test.ts`, which does
+ * nothing but import the same seam this file exports for testing.
  */
-const REDACTED_FINDING_KINDS = new Set(["bearer-token", "token-looking", "meta-api-key"]);
+if (import.meta.main) {
+  runScan();
+}
 
-const findings = gitLsFiles()
-  .filter(existsSync)
-  .filter(shouldScan)
-  .flatMap(scanFile);
+function runScan(): void {
+  const findings = gitLsFiles()
+    .filter(existsSync)
+    .filter(shouldScan)
+    .flatMap(scanFile);
 
-if (findings.length > 0) {
-  console.error("Privacy scan failed:");
-  for (const finding of findings) {
-    // A credential finding must not be echoed: this output goes to stderr and into CI
-    // logs, so printing the match would copy a leaked secret from one place it should
-    // not be into another — and CI logs are far more widely readable than a diff.
-    // The location and kind are enough to find it; the value is one `git show` away
-    // for whoever is fixing it.
-    const shown = REDACTED_FINDING_KINDS.has(finding.kind) ? "<redacted>" : finding.value;
-    console.error(`${finding.file}:${finding.line} ${finding.kind}: ${shown}`);
+  if (findings.length > 0) {
+    console.error("Privacy scan failed:");
+    for (const finding of findings) {
+      // A credential finding must not be echoed: this output goes to stderr and into CI
+      // logs, so printing the match would copy a leaked secret from one place it should
+      // not be into another — and CI logs are far more widely readable than a diff.
+      // The location and kind are enough to find it; the value is one `git show` away
+      // for whoever is fixing it.
+      const shown = REDACTED_FINDING_KINDS.has(finding.kind) ? "<redacted>" : finding.value;
+      console.error(`${finding.file}:${finding.line} ${finding.kind}: ${shown}`);
+    }
+    process.exit(1);
   }
-  process.exit(1);
-}
 
-console.log("Privacy scan passed");
+  console.log("Privacy scan passed");
+}

tests/ci-workflows/privacy-scan-ssh-endpoint.test.ts

@@ -0,0 +1,60 @@
+import { describe, expect, test } from "bun:test";
+import { scanText } from "../../scripts/privacy-scan";
+
+/**
+ * #4623 removed a working SSH `Host` block from a published devlog by hand.
+ * `privacy:scan` passed on that file, because it knew about tokens, emails and
+ * home paths but nothing about infrastructure endpoints. These pin the detector
+ * that closes it — and, just as importantly, the shapes it must NOT fire on,
+ * since two rounds of false positives on ordinary prose and code are what
+ * narrowed it to `HostName`/`ProxyCommand`.
+ */
+describe("privacy-scan — ssh-endpoint", () => {
+  const kinds = (text: string) => scanText("devlog/x.md", text).map(f => f.kind);
+
+  test("catches a Host block of the shape that shipped", () => {
+    // Shaped like the block #4623 is removing, with a synthetic endpoint. Using
+    // the real one would reintroduce it here permanently and undo that cleanup;
+    // the regex cannot tell the difference, so there is nothing to be gained.
+    const block = [
+      "Host runner-cf",
+      "    HostName ssh-runner.internal-buildfarm.net",
+      "    ProxyCommand /opt/homebrew/bin/cloudflared access ssh --hostname %h",
+    ].join("\n");
+    const k = kinds(block);
+    expect(k).toContain("ssh-endpoint");        // redacted in the report; file:line locates it
+    expect(k).toContain("ssh-proxy-command");   // redacted too, see REDACTED_FINDING_KINDS
+  });
+
+  test("a templated or reserved host is documentation, not infrastructure", () => {
+    for (const line of [
+      "    HostName example.com",
+      "    HostName <your-runner>",
+      "    HostName $RUNNER_HOST",
+      "    HostName localhost",
+      "    ProxyCommand %h",
+    ]) {
+      expect(kinds(line)).not.toContain("ssh-endpoint");
+      expect(kinds(line)).not.toContain("ssh-proxy-command");
+    }
+  });
+
+  test("does not fire on prose or code that merely starts with a directive word", () => {
+    for (const line of [
+      "User aliases are display metadata only. Codex pool aliases live on `CodexAccount`",
+      "user configuration.",
+      "user notice.",
+      "          hostname === undefined ? { grokHome } : { grokHome, hostname },",
+      "The hostname is resolved by the adapter.",
+    ]) {
+      expect(kinds(line)).not.toContain("ssh-endpoint");
+    }
+  });
+
+  test("a ProxyCommand that merely contains %h is still the real command", () => {
+    // The substitution token does not make the binary path, the access method or
+    // the tunnel any less of a leak.
+    expect(kinds("    ProxyCommand /opt/homebrew/bin/cloudflared access ssh --hostname %h"))
+      .toContain("ssh-proxy-command");
+  });
+});