fix(stats): restricted dashboard access
Bound the dashboard and reuse probe to IPv4 loopback, removed wildcard CORS, and reported the actual listening hostname. Added regression coverage for non-loopback refusal and absent cross-origin access. Fixes #7633
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Restricted the stats dashboard to IPv4 loopback and removed wildcard CORS access to its API ([#7633](https://github.com/can1357/oh-my-pi/issues/7633)).
|
||||
|
||||
## [17.2.4] - 2026-08-01
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -171,8 +171,8 @@ Examples:
|
||||
|
||||
// Start server
|
||||
const port = parseInt(values.port || "3847", 10);
|
||||
const { port: actualPort } = await startServer(port);
|
||||
console.log(`Dashboard available at: http://localhost:${actualPort}`);
|
||||
const { hostname, port: actualPort } = await startServer(port);
|
||||
console.log(`Dashboard available at: http://${hostname}:${actualPort}`);
|
||||
console.log("Press Ctrl+C to stop\n");
|
||||
|
||||
// Keep process running
|
||||
|
||||
@@ -18,9 +18,12 @@ interface PortHolder {
|
||||
/** Header stamped on every dashboard response so reuse probes can identify us. */
|
||||
export const STATS_DASHBOARD_HEADER = "x-omp-stats-dashboard";
|
||||
|
||||
/** IPv4 loopback address shared by the dashboard server and reuse probe. */
|
||||
export const STATS_DASHBOARD_HOSTNAME = "127.0.0.1";
|
||||
|
||||
async function probeStatsDashboard(port: number): Promise<boolean> {
|
||||
try {
|
||||
const response = await fetch(`http://localhost:${port}/api/stats/models`, {
|
||||
const response = await fetch(`http://${STATS_DASHBOARD_HOSTNAME}:${port}/api/stats/models`, {
|
||||
signal: AbortSignal.timeout(STATS_PROBE_TIMEOUT_MS),
|
||||
});
|
||||
if (response.status !== 200) {
|
||||
|
||||
@@ -21,7 +21,7 @@ import {
|
||||
import { decodeEmbeddedClientArchive } from "./embedded-client";
|
||||
import embeddedClientArchiveTxt from "./embedded-client.generated.txt";
|
||||
import { getGainDashboardStats } from "./gain-aggregator";
|
||||
import { recoverStatsPort, STATS_DASHBOARD_HEADER } from "./port-conflict";
|
||||
import { recoverStatsPort, STATS_DASHBOARD_HEADER, STATS_DASHBOARD_HOSTNAME } from "./port-conflict";
|
||||
|
||||
const EMBEDDED_CLIENT_ARCHIVE = decodeEmbeddedClientArchive(embeddedClientArchiveTxt);
|
||||
|
||||
@@ -303,21 +303,19 @@ async function handleStatic(requestPath: string): Promise<Response> {
|
||||
function createDashboardServer(port: number) {
|
||||
const server = Bun.serve({
|
||||
port,
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
async fetch(req) {
|
||||
const url = new URL(req.url);
|
||||
const path = url.pathname;
|
||||
|
||||
// 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",
|
||||
// The identity header lets another omp session's reuse probe positively
|
||||
// recognize this dashboard without allowing cross-origin API reads.
|
||||
const dashboardHeaders: Record<string, string> = {
|
||||
[STATS_DASHBOARD_HEADER]: "1",
|
||||
};
|
||||
|
||||
if (req.method === "OPTIONS") {
|
||||
return new Response(null, { headers: corsHeaders });
|
||||
return new Response(null, { headers: dashboardHeaders });
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -329,10 +327,10 @@ function createDashboardServer(port: number) {
|
||||
response = await handleStatic(path);
|
||||
}
|
||||
|
||||
// Add CORS headers to all responses
|
||||
// Add the dashboard identity header to all responses.
|
||||
const headers = new Headers(response.headers);
|
||||
for (const key in corsHeaders) {
|
||||
headers.set(key, corsHeaders[key]);
|
||||
for (const key in dashboardHeaders) {
|
||||
headers.set(key, dashboardHeaders[key]);
|
||||
}
|
||||
|
||||
return new Response(response.body, {
|
||||
@@ -343,7 +341,7 @@ function createDashboardServer(port: number) {
|
||||
console.error("Server error:", error);
|
||||
return Response.json(
|
||||
{ error: error instanceof Error ? error.message : "Unknown error" },
|
||||
{ status: 500, headers: corsHeaders },
|
||||
{ status: 500, headers: dashboardHeaders },
|
||||
);
|
||||
}
|
||||
},
|
||||
@@ -354,12 +352,13 @@ function createDashboardServer(port: number) {
|
||||
/**
|
||||
* Start the HTTP server, reusing a live dashboard or reclaiming a stale omp listener.
|
||||
*/
|
||||
export async function startServer(port = 3847): Promise<{ port: number; stop: () => void }> {
|
||||
export async function startServer(port = 3847): Promise<{ hostname: string; port: number; stop: () => void }> {
|
||||
await ensureClientBuild();
|
||||
|
||||
try {
|
||||
const server = createDashboardServer(port);
|
||||
return {
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
port: server.port ?? port,
|
||||
stop: () => server.stop(),
|
||||
};
|
||||
@@ -368,12 +367,13 @@ export async function startServer(port = 3847): Promise<{ port: number; stop: ()
|
||||
|
||||
const recovery = await recoverStatsPort(port);
|
||||
if (recovery === "reuse") {
|
||||
return { port, stop: () => {} };
|
||||
return { hostname: STATS_DASHBOARD_HOSTNAME, port, stop: () => {} };
|
||||
}
|
||||
|
||||
try {
|
||||
const server = createDashboardServer(port);
|
||||
return {
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
port: server.port ?? port,
|
||||
stop: () => server.stop(),
|
||||
};
|
||||
|
||||
@@ -1,22 +1,22 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import type { Subprocess } from "bun";
|
||||
import { STATS_DASHBOARD_HEADER } from "../src/port-conflict";
|
||||
import { STATS_DASHBOARD_HEADER, STATS_DASHBOARD_HOSTNAME } from "../src/port-conflict";
|
||||
import { startServer } from "../src/server";
|
||||
|
||||
const holderProcesses: Array<Subprocess<"ignore", "pipe", "pipe">> = [];
|
||||
|
||||
async function startBunHolder(responseExpr: string, options?: { statsOwned?: boolean }) {
|
||||
// Bind the wildcard address: `startServer` binds the wildcard too, and on
|
||||
// macOS SO_REUSEADDR lets a wildcard bind coexist with a 127.0.0.1-only
|
||||
// listener, which would bypass the EADDRINUSE path this suite exercises.
|
||||
// Bind the same loopback address as `startServer` so macOS cannot let a
|
||||
// wildcard and address-specific listener coexist on the reserved port.
|
||||
const reservation = Bun.serve({
|
||||
port: 0,
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
fetch: () => new Response("reserved"),
|
||||
});
|
||||
const port = reservation.port;
|
||||
reservation.stop(true);
|
||||
|
||||
const source = `Bun.serve({ port: ${port}, fetch: () => ${responseExpr} }); process.stdout.write("ready"); await Promise.withResolvers().promise;`;
|
||||
const source = `Bun.serve({ port: ${port}, hostname: "${STATS_DASHBOARD_HOSTNAME}", fetch: () => ${responseExpr} }); process.stdout.write("ready"); await Promise.withResolvers().promise;`;
|
||||
const args = [process.execPath, "-e", source];
|
||||
if (options?.statsOwned) args.push("omp-stats");
|
||||
const child = Bun.spawn(args, {
|
||||
@@ -46,10 +46,34 @@ afterEach(async () => {
|
||||
holderProcesses.length = 0;
|
||||
});
|
||||
|
||||
describe("startServer access", () => {
|
||||
it("only serves loopback requests without cross-origin access", async () => {
|
||||
const server = await startServer(0);
|
||||
|
||||
try {
|
||||
expect(server.hostname).toBe(STATS_DASHBOARD_HOSTNAME);
|
||||
const response = await fetch(`http://${server.hostname}:${server.port}/api/stats/models`);
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers.get(STATS_DASHBOARD_HEADER)).toBe("1");
|
||||
expect(response.headers.get("Access-Control-Allow-Origin")).toBeNull();
|
||||
await response.body?.cancel();
|
||||
|
||||
await expect(
|
||||
fetch(`http://127.0.0.2:${server.port}/api/stats/models`, {
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
}),
|
||||
).rejects.toThrow();
|
||||
} finally {
|
||||
server.stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("startServer port conflicts", () => {
|
||||
it("reuses a live stats dashboard identified by its header", async () => {
|
||||
const existing = Bun.serve({
|
||||
port: 0,
|
||||
hostname: STATS_DASHBOARD_HOSTNAME,
|
||||
fetch: request =>
|
||||
new URL(request.url).pathname === "/api/stats/models"
|
||||
? Response.json([], { headers: { [STATS_DASHBOARD_HEADER]: "1" } })
|
||||
@@ -62,7 +86,7 @@ describe("startServer port conflicts", () => {
|
||||
server.stop();
|
||||
|
||||
// The existing dashboard is untouched: it still answers on the port.
|
||||
const response = await fetch(`http://127.0.0.1:${existing.port}/api/stats/models`);
|
||||
const response = await fetch(`http://${STATS_DASHBOARD_HOSTNAME}:${existing.port}/api/stats/models`);
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers.get(STATS_DASHBOARD_HEADER)).toBe("1");
|
||||
await response.body?.cancel();
|
||||
@@ -76,7 +100,7 @@ describe("startServer port conflicts", () => {
|
||||
|
||||
await expect(startServer(holder.port)).rejects.toThrow("not identifiable as an omp stats dashboard");
|
||||
expect(holder.child.exitCode).toBeNull();
|
||||
const response = await fetch(`http://127.0.0.1:${holder.port}/api/stats/models`);
|
||||
const response = await fetch(`http://${STATS_DASHBOARD_HOSTNAME}:${holder.port}/api/stats/models`);
|
||||
expect(await response.json()).toEqual({ app: "spa" });
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user