Merge pull request #2 from Strum355/nsc/npm-execsync

chore: use invokeCommand instead of execSync

Author: ruromero

package.json

@@ -104,5 +104,6 @@
 	},
 	"eslintIgnore": [
 		"index.js"
-	]
+	],
+	"packageManager": "[email protected]+sha512.0e82714d1b5b43c74610193cb20734897c1d00de89d0e18420aebc5977fa13d780a9cb05734624e81ebd81cc876cd464794850641c48b9544326b5622ca29971"
 }

src/providers/base_java.js

@@ -109,8 +109,7 @@ export default class Base_Java {
 	/** this method invokes command string in a process in a synchronous way.
 	 * @param bin - the command to be invoked
 	 * @param args - the args to pass to the binary
-	 * @param callback - function to invoke if an error was thrown
 	 * @protected
 	 */
-	_invokeCommand(bin, args, callback, opts={}) { return invokeCommand(bin, args, callback, opts) }
+	_invokeCommand(bin, args, opts={}) { return invokeCommand(bin, args, opts) }
 }

src/providers/base_javascript.js

@@ -1,7 +1,6 @@
-import { execSync } from "node:child_process"
 import fs from 'node:fs'
 import os from "node:os";
-import { handleSpacesInPath } from "../tools.js";
+import { handleSpacesInPath, invokeCommand } from "../tools.js";
 import path from 'node:path'
 import Sbom from '../sbom.js'
 import { PackageURL } from 'packageurl-js'
@@ -29,10 +28,16 @@ export default class Base_javascript {
 		throw new TypeError("_cmdName must be implemented");
 	}
 
+	/**
+	 * @returns {Array<string>}
+	 */
 	_listCmdArgs() {
 		throw new TypeError("_listCmdArgs must be implemented");
 	}
 
+	/**
+	 * @returns {Array<string>}
+	 */
 	_updateLockFileCmdArgs() {
 		throw new TypeError("_updateLockFileCmdArgs must be implemented");
 	}
@@ -97,8 +102,7 @@ export default class Base_javascript {
 		if (parts.length === 2) {
 			purlNs = parts[0];
 			purlName = parts[1];
-		}
-		else {
+		} else {
 			purlName = parts[0];
 		}
 		return new PackageURL('npm', purlNs, purlName, version, undefined, undefined);
@@ -154,8 +158,7 @@ export default class Base_javascript {
 		Object.entries(dependencies)
 			.filter(entry => entry[1].version !== undefined)
 			.forEach(entry => {
-				let name, artifact;
-				[name, artifact] = entry;
+				let [name, artifact] = entry;
 				let purl = this.#toPurl(name, artifact.version);
 				sbom.addDependency(from, purl)
 				let transitiveDeps = artifact.dependencies
@@ -168,22 +171,20 @@ export default class Base_javascript {
 	#executeListCmd(includeTransitive, manifestDir) {
 		const listArgs = this._listCmdArgs(includeTransitive, manifestDir);
 		try {
-			return execSync(listArgs, {
-				encoding: 'utf-8'
-			});
-		} catch (err) {
-			throw new Error(`failed to execute command ${listArgs} - Error: ${err}`);
+			invokeCommand(this._cmdName(), listArgs)
+		} catch (error) {
+			throw new Error(`failed to list dependencies via "${this._cmdName()} ${listArgs.join(' ')}" - Error: ${error}`, {cause: error});
 		}
 	}
 
 	#version() {
 		try {
-			execSync(`${handleSpacesInPath(this._cmdName())} --version`, {
-				stdio: 'ignore',
-				encoding: "utf8",
-			});
-		} catch (err) {
-			throw new Error(`${this._cmdName()} is not accessible: ${err}`);
+			invokeCommand(this._cmdName(), ['--version'], {stdio: 'ignore'});
+		} catch (error) {
+			if (error.code === 'ENOENT') {
+				throw new Error(`${this._cmdName()} is not accessible`);
+			}
+			throw new Error(`failed to check for package manager binary at ${this._cmdName()}`, {cause: error})
 		}
 	}
 
@@ -194,21 +195,17 @@ export default class Base_javascript {
 		if (os.platform() === 'win32') {
 			process.chdir(manifestDir)
 		}
-		const args = this._updateLockFileCmdArgs(manifestDir);
 
 		try {
-			return execSync(args, {
-				encoding: 'utf-8'
-			});
-		} catch (err) {
-			throw new Error(`failed to execute command ${args} - Error: ${err}`);
+			const args = this._updateLockFileCmdArgs(manifestDir);
+			invokeCommand(this._cmdName(), args)
+		} catch (error) {
+			throw new Error(`failed to create lockfile "${args}" - Error: ${error}`, {cause: error});
 		} finally {
 			if (os.platform() === 'win32') {
 				process.chdir(originalDir)
 			}
 		}
-
-
 	}
 }
 

src/providers/java_gradle.js

@@ -193,13 +193,12 @@ export default class Java_gradle extends Base_java {
 	 */
 	#getProperties(manifestPath, opts) {
 		let gradle = getCustomPath("gradle", opts);
-		let properties
-		properties = this._invokeCommand(gradle, ['properties'], error => {
+		try {
+			let properties = this._invokeCommand(gradle, ['properties'], {cwd: path.dirname(manifestPath)})
+			return properties.toString()
+		} catch (error) {
 			throw new Error(`Couldn't get properties of ${this._getManifestName()} file , Error message returned from gradle binary => ${EOL} ${error.message}`)
-		}, {
-			cwd: path.dirname(manifestPath)
-		})
-		return properties.toString()
+		}
 	}
 
 	/**
@@ -237,12 +236,12 @@ export default class Java_gradle extends Base_java {
 
 	#getDependencies(manifest) {
 		const gradle = getCustomPath("gradle")
-		const commandResult = this._invokeCommand(gradle, ['dependencies'], error => {
+		try {
+			const commandResult = this._invokeCommand(gradle, ['dependencies'], {cwd: path.dirname(manifest)})
+			return commandResult.toString()
+		} catch (error) {
 			throw new Error(`Couldn't run gradle dependencies command, error message returned from gradle binary => ${EOL} ${error.message}`)
-		}, {
-			cwd: path.dirname(manifest)
-		})
-		return commandResult.toString()
+		}
 	}
 
 	/**

src/providers/java_maven.js

@@ -74,9 +74,11 @@ export default class Java_maven extends Base_java {
 		const mvn = this.#selectMvnRuntime(manifest, opts)
 
 		// clean maven target
-		this._invokeCommand(mvn, ['-q', 'clean', '-f', manifest], error => {
+		try {
+			this._invokeCommand(mvn, ['-q', 'clean', '-f', manifest])
+		} catch (error) {
 			throw new Error(`failed to clean maven target`, {cause: error})
-		})
+		}
 
 		// create dependency graph in a temp file
 		let tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'exhort_'))
@@ -93,9 +95,11 @@ export default class Java_maven extends Base_java {
 			}
 		})
 		// execute dependency tree command
-		this._invokeCommand(mvn, depTreeCmdArgs, error => {
+		try {
+			this._invokeCommand(mvn, depTreeCmdArgs)
+		} catch (error) {
 			throw new Error(`failed creating maven dependency tree`, {cause: error})
-		})
+		}
 		// read dependency tree from temp file
 		let content = fs.readFileSync(`${tmpDepTree}`)
 		if (process.env["EXHORT_DEBUG"] === "true") {
@@ -138,9 +142,11 @@ export default class Java_maven extends Base_java {
 		const targetPom = manifestPath
 
 		// create effective pom and save to temp file
-		this._invokeCommand(mvn, ['-q', 'help:effective-pom', `-Doutput=${tmpEffectivePom}`, '-f', targetPom], error => {
+		try {
+			this._invokeCommand(mvn, ['-q', 'help:effective-pom', `-Doutput=${tmpEffectivePom}`, '-f', targetPom])
+		} catch (error) {
 			throw new Error(`failed creating maven effective pom`, {cause: error})
-		})
+		}
 		// iterate over all dependencies in original pom and collect all ignored ones
 		let ignored = this.#getDependencies(targetPom).filter(d => d.ignore)
 		// iterate over all dependencies and create a package for every non-ignored one
@@ -205,24 +211,28 @@ export default class Java_maven extends Base_java {
 		if (useMvnw) {
 			const mvnw = this.#traverseForMvnw(manifestPath)
 			if (mvnw !== undefined) {
-				this._invokeCommand(mvnw, ['--version'], error => {
+				try {
+					this._invokeCommand(mvnw, ['--version'])
+				} catch (error) {
 					if (error.code === 'ENOENT') {
 						useMvnw = false
 					} else {
 						throw new Error(`failed to check for mvnw`, {cause: error})
 					}
-				})
+				}
 				mvn = useMvnw ? mvnw : mvn
 			}
 		} else {
 			// verify maven is accessible
-			this._invokeCommand(mvn, ['--version'], error => {
+			try {
+				this._invokeCommand(mvn, ['--version'])
+			} catch (error) {
 				if (error.code === 'ENOENT') {
 					throw new Error(`maven not accessible at "${mvn}"`)
 				} else {
 					throw new Error(`failed to check for maven`, {cause: error})
 				}
-			})
+			}
 		}
 		return mvn
 	}

src/providers/javascript_npm.js

@@ -11,15 +11,18 @@ export default class Javascript_npm extends Base_javascript {
 	}
 
 	_listCmdArgs(includeTransitive, manifestDir) {
-		const depthArg = includeTransitive ? "--all" : "--depth=0";
-		const manifestArg = manifestDir ? `--prefix ${manifestDir}` : "";
-
-		return `${this._cmdName()} ls ${depthArg} --package-lock-only --omit=dev --json ${manifestArg}`;
+		const args = ['ls', includeTransitive ? '--all' : '--depth=0', '--package-lock-only', '--omit=dev', '--json']
+		if (manifestDir) {
+			args.push('--prefix', manifestDir)
+		}
+		return args
 	}
 
 	_updateLockFileCmdArgs(manifestDir) {
-		const manifestArg = manifestDir ? `--dir ${manifestDir}` : "";
-		return `${this._cmdName()} install --package-lock-only ${manifestArg}`;
+		const args = ['install', '--package-lock-only']
+		if (manifestDir) {
+			args.push('--dir', manifestDir)
+		}
+		return args;
 	}
-
 }

src/providers/javascript_pnpm.js

@@ -11,14 +11,20 @@ export default class Javascript_pnpm extends Base_javascript {
 	}
 
 	_listCmdArgs(includeTransitive, manifestDir) {
-		const depthArg = includeTransitive ? "--depth=Infinity" : "--depth=0";
-		const manifestArg = manifestDir ? `--dir ${manifestDir}` : "";
-		return `${this._cmdName()} ls ${depthArg} ${manifestArg} --prod --json`;
+		const args = ['ls', includeTransitive ? '--all' : '--depth=0', '--prod', '--json'];
+		if (manifestDir) {
+			args.push('--prefix', manifestDir);
+		}
+		return args;
 	}
 
 	_updateLockFileCmdArgs(manifestDir) {
-		const manifestArg = manifestDir ? `--dir ${manifestDir}` : "";
-		return `${this._cmdName()} install --frozen-lockfile ${manifestArg}`;
+		const args = ['install', '--frozen-lockfile'];
+		if (manifestDir) {
+			args.push('--prefix', manifestDir)
+		}
+		args.push(...[])
+		return args;
 	}
 
 	_buildDependencyTree(includeTransitive, manifest) {

src/tools.js

@@ -106,42 +106,31 @@ function hasSpaces(path) {
 	return path.trim().includes(" ")
 }
 
-
 /**
  *
  * @param {string} cwd - directory for which to find the root of the git repository.
  */
 export function getGitRootDir(cwd) {
-	const root = invokeCommand('git', ['rev-parse', '--show-toplevel'], () => {}, {cwd: cwd})
-	if (!root) {
+	try {
+		const root = invokeCommand('git', ['rev-parse', '--show-toplevel'], {cwd: cwd})
+		return root.toString().trim()
+	} catch (error) {
 		return undefined
 	}
-	return root.toString().trim()
 }
 
 /** this method invokes command string in a process in a synchronous way.
  * @param {string} bin - the command to be invoked
  * @param {Array<string>} args - the args to pass to the binary
- * @param callback - function to invoke if an error was thrown
- * @protected
  */
-export function invokeCommand(bin, args, callback, opts={}) {
+export function invokeCommand(bin, args, opts={}) {
 	// .bat and .cmd files can't be executed in windows with execFileSync, so we special case them
 	// to use execSync here to keep the amount of escaping we need to do to a minimum.
 	// https://nodejs.org/docs/latest-v20.x/api/child_process.html#spawning-bat-and-cmd-files-on-windows
 	if (process.platform === 'win32' && (bin.endsWith(".bat") || bin.endsWith(".cmd"))) {
-		try {
-			args = args.map(arg => handleSpacesInPath(arg))
-			return execSync(`${handleSpacesInPath(bin)} ${args.join(" ")}`, {...{stdio: 'pipe'}, ...opts})
-		} catch(error) {
-			callback(error)
-		}
-		return
+		args = args.map(arg => handleSpacesInPath(arg))
+		return execSync(`${handleSpacesInPath(bin)} ${args.join(" ")}`, {...{stdio: 'pipe', encoding: 'utf-8'}, ...opts})
 	}
 
-	try {
-		return execFileSync(bin, args, {...{stdio: 'pipe'}, ...opts})
-	} catch(error) {
-		callback(error)
-	}
+	return execFileSync(bin, args, {...{stdio: 'pipe', encoding: 'utf-8'}, ...opts})
 }