From 3eb53adfc3535c9bdee2f370e8dacf3a2a288989 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 28 Feb 2026 12:00:02 +0900 Subject: [PATCH] fix(hooks): resolve cubic review issues - Replace two-pass env interpolation with single-pass combined regex to prevent re-interpolation of $-sequences in substituted header values - Convert HookEntry to discriminated union so type: "http" requires url, preventing invalid configs from passing type checking - Add regression test for double-interpolation edge case --- src/features/claude-code-plugin-loader/types.ts | 15 +++++---------- .../claude-code-hooks/execute-http-hook.test.ts | 9 +++++++++ src/hooks/claude-code-hooks/execute-http-hook.ts | 13 ++----------- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/features/claude-code-plugin-loader/types.ts b/src/features/claude-code-plugin-loader/types.ts index ff511fd5..f384f4ef 100644 --- a/src/features/claude-code-plugin-loader/types.ts +++ b/src/features/claude-code-plugin-loader/types.ts @@ -80,16 +80,11 @@ export interface PluginManifest { /** * Hooks configuration */ -export interface HookEntry { - type: "command" | "prompt" | "agent" | "http" - command?: string - prompt?: string - agent?: string - url?: string - headers?: Record - allowedEnvVars?: string[] - timeout?: number -} +export type HookEntry = + | { type: "command"; command?: string } + | { type: "prompt"; prompt?: string } + | { type: "agent"; agent?: string } + | { type: "http"; url: string; headers?: Record; allowedEnvVars?: string[]; timeout?: number } export interface HookMatcher { matcher?: string diff --git a/src/hooks/claude-code-hooks/execute-http-hook.test.ts b/src/hooks/claude-code-hooks/execute-http-hook.test.ts index a51b0b50..bc7e1f59 100644 --- a/src/hooks/claude-code-hooks/execute-http-hook.test.ts +++ b/src/hooks/claude-code-hooks/execute-http-hook.test.ts @@ -227,6 +227,15 @@ describe("interpolateEnvVars", () => { expect(result).toBe("abc:") }) + it("#given ${VAR} where value contains $ANOTHER #when both allowed #then does not double-interpolate", async () => { + process.env = { ...process.env, TOKEN: "val$SECRET", SECRET: "oops" } + const { interpolateEnvVars } = await import("./execute-http-hook") + + const result = interpolateEnvVars("Bearer ${TOKEN}", ["TOKEN", "SECRET"]) + + expect(result).toBe("Bearer val$SECRET") + }) + it("#given no allowedEnvVars #when called #then replaces all with empty", async () => { const { interpolateEnvVars } = await import("./execute-http-hook") diff --git a/src/hooks/claude-code-hooks/execute-http-hook.ts b/src/hooks/claude-code-hooks/execute-http-hook.ts index 43d65bba..bd985dbc 100644 --- a/src/hooks/claude-code-hooks/execute-http-hook.ts +++ b/src/hooks/claude-code-hooks/execute-http-hook.ts @@ -9,23 +9,14 @@ export function interpolateEnvVars( ): string { const allowedSet = new Set(allowedEnvVars) - let result = value.replace(/\$\{(\w+)\}/g, (_match, varName: string) => { + return value.replace(/\$\{(\w+)\}|\$(\w+)/g, (_match, bracedVar: string | undefined, bareVar: string | undefined) => { + const varName = (bracedVar ?? bareVar) as string if (allowedSet.has(varName)) { return process.env[varName] ?? "" } return "" }) - - result = result.replace(/\$(\w+)/g, (_match, varName: string) => { - if (allowedSet.has(varName)) { - return process.env[varName] ?? "" - } - return "" - }) - - return result } - function resolveHeaders( hook: HookHttp ): Record {