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
This commit is contained in:
YeonGyu-Kim
2026-02-28 12:00:02 +09:00
parent 43dfdb2380
commit 3eb53adfc3
3 changed files with 16 additions and 21 deletions

View File

@@ -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<string, string>
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<string, string>; allowedEnvVars?: string[]; timeout?: number }
export interface HookMatcher {
matcher?: string

View File

@@ -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")

View File

@@ -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<string, string> {