secrets + cleanup

Avoid using `shell: true` as I had bad experiences with it on Windows.

Instead sanitize arguments to hide secrets when prompting.

Author: paul-marechal

scripts/check_3pp_licenses.js

@@ -18,37 +18,37 @@
 const cp = require('child_process');
 const fs = require('fs');
 const path = require('path');
-const { env, argv } = require('process');
 const readline = require('readline');
+const kSECRET = Symbol('secret');
+
 // Submit any suspicious dependencies for review by the Eclipse Foundation, using dash-license "review" mode?
-const autoReviewMode = (process.argv.slice(2))[0] == "--review" ? true:false
+const autoReviewMode = process.argv.includes('--review');
+const project = process.argv.find(arg => /--project=(\S+)/.exec(arg)?.[1]) ?? 'ecd.theia';
 
 const NO_COLOR = Boolean(process.env['NO_COLOR']);
 const dashLicensesJar = path.resolve(__dirname, 'download/dash-licenses.jar');
 const dashLicensesSummary = path.resolve(__dirname, '../dependency-check-summary.txt');
 const dashLicensesBaseline = path.resolve(__dirname, '../dependency-check-baseline.json');
 const dashLicensesUrl = 'https://repo.eclipse.org/service/local/artifact/maven/redirect?r=dash-licenses&g=org.eclipse.dash&a=org.eclipse.dash.licenses&v=LATEST';
-const project = "ecd.theia";
 
 // A Eclipse Foundation Gitlab Personal Access Token, generated by an Eclipse committer,
-// is required to use dash-licenses in "review" mode. For more information see: 
+// is required to use dash-licenses in "review" mode. For more information see:
 //  https://github.com/eclipse/dash-licenses#automatic-ip-team-review-requests
-// e.g. Set the token like so (bash shell): 
+// e.g. Set the token like so (bash shell):
 // $> export DASH_LICENSES_PAT="<PAT>"
-const gitlabTokenDefined = env.DASH_LICENSES_PAT ? true : false;
-
-if (autoReviewMode && !gitlabTokenDefined) {
-    console.error("Please setup an Eclipse Foundation Gitlab Personal Access Token to run the license check in 'review' mode");
-    console.error("It should be set in an environment variable named 'DASH_LICENSES_PAT'");
-    process.exit(1);
-}
+const personalAccessToken = secret(process.env.DASH_LICENSES_PAT);
 
 main().catch(error => {
     console.error(error);
     process.exit(1);
 });
 
 async function main() {
+    if (autoReviewMode && !personalAccessToken) {
+        error('Please setup an Eclipse Foundation Gitlab Personal Access Token to run the license check in "review" mode');
+        error('It should be set in an environment variable named "DASH_LICENSES_PAT"');
+        process.exit(1);
+    }
     if (!fs.existsSync(dashLicensesJar)) {
         info('Fetching dash-licenses...');
         fs.mkdirSync(path.dirname(dashLicensesJar), { recursive: true });
@@ -65,20 +65,14 @@ async function main() {
         fs.renameSync(dashLicensesSummary, `${dashLicensesSummary}.old`);
     }
     info('Running dash-licenses...');
-    var args = ['-jar', dashLicensesJar, 'yarn.lock', '-batch', '50', '-timeout', '240', '-summary', dashLicensesSummary]
-    if (autoReviewMode && gitlabTokenDefined) {
-        info('using "review" mode');
-        args.push('-review', '-token', '$DASH_LICENSES_PAT', '-project', project);
+    const args = ['-jar', dashLicensesJar, 'yarn.lock', '-batch', '50', '-timeout', '240', '-summary', dashLicensesSummary];
+    if (autoReviewMode && personalAccessToken) {
+        info(`Using "review" mode for project: ${project}`);
+        args.push('-review', '-token', personalAccessToken, '-project', project);
     }
-
-    // note: "shell:true" is required so we can reference the 
-    // Gitlab Personal Access Token through an environment variable
-    // at invocation of "dash-license". This is necessary to avoid 
-    // leaking the token's value
-    const dashError = getErrorFromStatus(spawn(
-        'java', args,
-        { stdio: ['ignore', 'ignore', 'inherit'], shell: true }
-    ));
+    const dashError = getErrorFromStatus(spawn('java', args, {
+        stdio: ['ignore', 'ignore', 'inherit']
+    }));
     if (dashError) {
         warn(dashError);
     }
@@ -172,25 +166,62 @@ function readBaseline(baseline) {
     process.exit(1);
 }
 
+/**
+ * @param {any} value
+ * @returns {object | undefined}
+ */
+function secret(value) {
+    if (value) {
+        return { [kSECRET]: value };
+    }
+}
+
+/**
+ * @param {(string | object)[]} array
+ * @returns {string[]}
+ */
+function withSecrets(array) {
+    return array.map(element => element[kSECRET] ?? element);
+}
+
+/**
+ * @param {(string | object)[]} array
+ * @returns {string[]}
+ */
+function withoutSecrets(array) {
+    return array.map(element => element[kSECRET] ? '***' : element);
+}
+
 /**
  * Spawn a process. Exits with code 1 on spawn error (e.g. file not found).
  * @param {string} bin
- * @param {string[]} args
+ * @param {(string | object)[]} args
  * @param {import('child_process').SpawnSyncOptions} [opts]
  * @returns {import('child_process').SpawnSyncReturns}
  */
 function spawn(bin, args, opts = {}) {
     opts = { stdio: 'inherit', ...opts };
+    function abort(spawnError, spawnBin, spawnArgs) {
+        if (spawnBin && spawnArgs) {
+            error(`Command: ${prettyCommand({ bin: spawnBin, args: spawnArgs })}`);
+        }
+        error(spawnError.stack ?? spawnError.message);
+        process.exit(1);
+    }
     /** @type {any} */
-    const status = cp.spawnSync(bin, args, opts);
+    let status;
+    try {
+        status = cp.spawnSync(bin, withSecrets(args), opts);
+    } catch (spawnError) {
+        abort(spawnError, bin, withoutSecrets(args));
+    }
     // Add useful fields to the returned status object:
     status.bin = bin;
-    status.args = args;
+    status.args = withoutSecrets(args);
     status.opts = opts;
     // Abort on spawn error:
     if (status.error) {
-        console.error(status.error);
-        process.exit(1);
+        abort(status.error, status.bin, status.args);
     }
     return status;
 }