diff --git a/CHANGELOG.md b/CHANGELOG.md index 750d877c..6cc8f42d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased (0.0.2-beta.7) ### Features +- Configurable `hint` field in `policyParams` — append custom guidance to deny/instruct messages without modifying policies - Auto-bump version after release (#73) ### Fixes diff --git a/README.md b/README.md index f1b533ee..34019a3c 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 56793dfc..5631f639 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 4302eb2b..5ab202d6 100644 --- a/docs/custom-policies.mdx +++ b/docs/custom-policies.mdx @@ -87,6 +87,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}`); }