fix: add PATH to restricted hook env, protect HOME/CLAUDE_PROJECT_DIR from allowlist override, reset plugin hooks state in tests
- P1: When allowedEnvVars is provided, PATH was missing from the base restricted env, causing non-builtin commands to fail at exec time - P2: Allowlisted HOME/CLAUDE_PROJECT_DIR could overwrite normalized values from getHomeDirectory()/cwd with ambient process.env values - P2: Test suite mutated shared pluginHooksState singleton without resetting it in afterEach, causing cross-test state leaks
This commit is contained in:
@@ -94,11 +94,12 @@ function getStopCommands(config: Awaited<ReturnType<typeof loadClaudeHooksConfig
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe("mergePluginHooksConfigs", () => {
|
describe("mergePluginHooksConfigs", () => {
|
||||||
const { mergePluginHooksConfigs, setPluginHooksConfigs, clearClaudeHooksConfigCache: _clearCache } = require("./config")
|
const { mergePluginHooksConfigs, setPluginHooksConfigs, clearClaudeHooksConfigCache: _clearCache, resetPluginHooksState } = require("./config")
|
||||||
const { setAdditionalAllowedMcpEnvVars, resetAdditionalAllowedMcpEnvVars } = require("../../features/claude-code-mcp-loader/configure-allowed-env-vars")
|
const { setAdditionalAllowedMcpEnvVars, resetAdditionalAllowedMcpEnvVars } = require("../../features/claude-code-mcp-loader/configure-allowed-env-vars")
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
resetAdditionalAllowedMcpEnvVars()
|
resetAdditionalAllowedMcpEnvVars()
|
||||||
|
resetPluginHooksState()
|
||||||
})
|
})
|
||||||
|
|
||||||
test("#given empty plugin hooks #when merged #then returns base unchanged", () => {
|
test("#given empty plugin hooks #when merged #then returns base unchanged", () => {
|
||||||
@@ -246,7 +247,7 @@ describe("mergePluginHooksConfigs", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("setPluginHooksConfigs", () => {
|
describe("setPluginHooksConfigs", () => {
|
||||||
const { setPluginHooksConfigs, loadClaudeHooksConfig, clearClaudeHooksConfigCache: _clearCache } = require("./config")
|
const { setPluginHooksConfigs, loadClaudeHooksConfig, clearClaudeHooksConfigCache: _clearCache, resetPluginHooksState } = require("./config")
|
||||||
const { resetAdditionalAllowedMcpEnvVars } = require("../../features/claude-code-mcp-loader/configure-allowed-env-vars")
|
const { resetAdditionalAllowedMcpEnvVars } = require("../../features/claude-code-mcp-loader/configure-allowed-env-vars")
|
||||||
let originalWorkingDirectory = ""
|
let originalWorkingDirectory = ""
|
||||||
|
|
||||||
@@ -258,6 +259,7 @@ describe("setPluginHooksConfigs", () => {
|
|||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
_clearCache()
|
_clearCache()
|
||||||
resetAdditionalAllowedMcpEnvVars()
|
resetAdditionalAllowedMcpEnvVars()
|
||||||
|
resetPluginHooksState()
|
||||||
process.chdir(originalWorkingDirectory)
|
process.chdir(originalWorkingDirectory)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -108,6 +108,10 @@ export function clearClaudeHooksConfigCache(): void {
|
|||||||
configCache.clear()
|
configCache.clear()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function resetPluginHooksState(): void {
|
||||||
|
pluginHooksState.clear()
|
||||||
|
}
|
||||||
|
|
||||||
function mergeHooksConfig(
|
function mergeHooksConfig(
|
||||||
base: ClaudeHooksConfig,
|
base: ClaudeHooksConfig,
|
||||||
override: ClaudeHooksConfig
|
override: ClaudeHooksConfig
|
||||||
|
|||||||
@@ -56,12 +56,20 @@ export async function executeHookCommand(
|
|||||||
|
|
||||||
const isWin32 = process.platform === "win32";
|
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>;
|
let env: Record<string, string | undefined>;
|
||||||
if (options?.allowedEnvVars) {
|
if (options?.allowedEnvVars) {
|
||||||
const allowedSet = new Set(options.allowedEnvVars);
|
const allowedSet = new Set(options.allowedEnvVars);
|
||||||
env = { HOME: home, CLAUDE_PROJECT_DIR: cwd };
|
env = {
|
||||||
|
HOME: home,
|
||||||
|
CLAUDE_PROJECT_DIR: cwd,
|
||||||
|
PATH: process.env.PATH,
|
||||||
|
};
|
||||||
for (const key of Object.keys(process.env)) {
|
for (const key of Object.keys(process.env)) {
|
||||||
if (allowedSet.has(key)) {
|
if (allowedSet.has(key) && !PROTECTED_ENV_KEYS.has(key)) {
|
||||||
env[key] = process.env[key];
|
env[key] = process.env[key];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user