Skip to content

Commit c1e11c3

Browse files
authored
feat: [CLI-194] upgrading shescape newest version, defaulting to shell: false on non-Windows systems (#686)
feat: re-adding `shescape` v2 # Conflicts: # package-lock.json # package.json
1 parent db7753f commit c1e11c3

File tree

4 files changed

+122
-145
lines changed

4 files changed

+122
-145
lines changed

lib/sub-process.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as childProcess from "child_process";
2-
import { quoteAll } from "shescape";
2+
import * as os from "node:os";
3+
import { escapeAll, quoteAll } from "shescape/stateless";
34

45
export { execute, CmdOutput };
56
interface CmdOutput {
@@ -13,13 +14,28 @@ function execute(
1314
options?,
1415
): Promise<CmdOutput> {
1516
const spawnOptions: any = {
16-
shell: true,
17+
shell: false,
1718
env: { ...process.env },
1819
};
1920
if (options && options.cwd) {
2021
spawnOptions.cwd = options.cwd;
2122
}
22-
args = quoteAll(args, spawnOptions);
23+
24+
if (args) {
25+
// Best practices, also security-wise, is to not invoke processes in a shell, but as a stand-alone command.
26+
// However, on Windows, we need to invoke the command in a shell, due to internal NodeJS problems with this approach
27+
// see: https://nodejs.org/docs/latest-v24.x/api/child_process.html#spawning-bat-and-cmd-files-on-windows
28+
const isWinLocal = /^win/.test(os.platform());
29+
if (isWinLocal) {
30+
spawnOptions.shell = true;
31+
// Further, we distinguish between quoting and escaping arguments since quoteAll does not support quoting without
32+
// supplying a shell, but escapeAll does.
33+
// See this (very long) discussion for more details: https://github.com/ericcornelissen/shescape/issues/2009
34+
args = quoteAll(args, { ...spawnOptions, flagProtection: false });
35+
} else {
36+
args = escapeAll(args, { ...spawnOptions, flagProtection: false });
37+
}
38+
}
2339

2440
// Before spawning an external process, we look if we need to restore the system proxy configuration,
2541
// which overrides the cli internal proxy configuration.
@@ -38,6 +54,13 @@ function execute(
3854
let stderr = "";
3955

4056
const proc = childProcess.spawn(command, args, spawnOptions);
57+
58+
// Handle spawn errors (e.g., ENOENT when command doesn't exist)
59+
proc.on("error", (error) => {
60+
stderr = error.message;
61+
reject({ stdout, stderr });
62+
});
63+
4164
proc.stdout.on("data", (data) => {
4265
stdout = stdout + data;
4366
});

package-lock.json

Lines changed: 92 additions & 83 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@
4545
"mkdirp": "^1.0.4",
4646
"packageurl-js": "1.2.0",
4747
"semver": "^7.7.1",
48-
"shescape": "^1.7.4",
48+
"shescape": "^2.1.6",
4949
"snyk-nodejs-lockfile-parser": "^2.2.2",
50-
"snyk-poetry-lockfile-parser": "^1.4.0",
50+
"snyk-poetry-lockfile-parser": "1.4.0",
5151
"snyk-resolve-deps": "^4.9.1",
5252
"tar-stream": "^2.1.0",
5353
"tmp": "^0.2.2",
@@ -62,7 +62,7 @@
6262
"@types/debug": "^4.1.5",
6363
"@types/jest": "^29.5.5",
6464
"@types/minimatch": "^5.1.2",
65-
"@types/node": "^14.14.31",
65+
"@types/node": "^20.19.1",
6666
"@types/tar-stream": "^1.6.1",
6767
"@types/tmp": "^0.2.0",
6868
"jest": "^29.7.0",
@@ -73,7 +73,7 @@
7373
"tsc-watch": "^4.2.8",
7474
"tslint": "^5.16.0",
7575
"tslint-config-prettier": "^1.18.0",
76-
"typescript": "~4.7.3"
76+
"typescript": "~5.4.2"
7777
},
7878
"release": {
7979
"branches": [

0 commit comments

Comments
 (0)