From 17952dabd2afa3bb0b8b070bfb4686585a5f2635 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Tue, 19 May 2026 00:03:40 -0400 Subject: [PATCH 1/2] fix(session): harden daemon HTTP requests --- CHANGELOG.md | 2 + packages/session-broker/src/daemon.test.ts | 24 +++++ packages/session-broker/src/daemon.ts | 14 +++ src/session-broker/brokerServer.test.ts | 111 ++++++++++++++++++- src/session-broker/brokerServer.ts | 118 +++++++++++++++++++++ 5 files changed, 268 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9972f60e..af706277 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All notable user-visible changes to Hunk are documented in this file. ### Fixed +- Hardened the local session daemon against browser-originated requests by validating Host and Origin headers and requiring JSON content types for API posts. + ## [0.13.0] - 2026-05-18 ### Added diff --git a/packages/session-broker/src/daemon.test.ts b/packages/session-broker/src/daemon.test.ts index 056118b4..44f33643 100644 --- a/packages/session-broker/src/daemon.test.ts +++ b/packages/session-broker/src/daemon.test.ts @@ -123,6 +123,7 @@ describe("session broker daemon", () => { const listResponse = await daemon.handleRequest( new Request("http://broker.test/broker", { method: "POST", + headers: { "content-type": "application/json" }, body: JSON.stringify({ action: "list" }), }), ); @@ -134,6 +135,7 @@ describe("session broker daemon", () => { const getResponse = await daemon.handleRequest( new Request("http://broker.test/broker", { method: "POST", + headers: { "content-type": "application/json" }, body: JSON.stringify({ action: "get", selector: { sessionId: "session-1" } }), }), ); @@ -147,6 +149,27 @@ describe("session broker daemon", () => { daemon.shutdown(); }); + test("requires JSON content type for raw broker API posts", async () => { + const daemon = createSessionBrokerDaemon({ + broker: createBroker(), + capabilities: { version: 1 }, + }); + + const response = await daemon.handleRequest( + new Request("http://broker.test/broker", { + method: "POST", + headers: { "content-type": "text/plain" }, + body: JSON.stringify({ action: "list" }), + }), + ); + + expect(response?.status).toBe(415); + await expect(response?.json()).resolves.toEqual({ + error: "Expected Content-Type application/json.", + }); + daemon.shutdown(); + }); + test("dispatches one raw command through the broker API", async () => { const daemon = createSessionBrokerDaemon({ broker: createBroker(), @@ -166,6 +189,7 @@ describe("session broker daemon", () => { const pendingResponse = daemon.handleRequest( new Request("http://broker.test/broker", { method: "POST", + headers: { "content-type": "application/json" }, body: JSON.stringify({ action: "dispatch", selector: { sessionId: "session-1" }, diff --git a/packages/session-broker/src/daemon.ts b/packages/session-broker/src/daemon.ts index cbea9a9a..d5ba086b 100644 --- a/packages/session-broker/src/daemon.ts +++ b/packages/session-broker/src/daemon.ts @@ -53,10 +53,20 @@ function parseSocketEnvelope(message: string) { : null; } +/** Return whether one raw broker API request body was explicitly sent as JSON. */ +function hasJsonContentType(request: Request) { + const contentType = request.headers.get("content-type"); + return contentType?.split(";", 1)[0]?.trim().toLowerCase() === "application/json"; +} + /** Decode one raw broker API request body and surface a friendly transport-level error. */ async function parseJsonRequest( request: Request, ) { + if (!hasJsonContentType(request)) { + throw new Error("Expected Content-Type application/json."); + } + try { return (await request.json()) as SessionBrokerDaemonRequest; } catch { @@ -322,6 +332,10 @@ export class SessionBrokerDaemon< return jsonError("Broker API requests must use POST.", 405); } + if (!hasJsonContentType(request)) { + return jsonError("Expected Content-Type application/json.", 415); + } + try { const input = await parseJsonRequest(request); let response: SessionBrokerDaemonResponse; diff --git a/src/session-broker/brokerServer.test.ts b/src/session-broker/brokerServer.test.ts index b35fc929..25b335cd 100644 --- a/src/session-broker/brokerServer.test.ts +++ b/src/session-broker/brokerServer.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { createServer } from "node:net"; +import { randomBytes } from "node:crypto"; +import { connect, createServer } from "node:net"; import { platform } from "node:os"; import { createTestSessionRegistration, @@ -116,6 +117,47 @@ async function openSessionSocket(port: number) { return socket; } +async function readRawWebSocketHandshake(port: number, headers: string[]) { + return new Promise((resolve, reject) => { + const socket = connect({ host: "127.0.0.1", port }, () => { + const key = randomBytes(16).toString("base64"); + socket.write( + [ + "GET /session HTTP/1.1", + `Host: 127.0.0.1:${port}`, + "Upgrade: websocket", + "Connection: Upgrade", + `Sec-WebSocket-Key: ${key}`, + "Sec-WebSocket-Version: 13", + ...headers, + "", + "", + ].join("\r\n"), + ); + }); + let response = ""; + const timeout = setTimeout(() => { + socket.destroy(); + reject(new Error("Timed out waiting for raw websocket handshake response.")); + }, 1_000); + + socket.on("data", (chunk) => { + response += chunk.toString("utf8"); + if (!response.includes("\r\n\r\n")) { + return; + } + + clearTimeout(timeout); + socket.destroy(); + resolve(response); + }); + socket.on("error", (error) => { + clearTimeout(timeout); + reject(error); + }); + }); +} + async function openRegisteredSession( port: number, sessionId = "session-1", @@ -252,6 +294,73 @@ describe("Hunk session daemon server", () => { } }); + test("rejects HTTP requests with non-loopback Host headers", async () => { + const port = await reserveLoopbackPort(); + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + + const server = serveSessionBrokerDaemon(); + + try { + const response = await fetch(`http://127.0.0.1:${port}/health`, { + headers: { host: `attacker.example:${port}` }, + }); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toEqual({ + error: "Host header is not allowed for the local session broker.", + }); + } finally { + server.stop(true); + } + }); + + test("rejects non-local Origin headers for HTTP and websocket requests", async () => { + const port = await reserveLoopbackPort(); + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + + const server = serveSessionBrokerDaemon(); + + try { + const response = await fetch(`http://127.0.0.1:${port}/session-api/capabilities`, { + headers: { origin: "https://attacker.example" }, + }); + expect(response.status).toBe(403); + await expect(response.json()).resolves.toEqual({ + error: "Origin is not allowed for the local session broker.", + }); + + const handshake = await readRawWebSocketHandshake(port, ["Origin: https://attacker.example"]); + expect(handshake).toStartWith("HTTP/1.1 403"); + } finally { + server.stop(true); + } + }); + + test("requires JSON content type for session API posts", async () => { + const port = await reserveLoopbackPort(); + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + + const server = serveSessionBrokerDaemon(); + + try { + const response = await fetch(`http://127.0.0.1:${port}/session-api`, { + method: "POST", + headers: { "content-type": "text/plain" }, + body: JSON.stringify({ action: "list" }), + }); + + expect(response.status).toBe(415); + await expect(response.json()).resolves.toEqual({ + error: "Expected Content-Type application/json.", + }); + } finally { + server.stop(true); + } + }); + test("closes snapshots for missing sessions with a specific not-registered reason", async () => { // Bun's Windows WebSocket client does not reliably surface this immediate server close. // The daemon-core test covers the close code/reason without the flaky transport layer. diff --git a/src/session-broker/brokerServer.ts b/src/session-broker/brokerServer.ts index e82d08ce..c64d8f39 100644 --- a/src/session-broker/brokerServer.ts +++ b/src/session-broker/brokerServer.ts @@ -6,6 +6,8 @@ import { import { LEGACY_MCP_PATH, SESSION_BROKER_SOCKET_PATH, + allowsUnsafeRemoteSessionBroker, + isLoopbackHost, resolveSessionBrokerConfig, } from "./brokerConfig"; import { @@ -89,6 +91,107 @@ function jsonError(message: string, status = 400) { return Response.json({ error: message }, { status }); } +/** Return whether one request body was explicitly sent as JSON. */ +function hasJsonContentType(request: Request) { + const contentType = request.headers.get("content-type"); + return contentType?.split(";", 1)[0]?.trim().toLowerCase() === "application/json"; +} + +/** Parse a Host-style value into hostname and optional port pieces. */ +function parseHostAndPort(value: string) { + const trimmed = value.trim(); + if (!trimmed) { + return null; + } + + if (trimmed.startsWith("[")) { + const closeBracketIndex = trimmed.indexOf("]"); + if (closeBracketIndex < 0) { + return null; + } + + const host = trimmed.slice(1, closeBracketIndex); + const rest = trimmed.slice(closeBracketIndex + 1); + if (!rest) { + return { host, port: undefined }; + } + + if (!rest.startsWith(":")) { + return null; + } + + const port = Number.parseInt(rest.slice(1), 10); + return Number.isInteger(port) && port > 0 ? { host, port } : null; + } + + const colonCount = [...trimmed].filter((character) => character === ":").length; + if (colonCount === 0) { + return { host: trimmed, port: undefined }; + } + + if (colonCount === 1) { + const [host, rawPort] = trimmed.split(":"); + const port = Number.parseInt(rawPort ?? "", 10); + return host && Number.isInteger(port) && port > 0 ? { host, port } : null; + } + + // Unbracketed IPv6 literals are invalid in Host headers, but accepting the address without a + // port keeps validation strict enough for DNS-rebinding while tolerating unusual native clients. + return { host: trimmed, port: undefined }; +} + +/** Return whether a parsed authority targets an accepted broker host and port. */ +function isAllowedHostPort( + hostPort: { host: string; port?: number }, + expectedPort: number, + options: { allowRemote: boolean }, +) { + const hostAllowed = options.allowRemote || isLoopbackHost(hostPort.host); + return hostAllowed && (hostPort.port === undefined || hostPort.port === expectedPort); +} + +/** Block DNS-rebinding style requests whose Host does not name a permitted broker endpoint. */ +function validateHostHeader(request: Request, expectedPort: number, allowRemote: boolean) { + const hostHeader = request.headers.get("host"); + if (!hostHeader) { + return jsonError("Expected Host header for the local session broker.", 400); + } + + const hostPort = parseHostAndPort(hostHeader); + if (!hostPort || !isAllowedHostPort(hostPort, expectedPort, { allowRemote })) { + return jsonError("Host header is not allowed for the local session broker.", 403); + } + + return null; +} + +/** Block browser-originated requests from non-local or wrong-port origins. */ +function validateOriginHeader(request: Request, expectedPort: number, allowRemote: boolean) { + const origin = request.headers.get("origin"); + if (!origin) { + return null; + } + + let url: URL; + try { + url = new URL(origin); + } catch { + return jsonError("Origin is not allowed for the local session broker.", 403); + } + + if (url.protocol !== "http:" && url.protocol !== "https:") { + return jsonError("Origin is not allowed for the local session broker.", 403); + } + + const defaultPort = url.protocol === "http:" ? 80 : 443; + const port = url.port ? Number.parseInt(url.port, 10) : defaultPort; + if (!isAllowedHostPort({ host: url.hostname, port }, expectedPort, { allowRemote })) { + return jsonError("Origin is not allowed for the local session broker.", 403); + } + + return null; +} + async function parseJsonRequest(request: Request) { try { return (await request.json()) as SessionDaemonRequest; @@ -102,6 +205,10 @@ async function handleSessionApiRequest(state: HunkSessionBrokerState, request: R return jsonError("Session API requests must use POST.", 405); } + if (!hasJsonContentType(request)) { + return jsonError("Expected Content-Type application/json.", 415); + } + try { const input = await parseJsonRequest(request); let response: SessionDaemonResponse; @@ -291,6 +398,7 @@ export function serveSessionBrokerDaemon( options: ServeSessionBrokerDaemonOptions = {}, ): RunningSessionBrokerDaemon { const config = resolveSessionBrokerConfig(); + const allowRemote = allowsUnsafeRemoteSessionBroker(); const idleTimeoutMs = options.idleTimeoutMs ?? DEFAULT_IDLE_TIMEOUT_MS; const staleSessionTtlMs = options.staleSessionTtlMs ?? DEFAULT_STALE_SESSION_TTL_MS; const staleSessionSweepIntervalMs = @@ -317,6 +425,16 @@ export function serveSessionBrokerDaemon( port: config.port, formatServeError: (error, _address) => formatDaemonServeError(error, config.host, config.port), handleRequest: async (request) => { + const hostError = validateHostHeader(request, config.port, allowRemote); + if (hostError) { + return hostError; + } + + const originError = validateOriginHeader(request, config.port, allowRemote); + if (originError) { + return originError; + } + const url = new URL(request.url); if (url.pathname === "/health") { From d16e2e1cb4524e4c9ea3bc3077afe4204226fbef Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Tue, 19 May 2026 00:14:41 -0400 Subject: [PATCH 2/2] fix(session): tighten daemon request validation --- packages/session-broker/src/daemon.ts | 4 ---- src/session-broker/brokerServer.test.ts | 17 +++++++++++++---- src/session-broker/brokerServer.ts | 4 +++- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/session-broker/src/daemon.ts b/packages/session-broker/src/daemon.ts index d5ba086b..edc17c8c 100644 --- a/packages/session-broker/src/daemon.ts +++ b/packages/session-broker/src/daemon.ts @@ -63,10 +63,6 @@ function hasJsonContentType(request: Request) { async function parseJsonRequest( request: Request, ) { - if (!hasJsonContentType(request)) { - throw new Error("Expected Content-Type application/json."); - } - try { return (await request.json()) as SessionBrokerDaemonRequest; } catch { diff --git a/src/session-broker/brokerServer.test.ts b/src/session-broker/brokerServer.test.ts index 25b335cd..40521538 100644 --- a/src/session-broker/brokerServer.test.ts +++ b/src/session-broker/brokerServer.test.ts @@ -294,7 +294,7 @@ describe("Hunk session daemon server", () => { } }); - test("rejects HTTP requests with non-loopback Host headers", async () => { + test("rejects HTTP requests with non-loopback or wrong-port Host headers", async () => { const port = await reserveLoopbackPort(); process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); @@ -302,12 +302,21 @@ describe("Hunk session daemon server", () => { const server = serveSessionBrokerDaemon(); try { - const response = await fetch(`http://127.0.0.1:${port}/health`, { + const attackerHostResponse = await fetch(`http://127.0.0.1:${port}/health`, { headers: { host: `attacker.example:${port}` }, }); - expect(response.status).toBe(403); - await expect(response.json()).resolves.toEqual({ + expect(attackerHostResponse.status).toBe(403); + await expect(attackerHostResponse.json()).resolves.toEqual({ + error: "Host header is not allowed for the local session broker.", + }); + + const missingPortResponse = await fetch(`http://127.0.0.1:${port}/health`, { + headers: { host: "127.0.0.1" }, + }); + + expect(missingPortResponse.status).toBe(403); + await expect(missingPortResponse.json()).resolves.toEqual({ error: "Host header is not allowed for the local session broker.", }); } finally { diff --git a/src/session-broker/brokerServer.ts b/src/session-broker/brokerServer.ts index c64d8f39..4093038e 100644 --- a/src/session-broker/brokerServer.ts +++ b/src/session-broker/brokerServer.ts @@ -147,7 +147,9 @@ function isAllowedHostPort( options: { allowRemote: boolean }, ) { const hostAllowed = options.allowRemote || isLoopbackHost(hostPort.host); - return hostAllowed && (hostPort.port === undefined || hostPort.port === expectedPort); + const defaultHttpPort = 80; + const port = hostPort.port ?? defaultHttpPort; + return hostAllowed && port === expectedPort; } /** Block DNS-rebinding style requests whose Host does not name a permitted broker endpoint. */