Skip to content

Commit 8ddba9a

Browse files
fix(desktop): render the offline page and server picker in packaged builds (#7464)
* fix(desktop): render the offline page and server picker in packaged builds * fix(desktop): keep retrying the origin past a broken offline page and drop the font copy
1 parent 99225b5 commit 8ddba9a

16 files changed

Lines changed: 689 additions & 58 deletions

apps/desktop/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ src/main/ # main process (bundled to dist/main.cjs)
1919
handoff.ts # 127.0.0.1 loopback login handoff + token redeem
2020
session-lifecycle.ts # sign-out teardown, 401 watcher, connect intercept
2121
load-health.ts # offline/error page, auto-retry, watchdog
22+
local-pages.ts # sim-shell: scheme for the bundled pages (file: cannot read app.asar with its privileges fused off)
2223
local-filesystem.ts # session-scoped read-only directory grants + localfs:// broker
2324
local-filesystem-grant-store.ts # those grants, encrypted at rest
2425
desktop-settings.ts # renderer-facing settings surface
@@ -38,7 +39,7 @@ src/preload/ # isolated renderer bridges
3839
index.ts # hosted-app contextBridge IPC bridge (dist/preload.cjs)
3940
browser/ # minimal agent-browser credential helper (dist/browser-preload.cjs)
4041
native/ # Node-API/AppKit bridge for native macOS Help docs search
41-
static/ # bundled local pages (offline.html)
42+
static/ # bundled local pages (offline.html, server.html), served over sim-shell:
4243
e2e/ # Playwright _electron smoke suite
4344
```
4445

@@ -130,7 +131,7 @@ Yes — the architecture has a single, clean seam for native features, and nothi
130131

131132
1. **One bridge.** The preload (`src/preload/index.ts`) exposes `window.simDesktop` via `contextBridge` on the main window. This is the *only* channel between web content and native capability. It exposes narrow, typed methods — never raw `ipcRenderer` (Electron security checklist item 20).
132133
2. **Feature-detect, never assume.** The same web app is served to browsers and to the desktop from one origin, so a desktop feature is progressive enhancement: `if (window.simDesktop) { … }`. In a browser `window.simDesktop` is `undefined` and the feature is simply absent. (`isHosted` already tags these sessions for analytics.)
133-
3. **Gate in main.** Every channel is validated in `src/main/ipc.ts` by sender frame — app-origin for capability calls, bundled `file:` pages for shell-control calls (checklist item 17). A new native feature adds one gated channel there.
134+
3. **Gate in main.** Every channel is validated in `src/main/ipc.ts` by sender frame — app-origin for capability calls, the bundled `sim-shell://pages/…` documents for shell-control calls (checklist item 17). A new native feature adds one gated channel there.
134135
4. **Single-source the contract.** `apps/sim` cannot import from `apps/desktop` (monorepo rule: `apps/* → packages/*` only). The bridge interface lives in the shared types-only `packages/desktop-bridge` package, which both the preload and web app consume.
135136

136137
Concrete example — a "Reveal in Finder" button:

apps/desktop/docs/electron-upgrade-checklist.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ The rendering-parity guarantee (identical to Chrome of the pinned version) is on
44

55
1. **Read the release notes.** Electron breaking-changes page for the target major, plus its Chromium/Node versions. Note anything touching: session/cookies, permissions, `setWindowOpenHandler`, `will-navigate`/`will-redirect`, preload/sandbox, `net`/loopback, fuses.
66
2. **Bump the pin** in `apps/desktop/package.json` (exact version), `bun install`, `bun run type-check && bun run test`.
7-
3. **Fuses:** the packaged smoke test asserts the complete fuse wire. Decide the policy for every new fuse, configure it in `electron-builder.yml` when supported, and update the expected wire only after verifying the packaged binary.
7+
3. **Fuses:** the packaged smoke test asserts the complete fuse wire. Decide the policy for every new fuse, configure it in `electron-builder.yml` when supported, and update the expected wire only after verifying the packaged binary. `grantFileProtocolExtraPrivileges` stays off, which is why the bundled pages are served over `sim-shell:` (`src/main/local-pages.ts`) rather than `file:` — with it off, `file:` cannot read inside `app.asar`. The packaged smoke test loads the offline page over remote debugging to prove the pages still render after an upgrade.
88
4. **Cookie-encryption go/no-go:** packaged build → sign in → quit → relaunch → still signed in. If the session is lost, flip `enableCookieEncryption: false`, file it in the README, and retest.
99
5. **Manual spot-checks (packaged build):**
1010
- Google sign-in via the system-browser handoff (127.0.0.1 loopback callback → token redeem).

apps/desktop/e2e/packaged-smoke.spec.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'
44
import { tmpdir } from 'node:os'
55
import { join } from 'node:path'
66
import { FuseV1Options, FuseVersion, getCurrentFuseWire } from '@electron/fuses'
7-
import { expect, test } from '@playwright/test'
7+
import { type Browser, chromium, expect, test } from '@playwright/test'
88

99
const FUSE_DISABLED = '0'.charCodeAt(0)
1010
const FUSE_ENABLED = '1'.charCodeAt(0)
@@ -82,3 +82,79 @@ test('packaged main process starts and records launch telemetry', async () => {
8282
rmSync(userDataPath, { recursive: true, force: true })
8383
}
8484
})
85+
86+
// The unpackaged suite cannot see this: the bundled pages live inside app.asar
87+
// only once packaged, and the file-protocol fuse is only off once packaged.
88+
// v0.8.13 through v0.8.19 shipped both pages blank because nothing loaded them
89+
// in that configuration. Chromium's remote-debugging switch is honoured by the
90+
// fused binary, which is what lets the test read the rendered page.
91+
test('packaged shell renders the bundled offline page', async () => {
92+
const executablePath = process.env.SIM_DESKTOP_EXECUTABLE
93+
if (!executablePath) throw new Error('SIM_DESKTOP_EXECUTABLE is required')
94+
const userDataPath = mkdtempSync(join(tmpdir(), 'sim-desktop-packaged-e2e-'))
95+
// Cookie encryption and safeStorage key their secret off the app's identity
96+
// in the login keychain. A build under test (unsigned locally, or the first
97+
// run on a machine that already has the real app's item) would block on a
98+
// Keychain prompt on its main thread, and the debugging endpoint with it.
99+
const child = spawn(executablePath, ['--remote-debugging-port=0', '--use-mock-keychain'], {
100+
env: {
101+
...process.env,
102+
SIM_DESKTOP_ORIGIN: 'http://127.0.0.1:1',
103+
SIM_DESKTOP_USER_DATA: userDataPath,
104+
},
105+
stdio: 'ignore',
106+
})
107+
const portFile = join(userDataPath, 'DevToolsActivePort')
108+
let browser: Browser | undefined
109+
110+
try {
111+
await expect
112+
.poll(
113+
() => {
114+
if (child.exitCode !== null || child.signalCode !== null) {
115+
throw new Error(
116+
`Packaged app exited with ${child.exitCode ?? child.signalCode ?? 'unknown status'}`
117+
)
118+
}
119+
return existsSync(portFile) && readFileSync(portFile, 'utf8').trim().length > 0
120+
},
121+
{ timeout: 15_000 }
122+
)
123+
.toBe(true)
124+
const port = Number(readFileSync(portFile, 'utf8').split('\n')[0])
125+
browser = await chromium.connectOverCDP(`http://127.0.0.1:${port}`)
126+
const findPage = (urlPrefix: string) =>
127+
browser
128+
?.contexts()
129+
.flatMap((context) => context.pages())
130+
.find((page) => page.url().startsWith(urlPrefix))
131+
await expect
132+
.poll(() => Boolean(findPage('sim-shell://pages/offline.html?')), { timeout: 15_000 })
133+
.toBe(true)
134+
const offline = findPage('sim-shell://pages/offline.html?')
135+
if (!offline) throw new Error('offline page disappeared')
136+
await expect(offline.locator('#title')).toHaveText('Can’t connect to Sim')
137+
await expect(offline.locator('#server')).toBeVisible()
138+
139+
// The picker is the recovery path from here. Opening it and reading the
140+
// pre-filled value crosses the local-page IPC gate twice, which packaged
141+
// builds also used to refuse: the allowlist was resolved against a working
142+
// directory that is `/` when Finder launches the app.
143+
await offline.locator('#server').click()
144+
await expect
145+
.poll(() => Boolean(findPage('sim-shell://pages/server.html')), { timeout: 15_000 })
146+
.toBe(true)
147+
const picker = findPage('sim-shell://pages/server.html')
148+
if (!picker) throw new Error('server picker disappeared')
149+
await expect(picker.locator('h1')).toHaveText('Sim server')
150+
await expect(picker.locator('#origin')).toHaveValue('http://127.0.0.1:1')
151+
} finally {
152+
await browser?.close().catch(() => {})
153+
if (child.exitCode === null && child.signalCode === null) {
154+
const exited = once(child, 'exit')
155+
child.kill('SIGKILL')
156+
await exited
157+
}
158+
rmSync(userDataPath, { recursive: true, force: true })
159+
}
160+
})

apps/desktop/e2e/smoke.spec.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ test.describe('desktop shell smoke', () => {
157157
app = await launchApp('http://127.0.0.1:1')
158158
const window = await app.firstWindow()
159159
await window.waitForSelector('#retry', { timeout: 30_000 })
160-
expect(window.url().startsWith('file:')).toBe(true)
160+
expect(window.url()).toMatch(/^sim-shell:\/\/pages\/offline\.html\?/)
161161
await expect(window.locator('.wordmark')).toBeVisible()
162162
await expect(window.locator('.wordmark')).toHaveAttribute('aria-label', 'Sim')
163163
await expect(window.locator('#title')).toHaveText('Can’t connect to Sim')
@@ -183,4 +183,29 @@ test.describe('desktop shell smoke', () => {
183183
await expect(window.locator('#retry')).toHaveCSS('outline-style', 'solid')
184184
await expect(window.locator('#detail')).toHaveAttribute('role', 'status')
185185
})
186+
187+
// The picker is the only way to repoint a shell whose server is unreachable.
188+
// Its page, the pre-filled value (which crosses the local-page IPC gate) and
189+
// Escape are asserted together because the packaged build once opened it as
190+
// a blank sheet with no way out.
191+
test('the offline page opens the server picker, pre-filled, and Escape closes it', async () => {
192+
app = await launchApp('http://127.0.0.1:1')
193+
const window = await app.firstWindow()
194+
await window.waitForSelector('#server', { timeout: 30_000 })
195+
196+
const pickerPromise = app.waitForEvent('window')
197+
await window.locator('#server').click()
198+
const picker = await pickerPromise
199+
200+
expect(picker.url()).toBe('sim-shell://pages/server.html')
201+
await expect(picker.locator('h1')).toHaveText('Sim server')
202+
await expect(picker.locator('#origin')).toHaveValue('http://127.0.0.1:1')
203+
204+
const closed = picker.waitForEvent('close')
205+
// The main process destroys the window on the key-down, so the key-up half
206+
// of `press` has no target to reach; the close event is the assertion.
207+
await picker.keyboard.press('Escape').catch(() => {})
208+
await closed
209+
expect(app.windows()).toHaveLength(1)
210+
})
186211
})

apps/desktop/src/main/index.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { join, resolve } from 'node:path'
1+
import { join } from 'node:path'
22
import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import type { OpenDialogOptions, Session, WebContents } from 'electron'
@@ -57,6 +57,12 @@ import { registerIpcHandlers } from '@/main/ipc'
5757
import { attachLoadHealth, type LoadHealthHandle } from '@/main/load-health'
5858
import { LocalFilesystemService } from '@/main/local-filesystem'
5959
import { createEncryptedLocalFilesystemGrantStore } from '@/main/local-filesystem-grant-store'
60+
import {
61+
attachLocalPageProtocol,
62+
isLocalPageUrl,
63+
localPageUrl,
64+
registerLocalPageScheme,
65+
} from '@/main/local-pages'
6066
import { installApplicationMenu } from '@/main/menu'
6167
import { openExternalSafe } from '@/main/navigation'
6268
import { createEventLog, installMainProcessFailureObservers } from '@/main/observability'
@@ -91,8 +97,6 @@ function reportHandoffFailure(error: unknown): void {
9197
logger.error('Sign-in handoff failed', { error: getErrorMessage(error) })
9298
}
9399

94-
const OFFLINE_PAGE = 'static/offline.html'
95-
const SERVER_PAGE = 'static/server.html'
96100
const DOCK_ICON_FOR_CHANNEL = {
97101
prod: 'dock-icon.png',
98102
staging: 'dock-icon-staging.png',
@@ -259,6 +263,7 @@ function main(): void {
259263
}
260264
configuredPartitions.add(partition)
261265
setupPermissionHandlers(ses, appOrigin)
266+
attachLocalPageProtocol(ses)
262267
attachCspFallback(ses, appOrigin)
263268
attachDownloadHandling(ses, events)
264269
attachTelemetryPolicy(ses, config.get('blockThirdPartyAnalytics') ?? true)
@@ -425,7 +430,7 @@ function main(): void {
425430
allowHttpLocalhost: allowHttpLocalhost(),
426431
})
427432
const loadHealth = attachLoadHealth(win, {
428-
offlinePagePath: OFFLINE_PAGE,
433+
offlinePageUrl: (query) => localPageUrl('offline.html', query),
429434
getStartUrl: () => `${appOrigin()}${route}`,
430435
isOnline: () => net.isOnline(),
431436
events,
@@ -524,7 +529,6 @@ function main(): void {
524529
const serverWindow = createServerWindow({
525530
config,
526531
defaultOrigin: DEFAULT_ORIGIN,
527-
pagePath: SERVER_PAGE,
528532
preloadPath,
529533
isPackaged: app.isPackaged,
530534
getParentWindow: getMainWindow,
@@ -754,7 +758,7 @@ function main(): void {
754758
appOrigin,
755759
allowHttpLocalhost,
756760
accountDataAvailable,
757-
localPagePaths: [resolve(OFFLINE_PAGE), resolve(SERVER_PAGE)],
761+
isLocalPageUrl,
758762
scopeEvents,
759763
retryLoad: (sender) => {
760764
const win = windowForContents(sender)
@@ -894,6 +898,10 @@ if (process.env.SIM_DESKTOP_USER_DATA) {
894898
app.setPath('userData', process.env.SIM_DESKTOP_USER_DATA)
895899
}
896900

901+
// The scheme the offline page and server picker load from must be declared
902+
// before the app is ready; the per-session handlers attach later.
903+
registerLocalPageScheme()
904+
897905
// Capture native minidumps for main/renderer/GPU crashes. Local-only: there is
898906
// no crash-ingest backend, so nothing is uploaded — the dumps land under
899907
// userData/Crashpad and the event log records where. Must start before the app

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ import { getSearchSuggestions } from '@/main/browser-search/suggestions'
133133
import { trackInputActivity } from '@/main/input-activity'
134134
import { type IpcDeps, openMicrophoneSettings, registerIpcHandlers } from '@/main/ipc'
135135
import { LocalFilesystemService } from '@/main/local-filesystem'
136+
import { isLocalPageUrl } from '@/main/local-pages'
136137
import { TerminalRegistry } from '@/main/terminal/registry'
137138
import { findCachedTerminalThemeProfile, listTerminalThemeProfiles } from '@/main/terminal-themes'
138139

@@ -224,14 +225,14 @@ function trackedSender() {
224225
}
225226

226227
const rejectedSender = () => trackedSender().sender
227-
const fileSender = rejectedSender()
228+
const localPageSender = rejectedSender()
228229
const appSender = rejectedSender()
229230
const evilSender = rejectedSender()
230231
const activeSender = trackedSender()
231232
const activeChooserSender = trackedSender()
232-
const fileEvent = {
233-
senderFrame: { url: 'file:///app/static/offline.html' },
234-
sender: fileSender,
233+
const localPageEvent = {
234+
senderFrame: { url: 'sim-shell://pages/offline.html?kind=dns&detail=probe' },
235+
sender: localPageSender,
235236
}
236237
const appEvent = { senderFrame: { url: `${APP}/workspace/ws1` }, sender: appSender }
237238
const activeAppEvent = {
@@ -246,7 +247,7 @@ const inactiveAppEvent = {
246247
const evilEvent = { senderFrame: { url: 'https://evil.example/page' }, sender: evilSender }
247248
const arbitraryFileEvent = {
248249
senderFrame: { url: 'file:///Users/example/private.html' },
249-
sender: fileSender,
250+
sender: localPageSender,
250251
}
251252
/** The chooser anchors a native menu, so it needs a sender with a window. */
252253
const FAKE_WINDOW = { id: 'main-window' }
@@ -288,7 +289,7 @@ describe('registerIpcHandlers', () => {
288289
appOrigin: () => APP,
289290
allowHttpLocalhost: () => false,
290291
accountDataAvailable: () => true,
291-
localPagePaths: ['/app/static/offline.html', '/app/static/server.html'],
292+
isLocalPageUrl,
292293
retryLoad: vi.fn(),
293294
beginOAuthConnect: vi.fn(async () => true),
294295
localFilesystem: new LocalFilesystemService({
@@ -402,7 +403,7 @@ describe('registerIpcHandlers', () => {
402403
const { invoke } = collectHandlers()
403404
const handler = invoke.get('desktop:oauth-connect')
404405
expect(await handler?.(evilEvent, 'slack')).toBe(false)
405-
expect(await handler?.(fileEvent, 'slack')).toBe(false)
406+
expect(await handler?.(localPageEvent, 'slack')).toBe(false)
406407
expect(await handler?.(appEvent, 'slack')).toBe(false)
407408
expect(deps.beginOAuthConnect).not.toHaveBeenCalled()
408409
expect(await handler?.(activeAppEvent, 42)).toBe(false)
@@ -681,8 +682,8 @@ describe('registerIpcHandlers', () => {
681682
expect(deps.retryLoad).not.toHaveBeenCalled()
682683
on.get('offline:retry')?.(arbitraryFileEvent)
683684
expect(deps.retryLoad).not.toHaveBeenCalled()
684-
on.get('offline:retry')?.(fileEvent)
685-
expect(deps.retryLoad).toHaveBeenCalledWith(fileSender)
685+
on.get('offline:retry')?.(localPageEvent)
686+
expect(deps.retryLoad).toHaveBeenCalledWith(localPageSender)
686687
})
687688

688689
it('registers every channel the preload bridge invokes or sends', () => {
@@ -738,7 +739,7 @@ describe('registerIpcHandlers', () => {
738739
ok: false,
739740
error: expect.stringContaining('not allowed'),
740741
})
741-
expect(await handler?.(fileEvent, 'tool-1', 'browser_navigate', {})).toMatchObject({
742+
expect(await handler?.(localPageEvent, 'tool-1', 'browser_navigate', {})).toMatchObject({
742743
ok: false,
743744
})
744745
expect(await handler?.(appEvent, 'tool-1', 'browser_snapshot', {}, 'chat-1')).toMatchObject({
@@ -1648,7 +1649,7 @@ describe('registerIpcHandlers', () => {
16481649
const handler = invoke.get('browser-import:list-profiles')
16491650

16501651
expect(await handler?.(evilEvent)).toEqual([])
1651-
expect(await handler?.(fileEvent)).toEqual([])
1652+
expect(await handler?.(localPageEvent)).toEqual([])
16521653
expect(listChromeImportProfiles).not.toHaveBeenCalled()
16531654

16541655
expect(await handler?.(appEvent)).toEqual([{ id: 'Default', label: 'Person 1' }])

apps/desktop/src/main/ipc.ts

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import { normalize } from 'node:path'
2-
import { fileURLToPath } from 'node:url'
31
import {
42
BROWSER_TOOL_AUTHORIZATION_TIMEOUT_MS,
53
type BrowserPanelAction,
@@ -332,8 +330,8 @@ export interface IpcDeps {
332330
allowHttpLocalhost: () => boolean
333331
/** False while local account-data persistence is unavailable or teardown must be retried. */
334332
accountDataAvailable: () => boolean
335-
/** Absolute paths of the bundled recovery pages allowed to control the shell. */
336-
localPagePaths: readonly string[]
333+
/** Whether a frame URL is one of the bundled pages allowed to control the shell. */
334+
isLocalPageUrl: (url: string) => boolean
337335
retryLoad: (sender: WebContents) => void
338336
localFilesystem: LocalFilesystemService
339337
terminal: TerminalRegistry
@@ -383,7 +381,8 @@ export interface IpcDeps {
383381
/**
384382
* Who may call a channel:
385383
* - `app-origin`: only the remote app origin (main window pages).
386-
* - `local-page`: only bundled `file:` pages (offline) — shell control.
384+
* - `local-page`: only the bundled pages served from the shell's own scheme
385+
* (offline, server) — shell control.
387386
* - `browser-page`: only the built-in browser's own tabs, identified by
388387
* WebContents rather than by URL. These carry reports from the browser
389388
* preload about untrusted pages, so they are the one inbound surface whose
@@ -435,16 +434,9 @@ type ChannelSpec =
435434

436435
function isLocalPageSender(
437436
event: IpcMainEvent | IpcMainInvokeEvent,
438-
localPagePaths: readonly string[]
437+
isLocalPageUrl: (url: string) => boolean
439438
): boolean {
440-
try {
441-
const url = new URL(event.senderFrame?.url ?? '')
442-
if (url.protocol !== 'file:') return false
443-
const senderPath = normalize(fileURLToPath(url))
444-
return localPagePaths.some((allowedPath) => senderPath === normalize(allowedPath))
445-
} catch {
446-
return false
447-
}
439+
return isLocalPageUrl(event.senderFrame?.url ?? '')
448440
}
449441

450442
/**
@@ -1929,7 +1921,7 @@ export function registerIpcHandlers(deps: IpcDeps): void {
19291921
if (gate === 'any') return true
19301922
if (gate === 'app-origin') return isAppOriginSender(event, deps.appOrigin())
19311923
if (gate === 'browser-page') return isAgentWebContents(event.sender)
1932-
return isLocalPageSender(event, deps.localPagePaths)
1924+
return isLocalPageSender(event, deps.isLocalPageUrl)
19331925
}
19341926

19351927
const featureAllowed = (feature: ChannelFeature | undefined): boolean => {

0 commit comments

Comments
 (0)