From a31f0fe5e1f577cf1487c1ab804b7a498185da8d Mon Sep 17 00:00:00 2001 From: chhhee10 Date: Thu, 20 Aug 2026 19:17:08 +0530 Subject: [PATCH 01/17] Make the guard against disabling failproofai the one policy nobody can disable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `block-self-pause` and `block-failproofai-commands` were two halves of one guard, and they disagreed with each other. `block-self-pause` had the hardened matcher — segments split on shell operators, runner prefixes and their flags walked off, the binary resolved by basename, the shell-unescaped form re-checked — but only ever looked for `config --pause`. `block-failproofai-commands` had the whole surface, any CLI invocation plus package-manager uninstall, on a regex a single prefix defeated: `sudo failproofai config --pause`, `npx failproofai policies --uninstall`, `env X=1 failproofai …`, `/usr/local/bin/failproofai …` and `timeout 30 failproofai …` were all ALLOWED by a default-on self-protection policy. The merged policy is the hardened matcher over the broad surface, and it keeps `PermissionRequest` from the merged-in half — a real enforcement point on Copilot and Devin that the survivor never subscribed to. Where the two contradicted each other, the merge keeps what machines actually did. `block-self-pause` deliberately allowed `config --resume`, `config --status` and `policies --install`; both policies were default-on and the sibling denied all three first, so that allow never ran anywhere. It is now `alwaysOn`, a new flag `registerBuiltinPolicies` honours ahead of the enabled set. That closes the three ways the old pair could go dark without anyone noticing: a name absent from `enabledPolicies`, an active session pause (`handler.ts` passes `[]`), and a config file that fails to parse (`hooks-config.ts` soft-fails to `{enabledPolicies: []}` at five sites, so corrupting one file disabled every policy including these two). `policies --disable block-failproofai-commands` now refuses with a reason instead of editing the config and reporting a success that changes nothing. --- CHANGELOG.md | 4 + .../builtin-policies-extended.e2e.test.ts | 16 ++- .../e2e/hooks/builtin-policies.e2e.test.ts | 14 ++- __tests__/hooks/builtin-policies.test.ts | 103 +++++++++++----- __tests__/hooks/configure-wizard.test.ts | 6 +- __tests__/hooks/install-prompt.test.ts | 4 +- __tests__/hooks/manager.test.ts | 23 ++++ __tests__/hooks/policy-presets.test.ts | 19 ++- docs/policies/builtin-catalog.mdx | 15 ++- docs/reference/failproof-cli.mdx | 2 +- src/hooks/builtin-policies.ts | 110 +++++++++++------- src/hooks/manager.ts | 23 ++++ src/hooks/policy-presets.ts | 6 +- src/hooks/policy-types.ts | 11 ++ 14 files changed, 259 insertions(+), 97 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c205dc270..c2cc1d17c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## 1.0.2-beta.0 — 2026-08-19 +### Features + +- Merge `block-self-pause` into `block-failproofai-commands` and make the result the one policy that cannot be switched off. The two were halves of one guard and disagreed with each other. `block-self-pause` had the hardened matcher — segments split on shell operators, runner prefixes and their flags walked off, the binary resolved by basename, and the shell-unescaped form re-checked — but only ever looked for `config --pause`. `block-failproofai-commands` had the whole surface (any CLI invocation, plus package-manager uninstall) on `/(?:^|;|&&|\|\||\|)\s*failproofai(?:\s|$)/`, which a single prefix defeated: `sudo failproofai config --pause`, `npx failproofai policies --uninstall`, `env X=1 failproofai …`, `/usr/local/bin/failproofai …` and `timeout 30 failproofai …` were all ALLOWED by a `defaultEnabled` self-protection policy. The merged policy is the hardened matcher over the broad surface, and it keeps `PermissionRequest` from the merged-in half — a real enforcement point on Copilot and Devin that the survivor did not subscribe to. Where the two contradicted each other the merge keeps what machines actually did: `block-self-pause` deliberately allowed `config --resume`, `config --status` and `policies --install`, but both policies were default-on and the sibling denied all three first, so that allow never ran anywhere. It is now `alwaysOn`, a new flag `registerBuiltinPolicies` honours ahead of the enabled set, which closes the three ways the old pair could be switched off silently — a name absent from `enabledPolicies`, an active session pause (`handler.ts` passes `[]`), and a config file that fails to parse (`hooks-config.ts` soft-fails to `{enabledPolicies: []}`, so corrupting one file disabled every policy including these). `policies --disable block-failproofai-commands` now refuses with a reason instead of editing the config and reporting a success that changes nothing. (#PR) + ### Docs - Put the new `r/failproofai` subreddit everywhere the Discord invite already lives, so the second community channel is discoverable from the same places as the first: the README community badges (English + the 28 translated copies under `docs/i18n/` and `docs-old/i18n/`), the docs-site navbar (with its own hover tooltip in `custom.css`), the `failproofai --help` LINKS banner, the dashboard launch banner, the dashboard's "Reach Us" dropdown, and the comment the contributor-welcome workflow posts on every outside PR. (#732) diff --git a/__tests__/e2e/hooks/builtin-policies-extended.e2e.test.ts b/__tests__/e2e/hooks/builtin-policies-extended.e2e.test.ts index 6e2379f7d..d8e6484e9 100644 --- a/__tests__/e2e/hooks/builtin-policies-extended.e2e.test.ts +++ b/__tests__/e2e/hooks/builtin-policies-extended.e2e.test.ts @@ -5,7 +5,7 @@ * sanitize-connection-strings fixtures that trigger the PostToolUse hook. */ import { describe, it } from "vitest"; -import { runHook, assertAllow, assertInstruct } from "../helpers/hook-runner"; +import { runHook, assertAllow, assertInstruct, assertPreToolUseDeny } from "../helpers/hook-runner"; import { createFixtureEnv } from "../helpers/fixture-env"; import { Payloads } from "../helpers/payloads"; @@ -36,19 +36,25 @@ describe("warn-package-publish extended", () => { // ── block-failproofai-commands — npx/bunx invocations ─────────────────────────── +// Both cases below asserted `allow` until `block-self-pause` was merged in and +// its tokenizer replaced the anchored regex. The old test NAMES stated the hole +// as the expectation — "regex requires failproofai at cmd start, not after npx" +// — so a package runner in front of the binary walked through a default-on +// self-protection policy. The merged matcher walks runner prefixes off before +// it looks for the binary, so these deny now. describe("block-failproofai-commands extended", () => { - it("allows npx failproofai (regex requires failproofai at cmd start, not after npx)", () => { + it("blocks npx failproofai — a runner prefix no longer hides the binary", () => { const env = createFixtureEnv(); env.writeConfig({ enabledPolicies: ["block-failproofai-commands"] }); const result = runHook("PreToolUse", Payloads.preToolUse.bash("npx failproofai --list-policies", env.cwd), { homeDir: env.home }); - assertAllow(result); + assertPreToolUseDeny(result); }); - it("allows bunx failproofai (regex requires failproofai at cmd start, not after bunx)", () => { + it("blocks bunx failproofai — same, through the other runner", () => { const env = createFixtureEnv(); env.writeConfig({ enabledPolicies: ["block-failproofai-commands"] }); const result = runHook("PreToolUse", Payloads.preToolUse.bash("bunx failproofai --hook PreToolUse", env.cwd), { homeDir: env.home }); - assertAllow(result); + assertPreToolUseDeny(result); }); }); diff --git a/__tests__/e2e/hooks/builtin-policies.e2e.test.ts b/__tests__/e2e/hooks/builtin-policies.e2e.test.ts index 3aeefb706..249edf0cb 100644 --- a/__tests__/e2e/hooks/builtin-policies.e2e.test.ts +++ b/__tests__/e2e/hooks/builtin-policies.e2e.test.ts @@ -486,19 +486,25 @@ describe("warn-package-publish extended", () => { }); }); +// Both cases below asserted `allow` until `block-self-pause` was merged in and +// its tokenizer replaced the anchored regex. The old test NAMES stated the hole +// as the expectation — "regex requires failproofai at cmd start, not after npx" +// — so a package runner in front of the binary walked through a default-on +// self-protection policy. The merged matcher walks runner prefixes off before +// it looks for the binary, so these deny now. describe("block-failproofai-commands extended", () => { - it("allows npx failproofai (regex requires failproofai at cmd start, not after npx)", () => { + it("blocks npx failproofai — a runner prefix no longer hides the binary", () => { const env = createFixtureEnv(); env.writeConfig({ enabledPolicies: ["block-failproofai-commands"] }); const result = runHook("PreToolUse", Payloads.preToolUse.bash("npx failproofai --list-policies", env.cwd), { homeDir: env.home }); - assertAllow(result); + assertPreToolUseDeny(result); }); - it("allows bunx failproofai (regex requires failproofai at cmd start, not after bunx)", () => { + it("blocks bunx failproofai — same, through the other runner", () => { const env = createFixtureEnv(); env.writeConfig({ enabledPolicies: ["block-failproofai-commands"] }); const result = runHook("PreToolUse", Payloads.preToolUse.bash("bunx failproofai --hook PreToolUse", env.cwd), { homeDir: env.home }); - assertAllow(result); + assertPreToolUseDeny(result); }); }); diff --git a/__tests__/hooks/builtin-policies.test.ts b/__tests__/hooks/builtin-policies.test.ts index 60d7f5743..cd3c4500b 100644 --- a/__tests__/hooks/builtin-policies.test.ts +++ b/__tests__/hooks/builtin-policies.test.ts @@ -6,7 +6,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { execSync, execFileSync } from "node:child_process"; import { BUILTIN_POLICIES, registerBuiltinPolicies, clearGitBranchCache } from "../../src/hooks/builtin-policies"; -import { getPoliciesForEvent, clearPolicies } from "../../src/hooks/policy-registry"; +import { getPoliciesForEvent, clearPolicies, getAllPolicies } from "../../src/hooks/policy-registry"; import type { PolicyContext } from "../../src/hooks/policy-types"; vi.mock("node:fs/promises", () => ({ @@ -37,13 +37,21 @@ describe("hooks/builtin-policies", () => { }); describe("BUILTIN_POLICIES", () => { - it("has 40 built-in policies", () => { - expect(BUILTIN_POLICIES).toHaveLength(40); + // 40 before `block-self-pause` was merged into `block-failproofai-commands`. + it("has 39 built-in policies", () => { + expect(BUILTIN_POLICIES).toHaveLength(39); }); - it("has 12 default-enabled policies", () => { + it("has 11 default-enabled policies", () => { const defaults = BUILTIN_POLICIES.filter((p) => p.defaultEnabled); - expect(defaults).toHaveLength(12); + expect(defaults).toHaveLength(11); + }); + + it("has exactly one alwaysOn policy — the self-protection guard", () => { + // A second one would be a policy nobody can switch off that nobody + // decided to make unswitchable. The flag is deliberately not general. + const always = BUILTIN_POLICIES.filter((p) => p.alwaysOn); + expect(always.map((p) => p.name)).toEqual(["block-failproofai-commands"]); }); }); @@ -51,8 +59,10 @@ describe("hooks/builtin-policies", () => { it("registers only specified policies (canonicalized to default namespace)", () => { registerBuiltinPolicies(["block-sudo", "block-rm-rf"]); const policies = getPoliciesForEvent("PreToolUse", "Bash"); - expect(policies).toHaveLength(2); + // The alwaysOn self-protection guard rides along with every registration. + expect(policies).toHaveLength(3); expect(policies.map((p) => p.name).sort()).toEqual([ + "failproofai/block-failproofai-commands", "failproofai/block-rm-rf", "failproofai/block-sudo", ]); @@ -61,8 +71,9 @@ describe("hooks/builtin-policies", () => { it("accepts qualified names in enabledPolicies (forward compat)", () => { registerBuiltinPolicies(["failproofai/block-sudo", "failproofai/block-rm-rf"]); const policies = getPoliciesForEvent("PreToolUse", "Bash"); - expect(policies).toHaveLength(2); + expect(policies).toHaveLength(3); expect(policies.map((p) => p.name).sort()).toEqual([ + "failproofai/block-failproofai-commands", "failproofai/block-rm-rf", "failproofai/block-sudo", ]); @@ -71,12 +82,15 @@ describe("hooks/builtin-policies", () => { it("treats flat and qualified names as equivalent (mixed config works)", () => { registerBuiltinPolicies(["block-sudo", "failproofai/block-rm-rf"]); const policies = getPoliciesForEvent("PreToolUse", "Bash"); - expect(policies).toHaveLength(2); + expect(policies).toHaveLength(3); }); - it("registers nothing for empty array", () => { + it("registers ONLY the alwaysOn guard for an empty array", () => { + // An empty array is what a session pause and an unparseable config both + // produce. Everything else must go; the self-protection guard must not. registerBuiltinPolicies([]); - expect(getPoliciesForEvent("PreToolUse", "Bash")).toHaveLength(0); + const policies = getPoliciesForEvent("PreToolUse", "Bash"); + expect(policies.map((p) => p.name)).toEqual(["failproofai/block-failproofai-commands"]); }); }); @@ -509,8 +523,12 @@ describe("hooks/builtin-policies", () => { }); }); - describe("block-self-pause", () => { - const policy = BUILTIN_POLICIES.find((p) => p.name === "block-self-pause")!; + // `block-self-pause` was merged into `block-failproofai-commands`. Every + // red-team spelling it was hardened against is kept verbatim below, now + // asserted against the merged policy — the hardened matcher is the half of + // the merge that had to survive. + describe("block-failproofai-commands (self-pause half)", () => { + const policy = BUILTIN_POLICIES.find((p) => p.name === "block-failproofai-commands")!; const decide = async (command: string) => (await policy.fn(makeCtx({ toolName: "Bash", toolInput: { command } }))).decision; @@ -607,23 +625,24 @@ describe("hooks/builtin-policies", () => { expect(await decide("p=proof; failp${p}ai config --pause")).toBe("allow"); }); - it("still allows resume and status in those same spellings", async () => { - // The widened match must not start denying the two commands that restore - // or merely report enforcement — that would make the policy costly to - // keep on, and a policy people switch off protects nobody. - expect(await decide("npx failproofai@latest config --resume")).toBe("allow"); - expect(await decide("/usr/local/bin/failproofai config --status")).toBe("allow"); - expect(await decide("node /path/to/failproofai.mjs config --resume")).toBe("allow"); - }); - - it("allows resume and status — neither removes enforcement", async () => { - expect(await decide("failproofai config --resume")).toBe("allow"); - expect(await decide("failproofai config --status")).toBe("allow"); - }); - - it("allows ordinary failproofai use and unrelated commands", async () => { - expect(await decide("failproofai config")).toBe("allow"); - expect(await decide("failproofai policies --install block-sudo")).toBe("allow"); + // These three asserted `allow` under the former `block-self-pause`, which + // narrowed itself to `--pause` so it would stay cheap to keep enabled. That + // reasoning does not survive the merge, and it never described a real + // machine: `block-failproofai-commands` was `defaultEnabled` too and denied + // every one of them first, so the allow was unreachable in production. The + // merged policy is `alwaysOn` and cannot be switched off, which removes the + // only argument for the narrower surface. + it("denies resume and status — the merged surface is every self-invocation", async () => { + expect(await decide("failproofai config --resume")).toBe("deny"); + expect(await decide("failproofai config --status")).toBe("deny"); + expect(await decide("npx failproofai@latest config --resume")).toBe("deny"); + expect(await decide("/usr/local/bin/failproofai config --status")).toBe("deny"); + expect(await decide("node /path/to/failproofai.mjs config --resume")).toBe("deny"); + }); + + it("denies ordinary failproofai use, and still allows unrelated commands", async () => { + expect(await decide("failproofai config")).toBe("deny"); + expect(await decide("failproofai policies --install block-sudo")).toBe("deny"); expect(await decide("git commit -m 'pause the rollout'")).toBe("allow"); }); @@ -1326,6 +1345,32 @@ describe("hooks/builtin-policies", () => { const ctx = makeCtx({ toolName: "Read", toolInput: { command: "failproofai --remove-policies" } }); expect((await policy.fn(ctx)).decision).toBe("allow"); }); + + // The half inherited from `block-self-pause`: the old regex here anchored on + // start-of-string or a shell operator, so ANY runner or prefix in front of + // the binary walked straight through a `defaultEnabled` self-protection + // guard. Each line below was allowed before the merge. + it("blocks the prefixes the old anchor let through", async () => { + const decide = async (command: string) => + (await policy.fn(makeCtx({ toolName: "Bash", toolInput: { command } }))).decision; + expect(await decide("sudo failproofai config --pause")).toBe("deny"); + expect(await decide("npx failproofai policies --uninstall")).toBe("deny"); + expect(await decide("env X=1 failproofai config --pause")).toBe("deny"); + expect(await decide("/usr/local/bin/failproofai --remove-policies")).toBe("deny"); + expect(await decide("timeout 30 failproofai --cache-clear")).toBe("deny"); + expect(await decide("doas failproofai config --pause")).toBe("deny"); + }); + + it("is alwaysOn, and registers with an empty enabled set", () => { + expect(policy.alwaysOn).toBe(true); + clearPolicies(); + // What `handler.ts` passes during a session pause, and what + // `hooks-config.ts` soft-fails to when the config will not parse. + registerBuiltinPolicies([]); + const names = getAllPolicies().map((r) => r.name); + expect(names).toContain("failproofai/block-failproofai-commands"); + expect(names).toHaveLength(1); + }); }); describe("block-kubectl", () => { diff --git a/__tests__/hooks/configure-wizard.test.ts b/__tests__/hooks/configure-wizard.test.ts index 8308add51..870cb8885 100644 --- a/__tests__/hooks/configure-wizard.test.ts +++ b/__tests__/hooks/configure-wizard.test.ts @@ -418,7 +418,7 @@ describe("configure-wizard pure builders", () => { }); it("reviewLines gives a taste of the policies without listing them all", () => { - // Two names say what KIND of thing these are; naming all fifteen turned a + // Two names say what KIND of thing these are; naming all fourteen turned a // four-line review into a thirteen-line one, and a screen nobody reads to // the bottom conveys less than a short one. const lines = reviewLines({ @@ -428,8 +428,8 @@ describe("configure-wizard pure builders", () => { cwd: "/tmp/proj", }); const joined = lines.join("\n"); - expect(joined).toContain("15 enabled"); - expect(joined).toContain("block-curl-pipe-sh, block-env-files +13"); + expect(joined).toContain("14 enabled"); + expect(joined).toContain("block-curl-pipe-sh, block-env-files +12"); // The other thirteen are NOT on screen. expect(joined).not.toContain("sanitize-private-key-content"); // One line for the count, one for the taste — never a paragraph. diff --git a/__tests__/hooks/install-prompt.test.ts b/__tests__/hooks/install-prompt.test.ts index 5f8483772..90b8741a0 100644 --- a/__tests__/hooks/install-prompt.test.ts +++ b/__tests__/hooks/install-prompt.test.ts @@ -30,11 +30,11 @@ describe("hooks/install-prompt", () => { expect(selected).toContain("block-curl-pipe-sh"); expect(selected).toContain("block-push-master"); expect(selected).toContain("block-failproofai-commands"); - expect(selected).toContain("block-self-pause"); expect(selected).not.toContain("block-rm-rf"); expect(selected).not.toContain("block-force-push"); expect(selected).not.toContain("block-secrets-write"); - expect(selected).toHaveLength(12); + // 12 before `block-self-pause` merged into `block-failproofai-commands`. + expect(selected).toHaveLength(11); }); it("returns preSelected when stdin is not a TTY and preSelected is provided", async () => { diff --git a/__tests__/hooks/manager.test.ts b/__tests__/hooks/manager.test.ts index 4b80874b2..28518934e 100644 --- a/__tests__/hooks/manager.test.ts +++ b/__tests__/hooks/manager.test.ts @@ -857,6 +857,29 @@ describe("hooks/manager", () => { expect(written.someOtherSetting).toBe(true); }); + it("refuses to disable the alwaysOn self-protection policy", async () => { + // Stripping it from enabledPolicies writes fine and changes nothing: + // `registerBuiltinPolicies` registers it regardless. Reporting success + // would tell the operator a policy is off while it keeps denying. + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue("{}"); + + const { removeHooks } = await import("../../src/hooks/manager"); + + await expect(removeHooks(["block-failproofai-commands"])).rejects.toThrow( + "Cannot disable: block-failproofai-commands", + ); + expect(writeFileSync).not.toHaveBeenCalled(); + }); + + it("still disables an ordinary policy alongside the refusal check", async () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue("{}"); + + const { removeHooks } = await import("../../src/hooks/manager"); + await expect(removeHooks(["block-sudo"])).resolves.not.toThrow(); + }); + it("handles missing settings file gracefully", async () => { vi.mocked(existsSync).mockReturnValue(false); diff --git a/__tests__/hooks/policy-presets.test.ts b/__tests__/hooks/policy-presets.test.ts index 80a770a5d..0724a3e6e 100644 --- a/__tests__/hooks/policy-presets.test.ts +++ b/__tests__/hooks/policy-presets.test.ts @@ -63,11 +63,12 @@ describe("policy-presets", () => { }); describe("RECOMMENDED_POLICIES", () => { - it("names 15 policies and every one of them is a real non-beta builtin", () => { + it("names 14 policies and every one of them is a real non-beta builtin", () => { // The count is asserted because it is a product promise the wizard PRINTS - // ("15 policies · global"). Changing the set is fine; changing it without + // ("14 policies · global"). Changing the set is fine; changing it without // noticing that the screen now advertises a different number is not. - expect(RECOMMENDED_POLICIES).toHaveLength(15); + // 15 before `block-self-pause` merged into `block-failproofai-commands`. + expect(RECOMMENDED_POLICIES).toHaveLength(14); for (const name of RECOMMENDED_POLICIES) { const policy = BUILTIN_POLICIES.find((p) => p.name === name); expect(policy, `${name} is not a builtin policy`).toBeDefined(); @@ -119,3 +120,15 @@ describe("RECOMMENDED_POLICIES", () => { expect(RECOMMENDED_POLICIES).toContain("block-secrets-write"); }); }); + +describe("always-on self-protection", () => { + it("is in RECOMMENDED_POLICIES even though it does not need to be", () => { + // It registers regardless. Listing it keeps the recommended set readable as + // the complete picture rather than quietly omitting its most important line. + expect(RECOMMENDED_POLICIES).toContain("block-failproofai-commands"); + }); + + it("leaves no default-on policy out of Recommended after the merge", () => { + expect(defaultsMissingFromRecommended()).toEqual([]); + }); +}); diff --git a/docs/policies/builtin-catalog.mdx b/docs/policies/builtin-catalog.mdx index 2296fd769..75029a507 100644 --- a/docs/policies/builtin-catalog.mdx +++ b/docs/policies/builtin-catalog.mdx @@ -15,12 +15,16 @@ sanitize-jwt sanitize-api-keys sanitize-connection-strings sanitize-private-key-content sanitize-bearer-tokens protect-env-vars block-env-files block-secrets-write -block-self-pause block-failproofai-commands -block-sudo block-curl-pipe-sh -block-rm-rf block-push-master -block-force-push +block-failproofai-commands block-sudo +block-curl-pipe-sh block-rm-rf +block-push-master block-force-push ``` +`block-failproofai-commands` is **always on**. It is listed above for +completeness, but it registers on every evaluation whether or not it appears in +your enabled set, and it cannot be disabled or paused — a guard against the +agent switching off enforcement that the agent can switch off is not a guard. + Recommended is deliberately narrower than **Everything**. Infrastructure and workflow policies can interrupt valid work and should be enabled for the repositories and machines that need them. ## Secrets and environment @@ -41,11 +45,10 @@ Recommended is deliberately narrower than **Everything**. Infrastructure and wor | Policy | Trigger | Outcome | | --- | --- | --- | -| `block-self-pause` | `PreToolUse`, `PermissionRequest` | Prevent an agent from pausing its own enforcement. | | `block-sudo` | `PreToolUse`, `PermissionRequest` | Block `sudo` unless an allow pattern matches. | | `block-curl-pipe-sh` | `PreToolUse` | Block downloaded scripts piped directly to a shell. | | `block-rm-rf` | `PreToolUse` | Block catastrophic recursive deletion patterns. | -| `block-failproofai-commands` | `PreToolUse` | Prevent agents from changing or uninstalling Failproof AI. | +| `block-failproofai-commands` | `PreToolUse`, `PermissionRequest` | **Always on, cannot be disabled.** Block every Failproof AI CLI invocation, self-pause, and package-manager uninstall. | | `block-kubectl` | `PreToolUse` | Gate Kubernetes commands. | | `block-terraform` | `PreToolUse` | Gate Terraform and OpenTofu commands. | | `block-aws-cli` | `PreToolUse` | Gate AWS CLI commands. | diff --git a/docs/reference/failproof-cli.mdx b/docs/reference/failproof-cli.mdx index 2e23e0cd1..38deaa4e3 100644 --- a/docs/reference/failproof-cli.mdx +++ b/docs/reference/failproof-cli.mdx @@ -62,7 +62,7 @@ Run `failproofai` without arguments to open the local policy dashboard. | `--session ` | Target an explicit session for pause or resume | | `--all` | With `--resume`, end every active pause | -Local pauses suspend builtin, custom, and convention policies for one session. They always expire and do not disable Cloud-managed policies. `block-self-pause` prevents an instrumented agent from using this escape hatch itself. +Local pauses suspend builtin, custom, and convention policies for one session. They always expire and do not disable Cloud-managed policies. `block-failproofai-commands` — which is always on and cannot itself be disabled or paused — prevents an instrumented agent from using this escape hatch itself. ## Policy flags diff --git a/src/hooks/builtin-policies.ts b/src/hooks/builtin-policies.ts index 738e630f9..07bd1db19 100644 --- a/src/hooks/builtin-policies.ts +++ b/src/hooks/builtin-policies.ts @@ -290,7 +290,17 @@ const RUNNER_OPERAND_RE = /^\d+[a-z]*$/i; * and denies. That is a far narrower miss than matching every mention, and it * errs toward refusing rather than toward silently suspending enforcement. */ -function namesSelfPause(command: string): boolean { +type SelfInvocation = "pause" | "cli"; + +/** + * Classify a command by what it does to failproofai itself, or null when it + * does nothing to failproofai at all. + * + * `pause` outranks `cli` wherever both appear, because the pause verdict is the + * one whose message has to explain that suspending enforcement is a human call. + */ +function classifySelfInvocation(command: string): SelfInvocation | null { + let found: SelfInvocation | null = null; for (const segment of command.split(SEGMENT_SEPARATORS)) { const tokens = segment.split(/\s+/).filter(Boolean); let i = 0; @@ -311,14 +321,16 @@ function namesSelfPause(command: string): boolean { if (i >= tokens.length) continue; if (!SELF_BINARY_TOKEN_RE.test(tokens[i])) continue; - // The binary IS the command here. Now require `config … --pause` in the - // argument order the CLI actually accepts. + // The binary IS the command here. `config … --pause` is singled out only for + // its message; every other subcommand is denied just the same. const args = tokens.slice(i + 1); const configAt = args.findIndex((a) => CONFIG_SUBCOMMAND_RE.test(a)); - if (configAt === -1) continue; - if (args.slice(configAt + 1).some((a) => PAUSE_FLAG_RE.test(a))) return true; + if (configAt !== -1 && args.slice(configAt + 1).some((a) => PAUSE_FLAG_RE.test(a))) { + return "pause"; + } + found = "cli"; } - return false; + return found; } /** @@ -400,7 +412,6 @@ const SECRET_FILE_CREDENTIALS_RE = /credentials/; const GIT_COMMIT_MERGE_RE = /git\s+(commit|merge|rebase|cherry-pick)\b/; // blockFailproofaiCommands -const FAILPROOFAI_CLI_RE = /(?:^|;|&&|\|\||\|)\s*failproofai(?:\s|$)/; const FAILPROOFAI_UNINSTALL_RE = /(?:npm\s+(?:uninstall|remove|un|r)\s.*failproofai|bun\s+remove\s.*failproofai|yarn\s+global\s+remove\s+failproofai|pnpm\s+(?:remove|uninstall|un)\s.*failproofai)/; // warnGitAmend @@ -968,20 +979,6 @@ function blockSudo(ctx: PolicyContext): PolicyResult { * can run `failproofai audit`. Neither gap should leave pausing reachable, so * this stays narrow, matches the runner forms, and survives that one being off. */ -function blockSelfPause(ctx: PolicyContext): PolicyResult { - if (ctx.toolName !== "Bash") return allow(); - const cmd = getCommand(ctx); - // The raw command AND its shell-unescaped form: a shell strips quotes and - // backslashes before running the binary, so `fail\proofai config --pause` - // reaches the pause CLI even though the literal name is broken. - if (namesSelfPause(cmd) || namesSelfPause(stripShellQuoting(cmd))) { - return deny( - "Pausing failproofai enforcement is a human action, not an agent one. " + - "If a policy is blocking legitimate work, say so and let the operator decide.", - ); - } - return allow(); -} function blockCurlPipeSh(ctx: PolicyContext): PolicyResult { if (ctx.toolName !== "Bash") return allow(); @@ -1386,20 +1383,49 @@ function blockWorkOnMain(ctx: PolicyContext): PolicyResult { return allow(); } +/** + * The one policy that cannot be turned off — see `alwaysOn` on its definition. + * + * Merged from the former `block-self-pause` and `block-failproofai-commands`, + * which were two halves of one guard that disagreed with each other. + * + * `block-self-pause` had the hardened matcher: it walks off runner prefixes and + * re-checks the shell-unescaped form, so `sudo failproofai …`, `npx failproofai + * …` and `fail\proofai …` do not get through. But it only ever looked for + * `config --pause`. `block-failproofai-commands` had the whole surface — any CLI + * call, plus package-manager uninstall — on a regex a single `sudo` defeated. + * Keeping the broad surface and dropping the weak matcher is the only + * combination stronger than either half. + * + * They also contradicted each other: `block-self-pause` deliberately ALLOWED + * `config --resume`, `config --status` and `policies --install`, while + * `block-failproofai-commands` denied them. Both were `defaultEnabled`, so the + * deny is what actually happened on every machine and the allow never ran. The + * merge keeps the behaviour users have. + */ function blockFailproofaiCommands(ctx: PolicyContext): PolicyResult { if (ctx.toolName !== "Bash") return allow(); const cmd = getCommand(ctx); + // The raw command AND its shell-unescaped form: a shell strips quotes and + // backslashes before running the binary, so `fail\proofai config --pause` + // reaches the pause CLI even though the literal name is broken. + const unescaped = stripShellQuoting(cmd); + const kind = classifySelfInvocation(cmd) ?? classifySelfInvocation(unescaped); - // Block direct failproofai CLI invocations - if (FAILPROOFAI_CLI_RE.test(cmd)) { + if (kind === "pause") { + return deny( + "Pausing failproofai enforcement is a human action, not an agent one. " + + "If a policy is blocking legitimate work, say so and let the operator decide.", + ); + } + if (kind === "cli") { return deny("Running failproofai CLI commands is blocked"); } - - // Block package-manager uninstallation of failproofai - if (FAILPROOFAI_UNINSTALL_RE.test(cmd)) { + // Package-manager removal puts the manager in command position, not the + // binary, so the walk above never reaches it. + if (FAILPROOFAI_UNINSTALL_RE.test(cmd) || FAILPROOFAI_UNINSTALL_RE.test(unescaped)) { return deny("Uninstalling failproofai is blocked"); } - return allow(); } @@ -2156,16 +2182,6 @@ export const BUILTIN_POLICIES: BuiltinPolicyDefinition[] = [ }, } satisfies PolicyParamsSchema, }, - { - name: "block-self-pause", - displayTitle: "Tried to pause failproofai enforcement", - impact: "An agent that can pause enforcement can switch off every other policy.", - description: "Block agents from pausing failproofai enforcement", - fn: blockSelfPause, - match: { events: ["PreToolUse", "PermissionRequest"], toolNames: ["Bash"] }, - defaultEnabled: true, - category: "Dangerous Commands", - }, { name: "block-sudo", displayTitle: "Tried to run a command with sudo", @@ -2214,12 +2230,16 @@ export const BUILTIN_POLICIES: BuiltinPolicyDefinition[] = [ }, { name: "block-failproofai-commands", - displayTitle: "Tried to disable or modify failproofai itself", - impact: "Prevents the agent from turning off the policies that protect you.", - description: "Block failproofai CLI commands and uninstallation", + displayTitle: "Tried to disable, pause or modify failproofai itself", + impact: "An agent that can pause or remove enforcement can switch off every other policy.", + description: "Block failproofai CLI commands, self-pause and uninstallation", fn: blockFailproofaiCommands, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + // PermissionRequest is carried over from the merged-in `block-self-pause`. + // It is a real enforcement point on Copilot and Devin, and dropping it would + // have left this guard blind on both. + match: { events: ["PreToolUse", "PermissionRequest"], toolNames: ["Bash"] }, defaultEnabled: true, + alwaysOn: true, category: "Dangerous Commands", }, { @@ -2614,7 +2634,13 @@ export function registerBuiltinPolicies(enabledNames: string[]): void { // forms in the user's enabledPolicies config — canonicalize both sides. const enabledSet = new Set(enabledNames.map(normalizePolicyName)); for (const policy of BUILTIN_POLICIES) { - if (enabledSet.has(normalizePolicyName(policy.name))) { + // `alwaysOn` deliberately bypasses the enabled set, and the caller's three + // ways of producing an empty one with it: a policy the user never enabled, + // an active session pause (`handler.ts` passes `[]`), and a config file that + // failed to parse (`hooks-config.ts` soft-fails to `{enabledPolicies: []}`). + // A guard against the agent disabling failproofai that any of those can + // switch off is not a guard. + if (policy.alwaysOn || enabledSet.has(normalizePolicyName(policy.name))) { registerPolicy(policy.name, policy.description, policy.fn, policy.match); } } diff --git a/src/hooks/manager.ts b/src/hooks/manager.ts index 2c9dd4cef..b60517601 100644 --- a/src/hooks/manager.ts +++ b/src/hooks/manager.ts @@ -74,6 +74,28 @@ function validatePolicyNames(names: string[]): void { } } +/** + * Refuse to "disable" a policy that will register anyway. + * + * Removing an `alwaysOn` name from `enabledPolicies` succeeds at the file level + * and changes nothing at the enforcement level, so without this the CLI reports + * a policy disabled while it keeps denying — the operator's mental model and the + * machine's behaviour diverge silently, which is the failure this policy exists + * to prevent in the first place. + */ +function rejectAlwaysOnPolicies(names: string[]): void { + const alwaysOn = new Set(BUILTIN_POLICIES.filter((p) => p.alwaysOn).map((p) => p.name)); + const refused = names.filter((n) => alwaysOn.has(n)); + if (refused.length > 0) { + throw new CliError( + `Cannot disable: ${refused.join(", ")}\n` + + `This policy stops an agent from switching off failproofai itself, so it ` + + `is always on and ships with the package. A guard the agent can disable ` + + `by the means it is meant to prevent is not a guard.` + ); + } +} + /** Return only scopes whose settings paths are unique (first wins). */ function deduplicateScopes(scopes: readonly HookScope[], cwd?: string): HookScope[] { const seen = new Set(); @@ -435,6 +457,7 @@ export async function removeHooks(policyNames?: string[], scope: HookScope | "al // Remove specific policies from config (keep hooks installed) if (policyNames && policyNames.length > 0 && !(policyNames.length === 1 && policyNames[0] === "all")) { validatePolicyNames(policyNames); + rejectAlwaysOnPolicies(policyNames); const config = readScopedHooksConfig(configScope, cwd); const removeSet = new Set(policyNames); const remaining = config.enabledPolicies.filter((p) => !removeSet.has(p)); diff --git a/src/hooks/policy-presets.ts b/src/hooks/policy-presets.ts index 6a6a86abf..dbfee380a 100644 --- a/src/hooks/policy-presets.ts +++ b/src/hooks/policy-presets.ts @@ -106,8 +106,10 @@ export const RECOMMENDED_POLICIES: readonly string[] = [ "protect-env-vars", "block-env-files", "block-secrets-write", - // The agent cannot disable its own guardrails. - "block-self-pause", + // The agent cannot disable its own guardrails. `block-failproofai-commands` + // is `alwaysOn` and registers with or without this entry; it is listed anyway + // so Recommended reads as the complete set rather than quietly omitting the + // most important line in it. "block-failproofai-commands", // Commands that are unrecoverable when they are wrong. "block-sudo", diff --git a/src/hooks/policy-types.ts b/src/hooks/policy-types.ts index 6f78b47a6..d91cc6b0c 100644 --- a/src/hooks/policy-types.ts +++ b/src/hooks/policy-types.ts @@ -51,6 +51,17 @@ export interface BuiltinPolicyDefinition { fn: PolicyFunction; match: PolicyMatcher; defaultEnabled: boolean; + /** + * Registered on every evaluation regardless of the user's `enabledPolicies`, + * an active session pause, or an unreadable config — and never eligible to + * move out of the package into a fetched pack. Reserved for the guard that + * stops an agent from disabling failproofai itself; a guard the agent can + * switch off by the same means it is meant to prevent is decorative. + * + * `defaultEnabled` stays `true` alongside it so the policy still appears + * enabled everywhere the catalog is listed rather than looking switched off. + */ + alwaysOn?: boolean; category: string; beta?: boolean; params?: PolicyParamsSchema; From 9f1394a76014276d034dc32822d6a86cc57b34ae Mon Sep 17 00:00:00 2001 From: chhhee10 Date: Thu, 20 Aug 2026 19:45:34 +0530 Subject: [PATCH 02/17] Separate what a policy IS from what it does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `policy-catalog.ts` now holds the metadata — name, description, category, `match`, `defaultEnabled`, `params` — as pure literal data, and `builtin-policies.ts` keeps the 39 implementations and joins them back on. `BUILTIN_POLICIES` keeps its exact shape, fields and order, so none of its nine source consumers change. This is what lets a machine list, search and render the catalog offline once the executable half moves to a fetched pack. Two constraints made the refactor narrower than it looks, and both were measured rather than assumed. `audit/cache.ts` hashes `fn.toString()` for all 39 policies into the audit cache's `engineVersion`, and `bun build` renames colliding top-level identifiers by module EMISSION ORDER — those renamed names appear inside policy bodies in the shipped bundle (`cwdWithSep2`, `execSync2`, `resolved3`). So inserting a module into the graph could have changed the emitted text, invalidated every user's audit cache and forced a ~104-second cold rescan on upgrade. Built before and after and compared: `engineVersion` is unchanged at `c1cea4ddf3030af4`. `SECRET_PATTERNS` stays here rather than being reclassified as catalog data. It is assembled from the very RegExps the five `sanitize-*` policies test against and is imported by the audit redactor, so moving it would have forced a catalog→implementation value edge and put an import cycle on the hook path. `policy-catalog.test.ts` pins the join against the failures that are otherwise silent, each verified to fail when the join is mutated: a wrapper collapsing 39 distinct `fn.toString()` hashes into one and freezing the cache key; a sort or regroup changing which policy name is attributed on a deny; a spread default-filling `beta`; a dropped row shrinking the catalog invisibly to `manager.ts` and `install-prompt.ts`, neither of which reads `.fn`. The bijection check throws at module load rather than warning, because a name with no implementation yields `fn: undefined`, whose `TypeError` `policy-evaluator.ts` swallows — the hook would allow, exit 0, and still report the policy as having run. --- CHANGELOG.md | 2 + __tests__/hooks/policy-catalog.test.ts | 222 +++++++++ src/hooks/builtin-policies.ts | 642 ++++--------------------- src/hooks/policy-catalog.ts | 535 +++++++++++++++++++++ src/hooks/policy-types.ts | 7 + 5 files changed, 866 insertions(+), 542 deletions(-) create mode 100644 __tests__/hooks/policy-catalog.test.ts create mode 100644 src/hooks/policy-catalog.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c2cc1d17c..b6e031651 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features +- Split the builtin policy catalog from its implementations, the first step of moving policies out of the npm package. `policy-catalog.ts` now holds the metadata every consumer actually reads — name, description, category, `match`, `defaultEnabled`, `params` — as pure literal data with no functions in it, and `builtin-policies.ts` keeps the 39 implementations and joins them back on, so `BUILTIN_POLICIES` has exactly the shape, fields and ORDER it always had and not one of its nine source consumers changes. That is what will let a machine list, search and render the catalog offline once the executable half moves to a fetched pack. Two things made the refactor narrower than it looks. `audit/cache.ts` hashes `fn.toString()` for all 39 policies into the audit cache's `engineVersion`, and `bun build` renames colliding top-level identifiers by module EMISSION ORDER — renamed names appear inside policy bodies in the shipped bundle (`cwdWithSep2`, `execSync2`, `resolved3`) — so inserting a module into the graph could have changed the emitted text, invalidated every user's audit cache and forced a ~104-second cold rescan on upgrade. Measured before and after: `engineVersion` is unchanged (`c1cea4ddf3030af4`), so nobody's cache moves. And `SECRET_PATTERNS` stays in `builtin-policies.ts` rather than being reclassified as catalog data: it is assembled from the very RegExps the five `sanitize-*` policies test against and is imported by the audit redactor, so moving it would have forced a catalog→implementation value edge and put an import cycle on the hook critical path. `policy-catalog.test.ts` pins the join against the failures that are otherwise silent — a wrapper collapsing 39 distinct `fn.toString()` hashes into one and freezing the cache key, a sort or regroup changing which policy name gets attributed on a deny, a spread default-filling `beta`, a dropped row shrinking the catalog invisibly to `manager.ts` and `install-prompt.ts` (neither reads `.fn`) — plus a JSON round-trip asserting the catalog is serializable and a source-level guard that it never value-imports back. A bijection check throws at module load rather than warning, because a name with no implementation yields `fn: undefined`, whose `TypeError` `policy-evaluator.ts` swallows: the hook would allow, exit 0, and still report the policy in `matchedPolicies`. (#PR) + - Merge `block-self-pause` into `block-failproofai-commands` and make the result the one policy that cannot be switched off. The two were halves of one guard and disagreed with each other. `block-self-pause` had the hardened matcher — segments split on shell operators, runner prefixes and their flags walked off, the binary resolved by basename, and the shell-unescaped form re-checked — but only ever looked for `config --pause`. `block-failproofai-commands` had the whole surface (any CLI invocation, plus package-manager uninstall) on `/(?:^|;|&&|\|\||\|)\s*failproofai(?:\s|$)/`, which a single prefix defeated: `sudo failproofai config --pause`, `npx failproofai policies --uninstall`, `env X=1 failproofai …`, `/usr/local/bin/failproofai …` and `timeout 30 failproofai …` were all ALLOWED by a `defaultEnabled` self-protection policy. The merged policy is the hardened matcher over the broad surface, and it keeps `PermissionRequest` from the merged-in half — a real enforcement point on Copilot and Devin that the survivor did not subscribe to. Where the two contradicted each other the merge keeps what machines actually did: `block-self-pause` deliberately allowed `config --resume`, `config --status` and `policies --install`, but both policies were default-on and the sibling denied all three first, so that allow never ran anywhere. It is now `alwaysOn`, a new flag `registerBuiltinPolicies` honours ahead of the enabled set, which closes the three ways the old pair could be switched off silently — a name absent from `enabledPolicies`, an active session pause (`handler.ts` passes `[]`), and a config file that fails to parse (`hooks-config.ts` soft-fails to `{enabledPolicies: []}`, so corrupting one file disabled every policy including these). `policies --disable block-failproofai-commands` now refuses with a reason instead of editing the config and reporting a success that changes nothing. (#PR) ### Docs diff --git a/__tests__/hooks/policy-catalog.test.ts b/__tests__/hooks/policy-catalog.test.ts new file mode 100644 index 000000000..089e34fa5 --- /dev/null +++ b/__tests__/hooks/policy-catalog.test.ts @@ -0,0 +1,222 @@ +// @vitest-environment node +/** + * Invariants for the catalog/implementation split. + * + * `builtin-policies.ts` no longer holds the policy metadata — `policy-catalog.ts` + * does, and the exported `BUILTIN_POLICIES` is a join of the two. Every + * assertion below guards a failure of that join that is SILENT: the suite that + * existed before this split passed against a join that dropped rows, reordered + * them, filled defaults, or wrapped every implementation in a closure. + * + * These are also the tripwires the pack migration leans on. When implementations + * move out of the package entirely, "the catalog says 39 and 39 ran" stops being + * a tautology and becomes the thing worth checking. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { BUILTIN_POLICIES, SECRET_PATTERNS, registerBuiltinPolicies } from "../../src/hooks/builtin-policies"; +import { POLICY_CATALOG } from "../../src/hooks/policy-catalog"; +import { clearPolicies, getAllPolicies } from "../../src/hooks/policy-registry"; + +const SRC = (p: string) => resolve(__dirname, "../../src", p); + +/** The exact catalog order. Order is not cosmetic: evaluation short-circuits on + * the first deny, so this decides which policy name reaches the agent, the + * activity log, PostHog and the audit report. Nothing else pins it — + * policy-presets.test.ts compares via a Set. */ +const EXPECTED_ORDER = [ + "sanitize-jwt", "sanitize-api-keys", "sanitize-connection-strings", + "sanitize-private-key-content", "sanitize-bearer-tokens", "protect-env-vars", + "block-env-files", "block-read-outside-cwd", "block-sudo", "block-curl-pipe-sh", + "block-rm-rf", "block-failproofai-commands", "block-kubectl", "block-terraform", + "block-aws-cli", "block-gcloud", "block-az-cli", "block-helm", "block-gh-pipeline", + "block-secrets-write", "block-push-master", "block-force-push", "block-work-on-main", + "warn-git-amend", "warn-git-stash-drop", "warn-all-files-staged", + "warn-destructive-sql", "warn-schema-alteration", "warn-package-publish", + "warn-global-package-install", "prefer-package-manager", "warn-large-file-write", + "warn-background-process", "warn-repeated-tool-calls", "require-commit-before-stop", + "require-push-before-stop", "require-pr-before-stop", + "require-no-conflicts-before-stop", "require-ci-green-before-stop", +]; + +describe("policy catalog / implementation split", () => { + describe("the join", () => { + it("keeps catalog and joined view the same length and order", () => { + expect(POLICY_CATALOG).toHaveLength(39); + expect(BUILTIN_POLICIES).toHaveLength(39); + expect(BUILTIN_POLICIES.map((p) => p.name)).toEqual(POLICY_CATALOG.map((e) => e.name)); + }); + + it("pins the exact positional order", () => { + // A join that iterated the implementation map, sorted for determinism, or + // grouped by category would reorder this and change first-deny attribution. + expect(BUILTIN_POLICIES.map((p) => p.name)).toEqual(EXPECTED_ORDER); + }); + + it("gives every catalog entry a real implementation", () => { + const holes = BUILTIN_POLICIES.filter((p) => typeof p.fn !== "function").map((p) => p.name); + expect(holes).toEqual([]); + }); + + it("assigns 39 DISTINCT implementations, never a shared wrapper", () => { + // The wrapper-collapse guard. `fn: (ctx) => IMPLS[name](ctx)` yields 39 + // distinct function OBJECTS with near-identical source text, which freezes + // audit/cache.ts's engineVersion — it then stops changing when policy logic + // changes and stale audit results are served for the full 30-day TTL with + // no symptom anywhere. + expect(new Set(BUILTIN_POLICIES.map((p) => p.fn.toString())).size).toBe(39); + }); + + it("has unique names", () => { + // resolveEverything() does not dedupe, findBuiltin takes the FIRST match and + // registerPolicy takes the LAST — a duplicate silently registers one policy + // fewer while the audit title comes from the other copy. + expect(new Set(BUILTIN_POLICIES.map((p) => p.name)).size).toBe(39); + }); + + it("adds no fields the catalog did not have", () => { + for (const entry of POLICY_CATALOG) { + const joined = BUILTIN_POLICIES.find((p) => p.name === entry.name)!; + expect(Object.keys(joined).sort()).toEqual([...Object.keys(entry), "fn"].sort()); + } + }); + }); + + describe("absent optionals stay absent", () => { + // Asserted with `in`, not truthiness: a join spreading defaults + // (`{beta: false, ...entry}`) would pass a truthiness check and still break + // builtin-policies.test.ts's `expect(p.beta).toBeUndefined()`. + it("sets beta on zero entries", () => { + expect(BUILTIN_POLICIES.filter((p) => "beta" in p).map((p) => p.name)).toEqual([]); + }); + + it("sets alwaysOn on exactly the self-protection policy", () => { + expect(BUILTIN_POLICIES.filter((p) => "alwaysOn" in p).map((p) => p.name)).toEqual([ + "block-failproofai-commands", + ]); + }); + + it("sets params on exactly the entries that take them", () => { + expect(BUILTIN_POLICIES.filter((p) => "params" in p).map((p) => p.name)).toEqual([ + "sanitize-api-keys", "block-read-outside-cwd", "block-sudo", "block-rm-rf", + "block-kubectl", "block-terraform", "block-aws-cli", "block-gcloud", + "block-az-cli", "block-helm", "block-gh-pipeline", "block-secrets-write", + "block-push-master", "block-work-on-main", "prefer-package-manager", + "warn-large-file-write", "require-push-before-stop", "require-pr-before-stop", + "require-no-conflicts-before-stop", + ]); + }); + }); + + describe("counts and ordering the UI depends on", () => { + it("has 11 default-enabled policies", () => { + expect(BUILTIN_POLICIES.filter((p) => p.defaultEnabled)).toHaveLength(11); + }); + + it("pins the category first-appearance order", () => { + // This is the section order in the TUI picker (install-prompt.ts) and in the + // dashboard (hooks-client.tsx). Neither has a test of its own, so a reshuffle + // ships green. + const seen: string[] = []; + for (const p of BUILTIN_POLICIES) if (!seen.includes(p.category)) seen.push(p.category); + expect(seen).toEqual([ + "Sanitize", "Environment", "Dangerous Commands", "Infra Commands", "Git", + "Database", "Packages & System", "AI Behavior", "Workflow", + ]); + }); + + it("registers in catalog order", () => { + clearPolicies(); + registerBuiltinPolicies(EXPECTED_ORDER); + expect(getAllPolicies().map((r) => r.name)).toEqual( + EXPECTED_ORDER.map((n) => `failproofai/${n}`), + ); + clearPolicies(); + }); + + it("registers ONLY the alwaysOn guard for an empty enabled set", () => { + clearPolicies(); + registerBuiltinPolicies([]); + expect(getAllPolicies().map((r) => r.name)).toEqual([ + "failproofai/block-failproofai-commands", + ]); + clearPolicies(); + }); + }); + + describe("the catalog is pure data", () => { + it("survives a JSON round-trip unchanged", () => { + // The property that lets the catalog become a shipped manifest rather than + // code. A RegExp or function smuggled into an entry survives every other + // test here and fails only once the catalog is serialized. + expect(JSON.parse(JSON.stringify(POLICY_CATALOG))).toEqual(POLICY_CATALOG); + }); + + it("carries no functions on any entry", () => { + const offenders: string[] = []; + const walk = (v: unknown, path: string) => { + if (typeof v === "function") offenders.push(path); + else if (v && typeof v === "object") { + for (const [k, sub] of Object.entries(v)) walk(sub, `${path}.${k}`); + } + }; + POLICY_CATALOG.forEach((e, i) => walk(e, `[${i}:${e.name}]`)); + expect(offenders).toEqual([]); + }); + + it("never value-imports from builtin-policies (cycle guard)", () => { + // policy-evaluator.ts builds POLICY_PARAMS_MAP from BUILTIN_POLICIES at + // MODULE SCOPE. A cycle here is a ReferenceError under ESM and + // `.filter of undefined` under the CJS bundle — thrown at import time, on + // the hook critical path. + const src = readFileSync(SRC("hooks/policy-catalog.ts"), "utf8"); + const valueImports = src + .split("\n") + .filter((l) => /^import\s/.test(l) && !/^import\s+type\s/.test(l)); + expect(valueImports.filter((l) => l.includes("builtin-policies"))).toEqual([]); + }); + }); + + describe("shared pattern list", () => { + it("still exports SECRET_PATTERNS from builtin-policies, intact", () => { + // Neither catalog metadata nor an implementation: the five sanitize-* fns + // test against it AND audit/redact-example.ts imports it from this path. + // Its hand-written most-specific-first ORDER is load-bearing — a + // Bearer-wrapped JWT reports as "JWT" today and as "bearer token" if two + // entries swap. + expect(SECRET_PATTERNS).toHaveLength(13); + for (const [re] of SECRET_PATTERNS) expect(re).toBeInstanceOf(RegExp); + }); + }); + + describe("hand-copied name tables still resolve", () => { + // The #337 drift class: tables authored against the catalog by hand, with + // nothing asserting they still match it. A rename makes the audit card fall + // back to generic copy AND flips `alreadyEnabled` to false — telling users to + // enable a policy they already have. + const findings = readFileSync(SRC("audit/findings.ts"), "utf8"); + const names = new Set(BUILTIN_POLICIES.map((p) => p.name)); + + const section = (start: string): string => { + const i = findings.indexOf(start); + expect(i, `${start} not found in findings.ts`).toBeGreaterThan(-1); + const j = findings.indexOf("\n};", i); + return findings.slice(i, j); + }; + + it("DETECTOR_TO_POLICY names a live policy in every primary/also", () => { + const block = section("const DETECTOR_TO_POLICY"); + const refs = [...block.matchAll(/(?:primary|also):\s*"([^"]+)"/g)].map((m) => m[1]); + expect(refs.length).toBeGreaterThan(0); + expect(refs.filter((r) => !names.has(r))).toEqual([]); + }); + + it("POLICY_META is keyed entirely by live policy names", () => { + const block = section("const POLICY_META"); + const keys = [...block.matchAll(/(?:^|\n)\s{2}"([^"]+)":\s*\{/g)].map((m) => m[1]); + expect(keys.length).toBeGreaterThan(0); + expect(keys.filter((k) => !names.has(k))).toEqual([]); + }); + }); +}); diff --git a/src/hooks/builtin-policies.ts b/src/hooks/builtin-policies.ts index 07bd1db19..5e9347805 100644 --- a/src/hooks/builtin-policies.ts +++ b/src/hooks/builtin-policies.ts @@ -6,7 +6,13 @@ import { statSync } from "node:fs"; import { readFile, writeFile } from "node:fs/promises"; import { execSync, execFileSync } from "node:child_process"; import { homedir } from "node:os"; -import type { BuiltinPolicyDefinition, PolicyContext, PolicyResult, PolicyParamsSchema } from "./policy-types"; +import type { + BuiltinPolicyDefinition, + PolicyContext, + PolicyFunction, + PolicyResult, +} from "./policy-types"; +import { POLICY_CATALOG } from "./policy-catalog"; import { allow, deny, instruct } from "./policy-helpers"; import { normalizePolicyName, registerPolicy } from "./policy-registry"; import { hookLogWarn } from "./hook-logger"; @@ -2087,547 +2093,99 @@ function requireCiGreenBeforeStop(ctx: PolicyContext): PolicyResult { // -- Registry -- -export const BUILTIN_POLICIES: BuiltinPolicyDefinition[] = [ - { - name: "sanitize-jwt", - description: "Stop Claude from reading JWTs in tool responses", - displayTitle: "Redacted JWT tokens from tool output", - impact: "Stops the agent from echoing auth tokens it saw in command output.", - fn: sanitizeJwt, - match: { events: ["PostToolUse"] }, - defaultEnabled: true, - category: "Sanitize", - }, - { - name: "sanitize-api-keys", - description: "Stop Claude from reading API keys (OpenAI, Anthropic, GitHub, AWS, Stripe, Google) in tool responses", - displayTitle: "Redacted API keys from tool output", - impact: "Catches OpenAI / Anthropic / GitHub / AWS / Stripe / Google keys before the model sees them.", - fn: sanitizeApiKeys, - match: { events: ["PostToolUse"] }, - defaultEnabled: true, - category: "Sanitize", - params: { - additionalPatterns: { - type: "pattern[]", - description: "Additional API key patterns to scrub, each with { regex, label }", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "sanitize-connection-strings", - description: "Stop Claude from reading database connection strings with embedded credentials in tool responses", - displayTitle: "Redacted database connection strings from tool output", - impact: "Strips embedded DB credentials before they reach the model context.", - fn: sanitizeConnectionStrings, - match: { events: ["PostToolUse"] }, - defaultEnabled: true, - category: "Sanitize", - }, - { - name: "sanitize-private-key-content", - description: "Stop Claude from reading PEM private key content in tool responses", - displayTitle: "Redacted PEM private keys from tool output", - impact: "Prevents private key bodies from being echoed into chat context.", - fn: sanitizePrivateKeyContent, - match: { events: ["PostToolUse"] }, - defaultEnabled: true, - category: "Sanitize", - }, - { - name: "sanitize-bearer-tokens", - displayTitle: "Redacted bearer tokens from tool output", - impact: "Strips Authorization: Bearer values before they hit the model.", - description: "Stop Claude from reading Authorization Bearer tokens in tool responses", - fn: sanitizeBearerTokens, - match: { events: ["PostToolUse"] }, - defaultEnabled: true, - category: "Sanitize", - }, - { - name: "protect-env-vars", - displayTitle: "Tried to dump environment variables to chat", - impact: "Env vars often contain secrets; blocking `env` / `printenv` keeps them out of the model context.", - description: "Prevent commands that read environment variables", - fn: protectEnvVars, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: true, - category: "Environment", - }, - { - name: "block-env-files", - displayTitle: "Tried to read or write a .env file", - impact: "`.env` files routinely contain API keys and DB credentials.", - description: "Block reading/writing .env files", - fn: blockEnvFiles, - match: { events: ["PreToolUse"] }, - defaultEnabled: true, - category: "Environment", - }, - { - name: "block-read-outside-cwd", - displayTitle: "Tried to read files outside your project directory", - impact: "Stops the agent from peeking at neighboring repos or your home directory.", - description: "Block file reads outside the session working directory", - fn: blockReadOutsideCwd, - match: { events: ["PreToolUse"], toolNames: ["Read", "Glob", "Grep", "Bash"] }, - defaultEnabled: false, - category: "Environment", - params: { - allowPaths: { - type: "string[]", - description: "Absolute paths outside cwd that are allowed to be read", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-sudo", - displayTitle: "Tried to run a command with sudo", - impact: "Sudo gives the agent root — blocked unless explicitly allow-listed.", - description: "Block sudo commands", - fn: blockSudo, - // PermissionRequest is Codex's escalation-approval event; fire the same - // sudo guard there so Codex sandbox bypasses are blocked too. - match: { events: ["PreToolUse", "PermissionRequest"], toolNames: ["Bash"] }, - defaultEnabled: true, - category: "Dangerous Commands", - params: { - allowPatterns: { - type: "string[]", - description: "Sudo command patterns to allow, matched token-by-token (e.g. 'sudo systemctl status')", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-curl-pipe-sh", - displayTitle: "Tried to pipe a downloaded script straight to a shell", - impact: "`curl ... | sh` runs unverified remote code on your machine.", - description: "Block piping downloads to shell", - fn: blockCurlPipeSh, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: true, - category: "Dangerous Commands", - }, - { - name: "block-rm-rf", - displayTitle: "Tried to recursively delete a system path", - impact: "Catches catastrophic `rm -rf /` and Windows equivalents.", - description: "Prevent catastrophic deletions", - fn: blockRmRf, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Dangerous Commands", - params: { - allowPaths: { - type: "string[]", - description: "Paths that are allowed to be recursively deleted", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-failproofai-commands", - displayTitle: "Tried to disable, pause or modify failproofai itself", - impact: "An agent that can pause or remove enforcement can switch off every other policy.", - description: "Block failproofai CLI commands, self-pause and uninstallation", - fn: blockFailproofaiCommands, - // PermissionRequest is carried over from the merged-in `block-self-pause`. - // It is a real enforcement point on Copilot and Devin, and dropping it would - // have left this guard blind on both. - match: { events: ["PreToolUse", "PermissionRequest"], toolNames: ["Bash"] }, - defaultEnabled: true, - alwaysOn: true, - category: "Dangerous Commands", - }, - { - name: "block-kubectl", - displayTitle: "Tried to run a Kubernetes command", - impact: "kubectl can change live cluster state — gated unless allow-listed.", - description: "Block kubectl commands (Kubernetes cluster mutations)", - fn: blockKubectl, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Infra Commands", - params: { - allowPatterns: { - type: "string[]", - description: "kubectl command patterns to allow, matched token-by-token (e.g. 'kubectl get *', 'kubectl describe *')", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-terraform", - displayTitle: "Tried to run a Terraform/OpenTofu command", - impact: "Terraform mutates real infrastructure — gated unless allow-listed.", - description: "Block terraform and tofu (OpenTofu) commands", - fn: blockTerraform, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Infra Commands", - params: { - allowPatterns: { - type: "string[]", - description: "terraform/tofu command patterns to allow (e.g. 'terraform plan', 'terraform validate')", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-aws-cli", - displayTitle: "Tried to run an AWS CLI command", - impact: "AWS CLI can spend money or break prod — gated.", - description: "Block aws CLI commands", - fn: blockAwsCli, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Infra Commands", - params: { - allowPatterns: { - type: "string[]", - description: "aws CLI command patterns to allow (e.g. 'aws s3 ls *', 'aws sts get-caller-identity')", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-gcloud", - displayTitle: "Tried to run a Google Cloud command", - impact: "gcloud can spend money or break prod — gated.", - description: "Block gcloud (Google Cloud) CLI commands", - fn: blockGcloud, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Infra Commands", - params: { - allowPatterns: { - type: "string[]", - description: "gcloud command patterns to allow (e.g. 'gcloud auth list', 'gcloud config list')", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-az-cli", - displayTitle: "Tried to run an Azure CLI command", - impact: "az can spend money or break prod — gated.", - description: "Block az (Azure) CLI commands", - fn: blockAzCli, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Infra Commands", - params: { - allowPatterns: { - type: "string[]", - description: "az CLI command patterns to allow (e.g. 'az account show', 'az group list')", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-helm", - displayTitle: "Tried to run a Helm command", - impact: "Helm releases mutate cluster state — gated.", - description: "Block helm commands", - fn: blockHelm, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Infra Commands", - params: { - allowPatterns: { - type: "string[]", - description: "helm command patterns to allow (e.g. 'helm list', 'helm status *')", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-gh-pipeline", - displayTitle: "Tried to run a privileged GitHub CLI pipeline command", - impact: "Catches `gh workflow run`, `gh pr merge`, `gh secret set`, etc.", - description: "Block gh CLI pipeline-trigger subcommands (workflow run, run rerun/cancel, pr merge, release create/delete, cache delete, secret set/delete)", - fn: blockGhPipeline, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Infra Commands", - params: { - allowPatterns: { - type: "string[]", - description: "gh pipeline command patterns to allow (e.g. specific scripted invocations); read-only gh subcommands like 'gh pr view' and 'gh run list' are not matched by this policy", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-secrets-write", - displayTitle: "Tried to write a secret-key file", - impact: "Stops the agent from creating `.pem`, `id_rsa`, `credentials.json`, etc.", - description: "Block writing secret key files", - fn: blockSecretsWrite, - match: { events: ["PreToolUse"], toolNames: ["Write"] }, - defaultEnabled: false, - category: "Dangerous Commands", - params: { - additionalPatterns: { - type: "string[]", - description: "Additional filename patterns (substrings) to block", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-push-master", - displayTitle: "Tried to push directly to main/master", - impact: "Direct pushes to a protected branch bypass review.", - description: "Block pushing to main/master", - fn: blockPushMaster, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: true, - category: "Git", - params: { - protectedBranches: { - type: "string[]", - description: "Branch names to protect from direct pushes", - default: ["main", "master"], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "block-force-push", - displayTitle: "Tried to force-push", - impact: "Force-pushes rewrite history and can clobber teammates' work.", - description: "Prevent force-pushing to any branch", - fn: blockForcePush, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Git", - }, - { - name: "block-work-on-main", - displayTitle: "Tried to commit or merge on main/master", - impact: "Work should land via PR — direct commits skip review.", - description: "Block git commits and merges on main/master branch", - fn: blockWorkOnMain, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Git", - params: { - protectedBranches: { - type: "string[]", - description: "Branch names where commits/merges are blocked", - default: ["main", "master"], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "warn-git-amend", - displayTitle: "Used git commit --amend", - impact: "Amending after a push rewrites history that others may have pulled.", - description: "Warns before amending git commits, which rewrites history", - fn: warnGitAmend, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Git", - }, - { - name: "warn-git-stash-drop", - displayTitle: "Tried to drop or clear git stash", - impact: "Stash deletions are permanent and silent.", - description: "Warns before permanently deleting stashed changes", - fn: warnGitStashDrop, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Git", - }, - { - name: "warn-all-files-staged", - displayTitle: "Staged all files with git add -A / .", - impact: "Wide stages routinely catch generated files or secrets you didn't intend to commit.", - description: "Warns before staging all working tree files with git add -A / . / --all", - fn: warnAllFilesStaged, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Git", - }, - { - name: "warn-destructive-sql", - displayTitle: "Ran destructive SQL (DROP / TRUNCATE / DELETE without WHERE)", - impact: "Easy way to wipe a table by accident.", - description: "Warn before executing destructive SQL (DROP/TRUNCATE/DELETE without WHERE) via database clients", - fn: warnDestructiveSql, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Database", - }, - { - name: "warn-schema-alteration", - displayTitle: "Altered a database schema column", - impact: "ALTER TABLE operations can lock tables and break readers.", - description: "Warns before SQL schema changes (ALTER TABLE with column or rename operations)", - fn: warnSchemaAlteration, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Database", - }, - { - name: "warn-package-publish", - displayTitle: "Tried to publish a package", - impact: "Publishes are irreversible — `npm publish` / `cargo publish` shouldn't happen without intent.", - description: "Warn before publishing packages to public registries (npm, PyPI, crates.io, RubyGems, etc.)", - fn: warnPackagePublish, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Packages & System", - }, - { - name: "warn-global-package-install", - displayTitle: "Installed a package globally", - impact: "`npm i -g`, `cargo install`, `pip --user` pollute your machine outside the project.", - description: "Warns before installing packages globally (npm -g, cargo install, etc.)", - fn: warnGlobalPackageInstall, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Packages & System", - }, - { - name: "prefer-package-manager", - displayTitle: "Used a non-preferred package manager", - impact: "Mixing package managers creates lockfile churn for your team.", - description: "Blocks non-preferred package managers and tells Claude to use an allowed one (e.g., uv instead of pip)", - fn: preferPackageManager, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Packages & System", - params: { - allowed: { - type: "string[]", - description: "Allowed package manager names (e.g. ['uv', 'bun']). Any detected manager not in this list is blocked.", - default: [], - }, - blocked: { - type: "string[]", - description: "Additional manager names to block beyond the built-in list (e.g. ['pdm', 'pipx']).", - default: [], - }, - } satisfies PolicyParamsSchema, - }, - { - name: "warn-large-file-write", - displayTitle: "Wrote a file larger than the configured threshold", - impact: "Catches accidentally large file writes (logs, binaries, model dumps).", - description: "Warn before writing files larger than 1MB (configurable via thresholdKb param)", - fn: warnLargeFileWrite, - match: { events: ["PreToolUse"], toolNames: ["Write"] }, - defaultEnabled: false, - category: "Packages & System", - params: { - thresholdKb: { - type: "number", - description: "File size threshold in KB above which a warning is issued", - default: 1024, - }, - } satisfies PolicyParamsSchema, - }, - { - name: "warn-background-process", - displayTitle: "Started a long-lived background process", - impact: "Catches `nohup` / `&` / `screen` / `tmux` / `disown` patterns that the agent often forgets to clean up.", - description: "Warns before starting detached or background processes", - fn: warnBackgroundProcess, - match: { events: ["PreToolUse"], toolNames: ["Bash"] }, - defaultEnabled: false, - category: "Packages & System", - }, - { - name: "warn-repeated-tool-calls", - displayTitle: "Called the same tool 3+ times with identical arguments", - impact: "Usually a sign of a stuck loop burning tokens.", - description: "Warn when the same tool is called 3+ times with identical parameters", - fn: warnRepeatedToolCalls, - match: { events: ["PreToolUse"] }, - defaultEnabled: false, - category: "AI Behavior", - }, - { - name: "require-commit-before-stop", - displayTitle: "Stopped with uncommitted changes", - impact: "Work not in a commit is invisible to teammates and easy to lose.", - description: "Require all changes to be committed before Claude stops", - fn: requireCommitBeforeStop, - match: { events: ["Stop"] }, - defaultEnabled: false, - category: "Workflow", - }, - { - name: "require-push-before-stop", - displayTitle: "Stopped with unpushed commits", - impact: "Local-only commits won't trigger CI or be reviewable.", - description: "Require all commits to be pushed to remote before Claude stops", - fn: requirePushBeforeStop, - match: { events: ["Stop"] }, - defaultEnabled: false, - category: "Workflow", - params: { - remote: { - type: "string", - description: "Remote name to push to (default: origin)", - default: "origin", - }, - baseBranch: { - type: "string", - description: "Base branch to compare against (default: main)", - default: "main", - }, - } satisfies PolicyParamsSchema, - }, - { - name: "require-pr-before-stop", - displayTitle: "Stopped without a PR for the branch", - impact: "Branches without PRs don't get reviewed.", - description: "Require a pull request to exist for the current branch before Claude stops", - fn: requirePrBeforeStop, - match: { events: ["Stop"] }, - defaultEnabled: false, - category: "Workflow", - params: { - baseBranch: { - type: "string", - description: "Base branch to compare against (default: main)", - default: "main", - }, - } satisfies PolicyParamsSchema, - }, - { - name: "require-no-conflicts-before-stop", - displayTitle: "Stopped with a branch that conflicts with main", - impact: "Conflicting branches can't merge — surface them early.", - description: "Require the current branch to merge cleanly with the base branch before Claude stops", - fn: requireNoConflictsBeforeStop, - match: { events: ["Stop"] }, - defaultEnabled: false, - category: "Workflow", - params: { - baseBranch: { - type: "string", - description: "Base branch to check for conflicts against (default: main)", - default: "main", - }, - } satisfies PolicyParamsSchema, - }, - { - name: "require-ci-green-before-stop", - displayTitle: "Stopped with failing CI", - impact: "Failing CI blocks deploy.", - description: "Require CI checks to pass on the current HEAD commit before Claude stops (ignores stale runs on prior commits)", - fn: requireCiGreenBeforeStop, - match: { events: ["Stop"] }, - defaultEnabled: false, - category: "Workflow", - }, -]; +/** + * Name → implementation. The other half of {@link POLICY_CATALOG}. + * + * Each value is the identical hoisted function object, never a wrapper. Two + * things depend on that and neither fails loudly: `audit/cache.ts` hashes + * `fn.toString()` into the audit cache's `engineVersion`, so wrapping every + * entry would collapse 39 distinct hashes into one and freeze the key — stale + * audit results would then be served for the full 30-day TTL with no symptom; + * and `gitBranchCache` is module-scoped, so a per-call factory would silently + * reset it on every hook event. + */ +const POLICY_IMPLEMENTATIONS: Record = { + "sanitize-jwt": sanitizeJwt, + "sanitize-api-keys": sanitizeApiKeys, + "sanitize-connection-strings": sanitizeConnectionStrings, + "sanitize-private-key-content": sanitizePrivateKeyContent, + "sanitize-bearer-tokens": sanitizeBearerTokens, + "protect-env-vars": protectEnvVars, + "block-env-files": blockEnvFiles, + "block-read-outside-cwd": blockReadOutsideCwd, + "block-sudo": blockSudo, + "block-curl-pipe-sh": blockCurlPipeSh, + "block-rm-rf": blockRmRf, + "block-failproofai-commands": blockFailproofaiCommands, + "block-kubectl": blockKubectl, + "block-terraform": blockTerraform, + "block-aws-cli": blockAwsCli, + "block-gcloud": blockGcloud, + "block-az-cli": blockAzCli, + "block-helm": blockHelm, + "block-gh-pipeline": blockGhPipeline, + "block-secrets-write": blockSecretsWrite, + "block-push-master": blockPushMaster, + "block-force-push": blockForcePush, + "block-work-on-main": blockWorkOnMain, + "warn-git-amend": warnGitAmend, + "warn-git-stash-drop": warnGitStashDrop, + "warn-all-files-staged": warnAllFilesStaged, + "warn-destructive-sql": warnDestructiveSql, + "warn-schema-alteration": warnSchemaAlteration, + "warn-package-publish": warnPackagePublish, + "warn-global-package-install": warnGlobalPackageInstall, + "prefer-package-manager": preferPackageManager, + "warn-large-file-write": warnLargeFileWrite, + "warn-background-process": warnBackgroundProcess, + "warn-repeated-tool-calls": warnRepeatedToolCalls, + "require-commit-before-stop": requireCommitBeforeStop, + "require-push-before-stop": requirePushBeforeStop, + "require-pr-before-stop": requirePrBeforeStop, + "require-no-conflicts-before-stop": requireNoConflictsBeforeStop, + "require-ci-green-before-stop": requireCiGreenBeforeStop, +}; + +/** + * Catalog and implementations must be in BIJECTION, and this throws rather than + * warns because the failure is otherwise invisible in the worst direction: a + * name present here but not there yields `fn: undefined`, `registerPolicy` + * stores it unvalidated, and the `TypeError` at `await policy.fn(ctx)` is + * swallowed by `policy-evaluator.ts` (warn, count, `continue`). The hook then + * ALLOWS, exits 0, and still lists the policy in `matchedPolicies` — a machine + * reporting that a guard ran when it never did. + */ +function assertCatalogBijection(): void { + const implNames = new Set(Object.keys(POLICY_IMPLEMENTATIONS)); + const missing = POLICY_CATALOG.filter((e) => !implNames.has(e.name)).map((e) => e.name); + if (missing.length > 0) { + throw new Error( + `failproofai: builtin policies missing an implementation: ${missing.join(", ")}`, + ); + } + const catalogNames = new Set(POLICY_CATALOG.map((e) => e.name)); + const orphaned = [...implNames].filter((n) => !catalogNames.has(n)); + if (orphaned.length > 0) { + throw new Error( + `failproofai: policy implementations with no catalog entry: ${orphaned.join(", ")}`, + ); + } +} +assertCatalogBijection(); + +/** + * The joined view, and the shape every consumer has always seen. + * + * Deliberately a positional, spread-only, eager `.map()`: it preserves catalog + * order, adds no fields, fills no defaults, drops no rows and calls no factory. + * Each of those alternatives changes observable behaviour — see the rules on + * {@link POLICY_CATALOG} and the invariants in + * `__tests__/hooks/policy-catalog.test.ts`. + */ +export const BUILTIN_POLICIES: BuiltinPolicyDefinition[] = POLICY_CATALOG.map((entry) => ({ + ...entry, + fn: POLICY_IMPLEMENTATIONS[entry.name] as PolicyFunction, +})); export function registerBuiltinPolicies(enabledNames: string[]): void { // Tolerate both flat ("sanitize-jwt") and qualified ("failproofai/sanitize-jwt") diff --git a/src/hooks/policy-catalog.ts b/src/hooks/policy-catalog.ts new file mode 100644 index 000000000..4c623a5fa --- /dev/null +++ b/src/hooks/policy-catalog.ts @@ -0,0 +1,535 @@ +/** + * The builtin policy CATALOG: what each policy is, never what it does. + * + * Split out of `builtin-policies.ts` so the metadata every consumer actually + * reads — the setup picker, `failproofai policies`, the presets, the audit + * report, the dashboard's server action — can be listed, searched and rendered + * without loading a single policy implementation. That is what lets a machine + * browse the catalog offline while the executable half moves to a fetched pack. + * + * ## Rules this file lives by + * + * - **Pure data.** No functions, no RegExp, no computed values. Every entry is + * a literal, so this module can be read, serialized, and eventually shipped + * as a manifest rather than as code. + * - **No value import from `builtin-policies.ts`.** Type-only imports are fine. + * `policy-evaluator.ts` builds `POLICY_PARAMS_MAP` from `BUILTIN_POLICIES` at + * MODULE SCOPE, so an import cycle here is a ReferenceError under ESM and a + * `.filter of undefined` under the CJS bundle — thrown at import time, on the + * hook critical path. + * - **Order is load-bearing and must not be sorted or regrouped.** Policy + * evaluation short-circuits on the first deny, so this array's order decides + * which policy name reaches the agent, the activity log, PostHog and the audit + * report. First appearance of each `category` is also what orders the sections + * in the TUI picker and the dashboard. + * - **Absent optionals stay absent.** `beta`, `alwaysOn` and `params` are read + * with `in` and `undefined` checks; default-filling them changes behaviour and + * fails existing tests. + * + * `params.default` values are handed to policies BY REFERENCE, so this must stay + * one module-level const — never a factory minting fresh defaults per call. + */ +import type { PolicyCatalogEntry, PolicyParamsSchema } from "./policy-types"; + +export const POLICY_CATALOG: PolicyCatalogEntry[] = [ + { + name: "sanitize-jwt", + description: "Stop Claude from reading JWTs in tool responses", + displayTitle: "Redacted JWT tokens from tool output", + impact: "Stops the agent from echoing auth tokens it saw in command output.", + match: { events: ["PostToolUse"] }, + defaultEnabled: true, + category: "Sanitize", + }, + { + name: "sanitize-api-keys", + description: "Stop Claude from reading API keys (OpenAI, Anthropic, GitHub, AWS, Stripe, Google) in tool responses", + displayTitle: "Redacted API keys from tool output", + impact: "Catches OpenAI / Anthropic / GitHub / AWS / Stripe / Google keys before the model sees them.", + match: { events: ["PostToolUse"] }, + defaultEnabled: true, + category: "Sanitize", + params: { + additionalPatterns: { + type: "pattern[]", + description: "Additional API key patterns to scrub, each with { regex, label }", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "sanitize-connection-strings", + description: "Stop Claude from reading database connection strings with embedded credentials in tool responses", + displayTitle: "Redacted database connection strings from tool output", + impact: "Strips embedded DB credentials before they reach the model context.", + match: { events: ["PostToolUse"] }, + defaultEnabled: true, + category: "Sanitize", + }, + { + name: "sanitize-private-key-content", + description: "Stop Claude from reading PEM private key content in tool responses", + displayTitle: "Redacted PEM private keys from tool output", + impact: "Prevents private key bodies from being echoed into chat context.", + match: { events: ["PostToolUse"] }, + defaultEnabled: true, + category: "Sanitize", + }, + { + name: "sanitize-bearer-tokens", + displayTitle: "Redacted bearer tokens from tool output", + impact: "Strips Authorization: Bearer values before they hit the model.", + description: "Stop Claude from reading Authorization Bearer tokens in tool responses", + match: { events: ["PostToolUse"] }, + defaultEnabled: true, + category: "Sanitize", + }, + { + name: "protect-env-vars", + displayTitle: "Tried to dump environment variables to chat", + impact: "Env vars often contain secrets; blocking `env` / `printenv` keeps them out of the model context.", + description: "Prevent commands that read environment variables", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: true, + category: "Environment", + }, + { + name: "block-env-files", + displayTitle: "Tried to read or write a .env file", + impact: "`.env` files routinely contain API keys and DB credentials.", + description: "Block reading/writing .env files", + match: { events: ["PreToolUse"] }, + defaultEnabled: true, + category: "Environment", + }, + { + name: "block-read-outside-cwd", + displayTitle: "Tried to read files outside your project directory", + impact: "Stops the agent from peeking at neighboring repos or your home directory.", + description: "Block file reads outside the session working directory", + match: { events: ["PreToolUse"], toolNames: ["Read", "Glob", "Grep", "Bash"] }, + defaultEnabled: false, + category: "Environment", + params: { + allowPaths: { + type: "string[]", + description: "Absolute paths outside cwd that are allowed to be read", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-sudo", + displayTitle: "Tried to run a command with sudo", + impact: "Sudo gives the agent root — blocked unless explicitly allow-listed.", + description: "Block sudo commands", + // PermissionRequest is Codex's escalation-approval event; fire the same + // sudo guard there so Codex sandbox bypasses are blocked too. + match: { events: ["PreToolUse", "PermissionRequest"], toolNames: ["Bash"] }, + defaultEnabled: true, + category: "Dangerous Commands", + params: { + allowPatterns: { + type: "string[]", + description: "Sudo command patterns to allow, matched token-by-token (e.g. 'sudo systemctl status')", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-curl-pipe-sh", + displayTitle: "Tried to pipe a downloaded script straight to a shell", + impact: "`curl ... | sh` runs unverified remote code on your machine.", + description: "Block piping downloads to shell", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: true, + category: "Dangerous Commands", + }, + { + name: "block-rm-rf", + displayTitle: "Tried to recursively delete a system path", + impact: "Catches catastrophic `rm -rf /` and Windows equivalents.", + description: "Prevent catastrophic deletions", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Dangerous Commands", + params: { + allowPaths: { + type: "string[]", + description: "Paths that are allowed to be recursively deleted", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-failproofai-commands", + displayTitle: "Tried to disable, pause or modify failproofai itself", + impact: "An agent that can pause or remove enforcement can switch off every other policy.", + description: "Block failproofai CLI commands, self-pause and uninstallation", + // PermissionRequest is carried over from the merged-in `block-self-pause`. + // It is a real enforcement point on Copilot and Devin, and dropping it would + // have left this guard blind on both. + match: { events: ["PreToolUse", "PermissionRequest"], toolNames: ["Bash"] }, + defaultEnabled: true, + alwaysOn: true, + category: "Dangerous Commands", + }, + { + name: "block-kubectl", + displayTitle: "Tried to run a Kubernetes command", + impact: "kubectl can change live cluster state — gated unless allow-listed.", + description: "Block kubectl commands (Kubernetes cluster mutations)", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Infra Commands", + params: { + allowPatterns: { + type: "string[]", + description: "kubectl command patterns to allow, matched token-by-token (e.g. 'kubectl get *', 'kubectl describe *')", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-terraform", + displayTitle: "Tried to run a Terraform/OpenTofu command", + impact: "Terraform mutates real infrastructure — gated unless allow-listed.", + description: "Block terraform and tofu (OpenTofu) commands", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Infra Commands", + params: { + allowPatterns: { + type: "string[]", + description: "terraform/tofu command patterns to allow (e.g. 'terraform plan', 'terraform validate')", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-aws-cli", + displayTitle: "Tried to run an AWS CLI command", + impact: "AWS CLI can spend money or break prod — gated.", + description: "Block aws CLI commands", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Infra Commands", + params: { + allowPatterns: { + type: "string[]", + description: "aws CLI command patterns to allow (e.g. 'aws s3 ls *', 'aws sts get-caller-identity')", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-gcloud", + displayTitle: "Tried to run a Google Cloud command", + impact: "gcloud can spend money or break prod — gated.", + description: "Block gcloud (Google Cloud) CLI commands", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Infra Commands", + params: { + allowPatterns: { + type: "string[]", + description: "gcloud command patterns to allow (e.g. 'gcloud auth list', 'gcloud config list')", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-az-cli", + displayTitle: "Tried to run an Azure CLI command", + impact: "az can spend money or break prod — gated.", + description: "Block az (Azure) CLI commands", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Infra Commands", + params: { + allowPatterns: { + type: "string[]", + description: "az CLI command patterns to allow (e.g. 'az account show', 'az group list')", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-helm", + displayTitle: "Tried to run a Helm command", + impact: "Helm releases mutate cluster state — gated.", + description: "Block helm commands", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Infra Commands", + params: { + allowPatterns: { + type: "string[]", + description: "helm command patterns to allow (e.g. 'helm list', 'helm status *')", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-gh-pipeline", + displayTitle: "Tried to run a privileged GitHub CLI pipeline command", + impact: "Catches `gh workflow run`, `gh pr merge`, `gh secret set`, etc.", + description: "Block gh CLI pipeline-trigger subcommands (workflow run, run rerun/cancel, pr merge, release create/delete, cache delete, secret set/delete)", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Infra Commands", + params: { + allowPatterns: { + type: "string[]", + description: "gh pipeline command patterns to allow (e.g. specific scripted invocations); read-only gh subcommands like 'gh pr view' and 'gh run list' are not matched by this policy", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-secrets-write", + displayTitle: "Tried to write a secret-key file", + impact: "Stops the agent from creating `.pem`, `id_rsa`, `credentials.json`, etc.", + description: "Block writing secret key files", + match: { events: ["PreToolUse"], toolNames: ["Write"] }, + defaultEnabled: false, + category: "Dangerous Commands", + params: { + additionalPatterns: { + type: "string[]", + description: "Additional filename patterns (substrings) to block", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-push-master", + displayTitle: "Tried to push directly to main/master", + impact: "Direct pushes to a protected branch bypass review.", + description: "Block pushing to main/master", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: true, + category: "Git", + params: { + protectedBranches: { + type: "string[]", + description: "Branch names to protect from direct pushes", + default: ["main", "master"], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "block-force-push", + displayTitle: "Tried to force-push", + impact: "Force-pushes rewrite history and can clobber teammates' work.", + description: "Prevent force-pushing to any branch", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Git", + }, + { + name: "block-work-on-main", + displayTitle: "Tried to commit or merge on main/master", + impact: "Work should land via PR — direct commits skip review.", + description: "Block git commits and merges on main/master branch", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Git", + params: { + protectedBranches: { + type: "string[]", + description: "Branch names where commits/merges are blocked", + default: ["main", "master"], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "warn-git-amend", + displayTitle: "Used git commit --amend", + impact: "Amending after a push rewrites history that others may have pulled.", + description: "Warns before amending git commits, which rewrites history", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Git", + }, + { + name: "warn-git-stash-drop", + displayTitle: "Tried to drop or clear git stash", + impact: "Stash deletions are permanent and silent.", + description: "Warns before permanently deleting stashed changes", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Git", + }, + { + name: "warn-all-files-staged", + displayTitle: "Staged all files with git add -A / .", + impact: "Wide stages routinely catch generated files or secrets you didn't intend to commit.", + description: "Warns before staging all working tree files with git add -A / . / --all", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Git", + }, + { + name: "warn-destructive-sql", + displayTitle: "Ran destructive SQL (DROP / TRUNCATE / DELETE without WHERE)", + impact: "Easy way to wipe a table by accident.", + description: "Warn before executing destructive SQL (DROP/TRUNCATE/DELETE without WHERE) via database clients", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Database", + }, + { + name: "warn-schema-alteration", + displayTitle: "Altered a database schema column", + impact: "ALTER TABLE operations can lock tables and break readers.", + description: "Warns before SQL schema changes (ALTER TABLE with column or rename operations)", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Database", + }, + { + name: "warn-package-publish", + displayTitle: "Tried to publish a package", + impact: "Publishes are irreversible — `npm publish` / `cargo publish` shouldn't happen without intent.", + description: "Warn before publishing packages to public registries (npm, PyPI, crates.io, RubyGems, etc.)", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Packages & System", + }, + { + name: "warn-global-package-install", + displayTitle: "Installed a package globally", + impact: "`npm i -g`, `cargo install`, `pip --user` pollute your machine outside the project.", + description: "Warns before installing packages globally (npm -g, cargo install, etc.)", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Packages & System", + }, + { + name: "prefer-package-manager", + displayTitle: "Used a non-preferred package manager", + impact: "Mixing package managers creates lockfile churn for your team.", + description: "Blocks non-preferred package managers and tells Claude to use an allowed one (e.g., uv instead of pip)", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Packages & System", + params: { + allowed: { + type: "string[]", + description: "Allowed package manager names (e.g. ['uv', 'bun']). Any detected manager not in this list is blocked.", + default: [], + }, + blocked: { + type: "string[]", + description: "Additional manager names to block beyond the built-in list (e.g. ['pdm', 'pipx']).", + default: [], + }, + } satisfies PolicyParamsSchema, + }, + { + name: "warn-large-file-write", + displayTitle: "Wrote a file larger than the configured threshold", + impact: "Catches accidentally large file writes (logs, binaries, model dumps).", + description: "Warn before writing files larger than 1MB (configurable via thresholdKb param)", + match: { events: ["PreToolUse"], toolNames: ["Write"] }, + defaultEnabled: false, + category: "Packages & System", + params: { + thresholdKb: { + type: "number", + description: "File size threshold in KB above which a warning is issued", + default: 1024, + }, + } satisfies PolicyParamsSchema, + }, + { + name: "warn-background-process", + displayTitle: "Started a long-lived background process", + impact: "Catches `nohup` / `&` / `screen` / `tmux` / `disown` patterns that the agent often forgets to clean up.", + description: "Warns before starting detached or background processes", + match: { events: ["PreToolUse"], toolNames: ["Bash"] }, + defaultEnabled: false, + category: "Packages & System", + }, + { + name: "warn-repeated-tool-calls", + displayTitle: "Called the same tool 3+ times with identical arguments", + impact: "Usually a sign of a stuck loop burning tokens.", + description: "Warn when the same tool is called 3+ times with identical parameters", + match: { events: ["PreToolUse"] }, + defaultEnabled: false, + category: "AI Behavior", + }, + { + name: "require-commit-before-stop", + displayTitle: "Stopped with uncommitted changes", + impact: "Work not in a commit is invisible to teammates and easy to lose.", + description: "Require all changes to be committed before Claude stops", + match: { events: ["Stop"] }, + defaultEnabled: false, + category: "Workflow", + }, + { + name: "require-push-before-stop", + displayTitle: "Stopped with unpushed commits", + impact: "Local-only commits won't trigger CI or be reviewable.", + description: "Require all commits to be pushed to remote before Claude stops", + match: { events: ["Stop"] }, + defaultEnabled: false, + category: "Workflow", + params: { + remote: { + type: "string", + description: "Remote name to push to (default: origin)", + default: "origin", + }, + baseBranch: { + type: "string", + description: "Base branch to compare against (default: main)", + default: "main", + }, + } satisfies PolicyParamsSchema, + }, + { + name: "require-pr-before-stop", + displayTitle: "Stopped without a PR for the branch", + impact: "Branches without PRs don't get reviewed.", + description: "Require a pull request to exist for the current branch before Claude stops", + match: { events: ["Stop"] }, + defaultEnabled: false, + category: "Workflow", + params: { + baseBranch: { + type: "string", + description: "Base branch to compare against (default: main)", + default: "main", + }, + } satisfies PolicyParamsSchema, + }, + { + name: "require-no-conflicts-before-stop", + displayTitle: "Stopped with a branch that conflicts with main", + impact: "Conflicting branches can't merge — surface them early.", + description: "Require the current branch to merge cleanly with the base branch before Claude stops", + match: { events: ["Stop"] }, + defaultEnabled: false, + category: "Workflow", + params: { + baseBranch: { + type: "string", + description: "Base branch to check for conflicts against (default: main)", + default: "main", + }, + } satisfies PolicyParamsSchema, + }, + { + name: "require-ci-green-before-stop", + displayTitle: "Stopped with failing CI", + impact: "Failing CI blocks deploy.", + description: "Require CI checks to pass on the current HEAD commit before Claude stops (ignores stale runs on prior commits)", + match: { events: ["Stop"] }, + defaultEnabled: false, + category: "Workflow", + }, +]; diff --git a/src/hooks/policy-types.ts b/src/hooks/policy-types.ts index d91cc6b0c..a61796a2e 100644 --- a/src/hooks/policy-types.ts +++ b/src/hooks/policy-types.ts @@ -45,6 +45,13 @@ export interface PolicyParamsSchema { }; } +/** + * A builtin policy minus its implementation — exactly what `policy-catalog.ts` + * holds. Derived from {@link BuiltinPolicyDefinition} rather than declared + * separately so a field added to one can never be forgotten on the other. + */ +export type PolicyCatalogEntry = Omit; + export interface BuiltinPolicyDefinition { name: string; description: string; From b990ff2559a072ad4c3ad41cdae9c69999402512 Mon Sep 17 00:00:00 2001 From: chhhee10 Date: Thu, 20 Aug 2026 20:23:34 +0530 Subject: [PATCH 03/17] Give policies a delivery path that is not the npm package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pack is one digest-pinned entry artifact plus a manifest describing what it contains, installed under ~/.failproofai/policies/packs/ beside the cloud artifacts and loaded through the custom-policy loader that already exists. Not a fourth loader — the same lane with a different tag. Packs are LOCAL policy. Cloud assignments are exempt from disabledCustomPolicies and from session pause because a locally-issued command must not switch off a CENTRALLY assigned policy; a pack the user installed by typing a command is not that, so it stays disableable and pausable. Copying the exemption would have been an unrelated capability arriving by copy-paste. Three refusals, each closing a silent failure: - A pack policy name may not contain `/`, and pack policies register under `pack/@/`. Verified live that without this a pack shipping the name `failproofai/block-sudo` REPLACES the compiled builtin — normalizePolicyName passes any name containing a slash through untouched and registerPolicy replaces by canonical name — so the machine would report block-sudo as enabled while running a stranger's code. - A pack may not declare `alwaysOn`: downloaded enforcement that no local command can turn off. - Byte-identical packs merge toward enforcement, with a warning. Artifacts are content-addressed, so identical source is one file, and the loser would otherwise vanish with its effect deciding nothing. Same collision that once silently downgraded a cloud policy to observe-only. Manifest and artifact are reconciled after load. The artifact is digest-pinned so what it registers is what the publisher shipped, but nothing bound the manifest to it: a declared policy the artifact never registers is a listing claiming protection that does not run. engineVersion, which keys the audit cache, folds in each pack's id|version|sha256 — by identity, not source text, because the loader rewrites a per-load temporary filename into every import specifier and hashing that would cold-rescan the whole history every run. A machine with no packs hashes byte-identically to a build with no pack support, verified at the source level and in the shipped bundle (c1cea4ddf3030af4, unchanged), so this costs no existing user the ~104-second rescan. Failure is per pack, not per manifest, and fails open with a recorded reason. That is sound only while the builtins still ship compiled in and keep enforcing underneath, and the catch says so — because the day builtins become a fetched pack, this exact behaviour is zero enforcement on a machine reporting healthy. --- CHANGELOG.md | 2 + __tests__/audit/engine-version-packs.test.ts | 100 ++++++++ __tests__/hooks/fp-home.test.ts | 19 ++ __tests__/hooks/pack-loading.test.ts | 189 ++++++++++++++ __tests__/hooks/pack-manifest.test.ts | 190 ++++++++++++++ src/audit/cache.ts | 43 +++- src/hooks/cloud-managed-policies.ts | 7 +- src/hooks/custom-hooks-loader.ts | 73 +++++- src/hooks/fp-home.ts | 27 ++ src/hooks/handler.ts | 38 ++- src/hooks/pack-manifest.ts | 251 +++++++++++++++++++ 11 files changed, 929 insertions(+), 10 deletions(-) create mode 100644 __tests__/audit/engine-version-packs.test.ts create mode 100644 __tests__/hooks/pack-loading.test.ts create mode 100644 __tests__/hooks/pack-manifest.test.ts create mode 100644 src/hooks/pack-manifest.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e031651..e47570bd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features +- Build the policy pack lane: the delivery path for policies that did not ship compiled into the build. A pack is one digest-pinned entry artifact plus a manifest describing what it contains, installed under `~/.failproofai/policies/packs/` as a sibling of the cloud artifacts and loaded through the custom-policy loader that already exists rather than a fourth loader. Packs are LOCAL policy: unlike cloud assignments they stay disableable via `disabledCustomPolicies` and are suspended by a session pause, because cloud's exemptions exist so a locally-issued command cannot switch off a CENTRALLY assigned policy and a pack the user installed by typing a command is not that. Three refusals close silent failures. A pack policy name may not contain `/`, and a pack's registered policies are prefixed `pack/@/` — verified live that without this a pack shipping the name `failproofai/block-sudo` REPLACES the compiled builtin, since `normalizePolicyName` passes any name containing a slash straight through and `registerPolicy` replaces by canonical name; the machine would then report `block-sudo` as enabled while running a stranger's code. A pack may not declare `alwaysOn`, which would be downloaded enforcement no local command can turn off. And byte-identical packs are merged toward enforcement with a warning, because content-addressed artifacts mean identical source is one file on disk and the loser would otherwise vanish with its effect deciding nothing — the same collision that once silently downgraded a cloud policy to observe-only. Manifest and artifact are reconciled after load and any disagreement is named: a declared policy the artifact never registers is a listing claiming protection that does not run. `engineVersion`, which keys the audit cache, now folds in each pack's `id|version|sha256` — by identity rather than source text, since the loader rewrites a per-load temporary filename into every import specifier and hashing that would cold-rescan the entire history on every run. A machine with no packs hashes byte-identically to a build without pack support, verified at both the source level and in the shipped bundle (`c1cea4ddf3030af4`, unchanged), so shipping this costs no existing user the ~104-second rescan. Failure is per pack, not per manifest, and fails open with a recorded reason — which is sound only while the builtins still ship compiled in and keep enforcing underneath, and is commented as such at the catch. (#PR) + - Split the builtin policy catalog from its implementations, the first step of moving policies out of the npm package. `policy-catalog.ts` now holds the metadata every consumer actually reads — name, description, category, `match`, `defaultEnabled`, `params` — as pure literal data with no functions in it, and `builtin-policies.ts` keeps the 39 implementations and joins them back on, so `BUILTIN_POLICIES` has exactly the shape, fields and ORDER it always had and not one of its nine source consumers changes. That is what will let a machine list, search and render the catalog offline once the executable half moves to a fetched pack. Two things made the refactor narrower than it looks. `audit/cache.ts` hashes `fn.toString()` for all 39 policies into the audit cache's `engineVersion`, and `bun build` renames colliding top-level identifiers by module EMISSION ORDER — renamed names appear inside policy bodies in the shipped bundle (`cwdWithSep2`, `execSync2`, `resolved3`) — so inserting a module into the graph could have changed the emitted text, invalidated every user's audit cache and forced a ~104-second cold rescan on upgrade. Measured before and after: `engineVersion` is unchanged (`c1cea4ddf3030af4`), so nobody's cache moves. And `SECRET_PATTERNS` stays in `builtin-policies.ts` rather than being reclassified as catalog data: it is assembled from the very RegExps the five `sanitize-*` policies test against and is imported by the audit redactor, so moving it would have forced a catalog→implementation value edge and put an import cycle on the hook critical path. `policy-catalog.test.ts` pins the join against the failures that are otherwise silent — a wrapper collapsing 39 distinct `fn.toString()` hashes into one and freezing the cache key, a sort or regroup changing which policy name gets attributed on a deny, a spread default-filling `beta`, a dropped row shrinking the catalog invisibly to `manager.ts` and `install-prompt.ts` (neither reads `.fn`) — plus a JSON round-trip asserting the catalog is serializable and a source-level guard that it never value-imports back. A bijection check throws at module load rather than warning, because a name with no implementation yields `fn: undefined`, whose `TypeError` `policy-evaluator.ts` swallows: the hook would allow, exit 0, and still report the policy in `matchedPolicies`. (#PR) - Merge `block-self-pause` into `block-failproofai-commands` and make the result the one policy that cannot be switched off. The two were halves of one guard and disagreed with each other. `block-self-pause` had the hardened matcher — segments split on shell operators, runner prefixes and their flags walked off, the binary resolved by basename, and the shell-unescaped form re-checked — but only ever looked for `config --pause`. `block-failproofai-commands` had the whole surface (any CLI invocation, plus package-manager uninstall) on `/(?:^|;|&&|\|\||\|)\s*failproofai(?:\s|$)/`, which a single prefix defeated: `sudo failproofai config --pause`, `npx failproofai policies --uninstall`, `env X=1 failproofai …`, `/usr/local/bin/failproofai …` and `timeout 30 failproofai …` were all ALLOWED by a `defaultEnabled` self-protection policy. The merged policy is the hardened matcher over the broad surface, and it keeps `PermissionRequest` from the merged-in half — a real enforcement point on Copilot and Devin that the survivor did not subscribe to. Where the two contradicted each other the merge keeps what machines actually did: `block-self-pause` deliberately allowed `config --resume`, `config --status` and `policies --install`, but both policies were default-on and the sibling denied all three first, so that allow never ran anywhere. It is now `alwaysOn`, a new flag `registerBuiltinPolicies` honours ahead of the enabled set, which closes the three ways the old pair could be switched off silently — a name absent from `enabledPolicies`, an active session pause (`handler.ts` passes `[]`), and a config file that fails to parse (`hooks-config.ts` soft-fails to `{enabledPolicies: []}`, so corrupting one file disabled every policy including these). `policies --disable block-failproofai-commands` now refuses with a reason instead of editing the config and reporting a success that changes nothing. (#PR) diff --git a/__tests__/audit/engine-version-packs.test.ts b/__tests__/audit/engine-version-packs.test.ts new file mode 100644 index 000000000..a9c7e8fb0 --- /dev/null +++ b/__tests__/audit/engine-version-packs.test.ts @@ -0,0 +1,100 @@ +// @vitest-environment node +/** + * The audit cache key, and the one property that decides whether shipping pack + * support costs every existing user a cold rescan. + * + * `engineVersion` keys on-disk audit cache entries. A machine that has never + * installed a pack must hash EXACTLY as it did before packs existed — otherwise + * merely upgrading invalidates the cache and forces a full re-scan of every + * transcript (~104s, per the note on CACHE_TTL_MS) for a feature nobody is using. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { createHash } from "node:crypto"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { BUILTIN_POLICIES } from "@/src/hooks/builtin-policies"; + +/** Exactly the pre-pack formula, reproduced here so the test is independent of + * the implementation it checks. */ +function prePackEngineVersion(): string { + const blob = BUILTIN_POLICIES.map((p) => `${p.name}|${p.fn.toString()}`).sort().join("\n"); + return createHash("sha1").update(blob).digest("hex").slice(0, 16); +} + +const ARTIFACT = "export const hooks = [];\n"; +const DIGEST = createHash("sha256").update(ARTIFACT).digest("hex"); + +let root: string; +let prevEnv: string | undefined; + +/** Fresh module each time — engineVersion memoizes per process. */ +async function engineVersion(): Promise { + const { getEngineVersionForTest } = await import("@/src/audit/cache"); + return getEngineVersionForTest(); +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "fpai-ev-packs-")); + mkdirSync(join(root, "artifacts"), { recursive: true }); + writeFileSync(join(root, "artifacts", `${DIGEST}.mjs`), ARTIFACT); + prevEnv = process.env.FAILPROOFAI_PACK_DIR; + process.env.FAILPROOFAI_PACK_DIR = root; + vi.resetModules(); +}); + +afterEach(() => { + if (prevEnv === undefined) delete process.env.FAILPROOFAI_PACK_DIR; + else process.env.FAILPROOFAI_PACK_DIR = prevEnv; + rmSync(root, { recursive: true, force: true }); +}); + +import { vi } from "vitest"; + +function installPack(id: string, version: string): void { + writeFileSync( + join(root, "installed.json"), + JSON.stringify({ + schemaVersion: 1, + packs: [{ + id, version, + source: `github:${id}@${version}`, + entry: `artifacts/${DIGEST}.mjs`, + sha256: DIGEST, + policies: [], + }], + }), + ); +} + +describe("engineVersion with packs", () => { + it("is UNCHANGED from the pre-pack formula when no pack is installed", async () => { + // The upgrade-cost guarantee. If this ever fails, shipping the change cold- + // rescans every existing user's whole transcript history. + expect(await engineVersion()).toBe(prePackEngineVersion()); + }); + + it("changes once a pack is installed", async () => { + installPack("acme/finance", "1.2.0"); + vi.resetModules(); + expect(await engineVersion()).not.toBe(prePackEngineVersion()); + }); + + it("changes again when the same pack moves to a new version", async () => { + installPack("acme/finance", "1.2.0"); + vi.resetModules(); + const at120 = await engineVersion(); + installPack("acme/finance", "1.3.0"); + vi.resetModules(); + expect(await engineVersion()).not.toBe(at120); + }); + + it("falls back to the builtin-only hash when the manifest is unreadable", async () => { + // A corrupt manifest must not change the cache key: the packs did not load, + // so the audit that runs is a builtin-only audit and should hit the cache a + // builtin-only audit wrote. + writeFileSync(join(root, "installed.json"), "not json"); + vi.resetModules(); + expect(await engineVersion()).toBe(prePackEngineVersion()); + }); +}); diff --git a/__tests__/hooks/fp-home.test.ts b/__tests__/hooks/fp-home.test.ts index 54724e2fd..fff3c4983 100644 --- a/__tests__/hooks/fp-home.test.ts +++ b/__tests__/hooks/fp-home.test.ts @@ -99,6 +99,23 @@ describe("fp-home layout", () => { expect(H.globalPolicyConfigFile().startsWith(`${H.policiesDir()}/`)).toBe(false); }); + it("hides pack artifacts from the convention loader", () => { + // Same property as the cloud case below, and the same reason it must be + // proven against a real directory: a pack artifact picked up by the + // convention loader would be loaded with NO digest check — the one thing + // pinning a pack by sha256 exists to prevent. + const artifacts = H.packArtifactsDir(); + mkdirSync(artifacts, { recursive: true }); + // Maximally attractive to both filters: the convention suffix on a loadable + // extension, so only the non-recursion keeps them out. + writeFileSync(resolve(artifacts, "aaa-policies.mjs"), "export default {}"); + writeFileSync(resolve(H.packsDir(), "installed-policies.mjs"), "export default {}"); + mkdirSync(resolve(H.policiesDir(), "packs-policies.mjs"), { recursive: true }); + + expect(discoverPolicyFiles(H.policiesDir())).toEqual([]); + expect(findSkippedPolicyFiles(H.policiesDir())).toEqual([]); + }); + it("hides cloud artifacts from the convention loader", () => { // THE property that makes nesting the fleet's policies inside the user's // directory safe. `discoverPolicyFiles` walking subdirectories would turn @@ -203,6 +220,8 @@ describe("HOME_CLASSES", () => { migrationLedgerFile: "migrationsDir", migrationBackupDir: "migrationsDir", stateDir: "stateDir", + packsInstalledFile: "packsDir", + packArtifactsDir: "packsDir", }; /** Every exported function that returns a path inside the home. */ diff --git a/__tests__/hooks/pack-loading.test.ts b/__tests__/hooks/pack-loading.test.ts new file mode 100644 index 000000000..64cbd2e4a --- /dev/null +++ b/__tests__/hooks/pack-loading.test.ts @@ -0,0 +1,189 @@ +// @vitest-environment node +/** + * A pack, loaded the way a real one is: real bytes on disk, a real sha256, and + * a real dynamic import through `loadAllCustomHooks`. + * + * The pack lane is deliberately the CUSTOM lane with a different tag, not a + * fourth loader. Everything below exists to prove the tag is applied where it + * has to be and — the part that matters — that a pack cannot reach the builtin + * namespace or skip its digest. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +vi.mock("@/src/hooks/hook-logger", async (orig) => ({ + ...(await orig>()), + hookLogWarn: vi.fn(), +})); +import { hookLogWarn } from "@/src/hooks/hook-logger"; +import { createHash } from "node:crypto"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { loadAllCustomHooks } from "@/src/hooks/custom-hooks-loader"; +import { clearCustomHooks } from "@/src/hooks/custom-hooks-registry"; +import type { ResolvedPack } from "@/src/hooks/pack-manifest"; + +const SRC = ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "block-refunds-over-limit", + description: "from a pack", + match: { events: ["PreToolUse"] }, + fn: async () => deny("refund exceeds the approved limit"), + }); +`; +const SHA = createHash("sha256").update(SRC).digest("hex"); + +let root: string; +let artifact: string; + +function packRecord(over: Partial = {}): ResolvedPack { + return { + id: "acme/finance", + version: "1.2.0", + source: "github:acme/finance@v1.2.0", + path: artifact, + sha256: SHA, + effect: "enforce", + policies: [], + ...over, + }; +} + +type Tagged = { __pack?: ResolvedPack; __policyId?: string; __cloudManaged?: unknown; name: string }; + +async function loadWith(packs: ResolvedPack[], paths: string[] = [artifact]) { + const result = await loadAllCustomHooks(paths, { sessionCwd: root, packs }); + return result.hooks as unknown as Tagged[]; +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "fpai-pack-load-")); + const artifacts = join(root, "artifacts"); + mkdirSync(artifacts, { recursive: true }); + artifact = join(artifacts, `${SHA}.mjs`); + writeFileSync(artifact, SRC, "utf8"); + clearCustomHooks(); +}); + +afterEach(() => { + clearCustomHooks(); + rmSync(root, { recursive: true, force: true }); +}); + +describe("pack loading", () => { + it("loads a pack's policy and tags it with the pack's identity", async () => { + const hooks = await loadWith([packRecord()]); + expect(hooks).toHaveLength(1); + expect(hooks[0].name).toBe("block-refunds-over-limit"); + expect(hooks[0].__pack?.id).toBe("acme/finance"); + expect(hooks[0].__pack?.version).toBe("1.2.0"); + // The id is what `disabledCustomPolicies` matches on, so it must carry the + // version: disabling a policy in 1.2.0 should not silently keep it disabled + // when the publisher ships 1.3.0 with different behaviour. + expect(hooks[0].__policyId).toBe("pack:acme/finance@1.2.0:block-refunds-over-limit"); + }); + + it("refuses to import an artifact whose bytes no longer match the manifest", async () => { + // The manifest read and the import are two moments. This is the one that + // binds the bytes actually EXECUTED to what was promised. + writeFileSync(artifact, SRC.replace("deny(", "allow("), "utf8"); + const hooks = await loadWith([packRecord()]); + expect(hooks).toHaveLength(0); + }); + + it("is not tagged as cloud-managed", async () => { + // Cloud policies are exempt from local disable and from session pause. A + // pack the user installed by typing a command is LOCAL policy, and picking + // up that exemption by mistake would make it undisableable. + const hooks = await loadWith([packRecord()]); + expect(hooks[0].__cloudManaged).toBeUndefined(); + }); + + it("merges byte-identical packs toward enforcement, and says so", async () => { + // Artifacts are content-addressed, so two packs with identical source share + // ONE file, and `loadedPaths` imports it exactly once. Whichever record wins + // decides enforcement — the same collision that silently downgraded a cloud + // policy to observe-only once already. + const hooks = await loadWith([ + packRecord({ id: "acme/finance", effect: "observe" }), + packRecord({ id: "other/dupe", effect: "enforce" }), + ]); + expect(hooks).toHaveLength(1); + expect(hooks[0].__pack?.effect).toBe("enforce"); + }); + + it("registers a pack policy exactly once even if its path is listed twice", async () => { + // `customPolicies.add` is an unconditional push, so a second import would + // register every hook again and fire it twice per event — which silently + // halves the ceiling of any counting policy. + const hooks = await loadWith([packRecord()], [artifact, artifact]); + expect(hooks).toHaveLength(1); + }); + + describe("manifest vs artifact", () => { + // Digest-pinning proves the bytes are the publisher's. It proves nothing + // about the manifest AGREEING with them, and a listing built from a manifest + // that disagrees is a listing that lies. + it("warns when the manifest declares a policy the artifact never registers", async () => { + const warn = vi.mocked(hookLogWarn); + warn.mockClear(); + await loadWith([ + packRecord({ + policies: [ + { name: "block-refunds-over-limit", description: "d", category: "c", defaultEnabled: true, match: {} }, + { name: "ghost-policy", description: "d", category: "c", defaultEnabled: true, match: {} }, + ] as ResolvedPack["policies"], + }), + ]); + const msg = warn.mock.calls.map((c) => String(c[0])).join("\n"); + expect(msg).toContain("ghost-policy"); + expect(msg).toContain("never runs"); + }); + + it("warns when the artifact registers a policy the manifest omits", async () => { + const warn = vi.mocked(hookLogWarn); + warn.mockClear(); + await loadWith([ + packRecord({ + policies: [ + { name: "something-else", description: "d", category: "c", defaultEnabled: true, match: {} }, + ] as ResolvedPack["policies"], + }), + ]); + const msg = warn.mock.calls.map((c) => String(c[0])).join("\n"); + expect(msg).toContain("block-refunds-over-limit"); + expect(msg).toContain("will not appear in listings"); + }); + + it("says nothing when they agree", async () => { + const warn = vi.mocked(hookLogWarn); + warn.mockClear(); + await loadWith([ + packRecord({ + policies: [ + { name: "block-refunds-over-limit", description: "d", category: "c", defaultEnabled: true, match: {} }, + ] as ResolvedPack["policies"], + }), + ]); + const msg = warn.mock.calls.map((c) => String(c[0])).join("\n"); + expect(msg).not.toContain("block-refunds-over-limit"); + }); + }); + + it("loads an ordinary custom policy from the same call without pack tagging", async () => { + const plain = join(root, "my-policies.mjs"); + writeFileSync( + plain, + `import { customPolicies, allow } from "failproofai"; + customPolicies.add({ name: "mine", description: "d", match: { events: ["PreToolUse"] }, fn: async () => allow() });`, + "utf8", + ); + const hooks = await loadWith([packRecord()], [artifact, plain]); + const byName = Object.fromEntries(hooks.map((h) => [h.name, h])); + expect(byName["block-refunds-over-limit"].__pack?.id).toBe("acme/finance"); + expect(byName["mine"].__pack).toBeUndefined(); + expect(byName["mine"].__policyId).toContain("custom:"); + }); +}); diff --git a/__tests__/hooks/pack-manifest.test.ts b/__tests__/hooks/pack-manifest.test.ts new file mode 100644 index 000000000..4701740d1 --- /dev/null +++ b/__tests__/hooks/pack-manifest.test.ts @@ -0,0 +1,190 @@ +// @vitest-environment node +/** + * The pack manifest reader, exercised against real files with real digests. + * + * Every assertion here is about a REFUSAL. The reader's whole job is to decide + * what may be imported, so a test that only proves the happy path proves the + * least interesting half. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { createHash } from "node:crypto"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +let root: string; +let prevEnv: string | undefined; + +const ARTIFACT = "export const hooks = [];\n"; +const DIGEST = createHash("sha256").update(ARTIFACT).digest("hex"); + +/** A minimal valid pack policy — the shape a publisher serializes. */ +const POLICY = { + name: "block-refunds-over-limit", + description: "Block refunds above the approved limit", + category: "Finance", + defaultEnabled: true, + match: { events: ["PreToolUse"] }, +}; + +function writeManifest(packs: unknown[], schemaVersion: unknown = 1): void { + writeFileSync(join(root, "installed.json"), JSON.stringify({ schemaVersion, packs })); +} + +function pack(over: Record = {}): Record { + return { + id: "acme/finance", + version: "1.2.0", + source: "github:acme/finance@v1.2.0", + entry: `artifacts/${DIGEST}.mjs`, + sha256: DIGEST, + policies: [POLICY], + ...over, + }; +} + +async function read() { + const mod = await import("../../src/hooks/pack-manifest"); + return mod.readInstalledPacks(); +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "fpai-packs-")); + mkdirSync(join(root, "artifacts"), { recursive: true }); + writeFileSync(join(root, "artifacts", `${DIGEST}.mjs`), ARTIFACT); + prevEnv = process.env.FAILPROOFAI_PACK_DIR; + process.env.FAILPROOFAI_PACK_DIR = root; +}); + +afterEach(() => { + if (prevEnv === undefined) delete process.env.FAILPROOFAI_PACK_DIR; + else process.env.FAILPROOFAI_PACK_DIR = prevEnv; + rmSync(root, { recursive: true, force: true }); +}); + +describe("readInstalledPacks", () => { + it("returns nothing, and no error, when no pack was ever installed", () => { + // The overwhelmingly common case. It must not look like a failure. + expect(read()).resolves.toEqual({ packs: [], errors: [] }); + }); + + it("resolves a valid pack and verifies its digest", async () => { + writeManifest([pack()]); + const { packs, errors } = await read(); + expect(errors).toEqual([]); + expect(packs).toHaveLength(1); + expect(packs[0].id).toBe("acme/finance"); + expect(packs[0].effect).toBe("enforce"); + expect(packs[0].policies.map((p) => p.name)).toEqual(["block-refunds-over-limit"]); + expect(packs[0].path).toBe(resolve(root, "artifacts", `${DIGEST}.mjs`)); + }); + + it("honours an explicit observe effect and refuses an unknown one", async () => { + writeManifest([pack({ effect: "observe" })]); + expect((await read()).packs[0].effect).toBe("observe"); + + writeManifest([pack({ effect: "audit" })]); + const { packs, errors } = await read(); + expect(packs).toEqual([]); + expect(errors[0].reason).toContain("unknown effect"); + }); + + describe("refusals", () => { + it("refuses a tampered artifact WITHOUT taking other packs down with it", async () => { + // The per-pack granularity that separates this from the cloud reader. One + // bad third-party pack must not switch off every other pack on the machine. + const other = "export const hooks = [1];\n"; + const otherDigest = createHash("sha256").update(other).digest("hex"); + writeFileSync(join(root, "artifacts", `${otherDigest}.mjs`), other); + writeManifest([ + pack(), + pack({ id: "good/pack", entry: `artifacts/${otherDigest}.mjs`, sha256: otherDigest }), + ]); + // Tamper with the FIRST pack's bytes after the manifest recorded its hash. + writeFileSync(join(root, "artifacts", `${DIGEST}.mjs`), "export const hooks = [99];\n"); + + const { packs, errors } = await read(); + expect(packs.map((p) => p.id)).toEqual(["good/pack"]); + expect(errors).toHaveLength(1); + expect(errors[0].id).toBe("acme/finance"); + expect(errors[0].reason).toContain("failed integrity verification"); + }); + + it("refuses an entry path that escapes the pack root", async () => { + writeManifest([pack({ entry: "../../../etc/passwd" })]); + const { packs, errors } = await read(); + expect(packs).toEqual([]); + expect(errors[0].reason).toMatch(/escapes its root|unsafe managed policy path/); + }); + + it("refuses an absolute entry path", async () => { + writeManifest([pack({ entry: "/etc/passwd" })]); + expect((await read()).errors[0].reason).toContain("unsafe managed policy path"); + }); + + it("refuses a pack that declares alwaysOn", async () => { + // alwaysOn means "cannot be disabled or paused". A downloaded file granting + // itself that would be enforcement no local command can switch off. + writeManifest([pack({ policies: [{ ...POLICY, alwaysOn: true }] })]); + const { packs, errors } = await read(); + expect(packs).toEqual([]); + expect(errors[0].reason).toContain("alwaysOn"); + }); + + it("refuses a policy name containing a slash", async () => { + // The namespace-hijack guard. `normalizePolicyName` passes a name with `/` + // through untouched and `registerPolicy` replaces by canonical name, so + // this exact string would otherwise overwrite the compiled builtin. + writeManifest([pack({ policies: [{ ...POLICY, name: "failproofai/block-sudo" }] })]); + const { packs, errors } = await read(); + expect(packs).toEqual([]); + expect(errors[0].reason).toContain("unsafe name"); + }); + + it("refuses a duplicate pack id", async () => { + writeManifest([pack(), pack()]); + const { packs, errors } = await read(); + expect(packs).toHaveLength(1); + expect(errors[0].reason).toContain("duplicate pack id"); + }); + + it("refuses a pack declaring the same policy twice", async () => { + writeManifest([pack({ policies: [POLICY, POLICY] })]); + expect((await read()).errors[0].reason).toContain("twice"); + }); + + it("refuses an unsafe pack id", async () => { + writeManifest([pack({ id: "../../evil" })]); + expect((await read()).errors[0].reason).toContain("unsafe pack id"); + }); + + it("refuses a policy missing required catalog fields", async () => { + for (const missing of ["description", "category", "defaultEnabled", "match"]) { + const p: Record = { ...POLICY }; + delete p[missing]; + writeManifest([pack({ policies: [p] })]); + const { errors } = await read(); + expect(errors[0].reason, missing).toContain(missing); + } + }); + }); + + describe("manifest-level failures never throw", () => { + it("records unreadable JSON as an error", async () => { + writeFileSync(join(root, "installed.json"), "not json"); + const { packs, errors } = await read(); + expect(packs).toEqual([]); + expect(errors[0].reason).toContain("unreadable pack manifest"); + }); + + it("records an unsupported schema version as an error", async () => { + writeManifest([pack()], 99); + expect((await read()).errors[0].reason).toContain("unsupported pack manifest schema"); + }); + + it("records a non-array packs field as an error", async () => { + writeFileSync(join(root, "installed.json"), JSON.stringify({ schemaVersion: 1, packs: {} })); + expect((await read()).errors[0].reason).toContain("not an array"); + }); + }); +}); diff --git a/src/audit/cache.ts b/src/audit/cache.ts index 29f60ca6f..91d420cf8 100644 --- a/src/audit/cache.ts +++ b/src/audit/cache.ts @@ -13,6 +13,7 @@ import { createHash } from "node:crypto"; import { existsSync, mkdirSync, readFileSync, writeFileSync, chmodSync } from "node:fs"; import { join } from "node:path"; import { BUILTIN_POLICIES } from "../hooks/builtin-policies"; +import { readInstalledPacks } from "../hooks/pack-manifest"; import { AUDIT_DETECTORS } from "./detectors"; import type { TranscriptAuditResult } from "./types"; import { auditCacheDir } from "../hooks/fp-home"; @@ -20,14 +21,43 @@ import { auditCacheDir } from "../hooks/fp-home"; let cachedEngineVersion: string | null = null; let cachedDetectorVersion: string | null = null; -/** Hash of every builtin policy's name + function body. Changes when policy - * code changes, invalidating downstream caches. */ +/** + * Hash of every builtin policy's name + function body, PLUS the identity of every + * installed pack. Changes when policy code changes, invalidating downstream + * caches. + * + * Packs belong in this key because they change what a machine would have caught, + * which is exactly what a cached audit result claims to know. They are folded in + * by `id|version|sha256` rather than by source text: the pack's bytes are already + * digest-pinned, and hashing the loaded source instead would drag in the + * per-load temporary filename the loader rewrites into every import specifier — + * which changes on every single run and would cold-rescan the whole history each + * time. + * + * **A machine with no packs must hash byte-identically to a build with no pack + * support at all.** Otherwise merely shipping this feature invalidates every + * user's audit cache and forces a ~104-second cold rescan on upgrade, for a + * capability they are not using. Hence the empty pack set contributes nothing — + * not an empty line, not a separator. + */ function getEngineVersion(): string { if (cachedEngineVersion) return cachedEngineVersion; - const blob = BUILTIN_POLICIES + const builtinBlob = BUILTIN_POLICIES .map((p) => `${p.name}|${p.fn.toString()}`) .sort() .join("\n"); + // Never throws by contract, but this runs inside the audit's own try/catch-free + // path and a cache key is not worth taking the run down for. + let packBlob = ""; + try { + packBlob = readInstalledPacks() + .packs.map((pack) => `${pack.id}|${pack.version}|${pack.sha256}`) + .sort() + .join("\n"); + } catch { + packBlob = ""; + } + const blob = packBlob ? `${builtinBlob}\n${packBlob}` : builtinBlob; cachedEngineVersion = createHash("sha1").update(blob).digest("hex").slice(0, 16); return cachedEngineVersion; } @@ -43,6 +73,13 @@ function getDetectorVersion(): string { return cachedDetectorVersion; } +/** Exposed for `engine-version-packs.test.ts`, which pins the upgrade-cost + * guarantee: a machine with no packs must hash exactly as it did before packs + * existed. Nothing else should call this. */ +export function getEngineVersionForTest(): string { + return getEngineVersion(); +} + function getCachePathFor(transcriptPath: string): string { const root = auditCacheDir(); const key = createHash("sha1").update(transcriptPath).digest("hex"); diff --git a/src/hooks/cloud-managed-policies.ts b/src/hooks/cloud-managed-policies.ts index 97506a31d..11ac2763a 100644 --- a/src/hooks/cloud-managed-policies.ts +++ b/src/hooks/cloud-managed-policies.ts @@ -102,7 +102,12 @@ function parseManifest(value: unknown): ActiveManifest { return { schemaVersion: raw.schemaVersion as number, deployment: deployment as number, policies }; } -function resolveManagedPath(root: string, candidate: string): string { +/** + * Shared with `pack-manifest.ts` deliberately. This is a path-escape guard on a + * file that is about to be IMPORTED; two copies of it would be two things to + * harden and one to forget. + */ +export function resolveManagedPath(root: string, candidate: string): string { if (!candidate || isAbsolute(candidate)) throw new Error(`unsafe managed policy path ${JSON.stringify(candidate)}`); const absolute = resolve(root, candidate); const lexicalRelative = relative(root, absolute); diff --git a/src/hooks/custom-hooks-loader.ts b/src/hooks/custom-hooks-loader.ts index 0613712b0..32182aedb 100644 --- a/src/hooks/custom-hooks-loader.ts +++ b/src/hooks/custom-hooks-loader.ts @@ -31,6 +31,7 @@ import { trackHookEvent } from "./hook-telemetry"; import { getInstanceId } from "../../lib/telemetry-id"; import type { CustomHook } from "./policy-types"; import type { CloudManagedPolicyArtifact } from "./cloud-managed-policies"; +import type { ResolvedPack } from "./pack-manifest"; import { customPoliciesDir, shimsDir } from "./fp-home"; const LOADING_KEY = "__FAILPROOFAI_LOADING_HOOKS__"; @@ -330,12 +331,47 @@ function warnSkippedPolicyFiles(dir: string, scope: "project" | "user"): void { ); } +/** + * A pack's manifest declares what it contains; its artifact decides what runs. + * Nothing binds the two, so they can disagree — and the disagreement is silent + * in both directions and worse in one. + * + * A policy the artifact registers but the manifest omits is enforcement that no + * `failproofai policies` listing will ever show. A policy the manifest declares + * but the artifact never registers is the dangerous one: the listing says the + * machine is protected against something nothing is checking. + * + * Both are announced rather than corrected. The artifact is digest-pinned, so + * what it registers IS what the publisher shipped and dropping any of it would + * be inventing a third answer; the manifest is what needs fixing, upstream. + */ +function reconcilePackManifest(pack: ResolvedPack | undefined, loaded: CustomHook[]): void { + if (!pack || pack.policies.length === 0) return; + const declared = new Set(pack.policies.map((p) => p.name)); + const registered = new Set(loaded.map((h) => h.name)); + const missing = [...declared].filter((n) => !registered.has(n)); + const extra = [...registered].filter((n) => !declared.has(n)); + if (missing.length > 0) { + hookLogWarn( + `pack ${pack.id}@${pack.version} declares ${missing.join(", ")} but its artifact does not ` + + `register ${missing.length === 1 ? "it" : "them"} — listed as protection that never runs`, + ); + } + if (extra.length > 0) { + hookLogWarn( + `pack ${pack.id}@${pack.version} registers undeclared ${extra.join(", ")} — ` + + `${extra.length === 1 ? "it enforces" : "they enforce"} but will not appear in listings`, + ); + } +} + export async function loadAllCustomHooks( customPoliciesPaths: string | string[] | undefined, opts?: { sessionCwd?: string; customPoliciesEnabled?: boolean; cloudManagedPolicies?: CloudManagedPolicyArtifact[]; + packs?: ResolvedPack[]; }, ): Promise { clearCustomHooks(); @@ -395,6 +431,30 @@ export async function loadAllCustomHooks( } } + // Installed packs, keyed by artifact path — the same content-addressing, and + // therefore the same collision, as the cloud map above: two packs whose entry + // file is byte-identical resolve to ONE artifact, `loadedPaths` imports it + // once, and the loser would vanish silently with its effect deciding nothing. + // Resolved the same way and for the same reason — toward ENFORCEMENT, because + // over-enforcing is visible to whoever hits it and under-enforcing is the + // silent failure — and announced, so an operator can act on it. + const packByPath = new Map(); + for (const pack of opts?.packs ?? []) { + const key = resolve(pack.path); + const existing = packByPath.get(key); + if (!existing) { + packByPath.set(key, pack); + continue; + } + hookLogWarn( + `packs ${existing.id} and ${pack.id} have identical source, so they share one artifact ` + + `and load as one pack; enforcing it if either asks to enforce`, + ); + if (existing.effect !== "enforce" && pack.effect === "enforce") { + packByPath.set(key, pack); + } + } + // 1. Explicit custom policy paths. Accept a string for callers/configs using // the legacy singular form. for (const customPoliciesPath of typeof customPoliciesPaths === "string" @@ -409,22 +469,33 @@ export async function loadAllCustomHooks( const hooksBefore = getCustomHooks().length; // A cloud-managed policy re-verifies its pinned digest at load, binding // the imported bytes to what desired-state promised. + // A pack re-verifies its pinned digest at load for the same reason a + // cloud policy does: the manifest read and the import are two moments, + // and only this one binds the bytes actually executed to what was + // promised. await loadSingleFile(absPath, { - verifyEntrySha: cloudManagedByPath.get(absPath)?.sha256, + verifyEntrySha: + cloudManagedByPath.get(absPath)?.sha256 ?? packByPath.get(absPath)?.sha256, }); for (const hook of getCustomHooks().slice(hooksBefore)) { const cloudManaged = cloudManagedByPath.get(absPath); + const pack = packByPath.get(absPath); const tagged = hook as CustomHook & { __policyId?: string; __cloudManaged?: CloudManagedPolicyArtifact; + __pack?: ResolvedPack; }; if (cloudManaged) { tagged.__cloudManaged = cloudManaged; tagged.__policyId = `cloud:${cloudManaged.id}@${cloudManaged.version}:${hook.name}`; + } else if (pack) { + tagged.__pack = pack; + tagged.__policyId = `pack:${pack.id}@${pack.version}:${hook.name}`; } else { tagged.__policyId = customPolicyId(absPath, hook.name); } } + reconcilePackManifest(packByPath.get(absPath), getCustomHooks().slice(hooksBefore)); } } else { hookLogWarn(`custom policy path not found: ${absPath}`); diff --git a/src/hooks/fp-home.ts b/src/hooks/fp-home.ts index a5e58d7c2..6b06b0351 100644 --- a/src/hooks/fp-home.ts +++ b/src/hooks/fp-home.ts @@ -210,6 +210,26 @@ export const globalPolicyConfigFile = (home?: string) => */ export const cloudPoliciesDir = (home?: string) => resolve(policiesDir(home), "cloud-policies"); +/** + * Installed policy packs — a sibling of `cloudPoliciesDir` and flat for the same + * reason: `artifacts/.mjs` is content-addressed, so an install can only + * ever write a file that does not exist yet and is structurally incapable of + * disturbing what is currently live. `installed.json` names which artifacts are + * active and is written last, so activation is one atomic flip. + * + * Living under `policies/` is safe ONLY because the convention loader does not + * recurse — `discoverPolicyFiles` and `findSkippedPolicyFiles` both filter on + * `isFile()`, so nothing under `packs/` can be picked up as an unverified + * convention policy. `fp-home.test.ts` pins that non-recursion. + */ +export const packsDir = (home?: string) => resolve(policiesDir(home), "packs"); + +/** The activation pointer. Written last and atomically. */ +export const packsInstalledFile = (home?: string) => resolve(packsDir(home), "installed.json"); + +/** Content-addressed pack artifacts, shared across packs and versions. */ +export const packArtifactsDir = (home?: string) => resolve(packsDir(home), "artifacts"); + // ── Collector ──────────────────────────────────────────────────────────────── /** Per-source watermarks. One directory per source — never shared: the cursor @@ -574,6 +594,13 @@ export const HOME_CLASSES: readonly { path: (home?: string) => string; class: Da // directory holds both the files a person wrote and the ones the fleet sent, // and only the second kind may be thrown away. { path: cloudPoliciesDir, class: "refetchable" }, + // Installed packs. Re-fetchable by `failproofai pack add` from the source and + // digest recorded in `installed.json`, so this is the fleet's argument again: + // one directory holds both what a person wrote and what a command fetched, and + // only the second kind may be thrown away. Without this row `packs/` inherits + // `policiesDir`'s `user-typed` and survives a reset that is supposed to clear + // re-fetchable state. + { path: packsDir, class: "refetchable" }, // ── Never touched ── // A downloaded daemon binary is large, version-pinned and re-verified on use, diff --git a/src/hooks/handler.ts b/src/hooks/handler.ts index 81606f16a..6f773be1d 100644 --- a/src/hooks/handler.ts +++ b/src/hooks/handler.ts @@ -43,6 +43,7 @@ import { getInstanceId } from "../../lib/telemetry-id"; import { hookLogInfo, hookLogWarn } from "./hook-logger"; import { readStdinPayload } from "./read-stdin"; import { readActiveCloudManagedPolicies, type CloudManagedPolicyArtifact } from "./cloud-managed-policies"; +import { readInstalledPacks, type ResolvedPack } from "./pack-manifest"; import { readActivePause, type ActivePause } from "./session-pause"; import { layoutWarningForHook } from "./fp-reset"; @@ -334,13 +335,30 @@ export async function evaluateHookEvent( // separate question from "what decided", and only the former can tell a // rollout that changed nothing from one that never reached the machine. cloudDeployment = cloudManagedPolicies[0]?.deployment; + // Installed packs. `readInstalledPacks` never throws: a bad manifest or a + // tampered artifact yields zero packs and a recorded reason, which is + // sound ONLY because the builtins still ship compiled in and keep + // enforcing underneath. See the fail-open note in pack-manifest.ts — the + // day builtins become a fetched pack this posture has to change with them. + let installedPacks: ResolvedPack[] = []; + try { + const packResult = readInstalledPacks(); + installedPacks = packResult.packs; + for (const err of packResult.errors) { + hookLogWarn(`pack ${err.id ?? "(unnamed)"} not loaded: ${err.reason}`); + } + } catch (err) { + hookLogWarn(`pack manifest unreadable: ${err instanceof Error ? err.message : String(err)}`); + } + const configuredCustomPaths = config.customPoliciesPaths ?? config.customPoliciesPath; const allExplicitPaths = - cloudManagedPolicies.length === 0 + cloudManagedPolicies.length === 0 && installedPacks.length === 0 ? configuredCustomPaths : [ ...(typeof configuredCustomPaths === "string" ? [configuredCustomPaths] : configuredCustomPaths ?? []), ...cloudManagedPolicies.map((policy) => policy.path), + ...installedPacks.map((pack) => pack.path), ]; // Load and register custom hooks (layer 2, after builtins) @@ -348,6 +366,7 @@ export async function evaluateHookEvent( sessionCwd: session.cwd, customPoliciesEnabled: config.customPoliciesEnabled, ...(cloudManagedPolicies.length > 0 ? { cloudManagedPolicies } : {}), + ...(installedPacks.length > 0 ? { packs: installedPacks } : {}), }); customHooksList = loadResult.hooks; const disabledCustomPolicies = new Set(config.disabledCustomPolicies ?? []); @@ -357,9 +376,11 @@ export async function evaluateHookEvent( const taggedHook = hook as CustomHook & { __policyId?: string; __cloudManaged?: CloudManagedPolicyArtifact; + __pack?: ResolvedPack; }; const policyId = taggedHook.__policyId; const cloudManaged = taggedHook.__cloudManaged; + const pack = taggedHook.__pack; // Local config cannot disable a centrally assigned policy merely by // copying its generated ID into disabledCustomPolicies. if (!cloudManaged && policyId && disabledCustomPolicies.has(policyId)) continue; @@ -368,16 +389,23 @@ export async function evaluateHookEvent( const hookName = hook.name; const conventionScope = (hook as CustomHook & { __conventionScope?: string }).__conventionScope; const isConvention = !!conventionScope; + // A pack's prefix carries its id and version, and it always contains a + // `/` — which is what keeps a pack policy structurally unable to + // normalize into the `failproofai/` namespace and REPLACE a builtin. + // pack-manifest.ts refuses a `/` in the declared name for the same + // reason; this is the second half of that guard. const prefix = cloudManaged ? `cloud/${cloudManaged.id}@${cloudManaged.version}` - : isConvention - ? `.failproofai-${conventionScope}` - : "custom"; + : pack + ? `pack/${pack.id}@${pack.version}` + : isConvention + ? `.failproofai-${conventionScope}` + : "custom"; // Observe mode: run it for real, record what it decided, then hand back // an allow. Evaluating and discarding is the whole point — a rollout is // measured against real traffic before it can break anyone's work, and // a policy that did not actually run would measure nothing. - const observeOnly = cloudManaged?.effect === "observe"; + const observeOnly = cloudManaged?.effect === "observe" || pack?.effect === "observe"; const fn: PolicyFunction = async (ctx): Promise => { if (observeOnly) { const shadow = await runObserved(hook, ctx, hookName, eventType, cli); diff --git a/src/hooks/pack-manifest.ts b/src/hooks/pack-manifest.ts new file mode 100644 index 000000000..65da877f7 --- /dev/null +++ b/src/hooks/pack-manifest.ts @@ -0,0 +1,251 @@ +/** + * Reads the installed policy packs a machine has on disk. + * + * A pack is the delivery unit for policies that did not ship compiled into this + * build: `failproofai pack add` writes the artifact and records it here, and the + * loader imports it through the same path custom policies already use. + * + * ## Why this is not `cloud-managed-policies.ts` + * + * It is modelled on it closely — same content-addressed artifacts, same + * verify-immediately-before-import boundary, same shared `resolveManagedPath`. + * Three things differ, each for a reason: + * + * - **Failure is PER PACK, not per manifest.** The cloud reader throws on the + * first bad entry, which is right for a fleet deployment: a manifest the + * server sent is one unit and half of it is not a deployment. Packs come from + * unrelated publishers, so one malformed third-party pack must not switch off + * every other pack on the machine. + * - **It never throws.** A caller gets `{packs, errors}`. See the fail-open note + * below — it is a load-bearing assumption, not a convenience. + * - **Two refusals cloud does not need**, both closing silent failures: + * a pack may not claim `alwaysOn`, and a pack policy name may not contain `/`. + * + * ## Fail-open, and the condition that makes it sound + * + * An unreadable manifest, a bad digest or a corrupt artifact yields zero packs + * and a recorded error — it does not deny. That is only defensible **while the + * builtin policies still ship compiled into the package and keep enforcing + * underneath**. The day builtins become a fetched pack, this exact behaviour + * becomes "zero enforcement on a machine reporting healthy", and this comment is + * the reason that must be revisited rather than inherited. + */ +import { createHash } from "node:crypto"; +import { existsSync, readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { packsDir, packsInstalledFile } from "./fp-home"; +import { resolveManagedPath } from "./cloud-managed-policies"; +import type { PolicyCatalogEntry } from "./policy-types"; +import type { PolicyEffect } from "./cloud-managed-policies"; + +/** Manifest schemas this reader accepts. */ +const ACCEPTED_PACK_SCHEMA_VERSIONS: readonly number[] = [1]; + +const SHA256_RE = /^[a-f0-9]{64}$/; +/** `/`, the shape a marketplace id takes. */ +const PACK_ID_RE = /^[A-Za-z0-9._-]{1,64}\/[A-Za-z0-9._-]{1,64}$/; +/** A version string, kept loose enough for semver and a tag. */ +const PACK_VERSION_RE = /^[A-Za-z0-9][A-Za-z0-9._+-]{0,63}$/; +/** + * A pack policy's own name. `/` is REFUSED, and that is the important character: + * `normalizePolicyName` passes any name containing one straight through, and + * `registerPolicy` REPLACES by canonical name — so a pack shipping a policy + * literally called `failproofai/block-sudo` would silently overwrite the + * compiled builtin with its own code. Names here are prefixed by the loader + * instead, which is what keeps a pack unable to reach the builtin namespace. + */ +const PACK_POLICY_NAME_RE = /^[A-Za-z0-9._-]{1,128}$/; + +export interface InstalledPackRecord { + id: string; + version: string; + /** Where it came from, verbatim, e.g. `github:acme/pack@v2.1.0`. */ + source: string; + /** Path to the single entry artifact, relative to `packsDir()`. */ + entry: string; + sha256: string; + effect?: string; + policies?: unknown; +} + +export interface ResolvedPack { + id: string; + version: string; + source: string; + /** Absolute, real, verified-to-be-inside-the-root path to the entry module. */ + path: string; + sha256: string; + effect: PolicyEffect; + /** The pack's own catalog, in declared order. */ + policies: PolicyCatalogEntry[]; +} + +export interface PackError { + /** The pack's id when it was readable enough to have one. */ + id: string | null; + reason: string; +} + +export interface PackReadResult { + packs: ResolvedPack[]; + /** Every pack that was declared and refused, and why. Never silently dropped. */ + errors: PackError[]; +} + +export function packsRoot(): string { + return process.env.FAILPROOFAI_PACK_DIR ?? packsDir(); +} + +function installedFilePath(): string { + return process.env.FAILPROOFAI_PACK_DIR + ? resolve(process.env.FAILPROOFAI_PACK_DIR, "installed.json") + : packsInstalledFile(); +} + +/** Validate one serialized catalog entry carried by a pack. */ +function parsePackPolicy(packId: string, value: unknown, index: number): PolicyCatalogEntry { + const where = `${packId} policy #${index}`; + if (!value || typeof value !== "object") throw new Error(`${where} is not an object`); + const raw = value as Record; + + const name = raw.name; + if (typeof name !== "string" || !PACK_POLICY_NAME_RE.test(name)) { + throw new Error(`${where} has an unsafe name ${JSON.stringify(name)}`); + } + if ("alwaysOn" in raw) { + // `alwaysOn` means "registers regardless of the enabled set, and cannot be + // disabled or paused". It is reserved for the guard that stops an agent + // switching failproofai off, which ships compiled in. A pack granting it to + // itself would be a downloaded file that no local command can turn off. + throw new Error(`${where} declares alwaysOn, which packs may not set`); + } + if (typeof raw.description !== "string" || raw.description.length === 0) { + throw new Error(`${where} is missing a description`); + } + if (typeof raw.category !== "string" || raw.category.length === 0) { + throw new Error(`${where} is missing a category`); + } + if (typeof raw.defaultEnabled !== "boolean") { + throw new Error(`${where} is missing defaultEnabled`); + } + if (!raw.match || typeof raw.match !== "object") { + throw new Error(`${where} is missing a match`); + } + return raw as unknown as PolicyCatalogEntry; +} + +function parsePack(root: string, value: unknown): ResolvedPack { + if (!value || typeof value !== "object") throw new Error("pack entry is not an object"); + const raw = value as InstalledPackRecord; + + if (typeof raw.id !== "string" || !PACK_ID_RE.test(raw.id)) { + throw new Error(`unsafe pack id ${JSON.stringify(raw.id)}`); + } + if (typeof raw.version !== "string" || !PACK_VERSION_RE.test(raw.version)) { + throw new Error(`invalid version for pack ${raw.id}`); + } + if (typeof raw.source !== "string" || raw.source.length === 0) { + throw new Error(`pack ${raw.id} has no source`); + } + if (typeof raw.sha256 !== "string" || !SHA256_RE.test(raw.sha256)) { + throw new Error(`invalid SHA-256 for pack ${raw.id}`); + } + if (raw.effect !== undefined && raw.effect !== "enforce" && raw.effect !== "observe") { + // Same reasoning as cloud: guessing means either enforcing what was not + // asked for, or observing what was meant to enforce. + throw new Error(`unknown effect ${JSON.stringify(raw.effect)} for pack ${raw.id}`); + } + + const path = resolveManagedPath(root, raw.entry); + const actual = createHash("sha256").update(readFileSync(path)).digest("hex"); + if (actual !== raw.sha256) { + throw new Error( + `pack ${raw.id} failed integrity verification: expected ${raw.sha256}, got ${actual}`, + ); + } + + if (!Array.isArray(raw.policies)) throw new Error(`pack ${raw.id} policies is not an array`); + const policies = raw.policies.map((p, i) => parsePackPolicy(raw.id, p, i)); + const names = new Set(); + for (const p of policies) { + if (names.has(p.name)) throw new Error(`pack ${raw.id} declares ${p.name} twice`); + names.add(p.name); + } + + return { + id: raw.id, + version: raw.version, + source: raw.source, + path, + sha256: raw.sha256, + effect: (raw.effect as PolicyEffect | undefined) ?? "enforce", + policies, + }; +} + +/** + * Read every installed pack, verifying each artifact's digest immediately before + * its path is handed to a caller that will import it. + * + * Never throws. A pack that fails any check is omitted and recorded in `errors`; + * a manifest that cannot be read at all yields no packs and one error. + */ +export function readInstalledPacks(): PackReadResult { + const manifestPath = installedFilePath(); + if (!existsSync(manifestPath)) return { packs: [], errors: [] }; + + let raw: unknown; + try { + raw = JSON.parse(readFileSync(manifestPath, "utf8")); + } catch (err) { + return { + packs: [], + errors: [{ id: null, reason: `unreadable pack manifest: ${errText(err)}` }], + }; + } + if (!raw || typeof raw !== "object") { + return { packs: [], errors: [{ id: null, reason: "pack manifest is not an object" }] }; + } + const manifest = raw as { schemaVersion?: unknown; packs?: unknown }; + if (!ACCEPTED_PACK_SCHEMA_VERSIONS.includes(manifest.schemaVersion as number)) { + return { + packs: [], + errors: [ + { + id: null, + reason: + `unsupported pack manifest schema ${String(manifest.schemaVersion)} ` + + `(supported: ${ACCEPTED_PACK_SCHEMA_VERSIONS.join(", ")})`, + }, + ], + }; + } + if (!Array.isArray(manifest.packs)) { + return { packs: [], errors: [{ id: null, reason: "pack manifest packs is not an array" }] }; + } + + const root = packsRoot(); + const packs: ResolvedPack[] = []; + const errors: PackError[] = []; + const seen = new Set(); + + for (const entry of manifest.packs) { + const declaredId = + entry && typeof entry === "object" && typeof (entry as InstalledPackRecord).id === "string" + ? (entry as InstalledPackRecord).id + : null; + try { + const pack = parsePack(root, entry); + if (seen.has(pack.id)) throw new Error(`duplicate pack id ${pack.id}`); + seen.add(pack.id); + packs.push(pack); + } catch (err) { + errors.push({ id: declaredId, reason: errText(err) }); + } + } + return { packs, errors }; +} + +function errText(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} From 29836c20676c384e3944813bb2da8916ebb4f971 Mon Sep 17 00:00:00 2001 From: chhhee10 Date: Thu, 20 Aug 2026 20:34:33 +0530 Subject: [PATCH 04/17] Let a policy that is not a builtin declare params that actually arrive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The params schema now travels on the RegisteredPolicy, next to `match`, instead of being looked up by name in a map built from BUILTIN_POLICIES. That map could only ever describe policies compiled into this build, so every pack policy, cloud assignment and custom hook fell through to the branch that never calls getConfigParamsFor. The consequence was worse than missing defaults: the user's OWN configured policyParams for those policies were discarded. A person who set protectedBranches on a cloud-assigned policy had it silently ignored, with nothing anywhere reporting it. A schema-less policy now receives whatever the user configured, and still `{}` when they configured nothing — which is every case that exists today. A policy declaring a schema gets defaults merged under the user's values, unchanged. Registration-carried rather than name-keyed also closes a hole the pack lane opened one commit ago: a name-keyed schema was handed to ANYTHING registered under that name, so a pack that took the `block-sudo` name would have inherited its params along with it. policy-evaluator.ts no longer imports the builtin catalog. That is a module graph change, which is the condition that can shift emitted text and move the audit cache key, so it was re-measured rather than assumed: the shipped engineVersion is still c1cea4ddf3030af4. --- CHANGELOG.md | 2 ++ __tests__/hooks/policy-evaluator.test.ts | 38 ++++++++++++++++++++++-- src/hooks/builtin-policies.ts | 2 +- src/hooks/handler.ts | 5 ++++ src/hooks/policy-evaluator.ts | 23 +++++++------- src/hooks/policy-registry.ts | 10 +++++-- src/hooks/policy-types.ts | 8 +++++ 7 files changed, 72 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e47570bd1..d02da8d3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features +- Carry a policy's params schema on the policy itself rather than looking it up by name in a map built from the builtin catalog, so a pack, a cloud assignment or a custom file can declare params that actually arrive. `POLICY_PARAMS_MAP` could only ever describe policies compiled into this build, so everything else fell through to a branch that never called `getConfigParamsFor` — which discarded the user's OWN configured `policyParams` for those policies, not merely the schema defaults. Someone who set `protectedBranches` on a cloud-assigned policy had it silently ignored. A schema-less policy now receives whatever the user configured (and still `{}` when they configured nothing, which is every existing case), and a policy that declares a schema gets defaults merged under the user's values exactly as before. The lookup moving from name-keyed to registration-carried also closes a hole the pack work opened: a name-keyed schema was handed to ANYTHING registered under that name, so a pack taking the `block-sudo` name would have inherited its params too. `policy-evaluator.ts` no longer imports the builtin catalog at all — the module graph change was re-measured and the shipped `engineVersion` is still `c1cea4ddf3030af4`. (#PR) + - Build the policy pack lane: the delivery path for policies that did not ship compiled into the build. A pack is one digest-pinned entry artifact plus a manifest describing what it contains, installed under `~/.failproofai/policies/packs/` as a sibling of the cloud artifacts and loaded through the custom-policy loader that already exists rather than a fourth loader. Packs are LOCAL policy: unlike cloud assignments they stay disableable via `disabledCustomPolicies` and are suspended by a session pause, because cloud's exemptions exist so a locally-issued command cannot switch off a CENTRALLY assigned policy and a pack the user installed by typing a command is not that. Three refusals close silent failures. A pack policy name may not contain `/`, and a pack's registered policies are prefixed `pack/@/` — verified live that without this a pack shipping the name `failproofai/block-sudo` REPLACES the compiled builtin, since `normalizePolicyName` passes any name containing a slash straight through and `registerPolicy` replaces by canonical name; the machine would then report `block-sudo` as enabled while running a stranger's code. A pack may not declare `alwaysOn`, which would be downloaded enforcement no local command can turn off. And byte-identical packs are merged toward enforcement with a warning, because content-addressed artifacts mean identical source is one file on disk and the loser would otherwise vanish with its effect deciding nothing — the same collision that once silently downgraded a cloud policy to observe-only. Manifest and artifact are reconciled after load and any disagreement is named: a declared policy the artifact never registers is a listing claiming protection that does not run. `engineVersion`, which keys the audit cache, now folds in each pack's `id|version|sha256` — by identity rather than source text, since the loader rewrites a per-load temporary filename into every import specifier and hashing that would cold-rescan the entire history on every run. A machine with no packs hashes byte-identically to a build without pack support, verified at both the source level and in the shipped bundle (`c1cea4ddf3030af4`, unchanged), so shipping this costs no existing user the ~104-second rescan. Failure is per pack, not per manifest, and fails open with a recorded reason — which is sound only while the builtins still ship compiled in and keep enforcing underneath, and is commented as such at the catch. (#PR) - Split the builtin policy catalog from its implementations, the first step of moving policies out of the npm package. `policy-catalog.ts` now holds the metadata every consumer actually reads — name, description, category, `match`, `defaultEnabled`, `params` — as pure literal data with no functions in it, and `builtin-policies.ts` keeps the 39 implementations and joins them back on, so `BUILTIN_POLICIES` has exactly the shape, fields and ORDER it always had and not one of its nine source consumers changes. That is what will let a machine list, search and render the catalog offline once the executable half moves to a fetched pack. Two things made the refactor narrower than it looks. `audit/cache.ts` hashes `fn.toString()` for all 39 policies into the audit cache's `engineVersion`, and `bun build` renames colliding top-level identifiers by module EMISSION ORDER — renamed names appear inside policy bodies in the shipped bundle (`cwdWithSep2`, `execSync2`, `resolved3`) — so inserting a module into the graph could have changed the emitted text, invalidated every user's audit cache and forced a ~104-second cold rescan on upgrade. Measured before and after: `engineVersion` is unchanged (`c1cea4ddf3030af4`), so nobody's cache moves. And `SECRET_PATTERNS` stays in `builtin-policies.ts` rather than being reclassified as catalog data: it is assembled from the very RegExps the five `sanitize-*` policies test against and is imported by the audit redactor, so moving it would have forced a catalog→implementation value edge and put an import cycle on the hook critical path. `policy-catalog.test.ts` pins the join against the failures that are otherwise silent — a wrapper collapsing 39 distinct `fn.toString()` hashes into one and freezing the cache key, a sort or regroup changing which policy name gets attributed on a deny, a spread default-filling `beta`, a dropped row shrinking the catalog invisibly to `manager.ts` and `install-prompt.ts` (neither reads `.fn`) — plus a JSON round-trip asserting the catalog is serializable and a source-level guard that it never value-imports back. A bijection check throws at module load rather than warning, because a name with no implementation yields `fn: undefined`, whose `TypeError` `policy-evaluator.ts` swallows: the hook would allow, exit 0, and still report the policy in `matchedPolicies`. (#PR) diff --git a/__tests__/hooks/policy-evaluator.test.ts b/__tests__/hooks/policy-evaluator.test.ts index 3f98809eb..15a63cce8 100644 --- a/__tests__/hooks/policy-evaluator.test.ts +++ b/__tests__/hooks/policy-evaluator.test.ts @@ -588,11 +588,14 @@ describe("hooks/policy-evaluator", () => { const { BUILTIN_POLICIES } = await import("../../src/hooks/builtin-policies"); const orig = BUILTIN_POLICIES.find((p) => p.name === "block-sudo")!; - // Wrap the original fn to capture params + // The schema is passed AT REGISTRATION now, not looked up by name. That + // is what lets a pack or cloud policy declare params at all — and it + // closes a hole: a name-keyed lookup handed `block-sudo`'s schema to + // ANYTHING registered under that name, including a pack that took it. registerPolicy("block-sudo", orig.description, async (ctx) => { capturedParams = ctx.params; return { decision: "allow" }; - }, orig.match); + }, orig.match, 0, orig.params); await evaluatePolicies("PreToolUse", { tool_name: "Bash", tool_input: { command: "ls" } }, undefined, { enabledPolicies: ["block-sudo"] }); @@ -600,6 +603,37 @@ describe("hooks/policy-evaluator", () => { expect((capturedParams as Record).allowPatterns).toEqual([]); }); + it("gives a policy that declares NO schema the user's configured params", async () => { + // Previously every schema-less policy — every custom hook, every cloud + // assignment, and every pack policy — received `{}`, so a user who + // configured params for one had them silently discarded. Not just the + // defaults: what they had explicitly written. + let captured: unknown = null; + registerPolicy("failproofai/no-schema", "d", async (ctx) => { + captured = ctx.params; + return { decision: "allow" }; + }, { events: ["PreToolUse"] }); + + await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, { + enabledPolicies: [], + policyParams: { "no-schema": { threshold: 7 } }, + } as never); + + expect(captured).toEqual({ threshold: 7 }); + }); + + it("still gives a schema-less policy {} when nothing is configured", async () => { + // The overwhelmingly common case must be unchanged. + let captured: unknown = null; + registerPolicy("failproofai/no-schema-2", "d", async (ctx) => { + captured = ctx.params; + return { decision: "allow" }; + }, { events: ["PreToolUse"] }); + + await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, { enabledPolicies: [] }); + expect(captured).toEqual({}); + }); + it("overrides schema defaults with policyParams from config", async () => { let capturedParams: unknown = null; const { BUILTIN_POLICIES } = await import("../../src/hooks/builtin-policies"); diff --git a/src/hooks/builtin-policies.ts b/src/hooks/builtin-policies.ts index 5e9347805..51677ab03 100644 --- a/src/hooks/builtin-policies.ts +++ b/src/hooks/builtin-policies.ts @@ -2199,7 +2199,7 @@ export function registerBuiltinPolicies(enabledNames: string[]): void { // A guard against the agent disabling failproofai that any of those can // switch off is not a guard. if (policy.alwaysOn || enabledSet.has(normalizePolicyName(policy.name))) { - registerPolicy(policy.name, policy.description, policy.fn, policy.match); + registerPolicy(policy.name, policy.description, policy.fn, policy.match, 0, policy.params); } } } diff --git a/src/hooks/handler.ts b/src/hooks/handler.ts index 6f773be1d..5a02c0b07 100644 --- a/src/hooks/handler.ts +++ b/src/hooks/handler.ts @@ -456,6 +456,11 @@ export async function evaluateHookEvent( fn, hook.match ?? {}, -1, // Custom hooks run after builtins (priority 0) + // A pack declares its policies' params in its manifest, so they work + // exactly like a builtin's — defaults merged under whatever the user + // configured. Matched by the pack's own name for the policy, which is + // the name before the `pack/@/` prefix is applied. + pack?.policies.find((p) => p.name === hook.name)?.params, ); } diff --git a/src/hooks/policy-evaluator.ts b/src/hooks/policy-evaluator.ts index cce0235b7..4e8726083 100644 --- a/src/hooks/policy-evaluator.ts +++ b/src/hooks/policy-evaluator.ts @@ -4,8 +4,7 @@ */ import type { HookEventType, SessionMetadata } from "./types"; import type { PolicyContext, HooksConfig } from "./policy-types"; -import { BUILTIN_POLICIES } from "./builtin-policies"; -import { DEFAULT_POLICY_NAMESPACE, getPoliciesForEvent, normalizePolicyName } from "./policy-registry"; +import { DEFAULT_POLICY_NAMESPACE, getPoliciesForEvent } from "./policy-registry"; import { hookLogInfo, hookLogWarn } from "./hook-logger"; import { trackHookEvent } from "./hook-telemetry"; import { getInstanceId } from "../../lib/telemetry-id"; @@ -28,11 +27,6 @@ export interface EvaluationResult { decision: "allow" | "deny" | "instruct"; } -// Build a map from canonical policy name to its params schema (for injecting defaults). -// Keyed by canonical name because registered policies always carry the canonical form. -const POLICY_PARAMS_MAP = new Map( - BUILTIN_POLICIES.filter((p) => p.params).map((p) => [normalizePolicyName(p.name), p.params!]), -); /** * Look up policy params for a canonical policy name in the user config, @@ -91,18 +85,25 @@ export async function evaluatePolicies( // Inject params: merge policyParams[policy.name] over schema defaults. // policy.name is canonical (e.g. "failproofai/block-force-push"); user // config keys may be flat or canonical — getConfigParamsFor accepts both. - const schema = POLICY_PARAMS_MAP.get(policy.name); + // The schema comes off the REGISTERED policy, so a pack's or a cloud + // assignment's declared params work exactly like a builtin's. It used to be + // looked up in a map built from the builtin catalog, which could only ever + // describe policies compiled into this build. + const schema = policy.params; + const userParams = getConfigParamsFor(config, policy.name) ?? {}; let ctx: PolicyContext; if (schema) { - const userParams = getConfigParamsFor(config, policy.name) ?? {}; const resolvedParams: Record = {}; for (const [key, spec] of Object.entries(schema)) { resolvedParams[key] = key in userParams ? userParams[key] : spec.default; } ctx = { ...baseCtx, params: resolvedParams }; } else { - // Custom hooks and policies without schema get empty params - ctx = { ...baseCtx, params: {} }; + // No schema means no defaults to merge — but the user may still have + // configured params for it, and silently dropping what they wrote is how + // a policy ends up ignoring its own configuration. Absent config still + // yields `{}`, which is what every schema-less policy saw before. + ctx = { ...baseCtx, params: userParams }; } let result: Awaited>; diff --git a/src/hooks/policy-registry.ts b/src/hooks/policy-registry.ts index d417f80eb..c314b7618 100644 --- a/src/hooks/policy-registry.ts +++ b/src/hooks/policy-registry.ts @@ -6,7 +6,7 @@ * chunk splitting and remains a true singleton across dynamic imports. */ import type { HookEventType } from "./types"; -import type { PolicyFunction, PolicyMatcher, RegisteredPolicy } from "./policy-types"; +import type { PolicyFunction, PolicyMatcher, PolicyParamsSchema, RegisteredPolicy } from "./policy-types"; const REGISTRY_KEY = "__FAILPROOFAI_POLICY_REGISTRY__"; const INDEX_CACHE_KEY = "__FAILPROOFAI_POLICY_INDEX_CACHE__"; @@ -57,11 +57,17 @@ export function registerPolicy( fn: PolicyFunction, match: PolicyMatcher, priority: number = 0, + params?: PolicyParamsSchema, ): void { const canonical = normalizePolicyName(name); const registry = getRegistry(); const idx = registry.findIndex((p) => p.name === canonical); - const entry: RegisteredPolicy = { name: canonical, description, fn, match, priority }; + const entry: RegisteredPolicy = { + name: canonical, description, fn, match, priority, + // Absent stays absent: `evaluatePolicies` distinguishes "declares a schema" + // from "declares none", and a spread `params: undefined` is neither. + ...(params ? { params } : {}), + }; if (idx >= 0) { registry[idx] = entry; } else { diff --git a/src/hooks/policy-types.ts b/src/hooks/policy-types.ts index a61796a2e..f4b0a73ee 100644 --- a/src/hooks/policy-types.ts +++ b/src/hooks/policy-types.ts @@ -35,6 +35,14 @@ export interface RegisteredPolicy { fn: PolicyFunction; match: PolicyMatcher; priority: number; + /** + * The policy's declared params, carried HERE rather than looked up in a map + * built from the builtin catalog. That map could only ever describe policies + * compiled into this build, so anything arriving from a pack, a cloud + * assignment or a custom file fell through to `params: {}` — which discarded + * the user's OWN configured `policyParams` for it, not merely the defaults. + */ + params?: PolicyParamsSchema; } export interface PolicyParamsSchema { From 01013720d46c4047e23c86b05a7126016462c047 Mon Sep 17 00:00:00 2001 From: chhhee10 Date: Fri, 21 Aug 2026 11:06:44 +0530 Subject: [PATCH 05/17] Put a pack on a machine, and refuse the ones that could not work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `failproofai pack add | remove | list`. A pack is fetched from a GitHub release by github:owner/repo@tag, verified against that release's SHA256SUMS, written to a content-addressed artifact, and activated by an installed.json written LAST and atomically — so until that rename lands, the downloaded file is one nothing points at. The tag is required and there is no `latest`. A moving source would change what a machine enforces whenever the publisher pushed, which is the drift the recorded digest exists to prevent. Every URL is constructed from what the user typed: no API call, no releases/latest redirect to follow, no rate limit, and no way to end up holding an artifact from a source nobody named. What the verification buys, stated precisely: SHA256SUMS ships in the same release as the artifact, so it is not a signature and proves nothing about publisher identity. It proves the bytes are the ones that release published — and because the digest is recorded at add time and re-verified before every import, a pack cannot change under a machine afterwards. A repository that retags or replaces an asset stops loading rather than silently running something else. Signing was considered and deliberately deferred. A pack is validated with the LOADER's own rules at add time, while nothing has been written. So a pack declaring alwaysOn, or a policy name that would reach the failproofai/ namespace and replace a builtin, is refused before it can install cleanly and then fail silently on the next tool call. `--only a,b` takes part of a pack. The choice is stored per-pack rather than as disabledCustomPolicies entries, because those are keyed by pack:@: — an upgrade stops matching them and everything the user deliberately left off comes quietly back on. Re-adding at a newer version carries the selection forward. `pack list` marks every policy on or off, including the ones not taken, and exits non-zero naming any installed pack that will not load: a machine enforcing less than its manifest claims is the state a person most needs told about. FAILPROOFAI_NO_DOWNLOAD refuses to fetch while installed packs keep enforcing. FAILPROOFAI_PACK_BASE_URL points at a mirror, or at the local HTTP server the tests serve a real release layout from. --- CHANGELOG.md | 2 + __tests__/hooks/pack-cli.test.ts | 136 ++++++++++++ __tests__/hooks/pack-loading.test.ts | 1 + __tests__/hooks/pack-store.test.ts | 238 ++++++++++++++++++++ bin/failproofai.mjs | 61 ++++- src/hooks/handler.ts | 4 + src/hooks/pack-cli.ts | 104 +++++++++ src/hooks/pack-manifest.ts | 26 ++- src/hooks/pack-store.ts | 321 +++++++++++++++++++++++++++ 9 files changed, 891 insertions(+), 2 deletions(-) create mode 100644 __tests__/hooks/pack-cli.test.ts create mode 100644 __tests__/hooks/pack-store.test.ts create mode 100644 src/hooks/pack-cli.ts create mode 100644 src/hooks/pack-store.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d02da8d3b..dc9a5e704 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features +- Add `failproofai pack add | remove | list`, the command that puts a policy pack on a machine. A pack is fetched from a GitHub release by `github:owner/repo@tag`, verified against that release's `SHA256SUMS`, written to a content-addressed artifact, and activated by an `installed.json` that is written LAST and atomically — so until that rename lands the downloaded file is one nothing points at. The tag is REQUIRED and there is no `latest`: a moving source would change what a machine enforces whenever the publisher pushed, which is the drift the recorded digest exists to prevent. Every URL is constructed from what the user typed — no API call, no `releases/latest` redirect, no rate limit, no way to end up holding an artifact from a source nobody named. Be precise about what this buys: `SHA256SUMS` ships in the same release as the artifact, so it is not a signature and proves nothing about publisher identity; what it does prove is that the bytes are the ones the release published, and because the digest is recorded at add time and re-verified before every import, a pack cannot change under a machine afterwards — a repository that retags or replaces an asset stops loading instead of silently running something else. Signing was considered and deferred. A pack is validated with the loader's own rules at ADD time, while nothing has been written, so a pack declaring `alwaysOn` or a policy name that would reach the `failproofai/` namespace is refused before it can install cleanly and fail silently on the next tool call. `--only a,b` takes part of a pack, the choice is stored per-pack rather than as `disabledCustomPolicies` entries (those are keyed by version, so an upgrade would stop matching and quietly switch back on everything the user left off), and re-adding at a newer version carries the selection forward. `pack list` marks every policy on or off — including the ones not taken — and exits non-zero naming any installed pack that will not load, because a machine enforcing less than its manifest claims is the state a person most needs told about. `FAILPROOFAI_NO_DOWNLOAD` refuses to fetch while already-installed packs keep enforcing; `FAILPROOFAI_PACK_BASE_URL` points the whole thing at a mirror or, in the tests, a local HTTP server serving a real release layout. (#PR) + - Carry a policy's params schema on the policy itself rather than looking it up by name in a map built from the builtin catalog, so a pack, a cloud assignment or a custom file can declare params that actually arrive. `POLICY_PARAMS_MAP` could only ever describe policies compiled into this build, so everything else fell through to a branch that never called `getConfigParamsFor` — which discarded the user's OWN configured `policyParams` for those policies, not merely the schema defaults. Someone who set `protectedBranches` on a cloud-assigned policy had it silently ignored. A schema-less policy now receives whatever the user configured (and still `{}` when they configured nothing, which is every existing case), and a policy that declares a schema gets defaults merged under the user's values exactly as before. The lookup moving from name-keyed to registration-carried also closes a hole the pack work opened: a name-keyed schema was handed to ANYTHING registered under that name, so a pack taking the `block-sudo` name would have inherited its params too. `policy-evaluator.ts` no longer imports the builtin catalog at all — the module graph change was re-measured and the shipped `engineVersion` is still `c1cea4ddf3030af4`. (#PR) - Build the policy pack lane: the delivery path for policies that did not ship compiled into the build. A pack is one digest-pinned entry artifact plus a manifest describing what it contains, installed under `~/.failproofai/policies/packs/` as a sibling of the cloud artifacts and loaded through the custom-policy loader that already exists rather than a fourth loader. Packs are LOCAL policy: unlike cloud assignments they stay disableable via `disabledCustomPolicies` and are suspended by a session pause, because cloud's exemptions exist so a locally-issued command cannot switch off a CENTRALLY assigned policy and a pack the user installed by typing a command is not that. Three refusals close silent failures. A pack policy name may not contain `/`, and a pack's registered policies are prefixed `pack/@/` — verified live that without this a pack shipping the name `failproofai/block-sudo` REPLACES the compiled builtin, since `normalizePolicyName` passes any name containing a slash straight through and `registerPolicy` replaces by canonical name; the machine would then report `block-sudo` as enabled while running a stranger's code. A pack may not declare `alwaysOn`, which would be downloaded enforcement no local command can turn off. And byte-identical packs are merged toward enforcement with a warning, because content-addressed artifacts mean identical source is one file on disk and the loser would otherwise vanish with its effect deciding nothing — the same collision that once silently downgraded a cloud policy to observe-only. Manifest and artifact are reconciled after load and any disagreement is named: a declared policy the artifact never registers is a listing claiming protection that does not run. `engineVersion`, which keys the audit cache, now folds in each pack's `id|version|sha256` — by identity rather than source text, since the loader rewrites a per-load temporary filename into every import specifier and hashing that would cold-rescan the entire history on every run. A machine with no packs hashes byte-identically to a build without pack support, verified at both the source level and in the shipped bundle (`c1cea4ddf3030af4`, unchanged), so shipping this costs no existing user the ~104-second rescan. Failure is per pack, not per manifest, and fails open with a recorded reason — which is sound only while the builtins still ship compiled in and keep enforcing underneath, and is commented as such at the catch. (#PR) diff --git a/__tests__/hooks/pack-cli.test.ts b/__tests__/hooks/pack-cli.test.ts new file mode 100644 index 000000000..c93b7fe0d --- /dev/null +++ b/__tests__/hooks/pack-cli.test.ts @@ -0,0 +1,136 @@ +// @vitest-environment node +/** + * The `pack` command's OUTPUT, which is the only thing this layer owns. + * + * Two behaviours here are deliberate rather than incidental: a partially-taken + * pack names what it left out (a count reads as fine right up until someone + * discovers which ones), and a pack that is installed but refuses to load exits + * NON-ZERO — the machine is enforcing less than its manifest claims, which is + * the state a person most needs told about. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { createHash } from "node:crypto"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { runPackCommand } from "@/src/hooks/pack-cli"; + +const ARTIFACT = "export const hooks = [];\n"; +const DIGEST = createHash("sha256").update(ARTIFACT).digest("hex"); +const POLICIES = [ + { name: "block-big-refund", description: "Block big refunds", category: "Finance", defaultEnabled: true, match: {} }, + { name: "require-note", description: "Require a note", category: "Finance", defaultEnabled: true, match: {} }, +]; + +let root: string; +let prev: string | undefined; + +function install(over: Record = {}): void { + writeFileSync( + join(root, "installed.json"), + JSON.stringify({ + schemaVersion: 1, + packs: [{ + id: "acme/finance", version: "1.2.0", source: "github:acme/finance@v1.2.0", + entry: `artifacts/${DIGEST}.mjs`, sha256: DIGEST, policies: POLICIES, ...over, + }], + }), + ); +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "fpai-pack-cli-")); + mkdirSync(join(root, "artifacts"), { recursive: true }); + writeFileSync(join(root, "artifacts", `${DIGEST}.mjs`), ARTIFACT); + prev = process.env.FAILPROOFAI_PACK_DIR; + process.env.FAILPROOFAI_PACK_DIR = root; +}); + +afterEach(() => { + if (prev === undefined) delete process.env.FAILPROOFAI_PACK_DIR; + else process.env.FAILPROOFAI_PACK_DIR = prev; + rmSync(root, { recursive: true, force: true }); +}); + +const text = (r: { lines: string[] }) => r.lines.join("\n"); + +describe("pack list", () => { + it("tells a user with no packs how to get one", async () => { + const r = await runPackCommand(["list"]); + expect(r.exitCode).toBe(0); + expect(text(r)).toContain("No packs installed."); + expect(text(r)).toContain("pack add github:owner/repo@tag"); + }); + + it("marks every policy on or off, including the ones not taken", async () => { + install({ enabled: ["block-big-refund"] }); + const r = await runPackCommand(["list"]); + expect(r.exitCode).toBe(0); + expect(text(r)).toContain("on block-big-refund"); + expect(text(r)).toContain("off require-note"); + expect(text(r)).toContain("github:acme/finance@v1.2.0"); + }); + + it("exits non-zero and names a pack that will not load", async () => { + // Enforcing less than the manifest claims. Reporting success here is how a + // machine ends up quietly unprotected. + install({ sha256: "0".repeat(64) }); + const r = await runPackCommand(["list"]); + expect(r.exitCode).toBe(1); + expect(text(r)).toContain("NOT LOADED"); + expect(text(r)).toContain("acme/finance"); + }); + + it("defaults to list with no subcommand", async () => { + expect((await runPackCommand([])).exitCode).toBe(0); + }); +}); + +describe("pack remove", () => { + it("removes an installed pack", async () => { + install(); + const r = await runPackCommand(["remove", "acme/finance"]); + expect(r.exitCode).toBe(0); + expect(text(r)).toContain("Removed acme/finance"); + expect(text(await runPackCommand(["list"]))).toContain("No packs installed."); + }); + + it("fails on an id that is not installed", async () => { + const r = await runPackCommand(["remove", "nope/nope"]); + expect(r.exitCode).toBe(1); + expect(text(r)).toContain("No installed pack with id nope/nope"); + }); + + it("needs an id", async () => { + expect((await runPackCommand(["remove"])).exitCode).toBe(1); + }); +}); + +describe("pack add usage", () => { + it("needs a source", async () => { + const r = await runPackCommand(["add"]); + expect(r.exitCode).toBe(1); + expect(text(r)).toContain("Usage:"); + }); + + it("rejects an empty --only rather than silently taking everything", async () => { + const r = await runPackCommand(["add", "github:a/b@v1", "--only"]); + expect(r.exitCode).toBe(1); + expect(text(r)).toContain("at least one policy name"); + }); + + it("reports a bad source as a failure, not a crash", async () => { + const r = await runPackCommand(["add", "not-a-source"]); + expect(r.exitCode).toBe(1); + expect(text(r)).toContain("Could not install pack"); + }); +}); + +describe("unknown subcommand", () => { + it("lists what it accepts", async () => { + const r = await runPackCommand(["frobnicate"]); + expect(r.exitCode).toBe(1); + expect(text(r)).toContain("add, remove, list"); + }); +}); diff --git a/__tests__/hooks/pack-loading.test.ts b/__tests__/hooks/pack-loading.test.ts index 64cbd2e4a..9291f9b9a 100644 --- a/__tests__/hooks/pack-loading.test.ts +++ b/__tests__/hooks/pack-loading.test.ts @@ -47,6 +47,7 @@ function packRecord(over: Partial = {}): ResolvedPack { sha256: SHA, effect: "enforce", policies: [], + enabled: null, ...over, }; } diff --git a/__tests__/hooks/pack-store.test.ts b/__tests__/hooks/pack-store.test.ts new file mode 100644 index 000000000..1750b92dc --- /dev/null +++ b/__tests__/hooks/pack-store.test.ts @@ -0,0 +1,238 @@ +// @vitest-environment node +/** + * `pack add`, driven against a real HTTP server serving a real release layout. + * + * The interesting assertions are the refusals, and specifically WHEN they + * happen: a pack that could never load must be refused while nothing has been + * written, not installed cleanly and then found broken on the next tool call. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { createServer, type Server } from "node:http"; +import { createHash } from "node:crypto"; +import { mkdtempSync, readFileSync, existsSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { AddressInfo } from "node:net"; + +import { + addPack, removePack, parsePackSpec, packAssetUrl, formatPackSpec, digestFor, +} from "@/src/hooks/pack-store"; +import { readInstalledPacks } from "@/src/hooks/pack-manifest"; + +const ENTRY = ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ name: "block-big-refund", description: "d", + match: { events: ["PreToolUse"] }, fn: async () => deny("no") }); +`; + +const POLICY = { + name: "block-big-refund", + description: "Block refunds above the approved limit", + category: "Finance", + defaultEnabled: true, + match: { events: ["PreToolUse"] }, +}; +const POLICY_2 = { ...POLICY, name: "require-approval-note" }; + +let server: Server; +let root: string; +let prevPackDir: string | undefined; +let prevBase: string | undefined; +let prevNoDownload: string | undefined; + +/** Mutable per-test release contents. */ +let assets: Record; + +function sha(s: string): string { + return createHash("sha256").update(s).digest("hex"); +} + +/** Build a well-formed release: manifest, entry, and matching SHA256SUMS. */ +function release(over: { policies?: unknown[]; id?: string; version?: string } = {}): void { + const manifest = JSON.stringify({ + id: over.id ?? "acme/finance", + version: over.version ?? "1.2.0", + policies: over.policies ?? [POLICY, POLICY_2], + }); + assets = { + "failproofai-pack.json": manifest, + "failproofai-pack.mjs": ENTRY, + SHA256SUMS: + `${sha(manifest)} failproofai-pack.json\n` + + `${sha(ENTRY)} failproofai-pack.mjs\n`, + }; +} + +beforeEach(async () => { + root = mkdtempSync(join(tmpdir(), "fpai-pack-store-")); + prevPackDir = process.env.FAILPROOFAI_PACK_DIR; + prevBase = process.env.FAILPROOFAI_PACK_BASE_URL; + prevNoDownload = process.env.FAILPROOFAI_NO_DOWNLOAD; + delete process.env.FAILPROOFAI_NO_DOWNLOAD; + process.env.FAILPROOFAI_PACK_DIR = root; + release(); + + // Serves ONLY the real release path, so a wrong owner/repo/tag 404s the way + // GitHub would — which also makes these tests prove the URL is constructed + // correctly rather than merely that some asset was fetched. + server = createServer((req, res) => { + const m = (req.url ?? "").match(/^\/([^/]+)\/([^/]+)\/releases\/download\/([^/]+)\/([^/]+)$/); + const body = m && m[1] === "acme" && m[2] === "finance" ? assets[m[4]] : undefined; + if (body === undefined) { + res.writeHead(404).end("no such asset"); + return; + } + res.writeHead(200).end(body); + }); + await new Promise((r) => server.listen(0, "127.0.0.1", r)); + process.env.FAILPROOFAI_PACK_BASE_URL = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; +}); + +afterEach(async () => { + await new Promise((r) => server.close(() => r())); + for (const [k, v] of Object.entries({ + FAILPROOFAI_PACK_DIR: prevPackDir, + FAILPROOFAI_PACK_BASE_URL: prevBase, + FAILPROOFAI_NO_DOWNLOAD: prevNoDownload, + })) { + if (v === undefined) delete process.env[k]; + else process.env[k] = v; + } + rmSync(root, { recursive: true, force: true }); +}); + +const installed = () => JSON.parse(readFileSync(join(root, "installed.json"), "utf8")); + +describe("parsePackSpec", () => { + it("accepts the scheme and the bare form", () => { + expect(parsePackSpec("github:acme/finance@v1.2.0")).toEqual({ owner: "acme", repo: "finance", tag: "v1.2.0" }); + expect(parsePackSpec("acme/finance@v1.2.0")).toEqual({ owner: "acme", repo: "finance", tag: "v1.2.0" }); + }); + + it("REQUIRES a tag", () => { + // A default of `main` or `latest` makes the URL a moving target, so what a + // machine enforces would change whenever the publisher pushed — exactly the + // drift the recorded digest exists to prevent. + expect(() => parsePackSpec("github:acme/finance")).toThrow(/must name a tag/); + }); + + it("refuses owner/repo/tag that could reshape the URL", () => { + expect(() => parsePackSpec("github:../evil/x@v1")).toThrow(/unsafe owner/); + expect(() => parsePackSpec("github:acme/../x@v1")).toThrow(/unsafe repo/); + expect(() => parsePackSpec("github:acme/finance@../../etc")).toThrow(/unsafe tag/); + }); + + it("builds the asset URL by construction, never discovery", () => { + const spec = parsePackSpec("github:acme/finance@v1.2.0"); + expect(packAssetUrl(spec, "SHA256SUMS")).toBe( + `${process.env.FAILPROOFAI_PACK_BASE_URL}/acme/finance/releases/download/v1.2.0/SHA256SUMS`, + ); + expect(formatPackSpec(spec)).toBe("github:acme/finance@v1.2.0"); + }); +}); + +describe("digestFor", () => { + it("returns null rather than passing when the asset has no line", () => { + expect(digestFor(`${"a".repeat(64)} other.mjs\n`, "failproofai-pack.mjs")).toBeNull(); + }); +}); + +describe("addPack", () => { + it("fetches, verifies and activates a pack", async () => { + const result = await addPack("github:acme/finance@v1.2.0"); + expect(result.id).toBe("acme/finance"); + expect(result.available).toEqual(["block-big-refund", "require-approval-note"]); + expect(result.enabled).toEqual(["block-big-refund", "require-approval-note"]); + + const file = installed(); + expect(file.schemaVersion).toBe(1); + expect(file.packs).toHaveLength(1); + expect(file.packs[0].source).toBe("github:acme/finance@v1.2.0"); + expect(file.packs[0].sha256).toBe(sha(ENTRY)); + expect(file.packs[0].entry).toBe(`artifacts/${sha(ENTRY)}.mjs`); + // And the reader accepts what the writer produced. + const { packs, errors } = readInstalledPacks(); + expect(errors).toEqual([]); + expect(packs[0].id).toBe("acme/finance"); + }); + + it("takes only the selected policies", async () => { + const result = await addPack("github:acme/finance@v1.2.0", { only: ["require-approval-note"] }); + expect(result.enabled).toEqual(["require-approval-note"]); + expect(installed().packs[0].enabled).toEqual(["require-approval-note"]); + expect(readInstalledPacks().packs[0].enabled).toEqual(["require-approval-note"]); + }); + + it("refuses a selection the pack does not contain", async () => { + await expect(addPack("github:acme/finance@v1.2.0", { only: ["nope"] })).rejects.toThrow(/does not contain nope/); + expect(existsSync(join(root, "installed.json"))).toBe(false); + }); + + it("carries a selection forward across an upgrade", async () => { + // Upgrading a pack must not quietly switch on the policies someone chose to + // leave off. + await addPack("github:acme/finance@v1.2.0", { only: ["require-approval-note"] }); + release({ version: "1.3.0" }); + const result = await addPack("github:acme/finance@v1.3.0"); + expect(result.enabled).toEqual(["require-approval-note"]); + expect(installed().packs).toHaveLength(1); + expect(installed().packs[0].version).toBe("1.3.0"); + }); + + describe("refuses BEFORE writing anything", () => { + const wroteNothing = () => expect(existsSync(join(root, "installed.json"))).toBe(false); + + it("when the artifact does not match SHA256SUMS", async () => { + assets["failproofai-pack.mjs"] = ENTRY + "\n// tampered\n"; + await expect(addPack("github:acme/finance@v1.2.0")).rejects.toThrow(/failed integrity verification/); + wroteNothing(); + }); + + it("when SHA256SUMS has no line for the artifact", async () => { + assets.SHA256SUMS = `${sha(assets["failproofai-pack.json"])} failproofai-pack.json\n`; + await expect(addPack("github:acme/finance@v1.2.0")).rejects.toThrow(/no entry for failproofai-pack.mjs/); + wroteNothing(); + }); + + it("when a policy declares alwaysOn", async () => { + release({ policies: [{ ...POLICY, alwaysOn: true }] }); + await expect(addPack("github:acme/finance@v1.2.0")).rejects.toThrow(/alwaysOn/); + wroteNothing(); + }); + + it("when a policy name would reach the builtin namespace", async () => { + release({ policies: [{ ...POLICY, name: "failproofai/block-sudo" }] }); + await expect(addPack("github:acme/finance@v1.2.0")).rejects.toThrow(/unsafe name/); + wroteNothing(); + }); + + it("when the release is missing entirely", async () => { + await expect(addPack("github:acme/nothing@v9")).rejects.toThrow(/404/); + wroteNothing(); + }); + + it("when downloads are disabled", async () => { + process.env.FAILPROOFAI_NO_DOWNLOAD = "1"; + await expect(addPack("github:acme/finance@v1.2.0")).rejects.toThrow(/FAILPROOFAI_NO_DOWNLOAD/); + wroteNothing(); + }); + }); +}); + +describe("removePack", () => { + it("deactivates a pack and leaves its artifact on disk", async () => { + const { artifact } = await addPack("github:acme/finance@v1.2.0"); + expect(removePack("acme/finance")).toBe(true); + expect(installed().packs).toEqual([]); + expect(readInstalledPacks().packs).toEqual([]); + // Content-addressed and inert once nothing points at it, so keeping it makes + // a re-add offline-safe. + expect(existsSync(artifact)).toBe(true); + }); + + it("reports false for a pack that was never installed", async () => { + await addPack("github:acme/finance@v1.2.0"); + expect(removePack("other/pack")).toBe(false); + expect(installed().packs).toHaveLength(1); + }); +}); diff --git a/bin/failproofai.mjs b/bin/failproofai.mjs index 6be9c4393..8606df6e6 100755 --- a/bin/failproofai.mjs +++ b/bin/failproofai.mjs @@ -271,7 +271,7 @@ if (hookIdx >= 0) { */ async function runCli() { // --help / -h (only when not inside a subcommand that handles its own --help) - const SUBCOMMANDS = ["policies", "policy", "audit", "config", "uninstall", "backfill", "flush", "harness"]; + const SUBCOMMANDS = ["policies", "policy", "audit", "config", "uninstall", "backfill", "flush", "harness", "pack"]; if ((args.includes("--help") || args.includes("-h")) && !SUBCOMMANDS.includes(args[0])) { const extraArgs = args.filter((a) => a !== "--help" && a !== "-h"); if (extraArgs.length > 0) { @@ -319,6 +319,9 @@ COMMANDS policies --help, -h Show this help for the policies command + pack list Show installed policy packs + pack add Install a policy pack from a GitHub release + pack remove Deactivate an installed pack harness list Show extra capture paths per agent CLI harness add-path Also capture sessions from for harness . Accepts \`