diff --git a/src/hooks/claude-code-hooks/execute-http-hook-security.test.ts b/src/hooks/claude-code-hooks/execute-http-hook-security.test.ts new file mode 100644 index 000000000..dc2b4ced9 --- /dev/null +++ b/src/hooks/claude-code-hooks/execute-http-hook-security.test.ts @@ -0,0 +1,179 @@ +import { describe, it, expect, mock, beforeEach, afterEach } from "bun:test" +import type { HookHttp } from "./types" + +const mockFetch = mock(() => + Promise.resolve(new Response(JSON.stringify({}), { status: 200 })) +) +const mockLog = mock(() => {}) + +const originalFetch = globalThis.fetch +const originalEnv = process.env + +async function importFreshExecuteHttpHook() { + const modulePath = `${new URL("./execute-http-hook.ts", import.meta.url).pathname}?t=${Date.now()}-${Math.random()}` + return import(modulePath) +} + +describe("executeHttpHook TLS security", () => { + beforeEach(() => { + globalThis.fetch = mockFetch as unknown as typeof fetch + mockFetch.mockReset() + mockFetch.mockImplementation(() => + Promise.resolve(new Response(JSON.stringify({}), { status: 200 })) + ) + }) + + afterEach(() => { + globalThis.fetch = originalFetch + process.env = originalEnv + mock.restore() + }) + + describe("#given production mode", () => { + beforeEach(() => { + process.env = { ...originalEnv, NODE_ENV: "production" } + }) + + it("#when hook uses remote http:// URL #then rejects with exit code 1", async () => { + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "http://example.com/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(1) + expect(result.stderr).toContain("HTTP hook URL must use HTTPS in production") + expect(mockFetch).not.toHaveBeenCalled() + }) + + it("#when hook uses remote HTTP:// URL #then rejects with exit code 1", async () => { + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "HTTP://example.com/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(1) + expect(result.stderr).toContain("HTTP hook URL must use HTTPS in production") + expect(mockFetch).not.toHaveBeenCalled() + }) + + it("#when hook uses remote http:// URL #then logs warning before rejection", async () => { + mock.module("../../shared", () => ({ + log: mockLog, + })) + const { executeHttpHook } = await importFreshExecuteHttpHook() + const hook: HookHttp = { type: "http", url: "http://example.com/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(1) + expect(mockLog).toHaveBeenCalledWith("HTTP hook URL uses insecure protocol", { + url: "http://example.com/hooks", + }) + expect(mockFetch).not.toHaveBeenCalled() + }) + + it("#when hook uses http://localhost #then allows execution", async () => { + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "http://localhost:8080/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(0) + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + + it("#when hook uses http://127.0.0.1 #then allows execution", async () => { + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "http://127.0.0.1:8080/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(0) + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + + it("#when hook uses https:// #then allows execution", async () => { + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "https://example.com/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(0) + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + }) + + describe("#given non-production mode", () => { + beforeEach(() => { + process.env = { ...originalEnv, NODE_ENV: "development" } + }) + + it("#when hook uses remote http:// URL #then allows execution", async () => { + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "http://example.com/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(0) + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + + it("#when hook uses http://localhost #then allows execution", async () => { + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "http://localhost:8080/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(0) + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + + it("#when hook uses https:// #then allows execution", async () => { + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "https://example.com/hooks" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(0) + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + + it("#when hook uses plain http:// URL #then writes warning log", async () => { + mock.module("../../shared", () => ({ + log: mockLog, + })) + const { executeHttpHook } = await importFreshExecuteHttpHook() + const hook: HookHttp = { type: "http", url: "http://example.com/hooks" } + + await executeHttpHook(hook, "{}") + + expect(mockLog).toHaveBeenCalledWith("HTTP hook URL uses insecure protocol", { + url: "http://example.com/hooks", + }) + }) + }) + + describe("#given invalid URL handling is preserved", () => { + it("#when URL is invalid #then rejects with exit code 1", async () => { + process.env = { ...originalEnv, NODE_ENV: "production" } + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "not-a-valid-url" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(1) + expect(result.stderr).toContain("HTTP hook URL is invalid") + }) + + it("#when URL uses disallowed scheme #then rejects with exit code 1", async () => { + process.env = { ...originalEnv, NODE_ENV: "production" } + const { executeHttpHook } = await import("./execute-http-hook") + const hook: HookHttp = { type: "http", url: "file:///etc/passwd" } + + const result = await executeHttpHook(hook, "{}") + + expect(result.exitCode).toBe(1) + expect(result.stderr).toContain('HTTP hook URL scheme "file:" is not allowed') + }) + }) +}) diff --git a/src/hooks/claude-code-hooks/execute-http-hook.ts b/src/hooks/claude-code-hooks/execute-http-hook.ts index 1e72817cf..af82c04df 100644 --- a/src/hooks/claude-code-hooks/execute-http-hook.ts +++ b/src/hooks/claude-code-hooks/execute-http-hook.ts @@ -1,9 +1,22 @@ import type { HookHttp } from "./types" import type { CommandResult } from "../../shared/command-executor/execute-hook-command" +import { log } from "../../shared" const DEFAULT_HTTP_HOOK_TIMEOUT_S = 30 const ALLOWED_SCHEMES = new Set(["http:", "https:"]) +function isProduction(): boolean { + return process.env.NODE_ENV === "production" +} + +function isLocalhost(url: URL): boolean { + return url.hostname === "localhost" || url.hostname === "127.0.0.1" +} + +function isPlainHttp(url: URL): boolean { + return url.protocol === "http:" +} + export function interpolateEnvVars( value: string, allowedEnvVars: string[] @@ -40,8 +53,9 @@ export async function executeHttpHook( hook: HookHttp, stdin: string ): Promise { + let parsed: URL try { - const parsed = new URL(hook.url) + parsed = new URL(hook.url) if (!ALLOWED_SCHEMES.has(parsed.protocol)) { return { exitCode: 1, @@ -52,6 +66,16 @@ export async function executeHttpHook( return { exitCode: 1, stderr: `HTTP hook URL is invalid: ${hook.url}` } } + if (isPlainHttp(parsed)) { + log("HTTP hook URL uses insecure protocol", { url: hook.url }) + if (isProduction() && !isLocalhost(parsed)) { + return { + exitCode: 1, + stderr: "HTTP hook URL must use HTTPS in production. Plain HTTP is only allowed for localhost/127.0.0.1.", + } + } + } + const timeoutS = hook.timeout ?? DEFAULT_HTTP_HOOK_TIMEOUT_S const headers = resolveHeaders(hook)