diff --git a/node_modules/@npmcli/promise-spawn/lib/index.js b/node_modules/@npmcli/promise-spawn/lib/index.js index 1faf62c9157df..d6d2c8cd048e5 100644 --- a/node_modules/@npmcli/promise-spawn/lib/index.js +++ b/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 { diff --git a/workspaces/libnpmexec/README.md b/workspaces/libnpmexec/README.md index 84512ac590498..67968555365aa 100644 --- a/workspaces/libnpmexec/README.md +++ b/workspaces/libnpmexec/README.md @@ -28,7 +28,7 @@ await libexec({ - `opts`: - `args`: List of pkgs to execute **Array**, 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) diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js index 4d7c126654689..56dcf7c736940 100644 --- a/workspaces/libnpmexec/lib/index.js +++ b/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) } })) diff --git a/workspaces/libnpmexec/lib/run-script.js b/workspaces/libnpmexec/lib/run-script.js index 2de40454f9dcf..f0061c2c6e3ee 100644 --- a/workspaces/libnpmexec/lib/run-script.js +++ b/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, `'\\''`)}'` diff --git a/workspaces/libnpmexec/test/registry.js b/workspaces/libnpmexec/test/registry.js index adbf4116107b4..c6de3b1d3d739 100644 --- a/workspaces/libnpmexec/test/registry.js +++ b/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'], diff --git a/workspaces/libnpmexec/test/run-script.js b/workspaces/libnpmexec/test/run-script.js index 5c92ce19397f6..88e5ea4f17d59 100644 --- a/workspaces/libnpmexec/test/run-script.js +++ b/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) +})