From 25a5e37075f40449339ac15d7a43f72ec2e287ee Mon Sep 17 00:00:00 2001 From: Alberto Vona Date: Mon, 11 May 2026 02:24:22 +0200 Subject: [PATCH] fix: resolve TypeError and unintended link opening on right-click - Fixes the TypeError 'conversion failure from null' when right-clicking UI elements (like settings tabs) without an href attribute. - Restricts the auxclick listener to middle-clicks only to restore standard right-click context menu behavior. - Scopes preventDefault to only handle successful link interceptions, preserving autoscroll on non-link elements. - Adds protocol validation to openExternal IPC handler to prevent RCE vulnerabilities. - Updates LinkProvider types to explicitly handle null/undefined values for better type safety. - Adds unit tests for LinkImpl to prevent regressions. --- src-electron/ipc/electron-hook.ts | 27 ++++++++++++--- src/App.vue | 10 ++++-- src/providers/components/LinkProvider.ts | 4 +-- src/r2mm/component_override/LinkImpl.ts | 12 ++++--- test/vitest/tests/unit/LinkImpl.spec.ts | 44 ++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 13 deletions(-) create mode 100644 test/vitest/tests/unit/LinkImpl.spec.ts diff --git a/src-electron/ipc/electron-hook.ts b/src-electron/ipc/electron-hook.ts index 28af58f81..f7a27cd69 100644 --- a/src-electron/ipc/electron-hook.ts +++ b/src-electron/ipc/electron-hook.ts @@ -2,20 +2,37 @@ import { BrowserWindow, ipcMain, shell, clipboard } from 'electron'; export function hookElectronIpc(browserWindow: BrowserWindow) { ipcMain.on('electron:shell:openExternal', (event, url) => { - shell.openExternal(url) + if (typeof url === 'string') { + try { + const parsedUrl = new URL(url); + if (['http:', 'https:'].includes(parsedUrl.protocol)) { + shell.openExternal(url); + } + } catch (e) { + // Silently ignore malformed URLs + } + } }); ipcMain.on('electron:shell:selectFile', (event, filePath) => { - shell.showItemInFolder(filePath) + if (typeof filePath === 'string') { + shell.showItemInFolder(filePath); + } }); ipcMain.on('electron:shell:openPath', (event, filePath) => { - shell.openPath(filePath) + if (typeof filePath === 'string') { + shell.openPath(filePath); + } }); ipcMain.on('electron:clipboard:copyText', (event, text) => { - clipboard.writeText(text); - event.returnValue = true; + if (typeof text === 'string') { + clipboard.writeText(text); + event.returnValue = true; + } else { + event.returnValue = false; + } }); ipcMain.handle('electron:getEnvironmentVariables', (event) => { diff --git a/src/App.vue b/src/App.vue index d5049e349..bbaa64ce9 100644 --- a/src/App.vue +++ b/src/App.vue @@ -67,11 +67,17 @@ provideStoreImplementation(() => store); const quasar = useQuasar(); document.addEventListener('auxclick', e => { + if (e.button !== 1) { + return; + } const target = e.target! as any; if (target.localName == 'a') { - LinkProvider.instance.openLink(target.getAttribute("href")) + const href = target.getAttribute("href"); + if (href !== null && href !== undefined) { + LinkProvider.instance.openLink(href); + } + e.preventDefault(); } - e.preventDefault(); }, false); const { diff --git a/src/providers/components/LinkProvider.ts b/src/providers/components/LinkProvider.ts index 3e263d774..303734af2 100644 --- a/src/providers/components/LinkProvider.ts +++ b/src/providers/components/LinkProvider.ts @@ -14,8 +14,8 @@ export default abstract class LinkProvider { return LinkProvider.provider(); } - public abstract openLink(url: string): void; + public abstract openLink(url: string | null | undefined): void; - public abstract selectFile(url: string): void; + public abstract selectFile(url: string | null | undefined): void; } diff --git a/src/r2mm/component_override/LinkImpl.ts b/src/r2mm/component_override/LinkImpl.ts index d47469481..317ef22e0 100644 --- a/src/r2mm/component_override/LinkImpl.ts +++ b/src/r2mm/component_override/LinkImpl.ts @@ -2,12 +2,16 @@ import LinkProvider from '../../providers/components/LinkProvider'; export default class LinkImpl extends LinkProvider { - openLink(url: string): void { - window.electron.openExternal(url); + openLink(url: string | null | undefined): void { + if (url !== null && url !== undefined) { + window.electron.openExternal(url); + } } - selectFile(url: string): void { - window.electron.selectFile(url); + selectFile(url: string | null | undefined): void { + if (url !== null && url !== undefined) { + window.electron.selectFile(url); + } } } diff --git a/test/vitest/tests/unit/LinkImpl.spec.ts b/test/vitest/tests/unit/LinkImpl.spec.ts new file mode 100644 index 000000000..888fac775 --- /dev/null +++ b/test/vitest/tests/unit/LinkImpl.spec.ts @@ -0,0 +1,44 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import LinkImpl from '../../../../src/r2mm/component_override/LinkImpl'; + +// Mock window.electron +(global as any).window = { + electron: { + openExternal: vi.fn(), + selectFile: vi.fn(), + } +}; + +describe('LinkImpl', () => { + let linkImpl: LinkImpl; + + beforeEach(() => { + linkImpl = new LinkImpl(); + vi.clearAllMocks(); + }); + + it('should call openExternal when url is valid', () => { + linkImpl.openLink('https://example.com'); + expect(window.electron.openExternal).toHaveBeenCalledWith('https://example.com'); + }); + + it('should NOT call openExternal when url is null', () => { + linkImpl.openLink(null as any); + expect(window.electron.openExternal).not.toHaveBeenCalled(); + }); + + it('should NOT call openExternal when url is undefined', () => { + linkImpl.openLink(undefined as any); + expect(window.electron.openExternal).not.toHaveBeenCalled(); + }); + + it('should call selectFile when url is valid', () => { + linkImpl.selectFile('path/to/file'); + expect(window.electron.selectFile).toHaveBeenCalledWith('path/to/file'); + }); + + it('should NOT call selectFile when url is null', () => { + linkImpl.selectFile(null as any); + expect(window.electron.selectFile).not.toHaveBeenCalled(); + }); +});