fix: verify fails if node not installed (#167)

* fix: verify fails if node not installed @W-9930707@ * chore: code cleanup * chore: code cleanup * test: node in slim an global * chore: code clean + path for win * Update src/lib/npmCommand.ts Co-authored-by: Shane McLaughlin <[email protected]> * Update src/lib/npmCommand.ts Co-authored-by: Shane McLaughlin <[email protected]> * chore: streamline code & update some comments * fix: remmove unnecessary string template * test: run nuts from CI * test: can't use answers to prompts on windows * fix: make sure node filepath is executable * chore: update unit tests so error messages are more descriptive * test: run nuts on branches, partial nuts on windows (#168) * test: add telemetry ack file * test: ignore errors on deleting testProjects * test: store correct filename * test: can't write to non-existent dir * test: smaller test plugin, json in ack file * test: no nuts for windows Co-authored-by: Benjamin Maggi <[email protected]> Co-authored-by: Shane McLaughlin <[email protected]> Co-authored-by: Rodrigo Espinosa de los Monteros <[email protected]>
salesforcecli · Sep 24, 2021 · 0393b90 · 0393b90
1 parent cfc0dd4
commit 0393b90
Show file tree

Hide file tree

Showing 10 changed files with 367 additions and 45 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -60,6 +60,17 @@ workflows:
                 node_version: lts
               - os: windows
                 node_version: maintenance
+      - release-management/test-nut:
+          name: nuts-on-linux
+          sfdx_version: latest
+          requires:
+            - release-management/test-package
+      # - release-management/test-nut:
+      #     name: nuts-on-windows
+      #     sfdx_version: latest
+      #     os: windows
+      #     requires:
+      #       - release-management/test-package
       - release-management/release-package:
           sign: true
           github-release: true

diff --git a/package.json b/package.json
@@ -26,6 +26,7 @@
     "@salesforce/prettier-config": "^0.0.2",
     "@salesforce/ts-sinon": "1.3.21",
     "@types/shelljs": "^0.8.9",
+    "@types/sinon-chai": "^3.2.5",
     "@typescript-eslint/eslint-plugin": "^4.2.0",
     "@typescript-eslint/parser": "^4.2.0",
     "chai": "^4.2.0",
@@ -47,6 +48,7 @@
     "pretty-quick": "^3.1.0",
     "shx": "0.3.3",
     "sinon": "10.0.0",
+    "sinon-chai": "^3.7.0",
     "ts-node": "^10.0.0",
     "typescript": "^4.1.3"
   },
@@ -114,7 +116,7 @@
     "test": "sf-test",
     "test:command-reference": "./bin/run commandreference:generate --erroronwarnings",
     "test:deprecation-policy": "./bin/run snapshot:compare",
-    "test:nuts": "nyc mocha \"**/*.nut.ts\" --slow 4500 --timeout 600000 --parallel",
+    "test:nuts": "nyc mocha \"**/*.nut.ts\" --slow 4500 --timeout 600000",
     "version": "oclif-dev readme"
   },
   "husky": {

diff --git a/src/commands/plugins/trust/verify.ts b/src/commands/plugins/trust/verify.ts
@@ -47,9 +47,10 @@ export class Verify extends SfdxCommand {
     const vConfig = new VerificationConfig();
 
     const configContext: ConfigContext = {
-      cacheDir: get(this.config, 'configDir') as string,
-      configDir: get(this.config, 'cacheDir') as string,
+      cacheDir: get(this.config, 'cacheDir') as string,
+      configDir: get(this.config, 'configDir') as string,
       dataDir: get(this.config, 'dataDir') as string,
+      cliRoot: get(this.config, 'root') as string,
     };
 
     this.logger.debug(`cacheDir: ${configContext.cacheDir}`);

diff --git a/src/hooks/verifyInstallSignature.ts b/src/hooks/verifyInstallSignature.ts
@@ -58,6 +58,7 @@ export const hook: Hook.PluginsPreinstall = async function (options) {
       cacheDir: options.config.cacheDir,
       configDir: options.config.configDir,
       dataDir: options.config.dataDir,
+      cliRoot: options.config.root,
     };
 
     const vConfig = VerificationConfigBuilder.build(npmName, configContext);

diff --git a/src/lib/installationVerification.ts b/src/lib/installationVerification.ts
@@ -29,6 +29,7 @@ export interface ConfigContext {
   configDir?: string;
   cacheDir?: string;
   dataDir?: string;
+  cliRoot?: string;
 }
 export interface Verifier {
   verify(): Promise<NpmMeta>;
@@ -306,7 +307,9 @@ export class InstallationVerification implements Verifier {
     // Make sure the cache path exists.
     try {
       await fs.mkdirp(this.getCachePath());
-      new NpmModule(npmMeta.moduleName, npmMeta.version).pack(getNpmRegistry().href, { cwd: this.getCachePath() });
+      new NpmModule(npmMeta.moduleName, npmMeta.version, this.config.cliRoot).pack(getNpmRegistry().href, {
+        cwd: this.getCachePath(),
+      });
       const tarBallFile = fs
         .readdirSync(this.getCachePath(), { withFileTypes: true })
         .find((entry) => entry.isFile() && entry.name.includes(npmMeta.version));
@@ -345,7 +348,7 @@ export class InstallationVerification implements Verifier {
       ? `@${this.pluginNpmName.scope}/${this.pluginNpmName.name}`
       : this.pluginNpmName.name;
 
-    const npmModule = new NpmModule(npmShowModule, this.pluginNpmName.tag);
+    const npmModule = new NpmModule(npmShowModule, this.pluginNpmName.tag, this.config.cliRoot);
     const npmMetadata = npmModule.show(npmRegistry.href);
     logger.debug('retrieveNpmMeta | Found npm meta information.');
     if (!npmMetadata.versions) {

diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts
@@ -6,9 +6,12 @@
  */
 /* eslint-disable @typescript-eslint/no-unused-vars */
 
+import { type as osType } from 'os';
 import * as path from 'path';
-import * as shelljs from 'shelljs';
+
 import npmRunPath from 'npm-run-path';
+import * as shelljs from 'shelljs';
+
 import { SfdxError, fs } from '@salesforce/core';
 
 export type NpmMeta = {
@@ -39,6 +42,7 @@ export type NpmShowResults = {
 type NpmCommandOptions = shelljs.ExecOptions & {
   json?: boolean;
   registry?: string;
+  cliRoot?: string;
 };
 
 type NpmCommandResult = NpmShowResults & {
@@ -55,9 +59,10 @@ export class NpmCommand {
   private static npmPkgPath = require.resolve('npm/package.json');
 
   public static runNpmCmd(cmd: string, options = {} as NpmCommandOptions): NpmCommandResult {
+    const nodeExecutable = NpmCommand.findNode(options.cliRoot);
     const npmCli = NpmCommand.npmCli();
-    const exec = `${npmCli} ${cmd} --registry=${options.registry} --json`;
-    const npmShowResult = shelljs.exec(exec, {
+    const command = `"${nodeExecutable}" "${npmCli}" ${cmd} --registry=${options.registry} --json`;
+    const npmShowResult = shelljs.exec(command, {
       ...options,
       silent: true,
       fatal: true,
@@ -78,28 +83,88 @@ export class NpmCommand {
     return this.npmPkgPath;
   }
 
+  /**
+   * Returns the path to the npm-cli.js file in this package's node_modules
+   *
+   * @private
+   */
   private static npmCli(): string {
     const pkgPath = NpmCommand.npmPackagePath();
     const pkgJson = fs.readJsonSync(pkgPath) as NpmPackage;
     const prjPath = pkgPath.substring(0, pkgPath.lastIndexOf(path.sep));
     return path.join(prjPath, pkgJson.bin['npm']);
   }
+
+  /**
+   * Locate node executable and return its absolute path
+   * First it tries to locate the node executable on the root path passed in
+   * If not found then tries to use whatver 'node' resolves to on the user's PATH
+   * If found return absolute path to the executable
+   * If the node executable cannot be found, an error is thrown
+   *
+   * @private
+   */
+  private static findNode(root: string = undefined): string {
+    const isExecutable = (filepath: string): boolean => {
+      if (osType() === 'Windows_NT') return filepath.endsWith('node.exe');
+
+      try {
+        if (filepath.endsWith('node')) {
+          // This checks if the filepath is executable on Mac or Linux, if it is not it errors.
+          fs.accessSync(filepath, fs.constants.X_OK);
+          return true;
+        }
+      } catch {
+        return false;
+      }
+      return false;
+    };
+
+    if (root) {
+      const sfdxBinDirs = NpmCommand.findSfdxBinDirs(root);
+      if (sfdxBinDirs.length > 0) {
+        // Find the node executable
+        const node = shelljs.find(sfdxBinDirs).filter((file) => isExecutable(file))[0];
+        if (node) {
+          return fs.realpathSync(node);
+        }
+      }
+    }
+
+    // Check to see if node is installed
+    const nodeShellString: shelljs.ShellString = shelljs.which('node');
+    if (nodeShellString?.code === 0 && nodeShellString?.stdout) return nodeShellString.stdout;
+
+    throw new SfdxError('Cannot locate node executable.', 'CannotFindNodeExecutable');
+  }
+
+  /**
+   * Finds the bin directory in the sfdx installation root path
+   *
+   * @param sfdxPath
+   * @private
+   */
+  private static findSfdxBinDirs(sfdxPath: string): string[] {
+    return sfdxPath
+      ? [path.join(sfdxPath, 'bin'), path.join(sfdxPath, 'client', 'bin')].filter((p) => fs.existsSync(p))
+      : [];
+  }
 }
 
 export class NpmModule {
   public npmMeta: NpmMeta;
-  public constructor(private module: string, private version: string = 'latest') {
+  public constructor(private module: string, private version: string = 'latest', private cliRoot: string = undefined) {
     this.npmMeta = {
       moduleName: module,
     };
   }
 
   public show(registry: string): NpmShowResults {
-    return NpmCommand.runNpmCmd(`show ${this.module}@${this.version}`, { registry });
+    return NpmCommand.runNpmCmd(`show ${this.module}@${this.version}`, { registry, cliRoot: this.cliRoot });
   }
 
   public pack(registry: string, options?: shelljs.ExecOptions): void {
-    NpmCommand.runNpmCmd(`pack ${this.module}@${this.version}`, { ...options, registry });
+    NpmCommand.runNpmCmd(`pack ${this.module}@${this.version}`, { ...options, registry, cliRoot: this.cliRoot });
     return;
   }
 }
diff --git a/test/lib/installationVerification.test.ts b/test/lib/installationVerification.test.ts
@@ -68,6 +68,18 @@ const getShelljsExecStub = (
         stderr,
         stdout: JSON.stringify(PACK_RESULT),
       };
+    } else if (cmd.includes('node')) {
+      return {
+        code: 0,
+        stderr,
+        stdout: 'node',
+      };
+    } else if (cmd.includes('sfdx')) {
+      return {
+        code: 0,
+        stderr,
+        stdout: 'sfdx',
+      };
     } else {
       throw new Error(`Unexpected test cmd - ${cmd}`);
     }
@@ -105,12 +117,17 @@ describe('InstallationVerification Tests', () => {
     get configDir() {
       return 'configDir';
     },
+    get cliRoot() {
+      return __dirname;
+    },
   };
   const currentRegistry = process.env.SFDX_NPM_REGISTRY;
+  let fsReaddirSyncStub: Sinon.SinonStub;
   let plugin: NpmName;
+  let realpathSyncStub: Sinon.SinonStub;
   let sandbox: sinon.SinonSandbox;
   let shelljsExecStub: Sinon.SinonStub;
-  let fsReaddirSyncStub: Sinon.SinonStub;
+  let shelljsFindStub: Sinon.SinonStub;
 
   beforeEach(() => {
     sandbox = Sinon.createSandbox();
@@ -122,11 +139,15 @@ describe('InstallationVerification Tests', () => {
         },
       },
     ]);
+    realpathSyncStub = stubMethod(sandbox, fs, 'realpathSync').returns('node.exe');
+    shelljsFindStub = stubMethod(sandbox, shelljs, 'find').returns(['node.exe']);
     plugin = NpmName.parse('foo');
   });
 
   afterEach(() => {
     fsReaddirSyncStub.restore();
+    realpathSyncStub.restore();
+    shelljsFindStub.restore();
     if (shelljsExecStub) {
       shelljsExecStub.restore();
     }