[luv-legion-722] Make secret detection prevent, not just report

CHANGELOG.md

@@ -4,6 +4,12 @@
 
 ### Features
 
+- Make secret detection *prevent* something. The five `sanitize-*` policies run on `PostToolUse`, which replaces the tool result on Codex and Copilot and is observation-only on the other ten CLIs — so on Claude Code a firing sanitizer meant the model read the secret **and** a note saying it had been blocked. Two new policies run at `PreToolUse`, which blocks on all twelve: **`block-secret-in-write`** stops a recognised key or token being written into a file's contents (nothing scanned write content before — `warn-large-file-write` read it only to measure its length), and **`block-credential-files`** stops reads and writes of SSH private keys, `~/.aws/credentials`, `.git-credentials`, `.netrc`, `.pypirc`, Docker/gcloud config, GCP service-account keys, keystores and GnuPG material, none of which any default-on policy covered. Both are in the recommended baseline. `block-secret-in-write` reads the new content and never `old_string`, so the Edit that *removes* a leaked key is still allowed — a whole-payload scan would have blocked the remediation — and Bash stays out of scope because grepping for the pattern is how you find the leak. It also scans every string field rather than a canonical `content` key, because only three of the twelve CLIs canonicalise one, and a policy that silently sees nothing on the rest while reading "enabled" is the failure this codebase has been burned by before. (#PR)
+
+- Add roughly 25 credential formats and the tier that decides which of them may deny. Slack (tokens and webhook URLs), GitLab, npm, PyPI, the four non-`ghp_` GitHub token types, AWS `ASIA` and secret access keys, Google OAuth, Azure storage, Supabase, Square, Shopify, Telegram, SendGrid, Hugging Face, Vault, Doppler, Linear, Notion, Figma, Postman, and HTTP Basic. Shapes that collide with things an agent reads constantly — Twilio's 32-hex SIDs, Discord's prefix-less base64, Sentry DSNs — are redact-only and cannot reach a blocking policy, enforced by a test rather than a convention. `sanitize-private-key-content` also learns the PGP header, whose trailing `BLOCK` had put it outside the pattern. (#PR)
+
+- Add `warn-assigned-secret`, off by default: a credential-named variable assigned a literal value, which no vendor prefix can match. It instructs rather than denies, and its name tuning is taken from the daemon's redactor rather than the audit one — the two disagree on 7 of 12 common names, and the audit version fires on a bare `key=`, which is React's prop on every JSX list. (#PR)
+
 - Give the canary box three images instead of one, and bake the commit into each. `failproofai-canary`, `failproofai-translate` and `failproofai-docs-audit` replace the single shared toolchain image, and each carries the checkout, its dependencies and its build products — the canary also carries a compiled `failproofaid`, which it used to cross-compile in a sibling rust container on every run. What happened at 02:00 and 11:00 in front of nobody — clone, fetch, checkout, `bun install`, two `bun build`s, a `cargo build` — happens in CI now, once per commit, where a failure is a red build rather than a night with no report. Installing the twelve vendor CLIs @latest deliberately stays at run time: that is the measurement, not setup. **Only the canary carries a docker client**, and only its cron line mounts the socket — the other two spawn nothing, and an image without the client cannot be talked into reaching the host daemon. That split is the reason for three images rather than one, and it is asserted rather than described. (#PR)
 
 - Give the canary box three images instead of one, and bake the commit into each. `failproofai-canary`, `failproofai-translate` and `failproofai-docs-audit` replace the single shared toolchain image, and each carries the checkout, its dependencies and its build products — the canary also carries a compiled `failproofaid`, which it used to cross-compile in a sibling rust container on every run. What happened at 02:00 and 11:00 in front of nobody — clone, fetch, checkout, `bun install`, two `bun build`s, a `cargo build` — happens in CI now, once per commit, where a failure is a red build rather than a night with no report. Installing the twelve vendor CLIs @latest deliberately stays at run time: that is the measurement, not setup. **Only the canary carries a docker client**, and only its cron line mounts the socket — the other two spawn nothing, and an image without the client cannot be talked into reaching the host daemon. That split is the reason for three images rather than one, and it is asserted rather than described. (#705)
@@ -14,6 +20,14 @@
 
 ### Fixes
 
+- Stop denying tool calls over things that are not credentials. The blocking patterns had no token boundaries, so `sk-` matched inside `risk-averse` and a `kubectl get pods -n risk-scoring` was refused as an "OpenAI API key"; formats with a fixed length matched past it, so `ghp_` plus 40 characters passed as a 36-character token and left four characters of a real one unredacted in audit digests. Both are anchored now, using the same character class the daemon's redactor uses so the two agree on what a boundary is. AWS's documentation keys are allowlisted by exact value — they are correctly shaped, they appear in roughly every AWS tutorial and README, and no anchoring can tell them apart from the real thing. (#PR)
+
+- Stop one malformed config line from denying every tool call on the machine. `sanitize-api-keys.additionalPatterns` compiled user regexes with no validation, and `new RegExp(undefined)` — what a bare `["foo"]` entry destructures to, which is exactly the shape the identically-named parameter on `block-secrets-write` takes — is `/(?:)/`, which matches everything. Nothing threw, so the `try/catch` never fired. Entries are now shape-checked, patterns that match the empty string or carry nested quantifiers are rejected rather than run, sources and counts are capped, the haystack is bounded, and compiled patterns are cached. `block-secrets-write` skips non-string entries instead of throwing, which the evaluator would have swallowed into a silently disabled policy. (#PR)
+
+- Stop the scrubber describing what it scrubbed. On Codex and Copilot a `PostToolUse` deny replaces the tool result, and the text sent was "Blocked Bash by failproofai because: JWT token detected in tool output" — announcing the secret in place of it. `PolicyResult.message`, which the five sanitizers have always set to `[REDACTED: … removed by failproofai]` and which nothing had ever read, is now that replacement text. (#PR)
+
+- Close three gaps found while covering the above: `.envrc` was readable though direnv files hold what `.env` holds; `block-secrets-write` gated `Write` but not `Edit`, matched only `id_rsa` among key names ed25519 has long since displaced, and blocked `id_rsa.pub`, which exists to be handed out; and `block-self-pause` had no `SIGNAL_MAP` entry, so an agent pausing its own enforcement counted for nothing in the audit's archetype. A test now asserts every policy and detector is mapped, and another reads `PREFIX_RULES` out of the Rust collector to assert the engine and the daemon recognise the same vendor prefixes — they had already diverged in both directions. (#PR)
+
 - Let the box pick the translation model per tier, and stop a re-install double-scheduling the box. `getModelForTier` now reads `TRANSLATE_MODEL_TIER1` / `TRANSLATE_MODEL_TIER23`, so the seven languages most readers actually arrive in can keep a strong model while the long tail runs on something cheap — the CLI's `--model` flag flattens every tier to one model, which is the opposite of what the tier split exists for. Any id the gateway serves over the Anthropic `/v1/messages` shape works, since that is the API the translator speaks (verified: `deepseek-v4-pro` and `deepseek-v4-flash` both answer there). Separately, `install.sh` now strips the pre-marker cron form as well as its own marker: a box set up before the marker existed carries a long-form inline `docker run … -e CANARY_JOB=<job>` line, and matching only the marker left it in place — six entries, every job scheduled twice, one on the old image and one on the new. The per-job flock keeps that from doing damage and turns it into something worse to diagnose: which image runs becomes a coin toss. Found on the real box, whose crontab is exactly that shape. (#705)
 
 - Make a non-PASS canary verdict explain itself. `probe-cli.sh` captured each agent's stdout and stderr into `$OUTA`/`$OUTB`, used them for two greps, and threw them away; `run.sh` then echoed `tail -20` of the probe on any non-PASS verdict — and the last 20 lines of that probe are the verdict block, so the log restated the verdict instead of giving the cause. Four CLIs sat yellow on the box for three consecutive days with nothing recorded anywhere but the word INCONCLUSIVE, and re-running produced the same nothing because the evidence was discarded both times. Each failing probe now prints the last 25 lines of what the CLI actually said, plus whether a hook fired at all, and the tail window widens to 80 so the explanation lands inside it. The daemon note is corrected in the same breath: `daemon: routed, no fail-closed denies` was printed whenever the grep for `daemon-unreachable` found nothing, which is equally what **no hook log at all** looks like — a run where the daemon was never asked anything now says so instead of claiming a real evaluation. (#705)

__tests__/audit/redact-example.test.ts

@@ -213,19 +213,24 @@ describe("maskAssignedSecrets — the shape the blocking patterns do not carry",
   // is `export VAR=…`, whose example is the WHOLE command. Every one of these
   // reached the server and the email verbatim before this masking existed:
   // `SECRET_PATTERNS` matches vendor prefixes, not assignments.
+  // Split by which pass is expected to fire, because that is now a real
+  // distinction: several of these vendor prefixes moved INTO SECRET_PATTERNS,
+  // so `maskSecrets` reaches them first and names them specifically. The
+  // invariant both groups share — the value does not survive — is asserted for
+  // every row; only the label differs.
   it("masks the value of an assignment whose name says it is a credential", () => {
-    const cases = [
+    // No recognisable vendor shape, so the name-based pass is the ONLY thing
+    // standing between these and the digest. This is what maskAssignedSecrets
+    // exists for and the group that must keep its specific label.
+    const noVendorShape = [
       "export DATABASE_PASSWORD=hunter2-prod-acme",
-      "export SLACK_BOT_TOKEN=xoxb-2314-4432-aBcDeFgHiJkLmNoPqRsTuVwX",
-      "export HF_TOKEN=hf_AbCdEfGhIjKlMnOpQrStUvWxYz012345",
-      "export NPM_TOKEN=npm_aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789",
-      "export GITLAB_TOKEN=glpat-AbCdEfGhIjKlMnOpQr",
+      // 38 chars, not AWS's 40, so the vendor rule correctly declines it.
       "export AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY",
       "FOO_SECRET=abc123 ./run.sh",
       "PGPASSWORD=letmein psql -h prod",
       "npm config set _authToken=abcdef123456",
     ];
-    for (const input of cases) {
+    for (const input of noVendorShape) {
       const out = redactExample(input, HOME);
       expect(out, input).toContain("[REDACTED: assigned secret]");
       // The secret itself must be gone; the NAME is kept on purpose, because
@@ -235,6 +240,26 @@ describe("maskAssignedSecrets — the shape the blocking patterns do not carry",
     }
   });
 
+  it("masks a vendor-shaped assignment value, naming the vendor", () => {
+    // Same threat, better label. `maskAssignedSecrets` deliberately declines to
+    // re-mask a value an earlier pass already named — re-masking would downgrade
+    // "Slack token" to the generic label and strip the marker's own tail.
+    const vendorShaped: Array<[string, string]> = [
+      ["export SLACK_BOT_TOKEN=xoxb-2314-4432-aBcDeFgHiJkLmNoPqRsTuVwX", "Slack token"],
+      ["export HF_TOKEN=hf_AbCdEfGhIjKlMnOpQrStUvWxYz012345", "Hugging Face token"],
+      ["export NPM_TOKEN=npm_aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789", "npm access token"],
+      ["export GITLAB_TOKEN=glpat-AbCdEfGhIjKlMnOpQr", "GitLab personal access token"],
+    ];
+    for (const [input, label] of vendorShaped) {
+      const out = redactExample(input, HOME);
+      expect(out, input).toContain(`[REDACTED: ${label}]`);
+      const value = input.split("=")[1].split(" ")[0];
+      expect(out, input).not.toContain(value);
+      // The variable name survives either way — that is the actionable half.
+      expect(out, input).toContain(input.split("=")[0].split(" ").pop()!);
+    }
+  });
+
   it("keeps the variable name, so the digest still says what was exposed", () => {
     expect(redactExample("export DATABASE_PASSWORD=hunter2", HOME)).toBe(
       "export DATABASE_PASSWORD=[REDACTED: assigned secret]",

__tests__/audit/signal-map-coverage.test.ts

@@ -0,0 +1,73 @@
+/**
+ * SIGNAL_MAP must name every builtin policy and every audit-only detector.
+ *
+ * The failure this prevents is silent in both directions and visible in
+ * neither: a policy with no entry contributes nothing to the archetype
+ * classifier, so an agent that trips it repeatedly is described as though it
+ * never did — and an entry naming a policy that no longer exists is dead weight
+ * that reads as coverage. `src/audit/features.ts` asserted this in prose ("every
+ * one of the 39 builtin policies…"), which had already drifted by four policies
+ * before this test existed.
+ */
+import { describe, it, expect } from "vitest";
+import { SIGNAL_MAP, shortName } from "../../src/audit/features";
+import { BUILTIN_POLICIES } from "../../src/hooks/builtin-policies";
+import { AUDIT_DETECTORS } from "../../src/audit/detectors";
+
+/**
+ * Policies deliberately left unmapped, each with the reason it is excluded.
+ *
+ * `block-read-outside-cwd` is off by default and fires on ambient absolute-path
+ * reads present in essentially every session. The audit replay force-registers
+ * every builtin regardless of config, so mapping it made it ≈37% of all signal
+ * and collapsed the population onto "the explorer" — the regression
+ * __tests__/audit/distribution.test.ts exists to hold down.
+ */
+const INTENTIONALLY_UNMAPPED = new Set(["block-read-outside-cwd"]);
+
+describe("SIGNAL_MAP coverage", () => {
+  it("maps every builtin policy exactly once", () => {
+    const missing = BUILTIN_POLICIES
+      .map((p) => shortName(p.name))
+      .filter((n) => !INTENTIONALLY_UNMAPPED.has(n) && !(n in SIGNAL_MAP));
+    expect(
+      missing,
+      `builtin policies with no SIGNAL_MAP entry — they will fire in audits and ` +
+      `contribute nothing to the archetype: ${missing.join(", ")}`,
+    ).toEqual([]);
+  });
+
+  it("maps every audit-only detector", () => {
+    const missing = AUDIT_DETECTORS
+      .map((d) => d.name)
+      .filter((n) => !(n in SIGNAL_MAP));
+    expect(missing, `detectors with no SIGNAL_MAP entry: ${missing.join(", ")}`).toEqual([]);
+  });
+
+  it("names nothing that no longer exists", () => {
+    const known = new Set([
+      ...BUILTIN_POLICIES.map((p) => shortName(p.name)),
+      ...AUDIT_DETECTORS.map((d) => d.name),
+    ]);
+    const orphans = Object.keys(SIGNAL_MAP).filter((n) => !known.has(n));
+    expect(
+      orphans,
+      `SIGNAL_MAP names these, but no policy or detector does: ${orphans.join(", ")}`,
+    ).toEqual([]);
+  });
+
+  it("keeps the intentional exclusions real", () => {
+    const known = new Set(BUILTIN_POLICIES.map((p) => shortName(p.name)));
+    for (const name of INTENTIONALLY_UNMAPPED) {
+      expect(known.has(name), `${name} is excluded but no longer exists`).toBe(true);
+      expect(name in SIGNAL_MAP, `${name} is excluded but IS mapped`).toBe(false);
+    }
+  });
+
+  it("gives every entry a positive weight and a real archetype", () => {
+    for (const [name, entry] of Object.entries(SIGNAL_MAP)) {
+      expect(entry.weight, `${name} weight`).toBeGreaterThan(0);
+      expect(typeof entry.archetype, `${name} archetype`).toBe("string");
+    }
+  });
+});

__tests__/e2e/helpers/hook-runner.ts

@@ -117,10 +117,19 @@ export function assertPostToolUseDeny(result: HookRunResult): void {
  * arrived would go green if a CLI were wired to the wrong one, in either
  * direction, which is precisely the regression these tests exist to catch.
  */
-export function assertPostToolUseBlockDecision(result: HookRunResult): void {
+export function assertPostToolUseBlockDecision(
+  result: HookRunResult,
+  expectedReason?: RegExp,
+): void {
   expect(result.exitCode).toBe(0);
   expect(result.parsed?.decision).toBe("block");
-  expect(result.parsed?.reason).toMatch(/Blocked/i);
+  // On these two CLIs `reason` REPLACES the tool result the model reads, so its
+  // text is policy-specific — a sanitize-* policy sends its redaction marker,
+  // everything else sends the blocked message. What is invariant, and what
+  // copilot's `vK` guard fails closed on, is that it is a non-empty STRING.
+  expect(typeof result.parsed?.reason).toBe("string");
+  expect((result.parsed?.reason as string).length).toBeGreaterThan(0);
+  if (expectedReason) expect(result.parsed?.reason).toMatch(expectedReason);
   // The nested shape must be ABSENT, not merely ignored: copilot's shipped
   // guard reads only the top level, so emitting both would leave the file
   // asserting a contract no consumer actually exercises.

__tests__/e2e/hooks/builtin-policies.e2e.test.ts

@@ -330,10 +330,23 @@ describe("block-env-files", () => {
     assertPreToolUseDeny(result);
   });
 
-  it("allows .envrc (different suffix)", () => {
+  // .envrc USED to be allowed here, on the reasoning that it is a different
+  // suffix. It is a direnv file and holds the same credentials .env holds, so
+  // the suffix was the only thing separating it from the rule that exists to
+  // protect exactly this content.
+  it("blocks .envrc, which holds the same thing", () => {
     const env = createFixtureEnv();
     env.writeConfig({ enabledPolicies: ["block-env-files"] });
     const result = runHook("PreToolUse", Payloads.preToolUse.bash("cat .envrc", env.cwd), { homeDir: env.home });
+    assertPreToolUseDeny(result);
+  });
+
+  // The original point of the case above — that the rule does not swallow every
+  // name beginning with .env — still needs holding down.
+  it("allows a file whose name merely starts with .env", () => {
+    const env = createFixtureEnv();
+    env.writeConfig({ enabledPolicies: ["block-env-files"] });
+    const result = runHook("PreToolUse", Payloads.preToolUse.read(".environment.md", env.cwd), { homeDir: env.home });
     assertAllow(result);
   });
 });

__tests__/e2e/hooks/codex-integration.e2e.test.ts

@@ -108,7 +108,12 @@ describe("E2E: Codex integration — hook protocol", () => {
         ),
         { homeDir: env.home, cli: "codex" },
       );
-      assertPostToolUseBlockDecision(result);
+      // The replacement text is the redaction marker, NOT "Blocked ... because:
+      // JWT token detected". These are the only two CLIs where this string
+      // replaces the tool result, so sending the diagnosis here would hand the
+      // model a description of the very secret being scrubbed.
+      assertPostToolUseBlockDecision(result, /REDACTED/i);
+      expect(result.parsed?.reason).not.toMatch(/JWT token detected/i);
     } finally {
       env.cleanup();
     }

__tests__/e2e/hooks/copilot-integration.e2e.test.ts

@@ -116,7 +116,12 @@ describe("E2E: Copilot integration — hook protocol", () => {
         ),
         { homeDir: env.home, cli: "copilot" },
       );
-      assertPostToolUseBlockDecision(result);
+      // The replacement text is the redaction marker, NOT "Blocked ... because:
+      // JWT token detected". These are the only two CLIs where this string
+      // replaces the tool result, so sending the diagnosis here would hand the
+      // model a description of the very secret being scrubbed.
+      assertPostToolUseBlockDecision(result, /REDACTED/i);
+      expect(result.parsed?.reason).not.toMatch(/JWT token detected/i);
     } finally {
       env.cleanup();
     }