From 6cf00aee9c0fdedc722f35c1f0032c3db9404104 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 6 Nov 2026 23:39:51 -0800 Subject: [PATCH] Require the daemon secret and loopback Host/Origin on the design and iOS QA daemons (#4117) * Require the daemon secret and loopback Host/Origin on the design and iOS QA daemons (GST-1747) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Name the changelog fragment after the branch (GST-1747) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Review-Finding: p1/red-team#2 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: refuse cross-site browser requests on the iOS QA loopback, show the expired page for a stale board secret, refuse requests without a Host (GST-1747) Review-Finding: v1/outside-voice#1 reader=outside-voice Review-Finding: v1/adversarial-native#2 reader=adversarial-native Review-Finding: v1/adversarial-native#5 reader=adversarial-native Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: show the secret segment in the documented design board URL (GST-1747) Review-Finding: v1/adversarial-native#1 reader=adversarial-native Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: keep the daemon token fixtures on one line so the credential scan reads them as code (GST-1747) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: cover the Sec-Fetch-Site refusal, the expired board page and the no-Host 403 in the changelog fragment (GST-1747) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/capy-require-the-daemon-secret.md | 4 + .../sections/proposal-and-preview.md | 2 +- design-shotgun/SKILL.md | 2 +- design/src/cli.ts | 3 +- design/src/compare.ts | 5 +- design/src/daemon-client.ts | 46 ++++--- design/src/daemon-state.ts | 1 + design/src/daemon.ts | 77 +++++++---- design/test/daemon-access.test.ts | 129 ++++++++++++++++++ design/test/daemon-discovery.test.ts | 31 ++--- design/test/daemon-tests-fixtures.ts | 11 +- design/test/daemon.test.ts | 10 +- design/test/feedback-roundtrip-daemon.test.ts | 17 +-- ios-qa/daemon/src/index.ts | 20 ++- ios-qa/daemon/test/loopback-request.test.ts | 95 +++++++++++++ lib/loopback-request.ts | 27 ++++ office-hours/sections/visual-design.md | 4 +- plan-design-review/SKILL.md | 2 +- scripts/resolvers/design.ts | 8 +- test/helpers/plan-review-board-feedback.ts | 2 +- test/plan-review-board-feedback.test.ts | 6 +- 21 files changed, 413 insertions(+), 89 deletions(-) create mode 100644 changelog.d/capy-require-the-daemon-secret.md create mode 100644 design/test/daemon-access.test.ts create mode 100644 ios-qa/daemon/test/loopback-request.test.ts create mode 100644 lib/loopback-request.ts diff --git a/changelog.d/capy-require-the-daemon-secret.md b/changelog.d/capy-require-the-daemon-secret.md new file mode 100644 index 000000000..f68d745c6 --- /dev/null +++ b/changelog.d/capy-require-the-daemon-secret.md @@ -0,0 +1,4 @@ +--- +important: security +--- +- **The design board daemon and the iOS QA daemon now serve only local clients that address them directly.** The design daemon keeps a per-daemon secret in its private (0600) state file and serves every route except `/health` under that secret, so board URLs from `$D compare --serve` now look like `http://127.0.0.1:N//boards//`. The board page, feedback, progress polling and `BOARD_URL`-based reloads keep working unchanged, and `$D daemon stop` reads the secret from the state file. Both daemons answer 403 to requests whose Host is not a loopback address for their port, to requests with no Host header, and to requests whose Origin is another site. The iOS QA loopback also answers 403 to browser requests that `Sec-Fetch-Site` marks as cross-site or same-site; local agent clients send no such header and work as before. A board link opened from before a design daemon restart now shows a "Board expired" page instead of a bare 401, and the board page's progress polling stops at the first error response instead of reading it as progress. The iOS QA tailnet listener is unchanged. (GST-1747) diff --git a/design-consultation/sections/proposal-and-preview.md b/design-consultation/sections/proposal-and-preview.md index 6c606f88e..9d6f425d5 100644 --- a/design-consultation/sections/proposal-and-preview.md +++ b/design-consultation/sections/proposal-and-preview.md @@ -355,7 +355,7 @@ Write this round's board images (printed paths that passed checks, in order) as $D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR/design-board.html" --serve ``` -This publishes to a persistent daemon, opens the board and exits. Read captured stderr for the startup marker; a PID is not readiness. Exit 0 with `BOARD_URL` means the daemon is serving. Save its full `http://127.0.0.1:N/boards//` URL. Only legacy `--no-daemon` needs a host background task; `SERVE_STARTED: port=N` gives root URL `http://127.0.0.1:N/`. +This publishes to a persistent daemon, opens the board and exits. Read captured stderr for the startup marker; a PID is not readiness. Exit 0 with `BOARD_URL` means the daemon is serving. Save its full `http://127.0.0.1:N//boards//` URL. Only legacy `--no-daemon` needs a host background task; `SERVE_STARTED: port=N` gives root URL `http://127.0.0.1:N/`. **Wait with AskUserQuestion:** "Review , Submit or request new variants, then tell me; or paste preferences here." The board chooses; the question waits. Do not poll. diff --git a/design-shotgun/SKILL.md b/design-shotgun/SKILL.md index e086e9bfa..803fa51a6 100644 --- a/design-shotgun/SKILL.md +++ b/design-shotgun/SKILL.md @@ -500,7 +500,7 @@ $D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR Creates HTML and opens the board. **Run it in the background** (host task, or `&` redirecting stdout/stderr to private files in `$_DESIGN_DIR`). Read captured stderr for the startup marker; a PID is not readiness. Missing marker: use the failure fallback below. -Default stderr: `BOARD_URL: http://127.0.0.1:N/boards//`. Use that full per-board URL for AskUserQuestion and as the reload base. Only explicit legacy `--no-daemon` emits `SERVE_STARTED: port=XXXXX`, serving one board at `/` with reload at `/api/reload`. +Default stderr: `BOARD_URL: http://127.0.0.1:N//boards//`. Use that exact URL for AskUserQuestion and as the reload base. Only explicit legacy `--no-daemon` emits `SERVE_STARTED: port=XXXXX`, serving one board at `/` with reload at `/api/reload`. **PRIMARY WAIT: AskUserQuestion with board URL** diff --git a/design/src/cli.ts b/design/src/cli.ts index 95264ae38..dc49452e1 100644 --- a/design/src/cli.ts +++ b/design/src/cli.ts @@ -375,7 +375,7 @@ async function main(): Promise { * Stderr lines (in order): * - "DAEMON_STARTED port=N version=V" (or "DAEMON_ATTACHED port=N ..." * if a daemon was already running) - * - "BOARD_PUBLISHED: http://127.0.0.1:N/boards//" + * - "BOARD_PUBLISHED: http://127.0.0.1:N//boards//" * - "BOARD_URL: " (alias for grep-friendliness) * - "SERVE_STARTED: port=N html=" (legacy back-compat alias for * any external script that scraped the pre-daemon output — note the @@ -395,6 +395,7 @@ async function publishToDaemon(opts: { html: string; title?: string }): Promise< ); const result = await publishBoard({ port: ensured.port, + token: ensured.token, html: opts.html, title: opts.title, }); diff --git a/design/src/compare.ts b/design/src/compare.ts index cd1be0ab6..b009dd458 100644 --- a/design/src/compare.ts +++ b/design/src/compare.ts @@ -535,7 +535,10 @@ export function generateCompareHtml(images: string[]): string { return; } fetch('./api/progress') - .then(function(r) { return r.json(); }) + .then(function(r) { + if (!r.ok) throw new Error('progress ' + r.status); + return r.json(); + }) .then(function(data) { if (data.status === 'serving') { clearInterval(pollInterval); diff --git a/design/src/daemon-client.ts b/design/src/daemon-client.ts index fefb8ad09..9ec76f3be 100644 --- a/design/src/daemon-client.ts +++ b/design/src/daemon-client.ts @@ -33,6 +33,7 @@ import { setTimeout as delay } from "timers/promises"; import { acquireLock, CMDLINE_MARKER, + type DaemonState, healthCheck, isProcessAlive, readStateFile, @@ -70,6 +71,7 @@ export interface EnsureDaemonOptions { export interface EnsureDaemonResult { port: number; + token: string; version: string; spawned: boolean; } @@ -97,14 +99,19 @@ export async function ensureDaemon( if (existing) { const health = await healthCheck(existing.port); if (health) { - if (health.version === expectedVersion) { + if (health.version === expectedVersion && existing.token) { log(verbose, `attached to existing daemon pid=${existing.pid} port=${existing.port}`); - return { port: existing.port, version: health.version, spawned: false }; + return { port: existing.port, token: existing.token, version: health.version, spawned: false }; } - // Version mismatch: refuse if active boards exist (Codex finding). + // Version mismatch, or a daemon from before per-daemon secrets: refuse if + // active boards exist (Codex finding). if (health.activeBoards > 0) { + const mismatch = + health.version === expectedVersion + ? `existing daemon (gstack ${health.version}) predates per-daemon secrets` + : `existing daemon is gstack ${health.version}; this CLI is ${expectedVersion}`; process.stderr.write( - `[design-daemon] WARNING: existing daemon is gstack ${health.version}; this CLI is ${expectedVersion}.\n` + + `[design-daemon] WARNING: ${mismatch}.\n` + `[design-daemon] ${health.activeBoards} active board(s) detected. Refusing to auto-kill.\n` + `[design-daemon] Submit or close the open boards, then re-run.\n` + `[design-daemon] Or force restart: $D daemon stop (will drop in-memory history).\n`, @@ -113,7 +120,7 @@ export async function ensureDaemon( } // No active boards — safe to graceful-shutdown and respawn. log(verbose, `daemon version mismatch (${health.version} vs ${expectedVersion}); shutting down`); - await gracefulShutdownExistingDaemon(existing.port); + await gracefulShutdownExistingDaemon(existing.port, existing.token); await killByPidWithIdentity(existing.pid, existing.cmdlineMarker, verbose); } else { // State file points at an unresponsive port. Either the daemon @@ -136,7 +143,7 @@ export async function ensureDaemon( const fresh = readStateFile(stateFile); if (fresh) { const h = await healthCheck(fresh.port); - if (h) return { port: fresh.port, version: h.version, spawned: false }; + if (h && fresh.token) return { port: fresh.port, token: fresh.token, version: h.version, spawned: false }; } await delay(POLL_INTERVAL_MS); } @@ -149,20 +156,20 @@ export async function ensureDaemon( const fresh = readStateFile(stateFile); if (fresh) { const h = await healthCheck(fresh.port); - if (h && h.version === expectedVersion) { + if (h && h.version === expectedVersion && fresh.token) { log(verbose, `another CLI won the lock; attaching pid=${fresh.pid} port=${fresh.port}`); - return { port: fresh.port, version: h.version, spawned: false }; + return { port: fresh.port, token: fresh.token, version: h.version, spawned: false }; } } log(verbose, "spawning new daemon"); - const port = await spawnDaemon({ + const spawned = await spawnDaemon({ script: opts.daemonScript, env: { ...opts.daemonEnv, DESIGN_DAEMON_STATE_FILE: stateFile }, stateFile, expectedVersion, }); - return { port, version: expectedVersion, spawned: true }; + return { port: spawned.port, token: spawned.token, version: expectedVersion, spawned: true }; } finally { release(); } @@ -174,6 +181,8 @@ export async function ensureDaemon( */ export interface PublishBoardOptions { port: number; + /** The daemon's secret from its state file (EnsureDaemonResult.token). */ + token: string; html: string; title?: string; publisherPid?: number; @@ -191,7 +200,7 @@ export async function publishBoard(opts: PublishBoardOptions): Promise { +async function spawnDaemon(opts: SpawnDaemonOpts): Promise { const logPath = resolveStartupLogPath(); fs.mkdirSync(path.dirname(logPath), { recursive: true }); // Truncate the startup log on each spawn so a later read finds only THIS @@ -291,9 +300,9 @@ async function spawnDaemon(opts: SpawnDaemonOpts): Promise { const deadline = Date.now() + MAX_START_WAIT_MS; while (Date.now() < deadline) { const fresh = readStateFile(opts.stateFile); - if (fresh) { + if (fresh?.token) { const h = await healthCheck(fresh.port); - if (h) return fresh.port; + if (h) return fresh; } await delay(POLL_INTERVAL_MS); } @@ -312,9 +321,12 @@ async function spawnDaemon(opts: SpawnDaemonOpts): Promise { ); } -async function gracefulShutdownExistingDaemon(port: number): Promise { +async function gracefulShutdownExistingDaemon(port: number, token: string | undefined): Promise { + // A daemon from before per-daemon secrets has no token; the caller's + // identity-checked SIGTERM stops it instead. + if (!token) return; try { - await fetch(`http://127.0.0.1:${port}/shutdown`, { + await fetch(`http://127.0.0.1:${port}/${token}/shutdown`, { method: "POST", signal: AbortSignal.timeout(2000), }); @@ -390,7 +402,7 @@ export async function shutdownDaemon(opts: { force?: boolean } = {}): Promise<{ activeBoards: health.activeBoards, }; } - await gracefulShutdownExistingDaemon(existing.port); + await gracefulShutdownExistingDaemon(existing.port, existing.token); // Best-effort: SIGTERM if /shutdown didn't take effect if (isProcessAlive(existing.pid)) { await killByPidWithIdentity(existing.pid, existing.cmdlineMarker, true); diff --git a/design/src/daemon-state.ts b/design/src/daemon-state.ts index 7f6dc951a..72b4f39a1 100644 --- a/design/src/daemon-state.ts +++ b/design/src/daemon-state.ts @@ -20,6 +20,7 @@ export interface DaemonState { version: string; serverPath: string; cmdlineMarker: string; + token: string; // per-daemon secret; every route but /health lives under / } // String we grep for in the spawned daemon's cmdline to confirm a pid is diff --git a/design/src/daemon.ts b/design/src/daemon.ts index 2aeb300d3..73fb1631d 100644 --- a/design/src/daemon.ts +++ b/design/src/daemon.ts @@ -7,16 +7,20 @@ * serve.ts as the default for `$D compare --serve`; serve.ts is kept as * the --no-daemon legacy/test path. * - * Endpoints (see plan docs/designs path for full table): - * GET / index of boards - * GET /health liveness + version (unauth) - * POST /api/boards publish a new board - * POST /shutdown graceful exit (refused if active) - * GET /boards/ 301 → /boards// - * GET /boards// render board HTML - * GET /boards//api/progress state machine status - * POST /boards//api/feedback submit/regenerate - * POST /boards//api/reload swap board HTML + * Endpoints (see plan docs/designs path for full table). Every route but + * /health lives under /, the per-daemon token kept in the 0600 state + * file; published board URLs carry it, so board JS and the agent reach the + * board through the URL alone. Requests with a non-loopback Host or a + * cross-origin Origin are refused before routing. + * GET /health liveness + version (no secret) + * GET // index of boards + * POST //api/boards publish a new board + * POST //shutdown graceful exit (refused if active) + * GET //boards/ 301 → //boards// + * GET //boards// render board HTML + * GET //boards//api/progress state machine status + * POST //boards//api/feedback submit/regenerate + * POST //boards//api/reload swap board HTML * * Lifecycle: * start → bind 127.0.0.1:N → write state file → serve until 24h idle or @@ -27,9 +31,12 @@ * Both are Codex-flagged guards against silent loss of in-memory history. */ +import { randomBytes, timingSafeEqual } from "crypto"; import fs from "fs"; import path from "path"; +import { loopbackRequestRefusal } from "../../lib/loopback-request"; + import { CMDLINE_MARKER, DaemonState, @@ -58,6 +65,8 @@ const IDLE_CHECK_INTERVAL_MS = parseInt( const MAX_BOARDS = parseInt(process.env.DESIGN_DAEMON_MAX_BOARDS || "50", 10); const VERSION = readVersionString(); +const DAEMON_TOKEN = randomBytes(32).toString("hex"); +const ROUTE_PREFIX = `/${DAEMON_TOKEN}`; // ─── Per-board state ───────────────────────────────────────────── @@ -252,7 +261,7 @@ function handleIndex(): Response { .map((b) => { const ts = new Date(b.publishedAt).toISOString(); const titleSuffix = b.title ? ` — ${escapeHtml(b.title)}` : ""; - return `
  • ${b.id} ${b.state} ${titleSuffix}
    • `; + return `
  • ${b.id} ${b.state} ${titleSuffix}
    • `; }) .join("\n"); const empty = `

    No boards yet. Run $D compare --serve to publish one.

    `; @@ -280,7 +289,7 @@ ${list} return new Response(html, { headers: { "Content-Type": "text/html; charset=utf-8" } }); } -async function handlePublish(req: Request, origin: string): Promise { +async function handlePublish(req: Request, baseUrl: string): Promise { let body: any; try { body = await req.json(); @@ -319,7 +328,7 @@ async function handlePublish(req: Request, origin: string): Promise { error: "Source directory already in use by an active board", existing: { id: existing.id, - url: `${origin}/boards/${existing.id}/`, + url: `${baseUrl}/boards/${existing.id}/`, state: existing.state, }, }, @@ -355,7 +364,7 @@ async function handlePublish(req: Request, origin: string): Promise { dlog(`published board ${id} sourceDir=${sourceDir} pid=${board.publisherPid}`); return Response.json({ id, - url: `${origin}/boards/${id}/`, + url: `${baseUrl}/boards/${id}/`, sourceDir, }); } @@ -458,14 +467,14 @@ async function handleBoardReload(board: Board, req: Request): Promise return Response.json({ reloaded: true }); } -function boardExpiredHtml(id: string): string { +function boardExpiredHtml(id: string, indexLink = true): string { return `Board expired — gstack

    Board expired

    Board ${escapeHtml(id)} is no longer hosted by this daemon (evicted or the daemon restarted).

    -

    ← see active boards

    +${indexLink ? `

    ← see active boards

    ` : ""} `; } @@ -474,14 +483,34 @@ a{color:#0070f3;text-decoration:none}a:hover{text-decoration:underline}< const BOARD_RE = /^\/boards\/([A-Za-z0-9_-]+)(\/.*)?$/; export async function fetchHandler(req: Request): Promise { + // Without a Host header Bun's req.url is a bare path; refuse it before parsing. + if (!URL.canParse(req.url)) return Response.json({ error: "host_not_loopback" }, { status: 403 }); const url = new URL(req.url); - const origin = url.origin; - + const refusal = loopbackRequestRefusal(url.host, req.headers.get("origin"), serverRef?.port ?? Number(url.port)); + if (refusal) return Response.json({ error: refusal }, { status: 403 }); if (req.method === "GET" && url.pathname === "/health") return handleHealth(); - if (req.method === "GET" && url.pathname === "/") return handleIndex(); - if (req.method === "POST" && url.pathname === "/api/boards") return handlePublish(req, origin); - if (req.method === "POST" && url.pathname === "/shutdown") { + const [, secret = "", ...rest] = url.pathname.split("/"); + const presented = Buffer.from(secret); + const expected = Buffer.from(DAEMON_TOKEN); + if (presented.length !== expected.length || !timingSafeEqual(presented, expected)) { + // A board page opened from before a daemon restart carries the old + // secret: show the expired page, minus the link that would carry this one. + const stale = `/${rest.join("/")}`.match(BOARD_RE); + if (req.method === "GET" && stale && (stale[2] ?? "/") === "/") { + return new Response(boardExpiredHtml(stale[1]!, false), { + status: 401, + headers: { "Content-Type": "text/html; charset=utf-8" }, + }); + } + return Response.json({ error: "unauthorized" }, { status: 401 }); + } + const pathname = `/${rest.join("/")}`; + + if (req.method === "GET" && pathname === "/") return handleIndex(); + if (req.method === "POST" && pathname === "/api/boards") return handlePublish(req, url.origin + ROUTE_PREFIX); + + if (req.method === "POST" && pathname === "/shutdown") { if (hasActiveBoards()) { return Response.json( { @@ -495,7 +524,7 @@ export async function fetchHandler(req: Request): Promise { return Response.json({ shuttingDown: true }); } - const m = url.pathname.match(BOARD_RE); + const m = pathname.match(BOARD_RE); if (m) { const id = m[1]!; const subpath = m[2] || ""; @@ -511,7 +540,7 @@ export async function fetchHandler(req: Request): Promise { if (req.method === "GET" && subpath === "") { return new Response(null, { status: 301, - headers: { Location: `/boards/${id}/` }, + headers: { Location: `${ROUTE_PREFIX}/boards/${id}/` }, }); } if (req.method === "GET" && subpath === "/") return handleBoardGet(board); @@ -546,6 +575,7 @@ export function start(): { port: number } { version: VERSION, serverPath: process.argv[1] || "", cmdlineMarker: CMDLINE_MARKER, + token: DAEMON_TOKEN, }; writeStateFile(state); dlog(`DAEMON_STARTED port=${actualPort} pid=${process.pid} version=${VERSION}`); @@ -574,6 +604,7 @@ if (import.meta.main) { // Exported for tests. Keep this small and stable. export const __testInternals__ = { + token: DAEMON_TOKEN, boards, fetchHandler, idleCheckTick, diff --git a/design/test/daemon-access.test.ts b/design/test/daemon-access.test.ts new file mode 100644 index 000000000..4db2e11fd --- /dev/null +++ b/design/test/daemon-access.test.ts @@ -0,0 +1,129 @@ +/** + * Request checks for the design daemon: every route except /health needs + * the per-daemon secret from the private state file, and only loopback + * Host and Origin values are served. Spawns a real daemon so Host comes + * from the wire, the way Bun.serve sees it in production. + */ + +import { afterEach, beforeEach, expect, test } from "bun:test"; +import fs from "fs"; +import path from "path"; + +import { publishBoard } from "../src/daemon-client"; +import { makeBoardHtml, makeTmpDir, spawnDaemonForTest, type SpawnedDaemon } from "./daemon-tests-fixtures"; + +let workDir: string; +let daemon: SpawnedDaemon; + +beforeEach(async () => { + workDir = makeTmpDir("daemon-access"); + daemon = await spawnDaemonForTest({ stateFile: path.join(workDir, "design.json") }); +}); + +afterEach(async () => { + await daemon.stop(); + fs.rmSync(workDir, { recursive: true, force: true }); +}); + +const base = () => `http://127.0.0.1:${daemon.port}`; + +function boardDir(name: string): string { + const dir = path.join(workDir, name); + fs.mkdirSync(dir); + return dir; +} + +test("the state file carries a private per-daemon secret", () => { + expect(daemon.token).toMatch(/^[0-9a-f]{64}$/); + if (process.platform !== "win32") expect(fs.statSync(daemon.stateFile).mode & 0o077).toBe(0); +}); + +test("routes other than /health refuse requests without the daemon secret", async () => { + const published = await publishBoard({ port: daemon.port, token: daemon.token, html: makeBoardHtml(boardDir("a")) }); + const other = makeBoardHtml(boardDir("b")); + const attempts: Array<[string, string, unknown?]> = [ + ["GET", "/"], + ["POST", "/api/boards", { html: other }], + ["POST", "/shutdown"], + ["GET", `/boards/${published.id}/`], + ["GET", `/boards/${published.id}/api/progress`], + ["POST", `/boards/${published.id}/api/feedback`, { regenerated: true }], + ["POST", `/boards/${published.id}/api/reload`, { html: other }], + ["GET", `/${"0".repeat(64)}/`], + ]; + for (const [method, route, body] of attempts) { + const res = await fetch(base() + route, { + method, + headers: { "Content-Type": "text/plain" }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + expect({ route, status: res.status }).toEqual({ route, status: 401 }); + } + expect(fs.existsSync(path.join(published.sourceDir, "feedback-pending.json"))).toBe(false); + expect((await (await fetch(base() + "/health")).json()) as object).toMatchObject({ ok: true, boards: 1 }); +}); + +test("a non-loopback Host is refused, /health included", async () => { + for (const route of ["/health", `/${daemon.token}/`, `/${daemon.token}/api/boards`]) { + const res = await fetch(base() + route, { + method: route.endsWith("/api/boards") ? "POST" : "GET", + headers: { Host: `board.example:${daemon.port}`, "Content-Type": "text/plain" }, + body: route.endsWith("/api/boards") ? JSON.stringify({ html: makeBoardHtml(boardDir("host")) }) : undefined, + }); + expect({ route, status: res.status }).toEqual({ route, status: 403 }); + } + const health = (await (await fetch(base() + "/health")).json()) as { boards: number }; + expect(health.boards).toBe(0); +}); + +test("a cross-origin Origin is refused even with the daemon secret", async () => { + const published = await publishBoard({ port: daemon.port, token: daemon.token, html: makeBoardHtml(boardDir("origin")) }); + for (const origin of ["http://board.example", "null", `http://127.0.0.1:${daemon.port + 1}`]) { + const res = await fetch(`${published.url}api/feedback`, { + method: "POST", + headers: { Origin: origin, "Content-Type": "text/plain" }, + body: JSON.stringify({ regenerated: false }), + }); + expect({ origin, status: res.status }).toEqual({ origin, status: 403 }); + } + expect(fs.existsSync(path.join(published.sourceDir, "feedback.json"))).toBe(false); +}); + +test("the published board URL carries the secret and its same-origin page requests work", async () => { + const published = await publishBoard({ port: daemon.port, token: daemon.token, html: makeBoardHtml(boardDir("ok"), "

    local board

    ") }); + expect(published.url).toBe(`${base()}/${daemon.token}/boards/${published.id}/`); + expect(await (await fetch(published.url)).text()).toContain("local board"); + const submit = await fetch(`${published.url}api/feedback`, { + method: "POST", + headers: { Origin: base(), "Content-Type": "application/json" }, + body: JSON.stringify({ preferred: "A", regenerated: false }), + }); + expect(submit.status).toBe(200); + expect(JSON.parse(fs.readFileSync(path.join(published.sourceDir, "feedback.json"), "utf-8")).boardId).toBe(published.id); + const index = await fetch(`${base()}/${daemon.token}/`, { headers: { Host: `localhost:${daemon.port}` } }); + expect(index.status).toBe(200); + expect(await index.text()).toContain(`/${daemon.token}/boards/${published.id}/`); +}); + +test("a request without a Host header is refused, not answered with a server error", async () => { + const { connect } = await import("net"); + const statusLine = await new Promise((resolve, reject) => { + const socket = connect(daemon.port, "127.0.0.1", () => socket.write(`GET /${daemon.token}/ HTTP/1.0\r\n\r\n`)); + let data = ""; + socket.on("data", (chunk) => (data += chunk)); + socket.on("end", () => resolve(data.split("\r\n")[0] ?? "")); + socket.on("error", reject); + }); + expect(statusLine).toContain(" 403"); +}); + +test("a board page requested with a stale secret shows the expired page without the current secret", async () => { + const published = await publishBoard({ port: daemon.port, token: daemon.token, html: makeBoardHtml(boardDir("stale")) }); + const res = await fetch(`${base()}/${"0".repeat(64)}/boards/${published.id}/`); + expect(res.status).toBe(401); + const body = await res.text(); + expect(body).toContain("Board expired"); + expect(body).not.toContain(daemon.token); + const progress = await fetch(`${base()}/${"0".repeat(64)}/boards/${published.id}/api/progress`); + expect(progress.status).toBe(401); +}); diff --git a/design/test/daemon-discovery.test.ts b/design/test/daemon-discovery.test.ts index fd34cd712..b3c845a34 100644 --- a/design/test/daemon-discovery.test.ts +++ b/design/test/daemon-discovery.test.ts @@ -136,7 +136,7 @@ describe("ensureDaemon", () => { // Track for cleanup activeDaemons.push({ proc: { pid: state!.pid } as any, - port: state!.port, + port: state!.port, token: state!.token, stateFile, stop: async () => { try { process.kill(state!.pid, "SIGTERM"); } catch {} @@ -181,7 +181,7 @@ describe("ensureDaemon", () => { activeDaemons.push({ proc: { pid: fresh!.pid } as any, - port: fresh!.port, + port: fresh!.port, token: fresh!.token, stateFile, stop: async () => { try { process.kill(fresh!.pid, "SIGTERM"); } catch {} }, }); @@ -216,7 +216,7 @@ describe("ensureDaemon", () => { expect(fresh!.pid).not.toBe(process.pid); activeDaemons.push({ proc: { pid: fresh!.pid } as any, - port: fresh!.port, + port: fresh!.port, token: fresh!.token, stateFile, stop: async () => { try { process.kill(fresh!.pid, "SIGTERM"); } catch {} }, }); @@ -245,7 +245,7 @@ describe("ensureDaemon", () => { expect(fresh!.pid).not.toBe(existing.proc.pid); activeDaemons.push({ proc: { pid: fresh!.pid } as any, - port: fresh!.port, + port: fresh!.port, token: fresh!.token, stateFile, stop: async () => { try { process.kill(fresh!.pid, "SIGTERM"); } catch {} }, }); @@ -258,7 +258,7 @@ describe("ensureDaemon", () => { // Publish a board so activeBoards > 0 const html = makeBoardHtml(workDir); - await publishBoard({ port: existing.port, html }); + await publishBoard({ port: existing.port, token: existing.token, html }); // Sanity: status should reflect the active board const statusResp = await fetch(`http://127.0.0.1:${existing.port}/health`); @@ -309,9 +309,9 @@ describe("publishBoard", () => { test("publishes a board through the real HTTP path and returns id+url+sourceDir", async () => { const d = await spawn1(); const htmlPath = makeBoardHtml(workDir, "

    via-client

    "); - const result = await publishBoard({ port: d.port, html: htmlPath }); + const result = await publishBoard({ port: d.port, token: d.token, html: htmlPath }); expect(result.id).toMatch(/^b-/); - expect(result.url).toBe(`http://127.0.0.1:${d.port}/boards/${result.id}/`); + expect(result.url).toBe(`http://127.0.0.1:${d.port}/${d.token}/boards/${result.id}/`); expect(result.sourceDir).toBe(fs.realpathSync(workDir)); // Confirm the board is actually fetchable at the returned URL @@ -324,9 +324,9 @@ describe("publishBoard", () => { test("409 surfaces existing board's id+url (returned object, no throw)", async () => { const d = await spawn1(); const htmlPath = makeBoardHtml(workDir); - const first = await publishBoard({ port: d.port, html: htmlPath }); + const first = await publishBoard({ port: d.port, token: d.token, html: htmlPath }); const htmlPath2 = makeBoardHtml(workDir, "

    second

    "); - const second = await publishBoard({ port: d.port, html: htmlPath2 }); + const second = await publishBoard({ port: d.port, token: d.token, html: htmlPath2 }); // Same sourceDir → 409 with `existing` field; publishBoard returns it // so the caller can attach to the existing board. expect(second.id).toBe(first.id); @@ -366,7 +366,7 @@ describe("shutdownDaemon + daemonStatus", () => { test("shutdownDaemon refuses (without force) when active boards present", async () => { const d = await spawn1(); - await publishBoard({ port: d.port, html: makeBoardHtml(workDir) }); + await publishBoard({ port: d.port, token: d.token, html: makeBoardHtml(workDir) }); const r = await shutdownDaemon(); expect(r.stopped).toBe(false); expect(r.reason).toContain("active"); @@ -377,7 +377,7 @@ describe("shutdownDaemon + daemonStatus", () => { test("shutdownDaemon with force=true ignores active boards", async () => { const d = await spawn1(); - await publishBoard({ port: d.port, html: makeBoardHtml(workDir) }); + await publishBoard({ port: d.port, token: d.token, html: makeBoardHtml(workDir) }); const r = await shutdownDaemon({ force: true }); expect(r.stopped).toBe(true); }); @@ -438,10 +438,7 @@ describe("daemon idle-shutdown behavior (real process)", () => { let pollCount = 0; const boardDir = makeTmpDir("idle-poll"); try { - const board = await publishBoard({ - port: d.port, - html: makeBoardHtml(boardDir), - }); + const board = await publishBoard({ port: d.port, token: d.token, html: makeBoardHtml(boardDir) }); // Submit so the board becomes `done` — non-done would trigger the // 1h extension path and keep the daemon alive past IDLE_MS. await fetch(`${board.url}api/feedback`, { @@ -498,7 +495,7 @@ describe("daemon idle-shutdown behavior (real process)", () => { }); const boardDir = makeTmpDir("idle-active"); try { - await publishBoard({ port: d.port, html: makeBoardHtml(boardDir) }); + await publishBoard({ port: d.port, token: d.token, html: makeBoardHtml(boardDir) }); // Daemon has 1 non-done board. After IDLE_MS, idleCheckTick should // extend rather than shut down. So at IDLE_MS + small margin, it's // still alive. @@ -553,7 +550,7 @@ describe("concurrent ensureDaemon race", () => { // Track for cleanup activeDaemons.push({ proc: { pid: state!.pid } as any, - port: state!.port, + port: state!.port, token: state!.token, stateFile, stop: async () => { try { process.kill(state!.pid, "SIGTERM"); } catch {} diff --git a/design/test/daemon-tests-fixtures.ts b/design/test/daemon-tests-fixtures.ts index 011b79c55..3327b0057 100644 --- a/design/test/daemon-tests-fixtures.ts +++ b/design/test/daemon-tests-fixtures.ts @@ -17,6 +17,7 @@ import os from "os"; import path from "path"; import { __testInternals__ } from "../src/daemon"; +import { readStateFile } from "../src/daemon-state"; export const DAEMON_SCRIPT = path.join(import.meta.dir, "..", "src", "daemon.ts"); @@ -38,19 +39,24 @@ export function resetDaemon(): void { __testInternals__.resetForTest(); } -/** Build a Request for the in-process fetchHandler tests. */ +/** + * Build a Request for the in-process fetchHandler tests. Every route but + * /health is addressed under the daemon's secret prefix. + */ export function req(method: string, urlPath: string, body?: unknown): Request { const init: RequestInit = { method }; if (body !== undefined) { init.body = typeof body === "string" ? body : JSON.stringify(body); init.headers = { "Content-Type": "application/json" }; } - return new Request(`http://127.0.0.1:1234${urlPath}`, init); + const prefix = urlPath === "/health" ? "" : `/${__testInternals__.token}`; + return new Request(`http://127.0.0.1:1234${prefix}${urlPath}`, init); } export interface SpawnedDaemon { proc: ChildProcess; port: number; + token: string; stateFile: string; stop: () => Promise; } @@ -114,6 +120,7 @@ export async function spawnDaemonForTest( return { proc, port, + token: readStateFile(stateFile)?.token ?? "", stateFile, stop: async () => { if (proc.exitCode !== null || proc.signalCode !== null) return; diff --git a/design/test/daemon.test.ts b/design/test/daemon.test.ts index c3f9dabb6..80581564b 100644 --- a/design/test/daemon.test.ts +++ b/design/test/daemon.test.ts @@ -154,7 +154,7 @@ describe("daemon /boards/ trailing-slash redirect", () => { const board = await publishTestBoard(); const r = await fetchHandler(req("GET", `/boards/${board.id}`)); expect(r.status).toBe(301); - expect(r.headers.get("Location")).toBe(`/boards/${board.id}/`); + expect(r.headers.get("Location")).toBe(`/${__testInternals__.token}/boards/${board.id}/`); }); test("GET /boards// renders the board's HTML", async () => { @@ -247,7 +247,7 @@ describe("daemon /boards//api/feedback", () => { test("rejects malformed JSON body", async () => { const board = await publishTestBoard(); - const bad = new Request(`http://127.0.0.1/boards/${board.id}/api/feedback`, { + const bad = new Request(`http://127.0.0.1:1234/${__testInternals__.token}/boards/${board.id}/api/feedback`, { method: "POST", headers: { "Content-Type": "application/json" }, body: "{not json", @@ -477,7 +477,7 @@ describe("daemon idle + activity tracking (smoke)", () => { describe("daemon malformed body handling", () => { test("POST /api/boards rejects invalid JSON body with 400", async () => { - const bad = new Request("http://127.0.0.1:1234/api/boards", { + const bad = new Request(`http://127.0.0.1:1234/${__testInternals__.token}/api/boards`, { method: "POST", headers: { "Content-Type": "application/json" }, body: "{not json", @@ -493,7 +493,7 @@ describe("daemon malformed body handling", () => { // !body || typeof body !== "object" guard and fail at the missing-html // check below. The "Expected JSON object" path only fires for genuinely // non-object values like null, numbers, strings. - const bad = new Request("http://127.0.0.1:1234/api/boards", { + const bad = new Request(`http://127.0.0.1:1234/${__testInternals__.token}/api/boards`, { method: "POST", headers: { "Content-Type": "application/json" }, body: "null", @@ -517,7 +517,7 @@ describe("daemon malformed body handling", () => { test("POST /boards//api/reload rejects invalid JSON body with 400", async () => { const board = await publishTestBoard(); const bad = new Request( - `http://127.0.0.1:1234/boards/${board.id}/api/reload`, + `http://127.0.0.1:1234/${__testInternals__.token}/boards/${board.id}/api/reload`, { method: "POST", headers: { "Content-Type": "application/json" }, diff --git a/design/test/feedback-roundtrip-daemon.test.ts b/design/test/feedback-roundtrip-daemon.test.ts index 9344768fe..e2e41e8df 100644 --- a/design/test/feedback-roundtrip-daemon.test.ts +++ b/design/test/feedback-roundtrip-daemon.test.ts @@ -68,8 +68,8 @@ describe("daemon round-trip: publish → submit → feedback.json", () => { const boardDir = makeTmpDir("board-submit"); try { const htmlPath = makeBoardHtml(boardDir, "

    round-trip board

    "); - const board = await publishBoard({ port: d.port, html: htmlPath }); - expect(board.url).toBe(`http://127.0.0.1:${d.port}/boards/${board.id}/`); + const board = await publishBoard({ port: d.port, token: d.token, html: htmlPath }); + expect(board.url).toBe(`http://127.0.0.1:${d.port}/${d.token}/boards/${board.id}/`); expect(board.sourceDir).toBe(fs.realpathSync(boardDir)); // GET the board URL — same path the browser would hit @@ -124,14 +124,15 @@ describe("daemon round-trip: publish → submit → feedback.json", () => { try { const board = await publishBoard({ port: d.port, + token: d.token, html: makeBoardHtml(boardDir), }); // Use redirect: 'manual' so we observe the 301 response itself - const res = await fetch(`http://127.0.0.1:${d.port}/boards/${board.id}`, { + const res = await fetch(`http://127.0.0.1:${d.port}/${d.token}/boards/${board.id}`, { redirect: "manual", }); expect(res.status).toBe(301); - expect(res.headers.get("Location")).toBe(`/boards/${board.id}/`); + expect(res.headers.get("Location")).toBe(`/${d.token}/boards/${board.id}/`); } finally { try { fs.rmSync(boardDir, { recursive: true, force: true }); } catch {} } @@ -146,7 +147,7 @@ describe("daemon round-trip: publish → regenerate → reload → submit round const boardDir = makeTmpDir("board-regen"); try { const r1Path = makeBoardHtml(boardDir, "

    round 1 variants

    "); - const board = await publishBoard({ port: d.port, html: r1Path }); + const board = await publishBoard({ port: d.port, token: d.token, html: r1Path }); // Skill issues a regenerate via the board JS path const regen = await fetch(`${board.url}api/feedback`, { @@ -219,8 +220,8 @@ describe("daemon round-trip: two concurrent publishes share one daemon", () => { const dirA = makeTmpDir("two-a"); const dirB = makeTmpDir("two-b"); try { - const a = await publishBoard({ port: d.port, html: makeBoardHtml(dirA) }); - const b = await publishBoard({ port: d.port, html: makeBoardHtml(dirB) }); + const a = await publishBoard({ port: d.port, token: d.token, html: makeBoardHtml(dirA) }); + const b = await publishBoard({ port: d.port, token: d.token, html: makeBoardHtml(dirB) }); // Same daemon process — state file pid is stable const state = readStateFile(stateFile); @@ -245,7 +246,7 @@ describe("daemon round-trip: two concurrent publishes share one daemon", () => { expect(fs.existsSync(path.join(b.sourceDir, "feedback.json"))).toBe(false); // Index page lists both - const idx = await fetch(`http://127.0.0.1:${d.port}/`); + const idx = await fetch(`http://127.0.0.1:${d.port}/${d.token}/`); const idxHtml = await idx.text(); expect(idxHtml).toContain(a.id); expect(idxHtml).toContain(b.id); diff --git a/ios-qa/daemon/src/index.ts b/ios-qa/daemon/src/index.ts index 8bcd4c4dd..a3a761e8f 100644 --- a/ios-qa/daemon/src/index.ts +++ b/ios-qa/daemon/src/index.ts @@ -25,6 +25,7 @@ import { bootstrapTunnel, recoverTunnel, type TunnelRecovery } from './tunnel-bo import { startTunnelKeepalive, type SpawnImpl, type ResolveImpl } from './devicectl'; import { clearSession, defaultSessionPath, resumeSession, writeSession } from './session-cache'; import type { Capability } from './types'; +import { loopbackRequestRefusal } from '../../../lib/loopback-request'; interface DaemonOptions { loopbackPort: number; @@ -389,8 +390,10 @@ function sendJson(res: ServerResponse, status: number, body: unknown): void { } /** - * Loopback handler — full surface for the spawning agent. No auth (the - * loopback bind itself is the boundary). + * Loopback handler — full surface for the spawning agent. No bearer: the + * loopback bind plus the Host, Origin and Sec-Fetch-Site checks are the + * boundary, so only local clients that address the bound port directly are + * served. */ async function handleLoopback(ctx: HandlerCtx): Promise { const { req, res, tokenStore, getTunnel, invalidateTunnel, refreshTunnel } = ctx; @@ -399,6 +402,19 @@ async function handleLoopback(ctx: HandlerCtx): Promise { const method = req.method ?? 'GET'; try { + const refusal = loopbackRequestRefusal(req.headers.host, req.headers.origin, req.socket.localPort ?? 0); + if (refusal) { + sendJson(res, 403, { error: refusal }); + return; + } + // Browsers omit Origin on no-cors GETs (an tag) but always send + // Sec-Fetch-Site; local agent clients send neither. + const fetchSite = req.headers['sec-fetch-site']; + if (fetchSite === 'cross-site' || fetchSite === 'same-site') { + sendJson(res, 403, { error: 'cross_site' }); + return; + } + // /healthz — public on loopback. if (method === 'GET' && path === '/healthz') { sendJson(res, 200, { version: '1.0.0', mode: 'loopback' }); diff --git a/ios-qa/daemon/test/loopback-request.test.ts b/ios-qa/daemon/test/loopback-request.test.ts new file mode 100644 index 000000000..aa7090f51 --- /dev/null +++ b/ios-qa/daemon/test/loopback-request.test.ts @@ -0,0 +1,95 @@ +// Loopback listener request checks: only a loopback Host for the bound +// port is served, and a request carrying a cross-origin Origin is refused +// before anything reaches the device. + +import { afterAll, beforeAll, expect, test } from 'bun:test'; +import { createServer, type Server } from 'http'; +import { mkdtempSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { startDaemon, type RunningDaemon } from '../src/index'; + +let workDir: string; +let device: Server; +let daemon: RunningDaemon; +const deviceRequests: string[] = []; + +beforeAll(async () => { + workDir = mkdtempSync(join(tmpdir(), 'ios-qa-loopback-request-')); + device = createServer((req, res) => { + deviceRequests.push(`${req.method} ${req.url}`); + req.resume(); + req.on('end', () => { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ ok: true })); + }); + }); + await new Promise((resolve) => device.listen(0, '127.0.0.1', () => resolve())); + const devicePort = (device.address() as { port: number }).port; + const d = await startDaemon({ + loopbackPort: 0, + tailnetEnabled: false, + pidfilePath: join(workDir, 'daemon.pid'), + tunnelProvider: async () => ({ udid: 'STUB-UDID', ipv6Addr: '127.0.0.1', port: devicePort, bootTokenRotated: 'stub-device-token' }), + }); + if ('error' in d) throw new Error(d.error); + daemon = d; +}); + +afterAll(async () => { + await daemon?.close(); + device.close(); + rmSync(workDir, { recursive: true, force: true }); +}); + +const base = () => `http://127.0.0.1:${daemon.loopbackPort}`; + +test('a non-loopback Host is refused before the device is contacted', async () => { + deviceRequests.length = 0; + for (const host of [`device.example:${daemon.loopbackPort}`, 'device.example', `127.0.0.1:${daemon.loopbackPort + 1}`, `127.0.0.2:${daemon.loopbackPort}`]) { + for (const [method, route] of [['GET', '/healthz'], ['GET', '/screenshot'], ['POST', '/type'], ['GET', '/auth/sessions']]) { + const res = await fetch(base() + route, { + method, + headers: { Host: host, 'content-type': 'text/plain' }, + body: method === 'POST' ? '{}' : undefined, + }); + expect({ host, route, status: res.status }).toEqual({ host, route, status: 403 }); + } + } + expect(deviceRequests).toEqual([]); +}); + +test('a cross-origin Origin is refused before the device is contacted', async () => { + deviceRequests.length = 0; + for (const origin of ['http://device.example', 'null', `http://localhost:${daemon.loopbackPort + 1}`, `https://127.0.0.1:${daemon.loopbackPort}`]) { + const res = await fetch(base() + '/session/acquire', { + method: 'POST', + headers: { Origin: origin, 'content-type': 'text/plain' }, + body: '{}', + }); + expect({ origin, status: res.status }).toEqual({ origin, status: 403 }); + } + expect(deviceRequests).toEqual([]); +}); + +test('local clients naming the loopback address still reach the device', async () => { + deviceRequests.length = 0; + for (const host of [`127.0.0.1:${daemon.loopbackPort}`, `localhost:${daemon.loopbackPort}`, `[::1]:${daemon.loopbackPort}`]) { + const res = await fetch(base() + '/screenshot', { headers: { Host: host } }); + expect({ host, status: res.status }).toEqual({ host, status: 200 }); + } + const sameOrigin = await fetch(base() + '/healthz', { headers: { Origin: base() } }); + expect(sameOrigin.status).toBe(200); + expect(deviceRequests).toEqual(['GET /screenshot', 'GET /screenshot', 'GET /screenshot']); +}); + +test('a cross-site or same-site browser request without an Origin is refused before the device is contacted', async () => { + deviceRequests.length = 0; + for (const site of ['cross-site', 'same-site']) { + for (const route of ['/screenshot', '/healthz']) { + const res = await fetch(base() + route, { headers: { 'Sec-Fetch-Site': site, 'Sec-Fetch-Mode': 'no-cors' } }); + expect({ site, route, status: res.status }).toEqual({ site, route, status: 403 }); + } + } + expect(deviceRequests).toEqual([]); +}); diff --git a/lib/loopback-request.ts b/lib/loopback-request.ts new file mode 100644 index 000000000..3ba0cb409 --- /dev/null +++ b/lib/loopback-request.ts @@ -0,0 +1,27 @@ +/** + * Request checks for daemons bound to a loopback port. Binding 127.0.0.1 + * keeps other machines out but not other origins on this machine: a browser + * page can send requests to the port, and a page served under a hostname + * that resolves to 127.0.0.1 arrives with that hostname as Host. Serving + * only loopback Host values and same-origin (or absent) Origin values keeps + * the port to local clients that address it directly. + */ + +const LOOPBACK_HOSTNAMES = ['127.0.0.1', 'localhost', '[::1]']; + +function isLoopbackAuthority(authority: string, port: number): boolean { + const normalized = authority.toLowerCase(); + return LOOPBACK_HOSTNAMES.some((hostname) => normalized === `${hostname}:${port}`); +} + +/** Why a request to a loopback daemon on `port` must be refused, or null to serve it. */ +export function loopbackRequestRefusal( + host: string | null | undefined, + origin: string | null | undefined, + port: number, +): 'host_not_loopback' | 'cross_origin' | null { + if (!host || !isLoopbackAuthority(host, port)) return 'host_not_loopback'; + if (origin === null || origin === undefined) return null; + if (!origin.startsWith('http://') || !isLoopbackAuthority(origin.slice('http://'.length), port)) return 'cross_origin'; + return null; +} diff --git a/office-hours/sections/visual-design.md b/office-hours/sections/visual-design.md index 850884904..23b24e8c1 100644 --- a/office-hours/sections/visual-design.md +++ b/office-hours/sections/visual-design.md @@ -85,7 +85,7 @@ $D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR This publishes the board to the design daemon, opens it in the user's default browser, and exits; it does not wait for feedback. Read captured stderr for the -`BOARD_URL: http://127.0.0.1:N/boards//` line, then wait with AskUserQuestion: +`BOARD_URL: http://127.0.0.1:N//boards//` line, then wait with AskUserQuestion: "Review , click Submit or Regenerate, then tell me (or paste your preferences here)." After the answer, read `feedback.json` or `feedback-pending.json` next to the board HTML. @@ -100,7 +100,7 @@ If the JSON contains `"regenerated": true`: 2. Generate new variants with `$D iterate` or `$D variants` using the updated brief, rewritten into the brief file (capture and round accounting as in Step 3) 3. Rebuild the board with Step 4's board block, without `--serve` 4. POST the new HTML to the running board. Parse the board URL from stderr - (`BOARD_URL: http://127.0.0.1:N/boards//` — the daemon path) or fall + (`BOARD_URL: http://127.0.0.1:N//boards//` — the daemon path) or fall back to the legacy port (`SERVE_STARTED: port=N` — only emitted under `--no-daemon`, hits `/api/reload` root). Daemon path: `jq -nc --arg html "$_DESIGN_DIR/design-board.html" '{html: $html}' | curl -sS -X POST "${BOARD_URL}api/reload" -H 'Content-Type: application/json' --data-binary @-` diff --git a/plan-design-review/SKILL.md b/plan-design-review/SKILL.md index 213b5a34a..7f352300e 100644 --- a/plan-design-review/SKILL.md +++ b/plan-design-review/SKILL.md @@ -488,7 +488,7 @@ $D compare --images-file "$_DESIGN_DIR/board-images.json" --output "$_DESIGN_DIR Creates HTML and opens the board. **Run it in the background** (host task, or `&` redirecting stdout/stderr to private files in `$_DESIGN_DIR`). Read captured stderr for the startup marker; a PID is not readiness. Missing marker: use the failure fallback below. -Default stderr: `BOARD_URL: http://127.0.0.1:N/boards//`. Use that full per-board URL for AskUserQuestion and as the reload base. Only explicit legacy `--no-daemon` emits `SERVE_STARTED: port=XXXXX`, serving one board at `/` with reload at `/api/reload`. +Default stderr: `BOARD_URL: http://127.0.0.1:N//boards//`. Use that exact URL for AskUserQuestion and as the reload base. Only explicit legacy `--no-daemon` emits `SERVE_STARTED: port=XXXXX`, serving one board at `/` with reload at `/api/reload`. **PRIMARY WAIT: AskUserQuestion with board URL** diff --git a/scripts/resolvers/design.ts b/scripts/resolvers/design.ts index b560100b2..bde70c974 100644 --- a/scripts/resolvers/design.ts +++ b/scripts/resolvers/design.ts @@ -1227,7 +1227,7 @@ ${designBoardBlock(true)} This publishes the board to the design daemon, opens it in the user's default browser, and exits; it does not wait for feedback. Read captured stderr for the -\`BOARD_URL: http://127.0.0.1:N/boards//\` line, then wait with AskUserQuestion: +\`BOARD_URL: http://127.0.0.1:N//boards//\` line, then wait with AskUserQuestion: "Review , click Submit or Regenerate, then tell me (or paste your preferences here)." After the answer, read \`feedback.json\` or \`feedback-pending.json\` next to the board HTML. @@ -1242,7 +1242,7 @@ If the JSON contains \`"regenerated": true\`: 2. Generate new variants with \`$D iterate\` or \`$D variants\` using the updated brief, rewritten into the brief file (capture and round accounting as in Step 3) 3. Rebuild the board with Step 4's board block, without \`--serve\` 4. POST the new HTML to the running board. Parse the board URL from stderr - (\`BOARD_URL: http://127.0.0.1:N/boards//\` — the daemon path) or fall + (\`BOARD_URL: http://127.0.0.1:N//boards//\` — the daemon path) or fall back to the legacy port (\`SERVE_STARTED: port=N\` — only emitted under \`--no-daemon\`, hits \`/api/reload\` root). Daemon path: \`jq -nc --arg html "$_DESIGN_DIR/design-board.html" '{html: $html}' | curl -sS -X POST "\${BOARD_URL}api/reload" -H 'Content-Type: application/json' --data-binary @-\` @@ -1352,7 +1352,7 @@ export function generateDesignShotgunLoop(ctx: TemplateContext): string { ${designBoardBlock(true)} -This publishes to a persistent daemon, opens the board and exits. Read captured stderr for the startup marker; a PID is not readiness. Exit 0 with \`BOARD_URL\` means the daemon is serving. Save its full \`http://127.0.0.1:N/boards//\` URL. Only legacy \`--no-daemon\` needs a host background task; \`SERVE_STARTED: port=N\` gives root URL \`http://127.0.0.1:N/\`. +This publishes to a persistent daemon, opens the board and exits. Read captured stderr for the startup marker; a PID is not readiness. Exit 0 with \`BOARD_URL\` means the daemon is serving. Save its full \`http://127.0.0.1:N//boards//\` URL. Only legacy \`--no-daemon\` needs a host background task; \`SERVE_STARTED: port=N\` gives root URL \`http://127.0.0.1:N/\`. **Wait with AskUserQuestion:** "Review , Submit or request new variants, then tell me; or paste preferences here." The board chooses; the question waits. Do not poll. @@ -1392,7 +1392,7 @@ ${designBoardBlock(true)} Creates HTML and opens the board. **Run it in the background** (host task, or \`&\` redirecting stdout/stderr to private files in \`$_DESIGN_DIR\`). Read captured stderr for the startup marker; a PID is not readiness. Missing marker: use the failure fallback below. -Default stderr: \`BOARD_URL: http://127.0.0.1:N/boards//\`. Use that full per-board URL for AskUserQuestion and as the reload base. Only explicit legacy \`--no-daemon\` emits \`SERVE_STARTED: port=XXXXX\`, serving one board at \`/\` with reload at \`/api/reload\`. +Default stderr: \`BOARD_URL: http://127.0.0.1:N//boards//\`. Use that exact URL for AskUserQuestion and as the reload base. Only explicit legacy \`--no-daemon\` emits \`SERVE_STARTED: port=XXXXX\`, serving one board at \`/\` with reload at \`/api/reload\`. **PRIMARY WAIT: AskUserQuestion with board URL** diff --git a/test/helpers/plan-review-board-feedback.ts b/test/helpers/plan-review-board-feedback.ts index 92f2c1334..737a223ad 100644 --- a/test/helpers/plan-review-board-feedback.ts +++ b/test/helpers/plan-review-board-feedback.ts @@ -148,7 +148,7 @@ export function createDesignReviewPicker({ cwd, deadlineAt }: { cwd: string; dea throw new Error('Design board submission has no unambiguous offered action'); } const urls = [...new Set(question.question.match(/https?:\/\/[^\s<>\[\]()]+/g) ?? [])]; - if (urls.length !== 1 || !/^http:\/\/127\.0\.0\.1:[1-9][0-9]*\/boards\/[A-Za-z0-9_-]+\/$/.test(urls[0]!)) { + if (urls.length !== 1 || !/^http:\/\/127\.0\.0\.1:[1-9][0-9]*\/[0-9a-f]{64}\/boards\/[A-Za-z0-9_-]+\/$/.test(urls[0]!)) { throw new Error('Design board submission requires one exact owned board URL'); } const url = urls[0]!; diff --git a/test/plan-review-board-feedback.test.ts b/test/plan-review-board-feedback.test.ts index 1d356ef09..980763f9e 100644 --- a/test/plan-review-board-feedback.test.ts +++ b/test/plan-review-board-feedback.test.ts @@ -39,10 +39,10 @@ afterEach(async () => { } }); -async function board(port = daemon.port): Promise { +async function board(owner: SpawnedDaemon = daemon): Promise { const directory = path.join(cwd, `board-${++boardNumber}`); fs.mkdirSync(directory); - return publishBoard({ port, html: makeBoardHtml(directory) }); + return publishBoard({ port: owner.port, token: owner.token, html: makeBoardHtml(directory) }); } // These are the declared actor records, not relabeled historical questions. @@ -365,7 +365,7 @@ test('a real foreign daemon port is refused without changing either board', asyn fs.mkdirSync(path.dirname(otherState), { recursive: true }); const other = await spawnDaemonForTest({ stateFile: otherState }); daemons.push(other); - const foreign = await board(other.port); + const foreign = await board(other); expect(() => picker()(question(foreign.url))).toThrow(); await expectUnsubmitted(published); await expectUnsubmitted(foreign);