From d081e8ef4f7dd6005eae148a34348204e9301977 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 3 Apr 2026 17:07:17 +0900 Subject: [PATCH 1/2] Fix HTTP hook HTTPS enforcement gaps Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../execute-http-hook-security.test.ts | 64 +++++++++++++++++-- .../claude-code-hooks/execute-http-hook.ts | 12 ++-- 2 files changed, 64 insertions(+), 12 deletions(-) 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 index dc2b4ced9..243e6944a 100644 --- a/src/hooks/claude-code-hooks/execute-http-hook-security.test.ts +++ b/src/hooks/claude-code-hooks/execute-http-hook-security.test.ts @@ -41,7 +41,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 +52,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() }) @@ -108,14 +108,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 () => { @@ -151,6 +152,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 uses manual redirect handling", 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", () => { diff --git a/src/hooks/claude-code-hooks/execute-http-hook.ts b/src/hooks/claude-code-hooks/execute-http-hook.ts index af82c04df..a50db4208 100644 --- a/src/hooks/claude-code-hooks/execute-http-hook.ts +++ b/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", "[::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 { @@ -68,10 +65,10 @@ export async function executeHttpHook( if (isPlainHttp(parsed)) { log("HTTP hook URL uses insecure protocol", { url: hook.url }) - if (isProduction() && !isLocalhost(parsed)) { + if (!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.", + stderr: "HTTP hook URL must use HTTPS. Plain HTTP is only allowed for localhost, 127.0.0.1, and ::1.", } } } @@ -84,6 +81,7 @@ export async function executeHttpHook( method: "POST", headers, body: stdin, + redirect: "manual", signal: AbortSignal.timeout(timeoutS * 1000), }) From b8f4037622b1699e34078c64d291b0b969f28f41 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 3 Apr 2026 17:30:57 +0900 Subject: [PATCH 2/2] Refine HTTP hook redirect enforcement Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../execute-http-hook-security.test.ts | 20 +++++++++++++++++-- .../claude-code-hooks/execute-http-hook.ts | 6 ++++-- 2 files changed, 22 insertions(+), 4 deletions(-) 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 index 243e6944a..c9b609896 100644 --- a/src/hooks/claude-code-hooks/execute-http-hook-security.test.ts +++ b/src/hooks/claude-code-hooks/execute-http-hook-security.test.ts @@ -1,3 +1,5 @@ +/// + import { describe, it, expect, mock, beforeEach, afterEach } from "bun:test" import type { HookHttp } from "./types" @@ -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" } @@ -139,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, })) @@ -187,7 +203,7 @@ describe("executeHttpHook TLS security", () => { process.env = { ...originalEnv, NODE_ENV: "production" } }) - it("#when hook uses https:// URL #then fetch uses manual redirect handling", async () => { + it("#when hook uses https:// URL #then fetch rejects redirects manually", async () => { mockFetch.mockImplementation(() => Promise.resolve(new Response("redirect", { status: 302, statusText: "Found" })) ) diff --git a/src/hooks/claude-code-hooks/execute-http-hook.ts b/src/hooks/claude-code-hooks/execute-http-hook.ts index a50db4208..99cdb5498 100644 --- a/src/hooks/claude-code-hooks/execute-http-hook.ts +++ b/src/hooks/claude-code-hooks/execute-http-hook.ts @@ -4,7 +4,7 @@ import { log } from "../../shared" const DEFAULT_HTTP_HOOK_TIMEOUT_S = 30 const ALLOWED_SCHEMES = new Set(["http:", "https:"]) -const LOCALHOST_HOSTNAMES = new Set(["localhost", "127.0.0.1", "::1", "[::1]"]) +const LOCALHOST_HOSTNAMES = new Set(["localhost", "127.0.0.1", "[::1]"]) function isLocalhost(url: URL): boolean { return LOCALHOST_HOSTNAMES.has(url.hostname) @@ -64,8 +64,8 @@ export async function executeHttpHook( } if (isPlainHttp(parsed)) { - log("HTTP hook URL uses insecure protocol", { url: hook.url }) if (!isLocalhost(parsed)) { + log("HTTP hook URL uses insecure protocol", { url: hook.url }) return { exitCode: 1, stderr: "HTTP hook URL must use HTTPS. Plain HTTP is only allowed for localhost, 127.0.0.1, and ::1.", @@ -81,6 +81,7 @@ 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), }) @@ -104,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: "" }