Skip to content

Commit fb3a95e

Browse files
authored
Merge pull request #1234 from afarber/1115-fix-dep0190-deprecation
fix: replace spawn shell option with explicit shell args to avoid Node.js DEP0190 warning
2 parents 6eb16c0 + fa8b5a7 commit fb3a95e

File tree

5 files changed

+38
-19
lines changed

5 files changed

+38
-19
lines changed

packages/cli/src/utils/handleAutoUpdate.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,12 @@ describe('handleAutoUpdate', () => {
241241
handleAutoUpdate(mockUpdateInfo, mockSettings, '/root', mockSpawn);
242242

243243
expect(mockSpawn).toHaveBeenCalledWith(
244-
'npm i -g @qwen-code/qwen-code@nightly',
244+
expect.stringMatching(/^(bash|cmd\.exe)$/),
245+
expect.arrayContaining([
246+
expect.stringMatching(/^(-c|\/c)$/),
247+
'npm i -g @qwen-code/qwen-code@nightly',
248+
]),
245249
{
246-
shell: true,
247250
stdio: 'pipe',
248251
},
249252
);

packages/cli/src/utils/handleAutoUpdate.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type { HistoryItem } from '../ui/types.js';
1212
import { MessageType } from '../ui/types.js';
1313
import { spawnWrapper } from './spawnWrapper.js';
1414
import type { spawn } from 'node:child_process';
15+
import os from 'node:os';
1516

1617
export function handleAutoUpdate(
1718
info: UpdateObject | null,
@@ -53,7 +54,10 @@ export function handleAutoUpdate(
5354
'@latest',
5455
isNightly ? '@nightly' : `@${info.update.latest}`,
5556
);
56-
const updateProcess = spawnFn(updateCommand, { stdio: 'pipe', shell: true });
57+
const isWindows = os.platform() === 'win32';
58+
const shell = isWindows ? 'cmd.exe' : 'bash';
59+
const shellArgs = isWindows ? ['/c', updateCommand] : ['-c', updateCommand];
60+
const updateProcess = spawnFn(shell, shellArgs, { stdio: 'pipe' });
5761
let errorOutput = '';
5862
updateProcess.stderr.on('data', (data) => {
5963
errorOutput += data.toString();

packages/cli/src/utils/sandbox.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ export async function start_sandbox(
291291
sandboxEnv['NO_PROXY'] = noProxy;
292292
sandboxEnv['no_proxy'] = noProxy;
293293
}
294-
proxyProcess = spawn(proxyCommand, {
294+
// Note: CodeQL flags this as js/shell-command-injection-from-environment.
295+
// This is intentional - CLI tool executes user-provided proxy commands.
296+
proxyProcess = spawn('bash', ['-c', proxyCommand], {
295297
stdio: ['ignore', 'pipe', 'pipe'],
296-
shell: true,
297298
detached: true,
298299
});
299300
// install handlers to stop proxy on exit/signal
@@ -781,9 +782,15 @@ export async function start_sandbox(
781782
if (proxyCommand) {
782783
// run proxyCommand in its own container
783784
const proxyContainerCommand = `${config.command} run --rm --init ${userFlag} --name ${SANDBOX_PROXY_NAME} --network ${SANDBOX_PROXY_NAME} -p 8877:8877 -v ${process.cwd()}:${workdir} --workdir ${workdir} ${image} ${proxyCommand}`;
784-
proxyProcess = spawn(proxyContainerCommand, {
785+
const isWindows = os.platform() === 'win32';
786+
const proxyShell = isWindows ? 'cmd.exe' : 'bash';
787+
const proxyShellArgs = isWindows
788+
? ['/c', proxyContainerCommand]
789+
: ['-c', proxyContainerCommand];
790+
// Note: CodeQL flags this as js/shell-command-injection-from-environment.
791+
// This is intentional - CLI tool executes user-provided proxy commands in container.
792+
proxyProcess = spawn(proxyShell, proxyShellArgs, {
785793
stdio: ['ignore', 'pipe', 'pipe'],
786-
shell: true,
787794
detached: true,
788795
});
789796
// install handlers to stop proxy on exit/signal

packages/core/src/services/shellExecutionService.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -580,9 +580,11 @@ describe('ShellExecutionService child_process fallback', () => {
580580
});
581581

582582
expect(mockCpSpawn).toHaveBeenCalledWith(
583-
'ls -l',
584-
[],
585-
expect.objectContaining({ shell: 'bash' }),
583+
'bash',
584+
['-c', 'ls -l'],
585+
expect.objectContaining({
586+
detached: true,
587+
}),
586588
);
587589
expect(result.exitCode).toBe(0);
588590
expect(result.signal).toBeNull();
@@ -825,10 +827,9 @@ describe('ShellExecutionService child_process fallback', () => {
825827
);
826828

827829
expect(mockCpSpawn).toHaveBeenCalledWith(
828-
'dir "foo bar"',
829-
[],
830+
'cmd.exe',
831+
['/c', 'dir "foo bar"'],
830832
expect.objectContaining({
831-
shell: true,
832833
detached: false,
833834
windowsHide: true,
834835
}),
@@ -840,10 +841,9 @@ describe('ShellExecutionService child_process fallback', () => {
840841
await simulateExecution('ls "foo bar"', (cp) => cp.emit('exit', 0, null));
841842

842843
expect(mockCpSpawn).toHaveBeenCalledWith(
843-
'ls "foo bar"',
844-
[],
844+
'bash',
845+
['-c', 'ls "foo bar"'],
845846
expect.objectContaining({
846-
shell: 'bash',
847847
detached: true,
848848
}),
849849
);

packages/core/src/services/shellExecutionService.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,17 @@ export class ShellExecutionService {
223223
): ShellExecutionHandle {
224224
try {
225225
const isWindows = os.platform() === 'win32';
226+
const shell = isWindows ? 'cmd.exe' : 'bash';
227+
const shellArgs = isWindows
228+
? ['/c', commandToExecute]
229+
: ['-c', commandToExecute];
226230

227-
const child = cpSpawn(commandToExecute, [], {
231+
// Note: CodeQL flags this as js/shell-command-injection-from-environment.
232+
// This is intentional - CLI tool executes user-provided shell commands.
233+
const child = cpSpawn(shell, shellArgs, {
228234
cwd,
229235
stdio: ['ignore', 'pipe', 'pipe'],
230-
windowsVerbatimArguments: true,
231-
shell: isWindows ? true : 'bash',
236+
windowsVerbatimArguments: isWindows,
232237
detached: !isWindows,
233238
windowsHide: isWindows,
234239
env: {

0 commit comments

Comments
 (0)