[codex] Fix desktop default-browser external links (#420)
* fix desktop default-browser external links * fix desktop navigation guard review follow-up
This commit is contained in:
+14
-1
@@ -22,6 +22,7 @@ import {
|
||||
waitForServerReady,
|
||||
} from './runtime.js';
|
||||
import { getDesktopRuntimeIconPath, getDesktopTrayIconPath } from './iconAssets.js';
|
||||
import { attachDesktopNavigationGuard, createSafeOpenExternal } from './navigationGuard.js';
|
||||
|
||||
const { autoUpdater } = electronUpdater;
|
||||
|
||||
@@ -138,6 +139,7 @@ function setupTray() {
|
||||
|
||||
async function createMainWindow() {
|
||||
if (mainWindow) return mainWindow;
|
||||
const frontendUrl = resolveFrontendUrl();
|
||||
|
||||
mainWindow = new BrowserWindow({
|
||||
width: 1480,
|
||||
@@ -153,6 +155,17 @@ async function createMainWindow() {
|
||||
},
|
||||
});
|
||||
|
||||
attachDesktopNavigationGuard({
|
||||
appUrl: frontendUrl,
|
||||
openExternal: createSafeOpenExternal({
|
||||
openExternal: (url) => shell.openExternal(url),
|
||||
onError: (url, error) => {
|
||||
log.error('Failed to open external URL', { url, error });
|
||||
},
|
||||
}),
|
||||
webContents: mainWindow.webContents,
|
||||
});
|
||||
|
||||
mainWindow.on('close', (event) => {
|
||||
if (!isQuitting) {
|
||||
event.preventDefault();
|
||||
@@ -164,7 +177,7 @@ async function createMainWindow() {
|
||||
mainWindow?.show();
|
||||
});
|
||||
|
||||
await mainWindow.loadURL(resolveFrontendUrl());
|
||||
await mainWindow.loadURL(frontendUrl);
|
||||
return mainWindow;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import {
|
||||
attachDesktopNavigationGuard,
|
||||
createSafeOpenExternal,
|
||||
resolveDesktopNavigationAction,
|
||||
type DesktopWebContentsLike,
|
||||
type DesktopWillNavigateEvent,
|
||||
type DesktopWindowOpenDetails,
|
||||
} from './navigationGuard.js';
|
||||
|
||||
function createWebContentsHarness() {
|
||||
let openHandler: ((details: DesktopWindowOpenDetails) => { action: 'allow' | 'deny' }) | null = null;
|
||||
let navigateHandler: ((event: DesktopWillNavigateEvent, url: string) => void) | null = null;
|
||||
|
||||
const webContents: DesktopWebContentsLike = {
|
||||
setWindowOpenHandler: (handler) => {
|
||||
openHandler = handler;
|
||||
},
|
||||
on: (event, listener) => {
|
||||
if (event === 'will-navigate') {
|
||||
navigateHandler = listener;
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
return {
|
||||
webContents,
|
||||
getOpenHandler() {
|
||||
if (!openHandler) throw new Error('window open handler not registered');
|
||||
return openHandler;
|
||||
},
|
||||
getNavigateHandler() {
|
||||
if (!navigateHandler) throw new Error('will-navigate handler not registered');
|
||||
return navigateHandler;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('desktop navigation guard', () => {
|
||||
const appUrl = 'http://127.0.0.1:5173/dashboard';
|
||||
|
||||
it('routes cross-origin popup links to the default browser', () => {
|
||||
expect(resolveDesktopNavigationAction('https://github.com/cita-777/metapi', appUrl)).toBe('deny');
|
||||
});
|
||||
|
||||
it('keeps same-origin popup links inside the desktop app', () => {
|
||||
expect(resolveDesktopNavigationAction('/monitor-proxy/ldoh/', appUrl)).toBe('allow');
|
||||
expect(resolveDesktopNavigationAction('about:blank', appUrl)).toBe('allow');
|
||||
});
|
||||
|
||||
it('denies malformed navigation targets by default', () => {
|
||||
expect(resolveDesktopNavigationAction('https://example.com', 'not a valid app url')).toBe('deny');
|
||||
expect(resolveDesktopNavigationAction('http://[::1', appUrl)).toBe('deny');
|
||||
});
|
||||
|
||||
it('opens cross-origin window.open targets externally and denies the popup', () => {
|
||||
const openExternal = vi.fn();
|
||||
const harness = createWebContentsHarness();
|
||||
|
||||
attachDesktopNavigationGuard({
|
||||
appUrl,
|
||||
openExternal,
|
||||
webContents: harness.webContents,
|
||||
});
|
||||
|
||||
const result = harness.getOpenHandler()({
|
||||
url: 'https://metapi.cita777.me',
|
||||
});
|
||||
|
||||
expect(result).toEqual({ action: 'deny' });
|
||||
expect(openExternal).toHaveBeenCalledWith('https://metapi.cita777.me');
|
||||
});
|
||||
|
||||
it('prevents same-window cross-origin navigations and opens them externally', () => {
|
||||
const openExternal = vi.fn();
|
||||
const preventDefault = vi.fn();
|
||||
const harness = createWebContentsHarness();
|
||||
|
||||
attachDesktopNavigationGuard({
|
||||
appUrl,
|
||||
openExternal,
|
||||
webContents: harness.webContents,
|
||||
});
|
||||
|
||||
harness.getNavigateHandler()({ preventDefault }, 'https://example.com/docs');
|
||||
|
||||
expect(preventDefault).toHaveBeenCalledTimes(1);
|
||||
expect(openExternal).toHaveBeenCalledWith('https://example.com/docs');
|
||||
});
|
||||
|
||||
it('does not block same-origin navigations', () => {
|
||||
const openExternal = vi.fn();
|
||||
const preventDefault = vi.fn();
|
||||
const harness = createWebContentsHarness();
|
||||
|
||||
attachDesktopNavigationGuard({
|
||||
appUrl,
|
||||
openExternal,
|
||||
webContents: harness.webContents,
|
||||
});
|
||||
|
||||
harness.getNavigateHandler()({ preventDefault }, 'http://127.0.0.1:5173/monitor-proxy/ldoh/');
|
||||
|
||||
expect(preventDefault).not.toHaveBeenCalled();
|
||||
expect(openExternal).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('reports external-open failures instead of leaving a rejected promise behind', async () => {
|
||||
const error = new Error('failed to open');
|
||||
const onError = vi.fn();
|
||||
const safeOpenExternal = createSafeOpenExternal({
|
||||
openExternal: vi.fn().mockRejectedValue(error),
|
||||
onError,
|
||||
});
|
||||
|
||||
safeOpenExternal('https://example.com/docs');
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(onError).toHaveBeenCalledWith('https://example.com/docs', error);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,81 @@
|
||||
export type DesktopWindowOpenAction = 'allow' | 'deny';
|
||||
|
||||
export type DesktopWindowOpenDetails = {
|
||||
url: string;
|
||||
};
|
||||
|
||||
export type DesktopWillNavigateEvent = {
|
||||
preventDefault: () => void;
|
||||
};
|
||||
|
||||
export type DesktopWebContentsLike = {
|
||||
setWindowOpenHandler: (
|
||||
handler: (details: DesktopWindowOpenDetails) => { action: DesktopWindowOpenAction },
|
||||
) => void;
|
||||
on: (
|
||||
event: 'will-navigate',
|
||||
listener: (event: DesktopWillNavigateEvent, url: string) => void,
|
||||
) => void;
|
||||
};
|
||||
|
||||
export type AttachDesktopNavigationGuardInput = {
|
||||
appUrl: string;
|
||||
openExternal: (url: string) => void;
|
||||
webContents: DesktopWebContentsLike;
|
||||
};
|
||||
|
||||
export type CreateSafeOpenExternalInput = {
|
||||
openExternal: (url: string) => Promise<void>;
|
||||
onError: (url: string, error: unknown) => void;
|
||||
};
|
||||
|
||||
export function resolveDesktopNavigationAction(
|
||||
targetUrl: string,
|
||||
appUrl: string,
|
||||
): DesktopWindowOpenAction {
|
||||
let appLocation: URL;
|
||||
let targetLocation: URL;
|
||||
|
||||
try {
|
||||
appLocation = new URL(appUrl);
|
||||
targetLocation = new URL(targetUrl, appLocation);
|
||||
} catch {
|
||||
return 'deny';
|
||||
}
|
||||
|
||||
if (targetLocation.protocol === 'about:') {
|
||||
return 'allow';
|
||||
}
|
||||
|
||||
return targetLocation.origin === appLocation.origin ? 'allow' : 'deny';
|
||||
}
|
||||
|
||||
export function createSafeOpenExternal(
|
||||
input: CreateSafeOpenExternalInput,
|
||||
): (url: string) => void {
|
||||
return (url: string) => {
|
||||
void input.openExternal(url).catch((error) => {
|
||||
input.onError(url, error);
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
export function attachDesktopNavigationGuard(
|
||||
input: AttachDesktopNavigationGuardInput,
|
||||
): void {
|
||||
input.webContents.setWindowOpenHandler(({ url }) => {
|
||||
const action = resolveDesktopNavigationAction(url, input.appUrl);
|
||||
if (action === 'deny') {
|
||||
void input.openExternal(url);
|
||||
}
|
||||
return { action };
|
||||
});
|
||||
|
||||
input.webContents.on('will-navigate', (event, url) => {
|
||||
if (resolveDesktopNavigationAction(url, input.appUrl) === 'allow') {
|
||||
return;
|
||||
}
|
||||
event.preventDefault();
|
||||
void input.openExternal(url);
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user