diff --git a/src/hooks/claude-code-hooks/config-loader.ts b/src/hooks/claude-code-hooks/config-loader.ts index ea494ffdb..2b079fbfc 100644 --- a/src/hooks/claude-code-hooks/config-loader.ts +++ b/src/hooks/claude-code-hooks/config-loader.ts @@ -11,7 +11,14 @@ export interface DisabledHooksConfig { Stop?: string[] PreToolUse?: string[] PostToolUse?: string[] + PostToolUseFailure?: string[] + PermissionRequest?: string[] UserPromptSubmit?: string[] + Notification?: string[] + SubagentStart?: string[] + SubagentStop?: string[] + SessionStart?: string[] + SessionEnd?: string[] PreCompact?: string[] } @@ -78,10 +85,17 @@ function mergeDisabledHooks( if (!base) return override return { - Stop: override.Stop ?? base.Stop, PreToolUse: override.PreToolUse ?? base.PreToolUse, PostToolUse: override.PostToolUse ?? base.PostToolUse, + PostToolUseFailure: override.PostToolUseFailure ?? base.PostToolUseFailure, + PermissionRequest: override.PermissionRequest ?? base.PermissionRequest, UserPromptSubmit: override.UserPromptSubmit ?? base.UserPromptSubmit, + Notification: override.Notification ?? base.Notification, + Stop: override.Stop ?? base.Stop, + SubagentStart: override.SubagentStart ?? base.SubagentStart, + SubagentStop: override.SubagentStop ?? base.SubagentStop, + SessionStart: override.SessionStart ?? base.SessionStart, + SessionEnd: override.SessionEnd ?? base.SessionEnd, PreCompact: override.PreCompact ?? base.PreCompact, } } diff --git a/src/hooks/claude-code-hooks/config.test.ts b/src/hooks/claude-code-hooks/config.test.ts index 2fdaa9c70..5de403701 100644 --- a/src/hooks/claude-code-hooks/config.test.ts +++ b/src/hooks/claude-code-hooks/config.test.ts @@ -93,4 +93,228 @@ function getStopCommands(config: Awaited { + const { mergePluginHooksConfigs, setPluginHooksConfigs, clearClaudeHooksConfigCache: _clearCache, resetPluginHooksState } = require("./config") + const { setAdditionalAllowedMcpEnvVars, resetAdditionalAllowedMcpEnvVars } = require("../../features/claude-code-mcp-loader/configure-allowed-env-vars") + + afterEach(() => { + resetAdditionalAllowedMcpEnvVars() + resetPluginHooksState() + }) + + test("#given empty plugin hooks #when merged #then returns base unchanged", () => { + // given + const base = { + Stop: [{ matcher: "*", hooks: [{ type: "command" as const, command: "echo stop" }] }], + } + + // when + const result = mergePluginHooksConfigs(base, []) + + // then + expect(result).toEqual(base) + }) + + test("#given plugin command hook #when merged #then allowedEnvVars is set to MCP allowlist", () => { + // given + setAdditionalAllowedMcpEnvVars(["MY_VAR"]) + const base = {} + const pluginConfig = { + hooks: { + Stop: [ + { + matcher: "*", + hooks: [{ type: "command", command: "echo hello" }], + }, + ], + }, + } + + // when + const result = mergePluginHooksConfigs(base, [pluginConfig]) + + // then + const stopHooks = result.Stop ?? [] + expect(stopHooks.length).toBe(1) + const hook = stopHooks[0].hooks[0] + expect(hook.type).toBe("command") + if (hook.type === "command") { + expect(hook.allowedEnvVars).toContain("MY_VAR") + } + }) + + test("#given plugin HTTP hook with allowedEnvVars #when merged #then vars are intersected with MCP allowlist", () => { + // given + setAdditionalAllowedMcpEnvVars(["MY_TOKEN"]) + const base = {} + const pluginConfig = { + hooks: { + PreToolUse: [ + { + matcher: "*", + hooks: [{ type: "http", url: "https://example.com/hook", allowedEnvVars: ["MY_TOKEN", "SECRET_KEY"] }], + }, + ], + }, + } + + // when + const result = mergePluginHooksConfigs(base, [pluginConfig]) + + // then + const preToolHooks = result.PreToolUse ?? [] + expect(preToolHooks.length).toBe(1) + const hook = preToolHooks[0].hooks[0] + expect(hook.type).toBe("http") + if (hook.type === "http") { + expect(hook.allowedEnvVars).toContain("MY_TOKEN") + expect(hook.allowedEnvVars).not.toContain("SECRET_KEY") + } + }) + + test("#given plugin HTTP hook with no allowedEnvVars #when merged #then hook passes through without crash", () => { + // given + const base = {} + const pluginConfig = { + hooks: { + PostToolUse: [ + { + matcher: "*", + hooks: [{ type: "http", url: "https://example.com/post" }], + }, + ], + }, + } + + // when + const result = mergePluginHooksConfigs(base, [pluginConfig]) + + // then + const postToolHooks = result.PostToolUse ?? [] + expect(postToolHooks.length).toBe(1) + const hook = postToolHooks[0].hooks[0] + expect(hook.type).toBe("http") + if (hook.type === "http") { + expect(hook.allowedEnvVars).toBeUndefined() + } + }) + + test("#given plugin hook with invalid action type #when merged #then it is filtered out", () => { + // given + const base = {} + const pluginConfig = { + hooks: { + Stop: [ + { + matcher: "*", + hooks: [ + { type: "command", command: "echo valid" }, + { type: "invalid", something: "bad" }, + { notAHook: true }, + ], + }, + ], + }, + } + + // when + const result = mergePluginHooksConfigs(base, [pluginConfig]) + + // then + const stopHooks = result.Stop ?? [] + expect(stopHooks.length).toBe(1) + expect(stopHooks[0].hooks.length).toBe(1) + expect(stopHooks[0].hooks[0].type).toBe("command") + }) + + test("#given plugin hook with non-array hooks #when merged #then it is skipped", () => { + // given + const base = {} + const pluginConfig = { + hooks: { + Stop: "not-an-array", + PreToolUse: 42, + }, + } + + // when + const result = mergePluginHooksConfigs(base, [pluginConfig]) + + // then + expect(result.Stop).toBeUndefined() + expect(result.PreToolUse).toBeUndefined() + }) +}) + +describe("setPluginHooksConfigs", () => { + const { setPluginHooksConfigs, loadClaudeHooksConfig, clearClaudeHooksConfigCache: _clearCache, resetPluginHooksState } = require("./config") + const { resetAdditionalAllowedMcpEnvVars } = require("../../features/claude-code-mcp-loader/configure-allowed-env-vars") + let originalWorkingDirectory = "" + + beforeEach(() => { + originalWorkingDirectory = process.cwd() + _clearCache() + }) + + afterEach(() => { + _clearCache() + resetAdditionalAllowedMcpEnvVars() + resetPluginHooksState() + process.chdir(originalWorkingDirectory) + }) + + test("#given configs set for directory A #when loading config from directory A #then plugin hooks are included", async () => { + // given + const dirA = process.cwd() + setPluginHooksConfigs(dirA, [ + { + hooks: { + Stop: [ + { + matcher: "*", + hooks: [{ type: "command", command: "echo plugin-stop" }], + }, + ], + }, + }, + ]) + + // when + const config = await loadClaudeHooksConfig() + + // then + const stopCommands = (config?.Stop ?? []).flatMap((m) => + m.hooks.flatMap((h) => (h.type === "command" && typeof h.command === "string" ? [h.command] : [])), + ) + expect(stopCommands).toContain("echo plugin-stop") + }) + + test("#given configs set for directory A #when loading config from directory B #then plugin hooks are NOT included", async () => { + // given + const dirA = "/tmp/omo-test-dir-a" + const dirB = process.cwd() + setPluginHooksConfigs(dirA, [ + { + hooks: { + Stop: [ + { + matcher: "*", + hooks: [{ type: "command", command: "echo plugin-stop-a" }], + }, + ], + }, + }, + ]) + + // when — cwd is dirB, not dirA + const config = await loadClaudeHooksConfig() + + // then — plugin hooks from dirA should NOT appear + const stopCommands = (config?.Stop ?? []).flatMap((m) => + m.hooks.flatMap((h) => (h.type === "command" && typeof h.command === "string" ? [h.command] : [])), + ) + expect(stopCommands).not.toContain("echo plugin-stop-a") + }) +}) + export {} diff --git a/src/hooks/claude-code-hooks/config.ts b/src/hooks/claude-code-hooks/config.ts index 7d6341cfe..362a2d021 100644 --- a/src/hooks/claude-code-hooks/config.ts +++ b/src/hooks/claude-code-hooks/config.ts @@ -2,7 +2,8 @@ import { join } from "path" import { existsSync } from "fs" import { getClaudeConfigDir } from "../../shared" import { bunFile } from "../../shared/bun-file-shim" -import type { ClaudeHooksConfig, HookMatcher, HookAction } from "./types" +import { getAllowedMcpEnvVars } from "../../features/claude-code-mcp-loader/configure-allowed-env-vars" +import type { ClaudeHooksConfig, HookMatcher, HookAction, PluginHooksConfig } from "./types" const CONFIG_CACHE_TTL_MS = 30_000 @@ -22,11 +23,33 @@ interface RawHookMatcher { interface RawClaudeHooksConfig { PreToolUse?: RawHookMatcher[] PostToolUse?: RawHookMatcher[] + PostToolUseFailure?: RawHookMatcher[] + PermissionRequest?: RawHookMatcher[] UserPromptSubmit?: RawHookMatcher[] + Notification?: RawHookMatcher[] Stop?: RawHookMatcher[] + SubagentStart?: RawHookMatcher[] + SubagentStop?: RawHookMatcher[] + SessionStart?: RawHookMatcher[] + SessionEnd?: RawHookMatcher[] PreCompact?: RawHookMatcher[] } +const ALL_HOOK_EVENT_TYPES: (keyof ClaudeHooksConfig)[] = [ + "PreToolUse", + "PostToolUse", + "PostToolUseFailure", + "PermissionRequest", + "UserPromptSubmit", + "Notification", + "Stop", + "SubagentStart", + "SubagentStop", + "SessionStart", + "SessionEnd", + "PreCompact", +] + function normalizeHookMatcher(raw: RawHookMatcher): HookMatcher { return { matcher: raw.matcher ?? raw.pattern ?? "*", @@ -36,15 +59,8 @@ function normalizeHookMatcher(raw: RawHookMatcher): HookMatcher { function normalizeHooksConfig(raw: RawClaudeHooksConfig): ClaudeHooksConfig { const result: ClaudeHooksConfig = {} - const eventTypes: (keyof RawClaudeHooksConfig)[] = [ - "PreToolUse", - "PostToolUse", - "UserPromptSubmit", - "Stop", - "PreCompact", - ] - for (const eventType of eventTypes) { + for (const eventType of ALL_HOOK_EVENT_TYPES) { if (raw[eventType]) { result[eventType] = raw[eventType].map(normalizeHookMatcher) } @@ -92,19 +108,16 @@ export function clearClaudeHooksConfigCache(): void { configCache.clear() } +export function resetPluginHooksState(): void { + pluginHooksState.clear() +} + function mergeHooksConfig( base: ClaudeHooksConfig, override: ClaudeHooksConfig ): ClaudeHooksConfig { const result: ClaudeHooksConfig = { ...base } - const eventTypes: (keyof ClaudeHooksConfig)[] = [ - "PreToolUse", - "PostToolUse", - "UserPromptSubmit", - "Stop", - "PreCompact", - ] - for (const eventType of eventTypes) { + for (const eventType of ALL_HOOK_EVENT_TYPES) { if (override[eventType]) { result[eventType] = [...(base[eventType] || []), ...override[eventType]] } @@ -112,6 +125,109 @@ function mergeHooksConfig( return result } +/** + * Encapsulates mutable plugin hooks state with per-project keying. + * Replaces module-level `let pendingPluginHooksConfigs`. + */ +class PluginHooksState { + private configs = new Map() + + setConfigs(directory: string, configs: PluginHooksConfig[]): void { + this.configs.set(directory, configs) + } + + getConfigs(directory: string): PluginHooksConfig[] { + return this.configs.get(directory) ?? [] + } + + clear(): void { + this.configs.clear() + } +} + +const pluginHooksState = new PluginHooksState() + +export function setPluginHooksConfigs(directory: string, configs: PluginHooksConfig[]): void { + pluginHooksState.setConfigs(directory, configs) + configCache.clear() +} + +function isHookAction(h: unknown): h is HookAction { + if (typeof h !== "object" || h === null) return false + const obj = h as Record + if (obj.type === "command" && typeof obj.command === "string") return true + if (obj.type === "http" && typeof obj.url === "string") return true + return false +} + +interface PluginHookMatcher { + matcher?: string + pattern?: string + hooks?: unknown[] +} + +function isPluginHookMatcher(m: unknown): m is PluginHookMatcher { + return typeof m === "object" && m !== null && Array.isArray((m as PluginHookMatcher).hooks) +} + +/** + * Intersect plugin hook allowedEnvVars with the MCP env allowlist. + * For HTTP hooks: filter allowedEnvVars to only allowlisted vars. + * For command hooks: set allowedEnvVars to the full MCP allowlist. + */ +function applyMcpEnvAllowlist(action: HookAction): HookAction { + const allowedVars = getAllowedMcpEnvVars() + + if (action.type === "http") { + if (!action.allowedEnvVars || action.allowedEnvVars.length === 0) { + return action + } + const filtered = action.allowedEnvVars.filter((v) => allowedVars.has(v)) + return { ...action, allowedEnvVars: filtered } + } + + if (action.type === "command") { + return { ...action, allowedEnvVars: [...allowedVars] } + } + + return action +} + +export function mergePluginHooksConfigs( + base: ClaudeHooksConfig, + pluginHooksConfigs: PluginHooksConfig[] +): ClaudeHooksConfig { + let result = { ...base } + + for (const pluginConfig of pluginHooksConfigs) { + if (!pluginConfig.hooks) continue + + const pluginOverrides: ClaudeHooksConfig = {} + for (const eventType of ALL_HOOK_EVENT_TYPES) { + const pluginMatchers = pluginConfig.hooks[eventType] + if (!Array.isArray(pluginMatchers)) continue + + const converted: HookMatcher[] = pluginMatchers + .filter(isPluginHookMatcher) + .map((m) => ({ + matcher: m.matcher ?? m.pattern ?? "*", + hooks: (m.hooks ?? []) + .filter(isHookAction) + .map(applyMcpEnvAllowlist), + })) + .filter((m) => m.hooks.length > 0) + + if (converted.length > 0) { + pluginOverrides[eventType] = converted + } + } + + result = mergeHooksConfig(result, pluginOverrides) + } + + return result +} + export async function loadClaudeHooksConfig( customSettingsPath?: string ): Promise { @@ -139,6 +255,12 @@ export async function loadClaudeHooksConfig( } } + // Merge plugin hooks configs for the current project directory + const projectConfigs = pluginHooksState.getConfigs(process.cwd()) + if (projectConfigs.length > 0) { + mergedConfig = mergePluginHooksConfigs(mergedConfig, projectConfigs) + } + const resolvedConfig = Object.keys(mergedConfig).length > 0 ? mergedConfig : null configCache.set(cacheKey, { value: resolvedConfig, diff --git a/src/hooks/claude-code-hooks/dispatch-hook.test.ts b/src/hooks/claude-code-hooks/dispatch-hook.test.ts new file mode 100644 index 000000000..b8c0bda53 --- /dev/null +++ b/src/hooks/claude-code-hooks/dispatch-hook.test.ts @@ -0,0 +1,67 @@ +const { afterEach, beforeEach, describe, expect, mock, test } = require("bun:test") + +const capturedOptions: Array> = [] + +const mockExecuteHookCommand = mock( + (_command: string, _stdin: string, _cwd: string, options?: Record) => { + capturedOptions.push(options ?? {}) + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }) + }, +) + +mock.module("../../shared/command-executor/execute-hook-command", () => ({ + executeHookCommand: mockExecuteHookCommand, +})) + +mock.module("./execute-http-hook", () => ({ + executeHttpHook: mock(() => Promise.resolve({ exitCode: 0, stdout: "", stderr: "" })), +})) + +const { dispatchHook } = await import("./dispatch-hook") + +describe("dispatchHook", () => { + beforeEach(() => { + mockExecuteHookCommand.mockClear() + capturedOptions.length = 0 + }) + + afterEach(() => { + mockExecuteHookCommand.mockClear() + capturedOptions.length = 0 + }) + + test("#given HookCommand with allowedEnvVars #when dispatchHook called #then options include allowedEnvVars", async () => { + // given + const hook = { + type: "command" as const, + command: "echo hello", + allowedEnvVars: ["MY_VAR"], + } + + // when + await dispatchHook(hook, "{}", "/tmp") + + // then + expect(mockExecuteHookCommand).toHaveBeenCalledTimes(1) + expect(capturedOptions.length).toBe(1) + expect(capturedOptions[0].allowedEnvVars).toEqual(["MY_VAR"]) + }) + + test("#given HookCommand without allowedEnvVars #when dispatchHook called #then options do NOT include allowedEnvVars", async () => { + // given + const hook = { + type: "command" as const, + command: "echo hello", + } + + // when + await dispatchHook(hook, "{}", "/tmp") + + // then + expect(mockExecuteHookCommand).toHaveBeenCalledTimes(1) + expect(capturedOptions.length).toBe(1) + expect(capturedOptions[0].allowedEnvVars).toBeUndefined() + }) +}) + +export {} diff --git a/src/hooks/claude-code-hooks/dispatch-hook.ts b/src/hooks/claude-code-hooks/dispatch-hook.ts index 5feeabb62..e53219ef3 100644 --- a/src/hooks/claude-code-hooks/dispatch-hook.ts +++ b/src/hooks/claude-code-hooks/dispatch-hook.ts @@ -1,5 +1,5 @@ -import type { HookAction } from "./types" -import type { CommandResult } from "../../shared/command-executor/execute-hook-command" +import type { HookAction, HookCommand } from "./types" +import type { CommandResult, ExecuteHookOptions } from "../../shared/command-executor/execute-hook-command" import { executeHookCommand } from "../../shared" import { executeHttpHook } from "./execute-http-hook" import { DEFAULT_CONFIG } from "./plugin-config" @@ -18,10 +18,19 @@ export async function dispatchHook( return executeHttpHook(hook, stdinJson) } + const cmdHook = hook as HookCommand + const options: ExecuteHookOptions = { + forceZsh: DEFAULT_CONFIG.forceZsh, + zshPath: DEFAULT_CONFIG.zshPath, + } + if (cmdHook.allowedEnvVars) { + options.allowedEnvVars = cmdHook.allowedEnvVars + } + return executeHookCommand( hook.command, stdinJson, cwd, - { forceZsh: DEFAULT_CONFIG.forceZsh, zshPath: DEFAULT_CONFIG.zshPath } + options ) } diff --git a/src/hooks/claude-code-hooks/types.ts b/src/hooks/claude-code-hooks/types.ts index 28924de10..28e90c637 100644 --- a/src/hooks/claude-code-hooks/types.ts +++ b/src/hooks/claude-code-hooks/types.ts @@ -6,8 +6,15 @@ export type ClaudeHookEvent = | "PreToolUse" | "PostToolUse" + | "PostToolUseFailure" + | "PermissionRequest" | "UserPromptSubmit" + | "Notification" | "Stop" + | "SubagentStart" + | "SubagentStop" + | "SessionStart" + | "SessionEnd" | "PreCompact" export interface HookMatcher { @@ -18,6 +25,8 @@ export interface HookMatcher { export interface HookCommand { type: "command" command: string + /** Env vars allowed to pass through to the spawned process (plugin-sourced hooks are intersected with mcp_env_allowlist) */ + allowedEnvVars?: string[] } export interface HookHttp { @@ -33,8 +42,15 @@ export type HookAction = HookCommand | HookHttp export interface ClaudeHooksConfig { PreToolUse?: HookMatcher[] PostToolUse?: HookMatcher[] + PostToolUseFailure?: HookMatcher[] + PermissionRequest?: HookMatcher[] UserPromptSubmit?: HookMatcher[] + Notification?: HookMatcher[] Stop?: HookMatcher[] + SubagentStart?: HookMatcher[] + SubagentStop?: HookMatcher[] + SessionStart?: HookMatcher[] + SessionEnd?: HookMatcher[] PreCompact?: HookMatcher[] } @@ -212,3 +228,11 @@ export interface PluginConfig { disabledHooks?: boolean | ClaudeHookEvent[] keywordDetectorDisabled?: boolean } + +/** + * Plugin hooks configuration shape. + * Replaces the loose `Array<{ hooks?: Record }>` with a proper typed interface. + */ +export interface PluginHooksConfig { + hooks?: Partial> +} diff --git a/src/plugin-handlers/config-handler.ts b/src/plugin-handlers/config-handler.ts index d77920c44..36961021f 100644 --- a/src/plugin-handlers/config-handler.ts +++ b/src/plugin-handlers/config-handler.ts @@ -4,6 +4,7 @@ import type { ModelCacheState } from "../plugin-state"; import { log } from "../shared"; import { applyAgentConfig } from "./agent-config-handler"; import { applyCommandConfig } from "./command-config-handler"; +import { applyHookConfig } from "./hook-config-handler"; import { applyMcpConfig } from "./mcp-config-handler"; import { applyProviderConfig } from "./provider-config-handler"; import { loadPluginComponents } from "./plugin-components-loader"; @@ -30,6 +31,8 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { const pluginComponents = await loadPluginComponents({ pluginConfig }); + applyHookConfig({ pluginComponents, ctx }); + const agentResult = await applyAgentConfig({ config, pluginConfig, diff --git a/src/plugin-handlers/hook-config-handler.test.ts b/src/plugin-handlers/hook-config-handler.test.ts new file mode 100644 index 000000000..1c9085e0a --- /dev/null +++ b/src/plugin-handlers/hook-config-handler.test.ts @@ -0,0 +1,83 @@ +const { afterEach, beforeEach, describe, expect, mock, test } = require("bun:test") + +const mockSetPluginHooksConfigs = mock(() => {}) + +mock.module("../hooks/claude-code-hooks/config", () => ({ + setPluginHooksConfigs: mockSetPluginHooksConfigs, +})) + +const { applyHookConfig } = await import("./hook-config-handler") + +describe("applyHookConfig", () => { + beforeEach(() => { + mockSetPluginHooksConfigs.mockClear() + }) + + afterEach(() => { + mockSetPluginHooksConfigs.mockClear() + }) + + test("#given ctx.directory #when applyHookConfig called #then setPluginHooksConfigs receives ctx.directory", () => { + // given + const testDirectory = "/test/dir" + const pluginComponents = { + commands: {}, + skills: {}, + agents: {}, + mcpServers: {}, + hooksConfigs: [ + { + hooks: { + Stop: [ + { + matcher: "*", + hooks: [{ type: "command", command: "echo test" }], + }, + ], + }, + }, + ], + plugins: [{ name: "test-plugin", version: "1.0.0" }], + errors: [], + } + + // when + applyHookConfig({ + pluginComponents, + ctx: { directory: testDirectory }, + }) + + // then + expect(mockSetPluginHooksConfigs).toHaveBeenCalledTimes(1) + expect(mockSetPluginHooksConfigs).toHaveBeenCalledWith( + testDirectory, + pluginComponents.hooksConfigs, + ) + }) + + test("#given empty hooksConfigs #when applyHookConfig called #then setPluginHooksConfigs still called with empty array", () => { + // given + const testDirectory = "/another/dir" + const pluginComponents = { + commands: {}, + skills: {}, + agents: {}, + mcpServers: {}, + hooksConfigs: [], + plugins: [], + errors: [], + } + + // when + applyHookConfig({ + pluginComponents, + ctx: { directory: testDirectory }, + }) + + // then + expect(mockSetPluginHooksConfigs).toHaveBeenCalledTimes(1) + expect(mockSetPluginHooksConfigs).toHaveBeenCalledWith(testDirectory, []) + }) +}) + +export {} diff --git a/src/plugin-handlers/hook-config-handler.ts b/src/plugin-handlers/hook-config-handler.ts new file mode 100644 index 000000000..38381628d --- /dev/null +++ b/src/plugin-handlers/hook-config-handler.ts @@ -0,0 +1,19 @@ +import type { PluginComponents } from "./plugin-components-loader" +import { setPluginHooksConfigs } from "../hooks/claude-code-hooks/config" +import { log } from "../shared" + +export function applyHookConfig(params: { + pluginComponents: PluginComponents; + ctx: { directory: string }; +}): void { + const { pluginComponents, ctx } = params + + if (pluginComponents.hooksConfigs.length > 0) { + log("[hook-config-handler] Merging plugin hooks configs", { + count: pluginComponents.hooksConfigs.length, + plugins: pluginComponents.plugins.map(p => p.name), + }) + } + + setPluginHooksConfigs(ctx.directory, pluginComponents.hooksConfigs) +} diff --git a/src/plugin-handlers/index.ts b/src/plugin-handlers/index.ts index fa9bde977..9fe4d58b2 100644 --- a/src/plugin-handlers/index.ts +++ b/src/plugin-handlers/index.ts @@ -3,6 +3,7 @@ export * from "./provider-config-handler"; export * from "./agent-config-handler"; export * from "./tool-config-handler"; export * from "./mcp-config-handler"; +export * from "./hook-config-handler"; export * from "./command-config-handler"; export * from "./plugin-components-loader"; export * from "./category-config-resolver"; diff --git a/src/plugin-handlers/plugin-components-loader.ts b/src/plugin-handlers/plugin-components-loader.ts index 7d122a39e..75e4f814b 100644 --- a/src/plugin-handlers/plugin-components-loader.ts +++ b/src/plugin-handlers/plugin-components-loader.ts @@ -1,5 +1,6 @@ import type { OhMyOpenCodeConfig } from "../config"; import { loadAllPluginComponents } from "../features/claude-code-plugin-loader"; +import type { PluginHooksConfig } from "../hooks/claude-code-hooks/types"; import { addConfigLoadError, log } from "../shared"; export type PluginComponents = { @@ -7,7 +8,7 @@ export type PluginComponents = { skills: Record; agents: Record; mcpServers: Record; - hooksConfigs: Array<{ hooks?: Record }>; + hooksConfigs: PluginHooksConfig[]; plugins: Array<{ name: string; version: string }>; errors: Array<{ pluginKey: string; installPath: string; error: string }>; }; diff --git a/src/shared/command-executor/execute-hook-command.test.ts b/src/shared/command-executor/execute-hook-command.test.ts new file mode 100644 index 000000000..c3688c91e --- /dev/null +++ b/src/shared/command-executor/execute-hook-command.test.ts @@ -0,0 +1,62 @@ +const { afterEach, beforeEach, describe, expect, test } = require("bun:test") +import { tmpdir } from "node:os" +import { join } from "node:path" +import { mkdtempSync, rmSync } from "node:fs" + +const { executeHookCommand } = await import("./execute-hook-command") + +describe("executeHookCommand", () => { + let tempDirectory = "" + + beforeEach(() => { + tempDirectory = mkdtempSync(join(tmpdir(), "omo-exec-hook-cmd-")) + }) + + afterEach(() => { + rmSync(tempDirectory, { recursive: true, force: true }) + }) + + test("#given allowedEnvVars provided #when executing command #then only allowed vars are in process.env", async () => { + // given + process.env.__OMO_TEST_ALLOWED_VAR = "visible" + process.env.__OMO_TEST_SECRET_VAR = "hidden" + + // when + const result = await executeHookCommand( + "echo $__OMO_TEST_ALLOWED_VAR $__OMO_TEST_SECRET_VAR", + "", + tempDirectory, + { allowedEnvVars: ["__OMO_TEST_ALLOWED_VAR"] }, + ) + + // then + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain("visible") + expect(result.stdout).not.toContain("hidden") + + // cleanup + delete process.env.__OMO_TEST_ALLOWED_VAR + delete process.env.__OMO_TEST_SECRET_VAR + }) + + test("#given no allowedEnvVars #when executing command #then full env is available", async () => { + // given + process.env.__OMO_TEST_FULL_ENV_VAR = "present" + + // when + const result = await executeHookCommand( + "echo $__OMO_TEST_FULL_ENV_VAR", + "", + tempDirectory, + ) + + // then + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain("present") + + // cleanup + delete process.env.__OMO_TEST_FULL_ENV_VAR + }) +}) + +export {} diff --git a/src/shared/command-executor/execute-hook-command.ts b/src/shared/command-executor/execute-hook-command.ts index d4ded4a55..43c628f2f 100644 --- a/src/shared/command-executor/execute-hook-command.ts +++ b/src/shared/command-executor/execute-hook-command.ts @@ -16,6 +16,8 @@ export interface ExecuteHookOptions { zshPath?: string; /** Timeout in milliseconds. Process is killed after this. Default: 30000 */ timeoutMs?: number; + /** When provided, scrub process.env to only include these vars plus HOME/PATH/etc. Used for plugin-sourced hooks. */ + allowedEnvVars?: string[]; } export async function executeHookCommand( @@ -53,11 +55,33 @@ export async function executeHookCommand( let killTimer: ReturnType | null = null; const isWin32 = process.platform === "win32"; + + // Keys that are always set from normalized sources and must not be + // overwritten by ambient process.env values during the allowlist merge. + const PROTECTED_ENV_KEYS = new Set(["HOME", "CLAUDE_PROJECT_DIR"]); + + let env: Record; + if (options?.allowedEnvVars) { + const allowedSet = new Set(options.allowedEnvVars); + env = { + HOME: home, + CLAUDE_PROJECT_DIR: cwd, + PATH: process.env.PATH, + }; + for (const key of Object.keys(process.env)) { + if (allowedSet.has(key) && !PROTECTED_ENV_KEYS.has(key)) { + env[key] = process.env[key]; + } + } + } else { + env = { ...process.env, HOME: home, CLAUDE_PROJECT_DIR: cwd }; + } + const proc = spawn(finalCommand, { cwd, shell: true, detached: !isWin32, - env: { ...process.env, HOME: home, CLAUDE_PROJECT_DIR: cwd }, + env, }); let stdout = "";