From c292d8a948b822cde2037b2766fcf158d7dc4592 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Sun, 30 Aug 2026 17:49:55 -0400 Subject: [PATCH] feat(session): migrate incompatible daemons safely --- .../secure-session-broker-integration.md | 5 + docs/agent-workflows.md | 10 +- src/main.tsx | 2 +- src/session/agent/cliClient.test.ts | 13 +- src/session/agent/cliClient.ts | 98 +++- src/session/agent/commands.daemon.test.ts | 1 - src/session/agent/commands.test.ts | 294 +++--------- src/session/agent/commands.ts | 105 +---- src/session/broker/appContract.ts | 15 + src/session/broker/brokerClient.test.ts | 426 +++++++++++++++--- src/session/broker/brokerClient.ts | 158 +++---- src/session/broker/brokerConfig.test.ts | 15 + src/session/broker/brokerLauncher.test.ts | 34 ++ src/session/broker/brokerLauncher.ts | 90 ++-- .../broker/brokerServer.helpers.test.ts | 4 +- src/session/broker/brokerServer.test.ts | 223 +++++++-- src/session/broker/brokerServer.ts | 160 +++++-- src/session/broker/credentials.test.ts | 91 ++++ src/session/broker/credentials.ts | 375 +++++++++++++++ src/session/broker/state.ts | 3 +- src/session/client/capabilities.ts | 9 +- test/session/broker-e2e.test.ts | 25 +- test/session/cli.test.ts | 54 +-- test/session/daemon.test.ts | 10 +- 24 files changed, 1584 insertions(+), 636 deletions(-) create mode 100644 .changeset/secure-session-broker-integration.md create mode 100644 src/session/broker/appContract.ts create mode 100644 src/session/broker/credentials.test.ts create mode 100644 src/session/broker/credentials.ts diff --git a/.changeset/secure-session-broker-integration.md b/.changeset/secure-session-broker-integration.md new file mode 100644 index 000000000..9e555da26 --- /dev/null +++ b/.changeset/secure-session-broker-integration.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Authenticate local session producers and CLI controls with automatically discovered owner-private credentials, signed responses, scoped reconnect replacement, and bounded handshakes. Expose only minimal public daemon health, refuse unsafe PID-based replacement, and let interactive Hunk windows reconnect automatically after an incompatible incumbent becomes idle. diff --git a/docs/agent-workflows.md b/docs/agent-workflows.md index 390247dd1..09ce109f1 100644 --- a/docs/agent-workflows.md +++ b/docs/agent-workflows.md @@ -25,15 +25,7 @@ When a Hunk TUI starts, it registers with a local loopback daemon. `hunk session Most users only need `hunk session ...`. Use `hunk mcp serve` only for manual startup or debugging of the local daemon. -If `hunk session list` reports no sessions while Hunk is visibly running, the agent sandbox may be blocking loopback access. Probe the daemon directly: - -```bash -curl -s -X POST http://127.0.0.1:47657/session-api \ - -H 'content-type: application/json' \ - --data '{"action":"list"}' -``` - -If this shows sessions, rerun the command with the agent's network/sandbox escalation. If you run the daemon with a custom `HUNK_MCP_PORT`, use that port instead. +If `hunk session list` reports no sessions while Hunk is visibly running, the agent sandbox may be blocking loopback access. Rerun `hunk session list --json` with the agent's network/sandbox escalation. Do not probe `/session-api` with raw `curl`: session controls require an automatically discovered, owner-private caller credential and signed responses, and Hunk intentionally exposes no credential flags. ## The commands you will use most diff --git a/src/main.tsx b/src/main.tsx index cf0992d60..848f1f905 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -21,7 +21,7 @@ async function main() { } if (startupPlan.kind === "daemon-serve") { - const server = serveSessionBrokerDaemon(); + const server = await serveSessionBrokerDaemon(); await server.stopped; return; } diff --git a/src/session/agent/cliClient.test.ts b/src/session/agent/cliClient.test.ts index ab4738856..b58b0a9f9 100644 --- a/src/session/agent/cliClient.test.ts +++ b/src/session/agent/cliClient.test.ts @@ -34,6 +34,9 @@ import { const selector = { sessionId: "session-1" } satisfies SessionSelectorInput; const originalFetch = globalThis.fetch; +const injectedCaller = { + request: (path: string, init?: RequestInit) => globalThis.fetch(path, init), +}; afterEach(() => { globalThis.fetch = originalFetch; @@ -134,7 +137,7 @@ describe("HTTP Hunk session CLI client", () => { return Response.json(responses[request.action as keyof typeof responses]); }) as typeof fetch; - const client = createHttpHunkSessionCliClient(); + const client = createHttpHunkSessionCliClient({ caller: injectedCaller }); expect(await client.getCapabilities()).toMatchObject({ version: HUNK_SESSION_API_VERSION }); expect(await client.listSessions()).toEqual([session]); @@ -327,7 +330,7 @@ describe("HTTP Hunk session CLI client", () => { }); }) as typeof fetch; - const client = createHttpHunkSessionCliClient({ timeoutMs: 10 }); + const client = createHttpHunkSessionCliClient({ timeoutMs: 10, caller: injectedCaller }); await expect(client.listSessions()).rejects.toThrow( "Timed out waiting for the Hunk session daemon to complete session list.", @@ -340,7 +343,7 @@ describe("HTTP Hunk session CLI client", () => { sessions: [{ sessionId: "partial", unknown: true }], })) as unknown as typeof fetch; - const client = createHttpHunkSessionCliClient(); + const client = createHttpHunkSessionCliClient({ caller: injectedCaller }); await expect(client.listSessions()).rejects.toThrow( "Invalid Hunk session daemon response for list.", ); @@ -356,7 +359,7 @@ describe("HTTP Hunk session CLI client", () => { globalThis.fetch = (async () => Response.json({ sessions: [session] })) as unknown as typeof fetch; - const client = createHttpHunkSessionCliClient(); + const client = createHttpHunkSessionCliClient({ caller: injectedCaller }); const result = await client.listSessions(); expect(result).toEqual([session]); expect(result[0]).not.toBe(session); @@ -369,7 +372,7 @@ describe("HTTP Hunk session CLI client", () => { { status: 404, statusText: "Not Found" }, )) as unknown as typeof fetch; - const client = createHttpHunkSessionCliClient(); + const client = createHttpHunkSessionCliClient({ caller: injectedCaller }); await expect(client.listSessions()).rejects.toThrow("No matching session."); globalThis.fetch = (async () => diff --git a/src/session/agent/cliClient.ts b/src/session/agent/cliClient.ts index ca22c0f56..b6ab01072 100644 --- a/src/session/agent/cliClient.ts +++ b/src/session/agent/cliClient.ts @@ -1,19 +1,28 @@ import { sanitizeTerminalText } from "../../lib/terminalText"; import { resolveSessionBrokerConfig } from "../broker/brokerConfig"; +import { + SessionBrokerCallerClient, + type SessionBrokerSignedRequestInit, +} from "@hunk/session-broker"; import type { SessionTerminalLocation, SessionTerminalMetadata } from "@hunk/session-broker-core"; -import { readHunkSessionDaemonCapabilities } from "../client/capabilities"; import { HUNK_SESSION_DAEMON_HTTP_TIMEOUT_MS, - requestSessionDaemonHttp, + withSessionDaemonHttpTimeout, } from "../client/daemonHttp"; +import { loadOrCreateHunkSessionBrokerCredentials } from "../broker/credentials"; +import { + HUNK_SESSION_BROKER_APP_ID, + HUNK_SESSION_BROKER_APP_REVISION, +} from "../broker/appContract"; import { HUNK_SESSION_API_PATH, + HUNK_SESSION_CAPABILITIES_PATH, type SessionDaemonAction, type SessionDaemonCapabilities, type SessionDaemonRequest, type SessionDaemonResponses, } from "../protocol"; -import { parseSessionDaemonResponse } from "../protocolSchemas"; +import { parseSessionDaemonCapabilities, parseSessionDaemonResponse } from "../protocolSchemas"; import type { AppliedCommentBatchResult, AppliedCommentResult, @@ -76,31 +85,60 @@ async function extractResponseError(response: Response) { return response.statusText || "Unknown Hunk session daemon error."; } +interface HunkCallerTransport { + request( + path: string, + init?: SessionBrokerSignedRequestInit, + options?: { readonly targetSpecific?: boolean }, + ): Promise; +} + class HttpHunkSessionCliClient implements HunkSessionCliClient { private readonly config = resolveSessionBrokerConfig(); - - constructor(private readonly timeoutMs = HUNK_SESSION_DAEMON_HTTP_TIMEOUT_MS) {} + private callerPromise: Promise | null = null; + + constructor( + private readonly timeoutMs = HUNK_SESSION_DAEMON_HTTP_TIMEOUT_MS, + private readonly injectedCaller?: HunkCallerTransport, + ) {} + + private caller() { + if (this.injectedCaller) return Promise.resolve(this.injectedCaller); + this.callerPromise ??= loadOrCreateHunkSessionBrokerCredentials().then( + (credentials) => + new SessionBrokerCallerClient({ + appId: HUNK_SESSION_BROKER_APP_ID, + appRevision: HUNK_SESSION_BROKER_APP_REVISION, + origin: this.config.httpOrigin, + credential: credentials.caller, + daemon: { + keyId: credentials.daemonIdentity.keyId, + publicKey: credentials.daemonPublicKey, + }, + }), + ); + return this.callerPromise; + } private async request( input: Extract, ): Promise { - return requestSessionDaemonHttp({ - config: this.config, - path: HUNK_SESSION_API_PATH, + return withSessionDaemonHttpTimeout({ operation: `complete session ${input.action}`, timeoutMs: this.timeoutMs, - init: { - method: "POST", - headers: { - "content-type": "application/json", - }, - body: JSON.stringify(input), - }, - parse: async (response) => { - if (!response.ok) { - throw new Error(await extractResponseError(response)); - } - + task: async (signal) => { + const caller = await this.caller(); + const response = await caller.request( + HUNK_SESSION_API_PATH, + { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(input), + signal, + }, + { targetSpecific: input.action !== "list" }, + ); + if (!response.ok) throw new Error(await extractResponseError(response)); let value: unknown; try { value = await response.json(); @@ -113,7 +151,20 @@ class HttpHunkSessionCliClient implements HunkSessionCliClient { } async getCapabilities() { - return readHunkSessionDaemonCapabilities(this.config, this.timeoutMs); + return withSessionDaemonHttpTimeout({ + operation: "report capabilities", + timeoutMs: this.timeoutMs, + task: async (signal) => { + const response = await ( + await this.caller() + ).request(HUNK_SESSION_CAPABILITIES_PATH, { + method: "GET", + signal, + }); + if (!response.ok) return null; + return parseSessionDaemonCapabilities(await response.json()); + }, + }); } async listSessions() { @@ -255,8 +306,9 @@ class HttpHunkSessionCliClient implements HunkSessionCliClient { /** Create the concrete Hunk session CLI client that speaks to the broker-backed HTTP API. */ export function createHttpHunkSessionCliClient({ timeoutMs, -}: { timeoutMs?: number } = {}): HunkSessionCliClient { - return new HttpHunkSessionCliClient(timeoutMs); + caller, +}: { timeoutMs?: number; caller?: HunkCallerTransport } = {}): HunkSessionCliClient { + return new HttpHunkSessionCliClient(timeoutMs, caller); } export function stringifyJson(value: unknown) { diff --git a/src/session/agent/commands.daemon.test.ts b/src/session/agent/commands.daemon.test.ts index f5c240a4c..dbb4c7f62 100644 --- a/src/session/agent/commands.daemon.test.ts +++ b/src/session/agent/commands.daemon.test.ts @@ -141,7 +141,6 @@ describe("text output formatting", () => { test("renders reload, comment-add, and comment-clear as non-empty text", async () => { setSessionCommandTestHooks({ resolveDaemonAvailability: async () => true, - restartDaemonForMissingAction: async () => {}, createClient: () => createFakeClient(), }); diff --git a/src/session/agent/commands.test.ts b/src/session/agent/commands.test.ts index 6eda84832..15c6c01ad 100644 --- a/src/session/agent/commands.test.ts +++ b/src/session/agent/commands.test.ts @@ -13,8 +13,8 @@ import { setSessionCommandTestHooks, type HunkDaemonCliClient, } from "./commands"; -import { HUNK_DAEMON_UPGRADE_RESTART_NOTICE } from "../client/capabilities"; import { HUNK_SESSION_API_VERSION, HUNK_SESSION_DAEMON_VERSION } from "../protocol"; +import { SessionBrokerClientAuthenticationError } from "@hunk/session-broker"; function createTestListedSession(sessionId: string) { return buildTestListedSession({ @@ -147,233 +147,87 @@ afterEach(() => { }); describe("session command compatibility checks", () => { - test("refreshes an older daemon without the session API before running context", async () => { - const selector: SessionSelectorInput = { sessionId: "session-1" }; - const restartCalls: Array<{ action: string; selector?: SessionSelectorInput }> = []; - const createdClients: string[] = []; - const notices: string[] = []; - const originalConsoleError = console.error; - console.error = (...args: unknown[]) => { - notices.push(args.map((value) => String(value)).join(" ")); - }; - - const clients = [ - createClient({ - getCapabilities: async () => { - createdClients.push("stale-capabilities"); - return null; - }, - }), - createClient({ - getSelectedContext: async (receivedSelector) => { - createdClients.push("fresh-context"); - expect(receivedSelector).toEqual(selector); - return createTestSelectedSessionContext(); - }, - }), - ]; - - try { - setSessionCommandTestHooks({ - createClient: () => { - const client = clients.shift(); - if (!client) { - throw new Error("No fake session client remaining."); - } - - return client; - }, - resolveDaemonAvailability: async () => true, - restartDaemonForMissingAction: async (action, receivedSelector) => { - restartCalls.push({ action, selector: receivedSelector }); - }, - }); + test("fails promptly without executing an action against an incompatible daemon", async () => { + let contextCalls = 0; + setSessionCommandTestHooks({ + createClient: () => + createClient({ + getCapabilities: async () => null, + getSelectedContext: async () => { + contextCalls += 1; + return createTestSelectedSessionContext(); + }, + }), + resolveDaemonAvailability: async () => true, + }); - const output = await runSessionCommand({ + await expect( + runSessionCommand({ kind: "session", action: "context", - selector, + selector: { sessionId: "session-1" }, output: "json", - } satisfies SessionCommandInput); - - expect(JSON.parse(output)).toMatchObject({ - context: { - sessionId: "session-1", - selectedFile: { - path: "README.md", - }, - selectedHunk: { - index: 0, - }, - }, - }); - expect(restartCalls).toEqual([ - { - action: "context", - selector, - }, - ]); - expect(createdClients).toEqual(["stale-capabilities", "fresh-context"]); - expect(notices).toContain(HUNK_DAEMON_UPGRADE_RESTART_NOTICE); - } finally { - console.error = originalConsoleError; - } + } satisfies SessionCommandInput), + ).rejects.toThrow( + "Close older Hunk windows, wait for the daemon to become idle, then retry this command.", + ); + expect(contextCalls).toBe(0); }); - test("refreshes an incompatible daemon version before running list", async () => { - const restartCalls: Array<{ action: string; selector?: SessionSelectorInput }> = []; - const createdClients: string[] = []; - const notices: string[] = []; - const originalConsoleError = console.error; - console.error = (...args: unknown[]) => { - notices.push(args.map((value) => String(value)).join(" ")); - }; - - const clients = [ - createClient({ - getCapabilities: async () => { - createdClients.push("stale-capabilities"); - return { - version: HUNK_SESSION_API_VERSION - 1, - daemonVersion: HUNK_SESSION_DAEMON_VERSION, - actions: ["list"], - }; - }, - }), - createClient({ - listSessions: async () => { - createdClients.push("fresh-list"); - return [createTestListedSession("session-1")]; - }, - }), - ]; - - try { - setSessionCommandTestHooks({ - createClient: () => { - const client = clients.shift(); - if (!client) { - throw new Error("No fake session client remaining."); - } - - return client; - }, - resolveDaemonAvailability: async () => true, - restartDaemonForMissingAction: async (action, receivedSelector) => { - restartCalls.push({ action, selector: receivedSelector }); - }, - }); - - const output = await runSessionCommand({ - kind: "session", - action: "list", - output: "json", - } satisfies SessionCommandInput); - - expect(JSON.parse(output)).toMatchObject({ - sessions: [ - { - sessionId: "session-1", + test("maps signed negotiation failure to quiescent upgrade guidance", async () => { + setSessionCommandTestHooks({ + createClient: () => + createClient({ + getCapabilities: async () => { + throw new SessionBrokerClientAuthenticationError(); }, - ], - }); - expect(restartCalls).toEqual([ - { - action: "list", - selector: undefined, - }, - ]); - expect(createdClients).toEqual(["stale-capabilities", "fresh-list"]); - expect(notices).toContain(HUNK_DAEMON_UPGRADE_RESTART_NOTICE); - } finally { - console.error = originalConsoleError; - } + }), + resolveDaemonAvailability: async () => true, + }); + + await expect( + runSessionCommand({ kind: "session", action: "list", output: "json" }), + ).rejects.toThrow("Close older Hunk windows"); }); - test("refreshes a stale daemon before running comment-add", async () => { - const selector: SessionSelectorInput = { sessionId: "session-1" }; - const restartCalls: Array<{ action: string; selector?: SessionSelectorInput }> = []; - const createdClients: string[] = []; - const notices: string[] = []; - const originalConsoleError = console.error; - console.error = (...args: unknown[]) => { - notices.push(args.map((value) => String(value)).join(" ")); - }; + test("preserves local credential-store failures", async () => { + setSessionCommandTestHooks({ + createClient: () => + createClient({ + getCapabilities: async () => { + throw new Error("owner-private credential store is unsafe"); + }, + }), + resolveDaemonAvailability: async () => true, + }); - const clients = [ - createClient({ - getCapabilities: async () => { - createdClients.push("stale-capabilities"); - return null; - }, - }), - createClient({ - addComment: async (input) => { - createdClients.push("fresh-comment-add"); - expect(input.selector).toEqual(selector); - expect(input.filePath).toBe("README.md"); - expect(input.side).toBe("new"); - expect(input.line).toBe(2); - expect(input.summary).toBe("Review note"); - return { - commentId: "comment-1", - fileId: "file-1", - filePath: "README.md", - hunkIndex: 0, - side: "new", - line: 2, - }; - }, - }), - ]; - - try { - setSessionCommandTestHooks({ - createClient: () => { - const client = clients.shift(); - if (!client) { - throw new Error("No fake session client remaining."); - } - - return client; - }, - resolveDaemonAvailability: async () => true, - restartDaemonForMissingAction: async (action, receivedSelector) => { - restartCalls.push({ action, selector: receivedSelector }); - }, - }); + await expect( + runSessionCommand({ kind: "session", action: "list", output: "json" }), + ).rejects.toThrow("owner-private credential store is unsafe"); + }); - const output = await runSessionCommand({ - kind: "session", - action: "comment-add", - selector, - filePath: "README.md", - side: "new", - line: 2, - summary: "Review note", - reveal: false, - output: "json", - } satisfies SessionCommandInput); + test("fails promptly when compatible capabilities omit the required action", async () => { + let listCalls = 0; + setSessionCommandTestHooks({ + createClient: () => + createClient({ + getCapabilities: async () => ({ + version: HUNK_SESSION_API_VERSION, + daemonVersion: HUNK_SESSION_DAEMON_VERSION, + actions: ["get"], + }), + listSessions: async () => { + listCalls += 1; + return []; + }, + }), + resolveDaemonAvailability: async () => true, + }); - expect(JSON.parse(output)).toMatchObject({ - result: { - commentId: "comment-1", - filePath: "README.md", - side: "new", - line: 2, - }, - }); - expect(restartCalls).toEqual([ - { - action: "comment-add", - selector, - }, - ]); - expect(createdClients).toEqual(["stale-capabilities", "fresh-comment-add"]); - expect(notices).toContain(HUNK_DAEMON_UPGRADE_RESTART_NOTICE); - } finally { - console.error = originalConsoleError; - } + await expect( + runSessionCommand({ kind: "session", action: "list", output: "json" }), + ).rejects.toThrow("missing required support for list"); + expect(listCalls).toBe(0); }); test("runs review commands through the daemon without raw patch text by default", async () => { @@ -865,9 +719,7 @@ describe("session command compatibility checks", () => { ); }); - test("does not restart when the daemon already exposes the needed session action", async () => { - const restartCalls: string[] = []; - + test("runs when the daemon already exposes the needed session action", async () => { setSessionCommandTestHooks({ createClient: () => createClient({ @@ -890,9 +742,6 @@ describe("session command compatibility checks", () => { }), }), resolveDaemonAvailability: async () => true, - restartDaemonForMissingAction: async (action) => { - restartCalls.push(action); - }, }); const output = await runSessionCommand({ @@ -903,7 +752,6 @@ describe("session command compatibility checks", () => { } satisfies SessionCommandInput); expect(JSON.parse(output)).toEqual({ comments: [] }); - expect(restartCalls).toEqual([]); }); test("normalizes session-path selectors for reload commands before calling the daemon client", async () => { diff --git a/src/session/agent/commands.ts b/src/session/agent/commands.ts index e5cfca35f..714c6df02 100644 --- a/src/session/agent/commands.ts +++ b/src/session/agent/commands.ts @@ -1,19 +1,10 @@ -import type { - SessionCommandInput, - SessionCommandOutput, - SessionSelectorInput, -} from "../../core/run/commandInputs"; +import type { SessionCommandInput, SessionCommandOutput } from "../../core/run/commandInputs"; import type { SessionLiveCommentSummary, SessionReviewNoteSummary } from "../types"; import { NO_ACTIVE_SESSIONS_MESSAGE } from "./errors"; -import { - ensureSessionBrokerAvailable, - isSessionBrokerHealthy, - isLoopbackPortReachable, - readSessionBrokerHealth, - waitForSessionBrokerShutdown, -} from "../broker/brokerLauncher"; +import { isSessionBrokerHealthy, isLoopbackPortReachable } from "../broker/brokerLauncher"; import { resolveSessionBrokerConfig } from "../broker/brokerConfig"; -import { matchesSessionSelector, normalizeSessionSelector } from "@hunk/session-broker-core"; +import { normalizeSessionSelector } from "@hunk/session-broker-core"; +import { SessionBrokerClientAuthenticationError } from "@hunk/session-broker"; import { createHttpHunkSessionCliClient, formatClearCommentsOutput, @@ -33,7 +24,6 @@ import { stringifyJson, type HunkSessionCliClient, } from "./cliClient"; -import { reportHunkDaemonUpgradeRestart } from "../client/capabilities"; import { HUNK_SESSION_API_VERSION, type SessionDaemonAction } from "../protocol"; const REQUIRED_ACTION_BY_COMMAND: Record = { @@ -57,10 +47,6 @@ export type HunkDaemonCliClient = HunkSessionCliClient; interface SessionCommandTestHooks { createClient?: () => HunkSessionCliClient; resolveDaemonAvailability?: (action: SessionCommandInput["action"]) => Promise; - restartDaemonForMissingAction?: ( - action: SessionDaemonAction, - selector?: SessionSelectorInput, - ) => Promise; } let sessionCommandTestHooks: SessionCommandTestHooks | null = null; @@ -73,80 +59,22 @@ function createDaemonCliClient() { return sessionCommandTestHooks?.createClient?.() ?? createHttpHunkSessionCliClient(); } -async function waitForSessionRegistration(selector?: SessionSelectorInput, timeoutMs = 8_000) { - const deadline = Date.now() + timeoutMs; - - while (Date.now() < deadline) { - const client = createDaemonCliClient(); - - try { - const sessions = await client.listSessions(); - if (sessions.some((session) => matchesSessionSelector(session, selector))) { - return true; - } - } catch { - // Keep polling while the fresh daemon/session reconnects. - } - - await Bun.sleep(200); - } - - return false; -} - -async function restartDaemonForMissingAction( - action: SessionDaemonAction, - selector?: SessionSelectorInput, -) { - const health = await readSessionBrokerHealth(); - const pid = health?.pid; - const hadSessions = (health?.sessions ?? 0) > 0; - if (!pid || pid === process.pid) { - throw new Error( - `The running Hunk session daemon is missing required support for ${action}. ` + - `Restart Hunk so it can launch a fresh daemon from the current source tree.`, - ); - } - - process.kill(pid, "SIGTERM"); - - const shutDown = await waitForSessionBrokerShutdown(); - if (!shutDown) { - throw new Error( - `Stopped waiting for the old Hunk session daemon to exit after it was found missing ${action}.`, - ); - } - - const config = resolveSessionBrokerConfig(); - await ensureSessionBrokerAvailable({ - config, - timeoutMs: 3_000, - timeoutMessage: "Timed out waiting for the refreshed Hunk session daemon to start.", - }); - - // `hunk session list` can recover from a stale daemon even when the old process belonged to a - // sibling worktree that reports sessions which will never reconnect to this fresh daemon. - if (selector || (hadSessions && action !== "list")) { - const registered = await waitForSessionRegistration(selector); - if (!registered) { - throw new Error( - "Timed out waiting for the live Hunk session to reconnect after refreshing the session daemon. " + - "Restart that Hunk window if it was launched from an older build.", - ); - } +async function ensureRequiredAction(action: SessionDaemonAction, client = createDaemonCliClient()) { + let capabilities; + try { + capabilities = await client.getCapabilities(); + } catch (error) { + if (!(error instanceof SessionBrokerClientAuthenticationError)) throw error; + capabilities = null; } -} - -async function ensureRequiredAction(action: SessionDaemonAction, selector?: SessionSelectorInput) { - const client = createDaemonCliClient(); - const capabilities = await client.getCapabilities(); if (capabilities?.version === HUNK_SESSION_API_VERSION && capabilities.actions.includes(action)) { return; } - reportHunkDaemonUpgradeRestart(); - await (sessionCommandTestHooks?.restartDaemonForMissingAction?.(action, selector) ?? - restartDaemonForMissingAction(action, selector)); + throw new Error( + `The running Hunk session daemon is incompatible or missing required support for ${action}. ` + + "Close older Hunk windows, wait for the daemon to become idle, then retry this command.", + ); } async function resolveDaemonAvailability(action: SessionCommandInput["action"]) { @@ -185,9 +113,8 @@ export async function runSessionCommand(input: SessionCommandInput) { const normalizedSelector = "selector" in input ? normalizeSessionSelector(input.selector) : null; const requiredAction = REQUIRED_ACTION_BY_COMMAND[input.action]; - await ensureRequiredAction(requiredAction, normalizedSelector ?? undefined); - const client = createDaemonCliClient(); + await ensureRequiredAction(requiredAction, client); switch (input.action) { case "list": { diff --git a/src/session/broker/appContract.ts b/src/session/broker/appContract.ts new file mode 100644 index 000000000..fbc2612ae --- /dev/null +++ b/src/session/broker/appContract.ts @@ -0,0 +1,15 @@ +import { + SESSION_BROKER_PROTOCOL_REVISION, + type BrokerAppContract, +} from "@hunk/session-broker-core"; +import { HUNK_SESSION_DAEMON_VERSION } from "../protocol"; + +/** Defines Hunk's immutable Phase-1 broker and application wire contract. */ +export const HUNK_SESSION_BROKER_APP_ID = "dev.hunk" as const; +export const HUNK_SESSION_BROKER_REVISION = SESSION_BROKER_PROTOCOL_REVISION; +export const HUNK_SESSION_BROKER_APP_REVISION = HUNK_SESSION_DAEMON_VERSION; +export const HUNK_SESSION_BROKER_FEATURES = Object.freeze([]) as readonly []; +export const HUNK_SESSION_BROKER_APP_CONTRACT: Readonly = Object.freeze({ + appRevision: HUNK_SESSION_BROKER_APP_REVISION, + features: HUNK_SESSION_BROKER_FEATURES, +}); diff --git a/src/session/broker/brokerClient.test.ts b/src/session/broker/brokerClient.test.ts index 00c6ebc5c..ee213dc38 100644 --- a/src/session/broker/brokerClient.test.ts +++ b/src/session/broker/brokerClient.test.ts @@ -1,17 +1,28 @@ import { afterEach, describe, expect, test } from "bun:test"; import { createServer } from "node:http"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { createTestSessionRegistration, createTestSessionReviewFile, createTestSessionSnapshot, } from "../../../test/helpers/session-daemon-fixtures"; import { HUNK_SESSION_API_VERSION, HUNK_SESSION_DAEMON_VERSION } from "../protocol"; -import { SessionBrokerClient } from "./brokerClient"; +import { SessionBroker, createSessionBrokerDaemon } from "@hunk/session-broker"; +import { serveSessionBrokerDaemon as serveBunSessionBrokerDaemon } from "@hunk/session-broker-bun"; +import { hunkSessionProtocolParsers } from "./protocolParsers"; +import { isQuiescentUpgradeRefusal, SessionBrokerClient } from "./brokerClient"; +import { loadOrCreateHunkSessionBrokerCredentials } from "./credentials"; +import { serveSessionBrokerDaemon as serveHunkSessionBrokerDaemon } from "./brokerServer"; +import { createHttpHunkSessionCliClient } from "../agent/cliClient"; +import { resolveSessionBrokerRuntimePaths } from "./brokerLauncher"; const originalHost = process.env.HUNK_MCP_HOST; const originalPort = process.env.HUNK_MCP_PORT; const originalDisable = process.env.HUNK_MCP_DISABLE; const originalUnsafeRemote = process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE; +const originalRuntimeDir = process.env.XDG_RUNTIME_DIR; const originalConsoleError = console.error; function createRegistration() { @@ -33,11 +44,16 @@ function createSnapshot() { }); } -async function waitUntil(label: string, fn: () => boolean, timeoutMs = 5_000, intervalMs = 50) { +async function waitUntil( + label: string, + fn: () => boolean | Promise, + timeoutMs = 5_000, + intervalMs = 50, +) { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { - if (fn()) { + if (await fn()) { return; } @@ -72,10 +88,37 @@ afterEach(() => { process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE = originalUnsafeRemote; } + if (originalRuntimeDir === undefined) { + delete process.env.XDG_RUNTIME_DIR; + } else { + process.env.XDG_RUNTIME_DIR = originalRuntimeDir; + } + console.error = originalConsoleError; }); describe("Hunk session daemon client", () => { + test("only treats exact pre-authentication compatibility closes as quiescent refusals", () => { + const reason = "Session broker authentication required; upgrade Hunk."; + expect(isQuiescentUpgradeRefusal({ code: 1008, reason, authenticated: false })).toBe(true); + expect( + isQuiescentUpgradeRefusal({ + code: 1008, + reason: "Malformed session broker protocol.", + authenticated: false, + }), + ).toBe(true); + expect(isQuiescentUpgradeRefusal({ code: 1008, reason, authenticated: true })).toBe(false); + expect(isQuiescentUpgradeRefusal({ code: 1006, reason, authenticated: false })).toBe(false); + expect( + isQuiescentUpgradeRefusal({ + code: 1008, + reason: "Session broker authentication failed.", + authenticated: false, + }), + ).toBe(false); + }); + test("keeps its previous registration when the live connection rejects replacement", () => { const registration = createRegistration(); const client = new SessionBrokerClient(registration, createSnapshot()); @@ -120,39 +163,23 @@ describe("Hunk session daemon client", () => { } }, 10_000); - test("restartIncompatibleDaemon lets startup recover when the stale daemon already exited", async () => { - const server = createServer((_request, response) => { - response.writeHead(404, { "content-type": "text/plain" }); - response.end("gone"); - }); - await new Promise((resolve, reject) => { - server.once("error", reject); - server.listen(0, "127.0.0.1", () => resolve()); - }); - - const address = server.address(); - const port = typeof address === "object" && address ? address.port : 0; - const config = { - host: "127.0.0.1", - port, - httpOrigin: `http://127.0.0.1:${port}`, - wsOrigin: `ws://127.0.0.1:${port}`, - }; - + test("does not retain the legacy PID-based incompatible-daemon replacement path", () => { const client = new SessionBrokerClient(createRegistration(), createSnapshot()); - - try { - await expect((client as any).restartIncompatibleDaemon(config)).resolves.toBeUndefined(); - } finally { - client.stop(); - await new Promise((resolve) => server.close(() => resolve())); - } + expect((client as any).restartIncompatibleDaemon).toBeUndefined(); + client.stop(); }); test("logs one actionable warning when a refreshed daemon rejects an older Hunk window", async () => { const listener = createServer((_request, response) => { response.writeHead(200, { "content-type": "application/json" }); - response.end(JSON.stringify({ ok: true, pid: process.pid, sessions: 0, pendingCommands: 0 })); + response.end( + JSON.stringify({ + ok: true, + pid: process.pid, + sessions: 0, + pendingCommands: 0, + }), + ); }); await new Promise((resolve, reject) => { listener.once("error", reject); @@ -170,7 +197,12 @@ describe("Hunk session daemon client", () => { fetch(request, bunServer) { const url = new URL(request.url); if (url.pathname === "/health") { - return Response.json({ ok: true, pid: process.pid, sessions: 0, pendingCommands: 0 }); + return Response.json({ + ok: true, + pid: process.pid, + sessions: 0, + pendingCommands: 0, + }); } if (url.pathname === "/session-api/capabilities") { @@ -194,21 +226,27 @@ describe("Hunk session daemon client", () => { websocket: { open(socket) { websocketOpens += 1; - socket.close(1008, "Incompatible session registration."); + setTimeout( + () => socket.close(1008, "Session broker authentication required; upgrade Hunk."), + 20, + ); }, message() {}, }, }); const messages: string[] = []; - const client = new SessionBrokerClient(createRegistration(), createSnapshot()); - let reconnectScheduled = false; - (client as any).scheduleReconnect = () => { - reconnectScheduled = true; - }; - (client as any).warnUnavailable = (error: unknown) => { - messages.push(error instanceof Error ? error.message : String(error)); + console.error = (...args: unknown[]) => { + messages.push(args.map((value) => String(value)).join(" ")); }; + const client = new SessionBrokerClient(createRegistration(), createSnapshot(), { + reconnectDelayMs: 10, + }); + const skewedRegistration = createRegistration(); + skewedRegistration.sessionId = "session-skewed"; + const skewedClient = new SessionBrokerClient(skewedRegistration, createSnapshot(), { + reconnectDelayMs: 17, + }); try { for (let attempt = 0; attempt < 20; attempt += 1) { @@ -224,30 +262,316 @@ describe("Hunk session daemon client", () => { await Bun.sleep(25); } - await (client as any).connect({ + const credentials = await loadOrCreateHunkSessionBrokerCredentials(); + const config = { host: "127.0.0.1", port, httpOrigin: `http://127.0.0.1:${port}`, wsOrigin: `ws://127.0.0.1:${port}`, - }); - await waitUntil("incompatible session warning", () => - messages.some((message) => - message.includes("too old for the refreshed session broker daemon"), - ), + }; + (client as any).credentials = credentials; + (client as any).connect(config); + await Bun.sleep(7); + (skewedClient as any).credentials = credentials; + (skewedClient as any).connect(config); + await waitUntil("both incompatible session warnings", () => messages.length === 2); + expect(messages.every((message) => message.includes("Close older Hunk windows"))).toBe(true); + await Bun.sleep(60); + expect(websocketOpens).toBe(2); + expect((client as any).waitingForIncumbentExit).toBe(true); + expect((skewedClient as any).waitingForIncumbentExit).toBe(true); + } finally { + client.stop(); + skewedClient.stop(); + server.stop(true); + } + }, 10_000); + + test("authenticates after a successor becomes healthy before the waiter observes absence", async () => { + const runtimeDir = mkdtempSync(join(tmpdir(), "hunk-missed-daemon-absence-")); + let helloAttempts = 0; + const incumbentDaemon = createSessionBrokerDaemon({ + broker: new SessionBroker({ + protocolParsers: hunkSessionProtocolParsers, + }), + appId: "dev.hunk", + appRevision: HUNK_SESSION_DAEMON_VERSION, + // This fixture rejects before endpoint binding participates in authentication. Let Bun own + // ephemeral-port selection so Windows never has to release and immediately rebind a probe. + producerEndpoint: "ws://127.0.0.1:0/session", + idleTimeoutMs: 0, + helloAuthenticator: { + async issueChallenge() { + helloAttempts += 1; + throw new Error("incompatible application revision"); + }, + async completeCallerHello() { + throw new Error("not used"); + }, + async completeProducerHello() { + throw new Error("not used"); + }, + }, + }); + const incumbent = serveBunSessionBrokerDaemon({ + daemon: incumbentDaemon, + hostname: "127.0.0.1", + port: 0, + }); + const port = incumbent.port; + if (!port) throw new Error("Expected Bun to select an ephemeral incumbent port."); + process.env.XDG_RUNTIME_DIR = runtimeDir; + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + void incumbentDaemon.stopped.then(() => incumbent.stop(true)); + let successor: Awaited> | null = null; + const client = new SessionBrokerClient(createRegistration(), createSnapshot(), { + // Leave enough time to replace the listener before this client polls again, preserving the + // missed-absence race this test exercises even when Windows delays port reuse. + reconnectDelayMs: 2_000, + }); + const metadataPath = resolveSessionBrokerRuntimePaths({ + host: "127.0.0.1", + port, + }).metadataPath; + mkdirSync(join(metadataPath, ".."), { recursive: true }); + const writeMetadata = (pid: number) => + writeFileSync( + metadataPath, + JSON.stringify({ + pid, + host: "127.0.0.1", + port, + command: "/fixture/hunk", + args: ["daemon", "serve"], + launchedAt: new Date(pid).toISOString(), + launchedByPid: pid, + launchCwd: "/fixture", + }), ); + writeMetadata(100); - expect(messages[0]).toContain( - "This window is too old for the refreshed session broker daemon.", + try { + await client.start(); + const retainedConnection = (client as any).connection; + await waitUntil("incompatible signed hello", () => helloAttempts === 1); + + incumbentDaemon.shutdown(); + await incumbentDaemon.stopped; + incumbent.stop(true); + await incumbent.stopped; + // Windows may delay reuse briefly after Bun closes a listener. The production outer retry + // handles that interval; this fixture waits inside the client's longer reconnect window. + if (process.platform === "win32") await Bun.sleep(1_000); + successor = await serveHunkSessionBrokerDaemon({ idleTimeoutMs: 0 }); + writeMetadata(200); + + await waitUntil( + "registration after missed endpoint absence", + async () => { + try { + return ( + ( + await createHttpHunkSessionCliClient({ + timeoutMs: 250, + }).listSessions() + ).length === 1 + ); + } catch { + return false; + } + }, + 5_000, + 25, ); - expect(messages[0]).toContain("Restart the window to reconnect."); - expect(reconnectScheduled).toBe(false); - expect(websocketOpens).toBe(1); + expect(helloAttempts).toBe(1); + expect((client as any).connection).toBe(retainedConnection); } finally { client.stop(); - server.stop(true); + incumbentDaemon.shutdown(); + incumbent.stop(true); + successor?.stop(true); + if (successor) await successor.stopped; + rmSync(runtimeDir, { recursive: true, force: true }); } }, 10_000); + test("waits out an incompatible incumbent and registers on its successor with one connection", async () => { + const runtimeDir = mkdtempSync(join(tmpdir(), "hunk-quiescent-upgrade-")); + const listener = createServer(); + await new Promise((resolve, reject) => { + listener.once("error", reject); + listener.listen(0, "127.0.0.1", resolve); + }); + const address = listener.address(); + const port = typeof address === "object" && address ? address.port : 0; + await new Promise((resolve) => listener.close(() => resolve())); + process.env.XDG_RUNTIME_DIR = runtimeDir; + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + + let helloAttempts = 0; + const incumbentDaemon = createSessionBrokerDaemon({ + broker: new SessionBroker({ + protocolParsers: hunkSessionProtocolParsers, + }), + appId: "dev.hunk", + appRevision: HUNK_SESSION_DAEMON_VERSION, + producerEndpoint: `ws://127.0.0.1:${port}/session`, + idleTimeoutMs: 150, + helloAuthenticator: { + async issueChallenge() { + helloAttempts += 1; + throw new Error("incompatible application revision"); + }, + async completeCallerHello() { + throw new Error("not used"); + }, + async completeProducerHello() { + throw new Error("not used"); + }, + }, + }); + const incumbent = serveBunSessionBrokerDaemon({ + daemon: incumbentDaemon, + hostname: "127.0.0.1", + port, + }); + void incumbentDaemon.stopped.then(() => incumbent.stop(true)); + let successor: Awaited> | null = null; + const client = new SessionBrokerClient(createRegistration(), createSnapshot(), { + reconnectDelayMs: 10, + }); + (client as any).ensureDaemonAvailable = async () => { + try { + if ((await fetch(`http://127.0.0.1:${port}/health`)).ok) return; + } catch { + // Launch the successor after the incumbent's short test-only quiescent lifetime. + } + successor ??= await serveHunkSessionBrokerDaemon({ idleTimeoutMs: 0 }); + }; + + try { + await client.start(); + const retainedConnection = (client as any).connection; + await waitUntil("first incompatible websocket", () => helloAttempts === 1); + await Bun.sleep(35); + expect(helloAttempts).toBe(1); + + await waitUntil( + "successor session registration", + async () => { + try { + return ( + ( + await createHttpHunkSessionCliClient({ + timeoutMs: 250, + }).listSessions() + ).length === 1 + ); + } catch { + return false; + } + }, + 5_000, + 50, + ); + expect((client as any).connection).toBe(retainedConnection); + + const firstSuccessor = successor as unknown as Awaited< + ReturnType + >; + firstSuccessor.stop(true); + await firstSuccessor.stopped; + successor = null; + await waitUntil( + "registration after a second daemon generation", + async () => { + try { + return ( + ( + await createHttpHunkSessionCliClient({ + timeoutMs: 250, + }).listSessions() + ).length === 1 + ); + } catch { + return false; + } + }, + 5_000, + 50, + ); + expect((client as any).connection).toBe(retainedConnection); + } finally { + client.stop(); + incumbent.stop(true); + const runningSuccessor = successor as Awaited< + ReturnType + > | null; + runningSuccessor?.stop(true); + if (runningSuccessor) await runningSuccessor.stopped; + rmSync(runtimeDir, { recursive: true, force: true }); + } + }, 10_000); + + test("retries the complete startup cycle and recovers without restarting the client", async () => { + const messages: string[] = []; + console.error = (...args: unknown[]) => { + messages.push(args.map((value) => String(value)).join(" ")); + }; + const client = new SessionBrokerClient(createRegistration(), createSnapshot(), { + reconnectDelayMs: 10, + }); + let attempts = 0; + (client as any).ensureDaemonAndConnect = async () => { + attempts += 1; + if (attempts === 1) throw new Error("incumbent incompatible"); + }; + + try { + await client.start(); + await waitUntil("second complete startup attempt", () => attempts === 2); + expect(messages).toEqual(["[session:broker] incumbent incompatible"]); + } finally { + client.stop(); + } + }); + + test("does no daemon work when start follows terminal stop", async () => { + const client = new SessionBrokerClient(createRegistration(), createSnapshot()); + let attempts = 0; + (client as any).ensureDaemonAndConnect = async () => { + attempts += 1; + }; + + client.stop(); + await client.start(); + expect(attempts).toBe(0); + }); + + test("does not schedule recovery after stop wins a startup race", async () => { + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + let reconnectScheduled = false; + const client = new SessionBrokerClient(createRegistration(), createSnapshot()); + (client as any).ensureDaemonAndConnect = async () => { + await gate; + throw new Error("late startup failure"); + }; + (client as any).scheduleReconnect = () => { + reconnectScheduled = true; + }; + + const startup = client.start(); + client.stop(); + release(); + await startup; + expect(reconnectScheduled).toBe(false); + }); + test("logs one actionable warning when a non-Hunk listener owns the session daemon port", async () => { const conflictingListener = createServer((_request, response) => { response.writeHead(404, { "content-type": "text/plain" }); diff --git a/src/session/broker/brokerClient.ts b/src/session/broker/brokerClient.ts index 53ab54330..3c68f385f 100644 --- a/src/session/broker/brokerClient.ts +++ b/src/session/broker/brokerClient.ts @@ -12,14 +12,16 @@ import { } from "./brokerConfig"; import { ensureSessionBrokerAvailable, - readSessionBrokerHealth, - waitForSessionBrokerShutdown, + isSessionBrokerHealthy, + readSessionBrokerLaunchFingerprint, } from "./brokerLauncher"; import { hunkSessionProtocolParsers } from "./protocolParsers"; import { - readHunkSessionDaemonCapabilities, - reportHunkDaemonUpgradeRestart, -} from "../client/capabilities"; + loadOrCreateHunkSessionBrokerCredentials, + type HunkSessionBrokerCredentials, +} from "./credentials"; +import { HUNK_SESSION_BROKER_APP_ID, HUNK_SESSION_BROKER_APP_REVISION } from "./appContract"; +import { HUNK_DAEMON_UPGRADE_WAIT_MESSAGE } from "../client/capabilities"; import type { HunkSessionCommandResult, HunkSessionInfo, @@ -31,9 +33,10 @@ const DAEMON_STARTUP_TIMEOUT_MS = 3_000; const RECONNECT_DELAY_MS = 3_000; const HEARTBEAT_INTERVAL_MS = 10_000; const INCOMPATIBLE_SESSION_CLOSE_CODE = 1008; -const INCOMPATIBLE_SESSION_CLOSE_REASON_PREFIX = "Incompatible session "; -const INCOMPATIBLE_SESSION_CLOSE_MESSAGE = - "This window is too old for the refreshed session broker daemon. Restart the window to reconnect."; +const QUIESCENT_REFUSAL_REASONS = new Set([ + "Session broker authentication required; upgrade Hunk.", + "Malformed session broker protocol.", +]); type SessionAppBridge = SessionBrokerConnectionBridge< HunkSessionServerMessage, @@ -45,6 +48,19 @@ interface SessionBrokerClientTiming { reconnectDelayMs?: number; } +/** Identify only known compatibility refusals before producer activation. */ +export function isQuiescentUpgradeRefusal(event: { + code: number; + reason: string; + authenticated?: boolean; +}) { + return ( + event.authenticated === false && + event.code === INCOMPATIBLE_SESSION_CLOSE_CODE && + QUIESCENT_REFUSAL_REASONS.has(event.reason) + ); +} + /** The concrete broker client bound to Hunk's session contracts. */ export type HunkSessionBrokerClient = SessionBrokerClient; @@ -62,6 +78,9 @@ export class SessionBrokerClient { private stopped = false; private startupPromise: Promise | null = null; private lastConnectionWarning: string | null = null; + private credentials: HunkSessionBrokerCredentials | null = null; + private waitingForIncumbentExit = false; + private incumbentLaunchFingerprint: string | null = null; constructor( private registration: SessionRegistration, @@ -70,7 +89,7 @@ export class SessionBrokerClient { ) {} start() { - if (process.env.HUNK_MCP_DISABLE === "1") { + if (this.stopped || process.env.HUNK_MCP_DISABLE === "1") { return; } @@ -127,6 +146,7 @@ export class SessionBrokerClient { private async ensureDaemonAndConnect() { const config = this.resolveConfig(); await this.ensureDaemonAvailable(config); + this.credentials ??= await loadOrCreateHunkSessionBrokerCredentials(); this.connect(config); } @@ -136,59 +156,8 @@ export class SessionBrokerClient { timeoutMs: this.timing.daemonStartupTimeoutMs ?? DAEMON_STARTUP_TIMEOUT_MS, }); - const capabilities = await readHunkSessionDaemonCapabilities(config); - if (!capabilities) { - await this.restartIncompatibleDaemon(config); - await ensureSessionBrokerAvailable({ - config, - timeoutMs: this.timing.daemonStartupTimeoutMs ?? DAEMON_STARTUP_TIMEOUT_MS, - }); - - if (!(await readHunkSessionDaemonCapabilities(config))) { - throw new Error( - "The running session broker daemon is incompatible with this build. " + - "Restart the app so it can launch a fresh daemon from the current source tree.", - ); - } - } - - this.lastConnectionWarning = null; - } - - private async restartIncompatibleDaemon(config: ResolvedSessionBrokerConfig) { - reportHunkDaemonUpgradeRestart(); - const health = await readSessionBrokerHealth(config); - const pid = health?.pid; - if (pid === process.pid) { - throw new Error( - "The running session broker daemon is incompatible with this build. " + - "Restart the app so it can launch a fresh daemon from the current source tree.", - ); - } - - // If the stale daemon already disappeared on its own, let the normal startup path launch a - // fresh one instead of turning that race into a manual restart error. - if (!pid) { - return; - } - - try { - process.kill(pid, "SIGTERM"); - } catch (error) { - if (!(error instanceof Error) || !("code" in error) || error.code !== "ESRCH") { - throw error; - } - } - - const shutDown = await waitForSessionBrokerShutdown({ - config, - timeoutMs: DAEMON_STARTUP_TIMEOUT_MS, - }); - if (!shutDown) { - throw new Error( - "Stopped waiting for the old session broker daemon to exit after it was found incompatible.", - ); - } + // Minimal health proves only liveness. Compatibility and identity are established by the + // signed websocket hello; an unverifiable incumbent is never signalled or replaced by PID. } setBridge(bridge: SessionAppBridge | null) { @@ -206,7 +175,8 @@ export class SessionBrokerClient { return; } - this.connection = createSessionBrokerConnection< + if (!this.credentials) return; + const connection = createSessionBrokerConnection< HunkSessionInfo, HunkSessionState, SessionBrokerSocketLike, @@ -219,16 +189,54 @@ export class SessionBrokerClient { snapshot: this.snapshot, bridge: this.bridge, protocolParsers: hunkSessionProtocolParsers, + producerAuthentication: { + appId: HUNK_SESSION_BROKER_APP_ID, + appRevision: HUNK_SESSION_BROKER_APP_REVISION, + credential: this.credentials.producer, + daemon: { + keyId: this.credentials.daemonIdentity.keyId, + publicKey: this.credentials.daemonPublicKey, + }, + }, heartbeatIntervalMs: HEARTBEAT_INTERVAL_MS, reconnectDelayMs: this.timing.reconnectDelayMs ?? RECONNECT_DELAY_MS, - resolveClose: (event) => - this.isIncompatibleSessionClose(event) - ? { reconnect: false, warning: INCOMPATIBLE_SESSION_CLOSE_MESSAGE } - : { reconnect: true }, + prepareReconnect: async () => { + if (this.waitingForIncumbentExit) { + const healthy = await isSessionBrokerHealthy(config); + if (healthy) { + const currentFingerprint = readSessionBrokerLaunchFingerprint(config); + // Owner-private metadata is only a generation-change hint. The signed hello remains the + // sole compatibility and identity authority, and unchanged/malformed metadata causes + // health-only polling so skewed waiters cannot keep the incumbent active. + if (currentFingerprint === this.incumbentLaunchFingerprint) { + throw new Error(HUNK_DAEMON_UPGRADE_WAIT_MESSAGE); + } + } + this.waitingForIncumbentExit = false; + } + await this.ensureDaemonAvailable(config); + }, + resolveClose: (event) => { + const preAuthenticationRefusal = isQuiescentUpgradeRefusal(event); + if (preAuthenticationRefusal) { + this.waitingForIncumbentExit = true; + this.incumbentLaunchFingerprint = readSessionBrokerLaunchFingerprint(config); + } + return { + reconnect: true, + ...(preAuthenticationRefusal ? { warning: HUNK_DAEMON_UPGRADE_WAIT_MESSAGE } : {}), + }; + }, + onConnected: () => { + this.waitingForIncumbentExit = false; + this.incumbentLaunchFingerprint = null; + this.lastConnectionWarning = null; + }, onWarning: (message) => this.warnUnavailable(message), }); - this.connection.start(); + this.connection = connection; + connection.start(); } private scheduleReconnect(delayMs = this.timing.reconnectDelayMs ?? RECONNECT_DELAY_MS) { @@ -243,17 +251,13 @@ export class SessionBrokerClient { this.reconnectTimer.unref?.(); } - /** Return whether the daemon explicitly rejected this session as incompatible after an upgrade. */ - private isIncompatibleSessionClose(event: { code: number; reason: string }) { - return ( - event.code === INCOMPATIBLE_SESSION_CLOSE_CODE && - event.reason.startsWith(INCOMPATIBLE_SESSION_CLOSE_REASON_PREFIX) - ); - } - private warnUnavailable(error: unknown) { const message = - error instanceof Error ? error.message : "Unknown session broker connection error."; + error instanceof Error + ? error.message + : typeof error === "string" + ? error + : "Unknown session broker connection error."; if (message === this.lastConnectionWarning) { return; } diff --git a/src/session/broker/brokerConfig.test.ts b/src/session/broker/brokerConfig.test.ts index e782f75f1..7eb925a4b 100644 --- a/src/session/broker/brokerConfig.test.ts +++ b/src/session/broker/brokerConfig.test.ts @@ -1,4 +1,11 @@ import { describe, expect, test } from "bun:test"; +import { HUNK_SESSION_DAEMON_VERSION } from "../protocol"; +import { + HUNK_SESSION_BROKER_APP_ID, + HUNK_SESSION_BROKER_APP_REVISION, + HUNK_SESSION_BROKER_FEATURES, + HUNK_SESSION_BROKER_REVISION, +} from "./appContract"; import { DEFAULT_SESSION_BROKER_HOST, DEFAULT_SESSION_BROKER_PORT, @@ -11,6 +18,14 @@ import { } from "./brokerConfig"; describe("Hunk session daemon config", () => { + test("exports one fixed Phase-1 Hunk contract", () => { + expect(HUNK_SESSION_BROKER_APP_ID).toBe("dev.hunk"); + expect(HUNK_SESSION_BROKER_REVISION).toBe(1); + expect(HUNK_SESSION_BROKER_APP_REVISION).toBe(HUNK_SESSION_DAEMON_VERSION); + expect(HUNK_SESSION_BROKER_FEATURES).toEqual([]); + expect(Object.isFrozen(HUNK_SESSION_BROKER_FEATURES)).toBe(true); + }); + test("resolves exported host and port metadata as runtime defaults", () => { expect(resolveSessionBrokerConfig({})).toMatchObject({ host: DEFAULT_SESSION_BROKER_HOST, diff --git a/src/session/broker/brokerLauncher.test.ts b/src/session/broker/brokerLauncher.test.ts index e3b9d6097..c544d2689 100644 --- a/src/session/broker/brokerLauncher.test.ts +++ b/src/session/broker/brokerLauncher.test.ts @@ -7,6 +7,7 @@ import { ensureSessionBrokerAvailable, isLoopbackPortReachable, parseSessionBrokerHealth, + readSessionBrokerLaunchFingerprint, resolveDaemonLaunchCommand, resolveSessionBrokerRuntimePaths, } from "./brokerLauncher"; @@ -35,6 +36,39 @@ afterEach(() => { }); describe("session daemon launcher", () => { + test("reads only bounded exact launch metadata as a generation hint", () => { + const runtime = createRuntimeDir(); + const env = { ...process.env, XDG_RUNTIME_DIR: runtime }; + const paths = resolveSessionBrokerRuntimePaths(testConfig, env); + mkdirSync(paths.runtimeDir, { recursive: true }); + const metadata = { + pid: 123, + host: testConfig.host, + port: testConfig.port, + command: "/fixture/hunk", + args: ["daemon", "serve"], + launchedAt: "2026-01-01T00:00:00.000Z", + launchedByPid: 122, + launchCwd: "/fixture", + }; + writeFileSync(paths.metadataPath, JSON.stringify(metadata)); + const first = readSessionBrokerLaunchFingerprint(testConfig, env); + expect(first).toBe(JSON.stringify(metadata)); + writeFileSync(paths.metadataPath, JSON.stringify({ ...metadata, pid: 124 })); + expect(readSessionBrokerLaunchFingerprint(testConfig, env)).not.toBe(first); + for (const malformed of [[], { ...metadata, extra: true }, { ...metadata, args: {} }]) { + writeFileSync(paths.metadataPath, JSON.stringify(malformed)); + expect(readSessionBrokerLaunchFingerprint(testConfig, env)).toBeNull(); + } + writeFileSync( + paths.metadataPath, + JSON.stringify(metadata).replace('{"pid"', '{"__proto__":true,"pid"'), + ); + expect(readSessionBrokerLaunchFingerprint(testConfig, env)).toBeNull(); + writeFileSync(paths.metadataPath, "x".repeat(16 * 1024 + 1)); + expect(readSessionBrokerLaunchFingerprint(testConfig, env)).toBeNull(); + }); + test("strictly parses minimal and legacy health responses", () => { expect(parseSessionBrokerHealth({ ok: true })).toEqual({ ok: true }); expect( diff --git a/src/session/broker/brokerLauncher.ts b/src/session/broker/brokerLauncher.ts index 3976484df..5149ea228 100644 --- a/src/session/broker/brokerLauncher.ts +++ b/src/session/broker/brokerLauncher.ts @@ -2,7 +2,7 @@ import { spawn } from "node:child_process"; import type { ChildProcess } from "node:child_process"; import { existsSync, mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { connect } from "node:net"; -import { tmpdir } from "node:os"; +import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; import { parseBrokerSafeInteger, @@ -15,6 +15,7 @@ const SCRIPT_ENTRYPOINT_PATTERN = /[\\/]|\.(?:[cm]?js|tsx?)$/; const DEFAULT_DAEMON_LOCK_STALE_MS = 15_000; const DEFAULT_DAEMON_STARTUP_TIMEOUT_MS = 3_000; const DEFAULT_DAEMON_HEALTH_POLL_INTERVAL_MS = 100; +const MAX_DAEMON_LAUNCH_METADATA_BYTES = 16 * 1024; export interface DaemonLaunchCommand { command: string; @@ -93,7 +94,11 @@ function safeRuntimeToken(value: string) { } function resolveRuntimeBaseDir(env: NodeJS.ProcessEnv = process.env) { - return env.XDG_RUNTIME_DIR?.trim() || tmpdir(); + const configured = env.XDG_RUNTIME_DIR?.trim(); + if (configured) return configured; + // Unix temporary directories are commonly shared across users. Keep the fallback beneath the + // current home directory instead of a predictable shared-/tmp name another account can pre-own. + return typeof process.getuid === "function" ? join(homedir(), ".hunk") : tmpdir(); } function isRunningPid(pid: number) { @@ -117,6 +122,41 @@ function readJsonFile(path: string) { } } +/** Parse exact launch metadata used only as a change-detection hint across daemon generations. */ +function parseSessionBrokerLaunchMetadata(value: unknown): SessionBrokerLaunchMetadata | null { + try { + const record = parseExactBrokerRecord(value, [ + "pid", + "host", + "port", + "command", + "args", + "launchedAt", + "launchedByPid", + "launchCwd", + ] as const); + if (!Array.isArray(record.args)) return null; + const args = record.args.map((argument) => parseBrokerString(argument)); + return { + pid: parseBrokerSafeInteger(record.pid, { minimum: 1 }), + host: parseBrokerString(record.host), + port: parseBrokerSafeInteger(record.port, { + minimum: 1, + maximum: 65_535, + }), + command: parseBrokerString(record.command), + args, + launchedAt: parseBrokerString(record.launchedAt), + launchedByPid: parseBrokerSafeInteger(record.launchedByPid, { + minimum: 1, + }), + launchCwd: parseBrokerString(record.launchCwd), + }; + } catch { + return null; + } +} + function removeFileIfPresent(path: string) { try { rmSync(path, { force: true }); @@ -146,7 +186,7 @@ function tryAcquireDaemonLaunchLock({ staleAfterMs: number; }): SessionBrokerLaunchLock | null { const paths = resolveSessionBrokerRuntimePaths(config, env); - mkdirSync(paths.runtimeDir, { recursive: true }); + mkdirSync(paths.runtimeDir, { recursive: true, mode: 0o700 }); const payload: SessionBrokerLaunchLockFile = { ownerPid: process.pid, @@ -376,6 +416,27 @@ export function parseSessionBrokerHealth(value: unknown): SessionBrokerHealth | } } +/** Read a bounded exact metadata fingerprint as a reconnect hint, never process authority. */ +export function readSessionBrokerLaunchFingerprint( + config: Pick = resolveSessionBrokerConfig(), + env: NodeJS.ProcessEnv = process.env, +) { + const { metadataPath } = resolveSessionBrokerRuntimePaths(config, env); + try { + const stat = statSync(metadataPath); + if (!stat.isFile() || stat.size <= 0 || stat.size > MAX_DAEMON_LAUNCH_METADATA_BYTES) + return null; + const bytes = readFileSync(metadataPath); + if (bytes.byteLength !== stat.size || bytes.byteLength > MAX_DAEMON_LAUNCH_METADATA_BYTES) { + return null; + } + const metadata = parseSessionBrokerLaunchMetadata(JSON.parse(bytes.toString("utf8"))); + return metadata ? JSON.stringify(metadata) : null; + } catch { + return null; + } +} + /** Read the daemon's health payload when one is reachable on the configured loopback port. */ export async function readSessionBrokerHealth( config: ResolvedSessionBrokerConfig = resolveSessionBrokerConfig(), @@ -439,29 +500,6 @@ export function isLoopbackPortReachable( }); } -/** Wait for the running daemon to stop responding on its health endpoint. */ -export async function waitForSessionBrokerShutdown({ - config = resolveSessionBrokerConfig(), - timeoutMs = 3_000, - intervalMs = 100, -}: { - config?: ResolvedSessionBrokerConfig; - timeoutMs?: number; - intervalMs?: number; -} = {}) { - const deadline = Date.now() + timeoutMs; - - while (Date.now() < deadline) { - if (!(await isSessionBrokerHealthy(config))) { - return true; - } - - await Bun.sleep(intervalMs); - } - - return false; -} - /** Launch the broker daemon in the background without tying it to the current TTY session. */ export function launchSessionBrokerDaemon({ cwd = process.cwd(), diff --git a/src/session/broker/brokerServer.helpers.test.ts b/src/session/broker/brokerServer.helpers.test.ts index d43f3f70e..9a5d3f7f1 100644 --- a/src/session/broker/brokerServer.helpers.test.ts +++ b/src/session/broker/brokerServer.helpers.test.ts @@ -78,8 +78,8 @@ describe("parseHostAndPort", () => { expect(parseHostAndPort("[::1]:0")).toBeNull(); }); - test("tolerates an unbracketed IPv6 literal by dropping the port", () => { - expect(parseHostAndPort("::1")).toEqual({ host: "::1", port: undefined }); + test("rejects ambiguous unbracketed IPv6 authorities", () => { + expect(parseHostAndPort("::1")).toBeNull(); }); }); diff --git a/src/session/broker/brokerServer.test.ts b/src/session/broker/brokerServer.test.ts index 7bdd791af..b042ec437 100644 --- a/src/session/broker/brokerServer.test.ts +++ b/src/session/broker/brokerServer.test.ts @@ -7,8 +7,19 @@ import { createTestSessionSnapshot, } from "../../../test/helpers/session-daemon-fixtures"; import { SessionBrokerState } from "@hunk/session-broker-core"; +import { + SessionBrokerCallerClient, + answerSessionBrokerHelloChallenge, + createSessionBrokerHelloRequest, + verifyProducerHelloAck, + type SessionBrokerHelloChallenge, + type SessionBrokerProducerHelloAck, + type SessionBrokerSignedRequestInit, +} from "@hunk/session-broker"; import { HUNK_SESSION_API_VERSION, HUNK_SESSION_DAEMON_VERSION } from "../protocol"; import { serveSessionBrokerDaemon } from "./brokerServer"; +import { loadOrCreateHunkSessionBrokerCredentials } from "./credentials"; +import { HUNK_SESSION_BROKER_APP_ID, HUNK_SESSION_BROKER_APP_REVISION } from "./appContract"; const originalHost = process.env.HUNK_MCP_HOST; const originalPort = process.env.HUNK_MCP_PORT; @@ -16,9 +27,9 @@ const originalUnsafeRemote = process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE; interface HealthResponse { ok: boolean; - pid: number; - sessions: number; - pendingCommands: number; + pid?: number; + sessions?: number; + pendingCommands?: number; paths?: Record; sessionApi?: string; sessionCapabilities?: string; @@ -85,10 +96,41 @@ async function waitForShutdown(port: number, timeoutMs = 1_500) { ); } +async function authenticatedFetch( + port: number, + path: string, + init: SessionBrokerSignedRequestInit = {}, +) { + const credentials = await loadOrCreateHunkSessionBrokerCredentials(); + const caller = new SessionBrokerCallerClient({ + appId: HUNK_SESSION_BROKER_APP_ID, + appRevision: HUNK_SESSION_BROKER_APP_REVISION, + origin: `http://127.0.0.1:${port}`, + credential: credentials.caller, + daemon: { keyId: credentials.daemonIdentity.keyId, publicKey: credentials.daemonPublicKey }, + }); + const action = + typeof init.body === "string" + ? ((JSON.parse(init.body) as { action?: string }).action ?? "") + : ""; + return caller.request(path, init, { + targetSpecific: path === "/session-api" && action !== "list", + }); +} + async function waitForSessionCount(port: number, count: number) { await waitUntil("session registration", async () => { - const health = await readHealth(port); - return health?.sessions === count ? health : null; + try { + const response = await authenticatedFetch(port, "/session-api", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ action: "list" }), + }); + const body = (await response.json()) as { sessions?: unknown[] }; + return body.sessions?.length === count ? body : null; + } catch { + return null; + } }); } @@ -169,6 +211,49 @@ async function openRegisteredSession( snapshotOverrides: Parameters[0] = {}, ) { const socket = await openSessionSocket(port); + const credentials = await loadOrCreateHunkSessionBrokerCredentials(); + const options = { + appId: HUNK_SESSION_BROKER_APP_ID, + appRevision: HUNK_SESSION_BROKER_APP_REVISION, + endpoint: `ws://127.0.0.1:${port}/session`, + credential: credentials.producer, + daemon: { keyId: credentials.daemonIdentity.keyId, publicKey: credentials.daemonPublicKey }, + }; + const hello = createSessionBrokerHelloRequest(options); + socket.send(JSON.stringify({ type: "hello-init", hello })); + const challenge = await new Promise((resolve, reject) => { + const timeout = setTimeout( + () => reject(new Error("Timed out waiting for producer challenge.")), + 1_000, + ); + socket.addEventListener( + "message", + (event) => { + clearTimeout(timeout); + resolve( + (JSON.parse(String(event.data)) as { challenge: SessionBrokerHelloChallenge }).challenge, + ); + }, + { once: true }, + ); + }); + const pending = await answerSessionBrokerHelloChallenge(options, hello, challenge); + socket.send(JSON.stringify({ type: "hello-proof", proof: pending.proof })); + const ack = await new Promise((resolve, reject) => { + const timeout = setTimeout( + () => reject(new Error("Timed out waiting for producer acknowledgement.")), + 1_000, + ); + socket.addEventListener( + "message", + (event) => { + clearTimeout(timeout); + resolve((JSON.parse(String(event.data)) as { ack: SessionBrokerProducerHelloAck }).ack); + }, + { once: true }, + ); + }); + await verifyProducerHelloAck(pending, ack); socket.send( JSON.stringify({ @@ -228,12 +313,12 @@ afterEach(() => { }); describe("Hunk session daemon server", () => { - test("refuses non-loopback binding unless explicitly allowed", () => { + test("refuses non-loopback binding unless explicitly allowed", async () => { process.env.HUNK_MCP_HOST = "0.0.0.0"; process.env.HUNK_MCP_PORT = "47657"; delete process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE; - expect(() => serveSessionBrokerDaemon()).toThrow("local-only by default"); + await expect(serveSessionBrokerDaemon()).rejects.toThrow("local-only by default"); }); test("reports a clear error when the daemon port is already in use", async () => { @@ -249,7 +334,7 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_PORT = String(port); try { - expect(() => serveSessionBrokerDaemon()).toThrow("port is already in use"); + await expect(serveSessionBrokerDaemon()).rejects.toThrow("port is already in use"); } finally { await new Promise((resolve) => listener.close(() => resolve())); } @@ -260,21 +345,13 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon(); + const server = await serveSessionBrokerDaemon(); try { const health = await fetch(`http://127.0.0.1:${port}/health`); expect(health.status).toBe(200); const healthPayload = (await health.json()) as HealthResponse; - expect(healthPayload.paths).toEqual({ - health: "/health", - socket: "/session", - }); - expect(healthPayload).toMatchObject({ - sessionApi: `http://127.0.0.1:${port}/session-api`, - sessionCapabilities: `http://127.0.0.1:${port}/session-api/capabilities`, - sessionSocket: `ws://127.0.0.1:${port}/session`, - }); + expect(healthPayload).toEqual({ ok: true }); const genericCapabilities = await fetch(`http://127.0.0.1:${port}/broker/capabilities`); expect(genericCapabilities.status).toBe(404); @@ -288,7 +365,7 @@ describe("Hunk session daemon server", () => { }); expect(genericBroker.status).toBe(404); - const capabilities = await fetch(`http://127.0.0.1:${port}/session-api/capabilities`); + const capabilities = await authenticatedFetch(port, "/session-api/capabilities"); expect(capabilities.status).toBe(200); await expect(capabilities.json()).resolves.toMatchObject({ version: HUNK_SESSION_API_VERSION, @@ -326,12 +403,41 @@ describe("Hunk session daemon server", () => { } }); + test("keeps generic caller and browser-review authority independent", async () => { + const port = await reserveLoopbackPort(); + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + const server = await serveSessionBrokerDaemon(); + try { + await expect(authenticatedFetch(port, "/review-api/missing/publication")).rejects.toThrow( + "daemon identity could not be verified", + ); + const genericHeadersWithoutReviewCapability = await fetch( + `http://127.0.0.1:${port}/review-api/missing/publication`, + { headers: { "x-session-broker-caller-session": "generic-only" } }, + ); + expect(genericHeadersWithoutReviewCapability.status).toBe(401); + + const reviewCapabilityOnSession = await fetch(`http://127.0.0.1:${port}/session-api`, { + method: "POST", + headers: { + "content-type": "application/json", + "hunk-review-capability": "review-only-capability", + }, + body: JSON.stringify({ action: "list" }), + }); + expect(reviewCapabilityOnSession.status).toBe(401); + } finally { + server.stop(true); + } + }); + 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(); + const server = await serveSessionBrokerDaemon(); try { const attackerHostResponse = await fetch(`http://127.0.0.1:${port}/health`, { @@ -361,7 +467,7 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon(); + const server = await serveSessionBrokerDaemon(); try { const response = await fetch(`http://127.0.0.1:${port}/session-api/capabilities`, { @@ -379,15 +485,35 @@ describe("Hunk session daemon server", () => { } }); + test("requires GET with an empty body for authenticated Hunk capabilities", async () => { + const port = await reserveLoopbackPort(); + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + const server = await serveSessionBrokerDaemon(); + try { + const wrongMethod = await authenticatedFetch(port, "/session-api/capabilities", { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); + expect(wrongMethod.status).toBe(405); + await expect(wrongMethod.json()).resolves.toEqual({ + error: "Capabilities require GET with an empty body.", + }); + } 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(); + const server = await serveSessionBrokerDaemon(); try { - const response = await fetch(`http://127.0.0.1:${port}/session-api`, { + const response = await authenticatedFetch(port, "/session-api", { method: "POST", headers: { "content-type": "text/plain" }, body: JSON.stringify({ action: "list" }), @@ -407,18 +533,22 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon(); + const server = await serveSessionBrokerDaemon(); try { const response = await fetch(`http://127.0.0.1:${port}/session-api`, { method: "POST", - headers: { "content-type": "application/json" }, + headers: { + "content-type": "application/json", + "x-session-broker-caller-session": "oversized-test-session", + }, body: JSON.stringify({ action: "list", filler: "x".repeat(5 * 1024 * 1024) }), }); expect(response.status).toBe(413); await expect(response.json()).resolves.toMatchObject({ - error: expect.stringContaining("session broker limit"), + error: "capacity-exceeded", + resource: "maxHttpBodyBytes", }); } finally { server.stop(true); @@ -436,7 +566,7 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon({ + const server = await serveSessionBrokerDaemon({ idleTimeoutMs: 250, staleSessionTtlMs: 500, staleSessionSweepIntervalMs: 25, @@ -455,7 +585,7 @@ describe("Hunk session daemon server", () => { await expect(closed).resolves.toEqual({ code: 1008, - reason: "Session ownership rejected.", + reason: "Session broker authentication required; upgrade Hunk.", }); } finally { socket.close(); @@ -468,7 +598,7 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon({ + const server = await serveSessionBrokerDaemon({ idleTimeoutMs: 250, staleSessionTtlMs: 500, staleSessionSweepIntervalMs: 25, @@ -498,7 +628,7 @@ describe("Hunk session daemon server", () => { 1_000, ); - const emptyList = await fetch(`http://127.0.0.1:${port}/session-api`, { + const emptyList = await authenticatedFetch(port, "/session-api", { method: "POST", headers: { "content-type": "application/json", @@ -510,7 +640,7 @@ describe("Hunk session daemon server", () => { const goodSocket = await openRegisteredSession(port, "session-good"); try { - const response = await fetch(`http://127.0.0.1:${port}/session-api`, { + const response = await authenticatedFetch(port, "/session-api", { method: "POST", headers: { "content-type": "application/json", @@ -536,7 +666,7 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon({ + const server = await serveSessionBrokerDaemon({ idleTimeoutMs: 60, staleSessionTtlMs: 500, staleSessionSweepIntervalMs: 25, @@ -545,10 +675,7 @@ describe("Hunk session daemon server", () => { try { await Bun.sleep(150); - await expect(waitForHealth(port)).resolves.toMatchObject({ - ok: true, - sessions: 1, - }); + await expect(waitForHealth(port)).resolves.toEqual({ ok: true }); } finally { socket.close(); server.stop(true); @@ -560,7 +687,7 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon({ + const server = await serveSessionBrokerDaemon({ idleTimeoutMs: 75, staleSessionTtlMs: 500, staleSessionSweepIntervalMs: 25, @@ -582,7 +709,7 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon({ + const server = await serveSessionBrokerDaemon({ idleTimeoutMs: 75, staleSessionTtlMs: 80, staleSessionSweepIntervalMs: 20, @@ -658,10 +785,10 @@ describe("Hunk session daemon server", () => { }; }; - const server = serveSessionBrokerDaemon(); + const server = await serveSessionBrokerDaemon(); try { - const response = await fetch(`http://127.0.0.1:${port}/session-api`, { + const response = await authenticatedFetch(port, "/session-api", { method: "POST", headers: { "content-type": "application/json", @@ -700,7 +827,7 @@ describe("Hunk session daemon server", () => { SessionBrokerState.prototype.dispatchCommand = (({ command, input }: any) => { expect(command).toBe("reload_session"); expect(input).toMatchObject({ - sessionPath: "/tmp/live-session", + sessionId: "session-1", sourcePath: "/tmp/source-repo", nextInput: { kind: "vcs", @@ -719,17 +846,17 @@ describe("Hunk session daemon server", () => { }); }) as SessionBrokerState["dispatchCommand"]; - const server = serveSessionBrokerDaemon(); + const server = await serveSessionBrokerDaemon(); try { - const response = await fetch(`http://127.0.0.1:${port}/session-api`, { + const response = await authenticatedFetch(port, "/session-api", { method: "POST", headers: { "content-type": "application/json", }, body: JSON.stringify({ action: "reload", - selector: { sessionPath: "/tmp/live-session" }, + selector: { sessionId: "session-1" }, sourcePath: "/tmp/source-repo", nextInput: { kind: "vcs", @@ -758,7 +885,7 @@ describe("Hunk session daemon server", () => { process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); - const server = serveSessionBrokerDaemon(); + const server = await serveSessionBrokerDaemon(); const socket = await openRegisteredSession(port, "session-1", { reviewNoteCount: 2, reviewNotes: [ @@ -783,7 +910,7 @@ describe("Hunk session daemon server", () => { }); try { - const listResponse = await fetch(`http://127.0.0.1:${port}/session-api`, { + const listResponse = await authenticatedFetch(port, "/session-api", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ @@ -853,10 +980,10 @@ describe("Hunk session daemon server", () => { }); }) as SessionBrokerState["dispatchCommand"]; - const server = serveSessionBrokerDaemon(); + const server = await serveSessionBrokerDaemon(); try { - const response = await fetch(`http://127.0.0.1:${port}/session-api`, { + const response = await authenticatedFetch(port, "/session-api", { method: "POST", headers: { "content-type": "application/json", diff --git a/src/session/broker/brokerServer.ts b/src/session/broker/brokerServer.ts index fb3c6972a..b2b0406cc 100644 --- a/src/session/broker/brokerServer.ts +++ b/src/session/broker/brokerServer.ts @@ -1,4 +1,9 @@ -import { createSessionBrokerDaemon, type SessionBrokerController } from "@hunk/session-broker"; +import { + SessionBrokerAuthenticator, + createSessionBrokerDaemon, + type SessionBrokerAuthenticatedControlFacts, + type SessionBrokerController, +} from "@hunk/session-broker"; import { serveSessionBrokerDaemon as serveSessionBrokerDaemonWithBun, type RunningSessionBrokerDaemon as RunningBunSessionBrokerDaemon, @@ -45,6 +50,8 @@ import { import { MAX_HUNK_REVIEW_ENVELOPE_BYTES } from "../reviewProtocol"; import { parseSessionDaemonRequest } from "../protocolSchemas"; import { hunkSessionProtocolParsers } from "./protocolParsers"; +import { loadOrCreateHunkSessionBrokerCredentials } from "./credentials"; +import { HUNK_SESSION_BROKER_APP_ID, HUNK_SESSION_BROKER_APP_REVISION } from "./appContract"; const DEFAULT_STALE_SESSION_TTL_MS = 45_000; const DEFAULT_STALE_SESSION_SWEEP_INTERVAL_MS = 15_000; @@ -119,7 +126,7 @@ function hasJsonContentType(request: Request) { /** Parse a Host-style value into hostname and optional port pieces. */ export function parseHostAndPort(value: string) { const trimmed = value.trim(); - if (!trimmed) { + if (!trimmed || trimmed.includes(",")) { return null; } @@ -139,8 +146,10 @@ export function parseHostAndPort(value: string) { return null; } - const port = Number.parseInt(rest.slice(1), 10); - return Number.isInteger(port) && port > 0 ? { host, port } : null; + const rawPort = rest.slice(1); + if (!/^[0-9]+$/.test(rawPort)) return null; + const port = Number(rawPort); + return Number.isInteger(port) && port > 0 && port <= 65_535 ? { host, port } : null; } const colonCount = [...trimmed].filter((character) => character === ":").length; @@ -150,13 +159,14 @@ export function parseHostAndPort(value: string) { if (colonCount === 1) { const [host, rawPort] = trimmed.split(":"); - const port = Number.parseInt(rawPort ?? "", 10); - return host && Number.isInteger(port) && port > 0 ? { host, port } : null; + if (!host || !/^[0-9]+$/.test(rawPort ?? "")) return null; + const port = Number(rawPort); + return Number.isInteger(port) && port > 0 && port <= 65_535 ? { 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 }; + // URL authorities require brackets around IPv6 literals; accepting another spelling would make + // listener-derived authority comparison ambiguous. + return null; } /** Return whether a parsed authority targets an accepted broker host and port. */ @@ -192,6 +202,9 @@ export function validateOriginHeader(request: Request, expectedPort: number, all if (!origin) { return null; } + if (origin === "null" || origin.includes(",")) { + return jsonError("Origin is not allowed for the local session broker.", 403); + } let url: URL; try { @@ -200,7 +213,15 @@ export function validateOriginHeader(request: Request, expectedPort: number, all return jsonError("Origin is not allowed for the local session broker.", 403); } - if (url.protocol !== "http:" && url.protocol !== "https:") { + if ( + (url.protocol !== "http:" && url.protocol !== "https:") || + url.username || + url.password || + url.pathname !== "/" || + url.search || + url.hash || + url.origin !== origin + ) { return jsonError("Origin is not allowed for the local session broker.", 403); } @@ -281,10 +302,43 @@ function resolveNavigateCommandInput( }; } +/** Map each Hunk action to the generic operation and exact producer command scope it requires. */ +function sessionApiAuthorizationFacts( + state: HunkSessionBrokerState, + bytes: Uint8Array, +): SessionBrokerAuthenticatedControlFacts { + const input = parseJsonRequestBytes(bytes); + if (input.action === "list") return { operation: "list", targetSpecific: false }; + const sessionId = input.selector.sessionId ?? state.getSession(input.selector).sessionId; + if (["get", "context", "review", "comment-list"].includes(input.action)) { + return { operation: "get", sessionId, targetSpecific: true }; + } + const commandByAction = { + navigate: "navigate_to_hunk", + reload: "reload_session", + "comment-add": "comment", + "comment-apply": "comment_batch", + "comment-rm": "remove_comment", + "comment-clear": "clear_comments", + "highlight-add": "highlight", + "highlight-clear": "clear_highlights", + } as const; + const command = commandByAction[input.action as keyof typeof commandByAction]; + if (!command) throw new Error("Unknown session API action."); + return { + operation: "dispatch", + sessionId, + command, + commandVersion: 1, + targetSpecific: true, + }; +} + export async function handleSessionApiRequest( state: HunkSessionBrokerState, request: Request, bodyBytes?: Uint8Array, + resolvedSessionId?: string, ) { if (request.method !== "POST") { return jsonError("Session API requests must use POST.", 405); @@ -295,9 +349,13 @@ export async function handleSessionApiRequest( } try { - const input = parseJsonRequestBytes( + const parsedInput = parseJsonRequestBytes( bodyBytes ?? (await readRequestBytesWithLimit(request, MAX_HTTP_BODY_BYTES)), ); + const input: SessionDaemonRequest = + resolvedSessionId && parsedInput.action !== "list" + ? { ...parsedInput, selector: { sessionId: resolvedSessionId } } + : parsedInput; let response: SessionDaemonResponse; switch (input.action) { @@ -499,8 +557,8 @@ function createHunkBrokerController( getSessionIds: () => state.listSessions().map((session) => session.sessionId), getSessionCount: () => state.getSessionCount(), getPendingCommandCount: () => state.getPendingCommandCount(), - registerSession: (connection, registrationInput, snapshotInput) => - state.registerSession(connection, registrationInput, snapshotInput), + registerSession: (connection, registrationInput, snapshotInput, options) => + state.registerSession(connection, registrationInput, snapshotInput, options), updateSnapshot: (connection, sessionId, snapshotInput) => state.updateSnapshot(connection, sessionId, snapshotInput), markSessionSeen: (connection, sessionId) => state.markSessionSeen(connection, sessionId), @@ -516,9 +574,9 @@ function createHunkBrokerController( } /** Serve the local session broker daemon and websocket broker transport. */ -export function serveSessionBrokerDaemon( +export async function serveSessionBrokerDaemon( options: ServeSessionBrokerDaemonOptions = {}, -): RunningSessionBrokerDaemon { +): Promise { const config = resolveSessionBrokerConfig(); const allowRemote = allowsUnsafeRemoteSessionBroker(); const idleTimeoutMs = options.idleTimeoutMs ?? DEFAULT_IDLE_TIMEOUT_MS; @@ -526,6 +584,18 @@ export function serveSessionBrokerDaemon( const staleSessionSweepIntervalMs = options.staleSessionSweepIntervalMs ?? DEFAULT_STALE_SESSION_SWEEP_INTERVAL_MS; const state = createHunkSessionBrokerState(); + const credentials = await loadOrCreateHunkSessionBrokerCredentials(); + const generation = `h_${crypto.randomUUID().replaceAll("-", "")}_0`; + const authenticator = new SessionBrokerAuthenticator({ + appId: HUNK_SESSION_BROKER_APP_ID, + appRevision: HUNK_SESSION_BROKER_APP_REVISION, + generation, + daemonIdentity: credentials.daemonIdentity, + credentials: [credentials.producer, credentials.caller], + // A CLI process normally performs capabilities plus one action, then exits. Retire its caller + // session quickly so repeated short-lived commands cannot fill the generic retained-session cap. + callerSessionTtlMs: 30_000, + }); const daemon = createSessionBrokerDaemon({ broker: createHunkBrokerController(state), capabilities: { @@ -536,6 +606,15 @@ export function serveSessionBrokerDaemon( idleTimeoutMs, staleSessionTtlMs, staleSessionSweepIntervalMs, + appId: HUNK_SESSION_BROKER_APP_ID, + appRevision: HUNK_SESSION_BROKER_APP_REVISION, + callerAuthenticator: authenticator, + helloAuthenticator: authenticator, + producerEndpoint: `${config.wsOrigin}${SESSION_BROKER_SOCKET_PATH}`, + authorizer: () => true, + // Hunk currently keeps audit decisions in-process; the generic hook guarantees only redacted + // principal/operation metadata can be wired to a future diagnostic sink. + audit: () => undefined, paths: { socket: SESSION_BROKER_SOCKET_PATH, }, @@ -568,29 +647,46 @@ export function serveSessionBrokerDaemon( const url = new URL(request.url); - if (url.pathname === "/health") { - // Extend the generic health payload with the Hunk-specific companion endpoints that older - // CLI clients and debugging workflows still expect to discover from one place. - return Response.json({ - ...daemon.getHealth(), - sessionApi: `${config.httpOrigin}${HUNK_SESSION_API_PATH}`, - sessionCapabilities: `${config.httpOrigin}${HUNK_SESSION_CAPABILITIES_PATH}`, - sessionSocket: `${config.wsOrigin}${SESSION_BROKER_SOCKET_PATH}`, - }); + if ( + (url.pathname === HUNK_SESSION_CAPABILITIES_PATH || + url.pathname === HUNK_SESSION_API_PATH) && + !request.headers.has("x-session-broker-caller-session") + ) { + return Response.json( + { + error: "authentication-required", + message: + "This Hunk session client must be upgraded to use automatic signed authentication.", + }, + { status: 401 }, + ); } if (url.pathname === HUNK_SESSION_CAPABILITIES_PATH) { - return Response.json(sessionCapabilities()); + return daemon.handleAuthenticatedControl(request, { + authenticationFailureOperation: "diagnostics", + resolve: () => ({ operation: "diagnostics", targetSpecific: false }), + handle: (body) => + request.method === "GET" && body.byteLength === 0 + ? { body: sessionCapabilities() as never } + : { + body: { error: "Capabilities require GET with an empty body." }, + status: request.method === "GET" ? 400 : 405, + }, + }); } - // Keep the richer Hunk session API here rather than in the shared package so commands like - // review, reload, and comment flows stay app-specific. + // Keep Hunk action parsing and lowering app-owned while the generic hook authenticates, + // authorizes, budgets, and signs the exact transport body and response. if (url.pathname === HUNK_SESSION_API_PATH) { - return daemon.handleBoundedControl( - request, - (body) => handleSessionApiRequest(state, request, body), - { payloadTooLarge: (error) => jsonError(error.message, 413) }, - ); + return daemon.handleAuthenticatedControl(request, { + resolve: (body) => sessionApiAuthorizationFacts(state, body), + resolveFailureTargetSpecific: (body) => parseJsonRequestBytes(body).action !== "list", + handle: async (body, facts) => { + const response = await handleSessionApiRequest(state, request, body, facts.sessionId); + return { body: (await response.json()) as never, status: response.status }; + }, + }); } // The review surface authorizes every one of its own routes with a per-session diff --git a/src/session/broker/credentials.test.ts b/src/session/broker/credentials.test.ts new file mode 100644 index 000000000..62b48cfb9 --- /dev/null +++ b/src/session/broker/credentials.test.ts @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { + chmodSync, + lstatSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { loadOrCreateHunkSessionBrokerCredentials } from "./credentials"; + +const roots: string[] = []; + +function isolatedEnv() { + const root = mkdtempSync(join(tmpdir(), "hunk-credentials-test-")); + roots.push(root); + return { ...process.env, XDG_RUNTIME_DIR: root }; +} + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +describe("Hunk session broker credential store", () => { + test("creates stable independent Ed25519 material with owner-private Unix permissions", async () => { + const env = isolatedEnv(); + const first = await loadOrCreateHunkSessionBrokerCredentials({ env }); + const second = await loadOrCreateHunkSessionBrokerCredentials({ env }); + + expect(second.daemonIdentity.keyId).toBe(first.daemonIdentity.keyId); + expect(second.producer.grant.keyId).toBe(first.producer.grant.keyId); + expect(second.caller.grant.keyId).toBe(first.caller.grant.keyId); + expect(first.producer.grant.keyId).not.toBe(first.caller.grant.keyId); + + const securityDir = join(env.XDG_RUNTIME_DIR!, "hunk-mcp", "security-v1"); + if (process.platform !== "win32") { + expect(lstatSync(securityDir).mode & 0o777).toBe(0o700); + for (const name of ["daemon.json", "producer.json", "caller.json"]) { + expect(lstatSync(join(securityDir, name)).mode & 0o777).toBe(0o600); + } + } + const callerFile = readFileSync(join(securityDir, "caller.json"), "utf8"); + expect(callerFile).not.toContain("hunk-review-capability"); + }); + + test("adopts one complete winner under concurrent first use", async () => { + const env = isolatedEnv(); + const results = await Promise.all( + Array.from({ length: 12 }, () => loadOrCreateHunkSessionBrokerCredentials({ env })), + ); + expect(new Set(results.map((value) => value.daemonIdentity.keyId)).size).toBe(1); + expect(new Set(results.map((value) => value.producer.grant.keyId)).size).toBe(1); + expect(new Set(results.map((value) => value.caller.grant.keyId)).size).toBe(1); + }); + + test("rejects malformed and overly permissive credential files without leaking private bytes", async () => { + const env = isolatedEnv(); + await loadOrCreateHunkSessionBrokerCredentials({ env }); + const callerPath = join(env.XDG_RUNTIME_DIR!, "hunk-mcp", "security-v1", "caller.json"); + const secret = "private-secret-sentinel"; + writeFileSync(callerPath, `{"privateKey":"${secret}"}`); + if (process.platform !== "win32") chmodSync(callerPath, 0o644); + + let message = ""; + try { + await loadOrCreateHunkSessionBrokerCredentials({ env }); + } catch (error) { + message = error instanceof Error ? error.message : String(error); + } + expect(message).toContain("unsafe or malformed"); + expect(message).not.toContain(secret); + }); + + test("rejects a symlinked security directory", async () => { + if (process.platform === "win32") return; + const env = isolatedEnv(); + const runtimeDir = join(env.XDG_RUNTIME_DIR!, "hunk-mcp"); + const target = join(env.XDG_RUNTIME_DIR!, "redirect"); + const { mkdirSync } = await import("node:fs"); + mkdirSync(runtimeDir, { mode: 0o700 }); + mkdirSync(target, { mode: 0o700 }); + symlinkSync(target, join(runtimeDir, "security-v1"), "dir"); + + await expect(loadOrCreateHunkSessionBrokerCredentials({ env })).rejects.toThrow( + "unsafe or malformed", + ); + }); +}); diff --git a/src/session/broker/credentials.ts b/src/session/broker/credentials.ts new file mode 100644 index 000000000..c3d715d73 --- /dev/null +++ b/src/session/broker/credentials.ts @@ -0,0 +1,375 @@ +import { + closeSync, + constants, + fsyncSync, + fstatSync, + lstatSync, + linkSync, + mkdirSync, + openSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { dirname, join } from "node:path"; +import { + SESSION_BROKER_SIGNATURE_ALGORITHM, + type CallerGrant, + type ProducerGrant, +} from "@hunk/session-broker-core"; +import { + importEd25519PrivateKey, + importEd25519PublicKey, + type SessionBrokerCredential, + type SessionBrokerDaemonIdentity, +} from "@hunk/session-broker"; +import { resolveSessionBrokerRuntimePaths } from "./brokerLauncher"; +import { HUNK_SESSION_BROKER_APP_ID } from "./appContract"; + +const CREDENTIAL_VERSION = 1; +const CREDENTIAL_LIFETIME_MS = 10 * 365 * 24 * 60 * 60 * 1_000; +const PRIVATE_MODE = 0o600; +const DIRECTORY_MODE = 0o700; + +const HUNK_COMMAND_SCOPES = [ + "navigate_to_hunk", + "reload_session", + "comment", + "comment_batch", + "remove_comment", + "clear_comments", + "highlight", + "clear_highlights", +].map((name) => ({ name, version: 1 })) as readonly { name: string; version: number }[]; + +interface StoredCredentialFile { + version: 1; + role: "daemon" | "producer" | "caller"; + keyId: string; + publicKey: string; + privateKey: string; + grant?: ProducerGrant | CallerGrant; +} + +export interface HunkSessionBrokerCredentials { + readonly daemonIdentity: SessionBrokerDaemonIdentity; + readonly daemonPublicKey: CryptoKey; + readonly producer: SessionBrokerCredential & { readonly privateKey: CryptoKey }; + readonly caller: SessionBrokerCredential & { readonly privateKey: CryptoKey }; +} + +export interface HunkCredentialStoreOptions { + readonly env?: NodeJS.ProcessEnv; + readonly now?: () => number; + readonly randomBytes?: (length: number) => Uint8Array; +} + +function securityError(): never { + throw new Error( + "Hunk session credentials are unavailable because their owner-private runtime state is unsafe or malformed.", + ); +} + +function encode(bytes: ArrayBuffer | Uint8Array) { + return Buffer.from(bytes instanceof Uint8Array ? bytes : new Uint8Array(bytes)).toString( + "base64url", + ); +} + +function decode(value: unknown): Uint8Array { + if (typeof value !== "string" || !/^[A-Za-z0-9_-]+$/.test(value)) securityError(); + const bytes = Buffer.from(value, "base64url"); + if (bytes.length === 0 || bytes.toString("base64url") !== value) securityError(); + return bytes; +} + +function randomId(randomBytes: (length: number) => Uint8Array) { + return `h_${Buffer.from(randomBytes(18)).toString("base64url")}_0`; +} + +/** Reject credential directories and files that can redirect reads or expose owner material. */ +function validateOwnerPrivatePath(path: string, kind: "directory" | "file") { + let stat; + try { + stat = lstatSync(path); + } catch { + securityError(); + } + if (stat.isSymbolicLink() || (kind === "directory" ? !stat.isDirectory() : !stat.isFile())) { + securityError(); + } + if (process.platform !== "win32") { + if (typeof process.getuid === "function" && stat.uid !== process.getuid()) securityError(); + const unsafeBits = kind === "directory" ? stat.mode & 0o077 : stat.mode & 0o177; + if (unsafeBits !== 0) securityError(); + } +} + +/** Validate the legacy namespace parent while allowing its historical read/execute mode. */ +function ensureRuntimeNamespace(path: string) { + mkdirSync(path, { recursive: true, mode: DIRECTORY_MODE }); + const stat = lstatSync(path); + if (stat.isSymbolicLink() || !stat.isDirectory()) securityError(); + if (process.platform !== "win32") { + if (typeof process.getuid === "function" && stat.uid !== process.getuid()) securityError(); + if ((stat.mode & 0o022) !== 0) securityError(); + } +} + +/** Create and validate the stable hunk-mcp owner-private security directory. */ +function ensureSecurityDirectory(path: string) { + mkdirSync(path, { recursive: true, mode: DIRECTORY_MODE }); + if (process.platform !== "win32") { + // mkdir honors umask by making permissions narrower, which is safe; never broaden an existing dir. + validateOwnerPrivatePath(path, "directory"); + } else { + validateOwnerPrivatePath(path, "directory"); + } +} + +/** Read a regular owner-private file through a no-follow descriptor where the runtime supports it. */ +function readPrivateFile(path: string): unknown { + validateOwnerPrivatePath(path, "file"); + let descriptor: number | null = null; + try { + const noFollow = (constants as typeof constants & { O_NOFOLLOW?: number }).O_NOFOLLOW ?? 0; + descriptor = openSync(path, constants.O_RDONLY | noFollow); + const stat = fstatSync(descriptor); + if (!stat.isFile() || stat.size <= 0 || stat.size > 64 * 1024) securityError(); + if (process.platform !== "win32") { + if (typeof process.getuid === "function" && stat.uid !== process.getuid()) securityError(); + if ((stat.mode & 0o177) !== 0) securityError(); + } + return JSON.parse(readFileSync(descriptor, "utf8")); + } catch { + securityError(); + } finally { + if (descriptor !== null) closeSync(descriptor); + } +} + +function parseStored(value: unknown, role: StoredCredentialFile["role"]): StoredCredentialFile { + if (!value || typeof value !== "object" || Array.isArray(value)) securityError(); + const record = value as Record; + const expected = new Set([ + "version", + "role", + "keyId", + "publicKey", + "privateKey", + ...(role === "daemon" ? [] : ["grant"]), + ]); + if ( + Object.keys(record).some((key) => !expected.has(key)) || + Object.keys(record).length !== expected.size + ) + securityError(); + if (record.version !== CREDENTIAL_VERSION || record.role !== role) securityError(); + if (typeof record.keyId !== "string" || !/^h_[A-Za-z0-9_-]+_0$/.test(record.keyId)) + securityError(); + decode(record.publicKey); + decode(record.privateKey); + if (role !== "daemon") { + const grant = record.grant as Record | undefined; + const grantKeys = new Set([ + "kind", + "appId", + "principalId", + "keyId", + "grantId", + "algorithm", + "issuedAt", + "expiresAt", + "revocationId", + "mayDelegate", + "operations", + ...(role === "caller" ? ["commands"] : []), + ]); + const expectedOperations = + role === "producer" ? ["register", "reconnect"] : ["list", "get", "dispatch", "diagnostics"]; + if ( + !grant || + Object.keys(grant).length !== grantKeys.size || + Object.keys(grant).some((key) => !grantKeys.has(key)) || + grant.kind !== role || + grant.appId !== HUNK_SESSION_BROKER_APP_ID || + grant.principalId !== `hunk-${role}` || + grant.keyId !== record.keyId || + grant.grantId !== `hunk-${role}-bootstrap-v1` || + grant.algorithm !== SESSION_BROKER_SIGNATURE_ALGORITHM || + !Number.isFinite(grant.issuedAt) || + !Number.isFinite(grant.expiresAt) || + (grant.issuedAt as number) >= (grant.expiresAt as number) || + grant.revocationId !== `hunk-${role}-bootstrap-v1` || + grant.mayDelegate !== false || + JSON.stringify(grant.operations) !== JSON.stringify(expectedOperations) || + (role === "caller" && JSON.stringify(grant.commands) !== JSON.stringify(HUNK_COMMAND_SCOPES)) + ) + securityError(); + } + return record as unknown as StoredCredentialFile; +} + +/** Atomically adopts a complete credential file without ever replacing a live winner. */ +function adoptPrivateFile( + path: string, + contents: string, + randomBytes: (length: number) => Uint8Array, +) { + const temp = `${path}.tmp-${process.pid}-${Buffer.from(randomBytes(9)).toString("hex")}`; + let descriptor: number | null = null; + try { + descriptor = openSync( + temp, + constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, + PRIVATE_MODE, + ); + writeFileSync(descriptor, contents, "utf8"); + fsyncSync(descriptor); + closeSync(descriptor); + descriptor = null; + try { + // A hard link publishes the already-complete inode and fails rather than replacing a winner. + requireLink(temp, path); + if (process.platform !== "win32") { + const directory = openSync(dirname(path), constants.O_RDONLY); + try { + fsyncSync(directory); + } catch (error) { + if (!["EINVAL", "ENOTSUP"].includes((error as NodeJS.ErrnoException).code ?? "")) { + throw error; + } + } finally { + closeSync(directory); + } + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; + } + } finally { + if (descriptor !== null) closeSync(descriptor); + rmSync(temp, { force: true }); + } +} + +function requireLink(source: string, destination: string) { + linkSync(source, destination); +} + +async function createStored( + role: StoredCredentialFile["role"], + now: number, + randomBytes: (length: number) => Uint8Array, +): Promise { + const pair = (await crypto.subtle.generateKey("Ed25519", true, [ + "sign", + "verify", + ])) as CryptoKeyPair; + const keyId = randomId(randomBytes); + const base = { + version: CREDENTIAL_VERSION, + role, + keyId, + publicKey: encode(await crypto.subtle.exportKey("spki", pair.publicKey)), + privateKey: encode(await crypto.subtle.exportKey("pkcs8", pair.privateKey)), + } as const; + if (role === "daemon") return base; + const common = { + kind: role, + appId: HUNK_SESSION_BROKER_APP_ID, + principalId: `hunk-${role}`, + keyId, + grantId: `hunk-${role}-bootstrap-v1`, + algorithm: SESSION_BROKER_SIGNATURE_ALGORITHM, + issuedAt: now, + expiresAt: now + CREDENTIAL_LIFETIME_MS, + revocationId: `hunk-${role}-bootstrap-v1`, + mayDelegate: false, + } as const; + const grant = + role === "producer" + ? ({ + ...common, + kind: "producer", + operations: ["register", "reconnect"], + } satisfies ProducerGrant) + : ({ + ...common, + kind: "caller", + operations: ["list", "get", "dispatch", "diagnostics"], + commands: HUNK_COMMAND_SCOPES, + } satisfies CallerGrant); + return { ...base, grant }; +} + +async function loadOrCreate( + path: string, + role: StoredCredentialFile["role"], + now: number, + randomBytes: (length: number) => Uint8Array, +) { + try { + return parseStored(readPrivateFile(path), role); + } catch (error) { + const code = (() => { + try { + lstatSync(path); + return "exists"; + } catch (cause) { + return (cause as NodeJS.ErrnoException).code; + } + })(); + if (code !== "ENOENT") throw error; + } + const generated = await createStored(role, now, randomBytes); + adoptPrivateFile(path, `${JSON.stringify(generated)}\n`, randomBytes); + return parseStored(readPrivateFile(path), role); +} + +/** Load or safely create Hunk's daemon, producer, and caller Ed25519 bootstrap material. */ +export async function loadOrCreateHunkSessionBrokerCredentials( + options: HunkCredentialStoreOptions = {}, +): Promise { + const env = options.env ?? process.env; + const randomBytes = + options.randomBytes ?? ((length) => crypto.getRandomValues(new Uint8Array(length))); + const runtimeDir = resolveSessionBrokerRuntimePaths(undefined, env).runtimeDir; + const securityDir = join(runtimeDir, "security-v1"); + ensureRuntimeNamespace(runtimeDir); + ensureSecurityDirectory(securityDir); + const now = (options.now ?? Date.now)(); + const [daemon, producer, caller] = await Promise.all([ + loadOrCreate(join(securityDir, "daemon.json"), "daemon", now, randomBytes), + loadOrCreate(join(securityDir, "producer.json"), "producer", now, randomBytes), + loadOrCreate(join(securityDir, "caller.json"), "caller", now, randomBytes), + ]); + const [ + daemonPublicKey, + daemonPrivateKey, + producerPublicKey, + producerPrivateKey, + callerPublicKey, + callerPrivateKey, + ] = await Promise.all([ + importEd25519PublicKey(decode(daemon.publicKey)), + importEd25519PrivateKey(decode(daemon.privateKey)), + importEd25519PublicKey(decode(producer.publicKey)), + importEd25519PrivateKey(decode(producer.privateKey)), + importEd25519PublicKey(decode(caller.publicKey)), + importEd25519PrivateKey(decode(caller.privateKey)), + ]); + return Object.freeze({ + daemonIdentity: Object.freeze({ keyId: daemon.keyId, privateKey: daemonPrivateKey }), + daemonPublicKey, + producer: Object.freeze({ + grant: producer.grant as ProducerGrant, + publicKey: producerPublicKey, + privateKey: producerPrivateKey, + }), + caller: Object.freeze({ + grant: caller.grant as CallerGrant, + publicKey: callerPublicKey, + privateKey: callerPrivateKey, + }), + }); +} diff --git a/src/session/broker/state.ts b/src/session/broker/state.ts index 48c8321c6..0a28fb64a 100644 --- a/src/session/broker/state.ts +++ b/src/session/broker/state.ts @@ -206,8 +206,9 @@ export class HunkSessionBrokerState extends SessionBrokerState< socket: HunkBrokerConnection, registrationInput: unknown, snapshotInput: unknown, + options?: { replaceOwner?: boolean }, ) { - const registered = super.registerSession(socket, registrationInput, snapshotInput); + const registered = super.registerSession(socket, registrationInput, snapshotInput, options); this.reconcileMirroredSessions(); if (registered !== "registered") { return registered; diff --git a/src/session/client/capabilities.ts b/src/session/client/capabilities.ts index 2892b5e98..2931d9353 100644 --- a/src/session/client/capabilities.ts +++ b/src/session/client/capabilities.ts @@ -6,13 +6,8 @@ import { HUNK_SESSION_CAPABILITIES_PATH, type SessionDaemonCapabilities } from " import { parseSessionDaemonCapabilities } from "../protocolSchemas"; import { HUNK_SESSION_DAEMON_HTTP_TIMEOUT_MS, requestSessionDaemonHttp } from "./daemonHttp"; -export const HUNK_DAEMON_UPGRADE_RESTART_NOTICE = - "[hunk:session] Restarting stale session daemon after upgrade."; - -/** Tell the user that Hunk is refreshing an old daemon left running across an upgrade. */ -export function reportHunkDaemonUpgradeRestart(log: (message: string) => void = console.error) { - log(HUNK_DAEMON_UPGRADE_RESTART_NOTICE); -} +export const HUNK_DAEMON_UPGRADE_WAIT_MESSAGE = + "An older or incompatible Hunk session daemon is running. Close older Hunk windows; this window will reconnect automatically."; /** * Read the live daemon's advertised compatibility, returning null when the daemon is too old for diff --git a/test/session/broker-e2e.test.ts b/test/session/broker-e2e.test.ts index 52ac4a208..51c2033f1 100644 --- a/test/session/broker-e2e.test.ts +++ b/test/session/broker-e2e.test.ts @@ -1,5 +1,5 @@ import { afterAll, afterEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -32,8 +32,6 @@ const ttyToolsAvailable = supportsControllableScript(); interface HealthResponse { ok: boolean; - pid: number; - sessions: number; } interface SessionListJson { @@ -224,6 +222,19 @@ async function waitUntil( } } +/** Read the PID only from this test's launch metadata for teardown, never from public health. */ +function readLaunchedDaemonPid(port: number) { + try { + const runtimeBase = process.env.XDG_RUNTIME_DIR?.trim() || tmpdir(); + const metadata = JSON.parse( + readFileSync(join(runtimeBase, "hunk-mcp", `daemon-127-0-0-1-${port}.json`), "utf8"), + ) as { pid?: unknown }; + return typeof metadata.pid === "number" && metadata.pid > 0 ? metadata.pid : null; + } catch { + return null; + } +} + async function waitForHealth(port: number, timeoutMs = 15_000) { return waitUntil( "session daemon health endpoint", @@ -288,7 +299,7 @@ describe("session broker end-to-end", () => { try { const health = await waitForHealth(port); - daemonPid = health.pid; + daemonPid = readLaunchedDaemonPid(port); expect(health.ok).toBe(true); const listed = await waitUntil("registered Hunk session", async () => { @@ -394,7 +405,7 @@ describe("session broker end-to-end", () => { try { const health = await waitForHealth(port); - daemonPid = health.pid; + daemonPid = readLaunchedDaemonPid(port); expect(health.ok).toBe(true); const listed = await waitUntil("registered Hunk session", async () => { @@ -489,7 +500,7 @@ describe("session broker end-to-end", () => { try { const health = await waitForHealth(port); - daemonPid = health.pid; + daemonPid = readLaunchedDaemonPid(port); expect(health.ok).toBe(true); const listed = await waitUntil("registered Hunk session", async () => { @@ -621,7 +632,7 @@ describe("session broker end-to-end", () => { try { const health = await waitForHealth(port, 20_000); - daemonPid = health.pid; + daemonPid = readLaunchedDaemonPid(port); expect(health.ok).toBe(true); const sessions = await waitUntil("two registered Hunk sessions", async () => { diff --git a/test/session/cli.test.ts b/test/session/cli.test.ts index f24058d59..e9bfd22ff 100644 --- a/test/session/cli.test.ts +++ b/test/session/cli.test.ts @@ -1,5 +1,5 @@ import { afterAll, afterEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { createServer } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -214,20 +214,24 @@ async function quitHunkSession( const ownedDaemonPids = new Map(); -/** Poll daemon health directly before exercising the CLI boundary once. */ +/** Poll through the authenticated CLI because public health intentionally exposes no session facts. */ async function waitForRegisteredSessions(port: number) { - await waitUntil("registered live session", async () => { - const health = await readDaemonHealth(port); - if (!health || (health.sessions ?? 0) === 0) return null; - ownedDaemonPids.set(port, health.pid); - return true; + return waitUntil("registered live session", () => { + const { proc, stdout } = runSessionCli(["list", "--json"], port); + if (proc.exitCode !== 0) return null; + const sessions = (JSON.parse(stdout) as SessionListJson).sessions; + if (sessions.length === 0) return null; + try { + const metadata = JSON.parse( + readFileSync(join(testRuntimeDir, "hunk-mcp", `daemon-127-0-0-1-${port}.json`), "utf8"), + ) as { pid?: unknown }; + if (typeof metadata.pid === "number" && metadata.pid > 0) + ownedDaemonPids.set(port, metadata.pid); + } catch { + // Teardown can still rely on daemon idleness if metadata publication raced this read. + } + return sessions; }); - - const { proc, stdout, stderr } = runSessionCli(["list", "--json"], port); - if (proc.exitCode !== 0) { - throw new Error(stderr.trim() || "Failed to list the registered Hunk session."); - } - return (JSON.parse(stdout) as SessionListJson).sessions; } /** Read one test daemon's health without leaking connection failures into teardown. */ @@ -235,7 +239,7 @@ async function readDaemonHealth(port: number) { try { const response = await fetch(`http://127.0.0.1:${port}/health`); if (!response.ok) return null; - return (await response.json()) as { pid: number; sessions?: number }; + return (await response.json()) as { ok: boolean }; } catch { return null; } @@ -267,9 +271,6 @@ async function waitForDaemonExit(port: number, pid: number, label: string) { label, async () => { const health = await readDaemonHealth(port); - if (health && health.pid !== pid) { - throw new Error(`Refusing to manage unexpected daemon ${health.pid} on port ${port}.`); - } return !isProcessRunning(pid) && health === null ? true : null; }, 1_500, @@ -283,17 +284,10 @@ async function stopTestDaemon(port: number) { ownedDaemonPids.delete(port); if (pid === undefined) return; - const health = await readDaemonHealth(port); - if (health && health.pid !== pid) { - throw new Error(`Refusing to stop unexpected daemon ${health.pid} on port ${port}.`); - } - signalProcess(pid, "SIGTERM"); try { await waitForDaemonExit(port, pid, "session daemon exit"); - } catch (error) { - const remaining = await readDaemonHealth(port); - if (remaining && remaining.pid !== pid) throw error; + } catch { signalProcess(pid, "SIGKILL"); await waitForDaemonExit(port, pid, "killed session daemon exit"); } @@ -491,7 +485,7 @@ sessionDescribe("session CLI integration", () => { } }, 20_000); - test("reload refuses option-like VCS ranges sent directly to the session API", async () => { + test("raw session API callers cannot present option-like VCS ranges", async () => { const port = await reserveLoopbackPort(); const fixture = createFixtureFiles( "reload-injection", @@ -505,8 +499,7 @@ sessionDescribe("session CLI integration", () => { const listed = await waitForRegisteredSessions(port); const sessionId = listed[0]!.sessionId; - // Bypass the CLI parser on purpose: the raw daemon surface is the attacker-controlled - // path, so reproduce the injected flag exactly as a hostile /session-api caller would. + // Raw callers never reach app parsing without the owner-private signed caller session. const sentinel = join(fixture.dir, "hunk-poc"); const response = await fetch(`http://127.0.0.1:${port}/session-api`, { method: "POST", @@ -523,9 +516,10 @@ sessionDescribe("session CLI integration", () => { }), }); - expect(response.status).toBe(400); + expect(response.status).toBe(401); await expect(response.json()).resolves.toMatchObject({ - error: expect.stringContaining("looks like a VCS option"), + error: "authentication-required", + message: expect.stringContaining("upgraded"), }); expect(existsSync(sentinel)).toBe(false); diff --git a/test/session/daemon.test.ts b/test/session/daemon.test.ts index c068dc7a2..7ccd8d903 100644 --- a/test/session/daemon.test.ts +++ b/test/session/daemon.test.ts @@ -52,7 +52,7 @@ async function readHealth(port: number) { return null; } - return (await response.json()) as { ok: boolean; pid: number }; + return (await response.json()) as { ok: boolean }; } catch { return null; } @@ -75,7 +75,9 @@ afterEach(async () => { describe("session daemon lifecycle", () => { test("exits cleanly after SIGTERM instead of hot-looping after server shutdown", async () => { const port = await reserveLoopbackPort(); - const proc = Bun.spawn(["bun", "run", "src/main.tsx", "daemon", "serve"], { + // Invoke the Bun executable directly so this handle owns the daemon on Windows instead of a + // `bun run` launcher that can exit before its child releases the listening socket. + const proc = Bun.spawn([process.execPath, "src/main.tsx", "daemon", "serve"], { cwd: repoRoot, stdin: "ignore", stdout: "pipe", @@ -96,8 +98,8 @@ describe("session daemon lifecycle", () => { exited = true; }); - // Windows may keep the `bun run` launcher separate from the child serving the daemon. - process.kill(health.pid, "SIGTERM"); + // This test owns the spawned process handle; public health intentionally exposes no PID. + proc.kill("SIGTERM"); await waitUntil("daemon serve process exit", () => (exited ? true : null), 1_500, 25); await waitUntil("daemon port close", async () =>