From ba80cf9ee9506bb4b0c31a6eafd65eb09b83a488 Mon Sep 17 00:00:00 2001 From: Adam Low Date: Mon, 15 Dec 2025 13:11:56 +0000 Subject: [PATCH 1/6] feat: enable context isolation for main window - Enable contextIsolation: true in main window webPreferences - Refactor preload-app.ts to use contextBridge.exposeInMainWorld() - Update all renderer code to use window.electronAPI.* instead of direct window properties - Fix locale module to lazy-load electronAPI to avoid timing issues - Remove path module usage from preload scripts (preparation for sandboxing) - Add documentation: ELECTRON_REMOTE_AUDIT.md and PR_BREAKDOWN.md This is PR 1 of the Electron security hardening effort. --- docs/ELECTRON_REMOTE_AUDIT.md | 483 ++++++++++++++++++ docs/PR_BREAKDOWN.md | 311 +++++++++++ electron/renderer/src/actions/index.ts | 2 +- .../src/components/WebView/Webview.tsx | 6 +- .../components/context/EditAccountMenu.tsx | 4 +- electron/renderer/src/lib/locale.ts | 67 ++- electron/src/main.ts | 2 +- electron/src/preload/preload-app.ts | 56 +- electron/src/preload/preload-webview.ts | 4 +- electron/src/types/globals.d.ts | 24 +- 10 files changed, 903 insertions(+), 56 deletions(-) create mode 100644 docs/ELECTRON_REMOTE_AUDIT.md create mode 100644 docs/PR_BREAKDOWN.md diff --git a/docs/ELECTRON_REMOTE_AUDIT.md b/docs/ELECTRON_REMOTE_AUDIT.md new file mode 100644 index 00000000000..290887b8ca0 --- /dev/null +++ b/docs/ELECTRON_REMOTE_AUDIT.md @@ -0,0 +1,483 @@ +# @electron/remote Usage Audit + +**Date**: 2024 +**Purpose**: Document all `@electron/remote` usage and plan replacement strategy for security hardening + +## Summary + +Total files using `@electron/remote`: **7 files** + +- Main process initialization: 1 file +- Preload scripts: 2 files +- Utility modules: 4 files + +## Main Process Initialization + +### File: `electron/src/main.ts` + +**Usage**: + +```typescript +import * as remoteMain from '@electron/remote/main'; +remoteMain.initialize(); // Line 78 +remoteMain.enable(main.webContents); // Line 307 +remoteMain.enable(contents); // Line 660 (for all webContents) +``` + +**Purpose**: + +- Initializes the remote module system +- Enables remote access for main window and all webviews + +**Replacement Strategy**: + +- Remove all `remoteMain.enable()` calls after replacing all remote usage +- Remove `remoteMain.initialize()` after all remote usage is replaced +- This is the LAST step - only remove after all other remote usage is eliminated + +**Dependencies**: All other remote usage depends on this + +--- + +## Preload Scripts + +### File: `electron/src/preload/preload-webview.ts` + +**Usage**: + +```typescript +const remote = require('@electron/remote'); + +// Line 52, 59: Get dark mode state +const useDarkMode = remote.nativeTheme.shouldUseDarkColors; + +// Line 69: Listen for theme changes +remote.nativeTheme.on('updated', () => updateWebAppTheme()); +``` + +**Context**: Webview preload script - runs in webview renderer process + +**Purpose**: + +- Detect system dark/light mode preference +- React to theme changes in real-time +- Publish theme updates to webapp via amplify events + +**Replacement Strategy**: + +1. **Create IPC Handler in Main Process** (`electron/src/main.ts`): + +```typescript +// Add to bindIpcEvents() or create new function +ipcMain.handle('native-theme:shouldUseDarkColors', () => { + return nativeTheme.shouldUseDarkColors; +}); + +// Listen for theme changes and notify renderers +nativeTheme.on('updated', () => { + // Broadcast to all webviews + main.webContents.send('native-theme:updated', { + shouldUseDarkColors: nativeTheme.shouldUseDarkColors, + }); +}); +``` + +2. **Expose via ContextBridge** (in preload script): + +```typescript +import {contextBridge, ipcRenderer} from 'electron'; + +contextBridge.exposeInMainWorld('electronAPI', { + nativeTheme: { + shouldUseDarkColors: () => ipcRenderer.invoke('native-theme:shouldUseDarkColors'), + onUpdated: (callback: (shouldUseDarkColors: boolean) => void) => { + ipcRenderer.on('native-theme:updated', (_event, {shouldUseDarkColors}) => { + callback(shouldUseDarkColors); + }); + }, + }, +}); +``` + +3. **Update Usage**: + +```typescript +// Replace: +const useDarkMode = remote.nativeTheme.shouldUseDarkColors; + +// With: +const useDarkMode = window.electronAPI.nativeTheme.shouldUseDarkColors(); + +// Replace: +remote.nativeTheme.on('updated', () => updateWebAppTheme()); + +// With: +window.electronAPI.nativeTheme.onUpdated(shouldUseDarkColors => { + // Update theme logic +}); +``` + +**Complexity**: Medium +**Risk**: Low - Theme detection is straightforward +**Testing**: Verify dark/light mode switching works + +--- + +### File: `electron/src/preload/menu/preload-context.ts` + +**Usage**: + +```typescript +const remote = require('@electron/remote'); + +// Line 64, 113, 116: Build context menus +remote.Menu.buildFromTemplate([...]); + +// Line 127: Get current webContents +const webContents = remote.getCurrentWebContents(); + +// Line 130: Get current window +const window = remote.getCurrentWindow(); +``` + +**Context**: Context menu preload script - runs in main window renderer process + +**Purpose**: + +- Create context menus (text menu, image menu, default menu) +- Get current webContents for menu operations +- Get current window for menu popup positioning + +**Replacement Strategy**: + +1. **Menu Building**: + + - Menus can be built in preload script using `Menu` from `electron` (not remote) + - `Menu` is available in preload context when context isolation is enabled + - **Change**: `const {Menu} = require('electron')` instead of `remote.Menu` + +2. **getCurrentWebContents()**: + + - In preload script, we can use `webContents` from the context + - However, since this is for the main window, we need to pass webContents ID from main process + - **Alternative**: Use IPC to get webContents ID, or pass it during preload initialization + - **Better approach**: Since this is in the main window preload, we can access `webContents` directly if available + - **Note**: Need to verify if `webContents` is accessible in preload with context isolation + +3. **getCurrentWindow()**: + - Similar to webContents - need window reference + - **Alternative**: Pass window ID via IPC or during initialization + - **Better approach**: Menu.popup() can work without explicit window reference in some cases + +**Detailed Replacement**: + +```typescript +// Replace remote import +import {Menu, WebContents} from 'electron'; + +// For getCurrentWebContents - we need to get it from main process +// Option 1: Pass webContents ID via IPC during initialization +// Option 2: Use a global that's set by main process +// Option 3: Create IPC handler to get current webContents ID + +// For getCurrentWindow - similar approach +// Menu.popup() can work with just options, window is optional + +// Create IPC handler in main process: +ipcMain.handle('get-current-webcontents-id', event => { + return event.sender.id; +}); + +ipcMain.handle('get-current-window-id', event => { + const window = BrowserWindow.fromWebContents(event.sender); + return window?.id; +}); + +// In preload: +const webContentsId = await ipcRenderer.invoke('get-current-webcontents-id'); +const windowId = await ipcRenderer.invoke('get-current-window-id'); + +// But actually, for context menus, we might not need the window reference +// Menu.popup() can infer it from the event +``` + +**Actually, simpler approach**: + +- `Menu.buildFromTemplate()` works with regular `Menu` import (not remote) +- For `menu.popup()`, we can pass `{window: null}` or get window via IPC +- For `webContents.replaceMisspelling()`, we need webContents reference - pass via IPC + +**Complexity**: High - Context menu system is complex +**Risk**: Medium - Context menus are user-facing, must work correctly +**Testing**: Test all context menu types (text, image, link, selection) + +--- + +## Utility Modules + +### File: `electron/src/logging/getLogger.ts` + +**Usage**: + +```typescript +const mainProcess = process || require('@electron/remote').process; +const app = Electron.app || require('@electron/remote').app; + +// Line 36: Check command line arguments +const forceLogging = mainProcess.argv.includes('--enable-logging'); + +// Line 32: Get user data path +const logDir = path.join(app.getPath('userData'), 'logs'); +``` + +**Context**: Utility module - may run in main or renderer process + +**Purpose**: + +- Access process.argv for logging flags +- Get app.getPath('userData') for log directory + +**Replacement Strategy**: + +1. **For Main Process** (when `process` is available): + + - Use `process` directly - no change needed + - Use `app` directly from `electron` - no change needed + +2. **For Renderer Process** (when remote is used): + + - **process.argv**: Create IPC handler `ipcMain.handle('process:argv', () => process.argv)` + - **app.getPath()**: Create IPC handler `ipcMain.handle('app:getPath', (event, name) => app.getPath(name))` + +3. **Update Code**: + +```typescript +// Detect if we're in main or renderer +const isMainProcess = typeof process !== 'undefined' && process.type === 'browser'; + +let processArgv: string[] = []; +let getUserDataPath: () => string; + +if (isMainProcess) { + processArgv = process.argv; + getUserDataPath = () => app.getPath('userData'); +} else { + // Renderer process - use IPC + processArgv = await ipcRenderer.invoke('process:argv'); + getUserDataPath = () => ipcRenderer.invoke('app:getPath', 'userData'); +} +``` + +**Note**: `getLogger` is used in both main process (main.ts, settings, etc.) and preload scripts (preload-app.ts, preload-webview.ts), so it must handle both contexts. + +**Implementation**: + +```typescript +import {ipcRenderer} from 'electron'; + +const isMainProcess = typeof process !== 'undefined' && process.type === 'browser'; + +let processArgv: string[] = []; +let getUserDataPath: () => string | Promise; + +if (isMainProcess) { + // Main process - direct access + processArgv = process.argv; + getUserDataPath = () => app.getPath('userData'); +} else { + // Renderer process - use IPC (requires contextBridge setup) + // Note: This will need to be async or use synchronous IPC + // For now, we can make it async and update callers + processArgv = []; // Will be populated via IPC + getUserDataPath = async () => { + // Requires IPC handler: ipcMain.handle('app:getPath', (event, name) => app.getPath(name)) + return await ipcRenderer.invoke('app:getPath', 'userData'); + }; +} + +// Update ENABLE_LOGGING to be async-aware or use synchronous approach +const forceLogging = isMainProcess ? process.argv.includes('--enable-logging') : false; // Will need IPC to check in renderer, or pass via contextBridge +``` + +**Alternative Simpler Approach**: Since logging initialization happens early, we can: + +1. Pass logging config via contextBridge during preload initialization +2. Or make the check synchronous by exposing it via contextBridge + +**Complexity**: Medium (due to dual context usage) +**Risk**: Low +**Testing**: Verify logging works in both main and renderer contexts + +--- + +### File: `electron/src/locale/index.ts` + +**Usage**: + +```typescript +const app = Electron.app || require('@electron/remote').app; + +// Line 144: Get system locale +const defaultLocale = parseLocale(app.getLocale().substring(0, 2)); +``` + +**Context**: Utility module - may run in main or renderer process + +**Purpose**: + +- Get system locale for language detection + +**Replacement Strategy**: + +1. **For Main Process**: Use `app` directly - no change needed + +2. **For Renderer Process**: + + - Create IPC handler: `ipcMain.handle('app:getLocale', () => app.getLocale())` + - Use IPC in renderer: `const locale = await ipcRenderer.invoke('app:getLocale')` + +3. **Update Code**: + +```typescript +const isMainProcess = typeof process !== 'undefined' && process.type === 'browser'; + +let getLocale: () => string; + +if (isMainProcess) { + getLocale = () => app.getLocale(); +} else { + getLocale = async () => await ipcRenderer.invoke('app:getLocale'); +} +``` + +**Complexity**: Low +**Risk**: Low +**Testing**: Verify locale detection works correctly + +--- + +### File: `electron/src/settings/SchemaUpdater.ts` + +**Usage**: + +```typescript +const app = Electron.app || require('@electron/remote').app; + +// Line 32: Get user data path +const defaultPathV0 = path.join(app.getPath('userData'), 'init.json'); +const defaultPathV1 = path.join(app.getPath('userData'), 'config/init.json'); +``` + +**Context**: Utility module - likely runs in main process only + +**Purpose**: + +- Get user data directory for config file paths + +**Replacement Strategy**: + +1. **Check where this is used**: If only in main process, use `app` directly +2. **If used in renderer**: Create IPC handler `ipcMain.handle('app:getPath', (event, name) => app.getPath(name))` + +**Complexity**: Low +**Risk**: Low +**Testing**: Verify config file paths are correct + +--- + +### File: `electron/src/sso/AutomatedSingleSignOn.ts` + +**Usage**: + +```typescript +const dialog = Electron.dialog || require('@electron/remote').dialog; + +// Line 42: Show error dialog +await dialog.showMessageBox({ + detail, + message, + type: 'warning', +}); +``` + +**Context**: SSO automation - runs in renderer process (preload-app.ts calls this) + +**Purpose**: + +- Show error dialog when maximum accounts reached during SSO + +**Replacement Strategy**: + +1. **Create IPC Handler** in main process: + +```typescript +ipcMain.handle('dialog:showMessageBox', async (event, options) => { + const window = BrowserWindow.fromWebContents(event.sender); + const result = await dialog.showMessageBox(window || undefined, options); + return result; +}); +``` + +2. **Update Code**: + +```typescript +// Remove remote import +// Add IPC call +const result = await ipcRenderer.invoke('dialog:showMessageBox', { + detail, + message, + type: 'warning', +}); +``` + +**Complexity**: Low +**Risk**: Medium - SSO error handling is important +**Testing**: Test SSO with maximum accounts reached, verify dialog appears + +--- + +## Replacement Priority Order + +1. **Low Risk, Simple**: + + - `locale/index.ts` - app.getLocale() + - `settings/SchemaUpdater.ts` - app.getPath() + - `logging/getLogger.ts` - process.argv, app.getPath() + +2. **Medium Risk, Medium Complexity**: + + - `preload/preload-webview.ts` - nativeTheme + - `sso/AutomatedSingleSignOn.ts` - dialog.showMessageBox + +3. **High Risk, High Complexity**: + + - `preload/menu/preload-context.ts` - Menu, getCurrentWebContents, getCurrentWindow + +4. **Final Step**: + - `main.ts` - Remove remoteMain.initialize() and remoteMain.enable() calls + +## Testing Checklist for Each Replacement + +- [ ] Functionality works as before +- [ ] No console errors +- [ ] No performance regressions +- [ ] SSO flow works (for SSO-related changes) +- [ ] Context menus work (for menu changes) +- [ ] Theme switching works (for theme changes) +- [ ] Error dialogs appear (for dialog changes) + +## Notes + +- All replacements assume context isolation is enabled (Phase 2) +- IPC handlers should be added to `bindIpcEvents()` or a similar centralized location +- Consider creating a typed IPC API for better maintainability +- Some replacements may require updating TypeScript types + +## Estimated Effort + +- Low complexity: 1-2 hours each (3 files) +- Medium complexity: 2-4 hours each (2 files) +- High complexity: 4-6 hours (1 file) +- Testing and verification: 4-6 hours + +**Total**: ~20-30 hours diff --git a/docs/PR_BREAKDOWN.md b/docs/PR_BREAKDOWN.md new file mode 100644 index 00000000000..ccff854c7c6 --- /dev/null +++ b/docs/PR_BREAKDOWN.md @@ -0,0 +1,311 @@ +# Electron Security Hardening - PR Breakdown + +## Prerequisites for Sandboxing + +**Sandboxing requires**: + +1. ✅ Context isolation enabled (mandatory) +2. ✅ @electron/remote removed (doesn't work with sandbox) +3. ✅ No Node.js APIs in preload scripts (sandbox restricts access) + +**Current blockers for sandboxing**: + +- `contextIsolation: false` in main window +- `@electron/remote` usage throughout codebase +- Node.js modules in preload scripts (`path`, `process`, etc.) + +## PR Structure + +### PR 1: Enable Context Isolation (Main Window) + +**Goal**: Enable context isolation and refactor preload scripts to use contextBridge + +**Files Changed**: + +- `electron/src/main.ts` - Change `contextIsolation: false` → `true` +- `electron/src/preload/preload-app.ts` - Refactor to use contextBridge +- `electron/src/preload/preload-webview.ts` - Refactor to use contextBridge +- `electron/renderer/src/**` - Update all renderer code to use new APIs + +**Key Changes**: + +- Replace `window.sendBadgeCount` with `window.electronAPI.sendBadgeCount` +- Replace all direct window assignments with contextBridge.exposeInMainWorld() +- Update all renderer code that uses these APIs + +**Testing**: + +- All existing functionality works +- SSO flow works +- IPC communication works + +**Dependencies**: None (can be first PR) + +--- + +### PR 2: Replace @electron/remote - Utility Modules (Low Risk) + +**Goal**: Replace remote usage in utility modules that don't affect UI + +**Files Changed**: + +- `electron/src/locale/index.ts` - Replace `remote.app.getLocale()` with IPC +- `electron/src/settings/SchemaUpdater.ts` - Replace `remote.app.getPath()` with IPC +- `electron/src/logging/getLogger.ts` - Replace `remote.process` and `remote.app` with IPC +- `electron/src/main.ts` - Add IPC handlers: `app:getLocale`, `app:getPath`, `process:argv` + +**Key Changes**: + +- Add IPC handlers in main process +- Update utility modules to use IPC when in renderer context +- Keep main process direct access when available + +**Testing**: + +- Locale detection works +- Config file paths are correct +- Logging works in both main and renderer + +**Dependencies**: PR 1 (needs context isolation for IPC in renderer) + +--- + +### PR 3: Replace @electron/remote - Native Theme + +**Goal**: Replace remote.nativeTheme usage with IPC + +**Files Changed**: + +- `electron/src/preload/preload-webview.ts` - Replace `remote.nativeTheme` with IPC +- `electron/src/main.ts` - Add IPC handlers: `native-theme:shouldUseDarkColors`, `native-theme:updated` event + +**Key Changes**: + +- Create IPC handler for theme detection +- Broadcast theme changes to all webviews +- Expose via contextBridge in preload + +**Testing**: + +- Dark/light mode detection works +- Theme switching works in real-time +- No console errors + +**Dependencies**: PR 1 (needs context isolation) + +--- + +### PR 4: Replace @electron/remote - Dialog (SSO) + +**Goal**: Replace remote.dialog usage in SSO error handling + +**Files Changed**: + +- `electron/src/sso/AutomatedSingleSignOn.ts` - Replace `remote.dialog.showMessageBox()` with IPC +- `electron/src/main.ts` - Add IPC handler: `dialog:showMessageBox` + +**Key Changes**: + +- Create IPC handler that shows dialog in main process +- Return dialog result via IPC +- Update SSO error handling to use IPC + +**Testing**: + +- SSO error dialog appears when max accounts reached +- Dialog buttons work correctly +- SSO flow continues to work + +**Dependencies**: PR 1 (needs context isolation) + +--- + +### PR 5: Replace @electron/remote - Context Menu (Complex) + +**Goal**: Replace remote.Menu, remote.getCurrentWebContents(), remote.getCurrentWindow() + +**Files Changed**: + +- `electron/src/preload/menu/preload-context.ts` - Replace all remote usage +- `electron/src/main.ts` - Add IPC handlers if needed (may not be needed - Menu works in preload) + +**Key Changes**: + +- Replace `remote.Menu` with `Menu` from `electron` (available in preload) +- Replace `remote.getCurrentWebContents()` - may need IPC or pass via contextBridge +- Replace `remote.getCurrentWindow()` - may need IPC or pass via contextBridge +- Update menu.popup() calls + +**Testing**: + +- All context menu types work (text, image, link, selection) +- Menu positioning is correct +- No console errors + +**Dependencies**: PR 1 (needs context isolation) + +--- + +### PR 6: Remove @electron/remote Package + +**Goal**: Remove all remoteMain initialization and the package dependency + +**Files Changed**: + +- `electron/src/main.ts` - Remove `remoteMain.initialize()` and all `remoteMain.enable()` calls +- `package.json` - Remove `@electron/remote` dependency + +**Key Changes**: + +- Remove remoteMain imports +- Remove remoteMain.initialize() +- Remove remoteMain.enable() calls +- Remove package from dependencies + +**Testing**: + +- App starts correctly +- No remote-related errors +- All functionality still works + +**Dependencies**: PRs 2, 3, 4, 5 (all remote usage must be replaced first) + +--- + +### PR 7: Enable Sandbox (Main Window) + +**Goal**: Enable sandbox for main window + +**Files Changed**: + +- `electron/src/main.ts` - Change `sandbox: false` → `true` in main window + +**Key Changes**: + +- Enable sandbox in webPreferences +- Verify preload scripts work in sandboxed context +- Ensure no Node.js APIs are used in preload + +**Blockers Check**: + +- ✅ Context isolation enabled (PR 1) +- ✅ @electron/remote removed (PR 6) +- ⚠️ Node.js modules in preload - need to check: + - `path` module in preload-app.ts and preload-webview.ts + - `getLogger` uses path - needs IPC or refactor + +**Testing**: + +- All functionality works +- SSO works +- No security warnings +- Performance is acceptable + +**Dependencies**: PR 1, PR 6 (context isolation + remote removal) + +--- + +### PR 8: Enable Sandbox (Webviews) + +**Goal**: Enable sandbox for webviews + +**Files Changed**: + +- `electron/src/main.ts` - Change `sandbox: false` → `true` in webview configuration (line 679) + +**Key Changes**: + +- Enable sandbox for webviews +- Verify SSO JavaScript injection still works (`executeJavaScriptWithoutResult`) +- Ensure webview preload works in sandboxed context + +**Critical SSO Test**: + +- SSO window opens +- SSO login completes +- JavaScript injection for SSO response works +- Cookie transfer works + +**Testing**: + +- All webview functionality works +- SSO flow works completely +- No security warnings + +**Dependencies**: PR 7 (main window sandbox first) + +--- + +## Node.js Modules in Preload Scripts + +### Current Usage: + +- `path` module in `preload-app.ts` and `preload-webview.ts` (line 23) +- Used for: `path.basename(__filename)` in getLogger calls + +### Solution Options: + +1. **Remove path usage**: Pass logger name as parameter instead of using `__filename` +2. **Use IPC**: Get path info via IPC (overkill for this) +3. **Use string manipulation**: Replace `path.basename(__filename)` with string operations + +**Recommended**: Option 1 - Pass logger name explicitly or use a different method + +**Files to Update**: + +- `electron/src/preload/preload-app.ts` - Remove `path` import, pass logger name explicitly +- `electron/src/preload/preload-webview.ts` - Remove `path` import, pass logger name explicitly + +This should be done in **PR 1** or **PR 7** (before enabling sandbox). + +--- + +## PR Dependencies Graph + +``` +PR 1 (Context Isolation) + ├─> PR 2 (Remote - Utilities) + ├─> PR 3 (Remote - Native Theme) + ├─> PR 4 (Remote - Dialog) + └─> PR 5 (Remote - Context Menu) + │ + └─> PR 6 (Remove Remote Package) + │ + └─> PR 7 (Sandbox Main Window) + │ + └─> PR 8 (Sandbox Webviews) +``` + +## Testing Strategy Per PR + +Each PR should include: + +1. Unit tests pass +2. Integration tests pass +3. Manual SSO testing (for PRs that touch SSO) +4. Manual functionality testing +5. No console errors or warnings + +## Estimated Timeline + +- PR 1: 3-5 days +- PR 2: 1-2 days +- PR 3: 1-2 days +- PR 4: 1-2 days +- PR 5: 3-4 days (most complex) +- PR 6: 1 day +- PR 7: 2-3 days +- PR 8: 2-3 days + +**Total**: ~2-3 weeks + +## Critical Path + +The critical path for sandboxing is: + +1. PR 1 (Context Isolation) - Foundation +2. PR 6 (Remove Remote) - Required for sandbox +3. PR 7 (Sandbox Main) - First sandbox +4. PR 8 (Sandbox Webviews) - Complete sandbox + +PRs 2-5 can be done in parallel after PR 1, but PR 6 must wait for all of them. diff --git a/electron/renderer/src/actions/index.ts b/electron/renderer/src/actions/index.ts index 5ab8b741224..1593fdfb492 100644 --- a/electron/renderer/src/actions/index.ts +++ b/electron/renderer/src/actions/index.ts @@ -191,7 +191,7 @@ export const updateAccountBadgeCount = (id: string, count: number) => { }, 0); const ignoreFlash = account?.availability === Availability.Type.BUSY; - window.sendBadgeCount(accumulatedCount, ignoreFlash); + window.electronAPI.sendBadgeCount(accumulatedCount, ignoreFlash); if (account) { const countHasChanged = account.badgeCount !== count; diff --git a/electron/renderer/src/components/WebView/Webview.tsx b/electron/renderer/src/components/WebView/Webview.tsx index 14bb0eab74f..2afec028da7 100644 --- a/electron/renderer/src/components/WebView/Webview.tsx +++ b/electron/renderer/src/components/WebView/Webview.tsx @@ -211,7 +211,7 @@ const Webview = ({ case EVENT_TYPE.LIFECYCLE.SIGNED_IN: { if (conversationJoinData) { const {code, key, domain} = conversationJoinData; - window.sendConversationJoinToHost(accountId, code, key, domain); + window.electronAPI.sendConversationJoinToHost(accountId, code, key, domain); setConversationJoinData(accountId, undefined); } updateAccountLifecycle(accountId, channel); @@ -241,7 +241,7 @@ const Webview = ({ if (isConversationJoinData(data)) { if (accountLifecycle === EVENT_TYPE.LIFECYCLE.SIGNED_IN) { - window.sendConversationJoinToHost(accountId, data.code, data.key, data.domain); + window.electronAPI.sendConversationJoinToHost(accountId, data.code, data.key, data.domain); setConversationJoinData(accountId, undefined); } else { setConversationJoinData(accountId, data); @@ -283,7 +283,7 @@ const Webview = ({ }, [account, accountLifecycle, conversationJoinData]); const deleteWebview = (account: Account) => { - window.sendDeleteAccount(account.id, account.sessionID).then(() => { + window.electronAPI.sendDeleteAccount(account.id, account.sessionID).then(() => { abortAccountCreation(account.id); }); }; diff --git a/electron/renderer/src/components/context/EditAccountMenu.tsx b/electron/renderer/src/components/context/EditAccountMenu.tsx index 2352fb7b6d4..c12ebed224a 100644 --- a/electron/renderer/src/components/context/EditAccountMenu.tsx +++ b/electron/renderer/src/components/context/EditAccountMenu.tsx @@ -64,7 +64,7 @@ const EditAccountMenu = ({ { connected.switchWebview(accountIndex); - window.sendLogoutAccount(accountId); + window.electronAPI.sendLogoutAccount(accountId); }} > {getText('wrapperLogOut')} @@ -73,7 +73,7 @@ const EditAccountMenu = ({ { - window.sendDeleteAccount(accountId, sessionID).then(() => { + window.electronAPI.sendDeleteAccount(accountId, sessionID).then(() => { connected.abortAccountCreation(accountId); }); }} diff --git a/electron/renderer/src/lib/locale.ts b/electron/renderer/src/lib/locale.ts index 064deee0af6..5e15ca934c4 100644 --- a/electron/renderer/src/lib/locale.ts +++ b/electron/renderer/src/lib/locale.ts @@ -17,14 +17,41 @@ * */ -import {i18nLanguageIdentifier} from '../../../src/locale'; +import {i18nLanguageIdentifier, SupportedI18nLanguage} from '../../../src/locale'; -window.locStrings = window.locStrings || {}; -window.locStringsDefault = window.locStringsDefault || {}; -window.locale = window.locale || 'en'; +// Get locale info from electronAPI (contextBridge) +// Made lazy to avoid accessing window.electronAPI before preload script has executed +const getLocaleInfo = () => { + if (window.electronAPI) { + return { + strings: window.electronAPI.locale.strings, + stringsDefault: window.electronAPI.locale.stringsDefault, + current: window.electronAPI.locale.current, + }; + } + // Fallback for development/testing or when electronAPI isn't available yet + return { + strings: {}, + stringsDefault: {}, + current: 'en' as SupportedI18nLanguage, + }; +}; + +// Cache locale info once it's available +let localeInfoCache: ReturnType | null = null; + +const getCachedLocaleInfo = () => { + if (!localeInfoCache) { + localeInfoCache = getLocaleInfo(); + } + return localeInfoCache; +}; export const getText = (stringIdentifier: i18nLanguageIdentifier, paramReplacements?: Record) => { - let str = window.locStrings[stringIdentifier] || window.locStringsDefault[stringIdentifier] || stringIdentifier; + const localeInfo = getCachedLocaleInfo(); + const strings = localeInfo.strings as Record; + const stringsDefault = localeInfo.stringsDefault as Record; + let str = strings[stringIdentifier] || stringsDefault[stringIdentifier] || stringIdentifier; const replacements = {...paramReplacements}; for (const replacement of Object.keys(replacements)) { @@ -37,4 +64,32 @@ export const getText = (stringIdentifier: i18nLanguageIdentifier, paramReplaceme return str; }; -export const wrapperLocale = window.locale; +// wrapperLocale is used as a constant in WindowUrl.ts and Webview.tsx +// We'll initialize it lazily - it will be 'en' initially and may update when electronAPI becomes available +// For now, we'll use a getter function approach, but export it as a value for compatibility +let _wrapperLocale: SupportedI18nLanguage = 'en' as SupportedI18nLanguage; + +// Try to initialize immediately, but don't fail if electronAPI isn't ready +try { + const info = getLocaleInfo(); + _wrapperLocale = info.current; +} catch { + // Ignore errors - will use 'en' as default +} + +// Update locale when electronAPI becomes available (if it wasn't available initially) +if (typeof window !== 'undefined') { + // Use a small delay to allow preload script to finish + setTimeout(() => { + try { + const info = getLocaleInfo(); + if (info.current !== 'en' || window.electronAPI) { + _wrapperLocale = info.current; + } + } catch { + // Ignore errors + } + }, 100); +} + +export const wrapperLocale: SupportedI18nLanguage = _wrapperLocale; diff --git a/electron/src/main.ts b/electron/src/main.ts index d8251d8d8e4..afc71f2f92b 100644 --- a/electron/src/main.ts +++ b/electron/src/main.ts @@ -283,7 +283,7 @@ const showMainWindow = async (mainWindowState: windowStateKeeper.State): Promise title: config.name, webPreferences: { backgroundThrottling: false, - contextIsolation: false, + contextIsolation: true, nodeIntegration: false, preload: PRELOAD_JS, sandbox: false, diff --git a/electron/src/preload/preload-app.ts b/electron/src/preload/preload-app.ts index 9cc9f489837..5e476019e84 100644 --- a/electron/src/preload/preload-app.ts +++ b/electron/src/preload/preload-app.ts @@ -17,11 +17,9 @@ * */ -import {ipcRenderer, webFrame} from 'electron'; +import {contextBridge, ipcRenderer, webFrame} from 'electron'; import {truncate} from 'lodash'; -import * as path from 'path'; - import {WebAppEvents} from '@wireapp/webapp-events'; import {EVENT_TYPE} from '../lib/eventType'; @@ -30,16 +28,10 @@ import {getLogger} from '../logging/getLogger'; import * as EnvironmentUtil from '../runtime/EnvironmentUtil'; import {AutomatedSingleSignOn} from '../sso/AutomatedSingleSignOn'; -const logger = getLogger(path.basename(__filename)); +const logger = getLogger('preload-app'); webFrame.setVisualZoomLevelLimits(1, 1); -window.locStrings = locale.LANGUAGES[locale.getCurrent()]; -window.locStringsDefault = locale.LANGUAGES.en; -window.locale = locale.getCurrent(); - -window.isMac = EnvironmentUtil.platform.IS_MAC_OS; - const getSelectedWebview = (): Electron.WebviewTag | null => document.querySelector('.Webview:not(.hide)'); const getWebviewById = (id: string): Electron.WebviewTag | null => @@ -99,16 +91,25 @@ const subscribeToMainProcessEvents = (): void => { }); }; -const setupIpcInterface = (): void => { - window.sendBadgeCount = (count: number, ignoreFlash: boolean): void => { +// Expose APIs via contextBridge for context isolation +const electronAPI = { + // Locale and environment info + locale: { + current: locale.getCurrent(), + strings: locale.LANGUAGES[locale.getCurrent()], + stringsDefault: locale.LANGUAGES.en, + }, + environment: { + isMac: EnvironmentUtil.platform.IS_MAC_OS, + }, + // IPC methods + sendBadgeCount: (count: number, ignoreFlash: boolean): void => { ipcRenderer.send(EVENT_TYPE.UI.BADGE_COUNT, {count, ignoreFlash}); - }; - - window.submitDeepLink = (url: string): void => { + }, + submitDeepLink: (url: string): void => { ipcRenderer.send(EVENT_TYPE.ACTION.DEEP_LINK_SUBMIT, url); - }; - - window.sendDeleteAccount = (accountId: string, sessionID?: string): Promise => { + }, + sendDeleteAccount: (accountId: string, sessionID?: string): Promise => { const truncatedId = truncate(accountId, {length: 5}); return new Promise((resolve, reject) => { @@ -123,27 +124,20 @@ const setupIpcInterface = (): void => { ipcRenderer.on(EVENT_TYPE.ACCOUNT.DATA_DELETED, () => resolve()); ipcRenderer.send(EVENT_TYPE.ACCOUNT.DELETE_DATA, viewInstanceId, accountId, sessionID); }); - }; - - window.sendLogoutAccount = async (accountId: string): Promise => { + }, + sendLogoutAccount: async (accountId: string): Promise => { const accountWebview = getWebviewById(accountId); logger.log(`Sending logout signal to webview for account "${truncate(accountId, {length: 5})}".`); await accountWebview?.send(EVENT_TYPE.ACTION.SIGN_OUT); - }; - - window.sendConversationJoinToHost = async ( - accountId: string, - code: string, - key: string, - domain?: string, - ): Promise => { + }, + sendConversationJoinToHost: async (accountId: string, code: string, key: string, domain?: string): Promise => { const accountWebview = getWebviewById(accountId); logger.log(`Sending conversation join data to webview for account "${truncate(accountId, {length: 5})}".`); await accountWebview?.send(WebAppEvents.CONVERSATION.JOIN, {code, key, domain}); - }; + }, }; -setupIpcInterface(); +contextBridge.exposeInMainWorld('electronAPI', electronAPI); subscribeToMainProcessEvents(); window.addEventListener('focus', () => { diff --git a/electron/src/preload/preload-webview.ts b/electron/src/preload/preload-webview.ts index d532bf28572..190155aa169 100644 --- a/electron/src/preload/preload-webview.ts +++ b/electron/src/preload/preload-webview.ts @@ -20,8 +20,6 @@ import {ipcRenderer, webFrame} from 'electron'; import type {Data as OpenGraphResult} from 'open-graph'; -import * as path from 'path'; - import type {Availability} from '@wireapp/protocol-messaging'; import {WebAppEvents} from '@wireapp/webapp-events'; @@ -44,7 +42,7 @@ interface TeamAccountInfo { type Theme = 'dark' | 'default'; -const logger = getLogger(path.basename(__filename)); +const logger = getLogger('preload-webview'); function subscribeToThemeChange(): void { function updateWebAppTheme(): void { diff --git a/electron/src/types/globals.d.ts b/electron/src/types/globals.d.ts index a554976b502..e44480d4660 100644 --- a/electron/src/types/globals.d.ts +++ b/electron/src/types/globals.d.ts @@ -51,15 +51,21 @@ export declare global { interface Window { amplify: amplify; - isMac: boolean; - locale: SupportedI18nLanguage; - locStrings: i18nStrings; - locStringsDefault: i18nStrings; - sendBadgeCount(count: number, ignoreFlash: boolean): void; - sendConversationJoinToHost(accountId: string, code: string, key: string, domain?: string): void; - sendDeleteAccount(accountId: string, sessionID?: string): Promise; - sendLogoutAccount(accountId: string): Promise; - submitDeepLink(url: string): void; + electronAPI: { + locale: { + current: SupportedI18nLanguage; + strings: i18nStrings; + stringsDefault: i18nStrings; + }; + environment: { + isMac: boolean; + }; + sendBadgeCount(count: number, ignoreFlash: boolean): void; + sendConversationJoinToHost(accountId: string, code: string, key: string, domain?: string): void; + sendDeleteAccount(accountId: string, sessionID?: string): Promise; + sendLogoutAccount(accountId: string): Promise; + submitDeepLink(url: string): void; + }; wire: any; z: { event: { From 91e1e90134c961f125c1208c8a41e7c725f62ba4 Mon Sep 17 00:00:00 2001 From: Adam Low Date: Mon, 15 Dec 2025 15:37:03 +0000 Subject: [PATCH 2/6] fix: execute context menu actions directly in webview context --- electron/src/preload/menu/preload-context.ts | 19 +++++++--- electron/src/preload/preload-app.ts | 37 +++++++++++++++++--- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/electron/src/preload/menu/preload-context.ts b/electron/src/preload/menu/preload-context.ts index ec9b54de826..dddae832be0 100644 --- a/electron/src/preload/menu/preload-context.ts +++ b/electron/src/preload/menu/preload-context.ts @@ -70,20 +70,29 @@ const createDefaultMenu = (copyContext: string) => const createTextMenu = (params: ContextMenuParams, webContents: WebContents): ElectronMenu => { const {editFlags, dictionarySuggestions} = params; + // Detect if context menu is triggered from a webview + const isWebview = webContents.getType() === 'webview'; + const webContentsId = webContents.id; const template: MenuItemConstructorOptions[] = [ { - click: (_menuItem, baseWindow) => sendToWebContents(baseWindow, EVENT_TYPE.EDIT.CUT), + click: isWebview + ? () => webContents.cut() + : (_menuItem, baseWindow) => sendToWebContents(baseWindow, EVENT_TYPE.EDIT.CUT, webContentsId), enabled: editFlags.canCut, label: locale.getText('menuCut'), }, { - click: (_menuItem, baseWindow) => sendToWebContents(baseWindow, EVENT_TYPE.EDIT.COPY), + click: isWebview + ? () => webContents.copy() + : (_menuItem, baseWindow) => sendToWebContents(baseWindow, EVENT_TYPE.EDIT.COPY, webContentsId), enabled: editFlags.canCopy, label: locale.getText('menuCopy'), }, { - click: (_menuItem, baseWindow) => sendToWebContents(baseWindow, EVENT_TYPE.EDIT.PASTE), + click: isWebview + ? () => webContents.paste() + : (_menuItem, baseWindow) => sendToWebContents(baseWindow, EVENT_TYPE.EDIT.PASTE, webContentsId), enabled: editFlags.canPaste, label: locale.getText('menuPaste'), }, @@ -91,7 +100,9 @@ const createTextMenu = (params: ContextMenuParams, webContents: WebContents): El type: 'separator', }, { - click: (_menuItem, baseWindow) => sendToWebContents(baseWindow, EVENT_TYPE.EDIT.SELECT_ALL), + click: isWebview + ? () => webContents.selectAll() + : (_menuItem, baseWindow) => sendToWebContents(baseWindow, EVENT_TYPE.EDIT.SELECT_ALL, webContentsId), enabled: editFlags.canSelectAll, label: locale.getText('menuSelectAll'), }, diff --git a/electron/src/preload/preload-app.ts b/electron/src/preload/preload-app.ts index 5e476019e84..858c2cbed63 100644 --- a/electron/src/preload/preload-app.ts +++ b/electron/src/preload/preload-app.ts @@ -37,6 +37,23 @@ const getSelectedWebview = (): Electron.WebviewTag | null => const getWebviewById = (id: string): Electron.WebviewTag | null => document.querySelector(`.Webview[data-accountid="${id}"]`); +// Helper to find and focus a webview by webContents ID +const focusWebviewByContentsId = (webContentsId: number): Electron.WebviewTag | null => { + const webviews = document.querySelectorAll('.Webview'); + for (const webview of webviews) { + try { + if (webview.getWebContentsId() === webContentsId) { + webview.blur(); + webview.focus(); + return webview; + } + } catch (error) { + // Ignore errors when getting webContentsId + } + } + return null; +}; + const subscribeToMainProcessEvents = (): void => { ipcRenderer.on(EVENT_TYPE.ACCOUNT.SSO_LOGIN, (_event, code: string) => new AutomatedSingleSignOn().start(code)); ipcRenderer.on( @@ -70,11 +87,23 @@ const subscribeToMainProcessEvents = (): void => { } }); - ipcRenderer.on(EVENT_TYPE.EDIT.COPY, () => getSelectedWebview()?.copy()); - ipcRenderer.on(EVENT_TYPE.EDIT.CUT, () => getSelectedWebview()?.cut()); - ipcRenderer.on(EVENT_TYPE.EDIT.PASTE, () => getSelectedWebview()?.paste()); + ipcRenderer.on(EVENT_TYPE.EDIT.COPY, (_event, webContentsId?: number) => { + const targetWebview = webContentsId !== undefined ? focusWebviewByContentsId(webContentsId) : getSelectedWebview(); + targetWebview?.copy(); + }); + ipcRenderer.on(EVENT_TYPE.EDIT.CUT, (_event, webContentsId?: number) => { + const targetWebview = webContentsId !== undefined ? focusWebviewByContentsId(webContentsId) : getSelectedWebview(); + targetWebview?.cut(); + }); + ipcRenderer.on(EVENT_TYPE.EDIT.PASTE, (_event, webContentsId?: number) => { + const targetWebview = webContentsId !== undefined ? focusWebviewByContentsId(webContentsId) : getSelectedWebview(); + targetWebview?.paste(); + }); ipcRenderer.on(EVENT_TYPE.EDIT.REDO, () => getSelectedWebview()?.redo()); - ipcRenderer.on(EVENT_TYPE.EDIT.SELECT_ALL, () => getSelectedWebview()?.selectAll()); + ipcRenderer.on(EVENT_TYPE.EDIT.SELECT_ALL, (_event, webContentsId?: number) => { + const targetWebview = webContentsId !== undefined ? focusWebviewByContentsId(webContentsId) : getSelectedWebview(); + targetWebview?.selectAll(); + }); ipcRenderer.on(EVENT_TYPE.EDIT.UNDO, () => getSelectedWebview()?.undo()); ipcRenderer.on(EVENT_TYPE.WRAPPER.RELOAD, (): void => { From eee12bd2e79d0ce11366d61ee7e98b8e7804a435 Mon Sep 17 00:00:00 2001 From: Adam Low Date: Mon, 15 Dec 2025 16:14:11 +0000 Subject: [PATCH 3/6] fix: handle account removal errors and webview state checks --- docs/ELECTRON_REMOTE_AUDIT.md | 483 ------------------ docs/PR_BREAKDOWN.md | 311 ----------- .../src/components/WebView/Webview.tsx | 19 +- .../components/context/EditAccountMenu.tsx | 12 +- electron/src/preload/preload-app.ts | 19 +- 5 files changed, 41 insertions(+), 803 deletions(-) delete mode 100644 docs/ELECTRON_REMOTE_AUDIT.md delete mode 100644 docs/PR_BREAKDOWN.md diff --git a/docs/ELECTRON_REMOTE_AUDIT.md b/docs/ELECTRON_REMOTE_AUDIT.md deleted file mode 100644 index 290887b8ca0..00000000000 --- a/docs/ELECTRON_REMOTE_AUDIT.md +++ /dev/null @@ -1,483 +0,0 @@ -# @electron/remote Usage Audit - -**Date**: 2024 -**Purpose**: Document all `@electron/remote` usage and plan replacement strategy for security hardening - -## Summary - -Total files using `@electron/remote`: **7 files** - -- Main process initialization: 1 file -- Preload scripts: 2 files -- Utility modules: 4 files - -## Main Process Initialization - -### File: `electron/src/main.ts` - -**Usage**: - -```typescript -import * as remoteMain from '@electron/remote/main'; -remoteMain.initialize(); // Line 78 -remoteMain.enable(main.webContents); // Line 307 -remoteMain.enable(contents); // Line 660 (for all webContents) -``` - -**Purpose**: - -- Initializes the remote module system -- Enables remote access for main window and all webviews - -**Replacement Strategy**: - -- Remove all `remoteMain.enable()` calls after replacing all remote usage -- Remove `remoteMain.initialize()` after all remote usage is replaced -- This is the LAST step - only remove after all other remote usage is eliminated - -**Dependencies**: All other remote usage depends on this - ---- - -## Preload Scripts - -### File: `electron/src/preload/preload-webview.ts` - -**Usage**: - -```typescript -const remote = require('@electron/remote'); - -// Line 52, 59: Get dark mode state -const useDarkMode = remote.nativeTheme.shouldUseDarkColors; - -// Line 69: Listen for theme changes -remote.nativeTheme.on('updated', () => updateWebAppTheme()); -``` - -**Context**: Webview preload script - runs in webview renderer process - -**Purpose**: - -- Detect system dark/light mode preference -- React to theme changes in real-time -- Publish theme updates to webapp via amplify events - -**Replacement Strategy**: - -1. **Create IPC Handler in Main Process** (`electron/src/main.ts`): - -```typescript -// Add to bindIpcEvents() or create new function -ipcMain.handle('native-theme:shouldUseDarkColors', () => { - return nativeTheme.shouldUseDarkColors; -}); - -// Listen for theme changes and notify renderers -nativeTheme.on('updated', () => { - // Broadcast to all webviews - main.webContents.send('native-theme:updated', { - shouldUseDarkColors: nativeTheme.shouldUseDarkColors, - }); -}); -``` - -2. **Expose via ContextBridge** (in preload script): - -```typescript -import {contextBridge, ipcRenderer} from 'electron'; - -contextBridge.exposeInMainWorld('electronAPI', { - nativeTheme: { - shouldUseDarkColors: () => ipcRenderer.invoke('native-theme:shouldUseDarkColors'), - onUpdated: (callback: (shouldUseDarkColors: boolean) => void) => { - ipcRenderer.on('native-theme:updated', (_event, {shouldUseDarkColors}) => { - callback(shouldUseDarkColors); - }); - }, - }, -}); -``` - -3. **Update Usage**: - -```typescript -// Replace: -const useDarkMode = remote.nativeTheme.shouldUseDarkColors; - -// With: -const useDarkMode = window.electronAPI.nativeTheme.shouldUseDarkColors(); - -// Replace: -remote.nativeTheme.on('updated', () => updateWebAppTheme()); - -// With: -window.electronAPI.nativeTheme.onUpdated(shouldUseDarkColors => { - // Update theme logic -}); -``` - -**Complexity**: Medium -**Risk**: Low - Theme detection is straightforward -**Testing**: Verify dark/light mode switching works - ---- - -### File: `electron/src/preload/menu/preload-context.ts` - -**Usage**: - -```typescript -const remote = require('@electron/remote'); - -// Line 64, 113, 116: Build context menus -remote.Menu.buildFromTemplate([...]); - -// Line 127: Get current webContents -const webContents = remote.getCurrentWebContents(); - -// Line 130: Get current window -const window = remote.getCurrentWindow(); -``` - -**Context**: Context menu preload script - runs in main window renderer process - -**Purpose**: - -- Create context menus (text menu, image menu, default menu) -- Get current webContents for menu operations -- Get current window for menu popup positioning - -**Replacement Strategy**: - -1. **Menu Building**: - - - Menus can be built in preload script using `Menu` from `electron` (not remote) - - `Menu` is available in preload context when context isolation is enabled - - **Change**: `const {Menu} = require('electron')` instead of `remote.Menu` - -2. **getCurrentWebContents()**: - - - In preload script, we can use `webContents` from the context - - However, since this is for the main window, we need to pass webContents ID from main process - - **Alternative**: Use IPC to get webContents ID, or pass it during preload initialization - - **Better approach**: Since this is in the main window preload, we can access `webContents` directly if available - - **Note**: Need to verify if `webContents` is accessible in preload with context isolation - -3. **getCurrentWindow()**: - - Similar to webContents - need window reference - - **Alternative**: Pass window ID via IPC or during initialization - - **Better approach**: Menu.popup() can work without explicit window reference in some cases - -**Detailed Replacement**: - -```typescript -// Replace remote import -import {Menu, WebContents} from 'electron'; - -// For getCurrentWebContents - we need to get it from main process -// Option 1: Pass webContents ID via IPC during initialization -// Option 2: Use a global that's set by main process -// Option 3: Create IPC handler to get current webContents ID - -// For getCurrentWindow - similar approach -// Menu.popup() can work with just options, window is optional - -// Create IPC handler in main process: -ipcMain.handle('get-current-webcontents-id', event => { - return event.sender.id; -}); - -ipcMain.handle('get-current-window-id', event => { - const window = BrowserWindow.fromWebContents(event.sender); - return window?.id; -}); - -// In preload: -const webContentsId = await ipcRenderer.invoke('get-current-webcontents-id'); -const windowId = await ipcRenderer.invoke('get-current-window-id'); - -// But actually, for context menus, we might not need the window reference -// Menu.popup() can infer it from the event -``` - -**Actually, simpler approach**: - -- `Menu.buildFromTemplate()` works with regular `Menu` import (not remote) -- For `menu.popup()`, we can pass `{window: null}` or get window via IPC -- For `webContents.replaceMisspelling()`, we need webContents reference - pass via IPC - -**Complexity**: High - Context menu system is complex -**Risk**: Medium - Context menus are user-facing, must work correctly -**Testing**: Test all context menu types (text, image, link, selection) - ---- - -## Utility Modules - -### File: `electron/src/logging/getLogger.ts` - -**Usage**: - -```typescript -const mainProcess = process || require('@electron/remote').process; -const app = Electron.app || require('@electron/remote').app; - -// Line 36: Check command line arguments -const forceLogging = mainProcess.argv.includes('--enable-logging'); - -// Line 32: Get user data path -const logDir = path.join(app.getPath('userData'), 'logs'); -``` - -**Context**: Utility module - may run in main or renderer process - -**Purpose**: - -- Access process.argv for logging flags -- Get app.getPath('userData') for log directory - -**Replacement Strategy**: - -1. **For Main Process** (when `process` is available): - - - Use `process` directly - no change needed - - Use `app` directly from `electron` - no change needed - -2. **For Renderer Process** (when remote is used): - - - **process.argv**: Create IPC handler `ipcMain.handle('process:argv', () => process.argv)` - - **app.getPath()**: Create IPC handler `ipcMain.handle('app:getPath', (event, name) => app.getPath(name))` - -3. **Update Code**: - -```typescript -// Detect if we're in main or renderer -const isMainProcess = typeof process !== 'undefined' && process.type === 'browser'; - -let processArgv: string[] = []; -let getUserDataPath: () => string; - -if (isMainProcess) { - processArgv = process.argv; - getUserDataPath = () => app.getPath('userData'); -} else { - // Renderer process - use IPC - processArgv = await ipcRenderer.invoke('process:argv'); - getUserDataPath = () => ipcRenderer.invoke('app:getPath', 'userData'); -} -``` - -**Note**: `getLogger` is used in both main process (main.ts, settings, etc.) and preload scripts (preload-app.ts, preload-webview.ts), so it must handle both contexts. - -**Implementation**: - -```typescript -import {ipcRenderer} from 'electron'; - -const isMainProcess = typeof process !== 'undefined' && process.type === 'browser'; - -let processArgv: string[] = []; -let getUserDataPath: () => string | Promise; - -if (isMainProcess) { - // Main process - direct access - processArgv = process.argv; - getUserDataPath = () => app.getPath('userData'); -} else { - // Renderer process - use IPC (requires contextBridge setup) - // Note: This will need to be async or use synchronous IPC - // For now, we can make it async and update callers - processArgv = []; // Will be populated via IPC - getUserDataPath = async () => { - // Requires IPC handler: ipcMain.handle('app:getPath', (event, name) => app.getPath(name)) - return await ipcRenderer.invoke('app:getPath', 'userData'); - }; -} - -// Update ENABLE_LOGGING to be async-aware or use synchronous approach -const forceLogging = isMainProcess ? process.argv.includes('--enable-logging') : false; // Will need IPC to check in renderer, or pass via contextBridge -``` - -**Alternative Simpler Approach**: Since logging initialization happens early, we can: - -1. Pass logging config via contextBridge during preload initialization -2. Or make the check synchronous by exposing it via contextBridge - -**Complexity**: Medium (due to dual context usage) -**Risk**: Low -**Testing**: Verify logging works in both main and renderer contexts - ---- - -### File: `electron/src/locale/index.ts` - -**Usage**: - -```typescript -const app = Electron.app || require('@electron/remote').app; - -// Line 144: Get system locale -const defaultLocale = parseLocale(app.getLocale().substring(0, 2)); -``` - -**Context**: Utility module - may run in main or renderer process - -**Purpose**: - -- Get system locale for language detection - -**Replacement Strategy**: - -1. **For Main Process**: Use `app` directly - no change needed - -2. **For Renderer Process**: - - - Create IPC handler: `ipcMain.handle('app:getLocale', () => app.getLocale())` - - Use IPC in renderer: `const locale = await ipcRenderer.invoke('app:getLocale')` - -3. **Update Code**: - -```typescript -const isMainProcess = typeof process !== 'undefined' && process.type === 'browser'; - -let getLocale: () => string; - -if (isMainProcess) { - getLocale = () => app.getLocale(); -} else { - getLocale = async () => await ipcRenderer.invoke('app:getLocale'); -} -``` - -**Complexity**: Low -**Risk**: Low -**Testing**: Verify locale detection works correctly - ---- - -### File: `electron/src/settings/SchemaUpdater.ts` - -**Usage**: - -```typescript -const app = Electron.app || require('@electron/remote').app; - -// Line 32: Get user data path -const defaultPathV0 = path.join(app.getPath('userData'), 'init.json'); -const defaultPathV1 = path.join(app.getPath('userData'), 'config/init.json'); -``` - -**Context**: Utility module - likely runs in main process only - -**Purpose**: - -- Get user data directory for config file paths - -**Replacement Strategy**: - -1. **Check where this is used**: If only in main process, use `app` directly -2. **If used in renderer**: Create IPC handler `ipcMain.handle('app:getPath', (event, name) => app.getPath(name))` - -**Complexity**: Low -**Risk**: Low -**Testing**: Verify config file paths are correct - ---- - -### File: `electron/src/sso/AutomatedSingleSignOn.ts` - -**Usage**: - -```typescript -const dialog = Electron.dialog || require('@electron/remote').dialog; - -// Line 42: Show error dialog -await dialog.showMessageBox({ - detail, - message, - type: 'warning', -}); -``` - -**Context**: SSO automation - runs in renderer process (preload-app.ts calls this) - -**Purpose**: - -- Show error dialog when maximum accounts reached during SSO - -**Replacement Strategy**: - -1. **Create IPC Handler** in main process: - -```typescript -ipcMain.handle('dialog:showMessageBox', async (event, options) => { - const window = BrowserWindow.fromWebContents(event.sender); - const result = await dialog.showMessageBox(window || undefined, options); - return result; -}); -``` - -2. **Update Code**: - -```typescript -// Remove remote import -// Add IPC call -const result = await ipcRenderer.invoke('dialog:showMessageBox', { - detail, - message, - type: 'warning', -}); -``` - -**Complexity**: Low -**Risk**: Medium - SSO error handling is important -**Testing**: Test SSO with maximum accounts reached, verify dialog appears - ---- - -## Replacement Priority Order - -1. **Low Risk, Simple**: - - - `locale/index.ts` - app.getLocale() - - `settings/SchemaUpdater.ts` - app.getPath() - - `logging/getLogger.ts` - process.argv, app.getPath() - -2. **Medium Risk, Medium Complexity**: - - - `preload/preload-webview.ts` - nativeTheme - - `sso/AutomatedSingleSignOn.ts` - dialog.showMessageBox - -3. **High Risk, High Complexity**: - - - `preload/menu/preload-context.ts` - Menu, getCurrentWebContents, getCurrentWindow - -4. **Final Step**: - - `main.ts` - Remove remoteMain.initialize() and remoteMain.enable() calls - -## Testing Checklist for Each Replacement - -- [ ] Functionality works as before -- [ ] No console errors -- [ ] No performance regressions -- [ ] SSO flow works (for SSO-related changes) -- [ ] Context menus work (for menu changes) -- [ ] Theme switching works (for theme changes) -- [ ] Error dialogs appear (for dialog changes) - -## Notes - -- All replacements assume context isolation is enabled (Phase 2) -- IPC handlers should be added to `bindIpcEvents()` or a similar centralized location -- Consider creating a typed IPC API for better maintainability -- Some replacements may require updating TypeScript types - -## Estimated Effort - -- Low complexity: 1-2 hours each (3 files) -- Medium complexity: 2-4 hours each (2 files) -- High complexity: 4-6 hours (1 file) -- Testing and verification: 4-6 hours - -**Total**: ~20-30 hours diff --git a/docs/PR_BREAKDOWN.md b/docs/PR_BREAKDOWN.md deleted file mode 100644 index ccff854c7c6..00000000000 --- a/docs/PR_BREAKDOWN.md +++ /dev/null @@ -1,311 +0,0 @@ -# Electron Security Hardening - PR Breakdown - -## Prerequisites for Sandboxing - -**Sandboxing requires**: - -1. ✅ Context isolation enabled (mandatory) -2. ✅ @electron/remote removed (doesn't work with sandbox) -3. ✅ No Node.js APIs in preload scripts (sandbox restricts access) - -**Current blockers for sandboxing**: - -- `contextIsolation: false` in main window -- `@electron/remote` usage throughout codebase -- Node.js modules in preload scripts (`path`, `process`, etc.) - -## PR Structure - -### PR 1: Enable Context Isolation (Main Window) - -**Goal**: Enable context isolation and refactor preload scripts to use contextBridge - -**Files Changed**: - -- `electron/src/main.ts` - Change `contextIsolation: false` → `true` -- `electron/src/preload/preload-app.ts` - Refactor to use contextBridge -- `electron/src/preload/preload-webview.ts` - Refactor to use contextBridge -- `electron/renderer/src/**` - Update all renderer code to use new APIs - -**Key Changes**: - -- Replace `window.sendBadgeCount` with `window.electronAPI.sendBadgeCount` -- Replace all direct window assignments with contextBridge.exposeInMainWorld() -- Update all renderer code that uses these APIs - -**Testing**: - -- All existing functionality works -- SSO flow works -- IPC communication works - -**Dependencies**: None (can be first PR) - ---- - -### PR 2: Replace @electron/remote - Utility Modules (Low Risk) - -**Goal**: Replace remote usage in utility modules that don't affect UI - -**Files Changed**: - -- `electron/src/locale/index.ts` - Replace `remote.app.getLocale()` with IPC -- `electron/src/settings/SchemaUpdater.ts` - Replace `remote.app.getPath()` with IPC -- `electron/src/logging/getLogger.ts` - Replace `remote.process` and `remote.app` with IPC -- `electron/src/main.ts` - Add IPC handlers: `app:getLocale`, `app:getPath`, `process:argv` - -**Key Changes**: - -- Add IPC handlers in main process -- Update utility modules to use IPC when in renderer context -- Keep main process direct access when available - -**Testing**: - -- Locale detection works -- Config file paths are correct -- Logging works in both main and renderer - -**Dependencies**: PR 1 (needs context isolation for IPC in renderer) - ---- - -### PR 3: Replace @electron/remote - Native Theme - -**Goal**: Replace remote.nativeTheme usage with IPC - -**Files Changed**: - -- `electron/src/preload/preload-webview.ts` - Replace `remote.nativeTheme` with IPC -- `electron/src/main.ts` - Add IPC handlers: `native-theme:shouldUseDarkColors`, `native-theme:updated` event - -**Key Changes**: - -- Create IPC handler for theme detection -- Broadcast theme changes to all webviews -- Expose via contextBridge in preload - -**Testing**: - -- Dark/light mode detection works -- Theme switching works in real-time -- No console errors - -**Dependencies**: PR 1 (needs context isolation) - ---- - -### PR 4: Replace @electron/remote - Dialog (SSO) - -**Goal**: Replace remote.dialog usage in SSO error handling - -**Files Changed**: - -- `electron/src/sso/AutomatedSingleSignOn.ts` - Replace `remote.dialog.showMessageBox()` with IPC -- `electron/src/main.ts` - Add IPC handler: `dialog:showMessageBox` - -**Key Changes**: - -- Create IPC handler that shows dialog in main process -- Return dialog result via IPC -- Update SSO error handling to use IPC - -**Testing**: - -- SSO error dialog appears when max accounts reached -- Dialog buttons work correctly -- SSO flow continues to work - -**Dependencies**: PR 1 (needs context isolation) - ---- - -### PR 5: Replace @electron/remote - Context Menu (Complex) - -**Goal**: Replace remote.Menu, remote.getCurrentWebContents(), remote.getCurrentWindow() - -**Files Changed**: - -- `electron/src/preload/menu/preload-context.ts` - Replace all remote usage -- `electron/src/main.ts` - Add IPC handlers if needed (may not be needed - Menu works in preload) - -**Key Changes**: - -- Replace `remote.Menu` with `Menu` from `electron` (available in preload) -- Replace `remote.getCurrentWebContents()` - may need IPC or pass via contextBridge -- Replace `remote.getCurrentWindow()` - may need IPC or pass via contextBridge -- Update menu.popup() calls - -**Testing**: - -- All context menu types work (text, image, link, selection) -- Menu positioning is correct -- No console errors - -**Dependencies**: PR 1 (needs context isolation) - ---- - -### PR 6: Remove @electron/remote Package - -**Goal**: Remove all remoteMain initialization and the package dependency - -**Files Changed**: - -- `electron/src/main.ts` - Remove `remoteMain.initialize()` and all `remoteMain.enable()` calls -- `package.json` - Remove `@electron/remote` dependency - -**Key Changes**: - -- Remove remoteMain imports -- Remove remoteMain.initialize() -- Remove remoteMain.enable() calls -- Remove package from dependencies - -**Testing**: - -- App starts correctly -- No remote-related errors -- All functionality still works - -**Dependencies**: PRs 2, 3, 4, 5 (all remote usage must be replaced first) - ---- - -### PR 7: Enable Sandbox (Main Window) - -**Goal**: Enable sandbox for main window - -**Files Changed**: - -- `electron/src/main.ts` - Change `sandbox: false` → `true` in main window - -**Key Changes**: - -- Enable sandbox in webPreferences -- Verify preload scripts work in sandboxed context -- Ensure no Node.js APIs are used in preload - -**Blockers Check**: - -- ✅ Context isolation enabled (PR 1) -- ✅ @electron/remote removed (PR 6) -- ⚠️ Node.js modules in preload - need to check: - - `path` module in preload-app.ts and preload-webview.ts - - `getLogger` uses path - needs IPC or refactor - -**Testing**: - -- All functionality works -- SSO works -- No security warnings -- Performance is acceptable - -**Dependencies**: PR 1, PR 6 (context isolation + remote removal) - ---- - -### PR 8: Enable Sandbox (Webviews) - -**Goal**: Enable sandbox for webviews - -**Files Changed**: - -- `electron/src/main.ts` - Change `sandbox: false` → `true` in webview configuration (line 679) - -**Key Changes**: - -- Enable sandbox for webviews -- Verify SSO JavaScript injection still works (`executeJavaScriptWithoutResult`) -- Ensure webview preload works in sandboxed context - -**Critical SSO Test**: - -- SSO window opens -- SSO login completes -- JavaScript injection for SSO response works -- Cookie transfer works - -**Testing**: - -- All webview functionality works -- SSO flow works completely -- No security warnings - -**Dependencies**: PR 7 (main window sandbox first) - ---- - -## Node.js Modules in Preload Scripts - -### Current Usage: - -- `path` module in `preload-app.ts` and `preload-webview.ts` (line 23) -- Used for: `path.basename(__filename)` in getLogger calls - -### Solution Options: - -1. **Remove path usage**: Pass logger name as parameter instead of using `__filename` -2. **Use IPC**: Get path info via IPC (overkill for this) -3. **Use string manipulation**: Replace `path.basename(__filename)` with string operations - -**Recommended**: Option 1 - Pass logger name explicitly or use a different method - -**Files to Update**: - -- `electron/src/preload/preload-app.ts` - Remove `path` import, pass logger name explicitly -- `electron/src/preload/preload-webview.ts` - Remove `path` import, pass logger name explicitly - -This should be done in **PR 1** or **PR 7** (before enabling sandbox). - ---- - -## PR Dependencies Graph - -``` -PR 1 (Context Isolation) - ├─> PR 2 (Remote - Utilities) - ├─> PR 3 (Remote - Native Theme) - ├─> PR 4 (Remote - Dialog) - └─> PR 5 (Remote - Context Menu) - │ - └─> PR 6 (Remove Remote Package) - │ - └─> PR 7 (Sandbox Main Window) - │ - └─> PR 8 (Sandbox Webviews) -``` - -## Testing Strategy Per PR - -Each PR should include: - -1. Unit tests pass -2. Integration tests pass -3. Manual SSO testing (for PRs that touch SSO) -4. Manual functionality testing -5. No console errors or warnings - -## Estimated Timeline - -- PR 1: 3-5 days -- PR 2: 1-2 days -- PR 3: 1-2 days -- PR 4: 1-2 days -- PR 5: 3-4 days (most complex) -- PR 6: 1 day -- PR 7: 2-3 days -- PR 8: 2-3 days - -**Total**: ~2-3 weeks - -## Critical Path - -The critical path for sandboxing is: - -1. PR 1 (Context Isolation) - Foundation -2. PR 6 (Remove Remote) - Required for sandbox -3. PR 7 (Sandbox Main) - First sandbox -4. PR 8 (Sandbox Webviews) - Complete sandbox - -PRs 2-5 can be done in parallel after PR 1, but PR 6 must wait for all of them. diff --git a/electron/renderer/src/components/WebView/Webview.tsx b/electron/renderer/src/components/WebView/Webview.tsx index 2afec028da7..66f6bb73b02 100644 --- a/electron/renderer/src/components/WebView/Webview.tsx +++ b/electron/renderer/src/components/WebView/Webview.tsx @@ -283,9 +283,24 @@ const Webview = ({ }, [account, accountLifecycle, conversationJoinData]); const deleteWebview = (account: Account) => { - window.electronAPI.sendDeleteAccount(account.id, account.sessionID).then(() => { + // For accounts being added (no webview yet), just abort creation directly + const accountWebview = document.querySelector(`.Webview[data-accountid="${account.id}"]`); + if (!accountWebview) { + // Webview doesn't exist yet (account being added), just abort creation abortAccountCreation(account.id); - }); + return; + } + + // For existing accounts, delete the webview data first + window.electronAPI.sendDeleteAccount(account.id, account.sessionID) + .then(() => { + abortAccountCreation(account.id); + }) + .catch((error) => { + console.error('Failed to delete account:', error); + // Still abort account creation even if deletion fails + abortAccountCreation(account.id); + }); }; return ( diff --git a/electron/renderer/src/components/context/EditAccountMenu.tsx b/electron/renderer/src/components/context/EditAccountMenu.tsx index c12ebed224a..f3cd398dc9d 100644 --- a/electron/renderer/src/components/context/EditAccountMenu.tsx +++ b/electron/renderer/src/components/context/EditAccountMenu.tsx @@ -73,9 +73,15 @@ const EditAccountMenu = ({ { - window.electronAPI.sendDeleteAccount(accountId, sessionID).then(() => { - connected.abortAccountCreation(accountId); - }); + window.electronAPI.sendDeleteAccount(accountId, sessionID) + .then(() => { + connected.abortAccountCreation(accountId); + }) + .catch((error) => { + console.error('Failed to delete account:', error); + // Still abort account creation even if deletion fails + connected.abortAccountCreation(accountId); + }); }} > {getText('wrapperRemoveAccount')} diff --git a/electron/src/preload/preload-app.ts b/electron/src/preload/preload-app.ts index 858c2cbed63..0d2a5e1733a 100644 --- a/electron/src/preload/preload-app.ts +++ b/electron/src/preload/preload-app.ts @@ -148,10 +148,21 @@ const electronAPI = { return reject(`Webview for account "${truncatedId}" does not exist`); } - logger.info(`Processing deletion of "${truncatedId}"`); - const viewInstanceId = accountWebview.getWebContentsId(); - ipcRenderer.on(EVENT_TYPE.ACCOUNT.DATA_DELETED, () => resolve()); - ipcRenderer.send(EVENT_TYPE.ACCOUNT.DELETE_DATA, viewInstanceId, accountId, sessionID); + try { + // getWebContentsId() may not be available until webview is fully attached + if (typeof accountWebview.getWebContentsId !== 'function') { + // eslint-disable-next-line prefer-promise-reject-errors + return reject(`Webview for account "${truncatedId}" is not ready (getWebContentsId not available)`); + } + + logger.info(`Processing deletion of "${truncatedId}"`); + const viewInstanceId = accountWebview.getWebContentsId(); + ipcRenderer.on(EVENT_TYPE.ACCOUNT.DATA_DELETED, () => resolve()); + ipcRenderer.send(EVENT_TYPE.ACCOUNT.DELETE_DATA, viewInstanceId, accountId, sessionID); + } catch (error) { + // eslint-disable-next-line prefer-promise-reject-errors + reject(`Failed to get webContents ID for account "${truncatedId}": ${error}`); + } }); }, sendLogoutAccount: async (accountId: string): Promise => { From 4d3635f15a63fe2dd85e8977d0826c3548a3a567 Mon Sep 17 00:00:00 2001 From: Adam Low Date: Mon, 15 Dec 2025 18:03:49 +0000 Subject: [PATCH 4/6] refactor: fix locale timing issue with getter function, improve code quality --- .../src/components/WebView/Webview.tsx | 4 +- electron/renderer/src/lib/WindowUrl.ts | 4 +- electron/renderer/src/lib/locale.ts | 40 +++++-------------- 3 files changed, 15 insertions(+), 33 deletions(-) diff --git a/electron/renderer/src/components/WebView/Webview.tsx b/electron/renderer/src/components/WebView/Webview.tsx index 66f6bb73b02..5d0d0fe0e75 100644 --- a/electron/renderer/src/components/WebView/Webview.tsx +++ b/electron/renderer/src/components/WebView/Webview.tsx @@ -38,7 +38,7 @@ import { } from '../../actions'; import {accountAction} from '../../actions/AccountAction'; import {State} from '../../index'; -import {getText, wrapperLocale} from '../../lib/locale'; +import {getText, getWrapperLocale} from '../../lib/locale'; import {WindowUrl} from '../../lib/WindowUrl'; import {AccountSelector} from '../../selector/AccountSelector'; import {Account, ConversationJoinData} from '../../types/account'; @@ -58,7 +58,7 @@ const getEnvironmentUrl = (account: Account) => { url.searchParams.set('id', account.id); // set the current language - url.searchParams.set('hl', wrapperLocale); + url.searchParams.set('hl', getWrapperLocale()); if (account.ssoCode && account.isAdding) { url.pathname = '/auth'; diff --git a/electron/renderer/src/lib/WindowUrl.ts b/electron/renderer/src/lib/WindowUrl.ts index d648e740b11..9e4198d8003 100644 --- a/electron/renderer/src/lib/WindowUrl.ts +++ b/electron/renderer/src/lib/WindowUrl.ts @@ -17,7 +17,7 @@ * */ -import {wrapperLocale} from './locale'; +import {getWrapperLocale} from './locale'; export class WindowUrl { static createWebAppUrl(localRendererUrl: URL | string, customBackendUrl: string) { @@ -32,7 +32,7 @@ export class WindowUrl { }); // set the current language - envUrlParams.set('hl', wrapperLocale); + envUrlParams.set('hl', getWrapperLocale()); return customBackendUrlParsed.href; } diff --git a/electron/renderer/src/lib/locale.ts b/electron/renderer/src/lib/locale.ts index 5e15ca934c4..7ea9f7c0ead 100644 --- a/electron/renderer/src/lib/locale.ts +++ b/electron/renderer/src/lib/locale.ts @@ -64,32 +64,14 @@ export const getText = (stringIdentifier: i18nLanguageIdentifier, paramReplaceme return str; }; -// wrapperLocale is used as a constant in WindowUrl.ts and Webview.tsx -// We'll initialize it lazily - it will be 'en' initially and may update when electronAPI becomes available -// For now, we'll use a getter function approach, but export it as a value for compatibility -let _wrapperLocale: SupportedI18nLanguage = 'en' as SupportedI18nLanguage; - -// Try to initialize immediately, but don't fail if electronAPI isn't ready -try { - const info = getLocaleInfo(); - _wrapperLocale = info.current; -} catch { - // Ignore errors - will use 'en' as default -} - -// Update locale when electronAPI becomes available (if it wasn't available initially) -if (typeof window !== 'undefined') { - // Use a small delay to allow preload script to finish - setTimeout(() => { - try { - const info = getLocaleInfo(); - if (info.current !== 'en' || window.electronAPI) { - _wrapperLocale = info.current; - } - } catch { - // Ignore errors - } - }, 100); -} - -export const wrapperLocale: SupportedI18nLanguage = _wrapperLocale; +// getWrapperLocale is used in WindowUrl.ts and Webview.tsx +// Use a getter function to always get the current locale value +// Electron guarantees preload scripts execute before renderer code, +// so window.electronAPI should always be available +export const getWrapperLocale = (): SupportedI18nLanguage => { + if (window.electronAPI) { + return window.electronAPI.locale.current; + } + // Fallback for development/testing or edge cases + return 'en' as SupportedI18nLanguage; +}; From 3f97a5ed3d68ded13134403913fa7346919d4786 Mon Sep 17 00:00:00 2001 From: Adam Low Date: Mon, 15 Dec 2025 18:26:22 +0000 Subject: [PATCH 5/6] fix: address React hooks linting warnings (pre-existing issues) --- .../src/components/WebView/Webview.tsx | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/electron/renderer/src/components/WebView/Webview.tsx b/electron/renderer/src/components/WebView/Webview.tsx index 5d0d0fe0e75..62531bde649 100644 --- a/electron/renderer/src/components/WebView/Webview.tsx +++ b/electron/renderer/src/components/WebView/Webview.tsx @@ -169,11 +169,12 @@ const Webview = ({ setWebviewError(error); } }; - webviewRef.current?.addEventListener(ON_WEBVIEW_ERROR, listener); + const webview = webviewRef.current; + webview?.addEventListener(ON_WEBVIEW_ERROR, listener); return () => { - if (webviewRef.current) { - webviewRef.current.removeEventListener(ON_WEBVIEW_ERROR, listener); + if (webview) { + webview.removeEventListener(ON_WEBVIEW_ERROR, listener); } }; }, [webviewRef, account]); @@ -272,11 +273,12 @@ const Webview = ({ } }; - webviewRef.current?.addEventListener(ON_IPC_MESSAGE, onIpcMessage); + const webview = webviewRef.current; + webview?.addEventListener(ON_IPC_MESSAGE, onIpcMessage); return () => { - if (webviewRef.current) { - webviewRef.current.removeEventListener(ON_IPC_MESSAGE, onIpcMessage); + if (webview) { + webview.removeEventListener(ON_IPC_MESSAGE, onIpcMessage); } }; // eslint-disable-next-line react-hooks/exhaustive-deps @@ -292,11 +294,12 @@ const Webview = ({ } // For existing accounts, delete the webview data first - window.electronAPI.sendDeleteAccount(account.id, account.sessionID) + window.electronAPI + .sendDeleteAccount(account.id, account.sessionID) .then(() => { abortAccountCreation(account.id); }) - .catch((error) => { + .catch(error => { console.error('Failed to delete account:', error); // Still abort account creation even if deletion fails abortAccountCreation(account.id); From b817bae5d63cfff3ba78efe33c23a972603391da Mon Sep 17 00:00:00 2001 From: Adam Low Date: Mon, 15 Dec 2025 18:27:14 +0000 Subject: [PATCH 6/6] fix: address ESLint formatting issues in EditAccountMenu --- electron/renderer/src/components/context/EditAccountMenu.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/electron/renderer/src/components/context/EditAccountMenu.tsx b/electron/renderer/src/components/context/EditAccountMenu.tsx index f3cd398dc9d..20339b075b8 100644 --- a/electron/renderer/src/components/context/EditAccountMenu.tsx +++ b/electron/renderer/src/components/context/EditAccountMenu.tsx @@ -73,11 +73,12 @@ const EditAccountMenu = ({ { - window.electronAPI.sendDeleteAccount(accountId, sessionID) + window.electronAPI + .sendDeleteAccount(accountId, sessionID) .then(() => { connected.abortAccountCreation(accountId); }) - .catch((error) => { + .catch(error => { console.error('Failed to delete account:', error); // Still abort account creation even if deletion fails connected.abortAccountCreation(accountId);