From 8d025489cf426d6adf0a2729b71699d6a5ff4c85 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 25 Jul 2026 21:52:26 -0500 Subject: [PATCH] fix(desktop): read a readiness-probe 401 by whether it was credentialed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The boot readiness probe calls the credential-free /api/health, and only a 404 flips it to the /api/status fallback. Both halves are wrong against a gated gateway. /api/health landed in ccab46ca4 (2026-07-24), after the v2026.7.20 tag, so every container pinned to a release lacks the route. On those backends the dashboard auth gate runs ahead of the SPA catch-all, so an unknown /api/* path is rejected as unauthenticated rather than 404 — a credential-free probe can never observe the 404 that the fallback keys on, and boot loops until the 45s deadline reporting a healthy backend as "did not become ready". Upgrading the backend is not a workaround for release-pinned deployments. Simulating a 0.19.0 backend (both the route and its PUBLIC_API_PATHS entry removed, since ccab46ca4 added the two together) shows the probe's 401 means two different things depending on whether credentials were sent: credential-free: /api/health -> 401 no_cookie, /api/status -> 200 credentialed: /api/health -> 404, /api/sessions -> 200 So the fix is not "fall back on any 401". Uncredentialed, a gate-shaped 401 identifies a missing route and must fall back. Credentialed, a 401/403 is a rejected session and must fail fast — falling back would hit the public /api/status, get a 200, and report a dead session as ready, deferring the no_cookie to the first real API call. Split the two cases and tag the credentialed rejection as a terminal reauth error. A generic 401 without the gate shape, plus 429 and 5xx, keep polling as before. --- apps/desktop/electron/backend-health.test.ts | 207 ++++++++++++++++++- apps/desktop/electron/backend-health.ts | 82 +++++++- 2 files changed, 284 insertions(+), 5 deletions(-) diff --git a/apps/desktop/electron/backend-health.test.ts b/apps/desktop/electron/backend-health.test.ts index f542f58c31d..99be2630a7f 100644 --- a/apps/desktop/electron/backend-health.test.ts +++ b/apps/desktop/electron/backend-health.test.ts @@ -2,7 +2,17 @@ import assert from 'node:assert/strict' import { test } from 'vitest' -import { DEFAULT_HEALTH_PROBE_TIMEOUT_MS, isMissingHealthEndpointError, waitForHermesReady } from './backend-health' +import { + DEFAULT_HEALTH_PROBE_TIMEOUT_MS, + isAuthRejectionError, + isGatedMissingHealthError, + isMissingHealthEndpointError, + isReauthRequiredError, + waitForHermesReady +} from './backend-health' + +const GATE_401 = + '401: {"error":"unauthenticated","detail":"Unauthorized","reason":"no_cookie","login_url":"/login"}' test('uses lightweight /api/health for current backends', async () => { const calls: string[][] = [] @@ -134,3 +144,198 @@ test('recognizes missing-route shapes only', () => { assert.equal(isMissingHealthEndpointError(new Error('Timed out connecting to Hermes backend after 15000ms')), false) assert.equal(isMissingHealthEndpointError(new Error('500: boom')), false) }) + +// --- Gated backends that predate /api/health (release 0.19.0 and earlier) --- +// +// The dashboard auth gate runs ahead of the SPA catch-all, so on a backend +// without the route an ANONYMOUS probe is rejected as unauthenticated rather +// than 404 — verified against a simulated 0.19.0 backend: +// credential-free: /api/health -> 401 no_cookie, /api/status -> 200 +// credentialed: /api/health -> 404, /api/sessions -> 200 + +test('anonymous gate-shaped 401 falls back to /api/status (backend predates /api/health)', async () => { + const calls: string[][] = [] + + await waitForHermesReady('http://192.168.1.132:9119', { + token: null, + fetchPublicJson: async url => { + calls.push(['public', url]) + throw new Error(GATE_401) + }, + fetchJson: async (url, token) => { + calls.push(['token', url, token == null ? 'null' : token]) + + return { version: '0.19.0', auth_required: true } + }, + sleep: async () => {}, + timeoutMs: 100, + pollMs: 1 + }) + + assert.deepEqual(calls, [ + ['public', 'http://192.168.1.132:9119/api/health'], + ['token', 'http://192.168.1.132:9119/api/status', 'null'] + ]) +}) + +test('a credentialed 401 fails fast for reauth instead of reporting a dead session ready', async () => { + // The regression a blanket 401->fallback introduces: /api/status is public, + // so an expired session would answer 200 and boot would report "ready", + // deferring the no_cookie to the first real API call. + const calls: string[][] = [] + + await assert.rejects( + waitForHermesReady('https://gateway.example', { + token: 'session-token', + fetchPublicJson: async () => { + throw new Error('public probe must not be used when credentialed') + }, + fetchJson: async url => { + calls.push(['status', url]) + + return { version: '0.19.0' } + }, + probeHealth: async url => { + calls.push(['probe', url]) + throw new Error(GATE_401) + }, + probeIsCredentialed: true, + sleep: async () => {}, + timeoutMs: 100, + pollMs: 1 + }), + (error: any) => { + assert.equal(isReauthRequiredError(error), true) + assert.equal(error.needsOauthLogin, true) + assert.match(error.message, /remote gateway session has expired/i) + + return true + } + ) + + // Fail fast: never reached the public /api/status leg. + assert.deepEqual(calls, [['probe', 'https://gateway.example/api/health']]) +}) + +test('a credentialed 403 is also a terminal reauth failure', async () => { + await assert.rejects( + waitForHermesReady('https://gateway.example', { + fetchPublicJson: async () => ({}), + fetchJson: async () => ({}), + probeHealth: async () => { + throw new Error('403: {"detail":"Forbidden"}') + }, + probeIsCredentialed: true, + sleep: async () => {}, + timeoutMs: 100, + pollMs: 1 + }), + (error: any) => isReauthRequiredError(error) + ) +}) + +test('a credentialed probe still uses the 404 fallback for a genuinely missing route', async () => { + // With credentials the gate lets the request through to the SPA catch-all, + // so an old backend answers a real 404 — that must still fall back, not be + // mistaken for a rejected session. + const calls: string[][] = [] + + await waitForHermesReady('https://gateway.example', { + token: 'session-token', + fetchPublicJson: async () => { + throw new Error('public probe must not be used when credentialed') + }, + fetchJson: async url => { + calls.push(['status', url]) + + return { version: '0.19.0' } + }, + probeHealth: async url => { + calls.push(['probe', url]) + throw new Error('404: {"detail":"Not Found"}') + }, + probeIsCredentialed: true, + sleep: async () => {}, + timeoutMs: 100, + pollMs: 1 + }) + + assert.deepEqual(calls, [ + ['probe', 'https://gateway.example/api/health'], + ['status', 'https://gateway.example/api/status'] + ]) +}) + +test('a non-gate 401 keeps polling rather than skipping a misconfigured health route', async () => { + const calls: string[][] = [] + let currentTime = 0 + + await assert.rejects( + waitForHermesReady('http://127.0.0.1:9000', { + fetchPublicJson: async url => { + calls.push(['public', url]) + throw new Error('401: {"detail":"Unauthorized"}') + }, + fetchJson: async url => { + calls.push(['token', url]) + }, + sleep: async () => {}, + now: () => { + currentTime += 20 + + return currentTime + }, + timeoutMs: 50, + pollMs: 1 + }), + /401: \{"detail":"Unauthorized"\}/ + ) + + assert.ok(calls.length > 0) + assert.ok(calls.every(call => call[0] === 'public' && call[1].endsWith('/api/health'))) +}) + +test('credentialed 5xx and 429 keep polling — only 401/403 are terminal', async () => { + for (const transient of ['500: boom', '429: {"detail":"Too Many Requests"}']) { + let attempts = 0 + let currentTime = 0 + + await assert.rejects( + waitForHermesReady('https://gateway.example', { + fetchPublicJson: async () => ({}), + fetchJson: async () => ({}), + probeHealth: async () => { + attempts += 1 + throw new Error(transient) + }, + probeIsCredentialed: true, + sleep: async () => {}, + now: () => { + currentTime += 20 + + return currentTime + }, + timeoutMs: 100, + pollMs: 1 + }), + (error: any) => isReauthRequiredError(error) === false + ) + + assert.ok(attempts > 1, `${transient} should have retried, got ${attempts} attempt(s)`) + } +}) + +test('error-shape predicates', () => { + assert.equal(isGatedMissingHealthError(new Error(GATE_401)), true) + assert.equal(isGatedMissingHealthError(new Error('401: {"detail":"Unauthorized"}')), false) + assert.equal(isGatedMissingHealthError(new Error('404: {"detail":"Not Found"}')), false) + + assert.equal(isAuthRejectionError(new Error(GATE_401)), true) + assert.equal(isAuthRejectionError(new Error('403: {"detail":"Forbidden"}')), true) + assert.equal(isAuthRejectionError(new Error('404: {"detail":"Not Found"}')), false) + assert.equal(isAuthRejectionError(new Error('429: slow down')), false) + assert.equal(isAuthRejectionError(new Error('500: boom')), false) + + // A gated 401 must NOT be conflated with a missing route by the 404 predicate. + assert.equal(isMissingHealthEndpointError(new Error(GATE_401)), false) +}) diff --git a/apps/desktop/electron/backend-health.ts b/apps/desktop/electron/backend-health.ts index 623208545e2..3da6c7a80fa 100644 --- a/apps/desktop/electron/backend-health.ts +++ b/apps/desktop/electron/backend-health.ts @@ -20,14 +20,72 @@ export interface HermesReadyOptions { healthProbeTimeoutMs?: number sleep?: (ms: number) => Promise now?: () => number + /** + * Credentialed health probe. When supplied, readiness is probed with the + * connection's own credentials instead of anonymously — which is what lets + * a gated backend answer 404 for a genuinely missing /api/health, and what + * makes a 401 from this probe mean "session rejected" rather than "route + * behind a gate". Defaults to the credential-free `fetchPublicJson`. + */ + probeHealth?: (url: string, options?: { timeoutMs?: number }) => Promise + /** + * Whether `probeHealth` actually presents credentials. Distinguishes the + * two very different meanings of a 401 (see `waitForHermesReady`). + */ + probeIsCredentialed?: boolean } +export const REMOTE_SESSION_EXPIRED_MESSAGE = + 'Your remote gateway session has expired. Open Settings → Gateway and click "Sign in" again.' + export function isMissingHealthEndpointError(error: unknown): boolean { const message = error instanceof Error ? error.message : String(error ?? '') return /^404:/.test(message) || message.includes('endpoint is likely missing') } +/** + * True for a hard auth rejection (401/403) as opposed to a transient failure. + * Deliberately shape-based: 429 is a throttle and 5xx is a server fault, and + * both must keep polling. + */ +export function isAuthRejectionError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error ?? '') + + return /^40[13]:/.test(message) +} + +/** + * True for an auth rejection carrying the dashboard gate's "no session at all" + * shape. On a backend that predates `/api/health`, the gate runs ahead of the + * SPA catch-all, so an unknown `/api/*` path is rejected as unauthenticated + * instead of 404 — this is the signal that an ANONYMOUS probe cannot reach the + * route, and the reason a credential-free 401 must fall back to `/api/status` + * rather than be reported as a boot failure. + */ +export function isGatedMissingHealthError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error ?? '') + + return isAuthRejectionError(error) && message.includes('no_cookie') +} + +/** Tag a terminal reauth failure the main process latches and the overlay keys on. */ +export function makeReauthRequiredError(detail?: string): Error { + const error = new Error(REMOTE_SESSION_EXPIRED_MESSAGE) as any + error.needsOauthLogin = true + error.isReauthRequired = true + + if (detail) { + error.detail = detail + } + + return error +} + +export function isReauthRequiredError(error: unknown): boolean { + return Boolean((error as any)?.isReauthRequired) +} + function supersededError() { const error: any = new Error('SSH bootstrap was superseded by newer connection settings.') error.kind = 'superseded' @@ -59,6 +117,8 @@ export async function waitForHermesReady(baseUrl: string, options: HermesReadyOp const base = baseUrl.replace(/\/+$/, '') const deadline = now() + timeoutMs + const probeHealth = options.probeHealth ?? options.fetchPublicJson + const probeIsCredentialed = Boolean(options.probeIsCredentialed) let lastError: unknown = null let useStatusFallback = false @@ -71,16 +131,30 @@ export async function waitForHermesReady(baseUrl: string, options: HermesReadyOp if (useStatusFallback) { await options.fetchJson(`${base}/api/status`, options.token) } else { - await options.fetchPublicJson(`${base}/api/health`, { timeoutMs: healthProbeTimeoutMs }) + await probeHealth(`${base}/api/health`, { timeoutMs: healthProbeTimeoutMs }) } return } catch (error) { lastError = error - // Only an explicitly missing route means the backend predates - // /api/health; timeouts and server errors keep polling health. - if (!useStatusFallback && isMissingHealthEndpointError(error)) { + // A confirmed 401/403 from a CREDENTIALED probe means the session was + // rejected, not that the route is missing. Fail fast into a reauth + // state: falling back to the public /api/status would answer 200 and + // report a dead session as "ready", deferring the failure to the first + // real API call. Applies to the /api/status leg too — it is routed + // through the same credentials. + if (probeIsCredentialed && isAuthRejectionError(error)) { + throw makeReauthRequiredError(error instanceof Error ? error.message : String(error)) + } + + // An explicitly missing route means the backend predates /api/health. + // So does a gate-shaped 401 on an ANONYMOUS probe: the dashboard auth + // gate runs ahead of the SPA catch-all, so a pre-/api/health backend + // rejects the unknown path as unauthenticated instead of 404 and a + // credential-free probe can never observe the 404. Timeouts, 5xx, 429, + // and non-gate 401s keep polling health. + if (!useStatusFallback && (isMissingHealthEndpointError(error) || isGatedMissingHealthError(error))) { useStatusFallback = true continue