W-23814642- resolve npm from PATH only, excluding CWD

package.json

@@ -18,7 +18,8 @@
     "marked-terminal": "^4.2.0",
     "open": "^10.2.0",
     "proxy-agent": "^6.5.0",
-    "semver": "^7.8.1"
+    "semver": "^7.8.1",
+    "which": "^4"
   },
   "devDependencies": {
     "@oclif/plugin-command-snapshot": "^5.3.34",
@@ -30,6 +31,7 @@
     "@types/marked": "^4.0.8",
     "@types/marked-terminal": "^3.1.3",
     "@types/semver": "^7.8.0",
+    "@types/which": "^3",
     "@types/sinon-chai": "^3.2.12",
     "eslint-plugin-sf-plugin": "^1.20.33",
     "oclif": "^4.23.29",

src/diagnostics.ts

@@ -15,10 +15,12 @@
  */
 
 import childProcess from 'node:child_process';
+import { dirname, resolve } from 'node:path';
 import { got } from 'got';
 import { Interfaces } from '@oclif/core';
 import { Lifecycle, Messages } from '@salesforce/core';
 import { Connection } from '@jsforce/jsforce-node';
+import which from 'which';
 import { SfDoctor, SfDoctorDiagnosis } from './doctor.js';
 
 export type DiagnosticStatus = {
@@ -69,29 +71,44 @@ export class Diagnostics {
     const cliName = this.config.name;
     const cliVersion = this.config.version;
 
-    return new Promise<void>((resolve) => {
+    return new Promise<void>((resolve_) => {
       const testName = 'using latest or latest-rc CLI version';
       let status: DiagnosticStatus['status'] = 'unknown';
 
-      // Use execFile instead of exec to avoid shell interpretation.
-      // exec invokes cmd.exe on Windows, which resolves commands from CWD before PATH.
-      childProcess.execFile('npm', ['view', cliName, 'dist-tags.latest'], (error, stdout, stderr) => {
-        const code = error?.code ?? 0;
-        if (code === 0) {
-          const latest = stdout.trim();
-          if (cliVersion < latest) {
-            status = 'fail';
-            this.doctor.addSuggestion(messages.getMessage('updateCliVersion', [cliVersion, latest]));
+      // Resolve npm from PATH only, excluding CWD. On Windows, both execFile (CreateProcess)
+      // and the `which` module resolve executables from CWD before PATH.
+      const cwd = resolve(process.cwd());
+      const npmPath = which.sync('npm', { nothrow: true });
+      if (!npmPath || dirname(resolve(npmPath)) === cwd) {
+        void Lifecycle.getInstance()
+          .emit('Doctor:diagnostic', { testName, status })
+          .then(() => resolve_());
+        return;
+      }
+
+      const useShell = /\.(cmd|bat)$/i.test(npmPath);
+      childProcess.execFile(
+        npmPath,
+        ['view', cliName, 'dist-tags.latest'],
+        { shell: useShell },
+        (error, stdout, stderr) => {
+          const code = error?.code ?? 0;
+          if (code === 0) {
+            const latest = stdout.trim();
+            if (cliVersion < latest) {
+              status = 'fail';
+              this.doctor.addSuggestion(messages.getMessage('updateCliVersion', [cliVersion, latest]));
+            } else {
+              status = 'pass';
+            }
           } else {
-            status = 'pass';
+            this.doctor.addSuggestion(messages.getMessage('latestCliVersionError', [stderr]));
           }
-        } else {
-          this.doctor.addSuggestion(messages.getMessage('latestCliVersionError', [stderr]));
+          void Lifecycle.getInstance()
+            .emit('Doctor:diagnostic', { testName, status })
+            .then(() => resolve_());
         }
-        void Lifecycle.getInstance()
-          .emit('Doctor:diagnostic', { testName, status })
-          .then(() => resolve());
-      });
+      );
     });
   }
 

test/diagnostics.test.ts

@@ -18,6 +18,7 @@ import childProcess from 'node:child_process';
 import Sinon from 'sinon';
 import { expect } from 'chai';
 import { fromStub, spyMethod, stubInterface, stubMethod } from '@salesforce/ts-sinon';
+import which from 'which';
 import { Config, Interfaces } from '@oclif/core';
 import { Lifecycle } from '@salesforce/core';
 import { ux } from '@oclif/core';
@@ -66,6 +67,7 @@ describe('Diagnostics', () => {
 
   beforeEach(() => {
     stubMethod(sandbox, ux, 'stdout');
+    sandbox.stub(which, 'sync').returns('/usr/local/bin/npm');
     childProcessExecFileStub = sandbox.stub(childProcess, 'execFile');
     drAddSuggestionSpy = spyMethod(sandbox, Doctor.prototype, 'addSuggestion');
     lifecycleEmitSpy = spyMethod(sandbox, lifecycle, 'emit');
@@ -236,8 +238,8 @@ describe('Diagnostics', () => {
   describe('outdatedCliVersionCheck', () => {
     it('passes when CLI version is equal to latest', async () => {
       childProcessExecFileStub.callsFake(
-        (cmd: string, args: string[], cb: (e: unknown, stdout: unknown, stderr: unknown) => void) => {
-          expect(cmd).to.equal('npm');
+        (cmd: string, args: string[], opts: unknown, cb: (e: unknown, stdout: unknown, stderr: unknown) => void) => {
+          expect(cmd).to.equal('/usr/local/bin/npm');
           expect(args).to.deep.equal(['view', 'sfdx-cli', 'dist-tags.latest']);
           cb(null, '7.160.0', '');
         }
@@ -257,8 +259,8 @@ describe('Diagnostics', () => {
 
     it('passes when CLI version is greater than latest', async () => {
       childProcessExecFileStub.callsFake(
-        (cmd: string, args: string[], cb: (e: unknown, stdout: unknown, stderr: unknown) => void) => {
-          expect(cmd).to.equal('npm');
+        (cmd: string, args: string[], opts: unknown, cb: (e: unknown, stdout: unknown, stderr: unknown) => void) => {
+          expect(cmd).to.equal('/usr/local/bin/npm');
           expect(args).to.deep.equal(['view', 'sfdx-cli', 'dist-tags.latest']);
           cb(null, '7.159.0', '');
         }
@@ -278,8 +280,8 @@ describe('Diagnostics', () => {
 
     it('fails when CLI version is less than latest', async () => {
       childProcessExecFileStub.callsFake(
-        (cmd: string, args: string[], cb: (e: unknown, stdout: unknown, stderr: unknown) => void) => {
-          expect(cmd).to.equal('npm');
+        (cmd: string, args: string[], opts: unknown, cb: (e: unknown, stdout: unknown, stderr: unknown) => void) => {
+          expect(cmd).to.equal('/usr/local/bin/npm');
           expect(args).to.deep.equal(['view', 'sfdx-cli', 'dist-tags.latest']);
           cb(null, '7.162.0', '');
         }
@@ -299,8 +301,8 @@ describe('Diagnostics', () => {
 
     it('fails when npm request fails', async () => {
       childProcessExecFileStub.callsFake(
-        (cmd: string, args: string[], cb: (e: unknown, stdout: unknown, stderr: unknown) => void) => {
-          expect(cmd).to.equal('npm');
+        (cmd: string, args: string[], opts: unknown, cb: (e: unknown, stdout: unknown, stderr: unknown) => void) => {
+          expect(cmd).to.equal('/usr/local/bin/npm');
           expect(args).to.deep.equal(['view', 'sfdx-cli', 'dist-tags.latest']);
           cb({ code: 1 }, '', 'connection timeout');
         }

yarn.lock

@@ -1700,6 +1700,11 @@
   resolved "https://registry.yarnpkg.com/@types/unist/-/unist-3.0.2.tgz#6dd61e43ef60b34086287f83683a5c1b2dc53d20"
   integrity sha512-dqId9J8K/vGi5Zr7oo212BGii5m3q5Hxlkwy3WpYuKPklmBEvsbMYYyLxAQpSffdLl/gdW0XUpKWFvYmyoWCoQ==
 
+"@types/which@^3":
+  version "3.0.4"
+  resolved "https://registry.yarnpkg.com/@types/which/-/which-3.0.4.tgz#2c3a89be70c56a84a6957a7264639f39ae4340a1"
+  integrity sha512-liyfuo/106JdlgSchJzXEQCVArk0CvevqPote8F8HgWgJ3dRCcTHgJIsLDuee0kxk/mhbInzIZk3QWSZJ8R+2w==
+
 "@types/wrap-ansi@^3.0.0":
   version "3.0.0"
   resolved "https://registry.yarnpkg.com/@types/wrap-ansi/-/wrap-ansi-3.0.0.tgz#18b97a972f94f60a679fd5c796d96421b9abb9fd"
@@ -4544,6 +4549,11 @@ isexe@^2.0.0:
   resolved "https://registry.yarnpkg.com/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10"
   integrity sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==
 
+isexe@^3.1.1:
+  version "3.1.5"
+  resolved "https://registry.yarnpkg.com/isexe/-/isexe-3.1.5.tgz#42e368f68d5e10dadfee4fda7b550bc2d8892dc9"
+  integrity sha512-6B3tLtFqtQS4ekarvLVMZ+X+VlvQekbe4taUkf/rhVO3d/h0M2rfARm/pXLcPEsjjMsFgrFgSrhQIxcSVrBz8w==
+
 istanbul-lib-coverage@^3.0.0, istanbul-lib-coverage@^3.2.0:
   version "3.2.0"
   resolved "https://registry.yarnpkg.com/istanbul-lib-coverage/-/istanbul-lib-coverage-3.2.0.tgz#189e7909d0a39fa5a3dfad5b03f71947770191d3"
@@ -7351,6 +7361,13 @@ which@^2.0.1:
   dependencies:
     isexe "^2.0.0"
 
+which@^4:
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/which/-/which-4.0.0.tgz#cd60b5e74503a3fbcfbf6cd6b4138a8bae644c1a"
+  integrity sha512-GlaYyEb07DPxYCKhKzplCWBJtvxZcZMrL+4UkrTSJHHPyZU4mYYTv3qaOe77H7EODLSSopAUFAc6W8U4yqvscg==
+  dependencies:
+    isexe "^3.1.1"
+
 widest-line@^3.1.0:
   version "3.1.0"
   resolved "https://registry.yarnpkg.com/widest-line/-/widest-line-3.1.0.tgz#8292333bbf66cb45ff0de1603b136b7ae1496eca"