fix(stats): verified dashboard identity before port reuse
Stamped an x-omp-stats-dashboard header on dashboard responses and required it (or the models JSON-array shape) in the reuse probe, so a foreign 200 responder such as an SPA dev server catch-all is no longer mistaken for a live dashboard. Fixes #5970
This commit is contained in:
@@ -14,14 +14,31 @@ interface PortHolder {
|
||||
image: string;
|
||||
}
|
||||
|
||||
/** Header stamped on every dashboard response so reuse probes can identify us. */
|
||||
export const STATS_DASHBOARD_HEADER = "x-omp-stats-dashboard";
|
||||
|
||||
async function probeStatsDashboard(port: number): Promise<boolean> {
|
||||
try {
|
||||
const response = await fetch(`http://localhost:${port}/api/stats/models`, {
|
||||
signal: AbortSignal.timeout(STATS_PROBE_TIMEOUT_MS),
|
||||
});
|
||||
const isDashboard = response.status === 200;
|
||||
await response.body?.cancel();
|
||||
return isDashboard;
|
||||
if (response.status !== 200) {
|
||||
await response.body?.cancel();
|
||||
return false;
|
||||
}
|
||||
// A live omp-stats dashboard stamps this header on every response.
|
||||
if (response.headers.get(STATS_DASHBOARD_HEADER)) {
|
||||
await response.body?.cancel();
|
||||
return true;
|
||||
}
|
||||
// Older dashboards predate the header; fall back to the response shape
|
||||
// (`/api/stats/models` returns a JSON array) so we never reuse — or later
|
||||
// kill — a foreign 200 responder such as an SPA dev server catch-all.
|
||||
if (!(response.headers.get("content-type") ?? "").includes("application/json")) {
|
||||
await response.body?.cancel();
|
||||
return false;
|
||||
}
|
||||
return Array.isArray(await response.json());
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -20,7 +20,7 @@ import {
|
||||
import { decodeEmbeddedClientArchive } from "./embedded-client";
|
||||
import embeddedClientArchiveTxt from "./embedded-client.generated.txt";
|
||||
import { getGainDashboardStats } from "./gain-aggregator";
|
||||
import { recoverStatsPort } from "./port-conflict";
|
||||
import { recoverStatsPort, STATS_DASHBOARD_HEADER } from "./port-conflict";
|
||||
|
||||
const EMBEDDED_CLIENT_ARCHIVE = decodeEmbeddedClientArchive(embeddedClientArchiveTxt);
|
||||
|
||||
@@ -301,11 +301,13 @@ function createDashboardServer(port: number) {
|
||||
const url = new URL(req.url);
|
||||
const path = url.pathname;
|
||||
|
||||
// CORS headers for local development
|
||||
// CORS headers for local development; the identity header lets another
|
||||
// omp session's reuse probe positively recognize this dashboard.
|
||||
const corsHeaders: Record<string, string> = {
|
||||
"Access-Control-Allow-Origin": "*",
|
||||
"Access-Control-Allow-Methods": "GET, POST, OPTIONS",
|
||||
"Access-Control-Allow-Headers": "Content-Type",
|
||||
[STATS_DASHBOARD_HEADER]: "1",
|
||||
};
|
||||
|
||||
if (req.method === "OPTIONS") {
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import type { Subprocess } from "bun";
|
||||
import { STATS_DASHBOARD_HEADER } from "../src/port-conflict";
|
||||
import { startServer } from "../src/server";
|
||||
|
||||
const holderProcesses: Array<Subprocess<"ignore", "pipe", "pipe">> = [];
|
||||
|
||||
async function startBunHolder(status: number) {
|
||||
async function startBunHolder(responseExpr: string) {
|
||||
const reservation = Bun.serve({
|
||||
hostname: "127.0.0.1",
|
||||
port: 0,
|
||||
@@ -13,7 +14,7 @@ async function startBunHolder(status: number) {
|
||||
const port = reservation.port;
|
||||
reservation.stop(true);
|
||||
|
||||
const source = `Bun.serve({ hostname: "127.0.0.1", port: ${port}, fetch: () => new Response("holder", { status: ${status} }) }); process.stdout.write("ready"); await Promise.withResolvers().promise;`;
|
||||
const source = `Bun.serve({ hostname: "127.0.0.1", port: ${port}, fetch: () => ${responseExpr} }); process.stdout.write("ready"); await Promise.withResolvers().promise;`;
|
||||
const child = Bun.spawn([process.execPath, "-e", source], {
|
||||
stdin: "ignore",
|
||||
stdout: "pipe",
|
||||
@@ -42,12 +43,14 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe("startServer port conflicts", () => {
|
||||
it("reuses a live stats dashboard without stopping it", async () => {
|
||||
it("reuses a live stats dashboard identified by its header", async () => {
|
||||
const existing = Bun.serve({
|
||||
hostname: "127.0.0.1",
|
||||
port: 0,
|
||||
fetch: request =>
|
||||
new URL(request.url).pathname === "/api/stats/models" ? Response.json([]) : new Response("dashboard"),
|
||||
new URL(request.url).pathname === "/api/stats/models"
|
||||
? Response.json([], { headers: { [STATS_DASHBOARD_HEADER]: "1" } })
|
||||
: new Response("dashboard"),
|
||||
});
|
||||
|
||||
try {
|
||||
@@ -55,16 +58,35 @@ describe("startServer port conflicts", () => {
|
||||
expect(server.port).toBe(existing.port);
|
||||
server.stop();
|
||||
|
||||
// The foreign server is untouched: it still answers on the port.
|
||||
const response = await fetch(`http://127.0.0.1:${existing.port}/api/stats/models`);
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers.get(STATS_DASHBOARD_HEADER)).toBe("1");
|
||||
await response.body?.cancel();
|
||||
} finally {
|
||||
existing.stop(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("does not reuse a foreign 200 responder and reclaims the port instead", async () => {
|
||||
// An SPA dev server catch-all: 200 JSON, but no dashboard header and not
|
||||
// the models array shape. Must not be treated as a reusable dashboard.
|
||||
const holder = await startBunHolder('Response.json({ app: "spa" })');
|
||||
const server = await startServer(holder.port);
|
||||
|
||||
try {
|
||||
expect(server.port).toBe(holder.port);
|
||||
expect(await holder.child.exited).not.toBe(0);
|
||||
const response = await fetch(`http://127.0.0.1:${holder.port}/api/stats/models`);
|
||||
expect(response.headers.get(STATS_DASHBOARD_HEADER)).toBe("1");
|
||||
await response.body?.cancel();
|
||||
} finally {
|
||||
server.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("reclaims an unresponsive Bun listener and starts the dashboard", async () => {
|
||||
const holder = await startBunHolder(404);
|
||||
const holder = await startBunHolder('new Response("holder", { status: 404 })');
|
||||
const server = await startServer(holder.port);
|
||||
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user