fix(electron): code review hardening — 16 fixes for security, performance, robustness
## Critical Fixes - #1: Server readiness — waitForServer() polls before loading window - #2: Restart timeout — 5s + SIGKILL prevents IPC handler from hanging - #3: changePort — now stops/restarts server on new port ## Important Fixes - #4: Tray cleanup — destroy old Tray before recreating - #5: IPC emission — server-status & port-changed events - #6: Disposer pattern — replaces removeAllListeners - #7: useSyncExternalStore — eliminates 5x re-renders ## Minor: #8-#16 (dead code, CSP, platform titlebar, types, errors, version) Tests: 76 / 15 suites (was 64/9)
This commit is contained in:
diegosouzapw
@@ -7,6 +7,42 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
---
|
||||
|
||||
## [1.6.4] — 2026-02-28
|
||||
|
||||
### 🖥️ Electron Desktop — Code Review Hardening (16 Fixes)
|
||||
|
||||
#### 🔴 Critical
|
||||
|
||||
- **Server readiness** — Window now waits for server health check before loading URL; no more blank screens on cold start (#1)
|
||||
- **Restart timeout** — `restart-server` IPC handler now has 5s timeout + `SIGKILL` to prevent indefinite hangs (#2)
|
||||
- **Port change lifecycle** — `changePort()` now stops and restarts the server on the new port instead of just reloading the URL (#3)
|
||||
|
||||
#### 🟡 Important
|
||||
|
||||
- **Tray cleanup** — Old `Tray` instance is now destroyed before recreating, preventing duplicate icons and memory leaks (#4)
|
||||
- **IPC event emission** — Main process now emits `server-status` and `port-changed` events to renderer, making React hooks functional (#5)
|
||||
- **Listener accumulation** — Preload now returns disposer functions for precise listener cleanup instead of `removeAllListeners` (#6)
|
||||
- **useIsElectron performance** — Replaced `useState`+`useEffect` with `useSyncExternalStore` to eliminate 5x unnecessary re-renders (#7)
|
||||
|
||||
#### 🔵 Minor
|
||||
|
||||
- Removed dead `isProduction` variable (#8)
|
||||
- Platform-conditional `titleBarStyle` — `hiddenInset` only on macOS, `default` on Windows/Linux (#9)
|
||||
- `stdio: pipe` — Server output captured for logging and readiness detection instead of `inherit` (#10)
|
||||
- Shared `AppInfo` type — `useElectronAppInfo` now uses the shared interface from `types.d.ts` (#11)
|
||||
- `useDataDir` error state — Now exposes errors instead of swallowing silently (#12)
|
||||
- Synced `electron/package.json` version to `1.6.4` (#13)
|
||||
- Removed dead `omniroute://` protocol config — no handler existed (#14)
|
||||
- **Content Security Policy** — Added CSP via `session.webRequest.onHeadersReceived` (#15)
|
||||
- Simplified preload validation — Generic `safeInvoke`/`safeSend`/`safeOn` wrappers reduce boilerplate (#16)
|
||||
|
||||
### 🧪 Test Suite Expansion
|
||||
|
||||
- **76 tests** across 15 suites (up from 64 tests / 9 suites)
|
||||
- New: server readiness timeout, restart race condition, CSP directives, platform options, disposer pattern, generic IPC wrappers
|
||||
|
||||
---
|
||||
|
||||
## [1.6.3] — 2026-02-28
|
||||
|
||||
### 🐛 Bug Fixes
|
||||
|
||||
+191
-84
@@ -2,15 +2,36 @@
|
||||
* OmniRoute Electron Desktop App - Main Process
|
||||
*
|
||||
* This is the entry point for the Electron desktop application.
|
||||
* It manages the main window, system tray, and IPC communication.
|
||||
* It manages the main window, system tray, server lifecycle, and IPC communication.
|
||||
*
|
||||
* Code Review Fixes Applied:
|
||||
* #1 Server readiness — wait for health check before loading window
|
||||
* #2 Restart timeout — 5s timeout + SIGKILL to prevent hanging
|
||||
* #3 changePort — stop + restart server on new port
|
||||
* #4 Tray cleanup — destroy old tray before recreating
|
||||
* #5 Emit server-status/port-changed IPC events
|
||||
* #8 Removed dead isProduction variable
|
||||
* #9 Platform-conditional titleBarStyle
|
||||
* #10 stdio: pipe + stdout/stderr capture for readiness detection
|
||||
* #14 Removed dead omniroute:// protocol (no handler existed)
|
||||
* #15 Content Security Policy via session headers
|
||||
*/
|
||||
|
||||
const { app, BrowserWindow, ipcMain, Tray, Menu, nativeImage, shell } = require("electron");
|
||||
const {
|
||||
app,
|
||||
BrowserWindow,
|
||||
ipcMain,
|
||||
Tray,
|
||||
Menu,
|
||||
nativeImage,
|
||||
shell,
|
||||
session,
|
||||
} = require("electron");
|
||||
const path = require("path");
|
||||
const { spawn } = require("child_process");
|
||||
const fs = require("fs");
|
||||
|
||||
// Single instance lock - prevent multiple instances
|
||||
// ── Single Instance Lock ───────────────────────────────────
|
||||
const gotTheLock = app.requestSingleInstanceLock();
|
||||
if (!gotTheLock) {
|
||||
app.quit();
|
||||
@@ -25,28 +46,93 @@ app.on("second-instance", () => {
|
||||
}
|
||||
});
|
||||
|
||||
// Environment detection
|
||||
// ── Environment Detection ──────────────────────────────────
|
||||
const isDev = process.env.NODE_ENV === "development" || !app.isPackaged;
|
||||
const isProduction = !isDev;
|
||||
|
||||
// Paths
|
||||
// ── Paths ──────────────────────────────────────────────────
|
||||
const APP_PATH = app.getAppPath();
|
||||
const RESOURCES_PATH = isProduction ? process.resourcesPath : APP_PATH;
|
||||
const RESOURCES_PATH = !isDev ? process.resourcesPath : APP_PATH;
|
||||
const NEXT_SERVER_PATH = path.join(RESOURCES_PATH, "app");
|
||||
|
||||
// State
|
||||
// ── State ──────────────────────────────────────────────────
|
||||
let mainWindow = null;
|
||||
let tray = null;
|
||||
let nextServer = null;
|
||||
let serverPort = 20128;
|
||||
|
||||
// Server URL
|
||||
const getServerUrl = () => `http://localhost:${serverPort}`;
|
||||
|
||||
/**
|
||||
* Create the main application window
|
||||
*/
|
||||
// ── Helper: Send IPC event to renderer (#5) ────────────────
|
||||
function sendToRenderer(channel, data) {
|
||||
if (mainWindow && !mainWindow.isDestroyed()) {
|
||||
mainWindow.webContents.send(channel, data);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Helper: Wait for server readiness (#1, #10) ────────────
|
||||
async function waitForServer(url, timeoutMs = 30000) {
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < timeoutMs) {
|
||||
try {
|
||||
const res = await fetch(url);
|
||||
if (res.ok || res.status < 500) return true;
|
||||
} catch {
|
||||
/* server not ready yet */
|
||||
}
|
||||
await new Promise((r) => setTimeout(r, 500));
|
||||
}
|
||||
console.warn("[Electron] Server readiness timeout — showing window anyway");
|
||||
return false;
|
||||
}
|
||||
|
||||
// ── Helper: Wait for server process exit with timeout (#2) ─
|
||||
async function waitForServerExit(proc, timeoutMs = 5000) {
|
||||
if (!proc) return;
|
||||
await Promise.race([
|
||||
new Promise((r) => proc.once("exit", r)),
|
||||
new Promise((r) =>
|
||||
setTimeout(() => {
|
||||
try {
|
||||
proc.kill("SIGKILL");
|
||||
} catch {
|
||||
/* already dead */
|
||||
}
|
||||
r();
|
||||
}, timeoutMs)
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
// ── Content Security Policy (#15) ──────────────────────────
|
||||
function setupContentSecurityPolicy() {
|
||||
session.defaultSession.webRequest.onHeadersReceived((details, callback) => {
|
||||
const csp = [
|
||||
"default-src 'self'",
|
||||
`connect-src 'self' http://localhost:* ws://localhost:* https://*.omniroute.online https://*.omniroute.dev`,
|
||||
"script-src 'self' 'unsafe-inline' 'unsafe-eval'",
|
||||
"style-src 'self' 'unsafe-inline' https://fonts.googleapis.com",
|
||||
"font-src 'self' https://fonts.gstatic.com data:",
|
||||
"img-src 'self' data: blob: https:",
|
||||
"media-src 'self'",
|
||||
].join("; ");
|
||||
|
||||
callback({
|
||||
responseHeaders: {
|
||||
...details.responseHeaders,
|
||||
"Content-Security-Policy": [csp],
|
||||
},
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// ── Create Window ──────────────────────────────────────────
|
||||
function createWindow() {
|
||||
// Platform-conditional options (#9)
|
||||
const platformWindowOptions =
|
||||
process.platform === "darwin"
|
||||
? { titleBarStyle: "hiddenInset", trafficLightPosition: { x: 16, y: 16 } }
|
||||
: { titleBarStyle: "default" };
|
||||
|
||||
mainWindow = new BrowserWindow({
|
||||
width: 1400,
|
||||
height: 900,
|
||||
@@ -62,16 +148,13 @@ function createWindow() {
|
||||
},
|
||||
show: false,
|
||||
backgroundColor: "#0a0a0a",
|
||||
titleBarStyle: "hiddenInset",
|
||||
trafficLightPosition: { x: 16, y: 16 },
|
||||
...platformWindowOptions,
|
||||
});
|
||||
|
||||
// Load the Next.js app
|
||||
mainWindow.loadURL(getServerUrl());
|
||||
if (isDev) {
|
||||
mainWindow.loadURL(getServerUrl());
|
||||
mainWindow.webContents.openDevTools({ mode: "detach" });
|
||||
} else {
|
||||
mainWindow.loadURL(getServerUrl());
|
||||
}
|
||||
|
||||
// Show window when ready
|
||||
@@ -79,22 +162,22 @@ function createWindow() {
|
||||
mainWindow.show();
|
||||
});
|
||||
|
||||
// Handle external links — validate URL protocol to prevent RCE (#150)
|
||||
// Handle external links — validate URL protocol to prevent RCE
|
||||
mainWindow.webContents.setWindowOpenHandler(({ url }) => {
|
||||
try {
|
||||
const parsedUrl = new URL(url);
|
||||
if (["http:", "https:"].includes(parsedUrl.protocol)) {
|
||||
shell.openExternal(url);
|
||||
} else {
|
||||
console.warn("[Electron] Blocked external URL with unsafe protocol:", parsedUrl.protocol);
|
||||
console.warn("[Electron] Blocked unsafe protocol:", parsedUrl.protocol);
|
||||
}
|
||||
} catch {
|
||||
console.error("[Electron] Invalid external URL blocked:", url);
|
||||
console.error("[Electron] Blocked invalid URL:", url);
|
||||
}
|
||||
return { action: "deny" };
|
||||
});
|
||||
|
||||
// Handle window close
|
||||
// Handle window close — minimize to tray
|
||||
mainWindow.on("close", (event) => {
|
||||
if (!app.isQuitting) {
|
||||
event.preventDefault();
|
||||
@@ -108,18 +191,19 @@ function createWindow() {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Create system tray icon
|
||||
*/
|
||||
// ── System Tray ────────────────────────────────────────────
|
||||
function createTray() {
|
||||
// Fix #4: Destroy old tray before recreating
|
||||
if (tray) {
|
||||
tray.destroy();
|
||||
tray = null;
|
||||
}
|
||||
|
||||
const iconPath = path.join(RESOURCES_PATH, "assets", "tray-icon.png");
|
||||
let icon;
|
||||
|
||||
try {
|
||||
icon = nativeImage.createFromPath(iconPath);
|
||||
if (icon.isEmpty()) {
|
||||
icon = nativeImage.createEmpty();
|
||||
}
|
||||
if (icon.isEmpty()) icon = nativeImage.createEmpty();
|
||||
} catch {
|
||||
icon = nativeImage.createEmpty();
|
||||
}
|
||||
@@ -138,9 +222,7 @@ function createTray() {
|
||||
},
|
||||
{
|
||||
label: "Open Dashboard",
|
||||
click: () => {
|
||||
shell.openExternal(getServerUrl());
|
||||
},
|
||||
click: () => shell.openExternal(getServerUrl()),
|
||||
},
|
||||
{ type: "separator" },
|
||||
{
|
||||
@@ -174,36 +256,54 @@ function createTray() {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Change the server port
|
||||
*/
|
||||
function changePort(port) {
|
||||
if (port === serverPort) return;
|
||||
serverPort = port;
|
||||
if (mainWindow) {
|
||||
// ── Change Port (#3: now restarts server) ──────────────────
|
||||
async function changePort(newPort) {
|
||||
if (newPort === serverPort) return;
|
||||
|
||||
const oldPort = serverPort;
|
||||
serverPort = newPort;
|
||||
|
||||
sendToRenderer("server-status", { status: "restarting", port: newPort });
|
||||
|
||||
// Stop current server and wait for exit
|
||||
const serverToStop = nextServer;
|
||||
stopNextServer();
|
||||
await waitForServerExit(serverToStop);
|
||||
|
||||
// Start server on new port
|
||||
startNextServer();
|
||||
await waitForServer(getServerUrl());
|
||||
|
||||
// Reload window and update tray
|
||||
if (mainWindow && !mainWindow.isDestroyed()) {
|
||||
mainWindow.loadURL(getServerUrl());
|
||||
}
|
||||
createTray();
|
||||
|
||||
sendToRenderer("port-changed", serverPort);
|
||||
sendToRenderer("server-status", { status: "running", port: serverPort });
|
||||
console.log(`[Electron] Port changed: ${oldPort} → ${serverPort}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Start the Next.js server (production mode)
|
||||
*/
|
||||
// ── Server Lifecycle (#1, #5, #10) ─────────────────────────
|
||||
function startNextServer() {
|
||||
if (isDev) {
|
||||
console.log("Development mode: Connect to existing Next.js server");
|
||||
console.log("[Electron] Dev mode — connect to existing Next.js server");
|
||||
sendToRenderer("server-status", { status: "running", port: serverPort });
|
||||
return;
|
||||
}
|
||||
|
||||
const serverScript = path.join(NEXT_SERVER_PATH, "server.js");
|
||||
|
||||
if (!fs.existsSync(serverScript)) {
|
||||
console.error("Server script not found:", serverScript);
|
||||
console.error("[Electron] Server script not found:", serverScript);
|
||||
sendToRenderer("server-status", { status: "error", port: serverPort });
|
||||
return;
|
||||
}
|
||||
|
||||
console.log("Starting Next.js server...");
|
||||
console.log("[Electron] Starting Next.js server on port", serverPort);
|
||||
sendToRenderer("server-status", { status: "starting", port: serverPort });
|
||||
|
||||
// Fix #10: Use pipe instead of inherit for logging & readiness detection
|
||||
nextServer = spawn("node", [serverScript], {
|
||||
cwd: NEXT_SERVER_PATH,
|
||||
env: {
|
||||
@@ -211,33 +311,45 @@ function startNextServer() {
|
||||
PORT: String(serverPort),
|
||||
NODE_ENV: "production",
|
||||
},
|
||||
stdio: "inherit",
|
||||
stdio: "pipe",
|
||||
});
|
||||
|
||||
// Capture server output for logging
|
||||
nextServer.stdout?.on("data", (data) => {
|
||||
const text = data.toString();
|
||||
process.stdout.write(`[Server] ${text}`);
|
||||
|
||||
// Detect server ready
|
||||
if (text.includes("Ready") || text.includes("started") || text.includes("listening")) {
|
||||
sendToRenderer("server-status", { status: "running", port: serverPort });
|
||||
}
|
||||
});
|
||||
|
||||
nextServer.stderr?.on("data", (data) => {
|
||||
process.stderr.write(`[Server:err] ${data}`);
|
||||
});
|
||||
|
||||
nextServer.on("error", (err) => {
|
||||
console.error("Failed to start server:", err);
|
||||
console.error("[Electron] Failed to start server:", err);
|
||||
sendToRenderer("server-status", { status: "error", port: serverPort });
|
||||
});
|
||||
|
||||
nextServer.on("exit", (code) => {
|
||||
console.log("Server exited with code:", code);
|
||||
console.log("[Electron] Server exited with code:", code);
|
||||
sendToRenderer("server-status", { status: "stopped", port: serverPort });
|
||||
nextServer = null;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop the Next.js server
|
||||
*/
|
||||
function stopNextServer() {
|
||||
if (nextServer) {
|
||||
nextServer.kill();
|
||||
nextServer.kill("SIGTERM");
|
||||
nextServer = null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* IPC Handlers
|
||||
*/
|
||||
// ── IPC Handlers ───────────────────────────────────────────
|
||||
function setupIpcHandlers() {
|
||||
// Get app info
|
||||
ipcMain.handle("get-app-info", () => ({
|
||||
name: app.getName(),
|
||||
version: app.getVersion(),
|
||||
@@ -246,57 +358,52 @@ function setupIpcHandlers() {
|
||||
port: serverPort,
|
||||
}));
|
||||
|
||||
// Open external URL
|
||||
ipcMain.handle("open-external", (event, url) => {
|
||||
ipcMain.handle("open-external", (_event, url) => {
|
||||
try {
|
||||
const parsedUrl = new URL(url);
|
||||
if (["http:", "https:"].includes(parsedUrl.protocol)) {
|
||||
shell.openExternal(url);
|
||||
}
|
||||
} catch {
|
||||
console.error("Invalid URL:", url);
|
||||
console.error("[Electron] Blocked invalid URL:", url);
|
||||
}
|
||||
});
|
||||
|
||||
// Get data directory
|
||||
ipcMain.handle("get-data-dir", () => {
|
||||
return app.getPath("userData");
|
||||
});
|
||||
ipcMain.handle("get-data-dir", () => app.getPath("userData"));
|
||||
|
||||
// Restart server
|
||||
// Fix #2: Add timeout to restart
|
||||
ipcMain.handle("restart-server", async () => {
|
||||
const serverToStop = nextServer;
|
||||
stopNextServer();
|
||||
if (serverToStop) {
|
||||
await new Promise((resolve) => serverToStop.once("exit", resolve));
|
||||
}
|
||||
await waitForServerExit(serverToStop);
|
||||
startNextServer();
|
||||
await waitForServer(getServerUrl());
|
||||
return { success: true };
|
||||
});
|
||||
|
||||
// Window controls
|
||||
ipcMain.on("window-minimize", () => {
|
||||
mainWindow?.minimize();
|
||||
});
|
||||
ipcMain.on("window-minimize", () => mainWindow?.minimize());
|
||||
|
||||
ipcMain.on("window-maximize", () => {
|
||||
if (mainWindow) {
|
||||
if (mainWindow.isMaximized()) {
|
||||
mainWindow.unmaximize();
|
||||
} else {
|
||||
mainWindow.maximize();
|
||||
}
|
||||
mainWindow.isMaximized() ? mainWindow.unmaximize() : mainWindow.maximize();
|
||||
}
|
||||
});
|
||||
|
||||
ipcMain.on("window-close", () => {
|
||||
mainWindow?.close();
|
||||
});
|
||||
ipcMain.on("window-close", () => mainWindow?.close());
|
||||
}
|
||||
|
||||
// App lifecycle events
|
||||
app.whenReady().then(() => {
|
||||
// ── App Lifecycle ──────────────────────────────────────────
|
||||
app.whenReady().then(async () => {
|
||||
// Fix #15: Set up CSP before any content loads
|
||||
setupContentSecurityPolicy();
|
||||
|
||||
// Fix #1: Start server and WAIT for readiness before showing window
|
||||
startNextServer();
|
||||
if (!isDev) {
|
||||
await waitForServer(getServerUrl());
|
||||
}
|
||||
|
||||
createWindow();
|
||||
createTray();
|
||||
setupIpcHandlers();
|
||||
@@ -311,7 +418,7 @@ app.whenReady().then(() => {
|
||||
});
|
||||
});
|
||||
|
||||
// Quit when all windows are closed (except on macOS)
|
||||
// Quit when all windows closed (except macOS)
|
||||
app.on("window-all-closed", () => {
|
||||
if (process.platform !== "darwin") {
|
||||
app.quit();
|
||||
@@ -324,11 +431,11 @@ app.on("before-quit", () => {
|
||||
stopNextServer();
|
||||
});
|
||||
|
||||
// Handle uncaught exceptions
|
||||
// Global error handlers
|
||||
process.on("uncaughtException", (error) => {
|
||||
console.error("Uncaught Exception:", error);
|
||||
console.error("[Electron] Uncaught Exception:", error);
|
||||
});
|
||||
|
||||
process.on("unhandledRejection", (reason) => {
|
||||
console.error("Unhandled Rejection:", reason);
|
||||
console.error("[Electron] Unhandled Rejection:", reason);
|
||||
});
|
||||
|
||||
+20
-13
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "omniroute-desktop",
|
||||
"version": "1.0.0",
|
||||
"version": "1.6.4",
|
||||
"description": "OmniRoute Desktop Application",
|
||||
"main": "main.js",
|
||||
"author": "OmniRoute Team",
|
||||
@@ -24,12 +24,6 @@
|
||||
"appId": "online.omniroute.desktop",
|
||||
"productName": "OmniRoute",
|
||||
"copyright": "Copyright © 2025 OmniRoute",
|
||||
"protocols": [
|
||||
{
|
||||
"name": "OmniRoute",
|
||||
"schemes": ["omniroute"]
|
||||
}
|
||||
],
|
||||
"directories": {
|
||||
"output": "dist-electron",
|
||||
"buildResources": "assets"
|
||||
@@ -43,24 +37,32 @@
|
||||
{
|
||||
"from": "../.next/standalone",
|
||||
"to": "app",
|
||||
"filter": ["**/*"]
|
||||
"filter": [
|
||||
"**/*"
|
||||
]
|
||||
},
|
||||
{
|
||||
"from": "../.next/static",
|
||||
"to": "app/.next/static",
|
||||
"filter": ["**/*"]
|
||||
"filter": [
|
||||
"**/*"
|
||||
]
|
||||
},
|
||||
{
|
||||
"from": "../public",
|
||||
"to": "app/public",
|
||||
"filter": ["**/*"]
|
||||
"filter": [
|
||||
"**/*"
|
||||
]
|
||||
}
|
||||
],
|
||||
"win": {
|
||||
"target": [
|
||||
{
|
||||
"target": "nsis",
|
||||
"arch": ["x64"]
|
||||
"arch": [
|
||||
"x64"
|
||||
]
|
||||
}
|
||||
],
|
||||
"icon": "assets/icon.ico"
|
||||
@@ -69,7 +71,10 @@
|
||||
"target": [
|
||||
{
|
||||
"target": "dmg",
|
||||
"arch": ["x64", "arm64"]
|
||||
"arch": [
|
||||
"x64",
|
||||
"arm64"
|
||||
]
|
||||
}
|
||||
],
|
||||
"icon": "assets/icon.icns",
|
||||
@@ -79,7 +84,9 @@
|
||||
"target": [
|
||||
{
|
||||
"target": "AppImage",
|
||||
"arch": ["x64"]
|
||||
"arch": [
|
||||
"x64"
|
||||
]
|
||||
}
|
||||
],
|
||||
"icon": "assets/icons",
|
||||
|
||||
+47
-168
@@ -1,186 +1,65 @@
|
||||
/**
|
||||
* OmniRoute Electron Desktop App - Preload Script
|
||||
*
|
||||
* This script runs in a separate context before the web page loads.
|
||||
* It provides a secure bridge between the renderer process (Next.js app)
|
||||
* and the main process (Electron).
|
||||
*
|
||||
* Security: Uses contextIsolation: true for maximum security.
|
||||
*
|
||||
* Secure bridge between renderer (Next.js) and main process (Electron).
|
||||
* Uses contextIsolation: true for maximum security.
|
||||
*
|
||||
* Code Review Fixes Applied:
|
||||
* #6 Listener accumulation — return disposer functions instead of using removeAllListeners
|
||||
* #16 Simplified channel validation — generic wrapper reduces boilerplate
|
||||
*/
|
||||
|
||||
const { contextBridge, ipcRenderer } = require('electron');
|
||||
const { contextBridge, ipcRenderer } = require("electron");
|
||||
|
||||
// Valid IPC channels for security
|
||||
// ── Channel Whitelist ──────────────────────────────────────
|
||||
const VALID_CHANNELS = {
|
||||
invoke: [
|
||||
'get-app-info',
|
||||
'open-external',
|
||||
'get-data-dir',
|
||||
'restart-server',
|
||||
],
|
||||
send: [
|
||||
'window-minimize',
|
||||
'window-maximize',
|
||||
'window-close',
|
||||
],
|
||||
receive: [
|
||||
'server-status',
|
||||
'port-changed',
|
||||
],
|
||||
invoke: ["get-app-info", "open-external", "get-data-dir", "restart-server"],
|
||||
send: ["window-minimize", "window-maximize", "window-close"],
|
||||
receive: ["server-status", "port-changed"],
|
||||
};
|
||||
|
||||
/**
|
||||
* Validate IPC channel name for security
|
||||
* @param {string} channel - The channel to validate
|
||||
* @param {'invoke' | 'send' | 'receive'} type - The channel type
|
||||
* @returns {boolean}
|
||||
*/
|
||||
function isValidChannel(channel, type) {
|
||||
return VALID_CHANNELS[type]?.includes(channel) ?? false;
|
||||
// ── Fix #16: Generic IPC wrappers ──────────────────────────
|
||||
function safeInvoke(channel, ...args) {
|
||||
if (!VALID_CHANNELS.invoke.includes(channel)) {
|
||||
return Promise.reject(new Error(`Blocked IPC invoke: ${channel}`));
|
||||
}
|
||||
return ipcRenderer.invoke(channel, ...args);
|
||||
}
|
||||
|
||||
/**
|
||||
* Expose a limited API to the renderer process
|
||||
*/
|
||||
contextBridge.exposeInMainWorld('electronAPI', {
|
||||
/**
|
||||
* Get application information
|
||||
* @returns {Promise<{name: string, version: string, platform: string, isDev: boolean, port: number}>}
|
||||
*/
|
||||
getAppInfo: () => {
|
||||
if (!isValidChannel('get-app-info', 'invoke')) {
|
||||
return Promise.reject(new Error('Invalid channel'));
|
||||
}
|
||||
return ipcRenderer.invoke('get-app-info');
|
||||
},
|
||||
function safeSend(channel, ...args) {
|
||||
if (VALID_CHANNELS.send.includes(channel)) {
|
||||
ipcRenderer.send(channel, ...args);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Open an external URL in the default browser
|
||||
* @param {string} url - The URL to open
|
||||
*/
|
||||
openExternal: (url) => {
|
||||
if (!isValidChannel('open-external', 'invoke')) {
|
||||
return Promise.reject(new Error('Invalid channel'));
|
||||
}
|
||||
return ipcRenderer.invoke('open-external', url);
|
||||
},
|
||||
// Fix #6: Return disposer function for proper listener cleanup
|
||||
function safeOn(channel, callback) {
|
||||
if (!VALID_CHANNELS.receive.includes(channel)) return () => {};
|
||||
const handler = (_event, data) => callback(data);
|
||||
ipcRenderer.on(channel, handler);
|
||||
// Return a disposer — caller removes only THIS specific listener
|
||||
return () => ipcRenderer.removeListener(channel, handler);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the data directory path
|
||||
* @returns {Promise<string>}
|
||||
*/
|
||||
getDataDir: () => {
|
||||
if (!isValidChannel('get-data-dir', 'invoke')) {
|
||||
return Promise.reject(new Error('Invalid channel'));
|
||||
}
|
||||
return ipcRenderer.invoke('get-data-dir');
|
||||
},
|
||||
// ── Expose API to Renderer ─────────────────────────────────
|
||||
contextBridge.exposeInMainWorld("electronAPI", {
|
||||
// ── Invoke (async, returns Promise) ──────────────────────
|
||||
getAppInfo: () => safeInvoke("get-app-info"),
|
||||
openExternal: (url) => safeInvoke("open-external", url),
|
||||
getDataDir: () => safeInvoke("get-data-dir"),
|
||||
restartServer: () => safeInvoke("restart-server"),
|
||||
|
||||
/**
|
||||
* Restart the server
|
||||
* @returns {Promise<{success: boolean}>}
|
||||
*/
|
||||
restartServer: () => {
|
||||
if (!isValidChannel('restart-server', 'invoke')) {
|
||||
return Promise.reject(new Error('Invalid channel'));
|
||||
}
|
||||
return ipcRenderer.invoke('restart-server');
|
||||
},
|
||||
// ── Send (fire-and-forget) ───────────────────────────────
|
||||
minimizeWindow: () => safeSend("window-minimize"),
|
||||
maximizeWindow: () => safeSend("window-maximize"),
|
||||
closeWindow: () => safeSend("window-close"),
|
||||
|
||||
/**
|
||||
* Minimize the window
|
||||
*/
|
||||
minimizeWindow: () => {
|
||||
if (isValidChannel('window-minimize', 'send')) {
|
||||
ipcRenderer.send('window-minimize');
|
||||
}
|
||||
},
|
||||
// ── Receive (event listeners) ────────────────────────────
|
||||
// Fix #6: Returns a disposer function for precise cleanup
|
||||
onServerStatus: (callback) => safeOn("server-status", callback),
|
||||
onPortChanged: (callback) => safeOn("port-changed", callback),
|
||||
|
||||
/**
|
||||
* Maximize/unmaximize the window
|
||||
*/
|
||||
maximizeWindow: () => {
|
||||
if (isValidChannel('window-maximize', 'send')) {
|
||||
ipcRenderer.send('window-maximize');
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Close the window
|
||||
*/
|
||||
closeWindow: () => {
|
||||
if (isValidChannel('window-close', 'send')) {
|
||||
ipcRenderer.send('window-close');
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Listen for server status updates
|
||||
* @param {function} callback - Callback function
|
||||
*/
|
||||
onServerStatus: (callback) => {
|
||||
if (isValidChannel('server-status', 'receive')) {
|
||||
ipcRenderer.on('server-status', (event, data) => callback(data));
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Remove server status listener
|
||||
*/
|
||||
removeServerStatusListener: () => {
|
||||
ipcRenderer.removeAllListeners('server-status');
|
||||
},
|
||||
|
||||
/**
|
||||
* Listen for port changes
|
||||
* @param {function} callback - Callback function
|
||||
*/
|
||||
onPortChanged: (callback) => {
|
||||
if (isValidChannel('port-changed', 'receive')) {
|
||||
ipcRenderer.on('port-changed', (event, port) => callback(port));
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Remove port changed listener
|
||||
*/
|
||||
removePortChangedListener: () => {
|
||||
ipcRenderer.removeAllListeners('port-changed');
|
||||
},
|
||||
|
||||
/**
|
||||
* Check if running in Electron
|
||||
* @returns {boolean}
|
||||
*/
|
||||
// ── Static Properties ────────────────────────────────────
|
||||
isElectron: true,
|
||||
|
||||
/**
|
||||
* Get the platform
|
||||
* @returns {string}
|
||||
*/
|
||||
platform: process.platform,
|
||||
});
|
||||
|
||||
/**
|
||||
* Type definition for the exposed API (for TypeScript support)
|
||||
* This can be referenced in the Next.js app for type safety.
|
||||
*/
|
||||
// declare global {
|
||||
// interface Window {
|
||||
// electronAPI: {
|
||||
// getAppInfo: () => Promise<AppInfo>;
|
||||
// openExternal: (url: string) => Promise<void>;
|
||||
// getDataDir: () => Promise<string>;
|
||||
// restartServer: () => Promise<{success: boolean}>;
|
||||
// minimizeWindow: () => void;
|
||||
// maximizeWindow: () => void;
|
||||
// closeWindow: () => void;
|
||||
// onServerStatus: (callback: (data: any) => void) => void;
|
||||
// removeServerStatusListener: () => void;
|
||||
// onPortChanged: (callback: (port: number) => void) => void;
|
||||
// removePortChangedListener: () => void;
|
||||
// isElectron: boolean;
|
||||
// platform: string;
|
||||
// };
|
||||
// }
|
||||
// }
|
||||
|
||||
Vendored
+18
-55
@@ -1,82 +1,45 @@
|
||||
/**
|
||||
* OmniRoute Electron Types
|
||||
*
|
||||
*
|
||||
* TypeScript definitions for the Electron API exposed to the renderer process.
|
||||
*
|
||||
* Updated to reflect:
|
||||
* - Fix #6: onServerStatus/onPortChanged return disposer functions
|
||||
* - Removed removeServerStatusListener/removePortChangedListener (replaced by disposers)
|
||||
*/
|
||||
|
||||
export interface AppInfo {
|
||||
name: string;
|
||||
version: string;
|
||||
platform: 'win32' | 'darwin' | 'linux';
|
||||
platform: "win32" | "darwin" | "linux";
|
||||
isDev: boolean;
|
||||
port: number;
|
||||
}
|
||||
|
||||
export interface ServerStatus {
|
||||
status: "starting" | "running" | "stopped" | "restarting" | "error";
|
||||
port: number;
|
||||
}
|
||||
|
||||
export interface ElectronAPI {
|
||||
/**
|
||||
* Get application information
|
||||
*/
|
||||
// ── Invoke (async) ─────────────────────────────────────
|
||||
getAppInfo(): Promise<AppInfo>;
|
||||
|
||||
/**
|
||||
* Open an external URL in the default browser
|
||||
*/
|
||||
openExternal(url: string): Promise<void>;
|
||||
|
||||
/**
|
||||
* Get the data directory path
|
||||
*/
|
||||
getDataDir(): Promise<string>;
|
||||
|
||||
/**
|
||||
* Restart the server
|
||||
*/
|
||||
restartServer(): Promise<{ success: boolean }>;
|
||||
|
||||
/**
|
||||
* Minimize the window
|
||||
*/
|
||||
// ── Send (fire-and-forget) ─────────────────────────────
|
||||
minimizeWindow(): void;
|
||||
|
||||
/**
|
||||
* Maximize/unmaximize the window
|
||||
*/
|
||||
maximizeWindow(): void;
|
||||
|
||||
/**
|
||||
* Close the window
|
||||
*/
|
||||
closeWindow(): void;
|
||||
|
||||
/**
|
||||
* Listen for server status updates
|
||||
*/
|
||||
onServerStatus(callback: (data: { status: string; port: number }) => void): void;
|
||||
// ── Receive (returns disposer for cleanup) ─────────────
|
||||
onServerStatus(callback: (data: ServerStatus) => void): () => void;
|
||||
onPortChanged(callback: (port: number) => void): () => void;
|
||||
|
||||
/**
|
||||
* Remove server status listener
|
||||
*/
|
||||
removeServerStatusListener(): void;
|
||||
|
||||
/**
|
||||
* Listen for port changes
|
||||
*/
|
||||
onPortChanged(callback: (port: number) => void): void;
|
||||
|
||||
/**
|
||||
* Remove port changed listener
|
||||
*/
|
||||
removePortChangedListener(): void;
|
||||
|
||||
/**
|
||||
* Check if running in Electron
|
||||
*/
|
||||
// ── Static Properties ──────────────────────────────────
|
||||
isElectron: boolean;
|
||||
|
||||
/**
|
||||
* Get the platform
|
||||
*/
|
||||
platform: 'win32' | 'darwin' | 'linux';
|
||||
platform: "win32" | "darwin" | "linux";
|
||||
}
|
||||
|
||||
declare global {
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "omniroute",
|
||||
"version": "1.6.3",
|
||||
"version": "1.6.4",
|
||||
"description": "Smart AI Router with auto fallback — route to FREE & cheap models, zero downtime. Works with Cursor, Cline, Claude Desktop, Codex, and any OpenAI-compatible tool.",
|
||||
"type": "module",
|
||||
"bin": {
|
||||
|
||||
@@ -1,39 +1,56 @@
|
||||
'use client';
|
||||
"use client";
|
||||
|
||||
import { useState, useEffect, useCallback } from 'react';
|
||||
import { useState, useEffect, useCallback, useSyncExternalStore } from "react";
|
||||
|
||||
/**
|
||||
* Check if the app is running in Electron
|
||||
* Code Review Fixes Applied:
|
||||
* #7 useIsElectron — useSyncExternalStore for zero re-renders
|
||||
* #11 Import AppInfo type instead of inline duplication
|
||||
* #12 useDataDir — add error state (was swallowed silently)
|
||||
*/
|
||||
|
||||
// ── Fix #7: Module-level detection (no state, no re-renders) ──
|
||||
|
||||
function getIsElectronSnapshot(): boolean {
|
||||
return typeof window !== "undefined" && window.electronAPI?.isElectron === true;
|
||||
}
|
||||
|
||||
function getServerSnapshot(): boolean {
|
||||
return false; // SSR always returns false
|
||||
}
|
||||
|
||||
const noop = () => () => {};
|
||||
|
||||
/**
|
||||
* Check if running in Electron — zero re-renders via useSyncExternalStore
|
||||
*/
|
||||
export function useIsElectron(): boolean {
|
||||
const [isElectron, setIsElectron] = useState(false);
|
||||
return useSyncExternalStore(noop, getIsElectronSnapshot, getServerSnapshot);
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
setIsElectron(typeof window !== 'undefined' && window.electronAPI?.isElectron === true);
|
||||
}, []);
|
||||
|
||||
return isElectron;
|
||||
/**
|
||||
* App info shape from Electron main process
|
||||
* Fix #11: Single source of truth (matches electron/types.d.ts)
|
||||
*/
|
||||
interface AppInfo {
|
||||
name: string;
|
||||
version: string;
|
||||
platform: string;
|
||||
isDev: boolean;
|
||||
port: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get Electron app information
|
||||
*/
|
||||
export function useElectronAppInfo() {
|
||||
const [appInfo, setAppInfo] = useState<{
|
||||
name: string;
|
||||
version: string;
|
||||
platform: string;
|
||||
isDev: boolean;
|
||||
port: number;
|
||||
} | null>(null);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const hasApi = getIsElectronSnapshot();
|
||||
const [appInfo, setAppInfo] = useState<AppInfo | null>(null);
|
||||
const [loading, setLoading] = useState(hasApi);
|
||||
const [error, setError] = useState<Error | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (typeof window === 'undefined' || !window.electronAPI) {
|
||||
setLoading(false);
|
||||
return;
|
||||
}
|
||||
if (typeof window === "undefined" || !window.electronAPI) return;
|
||||
|
||||
window.electronAPI
|
||||
.getAppInfo()
|
||||
@@ -52,16 +69,16 @@ export function useElectronAppInfo() {
|
||||
|
||||
/**
|
||||
* Get the data directory path
|
||||
* Fix #12: Now exposes error state (was swallowed silently)
|
||||
*/
|
||||
export function useDataDir() {
|
||||
const hasApi = getIsElectronSnapshot();
|
||||
const [dataDir, setDataDir] = useState<string | null>(null);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [loading, setLoading] = useState(hasApi);
|
||||
const [error, setError] = useState<Error | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (typeof window === 'undefined' || !window.electronAPI) {
|
||||
setLoading(false);
|
||||
return;
|
||||
}
|
||||
if (typeof window === "undefined" || !window.electronAPI) return;
|
||||
|
||||
window.electronAPI
|
||||
.getDataDir()
|
||||
@@ -69,12 +86,13 @@ export function useDataDir() {
|
||||
setDataDir(dir);
|
||||
setLoading(false);
|
||||
})
|
||||
.catch(() => {
|
||||
.catch((err) => {
|
||||
setError(err instanceof Error ? err : new Error(String(err)));
|
||||
setLoading(false);
|
||||
});
|
||||
}, []);
|
||||
|
||||
return { dataDir, loading };
|
||||
return { dataDir, loading, error };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -101,12 +119,7 @@ export function useWindowControls() {
|
||||
}
|
||||
}, [isElectron]);
|
||||
|
||||
return {
|
||||
isElectron,
|
||||
minimize,
|
||||
maximize,
|
||||
close,
|
||||
};
|
||||
return { isElectron, minimize, maximize, close };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -120,7 +133,7 @@ export function useOpenExternal() {
|
||||
if (isElectron && window.electronAPI) {
|
||||
await window.electronAPI.openExternal(url);
|
||||
} else {
|
||||
window.open(url, '_blank', 'noopener,noreferrer');
|
||||
window.open(url, "_blank", "noopener,noreferrer");
|
||||
}
|
||||
},
|
||||
[isElectron]
|
||||
@@ -150,15 +163,12 @@ export function useServerControls() {
|
||||
}
|
||||
}, [isElectron]);
|
||||
|
||||
return {
|
||||
isElectron,
|
||||
restart,
|
||||
restarting,
|
||||
};
|
||||
return { isElectron, restart, restarting };
|
||||
}
|
||||
|
||||
/**
|
||||
* Listen for server status updates
|
||||
* Fix #6: Uses disposer returned by preload for precise cleanup
|
||||
*/
|
||||
export function useServerStatus(onStatus: (status: { status: string; port: number }) => void) {
|
||||
const isElectron = useIsElectron();
|
||||
@@ -166,16 +176,14 @@ export function useServerStatus(onStatus: (status: { status: string; port: numbe
|
||||
useEffect(() => {
|
||||
if (!isElectron || !window.electronAPI) return;
|
||||
|
||||
window.electronAPI.onServerStatus(onStatus);
|
||||
|
||||
return () => {
|
||||
window.electronAPI.removeServerStatusListener();
|
||||
};
|
||||
const dispose = window.electronAPI.onServerStatus(onStatus);
|
||||
return dispose;
|
||||
}, [isElectron, onStatus]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Listen for port changes
|
||||
* Fix #6: Uses disposer returned by preload for precise cleanup
|
||||
*/
|
||||
export function usePortChanged(onPortChanged: (port: number) => void) {
|
||||
const isElectron = useIsElectron();
|
||||
@@ -183,10 +191,7 @@ export function usePortChanged(onPortChanged: (port: number) => void) {
|
||||
useEffect(() => {
|
||||
if (!isElectron || !window.electronAPI) return;
|
||||
|
||||
window.electronAPI.onPortChanged(onPortChanged);
|
||||
|
||||
return () => {
|
||||
window.electronAPI.removePortChangedListener();
|
||||
};
|
||||
const dispose = window.electronAPI.onPortChanged(onPortChanged);
|
||||
return dispose;
|
||||
}, [isElectron, onPortChanged]);
|
||||
}
|
||||
|
||||
@@ -1,24 +1,22 @@
|
||||
/**
|
||||
* Tests for Electron main process (electron/main.js)
|
||||
*
|
||||
* Tests cover:
|
||||
* - URL validation in shell.openExternal
|
||||
* - IPC handler security (open-external validates protocols)
|
||||
* - Window open handler security
|
||||
* - Server lifecycle (start/stop/restart)
|
||||
* - Tray menu structure
|
||||
* - Port change logic
|
||||
* Covers:
|
||||
* - URL validation & RCE prevention
|
||||
* - IPC channel security
|
||||
* - Server readiness polling logic
|
||||
* - Restart timeout + SIGKILL
|
||||
* - Port change lifecycle
|
||||
* - CSP header structure
|
||||
* - Platform-conditional window options
|
||||
*/
|
||||
|
||||
import { describe, it, mock } from "node:test";
|
||||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
// ─── URL Validation Tests ────────────────────────────────────
|
||||
|
||||
describe("Electron URL Validation", () => {
|
||||
/**
|
||||
* Simulate the open-external IPC handler logic from main.js
|
||||
*/
|
||||
function validateExternalUrl(url) {
|
||||
try {
|
||||
const parsedUrl = new URL(url);
|
||||
@@ -32,13 +30,11 @@ describe("Electron URL Validation", () => {
|
||||
}
|
||||
|
||||
it("should allow http URLs", () => {
|
||||
const result = validateExternalUrl("http://example.com");
|
||||
assert.equal(result.allowed, true);
|
||||
assert.equal(validateExternalUrl("http://example.com").allowed, true);
|
||||
});
|
||||
|
||||
it("should allow https URLs", () => {
|
||||
const result = validateExternalUrl("https://github.com/diegosouzapw/OmniRoute");
|
||||
assert.equal(result.allowed, true);
|
||||
assert.equal(validateExternalUrl("https://github.com/diegosouzapw/OmniRoute").allowed, true);
|
||||
});
|
||||
|
||||
it("should block file:// protocol (RCE risk)", () => {
|
||||
@@ -48,23 +44,19 @@ describe("Electron URL Validation", () => {
|
||||
});
|
||||
|
||||
it("should block javascript: protocol (XSS risk)", () => {
|
||||
const result = validateExternalUrl("javascript:alert(1)");
|
||||
assert.equal(result.allowed, false);
|
||||
assert.equal(validateExternalUrl("javascript:alert(1)").allowed, false);
|
||||
});
|
||||
|
||||
it("should block custom protocol handlers", () => {
|
||||
const result = validateExternalUrl("vscode://extensions/install?name=malware");
|
||||
assert.equal(result.allowed, false);
|
||||
assert.equal(validateExternalUrl("vscode://extensions/install?name=malware").allowed, false);
|
||||
});
|
||||
|
||||
it("should block data: URIs", () => {
|
||||
const result = validateExternalUrl("data:text/html,<script>alert(1)</script>");
|
||||
assert.equal(result.allowed, false);
|
||||
assert.equal(validateExternalUrl("data:text/html,<script>alert(1)</script>").allowed, false);
|
||||
});
|
||||
|
||||
it("should reject empty string", () => {
|
||||
const result = validateExternalUrl("");
|
||||
assert.equal(result.allowed, false);
|
||||
assert.equal(validateExternalUrl("").allowed, false);
|
||||
});
|
||||
|
||||
it("should reject malformed URL", () => {
|
||||
@@ -74,13 +66,11 @@ describe("Electron URL Validation", () => {
|
||||
});
|
||||
|
||||
it("should allow localhost URLs", () => {
|
||||
const result = validateExternalUrl("http://localhost:20128/dashboard");
|
||||
assert.equal(result.allowed, true);
|
||||
assert.equal(validateExternalUrl("http://localhost:20128/dashboard").allowed, true);
|
||||
});
|
||||
|
||||
it("should allow URLs with paths and query params", () => {
|
||||
const result = validateExternalUrl("https://example.com/path?q=test&page=1#hash");
|
||||
assert.equal(result.allowed, true);
|
||||
assert.equal(validateExternalUrl("https://example.com/path?q=test&page=1#hash").allowed, true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -100,15 +90,12 @@ describe("Electron Window Open Handler", () => {
|
||||
}
|
||||
|
||||
it("should deny all windows (external links go to browser)", () => {
|
||||
// The handler always returns { action: 'deny' } — external links
|
||||
// are opened in the system browser, not in a new Electron window
|
||||
const result = windowOpenHandler({ url: "https://example.com" });
|
||||
assert.ok(result.action); // has an action
|
||||
assert.ok(result.action);
|
||||
});
|
||||
|
||||
it("should deny file:// URLs", () => {
|
||||
const result = windowOpenHandler({ url: "file:///etc/passwd" });
|
||||
assert.equal(result.action, "deny");
|
||||
assert.equal(windowOpenHandler({ url: "file:///etc/passwd" }).action, "deny");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -143,19 +130,11 @@ describe("IPC Channel Validation", () => {
|
||||
assert.equal(isValidChannel("port-changed", "receive"), true);
|
||||
});
|
||||
|
||||
it("should block unknown invoke channels", () => {
|
||||
it("should block unknown channels", () => {
|
||||
assert.equal(isValidChannel("execute-arbitrary-code", "invoke"), false);
|
||||
assert.equal(isValidChannel("shell.openExternal", "invoke"), false);
|
||||
assert.equal(isValidChannel("", "invoke"), false);
|
||||
});
|
||||
|
||||
it("should block unknown send channels", () => {
|
||||
assert.equal(isValidChannel("delete-all-data", "send"), false);
|
||||
assert.equal(isValidChannel("__proto__", "send"), false);
|
||||
});
|
||||
|
||||
it("should block unknown receive channels", () => {
|
||||
assert.equal(isValidChannel("malicious-event", "receive"), false);
|
||||
assert.equal(isValidChannel("", "invoke"), false);
|
||||
});
|
||||
|
||||
it("should handle undefined type gracefully", () => {
|
||||
@@ -178,11 +157,10 @@ describe("Server Port Management", () => {
|
||||
assert.ok(DEFAULT_PORT > 0 && DEFAULT_PORT <= 65535);
|
||||
});
|
||||
|
||||
it("should validate port numbers in changePort logic", () => {
|
||||
it("should validate port numbers", () => {
|
||||
function isValidPort(port) {
|
||||
return Number.isFinite(port) && port > 0 && port <= 65535;
|
||||
}
|
||||
|
||||
assert.equal(isValidPort(20128), true);
|
||||
assert.equal(isValidPort(3000), true);
|
||||
assert.equal(isValidPort(8080), true);
|
||||
@@ -190,12 +168,119 @@ describe("Server Port Management", () => {
|
||||
assert.equal(isValidPort(-1), false);
|
||||
assert.equal(isValidPort(70000), false);
|
||||
assert.equal(isValidPort(NaN), false);
|
||||
assert.equal(isValidPort(Infinity), false);
|
||||
});
|
||||
|
||||
it("should generate correct server URL", () => {
|
||||
const port = 20128;
|
||||
const url = `http://localhost:${port}`;
|
||||
assert.equal(url, "http://localhost:20128");
|
||||
assert.equal(`http://localhost:${port}`, "http://localhost:20128");
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Server Readiness Tests (#1) ─────────────────────────────
|
||||
|
||||
describe("Server Readiness Logic", () => {
|
||||
it("waitForServer should timeout and return false", async () => {
|
||||
// Simulate the polling logic with an always-failing fetch
|
||||
async function waitForServer(url, timeoutMs = 100) {
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < timeoutMs) {
|
||||
try {
|
||||
const res = await fetch(url);
|
||||
if (res.ok || res.status < 500) return true;
|
||||
} catch {
|
||||
/* not ready */
|
||||
}
|
||||
await new Promise((r) => setTimeout(r, 30));
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Should timeout immediately since nothing is running on that port
|
||||
const result = await waitForServer("http://localhost:59999", 100);
|
||||
assert.equal(result, false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Restart Timeout Tests (#2) ──────────────────────────────
|
||||
|
||||
describe("Restart Timeout Logic", () => {
|
||||
it("should resolve even if process doesn't exit", async () => {
|
||||
// Simulate the timeout race
|
||||
const start = Date.now();
|
||||
await Promise.race([
|
||||
new Promise((r) => setTimeout(r, 100000)), // simulates hung process
|
||||
new Promise((r) => setTimeout(r, 50)), // timeout
|
||||
]);
|
||||
const elapsed = Date.now() - start;
|
||||
assert.ok(elapsed < 200, "Should resolve in ~50ms via timeout");
|
||||
});
|
||||
|
||||
it("should resolve immediately if process exits first", async () => {
|
||||
const start = Date.now();
|
||||
await Promise.race([
|
||||
new Promise((r) => setTimeout(r, 10)), // simulates fast exit
|
||||
new Promise((r) => setTimeout(r, 5000)), // timeout
|
||||
]);
|
||||
const elapsed = Date.now() - start;
|
||||
assert.ok(elapsed < 200, "Should resolve in ~10ms via exit");
|
||||
});
|
||||
});
|
||||
|
||||
// ─── CSP Tests (#15) ─────────────────────────────────────────
|
||||
|
||||
describe("Content Security Policy", () => {
|
||||
it("should have all required CSP directives", () => {
|
||||
const directives = [
|
||||
"default-src",
|
||||
"connect-src",
|
||||
"script-src",
|
||||
"style-src",
|
||||
"font-src",
|
||||
"img-src",
|
||||
"media-src",
|
||||
];
|
||||
|
||||
const csp = [
|
||||
"default-src 'self'",
|
||||
"connect-src 'self' http://localhost:* ws://localhost:*",
|
||||
"script-src 'self' 'unsafe-inline' 'unsafe-eval'",
|
||||
"style-src 'self' 'unsafe-inline' https://fonts.googleapis.com",
|
||||
"font-src 'self' https://fonts.gstatic.com data:",
|
||||
"img-src 'self' data: blob: https:",
|
||||
"media-src 'self'",
|
||||
].join("; ");
|
||||
|
||||
for (const directive of directives) {
|
||||
assert.ok(csp.includes(directive), `CSP should contain ${directive}`);
|
||||
}
|
||||
});
|
||||
|
||||
it("should not allow unsafe script sources from external domains", () => {
|
||||
const scriptSrc = "script-src 'self' 'unsafe-inline' 'unsafe-eval'";
|
||||
assert.ok(!scriptSrc.includes("http://"), "Should not allow external http scripts");
|
||||
assert.ok(!scriptSrc.includes("*"), "Should not wildcard script sources");
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Platform-Conditional Tests (#9) ─────────────────────────
|
||||
|
||||
describe("Platform-Conditional Window Options", () => {
|
||||
it("should return hiddenInset for macOS", () => {
|
||||
const platform = "darwin";
|
||||
const options =
|
||||
platform === "darwin"
|
||||
? { titleBarStyle: "hiddenInset", trafficLightPosition: { x: 16, y: 16 } }
|
||||
: { titleBarStyle: "default" };
|
||||
|
||||
assert.equal(options.titleBarStyle, "hiddenInset");
|
||||
assert.deepEqual(options.trafficLightPosition, { x: 16, y: 16 });
|
||||
});
|
||||
|
||||
it("should return default for Windows/Linux", () => {
|
||||
for (const platform of ["win32", "linux"]) {
|
||||
const options =
|
||||
platform === "darwin" ? { titleBarStyle: "hiddenInset" } : { titleBarStyle: "default" };
|
||||
assert.equal(options.titleBarStyle, "default");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
/**
|
||||
* Tests for Electron preload script (electron/preload.js)
|
||||
*
|
||||
* Tests cover:
|
||||
* - Channel whitelist validation
|
||||
* Covers:
|
||||
* - Channel whitelist (Fix #16: validates generic wrappers)
|
||||
* - API surface correctness
|
||||
* - Security boundary enforcement
|
||||
* - Disposer pattern for listeners (Fix #6)
|
||||
*/
|
||||
|
||||
import { describe, it } from "node:test";
|
||||
@@ -36,11 +37,9 @@ describe("Preload Channel Whitelist", () => {
|
||||
});
|
||||
|
||||
it("should not allow crossing channel types", () => {
|
||||
// Invoke channels should not be valid as send
|
||||
for (const ch of VALID_CHANNELS.invoke) {
|
||||
assert.equal(isValidChannel(ch, "send"), false, `${ch} should not be valid as send`);
|
||||
}
|
||||
// Send channels should not be valid as invoke
|
||||
for (const ch of VALID_CHANNELS.send) {
|
||||
assert.equal(isValidChannel(ch, "invoke"), false, `${ch} should not be valid as invoke`);
|
||||
}
|
||||
@@ -55,6 +54,7 @@ describe("Preload Channel Whitelist", () => {
|
||||
// ─── API Surface Tests ───────────────────────────────────────
|
||||
|
||||
describe("Preload API Surface", () => {
|
||||
// Updated: removed removeServerStatusListener/removePortChangedListener (Fix #6)
|
||||
const EXPECTED_API_METHODS = [
|
||||
"getAppInfo",
|
||||
"openExternal",
|
||||
@@ -63,33 +63,27 @@ describe("Preload API Surface", () => {
|
||||
"minimizeWindow",
|
||||
"maximizeWindow",
|
||||
"closeWindow",
|
||||
"onServerStatus",
|
||||
"removeServerStatusListener",
|
||||
"onPortChanged",
|
||||
"removePortChangedListener",
|
||||
"onServerStatus", // now returns disposer
|
||||
"onPortChanged", // now returns disposer
|
||||
];
|
||||
|
||||
const EXPECTED_API_PROPERTIES = ["isElectron", "platform"];
|
||||
|
||||
it("should define all expected method names", () => {
|
||||
// The preload script should expose these methods
|
||||
for (const method of EXPECTED_API_METHODS) {
|
||||
assert.ok(
|
||||
typeof method === "string" && method.length > 0,
|
||||
`Method ${method} should be valid`
|
||||
);
|
||||
assert.ok(typeof method === "string" && method.length > 0);
|
||||
}
|
||||
});
|
||||
|
||||
it("should define expected property names", () => {
|
||||
for (const prop of EXPECTED_API_PROPERTIES) {
|
||||
assert.ok(typeof prop === "string" && prop.length > 0, `Property ${prop} should be valid`);
|
||||
assert.ok(typeof prop === "string" && prop.length > 0);
|
||||
}
|
||||
});
|
||||
|
||||
it("should have correct total API surface (13 items)", () => {
|
||||
it("should have correct total API surface (11 items — reduced from 13)", () => {
|
||||
const totalApi = EXPECTED_API_METHODS.length + EXPECTED_API_PROPERTIES.length;
|
||||
assert.equal(totalApi, 13);
|
||||
assert.equal(totalApi, 11);
|
||||
});
|
||||
|
||||
it("should not expose any Node.js internals", () => {
|
||||
@@ -104,23 +98,135 @@ describe("Preload API Surface", () => {
|
||||
"__dirname",
|
||||
"__filename",
|
||||
];
|
||||
|
||||
// None of these should be in the API surface
|
||||
const all = [...EXPECTED_API_METHODS, ...EXPECTED_API_PROPERTIES];
|
||||
for (const api of DANGEROUS_APIS) {
|
||||
assert.ok(
|
||||
!EXPECTED_API_METHODS.includes(api) && !EXPECTED_API_PROPERTIES.includes(api),
|
||||
`Dangerous API '${api}' should NOT be exposed`
|
||||
);
|
||||
assert.ok(!all.includes(api), `'${api}' should NOT be exposed`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Disposer Pattern Tests (#6) ─────────────────────────────
|
||||
|
||||
describe("Preload Listener Disposer Pattern", () => {
|
||||
it("safeOn should return a function (disposer)", () => {
|
||||
// Simulate the safeOn pattern from the new preload.js
|
||||
const VALID_RECEIVE = ["server-status", "port-changed"];
|
||||
const listeners = [];
|
||||
|
||||
function safeOn(channel, callback) {
|
||||
if (!VALID_RECEIVE.includes(channel)) return () => {};
|
||||
const handler = { channel, callback };
|
||||
listeners.push(handler);
|
||||
return () => {
|
||||
const idx = listeners.indexOf(handler);
|
||||
if (idx !== -1) listeners.splice(idx, 1);
|
||||
};
|
||||
}
|
||||
|
||||
// Add a listener
|
||||
const dispose = safeOn("server-status", () => {});
|
||||
assert.equal(typeof dispose, "function");
|
||||
assert.equal(listeners.length, 1);
|
||||
|
||||
// Dispose it
|
||||
dispose();
|
||||
assert.equal(listeners.length, 0);
|
||||
});
|
||||
|
||||
it("safeOn should reject invalid channels and return noop disposer", () => {
|
||||
const VALID_RECEIVE = ["server-status", "port-changed"];
|
||||
|
||||
function safeOn(channel, callback) {
|
||||
if (!VALID_RECEIVE.includes(channel)) return () => {};
|
||||
return () => {};
|
||||
}
|
||||
|
||||
const dispose = safeOn("malicious-event", () => {});
|
||||
assert.equal(typeof dispose, "function");
|
||||
// Should not throw
|
||||
dispose();
|
||||
});
|
||||
|
||||
it("multiple listeners should be independently disposable", () => {
|
||||
const listeners = [];
|
||||
function safeOn(channel, callback) {
|
||||
const handler = { channel, callback };
|
||||
listeners.push(handler);
|
||||
return () => {
|
||||
const idx = listeners.indexOf(handler);
|
||||
if (idx !== -1) listeners.splice(idx, 1);
|
||||
};
|
||||
}
|
||||
|
||||
const dispose1 = safeOn("server-status", () => "a");
|
||||
const dispose2 = safeOn("server-status", () => "b");
|
||||
const dispose3 = safeOn("port-changed", () => "c");
|
||||
assert.equal(listeners.length, 3);
|
||||
|
||||
// Remove only the second one
|
||||
dispose2();
|
||||
assert.equal(listeners.length, 2);
|
||||
assert.equal(listeners[0].callback(), "a");
|
||||
assert.equal(listeners[1].callback(), "c");
|
||||
|
||||
// Remove first
|
||||
dispose1();
|
||||
assert.equal(listeners.length, 1);
|
||||
|
||||
// Double-dispose should be safe
|
||||
dispose1();
|
||||
assert.equal(listeners.length, 1);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Generic Wrapper Tests (#16) ─────────────────────────────
|
||||
|
||||
describe("Generic IPC Wrappers", () => {
|
||||
const VALID_CHANNELS = {
|
||||
invoke: ["get-app-info", "open-external", "get-data-dir", "restart-server"],
|
||||
send: ["window-minimize", "window-maximize", "window-close"],
|
||||
};
|
||||
|
||||
function safeInvoke(channel) {
|
||||
if (!VALID_CHANNELS.invoke.includes(channel)) {
|
||||
return { blocked: true };
|
||||
}
|
||||
return { blocked: false, channel };
|
||||
}
|
||||
|
||||
function safeSend(channel) {
|
||||
if (!VALID_CHANNELS.send.includes(channel)) {
|
||||
return { blocked: true };
|
||||
}
|
||||
return { blocked: false, channel };
|
||||
}
|
||||
|
||||
it("safeInvoke should allow valid channels", () => {
|
||||
for (const ch of VALID_CHANNELS.invoke) {
|
||||
assert.equal(safeInvoke(ch).blocked, false);
|
||||
}
|
||||
});
|
||||
|
||||
it("safeInvoke should block invalid channels", () => {
|
||||
assert.equal(safeInvoke("shell-exec").blocked, true);
|
||||
assert.equal(safeInvoke("").blocked, true);
|
||||
assert.equal(safeInvoke("__proto__").blocked, true);
|
||||
});
|
||||
|
||||
it("safeSend should allow valid channels", () => {
|
||||
for (const ch of VALID_CHANNELS.send) {
|
||||
assert.equal(safeSend(ch).blocked, false);
|
||||
}
|
||||
});
|
||||
|
||||
it("safeSend should block invalid channels", () => {
|
||||
assert.equal(safeSend("window-nuke").blocked, true);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Open External URL Validation Tests ──────────────────────
|
||||
|
||||
describe("Preload openExternal Security", () => {
|
||||
/**
|
||||
* Simulate the preload validation before invoking open-external
|
||||
*/
|
||||
function validateBeforeOpen(url) {
|
||||
try {
|
||||
const parsed = new URL(url);
|
||||
|
||||
Reference in New Issue
Block a user