Skip to content

Commit 09b40cb

Browse files
committed
fix(desktop): validate manual-update download urls against the scheme allowlist
`buildManualEngine` regex-extracted `url:` values from the manifest served by the configured origin and handed the first `.dmg`/`.zip` straight to `shell.openExternal` — the only openExternal in non-test code that skipped `openExternalSafe`, whose own docs state "Every openExternal in the app goes through here". A hostile feed, or a hostile self-host origin the user was tricked into configuring, could return `version: 999.0.0` plus `url: smb://attacker/share/x.dmg` or `file:///…`; both pass the suffix test, so a Download click handed an arbitrary scheme to the macOS URL handler, launching a registered protocol handler instead of downloading. Candidates are now filtered with `isSafeExternalUrl` at selection, so an unusable url is never advertised as an available update at all, and the open goes through `openExternalSafe` so the allowlist also holds at the sink. Loopback http stays allowed because `feedUrlForOrigin` accepts an http origin, so a self-host on localhost is legitimate. Reachability is capped: `detectSelfUpdateCapability` selects the signature-verifying electron-updater engine on Developer-ID builds, so the manual engine runs only on ad-hoc-signed local/CI prerelease builds.
1 parent b2cefe2 commit 09b40cb

2 files changed

Lines changed: 66 additions & 3 deletions

File tree

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,51 @@ describe('initUpdater manual mode (no Developer ID signature)', () => {
326326
expect(shell.openExternal).toHaveBeenCalledTimes(2)
327327
})
328328

329+
it('refuses a manifest whose download urls are not http(s)', async () => {
330+
const hostile = [
331+
'version: 9.9.9',
332+
'files:',
333+
' - url: smb://attacker.example/share/Sim-9.9.9-universal.dmg',
334+
' sha512: abc',
335+
' - url: file:///Applications/Calculator.app',
336+
' sha512: def',
337+
"releaseDate: '2026-07-23T00:00:00.000Z'",
338+
].join('\n')
339+
const { handle } = await createManualUpdater(async () => hostile)
340+
341+
handle.check()
342+
await vi.advanceTimersByTimeAsync(0)
343+
344+
// Never advertised, so the user is never offered a Download button for it.
345+
expect(handle.getState()).toEqual({ status: 'idle', manual: true })
346+
347+
handle.check()
348+
handle.install()
349+
expect(shell.openExternal).not.toHaveBeenCalled()
350+
})
351+
352+
it('skips an unusable url but still offers a safe one from the same manifest', async () => {
353+
const mixed = [
354+
'version: 9.9.9',
355+
'files:',
356+
' - url: javascript:alert(1)//Sim-9.9.9-universal.dmg',
357+
' sha512: abc',
358+
' - url: https://github.com/simstudioai/sim/releases/download/v9.9.9/Sim-9.9.9-universal.dmg',
359+
' sha512: def',
360+
"releaseDate: '2026-07-23T00:00:00.000Z'",
361+
].join('\n')
362+
const { handle } = await createManualUpdater(async () => mixed)
363+
364+
handle.check()
365+
await vi.advanceTimersByTimeAsync(0)
366+
expect(handle.getState()).toEqual({ status: 'available', version: '9.9.9', manual: true })
367+
368+
handle.check()
369+
expect(shell.openExternal).toHaveBeenCalledWith(
370+
'https://github.com/simstudioai/sim/releases/download/v9.9.9/Sim-9.9.9-universal.dmg'
371+
)
372+
})
373+
329374
it('stays idle when the feed version is not newer', async () => {
330375
const { handle } = await createManualUpdater(async () => manifest(app.getVersion()))
331376
handle.check()

apps/desktop/src/main/updater.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import type { DesktopUpdateState } from '@sim/desktop-bridge'
33
import { createLogger } from '@sim/logger'
44
import { getErrorMessage } from '@sim/utils/errors'
55
import type { BrowserWindow } from 'electron'
6-
import { app, dialog, net, shell } from 'electron'
6+
import { app, dialog, net } from 'electron'
7+
import { isSafeExternalUrl, openExternalSafe } from '@/main/navigation'
78
import type { EventRecorder } from '@/main/observability'
89

910
const logger = createLogger('DesktopUpdater')
@@ -434,12 +435,26 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
434435
}
435436
// The feed rewrites manifest urls to absolute GitHub asset URLs;
436437
// prefer the dmg for a human download.
437-
const urls = Array.from(manifest.matchAll(/^\s*(?:-\s*)?url:\s*(\S+)\s*$/gm), (m) => m[1])
438+
//
439+
// Filtered before selection, not just before opening: the manifest is
440+
// whatever the configured origin served, so `smb://…/x.dmg` or a bare
441+
// `file:///…` would otherwise pass the suffix test and be advertised as
442+
// an available update. Loopback http is kept because feedUrlForOrigin
443+
// accepts an http origin, so a self-host on localhost is legitimate.
444+
const urls = Array.from(
445+
manifest.matchAll(/^\s*(?:-\s*)?url:\s*(\S+)\s*$/gm),
446+
(m) => m[1]
447+
).filter((url) => isSafeExternalUrl(url, true))
438448
downloadUrl =
439449
urls.find((url) => url.endsWith('.dmg')) ??
440450
urls.find((url) => url.endsWith('.zip')) ??
441451
urls[0] ??
442452
null
453+
if (!downloadUrl) {
454+
logger.warn('Update manifest had no usable download url')
455+
setState({ status: 'idle', manual: true })
456+
return
457+
}
443458
deps.events.record('update_check', { available: version, manual: true })
444459
setState({ status: 'available', version, manual: true })
445460
} catch (error) {
@@ -451,7 +466,10 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
451466
const openDownload = () => {
452467
if (downloadUrl) {
453468
deps.events.record('update_manual_download', { url: downloadUrl })
454-
void shell.openExternal(downloadUrl)
469+
// Through openExternalSafe like every other external open in the app,
470+
// so the scheme allowlist is enforced at the sink and not only where
471+
// the url was chosen.
472+
void openExternalSafe(downloadUrl, true)
455473
}
456474
}
457475

0 commit comments

Comments
 (0)