fix(libnpmexec): escape inferred executable names for selected shell

workspaces/libnpmexec/README.md

@@ -28,7 +28,7 @@ await libexec({
 
 - `opts`:
   - `args`: List of pkgs to execute **Array<String>**, defaults to `[]`
-  - `call`: An alternative command to run when using `packages` option **String**, defaults to empty string.
+  - `call`: An alternative shell script to run when using `packages` option **String**, defaults to empty string. Unlike an executable selected from `args` or a package's `bin`, this is intentionally interpreted as shell syntax.
   - `cache`: The path location to where the npm cache folder is placed **String**
   - `npxCache`: The path location to where the npx cache folder is placed **String**
   - `chalk`: Chalk instance to use for colors? **Required**
@@ -42,6 +42,14 @@ await libexec({
   - `yes`: Should skip download confirmation prompt when fetching missing packages from the registry? **Boolean**
   - `registry`, `cache`, and more options that are forwarded to [@npmcli/arborist](https://github.com/npm/cli/blob/latest/workspaces/arborist/README.md) and [pacote](https://github.com/npm/pacote/#options) **Object**
 
+  Executable names are escaped for the selected shell, including POSIX shells and
+  PowerShell used on Windows. For inferred PowerShell invocations, arguments are
+  also escaped as literal strings. With `cmd.exe`, executable names containing double
+  quotes, percent signs, exclamation marks, or control characters are rejected
+  with `EINVALIDCOMMAND`. Use a bin name without these characters. On a matching
+  npx-cache hit, command selection uses the installed package's `bin` rather than
+  the registry's bin metadata.
+
 ## LICENSE
 
 [ISC](./LICENSE)

workspaces/libnpmexec/lib/index.js

@@ -267,8 +267,8 @@ const exec = async (opts) => {
     })
     const lockPath = join(installDir, 'concurrency.lock')
     const npxTree = await withLock(lockPath, () => npxArb.loadActual())
-    await Promise.all(needInstall.map(async ({ spec }) => {
-      const { manifest } = await missingFromTree({
+    await Promise.all(needInstall.map(async ({ spec, manifest: requestedManifest }) => {
+      const { manifest, node } = await missingFromTree({
         spec,
         tree: npxTree,
         flatOptions,
@@ -281,6 +281,10 @@ const exec = async (opts) => {
         } else {
           add.push(manifest._id)
         }
+      } else if (needPackageCommandSwap && commandManifest === requestedManifest) {
+        // A cache hit must use the installed bin, not mutable registry metadata.
+        commandManifest = node.package
+        args[0] = getBinFromManifest(commandManifest)
       }
     }))
 

workspaces/libnpmexec/lib/run-script.js

@@ -3,7 +3,10 @@ const runScript = require('@npmcli/run-script')
 const pkgJson = require('@npmcli/package-json')
 const { log, output } = require('proc-log')
 const noTTY = require('./no-tty.js')
-const isWindowsShell = require('./is-windows.js')
+const isWindows = require('./is-windows.js')
+
+const isCmd = shell => /(?:^|[\\/])cmd(?:\.exe)?$/i.test(shell)
+const isPowerShell = shell => /(?:^|[\\/])(?:powershell|pwsh)(?:\.exe)?$/i.test(shell)
 
 const run = async ({
   args,
@@ -15,10 +18,24 @@ const run = async ({
   runPath,
   scriptShell,
 }) => {
-  // escape executable path
-  // necessary for preventing bash/cmd keywords from overriding
-  if (!isWindowsShell) {
-    if (args.length > 0) {
+  if (!call && args.length > 0) {
+    const shell = scriptShell || (isWindows ? process.env.ComSpec || 'cmd' : 'sh')
+    if (isCmd(shell)) {
+      // Variable expansion (including delayed expansion) and embedded quotes
+      // cannot be safely escaped here. Control characters are invalid filenames.
+      if (/["%!]/.test(args[0]) || [...args[0]].some(c => c.charCodeAt(0) < 32)) {
+        throw Object.assign(
+          new Error(`Invalid executable name for cmd.exe: ${JSON.stringify(args[0])}`),
+          { code: 'EINVALIDCOMMAND' }
+        )
+      }
+      // Protect both cmd.exe's metacharacter parsing and executable-name parsing.
+      args[0] = `"${args[0]}"`.replace(/[ ^&()<>|";,*?=@]/g, '^$&')
+    } else if (isPowerShell(shell)) {
+      // Encode the whole invocation to avoid promise-spawn's POSIX argument escaping.
+      const quotedArgs = args.map(arg => `'${arg.replace(/['\u2018-\u201b]/g, '$&$&')}'`)
+      args = [`& ${quotedArgs.join(' ')}`]
+    } else {
       // single-quote so shell metacharacters in the executable name are taken
       // literally; double quotes still expand $(), backticks, $var and "
       args[0] = `'${args[0].replace(/'/g, `'\\''`)}'`

workspaces/libnpmexec/test/registry.js

@@ -3,6 +3,7 @@ const t = require('tap')
 const { setup, createPkg, merge } = require('./fixtures/setup.js')
 const crypto = require('node:crypto')
 const { existsSync } = require('node:fs')
+const PackageJson = require('@npmcli/package-json')
 
 t.test('run from registry - no local packages', async t => {
   const { fixtures, package } = createPkg({ versions: ['2.0.0'] })
@@ -126,6 +127,81 @@ t.test('avoid install when exec from registry an available pkg', async t => {
   })
 })
 
+const binNames = [
+  'two words',
+  'bin&echo cli170-injected',
+  'bin^name',
+  'bin(name)',
+  "bin'name",
+  'bin;name',
+  'bin=name',
+  'if',
+]
+const posixBinNames = [
+  'bin!name',
+  'bin$(echo injected>cli170-marker)',
+  'bin`echo injected>cli170-marker`',
+  "bin';echo injected>cli170-marker;#",
+]
+
+for (const binName of [...binNames, ...posixBinNames]) {
+  t.test(`executes normalized bin name literally: ${binName}`, {
+    skip: process.platform === 'win32' && posixBinNames.includes(binName),
+  }, async t => {
+    const { pkg, fixtures, package: mockPackage } = createPkg({
+      versions: ['1.0.0'],
+      bin: { [binName]: 'bin-file.js' },
+    })
+    const normalized = await new PackageJson().fromContent(pkg).normalize()
+    t.same(Object.keys(normalized.content.bin), [binName], 'payload survives normalization')
+
+    const { exec, path, registry, readOutput } = setup(t, { testdir: fixtures })
+    await mockPackage({ registry, path })
+    const args = ['an argument', 'literal&argument', 'literal^argument', '"quoted"']
+    await exec({ args: ['@npmcli/create-index', ...args] })
+    t.match(await readOutput('@npmcli-create-index'), { value: 'packages-1.0.0', args })
+    t.notOk(existsSync(resolve(path, 'cli170-marker')), 'no injected command ran')
+  })
+}
+
+for (const binName of [
+  'renamed-bin',
+  'create-index&echo injected>cli170-marker',
+  'create-index$(echo injected>cli170-marker)',
+  'create-index%CLI170_COMMAND%',
+]) {
+  t.test(`cache hit ignores registry bin metadata: ${binName}`, async t => {
+    const { pkg, fixtures, package: mockPackage } = createPkg({ versions: ['1.0.0'] })
+    const { exec, path, registry, readOutput, rmOutput } = setup(t, { testdir: fixtures })
+    await mockPackage({ registry, path })
+    await exec({ args: ['@npmcli/create-index'] })
+    await rmOutput('@npmcli-create-index')
+
+    const altered = await new PackageJson().fromContent({
+      ...pkg,
+      bin: { [binName]: 'bin-file.js' },
+    }).normalize()
+    t.same(Object.keys(altered.content.bin), [binName], 'payload survives normalization')
+    await mockPackage({
+      registry,
+      path,
+      tarballs: [],
+      times: 1,
+      manifest: registry.manifest({
+        name: pkg.name,
+        packuments: [altered.content],
+      }),
+    })
+
+    await exec({ args: ['@npmcli/create-index', 'cached argument'] })
+    t.match(await readOutput('@npmcli-create-index'), {
+      value: 'packages-1.0.0',
+      args: ['cached argument'],
+    })
+    t.notOk(existsSync(resolve(path, 'cli170-marker')), 'no injected command ran')
+  })
+}
+
 t.test('run multiple from registry', async t => {
   const indexPkg = createPkg({
     versions: ['2.0.0'],