diff --git a/CHANGELOG.md b/CHANGELOG.md index af706277..36229582 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable user-visible changes to Hunk are documented in this file. ### Fixed - Hardened the local session daemon against browser-originated requests by validating Host and Origin headers and requiring JSON content types for API posts. +- Disabled the generic broker HTTP API by default so Hunk's supported session API is the only app-daemon command surface. ## [0.13.0] - 2026-05-18 diff --git a/packages/session-broker-bun/README.md b/packages/session-broker-bun/README.md index 9775c3d8..fb1240e2 100644 --- a/packages/session-broker-bun/README.md +++ b/packages/session-broker-bun/README.md @@ -10,7 +10,7 @@ Use this package when you want to serve a runtime-neutral `SessionBrokerDaemon` - upgrades websocket requests on the daemon socket path - forwards websocket messages and close events into the daemon - exposes a `stopped` promise compatible with Hunk's daemon lifecycle -- lets callers override or add custom HTTP routes before the generic broker routes +- lets callers override or add custom HTTP routes before the daemon's built-in routes ## Usage @@ -55,7 +55,7 @@ const server = serveSessionBrokerDaemon({ }); ``` -Return `undefined` to fall through to the generic broker routes. +Return `undefined` to fall through to the daemon's built-in routes. The raw `/broker` HTTP API is available only when the daemon was created with `exposeHttpApi: true`. ## License diff --git a/packages/session-broker-bun/src/serve.test.ts b/packages/session-broker-bun/src/serve.test.ts index 12128a2f..19eadf84 100644 --- a/packages/session-broker-bun/src/serve.test.ts +++ b/packages/session-broker-bun/src/serve.test.ts @@ -139,7 +139,11 @@ describe("session broker bun adapter", () => { parseRegistration: (value) => parseSessionRegistrationEnvelope(value, parseInfo), parseSnapshot: (value) => parseSessionSnapshotEnvelope(value, parseState), }); - const daemon = createSessionBrokerDaemon({ broker, capabilities: { version: 1 } }); + const daemon = createSessionBrokerDaemon({ + broker, + capabilities: { version: 1 }, + exposeHttpApi: true, + }); const port = await reserveLoopbackPort(); const server = serveSessionBrokerDaemon({ daemon, diff --git a/packages/session-broker-node/src/serve.test.ts b/packages/session-broker-node/src/serve.test.ts index 5a46abf8..92f630a0 100644 --- a/packages/session-broker-node/src/serve.test.ts +++ b/packages/session-broker-node/src/serve.test.ts @@ -106,7 +106,11 @@ describe("session broker node adapter", () => { parseRegistration: (value) => parseSessionRegistrationEnvelope(value, parseInfo), parseSnapshot: (value) => parseSessionSnapshotEnvelope(value, parseState), }); - const daemon = createSessionBrokerDaemon({ broker, capabilities: { version: 1 } }); + const daemon = createSessionBrokerDaemon({ + broker, + capabilities: { version: 1 }, + exposeHttpApi: true, + }); const port = await reserveLoopbackPort(); const server = await serveSessionBrokerDaemon({ daemon, diff --git a/packages/session-broker/README.md b/packages/session-broker/README.md index 57ef1758..b0da2742 100644 --- a/packages/session-broker/README.md +++ b/packages/session-broker/README.md @@ -9,7 +9,7 @@ Use this package when you want to: - track live sessions - register and update session snapshots - route commands to one live session -- expose broker health and raw list/get/dispatch APIs +- expose broker health and optional raw list/get/dispatch APIs - manage session-side websocket connection state ## Package roles @@ -29,7 +29,7 @@ If you are choosing one package to build against, start here. - `SessionBrokerDaemon` runtime-neutral daemon engine - `SessionBrokerConnection` runtime-neutral session-side websocket helper - raw broker HTTP request types -- health and capabilities handling +- health handling and optional capabilities API handling - stale-session pruning and idle shutdown ## What this package does not own @@ -103,11 +103,22 @@ const daemon = createSessionBrokerDaemon({ At this point the daemon can: - handle health requests -- handle capabilities requests -- handle raw `list` / `get` / `dispatch` broker API requests - process websocket register/snapshot/heartbeat/result messages - prune stale sessions and request idle shutdown +The raw HTTP broker API is opt-in. Enable it only when your host application wants to expose the generic `list` / `get` / `dispatch` command surface: + +```ts +const daemon = createSessionBrokerDaemon({ + broker, + capabilities: { + version: 1, + name: "example-broker", + }, + exposeHttpApi: true, +}); +``` + ### 3. Serve it through a runtime adapter #### Bun @@ -167,7 +178,7 @@ The helper owns: ## Raw broker API -The daemon's runtime-neutral HTTP API is intentionally small: +The daemon's runtime-neutral HTTP API is intentionally small and disabled by default. When `exposeHttpApi: true` is set, it serves: - `GET /health` - `GET /broker/capabilities` diff --git a/packages/session-broker/src/daemon.test.ts b/packages/session-broker/src/daemon.test.ts index 44f33643..152b84cb 100644 --- a/packages/session-broker/src/daemon.test.ts +++ b/packages/session-broker/src/daemon.test.ts @@ -98,10 +98,11 @@ function createConnection() { } describe("session broker daemon", () => { - test("serves health and raw list/get requests", async () => { + test("serves health and raw list/get requests when the HTTP API is enabled", async () => { const daemon = createSessionBrokerDaemon({ broker: createBroker(), capabilities: { version: 1, name: "test-broker" }, + exposeHttpApi: true, }); const { connection } = createConnection(); daemon.handleConnectionMessage( @@ -149,10 +150,38 @@ describe("session broker daemon", () => { daemon.shutdown(); }); + test("does not expose the raw broker HTTP API by default", async () => { + const daemon = createSessionBrokerDaemon({ + broker: createBroker(), + capabilities: { version: 1 }, + }); + + await expect( + daemon.handleRequest(new Request("http://broker.test/broker/capabilities")), + ).resolves.toBeNull(); + + await expect( + daemon.handleRequest( + new Request("http://broker.test/broker", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ action: "list" }), + }), + ), + ).resolves.toBeNull(); + + await expect( + daemon.handleRequest(new Request("http://broker.test/health")), + ).resolves.toBeInstanceOf(Response); + expect(daemon.paths).toEqual({ health: "/health", socket: "/session" }); + daemon.shutdown(); + }); + test("requires JSON content type for raw broker API posts", async () => { const daemon = createSessionBrokerDaemon({ broker: createBroker(), capabilities: { version: 1 }, + exposeHttpApi: true, }); const response = await daemon.handleRequest( @@ -174,6 +203,7 @@ describe("session broker daemon", () => { const daemon = createSessionBrokerDaemon({ broker: createBroker(), capabilities: { version: 1 }, + exposeHttpApi: true, }); const session = createConnection(); const { connection, sent } = session; diff --git a/packages/session-broker/src/daemon.ts b/packages/session-broker/src/daemon.ts index edc17c8c..1dc2d3c5 100644 --- a/packages/session-broker/src/daemon.ts +++ b/packages/session-broker/src/daemon.ts @@ -25,6 +25,7 @@ export interface SessionBrokerDaemonOptions< broker: SessionBrokerController; capabilities?: SessionBrokerCapabilities; paths?: Partial; + exposeHttpApi?: boolean; idleTimeoutMs?: number; staleSessionTtlMs?: number; staleSessionSweepIntervalMs?: number; @@ -105,11 +106,14 @@ export class SessionBrokerDaemon< "broker" > = {}, ) { + const exposeHttpApi = options.exposeHttpApi ?? false; this.paths = { health: options.paths?.health ?? DEFAULT_SESSION_BROKER_HEALTH_PATH, - api: options.paths?.api ?? DEFAULT_SESSION_BROKER_API_PATH, - capabilities: options.paths?.capabilities ?? DEFAULT_SESSION_BROKER_CAPABILITIES_PATH, socket: options.paths?.socket ?? DEFAULT_SESSION_BROKER_SOCKET_PATH, + api: exposeHttpApi ? (options.paths?.api ?? DEFAULT_SESSION_BROKER_API_PATH) : undefined, + capabilities: exposeHttpApi + ? (options.paths?.capabilities ?? DEFAULT_SESSION_BROKER_CAPABILITIES_PATH) + : undefined, }; this.capabilities = options.capabilities ?? { version: 1 }; this.idleTimeoutMs = options.idleTimeoutMs ?? DEFAULT_IDLE_TIMEOUT_MS; @@ -162,12 +166,12 @@ export class SessionBrokerDaemon< return Response.json(this.getHealth()); } - if (url.pathname === this.paths.capabilities) { + if (this.paths.capabilities && url.pathname === this.paths.capabilities) { this.noteActivity(); return Response.json(this.capabilities); } - if (url.pathname === this.paths.api) { + if (this.paths.api && url.pathname === this.paths.api) { this.noteActivity(); return this.handleApiRequest(request); } diff --git a/packages/session-broker/src/types.ts b/packages/session-broker/src/types.ts index 2f420b19..cade3397 100644 --- a/packages/session-broker/src/types.ts +++ b/packages/session-broker/src/types.ts @@ -15,9 +15,9 @@ export interface SessionBrokerCapabilities { export interface SessionBrokerHttpPaths { health: string; - api: string; - capabilities: string; socket: string; + api?: string; + capabilities?: string; } export type SessionBrokerDaemonRequest< diff --git a/src/hunk-session/cli.test.ts b/src/hunk-session/cli.test.ts index bebd095d..2e6273ab 100644 --- a/src/hunk-session/cli.test.ts +++ b/src/hunk-session/cli.test.ts @@ -388,6 +388,7 @@ describe("Hunk session CLI formatters", () => { "Selected hunk: -", "Agent notes visible: no", "Live comments: 1", + "Review notes: 0", "Files:", " - src/first.ts (+2 -1, hunks: 2)", " hunk 1: @@ -1,1 +1,2 @@", diff --git a/src/session-broker/brokerServer.test.ts b/src/session-broker/brokerServer.test.ts index 40521538..54e1047b 100644 --- a/src/session-broker/brokerServer.test.ts +++ b/src/session-broker/brokerServer.test.ts @@ -18,6 +18,10 @@ interface HealthResponse { pid: number; sessions: number; pendingCommands: number; + paths?: Record; + sessionApi?: string; + sessionCapabilities?: string; + sessionSocket?: string; } async function reserveLoopbackPort() { @@ -250,7 +254,7 @@ describe("Hunk session daemon server", () => { } }); - test("exposes session capabilities and rejects the old MCP tool endpoint", async () => { + test("exposes only Hunk session endpoints and rejects the old MCP tool endpoint", async () => { const port = await reserveLoopbackPort(); process.env.HUNK_MCP_HOST = "127.0.0.1"; process.env.HUNK_MCP_PORT = String(port); @@ -258,6 +262,31 @@ describe("Hunk session daemon server", () => { const server = 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`, + }); + + const genericCapabilities = await fetch(`http://127.0.0.1:${port}/broker/capabilities`); + expect(genericCapabilities.status).toBe(404); + + const genericBroker = await fetch(`http://127.0.0.1:${port}/broker`, { + method: "POST", + headers: { + "content-type": "application/json", + }, + body: JSON.stringify({ action: "list" }), + }); + expect(genericBroker.status).toBe(404); + const capabilities = await fetch(`http://127.0.0.1:${port}/session-api/capabilities`); expect(capabilities.status).toBe(200); await expect(capabilities.json()).resolves.toMatchObject({ diff --git a/src/ui/diff/plannedReviewRows.test.ts b/src/ui/diff/plannedReviewRows.test.ts index 66205d04..11aa7803 100644 --- a/src/ui/diff/plannedReviewRows.test.ts +++ b/src/ui/diff/plannedReviewRows.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import type { VisibleAgentNote } from "../lib/agentAnnotations"; import { reviewRowId } from "../lib/ids"; import type { PlannedReviewRow } from "./reviewRenderPlan"; import { @@ -78,6 +79,14 @@ function splitLine(key: string, hunkIndex: number, anchorId?: string): PlannedRe } function inlineNote(key: string, hunkIndex: number): PlannedReviewRow { + const annotation = { + id: "note-1", + newRange: [1, 1] as [number, number], + summary: "Explain why this branch changed.", + rationale: "The note should reserve space in the hunk bounds.", + }; + const note: VisibleAgentNote = { id: "note-1", annotation }; + return { kind: "inline-note", key, @@ -85,26 +94,19 @@ function inlineNote(key: string, hunkIndex: number): PlannedReviewRow { fileId: "file-1", hunkIndex, annotationId: "note-1", - annotation: { - id: "note-1", - newRange: [1, 1], - summary: "Explain why this branch changed.", - rationale: "The note should reserve space in the hunk bounds.", - }, + annotation, + note, anchorSide: "new", noteCount: 1, noteIndex: 0, }; } -function guideCap(key: string, hunkIndex: number): PlannedReviewRow { +function guidedLine(key: string, hunkIndex: number): PlannedReviewRow { + const row = splitLine(key, hunkIndex) as Extract; return { - kind: "note-guide-cap", - key, - stableKey: key, - fileId: "file-1", - hunkIndex, - side: "new", + ...row, + noteGuideSide: "new", }; } @@ -124,17 +126,17 @@ describe("planned review row geometry", () => { }), ).toBe(false); expect(plannedReviewRowHeight(splitLine("line", 0), baseOptions)).toBe(1); - expect(plannedReviewRowHeight(guideCap("cap", 0), baseOptions)).toBe(1); + expect(plannedReviewRowHeight(guidedLine("guide", 0), baseOptions)).toBe(1); expect(plannedReviewRowHeight(inlineNote("note", 0), baseOptions)).toBeGreaterThan(3); }); - test("measured hunk bounds ignore collapsed gaps but include inline notes and guide caps", () => { + test("measured hunk bounds ignore collapsed gaps but include inline notes and guide rows", () => { const rows = [ hunkHeader("h0", 0, "hunk-0"), splitLine("line-0", 0), collapsedRow("gap", 0), inlineNote("note", 0), - guideCap("cap", 0), + guidedLine("guide", 0), hunkHeader("h1", 1, "hunk-1"), splitLine("line-1", 1), ]; @@ -149,7 +151,7 @@ describe("planned review row geometry", () => { top: 0, height: 3 + noteHeight, startRowId: reviewRowId("h0"), - endRowId: reviewRowId("cap"), + endRowId: reviewRowId("guide"), }); expect(measured.hunkBounds.get(1)).toEqual({ top: 4 + noteHeight,