mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
fix(cli): avoid headless browser open crashes (#4716)
* fix(cli): avoid headless browser open crashes * fix(cli): reuse browser launch guard * test(core): isolate browser launcher env * fix(core): make browser env launches non-blocking * fix(core): handle LSP stdio write failures
This commit is contained in:
Yufeng He
GitHub
parent
1cd5b81aa3
commit
adbdff8b1f
18 changed files with 906 additions and 211 deletions
12
package-lock.json
generated
12
package-lock.json
generated
|
|
@ -7625,6 +7625,7 @@
|
|||
"version": "4.1.0",
|
||||
"resolved": "https://registry.npmjs.org/bundle-name/-/bundle-name-4.1.0.tgz",
|
||||
"integrity": "sha512-tjwM5exMg6BGRI+kNmTntNsvdZS1X8BFYS6tnJ2hdH0kVxM6/eVZ2xy+FqStSWvYmtfFMDLIxurorHwDKfDz5Q==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"run-applescript": "^7.0.0"
|
||||
|
|
@ -9390,6 +9391,7 @@
|
|||
"version": "5.2.1",
|
||||
"resolved": "https://registry.npmjs.org/default-browser/-/default-browser-5.2.1.tgz",
|
||||
"integrity": "sha512-WY/3TUME0x3KPYdRRxEJJvXRHV4PyPoUsxtZa78lwItwRQRHhd2U9xOscaT/YTf8uCXIAjeJOFBVEh/7FtD8Xg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"bundle-name": "^4.1.0",
|
||||
|
|
@ -9406,6 +9408,7 @@
|
|||
"version": "5.0.0",
|
||||
"resolved": "https://registry.npmjs.org/default-browser-id/-/default-browser-id-5.0.0.tgz",
|
||||
"integrity": "sha512-A6p/pu/6fyBcA1TRz/GqWYPViplrftcW2gZC9q79ngNCKAeR/X3gcEdXQHl4KNXV+3wgIJ1CPkJQ3IHM6lcsyA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
|
|
@ -9436,6 +9439,7 @@
|
|||
"version": "3.0.0",
|
||||
"resolved": "https://registry.npmjs.org/define-lazy-prop/-/define-lazy-prop-3.0.0.tgz",
|
||||
"integrity": "sha512-N+MeXYoqr3pOgn8xfyRPREN7gHakLYjhsHhWGT3fWAiL4IkAt0iDw14QiiEm2bE30c5XX5q0FtAA3CK5f9/BUg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=12"
|
||||
|
|
@ -13227,6 +13231,7 @@
|
|||
"version": "3.0.0",
|
||||
"resolved": "https://registry.npmjs.org/is-docker/-/is-docker-3.0.0.tgz",
|
||||
"integrity": "sha512-eljcgEDlEns/7AXFosB5K/2nCM4P7FQPkGc/DWLy5rmFEWvZayGrik1d9/QIY5nJ4f9YsVvBkA6kJpHn9rISdQ==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"bin": {
|
||||
"is-docker": "cli.js"
|
||||
|
|
@ -13334,6 +13339,7 @@
|
|||
"version": "1.0.0",
|
||||
"resolved": "https://registry.npmjs.org/is-inside-container/-/is-inside-container-1.0.0.tgz",
|
||||
"integrity": "sha512-KIYLCCJghfHZxqjYBE7rEy0OBuTd5xCHS7tHVgvCLkx7StIoaxwNW3hCALgEUjFfeRk+MG/Qxmp/vtETEF3tRA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"is-docker": "^3.0.0"
|
||||
|
|
@ -13635,6 +13641,7 @@
|
|||
"version": "3.1.0",
|
||||
"resolved": "https://registry.npmjs.org/is-wsl/-/is-wsl-3.1.0.tgz",
|
||||
"integrity": "sha512-UcVfVfaK4Sc4m7X3dUSoHoozQGBEFeDC+zVo06t98xe8CzHSZZBekNXH+tu0NalHolcJ/QAGqS46Hef7QXBIMw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"is-inside-container": "^1.0.0"
|
||||
|
|
@ -16745,6 +16752,7 @@
|
|||
"version": "10.2.0",
|
||||
"resolved": "https://registry.npmjs.org/open/-/open-10.2.0.tgz",
|
||||
"integrity": "sha512-YgBpdJHPyQ2UE5x+hlSXcnejzAvD0b22U2OuAP+8OnlJT+PjWPxtgmGqKKc+RgTM63U9gN0YzrYc71R2WT/hTA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"default-browser": "^5.2.1",
|
||||
|
|
@ -18805,6 +18813,7 @@
|
|||
"version": "7.0.0",
|
||||
"resolved": "https://registry.npmjs.org/run-applescript/-/run-applescript-7.0.0.tgz",
|
||||
"integrity": "sha512-9by4Ij99JUr/MCFBUkDKLWK3G9HVXmabKz9U5MlIAIuvuzkiOicRYs8XJLxX+xahD+mLiiCYDqF9dKAgtzKP1A==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
|
|
@ -22084,6 +22093,7 @@
|
|||
"version": "0.1.0",
|
||||
"resolved": "https://registry.npmjs.org/wsl-utils/-/wsl-utils-0.1.0.tgz",
|
||||
"integrity": "sha512-h3Fbisa2nKGPxCpm89Hk33lBLsnaGBvctQopaBSOW/uIs6FTe1ATyAnKFJrzVs9vpGdsTe73WF3V4lIsk4Gacw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"is-wsl": "^3.1.0"
|
||||
|
|
@ -22489,7 +22499,6 @@
|
|||
"ink-link": "^4.1.0",
|
||||
"ink-spinner": "^5.0.0",
|
||||
"lowlight": "^3.3.0",
|
||||
"open": "^10.1.2",
|
||||
"p-limit": "^7.3.0",
|
||||
"prompts": "^2.4.2",
|
||||
"react": "^19.2.4",
|
||||
|
|
@ -22732,7 +22741,6 @@
|
|||
"marked": "^15.0.12",
|
||||
"mime": "4.0.7",
|
||||
"mnemonist": "^0.40.3",
|
||||
"open": "^10.1.2",
|
||||
"openai": "5.11.0",
|
||||
"picomatch": "^4.0.1",
|
||||
"prompts": "^2.4.2",
|
||||
|
|
|
|||
|
|
@ -67,7 +67,6 @@
|
|||
"ink-link": "^4.1.0",
|
||||
"ink-spinner": "^5.0.0",
|
||||
"lowlight": "^3.3.0",
|
||||
"open": "^10.1.2",
|
||||
"p-limit": "^7.3.0",
|
||||
"prompts": "^2.4.2",
|
||||
"react": "^19.2.4",
|
||||
|
|
|
|||
|
|
@ -5,15 +5,23 @@
|
|||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import open from 'open';
|
||||
import { bugCommand } from './bugCommand.js';
|
||||
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
|
||||
import { GIT_COMMIT_INFO } from '../../generated/git-commit.js';
|
||||
import { AuthType } from '@qwen-code/qwen-code-core';
|
||||
import * as systemInfoUtils from '../../utils/systemInfo.js';
|
||||
|
||||
const mockOpenBrowserSecurely = vi.hoisted(() => vi.fn());
|
||||
|
||||
// Mock dependencies
|
||||
vi.mock('open');
|
||||
vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('@qwen-code/qwen-code-core')>();
|
||||
return {
|
||||
...actual,
|
||||
openBrowserSecurely: mockOpenBrowserSecurely,
|
||||
};
|
||||
});
|
||||
vi.mock('../../utils/systemInfo.js');
|
||||
|
||||
describe('bugCommand', () => {
|
||||
|
|
@ -36,6 +44,8 @@ describe('bugCommand', () => {
|
|||
? GIT_COMMIT_INFO
|
||||
: undefined,
|
||||
});
|
||||
mockOpenBrowserSecurely.mockClear();
|
||||
mockOpenBrowserSecurely.mockResolvedValue(undefined);
|
||||
vi.stubEnv('SANDBOX', 'qwen-test');
|
||||
});
|
||||
|
||||
|
|
@ -84,7 +94,7 @@ Memory Usage: 100 MB`;
|
|||
},
|
||||
expect.any(Number),
|
||||
);
|
||||
expect(open).toHaveBeenCalledWith(expectedUrl);
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(expectedUrl);
|
||||
});
|
||||
|
||||
it('should use a custom URL template from config if provided', async () => {
|
||||
|
|
@ -128,7 +138,7 @@ Memory Usage: 100 MB`;
|
|||
},
|
||||
expect.any(Number),
|
||||
);
|
||||
expect(open).toHaveBeenCalledWith(expectedUrl);
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(expectedUrl);
|
||||
});
|
||||
|
||||
it('should include Base URL when auth type is OpenAI', async () => {
|
||||
|
|
@ -193,6 +203,28 @@ Memory Usage: 100 MB`;
|
|||
},
|
||||
expect.any(Number),
|
||||
);
|
||||
expect(open).toHaveBeenCalledWith(expectedUrl);
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(expectedUrl);
|
||||
});
|
||||
|
||||
it('should report browser launch failures without failing the command', async () => {
|
||||
mockOpenBrowserSecurely.mockRejectedValueOnce(new Error('browser failed'));
|
||||
const mockContext = createMockCommandContext({
|
||||
services: {
|
||||
config: {
|
||||
getBugCommand: () => undefined,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
if (!bugCommand.action) throw new Error('Action is not defined');
|
||||
await bugCommand.action(mockContext, 'Browser failure');
|
||||
|
||||
expect(mockContext.ui.addItem).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
type: 'error',
|
||||
text: 'Could not open URL in browser: browser failed',
|
||||
}),
|
||||
expect.any(Number),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -4,12 +4,12 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import open from 'open';
|
||||
import {
|
||||
type CommandContext,
|
||||
type SlashCommand,
|
||||
CommandKind,
|
||||
} from './types.js';
|
||||
import { openBrowserSecurely } from '@qwen-code/qwen-code-core';
|
||||
import { MessageType, type HistoryItem } from '../types.js';
|
||||
import { getExtendedSystemInfo } from '../../utils/systemInfo.js';
|
||||
import { getSystemInfoFields } from '../../utils/systemInfoFields.js';
|
||||
|
|
@ -55,7 +55,7 @@ export const bugCommand: SlashCommand = {
|
|||
context.ui.addItem(bugReportItem, Date.now());
|
||||
|
||||
try {
|
||||
await open(bugReportUrl);
|
||||
await openBrowserSecurely(bugReportUrl);
|
||||
} catch (error) {
|
||||
const errorMessage =
|
||||
error instanceof Error ? error.message : String(error);
|
||||
|
|
|
|||
|
|
@ -5,24 +5,29 @@
|
|||
*/
|
||||
|
||||
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import open from 'open';
|
||||
import { docsCommand } from './docsCommand.js';
|
||||
import { type CommandContext } from './types.js';
|
||||
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
|
||||
import { MessageType } from '../types.js';
|
||||
|
||||
// Mock the 'open' library
|
||||
vi.mock('open', () => ({
|
||||
default: vi.fn(),
|
||||
}));
|
||||
const mockOpenBrowserSecurely = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('@qwen-code/qwen-code-core')>();
|
||||
return {
|
||||
...actual,
|
||||
openBrowserSecurely: mockOpenBrowserSecurely,
|
||||
};
|
||||
});
|
||||
|
||||
describe('docsCommand', () => {
|
||||
let mockContext: CommandContext;
|
||||
beforeEach(() => {
|
||||
// Create a fresh mock context before each test
|
||||
mockContext = createMockCommandContext();
|
||||
// Reset the `open` mock
|
||||
vi.mocked(open).mockClear();
|
||||
mockOpenBrowserSecurely.mockClear();
|
||||
mockOpenBrowserSecurely.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
|
@ -47,7 +52,7 @@ describe('docsCommand', () => {
|
|||
expect.any(Number),
|
||||
);
|
||||
|
||||
expect(open).toHaveBeenCalledWith(docsUrl);
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(docsUrl);
|
||||
});
|
||||
|
||||
it('should only add an info message in a sandbox environment', async () => {
|
||||
|
|
@ -70,7 +75,7 @@ describe('docsCommand', () => {
|
|||
);
|
||||
|
||||
// Ensure 'open' was not called in the sandbox
|
||||
expect(open).not.toHaveBeenCalled();
|
||||
expect(mockOpenBrowserSecurely).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should not open browser for 'sandbox-exec'", async () => {
|
||||
|
|
@ -93,8 +98,27 @@ describe('docsCommand', () => {
|
|||
expect.any(Number),
|
||||
);
|
||||
|
||||
// 'open' should be called in this specific sandbox case
|
||||
expect(open).toHaveBeenCalledWith(docsUrl);
|
||||
// Browser launch should be called in this specific sandbox case.
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(docsUrl);
|
||||
});
|
||||
|
||||
it('should show the docs URL when browser opening throws unexpectedly', async () => {
|
||||
if (!docsCommand.action) {
|
||||
throw new Error('docsCommand must have an action.');
|
||||
}
|
||||
|
||||
const docsUrl = 'https://qwenlm.github.io/qwen-code-docs/en';
|
||||
mockOpenBrowserSecurely.mockRejectedValueOnce(new Error('bad url'));
|
||||
|
||||
await docsCommand.action(mockContext, '');
|
||||
|
||||
expect(mockContext.ui.addItem).toHaveBeenCalledWith(
|
||||
{
|
||||
type: MessageType.ERROR,
|
||||
text: `Failed to open browser. View documentation at ${docsUrl}`,
|
||||
},
|
||||
expect.any(Number),
|
||||
);
|
||||
});
|
||||
|
||||
describe('non-interactive mode', () => {
|
||||
|
|
@ -112,7 +136,7 @@ describe('docsCommand', () => {
|
|||
messageType: 'info',
|
||||
content: expect.stringContaining('qwenlm.github.io'),
|
||||
});
|
||||
expect(open).not.toHaveBeenCalled();
|
||||
expect(mockOpenBrowserSecurely).not.toHaveBeenCalled();
|
||||
expect(nonInteractiveContext.ui.addItem).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -4,13 +4,13 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import open from 'open';
|
||||
import process from 'node:process';
|
||||
import {
|
||||
type CommandContext,
|
||||
type SlashCommand,
|
||||
CommandKind,
|
||||
} from './types.js';
|
||||
import { openBrowserSecurely } from '@qwen-code/qwen-code-core';
|
||||
import { MessageType } from '../types.js';
|
||||
import { t, getCurrentLanguage } from '../../i18n/index.js';
|
||||
|
||||
|
|
@ -57,7 +57,19 @@ export const docsCommand: SlashCommand = {
|
|||
},
|
||||
Date.now(),
|
||||
);
|
||||
await open(docsUrl);
|
||||
try {
|
||||
await openBrowserSecurely(docsUrl);
|
||||
} catch (_error) {
|
||||
context.ui.addItem(
|
||||
{
|
||||
type: MessageType.ERROR,
|
||||
text: t('Failed to open browser. View documentation at {{url}}', {
|
||||
url: docsUrl,
|
||||
}),
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
}
|
||||
}
|
||||
return;
|
||||
},
|
||||
|
|
|
|||
|
|
@ -22,11 +22,14 @@ import {
|
|||
parseInstallSource,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
|
||||
const mockOpenBrowserSecurely = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('@qwen-code/qwen-code-core')>();
|
||||
return {
|
||||
...actual,
|
||||
openBrowserSecurely: mockOpenBrowserSecurely,
|
||||
parseInstallSource: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
|
@ -46,6 +49,7 @@ describe('extensionsCommand', () => {
|
|||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
mockOpenBrowserSecurely.mockResolvedValue(undefined);
|
||||
mockExtensionManager = createMockExtensionManager();
|
||||
mockGetExtensions.mockReturnValue([]);
|
||||
mockGetLoadedExtensions.mockReturnValue([]);
|
||||
|
|
@ -64,6 +68,47 @@ describe('extensionsCommand', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('explore', () => {
|
||||
const exploreAction = extensionsCommand.subCommands?.find(
|
||||
(cmd) => cmd.name === 'explore',
|
||||
)?.action;
|
||||
|
||||
if (!exploreAction) {
|
||||
throw new Error('Explore action not found');
|
||||
}
|
||||
|
||||
it('should open the selected extensions gallery through the shared browser helper', async () => {
|
||||
vi.stubEnv('NODE_ENV', 'development');
|
||||
await exploreAction(mockContext, 'Gemini');
|
||||
|
||||
expect(mockContext.ui.addItem).toHaveBeenCalledWith(
|
||||
{
|
||||
type: MessageType.INFO,
|
||||
text: 'Opening extensions page in your browser: https://geminicli.com/extensions/',
|
||||
},
|
||||
expect.any(Number),
|
||||
);
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(
|
||||
'https://geminicli.com/extensions/',
|
||||
);
|
||||
});
|
||||
|
||||
it('should show the URL when browser launch fails', async () => {
|
||||
vi.stubEnv('NODE_ENV', 'development');
|
||||
mockOpenBrowserSecurely.mockRejectedValue(new Error('no browser'));
|
||||
|
||||
await exploreAction(mockContext, 'ClaudeCode');
|
||||
|
||||
expect(mockContext.ui.addItem).toHaveBeenCalledWith(
|
||||
{
|
||||
type: MessageType.ERROR,
|
||||
text: 'Failed to open browser. Check out the extensions gallery at https://claudemarketplaces.com/',
|
||||
},
|
||||
expect.any(Number),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('default action (manage)', () => {
|
||||
it('should open extensions manager dialog when extensions exist', async () => {
|
||||
if (!extensionsCommand.action) throw new Error('Action not defined');
|
||||
|
|
|
|||
|
|
@ -14,11 +14,11 @@ import {
|
|||
import { t } from '../../i18n/index.js';
|
||||
import {
|
||||
ExtensionManager,
|
||||
openBrowserSecurely,
|
||||
parseInstallSource,
|
||||
createDebugLogger,
|
||||
redactUrlCredentials,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
import open from 'open';
|
||||
|
||||
const debugLogger = createDebugLogger('EXTENSIONS_COMMAND');
|
||||
const EXTENSION_EXPLORE_URL = {
|
||||
|
|
@ -85,7 +85,7 @@ async function exploreAction(context: CommandContext, args: string) {
|
|||
Date.now(),
|
||||
);
|
||||
try {
|
||||
await open(extensionsUrl);
|
||||
await openBrowserSecurely(extensionsUrl);
|
||||
} catch (_error) {
|
||||
context.ui.addItem(
|
||||
{
|
||||
|
|
@ -144,10 +144,11 @@ async function listTextAction(context: CommandContext, _args: string) {
|
|||
}
|
||||
|
||||
const active = extensions.filter((e) => e.isActive);
|
||||
let output = t('**Installed Extensions ({{total}} total, {{active}} active)**', {
|
||||
total: String(extensions.length),
|
||||
active: String(active.length),
|
||||
}) + '\n\n';
|
||||
let output =
|
||||
t('**Installed Extensions ({{total}} total, {{active}} active)**', {
|
||||
total: String(extensions.length),
|
||||
active: String(active.length),
|
||||
}) + '\n\n';
|
||||
|
||||
for (const ext of extensions) {
|
||||
const status = ext.isActive ? '✓' : '✗';
|
||||
|
|
@ -155,9 +156,7 @@ async function listTextAction(context: CommandContext, _args: string) {
|
|||
? ` (${redactUrlCredentials(ext.installMetadata.source)})`
|
||||
: '';
|
||||
const caps: string[] = [];
|
||||
const mcpCount = ext.mcpServers
|
||||
? Object.keys(ext.mcpServers).length
|
||||
: 0;
|
||||
const mcpCount = ext.mcpServers ? Object.keys(ext.mcpServers).length : 0;
|
||||
if (mcpCount > 0) {
|
||||
caps.push(t('{{count}} MCP servers', { count: String(mcpCount) }));
|
||||
}
|
||||
|
|
@ -165,7 +164,9 @@ async function listTextAction(context: CommandContext, _args: string) {
|
|||
caps.push(t('{{count}} skills', { count: String(ext.skills.length) }));
|
||||
}
|
||||
if (ext.commands && ext.commands.length > 0) {
|
||||
caps.push(t('{{count}} commands', { count: String(ext.commands.length) }));
|
||||
caps.push(
|
||||
t('{{count}} commands', { count: String(ext.commands.length) }),
|
||||
);
|
||||
}
|
||||
const capsStr = caps.length > 0 ? ` [${caps.join(', ')}]` : '';
|
||||
output += `- [${status}] **${ext.name}** v${ext.version}${source}${capsStr}\n`;
|
||||
|
|
|
|||
|
|
@ -6,13 +6,23 @@
|
|||
|
||||
import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import path from 'path';
|
||||
import open from 'open';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import { parseInsightMessage, Storage } from '@qwen-code/qwen-code-core';
|
||||
import { insightCommand } from './insightCommand.js';
|
||||
import type { CommandContext } from './types.js';
|
||||
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
|
||||
|
||||
const mockGenerateStaticInsight = vi.fn();
|
||||
const mockOpenBrowserSecurely = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('@qwen-code/qwen-code-core')>();
|
||||
return {
|
||||
...actual,
|
||||
openBrowserSecurely: mockOpenBrowserSecurely,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('../../services/insight/generators/StaticInsightGenerator.js', () => ({
|
||||
StaticInsightGenerator: vi.fn(() => ({
|
||||
|
|
@ -20,10 +30,6 @@ vi.mock('../../services/insight/generators/StaticInsightGenerator.js', () => ({
|
|||
})),
|
||||
}));
|
||||
|
||||
vi.mock('open', () => ({
|
||||
default: vi.fn(),
|
||||
}));
|
||||
|
||||
describe('insightCommand', () => {
|
||||
let mockContext: CommandContext;
|
||||
|
||||
|
|
@ -32,7 +38,8 @@ describe('insightCommand', () => {
|
|||
mockGenerateStaticInsight.mockResolvedValue(
|
||||
path.resolve('runtime-output', 'insights', 'insight-2026-03-05.html'),
|
||||
);
|
||||
vi.mocked(open).mockResolvedValue(undefined as never);
|
||||
mockOpenBrowserSecurely.mockClear();
|
||||
mockOpenBrowserSecurely.mockResolvedValue(undefined);
|
||||
|
||||
mockContext = createMockCommandContext({
|
||||
services: {
|
||||
|
|
@ -62,6 +69,41 @@ describe('insightCommand', () => {
|
|||
path.join(Storage.getRuntimeBaseDir(), 'projects'),
|
||||
expect.any(Function),
|
||||
);
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(
|
||||
pathToFileURL(
|
||||
path.resolve('runtime-output', 'insights', 'insight-2026-03-05.html'),
|
||||
).href,
|
||||
{
|
||||
allowFile: true,
|
||||
allowedFilePaths: [
|
||||
path.resolve('runtime-output', 'insights', 'insight-2026-03-05.html'),
|
||||
],
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('shows the generated file path before attempting to open the browser', async () => {
|
||||
if (!insightCommand.action) {
|
||||
throw new Error('insight command must have action');
|
||||
}
|
||||
|
||||
await insightCommand.action(mockContext, '');
|
||||
|
||||
const messages = vi
|
||||
.mocked(mockContext.ui.addItem)
|
||||
.mock.calls.map(([item]) =>
|
||||
'text' in item && typeof item.text === 'string' ? item.text : '',
|
||||
);
|
||||
expect(messages).toContain(
|
||||
`Insights generated at: ${path.resolve(
|
||||
'runtime-output',
|
||||
'insights',
|
||||
'insight-2026-03-05.html',
|
||||
)}. If the browser does not open automatically, open this file manually.`,
|
||||
);
|
||||
expect(
|
||||
messages.some((message) => message.includes('Opening insights')),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('streams ACP progress messages without waiting for generation to finish', async () => {
|
||||
|
|
@ -198,7 +240,7 @@ describe('insightCommand', () => {
|
|||
expect((result as { content: string }).content).toContain(
|
||||
path.resolve('runtime-output', 'insights', 'insight-2026-03-05.html'),
|
||||
);
|
||||
expect(open).not.toHaveBeenCalled();
|
||||
expect(mockOpenBrowserSecurely).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('non_interactive: returns error message when generation fails', async () => {
|
||||
|
|
@ -227,6 +269,6 @@ describe('insightCommand', () => {
|
|||
messageType: 'error',
|
||||
});
|
||||
expect((result as { content: string }).content).toContain('disk full');
|
||||
expect(open).not.toHaveBeenCalled();
|
||||
expect(mockOpenBrowserSecurely).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -10,15 +10,16 @@ import { MessageType } from '../types.js';
|
|||
import type { HistoryItemInsightProgress } from '../types.js';
|
||||
import { t } from '../../i18n/index.js';
|
||||
import { join } from 'path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import { StaticInsightGenerator } from '../../services/insight/generators/StaticInsightGenerator.js';
|
||||
import {
|
||||
createDebugLogger,
|
||||
encodeInsightErrorMessage,
|
||||
encodeInsightProgressMessage,
|
||||
encodeInsightReadyMessage,
|
||||
openBrowserSecurely,
|
||||
Storage,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
import open from 'open';
|
||||
|
||||
const logger = createDebugLogger('DataProcessor');
|
||||
|
||||
|
|
@ -221,33 +222,26 @@ export const insightCommand: SlashCommand = {
|
|||
Date.now(),
|
||||
);
|
||||
|
||||
try {
|
||||
await open(outputPath);
|
||||
|
||||
context.ui.addItem(
|
||||
{
|
||||
type: MessageType.INFO,
|
||||
text: t('Opening insights in your browser: {{path}}', {
|
||||
context.ui.addItem(
|
||||
{
|
||||
type: MessageType.INFO,
|
||||
text: t(
|
||||
'Insights generated at: {{path}}. If the browser does not open automatically, open this file manually.',
|
||||
{
|
||||
path: outputPath,
|
||||
}),
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
},
|
||||
),
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
|
||||
try {
|
||||
await openBrowserSecurely(pathToFileURL(outputPath).href, {
|
||||
allowFile: true,
|
||||
allowedFilePaths: [outputPath],
|
||||
});
|
||||
} catch (browserError) {
|
||||
logger.error('Failed to open browser automatically:', browserError);
|
||||
|
||||
context.ui.addItem(
|
||||
{
|
||||
type: MessageType.INFO,
|
||||
text: t(
|
||||
'Insights generated at: {{path}}. Please open this file in your browser.',
|
||||
{
|
||||
path: outputPath,
|
||||
},
|
||||
),
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
}
|
||||
|
||||
context.ui.setDebugMessage(t('Insights ready.'));
|
||||
|
|
|
|||
|
|
@ -61,7 +61,6 @@
|
|||
"marked": "^15.0.12",
|
||||
"mime": "4.0.7",
|
||||
"mnemonist": "^0.40.3",
|
||||
"open": "^10.1.2",
|
||||
"openai": "5.11.0",
|
||||
"picomatch": "^4.0.1",
|
||||
"prompts": "^2.4.2",
|
||||
|
|
|
|||
|
|
@ -374,6 +374,7 @@ export {
|
|||
export * from './utils/runtimeStatus.js';
|
||||
export * from './utils/schemaValidator.js';
|
||||
export * from './utils/sessionIdContext.js';
|
||||
export * from './utils/secure-browser-launcher.js';
|
||||
export * from './utils/shell-utils.js';
|
||||
export * from './utils/subagentGenerator.js';
|
||||
export * from './utils/symlink.js';
|
||||
|
|
|
|||
|
|
@ -89,7 +89,11 @@ class JsonRpcConnection {
|
|||
}
|
||||
this.disposed = true;
|
||||
this.disposePending();
|
||||
this.disposer?.();
|
||||
try {
|
||||
this.disposer?.();
|
||||
} catch (error) {
|
||||
debugLogger.warn('LSP disposer failed:', error);
|
||||
}
|
||||
}
|
||||
|
||||
private sendRequest(method: string, params: unknown): Promise<unknown> {
|
||||
|
|
@ -231,7 +235,12 @@ class JsonRpcConnection {
|
|||
}
|
||||
const json = JSON.stringify(message);
|
||||
const header = `Content-Length: ${Buffer.byteLength(json, 'utf8')}\r\n\r\n`;
|
||||
this.writer(header + json);
|
||||
try {
|
||||
this.writer(header + json);
|
||||
} catch (error) {
|
||||
debugLogger.warn('LSP write failed:', error);
|
||||
this.end();
|
||||
}
|
||||
}
|
||||
|
||||
private disposePending(error?: Error): void {
|
||||
|
|
@ -318,6 +327,11 @@ export class LspConnectionFactory {
|
|||
() => processInstance.stdin?.end(),
|
||||
);
|
||||
|
||||
processInstance.stdin.on('error', (err) => {
|
||||
debugLogger.warn(`LSP stdin stream error for ${command}:`, err);
|
||||
connection.end();
|
||||
});
|
||||
|
||||
connection.listen(processInstance.stdout);
|
||||
const recordProcessExit = (
|
||||
code: number | null,
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@
|
|||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { EventEmitter } from 'events';
|
||||
import { type ChildProcess } from 'child_process';
|
||||
import type { Config } from '../config/config.js';
|
||||
import {
|
||||
generateCodeChallenge,
|
||||
|
|
@ -36,6 +35,8 @@ interface MockSharedTokenManager {
|
|||
clearCache(): void;
|
||||
}
|
||||
|
||||
const mockOpenBrowserSecurely = vi.hoisted(() => vi.fn());
|
||||
|
||||
// Mock SharedTokenManager
|
||||
vi.mock('./sharedTokenManager.js', () => ({
|
||||
SharedTokenManager: class {
|
||||
|
|
@ -91,9 +92,8 @@ vi.mock('./sharedTokenManager.js', () => ({
|
|||
},
|
||||
}));
|
||||
|
||||
// Mock open
|
||||
vi.mock('open', () => ({
|
||||
default: vi.fn(),
|
||||
vi.mock('../utils/secure-browser-launcher.js', () => ({
|
||||
openBrowserSecurely: mockOpenBrowserSecurely,
|
||||
}));
|
||||
|
||||
// Mock process.stdout.write
|
||||
|
|
@ -118,6 +118,11 @@ vi.mock('node:fs', () => ({
|
|||
},
|
||||
}));
|
||||
|
||||
beforeEach(() => {
|
||||
mockOpenBrowserSecurely.mockReset();
|
||||
mockOpenBrowserSecurely.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
describe('PKCE Code Generation', () => {
|
||||
describe('generateCodeVerifier', () => {
|
||||
it('should generate a code verifier with correct length', () => {
|
||||
|
|
@ -1562,6 +1567,7 @@ describe('authWithQwenDeviceFlow - Comprehensive Testing', () => {
|
|||
|
||||
expect(client).toBeInstanceOf(Object);
|
||||
expect(mockConfig.isBrowserLaunchSuppressed).toHaveBeenCalled();
|
||||
expect(mockOpenBrowserSecurely).not.toHaveBeenCalled();
|
||||
|
||||
SharedTokenManager.getInstance = originalGetInstance;
|
||||
});
|
||||
|
|
@ -1592,9 +1598,15 @@ describe('Browser Launch and Error Handling', () => {
|
|||
new Error('No cached credentials'),
|
||||
);
|
||||
|
||||
// Mock open to throw error
|
||||
const open = await import('open');
|
||||
vi.mocked(open.default).mockRejectedValue(
|
||||
const mockTokenManager = {
|
||||
getValidCredentials: vi
|
||||
.fn()
|
||||
.mockRejectedValue(new Error('No credentials')),
|
||||
};
|
||||
const originalGetInstance = SharedTokenManager.getInstance;
|
||||
SharedTokenManager.getInstance = vi.fn().mockReturnValue(mockTokenManager);
|
||||
|
||||
mockOpenBrowserSecurely.mockRejectedValue(
|
||||
new Error('Browser launch failed'),
|
||||
);
|
||||
|
||||
|
|
@ -1629,27 +1641,26 @@ describe('Browser Launch and Error Handling', () => {
|
|||
);
|
||||
|
||||
expect(client).toBeInstanceOf(Object);
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(
|
||||
'https://chat.qwen.ai/device?code=TEST123',
|
||||
);
|
||||
|
||||
SharedTokenManager.getInstance = originalGetInstance;
|
||||
});
|
||||
|
||||
it('should handle browser child process error gracefully', async () => {
|
||||
it('should launch the device flow URL through the shared browser helper', async () => {
|
||||
const { promises: fs } = await import('node:fs');
|
||||
vi.mocked(fs.readFile).mockRejectedValue(
|
||||
new Error('No cached credentials'),
|
||||
);
|
||||
|
||||
// Mock open to return a child process that will emit error
|
||||
const open = await import('open');
|
||||
const mockChildProcess = {
|
||||
on: vi.fn((event: string, callback: (error: Error) => void) => {
|
||||
if (event === 'error') {
|
||||
// Call the error handler immediately for testing
|
||||
setTimeout(() => callback(new Error('Process spawn failed')), 0);
|
||||
}
|
||||
}),
|
||||
const mockTokenManager = {
|
||||
getValidCredentials: vi
|
||||
.fn()
|
||||
.mockRejectedValue(new Error('No credentials')),
|
||||
};
|
||||
vi.mocked(open.default).mockResolvedValue(
|
||||
mockChildProcess as unknown as ChildProcess,
|
||||
);
|
||||
const originalGetInstance = SharedTokenManager.getInstance;
|
||||
SharedTokenManager.getInstance = vi.fn().mockReturnValue(mockTokenManager);
|
||||
|
||||
const mockAuthResponse = {
|
||||
ok: true,
|
||||
|
|
@ -1682,6 +1693,11 @@ describe('Browser Launch and Error Handling', () => {
|
|||
);
|
||||
|
||||
expect(client).toBeInstanceOf(Object);
|
||||
expect(mockOpenBrowserSecurely).toHaveBeenCalledWith(
|
||||
'https://chat.qwen.ai/device?code=TEST123',
|
||||
);
|
||||
|
||||
SharedTokenManager.getInstance = originalGetInstance;
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -7,14 +7,13 @@
|
|||
import crypto from 'crypto';
|
||||
import path from 'node:path';
|
||||
import { promises as fs } from 'node:fs';
|
||||
import type { ChildProcess } from 'node:child_process';
|
||||
import open from 'open';
|
||||
import { EventEmitter } from 'events';
|
||||
import type { Config } from '../config/config.js';
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { formatFetchErrorForUser } from '../utils/fetch.js';
|
||||
import { createDebugLogger } from '../utils/debugLogger.js';
|
||||
import { combineAbortSignals } from '../utils/abortController.js';
|
||||
import { openBrowserSecurely } from '../utils/secure-browser-launcher.js';
|
||||
import {
|
||||
SharedTokenManager,
|
||||
TokenManagerError,
|
||||
|
|
@ -838,30 +837,9 @@ async function authWithQwenDeviceFlow(
|
|||
|
||||
// Helper to handle browser launch with error handling
|
||||
const launchBrowser = async (url: string): Promise<void> => {
|
||||
let childProcess: ChildProcess | undefined;
|
||||
|
||||
try {
|
||||
// Call open and get the process
|
||||
childProcess = await open(url);
|
||||
|
||||
// CRITICAL FIX: Attach error listener IMMEDIATELY if a process object exists.
|
||||
// We do this outside the 'if' check scope to ensure it's bound as soon as possible.
|
||||
if (childProcess && typeof childProcess.on === 'function') {
|
||||
childProcess.on('error', (err: Error) => {
|
||||
debugLogger.warn(`Browser launch process error: ${err.message}`);
|
||||
debugLogger.info(`Please open this URL manually: ${url}`);
|
||||
});
|
||||
|
||||
// Optional: Also listen for 'close' or 'exit' if needed for cleanup,
|
||||
// but 'error' is the main crasher.
|
||||
} else {
|
||||
// Fallback: If open() didn't return a valid process object, log a warning
|
||||
debugLogger.debug(
|
||||
'open() did not return a valid child process object.',
|
||||
);
|
||||
}
|
||||
await openBrowserSecurely(url);
|
||||
} catch (err) {
|
||||
// Handle synchronous errors or promise rejections from open()
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
debugLogger.warn(`Failed to open browser automatically: ${errorMessage}`);
|
||||
debugLogger.info(`Please open this URL manually: ${url}`);
|
||||
|
|
|
|||
|
|
@ -4,6 +4,17 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
const browserBlocklist = ['www-browser'];
|
||||
|
||||
export function isBrowserCommandBlocked(command: string): boolean {
|
||||
const commandName = command.replace(/\\/g, '/').split('/').pop();
|
||||
return !!commandName && browserBlocklist.includes(commandName);
|
||||
}
|
||||
|
||||
type BrowserLaunchEnvironmentOptions = {
|
||||
ignoreBrowserBlocklist?: boolean;
|
||||
};
|
||||
|
||||
/**
|
||||
* Determines if we should attempt to launch a browser for authentication
|
||||
* based on the user's environment.
|
||||
|
|
@ -11,12 +22,17 @@
|
|||
* This is an adaptation of the logic from the Google Cloud SDK.
|
||||
* @returns True if the tool should attempt to launch a browser.
|
||||
*/
|
||||
export function shouldAttemptBrowserLaunch(): boolean {
|
||||
// A list of browser names that indicate we should not attempt to open a
|
||||
// web browser for the user.
|
||||
const browserBlocklist = ['www-browser'];
|
||||
const browserEnv = process.env['BROWSER'];
|
||||
if (browserEnv && browserBlocklist.includes(browserEnv)) {
|
||||
export function shouldAttemptBrowserLaunch(
|
||||
options: BrowserLaunchEnvironmentOptions = {},
|
||||
): boolean {
|
||||
const browserEnv = process.env['BROWSER']?.trim();
|
||||
const browserCommand = browserEnv?.match(/^\S+/)?.[0];
|
||||
if (
|
||||
!options.ignoreBrowserBlocklist &&
|
||||
process.platform !== 'win32' &&
|
||||
browserCommand &&
|
||||
isBrowserCommandBlocked(browserCommand)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
// Common environment variables used in CI/CD or other non-interactive shells.
|
||||
|
|
|
|||
|
|
@ -5,13 +5,35 @@
|
|||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { resolve } from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import { openBrowserSecurely } from './secure-browser-launcher.js';
|
||||
|
||||
// Create mock function using vi.hoisted
|
||||
const mockExecFile = vi.hoisted(() => vi.fn());
|
||||
const mockSpawn = vi.hoisted(() =>
|
||||
vi.fn(() => {
|
||||
const child: {
|
||||
once: ReturnType<typeof vi.fn>;
|
||||
unref: ReturnType<typeof vi.fn>;
|
||||
} = {
|
||||
once: vi.fn((event: string, callback: () => void) => {
|
||||
if (event === 'spawn') {
|
||||
callback();
|
||||
}
|
||||
return child;
|
||||
}),
|
||||
unref: vi.fn(),
|
||||
};
|
||||
return child;
|
||||
}),
|
||||
);
|
||||
|
||||
// Mock modules
|
||||
vi.mock('node:child_process');
|
||||
vi.mock('node:child_process', () => ({
|
||||
execFile: vi.fn(),
|
||||
spawn: mockSpawn,
|
||||
}));
|
||||
vi.mock('node:util', () => ({
|
||||
promisify: () => mockExecFile,
|
||||
}));
|
||||
|
|
@ -22,13 +44,33 @@ describe('secure-browser-launcher', () => {
|
|||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockExecFile.mockResolvedValue({ stdout: '', stderr: '' });
|
||||
mockSpawn.mockImplementation(() => {
|
||||
const child: {
|
||||
once: ReturnType<typeof vi.fn>;
|
||||
unref: ReturnType<typeof vi.fn>;
|
||||
} = {
|
||||
once: vi.fn((event: string, callback: () => void) => {
|
||||
if (event === 'spawn') {
|
||||
callback();
|
||||
}
|
||||
return child;
|
||||
}),
|
||||
unref: vi.fn(),
|
||||
};
|
||||
return child;
|
||||
});
|
||||
originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform');
|
||||
vi.stubEnv('BROWSER', '');
|
||||
vi.stubEnv('CI', '');
|
||||
vi.stubEnv('DEBIAN_FRONTEND', '');
|
||||
vi.stubEnv('SSH_CONNECTION', '');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalPlatform) {
|
||||
Object.defineProperty(process, 'platform', originalPlatform);
|
||||
}
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
function setPlatform(platform: string) {
|
||||
|
|
@ -71,6 +113,46 @@ describe('secure-browser-launcher', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should allow file URLs only when explicitly requested with an allow-list', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
const filePath = resolve('report.html');
|
||||
const fileUrl = pathToFileURL(filePath).href;
|
||||
|
||||
await expect(
|
||||
openBrowserSecurely('file:///tmp/report.html'),
|
||||
).rejects.toThrow('Unsafe protocol');
|
||||
|
||||
await expect(
|
||||
openBrowserSecurely(fileUrl, {
|
||||
allowFile: true,
|
||||
}),
|
||||
).rejects.toThrow('allowedFilePaths is required');
|
||||
|
||||
await openBrowserSecurely(fileUrl, {
|
||||
allowFile: true,
|
||||
allowedFilePaths: [filePath],
|
||||
});
|
||||
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
'xdg-open',
|
||||
[fileUrl],
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should restrict file URLs to the caller allow-list when provided', async () => {
|
||||
const allowedPath = resolve('report.html');
|
||||
const otherPath = resolve('other.html');
|
||||
|
||||
await expect(
|
||||
openBrowserSecurely(pathToFileURL(otherPath).href, {
|
||||
allowFile: true,
|
||||
allowedFilePaths: [allowedPath],
|
||||
}),
|
||||
).rejects.toThrow('allowed file set');
|
||||
});
|
||||
|
||||
it('should reject invalid URLs', async () => {
|
||||
await expect(openBrowserSecurely('not-a-url')).rejects.toThrow(
|
||||
'Invalid URL',
|
||||
|
|
@ -184,6 +266,7 @@ describe('secure-browser-launcher', () => {
|
|||
|
||||
it('should use xdg-open on Linux', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
await openBrowserSecurely('https://example.com');
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
'xdg-open',
|
||||
|
|
@ -198,6 +281,265 @@ describe('secure-browser-launcher', () => {
|
|||
'Unsupported platform',
|
||||
);
|
||||
});
|
||||
|
||||
it('should prefer BROWSER when it is configured', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
vi.stubEnv('BROWSER', 'firefox --new-tab');
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'firefox',
|
||||
['--new-tab', 'https://example.com'],
|
||||
expect.objectContaining({
|
||||
detached: true,
|
||||
stdio: 'ignore',
|
||||
}),
|
||||
);
|
||||
expect(mockExecFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should substitute the BROWSER placeholder instead of appending the URL', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
vi.stubEnv('BROWSER', 'firefox --new-tab %s');
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'firefox',
|
||||
['--new-tab', 'https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should substitute BROWSER placeholders literally', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
vi.stubEnv('BROWSER', 'firefox --new-tab %s');
|
||||
const url = 'https://example.com/callback?code=$&state=abc';
|
||||
|
||||
await openBrowserSecurely(url);
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'firefox',
|
||||
['--new-tab', url],
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should parse quoted arguments inside BROWSER tokens', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
vi.stubEnv(
|
||||
'BROWSER',
|
||||
'chromium --user-data-dir="/tmp/my profile" --new-tab %s',
|
||||
);
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'chromium',
|
||||
['--user-data-dir=/tmp/my profile', '--new-tab', 'https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should parse quoted command paths in BROWSER', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
vi.stubEnv('BROWSER', '"/opt/my browser/chromium" --new-tab %s');
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'/opt/my browser/chromium',
|
||||
['--new-tab', 'https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(mockExecFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should let an explicit BROWSER override headless Linux detection', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', '');
|
||||
vi.stubEnv('WAYLAND_DISPLAY', '');
|
||||
vi.stubEnv('MIR_SOCKET', '');
|
||||
vi.stubEnv('BROWSER', 'firefox');
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'firefox',
|
||||
['https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(mockExecFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should fall back to the platform opener for blocklisted BROWSER values', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
vi.stubEnv('BROWSER', 'www-browser --headless');
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).not.toHaveBeenCalled();
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
'xdg-open',
|
||||
['https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should block BROWSER values by command basename before using the platform opener', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
vi.stubEnv('BROWSER', '/usr/bin/www-browser --headless');
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).not.toHaveBeenCalled();
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
'xdg-open',
|
||||
['https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should still skip blocklisted BROWSER values in headless Linux', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', '');
|
||||
vi.stubEnv('WAYLAND_DISPLAY', '');
|
||||
vi.stubEnv('MIR_SOCKET', '');
|
||||
vi.stubEnv('BROWSER', 'www-browser');
|
||||
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).not.toHaveBeenCalled();
|
||||
expect(mockExecFile).not.toHaveBeenCalled();
|
||||
expect(consoleSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Please open this URL manually'),
|
||||
);
|
||||
|
||||
consoleSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should ignore BROWSER on Windows and keep the PowerShell opener', async () => {
|
||||
setPlatform('win32');
|
||||
vi.stubEnv('BROWSER', 'firefox --new-tab');
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).not.toHaveBeenCalled();
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
'powershell.exe',
|
||||
expect.arrayContaining([
|
||||
'-Command',
|
||||
`Start-Process 'https://example.com'`,
|
||||
]),
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should ignore blocklisted BROWSER values on Windows', async () => {
|
||||
setPlatform('win32');
|
||||
vi.stubEnv('BROWSER', 'www-browser');
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).not.toHaveBeenCalled();
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
'powershell.exe',
|
||||
expect.arrayContaining([
|
||||
'-Command',
|
||||
`Start-Process 'https://example.com'`,
|
||||
]),
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should fall back to the platform opener for invalid BROWSER values', async () => {
|
||||
setPlatform('darwin');
|
||||
vi.stubEnv('BROWSER', '"');
|
||||
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).not.toHaveBeenCalled();
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
'open',
|
||||
['https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(consoleSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Invalid BROWSER environment variable'),
|
||||
);
|
||||
|
||||
consoleSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should fall back to the platform opener when explicit BROWSER launch fails', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
vi.stubEnv('BROWSER', 'firefox --new-tab');
|
||||
mockExecFile.mockResolvedValueOnce({ stdout: '', stderr: '' });
|
||||
mockSpawn.mockImplementationOnce(() => {
|
||||
const child: {
|
||||
once: ReturnType<typeof vi.fn>;
|
||||
unref: ReturnType<typeof vi.fn>;
|
||||
} = {
|
||||
once: vi.fn((event: string, callback: (error?: Error) => void) => {
|
||||
if (event === 'error') {
|
||||
callback(new Error('spawn failed'));
|
||||
}
|
||||
return child;
|
||||
}),
|
||||
unref: vi.fn(),
|
||||
};
|
||||
return child;
|
||||
});
|
||||
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
await expect(
|
||||
openBrowserSecurely('https://example.com'),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'firefox',
|
||||
['--new-tab', 'https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
'xdg-open',
|
||||
['https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(consoleSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Failed to open BROWSER command firefox'),
|
||||
);
|
||||
|
||||
consoleSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should skip browser launch in headless Linux', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', '');
|
||||
vi.stubEnv('WAYLAND_DISPLAY', '');
|
||||
vi.stubEnv('MIR_SOCKET', '');
|
||||
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockExecFile).not.toHaveBeenCalled();
|
||||
expect(consoleSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Please open this URL manually'),
|
||||
);
|
||||
|
||||
consoleSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Error handling', () => {
|
||||
|
|
@ -222,6 +564,7 @@ describe('secure-browser-launcher', () => {
|
|||
describe('Linux Fallback', () => {
|
||||
it('should try fallback browsers on Linux', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
|
||||
mockExecFile.mockRejectedValueOnce(new Error('Command not found'));
|
||||
mockExecFile.mockResolvedValueOnce({ stdout: '', stderr: '' });
|
||||
|
|
@ -242,5 +585,23 @@ describe('secure-browser-launcher', () => {
|
|||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it('should detach real browser fallback commands', async () => {
|
||||
setPlatform('linux');
|
||||
vi.stubEnv('DISPLAY', ':1');
|
||||
|
||||
mockExecFile
|
||||
.mockRejectedValueOnce(new Error('xdg-open missing'))
|
||||
.mockRejectedValueOnce(new Error('gnome-open missing'))
|
||||
.mockRejectedValueOnce(new Error('kde-open missing'));
|
||||
|
||||
await openBrowserSecurely('https://example.com');
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'firefox',
|
||||
['https://example.com'],
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -4,21 +4,35 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { execFile } from 'node:child_process';
|
||||
import { execFile, spawn, type SpawnOptions } from 'node:child_process';
|
||||
import { promisify } from 'node:util';
|
||||
import { platform } from 'node:os';
|
||||
import { URL } from 'node:url';
|
||||
import { resolve } from 'node:path';
|
||||
import { URL, fileURLToPath } from 'node:url';
|
||||
import {
|
||||
isBrowserCommandBlocked,
|
||||
shouldAttemptBrowserLaunch,
|
||||
} from './browser.js';
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
||||
type BrowserLaunchOptions = {
|
||||
allowFile?: boolean;
|
||||
allowedFilePaths?: string[];
|
||||
};
|
||||
|
||||
/**
|
||||
* Validates that a URL is safe to open in a browser.
|
||||
* Only allows HTTP and HTTPS URLs to prevent command injection.
|
||||
* Only allows HTTP and HTTPS URLs by default. file:// URLs are allowed only
|
||||
* when a caller opts in for locally generated reports.
|
||||
*
|
||||
* @param url The URL to validate
|
||||
* @throws Error if the URL is invalid or uses an unsafe protocol
|
||||
*/
|
||||
function validateUrl(url: string): void {
|
||||
function validateUrl(
|
||||
url: string,
|
||||
{ allowFile = false, allowedFilePaths }: BrowserLaunchOptions = {},
|
||||
): void {
|
||||
let parsedUrl: URL;
|
||||
|
||||
try {
|
||||
|
|
@ -27,13 +41,30 @@ function validateUrl(url: string): void {
|
|||
throw new Error(`Invalid URL: ${url}`);
|
||||
}
|
||||
|
||||
// Only allow HTTP and HTTPS protocols
|
||||
if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
|
||||
const allowedProtocols = allowFile
|
||||
? ['http:', 'https:', 'file:']
|
||||
: ['http:', 'https:'];
|
||||
|
||||
// Only allow browser-safe protocols by default.
|
||||
if (!allowedProtocols.includes(parsedUrl.protocol)) {
|
||||
throw new Error(
|
||||
`Unsafe protocol: ${parsedUrl.protocol}. Only HTTP and HTTPS are allowed.`,
|
||||
`Unsafe protocol: ${parsedUrl.protocol}. Only ${allowedProtocols.join(
|
||||
', ',
|
||||
)} are allowed.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (parsedUrl.protocol === 'file:' && allowFile) {
|
||||
if (!allowedFilePaths?.length) {
|
||||
throw new Error('allowedFilePaths is required when allowFile is true');
|
||||
}
|
||||
const requestedPath = resolve(fileURLToPath(parsedUrl));
|
||||
const allowed = allowedFilePaths.map((filePath) => resolve(filePath));
|
||||
if (!allowed.includes(requestedPath)) {
|
||||
throw new Error('File URL is not in the allowed file set');
|
||||
}
|
||||
}
|
||||
|
||||
// Additional validation: ensure no newlines or control characters
|
||||
// eslint-disable-next-line no-control-regex
|
||||
if (/[\r\n\x00-\x1f]/.test(url)) {
|
||||
|
|
@ -49,51 +80,85 @@ function validateUrl(url: string): void {
|
|||
* and resolves successfully to prevent application crashes.
|
||||
*
|
||||
* @param url - The URL to open.
|
||||
* @param options.allowFile - Allow file:// URLs for locally generated reports.
|
||||
* @returns A promise that resolves when the attempt is made (whether successful or logged).
|
||||
*/
|
||||
export async function openBrowserSecurely(url: string): Promise<void> {
|
||||
export async function openBrowserSecurely(
|
||||
url: string,
|
||||
browserOptions: BrowserLaunchOptions = {},
|
||||
): Promise<void> {
|
||||
// Validate the URL first
|
||||
validateUrl(url);
|
||||
validateUrl(url, browserOptions);
|
||||
|
||||
const platformName = platform();
|
||||
let command: string;
|
||||
let args: string[];
|
||||
|
||||
switch (platformName) {
|
||||
case 'darwin':
|
||||
// macOS
|
||||
command = 'open';
|
||||
args = [url];
|
||||
break;
|
||||
const browserEnv = process.env['BROWSER']?.trim();
|
||||
const browserCommand =
|
||||
platformName === 'win32' ? undefined : buildBrowserCommand(browserEnv, url);
|
||||
|
||||
case 'win32':
|
||||
// Windows - use PowerShell with Start-Process
|
||||
// This avoids the cmd.exe shell which is vulnerable to injection
|
||||
command = 'powershell.exe';
|
||||
args = [
|
||||
'-NoProfile',
|
||||
'-NonInteractive',
|
||||
'-WindowStyle',
|
||||
'Hidden',
|
||||
'-Command',
|
||||
`Start-Process '${url.replace(/'/g, "''")}'`,
|
||||
];
|
||||
break;
|
||||
|
||||
case 'linux':
|
||||
case 'freebsd':
|
||||
case 'openbsd':
|
||||
// Linux and BSD variants
|
||||
// Try xdg-open first, fall back to other options
|
||||
command = 'xdg-open';
|
||||
args = [url];
|
||||
break;
|
||||
|
||||
default:
|
||||
throw new Error(`Unsupported platform: ${platformName}`);
|
||||
if (browserCommand) {
|
||||
try {
|
||||
await launchDetached(browserCommand.command, browserCommand.args);
|
||||
return;
|
||||
} catch (_error) {
|
||||
/* eslint-disable no-console */
|
||||
console.warn(
|
||||
`Failed to open BROWSER command ${browserCommand.command}: ${formatLaunchError(
|
||||
_error,
|
||||
)}. Falling back to the platform browser opener.`,
|
||||
);
|
||||
/* eslint-enable no-console */
|
||||
}
|
||||
}
|
||||
|
||||
const options: Record<string, unknown> = {
|
||||
if (!shouldAttemptBrowserLaunch({ ignoreBrowserBlocklist: true })) {
|
||||
/* eslint-disable no-console */
|
||||
console.warn(
|
||||
`Browser launch is not available in this environment. Please open this URL manually: ${url}`,
|
||||
);
|
||||
/* eslint-enable no-console */
|
||||
return;
|
||||
}
|
||||
|
||||
{
|
||||
switch (platformName) {
|
||||
case 'darwin':
|
||||
// macOS
|
||||
command = 'open';
|
||||
args = [url];
|
||||
break;
|
||||
|
||||
case 'win32':
|
||||
// Windows - use PowerShell with Start-Process
|
||||
// This avoids the cmd.exe shell which is vulnerable to injection
|
||||
command = 'powershell.exe';
|
||||
args = [
|
||||
'-NoProfile',
|
||||
'-NonInteractive',
|
||||
'-WindowStyle',
|
||||
'Hidden',
|
||||
'-Command',
|
||||
`Start-Process '${url.replace(/'/g, "''")}'`,
|
||||
];
|
||||
break;
|
||||
|
||||
case 'linux':
|
||||
case 'freebsd':
|
||||
case 'openbsd':
|
||||
// Linux and BSD variants
|
||||
// Try xdg-open first, fall back to other options
|
||||
command = 'xdg-open';
|
||||
args = [url];
|
||||
break;
|
||||
|
||||
default:
|
||||
throw new Error(`Unsupported platform: ${platformName}`);
|
||||
}
|
||||
}
|
||||
|
||||
const execOptions: Record<string, unknown> = {
|
||||
// Don't inherit parent's environment to avoid potential issues
|
||||
env: {
|
||||
...process.env,
|
||||
|
|
@ -106,7 +171,7 @@ export async function openBrowserSecurely(url: string): Promise<void> {
|
|||
};
|
||||
|
||||
try {
|
||||
await execFileAsync(command, args, options);
|
||||
await execFileAsync(command, args, execOptions);
|
||||
} catch (_error) {
|
||||
// For Linux, try fallback commands if xdg-open fails
|
||||
if (
|
||||
|
|
@ -116,17 +181,21 @@ export async function openBrowserSecurely(url: string): Promise<void> {
|
|||
command === 'xdg-open'
|
||||
) {
|
||||
const fallbackCommands = [
|
||||
'gnome-open',
|
||||
'kde-open',
|
||||
'firefox',
|
||||
'chromium',
|
||||
'google-chrome',
|
||||
'microsoft-edge',
|
||||
{ command: 'gnome-open', detached: false },
|
||||
{ command: 'kde-open', detached: false },
|
||||
{ command: 'firefox', detached: true },
|
||||
{ command: 'chromium', detached: true },
|
||||
{ command: 'google-chrome', detached: true },
|
||||
{ command: 'microsoft-edge', detached: true },
|
||||
];
|
||||
|
||||
for (const fallbackCommand of fallbackCommands) {
|
||||
for (const { command: fallbackCommand, detached } of fallbackCommands) {
|
||||
try {
|
||||
await execFileAsync(fallbackCommand, [url], options);
|
||||
if (detached) {
|
||||
await launchDetached(fallbackCommand, [url]);
|
||||
} else {
|
||||
await execFileAsync(fallbackCommand, [url], execOptions);
|
||||
}
|
||||
return; // Success!
|
||||
} catch {
|
||||
// Try next command
|
||||
|
|
@ -145,6 +214,129 @@ export async function openBrowserSecurely(url: string): Promise<void> {
|
|||
}
|
||||
}
|
||||
|
||||
async function launchDetached(command: string, args: string[]): Promise<void> {
|
||||
const spawnOptions: SpawnOptions = {
|
||||
env: {
|
||||
...process.env,
|
||||
SHELL: undefined,
|
||||
},
|
||||
detached: true,
|
||||
stdio: 'ignore',
|
||||
};
|
||||
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const child = spawn(command, args, spawnOptions);
|
||||
let settled = false;
|
||||
|
||||
child.once('error', (error) => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
reject(error);
|
||||
});
|
||||
|
||||
child.once('spawn', () => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
child.unref();
|
||||
resolve();
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function formatLaunchError(error: unknown): string {
|
||||
return error instanceof Error ? error.message : String(error);
|
||||
}
|
||||
|
||||
function buildBrowserCommand(
|
||||
browserEnv: string | undefined,
|
||||
url: string,
|
||||
): { command: string; args: string[] } | undefined {
|
||||
if (!browserEnv) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const browserCommand = parseBrowserCommand(browserEnv);
|
||||
if (!browserCommand) {
|
||||
/* eslint-disable no-console */
|
||||
console.warn(
|
||||
'Invalid BROWSER environment variable, falling back to platform default.',
|
||||
);
|
||||
/* eslint-enable no-console */
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if (isBrowserCommandBlocked(browserCommand.command)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
let usedPlaceholder = false;
|
||||
const args = browserCommand.args.map((arg) => {
|
||||
if (!arg.includes('%s')) {
|
||||
return arg;
|
||||
}
|
||||
usedPlaceholder = true;
|
||||
return arg.replace(/%s/g, () => url);
|
||||
});
|
||||
|
||||
if (!usedPlaceholder) {
|
||||
args.push(url);
|
||||
}
|
||||
|
||||
return { command: browserCommand.command, args };
|
||||
}
|
||||
|
||||
function parseBrowserCommand(
|
||||
browserEnv: string,
|
||||
): { command: string; args: string[] } | undefined {
|
||||
const parts: string[] = [];
|
||||
let current = '';
|
||||
let quote: '"' | "'" | undefined;
|
||||
|
||||
for (const char of browserEnv) {
|
||||
if (quote) {
|
||||
if (char === quote) {
|
||||
quote = undefined;
|
||||
} else {
|
||||
current += char;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if (char === '"' || char === "'") {
|
||||
quote = char;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (/\s/.test(char)) {
|
||||
if (current) {
|
||||
parts.push(current);
|
||||
current = '';
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
current += char;
|
||||
}
|
||||
|
||||
if (quote) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if (current) {
|
||||
parts.push(current);
|
||||
}
|
||||
|
||||
const [command, ...args] = parts;
|
||||
if (!command) {
|
||||
return undefined;
|
||||
}
|
||||
return { command, args };
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if the current environment should attempt to launch a browser.
|
||||
* This is the same logic as in browser.ts for consistency.
|
||||
|
|
@ -152,44 +344,5 @@ export async function openBrowserSecurely(url: string): Promise<void> {
|
|||
* @returns True if the tool should attempt to launch a browser
|
||||
*/
|
||||
export function shouldLaunchBrowser(): boolean {
|
||||
// A list of browser names that indicate we should not attempt to open a
|
||||
// web browser for the user.
|
||||
const browserBlocklist = ['www-browser'];
|
||||
const browserEnv = process.env['BROWSER'];
|
||||
if (browserEnv && browserBlocklist.includes(browserEnv)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Common environment variables used in CI/CD or other non-interactive shells.
|
||||
if (
|
||||
process.env['CI'] ||
|
||||
process.env['DEBIAN_FRONTEND'] === 'noninteractive'
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// The presence of SSH_CONNECTION indicates a remote session.
|
||||
// We should not attempt to launch a browser unless a display is explicitly available
|
||||
// (checked below for Linux).
|
||||
const isSSH = !!process.env['SSH_CONNECTION'];
|
||||
|
||||
// On Linux, the presence of a display server is a strong indicator of a GUI.
|
||||
if (platform() === 'linux') {
|
||||
// These are environment variables that can indicate a running compositor on Linux.
|
||||
const displayVariables = ['DISPLAY', 'WAYLAND_DISPLAY', 'MIR_SOCKET'];
|
||||
const hasDisplay = displayVariables.some((v) => !!process.env[v]);
|
||||
if (!hasDisplay) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// If in an SSH session on a non-Linux OS (e.g., macOS), don't launch browser.
|
||||
// The Linux case is handled above (it's allowed if DISPLAY is set).
|
||||
if (isSSH && platform() !== 'linux') {
|
||||
return false;
|
||||
}
|
||||
|
||||
// For non-Linux OSes, we generally assume a GUI is available
|
||||
// unless other signals (like SSH) suggest otherwise.
|
||||
return true;
|
||||
return shouldAttemptBrowserLaunch();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue