Merge pull request #43 from snyk/dotkas/CLI-194/add-shescape-hardening

feat: [CLI-194] bumping shescape, adding hardening

Author: dotkas

lib/index.ts

@@ -181,7 +181,7 @@ async function cocoapodsVersion(root: string): Promise<string> {
   let podVersionOutput = '';
   try {
     // 1st: try to run CocoaPods via bundler
-    podVersionOutput = await subProcess.execute('bundle exec pod', ['--version'], {cwd: root});
+    podVersionOutput = await subProcess.execute('bundle', ['exec', 'pod', '--version'], {cwd: root});
   } catch {
     try {
       // 2nd: try to run CocoaPods directly

lib/sub-process.ts

@@ -1,5 +1,6 @@
 import * as childProcess from 'child_process';
-import { quoteAll } from 'shescape/stateless';
+import { escapeAll, quoteAll } from 'shescape/stateless';
+import * as os from 'node:os';
 
 export function execute(
   command: string,
@@ -9,13 +10,26 @@ export function execute(
   const spawnOptions: {
     shell: boolean;
     cwd?: string;
-  } = { shell: true };
+  } = { shell: false };
   if (options && options.cwd) {
     spawnOptions.cwd = options.cwd;
   }
 
-  args = quoteAll(args, { flagProtection: false });
-
+  if (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
+      args = quoteAll(args, { ...spawnOptions, flagProtection: false });
+    } else {
+      args = escapeAll(args, { ...spawnOptions, flagProtection: false });
+    }
+  }
   return new Promise((resolve, reject) => {
     let stdout = '';
     let stderr = '';
@@ -28,6 +42,12 @@ export function execute(
       stderr = stderr + data;
     });
 
+    // Handle spawn errors (e.g., ENOENT when command doesn't exist)
+    proc.on('error', (error) => {
+      stderr = error.message;
+      reject({ stdout, stderr });
+    });
+
     proc.on('close', (code: number) => {
       if (code !== 0) {
         return reject(new Error(stdout || stderr));

package.json

@@ -34,7 +34,7 @@
     "@snyk/cli-interface": "^2.11.0",
     "@snyk/cocoapods-lockfile-parser": "3.6.2",
     "@snyk/dep-graph": "^1.23.1",
-    "shescape": "2.1.4",
+    "shescape": "2.1.6",
     "source-map-support": "^0.5.7",
     "tslib": "^2.0.0"
   },

test/lib/index.test.ts

@@ -36,7 +36,7 @@ describe('inspect(rootDir, targetFile?, options?)', () => {
       });
 
       expect(mockedExecute.mock.calls).toEqual([
-        ['bundle exec pod', ['--version'], { cwd: rootDir }],
+        ['bundle', ['exec', 'pod', '--version'], { cwd: rootDir }],
       ]);
     });
 
@@ -54,7 +54,7 @@ describe('inspect(rootDir, targetFile?, options?)', () => {
       });
 
       expect(mockedExecute.mock.calls).toEqual([
-        ['bundle exec pod', ['--version'], { cwd: rootDir }],
+        ['bundle', ['exec', 'pod', '--version'], { cwd: rootDir }],
         ['pod', ['--version'], { cwd: rootDir }],
       ]);
     });
@@ -73,7 +73,7 @@ describe('inspect(rootDir, targetFile?, options?)', () => {
       });
 
       expect(mockedExecute.mock.calls).toEqual([
-        ['bundle exec pod', ['--version'], { cwd: rootDir }],
+        ['bundle', ['exec', 'pod', '--version'], { cwd: rootDir }],
         ['pod', ['--version'], { cwd: rootDir }],
       ]);
     });

test/lib/sub-process.test.ts

@@ -3,31 +3,37 @@ import * as subProcess from '../../lib/sub-process';
 describe('execute()', () => {
   test('Resolves when the command succeeds', async () => {
     await expect(
-      subProcess.execute('echo "success" && echo "ignore" >&2 && true'),
+      subProcess.execute('sh', [
+        '-c',
+        'echo "success" && echo "ignore" >&2 && true',
+      ]),
     ).resolves.toEqual('success\n');
   });
 
   test('Resolves with stderr when the command succeeds and there is no stdout', async () => {
     await expect(
-      subProcess.execute('echo "warn" >&2 && true'),
+      subProcess.execute('sh', ['-c', 'echo "warn" >&2 && true']),
     ).resolves.toMatch('warn\n');
   });
 
   test('Rejects with stdout when the command fails', async () => {
     await expect(
-      subProcess.execute('echo "error msg" && echo "ignore" >&2 && false'),
+      subProcess.execute('sh', [
+        '-c',
+        'echo "error msg" && echo "ignore" >&2 && false',
+      ]),
     ).rejects.toThrow('error msg\n');
   });
 
   test('Rejects with stderr when the command fails and there is no stdout', async () => {
     await expect(
-      subProcess.execute('echo "error msg" >&2 && false'),
+      subProcess.execute('sh', ['-c', 'echo "error msg" >&2 && false']),
     ).rejects.toThrow('error msg\n');
   });
 
   test('Considers option.cwd', async () => {
     await expect(
-      subProcess.execute('basename $PWD', [], { cwd: __dirname }),
+      subProcess.execute('sh', ['-c', 'basename $PWD'], { cwd: __dirname }),
     ).resolves.toEqual('lib\n');
   });