diff --git a/packages/gittensory-miner/lib/pretooluse-hook.d.ts b/packages/gittensory-miner/lib/pretooluse-hook.d.ts new file mode 100644 index 0000000000..2147dbbc51 --- /dev/null +++ b/packages/gittensory-miner/lib/pretooluse-hook.d.ts @@ -0,0 +1,31 @@ +import type { DenyRule } from "./deny-hooks.js"; +import type { AppendGovernorEventInput, GovernorLedgerEntry } from "./governor-ledger.js"; + +export type BuildHouseRulesPreToolUseHookConfig = { + rules?: readonly DenyRule[]; + repoFullName?: string; +}; + +export type BuildHouseRulesPreToolUseHookOptions = { + append?: (event: AppendGovernorEventInput) => GovernorLedgerEntry; +}; + +/** Minimal shape this module reads from the real Agent SDK `PreToolUseHookInput`. */ +export type PreToolUseHookLikeInput = { + tool_name?: string; + tool_input?: Record; + hook_event_name?: string; +}; + +export type PreToolUseHookJSONOutput = { + hookSpecificOutput?: { + hookEventName: "PreToolUse"; + permissionDecision: "deny"; + permissionDecisionReason: string; + }; +}; + +export function buildHouseRulesPreToolUseHook( + config?: BuildHouseRulesPreToolUseHookConfig, + options?: BuildHouseRulesPreToolUseHookOptions, +): (input: PreToolUseHookLikeInput, toolUseId?: string, context?: unknown) => Promise>; diff --git a/packages/gittensory-miner/lib/pretooluse-hook.js b/packages/gittensory-miner/lib/pretooluse-hook.js new file mode 100644 index 0000000000..124f0007d2 --- /dev/null +++ b/packages/gittensory-miner/lib/pretooluse-hook.js @@ -0,0 +1,93 @@ +// PreToolUse-hook-enforced house rules (#2343). Wraps the pure `evaluateDenyHooks` decision function +// (deny-hooks.js, #2295) into a real Claude Agent SDK PreToolUse hook callback -- the actual live +// interception point a CodingAgentDriver session registers via `options.hooks.PreToolUse` (the exact +// seam `agent-sdk-driver.ts`'s `hooks` passthrough documents as "#2343's stated attachment point"). +// +// WHY THIS HOLDS EVEN UNDER bypassPermissions: per the Agent SDK's own documented permission-evaluation +// order (https://code.claude.com/docs/en/agent-sdk/permissions), hooks run FIRST -- before deny rules, +// ask rules, the permission mode check, and allow rules -- and "Hooks still execute and can block +// operations if needed" even when `permissionMode: 'bypassPermissions'` is set: "Deny rules +// (disallowed_tools), explicit ask rules, and hooks are evaluated before the mode check and can still +// block a tool." This module does not implement that guarantee -- the SDK does. This module's job is +// only to return a correctly-shaped, fail-closed deny decision every time; the SDK is what makes that +// decision unbypassable. +// +// FAIL CLOSED: any internal error (a malformed tool-call shape, a governor-ledger append failure) denies +// rather than silently allowing. + +import { DEFAULT_DENY_RULES, evaluateDenyHooks } from "./deny-hooks.js"; +import { appendGovernorEvent } from "./governor-ledger.js"; + +function recordDenial(append, repoFullName, reason, payload) { + try { + append({ + eventType: "denied", + repoFullName: repoFullName ?? null, + actionClass: "pretooluse_hook", + decision: "deny", + reason, + payload, + }); + } catch { + // A ledger append failure must never suppress or alter the deny decision itself -- the tool call is + // still blocked even if the audit write fails. Silently allowing on a logging failure would be a far + // worse outcome for a security boundary than an unrecorded (but still enforced) denial. + } +} + +function denyOutput(reason) { + return { + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: reason, + }, + }; +} + +/** + * Build a Claude Agent SDK `PreToolUse` hook callback enforcing the house-rule denylist. Register the + * returned function under `options.hooks.PreToolUse` (e.g. `{ hooks: [PreToolUse: [{ hooks: [built] }]] }` + * on the object passed to `createAgentSdkCodingAgentDriver({ hooks })`). + * + * House rules are sourced from a single, auditable list: {@link DEFAULT_DENY_RULES} by default, or an + * effective rule set built by the caller (e.g. `resolveEffectiveDenyRules` from deny-hook-synthesis.js, + * merging in maintainer-approved synthesized rules) — this module composes whatever rule set it is given, + * it does not own deriving one. + * + * @param {object} [config] + * @param {ReadonlyArray} [config.rules] defaults to DEFAULT_DENY_RULES + * @param {string} [config.repoFullName] target repo, for governor-ledger scoping of recorded denials + * @param {{ append?: typeof appendGovernorEvent }} [options] + * @returns {(input: unknown, toolUseId?: string, context?: unknown) => Promise>} + */ +export function buildHouseRulesPreToolUseHook(config = {}, options = {}) { + const rules = config.rules ?? DEFAULT_DENY_RULES; + const repoFullName = config.repoFullName; + const append = options.append ?? appendGovernorEvent; + + return async function houseRulesPreToolUseHook(input) { + try { + const toolName = input && typeof input === "object" ? input.tool_name : undefined; + const toolInput = input && typeof input === "object" ? input.tool_input : undefined; + const verdict = evaluateDenyHooks({ name: toolName, input: toolInput }, rules); + + if (verdict.allowed) return {}; + + // `verdict.blockedBy` is always set together with `!verdict.allowed` (evaluateDenyHooks's only two return + // shapes), and `.matcher` is always a defined string on it (ruleMatches gates every match on + // `typeof rule.matcher === "string"` -- a rule can never become `blockedBy` otherwise). `.reason` has no + // equivalent gate, so a caller-supplied custom rule omitting it is a real, reachable case. + const reason = verdict.blockedBy.reason ?? "House rule denylist match."; + recordDenial(append, repoFullName, reason, { + toolName: typeof toolName === "string" ? toolName : null, + matcher: verdict.blockedBy.matcher, + }); + return denyOutput(reason); + } catch (error) { + const reason = `pretooluse_hook_internal_error: ${error instanceof Error ? error.message : String(error)}`; + recordDenial(append, repoFullName, reason, {}); + return denyOutput(reason); + } + }; +} diff --git a/packages/gittensory-miner/package.json b/packages/gittensory-miner/package.json index 63f6472b00..65fde4cb5b 100644 --- a/packages/gittensory-miner/package.json +++ b/packages/gittensory-miner/package.json @@ -32,7 +32,7 @@ "expected-engine.version" ], "scripts": { - "build": "node --check bin/gittensory-miner.js && node --check lib/version.js && node --check lib/local-store.js && node --check lib/cli.js && node --check lib/deny-check.js && node --check lib/run-state-cli.js && node --check lib/update-check.js && node --check lib/worktree-allocator.js && node --check lib/opportunity-fanout.js && node --check lib/ci-poller.js && node --check lib/run-state.js && node --check lib/deny-hooks.js && node --check lib/event-ledger.js && node --check lib/event-ledger-cli.js && node --check lib/claim-ledger.js && node --check lib/claim-ledger-expiry.js && node --check lib/claim-ledger-cli.js && node --check lib/claim-adjudication.js && node --check lib/portfolio-queue.js && node --check lib/portfolio-queue-cli.js && node --check lib/portfolio-queue-manager.js && node --check lib/portfolio-discovery.js && node --check lib/opportunity-ranker.js && node --check lib/plan-store.js && node --check lib/plan-store-cli.js && node --check lib/rejection-templates.js && node --check lib/governor-ledger.js && node --check lib/governor-ledger-cli.js && node --check lib/governor-open-pr.js && node --check lib/governor-write-rate-limit.js && node --check lib/governor-run-halt.js && node --check lib/governor-kill-switch.js && node --check lib/attempt-log.js && node --check lib/manage-status.js && node --check lib/manage-poll.js && node --check lib/status.js && node --check lib/laptop-init.js && node --check lib/replay-objective-anchor.js && node --check lib/replay-task-generation.js && node --check lib/calibration-types.js && node --check lib/calibration.js" + "build": "node --check bin/gittensory-miner.js && node --check lib/version.js && node --check lib/local-store.js && node --check lib/cli.js && node --check lib/deny-check.js && node --check lib/run-state-cli.js && node --check lib/update-check.js && node --check lib/worktree-allocator.js && node --check lib/opportunity-fanout.js && node --check lib/ci-poller.js && node --check lib/run-state.js && node --check lib/deny-hooks.js && node --check lib/deny-hook-synthesis.js && node --check lib/pretooluse-hook.js && node --check lib/event-ledger.js && node --check lib/event-ledger-cli.js && node --check lib/claim-ledger.js && node --check lib/claim-ledger-expiry.js && node --check lib/claim-ledger-cli.js && node --check lib/claim-adjudication.js && node --check lib/portfolio-queue.js && node --check lib/portfolio-queue-cli.js && node --check lib/portfolio-queue-manager.js && node --check lib/portfolio-discovery.js && node --check lib/opportunity-ranker.js && node --check lib/plan-store.js && node --check lib/plan-store-cli.js && node --check lib/rejection-templates.js && node --check lib/governor-ledger.js && node --check lib/governor-ledger-cli.js && node --check lib/governor-open-pr.js && node --check lib/governor-write-rate-limit.js && node --check lib/governor-run-halt.js && node --check lib/attempt-log.js && node --check lib/manage-status.js && node --check lib/manage-poll.js && node --check lib/status.js && node --check lib/laptop-init.js && node --check lib/replay-objective-anchor.js && node --check lib/replay-task-generation.js && node --check lib/calibration-types.js && node --check lib/calibration.js" }, "dependencies": { "@jsonbored/gittensory-engine": "*" diff --git a/test/unit/miner-pretooluse-hook.test.ts b/test/unit/miner-pretooluse-hook.test.ts new file mode 100644 index 0000000000..a28b9698f5 --- /dev/null +++ b/test/unit/miner-pretooluse-hook.test.ts @@ -0,0 +1,200 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { buildHouseRulesPreToolUseHook } from "../../packages/gittensory-miner/lib/pretooluse-hook.js"; +import { initGovernorLedger } from "../../packages/gittensory-miner/lib/governor-ledger.js"; + +// The live Agent SDK PreToolUse interception point (#2343). deny-hooks.js's own rule logic (matcher, glob, +// path-tokenizing, force-push detection) is already exhaustively tested in miner-deny-hooks.test.ts — these +// tests cover only this wrapper's own job: translating the real hook input shape, returning the exact +// SDK-documented deny/allow output shape, recording denials to the governor ledger, and failing closed. +// +// NOTE on "enforced even under bypassPermissions": the Agent SDK's own documented permission-evaluation order +// runs hooks BEFORE the permission-mode check, and its docs state plainly that hook denials still apply in +// bypassPermissions mode. That guarantee is the SDK's responsibility, not this module's -- it cannot be +// exercised by a unit test here without a live SDK session. What IS this module's responsibility, and what +// these tests cover, is returning a correctly-shaped deny every time, regardless of mode. + +const roots: string[] = []; +const ledgers: Array<{ close(): void }> = []; + +afterEach(() => { + for (const ledger of ledgers.splice(0)) ledger.close(); + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +function openLedger() { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-pretooluse-hook-")); + roots.push(root); + const ledger = initGovernorLedger(join(root, "governor-ledger.sqlite3")); + ledgers.push(ledger); + return ledger; +} + +describe("buildHouseRulesPreToolUseHook (#2343)", () => { + it("allows a tool call matching no house rule, returning an empty object unmodified", async () => { + const hook = buildHouseRulesPreToolUseHook(); + const result = await hook({ + hook_event_name: "PreToolUse", + tool_name: "Read", + tool_input: { file_path: "src/index.ts" }, + }); + expect(result).toEqual({}); + }); + + it("denies a tool call matching a house rule, in the exact SDK-documented hookSpecificOutput shape", async () => { + const hook = buildHouseRulesPreToolUseHook(); + const result = await hook({ + hook_event_name: "PreToolUse", + tool_name: "Write", + tool_input: { file_path: ".env" }, + }); + expect(result).toEqual({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: expect.stringContaining("environment files"), + }, + }); + }); + + it("records every denial to the governor ledger with the specific rule's reason", async () => { + const ledger = openLedger(); + const hook = buildHouseRulesPreToolUseHook( + { repoFullName: "acme/widgets" }, + { append: (event) => ledger.appendGovernorEvent(event) }, + ); + + await hook({ hook_event_name: "PreToolUse", tool_name: "Bash", tool_input: { command: "git push --force" } }); + + const rows = ledger.readGovernorEvents({ repoFullName: "acme/widgets" }); + expect(rows).toHaveLength(1); + expect(rows[0]?.actionClass).toBe("pretooluse_hook"); + expect(rows[0]?.decision).toBe("deny"); + expect(rows[0]?.reason).toContain("force-push"); + expect(rows[0]?.payload).toMatchObject({ toolName: "Bash" }); + }); + + it("does not record anything to the ledger for an allowed tool call", async () => { + const ledger = openLedger(); + const append = vi.fn((event: Parameters[0]) => ledger.appendGovernorEvent(event)); + const hook = buildHouseRulesPreToolUseHook({}, { append }); + + await hook({ hook_event_name: "PreToolUse", tool_name: "Read", tool_input: { file_path: "README.md" } }); + + expect(append).not.toHaveBeenCalled(); + }); + + it("accepts a caller-supplied effective rule set instead of the built-in defaults", async () => { + const hook = buildHouseRulesPreToolUseHook({ + rules: [{ matcher: "*", pathPattern: "**/custom-blocked.txt", reason: "Custom house rule." }], + }); + + const denied = await hook({ hook_event_name: "PreToolUse", tool_name: "Write", tool_input: { file_path: "custom-blocked.txt" } }); + expect(denied).toMatchObject({ hookSpecificOutput: { permissionDecision: "deny" } }); + + // A path that only the BUILT-IN defaults would block is allowed here, proving the custom set replaced + // (not merged with) the defaults -- composition is the caller's job (e.g. resolveEffectiveDenyRules). + const allowed = await hook({ hook_event_name: "PreToolUse", tool_name: "Read", tool_input: { file_path: ".env" } }); + expect(allowed).toEqual({}); + }); + + it("fails closed: an internal error while evaluating rules denies rather than silently allowing", async () => { + // A rule whose `matcher` property throws when accessed forces a genuine exception through the real + // evaluateDenyHooks call, exercising the wrapper's own catch-all without needing a test-only seam in + // production code. + const throwingRule = new Proxy( + {}, + { + get() { + throw new Error("synthetic rule access failure"); + }, + }, + ); + const hook = buildHouseRulesPreToolUseHook({ rules: [throwingRule as never] }); + + const result = await hook({ hook_event_name: "PreToolUse", tool_name: "Read", tool_input: { file_path: "anything.ts" } }); + + expect(result).toMatchObject({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: expect.stringContaining("pretooluse_hook_internal_error"), + }, + }); + }); + + it("fails closed even when the governor ledger append itself throws", async () => { + const throwingAppend = () => { + throw new Error("ledger unavailable"); + }; + const hook = buildHouseRulesPreToolUseHook({}, { append: throwingAppend }); + + const result = await hook({ hook_event_name: "PreToolUse", tool_name: "Write", tool_input: { file_path: ".env" } }); + + // The tool call is still denied even though the audit write failed -- a logging outage must never + // downgrade a security decision to allow. + expect(result).toMatchObject({ hookSpecificOutput: { permissionDecision: "deny" } }); + }); + + it("a non-object hook input (malformed upstream event) has no shape to match against, so it allows rather than crashing", async () => { + // `input` is typed `unknown` at this boundary -- a real SDK/harness bug could hand this wrapper something + // that isn't the documented `{ tool_name, tool_input }` shape at all. With nothing recognizable to test + // against, no house rule CAN fire (every default rule keys off tool_name/path/command content) -- this is + // "no signal to act on" rather than a security gap, since the built-in rules never produce a false allow + // from a malformed shape (they simply find nothing to match). + const hook = buildHouseRulesPreToolUseHook(); + const result = await hook(null as never); + expect(result).toEqual({}); + }); + + it("a custom rule omitting `reason` falls back to the generic denylist message", async () => { + const hook = buildHouseRulesPreToolUseHook({ + rules: [{ matcher: "*", pathPattern: "**/custom-blocked.txt" } as never], + }); + + const result = await hook({ hook_event_name: "PreToolUse", tool_name: "Write", tool_input: { file_path: "custom-blocked.txt" } }); + + expect(result).toMatchObject({ + hookSpecificOutput: { permissionDecision: "deny", permissionDecisionReason: "House rule denylist match." }, + }); + }); + + it("a deny with no string-typed tool_name records a null toolName in the ledger payload rather than crashing", async () => { + // The matcher `"*"` fires even with no tool name to test (matcherMatches substitutes "" for a non-string + // toolName before the regex test) -- a rule can deny purely on path/command content. This exercises the + // `typeof toolName === "string" ? toolName : null` fallback with a REAL deny, not a synthetic shape. + const ledger = openLedger(); + const hook = buildHouseRulesPreToolUseHook({ repoFullName: "acme/widgets" }, { append: (event) => ledger.appendGovernorEvent(event) }); + + const result = await hook({ hook_event_name: "PreToolUse", tool_input: { file_path: ".env" } }); + + expect(result).toMatchObject({ hookSpecificOutput: { permissionDecision: "deny" } }); + const rows = ledger.readGovernorEvents({ repoFullName: "acme/widgets" }); + expect(rows[0]?.payload).toMatchObject({ toolName: null }); + }); + + it("fails closed with a formatted reason even when the thrown value is not an Error instance", async () => { + const throwingRule = new Proxy( + {}, + { + get() { + // Deliberately a plain string, not `new Error(...)` -- exercises the `String(error)` fallback arm + // distinctly from the existing Error-instance fail-closed test above. + throw "synthetic non-Error rule access failure"; + }, + }, + ); + const hook = buildHouseRulesPreToolUseHook({ rules: [throwingRule as never] }); + + const result = await hook({ hook_event_name: "PreToolUse", tool_name: "Read", tool_input: { file_path: "anything.ts" } }); + + expect(result).toMatchObject({ + hookSpecificOutput: { + permissionDecisionReason: expect.stringContaining("pretooluse_hook_internal_error: synthetic non-Error rule access failure"), + }, + }); + }); +});