From 1bdf221db7538dc89d609001ecbcbeb7697d03e4 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Mon, 13 Apr 2026 16:37:45 -0700 Subject: [PATCH 1/5] feat: convention-based policy auto-discovery from .failproofai/policies/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add git-hooks-like auto-discovery: drop *policies.{js,mjs,ts} files into .failproofai/policies/ at project or user level and they're loaded automatically — no --custom flag or config changes needed. - Add discoverPolicyFiles() and loadAllCustomHooks() to custom-hooks-loader - Convention hooks registered with "convention/" prefix for telemetry - PostHog telemetry: convention_policies_loaded event, is_convention_policy flag on hook_policy_triggered - listHooks() shows discovered convention policies - CLI help updated with CONVENTION POLICIES section - 6 new unit tests, 16 new e2e tests covering discovery, union loading, fail-open, file filtering, coexistence with --custom - Example files in examples/convention-policies/ - Mintlify docs and README updated Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 1 + README.md | 15 + __tests__/e2e/helpers/fixture-env.ts | 19 ++ __tests__/e2e/hooks/custom-hooks.e2e.test.ts | 293 ++++++++++++++++++ __tests__/hooks/custom-hooks-loader.test.ts | 100 +++++- __tests__/hooks/handler.test.ts | 49 +-- __tests__/hooks/manager.test.ts | 1 + bin/failproofai.mjs | 5 + docs/configuration.mdx | 17 + docs/custom-policies.mdx | 64 +++- .../convention-policies/security-policies.mjs | 40 +++ .../convention-policies/workflow-policies.mjs | 41 +++ src/hooks/custom-hooks-loader.ts | 174 +++++++++-- src/hooks/handler.ts | 32 +- src/hooks/manager.ts | 33 +- 15 files changed, 831 insertions(+), 53 deletions(-) create mode 100644 examples/convention-policies/security-policies.mjs create mode 100644 examples/convention-policies/workflow-policies.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fc464f2..d4d8456d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features - Check third-party bot statuses (CodeRabbit, SonarCloud, etc.) in `require-ci-green-before-stop` policy (#90) +- Convention-based policy auto-discovery: drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` at project or user level for automatic loading — no config changes needed (#88) - Auto-bump version after release (#73) ### Fixes diff --git a/README.md b/README.md index f1b533ee..627d4f1e 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,21 @@ failproofai policies --install --custom ./my-policies.js Custom hooks support transitive local imports, async/await, and access to `process.env`. Errors are fail-open (logged to `~/.failproofai/hook.log`, built-in policies continue). See [docs/custom-hooks.mdx](docs/custom-hooks.mdx) for the full guide. +### Convention-based policies (v0.0.2-beta.7+) + +Drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` and they're automatically loaded — no `--custom` flag or config changes needed. Works like git hooks: drop a file, it just works. + +``` +# Project level — committed to git, shared with the team +.failproofai/policies/security-policies.mjs +.failproofai/policies/workflow-policies.mjs + +# User level — personal, applies to all projects +~/.failproofai/policies/my-policies.mjs +``` + +Both levels load (union). Files are loaded alphabetically within each directory. Prefix with `01-`, `02-`, etc. to control order. See [examples/convention-policies/](examples/convention-policies/) for ready-to-use examples. + --- ## Telemetry diff --git a/__tests__/e2e/helpers/fixture-env.ts b/__tests__/e2e/helpers/fixture-env.ts index ada1824b..759e5ab3 100644 --- a/__tests__/e2e/helpers/fixture-env.ts +++ b/__tests__/e2e/helpers/fixture-env.ts @@ -31,6 +31,16 @@ export interface FixtureEnv { * Returns the absolute path (suitable for use as customPoliciesPath in config). */ writeHook(filename: string, content: string): string; + /** + * Write a convention policy file into .failproofai/policies/. + * Returns the absolute path. + * + * @param filename - e.g. "deny-policies.mjs" + * @param content - JS/TS source code + * @param scope - "project" → {cwd}/.failproofai/policies/ (default) + * "global" → {home}/.failproofai/policies/ + */ + writePolicyFile(filename: string, content: string, scope?: "project" | "global"): string; } export function createFixtureEnv(): FixtureEnv { @@ -70,5 +80,14 @@ export function createFixtureEnv(): FixtureEnv { writeFileSync(filePath, content, "utf8"); return filePath; }, + + writePolicyFile(filename: string, content: string, scope: "project" | "global" = "project"): string { + const base = scope === "global" ? home : cwd; + const dir = join(base, ".failproofai", "policies"); + mkdirSync(dir, { recursive: true }); + const filePath = join(dir, filename); + writeFileSync(filePath, content, "utf8"); + return filePath; + }, }; } diff --git a/__tests__/e2e/hooks/custom-hooks.e2e.test.ts b/__tests__/e2e/hooks/custom-hooks.e2e.test.ts index a32b005f..a7601d0e 100644 --- a/__tests__/e2e/hooks/custom-hooks.e2e.test.ts +++ b/__tests__/e2e/hooks/custom-hooks.e2e.test.ts @@ -375,3 +375,296 @@ describe("custom-hooks — customPoliciesPath scope levels", () => { assertAllow(result); }); }); + +// ── Convention-based policies (.failproofai/policies/) ────────────────────── + +describe("convention-based policies (.failproofai/policies/)", () => { + // ── Basic discovery and execution ───────────────────────────────────────── + + it("project convention policy fires: deny hook in .failproofai/policies/", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + env.writePolicyFile("block-policies.mjs", ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "conv-deny", + description: "Convention deny policy", + match: { events: ["PreToolUse"] }, + fn: async () => deny("blocked by convention policy"), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); + + it("user-level convention policy fires: deny hook in ~/.failproofai/policies/", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + env.writePolicyFile("user-policies.mjs", ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "user-conv-deny", + match: { events: ["PreToolUse"] }, + fn: async () => deny("blocked by user convention"), + }); + `, "global"); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); + + it("convention policy with instruct() returns instruction to Claude", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + env.writePolicyFile("guide-policies.mjs", ` + import { customPolicies, instruct } from "failproofai"; + customPolicies.add({ + name: "conv-instruct", + match: { events: ["PreToolUse"] }, + fn: async () => instruct("Remember to check the linter output"), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertInstruct(result); + }); + + it("convention policy with allow() passes through", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + env.writePolicyFile("pass-policies.mjs", ` + import { customPolicies, allow } from "failproofai"; + customPolicies.add({ + name: "conv-allow", + match: { events: ["PreToolUse"] }, + fn: async () => allow(), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertAllow(result); + }); + + // ── Multi-level (union) ─────────────────────────────────────────────────── + + it("both project and user convention policies load (union)", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + // Project policy: allow + env.writePolicyFile("proj-policies.mjs", ` + import { customPolicies, allow } from "failproofai"; + customPolicies.add({ + name: "proj-allow", + match: { events: ["PreToolUse"] }, + fn: async () => allow(), + }); + `); + // User policy: deny — should fire because both load (union) + env.writePolicyFile("user-policies.mjs", ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "user-deny", + match: { events: ["PreToolUse"] }, + fn: async () => deny("user level block"), + }); + `, "global"); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); + + // ── Multiple files in one directory ─────────────────────────────────────── + + it("multiple convention files in the same directory all load", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + // First file: allow + env.writePolicyFile("aaa-policies.mjs", ` + import { customPolicies, allow } from "failproofai"; + customPolicies.add({ + name: "first-allow", + match: { events: ["PreToolUse"] }, + fn: async () => allow(), + }); + `); + // Second file: deny — both should load (alphabetical order) + env.writePolicyFile("zzz-policies.mjs", ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "second-deny", + match: { events: ["PreToolUse"] }, + fn: async () => deny("second file blocks"), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); + + // ── Coexistence with customPoliciesPath ─────────────────────────────────── + + it("coexists with customPoliciesPath — both explicit and convention hooks load", () => { + const env = createFixtureEnv(); + // Explicit custom hook: allow + const hookPath = env.writeHook("explicit.mjs", ` + import { customPolicies, allow } from "failproofai"; + customPolicies.add({ + name: "explicit-allow", + match: { events: ["PreToolUse"] }, + fn: async () => allow(), + }); + `); + env.writeConfig({ enabledPolicies: [], customPoliciesPath: hookPath }); + // Convention hook: deny — should ALSO fire + env.writePolicyFile("deny-policies.mjs", ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "convention-deny", + match: { events: ["PreToolUse"] }, + fn: async () => deny("convention overrides"), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); + + // ── File filtering ──────────────────────────────────────────────────────── + + it("non-matching filenames are ignored (e.g. utils.js, helpers.mjs)", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + // This file should NOT be loaded (doesn't match *policies.{js,mjs,ts}) + env.writePolicyFile("helpers.mjs", ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "should-not-load", + match: { events: ["PreToolUse"] }, + fn: async () => deny("this should never fire"), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertAllow(result); + }); + + it("files with .json or .md extensions in policies dir are ignored", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + env.writePolicyFile("readme.md", "# This is not a policy file"); + env.writePolicyFile("config.json", '{"not": "a policy"}'); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertAllow(result); + }); + + it("supports .js, .mjs, and .ts extensions", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + // .js file that denies + env.writePolicyFile("a-policies.js", ` + const { customPolicies, deny } = require("failproofai"); + customPolicies.add({ + name: "js-deny", + match: { events: ["PreToolUse"] }, + fn: async () => deny("js policy"), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); + + // ── Fail-open per file ──────────────────────────────────────────────────── + + it("fail-open: broken file does not prevent valid files from loading", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + // First file (alphabetically): syntax error + env.writePolicyFile("aaa-policies.mjs", ` + this is not valid javascript !!!@@## + `); + // Second file: valid deny hook — should still fire + env.writePolicyFile("zzz-policies.mjs", ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "valid-deny", + match: { events: ["PreToolUse"] }, + fn: async () => deny("valid policy fires despite broken sibling"), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); + + // ── No convention directory ─────────────────────────────────────────────── + + it("no convention directory → no crash, allow", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + // Don't create .failproofai/policies/ at all + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertAllow(result); + }); + + it("empty convention directory → no crash, allow", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + // Create the directory but put no policy files in it + env.writePolicyFile("readme.md", "# empty policies dir"); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertAllow(result); + }); + + // ── Convention + builtin policies ───────────────────────────────────────── + + it("convention policies work alongside builtin policies", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: ["block-sudo"] }); + env.writePolicyFile("extra-policies.mjs", ` + import { customPolicies, allow } from "failproofai"; + customPolicies.add({ + name: "extra-allow", + match: { events: ["PreToolUse"] }, + fn: async () => allow(), + }); + `); + // block-sudo should still fire + const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); + + // ── Event matching ──────────────────────────────────────────────────────── + + it("convention policy respects event matching (PostToolUse only)", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + env.writePolicyFile("post-only-policies.mjs", ` + import { customPolicies, deny } from "failproofai"; + customPolicies.add({ + name: "post-only", + match: { events: ["PostToolUse"] }, + fn: async () => deny("post tool use block"), + }); + `); + // PreToolUse should NOT trigger this policy + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertAllow(result); + }); + + // ── Multiple hooks per file ─────────────────────────────────────────────── + + it("single convention file can register multiple hooks", () => { + const env = createFixtureEnv(); + env.writeConfig({ enabledPolicies: [] }); + env.writePolicyFile("multi-policies.mjs", ` + import { customPolicies, allow, deny } from "failproofai"; + customPolicies.add({ + name: "first-hook", + match: { events: ["PreToolUse"] }, + fn: async (ctx) => { + if (ctx.toolName === "Bash") return deny("bash blocked"); + return allow(); + }, + }); + customPolicies.add({ + name: "second-hook", + match: { events: ["PreToolUse"] }, + fn: async () => allow(), + }); + `); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("ls", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + }); +}); diff --git a/__tests__/hooks/custom-hooks-loader.test.ts b/__tests__/hooks/custom-hooks-loader.test.ts index 4ea5e3f5..23d8c5b4 100644 --- a/__tests__/hooks/custom-hooks-loader.test.ts +++ b/__tests__/hooks/custom-hooks-loader.test.ts @@ -21,7 +21,7 @@ vi.mock("../../src/hooks/loader-utils", () => ({ vi.mock("node:fs", async () => { const actual = await vi.importActual("node:fs"); - return { ...actual, existsSync: vi.fn() }; + return { ...actual, existsSync: vi.fn(), readdirSync: vi.fn(() => []) }; }); describe("hooks/custom-hooks-loader", () => { @@ -137,3 +137,101 @@ describe("hooks/custom-hooks-loader", () => { expect(Array.isArray(result)).toBe(true); }); }); + +describe("discoverPolicyFiles", () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it("returns [] when directory does not exist", async () => { + const { existsSync } = await import("node:fs"); + vi.mocked(existsSync).mockReturnValue(false); + + const { discoverPolicyFiles } = await import("../../src/hooks/custom-hooks-loader"); + expect(discoverPolicyFiles("/nonexistent/dir")).toEqual([]); + }); + + it("returns only *policies.{js,mjs,ts} files, sorted alphabetically", async () => { + const { existsSync, readdirSync } = await import("node:fs"); + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readdirSync).mockReturnValue([ + { name: "z-policies.js", isFile: () => true, isDirectory: () => false }, + { name: "a-policies.mjs", isFile: () => true, isDirectory: () => false }, + { name: "utils.js", isFile: () => true, isDirectory: () => false }, + { name: "b-policies.ts", isFile: () => true, isDirectory: () => false }, + { name: "readme.md", isFile: () => true, isDirectory: () => false }, + { name: "data.json", isFile: () => true, isDirectory: () => false }, + ] as never); + + const { discoverPolicyFiles } = await import("../../src/hooks/custom-hooks-loader"); + const files = discoverPolicyFiles("/some/dir"); + + expect(files).toHaveLength(3); + expect(files[0]).toContain("a-policies.mjs"); + expect(files[1]).toContain("b-policies.ts"); + expect(files[2]).toContain("z-policies.js"); + }); + + it("ignores subdirectories even if they match the naming pattern", async () => { + const { existsSync, readdirSync } = await import("node:fs"); + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readdirSync).mockReturnValue([ + { name: "real-policies.js", isFile: () => true, isDirectory: () => false }, + { name: "dir-policies.js", isFile: () => false, isDirectory: () => true }, + ] as never); + + const { discoverPolicyFiles } = await import("../../src/hooks/custom-hooks-loader"); + const files = discoverPolicyFiles("/some/dir"); + + expect(files).toHaveLength(1); + expect(files[0]).toContain("real-policies.js"); + }); +}); + +describe("loadAllCustomHooks", () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it("returns empty hooks and no convention sources when nothing configured", async () => { + const { existsSync } = await import("node:fs"); + vi.mocked(existsSync).mockReturnValue(false); + + const { getCustomHooks } = await import("../../src/hooks/custom-hooks-registry"); + vi.mocked(getCustomHooks).mockReturnValue([]); + + const { loadAllCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); + const result = await loadAllCustomHooks(undefined, { sessionCwd: "/tmp/fake" }); + + expect(result.hooks).toEqual([]); + expect(result.conventionSources).toEqual([]); + }); + + it("calls clearCustomHooks exactly once", async () => { + const { existsSync } = await import("node:fs"); + vi.mocked(existsSync).mockReturnValue(false); + + const { clearCustomHooks, getCustomHooks } = await import("../../src/hooks/custom-hooks-registry"); + vi.mocked(getCustomHooks).mockReturnValue([]); + + const { loadAllCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); + await loadAllCustomHooks(undefined, { sessionCwd: "/tmp/fake" }); + + expect(clearCustomHooks).toHaveBeenCalledTimes(1); + }); + + it("logs warning when customPoliciesPath does not exist", async () => { + const { existsSync } = await import("node:fs"); + vi.mocked(existsSync).mockReturnValue(false); + + const { getCustomHooks } = await import("../../src/hooks/custom-hooks-registry"); + vi.mocked(getCustomHooks).mockReturnValue([]); + + const { hookLogWarn } = await import("../../src/hooks/hook-logger"); + const { loadAllCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); + + await loadAllCustomHooks("/nonexistent/file.js", { sessionCwd: "/tmp/fake" }); + + expect(hookLogWarn).toHaveBeenCalledWith(expect.stringContaining("not found")); + }); +}); diff --git a/__tests__/hooks/handler.test.ts b/__tests__/hooks/handler.test.ts index 6913bf87..464e2fdc 100644 --- a/__tests__/hooks/handler.test.ts +++ b/__tests__/hooks/handler.test.ts @@ -27,7 +27,7 @@ vi.mock("../../src/hooks/policy-registry", () => ({ })); vi.mock("../../src/hooks/custom-hooks-loader", () => ({ - loadCustomHooks: vi.fn(() => Promise.resolve([])), + loadAllCustomHooks: vi.fn(() => Promise.resolve({ hooks: [], conventionSources: [] })), })); vi.mock("../../src/hooks/hook-activity-store", () => ({ @@ -117,16 +117,16 @@ describe("hooks/handler", () => { expect(registerBuiltinPolicies).toHaveBeenCalledWith(["block-sudo"]); }); - it("passes session cwd to loadCustomHooks for relative customPoliciesPath resolution", async () => { + it("passes session cwd to loadAllCustomHooks for relative customPoliciesPath resolution", async () => { const sessionPayload = JSON.stringify({ cwd: "/home/user/project", }); mockStdin(sessionPayload); - const { loadCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); + const { loadAllCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); await handleHookEvent("PreToolUse"); - expect(loadCustomHooks).toHaveBeenCalledWith( + expect(loadAllCustomHooks).toHaveBeenCalledWith( undefined, expect.objectContaining({ sessionCwd: "/home/user/project" }), ); @@ -196,6 +196,7 @@ describe("hooks/handler", () => { policy_name: "block-sudo", decision: "deny", is_custom_hook: false, + is_convention_policy: false, has_custom_params: false, param_keys_overridden: [], }, @@ -226,6 +227,7 @@ describe("hooks/handler", () => { policy_name: "warn-repeated-tool-calls", decision: "instruct", is_custom_hook: false, + is_convention_policy: false, has_custom_params: false, param_keys_overridden: [], }, @@ -344,11 +346,14 @@ describe("hooks/handler", () => { }); it("fires custom_hooks_loaded with count, names, and event types when custom hooks are present", async () => { - const { loadCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); - vi.mocked(loadCustomHooks).mockResolvedValueOnce([ - { name: "hook-a", fn: async () => ({ decision: "allow" as const }), match: { events: ["PreToolUse" as never] } }, - { name: "hook-b", fn: async () => ({ decision: "allow" as const }), match: { events: ["Stop" as never] } }, - ]); + const { loadAllCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); + vi.mocked(loadAllCustomHooks).mockResolvedValueOnce({ + hooks: [ + { name: "hook-a", fn: async () => ({ decision: "allow" as const }), match: { events: ["PreToolUse" as never] } }, + { name: "hook-b", fn: async () => ({ decision: "allow" as const }), match: { events: ["Stop" as never] } }, + ], + conventionSources: [], + }); mockStdin(); const { trackHookEvent } = await import("../../src/hooks/hook-telemetry"); @@ -376,10 +381,13 @@ describe("hooks/handler", () => { }); it("fires custom_hook_error with error_type exception when hook throws", async () => { - const { loadCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); - vi.mocked(loadCustomHooks).mockResolvedValueOnce([ - { name: "bad-hook", fn: async () => { throw new Error("oops"); } }, - ]); + const { loadAllCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); + vi.mocked(loadAllCustomHooks).mockResolvedValueOnce({ + hooks: [ + { name: "bad-hook", fn: async () => { throw new Error("oops"); } }, + ], + conventionSources: [], + }); const { registerPolicy } = await import("../../src/hooks/policy-registry"); const { trackHookEvent } = await import("../../src/hooks/hook-telemetry"); @@ -397,15 +405,18 @@ describe("hooks/handler", () => { expect(trackHookEvent).toHaveBeenCalledWith( "test-instance-id", "custom_hook_error", - { hook_name: "bad-hook", error_type: "exception", event_type: "PreToolUse" }, + { hook_name: "bad-hook", error_type: "exception", event_type: "PreToolUse", is_convention_policy: false }, ); }); it("fires custom_hook_error with error_type timeout when hook times out", async () => { - const { loadCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); - vi.mocked(loadCustomHooks).mockResolvedValueOnce([ - { name: "slow-hook", fn: async () => { throw new Error("timeout"); } }, - ]); + const { loadAllCustomHooks } = await import("../../src/hooks/custom-hooks-loader"); + vi.mocked(loadAllCustomHooks).mockResolvedValueOnce({ + hooks: [ + { name: "slow-hook", fn: async () => { throw new Error("timeout"); } }, + ], + conventionSources: [], + }); const { registerPolicy } = await import("../../src/hooks/policy-registry"); const { trackHookEvent } = await import("../../src/hooks/hook-telemetry"); @@ -423,7 +434,7 @@ describe("hooks/handler", () => { expect(trackHookEvent).toHaveBeenCalledWith( "test-instance-id", "custom_hook_error", - { hook_name: "slow-hook", error_type: "timeout", event_type: "PreToolUse" }, + { hook_name: "slow-hook", error_type: "timeout", event_type: "PreToolUse", is_convention_policy: false }, ); }); diff --git a/__tests__/hooks/manager.test.ts b/__tests__/hooks/manager.test.ts index 1cc1f6cc..663897dc 100644 --- a/__tests__/hooks/manager.test.ts +++ b/__tests__/hooks/manager.test.ts @@ -46,6 +46,7 @@ vi.mock("../../lib/telemetry-id", () => ({ vi.mock("../../src/hooks/custom-hooks-loader", () => ({ loadCustomHooks: vi.fn(() => Promise.resolve([])), + discoverPolicyFiles: vi.fn(() => []), })); const USER_SETTINGS_PATH = resolve(homedir(), ".claude", "settings.json"); diff --git a/bin/failproofai.mjs b/bin/failproofai.mjs index 6c21d943..52ad3fd9 100755 --- a/bin/failproofai.mjs +++ b/bin/failproofai.mjs @@ -97,6 +97,11 @@ COMMANDS --version, -v Print version and exit --help, -h Show this help message +CONVENTION POLICIES + Drop *policies.{js,mjs,ts} files into .failproofai/policies/ for auto-loading. + Works at project level (.failproofai/policies/) and user level (~/.failproofai/policies/). + No --custom flag or config changes needed — just drop files and they're picked up. + EXAMPLES failproofai policies failproofai policies --install diff --git a/docs/configuration.mdx b/docs/configuration.mdx index 56793dfc..9d9499c2 100644 --- a/docs/configuration.mdx +++ b/docs/configuration.mdx @@ -124,6 +124,23 @@ Path to a JavaScript file containing custom hook policies. This is set automatic The file is loaded fresh on every hook event - there is no caching. See [Custom Policies](/custom-policies) for authoring details. +### Convention-based policies (v0.0.2-beta.7+) + +In addition to the explicit `customPoliciesPath`, failproofai automatically discovers and loads policy files from `.failproofai/policies/` directories: + +| Level | Directory | Scope | +|-------|-----------|-------| +| Project | `.failproofai/policies/` | Shared with team via version control | +| User | `~/.failproofai/policies/` | Personal, applies to all projects | + +**File matching:** Only files matching `*policies.{js,mjs,ts}` are loaded (e.g. `security-policies.mjs`, `workflow-policies.js`). Other files in the directory are ignored. + +**No config needed:** Convention policies require no entries in `policies-config.json`. Just drop files into the directory and they're picked up on the next hook event. + +**Union loading:** Both project and user convention directories are scanned. All matching files from both levels are loaded (unlike `customPoliciesPath` which uses first-scope-wins). + +See [Custom Policies](/custom-policies) for more details and examples. + ### `llm` Type: `object` (optional) diff --git a/docs/custom-policies.mdx b/docs/custom-policies.mdx index 4302eb2b..aae75a6e 100644 --- a/docs/custom-policies.mdx +++ b/docs/custom-policies.mdx @@ -37,7 +37,33 @@ failproofai policies --install --custom ./my-policies.js --- -## Installing and updating +## Two ways to load custom policies + +### Option 1: Convention-based (recommended, v0.0.2-beta.7+) + +Drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` and they're automatically loaded — no flags or config changes needed. This works like git hooks: drop a file, it just works. + +``` +# Project level — committed to git, shared with the team +.failproofai/policies/security-policies.mjs +.failproofai/policies/workflow-policies.mjs + +# User level — personal, applies to all projects +~/.failproofai/policies/my-policies.mjs +``` + +**How it works:** +- Both project and user directories are scanned (union — not first-scope-wins) +- Files are loaded alphabetically within each directory. Prefix with `01-`, `02-` to control order +- Only files matching `*policies.{js,mjs,ts}` are loaded; other files are ignored +- Each file is loaded independently (fail-open per file) +- Works alongside explicit `--custom` and built-in policies + + +Convention policies are the easiest way to share policies across a team. Commit `.failproofai/policies/` to git and every team member gets them automatically. + + +### Option 2: Explicit file path ```bash # Install with a custom policies file @@ -52,6 +78,14 @@ failproofai policies --uninstall --custom The resolved absolute path is stored in `policies-config.json` as `customPoliciesPath`. The file is loaded fresh on every hook event - there is no caching between events. +### Using both together + +Convention policies and the explicit `--custom` file can coexist. Load order: + +1. Explicit `customPoliciesPath` file (if configured) +2. Project convention files (`{cwd}/.failproofai/policies/`, alphabetical) +3. User convention files (`~/.failproofai/policies/`, alphabetical) + --- ## API @@ -155,7 +189,9 @@ customPolicies.add({ Policies are evaluated in this order: 1. Built-in policies (in definition order) -2. Custom policies (in `.add()` order) +2. Explicit custom policies from `customPoliciesPath` (in `.add()` order) +3. Convention policies from project `.failproofai/policies/` (files alphabetical, `.add()` order within) +4. Convention policies from user `~/.failproofai/policies/` (files alphabetical, `.add()` order within) The first `deny` short-circuits all subsequent policies. The first `instruct` wins — subsequent `instruct` returns are ignored. @@ -212,11 +248,13 @@ Custom policies are **fail-open**: errors never block built-in policies or crash | Failure | Behavior | |---------|----------| -| `customPoliciesPath` not set | No custom policies run; built-ins continue normally | +| `customPoliciesPath` not set | No explicit custom policies run; convention policies and built-ins continue normally | | File not found | Warning logged to `~/.failproofai/hook.log`; built-ins continue | -| Syntax/import error | Error logged to `~/.failproofai/hook.log`; all custom policies skipped | +| Syntax/import error (explicit) | Error logged to `~/.failproofai/hook.log`; explicit custom policies skipped | +| Syntax/import error (convention) | Error logged; that file skipped, other convention files still load | | `fn` throws at runtime | Error logged; that hook treated as `allow`; other hooks continue | | `fn` takes longer than 10s | Timeout logged; treated as `allow` | +| Convention directory missing | No convention policies run; no error | To debug custom policy errors, watch the log file: @@ -291,9 +329,25 @@ The `examples/` directory contains ready-to-run policy files: |------|----------| | `examples/policies-basic.js` | Five starter policies covering common agent failure modes | | `examples/policies-advanced/index.js` | Advanced patterns: transitive imports, async calls, output scrubbing, and session-end hooks | +| `examples/convention-policies/security-policies.mjs` | Convention-based security policies (block .env writes, prevent git history rewriting) | +| `examples/convention-policies/workflow-policies.mjs` | Convention-based workflow policies (test reminders, audit file writes) | -Install the basic examples: +### Using explicit file examples ```bash failproofai policies --install --custom ./examples/policies-basic.js ``` + +### Using convention-based examples + +```bash +# Copy to project level +mkdir -p .failproofai/policies +cp examples/convention-policies/*.mjs .failproofai/policies/ + +# Or copy to user level +mkdir -p ~/.failproofai/policies +cp examples/convention-policies/*.mjs ~/.failproofai/policies/ +``` + +No install command needed — the files are picked up automatically on the next hook event. diff --git a/examples/convention-policies/security-policies.mjs b/examples/convention-policies/security-policies.mjs new file mode 100644 index 00000000..62d0e48d --- /dev/null +++ b/examples/convention-policies/security-policies.mjs @@ -0,0 +1,40 @@ +/** + * security-policies.mjs — Convention-based security policies + * + * Drop this file into .failproofai/policies/ at the project or user level + * and it will be automatically loaded — no --custom flag needed. + * + * Project level: .failproofai/policies/security-policies.mjs + * User level: ~/.failproofai/policies/security-policies.mjs + */ +import { customPolicies, allow, deny } from "failproofai"; + +// Block writes to .env files +customPolicies.add({ + name: "block-env-writes", + description: "Prevent Claude from writing to .env files", + match: { events: ["PreToolUse"] }, + fn: async (ctx) => { + if (!["Write", "Edit"].includes(ctx.toolName ?? "")) return allow(); + const path = String(ctx.toolInput?.file_path ?? ""); + if (/\.env($|\.)/.test(path)) { + return deny(`Writing to .env files is blocked: ${path}`); + } + return allow(); + }, +}); + +// Block commands that delete git history +customPolicies.add({ + name: "block-git-history-rewrite", + description: "Prevent destructive git history operations", + match: { events: ["PreToolUse"] }, + fn: async (ctx) => { + if (ctx.toolName !== "Bash") return allow(); + const cmd = String(ctx.toolInput?.command ?? ""); + if (/git\s+(rebase\s+-i|filter-branch|reflog\s+expire)/.test(cmd)) { + return deny("Rewriting git history is not allowed — use a revert commit instead"); + } + return allow(); + }, +}); diff --git a/examples/convention-policies/workflow-policies.mjs b/examples/convention-policies/workflow-policies.mjs new file mode 100644 index 00000000..05270836 --- /dev/null +++ b/examples/convention-policies/workflow-policies.mjs @@ -0,0 +1,41 @@ +/** + * workflow-policies.mjs — Convention-based workflow policies + * + * Drop this file into .failproofai/policies/ at the project or user level + * and it will be automatically loaded — no --custom flag needed. + * + * Project level: .failproofai/policies/workflow-policies.mjs + * User level: ~/.failproofai/policies/workflow-policies.mjs + */ +import { customPolicies, allow, instruct } from "failproofai"; + +// Remind to run tests before committing +customPolicies.add({ + name: "test-before-commit", + description: "Remind Claude to run tests before git commit", + match: { events: ["PreToolUse"] }, + fn: async (ctx) => { + if (ctx.toolName !== "Bash") return allow(); + const cmd = String(ctx.toolInput?.command ?? ""); + if (/git\s+commit/.test(cmd)) { + return instruct( + "Before committing, make sure all tests pass. " + + "Run the test suite first if you haven't already." + ); + } + return allow(); + }, +}); + +// Log all file writes for audit trail +customPolicies.add({ + name: "audit-file-writes", + description: "Log all file write operations", + match: { events: ["PostToolUse"] }, + fn: async (ctx) => { + if (!["Write", "Edit"].includes(ctx.toolName ?? "")) return allow(); + const path = ctx.toolInput?.file_path ?? "unknown"; + console.error(`[audit] File written: ${path}`); + return allow(); + }, +}); diff --git a/src/hooks/custom-hooks-loader.ts b/src/hooks/custom-hooks-loader.ts index 86a55d15..83de4b18 100644 --- a/src/hooks/custom-hooks-loader.ts +++ b/src/hooks/custom-hooks-loader.ts @@ -1,39 +1,51 @@ /** - * Loads a user-authored hooks.js file with ESM import rewriting. + * Loads user-authored policy files with ESM import rewriting. * Supports transitive local imports and `import { ... } from 'failproofai'`. * + * Two loading modes: + * 1. Explicit: a single file via `customPoliciesPath` in policies-config.json + * 2. Convention: auto-discovered *policies.{js,mjs,ts} files from + * .failproofai/policies/ at project and user level (git-hooks style) + * * Fail-open: any error (file not found, syntax error, import failure) is logged - * and results in an empty hook list. Builtins continue running normally. + * and results in an empty hook list for that file. Builtins continue normally. */ -import { resolve, isAbsolute } from "node:path"; -import { existsSync } from "node:fs"; +import { resolve, isAbsolute, basename } from "node:path"; +import { existsSync, readdirSync } from "node:fs"; import { pathToFileURL } from "node:url"; -import { hookLogWarn, hookLogError } from "./hook-logger"; +import { homedir } from "node:os"; +import { hookLogWarn, hookLogError, hookLogInfo } from "./hook-logger"; import { getCustomHooks, clearCustomHooks } from "./custom-hooks-registry"; import { findDistIndex, rewriteFileTree, TMP_SUFFIX, cleanupTmpFiles } from "./loader-utils"; import type { CustomHook } from "./policy-types"; const LOADING_KEY = "__FAILPROOFAI_LOADING_HOOKS__"; -export async function loadCustomHooks( - customPoliciesPath: string | undefined, - opts?: { strict?: boolean; sessionCwd?: string }, -): Promise { - if (!customPoliciesPath) return []; +/** Regex matching convention policy filenames: *policies.{js,mjs,ts} */ +const CONVENTION_FILE_RE = /policies\.(js|mjs|ts)$/; - const absPath = isAbsolute(customPoliciesPath) - ? customPoliciesPath - : resolve(opts?.sessionCwd ?? process.cwd(), customPoliciesPath); - - if (!existsSync(absPath)) { - if (opts?.strict) throw new Error(`Custom hooks file not found: ${absPath}`); - hookLogWarn(`customPoliciesPath not found: ${absPath}`); +/** + * Scan a directory for convention policy files (*policies.{js,mjs,ts}). + * Returns sorted absolute paths. Returns [] if the directory doesn't exist. + */ +export function discoverPolicyFiles(dir: string): string[] { + if (!existsSync(dir)) return []; + try { + const entries = readdirSync(dir, { withFileTypes: true }); + return entries + .filter((e) => e.isFile() && CONVENTION_FILE_RE.test(e.name)) + .sort((a, b) => a.name.localeCompare(b.name)) + .map((e) => resolve(dir, e.name)); + } catch { return []; } +} - // Clear registry before loading so each invocation starts fresh - clearCustomHooks(); - +/** + * Load a single policy file into the globalThis custom hooks registry. + * Does NOT clear the registry — caller is responsible for that. + */ +async function loadSingleFile(absPath: string, opts?: { strict?: boolean }): Promise { const g = globalThis as Record; g[LOADING_KEY] = true; @@ -51,11 +63,131 @@ export async function loadCustomHooks( const msg = err instanceof Error ? err.message : String(err); if (opts?.strict) throw new Error(`Failed to load custom hooks from ${absPath}: ${msg}`); hookLogError(`failed to load custom hooks from ${absPath}: ${msg}`); - return []; } finally { g[LOADING_KEY] = false; await cleanupTmpFiles(tmpFiles); } +} + +/** + * Load a single explicit custom hooks file (legacy API). + * Clears the registry, loads the file, returns registered hooks. + */ +export async function loadCustomHooks( + customPoliciesPath: string | undefined, + opts?: { strict?: boolean; sessionCwd?: string }, +): Promise { + if (!customPoliciesPath) return []; + + const absPath = isAbsolute(customPoliciesPath) + ? customPoliciesPath + : resolve(opts?.sessionCwd ?? process.cwd(), customPoliciesPath); + + if (!existsSync(absPath)) { + if (opts?.strict) throw new Error(`Custom hooks file not found: ${absPath}`); + hookLogWarn(`customPoliciesPath not found: ${absPath}`); + return []; + } + clearCustomHooks(); + await loadSingleFile(absPath, opts); return getCustomHooks(); } + +/** Source metadata for a loaded convention policy file. */ +export interface ConventionSource { + scope: "project" | "user"; + file: string; + hookNames: string[]; +} + +/** Result of loadAllCustomHooks with source metadata. */ +export interface LoadAllResult { + hooks: CustomHook[]; + conventionSources: ConventionSource[]; +} + +/** + * Load ALL custom hooks: explicit customPoliciesPath + convention-discovered files. + * + * Load order: + * 1. Explicit customPoliciesPath (if configured) + * 2. Project convention: {cwd}/.failproofai/policies/*policies.{js,mjs,ts} (alphabetical) + * 3. User convention: ~/.failproofai/policies/*policies.{js,mjs,ts} (alphabetical) + * + * Each file is loaded independently (fail-open per file). + * Convention hooks are tagged with __conventionSource so the handler can distinguish them. + */ +export async function loadAllCustomHooks( + customPoliciesPath: string | undefined, + opts?: { sessionCwd?: string }, +): Promise { + clearCustomHooks(); + + const conventionSources: ConventionSource[] = []; + + // 1. Explicit customPoliciesPath (existing behavior) + if (customPoliciesPath) { + const absPath = isAbsolute(customPoliciesPath) + ? customPoliciesPath + : resolve(opts?.sessionCwd ?? process.cwd(), customPoliciesPath); + if (existsSync(absPath)) { + await loadSingleFile(absPath); + } else { + hookLogWarn(`customPoliciesPath not found: ${absPath}`); + } + } + + const hooksBeforeConvention = getCustomHooks().length; + + // 2. Project convention: {cwd}/.failproofai/policies/*policies.{js,mjs,ts} + const projectDir = resolve(opts?.sessionCwd ?? process.cwd(), ".failproofai", "policies"); + const projectFiles = discoverPolicyFiles(projectDir); + for (const file of projectFiles) { + const hooksBefore = getCustomHooks().length; + await loadSingleFile(file); + const newHooks = getCustomHooks().slice(hooksBefore); + if (newHooks.length > 0) { + conventionSources.push({ + scope: "project", + file: basename(file), + hookNames: newHooks.map((h) => h.name), + }); + } + } + + // 3. User convention: ~/.failproofai/policies/*policies.{js,mjs,ts} + const userDir = resolve(homedir(), ".failproofai", "policies"); + const userFiles = discoverPolicyFiles(userDir); + for (const file of userFiles) { + const hooksBefore = getCustomHooks().length; + await loadSingleFile(file); + const newHooks = getCustomHooks().slice(hooksBefore); + if (newHooks.length > 0) { + conventionSources.push({ + scope: "user", + file: basename(file), + hookNames: newHooks.map((h) => h.name), + }); + } + } + + const allHooks = getCustomHooks(); + const conventionCount = allHooks.length - hooksBeforeConvention; + + if (projectFiles.length > 0 || userFiles.length > 0) { + hookLogInfo( + `convention policies: ${projectFiles.length} project file(s), ${userFiles.length} user file(s), ${conventionCount} hook(s)`, + ); + } + + // Tag convention hooks so the handler can register them with a "convention/" prefix + const conventionHookNames = new Set(conventionSources.flatMap((s) => s.hookNames)); + for (const hook of allHooks) { + if (conventionHookNames.has(hook.name)) { + (hook as CustomHook & { __conventionSource?: boolean }).__conventionSource = true; + } + } + + return { hooks: allHooks, conventionSources }; +} diff --git a/src/hooks/handler.ts b/src/hooks/handler.ts index 1bdb6e39..814ce9f2 100644 --- a/src/hooks/handler.ts +++ b/src/hooks/handler.ts @@ -11,7 +11,8 @@ import { readMergedHooksConfig } from "./hooks-config"; import { registerBuiltinPolicies } from "./builtin-policies"; import { evaluatePolicies } from "./policy-evaluator"; import { clearPolicies, registerPolicy } from "./policy-registry"; -import { loadCustomHooks } from "./custom-hooks-loader"; +import { loadAllCustomHooks } from "./custom-hooks-loader"; +import type { CustomHook } from "./policy-types"; import { persistHookActivity } from "./hook-activity-store"; import { trackHookEvent } from "./hook-telemetry"; import { getInstanceId } from "../../lib/telemetry-id"; @@ -71,9 +72,14 @@ export async function handleHookEvent(eventType: string): Promise { registerBuiltinPolicies(config.enabledPolicies); // Load and register custom hooks (layer 2, after builtins) - const customHooksList = await loadCustomHooks(config.customPoliciesPath, { sessionCwd: session.cwd }); + const loadResult = await loadAllCustomHooks(config.customPoliciesPath, { sessionCwd: session.cwd }); + const customHooksList = loadResult.hooks; + const conventionHookNames = new Set(loadResult.conventionSources.flatMap((s) => s.hookNames)); + for (const hook of customHooksList) { const hookName = hook.name; + const isConvention = (hook as CustomHook & { __conventionSource?: boolean }).__conventionSource === true; + const prefix = isConvention ? "convention" : "custom"; const fn: PolicyFunction = async (ctx): Promise => { try { const result = await Promise.race([ @@ -86,17 +92,18 @@ export async function handleHookEvent(eventType: string): Promise { } catch (err) { const msg = err instanceof Error ? err.message : String(err); const isTimeout = msg === "timeout"; - hookLogWarn(`custom hook "${hookName}" failed: ${msg}`); + hookLogWarn(`${prefix} hook "${hookName}" failed: ${msg}`); void trackHookEvent(getInstanceId(), "custom_hook_error", { hook_name: hookName, error_type: isTimeout ? "timeout" : "exception", event_type: eventType, + is_convention_policy: isConvention, }); return { decision: "allow" }; } }; registerPolicy( - `custom/${hookName}`, + `${prefix}/${hookName}`, hook.description ?? "", fn, hook.match ?? {}, @@ -113,7 +120,18 @@ export async function handleHookEvent(eventType: string): Promise { }); } - hookLogInfo(`event=${eventType} policies=${config.enabledPolicies.length} custom=${customHooksList.length}`); + // Fire telemetry for convention-based policy discovery + if (loadResult.conventionSources.length > 0) { + void trackHookEvent(getInstanceId(), "convention_policies_loaded", { + event_type: eventType, + project_file_count: loadResult.conventionSources.filter((s) => s.scope === "project").length, + user_file_count: loadResult.conventionSources.filter((s) => s.scope === "user").length, + convention_hook_count: conventionHookNames.size, + convention_hook_names: [...conventionHookNames], + }); + } + + hookLogInfo(`event=${eventType} policies=${config.enabledPolicies.length} custom=${customHooksList.length} convention=${conventionHookNames.size}`); // Evaluate policies const result = await evaluatePolicies(eventType as HookEventType, parsed, session, config); @@ -152,8 +170,9 @@ export async function handleHookEvent(eventType: string): Promise { if (result.decision === "deny" || result.decision === "instruct") { try { const isCustomHook = result.policyName?.startsWith("custom/") ?? false; + const isConventionPolicy = result.policyName?.startsWith("convention/") ?? false; const hasCustomParams = - !isCustomHook && !!(result.policyName && config.policyParams?.[result.policyName]); + !isCustomHook && !isConventionPolicy && !!(result.policyName && config.policyParams?.[result.policyName]); const paramKeysOverridden = hasCustomParams ? Object.keys(config.policyParams![result.policyName!]) : []; @@ -164,6 +183,7 @@ export async function handleHookEvent(eventType: string): Promise { policy_name: result.policyName, decision: result.decision, is_custom_hook: isCustomHook, + is_convention_policy: isConventionPolicy, has_custom_params: hasCustomParams, param_keys_overridden: paramKeysOverridden, }); diff --git a/src/hooks/manager.ts b/src/hooks/manager.ts index 19ace5cb..3ef62cbf 100644 --- a/src/hooks/manager.ts +++ b/src/hooks/manager.ts @@ -18,7 +18,7 @@ import { promptPolicySelection } from "./install-prompt"; import { readMergedHooksConfig, readScopedHooksConfig, writeScopedHooksConfig } from "./hooks-config"; import type { HooksConfig } from "./policy-types"; import { BUILTIN_POLICIES } from "./builtin-policies"; -import { loadCustomHooks } from "./custom-hooks-loader"; +import { loadCustomHooks, discoverPolicyFiles } from "./custom-hooks-loader"; import { trackHookEvent } from "./hook-telemetry"; import { getInstanceId, hashToId } from "../../lib/telemetry-id"; import { CliError } from "../cli-error"; @@ -650,4 +650,35 @@ export async function listHooks(cwd?: string): Promise { } console.log(); } + + // Convention Policies section (.failproofai/policies/*policies.{js,mjs,ts}) + const base = cwd ? resolve(cwd) : process.cwd(); + const conventionDirs: { label: string; dir: string }[] = [ + { label: "Project", dir: resolve(base, ".failproofai", "policies") }, + { label: "User", dir: resolve(homedir(), ".failproofai", "policies") }, + ]; + + for (const { label, dir } of conventionDirs) { + const files = discoverPolicyFiles(dir); + if (files.length === 0) continue; + + console.log(`\n \u2500\u2500 Convention Policies \u2014 ${label} (${dir}) \u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500`); + for (const file of files) { + try { + const hooks = await loadCustomHooks(file); + if (hooks.length === 0) { + const filename = file.split("/").pop() ?? file; + console.log(` \x1B[31m\u2717\x1B[0m ${filename.padEnd(nameColWidth)}\x1B[31mfailed to load\x1B[0m`); + } else { + const filename = file.split("/").pop() ?? file; + const hookSummary = hooks.map((h) => h.name).join(", "); + console.log(` \x1B[32m\u2713\x1B[0m ${filename.padEnd(nameColWidth)}${hooks.length} hook(s): ${hookSummary}`); + } + } catch { + const filename = file.split("/").pop() ?? file; + console.log(` \x1B[31m\u2717\x1B[0m ${filename.padEnd(nameColWidth)}\x1B[31merror\x1B[0m`); + } + } + console.log(); + } } From bec03cf2d8ff9b3d2bc23892114b0607e1efcada Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Mon, 13 Apr 2026 16:55:01 -0700 Subject: [PATCH 2/5] fix: address CodeRabbit review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix PR number reference in CHANGELOG (#88 → #89) - Add `text` language identifier to fenced code block in README - Track convention hooks by object reference instead of name to prevent mis-tagging explicit hooks that share a convention hook name - Use `basename()` instead of `file.split("/")` for cross-platform paths Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 2 +- README.md | 2 +- src/hooks/custom-hooks-loader.ts | 11 ++++++++--- src/hooks/manager.ts | 8 ++++---- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4d8456d..eb0c78e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features - Check third-party bot statuses (CodeRabbit, SonarCloud, etc.) in `require-ci-green-before-stop` policy (#90) -- Convention-based policy auto-discovery: drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` at project or user level for automatic loading — no config changes needed (#88) +- Convention-based policy auto-discovery: drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` at project or user level for automatic loading — no config changes needed (#89) - Auto-bump version after release (#73) ### Fixes diff --git a/README.md b/README.md index 627d4f1e..b09effac 100644 --- a/README.md +++ b/README.md @@ -220,7 +220,7 @@ Custom hooks support transitive local imports, async/await, and access to `proce Drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` and they're automatically loaded — no `--custom` flag or config changes needed. Works like git hooks: drop a file, it just works. -``` +```text # Project level — committed to git, shared with the team .failproofai/policies/security-policies.mjs .failproofai/policies/workflow-policies.mjs diff --git a/src/hooks/custom-hooks-loader.ts b/src/hooks/custom-hooks-loader.ts index 83de4b18..e978475b 100644 --- a/src/hooks/custom-hooks-loader.ts +++ b/src/hooks/custom-hooks-loader.ts @@ -181,10 +181,15 @@ export async function loadAllCustomHooks( ); } - // Tag convention hooks so the handler can register them with a "convention/" prefix - const conventionHookNames = new Set(conventionSources.flatMap((s) => s.hookNames)); + // Tag convention hooks so the handler can register them with a "convention/" prefix. + // Track by object reference (not name) to avoid mis-tagging an explicit custom hook + // that happens to share the same name as a convention hook. + const conventionHookRefs = new Set(); + for (const hook of allHooks.slice(hooksBeforeConvention)) { + conventionHookRefs.add(hook); + } for (const hook of allHooks) { - if (conventionHookNames.has(hook.name)) { + if (conventionHookRefs.has(hook)) { (hook as CustomHook & { __conventionSource?: boolean }).__conventionSource = true; } } diff --git a/src/hooks/manager.ts b/src/hooks/manager.ts index 3ef62cbf..6547f11e 100644 --- a/src/hooks/manager.ts +++ b/src/hooks/manager.ts @@ -3,7 +3,7 @@ */ import { execSync } from "node:child_process"; import { readFileSync, writeFileSync, existsSync, mkdirSync } from "node:fs"; -import { resolve, dirname } from "node:path"; +import { resolve, dirname, basename } from "node:path"; import { homedir, platform, arch, release, hostname } from "node:os"; import { HOOK_EVENT_TYPES, @@ -667,15 +667,15 @@ export async function listHooks(cwd?: string): Promise { try { const hooks = await loadCustomHooks(file); if (hooks.length === 0) { - const filename = file.split("/").pop() ?? file; + const filename = basename(file); console.log(` \x1B[31m\u2717\x1B[0m ${filename.padEnd(nameColWidth)}\x1B[31mfailed to load\x1B[0m`); } else { - const filename = file.split("/").pop() ?? file; + const filename = basename(file); const hookSummary = hooks.map((h) => h.name).join(", "); console.log(` \x1B[32m\u2713\x1B[0m ${filename.padEnd(nameColWidth)}${hooks.length} hook(s): ${hookSummary}`); } } catch { - const filename = file.split("/").pop() ?? file; + const filename = basename(file); console.log(` \x1B[31m\u2717\x1B[0m ${filename.padEnd(nameColWidth)}\x1B[31merror\x1B[0m`); } } From 0d8a4a62eada6950d0e63951d2375f8f510efe8b Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Mon, 13 Apr 2026 16:53:27 -0700 Subject: [PATCH 3/5] feat: configurable `hint` in policyParams for deny/instruct messages Allow users to append custom guidance to deny and instruct messages by adding a `hint` string to any policy's policyParams config. The hint is appended to the reason before it's sent back to Claude, giving actionable next steps without modifying the policy itself. Works with built-in, custom, and convention policies. Non-string and empty values are silently ignored for backward compatibility. Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 1 + README.md | 6 +- __tests__/e2e/hooks/policy-params.e2e.test.ts | 139 +++++++++++++ __tests__/hooks/policy-evaluator.test.ts | 190 ++++++++++++++++++ docs/configuration.mdx | 29 +++ docs/custom-policies.mdx | 4 + src/hooks/policy-evaluator.ts | 13 +- 7 files changed, 378 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb0c78e4..0dc749e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Check third-party bot statuses (CodeRabbit, SonarCloud, etc.) in `require-ci-green-before-stop` policy (#90) - Convention-based policy auto-discovery: drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` at project or user level for automatic loading — no config changes needed (#89) +- Configurable `hint` field in `policyParams` — append custom guidance to deny/instruct messages without modifying policies (#90) - Auto-bump version after release (#73) ### Fixes diff --git a/README.md b/README.md index b09effac..6cefc033 100644 --- a/README.md +++ b/README.md @@ -111,10 +111,12 @@ Policy configuration lives in `~/.failproofai/policies-config.json` (global) or ], "policyParams": { "block-sudo": { - "allowPatterns": ["sudo systemctl status", "sudo journalctl"] + "allowPatterns": ["sudo systemctl status", "sudo journalctl"], + "hint": "Use apt-get directly without sudo." }, "block-push-master": { - "protectedBranches": ["main", "release", "prod"] + "protectedBranches": ["main", "release", "prod"], + "hint": "Try creating a fresh branch instead." }, "sanitize-api-keys": { "additionalPatterns": [ diff --git a/__tests__/e2e/hooks/policy-params.e2e.test.ts b/__tests__/e2e/hooks/policy-params.e2e.test.ts index 201355c2..009f4186 100644 --- a/__tests__/e2e/hooks/policy-params.e2e.test.ts +++ b/__tests__/e2e/hooks/policy-params.e2e.test.ts @@ -240,6 +240,145 @@ describe("block-read-outside-cwd allowPaths", () => { }); }); +// ── hint — cross-cutting policyParams field ───────────────────────────────── + +describe("policyParams hint", () => { + it("appends hint to deny message for PreToolUse", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["block-sudo"], + policyParams: { "block-sudo": { hint: "Use apt-get directly instead." } }, + }); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + const output = result.parsed?.hookSpecificOutput as Record; + expect(output.permissionDecisionReason).toContain("Use apt-get directly instead."); + }); + + it("appends hint to instruct message for PreToolUse", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["warn-large-file-write"], + policyParams: { "warn-large-file-write": { thresholdKb: 100, hint: "Split into smaller files." } }, + }); + const content = "x".repeat(150 * 1024); // 150KB > 100KB threshold + const result = runHook("PreToolUse", Payloads.preToolUse.write(`${env.cwd}/out.txt`, content, env.cwd), { homeDir: env.home }); + assertInstruct(result); + const output = result.parsed?.hookSpecificOutput as Record; + expect(output.additionalContext).toContain("Split into smaller files."); + }); + + it("deny message is unchanged when no hint is configured", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["block-sudo"], + }); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + const output = result.parsed?.hookSpecificOutput as Record; + const reason = output.permissionDecisionReason as string; + // Should contain the standard deny message but NOT any hint appendage + expect(reason).toContain("failproofai because:"); + expect(reason).not.toContain(". ."); + }); + + it("appends hint to PostToolUse deny message", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["sanitize-api-keys"], + policyParams: { "sanitize-api-keys": { hint: "Redact the key before sharing." } }, + }); + const output = "sk-ant-api03-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + const result = runHook("PostToolUse", Payloads.postToolUse.bash("cat key.txt", output, env.cwd), { homeDir: env.home }); + assertPostToolUseDeny(result); + const hookOutput = result.parsed?.hookSpecificOutput as Record; + expect(hookOutput.additionalContext).toContain("Redact the key before sharing."); + }); + + it("ignores non-string hint value", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["block-sudo"], + policyParams: { "block-sudo": { hint: 42 } }, + }); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + const output = result.parsed?.hookSpecificOutput as Record; + const reason = output.permissionDecisionReason as string; + // Should not have ". 42" appended + expect(reason).not.toContain("42"); + }); +}); + +// ── hint — cross-cutting policyParams field ───────────────────────────────── + +describe("policyParams hint", () => { + it("appends hint to deny message for PreToolUse", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["block-sudo"], + policyParams: { "block-sudo": { hint: "Use apt-get directly instead." } }, + }); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + const output = result.parsed?.hookSpecificOutput as Record; + expect(output.permissionDecisionReason).toContain("Use apt-get directly instead."); + }); + + it("appends hint to instruct message for PreToolUse", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["warn-large-file-write"], + policyParams: { "warn-large-file-write": { thresholdKb: 100, hint: "Split into smaller files." } }, + }); + const content = "x".repeat(150 * 1024); // 150KB > 100KB threshold + const result = runHook("PreToolUse", Payloads.preToolUse.write(`${env.cwd}/out.txt`, content, env.cwd), { homeDir: env.home }); + assertInstruct(result); + const output = result.parsed?.hookSpecificOutput as Record; + expect(output.additionalContext).toContain("Split into smaller files."); + }); + + it("deny message is unchanged when no hint is configured", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["block-sudo"], + }); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + const output = result.parsed?.hookSpecificOutput as Record; + const reason = output.permissionDecisionReason as string; + expect(reason).toContain("failproofai because:"); + expect(reason).not.toContain(". ."); + }); + + it("appends hint to PostToolUse deny message", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["sanitize-jwt"], + policyParams: { "sanitize-jwt": { hint: "Redact the token before sharing." } }, + }); + // Fake JWT that triggers sanitize-jwt + const jwtOutput = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U"; + const result = runHook("PostToolUse", Payloads.postToolUse.bash("cat token.txt", jwtOutput, env.cwd), { homeDir: env.home }); + assertPostToolUseDeny(result); + const hookOutput = result.parsed?.hookSpecificOutput as Record; + expect(hookOutput.additionalContext).toContain("Redact the token before sharing."); + }); + + it("ignores non-string hint value", () => { + const env = createFixtureEnv(); + env.writeConfig({ + enabledPolicies: ["block-sudo"], + policyParams: { "block-sudo": { hint: 42 } }, + }); + const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); + assertPreToolUseDeny(result); + const output = result.parsed?.hookSpecificOutput as Record; + const reason = output.permissionDecisionReason as string; + expect(reason).not.toContain("42"); + }); +}); + // ── block-work-on-main — protectedBranches ─────────────────────────────────── describe("block-work-on-main protectedBranches", () => { diff --git a/__tests__/hooks/policy-evaluator.test.ts b/__tests__/hooks/policy-evaluator.test.ts index d32d3aa1..933d465d 100644 --- a/__tests__/hooks/policy-evaluator.test.ts +++ b/__tests__/hooks/policy-evaluator.test.ts @@ -482,4 +482,194 @@ describe("hooks/policy-evaluator", () => { expect(result.policyName).toBe("checker"); }); }); + + describe("hint appending", () => { + it("appends hint to deny reason for PreToolUse", async () => { + registerPolicy("block-force-push", "desc", () => ({ + decision: "deny", + reason: "Force-pushing is blocked", + }), { events: ["PreToolUse"] }); + + const config = { + enabledPolicies: ["block-force-push"], + policyParams: { "block-force-push": { hint: "Try creating a fresh branch instead." } }, + }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Bash", tool_input: { command: "git push --force" } }, undefined, config); + expect(result.decision).toBe("deny"); + expect(result.reason).toBe("Force-pushing is blocked. Try creating a fresh branch instead."); + const parsed = JSON.parse(result.stdout); + expect(parsed.hookSpecificOutput.permissionDecisionReason).toContain("Try creating a fresh branch instead."); + }); + + it("appends hint to deny reason for PostToolUse", async () => { + registerPolicy("scrubber", "desc", () => ({ + decision: "deny", + reason: "Secret detected", + }), { events: ["PostToolUse"] }); + + const config = { + enabledPolicies: ["scrubber"], + policyParams: { scrubber: { hint: "Remove the secret before retrying." } }, + }; + const result = await evaluatePolicies("PostToolUse", { tool_name: "Bash" }, undefined, config); + expect(result.decision).toBe("deny"); + expect(result.reason).toBe("Secret detected. Remove the secret before retrying."); + const parsed = JSON.parse(result.stdout); + expect(parsed.hookSpecificOutput.additionalContext).toContain("Remove the secret before retrying."); + }); + + it("appends hint to deny reason for other event types (exit 2)", async () => { + registerPolicy("blocker", "desc", () => ({ + decision: "deny", + reason: "nope", + }), { events: ["SessionStart"] }); + + const config = { + enabledPolicies: ["blocker"], + policyParams: { blocker: { hint: "Ask admin for access." } }, + }; + const result = await evaluatePolicies("SessionStart", {}, undefined, config); + expect(result.exitCode).toBe(2); + expect(result.reason).toBe("nope. Ask admin for access."); + expect(result.stderr).toBe("nope. Ask admin for access."); + }); + + it("appends hint to instruct reason", async () => { + registerPolicy("advisor", "desc", () => ({ + decision: "instruct", + reason: "Large file detected", + }), { events: ["PreToolUse"] }); + + const config = { + enabledPolicies: ["advisor"], + policyParams: { advisor: { hint: "Consider splitting into smaller files." } }, + }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Write" }, undefined, config); + expect(result.decision).toBe("instruct"); + expect(result.reason).toBe("Large file detected. Consider splitting into smaller files."); + const parsed = JSON.parse(result.stdout); + expect(parsed.hookSpecificOutput.additionalContext).toContain("Consider splitting into smaller files."); + }); + + it("appends hint to instruct reason on Stop event", async () => { + registerPolicy("verify", "desc", () => ({ + decision: "instruct", + reason: "Unsatisfied intents", + }), { events: ["Stop"] }); + + const config = { + enabledPolicies: ["verify"], + policyParams: { verify: { hint: "Run the test suite first." } }, + }; + const result = await evaluatePolicies("Stop", {}, undefined, config); + expect(result.exitCode).toBe(2); + expect(result.decision).toBe("instruct"); + expect(result.reason).toBe("Unsatisfied intents. Run the test suite first."); + expect(result.stderr).toBe("Unsatisfied intents. Run the test suite first."); + }); + + it("does not alter reason when no hint is configured", async () => { + registerPolicy("blocker", "desc", () => ({ + decision: "deny", + reason: "blocked", + }), { events: ["PreToolUse"] }); + + const config = { enabledPolicies: ["blocker"] }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, config); + expect(result.reason).toBe("blocked"); + }); + + it("does not alter reason when policyParams has no hint key", async () => { + registerPolicy("blocker", "desc", () => ({ + decision: "deny", + reason: "blocked", + }), { events: ["PreToolUse"] }); + + const config = { + enabledPolicies: ["blocker"], + policyParams: { blocker: { someOtherParam: "value" } }, + }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, config); + expect(result.reason).toBe("blocked"); + }); + + it("ignores hint when it is not a string (number)", async () => { + registerPolicy("blocker", "desc", () => ({ + decision: "deny", + reason: "blocked", + }), { events: ["PreToolUse"] }); + + const config = { + enabledPolicies: ["blocker"], + policyParams: { blocker: { hint: 123 } }, + }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, config); + expect(result.reason).toBe("blocked"); + }); + + it("ignores hint when it is an empty string", async () => { + registerPolicy("blocker", "desc", () => ({ + decision: "deny", + reason: "blocked", + }), { events: ["PreToolUse"] }); + + const config = { + enabledPolicies: ["blocker"], + policyParams: { blocker: { hint: "" } }, + }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, config); + expect(result.reason).toBe("blocked"); + }); + + it("works with custom/ prefixed policy names", async () => { + registerPolicy("custom/my-hook", "custom", () => ({ + decision: "deny", + reason: "custom block", + }), { events: ["PreToolUse"] }, -1); + + const config = { + enabledPolicies: [], + policyParams: { "custom/my-hook": { hint: "Ask the user for approval." } }, + }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, config); + expect(result.reason).toBe("custom block. Ask the user for approval."); + }); + + it("works with convention/ prefixed policy names", async () => { + registerPolicy("convention/my-policy", "convention", () => ({ + decision: "deny", + reason: "convention block", + }), { events: ["PreToolUse"] }, -1); + + const config = { + enabledPolicies: [], + policyParams: { "convention/my-policy": { hint: "Check project CLAUDE.md." } }, + }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, config); + expect(result.reason).toBe("convention block. Check project CLAUDE.md."); + }); + + it("hint on instruct does not affect subsequent deny", async () => { + registerPolicy("advisor", "desc", () => ({ + decision: "instruct", + reason: "heads up", + }), { events: ["PreToolUse"] }); + registerPolicy("blocker", "desc", () => ({ + decision: "deny", + reason: "hard block", + }), { events: ["PreToolUse"] }); + + const config = { + enabledPolicies: ["advisor", "blocker"], + policyParams: { + advisor: { hint: "instruct hint" }, + blocker: { hint: "deny hint" }, + }, + }; + const result = await evaluatePolicies("PreToolUse", { tool_name: "Bash" }, undefined, config); + // Deny still takes precedence + expect(result.decision).toBe("deny"); + expect(result.reason).toBe("hard block. deny hint"); + }); + }); }); diff --git a/docs/configuration.mdx b/docs/configuration.mdx index 9d9499c2..b8123b24 100644 --- a/docs/configuration.mdx +++ b/docs/configuration.mdx @@ -116,6 +116,35 @@ If a policy has parameters but you don't specify them, the policy's built-in def Unknown keys inside a policy's params block are silently ignored at hook-fire time but flagged as warnings when you run `failproofai policies`. +#### `hint` (cross-cutting) + +Type: `string` (optional) + +A message appended to the reason when a policy returns `deny` or `instruct`. Use it to give Claude actionable guidance without modifying the policy itself. + +Works with any policy type — built-in, custom (`custom/`), or convention (`convention/`). + +```json +{ + "policyParams": { + "block-force-push": { + "hint": "Try creating a fresh branch instead." + }, + "block-sudo": { + "allowPatterns": ["sudo apt-get"], + "hint": "Use apt-get directly without sudo." + }, + "custom/my-policy": { + "hint": "Ask the user for approval first." + } + } +} +``` + +When `block-force-push` denies, Claude sees: *"Force-pushing is blocked. Try creating a fresh branch instead."* + +Non-string values and empty strings are silently ignored. If `hint` is not set, behavior is unchanged (backward-compatible). + ### `customPoliciesPath` Type: `string` (absolute path) diff --git a/docs/custom-policies.mdx b/docs/custom-policies.mdx index aae75a6e..1389c34d 100644 --- a/docs/custom-policies.mdx +++ b/docs/custom-policies.mdx @@ -121,6 +121,10 @@ customPolicies.add({ `instruct(message)` - the message is appended to Claude's context for the current tool call. The first `instruct` wins — subsequent `instruct` returns from other policies are ignored. + +You can append extra guidance to any `deny` or `instruct` message by adding a `hint` field in `policyParams` — no code change needed. This works for custom (`custom/`) and convention (`convention/`) policies too. See [Configuration → hint](/configuration#hint-cross-cutting) for details. + + ### Informational allow messages (beta) diff --git a/src/hooks/policy-evaluator.ts b/src/hooks/policy-evaluator.ts index 5faeac9e..7053237f 100644 --- a/src/hooks/policy-evaluator.ts +++ b/src/hooks/policy-evaluator.ts @@ -80,7 +80,11 @@ export async function evaluatePolicies( } if (result.decision === "deny") { - const reason = result.reason ?? `Blocked by policy: ${policy.name}`; + let reason = result.reason ?? `Blocked by policy: ${policy.name}`; + const denyHint = config?.policyParams?.[policy.name]?.hint; + if (typeof denyHint === "string" && denyHint) { + reason = `${reason}. ${denyHint}`; + } hookLogInfo(`deny by "${policy.name}": ${reason}`); const displayTool = ctx.toolName ?? "unknown tool"; @@ -134,7 +138,12 @@ export async function evaluatePolicies( // Accumulate first instruct (does not short-circuit — later policies can still deny) if (result.decision === "instruct" && !instructPolicyName) { instructPolicyName = policy.name; - instructReason = result.reason ?? `Instruction from policy: ${policy.name}`; + let reason = result.reason ?? `Instruction from policy: ${policy.name}`; + const instructHint = config?.policyParams?.[policy.name]?.hint; + if (typeof instructHint === "string" && instructHint) { + reason = `${reason}. ${instructHint}`; + } + instructReason = reason; hookLogInfo(`instruct by "${policy.name}": ${instructReason}`); } From 631d02faefde8340204f4131bfa1f219399b3401 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Mon, 13 Apr 2026 23:57:59 +0000 Subject: [PATCH 4/5] fix: add missing vitest expect import in policy-params e2e test Co-Authored-By: Claude Opus 4.6 --- __tests__/e2e/hooks/policy-params.e2e.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/e2e/hooks/policy-params.e2e.test.ts b/__tests__/e2e/hooks/policy-params.e2e.test.ts index 009f4186..b003a279 100644 --- a/__tests__/e2e/hooks/policy-params.e2e.test.ts +++ b/__tests__/e2e/hooks/policy-params.e2e.test.ts @@ -4,7 +4,7 @@ * Tests that policyParams in config are correctly injected into policy functions * via policy-evaluator.ts, overriding schema defaults. */ -import { describe, it } from "vitest"; +import { describe, it, expect } from "vitest"; import { runHook, assertAllow, assertPreToolUseDeny, assertPostToolUseDeny, assertInstruct } from "../helpers/hook-runner"; import { createFixtureEnv } from "../helpers/fixture-env"; import { Payloads } from "../helpers/payloads"; From 67b7c334f4cd77115f175b85d5adeb788d8b8267 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Tue, 14 Apr 2026 05:28:01 +0000 Subject: [PATCH 5/5] fix: address CodeRabbit review feedback (round 2) - Remove duplicate policyParams hint test suite, merge unique sanitize-jwt test - Fix CHANGELOG PR references from #89/#90 to #91 - Extract appendHint() helper to trim/normalize hint concatenation Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 4 +- __tests__/e2e/hooks/policy-params.e2e.test.ts | 57 +------------------ src/hooks/policy-evaluator.ts | 27 +++++---- 3 files changed, 19 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dc749e4..2629f250 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,8 @@ ### Features - Check third-party bot statuses (CodeRabbit, SonarCloud, etc.) in `require-ci-green-before-stop` policy (#90) -- Convention-based policy auto-discovery: drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` at project or user level for automatic loading — no config changes needed (#89) -- Configurable `hint` field in `policyParams` — append custom guidance to deny/instruct messages without modifying policies (#90) +- Convention-based policy auto-discovery: drop `*policies.{js,mjs,ts}` files into `.failproofai/policies/` at project or user level for automatic loading — no config changes needed (#91) +- Configurable `hint` field in `policyParams` — append custom guidance to deny/instruct messages without modifying policies (#91) - Auto-bump version after release (#73) ### Fixes diff --git a/__tests__/e2e/hooks/policy-params.e2e.test.ts b/__tests__/e2e/hooks/policy-params.e2e.test.ts index b003a279..0dbfb784 100644 --- a/__tests__/e2e/hooks/policy-params.e2e.test.ts +++ b/__tests__/e2e/hooks/policy-params.e2e.test.ts @@ -308,50 +308,8 @@ describe("policyParams hint", () => { // Should not have ". 42" appended expect(reason).not.toContain("42"); }); -}); - -// ── hint — cross-cutting policyParams field ───────────────────────────────── - -describe("policyParams hint", () => { - it("appends hint to deny message for PreToolUse", () => { - const env = createFixtureEnv(); - env.writeConfig({ - enabledPolicies: ["block-sudo"], - policyParams: { "block-sudo": { hint: "Use apt-get directly instead." } }, - }); - const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); - assertPreToolUseDeny(result); - const output = result.parsed?.hookSpecificOutput as Record; - expect(output.permissionDecisionReason).toContain("Use apt-get directly instead."); - }); - - it("appends hint to instruct message for PreToolUse", () => { - const env = createFixtureEnv(); - env.writeConfig({ - enabledPolicies: ["warn-large-file-write"], - policyParams: { "warn-large-file-write": { thresholdKb: 100, hint: "Split into smaller files." } }, - }); - const content = "x".repeat(150 * 1024); // 150KB > 100KB threshold - const result = runHook("PreToolUse", Payloads.preToolUse.write(`${env.cwd}/out.txt`, content, env.cwd), { homeDir: env.home }); - assertInstruct(result); - const output = result.parsed?.hookSpecificOutput as Record; - expect(output.additionalContext).toContain("Split into smaller files."); - }); - - it("deny message is unchanged when no hint is configured", () => { - const env = createFixtureEnv(); - env.writeConfig({ - enabledPolicies: ["block-sudo"], - }); - const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); - assertPreToolUseDeny(result); - const output = result.parsed?.hookSpecificOutput as Record; - const reason = output.permissionDecisionReason as string; - expect(reason).toContain("failproofai because:"); - expect(reason).not.toContain(". ."); - }); - it("appends hint to PostToolUse deny message", () => { + it("appends hint to PostToolUse deny message (sanitize-jwt)", () => { const env = createFixtureEnv(); env.writeConfig({ enabledPolicies: ["sanitize-jwt"], @@ -364,19 +322,6 @@ describe("policyParams hint", () => { const hookOutput = result.parsed?.hookSpecificOutput as Record; expect(hookOutput.additionalContext).toContain("Redact the token before sharing."); }); - - it("ignores non-string hint value", () => { - const env = createFixtureEnv(); - env.writeConfig({ - enabledPolicies: ["block-sudo"], - policyParams: { "block-sudo": { hint: 42 } }, - }); - const result = runHook("PreToolUse", Payloads.preToolUse.bash("sudo rm -rf /", env.cwd), { homeDir: env.home }); - assertPreToolUseDeny(result); - const output = result.parsed?.hookSpecificOutput as Record; - const reason = output.permissionDecisionReason as string; - expect(reason).not.toContain("42"); - }); }); // ── block-work-on-main — protectedBranches ─────────────────────────────────── diff --git a/src/hooks/policy-evaluator.ts b/src/hooks/policy-evaluator.ts index 7053237f..26aa5462 100644 --- a/src/hooks/policy-evaluator.ts +++ b/src/hooks/policy-evaluator.ts @@ -8,6 +8,14 @@ import { BUILTIN_POLICIES } from "./builtin-policies"; import { getPoliciesForEvent } from "./policy-registry"; import { hookLogInfo, hookLogWarn } from "./hook-logger"; +function appendHint(baseReason: string, hint: unknown): string { + const base = baseReason.trim(); + const normalizedHint = typeof hint === "string" ? hint.trim() : ""; + if (!normalizedHint) return base; + if (!base) return normalizedHint; + return `${base}. ${normalizedHint}`; +} + export interface EvaluationResult { exitCode: number; stdout: string; @@ -80,11 +88,10 @@ export async function evaluatePolicies( } if (result.decision === "deny") { - let reason = result.reason ?? `Blocked by policy: ${policy.name}`; - const denyHint = config?.policyParams?.[policy.name]?.hint; - if (typeof denyHint === "string" && denyHint) { - reason = `${reason}. ${denyHint}`; - } + const reason = appendHint( + result.reason ?? `Blocked by policy: ${policy.name}`, + config?.policyParams?.[policy.name]?.hint, + ); hookLogInfo(`deny by "${policy.name}": ${reason}`); const displayTool = ctx.toolName ?? "unknown tool"; @@ -138,12 +145,10 @@ export async function evaluatePolicies( // Accumulate first instruct (does not short-circuit — later policies can still deny) if (result.decision === "instruct" && !instructPolicyName) { instructPolicyName = policy.name; - let reason = result.reason ?? `Instruction from policy: ${policy.name}`; - const instructHint = config?.policyParams?.[policy.name]?.hint; - if (typeof instructHint === "string" && instructHint) { - reason = `${reason}. ${instructHint}`; - } - instructReason = reason; + instructReason = appendHint( + result.reason ?? `Instruction from policy: ${policy.name}`, + config?.policyParams?.[policy.name]?.hint, + ); hookLogInfo(`instruct by "${policy.name}": ${instructReason}`); }