From 6317af8eb828f7fda6e8c3b0f4cc53f6ba4572d0 Mon Sep 17 00:00:00 2001 From: woodsonl <65194841+woodsonl@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:37:46 -0500 Subject: [PATCH] fix: only accept the dev renderer URL when one is configured isKnownSender treated any URL as known in a packaged build: ELECTRON_RENDERER_URL is unset there, so process.env.ELECTRON_RENDERER_URL ?? '' yielded the empty string and url.startsWith('') is always true. The sender check then admitted any frame, not just the dev renderer. Require a configured dev URL and match it exactly (or as a path prefix), so the check admits the dev server origin and nothing else. A trailing slash on the configured URL is stripped first, otherwise the '' + '/' prefix doubles up and rejects the dev server's own query-string loads. Add coverage for the unset, exact-match, trailing-slash, and unrelated-URL cases. Signed-off-by: woodsonl <65194841+woodsonl@users.noreply.github.com> --- desktop/src/electron/ipc/safe-handle.ts | 7 +- .../tests/modular/safe-handle-sender.test.ts | 148 ++++++++++++++++++ 2 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 desktop/tests/modular/safe-handle-sender.test.ts diff --git a/desktop/src/electron/ipc/safe-handle.ts b/desktop/src/electron/ipc/safe-handle.ts index b96d72b9..f460bd1f 100644 --- a/desktop/src/electron/ipc/safe-handle.ts +++ b/desktop/src/electron/ipc/safe-handle.ts @@ -16,7 +16,12 @@ function isKnownSender(event: IpcMainInvokeEvent): boolean { const url = event.sender.getURL() if (url.startsWith('file://')) return true - if (url.startsWith(process.env.ELECTRON_RENDERER_URL ?? '')) return true + const raw = process.env.ELECTRON_RENDERER_URL + // A trailing slash would make devUrl + '/' double up and reject the dev + // server's own query-string loads (window.ts appends '?window=tray'). + const devUrl = raw?.replace(/\/$/, '') + if (devUrl !== undefined && devUrl !== '' && (url === devUrl || url.startsWith(devUrl + '/'))) + return true return false } diff --git a/desktop/tests/modular/safe-handle-sender.test.ts b/desktop/tests/modular/safe-handle-sender.test.ts new file mode 100644 index 00000000..58153ade --- /dev/null +++ b/desktop/tests/modular/safe-handle-sender.test.ts @@ -0,0 +1,148 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, describe, expect, it, vi } from 'vitest' + +interface SenderFixture { + url: string + window: object | null + getURL: () => string +} + +const mocks = vi.hoisted(() => ({ + sender: null as SenderFixture | null, + handlers: new Map unknown>() +})) + +vi.mock('electron', () => ({ + BrowserWindow: { + fromWebContents: (contents: SenderFixture | null) => (contents ? contents.window : null) + }, + ipcMain: { + handle: (channel: string, fn: (event: unknown, ...args: unknown[]) => unknown) => { + mocks.handlers.set(channel, fn) + } + } +})) + +import { safeHandle } from '@/electron/ipc/safe-handle' + +function senderWith(url: string, withWindow = true): { sender: SenderFixture } { + mocks.sender = { url, window: withWindow ? {} : null, getURL: () => url } + return { sender: mocks.sender } +} + +function invoke(channel: string, url: string, withWindow = true) { + const handler = mocks.handlers.get(channel) + if (!handler) throw new Error(`no handler registered for ${channel}`) + const ev = senderWith(url, withWindow).sender + return handler({ sender: ev } as never) +} + +const previousDevUrl = process.env.ELECTRON_RENDERER_URL + +afterEach(() => { + mocks.handlers.clear() + mocks.sender = null + if (previousDevUrl === undefined) { + delete process.env.ELECTRON_RENDERER_URL + } else { + process.env.ELECTRON_RENDERER_URL = previousDevUrl + } +}) + +describe('safeHandle sender authorization', () => { + it('registers the handler and authorizes a sender on the dev URL', async () => { + process.env.ELECTRON_RENDERER_URL = 'http://localhost:5173' + safeHandle('test:sender-ok' as never, (() => 'ok') as never) + await expect(invoke('test:sender-ok', 'http://localhost:5173/app')).resolves.toEqual({ + success: true, + data: 'ok' + }) + }) + + it('authorizes a file:// sender regardless of the dev URL', async () => { + delete process.env.ELECTRON_RENDERER_URL + safeHandle('test:sender-file' as never, (() => 'ok') as never) + await expect(invoke('test:sender-file', 'file://app/index.html')).resolves.toEqual({ + success: true, + data: 'ok' + }) + }) + + it('rejects a sender when ELECTRON_RENDERER_URL is unset', async () => { + delete process.env.ELECTRON_RENDERER_URL + safeHandle('test:sender-nor-dev' as never, (() => 'ok') as never) + await expect(invoke('test:sender-nor-dev', 'http://localhost:5173/app')).resolves.toEqual({ + success: false, + error: 'Unauthorized sender' + }) + }) + + it('rejects a sender when ELECTRON_RENDERER_URL is set to an empty string', async () => { + process.env.ELECTRON_RENDERER_URL = '' + safeHandle('test:sender-empty-dev' as never, (() => 'ok') as never) + await expect(invoke('test:sender-empty-dev', 'http://anything.test/app')).resolves.toEqual({ + success: false, + error: 'Unauthorized sender' + }) + }) + + it('authorizes an exact dev-URL match with no path', async () => { + process.env.ELECTRON_RENDERER_URL = 'http://localhost:5173' + safeHandle('test:sender-exact' as never, (() => 'ok') as never) + await expect(invoke('test:sender-exact', 'http://localhost:5173')).resolves.toEqual({ + success: true, + data: 'ok' + }) + }) + + it('authorizes a dev-URL sender when the configured URL has a trailing slash', async () => { + process.env.ELECTRON_RENDERER_URL = 'http://localhost:5173/' + safeHandle('test:sender-slash' as never, (() => 'ok') as never) + await expect( + invoke('test:sender-slash', 'http://localhost:5173/?window=tray') + ).resolves.toEqual({ + success: true, + data: 'ok' + }) + }) + + it('rejects a sender whose URL only shares a prefix with the dev URL (lookalike host)', async () => { + process.env.ELECTRON_RENDERER_URL = 'http://localhost:5173' + safeHandle('test:sender-lookalike' as never, (() => 'ok') as never) + await expect( + invoke('test:sender-lookalike', 'http://localhost:5173.evil.test/app') + ).resolves.toEqual({ + success: false, + error: 'Unauthorized sender' + }) + }) + + it('rejects a sender with dev-URL userinfo spoofing', async () => { + process.env.ELECTRON_RENDERER_URL = 'http://localhost:5173' + safeHandle('test:sender-userinfo' as never, (() => 'ok') as never) + await expect( + invoke('test:sender-userinfo', 'http://localhost:5173@evil.test/') + ).resolves.toEqual({ + success: false, + error: 'Unauthorized sender' + }) + }) + + it('rejects a sender with no attached BrowserWindow', async () => { + process.env.ELECTRON_RENDERER_URL = 'http://localhost:5173' + safeHandle('test:sender-nowin' as never, (() => 'ok') as never) + const handler = mocks.handlers.get('test:sender-nowin') + if (!handler) throw new Error('handler missing') + mocks.sender = { + url: 'http://localhost:5173/app', + window: null, + getURL: () => 'http://localhost:5173/app' + } + await expect(handler({ sender: mocks.sender } as never)).resolves.toEqual({ + success: false, + error: 'Unauthorized sender' + }) + }) +})