Refine HTTP hook redirect enforcement
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -1,3 +1,5 @@
|
|||||||
|
/// <reference types="bun-types" />
|
||||||
|
|
||||||
import { describe, it, expect, mock, beforeEach, afterEach } from "bun:test"
|
import { describe, it, expect, mock, beforeEach, afterEach } from "bun:test"
|
||||||
import type { HookHttp } from "./types"
|
import type { HookHttp } from "./types"
|
||||||
|
|
||||||
@@ -82,6 +84,20 @@ describe("executeHttpHook TLS security", () => {
|
|||||||
expect(mockFetch).toHaveBeenCalledTimes(1)
|
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 () => {
|
it("#when hook uses http://127.0.0.1 #then allows execution", async () => {
|
||||||
const { executeHttpHook } = await import("./execute-http-hook")
|
const { executeHttpHook } = await import("./execute-http-hook")
|
||||||
const hook: HookHttp = { type: "http", url: "http://127.0.0.1:8080/hooks" }
|
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)
|
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", () => ({
|
mock.module("../../shared", () => ({
|
||||||
log: mockLog,
|
log: mockLog,
|
||||||
}))
|
}))
|
||||||
@@ -187,7 +203,7 @@ describe("executeHttpHook TLS security", () => {
|
|||||||
process.env = { ...originalEnv, NODE_ENV: "production" }
|
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(() =>
|
mockFetch.mockImplementation(() =>
|
||||||
Promise.resolve(new Response("redirect", { status: 302, statusText: "Found" }))
|
Promise.resolve(new Response("redirect", { status: 302, statusText: "Found" }))
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ import { log } from "../../shared"
|
|||||||
|
|
||||||
const DEFAULT_HTTP_HOOK_TIMEOUT_S = 30
|
const DEFAULT_HTTP_HOOK_TIMEOUT_S = 30
|
||||||
const ALLOWED_SCHEMES = new Set(["http:", "https:"])
|
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 {
|
function isLocalhost(url: URL): boolean {
|
||||||
return LOCALHOST_HOSTNAMES.has(url.hostname)
|
return LOCALHOST_HOSTNAMES.has(url.hostname)
|
||||||
@@ -64,8 +64,8 @@ export async function executeHttpHook(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (isPlainHttp(parsed)) {
|
if (isPlainHttp(parsed)) {
|
||||||
log("HTTP hook URL uses insecure protocol", { url: hook.url })
|
|
||||||
if (!isLocalhost(parsed)) {
|
if (!isLocalhost(parsed)) {
|
||||||
|
log("HTTP hook URL uses insecure protocol", { url: hook.url })
|
||||||
return {
|
return {
|
||||||
exitCode: 1,
|
exitCode: 1,
|
||||||
stderr: "HTTP hook URL must use HTTPS. Plain HTTP is only allowed for localhost, 127.0.0.1, and ::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",
|
method: "POST",
|
||||||
headers,
|
headers,
|
||||||
body: stdin,
|
body: stdin,
|
||||||
|
// Reject all redirects so HTTPS hooks cannot be silently rewritten to a different origin or protocol.
|
||||||
redirect: "manual",
|
redirect: "manual",
|
||||||
signal: AbortSignal.timeout(timeoutS * 1000),
|
signal: AbortSignal.timeout(timeoutS * 1000),
|
||||||
})
|
})
|
||||||
@@ -104,6 +105,7 @@ export async function executeHttpHook(
|
|||||||
return { exitCode: parsed.exitCode, stdout: body, stderr: "" }
|
return { exitCode: parsed.exitCode, stdout: body, stderr: "" }
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
|
// Non-JSON bodies are allowed and returned as stdout below.
|
||||||
}
|
}
|
||||||
|
|
||||||
return { exitCode: 0, stdout: body, stderr: "" }
|
return { exitCode: 0, stdout: body, stderr: "" }
|
||||||
|
|||||||
Reference in New Issue
Block a user