Skip to content

fix(session): harden daemon HTTP requests #329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
benvinegar merged 2 commits into from
May 19, 2026
Merged

fix(session): harden daemon HTTP requests #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17952da
fix(session): harden daemon HTTP requests
benvinegar May 19, 2026
d16e2e1
fix(session): tighten daemon request validation
benvinegar May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions packages/session-broker/src/daemon.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }),
}),
);
Expand All @@ -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" } }),
}),
);
Expand All @@ -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(),
Expand All @@ -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" },
Expand Down
10 changes: 10 additions & 0 deletions packages/session-broker/src/daemon.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ 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<CommandName extends string = string, CommandInput = unknown>(
request: Request,
Expand Down Expand Up @@ -322,6 +328,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<ServerMessage["command"]>(request);
let response: SessionBrokerDaemonResponse<SessionView, CommandResult>;
Expand Down
120 changes: 119 additions & 1 deletion src/session-broker/brokerServer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -116,6 +117,47 @@ async function openSessionSocket(port: number) {
return socket;
}

async function readRawWebSocketHandshake(port: number, headers: string[]) {
return new Promise<string>((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",
Expand Down Expand Up @@ -252,6 +294,82 @@ describe("Hunk session daemon server", () => {
}
});

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);

const server = serveSessionBrokerDaemon();

try {
const attackerHostResponse = await fetch(`http://127.0.0.1:${port}/health`, {
headers: { host: `attacker.example:${port}` },
});

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 {
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.
Expand Down
120 changes: 120 additions & 0 deletions src/session-broker/brokerServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
import {
LEGACY_MCP_PATH,
SESSION_BROKER_SOCKET_PATH,
allowsUnsafeRemoteSessionBroker,
isLoopbackHost,
resolveSessionBrokerConfig,
} from "./brokerConfig";
import {
Expand Down Expand Up @@ -89,6 +91,109 @@ 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);
const defaultHttpPort = 80;
const port = hostPort.port ?? defaultHttpPort;
return hostAllowed && port === expectedPort;
}
Comment on lines +144 to +153
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Port validation is skipped when the Host header omits the port

isAllowedHostPort returns true whenever hostPort.port === undefined, regardless of the daemon's actual listening port. A crafted HTTP client that sends Host: localhost or Host: 127.0.0.1 (without a port number) will pass this check even when the daemon is running on a non-standard port like 47657. DNS-rebinding attacks from browsers always include the port in the Host header, so real-world risk is low, but the port guard provides no defence against non-browser clients that omit the port.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/session-broker/brokerServer.ts
Line: 144-151

Comment:
**Port validation is skipped when the Host header omits the port**

`isAllowedHostPort` returns `true` whenever `hostPort.port === undefined`, regardless of the daemon's actual listening port. A crafted HTTP client that sends `Host: localhost` or `Host: 127.0.0.1` (without a port number) will pass this check even when the daemon is running on a non-standard port like 47657. DNS-rebinding attacks from browsers always include the port in the Host header, so real-world risk is low, but the port guard provides no defence against non-browser clients that omit the port.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

@benvinegar benvinegar May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tightened host validation so an omitted Host port is treated as the HTTP default port and rejected unless it matches the daemon port. Added regression coverage for Host: 127.0.0.1 without a port.

Responded by Pi using OpenAI GPT-5.


/** 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;
Expand All @@ -102,6 +207,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;
Expand Down Expand Up @@ -291,6 +400,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 =
Expand All @@ -317,6 +427,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") {
Expand Down
Loading