From 675d27dfc49972282bd6e90bcb6d1385d2e637b5 Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Mon, 20 Jul 2026 13:25:40 +0200 Subject: [PATCH 1/5] feat(agent): configure PostHog exec permissions --- packages/agent/README.md | 16 +- .../agent/src/adapters/claude/claude-agent.ts | 6 + .../agent/src/adapters/claude/hooks.test.ts | 26 ++- packages/agent/src/adapters/claude/hooks.ts | 29 +++- .../permissions/permission-handlers.test.ts | 163 ++++++------------ .../claude/permissions/permission-handlers.ts | 65 +++---- .../permissions/posthog-exec-gate.test.ts | 84 --------- .../claude/permissions/posthog-exec-gate.ts | 30 ---- .../src/adapters/claude/session/options.ts | 5 +- .../adapters/claude/session/settings.test.ts | 50 ------ .../src/adapters/claude/session/settings.ts | 48 ------ packages/agent/src/adapters/claude/types.ts | 2 + .../codex-app-server-agent.test.ts | 7 +- .../codex-app-server-agent.ts | 9 +- .../codex-app-server/mcp-config.test.ts | 37 ++++ .../adapters/codex-app-server/mcp-config.ts | 28 ++- .../agent/src/posthog-exec-permission.test.ts | 84 +++++++++ packages/agent/src/posthog-exec-permission.ts | 44 +++++ .../agent/src/server/agent-server.test.ts | 140 +++++++++++++++ packages/agent/src/server/agent-server.ts | 77 ++++++++- packages/agent/src/server/bin.ts | 30 ++++ packages/agent/src/server/schemas.test.ts | 17 ++ packages/agent/src/server/schemas.ts | 16 ++ packages/agent/src/server/types.ts | 5 + 24 files changed, 616 insertions(+), 402 deletions(-) delete mode 100644 packages/agent/src/adapters/claude/permissions/posthog-exec-gate.test.ts delete mode 100644 packages/agent/src/adapters/claude/permissions/posthog-exec-gate.ts create mode 100644 packages/agent/src/posthog-exec-permission.test.ts create mode 100644 packages/agent/src/posthog-exec-permission.ts diff --git a/packages/agent/README.md b/packages/agent/README.md index ca1ead4470..c86db3da08 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -74,6 +74,14 @@ Four modes defined in `src/execution-mode.ts`: In cloud background mode, permissions are always auto-approved. In interactive mode, the permission system is active and configurable per session. Tool categorization lives in `src/adapters/claude/tools.ts` — each tool belongs to a group (read, write, bash, search, web, agent) and modes whitelist groups. +Cloud provisioning can pass `--posthogExecPermissionRegex ` to require +one-time client approval for matching PostHog MCP `exec` sub-tools in every +interactive Claude and Codex permission mode. Matching is case-insensitive +against the delegated name in `call [--json] ...`. These prompts do +not offer an always-allow choice and are not remembered. Background runs keep +their existing auto-approval behavior. The default is +`(^|-)(partial-update|update|patch|delete|destroy)(-|$)`. + ## ACP connection layer `createAcpConnection()` in `src/adapters/acp-connection.ts` is the heart of the package. It's a factory that returns a `{ clientStreams, cleanup }` object — a pair of ndJson `ReadableStream`/`WritableStream` that the caller uses to speak ACP. @@ -142,9 +150,12 @@ JWT validation (`src/server/jwt.ts`) uses RS256 with a configurable public key. When `POST /command` receives a `user_message`, it doesn't handle it directly — it calls `clientConnection.prompt()` on the ACP `ClientSideConnection`, which sends a `session/prompt` message through the ACP streams to the agent. Similarly, `cancel` sends `session/cancel`. This means all commands follow the same path as in-process calls from PostHog Code, with the HTTP layer just being a thin translation. -### Auto-approval in cloud mode +### Permission routing in cloud mode -The `AgentServer` provides a `requestPermission` callback to the `ClientSideConnection` that always selects the "allow" option. In background mode this is necessary (no human to ask). In interactive mode it currently does the same, with a TODO for future per-tool approval via SSE round-trips. +The `AgentServer` provides the `requestPermission` callback to the +`ClientSideConnection`. Background mode selects an allow option automatically. +Interactive mode relays approvals that need a person over SSE and parks them +until a client responds; other requests follow the selected permission mode. ### Checkpoint capture @@ -157,6 +168,7 @@ npx agent-server \ --port 3001 \ --mode interactive \ --repositoryPath /path/to/repo \ + --posthogExecPermissionRegex '(^|-)(partial-update|update|patch|delete|destroy)(-|$)' \ --taskId task_123 \ --runId run_456 ``` diff --git a/packages/agent/src/adapters/claude/claude-agent.ts b/packages/agent/src/adapters/claude/claude-agent.ts index 402d8dbdd4..d97d26f5b3 100644 --- a/packages/agent/src/adapters/claude/claude-agent.ts +++ b/packages/agent/src/adapters/claude/claude-agent.ts @@ -57,6 +57,7 @@ import { type Enrichment, type FileEnrichmentDeps, } from "../../enrichment/file-enricher"; +import { compilePostHogExecPermissionRegex } from "../../posthog-exec-permission"; import { classifyPostHogExecCall, isUnclassifiedPostHogSubTool, @@ -1950,12 +1951,16 @@ export class ClaudeAcpAgent extends BaseAcpAgent { CODE_EXECUTION_MODES.includes(meta.permissionMode as CodeExecutionMode) ? (meta.permissionMode as CodeExecutionMode) : "default"; + const posthogExecPermissionRegex = meta?.posthogExecPermissionRegex + ? compilePostHogExecPermissionRegex(meta.posthogExecPermissionRegex) + : undefined; const taskState: TaskState = new Map(); const options = buildSessionOptions({ cwd, mcpServers, permissionMode, + posthogExecPermissionRegex, canUseTool: this.createCanUseTool(sessionId, meta?.allowedDomains), logger: this.logger, systemPrompt, @@ -2010,6 +2015,7 @@ export class ClaudeAcpAgent extends BaseAcpAgent { cancelled: false, settingsManager, permissionMode, + posthogExecPermissionRegex, abortController, accumulatedUsage: { inputTokens: 0, diff --git a/packages/agent/src/adapters/claude/hooks.test.ts b/packages/agent/src/adapters/claude/hooks.test.ts index f12669dffc..da1c21a4fc 100644 --- a/packages/agent/src/adapters/claude/hooks.test.ts +++ b/packages/agent/src/adapters/claude/hooks.test.ts @@ -351,6 +351,8 @@ function buildSettingsManagerStub( describe("createPreToolUseHook", () => { const logger = new Logger({ debug: false }); + const posthogExecPermissionRegex = + /(^|-)(partial-update|update|patch|delete|destroy)(-|$)/i; test("defers destructive PostHog exec sub-tool to canUseTool via ask", async () => { const settingsManager = buildSettingsManagerStub({ @@ -358,7 +360,11 @@ describe("createPreToolUseHook", () => { rule: "mcp__posthog__exec", source: "allow", }); - const hook = createPreToolUseHook(settingsManager, logger); + const hook = createPreToolUseHook( + settingsManager, + logger, + posthogExecPermissionRegex, + ); const result = await hook( buildPreToolUseHookInput("mcp__posthog__exec", { command: 'call dashboard-update {"id": 1, "name": "x"}', @@ -382,7 +388,11 @@ describe("createPreToolUseHook", () => { rule: "mcp__posthog__exec", source: "allow", }); - const hook = createPreToolUseHook(settingsManager, logger); + const hook = createPreToolUseHook( + settingsManager, + logger, + posthogExecPermissionRegex, + ); const result = await hook( buildPreToolUseHookInput("mcp__posthog__exec", { command: 'call experiment-get {"id": 1}', @@ -408,7 +418,11 @@ describe("createPreToolUseHook", () => { rule: "Bash(ls:*)", source: "allow", }); - const hook = createPreToolUseHook(settingsManager, logger); + const hook = createPreToolUseHook( + settingsManager, + logger, + posthogExecPermissionRegex, + ); const result = await hook( buildPreToolUseHookInput("Bash", { command: "ls -la" }), undefined, @@ -426,7 +440,11 @@ describe("createPreToolUseHook", () => { rule: "mcp__posthog__exec", source: "allow", }); - const hook = createPreToolUseHook(settingsManager, logger); + const hook = createPreToolUseHook( + settingsManager, + logger, + posthogExecPermissionRegex, + ); const result = await hook( buildPreToolUseHookInput("mcp__posthog__exec", { command: 'call cohorts-partial-update {"id": 1}', diff --git a/packages/agent/src/adapters/claude/hooks.ts b/packages/agent/src/adapters/claude/hooks.ts index 467df4bcfb..e5745e6a87 100644 --- a/packages/agent/src/adapters/claude/hooks.ts +++ b/packages/agent/src/adapters/claude/hooks.ts @@ -3,17 +3,17 @@ import { enrichFileForAgent, type FileEnrichmentDeps, } from "../../enrichment/file-enricher"; +import { + extractPostHogSubTool, + isPostHogExecTool, + matchesPostHogExecPermission, +} from "../../posthog-exec-permission"; import type { Logger } from "../../utils/logger"; import { SIGNED_COMMIT_QUALIFIED_TOOL_NAME } from "../signed-commit-shared"; import { stripCatLineNumbers } from "./conversion/sdk-to-acp"; import type { TaskState } from "./conversion/task-state"; import { gitSubcommand } from "./git-command"; import { neutralizeUnprocessableImages } from "./image-sanitization"; -import { - extractPostHogSubTool, - isPostHogDestructiveSubTool, - isPostHogExecTool, -} from "./permissions/posthog-exec-gate"; import type { SettingsManager } from "./session/settings"; import type { CodeExecutionMode } from "./tools"; @@ -381,7 +381,11 @@ export const createSignedCommitGuardHook = }; export const createPreToolUseHook = - (settingsManager: SettingsManager, logger: Logger): HookCallback => + ( + settingsManager: SettingsManager, + logger: Logger, + posthogExecPermissionRegex?: RegExp, + ): HookCallback => async (input: HookInput, _toolUseID: string | undefined) => { if (input.hook_event_name !== "PreToolUse") { return { continue: true }; @@ -405,15 +409,22 @@ export const createPreToolUseHook = // 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)) { + if ( + posthogExecPermissionRegex && + permissionCheck.decision === "allow" && + isPostHogExecTool(toolName) + ) { const subTool = extractPostHogSubTool(toolInput); - if (subTool && isPostHogDestructiveSubTool(subTool)) { + if ( + subTool && + matchesPostHogExecPermission(subTool, posthogExecPermissionRegex) + ) { return { continue: true, hookSpecificOutput: { hookEventName: "PreToolUse" as const, permissionDecision: "ask" as const, - permissionDecisionReason: `Destructive PostHog sub-tool '${subTool}' requires explicit approval`, + permissionDecisionReason: `PostHog sub-tool '${subTool}' matches the configured permission regex`, }, }; } diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts index 60a200126c..adbbde72a1 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts @@ -1,10 +1,18 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + compilePostHogExecPermissionRegex, + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, +} from "../../../posthog-exec-permission"; import { clearMcpToolMetadataCache, setMcpToolApprovalStates, } from "../mcp/tool-metadata"; import { canUseTool } from "./permission-handlers"; +const posthogExecPermissionRegex = compilePostHogExecPermissionRegex( + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, +); + function createClient(response: Record) { return { sessionUpdate: vi.fn().mockResolvedValue(undefined), @@ -200,63 +208,55 @@ 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, + it.each([ + "default", + "acceptEdits", + "plan", + "auto", + "bypassPermissions", + ] as const)( + "prompts for a configured PostHog exec match in %s mode without remembering", + async (permissionMode) => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call notebooks-destroy {}" }, + session: { + permissionMode, + posthogExecPermissionRegex, + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + }, }, - }, - }); - const result = await canUseTool(context); + }); + const result = await canUseTool(context); - expect(result.behavior).toBe("allow"); - expect(context.client.requestPermission).not.toHaveBeenCalled(); - expect(hasApproval).not.toHaveBeenCalled(); - expect(addApproval).not.toHaveBeenCalled(); - }); + expect(result.behavior).toBe("allow"); + expect(context.client.requestPermission).toHaveBeenCalledWith( + expect.objectContaining({ + options: [ + expect.objectContaining({ kind: "allow_once" }), + expect.objectContaining({ kind: "reject_once" }), + ], + toolCall: expect.objectContaining({ + title: "The agent wants to run `notebooks-destroy` on PostHog", + _meta: { claudeCode: { toolName: "mcp__posthog__exec" } }, + }), + }), + ); + }, + ); - it("bypasses the PostHog exec gate in bypassPermissions mode", async () => { + it("does not gate a nonmatching PostHog sub-tool", async () => { setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); const context = createContext("mcp__posthog__exec", { - toolInput: { command: "call feature-flag-delete {}" }, + toolInput: { command: "call experiment-get-all {}" }, session: { permissionMode: "bypassPermissions", + posthogExecPermissionRegex, 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(), }, }, }); @@ -266,85 +266,22 @@ describe("canUseTool MCP approval enforcement", () => { expect(context.client.requestPermission).not.toHaveBeenCalled(); }); - it("prompts for an unapproved destructive PostHog sub-tool and persists on allow_always", async () => { + it("does not gate matching sub-tools when the regex is not configured", 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: createClient({ - 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", - _meta: { claudeCode: { toolName: "mcp__posthog__exec" } }, - }), - }), - ); - 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: createClient({ - 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", + permissionMode: "bypassPermissions", 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(); + expect(context.client.requestPermission).not.toHaveBeenCalled(); }); it("emits tool denial notification for do_not_use", async () => { diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index 979f62140e..22c4ed7025 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -7,6 +7,11 @@ import type { PermissionRuleValue, PermissionUpdate, } from "@anthropic-ai/claude-agent-sdk"; +import { + extractPostHogSubTool, + isPostHogExecTool, + matchesPostHogExecPermission, +} from "../../../posthog-exec-permission"; import { text } from "../../../utils/acp-content"; import type { Logger } from "../../../utils/logger"; import { qualifiedLocalToolName } from "../../local-tools"; @@ -34,11 +39,6 @@ import { buildExitPlanModePermissionOptions, buildPermissionOptions, } from "./permission-options"; -import { - extractPostHogSubTool, - isPostHogDestructiveSubTool, - isPostHogExecTool, -} from "./posthog-exec-gate"; const SPEAK_TOOL_ID = qualifiedLocalToolName(SPEAK_TOOL_NAME); @@ -586,16 +586,11 @@ async function handlePostHogExecApprovalFlow( context: ToolHandlerContext, subTool: string, ): Promise { - const { toolName, toolInput, toolUseID, sessionId, session } = context; + const { toolName, toolInput, toolUseID, sessionId } = context; const response = await requestPermissionFromClient(context, { options: [ { kind: "allow_once", name: "Yes", optionId: "allow" }, - { - kind: "allow_always", - name: "Yes, always allow", - optionId: "allow_always", - }, { kind: "reject_once", name: "Type here to tell the agent what to do differently", @@ -627,19 +622,8 @@ async function handlePostHogExecApprovalFlow( if ( response.outcome?.outcome === "selected" && - (response.outcome.optionId === "allow" || - response.outcome.optionId === "allow_always") + response.outcome.optionId === "allow" ) { - if (response.outcome.optionId === "allow_always") { - try { - await session.settingsManager.addPostHogExecApproval(subTool); - } catch (error) { - context.logger.warn( - "[canUseTool] Failed to persist PostHog exec approval", - { error: error instanceof Error ? error.message : String(error) }, - ); - } - } return { behavior: "allow", updatedInput: toolInput as Record, @@ -761,6 +745,19 @@ export async function canUseTool( return { behavior: "deny", message, interrupt: false }; } + if (session.posthogExecPermissionRegex && isPostHogExecTool(toolName)) { + const subTool = extractPostHogSubTool(toolInput); + if ( + subTool && + matchesPostHogExecPermission( + subTool, + session.posthogExecPermissionRegex, + ) + ) { + return handlePostHogExecApprovalFlow(context, subTool); + } + } + // Narration is a fire-and-forget no-op on the agent side; a permission // prompt for it interrupts the user to approve a line they may never hear. // An explicit do_not_use block above still wins. @@ -774,28 +771,6 @@ export async function canUseTool( if (approvalState === "needs_approval") { return handleMcpApprovalFlow(context); } - - if (isPostHogExecTool(toolName)) { - const subTool = extractPostHogSubTool(toolInput); - if (subTool && isPostHogDestructiveSubTool(subTool)) { - if ( - session.permissionMode === "auto" || - session.permissionMode === "bypassPermissions" - ) { - return { - behavior: "allow", - updatedInput: toolInput as Record, - }; - } - if (session.settingsManager.hasPostHogExecApproval(subTool)) { - return { - behavior: "allow", - updatedInput: toolInput as Record, - }; - } - return handlePostHogExecApprovalFlow(context, subTool); - } - } } if (isToolAllowedForMode(toolName, session.permissionMode)) { diff --git a/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.test.ts b/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.test.ts deleted file mode 100644 index 6c2e82a930..0000000000 --- a/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.test.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { - extractPostHogSubTool, - isPostHogDestructiveSubTool, - isPostHogExecTool, -} from "./posthog-exec-gate"; - -describe("isPostHogExecTool", () => { - it("matches the bare posthog exec tool", () => { - expect(isPostHogExecTool("mcp__posthog__exec")).toBe(true); - }); - - it("matches plugin-prefixed variants", () => { - expect(isPostHogExecTool("mcp__posthog_posthog__exec")).toBe(true); - expect(isPostHogExecTool("mcp__posthog_cloud__exec")).toBe(true); - }); - - it("rejects other MCP tools", () => { - expect(isPostHogExecTool("mcp__posthog__list")).toBe(false); - expect(isPostHogExecTool("mcp__other__exec")).toBe(false); - expect(isPostHogExecTool("mcp__acp__Bash")).toBe(false); - expect(isPostHogExecTool("Bash")).toBe(false); - }); -}); - -describe("extractPostHogSubTool", () => { - it("parses a bare `call ` command", () => { - expect(extractPostHogSubTool({ command: "call experiment-update" })).toBe( - "experiment-update", - ); - }); - - it("parses `call --json `", () => { - expect( - extractPostHogSubTool({ - command: 'call --json experiment-update {"id":1}', - }), - ).toBe("experiment-update"); - }); - - it("tolerates leading whitespace", () => { - expect(extractPostHogSubTool({ command: " call foo-delete" })).toBe( - "foo-delete", - ); - }); - - it("returns null for non-`call` verbs", () => { - expect(extractPostHogSubTool({ command: "tools" })).toBeNull(); - expect(extractPostHogSubTool({ command: "search experiments" })).toBeNull(); - expect(extractPostHogSubTool({ command: "info flag-get" })).toBeNull(); - }); - - it("returns null for missing or malformed input", () => { - expect(extractPostHogSubTool(undefined)).toBeNull(); - expect(extractPostHogSubTool(null)).toBeNull(); - expect(extractPostHogSubTool({})).toBeNull(); - expect(extractPostHogSubTool({ command: 42 })).toBeNull(); - expect(extractPostHogSubTool({ command: "" })).toBeNull(); - }); -}); - -describe("isPostHogDestructiveSubTool", () => { - it("matches update/delete/destroy/partial-update as whole segments", () => { - expect(isPostHogDestructiveSubTool("experiment-update")).toBe(true); - expect(isPostHogDestructiveSubTool("feature-flag-delete")).toBe(true); - expect(isPostHogDestructiveSubTool("notebooks-destroy")).toBe(true); - expect(isPostHogDestructiveSubTool("experiment-partial-update")).toBe(true); - expect(isPostHogDestructiveSubTool("update-something")).toBe(true); - expect(isPostHogDestructiveSubTool("delete")).toBe(true); - }); - - it("does not match read verbs or unrelated tokens", () => { - expect(isPostHogDestructiveSubTool("experiment-get")).toBe(false); - expect(isPostHogDestructiveSubTool("feature-flag-list")).toBe(false); - expect(isPostHogDestructiveSubTool("experiment-create")).toBe(false); - expect(isPostHogDestructiveSubTool("insights-pause")).toBe(false); - }); - - it("does not match substrings inside other words", () => { - // "updated" should not count — must be a whole segment - expect(isPostHogDestructiveSubTool("get-updated-events")).toBe(false); - expect(isPostHogDestructiveSubTool("deleter-test")).toBe(false); - }); -}); diff --git a/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.ts b/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.ts deleted file mode 100644 index 2ecdb8c6ec..0000000000 --- a/packages/agent/src/adapters/claude/permissions/posthog-exec-gate.ts +++ /dev/null @@ -1,30 +0,0 @@ -/** - * The PostHog MCP exposes a single `exec` dispatcher tool that runs - * subcommands like `call [--json] [json]`. Once the user approves - * `mcp__posthog__exec` once, every subsequent call goes through silently — - * including destructive ones. These helpers let `canUseTool` re-gate the - * destructive subset (update/delete family) at sub-tool granularity. - */ - -const POSTHOG_EXEC_TOOL_RE = /^mcp__posthog(?:_[^_]+)*__exec$/; - -const POSTHOG_CALL_COMMAND_RE = /^\s*call\s+(?:--json\s+)?([a-zA-Z0-9_-]+)/; - -const POSTHOG_DESTRUCTIVE_SUBTOOL_RE = - /(^|-)(partial-update|update|delete|destroy)(-|$)/i; - -export function isPostHogExecTool(toolName: string): boolean { - return POSTHOG_EXEC_TOOL_RE.test(toolName); -} - -export function extractPostHogSubTool(toolInput: unknown): string | null { - if (!toolInput || typeof toolInput !== "object") return null; - const command = (toolInput as { command?: unknown }).command; - if (typeof command !== "string") return null; - const match = command.match(POSTHOG_CALL_COMMAND_RE); - return match ? (match[1] ?? null) : null; -} - -export function isPostHogDestructiveSubTool(subTool: string): boolean { - return POSTHOG_DESTRUCTIVE_SUBTOOL_RE.test(subTool); -} diff --git a/packages/agent/src/adapters/claude/session/options.ts b/packages/agent/src/adapters/claude/session/options.ts index 220d96b90f..5b8808e807 100644 --- a/packages/agent/src/adapters/claude/session/options.ts +++ b/packages/agent/src/adapters/claude/session/options.ts @@ -67,6 +67,7 @@ export interface BuildOptionsParams { cwd: string; mcpServers: Record; permissionMode: CodeExecutionMode; + posthogExecPermissionRegex?: RegExp; canUseTool: CanUseTool; logger: Logger; systemPrompt?: Options["systemPrompt"]; @@ -218,6 +219,7 @@ function buildHooks( | ((subTool: string, commandText?: string) => void) | undefined, settingsManager: SettingsManager, + posthogExecPermissionRegex: RegExp | undefined, logger: Logger, enrichmentDeps: FileEnrichmentDeps | undefined, enrichedReadCache: EnrichedReadCache | undefined, @@ -242,7 +244,7 @@ function buildHooks( } const preToolUseHooks = [ - createPreToolUseHook(settingsManager, logger), + createPreToolUseHook(settingsManager, logger, posthogExecPermissionRegex), createSubagentRewriteHook(logger, registeredAgents), ]; if (cloudMode) { @@ -471,6 +473,7 @@ export function buildSessionOptions(params: BuildOptionsParams): Options { params.onModeChange, params.onPostHogResourceUsed, params.settingsManager, + params.posthogExecPermissionRegex, params.logger, params.enrichmentDeps, params.enrichedReadCache, diff --git a/packages/agent/src/adapters/claude/session/settings.test.ts b/packages/agent/src/adapters/claude/session/settings.test.ts index 5f6a91f425..30c62e383c 100644 --- a/packages/agent/src/adapters/claude/session/settings.test.ts +++ b/packages/agent/src/adapters/claude/session/settings.test.ts @@ -127,56 +127,6 @@ describe("SettingsManager per-repo persistence", () => { expect(await fs.promises.readFile(filePath, "utf-8")).toBe(original); }); - it("persists PostHog exec approvals and sees them across worktrees", async () => { - const writer = new SettingsManager(worktree); - await writer.initialize(); - await writer.addPostHogExecApproval("experiment-update"); - - const filePath = path.join(mainRepo, ".claude", "settings.local.json"); - const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); - expect(contents.posthogApprovedExecTools).toEqual(["experiment-update"]); - - const sibling = path.join(tmpRoot, "wt-ph"); - runGit(mainRepo, ["worktree", "add", "-b", "other-ph", sibling]); - const reader = new SettingsManager(sibling); - await reader.initialize(); - expect(reader.hasPostHogExecApproval("experiment-update")).toBe(true); - expect(reader.hasPostHogExecApproval("experiment-delete")).toBe(false); - }); - - it("dedupes repeated PostHog exec approvals", async () => { - const manager = new SettingsManager(worktree); - await manager.initialize(); - - await manager.addPostHogExecApproval("foo-update"); - await manager.addPostHogExecApproval("foo-update"); - await manager.addPostHogExecApproval("bar-delete"); - - const filePath = path.join(mainRepo, ".claude", "settings.local.json"); - const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); - expect(contents.posthogApprovedExecTools).toEqual([ - "foo-update", - "bar-delete", - ]); - }); - - it("concurrent addPostHogExecApproval calls do not clobber each other", async () => { - const manager = new SettingsManager(worktree); - await manager.initialize(); - - await Promise.all([ - manager.addPostHogExecApproval("a-update"), - manager.addPostHogExecApproval("b-delete"), - manager.addPostHogExecApproval("c-destroy"), - ]); - - const filePath = path.join(mainRepo, ".claude", "settings.local.json"); - const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); - expect(contents.posthogApprovedExecTools).toEqual( - expect.arrayContaining(["a-update", "b-delete", "c-destroy"]), - ); - }); - it("concurrent addAllowRules calls do not clobber each other", async () => { const manager = new SettingsManager(worktree); await manager.initialize(); diff --git a/packages/agent/src/adapters/claude/session/settings.ts b/packages/agent/src/adapters/claude/session/settings.ts index b4aa9ab6ed..6e8070f624 100644 --- a/packages/agent/src/adapters/claude/session/settings.ts +++ b/packages/agent/src/adapters/claude/session/settings.ts @@ -197,7 +197,6 @@ export interface ClaudeCodeSettings { env?: Record; model?: string; availableModels?: string[]; - posthogApprovedExecTools?: string[]; } type SettingsLayer = "user" | "project" | "local" | "enterprise"; @@ -318,7 +317,6 @@ export class SettingsManager { ask: [], }; const merged: ClaudeCodeSettings = { permissions }; - const posthogApprovedExecTools = new Set(); for (const { layer, settings } of allSettings) { if (settings.permissions) { @@ -352,15 +350,6 @@ export class SettingsManager { settings.availableModels, layer, ); - if (settings.posthogApprovedExecTools) { - for (const tool of settings.posthogApprovedExecTools) { - posthogApprovedExecTools.add(tool); - } - } - } - - if (posthogApprovedExecTools.size > 0) { - merged.posthogApprovedExecTools = Array.from(posthogApprovedExecTools); } this.mergedSettings = merged; @@ -443,43 +432,6 @@ export class SettingsManager { } } - hasPostHogExecApproval(subTool: string): boolean { - return ( - this.mergedSettings.posthogApprovedExecTools?.includes(subTool) ?? false - ); - } - - /** - * Persists an approved PostHog MCP `exec` sub-tool (e.g. `experiment-update`) - * to the local settings file so future calls skip the prompt. Mirrors - * `addAllowRules` — serialised via `writeMutex`, atomic temp-file + rename. - */ - async addPostHogExecApproval(subTool: string): Promise { - if (!subTool) return; - if (!this.initialized) await this.initialize(); - await this.writeMutex.acquire(); - try { - const filePath = this.getLocalSettingsPath(); - const existing = await readSettingsFileForUpdate(filePath); - const current = new Set(existing.posthogApprovedExecTools ?? []); - if (current.has(subTool)) { - return; - } - current.add(subTool); - const next: ClaudeCodeSettings = { - ...existing, - posthogApprovedExecTools: Array.from(current), - }; - await fs.promises.mkdir(path.dirname(filePath), { recursive: true }); - await writeFileAtomic(filePath, `${JSON.stringify(next, null, 2)}\n`); - - this.localSettings = next; - this.mergeAllSettings(); - } finally { - this.writeMutex.release(); - } - } - async setCwd(cwd: string): Promise { if (this.cwd === cwd) return; if (this.initPromise) await this.initPromise; diff --git a/packages/agent/src/adapters/claude/types.ts b/packages/agent/src/adapters/claude/types.ts index 80078dfa8f..cceafbf13b 100644 --- a/packages/agent/src/adapters/claude/types.ts +++ b/packages/agent/src/adapters/claude/types.ts @@ -67,6 +67,7 @@ export type Session = BaseSession & { input: Pushable; settingsManager: SettingsManager; permissionMode: CodeExecutionMode; + posthogExecPermissionRegex?: RegExp; modeBeforePlan?: CodeExecutionMode; modelId?: string; cwd: string; @@ -202,6 +203,7 @@ export type NewSessionMeta = { spokenNarration?: boolean; jsonSchema?: Record | null; mcpToolApprovals?: McpToolApprovals; + posthogExecPermissionRegex?: string; claudeCode?: { options?: Options; emitRawSDKMessages?: boolean | SDKMessageFilter[]; diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts index 23355aee71..2b1752255c 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts @@ -1039,7 +1039,11 @@ describe("CodexAppServerAgent", () => { await agent.newSession({ cwd: "/r", - _meta: { systemPrompt: "You are a repo selector." }, + _meta: { + systemPrompt: "You are a repo selector.", + permissionMode: "bypassPermissions", + posthogExecPermissionRegex: "delete|destroy", + }, mcpServers: [ { name: "posthog", @@ -1059,6 +1063,7 @@ describe("CodexAppServerAgent", () => { command: "node", args: ["server.js"], env: { TOKEN: "abc" }, + tools: { exec: { approval_mode: "prompt" } }, }, }, }, diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts index 8b1ce61b89..36fac88e98 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts @@ -94,6 +94,7 @@ type AppServerSessionMeta = { channelMode?: boolean; spokenNarration?: boolean; baseBranch?: string; + posthogExecPermissionRegex?: string; nativeGoal?: NativeGoalState; }; @@ -492,10 +493,10 @@ export class CodexAppServerAgent extends BaseAcpAgent { { error: String(err) }, ); } - const mcpServers = toCodexMcpServers([ - ...(params.mcpServers ?? []), - ...(localTools ? [localTools] : []), - ]); + const mcpServers = toCodexMcpServers( + [...(params.mcpServers ?? []), ...(localTools ? [localTools] : [])], + params.meta?.posthogExecPermissionRegex, + ); const config = buildThreadConfig(mcpServers, params.additionalDirectories); const result = await this.rpc.request<{ thread?: AppServerThread }>( diff --git a/packages/agent/src/adapters/codex-app-server/mcp-config.test.ts b/packages/agent/src/adapters/codex-app-server/mcp-config.test.ts index 912d253b38..98e1943da2 100644 --- a/packages/agent/src/adapters/codex-app-server/mcp-config.test.ts +++ b/packages/agent/src/adapters/codex-app-server/mcp-config.test.ts @@ -57,4 +57,41 @@ describe("toCodexMcpServers", () => { }, }); }); + + it("prompts for PostHog exec when the permission regex is configured", () => { + const servers = [ + { + type: "http", + name: "posthog_cloud", + url: "https://mcp.example/mcp", + }, + { + type: "http", + name: "other", + url: "https://other.example/mcp", + }, + ] as unknown as McpServer[]; + + expect(toCodexMcpServers(servers, "delete|destroy")).toEqual({ + posthog_cloud: { + url: "https://mcp.example/mcp", + tools: { exec: { approval_mode: "prompt" } }, + }, + other: { url: "https://other.example/mcp" }, + }); + }); + + it("leaves PostHog exec unchanged when no permission regex is configured", () => { + const servers = [ + { + type: "http", + name: "posthog", + url: "https://mcp.example/mcp", + }, + ] as unknown as McpServer[]; + + expect(toCodexMcpServers(servers)).toEqual({ + posthog: { url: "https://mcp.example/mcp" }, + }); + }); }); diff --git a/packages/agent/src/adapters/codex-app-server/mcp-config.ts b/packages/agent/src/adapters/codex-app-server/mcp-config.ts index 4e4873e516..7e35b35c86 100644 --- a/packages/agent/src/adapters/codex-app-server/mcp-config.ts +++ b/packages/agent/src/adapters/codex-app-server/mcp-config.ts @@ -1,12 +1,28 @@ import type { McpServer } from "@agentclientprotocol/sdk"; +import { isPostHogExecDescriptor } from "../../posthog-exec-permission"; + +interface CodexMcpServerToolConfig { + approval_mode: "prompt"; +} + +interface CodexMcpServerPolicyConfig { + tools?: Record; +} /** * Codex's per-thread `mcp_servers` config entry (stdio: command/args/env; http: * url + headers), accepted under `thread/start`'s `config.mcp_servers`. */ export type CodexMcpServerConfig = - | { command: string; args: string[]; env?: Record } - | { url: string; http_headers?: Record }; + | (CodexMcpServerPolicyConfig & { + command: string; + args: string[]; + env?: Record; + }) + | (CodexMcpServerPolicyConfig & { + url: string; + http_headers?: Record; + }); /** * Translates the ACP `McpServer[]` into the shape Codex's app-server expects under @@ -15,6 +31,7 @@ export type CodexMcpServerConfig = */ export function toCodexMcpServers( servers: McpServer[] | undefined, + posthogExecPermissionRegex?: string, ): Record | undefined { if (!servers || servers.length === 0) { return undefined; @@ -22,18 +39,25 @@ export function toCodexMcpServers( const out: Record = {}; for (const server of servers) { + const policy = + posthogExecPermissionRegex && + isPostHogExecDescriptor({ server: server.name, tool: "exec" }) + ? { tools: { exec: { approval_mode: "prompt" as const } } } + : {}; if ("command" in server && server.command) { const env = pairsToRecord(server.env); out[server.name] = { command: server.command, args: server.args ?? [], ...(env ? { env } : {}), + ...policy, }; } else if ("url" in server && server.url) { const headers = pairsToRecord(server.headers); out[server.name] = { url: server.url, ...(headers ? { http_headers: headers } : {}), + ...policy, }; } } diff --git a/packages/agent/src/posthog-exec-permission.test.ts b/packages/agent/src/posthog-exec-permission.test.ts new file mode 100644 index 0000000000..ee08343e4d --- /dev/null +++ b/packages/agent/src/posthog-exec-permission.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from "vitest"; +import { + compilePostHogExecPermissionRegex, + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + extractPostHogSubTool, + isPostHogExecDescriptor, + isPostHogExecTool, + matchesPostHogExecPermission, +} from "./posthog-exec-permission"; + +const permissionRegex = compilePostHogExecPermissionRegex( + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, +); + +describe("PostHog exec identification", () => { + it("matches bare and plugin-prefixed PostHog exec tools", () => { + expect(isPostHogExecTool("mcp__posthog__exec")).toBe(true); + expect(isPostHogExecTool("mcp__posthog_posthog__exec")).toBe(true); + expect(isPostHogExecTool("mcp__posthog_cloud__exec")).toBe(true); + expect(isPostHogExecDescriptor({ server: "posthog", tool: "exec" })).toBe( + true, + ); + }); + + it("rejects other servers and tools", () => { + expect(isPostHogExecTool("mcp__posthog__list")).toBe(false); + expect(isPostHogExecTool("mcp__other__exec")).toBe(false); + expect(isPostHogExecDescriptor({ server: "other", tool: "exec" })).toBe( + false, + ); + }); +}); + +describe("extractPostHogSubTool", () => { + it.each([ + ["call experiment-update", "experiment-update"], + ['call --json experiment-update {"id":1}', "experiment-update"], + [" call foo-delete", "foo-delete"], + ])("extracts the sub-tool from %s", (command, expected) => { + expect(extractPostHogSubTool({ command })).toBe(expected); + }); + + it.each([ + { command: "tools" }, + { command: "search experiments" }, + { command: "info flag-get" }, + undefined, + null, + {}, + { command: 42 }, + { command: "" }, + ])("returns null for non-call or malformed input", (input) => { + expect(extractPostHogSubTool(input)).toBeNull(); + }); +}); + +describe("configured permission regex", () => { + it.each([ + "experiment-update", + "feature-flag-delete", + "notebooks-destroy", + "experiment-partial-update", + "feature-flag-patch", + "UPDATE-something", + "delete", + ])("matches %s case-insensitively", (subTool) => { + expect(matchesPostHogExecPermission(subTool, permissionRegex)).toBe(true); + }); + + it.each([ + "experiment-get", + "feature-flag-list", + "experiment-create", + "insights-pause", + "get-updated-events", + "deleter-test", + ])("does not match %s", (subTool) => { + expect(matchesPostHogExecPermission(subTool, permissionRegex)).toBe(false); + }); + + it("rejects invalid regex source", () => { + expect(() => compilePostHogExecPermissionRegex("[")).toThrow(); + }); +}); diff --git a/packages/agent/src/posthog-exec-permission.ts b/packages/agent/src/posthog-exec-permission.ts new file mode 100644 index 0000000000..6dbb0eb560 --- /dev/null +++ b/packages/agent/src/posthog-exec-permission.ts @@ -0,0 +1,44 @@ +/** + * The PostHog MCP exposes a single `exec` dispatcher tool that runs + * subcommands like `call [--json] [json]`. These helpers identify + * that dispatcher, extract the delegated tool, and apply the externally + * configured permission regex at sub-tool granularity. + */ + +const POSTHOG_EXEC_TOOL_RE = /^mcp__posthog(?:_[^_]+)*__exec$/; + +const POSTHOG_CALL_COMMAND_RE = /^\s*call\s+(?:--json\s+)?([a-zA-Z0-9_-]+)/; + +export const DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE = + "(^|-)(partial-update|update|patch|delete|destroy)(-|$)"; + +export function compilePostHogExecPermissionRegex(source: string): RegExp { + return new RegExp(source, "i"); +} + +export function isPostHogExecTool(toolName: string): boolean { + return POSTHOG_EXEC_TOOL_RE.test(toolName); +} + +export function isPostHogExecDescriptor(descriptor: { + server: string; + tool: string; +}): boolean { + return isPostHogExecTool(`mcp__${descriptor.server}__${descriptor.tool}`); +} + +export function extractPostHogSubTool(toolInput: unknown): string | null { + if (!toolInput || typeof toolInput !== "object") return null; + const command = (toolInput as { command?: unknown }).command; + if (typeof command !== "string") return null; + const match = command.match(POSTHOG_CALL_COMMAND_RE); + return match ? (match[1] ?? null) : null; +} + +export function matchesPostHogExecPermission( + subTool: string, + permissionRegex: RegExp, +): boolean { + permissionRegex.lastIndex = 0; + return permissionRegex.test(subTool); +} diff --git a/packages/agent/src/server/agent-server.test.ts b/packages/agent/src/server/agent-server.test.ts index fd0d106dec..ac048f911c 100644 --- a/packages/agent/src/server/agent-server.test.ts +++ b/packages/agent/src/server/agent-server.test.ts @@ -1297,6 +1297,8 @@ describe("AgentServer HTTP Mode", () => { session: { hasDesktopConnected?: boolean } | null; eventStreamSender: unknown; relayPermissionToClient: (params: unknown) => Promise; + pendingPermissions: Map; + resolvePermission: (requestId: string, optionId: string) => boolean; createCloudClient(payload: { run_id: string; task_id: string; @@ -1343,6 +1345,41 @@ describe("AgentServer HTTP Mode", () => { }; } + function posthogExecPermissionOptions() { + return [ + { optionId: "allow_once", kind: "allow_once" }, + { optionId: "allow_always", kind: "allow_always" }, + { optionId: "reject_once", kind: "reject_once" }, + ]; + } + + function claudePosthogExecPermissionRequest(command: string) { + return { + options: posthogExecPermissionOptions(), + toolCall: { + kind: "other", + _meta: { claudeCode: { toolName: "mcp__posthog__exec" } }, + rawInput: { command }, + }, + }; + } + + function codexPosthogExecPermissionRequest(command: string) { + return { + options: posthogExecPermissionOptions(), + toolCall: { + kind: "other", + _meta: { + posthog: { + toolName: "mcp__posthog_cloud__exec", + mcp: { server: "posthog_cloud", tool: "exec" }, + }, + }, + rawInput: { command }, + }, + }; + } + const basePayload = { run_id: "run-1", task_id: "task-1", @@ -1478,6 +1515,109 @@ describe("AgentServer HTTP Mode", () => { expect(relaySpy).not.toHaveBeenCalled(); expect(result.outcome).toEqual({ outcome: "cancelled" }); }); + + it.each([ + { + adapter: "Claude", + request: claudePosthogExecPermissionRequest( + "call notebooks-destroy {}", + ), + }, + { + adapter: "Codex", + request: codexPosthogExecPermissionRequest("call notebooks-destroy {}"), + }, + ])( + "relays a configured PostHog exec match from $adapter without an always-allow option", + async ({ request }) => { + const testServer = exposeCloudClient(createServer()); + testServer.session = null; + testServer.eventStreamSender = null; + const relaySpy = vi + .spyOn(testServer, "relayPermissionToClient") + .mockResolvedValue({ + outcome: { outcome: "selected", optionId: "allow_once" }, + }); + + const { requestPermission } = testServer.createCloudClient(basePayload); + const result = await requestPermission(request); + + expect(relaySpy).toHaveBeenCalledWith( + expect.objectContaining({ + options: [ + expect.objectContaining({ kind: "allow_once" }), + expect.objectContaining({ kind: "reject_once" }), + ], + }), + ); + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "allow_once", + }); + }, + ); + + it("auto-approves a nonmatching PostHog exec sub-tool", async () => { + const testServer = exposeCloudClient( + createServer({ posthogExecPermissionRegex: "delete|destroy" }), + ); + const relaySpy = vi.spyOn(testServer, "relayPermissionToClient"); + + const { requestPermission } = testServer.createCloudClient(basePayload); + const result = await requestPermission( + claudePosthogExecPermissionRequest("call experiment-get {}"), + ); + + expect(relaySpy).not.toHaveBeenCalled(); + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "allow_once", + }); + }); + + it("keeps background PostHog exec matches auto-approved", async () => { + const testServer = exposeCloudClient( + createServer({ posthogExecPermissionRegex: "delete|destroy" }), + ); + const relaySpy = vi.spyOn(testServer, "relayPermissionToClient"); + + const { requestPermission } = testServer.createCloudClient({ + ...basePayload, + mode: "background", + }); + const result = await requestPermission( + codexPosthogExecPermissionRequest("call experiment-delete {}"), + ); + + expect(relaySpy).not.toHaveBeenCalled(); + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "allow_once", + }); + }); + + it("rejects permission responses for options that were not offered", async () => { + const testServer = exposeCloudClient(createServer()); + const pending = testServer.relayPermissionToClient({ + options: [ + { optionId: "allow_once", kind: "allow_once" }, + { optionId: "reject_once", kind: "reject_once" }, + ], + }); + const requestId = [...testServer.pendingPermissions.keys()][0]; + + expect(requestId).toBeDefined(); + expect( + testServer.resolvePermission(requestId as string, "allow_always"), + ).toBe(false); + expect(testServer.pendingPermissions.has(requestId as string)).toBe(true); + expect( + testServer.resolvePermission(requestId as string, "allow_once"), + ).toBe(true); + await expect(pending).resolves.toEqual({ + outcome: { outcome: "selected", optionId: "allow_once" }, + }); + }); }); describe("refresh_session relay re-append", () => { diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index 140863f0da..80bafd3957 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -21,6 +21,7 @@ import { buildPrOutput, getErrorMessage, mergePrUrls, + parseMcpToolName, readMcpToolDescriptor, readPrUrls, } from "@posthog/shared"; @@ -56,6 +57,13 @@ import { DEFAULT_CODEX_MODEL, fetchGatewayModels } from "../gateway-models"; import { HandoffCheckpointTracker } from "../handoff-checkpoint"; import { configurePersistentAgentState } from "../persistent-agent-state"; import { PostHogAPIClient } from "../posthog-api"; +import { + compilePostHogExecPermissionRegex, + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + extractPostHogSubTool, + isPostHogExecDescriptor, + matchesPostHogExecPermission, +} from "../posthog-exec-permission"; import { findPrUrls, wasCreatedByLogin, @@ -385,8 +393,11 @@ export class AgentServer { _meta?: Record; }) => void; toolCallId?: string; + optionIds: Set; } >(); + private readonly posthogExecPermissionRegex: RegExp; + private readonly posthogExecPermissionRegexSource: string; private mcpRelayServer: McpRelayServer | null = null; /** @@ -448,6 +459,12 @@ export class AgentServer { constructor(config: AgentServerConfig) { this.config = config; + this.posthogExecPermissionRegexSource = + config.posthogExecPermissionRegex ?? + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE; + this.posthogExecPermissionRegex = compilePostHogExecPermissionRegex( + this.posthogExecPermissionRegexSource, + ); this.logger = new Logger({ debug: true, prefix: "[AgentServer]" }); this.posthogAPI = new PostHogAPIClient({ apiUrl: config.apiUrl, @@ -1498,6 +1515,7 @@ export class AgentServer { allowedDomains: this.config.allowedDomains, jsonSchema: preTask?.json_schema ?? null, permissionMode: initialPermissionMode, + posthogExecPermissionRegex: this.posthogExecPermissionRegexSource, ...(this.config.baseBranch && { baseBranch: this.config.baseBranch }), ...this.buildClaudeCodeSessionMeta(runtimeAdapter), }; @@ -3618,6 +3636,33 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} ); } + private readPermissionMcpDescriptor( + params: RequestPermissionRequest, + ): { server: string; tool: string } | undefined { + const descriptor = readMcpToolDescriptor(params.toolCall?._meta); + if (descriptor) return descriptor; + + const rawInput = params.toolCall?.rawInput as + | { toolName?: unknown } + | undefined; + return typeof rawInput?.toolName === "string" + ? parseMcpToolName(rawInput.toolName) + : undefined; + } + + private matchesPostHogExecPermissionRequest( + params: RequestPermissionRequest, + ): string | null { + const descriptor = this.readPermissionMcpDescriptor(params); + if (!descriptor || !isPostHogExecDescriptor(descriptor)) return null; + + const subTool = extractPostHogSubTool(params.toolCall?.rawInput); + return subTool && + matchesPostHogExecPermission(subTool, this.posthogExecPermissionRegex) + ? subTool + : null; + } + private createCloudClient(payload: JwtPayload) { const mode = this.getEffectiveMode(payload); const interactionOrigin = @@ -3666,15 +3711,8 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} // falling back to Claude's `rawInput.toolName`. Keying off only the // Claude channel would silently skip this gate for codex and let a // relayed tool auto-run in non-asking modes. - const rawInput = params.toolCall?.rawInput as - | { toolName?: string } - | undefined; const mcpServerName = - readMcpToolDescriptor(params.toolCall?._meta)?.server ?? - (typeof rawInput?.toolName === "string" && - rawInput.toolName.startsWith("mcp__") - ? rawInput.toolName.split("__")[1] - : undefined); + this.readPermissionMcpDescriptor(params)?.server; if ( mcpServerName && (this.config.relayMcpServers ?? []).includes(mcpServerName) @@ -3694,6 +3732,22 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} } } + const posthogExecSubTool = + this.matchesPostHogExecPermissionRequest(params); + if (mode !== "background" && posthogExecSubTool) { + const promptOnceParams = { + ...params, + options: params.options.filter( + (option) => option.kind !== "allow_always", + ), + }; + this.logger.debug("Relaying configured PostHog exec permission", { + subTool: posthogExecSubTool, + sessionPermissionMode: this.getSessionPermissionMode(), + }); + return this.relayPermissionToClient(promptOnceParams); + } + // Relay permission requests to the connected client when: // - Plan approvals: always relay because they gate autonomy changes // that require human confirmation (buffered until desktop connects) @@ -4354,7 +4408,11 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} }); return new Promise((resolve) => { - this.pendingPermissions.set(requestId, { resolve, toolCallId }); + this.pendingPermissions.set(requestId, { + resolve, + toolCallId, + optionIds: new Set(params.options.map((option) => option.optionId)), + }); }); } @@ -4379,6 +4437,7 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} ): boolean { const pending = this.pendingPermissions.get(requestId); if (!pending) return false; + if (!pending.optionIds.has(optionId)) return false; this.pendingPermissions.delete(requestId); diff --git a/packages/agent/src/server/bin.ts b/packages/agent/src/server/bin.ts index 6f5a85c6f6..1bdfef445e 100644 --- a/packages/agent/src/server/bin.ts +++ b/packages/agent/src/server/bin.ts @@ -2,10 +2,12 @@ import { Command } from "commander"; import { z } from "zod/v4"; import { isSupportedReasoningEffort } from "../adapters/reasoning-effort"; +import { DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE } from "../posthog-exec-permission"; import { AgentServer } from "./agent-server"; import { claudeCodeConfigSchema, mcpServersSchema, + posthogExecPermissionRegexSchema, relayMcpServerNamesSchema, } from "./schemas"; @@ -90,6 +92,23 @@ function parseJsonOption( return result.data; } +function parseStringOption( + raw: string | undefined, + schema: z.ZodType, + flag: string, +): string | undefined { + if (raw === undefined) return undefined; + + const result = schema.safeParse(raw); + if (!result.success) { + const errors = result.error.issues + .map((issue) => ` - ${issue.message}`) + .join("\n"); + program.error(`${flag} validation failed:\n${errors}`); + } + return result.data; +} + program .name("agent-server") .description("PostHog cloud agent server - runs in sandbox environments") @@ -114,6 +133,11 @@ program "--relayMcpServers ", "Desktop-relayed MCP server names as JSON array (docs/cloud-mcp-relay.md)", ) + .option( + "--posthogExecPermissionRegex ", + "Case-insensitive regex for PostHog exec sub-tools that require client approval", + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + ) .option("--createPr ", "Whether this run may publish changes") .option( "--autoPublish ", @@ -158,6 +182,11 @@ program relayMcpServerNamesSchema, "--relayMcpServers", ); + const posthogExecPermissionRegex = parseStringOption( + options.posthogExecPermissionRegex, + posthogExecPermissionRegexSchema, + "--posthogExecPermissionRegex", + ); const claudeCode = parseJsonOption( options.claudeCodeConfig, claudeCodeConfigSchema, @@ -208,6 +237,7 @@ program autoPublish, mcpServers, relayMcpServers, + posthogExecPermissionRegex, baseBranch: options.baseBranch, claudeCode, allowedDomains, diff --git a/packages/agent/src/server/schemas.test.ts b/packages/agent/src/server/schemas.test.ts index ee45fceccd..a31eef90a1 100644 --- a/packages/agent/src/server/schemas.test.ts +++ b/packages/agent/src/server/schemas.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { mcpServersSchema, + posthogExecPermissionRegexSchema, relayMcpServerNamesSchema, validateCommandParams, } from "./schemas"; @@ -120,6 +121,22 @@ describe("mcpServersSchema", () => { }); }); +describe("posthogExecPermissionRegexSchema", () => { + it("accepts a valid permission regex", () => { + expect( + posthogExecPermissionRegexSchema.safeParse( + "(^|-)(update|patch|delete|destroy)(-|$)", + ).success, + ).toBe(true); + }); + + it.each(["", "["])("rejects %j", (source) => { + expect(posthogExecPermissionRegexSchema.safeParse(source).success).toBe( + false, + ); + }); +}); + describe("validateCommandParams", () => { it("accepts structured user_message content arrays", () => { const result = validateCommandParams("user_message", { diff --git a/packages/agent/src/server/schemas.ts b/packages/agent/src/server/schemas.ts index fd3f2789fe..2290c56fdc 100644 --- a/packages/agent/src/server/schemas.ts +++ b/packages/agent/src/server/schemas.ts @@ -1,4 +1,5 @@ import { z } from "zod/v4"; +import { compilePostHogExecPermissionRegex } from "../posthog-exec-permission"; const httpHeaderSchema = z.object({ name: z.string(), @@ -27,6 +28,21 @@ const remoteMcpServerSchema = z.object({ export const mcpServersSchema = z.array(remoteMcpServerSchema); +export const posthogExecPermissionRegexSchema = z + .string() + .min(1, "PostHog exec permission regex cannot be empty") + .refine( + (source) => { + try { + compilePostHogExecPermissionRegex(source); + return true; + } catch { + return false; + } + }, + { error: "PostHog exec permission regex must be valid" }, + ); + export type RemoteMcpServer = z.infer; export const claudeCodeConfigSchema = z.object({ diff --git a/packages/agent/src/server/types.ts b/packages/agent/src/server/types.ts index 460a455fed..34b94a45e1 100644 --- a/packages/agent/src/server/types.ts +++ b/packages/agent/src/server/types.ts @@ -33,6 +33,11 @@ export interface AgentServerConfig { autoPublish?: boolean; version?: string; mcpServers?: RemoteMcpServer[]; + /** + * Case-insensitive JavaScript regex matched against PostHog `exec` sub-tool + * names. Overrides the default approval regex for interactive calls. + */ + posthogExecPermissionRegex?: string; /** * Names of desktop-only local MCP servers to expose through loopback relay * endpoints (docs/cloud-mcp-relay.md). Names only; the desktop resolves From c2ca91a2b2a4c548ea49136e1deff62b0dd99525 Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Mon, 20 Jul 2026 13:41:52 +0200 Subject: [PATCH 2/5] fix(agent): restore remembered Claude exec approvals --- packages/agent/README.md | 5 +- .../permissions/permission-handlers.test.ts | 53 ++++++++++++++++++- .../claude/permissions/permission-handlers.ts | 26 ++++++++- .../adapters/claude/session/settings.test.ts | 50 +++++++++++++++++ .../src/adapters/claude/session/settings.ts | 48 +++++++++++++++++ .../agent/src/server/agent-server.test.ts | 18 +++---- packages/agent/src/server/agent-server.ts | 15 ++++-- 7 files changed, 196 insertions(+), 19 deletions(-) diff --git a/packages/agent/README.md b/packages/agent/README.md index c86db3da08..6aeed14107 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -78,8 +78,9 @@ Cloud provisioning can pass `--posthogExecPermissionRegex ` to require one-time client approval for matching PostHog MCP `exec` sub-tools in every interactive Claude and Codex permission mode. Matching is case-insensitive against the delegated name in `call [--json] ...`. These prompts do -not offer an always-allow choice and are not remembered. Background runs keep -their existing auto-approval behavior. The default is +offer Claude users an always-allow choice remembered in local repository +settings; Codex approvals remain one-time. Background runs keep their existing +auto-approval behavior. The default is `(^|-)(partial-update|update|patch|delete|destroy)(-|$)`. ## ACP connection layer diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts index adbbde72a1..a2f94f99a8 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts @@ -215,7 +215,7 @@ describe("canUseTool MCP approval enforcement", () => { "auto", "bypassPermissions", ] as const)( - "prompts for a configured PostHog exec match in %s mode without remembering", + "prompts for a configured PostHog exec match in %s mode with a remembered choice", async (permissionMode) => { setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); @@ -226,6 +226,8 @@ describe("canUseTool MCP approval enforcement", () => { posthogExecPermissionRegex, settingsManager: { getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi.fn().mockReturnValue(false), + addPostHogExecApproval: vi.fn(), }, }, }); @@ -236,6 +238,7 @@ describe("canUseTool MCP approval enforcement", () => { expect.objectContaining({ options: [ expect.objectContaining({ kind: "allow_once" }), + expect.objectContaining({ kind: "allow_always" }), expect.objectContaining({ kind: "reject_once" }), ], toolCall: expect.objectContaining({ @@ -247,6 +250,54 @@ describe("canUseTool MCP approval enforcement", () => { }, ); + it("skips the prompt for a remembered PostHog exec sub-tool", async () => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call experiment-update {}" }, + session: { + permissionMode: "default", + posthogExecPermissionRegex, + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi.fn().mockReturnValue(true), + addPostHogExecApproval: vi.fn(), + }, + }, + }); + + const result = await canUseTool(context); + + expect(result.behavior).toBe("allow"); + expect(context.client.requestPermission).not.toHaveBeenCalled(); + }); + + it("persists a PostHog exec sub-tool selected with allow always", async () => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + const addPostHogExecApproval = vi.fn().mockResolvedValue(undefined); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call notebooks-destroy {}" }, + session: { + permissionMode: "default", + posthogExecPermissionRegex, + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi.fn().mockReturnValue(false), + addPostHogExecApproval, + }, + }, + client: createClient({ + outcome: { outcome: "selected", optionId: "allow_always" }, + }), + }); + + const result = await canUseTool(context); + + expect(result.behavior).toBe("allow"); + expect(addPostHogExecApproval).toHaveBeenCalledWith("notebooks-destroy"); + }); + it("does not gate a nonmatching PostHog sub-tool", async () => { setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index 22c4ed7025..0979a76a40 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -586,11 +586,16 @@ async function handlePostHogExecApprovalFlow( context: ToolHandlerContext, subTool: string, ): Promise { - const { toolName, toolInput, toolUseID, sessionId } = context; + const { toolName, toolInput, toolUseID, sessionId, session } = context; const response = await requestPermissionFromClient(context, { options: [ { kind: "allow_once", name: "Yes", optionId: "allow" }, + { + kind: "allow_always", + name: "Yes, always allow", + optionId: "allow_always", + }, { kind: "reject_once", name: "Type here to tell the agent what to do differently", @@ -622,8 +627,19 @@ async function handlePostHogExecApprovalFlow( if ( response.outcome?.outcome === "selected" && - response.outcome.optionId === "allow" + (response.outcome.optionId === "allow" || + response.outcome.optionId === "allow_always") ) { + if (response.outcome.optionId === "allow_always") { + try { + await session.settingsManager.addPostHogExecApproval(subTool); + } catch (error) { + context.logger.warn( + "[canUseTool] Failed to persist PostHog exec approval", + { error: error instanceof Error ? error.message : String(error) }, + ); + } + } return { behavior: "allow", updatedInput: toolInput as Record, @@ -754,6 +770,12 @@ export async function canUseTool( session.posthogExecPermissionRegex, ) ) { + if (session.settingsManager.hasPostHogExecApproval(subTool)) { + return { + behavior: "allow", + updatedInput: toolInput as Record, + }; + } return handlePostHogExecApprovalFlow(context, subTool); } } diff --git a/packages/agent/src/adapters/claude/session/settings.test.ts b/packages/agent/src/adapters/claude/session/settings.test.ts index 30c62e383c..5f6a91f425 100644 --- a/packages/agent/src/adapters/claude/session/settings.test.ts +++ b/packages/agent/src/adapters/claude/session/settings.test.ts @@ -127,6 +127,56 @@ describe("SettingsManager per-repo persistence", () => { expect(await fs.promises.readFile(filePath, "utf-8")).toBe(original); }); + it("persists PostHog exec approvals and sees them across worktrees", async () => { + const writer = new SettingsManager(worktree); + await writer.initialize(); + await writer.addPostHogExecApproval("experiment-update"); + + const filePath = path.join(mainRepo, ".claude", "settings.local.json"); + const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); + expect(contents.posthogApprovedExecTools).toEqual(["experiment-update"]); + + const sibling = path.join(tmpRoot, "wt-ph"); + runGit(mainRepo, ["worktree", "add", "-b", "other-ph", sibling]); + const reader = new SettingsManager(sibling); + await reader.initialize(); + expect(reader.hasPostHogExecApproval("experiment-update")).toBe(true); + expect(reader.hasPostHogExecApproval("experiment-delete")).toBe(false); + }); + + it("dedupes repeated PostHog exec approvals", async () => { + const manager = new SettingsManager(worktree); + await manager.initialize(); + + await manager.addPostHogExecApproval("foo-update"); + await manager.addPostHogExecApproval("foo-update"); + await manager.addPostHogExecApproval("bar-delete"); + + const filePath = path.join(mainRepo, ".claude", "settings.local.json"); + const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); + expect(contents.posthogApprovedExecTools).toEqual([ + "foo-update", + "bar-delete", + ]); + }); + + it("concurrent addPostHogExecApproval calls do not clobber each other", async () => { + const manager = new SettingsManager(worktree); + await manager.initialize(); + + await Promise.all([ + manager.addPostHogExecApproval("a-update"), + manager.addPostHogExecApproval("b-delete"), + manager.addPostHogExecApproval("c-destroy"), + ]); + + const filePath = path.join(mainRepo, ".claude", "settings.local.json"); + const contents = JSON.parse(await fs.promises.readFile(filePath, "utf-8")); + expect(contents.posthogApprovedExecTools).toEqual( + expect.arrayContaining(["a-update", "b-delete", "c-destroy"]), + ); + }); + it("concurrent addAllowRules calls do not clobber each other", async () => { const manager = new SettingsManager(worktree); await manager.initialize(); diff --git a/packages/agent/src/adapters/claude/session/settings.ts b/packages/agent/src/adapters/claude/session/settings.ts index 6e8070f624..b4aa9ab6ed 100644 --- a/packages/agent/src/adapters/claude/session/settings.ts +++ b/packages/agent/src/adapters/claude/session/settings.ts @@ -197,6 +197,7 @@ export interface ClaudeCodeSettings { env?: Record; model?: string; availableModels?: string[]; + posthogApprovedExecTools?: string[]; } type SettingsLayer = "user" | "project" | "local" | "enterprise"; @@ -317,6 +318,7 @@ export class SettingsManager { ask: [], }; const merged: ClaudeCodeSettings = { permissions }; + const posthogApprovedExecTools = new Set(); for (const { layer, settings } of allSettings) { if (settings.permissions) { @@ -350,6 +352,15 @@ export class SettingsManager { settings.availableModels, layer, ); + if (settings.posthogApprovedExecTools) { + for (const tool of settings.posthogApprovedExecTools) { + posthogApprovedExecTools.add(tool); + } + } + } + + if (posthogApprovedExecTools.size > 0) { + merged.posthogApprovedExecTools = Array.from(posthogApprovedExecTools); } this.mergedSettings = merged; @@ -432,6 +443,43 @@ export class SettingsManager { } } + hasPostHogExecApproval(subTool: string): boolean { + return ( + this.mergedSettings.posthogApprovedExecTools?.includes(subTool) ?? false + ); + } + + /** + * Persists an approved PostHog MCP `exec` sub-tool (e.g. `experiment-update`) + * to the local settings file so future calls skip the prompt. Mirrors + * `addAllowRules` — serialised via `writeMutex`, atomic temp-file + rename. + */ + async addPostHogExecApproval(subTool: string): Promise { + if (!subTool) return; + if (!this.initialized) await this.initialize(); + await this.writeMutex.acquire(); + try { + const filePath = this.getLocalSettingsPath(); + const existing = await readSettingsFileForUpdate(filePath); + const current = new Set(existing.posthogApprovedExecTools ?? []); + if (current.has(subTool)) { + return; + } + current.add(subTool); + const next: ClaudeCodeSettings = { + ...existing, + posthogApprovedExecTools: Array.from(current), + }; + await fs.promises.mkdir(path.dirname(filePath), { recursive: true }); + await writeFileAtomic(filePath, `${JSON.stringify(next, null, 2)}\n`); + + this.localSettings = next; + this.mergeAllSettings(); + } finally { + this.writeMutex.release(); + } + } + async setCwd(cwd: string): Promise { if (this.cwd === cwd) return; if (this.initPromise) await this.initPromise; diff --git a/packages/agent/src/server/agent-server.test.ts b/packages/agent/src/server/agent-server.test.ts index ac048f911c..4e53e83f86 100644 --- a/packages/agent/src/server/agent-server.test.ts +++ b/packages/agent/src/server/agent-server.test.ts @@ -1522,14 +1522,16 @@ describe("AgentServer HTTP Mode", () => { request: claudePosthogExecPermissionRequest( "call notebooks-destroy {}", ), + expectedKinds: ["allow_once", "allow_always", "reject_once"], }, { adapter: "Codex", request: codexPosthogExecPermissionRequest("call notebooks-destroy {}"), + expectedKinds: ["allow_once", "reject_once"], }, ])( - "relays a configured PostHog exec match from $adapter without an always-allow option", - async ({ request }) => { + "relays a configured PostHog exec match from $adapter with adapter-specific choices", + async ({ request, expectedKinds }) => { const testServer = exposeCloudClient(createServer()); testServer.session = null; testServer.eventStreamSender = null; @@ -1542,13 +1544,11 @@ describe("AgentServer HTTP Mode", () => { const { requestPermission } = testServer.createCloudClient(basePayload); const result = await requestPermission(request); - expect(relaySpy).toHaveBeenCalledWith( - expect.objectContaining({ - options: [ - expect.objectContaining({ kind: "allow_once" }), - expect.objectContaining({ kind: "reject_once" }), - ], - }), + const relayed = relaySpy.mock.calls[0]?.[0] as { + options: Array<{ kind: string }>; + }; + expect(relayed.options.map((option) => option.kind)).toEqual( + expectedKinds, ); expect(result.outcome).toEqual({ outcome: "selected", diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index 80bafd3957..cedc8d0762 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -3735,17 +3735,22 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} const posthogExecSubTool = this.matchesPostHogExecPermissionRequest(params); if (mode !== "background" && posthogExecSubTool) { - const promptOnceParams = { + const isClaudeCodeRequest = Boolean( + params.toolCall?._meta?.claudeCode, + ); + const relayParams = { ...params, - options: params.options.filter( - (option) => option.kind !== "allow_always", - ), + options: isClaudeCodeRequest + ? params.options + : params.options.filter( + (option) => option.kind !== "allow_always", + ), }; this.logger.debug("Relaying configured PostHog exec permission", { subTool: posthogExecSubTool, sessionPermissionMode: this.getSessionPermissionMode(), }); - return this.relayPermissionToClient(promptOnceParams); + return this.relayPermissionToClient(relayParams); } // Relay permission requests to the connected client when: From bdf0d87b3adf831bae4807ed4660e6f4e497c987 Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Mon, 20 Jul 2026 15:26:35 +0200 Subject: [PATCH 3/5] fix(agent): default Claude PostHog exec guard --- packages/agent/README.md | 11 +- .../claude/claude-agent.resume-model.test.ts | 132 +++++++++++++++++- .../agent/src/adapters/claude/claude-agent.ts | 13 +- .../permissions/permission-handlers.test.ts | 30 +++- .../claude/permissions/permission-handlers.ts | 14 ++ packages/agent/src/adapters/claude/types.ts | 2 + .../codex-app-server-agent.test.ts | 31 ++++ .../codex-app-server-agent.ts | 4 +- .../agent/src/server/agent-server.test.ts | 53 ++++--- 9 files changed, 258 insertions(+), 32 deletions(-) diff --git a/packages/agent/README.md b/packages/agent/README.md index 6aeed14107..030d91febb 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -76,11 +76,12 @@ In cloud background mode, permissions are always auto-approved. In interactive m Cloud provisioning can pass `--posthogExecPermissionRegex ` to require one-time client approval for matching PostHog MCP `exec` sub-tools in every -interactive Claude and Codex permission mode. Matching is case-insensitive -against the delegated name in `call [--json] ...`. These prompts do -offer Claude users an always-allow choice remembered in local repository -settings; Codex approvals remain one-time. Background runs keep their existing -auto-approval behavior. The default is +interactive cloud Claude and Codex permission mode. Local Claude `auto` and +`bypassPermissions` modes remain hands-off. Matching is case-insensitive against +the delegated name in `call [--json] ...`. These prompts offer Claude +users an always-allow choice remembered in local repository settings; Codex +approvals remain one-time. Background runs keep their existing auto-approval +behavior. The default is `(^|-)(partial-update|update|patch|delete|destroy)(-|$)`. ## ACP connection layer diff --git a/packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts b/packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts index 3ce2eebe0f..404471c9a1 100644 --- a/packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts +++ b/packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts @@ -1,7 +1,8 @@ -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import type { AgentSideConnection } from "@agentclientprotocol/sdk"; +import type { HookInput, Options } from "@anthropic-ai/claude-agent-sdk"; import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_GATEWAY_MODEL } from "../../gateway-models"; @@ -38,11 +39,13 @@ function makeQueryHandle(): SdkQueryHandle { } const createdQueries: SdkQueryHandle[] = []; +const createdQueryOptions: Options[] = []; vi.mock("@anthropic-ai/claude-agent-sdk", () => ({ - query: vi.fn(() => { + query: vi.fn(({ options }: { options: Options }) => { const handle = makeQueryHandle(); createdQueries.push(handle); + createdQueryOptions.push(options); return handle; }), getSessionMessages: vi.fn().mockResolvedValue([]), @@ -87,6 +90,14 @@ const cwd = mkdtempSync(path.join(os.tmpdir(), "claude-agent-test-cwd-")); const configDir = mkdtempSync( path.join(os.tmpdir(), "claude-agent-test-config-"), ); +const permissionCwd = mkdtempSync( + path.join(os.tmpdir(), "claude-agent-permission-test-cwd-"), +); +mkdirSync(path.join(permissionCwd, ".claude"), { recursive: true }); +writeFileSync( + path.join(permissionCwd, ".claude", "settings.json"), + JSON.stringify({ permissions: { allow: ["mcp__posthog__exec"] } }), +); const savedEnv = { ANTHROPIC_BASE_URL: process.env.ANTHROPIC_BASE_URL, CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, @@ -95,6 +106,7 @@ const savedEnv = { afterAll(() => { rmSync(cwd, { recursive: true, force: true }); rmSync(configDir, { recursive: true, force: true }); + rmSync(permissionCwd, { recursive: true, force: true }); process.env.ANTHROPIC_BASE_URL = savedEnv.ANTHROPIC_BASE_URL; process.env.CLAUDE_CONFIG_DIR = savedEnv.CLAUDE_CONFIG_DIR; if (savedEnv.ANTHROPIC_BASE_URL === undefined) { @@ -105,9 +117,10 @@ afterAll(() => { } }); -describe("ClaudeAcpAgent session model on resume", () => { +describe("ClaudeAcpAgent session creation", () => { beforeEach(() => { createdQueries.length = 0; + createdQueryOptions.length = 0; nextInitPromise = Promise.resolve({ result: "success", commands: [], @@ -119,6 +132,119 @@ describe("ClaudeAcpAgent session model on resume", () => { process.env.CLAUDE_CONFIG_DIR = configDir; }); + async function runPostHogExecPreToolUse( + options: Options, + subTool: string, + ): Promise { + const input = { + session_id: "permission-session", + transcript_path: "/tmp/transcript", + cwd: permissionCwd, + hook_event_name: "PreToolUse", + tool_name: "mcp__posthog__exec", + tool_use_id: "toolu_permission", + tool_input: { command: `call ${subTool} {}` }, + } as HookInput; + + for (const hook of (options.hooks?.PreToolUse ?? []).flatMap( + (entry) => entry.hooks ?? [], + )) { + const result = await hook(input, undefined, { + signal: new AbortController().signal, + }); + const decision = ( + result as { + hookSpecificOutput?: { permissionDecision?: string }; + } + ).hookSpecificOutput?.permissionDecision; + if (decision) return decision; + } + + return undefined; + } + + it.each(["new", "resume", "load"] as const)( + "uses the default PostHog exec permission regex for local %s sessions when metadata omits it", + async (sessionKind) => { + const agent = makeAgent(); + const sessionIds = { + new: "0197a000-0000-7000-8000-000000000101", + resume: "0197a000-0000-7000-8000-000000000102", + load: "0197a000-0000-7000-8000-000000000103", + }; + const params = { + sessionId: sessionIds[sessionKind], + cwd: permissionCwd, + mcpServers: [], + _meta: { taskRunId: `run-permission-${sessionKind}` }, + }; + + if (sessionKind === "new") { + await agent.newSession(params); + } else if (sessionKind === "resume") { + await agent.resumeSession(params); + } else { + await agent.loadSession(params); + } + + expect(createdQueryOptions).toHaveLength(1); + await expect( + runPostHogExecPreToolUse( + createdQueryOptions[0] as Options, + "dashboard-update", + ), + ).resolves.toBe("ask"); + }, + ); + + it("uses an explicit PostHog exec permission regex instead of the default", async () => { + const agent = makeAgent(); + + await agent.newSession({ + cwd: permissionCwd, + mcpServers: [], + _meta: { + taskRunId: "run-permission-custom", + posthogExecPermissionRegex: "(^|-)archive(-|$)", + }, + }); + + expect(createdQueryOptions).toHaveLength(1); + await expect( + runPostHogExecPreToolUse( + createdQueryOptions[0] as Options, + "dashboard-update", + ), + ).resolves.toBe("allow"); + await expect( + runPostHogExecPreToolUse( + createdQueryOptions[0] as Options, + "dashboard-archive", + ), + ).resolves.toBe("ask"); + }); + + it.each([ + { environment: "local", expectedCloudMode: false }, + { environment: "cloud", expectedCloudMode: true }, + ] as const)( + "records $environment sessions as cloudMode=$expectedCloudMode", + async ({ environment, expectedCloudMode }) => { + const agent = makeAgent(); + + await agent.newSession({ + cwd, + mcpServers: [], + _meta: { environment, taskRunId: `run-${environment}` }, + }); + + expect( + (agent as unknown as { session: { cloudMode: boolean } }).session + .cloudMode, + ).toBe(expectedCloudMode); + }, + ); + // The SDK does not carry the model across resume — without an explicit // setModel the resumed session silently runs the SDK default (opus). it.each([ diff --git a/packages/agent/src/adapters/claude/claude-agent.ts b/packages/agent/src/adapters/claude/claude-agent.ts index d97d26f5b3..8087574996 100644 --- a/packages/agent/src/adapters/claude/claude-agent.ts +++ b/packages/agent/src/adapters/claude/claude-agent.ts @@ -57,7 +57,10 @@ import { type Enrichment, type FileEnrichmentDeps, } from "../../enrichment/file-enricher"; -import { compilePostHogExecPermissionRegex } from "../../posthog-exec-permission"; +import { + compilePostHogExecPermissionRegex, + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, +} from "../../posthog-exec-permission"; import { classifyPostHogExecCall, isUnclassifiedPostHogSubTool, @@ -1951,9 +1954,10 @@ export class ClaudeAcpAgent extends BaseAcpAgent { CODE_EXECUTION_MODES.includes(meta.permissionMode as CodeExecutionMode) ? (meta.permissionMode as CodeExecutionMode) : "default"; - const posthogExecPermissionRegex = meta?.posthogExecPermissionRegex - ? compilePostHogExecPermissionRegex(meta.posthogExecPermissionRegex) - : undefined; + const posthogExecPermissionRegex = compilePostHogExecPermissionRegex( + meta?.posthogExecPermissionRegex ?? + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + ); const taskState: TaskState = new Map(); const options = buildSessionOptions({ @@ -2015,6 +2019,7 @@ export class ClaudeAcpAgent extends BaseAcpAgent { cancelled: false, settingsManager, permissionMode, + cloudMode: cloudRun, posthogExecPermissionRegex, abortController, accumulatedUsage: { diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts index a2f94f99a8..dc308f3ea0 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts @@ -27,6 +27,7 @@ function createContext( return { session: { permissionMode: "default" as const, + cloudMode: false, settingsManager: { getRepoRoot: vi.fn().mockReturnValue("/repo"), }, @@ -215,7 +216,7 @@ describe("canUseTool MCP approval enforcement", () => { "auto", "bypassPermissions", ] as const)( - "prompts for a configured PostHog exec match in %s mode with a remembered choice", + "prompts for a configured PostHog exec match in cloud %s mode with a remembered choice", async (permissionMode) => { setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); @@ -223,6 +224,7 @@ describe("canUseTool MCP approval enforcement", () => { toolInput: { command: "call notebooks-destroy {}" }, session: { permissionMode, + cloudMode: true, posthogExecPermissionRegex, settingsManager: { getRepoRoot: vi.fn().mockReturnValue("/repo"), @@ -250,6 +252,32 @@ describe("canUseTool MCP approval enforcement", () => { }, ); + it.each(["auto", "bypassPermissions"] as const)( + "keeps local %s mode hands-off for a configured PostHog exec match", + async (permissionMode) => { + setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call notebooks-destroy {}" }, + session: { + permissionMode, + cloudMode: false, + posthogExecPermissionRegex, + 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("skips the prompt for a remembered PostHog exec sub-tool", async () => { setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index 0979a76a40..0ba613a4b9 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -776,6 +776,20 @@ export async function canUseTool( updatedInput: toolInput as Record, }; } + // Local hands-off modes retain their normal no-prompt behavior. Cloud + // sessions must send the request to AgentServer, which uses the run's + // effective mode to relay interactive approvals and auto-approve + // background runs. + if ( + !session.cloudMode && + (session.permissionMode === "auto" || + session.permissionMode === "bypassPermissions") + ) { + return { + behavior: "allow", + updatedInput: toolInput as Record, + }; + } return handlePostHogExecApprovalFlow(context, subTool); } } diff --git a/packages/agent/src/adapters/claude/types.ts b/packages/agent/src/adapters/claude/types.ts index cceafbf13b..573c5cde37 100644 --- a/packages/agent/src/adapters/claude/types.ts +++ b/packages/agent/src/adapters/claude/types.ts @@ -67,6 +67,8 @@ export type Session = BaseSession & { input: Pushable; settingsManager: SettingsManager; permissionMode: CodeExecutionMode; + /** Whether permission decisions are delegated to the cloud AgentServer. */ + cloudMode: boolean; posthogExecPermissionRegex?: RegExp; modeBeforePlan?: CodeExecutionMode; modelId?: string; diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts index 2b1752255c..08c60e5ad2 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts @@ -1070,6 +1070,37 @@ describe("CodexAppServerAgent", () => { }); }); + it("uses the default PostHog exec prompt policy when session metadata omits the regex", async () => { + const stub = makeStubRpc({ "thread/start": { thread: { id: "t" } } }); + const { client } = makeFakeClient(); + const agent = new CodexAppServerAgent(client, { + processOptions: { binaryPath: "/x/codex" }, + rpcFactory: stub.factory, + }); + + await agent.newSession({ + cwd: "/r", + mcpServers: [ + { + name: "posthog", + command: "node", + args: ["server.js"], + }, + ], + } as unknown as NewSessionRequest); + + const threadStart = stub.requests.find((r) => r.method === "thread/start"); + expect(threadStart?.params).toMatchObject({ + config: { + mcp_servers: { + posthog: { + tools: { exec: { approval_mode: "prompt" } }, + }, + }, + }, + }); + }); + it("flattens the host's {append} systemPrompt and dedupes it against developerInstructions", async () => { const stub = makeStubRpc({ "thread/start": { thread: { id: "t" } } }); const { client } = makeFakeClient(); diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts index 36fac88e98..14df545f46 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts @@ -30,6 +30,7 @@ import { POSTHOG_NOTIFICATIONS, } from "../../acp-extensions"; import { DEFAULT_CODEX_MODEL } from "../../gateway-models"; +import { DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE } from "../../posthog-exec-permission"; import type { ProcessSpawnedCallback } from "../../types"; import { ALLOW_BYPASS } from "../../utils/common"; import { Logger } from "../../utils/logger"; @@ -495,7 +496,8 @@ export class CodexAppServerAgent extends BaseAcpAgent { } const mcpServers = toCodexMcpServers( [...(params.mcpServers ?? []), ...(localTools ? [localTools] : [])], - params.meta?.posthogExecPermissionRegex, + params.meta?.posthogExecPermissionRegex ?? + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, ); const config = buildThreadConfig(mcpServers, params.additionalDirectories); diff --git a/packages/agent/src/server/agent-server.test.ts b/packages/agent/src/server/agent-server.test.ts index 4e53e83f86..ccc14c6b7f 100644 --- a/packages/agent/src/server/agent-server.test.ts +++ b/packages/agent/src/server/agent-server.test.ts @@ -1575,26 +1575,43 @@ describe("AgentServer HTTP Mode", () => { }); }); - it("keeps background PostHog exec matches auto-approved", async () => { - const testServer = exposeCloudClient( - createServer({ posthogExecPermissionRegex: "delete|destroy" }), - ); - const relaySpy = vi.spyOn(testServer, "relayPermissionToClient"); + it.each([ + { + modeSource: "JWT payload", + configMode: "interactive", + payloadMode: "background", + }, + { + modeSource: "server config", + configMode: "background", + payloadMode: undefined, + }, + ] as const)( + "keeps PostHog exec matches auto-approved in background mode from $modeSource", + async ({ configMode, payloadMode }) => { + const testServer = exposeCloudClient( + createServer({ + mode: configMode, + posthogExecPermissionRegex: "delete|destroy", + }), + ); + const relaySpy = vi.spyOn(testServer, "relayPermissionToClient"); - const { requestPermission } = testServer.createCloudClient({ - ...basePayload, - mode: "background", - }); - const result = await requestPermission( - codexPosthogExecPermissionRequest("call experiment-delete {}"), - ); + const { requestPermission } = testServer.createCloudClient({ + ...basePayload, + ...(payloadMode ? { mode: payloadMode } : {}), + }); + const result = await requestPermission( + codexPosthogExecPermissionRequest("call experiment-delete {}"), + ); - expect(relaySpy).not.toHaveBeenCalled(); - expect(result.outcome).toEqual({ - outcome: "selected", - optionId: "allow_once", - }); - }); + expect(relaySpy).not.toHaveBeenCalled(); + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "allow_once", + }); + }, + ); it("rejects permission responses for options that were not offered", async () => { const testServer = exposeCloudClient(createServer()); From a47c38c46c39a507ead120c3f31657bacf508e0e Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Mon, 20 Jul 2026 18:19:10 +0200 Subject: [PATCH 4/5] fix(agent): harden PostHog exec permission gating Review fixes for the configurable exec-permission feature: restore the needs_approval check ahead of the Claude exec gate so an explicit ask-me setting can't be bypassed by remembered approvals or hands-off modes; validate _meta.posthogExecPermissionRegex through the shared Zod schema in both adapters, warning and falling back to the default instead of crashing or silently flipping gating; filter Codex exec sub-tools in the adapter's approval handlers so local sessions only prompt for matching destructive calls (with Claude-parity hands-off in auto/full-access); make toCodexMcpServers take an honest gatePosthogExec flag; return a discriminated result from resolvePermission with distinct errors for unknown requests vs unoffered options; exempt question responses (synthetic option_ ids) from the offered-option check, which was hanging cloud question relays; parse exec input via Zod; document the posthog server-name contract; unwrap README prose. Co-Authored-By: Claude Fable 5 --- packages/agent/README.md | 15 +- .../claude/claude-agent.resume-model.test.ts | 35 ++++ .../agent/src/adapters/claude/claude-agent.ts | 15 +- .../permissions/permission-handlers.test.ts | 49 +++++ .../claude/permissions/permission-handlers.ts | 31 +-- .../adapters/codex-app-server/approvals.ts | 14 ++ .../codex-app-server-agent.test.ts | 181 +++++++++++++++++- .../codex-app-server-agent.ts | 56 +++++- .../codex-app-server/mcp-config.test.ts | 6 +- .../adapters/codex-app-server/mcp-config.ts | 7 +- .../agent/src/posthog-exec-permission.test.ts | 36 +++- packages/agent/src/posthog-exec-permission.ts | 62 +++++- .../agent/src/server/agent-server.test.ts | 47 ++++- packages/agent/src/server/agent-server.ts | 29 ++- .../agent/src/server/question-relay.test.ts | 9 +- packages/agent/src/server/schemas.ts | 18 +- 16 files changed, 529 insertions(+), 81 deletions(-) diff --git a/packages/agent/README.md b/packages/agent/README.md index 030d91febb..8826add45f 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -74,15 +74,7 @@ Four modes defined in `src/execution-mode.ts`: In cloud background mode, permissions are always auto-approved. In interactive mode, the permission system is active and configurable per session. Tool categorization lives in `src/adapters/claude/tools.ts` — each tool belongs to a group (read, write, bash, search, web, agent) and modes whitelist groups. -Cloud provisioning can pass `--posthogExecPermissionRegex ` to require -one-time client approval for matching PostHog MCP `exec` sub-tools in every -interactive cloud Claude and Codex permission mode. Local Claude `auto` and -`bypassPermissions` modes remain hands-off. Matching is case-insensitive against -the delegated name in `call [--json] ...`. These prompts offer Claude -users an always-allow choice remembered in local repository settings; Codex -approvals remain one-time. Background runs keep their existing auto-approval -behavior. The default is -`(^|-)(partial-update|update|patch|delete|destroy)(-|$)`. +Cloud provisioning can pass `--posthogExecPermissionRegex ` to require one-time client approval for matching PostHog MCP `exec` sub-tools in every interactive cloud Claude and Codex permission mode. Non-matching sub-tools never prompt. Locally, hands-off modes stay hands-off: Claude `auto` and `bypassPermissions`, and Codex `auto` and `full-access`, auto-approve matching sub-tools; other local modes prompt. Matching is case-insensitive against the delegated name in `call [--json] ...`. These prompts offer Claude users an always-allow choice remembered in local repository settings; Codex approvals remain one-time. An invalid or empty regex is logged and falls back to the default. Background runs keep their existing auto-approval behavior. The default is `(^|-)(partial-update|update|patch|delete|destroy)(-|$)`. ## ACP connection layer @@ -154,10 +146,7 @@ When `POST /command` receives a `user_message`, it doesn't handle it directly ### Permission routing in cloud mode -The `AgentServer` provides the `requestPermission` callback to the -`ClientSideConnection`. Background mode selects an allow option automatically. -Interactive mode relays approvals that need a person over SSE and parks them -until a client responds; other requests follow the selected permission mode. +The `AgentServer` provides the `requestPermission` callback to the `ClientSideConnection`. Background mode selects an allow option automatically. Interactive mode relays approvals that need a person over SSE and parks them until a client responds; other requests follow the selected permission mode. ### Checkpoint capture diff --git a/packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts b/packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts index 404471c9a1..04610b6f00 100644 --- a/packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts +++ b/packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts @@ -224,6 +224,41 @@ describe("ClaudeAcpAgent session creation", () => { ).resolves.toBe("ask"); }); + it.each(["[", ""])( + "warns and falls back to the default regex when metadata carries the invalid regex %j", + async (posthogExecPermissionRegex) => { + const agent = makeAgent(); + const warnSpy = vi.spyOn(agent.logger, "warn"); + + await agent.newSession({ + cwd: permissionCwd, + mcpServers: [], + _meta: { + taskRunId: "run-permission-invalid", + posthogExecPermissionRegex, + }, + }); + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid posthogExecPermissionRegex"), + expect.anything(), + ); + expect(createdQueryOptions).toHaveLength(1); + await expect( + runPostHogExecPreToolUse( + createdQueryOptions[0] as Options, + "dashboard-update", + ), + ).resolves.toBe("ask"); + await expect( + runPostHogExecPreToolUse( + createdQueryOptions[0] as Options, + "dashboard-get", + ), + ).resolves.toBe("allow"); + }, + ); + it.each([ { environment: "local", expectedCloudMode: false }, { environment: "cloud", expectedCloudMode: true }, diff --git a/packages/agent/src/adapters/claude/claude-agent.ts b/packages/agent/src/adapters/claude/claude-agent.ts index 8087574996..5b8e1845cc 100644 --- a/packages/agent/src/adapters/claude/claude-agent.ts +++ b/packages/agent/src/adapters/claude/claude-agent.ts @@ -57,10 +57,7 @@ import { type Enrichment, type FileEnrichmentDeps, } from "../../enrichment/file-enricher"; -import { - compilePostHogExecPermissionRegex, - DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, -} from "../../posthog-exec-permission"; +import { resolvePostHogExecPermissionRegex } from "../../posthog-exec-permission"; import { classifyPostHogExecCall, isUnclassifiedPostHogSubTool, @@ -1954,9 +1951,13 @@ export class ClaudeAcpAgent extends BaseAcpAgent { CODE_EXECUTION_MODES.includes(meta.permissionMode as CodeExecutionMode) ? (meta.permissionMode as CodeExecutionMode) : "default"; - const posthogExecPermissionRegex = compilePostHogExecPermissionRegex( - meta?.posthogExecPermissionRegex ?? - DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + const posthogExecPermissionRegex = resolvePostHogExecPermissionRegex( + meta?.posthogExecPermissionRegex, + (message) => + this.logger.warn( + "Invalid posthogExecPermissionRegex in session metadata; using default", + { message }, + ), ); const taskState: TaskState = new Map(); diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts index dc308f3ea0..f6b639f2b1 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts @@ -345,6 +345,55 @@ describe("canUseTool MCP approval enforcement", () => { expect(context.client.requestPermission).not.toHaveBeenCalled(); }); + // An explicit needs_approval MCP setting must win over every exec-gate + // shortcut: a remembered sub-tool approval and the local hands-off modes. + it.each([ + { + label: "a remembered sub-tool approval", + permissionMode: "default" as const, + hasApproval: true, + }, + { + label: "local auto mode", + permissionMode: "auto" as const, + hasApproval: false, + }, + { + label: "local bypassPermissions mode", + permissionMode: "bypassPermissions" as const, + hasApproval: false, + }, + ])( + "still prompts via the MCP approval flow for a needs_approval exec tool despite $label", + async ({ permissionMode, hasApproval }) => { + setMcpToolApprovalStates({ mcp__posthog__exec: "needs_approval" }); + + const context = createContext("mcp__posthog__exec", { + toolInput: { command: "call notebooks-destroy {}" }, + session: { + permissionMode, + cloudMode: false, + posthogExecPermissionRegex, + settingsManager: { + getRepoRoot: vi.fn().mockReturnValue("/repo"), + hasPostHogExecApproval: vi.fn().mockReturnValue(hasApproval), + addPostHogExecApproval: vi.fn(), + }, + }, + }); + 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 call exec (posthog)", + }), + }), + ); + }, + ); + it("does not gate matching sub-tools when the regex is not configured", async () => { setMcpToolApprovalStates({ mcp__posthog__exec: "approved" }); diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index 0ba613a4b9..1a7f623254 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -761,6 +761,23 @@ export async function canUseTool( return { behavior: "deny", message, interrupt: false }; } + // Narration is a fire-and-forget no-op on the agent side; a permission + // prompt for it interrupts the user to approve a line they may never hear. + // An explicit do_not_use block above still wins. + if (toolName === SPEAK_TOOL_ID) { + return { + behavior: "allow", + updatedInput: toolInput as Record, + }; + } + + // An explicit needs_approval setting always prompts — it must precede the + // PostHog exec gate so a remembered sub-tool approval or a local hands-off + // mode cannot silently allow a tool the user asked to be asked about. + if (approvalState === "needs_approval") { + return handleMcpApprovalFlow(context); + } + if (session.posthogExecPermissionRegex && isPostHogExecTool(toolName)) { const subTool = extractPostHogSubTool(toolInput); if ( @@ -793,20 +810,6 @@ export async function canUseTool( return handlePostHogExecApprovalFlow(context, subTool); } } - - // Narration is a fire-and-forget no-op on the agent side; a permission - // prompt for it interrupts the user to approve a line they may never hear. - // An explicit do_not_use block above still wins. - if (toolName === SPEAK_TOOL_ID) { - return { - behavior: "allow", - updatedInput: toolInput as Record, - }; - } - - if (approvalState === "needs_approval") { - return handleMcpApprovalFlow(context); - } } if (isToolAllowedForMode(toolName, session.permissionMode)) { diff --git a/packages/agent/src/adapters/codex-app-server/approvals.ts b/packages/agent/src/adapters/codex-app-server/approvals.ts index 950e978307..5f9a1dfc88 100644 --- a/packages/agent/src/adapters/codex-app-server/approvals.ts +++ b/packages/agent/src/adapters/codex-app-server/approvals.ts @@ -119,6 +119,17 @@ export interface HandleServerRequestOptions { resolveMcpToolCall?: ( serverName: string, ) => { server: string; tool: string; args: unknown } | undefined; + /** + * When an elicitation gates a known in-flight MCP call, accept it without + * prompting if this returns true (e.g. a PostHog exec sub-tool the session's + * permission policy does not gate). Elicitations with no resolvable call, or + * from other servers, always prompt. + */ + shouldAutoAcceptMcpToolCall?: (mcp: { + server: string; + tool: string; + args: unknown; + }) => boolean; } /** @@ -343,6 +354,9 @@ async function handleMcpElicitation( // If the elicitation gates a known in-flight MCP call, carry its real tool + // args + `_meta.posthog` so the host renders the proper MCP permission. const mcp = opts.resolveMcpToolCall?.(params.serverName); + if (mcp && opts.shouldAutoAcceptMcpToolCall?.(mcp)) { + return { action: "accept", content: {}, _meta: null }; + } const toolCall = mcp ? { toolCallId: `${params.serverName}:elicitation`, diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts index 08c60e5ad2..3fac908a6d 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts @@ -5,7 +5,7 @@ import type { PromptRequest, } from "@agentclientprotocol/sdk"; import { RequestError } from "@agentclientprotocol/sdk"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import type { AppServerClientHandlers, AppServerRpc, @@ -645,7 +645,12 @@ describe("CodexAppServerAgent", () => { rpcFactory: stub.factory, }); await agent.initialize(init); - await agent.newSession({ cwd: "/repo" } as unknown as NewSessionRequest); + // Cloud session with a gated sub-tool — the local hands-off and + // non-matching auto-accepts would otherwise skip the prompt entirely. + await agent.newSession({ + cwd: "/repo", + _meta: { environment: "cloud" }, + } as unknown as NewSessionRequest); // The MCP tool call item arrives first, then codex approves it via a command-execution request. stub.emit("item/started", { @@ -654,7 +659,7 @@ describe("CodexAppServerAgent", () => { id: "m1", server: "posthog", tool: "exec", - arguments: { command: "call execute-sql {}" }, + arguments: { command: "call dashboard-delete {}" }, }, }); const decision = await stub.invokeRequest( @@ -670,7 +675,7 @@ describe("CodexAppServerAgent", () => { expect(permissionToolCalls[0]).toMatchObject({ toolCallId: "m1", kind: "other", - rawInput: { command: "call execute-sql {}" }, + rawInput: { command: "call dashboard-delete {}" }, _meta: { posthog: { toolName: "mcp__posthog__exec", @@ -680,6 +685,104 @@ describe("CodexAppServerAgent", () => { }); }); + it("auto-accepts a PostHog exec approval for a sub-tool the permission regex does not gate", async () => { + const stub = makeStubRpc({ + initialize: {}, + "thread/start": { thread: { id: "thr_1" } }, + }); + const requestPermission = vi.fn(); + const client = { + sessionUpdate: async () => {}, + requestPermission, + extNotification: async () => {}, + } as unknown as AgentSideConnection; + const agent = new CodexAppServerAgent(client, { + processOptions: { binaryPath: "/bundle/codex" }, + model: "gpt-5.5", + rpcFactory: stub.factory, + }); + await agent.initialize(init); + await agent.newSession({ + cwd: "/repo", + _meta: { environment: "cloud" }, + } as unknown as NewSessionRequest); + + stub.emit("item/started", { + item: { + type: "mcpToolCall", + id: "m1", + server: "posthog", + tool: "exec", + arguments: { command: "call execute-sql {}" }, + }, + }); + const commandDecision = await stub.invokeRequest( + "item/commandExecution/requestApproval", + { + itemId: "m1", + command: 'Allow the posthog MCP server to run tool "exec"?', + }, + ); + const elicitationDecision = await stub.invokeRequest( + "mcpServer/elicitation/request", + { + threadId: "thr_1", + turnId: "turn_1", + serverName: "posthog", + mode: "form", + message: 'Allow the posthog MCP server to run tool "exec"?', + }, + ); + + expect(commandDecision).toEqual({ decision: "accept" }); + expect(elicitationDecision).toMatchObject({ action: "accept" }); + expect(requestPermission).not.toHaveBeenCalled(); + }); + + it("auto-accepts a gated PostHog exec sub-tool in local hands-off modes", async () => { + const stub = makeStubRpc({ + initialize: {}, + "thread/start": { thread: { id: "thr_1" } }, + }); + const requestPermission = vi.fn(); + const client = { + sessionUpdate: async () => {}, + requestPermission, + extNotification: async () => {}, + } as unknown as AgentSideConnection; + const agent = new CodexAppServerAgent(client, { + processOptions: { binaryPath: "/bundle/codex" }, + model: "gpt-5.5", + rpcFactory: stub.factory, + }); + await agent.initialize(init); + // Local session in codex's default hands-off "auto" mode. + await agent.newSession({ + cwd: "/repo", + _meta: { environment: "local" }, + } as unknown as NewSessionRequest); + + stub.emit("item/started", { + item: { + type: "mcpToolCall", + id: "m1", + server: "posthog", + tool: "exec", + arguments: { command: "call dashboard-delete {}" }, + }, + }); + const decision = await stub.invokeRequest( + "item/commandExecution/requestApproval", + { + itemId: "m1", + command: 'Allow the posthog MCP server to run tool "exec"?', + }, + ); + + expect(decision).toEqual({ decision: "accept" }); + expect(requestPermission).not.toHaveBeenCalled(); + }); + it("enriches the MCP elicitation approval (posthog exec) from the in-flight tool call", async () => { // codex gates PostHog `exec` behind a generic elicitation (serverName only, no tool/args); // the adapter correlates it to the in-flight mcpToolCall so the real tool + command render. @@ -704,7 +807,10 @@ describe("CodexAppServerAgent", () => { rpcFactory: stub.factory, }); await agent.initialize(init); - await agent.newSession({ cwd: "/repo" } as unknown as NewSessionRequest); + await agent.newSession({ + cwd: "/repo", + _meta: { environment: "cloud" }, + } as unknown as NewSessionRequest); stub.emit("item/started", { item: { @@ -712,7 +818,7 @@ describe("CodexAppServerAgent", () => { id: "m1", server: "posthog", tool: "exec", - arguments: { command: "call execute-sql {}" }, + arguments: { command: "call dashboard-delete {}" }, }, }); const decision = await stub.invokeRequest("mcpServer/elicitation/request", { @@ -726,7 +832,7 @@ describe("CodexAppServerAgent", () => { expect(decision).toMatchObject({ action: "accept" }); expect(permissionToolCalls[0]).toMatchObject({ toolCallId: "posthog:elicitation", - rawInput: { command: "call execute-sql {}" }, + rawInput: { command: "call dashboard-delete {}" }, _meta: { posthog: { toolName: "mcp__posthog__exec", @@ -1101,6 +1207,67 @@ describe("CodexAppServerAgent", () => { }); }); + it.each(["[", ""])( + "falls back to the default regex when session metadata carries the invalid regex %j", + async (posthogExecPermissionRegex) => { + const stub = makeStubRpc({ + initialize: {}, + "thread/start": { thread: { id: "thr_1" } }, + }); + const requestPermission = vi.fn().mockResolvedValue({ + outcome: { outcome: "selected", optionId: "allow" }, + }); + const client = { + sessionUpdate: async () => {}, + requestPermission, + extNotification: async () => {}, + } as unknown as AgentSideConnection; + const agent = new CodexAppServerAgent(client, { + processOptions: { binaryPath: "/bundle/codex" }, + model: "gpt-5.5", + rpcFactory: stub.factory, + }); + await agent.initialize(init); + await agent.newSession({ + cwd: "/repo", + _meta: { environment: "cloud", posthogExecPermissionRegex }, + } as unknown as NewSessionRequest); + + stub.emit("item/started", { + item: { + type: "mcpToolCall", + id: "m1", + server: "posthog", + tool: "exec", + arguments: { command: "call execute-sql {}" }, + }, + }); + const nonMatching = await stub.invokeRequest( + "item/commandExecution/requestApproval", + { itemId: "m1", command: "exec" }, + ); + stub.emit("item/started", { + item: { + type: "mcpToolCall", + id: "m2", + server: "posthog", + tool: "exec", + arguments: { command: "call dashboard-delete {}" }, + }, + }); + const matching = await stub.invokeRequest( + "item/commandExecution/requestApproval", + { itemId: "m2", command: "exec" }, + ); + + // The default destructive-verbs regex applies: execute-sql auto-accepts + // without a prompt, dashboard-delete relays for approval. + expect(nonMatching).toEqual({ decision: "accept" }); + expect(matching).toEqual({ decision: "accept" }); + expect(requestPermission).toHaveBeenCalledTimes(1); + }, + ); + it("flattens the host's {append} systemPrompt and dedupes it against developerInstructions", async () => { const stub = makeStubRpc({ "thread/start": { thread: { id: "t" } } }); const { client } = makeFakeClient(); diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts index 14df545f46..5aebe1c583 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts @@ -30,7 +30,12 @@ import { POSTHOG_NOTIFICATIONS, } from "../../acp-extensions"; import { DEFAULT_CODEX_MODEL } from "../../gateway-models"; -import { DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE } from "../../posthog-exec-permission"; +import { + extractPostHogSubTool, + isPostHogExecDescriptor, + matchesPostHogExecPermission, + resolvePostHogExecPermissionRegex, +} from "../../posthog-exec-permission"; import type { ProcessSpawnedCallback } from "../../types"; import { ALLOW_BYPASS } from "../../utils/common"; import { Logger } from "../../utils/logger"; @@ -237,6 +242,9 @@ export class CodexAppServerAgent extends BaseAcpAgent { private taskRunId?: string; /** Deployment environment; on "cloud" a non-danger sandbox would panic, so we skip the override. */ private environment?: "local" | "cloud"; + /** Gates PostHog exec sub-tools; set per session, defaults to the destructive-verbs regex. */ + private posthogExecPermissionRegex = + resolvePostHogExecPermissionRegex(undefined); private readonly commandOutputs = new Map(); /** Extra writable roots for this session, folded into workspaceWrite sandbox turns. */ private additionalDirectories?: string[]; @@ -494,10 +502,17 @@ export class CodexAppServerAgent extends BaseAcpAgent { { error: String(err) }, ); } + this.posthogExecPermissionRegex = resolvePostHogExecPermissionRegex( + params.meta?.posthogExecPermissionRegex, + (message) => + this.logger.warn( + "Invalid posthogExecPermissionRegex in session metadata; using default", + { message }, + ), + ); const mcpServers = toCodexMcpServers( [...(params.mcpServers ?? []), ...(localTools ? [localTools] : [])], - params.meta?.posthogExecPermissionRegex ?? - DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + { gatePosthogExec: true }, ); const config = buildThreadConfig(mcpServers, params.additionalDirectories); @@ -1543,6 +1558,36 @@ export class CodexAppServerAgent extends BaseAcpAgent { ); } + /** + * Sub-tool policy for a gated PostHog exec call. Codex's `approval_mode: + * "prompt"` gates the whole exec tool, so this is where the configured regex + * actually filters: non-matching sub-tools never prompt, and matching ones + * stay hands-off in local auto/full-access modes (parity with the Claude + * adapter's `!cloudMode && (auto || bypassPermissions)` branch). Cloud + * sessions always relay matching sub-tools so AgentServer routes them by the + * run's effective mode. + */ + private shouldAutoAcceptPostHogExec(mcp: { + server: string; + tool: string; + args: unknown; + }): boolean { + if (!isPostHogExecDescriptor({ server: mcp.server, tool: mcp.tool })) { + return false; + } + const subTool = extractPostHogSubTool(mcp.args); + if ( + !subTool || + !matchesPostHogExecPermission(subTool, this.posthogExecPermissionRegex) + ) { + return true; + } + return ( + this.environment !== "cloud" && + (this.config.mode === "auto" || this.config.mode === "full-access") + ); + } + /** * Server-initiated requests. Simple approvals resolve to a `{ decision }` envelope (a bare * string is rejected); richer ones (AskUserQuestion / permission profile / elicitation) go @@ -1556,6 +1601,8 @@ export class CodexAppServerAgent extends BaseAcpAgent { sessionId: this.sessionId, logger: this.logger, resolveMcpToolCall: (serverName) => this.mcp.byServer(serverName), + shouldAutoAcceptMcpToolCall: (mcp) => + this.shouldAutoAcceptPostHogExec(mcp), }); if (richer.handled) { return richer.response; @@ -1603,6 +1650,9 @@ export class CodexAppServerAgent extends BaseAcpAgent { // Codex has no MCP-specific approval; a known MCP call surfaces the real server/tool/args // so the host renders the proper MCP permission (incl. PostHog `exec` unwrapping). const mcp = this.mcp.byItemId(detail.itemId); + if (mcp && this.shouldAutoAcceptPostHogExec(mcp)) { + return { decision: "accept" }; + } // kind + content route plain command/file approvals to Execute/EditPermission (not the fallback). const toolCall = mcp ? { diff --git a/packages/agent/src/adapters/codex-app-server/mcp-config.test.ts b/packages/agent/src/adapters/codex-app-server/mcp-config.test.ts index 98e1943da2..01c1196e20 100644 --- a/packages/agent/src/adapters/codex-app-server/mcp-config.test.ts +++ b/packages/agent/src/adapters/codex-app-server/mcp-config.test.ts @@ -58,7 +58,7 @@ describe("toCodexMcpServers", () => { }); }); - it("prompts for PostHog exec when the permission regex is configured", () => { + it("prompts for PostHog exec when gating is enabled", () => { const servers = [ { type: "http", @@ -72,7 +72,7 @@ describe("toCodexMcpServers", () => { }, ] as unknown as McpServer[]; - expect(toCodexMcpServers(servers, "delete|destroy")).toEqual({ + expect(toCodexMcpServers(servers, { gatePosthogExec: true })).toEqual({ posthog_cloud: { url: "https://mcp.example/mcp", tools: { exec: { approval_mode: "prompt" } }, @@ -81,7 +81,7 @@ describe("toCodexMcpServers", () => { }); }); - it("leaves PostHog exec unchanged when no permission regex is configured", () => { + it("leaves PostHog exec unchanged when gating is not enabled", () => { const servers = [ { type: "http", diff --git a/packages/agent/src/adapters/codex-app-server/mcp-config.ts b/packages/agent/src/adapters/codex-app-server/mcp-config.ts index 7e35b35c86..36465825a2 100644 --- a/packages/agent/src/adapters/codex-app-server/mcp-config.ts +++ b/packages/agent/src/adapters/codex-app-server/mcp-config.ts @@ -31,7 +31,7 @@ export type CodexMcpServerConfig = */ export function toCodexMcpServers( servers: McpServer[] | undefined, - posthogExecPermissionRegex?: string, + options?: { gatePosthogExec?: boolean }, ): Record | undefined { if (!servers || servers.length === 0) { return undefined; @@ -39,8 +39,11 @@ export function toCodexMcpServers( const out: Record = {}; for (const server of servers) { + // `approval_mode: "prompt"` makes codex ask before every exec call; the + // per-sub-tool regex filtering happens in the adapter's approval handlers, + // which auto-accept calls the session's permission policy does not gate. const policy = - posthogExecPermissionRegex && + options?.gatePosthogExec && isPostHogExecDescriptor({ server: server.name, tool: "exec" }) ? { tools: { exec: { approval_mode: "prompt" as const } } } : {}; diff --git a/packages/agent/src/posthog-exec-permission.test.ts b/packages/agent/src/posthog-exec-permission.test.ts index ee08343e4d..c83389a105 100644 --- a/packages/agent/src/posthog-exec-permission.test.ts +++ b/packages/agent/src/posthog-exec-permission.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { compilePostHogExecPermissionRegex, DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, @@ -6,6 +6,7 @@ import { isPostHogExecDescriptor, isPostHogExecTool, matchesPostHogExecPermission, + resolvePostHogExecPermissionRegex, } from "./posthog-exec-permission"; const permissionRegex = compilePostHogExecPermissionRegex( @@ -82,3 +83,36 @@ describe("configured permission regex", () => { expect(() => compilePostHogExecPermissionRegex("[")).toThrow(); }); }); + +describe("resolvePostHogExecPermissionRegex", () => { + it.each([undefined, null])("compiles the default for %s", (value) => { + const onInvalid = vi.fn(); + const regex = resolvePostHogExecPermissionRegex(value, onInvalid); + expect(regex.source).toBe( + compilePostHogExecPermissionRegex( + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + ).source, + ); + expect(onInvalid).not.toHaveBeenCalled(); + }); + + it("compiles a valid custom source case-insensitively", () => { + const regex = resolvePostHogExecPermissionRegex("(^|-)archive(-|$)"); + expect(regex.test("Dashboard-Archive")).toBe(true); + expect(regex.test("dashboard-delete")).toBe(false); + }); + + it.each(["", "[", 42])( + "falls back to the default and reports %j as invalid", + (value) => { + const onInvalid = vi.fn(); + const regex = resolvePostHogExecPermissionRegex(value, onInvalid); + expect(regex.source).toBe( + compilePostHogExecPermissionRegex( + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + ).source, + ); + expect(onInvalid).toHaveBeenCalledTimes(1); + }, + ); +}); diff --git a/packages/agent/src/posthog-exec-permission.ts b/packages/agent/src/posthog-exec-permission.ts index 6dbb0eb560..3b451ec649 100644 --- a/packages/agent/src/posthog-exec-permission.ts +++ b/packages/agent/src/posthog-exec-permission.ts @@ -1,3 +1,5 @@ +import { z } from "zod/v4"; + /** * The PostHog MCP exposes a single `exec` dispatcher tool that runs * subcommands like `call [--json] [json]`. These helpers identify @@ -5,17 +7,70 @@ * configured permission regex at sub-tool granularity. */ +/** + * Naming contract: the PostHog exec dispatcher is recognized only for server + * names of the form `posthog` plus optional `_`-separated suffixes (e.g. + * `posthog_cloud`). The dispatcher is always registered under the literal name + * `posthog` (workspace-server auth-adapter), which is also reserved against + * user MCP imports (core localMcpImport). Cloud provisioning must pass a + * conforming name — a hyphenated or prefixed name (e.g. `posthog-eu`) is not + * recognized and the exec guard silently won't apply. + */ const POSTHOG_EXEC_TOOL_RE = /^mcp__posthog(?:_[^_]+)*__exec$/; const POSTHOG_CALL_COMMAND_RE = /^\s*call\s+(?:--json\s+)?([a-zA-Z0-9_-]+)/; +const posthogExecInputSchema = z.looseObject({ command: z.string() }); + export const DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE = "(^|-)(partial-update|update|patch|delete|destroy)(-|$)"; +export const posthogExecPermissionRegexSchema = z + .string() + .min(1, "PostHog exec permission regex cannot be empty") + .refine( + (source) => { + try { + compilePostHogExecPermissionRegex(source); + return true; + } catch { + return false; + } + }, + { error: "PostHog exec permission regex must be valid" }, + ); + export function compilePostHogExecPermissionRegex(source: string): RegExp { return new RegExp(source, "i"); } +/** + * Resolves a session-metadata regex value to a compiled regex. Absent values + * use the default; invalid values (non-string, empty, uncompilable) report via + * `onInvalid` and fall back to the default rather than failing the session. + */ +export function resolvePostHogExecPermissionRegex( + value: unknown, + onInvalid?: (message: string) => void, +): RegExp { + if (value === undefined || value === null) { + return compilePostHogExecPermissionRegex( + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + ); + } + const parsed = posthogExecPermissionRegexSchema.safeParse(value); + if (!parsed.success) { + onInvalid?.( + parsed.error.issues[0]?.message ?? + "PostHog exec permission regex must be a valid regex string", + ); + return compilePostHogExecPermissionRegex( + DEFAULT_POSTHOG_EXEC_PERMISSION_REGEX_SOURCE, + ); + } + return compilePostHogExecPermissionRegex(parsed.data); +} + export function isPostHogExecTool(toolName: string): boolean { return POSTHOG_EXEC_TOOL_RE.test(toolName); } @@ -28,10 +83,9 @@ export function isPostHogExecDescriptor(descriptor: { } export function extractPostHogSubTool(toolInput: unknown): string | null { - if (!toolInput || typeof toolInput !== "object") return null; - const command = (toolInput as { command?: unknown }).command; - if (typeof command !== "string") return null; - const match = command.match(POSTHOG_CALL_COMMAND_RE); + const parsed = posthogExecInputSchema.safeParse(toolInput); + if (!parsed.success) return null; + const match = parsed.data.command.match(POSTHOG_CALL_COMMAND_RE); return match ? (match[1] ?? null) : null; } diff --git a/packages/agent/src/server/agent-server.test.ts b/packages/agent/src/server/agent-server.test.ts index ccc14c6b7f..331202f163 100644 --- a/packages/agent/src/server/agent-server.test.ts +++ b/packages/agent/src/server/agent-server.test.ts @@ -1298,7 +1298,10 @@ describe("AgentServer HTTP Mode", () => { eventStreamSender: unknown; relayPermissionToClient: (params: unknown) => Promise; pendingPermissions: Map; - resolvePermission: (requestId: string, optionId: string) => boolean; + resolvePermission: ( + requestId: string, + optionId: string, + ) => "resolved" | "not_found" | "invalid_option"; createCloudClient(payload: { run_id: string; task_id: string; @@ -1626,15 +1629,53 @@ describe("AgentServer HTTP Mode", () => { expect(requestId).toBeDefined(); expect( testServer.resolvePermission(requestId as string, "allow_always"), - ).toBe(false); + ).toBe("invalid_option"); expect(testServer.pendingPermissions.has(requestId as string)).toBe(true); + expect(testServer.resolvePermission("nope", "allow_once")).toBe( + "not_found", + ); expect( testServer.resolvePermission(requestId as string, "allow_once"), - ).toBe(true); + ).toBe("resolved"); await expect(pending).resolves.toEqual({ outcome: { outcome: "selected", optionId: "allow_once" }, }); }); + + it("distinguishes unknown requests from unoffered options in permission_response errors", async () => { + const server = createServer(); + const testServer = exposeCloudClient(server); + const commandServer = server as unknown as { + session: unknown; + executeCommand( + method: string, + params: Record, + ): Promise; + }; + void testServer.relayPermissionToClient({ + options: [{ optionId: "allow_once", kind: "allow_once" }], + }); + const requestId = [...testServer.pendingPermissions.keys()][0] as string; + // Both error paths return before touching the session; the guard at the + // top of executeCommand only needs it to exist. + commandServer.session = {}; + + await expect( + commandServer.executeCommand("permission_response", { + requestId: "missing", + optionId: "allow_once", + }), + ).rejects.toThrow("No pending permission request found for id: missing"); + await expect( + commandServer.executeCommand("permission_response", { + requestId, + optionId: "allow_always", + }), + ).rejects.toThrow( + `Option "allow_always" was not offered for permission request ${requestId}`, + ); + expect(testServer.pendingPermissions.has(requestId)).toBe(true); + }); }); describe("refresh_session relay re-append", () => { diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index cedc8d0762..b94e862182 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -394,6 +394,12 @@ export class AgentServer { }) => void; toolCallId?: string; optionIds: Set; + /** + * Question responses carry synthetic `option_`/submit ids built by + * the client from the question `_meta`, not from the relayed options, so + * the offered-option check must not apply to them. + */ + validateOptionIds: boolean; } >(); private readonly posthogExecPermissionRegex: RegExp; @@ -1245,11 +1251,16 @@ export class AgentServer { customInput, answers, ); - if (!resolved) { + if (resolved === "not_found") { throw new Error( `No pending permission request found for id: ${requestId}`, ); } + if (resolved === "invalid_option") { + throw new Error( + `Option "${optionId}" was not offered for permission request ${requestId}`, + ); + } return { resolved: true }; } @@ -4412,11 +4423,15 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} toolCall: params.toolCall, }); + const toolCallMeta = params.toolCall?._meta as + | { codeToolKind?: unknown } + | undefined; return new Promise((resolve) => { this.pendingPermissions.set(requestId, { resolve, toolCallId, optionIds: new Set(params.options.map((option) => option.optionId)), + validateOptionIds: toolCallMeta?.codeToolKind !== "question", }); }); } @@ -4439,10 +4454,14 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} optionId: string, customInput?: string, answers?: Record, - ): boolean { + ): "resolved" | "not_found" | "invalid_option" { const pending = this.pendingPermissions.get(requestId); - if (!pending) return false; - if (!pending.optionIds.has(optionId)) return false; + if (!pending) return "not_found"; + // The request stays parked and resolvable — a corrected response with an + // offered option can still settle it. + if (pending.validateOptionIds && !pending.optionIds.has(optionId)) { + return "invalid_option"; + } this.pendingPermissions.delete(requestId); @@ -4460,6 +4479,6 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} outcome: { outcome: "selected" as const, optionId }, ...(Object.keys(meta).length > 0 ? { _meta: meta } : {}), }); - return true; + return "resolved"; } } diff --git a/packages/agent/src/server/question-relay.test.ts b/packages/agent/src/server/question-relay.test.ts index 3f8648f158..66ccd6a7f3 100644 --- a/packages/agent/src/server/question-relay.test.ts +++ b/packages/agent/src/server/question-relay.test.ts @@ -323,7 +323,7 @@ describe("Question relay", () => { optionId: string, customInput?: string, answers?: Record, - ) => boolean; + ) => "resolved" | "not_found" | "invalid_option"; }; srv.session = { payload: TEST_PAYLOAD, @@ -465,7 +465,10 @@ describe("Question relay", () => { options: unknown[]; toolCall?: unknown; }) => Promise<{ outcome: { outcome: string; optionId: string } }>; - resolvePermission: (requestId: string, optionId: string) => boolean; + resolvePermission: ( + requestId: string, + optionId: string, + ) => "resolved" | "not_found" | "invalid_option"; session: { payload: typeof TEST_PAYLOAD; sseController: null; @@ -494,7 +497,7 @@ describe("Question relay", () => { expect(request.params.toolCallId).toBe("tool-1"); const requestId = request.params.requestId; - expect(srv.resolvePermission(requestId, "allow")).toBe(true); + expect(srv.resolvePermission(requestId, "allow")).toBe("resolved"); const resolved = logged("_posthog/permission_resolved"); expect(resolved).toBeTruthy(); diff --git a/packages/agent/src/server/schemas.ts b/packages/agent/src/server/schemas.ts index 2290c56fdc..46b8ae16b9 100644 --- a/packages/agent/src/server/schemas.ts +++ b/packages/agent/src/server/schemas.ts @@ -1,5 +1,6 @@ import { z } from "zod/v4"; -import { compilePostHogExecPermissionRegex } from "../posthog-exec-permission"; + +export { posthogExecPermissionRegexSchema } from "../posthog-exec-permission"; const httpHeaderSchema = z.object({ name: z.string(), @@ -28,21 +29,6 @@ const remoteMcpServerSchema = z.object({ export const mcpServersSchema = z.array(remoteMcpServerSchema); -export const posthogExecPermissionRegexSchema = z - .string() - .min(1, "PostHog exec permission regex cannot be empty") - .refine( - (source) => { - try { - compilePostHogExecPermissionRegex(source); - return true; - } catch { - return false; - } - }, - { error: "PostHog exec permission regex must be valid" }, - ); - export type RemoteMcpServer = z.infer; export const claudeCodeConfigSchema = z.object({ From 999258f80868ee2a259c4f11a8bf1bc9cdd4ecc0 Mon Sep 17 00:00:00 2001 From: Georgiy Tarasov Date: Tue, 21 Jul 2026 12:15:28 +0200 Subject: [PATCH 5/5] fix --- packages/agent/src/posthog-exec-permission.test.ts | 3 +++ packages/agent/src/posthog-exec-permission.ts | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/agent/src/posthog-exec-permission.test.ts b/packages/agent/src/posthog-exec-permission.test.ts index c83389a105..0751515633 100644 --- a/packages/agent/src/posthog-exec-permission.test.ts +++ b/packages/agent/src/posthog-exec-permission.test.ts @@ -36,6 +36,8 @@ describe("extractPostHogSubTool", () => { it.each([ ["call experiment-update", "experiment-update"], ['call --json experiment-update {"id":1}', "experiment-update"], + ['call --confirm dashboard-update {"id":2}', "dashboard-update"], + ['call --json --confirm dashboard-update {"id":2}', "dashboard-update"], [" call foo-delete", "foo-delete"], ])("extracts the sub-tool from %s", (command, expected) => { expect(extractPostHogSubTool({ command })).toBe(expected); @@ -45,6 +47,7 @@ describe("extractPostHogSubTool", () => { { command: "tools" }, { command: "search experiments" }, { command: "info flag-get" }, + { command: "call --confirm" }, undefined, null, {}, diff --git a/packages/agent/src/posthog-exec-permission.ts b/packages/agent/src/posthog-exec-permission.ts index 3b451ec649..699fad4bf5 100644 --- a/packages/agent/src/posthog-exec-permission.ts +++ b/packages/agent/src/posthog-exec-permission.ts @@ -2,7 +2,7 @@ import { z } from "zod/v4"; /** * The PostHog MCP exposes a single `exec` dispatcher tool that runs - * subcommands like `call [--json] [json]`. These helpers identify + * subcommands like `call [--flags] [json]`. These helpers identify * that dispatcher, extract the delegated tool, and apply the externally * configured permission regex at sub-tool granularity. */ @@ -18,7 +18,14 @@ import { z } from "zod/v4"; */ const POSTHOG_EXEC_TOOL_RE = /^mcp__posthog(?:_[^_]+)*__exec$/; -const POSTHOG_CALL_COMMAND_RE = /^\s*call\s+(?:--json\s+)?([a-zA-Z0-9_-]+)/; +// Skip every `--flag` token after `call` (`--json`, `--confirm`, future ones) +// before capturing the sub-tool, and require the sub-tool to start with an +// alphanumeric. Matching only `--json` let `call --confirm dashboard-update` +// capture `--confirm` as the sub-tool, which never matches the destructive +// regex — so exactly the calls the dispatcher flags as destructive bypassed +// the permission gate. +const POSTHOG_CALL_COMMAND_RE = + /^\s*call\s+(?:--\S+\s+)*([a-zA-Z0-9][a-zA-Z0-9_-]*)/; const posthogExecInputSchema = z.looseObject({ command: z.string() });