Scan reachable history for secrets, not just the working tree

bin/iak-pending.mjs

@@ -42,14 +42,15 @@
 // only to hosts that helper already trusts. It is never printed, never logged
 // and never placed in argv.
 
-import { readFileSync, writeFileSync, renameSync, unlinkSync, mkdirSync, realpathSync } from 'node:fs';
+import { readFileSync, writeFileSync, renameSync, unlinkSync, mkdirSync } from 'node:fs';
 import { homedir } from 'node:os';
 import { dirname, join } from 'node:path';
 import { fileURLToPath } from 'node:url';
 
 import { loadConfig } from '../src/config.mjs';
 import { gateAuthHeadersFor } from '../src/mcp-server.mjs';
 import { lanIpReason } from '../packages/user-intent-kit/src/model-capacity.js';
+import { isMainModule } from '../src/common/entrypoint.mjs';
 
 const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..');
 const DEFAULT_TIMEOUT_MS = 5000;
@@ -773,9 +774,11 @@ export async function main(argv = process.argv.slice(2), { env = process.env, ou
   return code;
 }
 
+// The comparison itself lives in src/common/entrypoint.mjs. It is correct here
+// too, but one implementation is the point: the idiom has been got wrong three
+// times in this repo, and a copy that is right today is a copy that can drift.
 function invokedDirectly() {
-  try { return !!process.argv[1] && realpathSync(process.argv[1]) === fileURLToPath(import.meta.url); }
-  catch { return false; }
+  return isMainModule(import.meta.url);
 }
 
 if (invokedDirectly()) {

bin/model-picker.mjs

@@ -104,7 +104,6 @@ import { parseArgs } from 'node:util';
 import { homedir } from 'node:os';
 import { join, dirname } from 'node:path';
 import { fileURLToPath } from 'node:url';
-import { realpathSync } from 'node:fs';
 import { readFile, writeFile, mkdir, rename } from 'node:fs/promises';
 
 import {
@@ -118,6 +117,7 @@ import {
 // That was fixed once already in PR #52; hand-rolling around the helper that
 // exists to prevent it would re-open it.
 import { gateAuthHeadersFor } from '../src/mcp-server.mjs';
+import { isMainModule } from '../src/common/entrypoint.mjs';
 
 const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..');
 
@@ -752,15 +752,14 @@ export async function main(argv = process.argv.slice(2), {
   }
 }
 
-// Only when run directly. realpathSync on both sides because import.meta.url
-// is already realpath-resolved and process.argv[1] is not: a `~/bin` symlink
-// (the documented way to put this on PATH) or macOS /tmp (itself a symlink)
-// made the comparison false, so main() never ran and the program exited 0 -
-// a code this file documents as "a selection was applied".
-const invokedDirectly = (() => {
-  try { return !!process.argv[1] && realpathSync(process.argv[1]) === fileURLToPath(import.meta.url); }
-  catch { return false; }
-})();
+// Only when run directly. The comparison lives in src/common/entrypoint.mjs,
+// which realpaths both sides because import.meta.url is already resolved and
+// process.argv[1] is not: a `~/bin` symlink (the documented way to put this on
+// PATH) or macOS /tmp (itself a symlink) made a naive comparison false, so
+// main() never ran and the program exited 0 - a code this file documents as
+// "a selection was applied". One implementation, because this repo has got the
+// idiom wrong three times in three different files.
+const invokedDirectly = isMainModule(import.meta.url);
 if (invokedDirectly) {
   process.exit(await main());
 }

package.json

@@ -6,10 +6,12 @@
   "bin": {
     "ide-agent-kit": "./bin/cli.mjs",
     "ide-agent-kit-mcp": "./bin/iak-mcp.mjs",
-    "iak-pending": "./bin/iak-pending.mjs"
+    "iak-pending": "./bin/iak-pending.mjs",
+    "iak-scan-history": "./scripts/scan-history-for-secrets.mjs"
   },
   "scripts": {
     "test": "node --test test/*.test.mjs packages/user-intent-kit/test/*.test.js",
+    "scan:history": "node scripts/scan-history-for-secrets.mjs",
     "start": "node bin/cli.mjs serve",
     "mcp": "node bin/iak-mcp.mjs",
     "relay": "node scripts/local-relay.mjs",

scripts/check-stageable-secrets.mjs

@@ -38,28 +38,20 @@
 
 import { execFileSync } from 'node:child_process';
 import { readFileSync, statSync } from 'node:fs';
+import { MAX_BYTES, decodeForScanning, looksLikeBinaryMedia, matchSecrets, renderClaims, ruleLabels, sanitizeForOutput } from '../src/secret-patterns.mjs';
 
-// Shapes worth stopping for. Deliberately narrow: a scanner that cries wolf
-// gets disabled, and a disabled scanner is worse than none. Every pattern
-// here is a real credential format we use or plausibly would.
-const PATTERNS = [
-  [/xfb_[a-f0-9]{32,}/i, 'GroupMind agent key'],
-  // sk-ant- BEFORE the general sk- rule: the broad one also matches an
-  // Anthropic key and would mislabel it, and a wrong label sends someone
-  // rotating the wrong credential.
-  [/sk-ant-[A-Za-z0-9_-]{20,}/, 'Anthropic API key'],
-  [/sk-[A-Za-z0-9_-]{20,}/, 'OpenAI-style secret key'],
-  [/AIza[0-9A-Za-z_-]{35}/, 'Google API key'],
-  [/gh[pousr]_[A-Za-z0-9]{36,}/, 'GitHub token'],
-  [/github_pat_[A-Za-z0-9_]{50,}/, 'GitHub fine-grained PAT'],
-  [/-----BEGIN [A-Z ]*PRIVATE KEY-----/, 'private key'],
-  [/\b(?:api[_-]?key|secret|password|token)\s*[:=]\s*['"]?[A-Za-z0-9_\-]{24,}/i,
-    'assigned secret-looking value'],
-];
-
-// Binaries and lockfiles produce noise, not credentials.
-const SKIP_EXT = /\.(png|jpe?g|gif|webp|ico|pdf|zip|gz|tgz|jar|aab|apk|keystore|jks|woff2?|ttf|mp[34]|mov|wav)$/i;
-const MAX_BYTES = 2 * 1024 * 1024;
+// The pattern list, the binary-extension skip list and the size cap now live
+// in src/secret-patterns.mjs, shared with scripts/scan-history-for-secrets.mjs.
+// They were moved there the day the history scanner was written, because the
+// alternative was a second list - and a second list is how one of them rots
+// unnoticed while still looking healthy. That is the same shape of mistake the
+// note above describes. Do not re-introduce a local copy here; a test asserts
+// that neither scanner has one.
+//
+// matchSecrets() returns rule names, lines and lengths, never the matched
+// text. That is why the finding below no longer prints a 6-character prefix of
+// the match: a prefix of a live key in a CI transcript is still a prefix of a
+// live key.
 
 const git = (args) =>
   execFileSync('git', args, { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 });
@@ -82,44 +74,107 @@ function stageableFiles() {
   return files;
 }
 
+// Files that had to be decoded lossily. Reported at the end: the ASCII scan is
+// sound, but "this was not valid UTF-8" is something the operator should see
+// rather than something the tool swallows.
+const lossyFiles = [];
+
+// A path is repo-controlled free text that this tool prints. It can carry ANSI
+// escapes or a newline to forge output lines, and it can itself be a credential
+// (keys/sk-live-xxx.txt), which printing would leak.
+function safeFile(file) {
+  const hits = matchSecrets(file);
+  if (hits.length > 0) return `(path withheld: it matches ${hits[0].label})`;
+  return sanitizeForOutput(file);
+}
+
 function scan(path) {
-  if (SKIP_EXT.test(path)) return null;
-  let text;
+  // No extension check. The same filename-decides bug lives here: a stageable
+  // foo.png holding an ASCII credential would have been skipped unread. Binary
+  // media is recognised below, by its bytes.
+  let bytes;
   try {
-    if (statSync(path).size > MAX_BYTES) return null;
-    text = readFileSync(path, 'utf8');
+    if (statSync(path).size > MAX_BYTES) return [];
+    bytes = readFileSync(path);
   } catch {
-    return null; // unreadable, gone, or a directory: not our problem
+    return []; // unreadable, gone, or a directory: not our problem
   }
-  if (text.includes('\0')) return null; // binary
-  for (const [re, label] of PATTERNS) {
-    const m = text.match(re);
-    if (m) {
-      // Report WHERE and WHAT, never the value itself. This output ends up in
-      // CI logs and terminal scrollback, and a scanner that prints the secret
-      // it found has simply moved the leak.
-      const line = text.slice(0, m.index).split('\n').length;
-      return { label, line, hint: `${m[0].slice(0, 6)}…(${m[0].length} chars)` };
-    }
+  // Read as bytes and decode for scanning. NOT "decode strictly, else skip":
+  // that is what this file did for one commit, and it printed PASS on a file
+  // holding a plain ASCII key beside a single stray 0xff byte - the baseline
+  // scanner caught that file, so the encoding check made the gate WORSE. An
+  // encoding problem must never suppress a match.
+  //
+  // Lossy is right here specifically. This scanner's job is to block a commit,
+  // not to classify encodings; credential formats are ASCII and survive a lossy
+  // decode byte for byte. Making an undecodable file exit non-zero instead was
+  // the other option, and it was wrong for THIS tool: stageable binaries are
+  // routine, blocking every commit that touches one gets the hook disabled, and
+  // a disabled scanner is worse than none. The history scanner, which reports
+  // rather than blocks, does mark such blobs could-not-complete AND scans them.
+  const { text, strict } = decodeForScanning(bytes);
+  if (!strict) {
+    // Recognised image or archive: nothing to read, and nothing to warn about.
+    if (looksLikeBinaryMedia(bytes)) return [];
+    lossyFiles.push(path);
   }
-  return null;
+  // Report WHERE and WHICH RULE, never the value itself. This output ends up
+  // in CI logs and terminal scrollback, and a scanner that prints the secret it
+  // found has simply moved the leak.
+  return matchSecrets(text).map((hit) => ({
+    label: hit.label,
+    line: hit.line,
+    hint: `${hit.length} chars, value not printed`,
+    detail: hit.detail,
+  }));
+}
+
+// Claims for a JWT hit: metadata only, never the token. Shared wording with
+// the history scanner so a finding reads the same wherever it surfaces.
+function describeDetail(detail) {
+  if (!detail || detail.kind !== 'jwt') return '';
+  const claims = renderClaims(detail.claims);
+  let expiry;
+  if (detail.noExpiry) expiry = 'NO EXPIRY CLAIM';
+  else if (detail.expired) expiry = `EXPIRED ${detail.expiresAt}`;
+  else expiry = `live until ${detail.expiresAt} (${detail.daysRemaining} days)`;
+  return `claims: ${claims} | ${expiry}`;
+}
+
+// Shared with the history scanner; a test compares the two outputs so the
+// lists cannot drift apart silently.
+if (process.argv.includes('--print-rules')) {
+  console.log(ruleLabels().join('\n'));
+  process.exit(0);
 }
 
 const findings = [];
 for (const f of stageableFiles()) {
-  const hit = scan(f);
-  if (hit) findings.push({ file: f, ...hit });
+  for (const hit of scan(f)) findings.push({ file: f, ...hit });
+}
+
+function reportLossy() {
+  if (lossyFiles.length === 0) return;
+  console.error(`NOTE: ${lossyFiles.length} stageable file(s) are not valid UTF-8 and were`);
+  console.error('      scanned as ASCII. Credential shapes are ASCII, so this finds them,');
+  console.error('      but text in another encoding would not have been read:');
+  for (const f of lossyFiles.slice(0, 10)) console.error(`        ${safeFile(f)}`);
 }
 
 if (findings.length === 0) {
+  reportLossy();
   console.log('PASS: nothing `git add -A` would stage looks like a credential.');
   process.exit(0);
 }
 
+reportLossy();
+
 console.error(`FAIL: ${findings.length} stageable file(s) contain credential-shaped data\n`);
 for (const f of findings) {
-  console.error(`  ${f.file}:${f.line}`);
-  console.error(`      ${f.label} — ${f.hint}\n`);
+  console.error(`  ${safeFile(f.file)}:${f.line}`);
+  console.error(`      ${f.label} - ${f.hint}`);
+  if (f.detail) console.error(`      ${describeDetail(f.detail)}`);
+  console.error('');
 }
 console.error('These are NOT committed yet, and this repo is public.');
 console.error('Fix by ignoring the file, not by deleting it — something may be using it:');

scripts/hosted-canary.mjs

@@ -33,8 +33,8 @@
 // Exit 0 healthy, 1 unhealthy, 2 misconfigured. Run it from cron or launchd.
 
 import { existsSync, writeFileSync, unlinkSync, readFileSync } from 'node:fs';
-import { pathToFileURL } from 'node:url';
 import { randomUUID } from 'node:crypto';
+import { isMainModule } from '../src/common/entrypoint.mjs';
 
 /** The message we post. A nonce, so reading it back proves OUR write landed
  *  rather than finding someone else's old row. */
@@ -162,6 +162,6 @@ async function main() {
   process.exit(result.ok ? 0 : 1);
 }
 
-if (import.meta.url === pathToFileURL(process.argv[1] || '').href) {
+if (isMainModule(import.meta.url)) {
   main();
 }

scripts/local-agent.mjs

@@ -137,7 +137,9 @@ export async function runAgent(cfg) {
 }
 
 // CLI: config from IAK dogfood config + env overrides.
-if (import.meta.url === `file://${process.argv[1]}`) {
+import { isMainModule } from '../src/common/entrypoint.mjs';
+
+if (isMainModule(import.meta.url)) {
   const { readFileSync } = await import('node:fs');
   const cfgPath = process.env.IAK_CONFIG || '/Users/petrus/ide-agent-kit/config/dogfood.json';
   let base = {};

scripts/local-relay.mjs

@@ -38,6 +38,7 @@ import { appendFileSync, mkdirSync, readFileSync, existsSync } from 'node:fs';
 import { randomUUID, timingSafeEqual, createHash } from 'node:crypto';
 import { homedir } from 'node:os';
 import { dirname, join } from 'node:path';
+import { isMainModule } from '../src/common/entrypoint.mjs';
 
 const DEFAULT_PORT = 18790;
 const DEFAULT_STORE = join(homedir(), '.iak', 'local-relay.jsonl');
@@ -209,7 +210,7 @@ export function startRelay(opts = {}) {
 }
 
 // CLI entry
-if (import.meta.url === `file://${process.argv[1]}`) {
+if (isMainModule(import.meta.url)) {
   const arg = (name) => {
     const i = process.argv.indexOf(name);
     return i !== -1 ? process.argv[i + 1] : undefined;

scripts/poller-health-alert.mjs

@@ -18,7 +18,7 @@
 //      supervisor loop - codex review of PR #87).
 import { existsSync, statSync, readFileSync, writeFileSync, unlinkSync } from 'node:fs';
 import { execFileSync } from 'node:child_process';
-import { pathToFileURL } from 'node:url';
+import { isMainModule } from '../src/common/entrypoint.mjs';
 
 export function heartbeatAge(path, now = Date.now()) {
   if (!existsSync(path)) return Infinity;
@@ -136,6 +136,6 @@ async function main() {
   }
 }
 
-if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
+if (isMainModule(import.meta.url)) {
   await main();
 }