Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
36 changes: 36 additions & 0 deletions gui/src/api-targets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,42 @@ export function isConnectedRuntime(): boolean {
return runtimeRoleFromDocument() === "client";
}

/**
* May this dashboard ask the user to type an admin token?
*
* This asks the BIND, not the topology. The server decides whether a typed credential is
* required with `isApiAuthRequired` — "is the bind hostname non-loopback" — and now states
* that answer in the served document alongside the role.
*
* The role is the wrong predicate, and it was tried first: `standalone` + `hostname:
* "0.0.0.0"` is an operator who deliberately exposed the dashboard and MUST type the admin
* token (tests/server-management-auth.test.ts, "a non-loopback binding never issues a GUI
* session from a forged loopback Host"), while a `hub` on loopback still mints its own
* session. Gating on `role === "hub"` would have hidden the prompt from exactly the
* operator who needs it.
*
* A loopback install mints its own session, so a refusal there is a Host/Origin
* misconfiguration rather than a missing credential: prompting asks the user a question they
* did not cause and cannot fix by answering (#3353). The published contract already promises
* loopback "never asks for a token"
* (docs-site/src/content/docs/guides/web-dashboard.md, "Sign-in").
*
* A missing tag means an older server, a separately hosted GUI, or the Vite dev server; those
* fall back to the role so a hub dashboard still works against a server that predates the
* tag, and everything else reads as loopback — the safe default this file already uses.
*/
export function adminTokenPromptAllowed(): boolean {
if (typeof document !== "undefined") {
const declared = document
.querySelector('meta[name="opencodex-management-auth-required"]')
?.getAttribute("content")
?.trim();
if (declared === "1") return true;
if (declared === "0") return false;
}
return runtimeRoleFromDocument() === "hub";
}

export interface ApiTarget {
id: ApiPlane;
baseUrl: string;
Expand Down
29 changes: 28 additions & 1 deletion gui/src/api.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import { promptForAdminToken, type AdminTokenVerifier } from "./admin-token-dialog";
import { createBoundedFetch } from "./bounded-fetch";
import { standaloneApiTargets, type ApiPlane, type ApiTarget, type ApiTargets } from "./api-targets";
import { adminTokenPromptAllowed, standaloneApiTargets, type ApiPlane, type ApiTarget, type ApiTargets } from "./api-targets";

/**
* Fired instead of the admin-token prompt when the dashboard cannot start a session on a
* deployment that has no admin token to type. The shell renders it as a notice; nothing
* blocks on it.
*/
export const SESSION_UNAVAILABLE_EVENT = "opencodex:session-unavailable";

const LEGACY_TOKEN_KEY = "opencodex-api-token";
const ADMIN_TOKEN_VALIDATION_PATH = "/api/settings";
Expand Down Expand Up @@ -35,6 +42,18 @@ let rebootstrapTimeoutMs = SESSION_REBOOTSTRAP_TIMEOUT_MS;
let resolutionWatchdogMs = RESOLUTION_WATCHDOG_MS;
const runtimes = new Map<ApiPlane, TargetRuntime>();

function reportSessionUnavailable(plane: ApiPlane): void {
if (typeof window === "undefined") return;
// Take the constructor off the same window we dispatch on: a test harness (and a
// sandboxed embed) can supply a document without installing CustomEvent globally.
const Ctor = (window as unknown as { CustomEvent?: typeof CustomEvent }).CustomEvent
?? (typeof CustomEvent === "function" ? CustomEvent : null);
if (!Ctor) return;
try {
window.dispatchEvent(new Ctor(SESSION_UNAVAILABLE_EVENT, { detail: { plane } }));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Render the session-unavailable event in the dashboard

When a loopback session bootstrap receives a definitive 4xx, this path suppresses all later authentication resolution and dispatches an event, but no production code listens for SESSION_UNAVAILABLE_EVENT (the only production occurrences are the declaration and this emitter). Consequently, the promised actionable Host/Origin notice never appears and the dashboard continues failing with 401 responses without explaining how to recover; add a shell-level listener that renders a localized alert before setting promptCancelled permanently.

Useful? React with 👍 / 👎.

} catch { /* a shell that cannot receive the notice must not break the fetch path */ }
}

function blankSession(): ApiSessionState {
return { token: null, csrfToken: null, browserOrigin: null, serverOrigin: null };
}
Expand Down Expand Up @@ -258,6 +277,14 @@ async function resolveTokenAfter401(plane: ApiPlane, failedToken: string | null,
]).finally(() => clearTimeout(watchdog));
if (renewed.kind === "minted") return renewed.token;
if (renewed.kind === "failed") return null;
// A non-hub deployment has no admin token the user could supply: the server mints the
// session itself, so a refusal is a Host/Origin misconfiguration. Surface that instead
// of a password box the user cannot answer (#3353, #3483).
if (!adminTokenPromptAllowed()) {
state.promptCancelled = true;
reportSessionUnavailable(plane);
return null;
}
const prompted = await requestAdminToken(token => verifyAdminToken(plane, token));
if (prompted) {
state.session = { token: prompted, csrfToken: null, browserOrigin: null, serverOrigin: state.target.serverOrigin };
Expand Down
93 changes: 93 additions & 0 deletions gui/tests/api-auth-deadline.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
resetApiAuthFetchForTests,
setRebootstrapTimeoutForTests,
setResolutionWatchdogForTests,
SESSION_UNAVAILABLE_EVENT,
} from "../src/api";
import { targetsFromMachineStatus, type MachineStatusV1 } from "../src/api-targets";

Expand Down Expand Up @@ -63,6 +64,28 @@ function pathnameOf(input: RequestInfo | URL): string {
return new URL(input instanceof Request ? input.url : String(input), "http://localhost/").pathname;
}

/**
* Declare the served runtime role, the way `src/server/gui-static.ts` does.
*
* The admin-token prompt is hub-only: a standalone loopback install mints its own session,
* so a refusal there is a Host/Origin misconfiguration no typed token can repair (#3353).
* A test that wants to observe the prompt has to say it is a hub.
*/
function declareRuntimeRole(role: string): void {
const meta = document.createElement("meta");
meta.setAttribute("name", "opencodex-runtime-role");
meta.setAttribute("content", role);
document.head.append(meta);
}

/** Declare the bind's credential requirement, as `serveGuiFile` does from `isApiAuthRequired`. */
function declareManagementAuthRequired(required: boolean): void {
const meta = document.createElement("meta");
meta.setAttribute("name", "opencodex-management-auth-required");
meta.setAttribute("content", required ? "1" : "0");
document.head.append(meta);
}

/** A hang that honors the abort signal, like real fetch does. */
function hangUntilAborted(signal?: AbortSignal | null): Promise<Response> {
return new Promise<Response>((_, reject) => {
Expand Down Expand Up @@ -152,6 +175,7 @@ test("hung bootstrap fails the wave within the deadline and a later wave re-boot
});

test("bootstrap timeout and 5xx never open the admin-token prompt; only refusal does", async () => {
declareRuntimeRole("hub");
setRebootstrapTimeoutForTests(40);
let mode: "hang" | "bad-gateway" | "refuse" = "hang";
const mockFetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
Expand All @@ -174,6 +198,74 @@ test("bootstrap timeout and 5xx never open the admin-token prompt; only refusal
expect(promptCalls).toBe(1);
});

/*
* #3353 / the local-user UX defect.
*
* A plain loopback install mints its own GUI session, so a definitive bootstrap refusal
* there is a Host/Origin misconfiguration — not a missing credential. The dashboard used
* to answer it with a password box the user could not fill and that would not have helped
* if they could. The published contract already promised loopback "never asks for a token".
*/
test("a standalone dashboard is never asked for an admin token, and says why instead", async () => {
declareRuntimeRole("standalone");
setRebootstrapTimeoutForTests(40);
const events: string[] = [];
window.addEventListener(SESSION_UNAVAILABLE_EVENT, () => { events.push("notice"); });

// Definitive refusal on both the API and the bootstrap: the exact shape that used to prompt.
const mockFetch = (async () => new Response("unauthorized", { status: 401 })) as typeof fetch;
await installMockAuthFetch(mockFetch);

expect((await fetch("/api/config")).status).toBe(401);
expect(promptCalls).toBe(0);
expect(events).toEqual(["notice"]);

// And it must not re-ask on every later failure.
expect((await fetch("/api/providers")).status).toBe(401);
expect(promptCalls).toBe(0);
});

/* An absent tag is an older server or the Vite dev server: still not a hub, still no prompt. */
test("a document with no runtime-role tag is treated as standalone, not hub", async () => {
setRebootstrapTimeoutForTests(40);
const mockFetch = (async () => new Response("unauthorized", { status: 401 })) as typeof fetch;
await installMockAuthFetch(mockFetch);

expect((await fetch("/api/config")).status).toBe(401);
expect(promptCalls).toBe(0);
});

/*
* The operator this fix must NOT lock out.
*
* `runtimeRole` is a topology signal, not an auth one: a standalone install bound to
* 0.0.0.0 deliberately exposed its dashboard and has to type the admin token, while a hub on
* loopback still mints its own session. Gating the prompt on the role would have hidden it
* from exactly this person, so the gate reads the bind's own requirement instead.
*/
test("an exposed standalone bind still gets the prompt", async () => {
declareRuntimeRole("standalone");
declareManagementAuthRequired(true);
setRebootstrapTimeoutForTests(40);
const mockFetch = (async () => new Response("unauthorized", { status: 401 })) as typeof fetch;
await installMockAuthFetch(mockFetch);

expect((await fetch("/api/config")).status).toBe(401);
expect(promptCalls).toBe(1);
});

/* And a hub on a loopback bind mints its own session, so it must not be asked. */
test("a loopback hub is not asked for a token", async () => {
declareRuntimeRole("hub");
declareManagementAuthRequired(false);
setRebootstrapTimeoutForTests(40);
const mockFetch = (async () => new Response("unauthorized", { status: 401 })) as typeof fetch;
await installMockAuthFetch(mockFetch);

expect((await fetch("/api/config")).status).toBe(401);
expect(promptCalls).toBe(0);
});

test("caller abort during a pending resolution unwinds only that caller", async () => {
setRebootstrapTimeoutForTests(5_000);
let releaseBootstrap: (() => void) | null = null;
Expand Down Expand Up @@ -274,6 +366,7 @@ test("the watchdog never bounds the prompt: slow user input stacks no dialogs an
// resolution escalates to the prompt. The prompt is user-controlled: the watchdog
// must NOT fire around it, and later 401 waves must join the pending body instead
// of opening another dialog (promptForAdminToken has no singleton guard).
declareRuntimeRole("hub");
setRebootstrapTimeoutForTests(50);
setResolutionWatchdogForTests(120);
let bootstrapCalls = 0;
Expand Down
21 changes: 21 additions & 0 deletions gui/tests/api-auth-memory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@ async function installMockAuthFetch(handler: typeof fetch): Promise<void> {
Object.defineProperty(globalThis, "fetch", { configurable: true, value: window.fetch });
}

/**
* Declare a bind that requires a typed credential, as `serveGuiFile` does from
* `isApiAuthRequired`. The admin-token prompt only exists for a non-loopback bind: a
* loopback dashboard mints its own session, so a refusal there is a Host/Origin
* misconfiguration no typed token can repair (#3353). A test that wants to observe the
* prompt fallback has to say it is that kind of deployment.
*/
function declareManagementAuthRequired(): void {
const meta = document.createElement("meta");
meta.setAttribute("name", "opencodex-management-auth-required");
meta.setAttribute("content", "1");
document.head.append(meta);
}

test("installApiAuthFetch deletes legacy sessionStorage token without reading it", () => {
sessionStorage.setItem(LEGACY_TOKEN_KEY, "legacy-secret");
let getItemCalls = 0;
Expand All @@ -73,6 +87,7 @@ test("installApiAuthFetch deletes legacy sessionStorage token without reading it
});

test("prompted API tokens stay memory-only and are not written to sessionStorage", async () => {
declareManagementAuthRequired();
sessionStorage.setItem(LEGACY_TOKEN_KEY, "legacy-secret");

let authorized = false;
Expand All @@ -96,6 +111,7 @@ test("prompted API tokens stay memory-only and are not written to sessionStorage
});

test("validates prompted tokens with a safe read before retrying the failed request", async () => {
declareManagementAuthRequired();
const validationResults: string[] = [];
const seenRequests: Array<[string, string | null]> = [];
resetApiAuthFetchForTests(async (verifyToken) => {
Expand Down Expand Up @@ -127,6 +143,7 @@ test("validates prompted tokens with a safe read before retrying the failed requ
});

test("cross-origin /api/* requests do not receive the API key or token prompt", async () => {
declareManagementAuthRequired();
let promptCalls = 0;
let phase: "seed" | "cross" = "seed";
const seenHeaders: Array<string | null> = [];
Expand Down Expand Up @@ -158,6 +175,7 @@ test("cross-origin /api/* requests do not receive the API key or token prompt",
});

test("concurrent 401s share one token prompt and all retry with the stored token", async () => {
declareManagementAuthRequired();
// Repro for #647: many /api/* requests start without a token (dashboard fan-out).
// Delivering 401s one-by-one after each auth cycle finishes matches the browser case where
// window.prompt blocks the main thread: each continuation still holds a captured null token
Expand Down Expand Up @@ -227,6 +245,7 @@ test("concurrent 401s share one token prompt and all retry with the stored token
});

test("stale concurrent 401 does not clear a token refreshed by another request", async () => {
declareManagementAuthRequired();
// Codex/CodeRabbit race: request A prompts and stores T2; request B still holding stale T1
// must not wipe T2 (clearTokenIfCurrent) before its re-read / shared gate join.
let promptCalls = 0;
Expand Down Expand Up @@ -285,6 +304,7 @@ test("stale concurrent 401 does not clear a token refreshed by another request",
});

test("canceling the token prompt once does not reopen it for the rest of the 401 fan-out", async () => {
declareManagementAuthRequired();
let promptCalls = 0;
const release401: Array<() => void> = [];
const mockFetch = (async (_input: RequestInfo | URL, init?: RequestInit) => {
Expand Down Expand Up @@ -439,6 +459,7 @@ test("expired session silently re-bootstraps from the served document without pr
});

test("a session minted for another origin is rejected and the prompt fallback stays", async () => {
declareManagementAuthRequired();
// Non-loopback dashboards never get server-minted sessions; a re-bootstrap document whose
// origin does not match must not be trusted, and the operator-only prompt remains.
let promptCalls = 0;
Expand Down
34 changes: 30 additions & 4 deletions src/server/gui-static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,22 @@ function runtimeRoleMeta(runtimeRole: string): string {
return `<meta name="opencodex-runtime-role" content="${escapeHtmlAttribute(runtimeRole)}">`;
}

/**
* Does this bind require a typed management credential?
*
* Emitted for the same reason as the role: so the dashboard can answer a question on first
* paint without asking a remote-hub endpoint. It is NOT the same question as the role.
* `standalone` + `hostname: "0.0.0.0"` is an operator who deliberately exposed the dashboard
* and must type the admin token, while a `hub` on loopback still mints its own session — so
* the role cannot stand in for this, and using it that way locked out exactly the operator
* who is supposed to see the prompt.
*
* Non-secret: it restates the bind the operator chose, which `/healthz` and the dashboard
* already reflect.
*/
function managementAuthRequiredMeta(required: boolean): string {
return `<meta name="opencodex-management-auth-required" content="${required ? "1" : "0"}">`;
}
function htmlDocumentResponse(html: string): Response {
return new Response(html, {
headers: {
Expand All @@ -101,9 +117,18 @@ function htmlDocumentResponse(html: string): Response {
});
}

function htmlResponse(path: string, session?: GuiSessionBootstrap, runtimeRole?: string): Response {
function htmlResponse(
path: string,
session?: GuiSessionBootstrap,
runtimeRole?: string,
managementAuthRequired?: boolean,
): Response {
let html = readFileSync(path, "utf8");
const bootstrap = `${runtimeRole ? runtimeRoleMeta(runtimeRole) : ""}${session ? sessionBootstrapMeta(session) : ""}`;
const bootstrap = [
runtimeRole ? runtimeRoleMeta(runtimeRole) : "",
managementAuthRequired === undefined ? "" : managementAuthRequiredMeta(managementAuthRequired),
session ? sessionBootstrapMeta(session) : "",
].join("");
if (bootstrap) {
html = html.includes("</head>") ? html.replace("</head>", `${bootstrap}</head>`) : `${bootstrap}${html}`;
}
Expand All @@ -126,6 +151,7 @@ export function serveGuiFile(
guiDist = findGuiDist(),
session?: GuiSessionBootstrap,
runtimeRole?: string,
managementAuthRequired?: boolean,
): Response | null {
if (!guiDist) return null;
const filePath = resolveGuiFilePath(guiDist, pathname);
Expand All @@ -135,15 +161,15 @@ export function serveGuiFile(
if (!extname(pathname)) {
const indexPath = join(guiDist, "index.html");
if (isFile(indexPath)) {
return htmlResponse(indexPath, session, runtimeRole);
return htmlResponse(indexPath, session, runtimeRole, managementAuthRequired);
}
}
return null;
}

const ext = extname(filePath);
const contentType = MIME_TYPES[ext] || "application/octet-stream";
if (ext === ".html") return htmlResponse(filePath, session, runtimeRole);
if (ext === ".html") return htmlResponse(filePath, session, runtimeRole, managementAuthRequired);
// Snapshot bytes before returning the response. Bun.file is lazy: if gui/dist is replaced
// after Bun frames the response but before the stream finishes, its Content-Length can
// describe the old file while the body comes from the new one (#2792).
Expand Down
1 change: 1 addition & 0 deletions src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2141,6 +2141,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server<W
undefined,
guiSessionCandidate ?? undefined,
config.runtimeRole ?? "standalone",
isApiAuthRequired(config),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add server coverage for the emitted auth-requirement tag

This new argument controls whether deployed dashboards can recover by entering an admin token, but the added GUI tests manually construct the meta tag and no server-side test asserts that startServer/serveGuiFile emits 1 for non-loopback binds and 0 for loopback binds. A wiring or predicate regression would therefore either lock remote operators out or restore the unwanted local prompt while all new tests remain green; add focused coverage at the server/static-serving boundary.

AGENTS.md reference: src/AGENTS.md:L24-L24

Useful? React with 👍 / 👎.

);
if (guiFile) return guiFile;
if (url.pathname === "/" && req.method === "GET") {
Expand Down
Loading