Merge pull request #3025 from code-yeongyu/fix/security-http-hooks

fix(security): enforce HTTPS for HTTP hook URLs

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')
+    })
+  })
+})

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<CommandResult> {
+  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)