From d6d5ae6182d61db44827b0e21fc83c6793faf6fc Mon Sep 17 00:00:00 2001 From: DarkSky <25152247+darkskygit@users.noreply.github.com> Date: Wed, 18 Mar 2026 14:58:22 +0800 Subject: [PATCH] fix(electron): create doc shortcut should follow default type in settings (#14678) --- .../src/app/effects/events.ts | 5 +- .../src/main/application-menu/create.ts | 2 +- .../src/main/application-menu/index.ts | 4 +- .../src/main/application-menu/subject.ts | 4 +- .../frontend/apps/electron/src/main/events.ts | 3 +- .../apps/electron/src/main/handlers.ts | 5 +- .../frontend/apps/electron/src/main/index.ts | 4 +- .../apps/electron/src/main/protocol.ts | 2 +- .../src/main/security-restrictions.ts | 120 +++++++++--------- .../windows-manager/custom-theme-window.ts | 2 +- .../src/main/windows-manager/main-window.ts | 2 +- .../src/main/windows-manager/onboarding.ts | 2 +- .../src/main/windows-manager/popup.ts | 2 +- .../src/main/windows-manager/tab-views.ts | 2 +- .../apps/electron/src/main/worker/pool.ts | 2 +- .../apps/electron/src/preload/bootstrap.ts | 18 ++- .../internal-origin.ts} | 10 ++ tests/affine-desktop/e2e/basic.spec.ts | 48 +++++++ 18 files changed, 156 insertions(+), 81 deletions(-) rename packages/frontend/apps/electron/src/{main/constants.ts => shared/internal-origin.ts} (76%) diff --git a/packages/frontend/apps/electron-renderer/src/app/effects/events.ts b/packages/frontend/apps/electron-renderer/src/app/effects/events.ts index eed585f5dd..ed16814005 100644 --- a/packages/frontend/apps/electron-renderer/src/app/effects/events.ts +++ b/packages/frontend/apps/electron-renderer/src/app/effects/events.ts @@ -46,7 +46,10 @@ export function setupEvents(frameworkProvider: FrameworkProvider) { const { workspace } = currentWorkspace; const docsService = workspace.scope.get(DocsService); - const page = docsService.createDoc({ primaryMode: type }); + const page = + type === 'default' + ? docsService.createDoc() + : docsService.createDoc({ primaryMode: type }); workspace.scope.get(WorkbenchService).workbench.openDoc(page.id); }) .catch(err => { diff --git a/packages/frontend/apps/electron/src/main/application-menu/create.ts b/packages/frontend/apps/electron/src/main/application-menu/create.ts index fab6debb16..73be8816b3 100644 --- a/packages/frontend/apps/electron/src/main/application-menu/create.ts +++ b/packages/frontend/apps/electron/src/main/application-menu/create.ts @@ -67,7 +67,7 @@ export function createApplicationMenu() { click: async () => { await initAndShowMainWindow(); // fixme: if the window is just created, the new page action will not be triggered - applicationMenuSubjects.newPageAction$.next('page'); + applicationMenuSubjects.newPageAction$.next('default'); }, }, ], diff --git a/packages/frontend/apps/electron/src/main/application-menu/index.ts b/packages/frontend/apps/electron/src/main/application-menu/index.ts index 2a3f1c6bb4..9e0ba86e4c 100644 --- a/packages/frontend/apps/electron/src/main/application-menu/index.ts +++ b/packages/frontend/apps/electron/src/main/application-menu/index.ts @@ -1,5 +1,5 @@ import type { MainEventRegister } from '../type'; -import { applicationMenuSubjects } from './subject'; +import { applicationMenuSubjects, type NewPageAction } from './subject'; export * from './create'; export * from './subject'; @@ -11,7 +11,7 @@ export const applicationMenuEvents = { /** * File -> New Doc */ - onNewPageAction: (fn: (type: 'page' | 'edgeless') => void) => { + onNewPageAction: (fn: (type: NewPageAction) => void) => { const sub = applicationMenuSubjects.newPageAction$.subscribe(fn); return () => { sub.unsubscribe(); diff --git a/packages/frontend/apps/electron/src/main/application-menu/subject.ts b/packages/frontend/apps/electron/src/main/application-menu/subject.ts index ae37013a1f..7c62d6a1a3 100644 --- a/packages/frontend/apps/electron/src/main/application-menu/subject.ts +++ b/packages/frontend/apps/electron/src/main/application-menu/subject.ts @@ -1,7 +1,9 @@ import { Subject } from 'rxjs'; +export type NewPageAction = 'page' | 'edgeless' | 'default'; + export const applicationMenuSubjects = { - newPageAction$: new Subject<'page' | 'edgeless'>(), + newPageAction$: new Subject(), openJournal$: new Subject(), openInSettingModal$: new Subject<{ activeTab: string; diff --git a/packages/frontend/apps/electron/src/main/events.ts b/packages/frontend/apps/electron/src/main/events.ts index 41a746eb67..eee308a69f 100644 --- a/packages/frontend/apps/electron/src/main/events.ts +++ b/packages/frontend/apps/electron/src/main/events.ts @@ -9,6 +9,7 @@ import { beforeAppQuit } from './cleanup'; import { logger } from './logger'; import { powerEvents } from './power'; import { recordingEvents } from './recording'; +import { checkSource } from './security-restrictions'; import { sharedStorageEvents } from './shared-storage'; import { uiEvents } from './ui/events'; import { updaterEvents } from './updater/event'; @@ -70,7 +71,7 @@ export function registerEvents() { action: 'subscribe' | 'unsubscribe', channel: string ) => { - if (typeof channel !== 'string') return; + if (!checkSource(event) || typeof channel !== 'string') return; if (action === 'subscribe') { addSubscription(event.sender, channel); if (channel === 'power:power-source') { diff --git a/packages/frontend/apps/electron/src/main/handlers.ts b/packages/frontend/apps/electron/src/main/handlers.ts index 17cb2d7615..b8f75ff568 100644 --- a/packages/frontend/apps/electron/src/main/handlers.ts +++ b/packages/frontend/apps/electron/src/main/handlers.ts @@ -7,6 +7,7 @@ import { configStorageHandlers } from './config-storage'; import { findInPageHandlers } from './find-in-page'; import { getLogFilePath, logger, revealLogFile } from './logger'; import { recordingHandlers } from './recording'; +import { checkSource } from './security-restrictions'; import { sharedStorageHandlers } from './shared-storage'; import { uiHandlers } from './ui/handlers'; import { updaterHandlers } from './updater'; @@ -49,7 +50,7 @@ export const registerHandlers = () => { ...args: any[] ) => { // args[0] is the `{namespace:key}` - if (typeof args[0] !== 'string') { + if (!checkSource(e) || typeof args[0] !== 'string') { logger.error('invalid ipc message', args); return; } @@ -97,6 +98,8 @@ export const registerHandlers = () => { }); ipcMain.on(AFFINE_API_CHANNEL_NAME, (e, ...args: any[]) => { + if (!checkSource(e)) return; + handleIpcMessage(e, ...args) .then(ret => { e.returnValue = ret; diff --git a/packages/frontend/apps/electron/src/main/index.ts b/packages/frontend/apps/electron/src/main/index.ts index a6992bdf44..833bd610f9 100644 --- a/packages/frontend/apps/electron/src/main/index.ts +++ b/packages/frontend/apps/electron/src/main/index.ts @@ -1,5 +1,3 @@ -import './security-restrictions'; - import path from 'node:path'; import * as Sentry from '@sentry/electron/main'; @@ -15,6 +13,7 @@ import { registerHandlers } from './handlers'; import { logger } from './logger'; import { registerProtocol } from './protocol'; import { setupRecordingFeature } from './recording/feature'; +import { registerSecurityRestrictions } from './security-restrictions'; import { setupTrayState } from './tray'; import { registerUpdater } from './updater'; import { launch } from './windows-manager/launcher'; @@ -105,6 +104,7 @@ app.on('activate', () => { }); setupDeepLink(app); +registerSecurityRestrictions(); /** * Create app window when background process will be ready diff --git a/packages/frontend/apps/electron/src/main/protocol.ts b/packages/frontend/apps/electron/src/main/protocol.ts index 564c1396c7..0bfe51a975 100644 --- a/packages/frontend/apps/electron/src/main/protocol.ts +++ b/packages/frontend/apps/electron/src/main/protocol.ts @@ -4,9 +4,9 @@ import { pathToFileURL } from 'node:url'; import { app, net, protocol, session } from 'electron'; import cookieParser from 'set-cookie-parser'; +import { anotherHost, mainHost } from '../shared/internal-origin'; import { isWindows, resourcesPath } from '../shared/utils'; import { buildType, isDev } from './config'; -import { anotherHost, mainHost } from './constants'; import { logger } from './logger'; const webStaticDir = join(resourcesPath, 'web-static'); diff --git a/packages/frontend/apps/electron/src/main/security-restrictions.ts b/packages/frontend/apps/electron/src/main/security-restrictions.ts index 4c28fb2e96..f507c74f7e 100644 --- a/packages/frontend/apps/electron/src/main/security-restrictions.ts +++ b/packages/frontend/apps/electron/src/main/security-restrictions.ts @@ -1,71 +1,71 @@ import { app } from 'electron'; -import { anotherHost, mainHost } from './constants'; +import { isInternalUrl } from '../shared/internal-origin'; +import { logger } from './logger'; import { openExternalSafely } from './security/open-external'; import { validateRedirectProxyUrl } from './security/redirect-proxy'; -app.on('web-contents-created', (_, contents) => { - const isInternalUrl = (url: string) => { - try { - const parsed = new URL(url); - if ( - parsed.protocol === 'assets:' && - (parsed.hostname === mainHost || parsed.hostname === anotherHost) - ) { - return true; - } - } catch {} - return false; - }; - /** - * Block navigation to origins not on the allowlist. - * - * Navigation is a common attack vector. If an attacker can convince the app to navigate away - * from its current page, they can possibly force the app to open web sites on the Internet. - * - * @see https://www.electronjs.org/docs/latest/tutorial/security#13-disable-or-limit-navigation - */ - contents.on('will-navigate', (event, url) => { - if (isInternalUrl(url)) { - return; - } - // Prevent navigation - event.preventDefault(); - openExternalSafely(url).catch(error => { - console.error('[security] Failed to open external URL:', error); - }); - }); +export const checkSource = ( + e: Electron.IpcMainInvokeEvent | Electron.IpcMainEvent +) => { + const url = e.senderFrame?.url || e.sender.getURL(); + const result = isInternalUrl(url); + if (!result) logger.error('invalid source', url); + return result; +}; - /** - * Hyperlinks to allowed sites open in the default browser. - * - * The creation of new `webContents` is a common attack vector. Attackers attempt to convince the app to create new windows, - * frames, or other renderer processes with more privileges than they had before; or with pages opened that they couldn't open before. - * You should deny any unexpected window creation. - * - * @see https://www.electronjs.org/docs/latest/tutorial/security#14-disable-or-limit-creation-of-new-windows - * @see https://www.electronjs.org/docs/latest/tutorial/security#15-do-not-use-openexternal-with-untrusted-content - */ - contents.setWindowOpenHandler(({ url }) => { - if (!isInternalUrl(url)) { +export const registerSecurityRestrictions = () => { + app.on('web-contents-created', (_, contents) => { + /** + * Block navigation to origins not on the allowlist. + * + * Navigation is a common attack vector. If an attacker can convince the app to navigate away + * from its current page, they can possibly force the app to open web sites on the Internet. + * + * @see https://www.electronjs.org/docs/latest/tutorial/security#13-disable-or-limit-navigation + */ + contents.on('will-navigate', (event, url) => { + if (isInternalUrl(url)) { + return; + } + // Prevent navigation + event.preventDefault(); openExternalSafely(url).catch(error => { console.error('[security] Failed to open external URL:', error); }); - } else if (url.includes('/redirect-proxy')) { - const result = validateRedirectProxyUrl(url); - if (!result.allow) { - console.warn( - `[security] Blocked redirect proxy: ${result.reason}`, - result.redirectTarget ?? url - ); - return { action: 'deny' }; - } + }); - openExternalSafely(result.redirectTarget).catch(error => { - console.error('[security] Failed to open external URL:', error); - }); - } - // Prevent creating new window in application - return { action: 'deny' }; + /** + * Hyperlinks to allowed sites open in the default browser. + * + * The creation of new `webContents` is a common attack vector. Attackers attempt to convince the app to create new windows, + * frames, or other renderer processes with more privileges than they had before; or with pages opened that they couldn't open before. + * You should deny any unexpected window creation. + * + * @see https://www.electronjs.org/docs/latest/tutorial/security#14-disable-or-limit-creation-of-new-windows + * @see https://www.electronjs.org/docs/latest/tutorial/security#15-do-not-use-openexternal-with-untrusted-content + */ + contents.setWindowOpenHandler(({ url }) => { + if (!isInternalUrl(url)) { + openExternalSafely(url).catch(error => { + console.error('[security] Failed to open external URL:', error); + }); + } else if (url.includes('/redirect-proxy')) { + const result = validateRedirectProxyUrl(url); + if (!result.allow) { + console.warn( + `[security] Blocked redirect proxy: ${result.reason}`, + result.redirectTarget ?? url + ); + return { action: 'deny' }; + } + + openExternalSafely(result.redirectTarget).catch(error => { + console.error('[security] Failed to open external URL:', error); + }); + } + // Prevent creating new window in application + return { action: 'deny' }; + }); }); -}); +}; diff --git a/packages/frontend/apps/electron/src/main/windows-manager/custom-theme-window.ts b/packages/frontend/apps/electron/src/main/windows-manager/custom-theme-window.ts index 01bb5e29ce..0b9587e901 100644 --- a/packages/frontend/apps/electron/src/main/windows-manager/custom-theme-window.ts +++ b/packages/frontend/apps/electron/src/main/windows-manager/custom-theme-window.ts @@ -2,8 +2,8 @@ import { join } from 'node:path'; import { BrowserWindow, type Display, screen } from 'electron'; +import { customThemeViewUrl } from '../../shared/internal-origin'; import { isMacOS } from '../../shared/utils'; -import { customThemeViewUrl } from '../constants'; import { logger } from '../logger'; import { buildWebPreferences } from '../web-preferences'; diff --git a/packages/frontend/apps/electron/src/main/windows-manager/main-window.ts b/packages/frontend/apps/electron/src/main/windows-manager/main-window.ts index 066010e735..5efa1cf0bf 100644 --- a/packages/frontend/apps/electron/src/main/windows-manager/main-window.ts +++ b/packages/frontend/apps/electron/src/main/windows-manager/main-window.ts @@ -4,10 +4,10 @@ import { BrowserWindow, nativeTheme } from 'electron'; import electronWindowState from 'electron-window-state'; import { BehaviorSubject, map, shareReplay } from 'rxjs'; +import { mainWindowOrigin } from '../../shared/internal-origin'; import { isLinux, isMacOS, isWindows, resourcesPath } from '../../shared/utils'; import { beforeAppQuit } from '../cleanup'; import { buildType } from '../config'; -import { mainWindowOrigin } from '../constants'; import { ensureHelperProcess } from '../helper-process'; import { logger } from '../logger'; import { MenubarStateKey, MenubarStateSchema } from '../shared-state-schema'; diff --git a/packages/frontend/apps/electron/src/main/windows-manager/onboarding.ts b/packages/frontend/apps/electron/src/main/windows-manager/onboarding.ts index 1c02e58de4..be100e7bab 100644 --- a/packages/frontend/apps/electron/src/main/windows-manager/onboarding.ts +++ b/packages/frontend/apps/electron/src/main/windows-manager/onboarding.ts @@ -2,8 +2,8 @@ import { join } from 'node:path'; import { BrowserWindow, screen } from 'electron'; +import { onboardingViewUrl } from '../../shared/internal-origin'; import { isDev } from '../config'; -import { onboardingViewUrl } from '../constants'; // import { getExposedMeta } from './exposed'; import { logger } from '../logger'; import { buildWebPreferences } from '../web-preferences'; diff --git a/packages/frontend/apps/electron/src/main/windows-manager/popup.ts b/packages/frontend/apps/electron/src/main/windows-manager/popup.ts index d856aa6086..d189fef2c2 100644 --- a/packages/frontend/apps/electron/src/main/windows-manager/popup.ts +++ b/packages/frontend/apps/electron/src/main/windows-manager/popup.ts @@ -8,7 +8,7 @@ import { } from 'electron'; import { BehaviorSubject } from 'rxjs'; -import { popupViewUrl } from '../constants'; +import { popupViewUrl } from '../../shared/internal-origin'; import { logger } from '../logger'; import type { MainEventRegister, NamespaceHandlers } from '../type'; import { buildWebPreferences } from '../web-preferences'; diff --git a/packages/frontend/apps/electron/src/main/windows-manager/tab-views.ts b/packages/frontend/apps/electron/src/main/windows-manager/tab-views.ts index 39b90db000..1fe982d342 100644 --- a/packages/frontend/apps/electron/src/main/windows-manager/tab-views.ts +++ b/packages/frontend/apps/electron/src/main/windows-manager/tab-views.ts @@ -24,9 +24,9 @@ import { type Unsubscribable, } from 'rxjs'; +import { mainWindowOrigin, shellViewUrl } from '../../shared/internal-origin'; import { isMacOS } from '../../shared/utils'; import { beforeAppQuit, onTabClose } from '../cleanup'; -import { mainWindowOrigin, shellViewUrl } from '../constants'; import { ensureHelperProcess } from '../helper-process'; import { logger } from '../logger'; import { diff --git a/packages/frontend/apps/electron/src/main/worker/pool.ts b/packages/frontend/apps/electron/src/main/worker/pool.ts index 4a8ec3c7c3..14ed2744fa 100644 --- a/packages/frontend/apps/electron/src/main/worker/pool.ts +++ b/packages/frontend/apps/electron/src/main/worker/pool.ts @@ -2,7 +2,7 @@ import { join } from 'node:path'; import { BrowserWindow, MessageChannelMain, type WebContents } from 'electron'; -import { backgroundWorkerViewUrl } from '../constants'; +import { backgroundWorkerViewUrl } from '../../shared/internal-origin'; import { ensureHelperProcess } from '../helper-process'; import { logger } from '../logger'; import { buildWebPreferences } from '../web-preferences'; diff --git a/packages/frontend/apps/electron/src/preload/bootstrap.ts b/packages/frontend/apps/electron/src/preload/bootstrap.ts index 2aee63cc20..ae31f1c493 100644 --- a/packages/frontend/apps/electron/src/preload/bootstrap.ts +++ b/packages/frontend/apps/electron/src/preload/bootstrap.ts @@ -2,13 +2,21 @@ import '@sentry/electron/preload'; import { contextBridge } from 'electron'; +import { isInternalUrl } from '../shared/internal-origin'; import { apis, appInfo, events } from './electron-api'; import { sharedStorage } from './shared-storage'; import { listenWorkerApis } from './worker'; -contextBridge.exposeInMainWorld('__appInfo', appInfo); -contextBridge.exposeInMainWorld('__apis', apis); -contextBridge.exposeInMainWorld('__events', events); -contextBridge.exposeInMainWorld('__sharedStorage', sharedStorage); +const locationLike = (globalThis as { location?: { href?: unknown } }).location; -listenWorkerApis(); +const currentUrl = + typeof locationLike?.href === 'string' ? locationLike.href : null; + +if (currentUrl && isInternalUrl(currentUrl)) { + contextBridge.exposeInMainWorld('__appInfo', appInfo); + contextBridge.exposeInMainWorld('__apis', apis); + contextBridge.exposeInMainWorld('__events', events); + contextBridge.exposeInMainWorld('__sharedStorage', sharedStorage); + + listenWorkerApis(); +} diff --git a/packages/frontend/apps/electron/src/main/constants.ts b/packages/frontend/apps/electron/src/shared/internal-origin.ts similarity index 76% rename from packages/frontend/apps/electron/src/main/constants.ts rename to packages/frontend/apps/electron/src/shared/internal-origin.ts index e4ed7c0077..6d68dbf380 100644 --- a/packages/frontend/apps/electron/src/main/constants.ts +++ b/packages/frontend/apps/electron/src/shared/internal-origin.ts @@ -1,5 +1,6 @@ export const mainHost = '.'; export const anotherHost = 'another-host'; +export const internalHosts = new Set([mainHost, anotherHost]); export const mainWindowOrigin = `assets://${mainHost}`; export const anotherOrigin = `assets://${anotherHost}`; @@ -13,3 +14,12 @@ export const customThemeViewUrl = `${mainWindowOrigin}/theme-editor`; // Notes from electron official docs: // "The zoom policy at the Chromium level is same-origin, meaning that the zoom level for a specific domain propagates across all instances of windows with the same domain. Differentiating the window URLs will make zoom work per-window." export const popupViewUrl = `${anotherOrigin}/popup.html`; + +export const isInternalUrl = (url: string) => { + try { + const parsed = new URL(url); + return parsed.protocol === 'assets:' && internalHosts.has(parsed.hostname); + } catch { + return false; + } +}; diff --git a/tests/affine-desktop/e2e/basic.spec.ts b/tests/affine-desktop/e2e/basic.spec.ts index 9dcfb97ab3..9e837e7bea 100644 --- a/tests/affine-desktop/e2e/basic.spec.ts +++ b/tests/affine-desktop/e2e/basic.spec.ts @@ -1,7 +1,12 @@ import { test } from '@affine-test/kit/electron'; +import { + ensureInEdgelessMode, + ensureInPageMode, +} from '@affine-test/kit/utils/editor'; import { clickNewPageButton, getBlockSuiteEditorTitle, + waitForEditorLoad, } from '@affine-test/kit/utils/page-logic'; import { clickSideBarSettingButton } from '@affine-test/kit/utils/sidebar'; import { createLocalWorkspace } from '@affine-test/kit/utils/workspace'; @@ -14,12 +19,55 @@ const historyShortcut = async (page: Page, command: 'goBack' | 'goForward') => { ); }; +const setNewDocDefaultMode = async ( + page: Page, + mode: 'page' | 'edgeless' | 'ask' +) => { + const modeTriggerByValue = { + page: 'page-mode-trigger', + edgeless: 'edgeless-mode-trigger', + ask: 'ask-every-time-trigger', + } as const; + + await clickSideBarSettingButton(page); + await page.getByTestId('editor-panel-trigger').click(); + await page.getByTestId('new-doc-default-mode-trigger').click(); + await page.getByTestId(modeTriggerByValue[mode]).click(); + await page.getByTestId('modal-close-button').click(); +}; + test('new page', async ({ page, workspace }) => { await clickNewPageButton(page); const flavour = (await workspace.current()).meta.flavour; expect(flavour).toBe('local'); }); +test('application menu respects default new doc mode', async ({ + electronApp, + page, +}) => { + await waitForEditorLoad(page); + await ensureInPageMode(page); + + await setNewDocDefaultMode(page, 'edgeless'); + await electronApp.evaluate(({ BrowserWindow, Menu }) => { + const menuItem = + Menu.getApplicationMenu()?.getMenuItemById('affine:new-page'); + const focusedWindow = BrowserWindow.getFocusedWindow(); + + if (!menuItem) { + throw new Error('Missing application menu item: affine:new-page'); + } + if (!focusedWindow) { + throw new Error('Missing focused window for application menu dispatch'); + } + + menuItem.click(undefined, focusedWindow, focusedWindow.webContents); + }); + + await ensureInEdgelessMode(page); +}); + test('app sidebar router forward/back', async ({ page }) => { // create pages await page.waitForTimeout(500);