diff --git a/lib/dependencies/sub-process.ts b/lib/dependencies/sub-process.ts index 0e12e75..29c3b6c 100644 --- a/lib/dependencies/sub-process.ts +++ b/lib/dependencies/sub-process.ts @@ -1,16 +1,41 @@ import { spawn, SpawnOptions, spawnSync } from 'child_process'; -import { quoteAll } from 'shescape/stateless'; +import { escapeAll, quoteAll } from 'shescape/stateless'; +import * as os from 'node:os'; interface ProcessOptions { cwd?: string; env?: { [name: string]: string }; } -function makeSpawnOptions(options?: ProcessOptions) { +function processArguments( + args: string[], + spawnOptions: SpawnOptions +): string[] { + if (!args) { + return args; + } + + // Best practices, also security-wise, is to not invoke processes in a shell, but as a stand-alone command. + // However, on Windows, we need to invoke the command in a shell, due to internal NodeJS problems with this approach + // see: https://nodejs.org/docs/latest-v24.x/api/child_process.html#spawning-bat-and-cmd-files-on-windows + const isWinLocal = /^win/.test(os.platform()); + if (isWinLocal) { + spawnOptions.shell = true; + // Further, we distinguish between quoting and escaping arguments since quoteAll does not support quoting without + // supplying a shell, but escapeAll does. + // See this (very long) discussion for more details: https://github.com/ericcornelissen/shescape/issues/2009 + return quoteAll(args, { ...spawnOptions, flagProtection: false }); + } else { + return escapeAll(args, { ...spawnOptions, flagProtection: false }); + } +} + +function makeSpawnOptions(options?: ProcessOptions): SpawnOptions { const spawnOptions: SpawnOptions = { - shell: true, + shell: false, env: { ...process.env }, }; + if (options && options.cwd) { spawnOptions.cwd = options.cwd; } @@ -19,7 +44,7 @@ function makeSpawnOptions(options?: ProcessOptions) { } // Before spawning an external process, we look if we need to restore the system proxy configuration, - // which overides the cli internal proxy configuration. + // which overrides the cli internal proxy configuration. if (process.env.SNYK_SYSTEM_HTTP_PROXY !== undefined) { spawnOptions.env.HTTP_PROXY = process.env.SNYK_SYSTEM_HTTP_PROXY; } @@ -39,19 +64,26 @@ export function execute( options?: ProcessOptions ): Promise { const spawnOptions = makeSpawnOptions(options); - args = quoteAll(args, { flagProtection: false }); + const processedArgs = processArguments(args, spawnOptions); + return new Promise((resolve, reject) => { let stdout = ''; let stderr = ''; - const proc = spawn(command, args, spawnOptions); + const proc = spawn(command, processedArgs, spawnOptions); proc.stdout.on('data', (data) => { stdout = stdout + data; }); + proc.stderr.on('data', (data) => { stderr = stderr + data; }); + proc.on('error', (err) => { + stderr = err.message; + return reject({ stdout, stderr }); + }); + proc.on('close', (code) => { if (code !== 0) { return reject(stdout || stderr); @@ -67,7 +99,7 @@ export function executeSync( options?: ProcessOptions ) { const spawnOptions = makeSpawnOptions(options); - args = quoteAll(args, { flagProtection: false }); + const processedArgs = processArguments(args, spawnOptions); - return spawnSync(command, args, spawnOptions); + return spawnSync(command, processedArgs, spawnOptions); } diff --git a/package.json b/package.json index 6f5f752..d8924db 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "dependencies": { "@snyk/cli-interface": "^2.11.2", "@snyk/dep-graph": "^1.28.1", - "shescape": "2.1.4", + "shescape": "2.1.6", "snyk-poetry-lockfile-parser": "^1.9.0", "tmp": "0.2.3" }, diff --git a/test/system/inspect.spec.ts b/test/system/inspect.spec.ts index 48f6838..ca4543f 100644 --- a/test/system/inspect.spec.ts +++ b/test/system/inspect.spec.ts @@ -150,7 +150,6 @@ describe('inspect', () => { { pkg: { name: 'jaraco.collections', - version: '5.1.0', }, directDeps: ['irc'], }, @@ -532,7 +531,6 @@ describe('inspect', () => { { pkg: { name: 'jaraco.collections', - version: '5.1.0', }, directDeps: ['irc'], }, diff --git a/test/test-utils.ts b/test/test-utils.ts index c013e38..49082ec 100644 --- a/test/test-utils.ts +++ b/test/test-utils.ts @@ -103,7 +103,7 @@ function createVenv(venvDir: string) { revert = deactivateVirtualenv(); } try { - let proc = subProcess.executeSync('python3 -m venv', [venvDir]); + let proc = subProcess.executeSync('python3', ['-m', 'venv', venvDir]); if (proc.status !== 0) { console.error(proc.stdout.toString() + '\n' + proc.stderr.toString()); throw new Error('Failed to create virtualenv in ' + venvDir);