Skip to content

Commit b2cefe2

Browse files
committed
fix(desktop): contain sign-in handoff failures when no window is available
`handleCallback` awaited `deps.ensureMainWindow()` as its first statement with no try/catch, and index.ts dispatches it fire-and-forget as `void authFlow.handleCallback(callback)`. The wired `ensureMainWindow` throws `Main window unavailable`, and main registers no `unhandledRejection` handler, so a user who closed the window while signing in through their browser turned the loopback callback into an unhandled rejection. `beginLoginHandoff` had the same shape, so both are fixed rather than one. Both entry points now resolve the window through a helper that records the failure via the existing `handoff_redeem_fail` event and returns null, and the two `void` dispatch sites carry a `.catch()` backstop for anything the flows do not record themselves. Not attacker-triggerable: `onLogin` fires only after `matchesPending()` validates the state that only the user's own browser holds.
1 parent b0fe9d4 commit b2cefe2

3 files changed

Lines changed: 90 additions & 4 deletions

File tree

apps/desktop/src/main/handoff.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
44
vi.mock('electron', () => import('@/test/electron-mock'))
55

66
import {
7+
type AuthFlowDeps,
78
buildRedeemScript,
89
type ConnectHandoffCallback,
10+
createAuthFlow,
911
createHandoffManager,
1012
type HandoffCallback,
1113
type HandoffCallbacks,
@@ -270,3 +272,51 @@ describe('connect handoff account pinning', () => {
270272
manager.clear()
271273
})
272274
})
275+
276+
describe('createAuthFlow window failures', () => {
277+
function makeAuthDeps(ensureMainWindow: () => Promise<never>) {
278+
const events = makeEvents()
279+
return {
280+
deps: {
281+
handoff: {
282+
begin: vi.fn(async () => true),
283+
consume: vi.fn(() => true),
284+
} as unknown as AuthFlowDeps['handoff'],
285+
origin: () => 'https://sim.ai',
286+
events,
287+
ensureMainWindow,
288+
} satisfies AuthFlowDeps,
289+
events,
290+
}
291+
}
292+
293+
it('records rather than rejects when no window can be opened for the callback', async () => {
294+
const { deps, events } = makeAuthDeps(async () => {
295+
throw new Error('Main window unavailable')
296+
})
297+
const flow = createAuthFlow(deps)
298+
299+
await expect(
300+
flow.handleCallback({ state: VALID_STATE, token: VALID_TOKEN } as HandoffCallback)
301+
).resolves.toBeUndefined()
302+
expect(events.record).toHaveBeenCalledWith('handoff_redeem_fail', {
303+
reason: 'callback_window',
304+
error: 'Main window unavailable',
305+
})
306+
expect(deps.handoff.consume).not.toHaveBeenCalled()
307+
})
308+
309+
it('records rather than rejects when no window can be opened to report a failed begin', async () => {
310+
const { deps, events } = makeAuthDeps(async () => {
311+
throw new Error('Main window unavailable')
312+
})
313+
deps.handoff.begin = vi.fn(async () => false)
314+
const flow = createAuthFlow(deps)
315+
316+
await expect(flow.beginLoginHandoff()).resolves.toBeUndefined()
317+
expect(events.record).toHaveBeenCalledWith('handoff_redeem_fail', {
318+
reason: 'begin_window',
319+
error: 'Main window unavailable',
320+
})
321+
})
322+
})

apps/desktop/src/main/handoff.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Server } from 'node:http'
22
import { createServer } from 'node:http'
33
import { createLogger } from '@sim/logger'
44
import { safeCompare } from '@sim/security/compare'
5+
import { getErrorMessage } from '@sim/utils/errors'
56
import { generateShortId } from '@sim/utils/id'
67
import type { BrowserWindow } from 'electron'
78
import { app, dialog } from 'electron'
@@ -364,6 +365,28 @@ export interface AuthFlow {
364365
* back on /login.
365366
*/
366367
export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
368+
/**
369+
* The main window, or null when one cannot be obtained.
370+
*
371+
* Both entry points below are dispatched fire-and-forget from index.ts, and
372+
* the wired `ensureMainWindow` throws when no window can be created or
373+
* restored — which is reachable if the user closed the window while signing
374+
* in through their browser. With no global `unhandledRejection` handler in
375+
* main, letting that escape turned it into an unhandled rejection raised from
376+
* the loopback callback. Recorded rather than swallowed: a sign-in that
377+
* cannot present itself is exactly what the event log is for.
378+
*/
379+
const resolveWindow = async (reason: string): Promise<BrowserWindow | null> => {
380+
try {
381+
return await deps.ensureMainWindow()
382+
} catch (error) {
383+
const message = getErrorMessage(error, 'Main window unavailable')
384+
deps.events.record('handoff_redeem_fail', { reason, error: message })
385+
logger.error('No window available for the sign-in handoff', { reason, error: message })
386+
return null
387+
}
388+
}
389+
367390
const failInWindow = async (win: BrowserWindow, reason: string, status?: number) => {
368391
deps.events.record(
369392
'handoff_redeem_fail',
@@ -383,7 +406,8 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
383406
async beginLoginHandoff() {
384407
const opened = await deps.handoff.begin()
385408
if (!opened) {
386-
const win = await deps.ensureMainWindow()
409+
const win = await resolveWindow('begin_window')
410+
if (!win) return
387411
void dialog.showMessageBox(win, {
388412
type: 'error',
389413
message: 'Couldn’t start sign-in',
@@ -392,7 +416,8 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
392416
}
393417
},
394418
async handleCallback(callback: HandoffCallback) {
395-
const win = await deps.ensureMainWindow()
419+
const win = await resolveWindow('callback_window')
420+
if (!win) return
396421
if (!deps.handoff.consume(callback.state, 'login')) {
397422
await failInWindow(win, 'state')
398423
return

apps/desktop/src/main/index.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { join } from 'node:path'
22
import { createLogger } from '@sim/logger'
3+
import { getErrorMessage } from '@sim/utils/errors'
34
import type { Session, WebContents } from 'electron'
45
import { app, BrowserWindow, crashReporter, net, session } from 'electron'
56
import { newChatRoute, settingsRoute } from '@/main/app-routes'
@@ -55,6 +56,16 @@ import { attachWindowOpenPolicy, isPopupContents } from '@/main/windows'
5556

5657
const logger = createLogger('DesktopMain')
5758

59+
/**
60+
* Backstop for the sign-in flows, which are dispatched fire-and-forget from a
61+
* loopback callback and a navigation guard. The flows record their own expected
62+
* failures; this catches anything they do not, so a rejection cannot surface as
63+
* an unhandled one — main registers no `unhandledRejection` handler.
64+
*/
65+
function reportHandoffFailure(error: unknown): void {
66+
logger.error('Sign-in handoff failed', { error: getErrorMessage(error) })
67+
}
68+
5869
const OFFLINE_PAGE = 'static/offline.html'
5970
const DOCK_ICON_FOR_CHANNEL = {
6071
prod: 'dock-icon.png',
@@ -138,7 +149,7 @@ function main(): void {
138149
currentUserId: () => readSessionUserId(ensureAppSession(), appOrigin()),
139150
},
140151
{
141-
onLogin: (callback) => void authFlow.handleCallback(callback),
152+
onLogin: (callback) => void authFlow.handleCallback(callback).catch(reportHandoffFailure),
142153
onConnect: (callback) => connectFlow.handleCallback(callback),
143154
}
144155
)
@@ -173,7 +184,7 @@ function main(): void {
173184
isPackaged: app.isPackaged,
174185
allowHttpLocalhost,
175186
isPopupContents,
176-
onLoginHandoff: () => void authFlow.beginLoginHandoff(),
187+
onLoginHandoff: () => void authFlow.beginLoginHandoff().catch(reportHandoffFailure),
177188
onConnectIntercept: (contents) => void handleConnectIntercept(contents, allowHttpLocalhost()),
178189
})
179190

0 commit comments

Comments
 (0)