Skip to content

Commit 4ebcb8d

Browse files
authored
avoid spawn with shell: true and arguments (#4456)
spawn with shell: true and arguments leads to the below deprecation warning [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated. This does not appear to presently cause us any security concerns as this is used only with safe input from our own integration scripts, but we can avoid the use of the shell entirely and protect from removal of this functionality in a later version, while still preserving win32 compatibility.
1 parent 4b92e1f commit 4ebcb8d

File tree

1 file changed

+16
-22
lines changed

1 file changed

+16
-22
lines changed

resources/utils.ts

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,41 +33,35 @@ interface NPMOptions extends SpawnOptions {
3333
}
3434

3535
export function npm(options?: NPMOptions) {
36-
const globalOptions = options?.quiet === true ? ['--quiet'] : [];
37-
38-
// `npm` points to an executable shell script; so it doesn't require a shell per se.
39-
// On Windows `shell: true` is required or `npm` will be picked rather than `npm.cmd`.
40-
// This could alternatively be handled by manually selecting `npm.cmd` on Windows.
41-
// See: https://github.com/nodejs/node/issues/3675 and in particular
42-
// https://github.com/nodejs/node/issues/3675#issuecomment-308963807.
43-
const npmOptions = { shell: true, ...options };
36+
let npmCmd;
37+
let globalOptions = options?.quiet === true ? ['--quiet'] : [];
38+
39+
// See: https://github.com/nodejs/node/issues/3675.
40+
if (process.platform === 'win32') {
41+
npmCmd = 'cmd';
42+
globalOptions = ['/c', 'npm.cmd', ...globalOptions];
43+
} else {
44+
npmCmd = 'npm';
45+
}
4446

4547
return {
4648
run(...args: ReadonlyArray<string>): void {
47-
spawn('npm', [...globalOptions, 'run', ...args], npmOptions);
49+
spawn(npmCmd, [...globalOptions, 'run', ...args], options);
4850
},
4951
install(...args: ReadonlyArray<string>): void {
50-
spawn('npm', [...globalOptions, 'install', ...args], npmOptions);
52+
spawn(npmCmd, [...globalOptions, 'install', ...args], options);
5153
},
5254
ci(...args: ReadonlyArray<string>): void {
53-
spawn('npm', [...globalOptions, 'ci', ...args], npmOptions);
55+
spawn(npmCmd, [...globalOptions, 'ci', ...args], options);
5456
},
5557
exec(...args: ReadonlyArray<string>): void {
56-
spawn('npm', [...globalOptions, 'exec', ...args], npmOptions);
58+
spawn(npmCmd, [...globalOptions, 'exec', ...args], options);
5759
},
5860
pack(...args: ReadonlyArray<string>): string {
59-
return spawnOutput(
60-
'npm',
61-
[...globalOptions, 'pack', ...args],
62-
npmOptions,
63-
);
61+
return spawnOutput(npmCmd, [...globalOptions, 'pack', ...args], options);
6462
},
6563
diff(...args: ReadonlyArray<string>): string {
66-
return spawnOutput(
67-
'npm',
68-
[...globalOptions, 'diff', ...args],
69-
npmOptions,
70-
);
64+
return spawnOutput(npmCmd, [...globalOptions, 'diff', ...args], options);
7165
},
7266
};
7367
}

0 commit comments

Comments
 (0)