diff --git a/src/main/window/privileged-window-navigation.test.ts b/src/main/window/privileged-window-navigation.test.ts new file mode 100644 index 00000000000..579d830f244 --- /dev/null +++ b/src/main/window/privileged-window-navigation.test.ts @@ -0,0 +1,96 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const openExternal = vi.fn() +vi.mock('electron', () => ({ + shell: { openExternal: (url: string) => openExternal(url) } +})) + +import { installPrivilegedWindowNavigationPolicy } from './privileged-window-navigation' + +describe('privileged window navigation policy', () => { + function createFixture(currentUrl: string) { + const handlers = new Map void>() + const contents = { + getURL: () => currentUrl, + setWindowOpenHandler: vi.fn(), + on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + handlers.set(event, handler) + }) + } + installPrivilegedWindowNavigationPolicy(contents as never) + return { + willNavigate(url: string) { + const event = { preventDefault: vi.fn() } + const handler = handlers.get('will-navigate') + // Why: the allow-cases assert preventDefault was *not* called, so a missing + // handler would pass them vacuously. + if (!handler) { + throw new Error('no will-navigate handler was registered') + } + handler(event, url) + return event + } + } + } + + beforeEach(() => { + openExternal.mockClear() + }) + + it('lets the packaged renderer document reload itself', () => { + const appUrl = + 'file:///Applications/Orca.app/Contents/Resources/app.asar/out/renderer/index.html' + const event = createFixture(appUrl).willNavigate(appUrl) + + expect(event.preventDefault).not.toHaveBeenCalled() + expect(openExternal).not.toHaveBeenCalled() + }) + + it('still blocks and hands off an external http target', () => { + const appUrl = + 'file:///Applications/Orca.app/Contents/Resources/app.asar/out/renderer/index.html' + const event = createFixture(appUrl).willNavigate('https://example.com/') + + expect(event.preventDefault).toHaveBeenCalled() + expect(openExternal).toHaveBeenCalledWith('https://example.com/') + }) + + it('lets the dev renderer origin navigate but blocks foreign documents', () => { + const fixture = createFixture('http://localhost:5173/') + + expect( + fixture.willNavigate('http://localhost:5173/index.html').preventDefault + ).not.toHaveBeenCalled() + expect(fixture.willNavigate('https://example.com/').preventDefault).toHaveBeenCalled() + expect( + fixture.willNavigate('blob:http://localhost:5173/attacker-document').preventDefault + ).toHaveBeenCalled() + }) + + // Why: the packaged file: path is the whole privilege boundary, so a foreign host or a + // non-file scheme that reuses it must not read as "our own document". + it('blocks a foreign file host and a data: URL that reuse the renderer path', () => { + const appUrl = + 'file:///Applications/Orca.app/Contents/Resources/app.asar/out/renderer/index.html' + const fixture = createFixture(appUrl) + + expect( + fixture.willNavigate( + 'file://evil.example/Applications/Orca.app/Contents/Resources/app.asar/out/renderer/index.html' + ).preventDefault + ).toHaveBeenCalled() + expect( + fixture.willNavigate('data:text/html,').preventDefault + ).toHaveBeenCalled() + expect(openExternal).not.toHaveBeenCalled() + }) + + it('still blocks navigation to an unrelated local file', () => { + const appUrl = + 'file:///Applications/Orca.app/Contents/Resources/app.asar/out/renderer/index.html' + const event = createFixture(appUrl).willNavigate('file:///Users/someone/.ssh/id_rsa') + + expect(event.preventDefault).toHaveBeenCalled() + expect(openExternal).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/window/privileged-window-navigation.ts b/src/main/window/privileged-window-navigation.ts index a5afbbe0a98..d11268f0340 100644 --- a/src/main/window/privileged-window-navigation.ts +++ b/src/main/window/privileged-window-navigation.ts @@ -1,6 +1,6 @@ import { shell, type WebContents } from 'electron' -import { is } from '@electron-toolkit/utils' import { normalizeExternalBrowserUrl } from '../../shared/browser-url' +import { isRendererDocumentNavigation } from './renderer-document-navigation' /** Keep remote documents from inheriting an Orca window's privileged preload. */ export function installPrivilegedWindowNavigationPolicy(contents: WebContents): void { @@ -13,19 +13,13 @@ export function installPrivilegedWindowNavigationPolicy(contents: WebContents): }) contents.on('will-navigate', (event, url) => { + // Why: location.reload() is a renderer-initiated navigation, so blocking it here + // silently kills the lazy-chunk recovery reload with no unload-prevented signal. + if (isRendererDocumentNavigation(contents.getURL(), url)) { + return + } const externalUrl = normalizeExternalBrowserUrl(url) if (externalUrl) { - if (is.dev && process.env.ELECTRON_RENDERER_URL) { - try { - const target = new URL(externalUrl) - const allowed = new URL(process.env.ELECTRON_RENDERER_URL) - if (target.origin === allowed.origin) { - return - } - } catch { - // Fall through and block malformed navigation targets. - } - } void shell.openExternal(externalUrl) } event.preventDefault() diff --git a/src/main/window/renderer-document-navigation.ts b/src/main/window/renderer-document-navigation.ts index 4179e24a511..50fb128b633 100644 --- a/src/main/window/renderer-document-navigation.ts +++ b/src/main/window/renderer-document-navigation.ts @@ -1,6 +1,12 @@ import type { WebContents } from 'electron' -function isRendererDocumentNavigation(currentUrl: string, nextUrl: string): boolean { +/** + * True when the target stays inside the window's own privileged document: for `file:` + * the exact same host+path (query/hash may differ, as a reload keeps them), for http(s) + * the same origin. Also gates preload privilege in installPrivilegedWindowNavigationPolicy, + * so loosening it past a same-origin document hands a foreign page the Orca bridge. + */ +export function isRendererDocumentNavigation(currentUrl: string, nextUrl: string): boolean { try { const current = new URL(currentUrl) const next = new URL(nextUrl) @@ -13,6 +19,7 @@ function isRendererDocumentNavigation(currentUrl: string, nextUrl: string): bool } return ( (current.protocol === 'http:' || current.protocol === 'https:') && + (next.protocol === 'http:' || next.protocol === 'https:') && next.origin === current.origin ) } catch {