diff --git a/apps/desktop/electron/backend-health.test.ts b/apps/desktop/electron/backend-health.test.ts index f542f58c31d9..99be2630a7f6 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 623208545e20..3da6c7a80fa7 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 diff --git a/apps/desktop/electron/backend-start-failure.test.ts b/apps/desktop/electron/backend-start-failure.test.ts index 0888d65fbc41..36d352d66cc2 100644 --- a/apps/desktop/electron/backend-start-failure.test.ts +++ b/apps/desktop/electron/backend-start-failure.test.ts @@ -2,7 +2,7 @@ import assert from 'node:assert/strict' import { test } from 'vitest' -import { shouldLatchBackendStartFailure } from './backend-start-failure' +import { shouldLatchBackendStartFailure, shouldLatchRemoteReauthFailure } from './backend-start-failure' test('latches a LOCAL backend failure so the install-retry loop is broken', () => { assert.equal(shouldLatchBackendStartFailure({ attemptedRemote: false }), true) @@ -21,3 +21,32 @@ test('the two branches are mutually exclusive (a failure either latches or stays assert.equal(latched, !attemptedRemote) } }) + +test('latches a CONFIRMED remote reauth failure so the overlay stays clickable', () => { + // Without this the non-latching remote path re-runs startHermes on every + // getConnection/api call, re-emits running:true, and the overlay hides + // itself — the "Sign in" button flickers away before it can be clicked. + assert.equal(shouldLatchRemoteReauthFailure({ attemptedRemote: true, isReauth: true }), true) +}) + +test('does not latch a transient remote failure as reauth', () => { + // A mint timeout or a host unreachable across sleep must still self-heal. + assert.equal(shouldLatchRemoteReauthFailure({ attemptedRemote: true, isReauth: false }), false) +}) + +test('never latches a LOCAL failure as reauth (that is backendStartFailure job)', () => { + assert.equal(shouldLatchRemoteReauthFailure({ attemptedRemote: false, isReauth: true }), false) + assert.equal(shouldLatchRemoteReauthFailure({ attemptedRemote: false, isReauth: false }), false) +}) + +test('the two latches never fire for the same failure', () => { + // They are complementary, not overlapping: local failures latch via + // backendStartFailure, confirmed remote reauth latches via its own flag. + for (const attemptedRemote of [true, false]) { + for (const isReauth of [true, false]) { + const start = shouldLatchBackendStartFailure({ attemptedRemote }) + const reauth = shouldLatchRemoteReauthFailure({ attemptedRemote, isReauth }) + assert.ok(!(start && reauth), `both latched for remote=${attemptedRemote} reauth=${isReauth}`) + } + } +}) diff --git a/apps/desktop/electron/backend-start-failure.ts b/apps/desktop/electron/backend-start-failure.ts index 4998b0164a70..3c5ffdbcad91 100644 --- a/apps/desktop/electron/backend-start-failure.ts +++ b/apps/desktop/electron/backend-start-failure.ts @@ -39,3 +39,34 @@ export interface BackendStartFailureContext { export function shouldLatchBackendStartFailure(context: BackendStartFailureContext): boolean { return !context.attemptedRemote } + +export interface RemoteReauthFailureContext { + /** True when the boot that just failed was dialing a REMOTE (or cloud) backend. */ + attemptedRemote: boolean + /** + * True when the failure was a CONFIRMED auth rejection (a credentialed + * probe got 401/403), not a transient connectivity fault. + */ + isReauth: boolean +} + +/** + * Whether a failed remote boot should latch as a reauth failure. + * + * This is the deliberate counterpart to `shouldLatchBackendStartFailure`, + * which never latches a remote failure because remote faults are usually + * transient and must stay retryable. A *confirmed* reauth rejection is the + * exception: it cannot self-heal, because nothing will change until the user + * signs in again. + * + * Without a latch, the non-latching remote path actively prevents recovery. + * Every subsequent `getConnection`/`api` call re-runs `startHermes`, re-emits + * `running: true`, and the boot-failure overlay (`visible = Boolean(boot.error) + * && !boot.running`) hides itself — so the "Sign in" button flickers out from + * under the user before they can click it. Latching holds the overlay still + * and clickable. Cleared on every recovery path (reset, repair, apply-config, + * and a confirmed sign-in) so a fresh session boots normally. + */ +export function shouldLatchRemoteReauthFailure(context: RemoteReauthFailureContext): boolean { + return context.attemptedRemote && context.isReauth +} diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 89b350f1b4b9..f967aa24c3b8 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -34,10 +34,10 @@ import { stopBackendChild as stopBackendChildImpl } from './backend-child' import { dashboardFallbackArgs, sourceDeclaresServe } from './backend-command' import { createBackendConnectionState } from './backend-connection-state' import { buildDesktopBackendEnv, normalizeHermesHomeRoot } from './backend-env' -import { waitForHermesReady } from './backend-health' +import { isReauthRequiredError, waitForHermesReady } from './backend-health' import { canImportHermesCli, shouldTrustHermesOverride, verifyHermesCli } from './backend-probes' import { waitForDashboardPortAnnouncement } from './backend-ready' -import { shouldLatchBackendStartFailure } from './backend-start-failure' +import { shouldLatchBackendStartFailure, shouldLatchRemoteReauthFailure } from './backend-start-failure' import { detectRemoteDisplay, isWindowsBinaryPathInWsl, isWslEnvironment } from './bootstrap-platform' import { runBootstrap } from './bootstrap-runner' import { applyConnectionChange, resolveTerminalConnection } from './connection-apply' @@ -119,7 +119,13 @@ import { } from './hardening' import { createLinkTitleWindow, guardLinkTitleSession, readLinkTitleWindowTitle } from './link-title-window' import { ensureMainWindow } from './main-window-lifecycle' -import { oauthSessionIsLive, resolveJsonBody, resolveOauthRestAuth } from './native-auth-decisions' +import { + oauthGuardMayHardFail, + oauthSessionIsLive, + resolveJsonBody, + resolveOauthRestAuth, + resolveReadinessProbeAuth +} from './native-auth-decisions' import { nativeRefreshUrl, type NativeTokenSet, @@ -1016,6 +1022,13 @@ let bootstrapFailure = null // Latched non-bootstrap backend spawn failure — stops getConnection() from // respawning hermes serve backend children in a tight loop while boot is broken. let backendStartFailure = null +// Latched CONFIRMED remote reauth failure. Remote failures deliberately do not +// latch via backendStartFailure (they're usually transient and must stay +// retryable), but a rejected session cannot self-heal — and the non-latching +// path actively breaks recovery: each retry re-emits running:true and hides +// the boot-failure overlay, so the "Sign in" button flickers away before it +// can be clicked. Cleared on every recovery path and on a confirmed sign-in. +let remoteReauthFailure = null // Active first-launch install, so the renderer's Cancel button (and app quit) // can abort the in-flight install.sh/ps1 instead of leaving it running. let bootstrapAbortController = null @@ -4815,12 +4828,87 @@ function closePreviewWatchers() { } } -async function waitForHermes(baseUrl, token, signal?) { +// Best-effort read of a gateway's advertised auth providers, cached per base +// URL for the life of the process. Used by the oauth pre-flight guard to tell +// a password-provider gateway (which cannot satisfy the bearer/cookie checks +// by design) from a real OAuth one. Any failure returns [] so callers keep the +// strict guard — backends predating /api/auth/providers are unaffected. +const gatewayAuthProvidersCache = new Map() + +async function gatewayAuthProviders(baseUrl) { + const cached = gatewayAuthProvidersCache.get(baseUrl) + + if (cached) { + return cached + } + + let providers = [] + + try { + const body = (await fetchPublicJson(`${baseUrl}/api/auth/providers`, { timeoutMs: 8_000 })) as any + + if (Array.isArray(body?.providers)) { + providers = body.providers + .filter(p => p && typeof p === 'object') + .map(p => ({ name: String(p.name || ''), supportsPassword: Boolean(p.supports_password) })) + .filter(p => p.name) + } + } catch { + // Optional metadata — an unreadable list keeps the strict guard. + } + + gatewayAuthProvidersCache.set(baseUrl, providers) + + return providers +} + +// Build the readiness probe for a connection's auth mode. A gated gateway +// must be probed with the SAME credentials the rest of the connection uses: +// an anonymous probe 401s forever against a live session, and it can never +// see the 404 that identifies a backend predating /api/health (the auth gate +// answers before the SPA catch-all). `probeIsCredentialed` tells +// waitForHermesReady how to read a 401 — rejected session vs gated route. +async function buildReadinessHealthProbe(baseUrl, authMode, token) { + const nativeAt = authMode === 'oauth' ? await ensureNativeAccessToken(baseUrl).catch(() => null) : null + const probeAuth = resolveReadinessProbeAuth(authMode, nativeAt, token) + + if (probeAuth.kind === 'bearer') { + return { + // fetchJson takes the bearer via `options.bearer` — a raw `headers` + // option is ignored, so passing one here would silently probe + // uncredentialed and reintroduce the 401 loop. + probeHealth: (url, options: any = {}) => fetchJson(url, null, { ...options, bearer: probeAuth.token }), + probeIsCredentialed: true + } + } + + if (probeAuth.kind === 'cookie') { + return { + probeHealth: (url, options: any = {}) => fetchJsonViaOauthSession(url, options), + probeIsCredentialed: true + } + } + + if (probeAuth.kind === 'token' && probeAuth.token) { + return { + probeHealth: (url, options: any = {}) => fetchJson(url, probeAuth.token, options), + probeIsCredentialed: true + } + } + + return { probeHealth: fetchPublicJson, probeIsCredentialed: false } +} + +async function waitForHermes(baseUrl, token, signal?, authMode?) { + const { probeHealth, probeIsCredentialed } = await buildReadinessHealthProbe(baseUrl, authMode, token) + return waitForHermesReady(baseUrl, { token, signal, fetchPublicJson, - fetchJson + fetchJson: probeIsCredentialed ? (url, _token, options) => probeHealth(url, options) : fetchJson, + probeHealth, + probeIsCredentialed }) } @@ -6822,7 +6910,10 @@ async function buildRemoteConnection( // here would reject a freshly-completed native sign-in and loop the UI back // into "not signed in" even though mintGatewayWsTicket would succeed with // the stored bearer. - if (!oauthSessionIsLive(hasNativeSession(baseUrl), await hasLiveOauthSession(baseUrl))) { + if ( + !oauthSessionIsLive(hasNativeSession(baseUrl), await hasLiveOauthSession(baseUrl)) && + oauthGuardMayHardFail(await gatewayAuthProviders(baseUrl)) + ) { const err = new Error( 'Remote Hermes gateway uses OAuth, but you are not signed in. ' + 'Open Settings → Gateway and click "Sign in", or switch back to Local.' @@ -7063,7 +7154,7 @@ async function bootstrapSshConnectionInner(profile, sshConfig, reuseToken, sourc forward: (localPort, remotePort) => ssh.forward(localPort, remotePort), cancelForward: (localPort, remotePort) => ssh.cancelForward(localPort, remotePort), pickLocalPort, - waitForHermes: (baseUrl, token) => waitForHermes(baseUrl, token, lease.signal), + waitForHermes: (baseUrl, token) => waitForHermes(baseUrl, token, lease.signal, 'token'), probeReuseProof: sshProbeReuseProof, adoptServedToken: adoptServedDashboardToken, rememberLog: sshRememberLog, @@ -7536,6 +7627,7 @@ function stopBackendChild(child) { // switch / crash recovery), which still resets boot progress + reloads. function resetHermesConnection({ soft = false } = {}) { backendStartFailure = null + remoteReauthFailure = null remoteLiveness.clear() const hermesProcess = backendConnectionState.invalidate() stopBackendChild(hermesProcess) @@ -7736,7 +7828,7 @@ async function spawnPoolBackend(profile, entry) { const remote = await resolveRemoteBackend(profile) if (remote) { - await waitForHermes(remote.baseUrl, remote.token) + await waitForHermes(remote.baseUrl, remote.token, undefined, remote.authMode) return { ...remote, @@ -7928,6 +8020,13 @@ async function startHermes() { throw backendStartFailure } + // A confirmed remote reauth rejection is terminal until the user signs in. + // Short-circuiting here keeps the boot-failure overlay latched and its + // "Sign in" button clickable, instead of re-driving boot on every retry. + if (remoteReauthFailure) { + throw remoteReauthFailure + } + // E2E: simulate a boot failure without breaking the real backend. The boot // progresses a few steps, then fails with the given error message. if (BOOT_FAKE_ERROR) { @@ -7954,7 +8053,7 @@ async function startHermes() { const connectionPromise = (async () => { const connectRemote = async remote => { await advanceBootProgress('backend.remote', `Connecting to remote Hermes backend at ${remote.baseUrl}`, 24) - await waitForHermes(remote.baseUrl, remote.token) + await waitForHermes(remote.baseUrl, remote.token, undefined, remote.authMode) updateBootProgress({ phase: 'backend.ready', message: 'Remote Hermes backend is ready', @@ -8191,6 +8290,12 @@ async function startHermes() { backendStartFailure = error instanceof Error ? error : new Error(message) } + // A confirmed reauth rejection latches separately: it can't self-heal, and + // leaving it unlatched hides the overlay's "Sign in" button on every retry. + if (shouldLatchRemoteReauthFailure({ attemptedRemote, isReauth: isReauthRequiredError(error) })) { + remoteReauthFailure = error instanceof Error ? error : new Error(message) + } + updateBootProgress( { error: message, @@ -8994,6 +9099,7 @@ ipcMain.handle('hermes:bootstrap:reset', async () => { await teardownPrimaryBackendAndWait() bootstrapFailure = null backendStartFailure = null + remoteReauthFailure = null getFirstRunSetupGate().resetForRetry() resetBootstrapSnapshot() @@ -9016,6 +9122,7 @@ ipcMain.handle('hermes:bootstrap:repair', async () => { bootstrapFailure = null backendStartFailure = null + remoteReauthFailure = null getFirstRunSetupGate().resetForRepair() resetHermesConnection() @@ -9125,6 +9232,9 @@ ipcMain.handle('hermes:connection-config:oauth-login', async (_event, rawUrl) => }) _storeNativeTokens(baseUrl, tokens) + // Confirmed sign-in — release the reauth latch so the next + // startHermes() re-dials instead of replaying the stale rejection. + remoteReauthFailure = null return { ok: true, baseUrl, connected: true } } catch (error) { @@ -9141,7 +9251,16 @@ ipcMain.handle('hermes:connection-config:oauth-login', async (_event, rawUrl) => // Legacy embedded-webview cookie flow. await openOauthLoginWindow(baseUrl) - return { ok: true, baseUrl, connected: await hasOauthSessionCookie(baseUrl) } + const connected = await hasOauthSessionCookie(baseUrl) + + // Only a CONFIRMED sign-in releases the latch. A cancelled/closed login + // window must leave it set, or the overlay's "Sign in" button starts + // flickering again on the next retry. + if (connected) { + remoteReauthFailure = null + } + + return { ok: true, baseUrl, connected } }) ipcMain.handle('hermes:connection-config:oauth-logout', async (_event, rawUrl) => { const baseUrl = rawUrl ? normalizeRemoteBaseUrl(rawUrl) : '' diff --git a/apps/desktop/electron/native-auth-decisions.test.ts b/apps/desktop/electron/native-auth-decisions.test.ts index 09f648c87192..d4cfc068cd66 100644 --- a/apps/desktop/electron/native-auth-decisions.test.ts +++ b/apps/desktop/electron/native-auth-decisions.test.ts @@ -1,7 +1,7 @@ /** - * Regression tests for electron/native-auth-decisions.ts — the three pure - * decision seams behind the RFC 8252 native-app auth flow, each of which was a - * real runtime bug that the mocked flow tests could not catch. + * Regression tests for electron/native-auth-decisions.ts — the pure decision + * seams behind the RFC 8252 native-app auth flow, each of which was a real + * runtime bug that the mocked flow tests could not catch. * * Run via the vitest `electron` project (electron/**\/*.test.ts). */ @@ -10,7 +10,13 @@ import assert from 'node:assert/strict' import { test } from 'vitest' -import { oauthSessionIsLive, resolveJsonBody, resolveOauthRestAuth } from './native-auth-decisions' +import { + oauthGuardMayHardFail, + oauthSessionIsLive, + resolveJsonBody, + resolveOauthRestAuth, + resolveReadinessProbeAuth +} from './native-auth-decisions' // --- 1. body encoding (guards the double-JSON.stringify 422) --- @@ -66,3 +72,61 @@ test('resolveOauthRestAuth falls back to cookie when there is no native token', // Empty string is not a usable bearer — must fall back, not send "Bearer ". assert.deepEqual(resolveOauthRestAuth(''), { kind: 'cookie' }) }) + +// --- 4. readiness-probe auth (guards the credential-free 401 boot loop) --- + +test('resolveReadinessProbeAuth reuses the oauth bearer-vs-cookie choice', () => { + assert.deepEqual(resolveReadinessProbeAuth('oauth', 'native-at'), { kind: 'bearer', token: 'native-at' }) + assert.deepEqual(resolveReadinessProbeAuth('oauth', null), { kind: 'cookie' }) + assert.deepEqual(resolveReadinessProbeAuth('oauth', ''), { kind: 'cookie' }) +}) + +test('resolveReadinessProbeAuth sends the session token for a token gateway', () => { + assert.deepEqual(resolveReadinessProbeAuth('token', null, 'session-token'), { + kind: 'token', + token: 'session-token' + }) + assert.deepEqual(resolveReadinessProbeAuth('token', null, null), { kind: 'token', token: null }) +}) + +test('resolveReadinessProbeAuth stays public for local and unknown modes', () => { + // A loopback backend has no gate; sending credentials it never issued is + // meaningless, and an unknown mode must not invent a credential. + assert.deepEqual(resolveReadinessProbeAuth('local', 'native-at', 'session-token'), { kind: 'public' }) + assert.deepEqual(resolveReadinessProbeAuth(undefined, 'native-at', 'session-token'), { kind: 'public' }) + assert.deepEqual(resolveReadinessProbeAuth('something-new', null, null), { kind: 'public' }) +}) + +// --- 5. oauth guard vs password gateways (guards the false "not signed in") --- + +test('oauthGuardMayHardFail is false only when EVERY provider is password-based', () => { + assert.equal(oauthGuardMayHardFail([{ name: 'basic', supportsPassword: true }]), false) + assert.equal( + oauthGuardMayHardFail([ + { name: 'basic', supportsPassword: true }, + { name: 'ldap', supportsPassword: true } + ]), + false + ) +}) + +test('oauthGuardMayHardFail keeps the strict guard for oauth and mixed deployments', () => { + assert.equal(oauthGuardMayHardFail([{ name: 'nous', supportsPassword: false }]), true) + assert.equal( + oauthGuardMayHardFail([ + { name: 'nous', supportsPassword: false }, + { name: 'basic', supportsPassword: true } + ]), + true + ) +}) + +test('oauthGuardMayHardFail keeps the strict guard when the list is unusable', () => { + // Backends predating /api/auth/providers, or an unreachable probe, must not + // silently weaken the guard. + assert.equal(oauthGuardMayHardFail([]), true) + assert.equal(oauthGuardMayHardFail(null), true) + assert.equal(oauthGuardMayHardFail(undefined), true) + assert.equal(oauthGuardMayHardFail('nonsense' as any), true) + assert.equal(oauthGuardMayHardFail([{ supportsPassword: true }]), true) +}) diff --git a/apps/desktop/electron/native-auth-decisions.ts b/apps/desktop/electron/native-auth-decisions.ts index d76746f669a3..c0978c3ecddc 100644 --- a/apps/desktop/electron/native-auth-decisions.ts +++ b/apps/desktop/electron/native-auth-decisions.ts @@ -21,7 +21,17 @@ * native bearer when present, else the cookie partition. Cookie-only * routing returns 401 no_cookie for a cookieless native session. * - * All three are trivial once named; the value is the test that pins the + * 4. resolveReadinessProbeAuth — the boot readiness probe must authenticate + * the same way the rest of the connection does. A credential-free probe + * against a gated gateway 401s forever; worse, it cannot tell a missing + * route from a rejected session (see backend-health.ts). + * + * 5. oauthGuardMayHardFail — `auth_required: true` means "this gateway is + * gated", NOT "this gateway speaks OAuth". A password-provider gateway + * can satisfy neither the native-bearer nor the OAuth-partition-cookie + * check by design, so the pre-flight guard must not hard-fail it. + * + * All five are trivial once named; the value is the test that pins the * contract so the god-file call sites can't drift back to the buggy shape. */ @@ -60,3 +70,75 @@ export function resolveOauthRestAuth(nativeAccessToken: string | null | undefine return { kind: 'cookie' } } + +export type ReadinessProbeAuth = OauthRestAuth | { kind: 'token'; token: string | null } | { kind: 'public' } + +/** + * Decide how the boot readiness probe authenticates. + * + * The probe must present the SAME credentials the rest of the connection + * will use. A credential-free probe against a gated gateway 401s until the + * boot deadline even though the session is perfectly valid — and because the + * dashboard auth gate runs ahead of the SPA catch-all, an unknown `/api/*` + * path answers 401 rather than 404, so the probe also cannot detect a backend + * that predates `/api/health`. Sending credentials is what lets a missing + * route surface as a real 404 (see `isMissingHealthEndpointError`). + * + * `oauth` reuses `resolveOauthRestAuth` so the probe and every other oauth + * REST call make the identical bearer-vs-cookie choice. `token` presents the + * connection's session token. `local` (and anything unrecognized) stays + * public: a loopback backend has no gate, and sending credentials it never + * issued would be meaningless. + */ +export function resolveReadinessProbeAuth( + authMode: string | null | undefined, + nativeAccessToken?: string | null, + connectionToken?: string | null +): ReadinessProbeAuth { + if (authMode === 'oauth') { + return resolveOauthRestAuth(nativeAccessToken) + } + + if (authMode === 'token') { + return { kind: 'token', token: connectionToken ?? null } + } + + return { kind: 'public' } +} + +export interface AdvertisedAuthProvider { + name?: string + supportsPassword?: boolean +} + +/** + * Whether the oauth pre-flight guard may hard-fail a connection for "not + * signed in". + * + * `authModeFromStatus` maps the gateway's `auth_required: true` onto + * `'oauth'`, but that flag only means the dashboard is GATED — it says + * nothing about how you authenticate. A gateway whose providers are all + * username/password cannot satisfy the guard's checks by construction: + * `start_login` raises NotImplementedError, `/auth/native/authorize` rejects + * password providers, and its cookies are set by a plain password-login POST + * rather than the `/auth/callback` redirect the OAuth partition is primed + * for. Hard-failing there rejects a live session one line before the + * ws-ticket mint that would have succeeded against that very partition. + * + * Returns false only when EVERY advertised provider is password-based. An + * unknown or empty list keeps the strict guard, so backends that predate + * `/api/auth/providers` are unaffected. + */ +export function oauthGuardMayHardFail(providers: AdvertisedAuthProvider[] | null | undefined): boolean { + if (!Array.isArray(providers) || providers.length === 0) { + return true + } + + const named = providers.filter(provider => provider && typeof provider === 'object' && provider.name) + + if (named.length === 0) { + return true + } + + return !named.every(provider => provider.supportsPassword) +} diff --git a/contributors/emails/0301chris@gmail.com b/contributors/emails/0301chris@gmail.com new file mode 100644 index 000000000000..e606f25d4666 --- /dev/null +++ b/contributors/emails/0301chris@gmail.com @@ -0,0 +1,2 @@ +0301chris +# PR #71555 salvage (desktop: gated /api/health readiness probe) diff --git a/contributors/emails/leo@gtmcore.ai b/contributors/emails/leo@gtmcore.ai new file mode 100644 index 000000000000..b61779c6ff74 --- /dev/null +++ b/contributors/emails/leo@gtmcore.ai @@ -0,0 +1,2 @@ +leoprodz +# PR #71602 salvage (desktop: gated /api/health readiness probe) diff --git a/contributors/emails/liruixinch@outlook.com b/contributors/emails/liruixinch@outlook.com new file mode 100644 index 000000000000..a1cf96ff89db --- /dev/null +++ b/contributors/emails/liruixinch@outlook.com @@ -0,0 +1,2 @@ +HexLab98 +# PR #71205 salvage (desktop: gated /api/health readiness probe) diff --git a/contributors/emails/stephenlopez2030@gmail.com b/contributors/emails/stephenlopez2030@gmail.com new file mode 100644 index 000000000000..e0710ccbcad4 --- /dev/null +++ b/contributors/emails/stephenlopez2030@gmail.com @@ -0,0 +1,2 @@ +Slopez2023 +# PR #71668 salvage (desktop: gated /api/health readiness probe)