feat(miner-governor): PreToolUse-hook-enforced house rules (#2343)

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<string, unknown>;
+  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<PreToolUseHookJSONOutput | Record<string, never>>;

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<import("./deny-hooks.js").DenyRule>} [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<Record<string, unknown>>}
+ */
+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);
+    }
+  };
+}

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": "*"

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<typeof ledger.appendGovernorEvent>[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"),
+      },
+    });
+  });
+});