mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-04-28 11:41:04 +00:00
fix(core): handle missing xdg-open gracefully in secure-browser-launcher (#1675)
- secure-browser-launcher now logs the URL and resolves instead of throwing when all browser commands fail, preventing callers from crashing on headless or minimal Linux environments - Add microsoft-edge to the Linux fallback list - Update JSDoc to match the new log-and-resolve contract - Pin test platform to darwin so the assertion doesn't get masked by the Linux fallback chain Note: this PR stabilizes the utility but does not by itself fix the RISC-V/OrangePi crash in #1674, which originates from the `open` npm package in qwenOAuth2.ts. See #3481 for the root-cause fix.
This commit is contained in:
parent
e49867a762
commit
ece51398dd
2 changed files with 31 additions and 25 deletions
|
|
@ -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', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -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<void> {
|
||||
// Validate the URL first
|
||||
|
|
@ -107,7 +107,7 @@ export async function openBrowserSecurely(url: string): Promise<void> {
|
|||
|
||||
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<void> {
|
|||
'firefox',
|
||||
'chromium',
|
||||
'google-chrome',
|
||||
'microsoft-edge',
|
||||
];
|
||||
|
||||
for (const fallbackCommand of fallbackCommands) {
|
||||
|
|
@ -134,10 +135,11 @@ export async function openBrowserSecurely(url: string): Promise<void> {
|
|||
}
|
||||
}
|
||||
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue