From e781d30842e9a3beba3fe1131b8f62ee2258f89e Mon Sep 17 00:00:00 2001 From: Brad Groux <3053586+BradGroux@users.noreply.github.com> Date: Sun, 12 Jul 2026 16:18:42 -0500 Subject: [PATCH] fix: externalize Electron runtime in desktop builds (#817) Fixes #809 --- CHANGELOG.md | 5 + desktop/README.md | 11 +- desktop/electron.vite.config.ts | 22 ++++ desktop/package.json | 2 +- desktop/src/main/index.ts | 48 +++++-- docs/DESKTOP-RELEASE.md | 3 + package.json | 1 + scripts/check-desktop-electron-artifacts.mjs | 117 ++++++++++++++++++ .../check-desktop-electron-artifacts.test.mjs | 34 +++++ 9 files changed, 225 insertions(+), 18 deletions(-) create mode 100644 scripts/check-desktop-electron-artifacts.mjs create mode 100644 scripts/check-desktop-electron-artifacts.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 83d7bbb5..e9d9a969 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Kept Electron's runtime API external in Vite 8/Rolldown desktop builds, + preventing the npm executable-path shim from replacing `app`, + `BrowserWindow`, `contextBridge`, and related native APIs in emitted main and + preload artifacts; desktop builds now fail closed if the shim reappears + (#809). - Isolated QMD result normalization coverage from unrelated persistent search collections and restored test environment state only after temporary search roots are removed, eliminating the intermittent teardown race (#793). diff --git a/desktop/README.md b/desktop/README.md index 9b2beba9..5e0a4e39 100644 --- a/desktop/README.md +++ b/desktop/README.md @@ -93,7 +93,10 @@ release guide are promoted. ## Production Scaffold `pnpm desktop:build` compiles the Electron main, preload, and fallback renderer. -Packaging, signing, notarization, updater metadata, and bundled server/web asset -layout are handled by later v5 desktop issues. Packaged mode expects a built -server entry at `server/dist/index.js` unless `VERITAS_DESKTOP_SERVER_ENTRY` is -provided. +The build then verifies that main and preload import Electron from the native +runtime and do not contain Electron's npm installer shim. Run the artifact +check directly with `pnpm desktop:check:electron-artifacts` when inspecting an +existing build. Packaging, signing, notarization, updater metadata, and bundled +server/web asset layout follow the release workflow documented above. Packaged +mode expects a built server entry at `server/dist/index.js` unless +`VERITAS_DESKTOP_SERVER_ENTRY` is provided. diff --git a/desktop/electron.vite.config.ts b/desktop/electron.vite.config.ts index d5a453b5..c32b8935 100644 --- a/desktop/electron.vite.config.ts +++ b/desktop/electron.vite.config.ts @@ -1,10 +1,22 @@ import { resolve } from 'node:path'; import { defineConfig, externalizeDepsPlugin } from 'electron-vite'; +const electronRuntimeExternal = ['electron', /^electron\/.+/]; + export default defineConfig({ main: { plugins: [externalizeDepsPlugin()], build: { + // Vite 8 builds with Rolldown. Electron Vite 5 still places its built-in + // runtime externals under rollupOptions, which Rolldown does not consume. + // Keep Electron explicitly external so the emitted main process receives + // Electron's runtime API instead of bundling the npm executable-path shim. + rolldownOptions: { + external: electronRuntimeExternal, + input: { + index: resolve(__dirname, 'src/main/index.ts'), + }, + }, rollupOptions: { input: { index: resolve(__dirname, 'src/main/index.ts'), @@ -15,6 +27,16 @@ export default defineConfig({ preload: { plugins: [externalizeDepsPlugin()], build: { + rolldownOptions: { + external: electronRuntimeExternal, + input: { + index: resolve(__dirname, 'src/preload/index.ts'), + }, + output: { + format: 'cjs', + entryFileNames: '[name].cjs', + }, + }, rollupOptions: { input: { index: resolve(__dirname, 'src/preload/index.ts'), diff --git a/desktop/package.json b/desktop/package.json index 26c92760..bf1c5fcb 100644 --- a/desktop/package.json +++ b/desktop/package.json @@ -12,7 +12,7 @@ "scripts": { "dev": "electron-vite dev", "dev:fresh": "VERITAS_DESKTOP_PROFILE=fresh electron-vite dev", - "build": "electron-vite build", + "build": "electron-vite build && pnpm --dir .. desktop:check:electron-artifacts", "package:prepare": "node ../scripts/prepare-desktop-release.mjs", "package:mac:dir": "node ../scripts/prepare-desktop-release.mjs && node ../scripts/run-desktop-builder.mjs --mac dir --publish never --config.mac.identity=null --config.mac.notarize=false", "package:mac:unsigned": "node ../scripts/prepare-desktop-release.mjs && node ../scripts/run-desktop-builder.mjs --mac dmg zip --publish never --config.mac.identity=null --config.mac.notarize=false", diff --git a/desktop/src/main/index.ts b/desktop/src/main/index.ts index b0b8cfb7..0961b78a 100644 --- a/desktop/src/main/index.ts +++ b/desktop/src/main/index.ts @@ -45,6 +45,24 @@ let quitting = false; let shutdownStarted = false; const pendingDeepLinks: string[] = []; +function activeMainWindow(): BrowserWindow | null { + if (!mainWindow || mainWindow.isDestroyed()) { + return null; + } + return mainWindow; +} + +function showDesktopError(message: string): void { + if (quitting) return; + + const window = activeMainWindow(); + if (!window) return; + + void window + .loadURL(statusPageUrl('Veritas Kanban desktop error', message, runtime?.snapshot())) + .catch(() => undefined); +} + function isPackagedRuntime(): boolean { return app.isPackaged || process.env.VERITAS_DESKTOP_PRODUCTION === 'true'; } @@ -99,6 +117,11 @@ function createMainWindow(savedState: DesktopWindowState): BrowserWindow { writeDesktopWindowStateSync(windowStatePaths, captureDesktopWindowState(window)); } }); + window.on('closed', () => { + if (mainWindow === window) { + mainWindow = null; + } + }); window.webContents.setWindowOpenHandler(({ url }) => { void openValidatedExternalUrl(shell, url); return { action: 'deny' }; @@ -131,7 +154,7 @@ function handleDeepLink(url: string): void { const deepLink = parseDesktopDeepLink(url); void commandDispatcher.dispatch(deepLink.command); } catch (error) { - mainWindow?.webContents.send(DESKTOP_BRIDGE_EVENTS.communicationCheck.channel, { + activeMainWindow()?.webContents.send(DESKTOP_BRIDGE_EVENTS.communicationCheck.channel, { target: 'external', state: 'failed', detail: error instanceof Error ? error.message : String(error), @@ -249,7 +272,7 @@ async function boot(): Promise { ), forceDevUpdateConfig: process.env.VERITAS_DESKTOP_UPDATER_FORCE_DEV === 'true', emitStatus: (status) => { - mainWindow?.webContents.send(DESKTOP_BRIDGE_EVENTS.updateStatus.channel, status); + activeMainWindow()?.webContents.send(DESKTOP_BRIDGE_EVENTS.updateStatus.channel, status); refreshDesktopMenu(); }, }); @@ -259,7 +282,7 @@ async function boot(): Promise { shell, quit: () => app.quit(), sendRendererCommand: (command) => { - mainWindow?.webContents.send(DESKTOP_BRIDGE_EVENTS.menuCommand.channel, command); + activeMainWindow()?.webContents.send(DESKTOP_BRIDGE_EVENTS.menuCommand.channel, command); }, checkForUpdates: () => updateService?.checkForUpdates() ?? Promise.resolve(updateServiceFallback(packaged)), @@ -283,20 +306,21 @@ async function boot(): Promise { registerDesktopBridge(ipcMain, runtime, shell, packaged, commandDispatcher, updateService, { toggleMaximize: () => { - if (!mainWindow) { + const window = activeMainWindow(); + if (!window) { return { maximized: false }; } - if (mainWindow.isMaximized()) { - mainWindow.unmaximize(); + if (window.isMaximized()) { + window.unmaximize(); } else { - mainWindow.maximize(); + window.maximize(); } - return { maximized: mainWindow.isMaximized() }; + return { maximized: window.isMaximized() }; }, }); refreshDesktopMenu(); runtime.on('status', (status) => { - mainWindow?.webContents.send(DESKTOP_BRIDGE_EVENTS.serverStatus.channel, status); + activeMainWindow()?.webContents.send(DESKTOP_BRIDGE_EVENTS.serverStatus.channel, status); refreshDesktopMenu(); }); @@ -359,12 +383,10 @@ app.on('activate', () => { }); process.on('uncaughtException', (error) => { - mainWindow?.loadURL( - statusPageUrl('Veritas Kanban desktop error', error.message, runtime?.snapshot()) - ); + showDesktopError(error.message); }); process.on('unhandledRejection', (reason) => { const message = reason instanceof Error ? reason.message : String(reason); - mainWindow?.loadURL(statusPageUrl('Veritas Kanban desktop error', message, runtime?.snapshot())); + showDesktopError(message); }); diff --git a/docs/DESKTOP-RELEASE.md b/docs/DESKTOP-RELEASE.md index e5775938..bc30b8fc 100644 --- a/docs/DESKTOP-RELEASE.md +++ b/docs/DESKTOP-RELEASE.md @@ -169,6 +169,9 @@ policy is tracked in - Update `CHANGELOG.md`. - Run `pnpm typecheck`, `pnpm lint:budget`, `pnpm build`, and `pnpm test:unit`. +- Confirm `pnpm desktop:check:electron-artifacts` passes. The emitted main and + preload bundles must import Electron's runtime API and must not contain the + npm install/download shim. - Run `pnpm desktop:smoke:mac:local` to verify local packaging does not prune root dev tooling. - Run `pnpm desktop:package:mac:unsigned` and inspect artifact names. diff --git a/package.json b/package.json index 5b1e6bba..4f15de74 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "desktop:dev": "pnpm --filter @veritas-kanban/desktop dev", "desktop:dev:fresh": "pnpm --filter @veritas-kanban/desktop dev:fresh", "desktop:build": "pnpm --filter @veritas-kanban/desktop build", + "desktop:check:electron-artifacts": "node --test scripts/check-desktop-electron-artifacts.test.mjs && node scripts/check-desktop-electron-artifacts.mjs", "desktop:package:mac:dir": "pnpm build && pnpm --filter @veritas-kanban/desktop package:mac:dir", "desktop:package:mac:unsigned": "pnpm build && pnpm --filter @veritas-kanban/desktop package:mac:unsigned", "desktop:package:linux:unsigned": "pnpm build && pnpm --filter @veritas-kanban/desktop package:linux:unsigned", diff --git a/scripts/check-desktop-electron-artifacts.mjs b/scripts/check-desktop-electron-artifacts.mjs new file mode 100644 index 00000000..11a4cb10 --- /dev/null +++ b/scripts/check-desktop-electron-artifacts.mjs @@ -0,0 +1,117 @@ +#!/usr/bin/env node +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + +const forbiddenShimPatterns = [ + /Downloading Electron binary/, + /Electron failed to install correctly/, + /(?:^|[\\/])install\.js/, +]; + +const artifacts = [ + { + file: 'desktop/out/main/index.js', + requiredSymbols: ['app', 'BrowserWindow'], + }, + { + file: 'desktop/out/preload/index.cjs', + requiredSymbols: ['contextBridge', 'ipcRenderer'], + }, +]; + +function parseBindings(bindings) { + return new Set( + bindings + .split(',') + .map((binding) => + binding + .trim() + .split(/\s+as\s+/)[0] + ?.trim() + ) + .filter(Boolean) + ); +} + +function electronBindings(source) { + const namedImport = source.match(/\bimport\s*{([^}]+)}\s*from\s*["']electron["']/s); + const destructuredRequire = source.match( + /\b(?:const|let|var)\s*{([^}]+)}\s*=\s*require\(\s*["']electron["']\s*\)/s + ); + const namespaceRequire = source.match( + /\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*require\(\s*["']electron["']\s*\)/ + ); + + return { + named: namedImport + ? parseBindings(namedImport[1]) + : destructuredRequire + ? parseBindings(destructuredRequire[1]) + : new Set(), + namespace: namespaceRequire?.[1], + }; +} + +export function validateElectronArtifact(source, requiredSymbols) { + const errors = []; + + for (const pattern of forbiddenShimPatterns) { + if (pattern.test(source)) { + errors.push(`contains bundled Electron installer shim pattern ${pattern}`); + } + } + + const bindings = electronBindings(source); + if (bindings.named.size === 0 && !bindings.namespace) { + errors.push('does not import the runtime-provided Electron module'); + } + + for (const symbol of requiredSymbols) { + const namedBinding = bindings.named.has(symbol); + const namespaceBinding = + bindings.namespace && new RegExp(`\\b${bindings.namespace}\\.${symbol}\\b`).test(source); + if (!namedBinding && !namespaceBinding) { + errors.push(`does not bind required Electron API symbol ${symbol} from Electron`); + } + } + + return errors; +} + +async function main() { + let failed = false; + + for (const artifact of artifacts) { + const absolutePath = path.join(rootDir, artifact.file); + let source; + + try { + source = await readFile(absolutePath, 'utf8'); + } catch { + failed = true; + console.error(`Desktop Electron artifact check failed: ${artifact.file} is missing.`); + continue; + } + + const errors = validateElectronArtifact(source, artifact.requiredSymbols); + if (errors.length > 0) { + failed = true; + for (const error of errors) { + console.error(`Desktop Electron artifact check failed: ${artifact.file} ${error}.`); + } + } else { + console.log(`Desktop Electron artifact check passed: ${artifact.file}`); + } + } + + if (failed) { + process.exitCode = 1; + } +} + +if (path.resolve(process.argv[1] || '') === fileURLToPath(import.meta.url)) { + await main(); +} diff --git a/scripts/check-desktop-electron-artifacts.test.mjs b/scripts/check-desktop-electron-artifacts.test.mjs new file mode 100644 index 00000000..f1191e0c --- /dev/null +++ b/scripts/check-desktop-electron-artifacts.test.mjs @@ -0,0 +1,34 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { validateElectronArtifact } from './check-desktop-electron-artifacts.mjs'; + +test('accepts named Electron runtime API imports in the main artifact', () => { + const source = 'import { app, BrowserWindow, shell } from "electron"; app.whenReady();'; + + assert.deepEqual(validateElectronArtifact(source, ['app', 'BrowserWindow']), []); +}); + +test('accepts Electron namespace access in the CommonJS preload artifact', () => { + const source = + 'const electron = require("electron"); electron.contextBridge.exposeInMainWorld("api", {}); electron.ipcRenderer.send("ready");'; + + assert.deepEqual(validateElectronArtifact(source, ['contextBridge', 'ipcRenderer']), []); +}); + +test('rejects unrelated local symbols even when Electron is imported for side effects', () => { + const source = 'import "electron"; const app = {}; const BrowserWindow = class {};'; + const errors = validateElectronArtifact(source, ['app', 'BrowserWindow']); + + assert.ok(errors.some((error) => error.includes('does not import'))); + assert.ok(errors.some((error) => error.includes('app from Electron'))); + assert.ok(errors.some((error) => error.includes('BrowserWindow from Electron'))); +}); + +test('rejects the bundled Electron installer shim even with valid-looking bindings', () => { + const source = + 'import { app, BrowserWindow } from "electron"; console.log("Downloading Electron binary...");'; + const errors = validateElectronArtifact(source, ['app', 'BrowserWindow']); + + assert.ok(errors.some((error) => error.includes('installer shim'))); +});