From a043880e14691d1bb18c735b46c202018b1d94dd Mon Sep 17 00:00:00 2001 From: Benjamin Lu Date: Thu, 11 Jun 2026 15:29:15 -0700 Subject: [PATCH 1/3] Expose ComfyUI telemetry bridge --- src/main/lib/ipc/registerTelemetryHandlers.ts | 28 ++++++++++++----- src/main/lib/telemetry.test.ts | 12 ++++++++ src/main/lib/telemetry.ts | 16 ++++++---- src/preload/comfyPreload.ts | 30 ++++++++++++------- 4 files changed, 64 insertions(+), 22 deletions(-) diff --git a/src/main/lib/ipc/registerTelemetryHandlers.ts b/src/main/lib/ipc/registerTelemetryHandlers.ts index a142ddd1..2ed6c8b0 100644 --- a/src/main/lib/ipc/registerTelemetryHandlers.ts +++ b/src/main/lib/ipc/registerTelemetryHandlers.ts @@ -34,17 +34,31 @@ function isTelemetryValue(v: unknown): v is mainTelemetry.TelemetryValue { ) } -// Per-key filter to the TelemetryValue contract; renderer payloads cross a -// trust boundary, so non-primitives (incl. arrays) are dropped per-key. -function asProps(value: unknown): Record { +function isTelemetryValueArray(v: unknown): v is mainTelemetry.TelemetryValue[] { + return Array.isArray(v) && v.every(isTelemetryValue) +} + +// Per-key filter to the TelemetryContext contract. +function asProps(value: unknown): mainTelemetry.TelemetryContext { if (!value || typeof value !== 'object' || Array.isArray(value)) return {} - const out: Record = {} + const out: mainTelemetry.TelemetryContext = {} for (const [key, raw] of Object.entries(value)) { if (typeof key !== 'string') continue if (isTelemetryValue(raw)) { out[key] = raw + } else if (isTelemetryValueArray(raw)) { + out[key] = raw } - // Drop anything else (objects, arrays, functions, symbols, etc.) silently. + } + return out +} + +function asPersonProps(value: unknown): Record { + if (!value || typeof value !== 'object' || Array.isArray(value)) return {} + const out: Record = {} + for (const [key, raw] of Object.entries(value)) { + if (typeof key !== 'string') continue + if (isTelemetryValue(raw)) out[key] = raw } return out } @@ -65,7 +79,7 @@ export function registerTelemetryHandlers(): void { }) ipcMain.on('telemetry:registerProperties', (_event, properties: unknown) => { - const props = asProps(properties) + const props = asPersonProps(properties) if (Object.keys(props).length === 0) return mainTelemetry.registerPersonProperties(props) }) @@ -76,7 +90,7 @@ export function registerTelemetryHandlers(): void { if (!payload || typeof payload !== 'object') return const userId = asString((payload as Record).userId) if (!userId) return - const properties = asProps((payload as Record).properties) + const properties = asPersonProps((payload as Record).properties) mainTelemetry.bindUserId(userId, properties) }) diff --git a/src/main/lib/telemetry.test.ts b/src/main/lib/telemetry.test.ts index 1f1e54cd..e4d98abe 100644 --- a/src/main/lib/telemetry.test.ts +++ b/src/main/lib/telemetry.test.ts @@ -366,6 +366,18 @@ describe('telemetry SDK-level privacy safety nets', () => { expect(msg).not.toContain('64911') }) + it('scrubs string array entries as a last-resort safety net', () => { + captured.length = 0 + telemetry.capture('comfy.desktop.execution.error', { + model_paths: ['C:\\Users\\64911\\Documents\\model.safetensors', 'LoadImage'] + }) + expect(captured).toHaveLength(1) + const paths = captured[0]!.properties?.model_paths as string[] + expect(paths[0]).toContain('[REDACTED]') + expect(paths[0]).not.toContain('64911') + expect(paths[1]).toBe('LoadImage') + }) + it('leaves non-string property types untouched', () => { captured.length = 0 telemetry.capture('comfy.desktop.execution.completed', { diff --git a/src/main/lib/telemetry.ts b/src/main/lib/telemetry.ts index 634edeec..3cd52e7a 100644 --- a/src/main/lib/telemetry.ts +++ b/src/main/lib/telemetry.ts @@ -700,11 +700,17 @@ function scrubProperties(properties: TelemetryContext): TelemetryContext { let mutated: TelemetryContext | null = null for (const key of Object.keys(properties)) { const value = properties[key] - if (typeof value !== 'string') continue - const cleaned = scrubAll(value) - if (cleaned === value) continue - if (!mutated) mutated = { ...properties } - mutated[key] = cleaned + if (typeof value === 'string') { + const cleaned = scrubAll(value) + if (cleaned === value) continue + if (!mutated) mutated = { ...properties } + mutated[key] = cleaned + } else if (Array.isArray(value)) { + const cleaned = value.map((entry) => (typeof entry === 'string' ? scrubAll(entry) : entry)) + if (cleaned.every((entry, index) => entry === value[index])) continue + if (!mutated) mutated = { ...properties } + mutated[key] = cleaned + } } return mutated ?? properties } diff --git a/src/preload/comfyPreload.ts b/src/preload/comfyPreload.ts index 5baa0490..f8dbc9d0 100644 --- a/src/preload/comfyPreload.ts +++ b/src/preload/comfyPreload.ts @@ -49,8 +49,7 @@ const Terminal = { * the inline injection doesn't need to know its own ID. */ openPopout: (): Promise => ipcRenderer.invoke('terminal-popout-open', null), onOutput: (callback: (data: string) => void): (() => void) => { - const handler = (_event: IpcRendererEvent, payload: { data: string }) => - callback(payload.data) + const handler = (_event: IpcRendererEvent, payload: { data: string }) => callback(payload.data) ipcRenderer.on('terminal-output', handler) return () => ipcRenderer.removeListener('terminal-output', handler) }, @@ -58,7 +57,7 @@ const Terminal = { const handler = () => callback() ipcRenderer.on('terminal-exited', handler) return () => ipcRenderer.removeListener('terminal-exited', handler) - }, + } } export interface LogsRestore { @@ -71,6 +70,9 @@ export interface LogsOutputMsg { text: string } +type TelemetryPrimitive = string | number | boolean | null | undefined +type TelemetryProperties = Record + /** * Read-only logs bridge. Subscribes to the shared per-install log * broadcast that mirrors every `comfy-output` IPC send. Used by the @@ -90,11 +92,16 @@ const Logs = { * so the inline injection doesn't need to know its own ID. */ openPopout: (): Promise => ipcRenderer.invoke('logs-popout-open', null), onOutput: (callback: (msg: LogsOutputMsg) => void): (() => void) => { - const handler = (_event: IpcRendererEvent, payload: LogsOutputMsg) => - callback(payload) + const handler = (_event: IpcRendererEvent, payload: LogsOutputMsg) => callback(payload) ipcRenderer.on('logs-output', handler) return () => ipcRenderer.removeListener('logs-output', handler) - }, + } +} + +const Telemetry = { + capture: (event: string, properties?: TelemetryProperties): void => { + ipcRenderer.send('telemetry:capture', { event, properties }) + } } contextBridge.exposeInMainWorld('__comfyDesktop2', { @@ -102,7 +109,11 @@ contextBridge.exposeInMainWorld('__comfyDesktop2', { return ipcRenderer.invoke('desktop2-download-model', { url, filename, directory }) }, downloadAsset: (url: string, filename: string, authToken?: string): Promise => { - return ipcRenderer.invoke('desktop2-download-asset', { url, filename, authToken: authToken || undefined }) + return ipcRenderer.invoke('desktop2-download-asset', { + url, + filename, + authToken: authToken || undefined + }) }, pauseDownload: (url: string): Promise => { return ipcRenderer.invoke('model-download-pause', { url }) @@ -113,9 +124,7 @@ contextBridge.exposeInMainWorld('__comfyDesktop2', { cancelDownload: (url: string): Promise => { return ipcRenderer.invoke('model-download-cancel', { url }) }, - onDownloadProgress: ( - callback: (data: ComfyDownloadProgress) => void - ): (() => void) => { + onDownloadProgress: (callback: (data: ComfyDownloadProgress) => void): (() => void) => { const handler = (_event: IpcRendererEvent, data: unknown) => callback(data as ComfyDownloadProgress) ipcRenderer.on('desktop2-download-progress', handler) @@ -126,4 +135,5 @@ contextBridge.exposeInMainWorld('__comfyDesktop2', { }, Terminal, Logs, + Telemetry }) From e1fab8a56d0ab3e1d9bac57a808230c4d9ca51d4 Mon Sep 17 00:00:00 2001 From: Benjamin Lu Date: Thu, 11 Jun 2026 18:13:48 -0700 Subject: [PATCH 2/3] Narrow ComfyUI telemetry bridge values --- src/preload/comfyPreload.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/preload/comfyPreload.ts b/src/preload/comfyPreload.ts index f8dbc9d0..9cd56106 100644 --- a/src/preload/comfyPreload.ts +++ b/src/preload/comfyPreload.ts @@ -70,7 +70,7 @@ export interface LogsOutputMsg { text: string } -type TelemetryPrimitive = string | number | boolean | null | undefined +type TelemetryPrimitive = string | number | boolean | null type TelemetryProperties = Record /** From db68fabe1da0817ca14ee38f61912087d8fb0812 Mon Sep 17 00:00:00 2001 From: Deep Mehta Date: Fri, 12 Jun 2026 13:35:13 -0700 Subject: [PATCH 3/3] fix(preload): guard Telemetry.capture send to keep the bridge fire-and-forget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ipcRenderer.send` throws synchronously on non-structured-cloneable payloads (functions, DOM nodes, symbols, circular refs). Since the new `__comfyDesktop2.Telemetry.capture` surface is exposed to the hosted ComfyUI frontend — code we don't fully control — a stray bad payload would otherwise propagate into unrelated frontend code and break it. Mirror the same try/catch contract already used by `window.api.captureTelemetry` in `src/preload/api.ts` (verbatim comment lifted: "telemetry must never break the renderer"). Codex flagged this as P2; agreeing because the contract is established elsewhere in the same repo and the new bridge regressed it. --- src/preload/comfyPreload.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/preload/comfyPreload.ts b/src/preload/comfyPreload.ts index 9cd56106..31d4661e 100644 --- a/src/preload/comfyPreload.ts +++ b/src/preload/comfyPreload.ts @@ -100,7 +100,18 @@ const Logs = { const Telemetry = { capture: (event: string, properties?: TelemetryProperties): void => { - ipcRenderer.send('telemetry:capture', { event, properties }) + // Telemetry must never break the caller. `ipcRenderer.send` throws + // synchronously on non-structured-cloneable values (functions, DOM + // nodes, symbols, circular refs); since this surface is exposed to + // the hosted ComfyUI frontend, a stray bad payload would otherwise + // propagate into unrelated frontend code. Mirrors the same + // try/catch contract as `window.api.captureTelemetry` in + // `src/preload/api.ts`. + try { + ipcRenderer.send('telemetry:capture', { event, properties }) + } catch { + // ignore: telemetry must never break the renderer + } } }