diff --git a/packages/core/src/utils/secure-browser-launcher.test.ts b/packages/core/src/utils/secure-browser-launcher.test.ts index de27ce6ffd..4e986b0786 100644 --- a/packages/core/src/utils/secure-browser-launcher.test.ts +++ b/packages/core/src/utils/secure-browser-launcher.test.ts @@ -95,13 +95,11 @@ describe('secure-browser-launcher', () => { it('should prevent PowerShell command injection on Windows', async () => { setPlatform('win32'); - // The POC from the vulnerability report const maliciousUrl = "http://127.0.0.1:8080/?param=example#$(Invoke-Expression([System.Text.Encoding]::UTF8.GetString([System.Convert]::FromBase64String('Y2FsYy5leGU='))))"; await openBrowserSecurely(maliciousUrl); - // Verify that execFile was called (not exec) and the URL is passed safely expect(mockExecFile).toHaveBeenCalledWith( 'powershell.exe', [ @@ -130,7 +128,6 @@ describe('secure-browser-launcher', () => { for (const url of urlsWithSpecialChars) { await openBrowserSecurely(url); - // Verify the URL is passed as an argument, not interpreted by shell expect(mockExecFile).toHaveBeenCalledWith( 'open', [url], @@ -146,7 +143,6 @@ describe('secure-browser-launcher', () => { "http://example.com/path?name=O'Brien&test='value'"; await openBrowserSecurely(urlWithSingleQuotes); - // Verify that single quotes are escaped by doubling them expect(mockExecFile).toHaveBeenCalledWith( 'powershell.exe', [ @@ -205,21 +201,29 @@ describe('secure-browser-launcher', () => { }); describe('Error handling', () => { - it('should handle browser launch failures gracefully', async () => { + it('should handle browser launch failures gracefully by logging instead of throwing', async () => { setPlatform('darwin'); mockExecFile.mockRejectedValueOnce(new Error('Command not found')); - await expect(openBrowserSecurely('https://example.com')).rejects.toThrow( - 'Failed to open browser', + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await expect( + openBrowserSecurely('https://example.com') + ).resolves.toBeUndefined(); + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('Failed to open browser automatically'), ); + + consoleSpy.mockRestore(); }); + }); + describe('Linux Fallback', () => { it('should try fallback browsers on Linux', async () => { setPlatform('linux'); - // First call to xdg-open fails mockExecFile.mockRejectedValueOnce(new Error('Command not found')); - // Second call to gnome-open succeeds mockExecFile.mockResolvedValueOnce({ stdout: '', stderr: '' }); await openBrowserSecurely('https://example.com'); @@ -239,4 +243,4 @@ describe('secure-browser-launcher', () => { ); }); }); -}); +}); \ No newline at end of file diff --git a/packages/core/src/utils/secure-browser-launcher.ts b/packages/core/src/utils/secure-browser-launcher.ts index c60a646d1d..facfa329bb 100644 --- a/packages/core/src/utils/secure-browser-launcher.ts +++ b/packages/core/src/utils/secure-browser-launcher.ts @@ -42,14 +42,14 @@ function validateUrl(url: string): void { } /** - * Opens a URL in the default browser using platform-specific commands. - * This implementation avoids shell injection vulnerabilities by: - * 1. Validating the URL to ensure it's HTTP/HTTPS only - * 2. Using execFile instead of exec to avoid shell interpretation - * 3. Passing the URL as an argument rather than constructing a command string + * Opens a URL in the user's default browser securely. * - * @param url The URL to open - * @throws Error if the URL is invalid or if opening the browser fails + * On failure (e.g., missing browser binary or command), this function does NOT throw an error. + * Instead, it logs the URL to the console error stream so the user can open it manually, + * and resolves successfully to prevent application crashes. + * + * @param url - The URL to open. + * @returns A promise that resolves when the attempt is made (whether successful or logged). */ export async function openBrowserSecurely(url: string): Promise { // Validate the URL first @@ -107,7 +107,7 @@ export async function openBrowserSecurely(url: string): Promise { try { await execFileAsync(command, args, options); - } catch (error) { + } catch (_error) { // For Linux, try fallback commands if xdg-open fails if ( (platformName === 'linux' || @@ -121,6 +121,7 @@ export async function openBrowserSecurely(url: string): Promise { 'firefox', 'chromium', 'google-chrome', + 'microsoft-edge', ]; for (const fallbackCommand of fallbackCommands) { @@ -134,10 +135,11 @@ export async function openBrowserSecurely(url: string): Promise { } } - // Re-throw the error if all attempts failed - throw new Error( - `Failed to open browser: ${error instanceof Error ? error.message : 'Unknown error'}`, - ); + // Log the URL so the user can open it manually instead of crashing. + /* eslint-disable no-console */ + console.warn(`Failed to open browser automatically. Please open this URL manually: ${url}`); + /* eslint-enable no-console */ + return; } } @@ -188,4 +190,4 @@ export function shouldLaunchBrowser(): boolean { // For non-Linux OSes, we generally assume a GUI is available // unless other signals (like SSH) suggest otherwise. return true; -} +} \ No newline at end of file