diff --git a/devlog/_plan/260905_admin_token_local_ux/assets/local-session-notice-before-after.png b/devlog/_plan/260905_admin_token_local_ux/assets/local-session-notice-before-after.png new file mode 100644 index 0000000000..7eb2f54bad Binary files /dev/null and b/devlog/_plan/260905_admin_token_local_ux/assets/local-session-notice-before-after.png differ diff --git a/gui/src/api-targets.ts b/gui/src/api-targets.ts index 7a1a1d17d6..83eed5e9a1 100644 --- a/gui/src/api-targets.ts +++ b/gui/src/api-targets.ts @@ -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; diff --git a/gui/src/api.ts b/gui/src/api.ts index 1c0827f25e..020183dd47 100644 --- a/gui/src/api.ts +++ b/gui/src/api.ts @@ -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"; @@ -35,6 +42,18 @@ let rebootstrapTimeoutMs = SESSION_REBOOTSTRAP_TIMEOUT_MS; let resolutionWatchdogMs = RESOLUTION_WATCHDOG_MS; const runtimes = new Map(); +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 } })); + } 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 }; } @@ -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 }; diff --git a/gui/tests/api-auth-deadline.test.ts b/gui/tests/api-auth-deadline.test.ts index 4623867698..da354ff715 100644 --- a/gui/tests/api-auth-deadline.test.ts +++ b/gui/tests/api-auth-deadline.test.ts @@ -6,6 +6,7 @@ import { resetApiAuthFetchForTests, setRebootstrapTimeoutForTests, setResolutionWatchdogForTests, + SESSION_UNAVAILABLE_EVENT, } from "../src/api"; import { targetsFromMachineStatus, type MachineStatusV1 } from "../src/api-targets"; @@ -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 { return new Promise((_, reject) => { @@ -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) => { @@ -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; @@ -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; diff --git a/gui/tests/api-auth-memory.test.ts b/gui/tests/api-auth-memory.test.ts index 6483fbcaf5..825df3806d 100644 --- a/gui/tests/api-auth-memory.test.ts +++ b/gui/tests/api-auth-memory.test.ts @@ -53,6 +53,20 @@ async function installMockAuthFetch(handler: typeof fetch): Promise { 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; @@ -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; @@ -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) => { @@ -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 = []; @@ -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 @@ -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; @@ -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) => { @@ -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; diff --git a/src/server/gui-static.ts b/src/server/gui-static.ts index 3d97ce451d..75f3369be0 100644 --- a/src/server/gui-static.ts +++ b/src/server/gui-static.ts @@ -90,6 +90,22 @@ function runtimeRoleMeta(runtimeRole: string): string { return ``; } +/** + * 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 ``; +} function htmlDocumentResponse(html: string): Response { return new Response(html, { headers: { @@ -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("") ? html.replace("", `${bootstrap}`) : `${bootstrap}${html}`; } @@ -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); @@ -135,7 +161,7 @@ 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; @@ -143,7 +169,7 @@ export function serveGuiFile( 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). diff --git a/src/server/index.ts b/src/server/index.ts index 014913519e..af2382f59e 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -2141,6 +2141,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server