feat(agent): gate destructive PostHog MCP exec sub-tools (#1991)

Author: skoob13

packages/agent/src/adapters/claude/hooks.test.ts

@@ -7,7 +7,16 @@ vi.mock("../../enrichment/file-enricher", () => ({
   enrichFileForAgent: enrichFileMock,
 }));
 
-import { createReadEnrichmentHook, type EnrichedReadCache } from "./hooks";
+import { Logger } from "../../utils/logger";
+import {
+  createPreToolUseHook,
+  createReadEnrichmentHook,
+  type EnrichedReadCache,
+} from "./hooks";
+import type {
+  PermissionCheckResult,
+  SettingsManager,
+} from "./session/settings";
 
 const stubDeps = {} as FileEnrichmentDeps;
 
@@ -187,3 +196,118 @@ describe("createReadEnrichmentHook", () => {
     expect(content).toBe("foo");
   });
 });
+
+function buildPreToolUseHookInput(
+  toolName: string,
+  toolInput: Record<string, unknown>,
+): HookInput {
+  return {
+    session_id: "test-session",
+    transcript_path: "/tmp/transcript",
+    cwd: "/tmp",
+    hook_event_name: "PreToolUse",
+    tool_name: toolName,
+    tool_use_id: "toolu_1",
+    tool_input: toolInput,
+  } as HookInput;
+}
+
+function buildSettingsManagerStub(
+  result: PermissionCheckResult,
+): SettingsManager {
+  return {
+    checkPermission: () => result,
+  } as unknown as SettingsManager;
+}
+
+describe("createPreToolUseHook", () => {
+  const logger = new Logger({ debug: false });
+
+  test("defers destructive PostHog exec sub-tool to canUseTool via ask", async () => {
+    const settingsManager = buildSettingsManagerStub({
+      decision: "allow",
+      rule: "mcp__posthog__exec",
+      source: "allow",
+    });
+    const hook = createPreToolUseHook(settingsManager, logger);
+    const result = await hook(
+      buildPreToolUseHookInput("mcp__posthog__exec", {
+        command: 'call dashboard-update {"id": 1, "name": "x"}',
+      }),
+      undefined,
+      { signal: new AbortController().signal },
+    );
+
+    expect(result).toMatchObject({
+      continue: true,
+      hookSpecificOutput: {
+        hookEventName: "PreToolUse",
+        permissionDecision: "ask",
+      },
+    });
+  });
+
+  test("allows non-destructive PostHog exec sub-tool via settings rule", async () => {
+    const settingsManager = buildSettingsManagerStub({
+      decision: "allow",
+      rule: "mcp__posthog__exec",
+      source: "allow",
+    });
+    const hook = createPreToolUseHook(settingsManager, logger);
+    const result = await hook(
+      buildPreToolUseHookInput("mcp__posthog__exec", {
+        command: 'call experiment-get {"id": 1}',
+      }),
+      undefined,
+      { signal: new AbortController().signal },
+    );
+
+    expect(result).toEqual({
+      continue: true,
+      hookSpecificOutput: {
+        hookEventName: "PreToolUse",
+        permissionDecision: "allow",
+        permissionDecisionReason:
+          "Allowed by settings rule: mcp__posthog__exec",
+      },
+    });
+  });
+
+  test("allows non-PostHog tool via settings rule unchanged", async () => {
+    const settingsManager = buildSettingsManagerStub({
+      decision: "allow",
+      rule: "Bash(ls:*)",
+      source: "allow",
+    });
+    const hook = createPreToolUseHook(settingsManager, logger);
+    const result = await hook(
+      buildPreToolUseHookInput("Bash", { command: "ls -la" }),
+      undefined,
+      { signal: new AbortController().signal },
+    );
+
+    expect(result).toMatchObject({
+      hookSpecificOutput: { permissionDecision: "allow" },
+    });
+  });
+
+  test("defers when destructive rule is partial-update", async () => {
+    const settingsManager = buildSettingsManagerStub({
+      decision: "allow",
+      rule: "mcp__posthog__exec",
+      source: "allow",
+    });
+    const hook = createPreToolUseHook(settingsManager, logger);
+    const result = await hook(
+      buildPreToolUseHookInput("mcp__posthog__exec", {
+        command: 'call cohorts-partial-update {"id": 1}',
+      }),
+      undefined,
+      { signal: new AbortController().signal },
+    );
+
+    expect(result).toMatchObject({
+      hookSpecificOutput: { permissionDecision: "ask" },
+    });
+  });
+});

packages/agent/src/adapters/claude/hooks.ts

@@ -5,6 +5,11 @@ import {
 } from "../../enrichment/file-enricher";
 import type { Logger } from "../../utils/logger";
 import { stripCatLineNumbers } from "./conversion/sdk-to-acp";
+import {
+  extractPostHogSubTool,
+  isPostHogDestructiveSubTool,
+  isPostHogExecTool,
+} from "./permissions/posthog-exec-gate";
 import type { SettingsManager } from "./session/settings";
 import type { CodeExecutionMode } from "./tools";
 
@@ -237,6 +242,25 @@ export const createPreToolUseHook =
       );
     }
 
+    // Defer destructive PostHog exec sub-tools to canUseTool so the
+    // sub-tool gate can re-prompt. Returning `{ continue: true }` is
+    // not enough — the SDK then falls back to its default permission
+    // flow which re-checks the same allow rule. We must force "ask"
+    // so the SDK invokes canUseTool.
+    if (permissionCheck.decision === "allow" && isPostHogExecTool(toolName)) {
+      const subTool = extractPostHogSubTool(toolInput);
+      if (subTool && isPostHogDestructiveSubTool(subTool)) {
+        return {
+          continue: true,
+          hookSpecificOutput: {
+            hookEventName: "PreToolUse" as const,
+            permissionDecision: "ask" as const,
+            permissionDecisionReason: `Destructive PostHog sub-tool '${subTool}' requires explicit approval`,
+          },
+        };
+      }
+    }
+
     switch (permissionCheck.decision) {
       case "allow":
         return {

packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts

@@ -143,6 +143,158 @@ describe("canUseTool MCP approval enforcement", () => {
     expect(result.behavior).toBe("allow");
   });
 
+  it("bypasses the PostHog exec gate in auto mode", async () => {
+    setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
+    const hasApproval = vi.fn().mockReturnValue(false);
+    const addApproval = vi.fn().mockResolvedValue(undefined);
+
+    const context = createContext("mcp__posthog__exec", {
+      toolInput: { command: "call experiment-update {}" },
+      session: {
+        permissionMode: "auto",
+        settingsManager: {
+          getRepoRoot: vi.fn().mockReturnValue("/repo"),
+          hasPostHogExecApproval: hasApproval,
+          addPostHogExecApproval: addApproval,
+        },
+      },
+    });
+    const result = await canUseTool(context);
+
+    expect(result.behavior).toBe("allow");
+    expect(context.client.requestPermission).not.toHaveBeenCalled();
+    expect(hasApproval).not.toHaveBeenCalled();
+    expect(addApproval).not.toHaveBeenCalled();
+  });
+
+  it("bypasses the PostHog exec gate in bypassPermissions mode", async () => {
+    setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
+
+    const context = createContext("mcp__posthog__exec", {
+      toolInput: { command: "call feature-flag-delete {}" },
+      session: {
+        permissionMode: "bypassPermissions",
+        settingsManager: {
+          getRepoRoot: vi.fn().mockReturnValue("/repo"),
+          hasPostHogExecApproval: vi.fn().mockReturnValue(false),
+          addPostHogExecApproval: vi.fn(),
+        },
+      },
+    });
+    const result = await canUseTool(context);
+
+    expect(result.behavior).toBe("allow");
+    expect(context.client.requestPermission).not.toHaveBeenCalled();
+  });
+
+  it("short-circuits when a PostHog exec sub-tool was previously approved", async () => {
+    setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
+
+    const context = createContext("mcp__posthog__exec", {
+      toolInput: { command: "call experiment-update {}" },
+      session: {
+        permissionMode: "default",
+        settingsManager: {
+          getRepoRoot: vi.fn().mockReturnValue("/repo"),
+          hasPostHogExecApproval: vi
+            .fn()
+            .mockImplementation((s: string) => s === "experiment-update"),
+          addPostHogExecApproval: vi.fn(),
+        },
+      },
+    });
+    const result = await canUseTool(context);
+
+    expect(result.behavior).toBe("allow");
+    expect(context.client.requestPermission).not.toHaveBeenCalled();
+  });
+
+  it("prompts for an unapproved destructive PostHog sub-tool and persists on allow_always", async () => {
+    setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
+    const addApproval = vi.fn().mockResolvedValue(undefined);
+
+    const context = createContext("mcp__posthog__exec", {
+      toolInput: { command: "call notebooks-destroy {}" },
+      session: {
+        permissionMode: "default",
+        settingsManager: {
+          getRepoRoot: vi.fn().mockReturnValue("/repo"),
+          hasPostHogExecApproval: vi.fn().mockReturnValue(false),
+          addPostHogExecApproval: addApproval,
+        },
+      },
+      client: {
+        sessionUpdate: vi.fn().mockResolvedValue(undefined),
+        requestPermission: vi.fn().mockResolvedValue({
+          outcome: { outcome: "selected", optionId: "allow_always" },
+        }),
+      },
+    });
+    const result = await canUseTool(context);
+
+    expect(result.behavior).toBe("allow");
+    expect(context.client.requestPermission).toHaveBeenCalledWith(
+      expect.objectContaining({
+        toolCall: expect.objectContaining({
+          title: "The agent wants to run `notebooks-destroy` on PostHog",
+        }),
+      }),
+    );
+    expect(addApproval).toHaveBeenCalledWith("notebooks-destroy");
+  });
+
+  it("prompts but does not persist on allow_once", async () => {
+    setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
+    const addApproval = vi.fn();
+
+    const context = createContext("mcp__posthog__exec", {
+      toolInput: { command: "call experiment-delete {}" },
+      session: {
+        permissionMode: "default",
+        settingsManager: {
+          getRepoRoot: vi.fn().mockReturnValue("/repo"),
+          hasPostHogExecApproval: vi.fn().mockReturnValue(false),
+          addPostHogExecApproval: addApproval,
+        },
+      },
+      client: {
+        sessionUpdate: vi.fn().mockResolvedValue(undefined),
+        requestPermission: vi.fn().mockResolvedValue({
+          outcome: { outcome: "selected", optionId: "allow" },
+        }),
+      },
+    });
+    const result = await canUseTool(context);
+
+    expect(result.behavior).toBe("allow");
+    expect(addApproval).not.toHaveBeenCalled();
+  });
+
+  it("does not gate non-destructive PostHog sub-tools", async () => {
+    setMcpToolApprovalStates({ mcp__posthog__exec: "approved" });
+    const addApproval = vi.fn();
+
+    const context = createContext("mcp__posthog__exec", {
+      toolInput: { command: "call experiment-get-all {}" },
+      session: {
+        permissionMode: "default",
+        settingsManager: {
+          getRepoRoot: vi.fn().mockReturnValue("/repo"),
+          hasPostHogExecApproval: vi.fn().mockReturnValue(false),
+          addPostHogExecApproval: addApproval,
+        },
+      },
+    });
+    const result = await canUseTool(context);
+
+    // Non-destructive sub-tool falls through the gate. With approved MCP state
+    // and non-read-only tool metadata, it hits the default permission flow,
+    // which auto-allows via our mocked requestPermission. The gate must not
+    // have prompted with a PostHog-specific title, and must not have persisted.
+    expect(result.behavior).toBe("allow");
+    expect(addApproval).not.toHaveBeenCalled();
+  });
+
   it("emits tool denial notification for do_not_use", async () => {
     setMcpToolApprovalStates({
       mcp__server__denied_tool: "do_not_use",