Merge pull request #3051 from code-yeongyu/fix/p0-2-https-enforcement-gaps

fix(security): enforce HTTPS for all non-loopback remote hooks

src/hooks/claude-code-hooks/execute-http-hook-security.test.ts

@@ -1,3 +1,5 @@
+/// <reference types="bun-types" />
+
 import { describe, it, expect, mock, beforeEach, afterEach } from "bun:test"
 import type { HookHttp } from "./types"
 
@@ -41,7 +43,7 @@ describe("executeHttpHook TLS security", () => {
       const result = await executeHttpHook(hook, "{}")
 
       expect(result.exitCode).toBe(1)
-      expect(result.stderr).toContain("HTTP hook URL must use HTTPS in production")
+      expect(result.stderr).toContain("HTTP hook URL must use HTTPS")
       expect(mockFetch).not.toHaveBeenCalled()
     })
 
@@ -52,7 +54,7 @@ describe("executeHttpHook TLS security", () => {
       const result = await executeHttpHook(hook, "{}")
 
       expect(result.exitCode).toBe(1)
-      expect(result.stderr).toContain("HTTP hook URL must use HTTPS in production")
+      expect(result.stderr).toContain("HTTP hook URL must use HTTPS")
       expect(mockFetch).not.toHaveBeenCalled()
     })
 
@@ -82,6 +84,20 @@ describe("executeHttpHook TLS security", () => {
       expect(mockFetch).toHaveBeenCalledTimes(1)
     })
 
+    it("#when hook uses http://localhost #then does not log insecure warning", async () => {
+      mock.module("../../shared", () => ({
+        log: mockLog,
+      }))
+      mockLog.mockReset()
+      const { executeHttpHook } = await importFreshExecuteHttpHook()
+      const hook: HookHttp = { type: "http", url: "http://localhost:8080/hooks" }
+
+      const result = await executeHttpHook(hook, "{}")
+
+      expect(result.exitCode).toBe(0)
+      expect(mockLog).not.toHaveBeenCalled()
+    })
+
     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" }
@@ -108,14 +124,15 @@ describe("executeHttpHook TLS security", () => {
       process.env = { ...originalEnv, NODE_ENV: "development" }
     })
 
-    it("#when hook uses remote http:// URL #then allows execution", async () => {
+    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(0)
-      expect(mockFetch).toHaveBeenCalledTimes(1)
+      expect(result.exitCode).toBe(1)
+      expect(result.stderr).toContain("HTTP hook URL must use HTTPS")
+      expect(mockFetch).not.toHaveBeenCalled()
     })
 
     it("#when hook uses http://localhost #then allows execution", async () => {
@@ -138,7 +155,7 @@ describe("executeHttpHook TLS security", () => {
       expect(mockFetch).toHaveBeenCalledTimes(1)
     })
 
-    it("#when hook uses plain http:// URL #then writes warning log", async () => {
+    it("#when hook uses plain remote http:// URL #then writes warning log", async () => {
       mock.module("../../shared", () => ({
         log: mockLog,
       }))
@@ -151,6 +168,59 @@ describe("executeHttpHook TLS security", () => {
         url: "http://example.com/hooks",
       })
     })
+
+    it("#when hook uses http://[::1] #then allows execution", async () => {
+      const { executeHttpHook } = await import("./execute-http-hook")
+      const hook: HookHttp = { type: "http", url: "http://[::1]:8080/hooks" }
+
+      const result = await executeHttpHook(hook, "{}")
+
+      expect(result.exitCode).toBe(0)
+      expect(mockFetch).toHaveBeenCalledTimes(1)
+    })
+  })
+
+  describe("#given NODE_ENV is unset", () => {
+    beforeEach(() => {
+      process.env = { ...originalEnv }
+      delete process.env.NODE_ENV
+    })
+
+    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")
+      expect(mockFetch).not.toHaveBeenCalled()
+    })
+  })
+
+  describe("#given redirect downgrade protection", () => {
+    beforeEach(() => {
+      process.env = { ...originalEnv, NODE_ENV: "production" }
+    })
+
+    it("#when hook uses https:// URL #then fetch rejects redirects manually", async () => {
+      mockFetch.mockImplementation(() =>
+        Promise.resolve(new Response("redirect", { status: 302, statusText: "Found" }))
+      )
+      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(1)
+      expect(result.stderr).toContain("HTTP hook returned status 302")
+      expect(mockFetch).toHaveBeenCalledWith(
+        "https://example.com/hooks",
+        expect.objectContaining({
+          redirect: "manual",
+        })
+      )
+    })
   })
 
   describe("#given invalid URL handling is preserved", () => {

src/hooks/claude-code-hooks/execute-http-hook.ts

@@ -4,13 +4,10 @@ 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"
-}
+const LOCALHOST_HOSTNAMES = new Set(["localhost", "127.0.0.1", "[::1]"])
 
 function isLocalhost(url: URL): boolean {
-  return url.hostname === "localhost" || url.hostname === "127.0.0.1"
+  return LOCALHOST_HOSTNAMES.has(url.hostname)
 }
 
 function isPlainHttp(url: URL): boolean {
@@ -67,11 +64,11 @@ export async function executeHttpHook(
   }
 
   if (isPlainHttp(parsed)) {
-    log("HTTP hook URL uses insecure protocol", { url: hook.url })
-    if (isProduction() && !isLocalhost(parsed)) {
+    if (!isLocalhost(parsed)) {
+      log("HTTP hook URL uses insecure protocol", { url: hook.url })
       return {
         exitCode: 1,
-        stderr: "HTTP hook URL must use HTTPS in production. Plain HTTP is only allowed for localhost/127.0.0.1.",
+        stderr: "HTTP hook URL must use HTTPS. Plain HTTP is only allowed for localhost, 127.0.0.1, and ::1.",
       }
     }
   }
@@ -84,6 +81,8 @@ export async function executeHttpHook(
       method: "POST",
       headers,
       body: stdin,
+      // Reject all redirects so HTTPS hooks cannot be silently rewritten to a different origin or protocol.
+      redirect: "manual",
       signal: AbortSignal.timeout(timeoutS * 1000),
     })
 
@@ -106,6 +105,7 @@ export async function executeHttpHook(
         return { exitCode: parsed.exitCode, stdout: body, stderr: "" }
       }
     } catch {
+      // Non-JSON bodies are allowed and returned as stdout below.
     }
 
     return { exitCode: 0, stdout: body, stderr: "" }