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

node_modules/@npmcli/promise-spawn/lib/index.js

@@ -83,20 +83,27 @@ const spawnWithShell = (cmd, args, opts, extra) => {
   if (isCmd) {
     let doubleEscape = false
 
-    // find the actual command we're running
+    // Decode the executable token so escaped batch shims still get double escaping.
     let initialCmd = ''
     let insideQuotes = false
     for (let i = 0; i < cmd.length; ++i) {
       const char = cmd.charAt(i)
+      if (char === '^' && !insideQuotes && i + 1 < cmd.length) {
+        initialCmd += cmd.charAt(++i)
+        continue
+      }
       if (char === ' ' && !insideQuotes) {
         break
       }
 
       initialCmd += char
-      if (char === '"' || char === "'") {
+      if (char === '"') {
         insideQuotes = !insideQuotes
       }
     }
+    if (initialCmd.startsWith('"') && initialCmd.endsWith('"')) {
+      initialCmd = initialCmd.slice(1, -1)
+    }
 
     let pathToInitial
     try {

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,13 @@ 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 used
+  on Windows. 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,7 @@ 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 run = async ({
   args,
@@ -15,10 +15,20 @@ 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 (/(?:^|\\)cmd(?:\.exe)?$/i.test(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 {
       // 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'],

workspaces/libnpmexec/test/run-script.js

@@ -1,4 +1,8 @@
 const t = require('tap')
+const { existsSync } = require('node:fs')
+const { resolve } = require('node:path')
+const PackageJson = require('@npmcli/package-json')
+const realRunScript = require('@npmcli/run-script')
 
 const mockRunScript = async (t, mocks, { level = 0 } = {}) => {
   const mockedRunScript = t.mock('../lib/run-script.js', mocks)
@@ -157,3 +161,155 @@ t.test('isNotWindows', async t => {
   // need both arguments and no arguments for code coverage
   await runScript()
 })
+
+t.test('escapes cmd.exe executable tokens', async t => {
+  for (const scriptShell of ['cmd', 'CMD.EXE', 'C:\\Windows\\System32\\cmd.exe']) {
+    const { runScript } = await mockRunScript(t, {
+      '@npmcli/run-script': async ({ pkg, args }) => {
+        t.equal(pkg.scripts.npx, '^"bin^&echo^ injected^"')
+        t.same(args, ['an argument', 'literal&argument'])
+      },
+      '../lib/is-windows.js': false,
+    })
+    await runScript({
+      args: ['bin&echo injected', 'an argument', 'literal&argument'],
+      scriptShell,
+    })
+  }
+})
+
+t.test('defaults to cmd when ComSpec is unavailable on Windows', async t => {
+  const comSpec = process.env.ComSpec
+  delete process.env.ComSpec
+  t.teardown(() => {
+    if (comSpec === undefined) {
+      delete process.env.ComSpec
+    } else {
+      process.env.ComSpec = comSpec
+    }
+  })
+
+  const { runScript } = await mockRunScript(t, {
+    '@npmcli/run-script': async ({ pkg }) => {
+      t.equal(pkg.scripts.npx, '^"if^"')
+    },
+    '../lib/is-windows.js': true,
+  })
+  await runScript({ args: ['if'] })
+})
+
+t.test('cmd.exe quoting preserves paths and literal metacharacters', async t => {
+  const cases = [
+    ['if', '^"if^"'],
+    ['two words', '^"two^ words^"'],
+    ['C:\\Program Files\\bin.cmd', '^"C:\\Program^ Files\\bin.cmd^"'],
+    ["bin'name", '^"bin\'name^"'],
+    ['bin(^);,=@', '^"bin^(^^^)^;^,^=^@^"'],
+  ]
+  let script
+  const { runScript } = await mockRunScript(t, {
+    '@npmcli/run-script': async ({ pkg }) => {
+      script = pkg.scripts.npx
+    },
+  })
+  for (const [command, expected] of cases) {
+    await runScript({ args: [command], scriptShell: 'cmd.exe' })
+    t.equal(script, expected, command)
+  }
+})
+
+t.test('rejects cmd.exe executable tokens that cannot be quoted safely', async t => {
+  const { runScript } = await mockRunScript(t, {
+    '@npmcli/run-script': async () => t.fail('must not spawn'),
+  })
+  for (const command of [
+    'bin%PATH%',
+    'bin!PATH!',
+    'bin" & echo injected',
+    'bin\necho injected',
+    'bin\recho injected',
+    'bin\0name',
+    'bin\tname',
+  ]) {
+    await t.rejects(runScript({ args: [command], scriptShell: 'cmd.exe' }), {
+      code: 'EINVALIDCOMMAND',
+      message: `Invalid executable name for cmd.exe: ${JSON.stringify(command)}`,
+    })
+  }
+})
+
+t.test('uses POSIX escaping for a POSIX shell on Windows', async t => {
+  const { runScript } = await mockRunScript(t, {
+    '@npmcli/run-script': async ({ pkg }) => {
+      t.equal(pkg.scripts.npx, '\'bin$(echo injected)\'\\\'\'name\'')
+    },
+    '../lib/is-windows.js': true,
+  })
+  await runScript({ args: ["bin$(echo injected)'name"], scriptShell: 'bash' })
+})
+
+t.test('call remains a shell script and its arguments are not executable tokens', async t => {
+  for (const scriptShell of ['sh', 'cmd.exe']) {
+    const { runScript } = await mockRunScript(t, {
+      '@npmcli/run-script': async ({ pkg, args }) => {
+        t.equal(pkg.scripts.npx, 'echo first && echo second')
+        t.same(args, ['an argument', '%literal%'])
+      },
+    })
+    await runScript({
+      call: 'echo first && echo second',
+      args: ['an argument', '%literal%'],
+      scriptShell,
+    })
+  }
+})
+
+t.test('normalized missing executable is not a shell script', async t => {
+  const path = t.testdir()
+  const windows = process.platform === 'win32'
+  const commands = windows ? [
+    'missing&echo injected>cli170-marker',
+    'missing|echo injected>cli170-marker',
+    'echo injected>cli170-marker',
+  ] : [
+    'missing$(echo injected>cli170-marker)',
+    'missing`echo injected>cli170-marker`',
+    "missing';echo injected>cli170-marker;#",
+  ]
+  const { runScript } = await mockRunScript(t, {
+    '@npmcli/run-script': opts => realRunScript({ ...opts, stdio: 'pipe' }),
+  })
+  if (windows) {
+    commands.push('%COMSPEC%', '!COMSPEC!')
+  }
+  for (const command of commands) {
+    const pkg = await new PackageJson().fromContent({
+      name: 'test',
+      version: '1.0.0',
+      bin: { [command]: 'bin.js' },
+    }).normalize()
+    t.same(Object.keys(pkg.content.bin), [command], 'payload survives normalization')
+    await t.rejects(runScript({
+      args: [command],
+      path,
+      runPath: path,
+      scriptShell: windows ? 'cmd.exe' : 'sh',
+    }), { code: /[%!]/.test(command) ? 'EINVALIDCOMMAND' : windows ? 1 : 127 })
+    t.notOk(existsSync(resolve(path, 'cli170-marker')), 'no injected command ran')
+  }
+})
+
+t.test('native executables receive literal arguments', async t => {
+  const path = t.testdir()
+  const args = ['two words', 'literal&argument', 'literal^argument', '"quoted"', "single'quote"]
+  const { runScript } = await mockRunScript(t, {
+    '@npmcli/run-script': opts => realRunScript({ ...opts, stdio: 'pipe' }),
+  })
+  const result = await runScript({
+    args: [process.execPath, '-e', 'console.log(JSON.stringify(process.argv.slice(1)))', ...args],
+    path,
+    runPath: path,
+    scriptShell: process.platform === 'win32' ? 'cmd.exe' : 'sh',
+  })
+  t.same(JSON.parse(result.stdout), args)
+})