diff --git a/electron-builder.yml b/electron-builder.yml index b85cc3b3..732b7c7a 100644 --- a/electron-builder.yml +++ b/electron-builder.yml @@ -26,6 +26,10 @@ publish: owner: Comfy-Org repo: Comfy-Desktop artifactName: Comfy-Desktop-${version}-${os}-${arch}.${ext} +protocols: + - name: Comfy Desktop + schemes: + - comfy win: target: nsis icon: assets/Comfy_Logo_x256.png diff --git a/src/main/index.ts b/src/main/index.ts index 3c6c6a49..65739020 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -10,6 +10,7 @@ import todesktop from '@todesktop/runtime' import * as ipc from './lib/ipc' import { getAppVersion } from './lib/ipc' import type { ExitCallbackInfo } from './lib/ipc' +import { startDesktopBeacon, stopDesktopBeacon } from './lib/desktopBeacon' import { closeAllPopouts } from './lib/popoutWindows' import { disposeAllTerminals } from './lib/terminal' import * as updater from './lib/updater' @@ -51,7 +52,8 @@ import { registerDownloadHandlers } from './lib/ipc/registerDownloadHandlers' import { get as getInstallation, installationEvents, - list as listInstallations + list as listInstallations, + CLOUD_SOURCE_ID } from './installations' import { startPeriodicReleaseChecks } from './lib/release-cache-startup' import { showSplashPage } from './lib/relaunchPage' @@ -86,8 +88,10 @@ import { import { initExperiments } from './lib/experiments' import { initCloudCapacity } from './lib/cloudCapacity' import { initUserTier } from './lib/userTier' +import { resolveDeepLink } from './lib/deepLink' import { + bringToFront, claimAttachHost, comfyWindows, computeBodyMode, @@ -1227,14 +1231,126 @@ function findInstallationIdForWindow(win: BrowserWindow): string | undefined { return undefined } +// --------------------------------------------------------------------------- +// `comfy://` deep links +// +// An OS `comfy://open?path=/workflows/123` link opens (or focuses) the app and +// loads `https://cloud.comfy.org/workflows/123` in the Comfy Cloud host window. +// The raw string is untrusted: `resolveDeepLink` allowlists the resolved origin +// to cloud.comfy.org and returns `null` for anything else (other scheme/origin, +// `//evil.com` tricks, relative paths). We never navigate to a non-cloud URL. +// --------------------------------------------------------------------------- + +/** A `comfy://` URL received before `app.whenReady()` resolved (macOS + * `open-url` can fire at cold start). Replayed once the app is ready. */ +let pendingDeepLink: string | null = null +/** Flipped true once `whenReady` has run so `handleDeepLink` knows whether to + * act immediately or buffer. */ +let appIsReady = false + +/** Pull the first `comfy://` argument out of a process argv array (the shape + * Windows/Linux deliver a deep link in — at cold start and on second-instance). */ +function findDeepLinkArg(argv: readonly string[]): string | null { + return argv.find((arg) => arg.startsWith('comfy://')) ?? null +} + +/** + * Resolve, then route a raw `comfy://` deep link to the cloud host window. + * Invalid links (per `resolveDeepLink`) are ignored. Buffers until the app is + * ready so a cold-start link isn't dropped. + */ +function handleDeepLink(rawUrl: string): void { + const target = resolveDeepLink(rawUrl) + if (!target) { + console.warn('[deeplink] ignored untrusted or malformed link', { rawUrl }) + return + } + if (!appIsReady) { + pendingDeepLink = target + return + } + void routeCloudDeepLink(target).catch((err) => { + console.warn('[deeplink] failed to route link', err) + }) +} + +/** + * Navigate the Comfy Cloud host to `targetUrl`. + * - If a cloud host window already exists, focus it and point its comfyView at + * the URL (covers the already-running case `handleLaunch` would refuse). + * - Otherwise stamp the URL onto the cloud install and launch it through the + * normal cloud source path, which opens the host window via `onLaunch`. + */ +/** Open the normal cold-start surface (dashboard / restore-last) unless a + * cold-start `comfy://` deep link is pending — in which case go straight to + * the cloud window so the user doesn't see the dashboard and the deep-linked + * cloud window pop up side-by-side. */ +function openColdStartSurface(): void { + if (pendingDeepLink !== null) { + const link = pendingDeepLink + pendingDeepLink = null + void routeCloudDeepLink(link).catch((err) => { + console.warn('[deeplink] failed to route buffered link', err) + }) + return + } + void openStartupSurface() +} + +async function routeCloudDeepLink(targetUrl: string): Promise { + for (const entry of comfyWindows.values()) { + if (entry.window.isDestroyed()) continue + if (entry.sourceCategory !== 'cloud') continue + entry.comfyUrl = targetUrl + bringToFront(entry.window) + if (!entry.comfyView.webContents.isDestroyed()) { + void entry.comfyView.webContents.loadURL(targetUrl).catch(() => {}) + } + return + } + + // No live cloud host — launch the cloud install pointed at the target URL. + const all = await listInstallations() + const cloudInstall = all.find((i) => i.sourceId === CLOUD_SOURCE_ID) + if (!cloudInstall) { + console.warn('[deeplink] no cloud installation to launch') + return + } + const inst = (await updateInstallation(cloudInstall.id, { remoteUrl: targetUrl })) ?? cloudInstall + const stubSender = { + isDestroyed: () => false, + send: () => {} + } as unknown as Electron.WebContents + const stubEvent = { sender: stubSender } as unknown as Electron.IpcMainInvokeEvent + await handleLaunch({ + event: stubEvent, + installationId: inst.id, + inst, + actionData: undefined + }) +} + +// macOS delivers `comfy://` links via `open-url`, which can fire BEFORE +// `whenReady` at cold start — register it eagerly so the link is buffered. +app.on('open-url', (event, url) => { + event.preventDefault() + handleDeepLink(url) +}) + if (app.isPackaged && !app.requestSingleInstanceLock()) { app.quit() } else { if (app.isPackaged) { - app.on('second-instance', () => { - // OS-level "open another instance" attempt — focus an existing - // host window (chooser or install-backed) instead of stacking - // a duplicate. + app.on('second-instance', (_event, argv) => { + // OS-level "open another instance" attempt. On Windows/Linux a + // `comfy://` deep link is delivered as the second instance's argv — + // route it instead of merely focusing. Otherwise just focus an + // existing host window rather than stacking a duplicate. + const deepLink = findDeepLinkArg(argv) + if (deepLink) { + handleDeepLink(deepLink) + return + } openOrFocusAnyHostWindow() }) } @@ -1267,6 +1383,31 @@ if (app.isPackaged && !app.requestSingleInstanceLock()) { }) app.whenReady().then(async () => { + // Register the app as the OS handler for `comfy://` links. On an + // unpackaged dev build the executable is Electron itself, so the + // launcher path + the app entry argv must be forwarded explicitly + // (Windows especially); packaged builds resolve to the real app. + if (process.defaultApp && process.argv.length >= 2) { + app.setAsDefaultProtocolClient('comfy', process.execPath, [path.resolve(process.argv[1]!)]) + } else { + app.setAsDefaultProtocolClient('comfy') + } + + // Localhost discovery beacon: lets cloud.comfy.org detect Desktop and + // hand off `?share=…` / canvas URLs without spamming the OS scheme + // prompt at users who don't have Desktop. Fire-and-forget — a failed + // bind silently disables discovery for the session rather than + // crashing the app. + void startDesktopBeacon(app.getVersion()).catch((err) => { + console.warn('[beacon] failed to start', err) + }) + + // Windows/Linux cold start: a `comfy://` link the OS used to launch us + // arrives in argv rather than via `open-url`. Buffer it now; it replays + // once the app finishes coming up (below, after host factories wire up). + const coldStartLink = findDeepLinkArg(process.argv) + if (coldStartLink) pendingDeepLink = resolveDeepLink(coldStartLink) + // Test-only hooks for the E2E suite. Registered before any host // opens so seeded state (downloads, install-update overrides, // app-update state) is visible to the very first title-bar paint. @@ -2063,7 +2204,7 @@ if (app.isPackaged && !app.requestSingleInstanceLock()) { clearQuitReason() if (updateSplash && !updateSplash.isDestroyed()) updateSplash.destroy() mainTelemetry.emit('comfy.desktop.app_update.startup_install_backstop_recovered', {}) - void openStartupSurface() + openColdStartSurface() }, STARTUP_INSTALL_QUIT_BACKSTOP_MS) } else { app.removeListener('before-quit', onUpdateInstallQuit) @@ -2073,8 +2214,10 @@ if (app.isPackaged && !app.requestSingleInstanceLock()) { // when launched, and the chooser host is the entry-point for // picking / creating installs. When the user last left an instance // window (and the reopen setting is on), restore that instance - // in-place on top of the freshly-opened chooser host. - void openStartupSurface() + // in-place on top of the freshly-opened chooser host. A pending + // cold-start deep link short-circuits the dashboard inside + // openColdStartSurface and goes straight to the cloud window. + openColdStartSurface() } // Single subscription rebroadcasts every install-list mutation @@ -2119,6 +2262,12 @@ if (app.isPackaged && !app.requestSingleInstanceLock()) { ? { intervalMs: _periodicIntervalMs } : {}) }) + + // App is fully up. Subsequent `comfy://` arrivals (open-url on macOS, + // second-instance argv on Windows/Linux) route directly via + // `handleDeepLink` now that `appIsReady` is true; cold-start links are + // already handled by `openColdStartSurface` above. + appIsReady = true }) app.on('activate', () => { @@ -2136,6 +2285,10 @@ if (app.isPackaged && !app.requestSingleInstanceLock()) { }) app.on('before-quit', () => { + // The beacon owns a `node:http` server bound to 127.0.0.1; close it + // unconditionally so its handle doesn't outlive the main process + // across an `app.relaunch`. + void stopDesktopBeacon() if (!isQuitInProgress()) { setQuitReason('user-quit') ipc.cancelAll() diff --git a/src/main/lib/deepLink.test.ts b/src/main/lib/deepLink.test.ts new file mode 100644 index 00000000..b4918513 --- /dev/null +++ b/src/main/lib/deepLink.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, it } from 'vitest' +import { resolveDeepLink } from './deepLink' + +describe('resolveDeepLink', () => { + describe('valid links', () => { + it('resolves a path link to the cloud origin', () => { + expect(resolveDeepLink('comfy://open?path=/workflows/123')).toBe( + 'https://cloud.comfy.org/workflows/123' + ) + }) + + it('resolves the bare cloud root path', () => { + expect(resolveDeepLink('comfy://open?path=/')).toBe('https://cloud.comfy.org/') + }) + + it('preserves a query string on a path link', () => { + // The query rides along in the `path` value. A fragment cannot: an + // unencoded `#` belongs to the outer `comfy://` URL, so anything that + // needs a hash must use the encoded `url=` form below. + expect(resolveDeepLink('comfy://open?path=/workflows/123?tab=runs')).toBe( + 'https://cloud.comfy.org/workflows/123?tab=runs' + ) + }) + + it('preserves query and hash via an encoded url param', () => { + const target = 'https://cloud.comfy.org/workflows/123?tab=runs#top' + expect(resolveDeepLink(`comfy://open?url=${encodeURIComponent(target)}`)).toBe(target) + }) + + it('resolves a full cloud url param', () => { + expect(resolveDeepLink('comfy://open?url=https://cloud.comfy.org/x')).toBe( + 'https://cloud.comfy.org/x' + ) + }) + + it('prefers url over path when both are present', () => { + expect( + resolveDeepLink('comfy://open?url=https://cloud.comfy.org/a&path=/b') + ).toBe('https://cloud.comfy.org/a') + }) + }) + + describe('rejected: wrong scheme', () => { + it('rejects https scheme', () => { + expect(resolveDeepLink('https://cloud.comfy.org/workflows/123')).toBeNull() + }) + + it('rejects http scheme', () => { + expect(resolveDeepLink('http://cloud.comfy.org/workflows/123')).toBeNull() + }) + + it('rejects file scheme', () => { + expect(resolveDeepLink('file:///etc/passwd')).toBeNull() + }) + + it('rejects javascript scheme', () => { + expect(resolveDeepLink('javascript:alert(1)')).toBeNull() + }) + }) + + describe('rejected: disallowed origin', () => { + it('rejects a non-cloud origin in url param', () => { + expect(resolveDeepLink('comfy://open?url=https://evil.com')).toBeNull() + }) + + it('rejects a non-cloud origin with a cloud-looking subdomain', () => { + expect( + resolveDeepLink('comfy://open?url=https://cloud.comfy.org.evil.com/x') + ).toBeNull() + }) + + it('rejects an http cloud url param (origin includes scheme)', () => { + expect(resolveDeepLink('comfy://open?url=http://cloud.comfy.org/x')).toBeNull() + }) + }) + + describe('rejected: path tricks', () => { + it('rejects protocol-relative path', () => { + expect(resolveDeepLink('comfy://open?path=//evil.com')).toBeNull() + }) + + it('rejects an absolute https url passed as path', () => { + expect(resolveDeepLink('comfy://open?path=https://evil.com')).toBeNull() + }) + + it('rejects a backslash trick path', () => { + expect(resolveDeepLink('comfy://open?path=/\\evil.com')).toBeNull() + }) + + it('rejects a relative path', () => { + expect(resolveDeepLink('comfy://open?path=workflows/123')).toBeNull() + }) + + it('rejects a missing path/url', () => { + expect(resolveDeepLink('comfy://open')).toBeNull() + }) + + it('rejects an empty path', () => { + expect(resolveDeepLink('comfy://open?path=')).toBeNull() + }) + }) + + describe('rejected: malformed input', () => { + it('rejects an empty string', () => { + expect(resolveDeepLink('')).toBeNull() + }) + + it('rejects garbage', () => { + expect(resolveDeepLink('not a url at all')).toBeNull() + }) + + it('rejects a non-string input', () => { + // @ts-expect-error exercising the runtime guard against non-string input + expect(resolveDeepLink(undefined)).toBeNull() + }) + }) +}) diff --git a/src/main/lib/deepLink.ts b/src/main/lib/deepLink.ts new file mode 100644 index 00000000..6c80aa61 --- /dev/null +++ b/src/main/lib/deepLink.ts @@ -0,0 +1,126 @@ +/** + * `comfy://` deep-link resolution. + * + * The OS hands us an arbitrary string when a `comfy://` link is opened. + * This module turns it into a fully-qualified cloud URL, or `null` when + * the input is anything we don't explicitly trust. It is deliberately + * pure (no Electron, no I/O) so the allowlist can be exhaustively + * unit-tested — the security of the feature lives here. + * + * Supported shapes: + * comfy://open?path=/workflows/123 → https://cloud.comfy.org/workflows/123 + * comfy://open?url=https://cloud.comfy.org/x → https://cloud.comfy.org/x + * + * Security model: the resolved URL's origin MUST be in CLOUD_ALLOWLIST. + * Anything that resolves to another origin or scheme — including + * protocol-relative `//evil.com` tricks smuggled through `path` — returns + * `null`. We never return a non-cloud URL. + */ + +/** Origins a deep link is allowed to navigate to. Production traffic is + * `https://cloud.comfy.org`; the test domain and local dev origins are + * included so the cloud-frontend banner can be verified end-to-end against + * a packaged Desktop install. Must stay in sync with the beacon CORS + * allowlist in `desktopBeacon.ts` — same threat model. */ +const CLOUD_ALLOWLIST = [ + 'https://cloud.comfy.org', + 'https://testcloud.comfy.org', + 'http://localhost:5173', + 'http://localhost:5174' +] as const + +/** Canonical cloud origin used to resolve `path=` links. (`path=` only + * applies to production cloud — dev frontends always pass full `url=`.) */ +const CLOUD_ORIGIN = CLOUD_ALLOWLIST[0] + +/** The single scheme we register and accept. */ +const DEEP_LINK_SCHEME = 'comfy:' + +function isAllowedOrigin(origin: string): boolean { + return (CLOUD_ALLOWLIST as readonly string[]).includes(origin) +} + +/** + * Resolve a raw `comfy://` deep link to a cloud URL, or `null` if the + * input is not a trusted cloud link. + */ +export function resolveDeepLink(rawUrl: string): string | null { + if (typeof rawUrl !== 'string' || rawUrl.length === 0) return null + + let link: URL + try { + link = new URL(rawUrl) + } catch { + return null + } + + // Only our own scheme. Rejects http:, https:, file:, javascript:, etc. + if (link.protocol !== DEEP_LINK_SCHEME) return null + + // Path-based: comfy://open?path=/some/path + const pathParam = link.searchParams.get('path') + // Full-URL-based: comfy://open?url= + const urlParam = link.searchParams.get('url') + + let candidate: string | null = null + + if (urlParam) { + candidate = resolveUrlParam(urlParam) + } else if (pathParam) { + candidate = resolvePathParam(pathParam) + } + + if (!candidate) return null + + // Final belt-and-braces check: re-parse and re-validate the origin so a + // bug in either resolver can never leak a non-cloud URL. + let resolved: URL + try { + resolved = new URL(candidate) + } catch { + return null + } + if (!isAllowedOrigin(resolved.origin)) return null + + return resolved.href +} + +/** Resolve a `url=` param: must already be an absolute URL on an allowed origin. */ +function resolveUrlParam(value: string): string | null { + let parsed: URL + try { + parsed = new URL(value) + } catch { + return null + } + if (!isAllowedOrigin(parsed.origin)) return null + return parsed.href +} + +/** + * Resolve a `path=` param against the canonical cloud origin. + * + * Must be a server-absolute path (`/...`). Rejects: + * - protocol-relative `//evil.com` (would change origin) + * - absolute URLs smuggled in (`https://evil.com`, `comfy://...`) + * - relative paths (`workflows/123`) + * by requiring a leading single slash and confirming the resolved origin + * is unchanged after joining. + */ +function resolvePathParam(value: string): string | null { + // Must start with a single `/` and not a second `/` (protocol-relative). + if (!value.startsWith('/') || value.startsWith('//')) return null + // A backslash can be normalised to `/` by some parsers — reject leading + // ones so `/\evil.com` style tricks can't sneak through. + if (value.includes('\\')) return null + + let joined: URL + try { + joined = new URL(value, CLOUD_ORIGIN + '/') + } catch { + return null + } + // Joining a true server-absolute path must not change the origin. + if (joined.origin !== CLOUD_ORIGIN) return null + return joined.href +} diff --git a/src/main/lib/desktopBeacon.test.ts b/src/main/lib/desktopBeacon.test.ts new file mode 100644 index 00000000..4d800cc9 --- /dev/null +++ b/src/main/lib/desktopBeacon.test.ts @@ -0,0 +1,303 @@ +// @vitest-environment node +import http from 'node:http' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { + BEACON_ALLOWED_ORIGIN, + BEACON_PORTS, + _getBeaconPort, + buildPingBody, + handleBeaconRequest, + startDesktopBeacon, + stopDesktopBeacon, +} from './desktopBeacon' + +const TEST_VERSION = '1.0.20' + +/** Fire a real HTTP request against a started beacon. Kept inline so tests + * exercise the same path the cloud frontend will. Uses a fresh agent per + * call so the global keep-alive pool can't hand us a stale socket from a + * previous test's now-dead server (which would surface as ECONNRESET). */ +async function fetchPing( + port: number, + init?: { origin?: string; method?: string; path?: string }, +): Promise<{ status: number; headers: http.IncomingHttpHeaders; body: string }> { + const agent = new http.Agent({ keepAlive: false }) + return new Promise((resolve, reject) => { + const req = http.request( + { + host: '127.0.0.1', + port, + path: init?.path ?? '/ping', + method: init?.method ?? 'GET', + headers: init?.origin ? { Origin: init.origin } : {}, + agent, + }, + (res) => { + const chunks: Buffer[] = [] + res.on('data', (c: Buffer) => chunks.push(c)) + res.on('end', () => { + agent.destroy() + resolve({ + status: res.statusCode ?? 0, + headers: res.headers, + body: Buffer.concat(chunks).toString('utf-8'), + }) + }) + }, + ) + req.on('error', (err) => { + agent.destroy() + reject(err) + }) + req.end() + }) +} + +describe('buildPingBody', () => { + it('exposes app + version only (no extra fields)', () => { + const body = JSON.parse(buildPingBody('1.2.3')) as Record + expect(body).toEqual({ app: 'comfy-desktop', version: '1.2.3' }) + }) +}) + +describe('handleBeaconRequest (unit)', () => { + function makeRes() { + let writtenStatus = 0 + let writtenHeaders: Record = {} + let body = '' + const inner = { + headersSent: false, + writeHead(status: number, headers?: http.OutgoingHttpHeaders): void { + writtenStatus = status + writtenHeaders = (headers ?? {}) as Record + inner.headersSent = true + }, + end(payload?: string): void { + if (payload) body += payload + }, + } + const res = inner as unknown as http.ServerResponse + return { + res, + get status() { + return writtenStatus + }, + get headers() { + return writtenHeaders + }, + get body() { + return body + }, + } + } + + it('GET /ping with allowed Origin returns 200 + CORS header + JSON body', () => { + const t = makeRes() + handleBeaconRequest( + { + method: 'GET', + url: '/ping', + headers: { origin: BEACON_ALLOWED_ORIGIN }, + } as unknown as http.IncomingMessage, + t.res, + TEST_VERSION, + ) + expect(t.status).toBe(200) + expect(t.headers['Access-Control-Allow-Origin']).toBe(BEACON_ALLOWED_ORIGIN) + expect(t.headers['Vary']).toBe('Origin') + expect(JSON.parse(t.body)).toEqual({ app: 'comfy-desktop', version: TEST_VERSION }) + }) + + it('GET /ping without an Origin header still returns 200 but omits CORS header', () => { + const t = makeRes() + handleBeaconRequest( + { method: 'GET', url: '/ping', headers: {} } as unknown as http.IncomingMessage, + t.res, + TEST_VERSION, + ) + // No-origin requests come from curl / health checks, not browsers. We + // still answer (so devs can verify the beacon's alive) but never set + // CORS, so a cross-origin script that omits the header can't read it. + expect(t.status).toBe(200) + expect(t.headers['Access-Control-Allow-Origin']).toBeUndefined() + }) + + it('GET /ping with a non-allowed Origin returns 200 but NO CORS header', () => { + const t = makeRes() + handleBeaconRequest( + { + method: 'GET', + url: '/ping', + headers: { origin: 'https://evil.com' }, + } as unknown as http.IncomingMessage, + t.res, + TEST_VERSION, + ) + expect(t.status).toBe(200) + expect(t.headers['Access-Control-Allow-Origin']).toBeUndefined() + }) + + it('returns 404 (and no CORS) for unknown paths', () => { + const t = makeRes() + handleBeaconRequest( + { + method: 'GET', + url: '/admin', + headers: { origin: BEACON_ALLOWED_ORIGIN }, + } as unknown as http.IncomingMessage, + t.res, + TEST_VERSION, + ) + expect(t.status).toBe(404) + expect(t.headers['Access-Control-Allow-Origin']).toBeUndefined() + }) + + it('returns 404 (and no CORS) for non-GET methods on /ping', () => { + const t = makeRes() + handleBeaconRequest( + { + method: 'POST', + url: '/ping', + headers: { origin: BEACON_ALLOWED_ORIGIN }, + } as unknown as http.IncomingMessage, + t.res, + TEST_VERSION, + ) + expect(t.status).toBe(404) + expect(t.headers['Access-Control-Allow-Origin']).toBeUndefined() + }) + + it('answers OPTIONS preflight for the allowed origin', () => { + const t = makeRes() + handleBeaconRequest( + { + method: 'OPTIONS', + url: '/ping', + headers: { origin: BEACON_ALLOWED_ORIGIN }, + } as unknown as http.IncomingMessage, + t.res, + TEST_VERSION, + ) + expect(t.status).toBe(204) + expect(t.headers['Access-Control-Allow-Origin']).toBe(BEACON_ALLOWED_ORIGIN) + expect(t.headers['Access-Control-Allow-Methods']).toBe('GET') + }) + + it('rejects OPTIONS preflight for a disallowed origin', () => { + const t = makeRes() + handleBeaconRequest( + { + method: 'OPTIONS', + url: '/ping', + headers: { origin: 'https://evil.com' }, + } as unknown as http.IncomingMessage, + t.res, + TEST_VERSION, + ) + expect(t.status).toBe(404) + expect(t.headers['Access-Control-Allow-Origin']).toBeUndefined() + }) +}) + +describe('startDesktopBeacon (integration)', () => { + afterEach(async () => { + await stopDesktopBeacon() + }) + + it('binds to 127.0.0.1 on a port in the allowlist and responds to /ping', async () => { + const port = await startDesktopBeacon(TEST_VERSION) + expect(port).not.toBeNull() + expect(BEACON_PORTS).toContain(port!) + expect(_getBeaconPort()).toBe(port) + + const res = await fetchPing(port!, { origin: BEACON_ALLOWED_ORIGIN }) + expect(res.status).toBe(200) + expect(res.headers['access-control-allow-origin']).toBe(BEACON_ALLOWED_ORIGIN) + expect(JSON.parse(res.body)).toEqual({ app: 'comfy-desktop', version: TEST_VERSION }) + }) + + it('is idempotent — calling start twice returns the same port without rebinding', async () => { + const first = await startDesktopBeacon(TEST_VERSION) + const second = await startDesktopBeacon(TEST_VERSION) + expect(first).toBe(second) + }) + + it('falls through to a later port when the first allowlisted port is taken', async () => { + // Pre-occupy the first port to force the fallthrough path. + const squatter = http.createServer() + await new Promise((resolve, reject) => { + squatter.once('error', reject) + squatter.listen(BEACON_PORTS[0], '127.0.0.1', () => resolve()) + }) + try { + const port = await startDesktopBeacon(TEST_VERSION) + expect(port).not.toBeNull() + expect(port).not.toBe(BEACON_PORTS[0]) + expect(BEACON_PORTS.slice(1)).toContain(port!) + } finally { + await new Promise((resolve) => squatter.close(() => resolve())) + } + }) + + it('returns null and stays disabled when every allowed port is taken', async () => { + const squatters: http.Server[] = [] + for (const p of BEACON_PORTS) { + const s = http.createServer() + await new Promise((resolve, reject) => { + s.once('error', reject) + s.listen(p, '127.0.0.1', () => resolve()) + }) + squatters.push(s) + } + try { + const port = await startDesktopBeacon(TEST_VERSION) + expect(port).toBeNull() + expect(_getBeaconPort()).toBeNull() + } finally { + for (const s of squatters) { + await new Promise((resolve) => s.close(() => resolve())) + } + } + }) + + it('a non-allowed Origin gets the body but no CORS header (so the browser blocks the read)', async () => { + const port = await startDesktopBeacon(TEST_VERSION) + const res = await fetchPing(port!, { origin: 'https://evil.com' }) + expect(res.status).toBe(200) + expect(res.headers['access-control-allow-origin']).toBeUndefined() + }) + + it('unknown paths 404 without leaking CORS headers', async () => { + const port = await startDesktopBeacon(TEST_VERSION) + const res = await fetchPing(port!, { origin: BEACON_ALLOWED_ORIGIN, path: '/admin' }) + expect(res.status).toBe(404) + expect(res.headers['access-control-allow-origin']).toBeUndefined() + }) + + it('refuses non-GET methods on /ping (locks down probing)', async () => { + const port = await startDesktopBeacon(TEST_VERSION) + const res = await fetchPing(port!, { origin: BEACON_ALLOWED_ORIGIN, method: 'POST' }) + expect(res.status).toBe(404) + expect(res.headers['access-control-allow-origin']).toBeUndefined() + }) +}) + +describe('stopDesktopBeacon', () => { + beforeEach(async () => { + await stopDesktopBeacon() + }) + + it('is a safe no-op when the beacon was never started', async () => { + await expect(stopDesktopBeacon()).resolves.toBeUndefined() + }) + + it('closes the server and clears state so a subsequent start re-binds', async () => { + const port = await startDesktopBeacon(TEST_VERSION) + expect(port).not.toBeNull() + await stopDesktopBeacon() + expect(_getBeaconPort()).toBeNull() + const port2 = await startDesktopBeacon(TEST_VERSION) + expect(port2).not.toBeNull() + await stopDesktopBeacon() + }) +}) diff --git a/src/main/lib/desktopBeacon.ts b/src/main/lib/desktopBeacon.ts new file mode 100644 index 00000000..c672216f --- /dev/null +++ b/src/main/lib/desktopBeacon.ts @@ -0,0 +1,197 @@ +/** + * Tiny localhost HTTP server that lets `cloud.comfy.org` discover whether + * Comfy Desktop is currently running on this machine. The cloud web app + * pings `/ping`; a successful response is the signal it uses to (a) decide + * whether to forward share links into the app via `comfy://`, and (b) cache + * a "Desktop has been seen" hint in `localStorage` so subsequent visits can + * still hand off even when Desktop is closed. + * + * Design rules: + * - Bind to 127.0.0.1 only — never `0.0.0.0` or a hostname. A wrong bind + * would expose the endpoint to anyone on the local network. + * - CORS strictly locked to `https://cloud.comfy.org`. No `*`, no wildcard + * subdomains. Other origins receive no `Access-Control-Allow-Origin` + * header, so they cannot read the response body. + * - Single endpoint, single method: `GET /ping`. Everything else returns + * 404 with no CORS headers — cross-origin scripts can't probe shape. + * - Three-port allowlist with first-free-wins. If all three are taken + * we silently disable detection that session rather than scanning a + * wider range (which would slow startup and look like nmap traffic). + * + * What the response reveals: `{ app: 'comfy-desktop', version: '' }`. + * Intentional — that's exactly the bit the frontend needs and nothing more. + * Same info the About box happily prints. + */ + +import http from 'node:http' +import type { AddressInfo } from 'node:net' + +/** CORS allowlist. Production traffic only ever comes from cloud.comfy.org + * (and the test domain we deploy to before promoting). Local dev origins + * are included so ComfyUI_frontend developers running `pnpm dev:cloud` + * against a packaged Desktop install can verify the discovery flow + * end-to-end. Anything not in this set gets no `Access-Control-Allow-Origin` + * header and the browser blocks the read. */ +export const BEACON_ALLOWED_ORIGINS: ReadonlySet = new Set([ + 'https://cloud.comfy.org', + 'https://testcloud.comfy.org', + 'http://localhost:5173', + 'http://localhost:5174', +]) + +/** Back-compat: the production origin tests and other call sites have used. */ +export const BEACON_ALLOWED_ORIGIN = 'https://cloud.comfy.org' + +/** Three-port allowlist. Chosen in the 51000-52000 range, outside common app + * ports and outside the ephemeral range Windows reserves. If a future + * Desktop release needs to move ports, keep the previous entry alongside + * for a transition window so old frontend builds keep working. */ +export const BEACON_PORTS: readonly number[] = [51823, 51824, 51825] + +interface BeaconState { + server: http.Server + port: number +} + +let _state: BeaconState | null = null + +/** Build the `/ping` response body. Factored out so tests can lock the + * shape down — the frontend depends on `app === 'comfy-desktop'` to + * distinguish us from any other localhost service on the same port. */ +export function buildPingBody(version: string): string { + return JSON.stringify({ app: 'comfy-desktop', version }) +} + +/** Single request handler. Pure-ish (no IO outside the response object) so + * it's exercised directly by unit tests without bind/listen. */ +export function handleBeaconRequest( + req: http.IncomingMessage, + res: http.ServerResponse, + version: string, +): void { + const origin = req.headers['origin'] + const isAllowedOrigin = typeof origin === 'string' && BEACON_ALLOWED_ORIGINS.has(origin) + + // Explicit `Content-Length: 0` on empty responses is required: without it + // Node falls back to chunked / connection-close framing and Windows TCP + // can RST the socket before the client reads the status, surfacing as + // ECONNRESET on the frontend instead of a clean 404. + const empty = (status: number, extraHeaders?: http.OutgoingHttpHeaders): void => { + res.writeHead(status, { 'Content-Length': '0', ...extraHeaders }) + res.end() + } + + // Preflight: the frontend uses a simple GET with no custom headers, so a + // CORS preflight only fires when the browser thinks it needs one. Answer + // it for the allowed origin; reject everything else with a bare 404. + if (req.method === 'OPTIONS') { + if (isAllowedOrigin && req.url === '/ping') { + empty(204, { + 'Access-Control-Allow-Origin': origin as string, + 'Access-Control-Allow-Methods': 'GET', + 'Access-Control-Max-Age': '600', + Vary: 'Origin', + }) + return + } + empty(404) + return + } + + if (req.method !== 'GET' || req.url !== '/ping') { + empty(404) + return + } + + const body = buildPingBody(version) + const headers: http.OutgoingHttpHeaders = { + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(body).toString(), + 'Cache-Control': 'no-store', + Vary: 'Origin', + } + if (isAllowedOrigin) { + headers['Access-Control-Allow-Origin'] = origin as string + } + res.writeHead(200, headers) + res.end(body) +} + +/** Try the port allowlist in order. Resolves to the bound port number, or + * `null` if every port is taken (in which case the beacon silently disables + * for this session). Loopback-only — `host = '127.0.0.1'` is non-negotiable. */ +async function bindFirstFreePort( + server: http.Server, + ports: readonly number[], +): Promise { + for (const port of ports) { + const ok = await new Promise((resolve) => { + const onError = (err: NodeJS.ErrnoException): void => { + server.removeListener('error', onError) + if (err.code === 'EADDRINUSE' || err.code === 'EACCES') { + resolve(false) + return + } + // Unexpected error — log and skip this port. + console.warn('[beacon] unexpected bind error', { port, code: err.code }) + resolve(false) + } + server.once('error', onError) + server.listen(port, '127.0.0.1', () => { + server.removeListener('error', onError) + const addr = server.address() as AddressInfo | null + resolve(addr !== null && addr.port === port) + }) + }) + if (ok) return port + } + return null +} + +/** Start the beacon. Idempotent — repeated calls return without re-binding. + * No-op when every allowed port is taken; callers see that as "detection + * silently disabled" rather than a hard error. */ +export async function startDesktopBeacon(version: string): Promise { + if (_state !== null) return _state.port + + const server = http.createServer((req, res) => { + try { + handleBeaconRequest(req, res, version) + } catch (err) { + // The handler is small and synchronous, but harden against a future + // refactor regressing it into something throw-prone. + console.warn('[beacon] request handler threw', err) + if (!res.headersSent) res.writeHead(500) + res.end() + } + }) + + // Errors after a successful bind (e.g. EMFILE under file-descriptor + // pressure) shouldn't crash the main process. Log and continue. + server.on('error', (err) => { + console.warn('[beacon] runtime server error', err) + }) + + const port = await bindFirstFreePort(server, BEACON_PORTS) + if (port === null) { + return null + } + _state = { server, port } + return port +} + +/** Stop the beacon. Safe to call when never started — used as an + * unconditional teardown in `before-quit`. */ +export async function stopDesktopBeacon(): Promise { + const state = _state + if (state === null) return + _state = null + await new Promise((resolve) => { + state.server.close(() => resolve()) + }) +} + +/** Test-only: bound port for assertions. Returns `null` when not running. */ +export function _getBeaconPort(): number | null { + return _state?.port ?? null +} diff --git a/todesktop.json b/todesktop.json index d32a451a..292c5886 100644 --- a/todesktop.json +++ b/todesktop.json @@ -35,7 +35,15 @@ }, "mac": { "icon": "./assets/Comfy_Logo_mac_x1024.png", - "category": "public.app-category.graphics-design" + "category": "public.app-category.graphics-design", + "extendInfo": { + "CFBundleURLTypes": [ + { + "CFBundleURLName": "org.comfy.deeplink", + "CFBundleURLSchemes": ["comfy"] + } + ] + } }, "dmg": { "title": "Comfy Desktop",