diff --git a/jest.config.js b/jest.config.js index cc7a293..9925390 100755 --- a/jest.config.js +++ b/jest.config.js @@ -22,6 +22,9 @@ module.exports = { coverageDirectory: '/coverage', collectCoverageFrom: [ '/src/renderer/components/**/*.{js,jsx,ts,tsx}', + '/src/main/index.ts', + '/src/main/preload.ts', + '/src/main/security/navigation-guard.ts', '/src/utils/**/*.{js,ts}', '!/src/**/*.d.ts', '!**/node_modules/**', diff --git a/src/main/index.ts b/src/main/index.ts index fc45df4..8b4f0b5 100755 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2,7 +2,7 @@ import fs from 'fs'; import { pathToFileURL } from 'node:url'; import path from 'path'; -import { app, BrowserWindow, dialog, ipcMain, net, protocol } from 'electron'; +import { app, BrowserWindow, dialog, ipcMain, net, protocol, shell } from 'electron'; import { autoUpdater } from 'electron-updater'; import { loadDefaultConfig } from '../utils/config-manager'; @@ -12,6 +12,10 @@ import { TokenCounter } from '../utils/token-counter'; import { getErrorMessage } from './errors'; import { initializeUpdaterFeatureFlags } from './feature-flags'; +import { + isAllowedExternalNavigationUrl, + isAllowedInAppNavigationUrl, +} from './security/navigation-guard'; import { isPathWithinRoot, isPathWithinTempRoot, @@ -122,8 +126,34 @@ let updaterService = createUpdaterService( const APP_ROOT = path.resolve(__dirname, '../../..'); const RENDERER_INDEX_PATH = path.join(APP_ROOT, 'src', 'renderer', 'public', 'index.html'); +const RENDERER_INDEX_URL = pathToFileURL(RENDERER_INDEX_PATH).toString(); const ASSETS_DIR = path.join(APP_ROOT, 'src', 'assets'); const createForbiddenAssetResponse = (): Response => new Response('Forbidden', { status: 403 }); +const isTestEnvironment = process.env.NODE_ENV === 'test'; + +const resolveTestPathOverride = (envKey: string): string | null => { + if (!isTestEnvironment) { + return null; + } + + const configuredPath = process.env[envKey]; + if (typeof configuredPath !== 'string' || configuredPath.trim().length === 0) { + return null; + } + + return path.resolve(configuredPath); +}; + +const openAllowedExternalUrl = (url: string) => { + if (!isAllowedExternalNavigationUrl(url)) { + console.warn(`Blocked external navigation URL: ${url}`); + return; + } + + void shell.openExternal(url).catch((error) => { + console.error(`Failed to open external URL: ${url}`, error); + }); +}; // Set environment const isDevelopment = process.env.NODE_ENV === 'development'; @@ -162,6 +192,20 @@ async function createWindow() { // Hide the menu bar completely in all modes mainWindow.setMenu(null); + mainWindow.webContents.setWindowOpenHandler(({ url }) => { + openAllowedExternalUrl(url); + return { action: 'deny' }; + }); + + mainWindow.webContents.on('will-navigate', (event, url) => { + if (isAllowedInAppNavigationUrl(url, RENDERER_INDEX_URL)) { + return; + } + + event.preventDefault(); + openAllowedExternalUrl(url); + }); + // Load the index.html file if (isDevelopment) { await mainWindow.loadFile(RENDERER_INDEX_PATH); @@ -290,6 +334,12 @@ ipcMain.handle( // Select directory dialog ipcMain.handle('dialog:selectDirectory', async () => { + const testDirectoryPath = resolveTestPathOverride('E2E_DIALOG_DIRECTORY_PATH'); + if (testDirectoryPath) { + authorizedRootPath = testDirectoryPath; + return testDirectoryPath; + } + const { canceled, filePaths } = await dialog.showOpenDialog(mainWindow ?? undefined, { properties: ['openDirectory'], }); @@ -393,6 +443,12 @@ ipcMain.handle( // Save output to file ipcMain.handle('fs:saveFile', async (_event, { content, defaultPath }: SaveFileOptions) => { + const testSavePath = resolveTestPathOverride('E2E_DIALOG_SAVE_PATH'); + if (testSavePath) { + fs.writeFileSync(testSavePath, content); + return testSavePath; + } + const safeDefaultPath = typeof defaultPath === 'string' ? defaultPath : ''; const defaultExtension = safeDefaultPath ? path.extname(safeDefaultPath).toLowerCase() : ''; const filters = diff --git a/src/main/preload.ts b/src/main/preload.ts index b3f24f6..16c8f10 100755 --- a/src/main/preload.ts +++ b/src/main/preload.ts @@ -18,6 +18,16 @@ import type { UpdaterStatus, } from '../types/ipc'; +// Keep preload self-contained: sandboxed preload cannot reliably require local modules. +const isAllowedExternalNavigationUrl = (url: string): boolean => { + try { + const parsedUrl = new URL(url); + return parsedUrl.protocol === 'https:' || parsedUrl.protocol === 'http:'; + } catch { + return false; + } +}; + type DevUtils = { clearLocalStorage: () => boolean; isDev: boolean; @@ -38,7 +48,12 @@ const devUtils: DevUtils = { const electronShellApi: ElectronShellApi = { shell: { - openExternal: (url: string) => shell.openExternal(url), + openExternal: async (url: string) => { + if (!isAllowedExternalNavigationUrl(url)) { + throw new Error(`Blocked external URL: ${url}`); + } + await shell.openExternal(url); + }, }, }; diff --git a/src/main/security/navigation-guard.ts b/src/main/security/navigation-guard.ts new file mode 100644 index 0000000..e685972 --- /dev/null +++ b/src/main/security/navigation-guard.ts @@ -0,0 +1,35 @@ +export const isAllowedExternalNavigationUrl = (url: string): boolean => { + try { + const parsedUrl = new URL(url); + return parsedUrl.protocol === 'https:' || parsedUrl.protocol === 'http:'; + } catch { + return false; + } +}; + +const normalizeFilePathname = (pathname: string): string => { + const driveLetterMatch = pathname.match(/^\/([A-Za-z]):/); + if (!driveLetterMatch) { + return pathname; + } + + const driveLetter = driveLetterMatch[1].toLowerCase(); + return `/${driveLetter}:${pathname.slice(3)}`; +}; + +export const isAllowedInAppNavigationUrl = (url: string, rendererIndexUrl: string): boolean => { + try { + const targetUrl = new URL(url); + const rendererUrl = new URL(rendererIndexUrl); + + if (targetUrl.protocol !== 'file:' || rendererUrl.protocol !== 'file:') { + return false; + } + + return ( + normalizeFilePathname(targetUrl.pathname) === normalizeFilePathname(rendererUrl.pathname) + ); + } catch { + return false; + } +}; diff --git a/tests/catalog.md b/tests/catalog.md index fb43194..3cb5aec 100644 --- a/tests/catalog.md +++ b/tests/catalog.md @@ -57,7 +57,9 @@ Purpose: quick map of what is covered, why it exists, and which command to run. | `tests/unit/main/updater.test.ts` | `src/main/updater.ts` | Alpha/stable channel selection, platform gating, update-check result handling | | `tests/unit/main/updater-smoke.test.ts` | `src/main/updater.ts` | Manual updater-check flow, stable-vs-alpha prerelease assertions, Linux-disabled guard, and structured updater check observability events | | `tests/unit/main/feature-flags.test.ts` | `src/main/feature-flags.ts` | OpenFeature normalization, env/remote merge rules, secure remote fetch behavior | +| `tests/unit/main/navigation-guard.test.ts` | `src/main/security/navigation-guard.ts` | External URL allowlist checks and in-app navigation allow/deny behavior | | `tests/unit/main/path-security.test.ts` | `src/main/security/path-guard.ts` | Root-path authorization, temp-root boundaries, symlink-aware realpath resolution | +| `tests/unit/main/preload.test.ts` | `src/main/preload.ts` | Preload bridge external URL protocol guard for `shell.openExternal` | | `tests/unit/main/provider-connection.test.ts` | `src/main/services/provider-connection.ts` | Provider defaults, URL validation/normalization, request construction, timeout/error handling | | `tests/unit/shared/provider-registry.test.ts` | `src/shared/provider-registry.ts` | Shared provider contract IDs, default base URLs, API-key requirement flags, and supported-provider guards | | `tests/unit/main/directory-tree.test.ts` | `src/main/services/directory-tree.ts` | Exclude/include pattern merge, symlink skip policy, canonical recursion-loop guard, parse-failure fallback | diff --git a/tests/e2e/electron-process-flow.spec.ts b/tests/e2e/electron-process-flow.spec.ts index 8b794c5..b0c736c 100644 --- a/tests/e2e/electron-process-flow.spec.ts +++ b/tests/e2e/electron-process-flow.spec.ts @@ -114,27 +114,6 @@ const createFixtureProject = (browserName: string): string => { return projectDir; }; -const stubNativeDialogs = async ( - electronApp: ElectronApplication, - projectDir: string, - savePath: string -) => { - await electronApp.evaluate( - ({ dialog }, { directoryPath, outputPath }) => { - dialog.showOpenDialog = async () => ({ - canceled: false, - filePaths: [directoryPath], - }); - - dialog.showSaveDialog = async () => ({ - canceled: false, - filePath: outputPath, - }); - }, - { directoryPath: projectDir, outputPath: savePath } - ); -}; - const configureFlowDefaults = async ( page: Page, exportFormat: 'markdown' | 'xml' = 'markdown' @@ -155,8 +134,11 @@ const configureFlowDefaults = async ( const openFixtureProject = async (page: Page, exportFormat: 'markdown' | 'xml' = 'markdown') => { await configureFlowDefaults(page, exportFormat); - await page.getByRole('button', { name: 'Select Folder' }).click(); - await expect(page.getByRole('tab', { name: 'Select Files' })).toHaveAttribute('aria-selected', 'true'); + const selectFolderButton = page.getByRole('button', { name: 'Select Folder' }); + const sourceTab = page.getByRole('tab', { name: 'Select Files' }); + + await selectFolderButton.click(); + await expect(sourceTab).toHaveAttribute('aria-selected', 'true', { timeout: 15_000 }); await expect(page.getByLabel('Select All')).toBeVisible(); }; @@ -224,6 +206,8 @@ const test = base.extend({ NODE_ENV: 'test', ELECTRON_USER_DATA_PATH: userDataDir, ELECTRON_DISABLE_SECURITY_WARNINGS: 'true', + E2E_DIALOG_DIRECTORY_PATH: projectDir, + E2E_DIALOG_SAVE_PATH: savePath, }; if (process.platform === 'linux') { @@ -236,8 +220,6 @@ const test = base.extend({ env: launchEnv, }); - await stubNativeDialogs(electronApp, projectDir, savePath); - await use(electronApp); await electronApp.close(); }, diff --git a/tests/integration/main-process/handlers.test.ts b/tests/integration/main-process/handlers.test.ts index e0b2901..127aca0 100644 --- a/tests/integration/main-process/handlers.test.ts +++ b/tests/integration/main-process/handlers.test.ts @@ -1,4 +1,5 @@ const fs = require('fs'); +const { pathToFileURL } = require('node:url'); const yaml = require('yaml'); const FAKE_GITHUB_TOKEN = ['ghp', 'AAAAAAAAAAAAAAAAAAAAAAAA'].join('_'); @@ -6,6 +7,9 @@ const FAKE_GITHUB_TOKEN = ['ghp', 'AAAAAAAAAAAAAAAAAAAAAAAA'].join('_'); const mockIpcHandlers = {}; const mockProtocolHandlers = {}; const mockNetFetch = jest.fn(); +let latestWindowOpenHandler = null; +let latestWillNavigateHandler = null; +let latestRendererIndexPath = null; const mockAutoUpdater = { checkForUpdates: jest.fn(), setFeedURL: jest.fn(), @@ -31,11 +35,22 @@ jest.mock('electron', () => ({ getVersion: jest.fn().mockReturnValue('0.2.0'), }, BrowserWindow: jest.fn().mockImplementation(() => ({ - loadFile: jest.fn().mockResolvedValue(null), + loadFile: jest.fn().mockImplementation(async (targetPath) => { + latestRendererIndexPath = targetPath; + return null; + }), on: jest.fn(), setMenu: jest.fn(), webContents: { openDevTools: jest.fn(), + setWindowOpenHandler: jest.fn((handler) => { + latestWindowOpenHandler = handler; + }), + on: jest.fn((eventName, handler) => { + if (eventName === 'will-navigate') { + latestWillNavigateHandler = handler; + } + }), }, })), ipcMain: mockIpcMain, @@ -51,6 +66,9 @@ jest.mock('electron', () => ({ net: { fetch: mockNetFetch, }, + shell: { + openExternal: jest.fn().mockResolvedValue(undefined), + }, })); jest.mock('fs'); @@ -328,6 +346,47 @@ describe('Main Process IPC Handlers', () => { }); }); + describe('window navigation guards', () => { + test('should deny window-open requests and only allow http(s) in external opener', () => { + const { shell } = require('electron'); + expect(latestWindowOpenHandler).toBeDefined(); + + expect(latestWindowOpenHandler({ url: 'file:///etc/passwd' })).toEqual({ action: 'deny' }); + expect(shell.openExternal).not.toHaveBeenCalled(); + + expect(latestWindowOpenHandler({ url: 'https://example.com/docs' })).toEqual({ + action: 'deny', + }); + expect(shell.openExternal).toHaveBeenCalledWith('https://example.com/docs'); + }); + + test('should prevent disallowed will-navigate targets and open allowed external urls', () => { + const { shell } = require('electron'); + expect(latestWillNavigateHandler).toBeDefined(); + expect(latestRendererIndexPath).toBeDefined(); + const rendererIndexUrl = pathToFileURL(latestRendererIndexPath).toString(); + + const allowedEvent = { preventDefault: jest.fn() }; + latestWillNavigateHandler(allowedEvent, rendererIndexUrl); + expect(allowedEvent.preventDefault).not.toHaveBeenCalled(); + + const blockedEvent = { preventDefault: jest.fn() }; + latestWillNavigateHandler(blockedEvent, 'https://example.com/security'); + expect(blockedEvent.preventDefault).toHaveBeenCalledTimes(1); + expect(shell.openExternal).toHaveBeenCalledWith('https://example.com/security'); + + const disallowedProtocolEvent = { preventDefault: jest.fn() }; + latestWillNavigateHandler(disallowedProtocolEvent, 'javascript:alert(1)'); + expect(disallowedProtocolEvent.preventDefault).toHaveBeenCalledTimes(1); + expect(shell.openExternal).not.toHaveBeenCalledWith('javascript:alert(1)'); + + const aboutBlankEvent = { preventDefault: jest.fn() }; + latestWillNavigateHandler(aboutBlankEvent, 'about:blank'); + expect(aboutBlankEvent.preventDefault).toHaveBeenCalledTimes(1); + expect(shell.openExternal).not.toHaveBeenCalledWith('about:blank'); + }); + }); + describe('fs:getDirectoryTree', () => { test('should filter directory tree based on config', async () => { // Setup diff --git a/tests/integration/main-process/xml-export-e2e.test.ts b/tests/integration/main-process/xml-export-e2e.test.ts index b27f661..319f094 100644 --- a/tests/integration/main-process/xml-export-e2e.test.ts +++ b/tests/integration/main-process/xml-export-e2e.test.ts @@ -37,6 +37,8 @@ describe('XML export end-to-end', () => { setMenu: jest.fn(), webContents: { openDevTools: jest.fn(), + setWindowOpenHandler: jest.fn(), + on: jest.fn(), }, })), ipcMain: { @@ -51,6 +53,9 @@ describe('XML export end-to-end', () => { net: { fetch: mockNetFetch, }, + shell: { + openExternal: jest.fn().mockResolvedValue(undefined), + }, protocol: { handle: jest.fn(), }, diff --git a/tests/stress/main-process/ipc-latency.stress.test.ts b/tests/stress/main-process/ipc-latency.stress.test.ts index 9f4ab31..054adc4 100644 --- a/tests/stress/main-process/ipc-latency.stress.test.ts +++ b/tests/stress/main-process/ipc-latency.stress.test.ts @@ -26,6 +26,8 @@ jest.mock('electron', () => ({ setMenu: jest.fn(), webContents: { openDevTools: jest.fn(), + setWindowOpenHandler: jest.fn(), + on: jest.fn(), }, })), ipcMain: { @@ -46,6 +48,9 @@ jest.mock('electron', () => ({ net: { fetch: jest.fn().mockResolvedValue({ ok: true, status: 200, url: 'file:///mock/icon.png' }), }, + shell: { + openExternal: jest.fn().mockResolvedValue(undefined), + }, })); jest.mock('fs'); diff --git a/tests/unit/main/navigation-guard.test.ts b/tests/unit/main/navigation-guard.test.ts new file mode 100644 index 0000000..ba43cdf --- /dev/null +++ b/tests/unit/main/navigation-guard.test.ts @@ -0,0 +1,43 @@ +import { pathToFileURL } from 'node:url'; +import path from 'path'; + +import { + isAllowedExternalNavigationUrl, + isAllowedInAppNavigationUrl, +} from '../../../src/main/security/navigation-guard'; + +describe('navigation-guard', () => { + const rendererIndexPath = path.resolve('/workspace/mock-app/src/renderer/public/index.html'); + const rendererIndexUrl = pathToFileURL(rendererIndexPath).toString(); + + test('allows only http and https external URLs', () => { + expect(isAllowedExternalNavigationUrl('https://example.com')).toBe(true); + expect(isAllowedExternalNavigationUrl('http://example.com/docs')).toBe(true); + expect(isAllowedExternalNavigationUrl('file:///etc/passwd')).toBe(false); + expect(isAllowedExternalNavigationUrl('javascript:alert(1)')).toBe(false); + expect(isAllowedExternalNavigationUrl('not-a-url')).toBe(false); + }); + + test('allows renderer index URL variants for in-app navigation', () => { + expect(isAllowedInAppNavigationUrl(rendererIndexUrl, rendererIndexUrl)).toBe(true); + expect( + isAllowedInAppNavigationUrl(`${rendererIndexUrl}?view=source#details`, rendererIndexUrl) + ).toBe(true); + }); + + test('allows renderer index URL when only Windows drive-letter casing differs', () => { + const windowsRendererUrl = 'file:///C:/Users/Alice/app/index.html'; + const windowsTargetUrl = 'file:///c:/Users/Alice/app/index.html?view=source'; + + expect(isAllowedInAppNavigationUrl(windowsTargetUrl, windowsRendererUrl)).toBe(true); + }); + + test('rejects non-index file URLs and external protocols for in-app navigation', () => { + expect( + isAllowedInAppNavigationUrl(pathToFileURL('/workspace/mock-app/other.html').toString(), rendererIndexUrl) + ).toBe(false); + expect(isAllowedInAppNavigationUrl('about:blank', rendererIndexUrl)).toBe(false); + expect(isAllowedInAppNavigationUrl('https://example.com', rendererIndexUrl)).toBe(false); + expect(isAllowedInAppNavigationUrl('not-a-url', rendererIndexUrl)).toBe(false); + }); +}); diff --git a/tests/unit/main/preload.test.ts b/tests/unit/main/preload.test.ts new file mode 100644 index 0000000..8e3ae7f --- /dev/null +++ b/tests/unit/main/preload.test.ts @@ -0,0 +1,51 @@ +describe('preload external URL guard', () => { + const exposeInMainWorld = jest.fn(); + const invoke = jest.fn(); + const openExternal = jest.fn(); + + const loadPreload = () => { + jest.resetModules(); + exposeInMainWorld.mockReset(); + invoke.mockReset(); + openExternal.mockReset().mockResolvedValue(undefined); + + jest.doMock('electron', () => ({ + contextBridge: { + exposeInMainWorld, + }, + ipcRenderer: { + invoke, + }, + shell: { + openExternal, + }, + })); + + require('../../../src/main/preload'); + }; + + const getElectronBridge = () => { + const exposed = exposeInMainWorld.mock.calls.find(([name]) => name === 'electron'); + return exposed?.[1]; + }; + + beforeEach(() => { + loadPreload(); + }); + + test('allows https URLs', async () => { + const electronBridge = getElectronBridge(); + expect(electronBridge?.shell?.openExternal).toBeDefined(); + + await expect(electronBridge.shell.openExternal('https://github.com')).resolves.toBeUndefined(); + expect(openExternal).toHaveBeenCalledWith('https://github.com'); + }); + + test('blocks non-http(s) URLs', async () => { + const electronBridge = getElectronBridge(); + await expect(electronBridge.shell.openExternal('file:///etc/passwd')).rejects.toThrow( + /Blocked external URL/ + ); + expect(openExternal).not.toHaveBeenCalled(); + }); +});