fix(desktop): recover Cloud cookie before gateway ticket mint (#110308)

This commit is contained in:
Gille
2026-09-25 14:42:44 -06:00
committed by GitHub
parent 4c286ae7a0
commit 8fe53200c7
11 changed files with 328 additions and 24 deletions

View File

@@ -0,0 +1,107 @@
import { describe, expect, it, vi } from 'vitest'
import { createCloudSessionRecovery } from './cloud-session-recovery'
const cloud = 'https://test-agent.agents.nousresearch.com'
const rejection = (statusCode = 401) => Object.assign(new Error('rejected'), { statusCode })
describe('Cloud cookie session recovery', () => {
it('recovers an expired cookie session before minting a new ticket', async () => {
const restore = vi.fn(async () => true)
const run = createCloudSessionRecovery({ hasNativeSession: () => false, restoreCookieSession: restore })
const mint = vi.fn().mockRejectedValueOnce(rejection()).mockResolvedValueOnce('fresh-ticket')
expect(await run(cloud, mint)).toBe('fresh-ticket')
expect(restore).toHaveBeenCalledExactlyOnceWith(cloud)
expect(mint).toHaveBeenCalledTimes(2)
})
it('does not recover non-Cloud, non-401 or native-session failures', async () => {
const restore = vi.fn(async () => true)
const run = createCloudSessionRecovery({
hasNativeSession: url => url.endsWith('native.agents.nousresearch.com'),
restoreCookieSession: restore
})
for (const [url, status] of [
['https://example.com', 401],
[cloud, 403],
[cloud, 500],
['https://native.agents.nousresearch.com', 401]
] as const) {
const error = rejection(status)
await expect(
run(url, async () => {
throw error
})
).rejects.toBe(error)
}
expect(restore).not.toHaveBeenCalled()
})
it('coalesces recovery without reusing single-use tickets', async () => {
let release!: (restored: boolean) => void
const restore = vi.fn(
() =>
new Promise<boolean>(resolve => {
release = resolve
})
)
const run = createCloudSessionRecovery({ hasNativeSession: () => false, restoreCookieSession: restore })
const firstMint = vi.fn().mockRejectedValueOnce(rejection()).mockResolvedValueOnce('ticket-1')
const secondMint = vi.fn().mockRejectedValueOnce(rejection()).mockResolvedValueOnce('ticket-2')
const first = run(cloud, firstMint)
const second = run(cloud, secondMint)
await vi.waitFor(() => expect(restore).toHaveBeenCalledTimes(1))
release(true)
expect(await Promise.all([first, second])).toEqual(['ticket-1', 'ticket-2'])
})
it('does not retry under a native identity selected during background recovery', async () => {
let native = false
const restore = vi.fn(async () => {
native = true
return true
})
const run = createCloudSessionRecovery({ hasNativeSession: () => native, restoreCookieSession: restore })
const error = rejection()
const mint = vi.fn(async () => {
throw error
})
await expect(run(cloud, mint)).rejects.toBe(error)
expect(mint).toHaveBeenCalledTimes(1)
})
it('backs off failed recovery and bounds a successful recovery to one retry', async () => {
let clock = 0
const restore = vi.fn().mockResolvedValueOnce(false).mockResolvedValue(true)
const run = createCloudSessionRecovery({
hasNativeSession: () => false,
restoreCookieSession: restore,
now: () => clock
})
const error = rejection()
const mint = vi.fn(async () => {
throw error
})
await expect(run(cloud, mint)).rejects.toBe(error)
await expect(run(cloud, mint)).rejects.toBe(error)
expect(restore).toHaveBeenCalledTimes(1)
clock = 60_001
await expect(run(cloud, mint)).rejects.toBe(error)
expect(restore).toHaveBeenCalledTimes(2)
expect(mint).toHaveBeenCalledTimes(4)
})
})

View File

@@ -0,0 +1,76 @@
import { isNousCloudAgentUrl } from './backend-health'
interface CloudRecoveryDeps {
hasNativeSession: (baseUrl: string) => boolean
restoreCookieSession: (baseUrl: string) => Promise<boolean>
onRecovered?: (baseUrl: string) => void
now?: () => number
}
/** Coalesce the cookie refresh, never the single-use ticket minted afterward. */
export function createCloudSessionRecovery(deps: CloudRecoveryDeps) {
const inflight = new Map<string, Promise<boolean>>()
const failedAt = new Map<string, number>()
const now = deps.now ?? Date.now
return async <T>(baseUrl: string, mint: () => Promise<T>): Promise<T> => {
const hadNativeSession = deps.hasNativeSession(baseUrl)
try {
return await mint()
} catch (error) {
if (
!isNousCloudAgentUrl(baseUrl) ||
hadNativeSession ||
deps.hasNativeSession(baseUrl) ||
(error as { statusCode?: number })?.statusCode !== 401
) {
throw error
}
let recovery = inflight.get(baseUrl)
if (!recovery) {
const failed = failedAt.get(baseUrl)
if (failed !== undefined && now() - failed < 60_000) {
throw error
}
recovery = Promise.resolve()
.then(() => deps.restoreCookieSession(baseUrl))
.catch(() => false)
.then(restored => {
if (!restored) {
failedAt.set(baseUrl, now())
}
return restored
})
.finally(() => inflight.delete(baseUrl))
inflight.set(baseUrl, recovery)
}
if (!(await recovery)) {
throw error
}
// Native login may have changed while the background cookie flow ran.
// Never turn that transition into a cross-identity fallback.
if (deps.hasNativeSession(baseUrl)) {
throw error
}
try {
const ticket = await mint()
failedAt.delete(baseUrl)
deps.onRecovered?.(baseUrl)
return ticket
} catch (retryError) {
failedAt.set(baseUrl, now())
throw retryError
}
}
}
}

View File

@@ -5,6 +5,7 @@ import { beforeEach, test } from 'vitest'
import {
connectionInstallIds,
evictConnectionCaches,
rosterSourceErrors,
sshInventoryAttemptedAt,
sshRosterCache
} from './connection-caches'
@@ -14,12 +15,14 @@ beforeEach(() => {
sshRosterCache.clear()
sshInventoryAttemptedAt.clear()
connectionInstallIds.clear()
rosterSourceErrors.clear()
})
function seed(id: string) {
sshRosterCache.set(id, ['default', 'dixie'])
sshInventoryAttemptedAt.set(id, Date.now())
connectionInstallIds.set(id, { id: 'aaa', ts: Date.now() })
rosterSourceErrors.set(id, 'previous failure')
}
test('evicting a connection id forgets every cache keyed by it', () => {
@@ -33,10 +36,12 @@ test('evicting a connection id forgets every cache keyed by it', () => {
assert.equal(sshRosterCache.has('mac-mini'), false)
assert.equal(sshInventoryAttemptedAt.has('mac-mini'), false)
assert.equal(connectionInstallIds.has('mac-mini'), false)
assert.equal(rosterSourceErrors.has('mac-mini'), false)
// Its neighbours are untouched.
assert.deepEqual(sshRosterCache.get('spark'), ['default', 'dixie'])
assert.equal(connectionInstallIds.get('spark')?.id, 'aaa')
assert.equal(rosterSourceErrors.get('spark'), 'previous failure')
})
test('an evicted id enumerates from the live target again instead of serving the old one', () => {
@@ -66,4 +71,5 @@ test('evicting an unknown or empty id is a no-op', () => {
assert.equal(sshRosterCache.size, 1)
assert.equal(sshInventoryAttemptedAt.size, 1)
assert.equal(connectionInstallIds.size, 1)
assert.equal(rosterSourceErrors.size, 1)
})

View File

@@ -24,7 +24,15 @@ export const sshInventoryAttemptedAt = new Map<string, number>()
*/
export const connectionInstallIds = new Map<string, { id?: string; ts: number }>()
const CONNECTION_SCOPED_CACHES: Map<string, unknown>[] = [sshRosterCache, sshInventoryAttemptedAt, connectionInstallIds]
/** Last logged roster failure per connection, so repeat polls do not spam the log. */
export const rosterSourceErrors = new Map<string, string>()
const CONNECTION_SCOPED_CACHES: Map<string, unknown>[] = [
sshRosterCache,
sshInventoryAttemptedAt,
connectionInstallIds,
rosterSourceErrors
]
/**
* Forget everything cached about a connection id. Call whenever that id stops naming the machine

View File

@@ -114,12 +114,14 @@ import { provisionCliLinks } from './cli-provision'
import { closeStopFailureMessage, finishWindowsCloseStop, type RuntimeLock } from './close-stop-kill'
import { shouldAttemptCloudBootCascade } from './cloud-boot-cascade'
import { discoverWithTeamFallback } from './cloud-discovery'
import { createCloudSessionRecovery } from './cloud-session-recovery'
import { installCommandScreenshot } from './command-screenshot'
import { writeComposerPaste } from './composer-paste'
import { applyConnectionChange, teardownSshState } from './connection-apply'
import {
connectionInstallIds,
evictConnectionCaches,
rosterSourceErrors,
sshInventoryAttemptedAt,
sshRosterCache
} from './connection-caches'
@@ -453,6 +455,7 @@ import { planLaunchSwitches, readDesktopLaunchConfig } from './renderer-heap-fla
import { loadRendererLoadErrorPage } from './renderer-load-error-page'
import { attachRendererConsoleCapture, formatRendererBoundaryReport } from './renderer-log'
import { fetchRosterSourceData } from './roster-source-fetch'
import { rosterSourceStatus } from './roster-source-status'
import {
classifyStoredSecret,
readSecretStoragePolicy,
@@ -7441,7 +7444,7 @@ async function clearOauthSession(baseUrl) {
// ``/auth/login`` → portal ``/oauth/authorize`` (auto-approves org members)
// → ``/auth/callback``, which sets the gateway cookie with NO interactive
// prompt. This is the per-agent cloud cascade (decisions.md Q5).
function openOauthLoginWindow(baseUrl, { silent = false } = {}) {
function openOauthLoginWindow(baseUrl, { silent = false, background = false } = {}) {
return new Promise((resolve, reject) => {
if (!app.isReady()) {
reject(new Error('Desktop is not ready to start an OAuth login.'))
@@ -7461,6 +7464,7 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) {
let win = null
let pollTimer = null
let revealTimer = null
let deadlineTimer = null
const finish = err => {
if (settled) {
@@ -7477,6 +7481,10 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) {
clearTimeout(revealTimer)
}
if (deadlineTimer) {
clearTimeout(deadlineTimer)
}
try {
if (win && !win.isDestroyed()) {
win.destroy()
@@ -7513,7 +7521,7 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) {
// only reveal it as a fallback if the cascade DOESN'T complete quickly
// (e.g. the portal session lapsed and the gate fell through to the
// interactive chooser) — see the reveal timer below.
show: !silent,
show: !silent && !background,
webPreferences: {
contextIsolation: true,
nodeIntegration: false,
@@ -7545,7 +7553,7 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) {
// loop-guard tripped, etc.) and the window is now showing an interactive
// page. Reveal it so the user can complete sign-in manually rather than
// staring at nothing. Cleared on finish().
if (silent && win) {
if (silent && win && !background) {
revealTimer = setTimeout(() => {
try {
if (!settled && win && !win.isDestroyed() && !win.isVisible()) {
@@ -7557,6 +7565,10 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) {
}, 2500)
}
if (background) {
deadlineTimer = setTimeout(() => finish(new Error('Cloud session recovery requires sign-in.')), 12_000)
}
win.on('closed', () => {
if (!settled) {
finish(new Error('Login window closed before authentication completed.'))
@@ -7576,6 +7588,14 @@ function openOauthLoginWindow(baseUrl, { silent = false } = {}) {
`OAuth login: attaching ${Object.keys(loginHeaders).length} extra gateway header(s) to ${new URL(normalizedBase).host}`
)
win.loadURL(loginUrl, oauthLoginLoadUrlOptions(loginHeaders)).catch(error => {
// Callback navigation can abort the original load after setting cookies.
// Keep the bounded hidden recovery alive long enough to observe them.
if (background && (Number(error?.code) === -3 || /\bERR_ABORTED\b/.test(String(error?.message)))) {
void checkCookie()
return
}
finish(error instanceof Error ? error : new Error(String(error)))
})
})
@@ -7876,20 +7896,40 @@ async function readGatewayFileDataUrl(connection: GatewayFileConnection, request
return dataUrl
}
// Mint a single-use WS ticket for a gated gateway. Native bearer first (one
// forced rotation on a confirmed 401, #95701), OAuth cookie partition second.
// Transient transport blips (brief host unreachable, 5xx, timeouts) are retried
// a few times before failing — those 1-3s flaps were promoting into the
// full-screen "couldn't start" lockout on reconnect. Ticket POSTs are
// replay-safe; arbitrary REST mutations never use this retry loop.
async function mintGatewayWsTicket(baseUrl: string, headers: Record<string, string> = {}): Promise<string> {
return withTransientRetries(
(): Promise<string> =>
mintOauthGatewayWsTicket(baseUrl, { ensureNativeAccessToken, fetchJson, fetchJsonViaOauthSession }, headers),
{
isRetryable: (error: Error): boolean =>
!(error instanceof NativeAuthChangedError) && !isGatewayAuthRejection(error)
// Recover a Cloud cookie only after a confirmed cookie-auth ticket 401.
// Each caller still mints its own single-use ticket after the shared recovery.
const recoverCloudCookieSession = createCloudSessionRecovery({
hasNativeSession,
onRecovered: baseUrl => rememberLog(`[cloud] saved gateway session recovered for ${hostLabelFromBaseUrl(baseUrl)}`),
restoreCookieSession: async baseUrl => {
// The saved portal identity is the authority for cookie agent sessions.
// Roster polling must never reveal an interactive sign-in window.
if (!(await hasLivePortalSession())) {
return false
}
if (!(await hasPortalAccessToken()) && !(await renewPortalAccessSilently())) {
return false
}
await openOauthLoginWindow(baseUrl, { silent: true, background: true })
return true
}
})
// Native bearer first (including one forced 401 rotation), OAuth cookie second.
// Transient ticket POST failures keep their bounded retry before Cloud recovery.
async function mintGatewayWsTicket(baseUrl: string, headers: Record<string, string> = {}): Promise<string> {
return recoverCloudCookieSession(baseUrl, () =>
withTransientRetries(
(): Promise<string> =>
mintOauthGatewayWsTicket(baseUrl, { ensureNativeAccessToken, fetchJson, fetchJsonViaOauthSession }, headers),
{
isRetryable: (error: Error): boolean =>
!(error instanceof NativeAuthChangedError) && !isGatewayAuthRejection(error)
}
)
)
}
@@ -15706,7 +15746,7 @@ ipcMain.handle('hermes:connections:test', async (_event, id) => {
// would spawn tunnels the user never asked for); once dialed, their pooled
// descriptor serves the enumeration like any remote. Last-known SSH profile
// lists are reused so switching the window back to local does not empty Bot Mode.
// These three live in ./connection-caches, which states (and tests) the invariant they share:
// Connection caches live in ./connection-caches, which states (and tests) the invariant they share:
// each is keyed by connection id and is only valid while that id names the same machine, so
// removing a connection or re-pointing it must evict them (`evictConnectionCaches`).
const SSH_INVENTORY_RETRY_MS = 60_000
@@ -15837,9 +15877,12 @@ async function enumerateRegistryAgentSources(registry = readDesktopConnectionsRe
return Promise.all(
registry.connections.map(async connection => {
let sourceFailureDetail = ''
let raw: {
connection: typeof connection
error?: string
needsSignIn?: boolean
installId?: string
profiles: null | string[]
profileMetadata?: Record<string, RosterProfileMetadata>
@@ -15953,7 +15996,26 @@ async function enumerateRegistryAgentSources(registry = readDesktopConnectionsRe
}
}
} catch (error: any) {
raw = { connection, profiles: null, error: String(error?.message || error) }
sourceFailureDetail = [error?.statusCode, error?.cause?.message].filter(Boolean).join(' | ')
raw = {
connection,
profiles: null,
error: redactSecrets(String(error?.message || error)),
needsSignIn: isReauthRequiredError(error)
}
}
if (raw.error && raw.error !== 'connect-on-demand') {
const diagnostic = redactSecrets([raw.error, sourceFailureDetail].filter(Boolean).join(' | '))
.replace(/[\r\n]+/g, ' ')
.slice(0, 800)
if (rosterSourceErrors.get(connection.id) !== diagnostic) {
rememberLog(`[fleet-roster] ${connection.id}: ${diagnostic}`)
rosterSourceErrors.set(connection.id, diagnostic)
}
} else if (raw.profiles && rosterSourceErrors.delete(connection.id)) {
rememberLog(`[fleet-roster] ${connection.id}: connection recovered`)
}
if (raw.profiles && raw.profiles.length > 0) {
@@ -15965,6 +16027,7 @@ async function enumerateRegistryAgentSources(registry = readDesktopConnectionsRe
return {
connection,
...remembered,
...(raw.needsSignIn ? { needsSignIn: true } : {}),
...(raw.installId ? { installId: raw.installId } : {}),
...(raw.profileMetadata ? { profileMetadata: raw.profileMetadata } : {})
}
@@ -15984,13 +16047,12 @@ ipcMain.handle('hermes:agents:roster', async () => {
// instead of appending duplicates (remote-only desktops doubled every
// bot otherwise; see #88344).
primaryConnectionId: registry.primary,
sources: enumerations.map(({ connection, error, installId, profiles }) => ({
sources: enumerations.map(({ connection, error, installId, profiles, needsSignIn }) => ({
connectionId: connection.id,
label: connection.label,
kind: connection.kind,
reachable: profiles !== null,
...(installId ? { installId } : {}),
...(error ? { error } : {})
...rosterSourceStatus({ profiles, error, needsSignIn }),
...(installId ? { installId } : {})
}))
}
})

View File

@@ -0,0 +1,13 @@
import { expect, it } from 'vitest'
import { rosterSourceStatus } from './roster-source-status'
it('does not report stale cached profiles as a reachable or signed-in gateway', () => {
expect(rosterSourceStatus({ profiles: ['default'], error: 'OAuth expired', needsSignIn: true })).toEqual({
reachable: false,
error: 'OAuth expired',
needsSignIn: true
})
expect(rosterSourceStatus({ profiles: ['default'], error: 'timed out' }).reachable).toBe(false)
expect(rosterSourceStatus({ profiles: ['default'] }).reachable).toBe(true)
})

View File

@@ -0,0 +1,7 @@
export function rosterSourceStatus(source: { profiles: string[] | null; error?: string; needsSignIn?: boolean }) {
return {
reachable: source.profiles !== null && (!source.error || source.error === 'connect-on-demand'),
...(source.error ? { error: source.error } : {}),
...(source.needsSignIn ? { needsSignIn: true } : {})
}
}

View File

@@ -95,6 +95,22 @@ describe('buildRestGroups', () => {
expect(vps?.named).toEqual([])
})
it('keeps an expired Cloud source visible but not reachable even with cached profiles', () => {
const expired: DesktopAgentRoster = {
...roster,
sources: roster.sources.map(source =>
source.connectionId === 'pandora'
? { ...source, reachable: false, error: 'OAuth expired', needsSignIn: true }
: source
)
}
const groups = buildRestGroups({ activeConnectionId: 'local', connections, roster: expired })
const cloud = groups.find(group => group.connectionId === 'pandora')
expect(cloud).toMatchObject({ reachable: false, error: 'OAuth expired', needsSignIn: true })
expect(cloud?.named.map(agent => agent.profile)).toEqual(['omer', 'scout'])
})
it('shows every gateway with just its default before the roster has loaded', () => {
const groups = buildRestGroups({ activeConnectionId: 'pandora', connections, roster: null })

View File

@@ -22,6 +22,8 @@ export interface FleetGroup {
kind: DesktopConnectionKind
label: string
reachable: boolean
error?: string
needsSignIn?: boolean
/** The gateway's default profile — every Hermes home has one, so a group
* always carries it even before the roster has been enumerated. */
defaultAgent: FleetAgent
@@ -94,6 +96,8 @@ export function buildRestGroups({
kind: connection.kind,
label: connection.label,
reachable: source?.reachable ?? true,
...(source?.error && source.error !== 'connect-on-demand' ? { error: source.error } : {}),
...(source?.needsSignIn ? { needsSignIn: true } : {}),
defaultAgent: toAgent(DEFAULT_PROFILE, defaultRow?.handle),
named
})

View File

@@ -1180,7 +1180,11 @@ function FleetRestGroup({
}) {
const { t } = useI18n()
const p = t.profiles
const dividerLabel = group.reachable ? p.fleet.gateway(group.label) : p.fleet.gatewayUnreachable(group.label)
const dividerLabel = group.reachable
? p.fleet.gateway(group.label)
: `${group.needsSignIn ? `${p.fleet.gateway(group.label)} · ${t.settings.toolsets.needsSignIn}` : p.fleet.gatewayUnreachable(group.label)}${group.error ? `\n${group.error}` : ''}`
const defaultKey = fleetRouteKey(group.connectionId, group.defaultAgent.profile)
// At rest, This device is a backend switch, not Home. The house glyph stays
// on the active gateway's default profile.

View File

@@ -1155,6 +1155,7 @@ export interface DesktopAgentRoster {
kind: DesktopConnectionKind
reachable: boolean
error?: string
needsSignIn?: boolean
// Stable backend identity (/api/status install_id) when known.
installId?: string
}[]