fix(hooks): prevent SSRF via URL scheme validation and extend disable mechanism to HTTP hooks
- Restrict HTTP hook URLs to http: and https: schemes only (blocks file://, data://, ftp://) - Extend hook disable config to cover HTTP hooks by matching against hook URL identifier - Update all 5 hook executors (pre-tool-use, post-tool-use, stop, pre-compact, user-prompt-submit) - Add 6 new tests for URL scheme validation (file, data, ftp rejection + http, https, invalid URL)
This commit is contained in:
@@ -33,7 +33,7 @@ describe("executeHttpHook", () => {
|
|||||||
await executeHttpHook(hook, stdinData)
|
await executeHttpHook(hook, stdinData)
|
||||||
|
|
||||||
expect(mockFetch).toHaveBeenCalledTimes(1)
|
expect(mockFetch).toHaveBeenCalledTimes(1)
|
||||||
const [url, options] = mockFetch.mock.calls[0] as [string, RequestInit]
|
const [url, options] = mockFetch.mock.calls[0] as unknown as [string, RequestInit]
|
||||||
expect(url).toBe("http://localhost:8080/hooks/pre-tool-use")
|
expect(url).toBe("http://localhost:8080/hooks/pre-tool-use")
|
||||||
expect(options.method).toBe("POST")
|
expect(options.method).toBe("POST")
|
||||||
expect(options.body).toBe(stdinData)
|
expect(options.body).toBe(stdinData)
|
||||||
@@ -44,7 +44,7 @@ describe("executeHttpHook", () => {
|
|||||||
|
|
||||||
await executeHttpHook(hook, stdinData)
|
await executeHttpHook(hook, stdinData)
|
||||||
|
|
||||||
const [, options] = mockFetch.mock.calls[0] as [string, RequestInit]
|
const [, options] = mockFetch.mock.calls[0] as unknown as [string, RequestInit]
|
||||||
const headers = options.headers as Record<string, string>
|
const headers = options.headers as Record<string, string>
|
||||||
expect(headers["Content-Type"]).toBe("application/json")
|
expect(headers["Content-Type"]).toBe("application/json")
|
||||||
})
|
})
|
||||||
@@ -72,7 +72,7 @@ describe("executeHttpHook", () => {
|
|||||||
|
|
||||||
await executeHttpHook(hook, "{}")
|
await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
const [, options] = mockFetch.mock.calls[0] as [string, RequestInit]
|
const [, options] = mockFetch.mock.calls[0] as unknown as [string, RequestInit]
|
||||||
const headers = options.headers as Record<string, string>
|
const headers = options.headers as Record<string, string>
|
||||||
expect(headers["Authorization"]).toBe("Bearer secret-123")
|
expect(headers["Authorization"]).toBe("Bearer secret-123")
|
||||||
})
|
})
|
||||||
@@ -88,7 +88,7 @@ describe("executeHttpHook", () => {
|
|||||||
|
|
||||||
await executeHttpHook(hook, "{}")
|
await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
const [, options] = mockFetch.mock.calls[0] as [string, RequestInit]
|
const [, options] = mockFetch.mock.calls[0] as unknown as [string, RequestInit]
|
||||||
const headers = options.headers as Record<string, string>
|
const headers = options.headers as Record<string, string>
|
||||||
expect(headers["Authorization"]).toBe("Bearer secret-123")
|
expect(headers["Authorization"]).toBe("Bearer secret-123")
|
||||||
})
|
})
|
||||||
@@ -104,7 +104,7 @@ describe("executeHttpHook", () => {
|
|||||||
|
|
||||||
await executeHttpHook(hook, "{}")
|
await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
const [, options] = mockFetch.mock.calls[0] as [string, RequestInit]
|
const [, options] = mockFetch.mock.calls[0] as unknown as [string, RequestInit]
|
||||||
const headers = options.headers as Record<string, string>
|
const headers = options.headers as Record<string, string>
|
||||||
expect(headers["Authorization"]).toBe("Bearer ")
|
expect(headers["Authorization"]).toBe("Bearer ")
|
||||||
})
|
})
|
||||||
@@ -121,11 +121,77 @@ describe("executeHttpHook", () => {
|
|||||||
|
|
||||||
await executeHttpHook(hook, "{}")
|
await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
const [, options] = mockFetch.mock.calls[0] as [string, RequestInit]
|
const [, options] = mockFetch.mock.calls[0] as unknown as [string, RequestInit]
|
||||||
expect(options.signal).toBeDefined()
|
expect(options.signal).toBeDefined()
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("#given hook URL scheme validation", () => {
|
||||||
|
it("#when URL uses file:// scheme #then rejects with exit code 1", async () => {
|
||||||
|
const hook: HookHttp = { type: "http", url: "file:///etc/passwd" }
|
||||||
|
const { executeHttpHook } = await import("./execute-http-hook")
|
||||||
|
|
||||||
|
const result = await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
|
expect(result.exitCode).toBe(1)
|
||||||
|
expect(result.stderr).toContain('HTTP hook URL scheme "file:" is not allowed')
|
||||||
|
expect(mockFetch).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#when URL uses data: scheme #then rejects with exit code 1", async () => {
|
||||||
|
const hook: HookHttp = { type: "http", url: "data:text/plain,hello" }
|
||||||
|
const { executeHttpHook } = await import("./execute-http-hook")
|
||||||
|
|
||||||
|
const result = await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
|
expect(result.exitCode).toBe(1)
|
||||||
|
expect(result.stderr).toContain('HTTP hook URL scheme "data:" is not allowed')
|
||||||
|
expect(mockFetch).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#when URL uses ftp:// scheme #then rejects with exit code 1", async () => {
|
||||||
|
const hook: HookHttp = { type: "http", url: "ftp://localhost/hooks" }
|
||||||
|
const { executeHttpHook } = await import("./execute-http-hook")
|
||||||
|
|
||||||
|
const result = await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
|
expect(result.exitCode).toBe(1)
|
||||||
|
expect(result.stderr).toContain('HTTP hook URL scheme "ftp:" is not allowed')
|
||||||
|
expect(mockFetch).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#when URL uses http:// scheme #then allows hook execution", async () => {
|
||||||
|
const hook: HookHttp = { type: "http", url: "http://localhost:8080/hooks" }
|
||||||
|
const { executeHttpHook } = await import("./execute-http-hook")
|
||||||
|
|
||||||
|
const result = await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
|
expect(result.exitCode).toBe(0)
|
||||||
|
expect(mockFetch).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#when URL uses https:// scheme #then allows hook execution", async () => {
|
||||||
|
const hook: HookHttp = { type: "http", url: "https://example.com/hooks" }
|
||||||
|
const { executeHttpHook } = await import("./execute-http-hook")
|
||||||
|
|
||||||
|
const result = await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
|
expect(result.exitCode).toBe(0)
|
||||||
|
expect(mockFetch).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#when URL is invalid #then rejects with exit code 1", async () => {
|
||||||
|
const hook: HookHttp = { type: "http", url: "not-a-valid-url" }
|
||||||
|
const { executeHttpHook } = await import("./execute-http-hook")
|
||||||
|
|
||||||
|
const result = await executeHttpHook(hook, "{}")
|
||||||
|
|
||||||
|
expect(result.exitCode).toBe(1)
|
||||||
|
expect(result.stderr).toContain("HTTP hook URL is invalid: not-a-valid-url")
|
||||||
|
expect(mockFetch).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("#given a successful HTTP response", () => {
|
describe("#given a successful HTTP response", () => {
|
||||||
it("#when response has JSON body #then returns parsed output", async () => {
|
it("#when response has JSON body #then returns parsed output", async () => {
|
||||||
mockFetch.mockImplementation(() =>
|
mockFetch.mockImplementation(() =>
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import type { HookHttp } from "./types"
|
|||||||
import type { CommandResult } from "../../shared/command-executor/execute-hook-command"
|
import type { CommandResult } from "../../shared/command-executor/execute-hook-command"
|
||||||
|
|
||||||
const DEFAULT_HTTP_HOOK_TIMEOUT_S = 30
|
const DEFAULT_HTTP_HOOK_TIMEOUT_S = 30
|
||||||
|
const ALLOWED_SCHEMES = new Set(["http:", "https:"])
|
||||||
|
|
||||||
export function interpolateEnvVars(
|
export function interpolateEnvVars(
|
||||||
value: string,
|
value: string,
|
||||||
@@ -39,6 +40,18 @@ export async function executeHttpHook(
|
|||||||
hook: HookHttp,
|
hook: HookHttp,
|
||||||
stdin: string
|
stdin: string
|
||||||
): Promise<CommandResult> {
|
): Promise<CommandResult> {
|
||||||
|
try {
|
||||||
|
const parsed = new URL(hook.url)
|
||||||
|
if (!ALLOWED_SCHEMES.has(parsed.protocol)) {
|
||||||
|
return {
|
||||||
|
exitCode: 1,
|
||||||
|
stderr: `HTTP hook URL scheme "${parsed.protocol}" is not allowed. Only http: and https: are permitted.`,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
return { exitCode: 1, stderr: `HTTP hook URL is invalid: ${hook.url}` }
|
||||||
|
}
|
||||||
|
|
||||||
const timeoutS = hook.timeout ?? DEFAULT_HTTP_HOOK_TIMEOUT_S
|
const timeoutS = hook.timeout ?? DEFAULT_HTTP_HOOK_TIMEOUT_S
|
||||||
const headers = resolveHeaders(hook)
|
const headers = resolveHeaders(hook)
|
||||||
|
|
||||||
|
|||||||
@@ -96,12 +96,12 @@ export async function executePostToolUseHooks(
|
|||||||
for (const hook of matcher.hooks) {
|
for (const hook of matcher.hooks) {
|
||||||
if (hook.type !== "command" && hook.type !== "http") continue
|
if (hook.type !== "command" && hook.type !== "http") continue
|
||||||
|
|
||||||
if (hook.type === "command" && isHookCommandDisabled("PostToolUse", hook.command, extendedConfig ?? null)) {
|
const hookName = getHookIdentifier(hook)
|
||||||
log("PostToolUse hook command skipped (disabled by config)", { command: hook.command, toolName: ctx.toolName })
|
if (isHookCommandDisabled("PostToolUse", hookName, extendedConfig ?? null)) {
|
||||||
|
log("PostToolUse hook command skipped (disabled by config)", { command: hookName, toolName: ctx.toolName })
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
const hookName = getHookIdentifier(hook)
|
|
||||||
if (!firstHookName) firstHookName = hookName
|
if (!firstHookName) firstHookName = hookName
|
||||||
|
|
||||||
const result = await dispatchHook(hook, JSON.stringify(stdinData), ctx.cwd)
|
const result = await dispatchHook(hook, JSON.stringify(stdinData), ctx.cwd)
|
||||||
|
|||||||
@@ -52,12 +52,12 @@ export async function executePreCompactHooks(
|
|||||||
for (const hook of matcher.hooks) {
|
for (const hook of matcher.hooks) {
|
||||||
if (hook.type !== "command" && hook.type !== "http") continue
|
if (hook.type !== "command" && hook.type !== "http") continue
|
||||||
|
|
||||||
if (hook.type === "command" && isHookCommandDisabled("PreCompact", hook.command, extendedConfig ?? null)) {
|
const hookName = getHookIdentifier(hook)
|
||||||
log("PreCompact hook command skipped (disabled by config)", { command: hook.command })
|
if (isHookCommandDisabled("PreCompact", hookName, extendedConfig ?? null)) {
|
||||||
|
log("PreCompact hook command skipped (disabled by config)", { command: hookName })
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
const hookName = getHookIdentifier(hook)
|
|
||||||
if (!firstHookName) firstHookName = hookName
|
if (!firstHookName) firstHookName = hookName
|
||||||
|
|
||||||
const result = await dispatchHook(hook, JSON.stringify(stdinData), ctx.cwd)
|
const result = await dispatchHook(hook, JSON.stringify(stdinData), ctx.cwd)
|
||||||
|
|||||||
@@ -79,12 +79,12 @@ export async function executePreToolUseHooks(
|
|||||||
for (const hook of matcher.hooks) {
|
for (const hook of matcher.hooks) {
|
||||||
if (hook.type !== "command" && hook.type !== "http") continue
|
if (hook.type !== "command" && hook.type !== "http") continue
|
||||||
|
|
||||||
if (hook.type === "command" && isHookCommandDisabled("PreToolUse", hook.command, extendedConfig ?? null)) {
|
const hookName = getHookIdentifier(hook)
|
||||||
log("PreToolUse hook command skipped (disabled by config)", { command: hook.command, toolName: ctx.toolName })
|
if (isHookCommandDisabled("PreToolUse", hookName, extendedConfig ?? null)) {
|
||||||
|
log("PreToolUse hook command skipped (disabled by config)", { command: hookName, toolName: ctx.toolName })
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
const hookName = getHookIdentifier(hook)
|
|
||||||
if (!firstHookName) firstHookName = hookName
|
if (!firstHookName) firstHookName = hookName
|
||||||
|
|
||||||
const result = await dispatchHook(hook, JSON.stringify(stdinData), ctx.cwd)
|
const result = await dispatchHook(hook, JSON.stringify(stdinData), ctx.cwd)
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ import type {
|
|||||||
ClaudeHooksConfig,
|
ClaudeHooksConfig,
|
||||||
} from "./types"
|
} from "./types"
|
||||||
import { findMatchingHooks, log } from "../../shared"
|
import { findMatchingHooks, log } from "../../shared"
|
||||||
import { dispatchHook } from "./dispatch-hook"
|
import { dispatchHook, getHookIdentifier } from "./dispatch-hook"
|
||||||
import { getTodoPath } from "./todo"
|
import { getTodoPath } from "./todo"
|
||||||
import { isHookCommandDisabled, type PluginExtendedConfig } from "./config-loader"
|
import { isHookCommandDisabled, type PluginExtendedConfig } from "./config-loader"
|
||||||
|
|
||||||
@@ -70,8 +70,9 @@ export async function executeStopHooks(
|
|||||||
for (const hook of matcher.hooks) {
|
for (const hook of matcher.hooks) {
|
||||||
if (hook.type !== "command" && hook.type !== "http") continue
|
if (hook.type !== "command" && hook.type !== "http") continue
|
||||||
|
|
||||||
if (hook.type === "command" && isHookCommandDisabled("Stop", hook.command, extendedConfig ?? null)) {
|
const hookName = getHookIdentifier(hook)
|
||||||
log("Stop hook command skipped (disabled by config)", { command: hook.command })
|
if (isHookCommandDisabled("Stop", hookName, extendedConfig ?? null)) {
|
||||||
|
log("Stop hook command skipped (disabled by config)", { command: hookName })
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ import type {
|
|||||||
ClaudeHooksConfig,
|
ClaudeHooksConfig,
|
||||||
} from "./types"
|
} from "./types"
|
||||||
import { findMatchingHooks, log } from "../../shared"
|
import { findMatchingHooks, log } from "../../shared"
|
||||||
import { dispatchHook } from "./dispatch-hook"
|
import { dispatchHook, getHookIdentifier } from "./dispatch-hook"
|
||||||
import { isHookCommandDisabled, type PluginExtendedConfig } from "./config-loader"
|
import { isHookCommandDisabled, type PluginExtendedConfig } from "./config-loader"
|
||||||
|
|
||||||
const USER_PROMPT_SUBMIT_TAG_OPEN = "<user-prompt-submit-hook>"
|
const USER_PROMPT_SUBMIT_TAG_OPEN = "<user-prompt-submit-hook>"
|
||||||
@@ -82,8 +82,9 @@ export async function executeUserPromptSubmitHooks(
|
|||||||
for (const hook of matcher.hooks) {
|
for (const hook of matcher.hooks) {
|
||||||
if (hook.type !== "command" && hook.type !== "http") continue
|
if (hook.type !== "command" && hook.type !== "http") continue
|
||||||
|
|
||||||
if (hook.type === "command" && isHookCommandDisabled("UserPromptSubmit", hook.command, extendedConfig ?? null)) {
|
const hookName = getHookIdentifier(hook)
|
||||||
log("UserPromptSubmit hook command skipped (disabled by config)", { command: hook.command })
|
if (isHookCommandDisabled("UserPromptSubmit", hookName, extendedConfig ?? null)) {
|
||||||
|
log("UserPromptSubmit hook command skipped (disabled by config)", { command: hookName })
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user