Skip to content

Commit 058048a

Browse files
gaurav-singh-9227claude
gaurav-singh-9227
and
claude
committed
fix: return browser-open command instead of spawning it
Resolves CodeQL js/command-line-injection (alert #2) at src/tools/applive-utils/start-session.ts. Instead of the server calling childProcess.spawn() with a user-influenced launch URL, it now returns the platform-appropriate open command as text. The host agent prompts the user before executing it, so the server never spawns a process and the command-injection surface is eliminated. The URL is still validated (https + *.browserstack.com allowlist) before being surfaced. Applies the same conversion to the identical openBrowser/spawn in live-utils/start-session.ts for parity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 29f480a commit 058048a

2 files changed

Lines changed: 74 additions & 62 deletions

File tree

‎src/tools/applive-utils/start-session.ts‎

Lines changed: 33 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { getBrowserStackAuth } from "../../lib/get-auth.js";
99
import { findDeviceByName } from "./device-search.js";
1010
import { pickVersion } from "./version-utils.js";
1111
import { DeviceEntry } from "./types.js";
12-
import childProcess from "child_process";
1312
import { BrowserStackConfig } from "../../lib/types.js";
1413
import envConfig from "../../config.js";
1514

@@ -114,51 +113,47 @@ export async function startSession(
114113
const launchUrl = `https://app-live.browserstack.com/dashboard#${params.toString()}&device=${deviceParam}`;
115114

116115
if (!envConfig.REMOTE_MCP) {
117-
openBrowser(launchUrl);
116+
const openCommand = getOpenBrowserCommand(launchUrl);
117+
if (openCommand) {
118+
return [
119+
`App Live session URL: ${launchUrl}${note}`,
120+
``,
121+
`To open the session in the default browser, run:`,
122+
` ${openCommand}`,
123+
].join("\n");
124+
}
118125
}
119126

120127
return launchUrl + note;
121128
}
122129

123130
/**
124-
* Opens the launch URL in the default browser.
125-
* @param launchUrl - The URL to open.
126-
* @throws Will throw an error if the browser fails to open.
131+
* Returns the platform-appropriate shell command to open `launchUrl` in the
132+
* default browser, or null if the URL is not a trusted BrowserStack URL.
133+
*
134+
* The command is returned to the MCP client so the host agent can prompt the
135+
* user before executing it. The server itself never spawns a process, which
136+
* eliminates the command-injection surface entirely.
127137
*/
128-
function openBrowser(launchUrl: string): void {
138+
function getOpenBrowserCommand(launchUrl: string): string | null {
139+
let parsed: URL;
129140
try {
130-
const parsed = new URL(launchUrl);
131-
if (
132-
parsed.protocol !== "https:" ||
133-
!/(^|\.)browserstack\.com$/i.test(parsed.hostname)
134-
) {
135-
logger.error(`Refusing to open untrusted URL: ${launchUrl}`);
136-
return;
137-
}
138-
139-
const command =
140-
process.platform === "darwin"
141-
? ["open", launchUrl]
142-
: process.platform === "win32"
143-
? ["cmd", "/c", "start", launchUrl]
144-
: ["xdg-open", launchUrl];
145-
146-
// nosemgrep:javascript.lang.security.detect-child-process.detect-child-process
147-
const child = childProcess.spawn(command[0], command.slice(1), {
148-
stdio: "ignore",
149-
detached: true,
150-
});
151-
152-
child.on("error", (error) => {
153-
logger.error(
154-
`Failed to open browser automatically: ${error}. Please open this URL manually: ${launchUrl}`,
155-
);
156-
});
141+
parsed = new URL(launchUrl);
142+
} catch {
143+
logger.error(`Refusing to surface malformed URL: ${launchUrl}`);
144+
return null;
145+
}
157146

158-
child.unref();
159-
} catch (error) {
160-
logger.error(
161-
`Failed to open browser automatically: ${error}. Please open this URL manually: ${launchUrl}`,
162-
);
147+
if (
148+
parsed.protocol !== "https:" ||
149+
!/(^|\.)browserstack\.com$/i.test(parsed.hostname)
150+
) {
151+
logger.error(`Refusing to surface untrusted URL: ${launchUrl}`);
152+
return null;
163153
}
154+
155+
const quoted = `"${parsed.toString()}"`;
156+
if (process.platform === "darwin") return `open ${quoted}`;
157+
if (process.platform === "win32") return `cmd /c start "" ${quoted}`;
158+
return `xdg-open ${quoted}`;
164159
}

‎src/tools/live-utils/start-session.ts‎

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import logger from "../../logger.js";
2-
import childProcess from "child_process";
32
import { filterDesktop } from "./desktop-filter.js";
43
import { filterMobile } from "./mobile-filter.js";
54
import {
@@ -73,10 +72,21 @@ export async function startBrowserSession(
7372
isLocal,
7473
)
7574
: buildMobileUrl(args as MobileSearchArgs, entry as MobileEntry, isLocal);
75+
const note = entry.notes ? `, ${entry.notes}` : "";
76+
7677
if (!envConfig.REMOTE_MCP) {
77-
openBrowser(url);
78+
const openCommand = getOpenBrowserCommand(url);
79+
if (openCommand) {
80+
return [
81+
`Live session URL: ${url}${note}`,
82+
``,
83+
`To open the session in the default browser, run:`,
84+
` ${openCommand}`,
85+
].join("\n");
86+
}
7887
}
79-
return entry.notes ? `${url}, ${entry.notes}` : url;
88+
89+
return `${url}${note}`;
8090
}
8191

8292
function buildDesktopUrl(
@@ -125,28 +135,35 @@ function buildMobileUrl(
125135
return `https://live.browserstack.com/dashboard#${params.toString()}`;
126136
}
127137

128-
// ——— Open a browser window ———
138+
// ——— Build a browser-open command for the host agent ———
129139

130-
function openBrowser(launchUrl: string): void {
140+
/**
141+
* Returns the platform-appropriate shell command to open `launchUrl` in the
142+
* default browser, or null if the URL is not a trusted BrowserStack URL.
143+
*
144+
* The command is returned to the MCP client so the host agent can prompt the
145+
* user before executing it. The server itself never spawns a process, which
146+
* eliminates the command-injection surface entirely.
147+
*/
148+
function getOpenBrowserCommand(launchUrl: string): string | null {
149+
let parsed: URL;
131150
try {
132-
const command =
133-
process.platform === "darwin"
134-
? ["open", launchUrl]
135-
: process.platform === "win32"
136-
? ["cmd", "/c", "start", `""`, `"${launchUrl}"`]
137-
: ["xdg-open", launchUrl];
138-
139-
// nosemgrep:javascript.lang.security.detect-child-process.detect-child-process
140-
const child = childProcess.spawn(command[0], command.slice(1), {
141-
stdio: "ignore",
142-
detached: true,
143-
...(process.platform === "win32" ? { shell: true } : {}),
144-
});
145-
child.on("error", (err) =>
146-
logger.error(`Failed to open browser: ${err}. URL: ${launchUrl}`),
147-
);
148-
child.unref();
149-
} catch (err) {
150-
logger.error(`Failed to launch browser: ${err}. URL: ${launchUrl}`);
151+
parsed = new URL(launchUrl);
152+
} catch {
153+
logger.error(`Refusing to surface malformed URL: ${launchUrl}`);
154+
return null;
155+
}
156+
157+
if (
158+
parsed.protocol !== "https:" ||
159+
!/(^|\.)browserstack\.com$/i.test(parsed.hostname)
160+
) {
161+
logger.error(`Refusing to surface untrusted URL: ${launchUrl}`);
162+
return null;
151163
}
164+
165+
const quoted = `"${parsed.toString()}"`;
166+
if (process.platform === "darwin") return `open ${quoted}`;
167+
if (process.platform === "win32") return `cmd /c start "" ${quoted}`;
168+
return `xdg-open ${quoted}`;
152169
}

0 commit comments

Comments
 (0)