From f1b6a6340c2d9119e6483f603aa3b505e99f4a89 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 12 Jul 2026 14:13:36 -0700 Subject: [PATCH] Address review feedback: System panel hardening (#2028) Correctness/reliability: - engine restart-all: compensating pause on resume failure so a project is never left marked active with a dead engine - desktop restart: app.quit() (runs before-quit teardown) not app.exit(0) - supervisor: SIGINT/SIGTERM during crash-backoff exit immediately; stopping-latch prevents respawn after intentional shutdown - coalesce concurrent /system/restart requests (restartScheduled guard) - rebuild job completion chain given a .catch (no stranded activeJob) Security: - same-origin CSRF guard on all mutating /system/* POSTs (safe under --no-auth / desktop) - redact secrets in host-process log history (reuse core redactSecrets) - report-bug: confirm before including server logs + escape ``` fences - sanitize restart reason; Object.hasOwn scope-guard (prototype-key 500) Frontend: - log tail dedup (drop redundant REST backfill; SSE heartbeat stops 45s reconnect churn) + X-Accel-Buffering:no on both SSE routes - restart-wait 90s timeout re-enables controls - hydrate buffered rebuild lines on mount (mid-build panel open) Maintainability + tests: - engineAvailable reflects centralCore; hoisted systemLogs option; typed desktop systemControl DTO - add tests: SSE routes, exit-86 respawn, compiled-binary respawn, engine-restart recovery, log redaction Co-Authored-By: Claude Fable 5 --- .../__tests__/dashboard-supervise.test.ts | 21 ++- .../src/commands/__tests__/dashboard.test.ts | 71 ++++++-- .../dashboard-tui/__tests__/log-sink.test.ts | 16 ++ .../src/commands/dashboard-tui/log-sink.ts | 7 +- packages/cli/src/commands/dashboard.ts | 63 +++++-- .../areas/SystemControlsArea.css | 5 + .../areas/SystemControlsArea.tsx | 92 ++++++++-- .../__tests__/register-system-routes.test.ts | 160 +++++++++++++++++- .../src/routes/register-system-routes.ts | 109 +++++++++++- packages/desktop/src/local-runtime.ts | 27 ++- 10 files changed, 499 insertions(+), 72 deletions(-) diff --git a/packages/cli/src/commands/__tests__/dashboard-supervise.test.ts b/packages/cli/src/commands/__tests__/dashboard-supervise.test.ts index 21ce4f33bd..b5d87f25e7 100644 --- a/packages/cli/src/commands/__tests__/dashboard-supervise.test.ts +++ b/packages/cli/src/commands/__tests__/dashboard-supervise.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; import { resolveSupervisorRespawnCommand, shouldSuperviseDashboard } from "../dashboard.js"; /* @@ -33,6 +33,16 @@ describe("shouldSuperviseDashboard", () => { }); describe("resolveSupervisorRespawnCommand", () => { + const originalBun = (globalThis as { Bun?: unknown }).Bun; + + afterEach(() => { + if (originalBun === undefined) { + delete (globalThis as { Bun?: unknown }).Bun; + } else { + (globalThis as { Bun?: unknown }).Bun = originalBun; + } + }); + it("re-execs the node entry script with execArgv preserved outside a compiled binary", () => { const respawn = resolveSupervisorRespawnCommand(); expect(respawn).not.toBeNull(); @@ -42,4 +52,13 @@ describe("resolveSupervisorRespawnCommand", () => { expect(respawn!.args[respawn!.args.length - 1]).toBe(process.argv[1]); expect(respawn!.args.slice(0, -1)).toEqual(process.execArgv); }); + + it("re-execs the compiled binary itself (no args) under a bun-compiled build", () => { + // A bun-compiled single-file `fn` binary exposes `Bun.embeddedFiles`; argv[1] + // is then a virtual embedded path, so the binary must re-exec process.execPath + // alone (empty args) rather than passing a bogus entry script. + (globalThis as { Bun?: { embeddedFiles: unknown[] } }).Bun = { embeddedFiles: [{}] }; + const respawn = resolveSupervisorRespawnCommand(); + expect(respawn).toEqual({ command: process.execPath, args: [] }); + }); }); diff --git a/packages/cli/src/commands/__tests__/dashboard.test.ts b/packages/cli/src/commands/__tests__/dashboard.test.ts index b9fa31472d..0b59e0a30a 100644 --- a/packages/cli/src/commands/__tests__/dashboard.test.ts +++ b/packages/cli/src/commands/__tests__/dashboard.test.ts @@ -49,22 +49,35 @@ plain node:child_process spawn (foreground, TUI-safe) instead of the detached superviseSpawn, so tests mock spawn and complete the loop by emitting a clean SIGINT close on a microtask (after the supervisor wires its close listener). */ -const { mockSupervisorSpawn } = vi.hoisted(() => ({ - mockSupervisorSpawn: vi.fn(() => { - const listeners: Record void>> = {}; - const child = { - on(event: string, cb: (...args: unknown[]) => void) { - (listeners[event] ??= []).push(cb); - return child; - }, - kill: () => true, - }; - queueMicrotask(() => { - for (const cb of listeners["close"] ?? []) cb(null, "SIGINT"); - }); - return child; - }), -})); +/* +FNXC:SystemPanel 2026-07-12-15:10: +supervisorCloseQueue lets a test script successive child exits (e.g. exit-86 +intentional restart followed by a clean exit-0) so the respawn loop can be +driven deterministically. Each spawn pops one queued close result; when the +queue is empty it defaults to a clean `{ code: 0, signal: null }` exit so the +existing single-spawn supervision tests still terminate the loop. +*/ +const { mockSupervisorSpawn, supervisorCloseQueue } = vi.hoisted(() => { + const supervisorCloseQueue: Array<{ code: number | null; signal: NodeJS.Signals | null }> = []; + return { + supervisorCloseQueue, + mockSupervisorSpawn: vi.fn(() => { + const listeners: Record void>> = {}; + const child = { + on(event: string, cb: (...args: unknown[]) => void) { + (listeners[event] ??= []).push(cb); + return child; + }, + kill: () => true, + }; + const result = supervisorCloseQueue.shift() ?? { code: 0, signal: null }; + queueMicrotask(() => { + for (const cb of listeners["close"] ?? []) cb(result.code, result.signal); + }); + return child; + }), + }; +}); vi.mock("../startup-model-sync.js", () => ({ syncStartupModels: mockSyncStartupModels, })); @@ -3532,6 +3545,32 @@ describe("runDashboard update check wiring", () => { describe("runDashboardSupervised — bounded restart behavior", () => { beforeEach(() => { mockSupervisorSpawn.mockClear(); + supervisorCloseQueue.length = 0; + }); + + it("respawns on the intentional restart exit code (86) then exits cleanly", async () => { + const mod = await import("../dashboard.js"); + const originalArgv = process.argv; + process.argv = [ + originalArgv[0] ?? process.execPath, + "/tmp/fn-entry.mjs", + "dashboard", + "--supervise", + ]; + + // First child exits 86 (System-panel restart request → respawn without + // consuming the crash budget); the respawned child exits 0 (clean stop). + supervisorCloseQueue.push({ code: 86, signal: null }, { code: 0, signal: null }); + + try { + await mod.runDashboardSupervised(0); + } finally { + process.argv = originalArgv; + } + + // Two spawns proves the exit-86 respawn happened and the loop then returned + // cleanly (no crash-budget exhaustion / process.exit). + expect(mockSupervisorSpawn).toHaveBeenCalledTimes(2); }); it("spawns an attached child without the supervision flags and advertises the restart contract", async () => { diff --git a/packages/cli/src/commands/dashboard-tui/__tests__/log-sink.test.ts b/packages/cli/src/commands/dashboard-tui/__tests__/log-sink.test.ts index c7fdca1c8f..e4c1e805ea 100644 --- a/packages/cli/src/commands/dashboard-tui/__tests__/log-sink.test.ts +++ b/packages/cli/src/commands/dashboard-tui/__tests__/log-sink.test.ts @@ -350,4 +350,20 @@ describe("DashboardLogSink system-log history", () => { expect(seen).toEqual(["first"]); }); + + // FNXC:SystemPanel 2026-07-12-14:40: The history is served over /system/logs + // and fed into diagnostics/bug reports, so secrets must be redacted before an + // entry enters the ring buffer or reaches a live subscriber. + it("redacts secrets before storing and before notifying subscribers", () => { + const sink = new DashboardLogSink(); + const seen: string[] = []; + sink.subscribeEntries((entry) => seen.push(entry.message)); + + sink.log("auth failed Authorization: Bearer sk-abcdef0123456789abcdef"); + + const stored = sink.getRecentEntries()[0].message; + expect(stored).toContain("[REDACTED]"); + expect(stored).not.toContain("sk-abcdef0123456789abcdef"); + expect(seen[0]).toBe(stored); + }); }); diff --git a/packages/cli/src/commands/dashboard-tui/log-sink.ts b/packages/cli/src/commands/dashboard-tui/log-sink.ts index 5a314caa65..d82bb5d1ff 100644 --- a/packages/cli/src/commands/dashboard-tui/log-sink.ts +++ b/packages/cli/src/commands/dashboard-tui/log-sink.ts @@ -1,3 +1,4 @@ +import { redactSecrets } from "@fusion/core"; import { LogRingBuffer, type LogEntry } from "./log-ring-buffer.js"; // ── formatConsoleArgs ───────────────────────────────────────────────────────── @@ -118,7 +119,11 @@ export class DashboardLogSink { } private record(level: LogEntry["level"], message: string, prefix?: string): void { - const entry: LogEntry = { timestamp: new Date(), level, message, prefix }; + // Redact before storing/broadcasting: the System panel serves this history + // over /system/logs + /system/logs/stream and into diagnostics/bug reports, + // so any secret that reaches a log line would otherwise be resurfaceable to + // a dashboard client. Mask before it enters the ring buffer or listeners. + const entry: LogEntry = { timestamp: new Date(), level, message: redactSecrets(message), prefix }; this.history.push(entry); for (const listener of this.entryListeners) { try { diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index 8a6ca08755..ab6d139956 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -1115,12 +1115,23 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: before the shutdown closure exists. */ let shutdownExitCode = 0; + // Coalesce concurrent restart requests: without this, a second /system/restart + // arriving inside the 300ms flush delay would return success and schedule a + // second shutdown() whose timer hits the shutdownInProgress fast-path and + // process.exit(86)s before the first (graceful) teardown finishes. + let restartScheduled = false; let requestSelfRestart: ((reason: string) => boolean) | null = null; const systemControlForServer = { supervised: process.env.FUSION_RESTART_SUPERVISED === "1", requestRestart: (reason: string) => (requestSelfRestart ? requestSelfRestart(reason) : false), sourceWorkspaceRoot: resolveFusionSourceWorkspaceRoot(), }; + // Built once and spread into both createServer() call sites (engine-mode and + // UI-only) so the System panel log surface stays a single definition. + const systemLogsForServer = { + getRecent: (limit?: number) => logSink.getRecentEntries(limit), + subscribe: (listener: (entry: import("./dashboard-tui/log-ring-buffer.js").LogEntry) => void) => logSink.subscribeEntries(listener), + }; /* * FNXC:DashboardShutdown 2026-06-27-10:32: @@ -2100,10 +2111,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: noAuth: opts.noAuth, runtimeLogger, systemControl: systemControlForServer, - systemLogs: { - getRecent: (limit?: number) => logSink.getRecentEntries(limit), - subscribe: (listener) => logSink.subscribeEntries(listener), - }, + systemLogs: systemLogsForServer, }); const shutdown = async (signal: NodeJS.Signals) => { @@ -2176,7 +2184,8 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: Restart is only honored when a supervising parent will respawn us. */ requestSelfRestart = (reason: string) => { - if (!systemControlForServer.supervised || shutdownInProgress) return false; + if (!systemControlForServer.supervised || shutdownInProgress || restartScheduled) return false; + restartScheduled = true; logSink.log(`restart requested (${reason}) — shutting down for supervised respawn`, "dashboard"); shutdownExitCode = FUSION_RESTART_EXIT_CODE; setTimeout(() => { @@ -2436,10 +2445,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: noAuth: opts.noAuth, runtimeLogger, systemControl: systemControlForServer, - systemLogs: { - getRecent: (limit?: number) => logSink.getRecentEntries(limit), - subscribe: (listener) => logSink.subscribeEntries(listener), - }, + systemLogs: systemLogsForServer, }); } @@ -2507,7 +2513,8 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: // FNXC:SystemPanel 2026-07-12-11:00: System panel restart binding for // UI-only mode — same contract as the engine-mode shutdown above. requestSelfRestart = (reason: string) => { - if (!systemControlForServer.supervised || shutdownInProgress) return false; + if (!systemControlForServer.supervised || shutdownInProgress || restartScheduled) return false; + restartScheduled = true; logSink.log(`restart requested (${reason}) — shutting down for supervised respawn`, "dashboard"); shutdownExitCode = FUSION_RESTART_EXIT_CODE; setTimeout(() => { @@ -3325,8 +3332,10 @@ export function shouldSuperviseDashboard( * interactive TUI must own the terminal: a background-process-group child * reading a TTY gets SIGTTIN/SIGTTOU-stopped, which is why detached * supervision was headless-only. Attached means terminal Ctrl+C reaches the - * child directly; the parent ignores SIGINT (waiting for the child's graceful - * exit) and forwards direct SIGTERM kills to the child. Exit code + * child directly; when a child is alive the parent waits for its graceful exit + * (and exits immediately on SIGINT during crash-backoff, when no child is alive + * to receive Ctrl+C), and forwards direct SIGTERM kills to the child. A parent + * signal latches `stopping` so an intentional shutdown never respawns. Exit code * FUSION_RESTART_EXIT_CODE is an operator-requested restart (System panel): * immediate respawn, no crash budget consumed. * @@ -3356,17 +3365,29 @@ export async function runDashboardSupervised( const restartCommand = formatSupervisorRestartCommand(respawn.command, respawn.args, childArgs); let activeChild: ReturnType | null = null; + // `stopping` latches once the operator asks to quit so the restart loop never + // respawns after an intentional shutdown, even if the child's post-signal + // exit code is non-zero. + let stopping = false; // Parent lifecycle: terminal Ctrl+C (SIGINT) already reaches the attached - // child via the shared foreground process group, so the parent just waits - // for the child's graceful exit. A direct SIGTERM to the parent (process - // managers, `kill`) is forwarded so the child shuts down too. If the parent - // dies unexpectedly, best-effort kill the child on exit. + // child via the shared foreground process group, so when a child is alive the + // parent just waits for its graceful exit. But during crash-backoff (or + // between spawns) there is NO child to receive the terminal SIGINT, so Ctrl+C + // would hang for up to the backoff window — exit immediately in that case. process.on("SIGINT", () => { - /* child receives terminal SIGINT directly; wait for its exit */ + stopping = true; + if (!activeChild) process.exit(130); }); + // A direct SIGTERM to the parent (process managers, `kill`) is forwarded so + // the child shuts down too, then the loop stops. During crash-backoff there + // is no child to forward to and the loop is parked in a sleep, so exit + // immediately rather than waiting out the backoff. If the parent dies + // unexpectedly, best-effort kill the child on exit. process.on("SIGTERM", () => { + stopping = true; + if (!activeChild) process.exit(143); try { - activeChild?.child.kill("SIGTERM"); + activeChild.child.kill("SIGTERM"); } catch { // Child may already be gone. } @@ -3396,6 +3417,12 @@ export async function runDashboardSupervised( const exitCode = exitResult.code ?? 1; const exitSignal = exitResult.signal; + // Operator asked to stop (SIGINT/SIGTERM to the parent) — never respawn, + // regardless of the child's post-signal exit code. + if (stopping) { + return; + } + // Clean exit — propagate without restart if (exitSignal === "SIGINT" || exitSignal === "SIGTERM" || exitCode === 0) { return; diff --git a/packages/dashboard/app/components/command-center/areas/SystemControlsArea.css b/packages/dashboard/app/components/command-center/areas/SystemControlsArea.css index 86eecf08ab..a464194ac3 100644 --- a/packages/dashboard/app/components/command-center/areas/SystemControlsArea.css +++ b/packages/dashboard/app/components/command-center/areas/SystemControlsArea.css @@ -69,6 +69,11 @@ never grows the page; the page must not scroll horizontally (pre wraps). justify-content: space-between; } +.cc-syscontrols-banner--error { + justify-content: space-between; + border-color: var(--danger, #f87171); +} + .cc-syscontrols-job-status { font-size: 0.8125rem; color: var(--text-muted); diff --git a/packages/dashboard/app/components/command-center/areas/SystemControlsArea.tsx b/packages/dashboard/app/components/command-center/areas/SystemControlsArea.tsx index 6c5813cf78..f99e738238 100644 --- a/packages/dashboard/app/components/command-center/areas/SystemControlsArea.tsx +++ b/packages/dashboard/app/components/command-center/areas/SystemControlsArea.tsx @@ -16,6 +16,7 @@ import { import { createBackup, fetchDashboardHealth, + fetchCurrentSystemRebuild, fetchSystemInfo, fetchSystemLogs, reloadAllSystemPlugins, @@ -52,10 +53,14 @@ runtime-metrics area. Requirements this encodes: const LOG_VIEW_CAP = 500; const RESTART_POLL_MS = 1500; const BACK_ONLINE_RELOAD_DELAY_MS = 3000; +// Bound the post-restart wait so a server that never comes back (crashed +// respawn, unsupervised restart that stopped) doesn't leave the panel polling +// forever with every control disabled. +const RESTART_WAIT_TIMEOUT_MS = 90_000; const BUG_URL_BODY_CAP = 5500; const GITHUB_NEW_ISSUE_URL = "https://github.com/Runfusion/Fusion/issues/new"; -type RestartPhase = null | "waiting" | "back"; +type RestartPhase = null | "waiting" | "back" | "timeout"; interface SystemControlsAreaProps { projectId?: string; @@ -95,7 +100,13 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr setInfo(next); setInfoError(null); if (next.activeRebuild) { - setJob((current) => (current && current.id === next.activeRebuild!.id ? current : next.activeRebuild)); + setJob((current) => { + if (current && current.id === next.activeRebuild!.id) return current; + // Adopting a different (resumed) job — clear stale lines so the new + // job's stream doesn't render mixed with the previous job's output. + setJobLines([]); + return next.activeRebuild; + }); } return next; } catch (err) { @@ -104,8 +115,28 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr } }, []); + // On mount, hydrate the buffered rebuild output. A running job's lines also + // arrive via the SSE replay-on-connect below, but a job that already + // succeeded/failed before the panel opened is never streamed (the stream + // effect skips non-running jobs), so without this the operator would see a + // finished job with an empty log — losing the output needed to diagnose it. useEffect(() => { - void loadInfo(); + let cancelled = false; + void (async () => { + await loadInfo(); + try { + const { job: current } = await fetchCurrentSystemRebuild(); + if (!cancelled && current) { + setJob(current); + setJobLines(current.lines ?? []); + } + } catch { + // Best-effort hydration; the live stream still fills a running job. + } + })(); + return () => { + cancelled = true; + }; }, [loadInfo]); // ── Rebuild job output streaming ────────────────────────────────────────── @@ -154,8 +185,13 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr useEffect(() => { if (restartPhase !== "waiting") return; let cancelled = false; + const startedAt = Date.now(); const timer = setInterval(() => { void (async () => { + if (Date.now() - startedAt > RESTART_WAIT_TIMEOUT_MS) { + if (!cancelled) setRestartPhase("timeout"); + return; + } try { const next = await fetchSystemInfo(); if (cancelled) return; @@ -164,7 +200,7 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr setRestartPhase("back"); } } catch { - // Server still restarting — keep polling. + // Server still restarting — keep polling until the timeout above. } })(); }, RESTART_POLL_MS); @@ -181,14 +217,13 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr }, [restartPhase]); // ── Live server log tail ────────────────────────────────────────────────── + // FNXC:SystemPanel 2026-07-12-14:05: The SSE stream replays the recent log + // ring on connect (and reconnect), so it is the single source of truth — a + // separate REST backfill here duplicated every recent line. Reset on open so + // a reconnect's replay overwrites rather than appends past the cap boundary. useEffect(() => { if (!logsOpen || !info?.logsSupported) return; - let unsubscribed = false; - void fetchSystemLogs(LOG_VIEW_CAP) - .then((response) => { - if (!unsubscribed) setLogEntries(response.entries); - }) - .catch(() => undefined); + setLogEntries([]); const unsubscribe = subscribeSse("/api/system/logs/stream", { events: { log: (event) => { @@ -203,11 +238,9 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr } }, }, + onReconnect: () => setLogEntries([]), }); - return () => { - unsubscribed = true; - unsubscribe(); - }; + return unsubscribe; }, [logsOpen, info?.logsSupported]); useEffect(() => { @@ -338,6 +371,20 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr .then((r) => r.entries.filter((entry) => entry.level === "error").slice(-5)) .catch(() => []) : []; + // FNXC:SystemPanel 2026-07-12-14:05: The recent-errors excerpt is + // server log content sent to github.com. Require explicit confirmation + // before including it (operator may not want internal logs public), and + // neutralize embedded ``` so a log line can't break out of the fence. + const includeErrors = + recentErrors.length > 0 && + window.confirm( + t( + "systemControls.reportBugConfirm", + "Include the last {{count}} server error log line(s) in the GitHub issue? They will be sent to github.com — review after the issue opens.", + { count: recentErrors.length }, + ), + ); + const fenceSafe = (text: string) => text.replace(/`/g, "'"); let body = [ "### What happened", "", @@ -348,8 +395,8 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr `- Platform: ${info?.platform ?? "unknown"} (${info?.arch ?? "?"}), Node ${info?.nodeVersion ?? "?"}`, `- Uptime: ${info?.uptimeSeconds ?? "?"}s, supervised: ${info?.supervised ?? false}`, "", - ...(recentErrors.length - ? ["### Recent server errors", "```", ...recentErrors.map((entry) => `${entry.prefix ? `[${entry.prefix}] ` : ""}${entry.message}`), "```"] + ...(includeErrors + ? ["### Recent server errors", "```", ...recentErrors.map((entry) => fenceSafe(`${entry.prefix ? `[${entry.prefix}] ` : ""}${entry.message}`)), "```"] : []), ].join("\n"); if (body.length > BUG_URL_BODY_CAP) body = `${body.slice(0, BUG_URL_BODY_CAP)}\n…(truncated)`; @@ -558,6 +605,19 @@ export function SystemControlsArea({ projectId, addToast }: SystemControlsAreaPr ) : null} + {restartPhase === "timeout" ? ( +
+ + {t( + "systemControls.restartTimeout", + "The server did not come back within the expected time. It may still be restarting, or the restart may have stopped it.", + )} + + +
+ ) : null}
{controls.filter((control) => !("hidden" in control && control.hidden)).map((control) => ( diff --git a/packages/dashboard/src/routes/__tests__/register-system-routes.test.ts b/packages/dashboard/src/routes/__tests__/register-system-routes.test.ts index ac791a91ed..eb5383dc65 100644 --- a/packages/dashboard/src/routes/__tests__/register-system-routes.test.ts +++ b/packages/dashboard/src/routes/__tests__/register-system-routes.test.ts @@ -1,8 +1,11 @@ // @vitest-environment node +import http from "node:http"; +import type { Socket } from "node:net"; import { mkdirSync, mkdtempSync, writeFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { PassThrough } from "node:stream"; import express from "express"; import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; import { request as performRequest } from "../../test-request.js"; @@ -155,7 +158,10 @@ describe("GET /system/info", () => { options: { systemControl: { supervised: true, requestRestart: vi.fn(() => true), sourceWorkspaceRoot: "/checkout" }, systemLogs: { getRecent: vi.fn(() => []), subscribe: vi.fn(() => () => {}) }, + // engineAvailable requires BOTH engineManager and centralCore, matching + // the /system/engine/restart guard. engineManager: {}, + centralCore: {}, }, }); const res = await getJson(app, "/api/system/info"); @@ -289,7 +295,7 @@ describe("POST /system/engine/restart", () => { }); it("pause+resumes each running project engine and reports failures", async () => { - const pauseProject = vi.fn(async () => {}); + const pauseProject = vi.fn(async (_id: string) => {}); const resumeProject = vi.fn(async (id: string) => { if (id === "p2") throw new Error("resume failed"); }); @@ -307,8 +313,36 @@ describe("POST /system/engine/restart", () => { expect(res.status).toBe(200); expect(res.body.restarted).toEqual(["p1"]); expect(res.body.failed).toEqual([{ projectId: "p2", error: "resume failed" }]); - // p3 has no running engine — must not be touched. - expect(pauseProject).toHaveBeenCalledTimes(2); + // p1 paused once (then resumed); p2 paused twice — the initial pause plus a + // compensating pause after resume failed, so the project isn't left marked + // active with a dead engine. p3 has no running engine and is untouched. + expect(pauseProject.mock.calls.map((c) => c[0])).toEqual(["p1", "p2", "p2"]); + expect(pauseProject).toHaveBeenCalledTimes(3); + }); + + it("still succeeds when the compensating pause of a failed project also throws", async () => { + const resumeProject = vi.fn(async () => { + throw new Error("resume failed"); + }); + // First pause (pre-resume) succeeds; the recovery pause in the catch throws + // — the route must swallow it and still report the original resume failure. + let pauseCalls = 0; + const pauseProject = vi.fn(async () => { + pauseCalls += 1; + if (pauseCalls === 2) throw new Error("pause failed too"); + }); + const engineManager = { + getEngine: () => ({}), + pauseProject, + resumeProject, + }; + const centralCore = { listProjects: vi.fn(async () => [{ id: "p1" }]) }; + const { app } = createApp({ options: { engineManager, centralCore } }); + + const res = await postJson(app, "/api/system/engine/restart"); + expect(res.status).toBe(200); + expect(res.body.restarted).toEqual([]); + expect(res.body.failed).toEqual([{ projectId: "p1", error: "resume failed" }]); }); }); @@ -375,3 +409,123 @@ describe("POST /system/plugins/reload-all", () => { expect(res.status).toBe(409); }); }); + +/* +FNXC:SystemPanel 2026-07-12-15:10: +SSE-stream contract tests. An open SSE response (/system/logs/stream with a live +provider) never calls res.end(), so the finish-based performRequest harness would +hang; instead openSseStream builds a MockSocket-backed req/res, drives the +(synchronous) handler, captures the replayed bytes, and lets the test simulate a +client disconnect via req "close" to assert unsubscribe/cleanup. Deterministic — +no real timers or network. Error paths (404 unknown job, 409 no provider) DO end +the response, so those ride the normal performRequest harness. +*/ +class SseMockSocket extends PassThrough { + public writable = true; + public readable = true; + public remoteAddress = "127.0.0.1"; + public encrypted = false; + setTimeout(): this { return this; } + setNoDelay(): this { return this; } + setKeepAlive(): this { return this; } + destroySoon(): void { this.destroy(); } +} + +function openSseStream(app: App, path: string, headers: Record = {}) { + const socket = new SseMockSocket(); + socket.resume(); + const req = new http.IncomingMessage(socket as unknown as Socket); + const res = new http.ServerResponse(req); + const chunks: Buffer[] = []; + + req.method = "GET"; + req.url = path; + req.httpVersion = "1.1"; + req.headers = Object.fromEntries( + Object.entries({ host: "127.0.0.1", ...headers }).map(([key, value]) => [key.toLowerCase(), value]), + ); + res.assignSocket(socket as unknown as Socket); + + const originalWrite = res.write.bind(res); + res.write = ((chunk: string | Buffer, encoding?: unknown, cb?: unknown) => { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk, typeof encoding === "string" ? (encoding as BufferEncoding) : undefined)); + return originalWrite(chunk as never, encoding as never, cb as never); + }) as typeof res.write; + + // The stream handlers are fully synchronous (no await before the initial + // replay/subscribe), and express.json() skips a bodyless GET synchronously, so + // routing + writes complete during this call. + (app as unknown as (req: http.IncomingMessage, res: http.ServerResponse) => void)(req, res); + + return { + req, + res, + text: () => Buffer.concat(chunks).toString("utf8"), + close: () => req.emit("close"), + }; +} + +describe("SSE streams", () => { + it("GET /system/jobs/:id/stream 404s for an unknown job id", async () => { + const { app } = createApp(); + const res = await performRequest(app, "GET", "/api/system/jobs/does-not-exist/stream"); + expect(res.status).toBe(404); + }); + + it("GET /system/logs/stream 409s when no systemLogs provider is wired", async () => { + const { app } = createApp(); + const res = await performRequest(app, "GET", "/api/system/logs/stream"); + expect(res.status).toBe(409); + }); + + it("GET /system/logs/stream replays getRecent(200) as SSE log events and unsubscribes on client close", () => { + const entries = [ + { timestamp: new Date(), level: "info" as const, message: "first-entry" }, + { timestamp: new Date(), level: "warn" as const, message: "second-entry" }, + ]; + const getRecent = vi.fn((limit?: number) => (limit === 200 ? entries : [])); + const unsubscribe = vi.fn(); + const subscribe = vi.fn(() => unsubscribe); + const { app } = createApp({ options: { systemLogs: { getRecent, subscribe } } }); + + const stream = openSseStream(app, "/api/system/logs/stream"); + + // Initial replay reads exactly the last-200 window, and the live tail is + // wired via a single subscribe(). + expect(getRecent).toHaveBeenCalledWith(200); + expect(subscribe).toHaveBeenCalledTimes(1); + + const text = stream.text(); + const logFrames = text.split("\n\n").filter((frame) => frame.startsWith("event: log")); + expect(logFrames).toHaveLength(2); + expect(text).toContain("first-entry"); + expect(text).toContain("second-entry"); + + // Client disconnect must tear down the live subscription (no leak). + expect(unsubscribe).not.toHaveBeenCalled(); + stream.close(); + expect(unsubscribe).toHaveBeenCalledTimes(1); + }); + + it("POST /system/restart rejects a cross-origin Origin with 403 and passes a same-origin Origin", async () => { + const { app } = createApp(); + + // Cross-origin: Origin host (evil.example) != Host (127.0.0.1) → the + // same-origin CSRF guard returns 403 before touching any restart logic. + const cross = await performRequest(app, "POST", "/api/system/restart", JSON.stringify({}), { + "content-type": "application/json", + origin: "https://evil.example", + host: "127.0.0.1", + }); + expect(cross.status).toBe(403); + + // Same-origin: Origin host matches Host → guard passes, so the request + // reaches the normal handler (409 here because no systemControl is wired). + const same = await performRequest(app, "POST", "/api/system/restart", JSON.stringify({}), { + "content-type": "application/json", + origin: "http://127.0.0.1", + host: "127.0.0.1", + }); + expect(same.status).toBe(409); + }); +}); diff --git a/packages/dashboard/src/routes/register-system-routes.ts b/packages/dashboard/src/routes/register-system-routes.ts index 574b6b937b..cf13479393 100644 --- a/packages/dashboard/src/routes/register-system-routes.ts +++ b/packages/dashboard/src/routes/register-system-routes.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import { existsSync } from "node:fs"; import { join } from "node:path"; -import type { Response } from "express"; +import type { Request, Response } from "express"; import { superviseSpawn, AgentStore } from "@fusion/core"; import { ApiError, badRequest, notFound } from "../api-error.js"; import { writeSSEEvent } from "../sse-buffer.js"; @@ -34,6 +34,57 @@ builds would corrupt each other's dist output. const JOB_LINE_CAP = 4_000; const REBUILD_MAX_LIFETIME_MS = 30 * 60_000; +/* +FNXC:SystemPanel 2026-07-12-14:05: +SSE heartbeat interval. The shared client sse-bus force-reconnects a stream +that goes silent for ~45s; each reconnect re-runs the route's replay-on-connect +(the last N log lines / job lines), which surfaced as the log tail duplicating +every entry. A periodic comment-frame heartbeat keeps the stream "live" so +steady-state reconnects don't happen. Cleared on client disconnect / job end. +*/ +const SSE_HEARTBEAT_MS = 25_000; + +/* +FNXC:SystemPanel 2026-07-12-14:05: +Same-origin CSRF guard for the mutating /system/* POSTs. These are privileged +operator actions (restart, rebuild, engine/agent bounce, plugin reload) and must +stay safe even when bearer auth is off — `--no-auth`, and the desktop embedded +server which runs unauthenticated on a random localhost port where a malicious +web page can reach it via DNS-rebinding / localhost port-scan. A same-origin +dashboard fetch sends an Origin matching Host; a cross-origin browser attack +sends a mismatched Origin; a CLI/curl call sends none. Reject only a present, +mismatched Origin so legitimate same-origin and non-browser callers pass. +Returns true (and responds 403) when the request was rejected. +*/ +function rejectCrossOrigin(req: Request, res: Response): boolean { + const origin = req.headers.origin; + if (typeof origin !== "string" || origin.length === 0) return false; + let originHost: string; + try { + originHost = new URL(origin).host; + } catch { + res.status(403).json({ error: "Invalid Origin header" }); + return true; + } + if (originHost !== req.headers.host) { + res.status(403).json({ error: "Cross-origin request rejected" }); + return true; + } + return false; +} + +function startSseHeartbeat(res: Response): () => void { + const timer = setInterval(() => { + try { + res.write(": heartbeat\n\n"); + } catch { + // Stream already closed; the close handler clears this timer. + } + }, SSE_HEARTBEAT_MS); + timer.unref?.(); + return () => clearInterval(timer); +} + type RebuildScope = "app" | "full" | "plugins"; interface SystemJobLine { @@ -199,7 +250,9 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep rebuildSupported: Boolean(systemControl?.sourceWorkspaceRoot), sourceWorkspaceRoot: systemControl?.sourceWorkspaceRoot, logsSupported: Boolean(systemLogs), - engineAvailable: Boolean(options?.engineManager), + // Engine restart needs both the manager and CentralCore (see the + // /system/engine/restart guard), so advertise availability on both. + engineAvailable: Boolean(options?.engineManager) && Boolean(options?.centralCore), pluginReloadSupported: Boolean(options?.pluginRunner?.reloadPlugin), pid: process.pid, uptimeSeconds: Math.floor(process.uptime()), @@ -214,12 +267,17 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep /** POST /api/system/restart — graceful restart via the supervising parent. */ router.post("/system/restart", (req, res) => { + if (rejectCrossOrigin(req, res)) return; if (!systemControl) { throw new ApiError(409, "Restart is not available: host process did not wire system control"); } - const reason = typeof (req.body as { reason?: unknown })?.reason === "string" - ? (req.body as { reason: string }).reason - : "operator-request"; + // Sanitize the client-supplied reason before it reaches host logs: strip + // control chars/newlines (log injection) and bound the length. + const rawReason = (req.body as { reason?: unknown })?.reason; + const reason = (typeof rawReason === "string" ? rawReason : "operator-request") + // eslint-disable-next-line no-control-regex -- deliberately strip C0 control chars (log injection) + .replace(/[\r\n-]+/g, " ") + .slice(0, 200); const accepted = systemControl.requestRestart(reason); if (!accepted) { throw new ApiError( @@ -233,6 +291,7 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep /** POST /api/system/rebuild — start a rebuild job. Body: { scope?, restart? } */ router.post("/system/rebuild", (req, res) => { + if (rejectCrossOrigin(req, res)) return; const root = systemControl?.sourceWorkspaceRoot; if (!root) { throw new ApiError(409, "Rebuild is only available when running from a Fusion source checkout"); @@ -243,7 +302,9 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep const body = (req.body ?? {}) as { scope?: unknown; restart?: unknown }; const scope = (body.scope ?? "app") as RebuildScope; - if (!(scope in rebuildScopes)) { + // Object.hasOwn (not `in`) so prototype keys like "constructor"/"toString" + // are rejected instead of passing validation and crashing on lookup. + if (typeof scope !== "string" || !Object.hasOwn(rebuildScopes, scope)) { throw badRequest(`Invalid scope "${String(body.scope)}". Expected one of: app, full, plugins.`); } const restartAfter = body.restart !== false && scope !== "plugins"; @@ -342,6 +403,13 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep } finishJob(job, "succeeded", { exitCode: 0, restartScheduled }); log.info("System rebuild succeeded", { jobId: job.id, scope, restartScheduled }); + }).catch((err) => { + // Never let an unexpected throw in the completion chain strand activeJob + // (which would 409 every subsequent rebuild until process restart). + const message = err instanceof Error ? err.message : String(err); + appendJobLine(job, "system", `Rebuild post-processing failed: ${message}`); + finishJob(job, "failed", { error: message }); + log.error("System rebuild post-processing failed", { jobId: job.id, scope, error: message }); }); res.status(202).json(jobSnapshot(job, false)); @@ -363,6 +431,10 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep res.setHeader("Content-Type", "text/event-stream"); res.setHeader("Cache-Control", "no-cache"); res.setHeader("Connection", "keep-alive"); + // Disable proxy buffering (nginx / Tailscale Serve) so live output streams + // in real time instead of appearing to hang — parity with the other SSE + // endpoints (routes.ts makeRunStreamHandler). + res.setHeader("X-Accel-Buffering", "no"); res.flushHeaders?.(); res.write(": connected\n\n"); @@ -380,9 +452,11 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep return; } + const stopHeartbeat = startSseHeartbeat(res); job.subscribers.add(res); req.on("close", () => { job.subscribers.delete(res); + stopHeartbeat(); }); }); @@ -404,6 +478,7 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep res.setHeader("Content-Type", "text/event-stream"); res.setHeader("Cache-Control", "no-cache"); res.setHeader("Connection", "keep-alive"); + res.setHeader("X-Accel-Buffering", "no"); res.flushHeaders?.(); res.write(": connected\n\n"); @@ -413,11 +488,16 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep const unsubscribe = systemLogs.subscribe((entry: SystemLogEntry) => { writeSSEEvent(res, "log", JSON.stringify(entry)); }); - req.on("close", unsubscribe); + const stopHeartbeat = startSseHeartbeat(res); + req.on("close", () => { + unsubscribe(); + stopHeartbeat(); + }); }); /** POST /api/system/engine/restart — bounce all running project engines. */ - router.post("/system/engine/restart", async (_req, res) => { + router.post("/system/engine/restart", async (req, res) => { + if (rejectCrossOrigin(req, res)) return; const engineManager = options?.engineManager; const centralCore = options?.centralCore; if (!engineManager || !centralCore) { @@ -436,6 +516,15 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep await engineManager.resumeProject(projectId); restarted.push(projectId); } catch (err) { + // resumeProject flips CentralCore status to "active" BEFORE + // ensureEngine(); a throw there would leave the project reading + // online while its engine is dead. Park it paused so status matches + // reality (best-effort — never mask the original failure). + try { + await engineManager.pauseProject(projectId); + } catch { + // Ignore — the primary failure below is what the operator needs. + } failed.push({ projectId, error: err instanceof Error ? err.message : String(err) }); } } @@ -448,6 +537,7 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep /** POST /api/system/agents/restart-all — pause+resume every active agent. */ router.post("/system/agents/restart-all", async (req, res) => { + if (rejectCrossOrigin(req, res)) return; try { const { store: scopedStore } = await getProjectContext(req); const agentStore = new AgentStore({ rootDir: scopedStore.getFusionDir() }); @@ -490,7 +580,8 @@ export function registerSystemRoutes(ctx: ApiRoutesContext, deps: SystemRouteDep }); /** POST /api/system/plugins/reload-all — hot-reload every started plugin. */ - router.post("/system/plugins/reload-all", async (_req, res) => { + router.post("/system/plugins/reload-all", async (req, res) => { + if (rejectCrossOrigin(req, res)) return; try { const result = await reloadStartedPlugins(); log.info("Plugin reload-all completed", { reloadedCount: result.reloaded.length, failedCount: result.failed.length }); diff --git a/packages/desktop/src/local-runtime.ts b/packages/desktop/src/local-runtime.ts index c27ddb9ea0..87bb79213d 100644 --- a/packages/desktop/src/local-runtime.ts +++ b/packages/desktop/src/local-runtime.ts @@ -103,19 +103,28 @@ async function createStoreDefault(rootDir: string): Promise { /* FNXC:SystemPanel 2026-07-12-14:20: Desktop restart support for the dashboard System panel. Electron owns the -process lifecycle, so "restart" = app.relaunch() + app.exit() (after a short -delay so the HTTP 202 flushes). Electron is resolved dynamically so this -module still loads under plain-node tests, where the electron package exports -a binary path instead of the runtime API — then systemControl is simply +process lifecycle, so "restart" = app.relaunch() then a GRACEFUL app.quit() +(after a short delay so the HTTP 202 flushes). quit() — not exit() — is +required so the app's `before-quit` teardown (which stops the embedded Fusion +runtime: engines, CentralCore, store) actually runs; app.exit() skipped it and +risked DB/state corruption on every restart. A bounded fallback still forces +app.exit(0) if quit is vetoed or stalls. Electron is resolved dynamically so +this module still loads under plain-node tests, where the electron package +exports a binary path instead of the runtime API — then systemControl is simply omitted and the System panel disables its restart controls. Rebuild controls never appear on desktop (no sourceWorkspaceRoot — nothing to rebuild). Cross-reference: local-server.ts carries the matching wiring for the other desktop startup path. */ -export async function resolveDesktopSystemControl(): Promise> { +const DESKTOP_RESTART_FLUSH_MS = 300; +const DESKTOP_QUIT_FALLBACK_MS = 5_000; + +export async function resolveDesktopSystemControl(): Promise< + Pick +> { try { const electron = (await import("electron")) as unknown as { - app?: { relaunch: () => void; exit: (code?: number) => void }; + app?: { relaunch: () => void; quit: () => void; exit: (code?: number) => void }; }; const electronApp = electron.app; if (!electronApp || typeof electronApp.relaunch !== "function") return {}; @@ -125,8 +134,10 @@ export async function resolveDesktopSystemControl(): Promise { setTimeout(() => { electronApp.relaunch(); - electronApp.exit(0); - }, 300); + // Graceful quit runs before-quit teardown; force-exit only if it stalls. + electronApp.quit(); + setTimeout(() => electronApp.exit(0), DESKTOP_QUIT_FALLBACK_MS).unref?.(); + }, DESKTOP_RESTART_FLUSH_MS); return true; }, },