fix(security): enforce HTTPS for HTTP hook URLs
Add TLS requirement for HTTP hook destinations: - Warn when plain http:// URLs are used - Reject remote http:// in production mode - Allow http://localhost and http://127.0.0.1 for dev Prevents secret exfiltration over unencrypted channels.
This commit is contained in:
@@ -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')
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user