Merge pull request #240 from snyk/fix/escape-child-process-arguments

fix: escape child process arguments
snyk · Nov 14, 2022 · 46fb3e3 · 46fb3e3
2 parents f115e10 + bb1c1c7
commit 46fb3e3
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 41 deletions.
diff --git a/.snyk b/.snyk
@@ -0,0 +1,10 @@
+# Snyk (https://snyk.io) policy file, patches or ignores known vulnerabilities.
+version: v1.25.0
+# ignores vulnerabilities until expiry date; change duration by modifying expiry date
+ignore:
+  'snyk:lic:npm:shescape:MPL-2.0':
+    - '*':
+        reason: Snyk CLI handles MPL-2.0 by appending dependency to snyk --about
+        expires: 2122-12-14T13:33:15.042Z
+        created: 2022-11-14T13:33:15.045Z
+patch: {}
diff --git a/lib/index.ts b/lib/index.ts
@@ -37,9 +37,6 @@ export function debugLog(s: string) {
   logger(s);
 }
 
-const isWin = /^win/.test(os.platform());
-const quot = isWin ? '"' : "'";
-
 const cannotResolveVariantMarkers = [
   'Cannot choose between the following',
   'Could not select value from candidates',
@@ -115,6 +112,7 @@ export async function inspect(
   let subProject = (options as api.SingleSubprojectInspectOptions).subProject;
   if (subProject) {
     subProject = subProject.trim();
+    (options as api.SingleSubprojectInspectOptions).subProject = subProject;
   }
   const plugin: api.PluginMetadata = {
     name: 'bundled:gradle',
@@ -671,13 +669,6 @@ function getCommand(root: string, targetFile: string) {
   return 'gradle';
 }
 
-export function formatArgWithWhiteSpace(arg: string): string {
-  if (/\s/.test(arg)) {
-    return quot + arg + quot;
-  }
-  return arg;
-}
-
 function buildArgs(
   root: string,
   targetFile: string | null,
@@ -692,31 +683,25 @@ function buildArgs(
   args.push(taskName, '-q');
 
   if (targetFile) {
-    if (!fs.existsSync(path.resolve(root, targetFile))) {
-      throw new Error('File not found: "' + targetFile + '"');
+    const resolvedTargetFilePath = path.resolve(root, targetFile);
+    if (!fs.existsSync(resolvedTargetFilePath)) {
+      throw new Error('File not found: "' + resolvedTargetFilePath + '"');
     }
     args.push('--build-file');
 
-    const formattedTargetFile = formatArgWithWhiteSpace(targetFile);
-    args.push(formattedTargetFile);
+    args.push(resolvedTargetFilePath);
   }
 
   // Arguments to init script are supplied as properties: https://stackoverflow.com/a/48370451
   if (options['configuration-matching']) {
-    args.push(
-      `-Pconfiguration=${quot}${options['configuration-matching']}${quot}`,
-    );
+    args.push(`-Pconfiguration=${options['configuration-matching']}`);
   }
   if (options['configuration-attributes']) {
-    args.push(
-      `-PconfAttr=${quot}${options['configuration-attributes']}${quot}`,
-    );
+    args.push(`-PconfAttr=${options['configuration-attributes']}`);
   }
 
   if (options.initScript) {
-    const formattedInitScript = formatArgWithWhiteSpace(
-      path.resolve(options.initScript),
-    );
+    const formattedInitScript = path.resolve(options.initScript);
     args.push('--init-script', formattedInitScript);
   }
 
@@ -738,7 +723,7 @@ function buildArgs(
     args.push('-PonlySubProject=' + (options.subProject || '.'));
   }
 
-  args.push('-I ' + initGradlePath);
+  args.push('-I', initGradlePath);
 
   if (options.args) {
     args.push(...options.args);
@@ -751,16 +736,16 @@ function buildArgs(
     // Transform --configuration=foo
     args[i] = a.replace(
       /^--configuration[= ]([a-zA-Z_]+)/,
-      `-Pconfiguration=${quot}^$1$$${quot}`,
+      `-Pconfiguration=^$1$$`,
     );
     // Transform --configuration foo
     if (a === '--configuration') {
-      args[i] = `-Pconfiguration=${quot}^${args[i + 1]}$${quot}`;
+      args[i] = `-Pconfiguration=^${args[i + 1]}$`;
       args[i + 1] = '';
     }
   });
 
-  return args;
+  return args.filter(Boolean);
 }
 
 export const exportsForTests = {

diff --git a/lib/sub-process.ts b/lib/sub-process.ts
@@ -1,5 +1,6 @@
 import * as childProcess from 'child_process';
 import debugModule = require('debug');
+import { quoteAll } from 'shescape';
 
 const debugLogging = debugModule('snyk-gradle-plugin');
 
@@ -14,6 +15,7 @@ export function execute(
   if (options && options.cwd) {
     spawnOptions.cwd = options.cwd;
   }
+  args = quoteAll(args, spawnOptions);
 
   return new Promise((resolve, reject) => {
     let stdout = '';

diff --git a/package.json b/package.json
@@ -51,6 +51,7 @@
     "debug": "^4.1.1",
     "p-map": "^4.0.0",
     "packageurl-js": "^1.0.0",
+    "shescape": "1.6.1",
     "tmp": "0.2.1",
     "tslib": "^2.0.0"
   },

diff --git a/test/functional/gradle-plugin.spec.ts b/test/functional/gradle-plugin.spec.ts
@@ -1,8 +1,4 @@
 import { exportsForTests as testableMethods } from '../../lib';
-import * as os from 'os';
-
-const isWin = /^win/.test(os.platform());
-const quot = isWin ? '"' : "'";
 
 const JEST_TIMEOUT = 15000;
 
@@ -16,7 +12,8 @@ describe('Gradle Plugin', () => {
       '-Dorg.gradle.parallel=',
       '-Dorg.gradle.console=plain',
       '-PonlySubProject=.',
-      '-I /tmp/init.gradle',
+      '-I',
+      '/tmp/init.gradle',
     ]);
   });
 
@@ -28,12 +25,13 @@ describe('Gradle Plugin', () => {
     expect(result).toEqual([
       'snykResolvedDepsJson',
       '-q',
-      `-Pconfiguration=${quot}confRegex${quot}`,
+      `-Pconfiguration=confRegex`,
       '--no-daemon',
       '-Dorg.gradle.parallel=',
       '-Dorg.gradle.console=plain',
       '-PonlySubProject=.',
-      '-I /tmp/init.gradle',
+      '-I',
+      '/tmp/init.gradle',
       '--build-file',
       'build.gradle',
     ]);
@@ -48,11 +46,12 @@ describe('Gradle Plugin', () => {
     expect(result).toEqual([
       'snykResolvedDepsJson',
       '-q',
-      `-Pconfiguration=${quot}confRegex${quot}`,
+      `-Pconfiguration=confRegex`,
       '-Dorg.gradle.parallel=',
       '-Dorg.gradle.console=plain',
       '-PonlySubProject=.',
-      '-I /tmp/init.gradle',
+      '-I',
+      '/tmp/init.gradle',
       '--build-file',
       'build.gradle',
     ]);
@@ -69,10 +68,11 @@ describe('Gradle Plugin', () => {
       '-Dorg.gradle.parallel=',
       '-Dorg.gradle.console=plain',
       '-PonlySubProject=.',
-      '-I /tmp/init.gradle',
+      '-I',
+      '/tmp/init.gradle',
       '--build-file',
       'build.gradle',
-      `-Pconfiguration=${quot}^compile$${quot}`,
+      `-Pconfiguration=^compile$`,
     ]);
   });
 
@@ -89,11 +89,11 @@ describe('Gradle Plugin', () => {
         '--no-daemon',
         '-Dorg.gradle.parallel=',
         '-Dorg.gradle.console=plain',
-        '-I /tmp/init.gradle',
+        '-I',
+        '/tmp/init.gradle',
         '--build-file',
         'build.gradle',
-        `-Pconfiguration=${quot}^compile$${quot}`,
-        '', // this is a harmless artifact of argument transformation
+        `-Pconfiguration=^compile$`,
       ]);
     },
     JEST_TIMEOUT,