BC: rework npm-cli detction

.gitignore

@@ -119,3 +119,6 @@ dist
 
 # TernJS port file
 .tern-port
+
+## test files
+sentinelFile*

src/npmRunner.ts

@@ -18,8 +18,9 @@ Copyright (c) OWASP Foundation. All Rights Reserved.
 */
 
 import { type CommonExecOptions, execFileSync, execSync, type ExecSyncOptionsWithBufferEncoding } from 'node:child_process'
-import { existsSync } from 'node:fs'
-import { resolve } from 'node:path'
+import { closeSync, existsSync, mkdtempDisposableSync, openSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join, resolve } from 'node:path'
 
 /** !attention: args might not be shell-save. */
 type runFunc = (args: string[], options: ExecSyncOptionsWithBufferEncoding) => Buffer
@@ -41,16 +42,14 @@ export class NpmRunner {
    *   - foo/npx-cli_js     // Invalid extension
    *   - npx-cli.js/foo.sh  // Directory of the same name
    */
-  static readonly #npxMatcher = /(^|\\|\/)npx-cli\.js$/
+  static readonly #npxMatcher = /(^|\\|\/)npx-cli\.[cm]?js$/
 
-  static readonly #winExeMatcher = /\.(exe|com)$/i
-  static readonly #winCmdMatcher = /\.(cmd|bat)$/i
+  run: runFunc
 
   constructor (process_: NodeJS.Process, console_: Console) {
     this.run = NpmRunner.#makeNpmRunner(process_, console_)
   }
 
-  run: runFunc
 
   #version: string | undefined
 
@@ -66,62 +65,82 @@ export class NpmRunner {
     return this.#version
   }
 
-  static #getExecPath (process_: NodeJS.Process, console_: Console): string | undefined {
+  static #getExecPathEnv (process_: NodeJS.Process, console_: Console): string | undefined {
+    console_.debug('DEBUG | looking up env NPM...')
     // `npm_execpath` will be whichever cli script has called this application by npm.
     // This can be `npm`, `npx`, or `undefined` if called by `node` directly.
-    const execPath = process_.env.npm_execpath ?? ''
-    if (execPath === '') {
+
+    let npmPath = process_.env.npm_execpath ?? ''
+    if (npmPath === '') {
+      console_.debug('DEBUG | env NPM empty')
       return undefined
     }
 
-    if (NpmRunner.#npxMatcher.test(execPath)) {
-      // `npm` must be used for executing `ls`.
+    if (NpmRunner.#npxMatcher.test(npmPath)) {
+      // https://github.com/npm/cli/issues/6662
       console_.debug('DEBUG | command: npx-cli.js usage detected, checking for npm-cli.js ...')
       // Typically `npm-cli.js` is alongside `npx-cli.js`, as such we attempt to use this and validate it exists.
       // Replace the script in the path, and normalise it with resolve (eliminates any extraneous path separators).
-      const npmPath = resolve(execPath.replace(NpmRunner.#npxMatcher, '$1npm-cli.js'))
-      if (existsSync(npmPath)) {
-        return npmPath
-      }
-    } else if (existsSync(execPath)) {
-      return execPath
+      npmPath = resolve(npmPath.replace(NpmRunner.#npxMatcher, '$1npm-cli.js'))
     }
 
-    throw new Error(`unexpected NPM execPath: ${execPath}`)
+    if (!existsSync(npmPath)) {
+      throw new Error(`Missing env NPM ${JSON.stringify(npmPath)}`)
+    }
+    console_.debug('DEBUG | env NPM found: %s', npmPath)
+    return npmPath
   }
 
-  static #getSystemNpmPath (process_: NodeJS.Process, console_: Console): string {
-    console_.debug('DEBUG | lookup system NPM...')
-    const npmPath = NpmRunner.#isWindows(process_)
-      ? execSync('where npm').toString().split(/\r?\n/).find(s => NpmRunner.#winExeMatcher.test(s) || NpmRunner.#winCmdMatcher.test(s))
-      : execSync('which npm').toString().trim()
-    if (npmPath === undefined || npmPath === '') {
-      throw new Error('missing system NPM')
+  static #getExecPathSys(process_: NodeJS.Process, console_: Console): string {
+    console_.debug('DEBUG | looking up system NPM...')
+    /* eslint-disable-next-line no-useless-assignment -- ack */
+    let npmPath = ''
+
+    const tmpDir = mkdtempDisposableSync(join(tmpdir(), 'cyclonedx-npm_execpath-'))
+    try {
+      const tempFile = openSync(join(tmpDir.path, 'package.json'), 'w')
+      try {
+        writeFileSync(tempFile, JSON.stringify({
+          'private': true,
+          'name': '@cyclonedx/cyclonedx-npm_execpath',
+          'scripts': {
+            // no quotes - stay OS independent
+            'npm_execpath': 'node -p process.env.npm_execpath'
+          }
+        }))
+      } finally {
+        closeSync(tempFile)
+      }
+      npmPath = execSync('npm run --silent npm_execpath', {
+        cwd: tmpDir.path,
+        env: process_.env,
+        stdio: ['ignore', 'pipe', 'ignore'],
+        encoding: 'buffer',
+        maxBuffer: Number.MAX_SAFE_INTEGER // DIRTY but effective
+      }).toString().trim()
+    } catch (err) {
+      throw new Error('Failed looking up system NPM', { cause: err })
+    } finally {
+      tmpDir.remove()
+    }
+
+    if (npmPath === '' || !existsSync(npmPath)) {
+      throw new Error(`Missing system NPM ${JSON.stringify(npmPath)}`)
     }
     console_.debug('DEBUG | system NPM found: %s', npmPath)
     return npmPath
   }
 
   static #makeNpmRunner (process_: NodeJS.Process, console_: Console): runFunc {
-    const execPath = NpmRunner.#getExecPath(process_, console_)
-                  ?? NpmRunner.#getSystemNpmPath(process_, console_)
+    const execPath = NpmRunner.#getExecPathEnv(process_, console_)
+      ?? NpmRunner.#getExecPathSys(process_, console_)
 
-    if (NpmRunner.#jsMatcher.test(execPath)) {
-      const nodeExecPath = process_.execPath
-      console_.debug('DEBUG | makeNpmRunner caused execFileSync "%s" with "-- %s"', nodeExecPath, execPath)
-      return (args, options) => execFileSync(nodeExecPath, ['--', execPath, ...args], options)
+    if (!NpmRunner.#jsMatcher.test(execPath)) {
+      throw new Error(`unexpected NPM execPath: ${execPath}`)
     }
 
-    if (NpmRunner.#isWindows(process_) && NpmRunner.#winCmdMatcher.test(execPath)) {
-      console_.debug('DEBUG | makeNpmRunner caused execFileSync "cmd.exe" with "/c %s"', execPath)
-      return (args, options) => execFileSync('cmd.exe', ['/c', execPath, ...args], options)
-    }
-
-    console_.debug('DEBUG | makeNpmRunner caused execFileSync "%s"', execPath)
-    return (args, options) => execFileSync(execPath, args, options)
-  }
-
-  static #isWindows(process_: NodeJS.Process): boolean {
-    return process_.platform.startsWith('win')
+    const nodeExecPath = process_.execPath
+    console_.debug('DEBUG | makeNpmRunner caused execFileSync "%s" with "-- %s"', nodeExecPath, execPath)
+    return (args, options) => execFileSync(nodeExecPath, ['--', execPath, ...args], options)
   }
 }

tests/_data/npm-ls_replacement/just-exit.cmd

@@ -0,0 +1,30 @@
+@echo off
+
+REM ------------------------------------------------------------
+REM This file is part of CycloneDX generator for NPM projects.
+REM
+REM Licensed under the Apache License, Version 2.0 (the "License");
+REM you may not use this file except in compliance with the License.
+REM You may obtain a copy of the License at
+REM
+REM    http://www.apache.org/licenses/LICENSE-2.0
+REM
+REM Unless required by applicable law or agreed to in writing, software
+REM distributed under the License is distributed on an "AS IS" BASIS,
+REM WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+REM See the License for the specific language governing permissions and
+REM limitations under the License.
+REM
+REM Copyright (c) OWASP Foundation. All Rights Reserved.
+REM SPDX-License-Identifier: Apache-2.0
+REM ------------------------------------------------------------
+
+if "%~1"=="--version" (
+    echo %CT_VERSION%
+    exit /b 0
+)
+
+set "EXIT_CODE=%CT_EXIT_CODE%"
+if "%EXIT_CODE%"=="" set "EXIT_CODE=0"
+
+exit /b %EXIT_CODE%

tests/integration/cli.args-pass-through.test.js

@@ -17,12 +17,12 @@ SPDX-License-Identifier: Apache-2.0
 Copyright (c) OWASP Foundation. All Rights Reserved.
 */
 
-const { existsSync, mkdirSync, readFileSync } = require('node:fs')
+const { existsSync, mkdirSync, readFileSync, writeFileSync } = require('node:fs')
 const { join } = require('node:path')
 
 const { describe, expect, test } = require('@jest/globals')
 
-const { dummyProjectsRoot, mkTemp, npmLsReplacement, runCLI } = require('./')
+const { NPM_LATETS, dummyProjectsRoot, mkTemp, npmLsReplacement, runCLI } = require('./')
 
 describe('integration.cli.args-pass-through', () => {
   const cliRunTestTimeout = 15000
@@ -83,6 +83,7 @@ describe('integration.cli.args-pass-through', () => {
 
       const { res, errFile } = runCLI([
         ...cdxArgs,
+        '-vvvv',
         '--',
         join('with-lockfile', 'package.json')
       ], logFileBase, cwd, {
@@ -104,47 +105,91 @@ describe('integration.cli.args-pass-through', () => {
     const tmpRootRun = join(tmpRoot, 'shell_injection_proof')
     mkdirSync(tmpRootRun)
 
-    function mkPayload (sentinelFile) {
-      return process.platform.startsWith('win')
-        ? `& type nul > "${sentinelFile}" &`
-        : `; touch '${sentinelFile}' ;`
+    const runsOnWindows = process.platform.startsWith('win')
+
+    const npmExecpaths = {
+      system: undefined,
+      'js-file': npmLsReplacement.justExit,
+    }
+    if (runsOnWindows) {
+      npmExecpaths['cmd-file'] = npmLsReplacement.justExitCmd
     }
 
-    test.each([
-      // region workspace
-      (function () {
-        const sentinelFile = join(tmpRootRun, 'sentinelFile_workspace-single.txt')
-        return [
-          'single --workspace with shell metacharacters',
-          ['--workspace', mkPayload(sentinelFile)],
-          sentinelFile
-        ]
-      })(),
-      (function () {
-        const sentinelFile = join(tmpRootRun, 'sentinelFile_workspace-chained.txt')
-        return [
-          'chained --workspace: legitimate then malicious',
-          ['--workspace', 'legitimate-workspace', '-w', mkPayload(sentinelFile)],
-          sentinelFile
-        ]
-      })(),
-      // endregion workspace
-    ])('%s', async (purpose, cdxArgs, sentinelFile) => {
+    const mkPayload4type = {
+      'shell metacharacters': function (sentinelFile) {
+        return runsOnWindows
+          ? `x & type nul > ${sentinelFile} & echo `
+          : `x ; touch ${sentinelFile} ; echo `
+      },
+      'shell metacharacters heading single-quote': function (sentinelFile) {
+        return runsOnWindows
+          ? `x' & type nul > ${sentinelFile} & echo `
+          : `x' ; touch ${sentinelFile} ; echo '`
+      },
+      'shell metacharacters heading double-quote': function (sentinelFile) {
+        return runsOnWindows
+          ? `x" & type nul > ${sentinelFile} & echo `
+          : `x" ; touch ${sentinelFile} ; echo "`
+      },
+      'shell metacharacters surrounding single-quote': function (sentinelFile) {
+        return runsOnWindows
+          ? `x' & type nul > ${sentinelFile} & echo '`
+          : `x' ; touch ${sentinelFile} ; echo '`
+      },
+      'shell metacharacters surrounding double-quote': function (sentinelFile) {
+        return runsOnWindows
+          ? `x" & type nul > ${sentinelFile} & echo "`
+          : `x" ; touch ${sentinelFile} ; echo "`
+      },
+    }
+
+    const cases = []
+    for (const [npmExecpathLabel, npmExecpath] of Object.entries(npmExecpaths)) {
+      for (const [payloadType, mkPayload] of Object.entries(mkPayload4type)) {
+        const payloadFilePrefix = `${npmExecpathLabel}-${payloadType}`.replace(/\W/g, '-')
+        let sentinelFileName = `sentinelFile_${payloadFilePrefix}_workspace-single.txt`
+        cases.push([
+          `${payloadType} on ${npmExecpathLabel} with single --workspace`,
+          npmExecpath,
+          ['--workspace', mkPayload(sentinelFileName)],
+          sentinelFileName
+        ])
+        sentinelFileName = `sentinelFile_${payloadFilePrefix}_workspace-chained.txt`
+        cases.push([
+          `${payloadType} on ${npmExecpathLabel} with chained --workspace`,
+          npmExecpath,
+          ['--workspace', 'legitimate-workspace', '-w', mkPayload(sentinelFileName)],
+          sentinelFileName
+        ])
+      }
+    }
+
+    writeFileSync(join(tmpRootRun, 'package.json'), '{}')
+    writeFileSync(join(tmpRootRun, 'package-lock.json'), '{}')
+
+    test.each(cases)('%s', async (purpose, npmExecpath, cdxArgs, sentinelFileName) => {
+      const sentinelFile = join(tmpRootRun, sentinelFileName)
       expect(existsSync(sentinelFile)).toBe(false)
 
       const logFileBase = join(tmpRootRun, purpose.replace(/\W/g, '_'))
-      const cwd = dummyProjectsRoot
+      const cwd = tmpRootRun
 
-      const { res } = runCLI([
+      const { res, errFile } = runCLI([
         ...cdxArgs,
-        '--',
-        join('with-lockfile', 'package.json')
+        '-vvvv',
       ], logFileBase, cwd, {
-        npm_execpath: undefined
+        npm_execpath: npmExecpath,
+        CT_VERSION: `${NPM_LATETS}.0.0`
       })
+
       await res.catch(() => { /* pass */ })
 
-      expect(existsSync(sentinelFile)).toBe(false)
+      try {
+        expect(existsSync(sentinelFile)).toBe(false)
+      } catch (err) {
+        process.stderr.write(readFileSync(errFile))
+        throw err
+      }
     }, cliRunTestTimeout)
   })
 })

tests/integration/index.js

@@ -41,6 +41,7 @@ const npmLsReplacement = {
   checkArgs: join(npmLsReplacementPath, 'check-args.js'),
   demoResults: join(npmLsReplacementPath, 'demo-results.js'),
   justExit: join(npmLsReplacementPath, 'just-exit.js'),
+  justExitCmd: join(npmLsReplacementPath, 'just-exit.cmd'),
   nonExistingBinary: join(npmLsReplacementPath, 'aNonExistingBinary')
 }
 
@@ -91,10 +92,10 @@ function runCLI (args, logFileBase, cwd, env) {
     platform: process.platform,
   }
 
-  /**
-   * @type {Promise<number>}
-   */
-  const res = cli.run(mockProcess)
+  const res = cli.run(mockProcess).finally(() => Promise.all([
+    new Promise(resolve => { stdout.end(resolve) }),
+    new Promise(resolve => { stderr.end(resolve) })
+  ]))
 
   return { res, outFile, errFile }
 }