Skip to content

fix: Critical security vulnerabilities #1312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Zeyad-Azima wants to merge 1 commit into from
+77 −6
Closed

fix: Critical security vulnerabilities #1312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 72 additions & 1 deletion src/main/presenter/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import path from 'path'
import { DialogPresenter } from './dialogPresenter/index'
import { ipcMain, IpcMainInvokeEvent, app } from 'electron'
import { ipcMain, IpcMainInvokeEvent, app, shell, dialog } from 'electron'
import { WindowPresenter } from './windowPresenter'
import { ShortcutPresenter } from './shortcutPresenter'
import {
Expand Down Expand Up @@ -201,6 +201,77 @@ export class Presenter implements IPresenter {
})

this.setupEventBus() // 设置事件总线监听
this.setupSecurityHandlers() // ✅ SECURITY: Setup secure IPC handlers
}

// ✅ SECURITY FIX: Secure IPC handlers for potentially dangerous operations
private setupSecurityHandlers() {
// Secure openExternal handler with protocol whitelist and user confirmation
ipcMain.handle('open-external-secure', async (_event: IpcMainInvokeEvent, url: string) => {
try {
const parsedUrl = new URL(url)

// 1. Protocol whitelist - only allow http and https
const allowedProtocols = ['http:', 'https:']
if (!allowedProtocols.includes(parsedUrl.protocol)) {
console.error('🔴 SECURITY: Blocked dangerous protocol:', parsedUrl.protocol, 'URL:', url)
return {
success: false,
error: `Protocol "${parsedUrl.protocol}" is not allowed. Only HTTP and HTTPS links are permitted.`
}
}

// 2. Domain whitelist for trusted domains (no confirmation needed)
const trustedDomains = [
'openai.com',
'api.openai.com',
'anthropic.com',
'api.anthropic.com',
'github.com',
'docs.deepchat.com',
'google.com',
'gemini.google.com'
]

const isTrusted = trustedDomains.some(domain =>
parsedUrl.hostname === domain || parsedUrl.hostname.endsWith('.' + domain)
)

// 3. Show user confirmation dialog for untrusted domains
if (!isTrusted) {
const { response } = await dialog.showMessageBox({
type: 'warning',
title: 'Open External Link?',
message: `DeepChat wants to open an external link:\n\n${url}\n\nDo you trust this website?`,
buttons: ['Cancel', 'Open Link'],
defaultId: 0,
cancelId: 0,
detail: 'Only open links from sources you trust. Malicious links can compromise your system.',
noLink: true
})

if (response !== 1) {
console.log('🛡️ SECURITY: User declined to open untrusted URL:', url)
return {
success: false,
error: 'User declined to open link'
}
}
}

// 4. Safe to open
await shell.openExternal(url)
console.log('✅ SECURITY: Opened external URL:', url, isTrusted ? '(trusted)' : '(user confirmed)')
return { success: true }

} catch (error) {
console.error('🔴 SECURITY: Invalid URL or error opening external link:', error)
return {
success: false,
error: error instanceof Error ? error.message : 'Invalid URL format'
}
}
})
}

public static getInstance(lifecycleManager: ILifecycleManager): Presenter {
Expand Down
4 changes: 2 additions & 2 deletions src/main/presenter/windowPresenter/FloatingChatWindow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ export class FloatingChatWindow {
nodeIntegration: false,
contextIsolation: true,
preload: path.join(__dirname, '../preload/index.mjs'),
webSecurity: false,
webSecurity: true, // ✅ SECURITY FIX: Enable same-origin policy to prevent file:// access
devTools: isDev,
sandbox: false
sandbox: true // ✅ SECURITY FIX: Enable sandbox for process isolation
}
})

Expand Down
6 changes: 3 additions & 3 deletions src/preload/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import {
nativeImage,
webUtils,
webFrame,
ipcRenderer,
shell
ipcRenderer
} from 'electron'
import { exposeElectronAPI } from '@electron-toolkit/preload'

Expand Down Expand Up @@ -44,7 +43,8 @@ const api = {
return cachedWebContentsId
},
openExternal: (url: string) => {
return shell.openExternal(url)
// ✅ SECURITY FIX: Use secure IPC instead of direct shell.openExternal
return ipcRenderer.invoke('open-external-secure', url)
},
Comment on lines 45 to 48
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n "openExternal" --type=ts --type=vue -C 3 -g '!node_modules/**' -g '!src/preload/**' -g '!src/main/**'

Repository: ThinkInAIXYZ/deepchat

Length of output: 92

🏁 Script executed:

rg -n "openExternal" --type ts --type js -C 3 -g '!node_modules/**' -g '!src/preload/**' -g '!src/main/**'

Repository: ThinkInAIXYZ/deepchat

Length of output: 4934

🏁 Script executed:

# Read the preload file to confirm return type
head -100 src/preload/index.ts | tail -60

Repository: ThinkInAIXYZ/deepchat

Length of output: 1842

🏁 Script executed:

# Search for all openExternal calls without the exclusions
fd -e ts -e tsx -e vue | xargs rg -l "openExternal" | head -20

Repository: ThinkInAIXYZ/deepchat

Length of output: 505

🏁 Script executed:

cat src/preload/index.d.ts

Repository: ThinkInAIXYZ/deepchat

Length of output: 637

🏁 Script executed:

# Check if AboutUsSettings.vue will fail TypeScript strict checks
grep -A 5 "const openExternalLink" src/renderer/settings/components/AboutUsSettings.vue

Repository: ThinkInAIXYZ/deepchat

Length of output: 244

Type definition mismatch: Update index.d.ts to reflect actual return type.

The type definition declares openExternal?(url: string): Promise<void>, but the implementation returns ipcRenderer.invoke('open-external-secure', url), which resolves to an object with {success: boolean, error?: string}. This breaks strict TypeScript type checking.

Update src/preload/index.d.ts to:

openExternal?(url: string): Promise<{success: boolean, error?: string}>

Then update the caller in src/renderer/settings/components/AboutUsSettings.vue (line 259) to handle the new return shape:

const result = await window.api.openExternal(url)
if (!result?.success) {
  console.error('Failed to open external link:', result?.error)
}
🤖 Prompt for AI Agents 
In `@src/preload/index.ts` around lines 45 - 48, The preload API's openExternal
implementation returns ipcRenderer.invoke('open-external-secure', url) which
resolves to {success: boolean, error?: string} but index.d.ts still types
openExternal as Promise<void>; update the declaration for openExternal in
index.d.ts to Promise<{success: boolean, error?: string}> and then update the
caller in AboutUsSettings.vue (the code invoking window.api.openExternal) to
await the result and check result?.success, handling/logging result?.error when
success is false.

toRelativePath: (filePath: string, baseDir?: string) => {
if (!baseDir) return filePath
Expand Down
Loading