Merge pull request #4180 from JacobZyy/fix/plugin-hooks-merge

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 {}

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<typeof setTimeout> | 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<string, string | undefined>;
+    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 = "";