fix(libnpmexec): escape inferred executable names for selected shell by martinrrm · Pull Request #10044 · npm/cli

martinrrm · 2026-09-25T20:08:12Z

Summary

When npx <package> infers an executable from package bin metadata, the executable name becomes part of a synthetic npm script. On Windows, that script is ultimately interpreted by cmd.exe. This change protects the inferred executable token according to the selected shell instead of forwarding it unquoted.

Supporting dependency PR: npm/promise-spawn#166

Changes

`lib/run-script.js`

Escape the executable token when no explicit call script was supplied.
Select the escaping strategy from scriptShell, including POSIX shells selected on Windows.
For cmd.exe, protect command separators, spaces, command keywords, and other cmd metacharacters.
Reject executable names containing double quotes, percent signs, exclamation marks, or control characters that cannot be safely represented as a cmd executable token.
Preserve call as an intentionally supplied shell script; its arguments are not treated as inferred executable names.

`lib/index.js`

On a matching npx-cache hit, select the inferred command from the installed package manifest rather than mutable registry metadata.

This cache-manifest adjustment is related hardening and may be split into a separate change during review if preferred.

Tests and documentation

Add unit coverage for cmd.exe token encoding and rejection cases.
Add coverage for POSIX escaping when a POSIX shell is selected on Windows.
Add normalized-bin registry regressions covering command separators and cache metadata mismatches.
Document the shell-specific executable handling and rejected cmd.exe characters.

Dependency status

The required promise-spawn fix is released and bundled as @npmcli/promise-spawn@10.0.1 through #10152. This branch is rebased onto latest and no longer carries a duplicate bundled dependency diff.

Validation

Focused and full libnpmexec tests, eslint, and template-oss postlint pass locally on Node 24.15.0.
Mutation testing confirms the shell and cache regressions fail without the production changes.
Added PowerShell-specific construction coverage and a Windows runtime test for inferred executable invocation.
The npm/cli CI matrix passes on Linux, macOS, and Windows across all configured Node versions.
No vulnerability reproduction or malicious command execution was run.

Review status

Ready for review.

## Summary Decode an already quoted and caret-escaped executable token before using it for Windows executable lookup. This is a supporting change for npm/cli#10044. That proposal protects an executable name before placing it in a command interpreted by `cmd.exe`. `promise-spawn` must still identify the protected executable as a `.cmd` or `.bat` shim so it can apply the existing extra escaping required for batch-file arguments. ## Why this is needed There are two distinct representations of the executable: 1. **Execution representation** — protected for `cmd.exe`, and kept unchanged in the final command string. 2. **Lookup representation** — decoded temporarily so `which.sync()` can resolve the executable and determine whether it is a batch shim. For example, a caller may pass this protected executable token: ```text ^"tool.cmd^" ``` The existing parser derives the lookup value directly from that text: ```text ^"tool.cmd^" ``` That is not an executable name that `which.sync()` can resolve. If lookup fails, `promise-spawn` may not recognize that the command is a `.cmd` shim, so `doubleEscape` remains false. The updated parser derives this lookup-only value instead: ```text tool.cmd ``` The original `^"tool.cmd^"` token is still retained in the command sent to `cmd.exe`; only the temporary value used for executable detection is decoded. ## Why batch shims need detection `promise-spawn` already distinguishes native executables from `.cmd` and `.bat` shims: ```js doubleEscape = pathToInitial.endsWith('.cmd') || pathToInitial.endsWith('.bat') ``` A batch shim causes its arguments to pass through an additional `cmd.exe` parsing layer. The existing `escape.cmd(arg, doubleEscape)` implementation therefore adds another level of caret escaping when `doubleEscape` is true. Using the safe argument `hello world` as an example: | Executable detection | Encoded argument | |---|---| | Native executable or unresolved command | `^"hello^ world^"` | | `.cmd`/`.bat` shim | `^^^"hello^^^ world^^^"` | If an escaped `tool.cmd` token cannot be resolved, the first form may be selected even though the second form is required for the batch shim. ## Data flow with npm CLI The related npm CLI proposal has this flow on Windows: ```text package bin name -> libnpmexec protects the executable token for cmd.exe -> promise-spawn receives the protected script -> promise-spawn decodes only the first token for which.sync() -> .cmd/.bat detection selects the existing double-escape mode -> the original protected script is sent to cmd.exe ``` This PR implements only the lookup step. It does not decide which executable names are valid, add shell quoting to arbitrary commands, or decode the command that is executed. ## Changes - Decode caret escapes outside double-quoted sections while preparing the temporary lookup token. - Treat only `"` as a `cmd.exe` quote delimiter; single quotes have no quoting semantics in `cmd.exe`. - Remove surrounding double quotes before calling `which.sync()`. - Leave the original command string unchanged for execution. - Preserve the existing `.cmd`/`.bat` argument double-escaping behavior. ## Before and after | Input command token | Previous lookup value | New lookup value | Executed token | |---|---|---|---| | `tool` | `tool` | `tool` | `tool` | | `"tool.cmd"` | `"tool.cmd"` | `tool.cmd` | `"tool.cmd"` | | `^"tool.cmd^"` | `^"tool.cmd^"` | `tool.cmd` | `^"tool.cmd^"` | | `^"two^ words.cmd^"` | `^"two^ words.cmd^"` | `two words.cmd` | `^"two^ words.cmd^"` | ## Tests Added mock-based regression coverage confirming that: - An escaped and quoted executable token is decoded before lookup. - An escaped `.cmd` shim is recognized. - Arguments for the recognized batch shim receive the existing double-escaping treatment. - The final command sent to `cmd.exe` retains the protected executable token. The tests validate command construction and executable lookup without executing an injection payload. ## Related change - npm/cli#10044 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use PowerShell's call operator and literal-string escaping for inferred executable names, with construction and Windows runtime coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e53db77-0fe4-4320-8ada-f2962ddd303c

Copilot

🟡 Changes recommended

PowerShell escaping still permits command injection through Unicode quote delimiters, while POSIX-style argument escaping breaks valid arguments containing apostrophes.

2 open issues

What changed in this PR

This PR hardens libnpmexec’s executable handling for npm exec and npx, using shell-specific quoting and installed package metadata on cache hits.

Changes:

Add executable escaping for cmd.exe, PowerShell, and POSIX shells while preserving explicit call scripts.
Select cached executables from installed manifests rather than registry metadata.
Add regression tests and document shell-specific restrictions.

File	Description
`workspaces/libnpmexec/test/run-script.js`	Tests shell quoting, rejected names, and literal argument handling.
`workspaces/libnpmexec/test/registry.js`	Tests normalized bin names and cache metadata mismatches.
`workspaces/libnpmexec/README.md`	Documents executable escaping, restrictions, and cache behavior.
`workspaces/libnpmexec/lib/run-script.js`	Adds shell-specific executable encoding and cmd.exe validation.
`workspaces/libnpmexec/lib/index.js`	Uses installed bin metadata on matching cache hits.

_{Review effort: Balanced}

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 155db33f-8c59-4859-a9b9-374c3a77e721

github-actions · 2026-11-03T20:32:06Z

🎉 Backport to release/v11 created: #10217

…10217) Backport of #10044 to `release/v11`. Co-authored-by: martin <martin.ruiz.mares@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e53db77-0fe4-4320-8ada-f2962ddd303c Copilot-Session: 155db33f-8c59-4859-a9b9-374c3a77e721

martinrrm mentioned this pull request Sep 25, 2026

fix: decode escaped Windows executable tokens npm/promise-spawn#166

Merged

azchohfi mentioned this pull request Oct 8, 2026

fix(perf): do not list network interfaces when loading config definitions #10097

Open

martinrrm mentioned this pull request Oct 21, 2026

deps: update bin-links, promise-spawn, and pacote #10152

Merged

martinrrm and others added 3 commits October 28, 2026 01:25

fix(libnpmexec): escape inferred Windows executable names

4356c45

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

test(libnpmexec): cover Windows shell fallback

67088eb

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

martinrrm force-pushed the martinrrm/cli-170-windows-fix branch from 8d1dd0b to 3f7f129 Compare October 28, 2026 01:42

martinrrm marked this pull request as ready for review October 28, 2026 01:53

martinrrm requested a review from a team as a code owner October 28, 2026 01:53

jeffkl previously approved these changes Oct 28, 2026

View reviewed changes

martinrrm requested a balanced review from Copilot November 2, 2026 18:53

Copilot started reviewing on behalf of martinrrm November 2, 2026 18:53

View session

Copilot AI reviewed Nov 2, 2026

View reviewed changes

Comment thread workspaces/libnpmexec/lib/run-script.js Resolved

Comment thread workspaces/libnpmexec/lib/run-script.js Resolved

fix(libnpmexec): quote PowerShell executables and arguments

006b2f6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 155db33f-8c59-4859-a9b9-374c3a77e721

martinrrm dismissed jeffkl’s stale review via 006b2f6 November 2, 2026 21:34

jeffkl approved these changes Nov 2, 2026

View reviewed changes

martinrrm merged commit 10ffa9f into latest Nov 3, 2026
22 checks passed

martinrrm deleted the martinrrm/cli-170-windows-fix branch November 3, 2026 20:25

github-actions Bot mentioned this pull request Nov 2, 2026

chore: release 12.2.2 #10156

Open

martinrrm added the backport:v11 label Nov 3, 2026

github-actions Bot mentioned this pull request Nov 3, 2026

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

Merged

Navigation Menu

Repository navigation

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

fix(libnpmexec): escape inferred executable names for selected shell#10044
martinrrm merged 4 commits into
latestfrom
martinrrm/cli-170-windows-fix

martinrrm commented Sep 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

martinrrm commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

lib/run-script.js

lib/index.js

Tests and documentation

Dependency status

Validation

Review status

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

martinrrm commented Sep 25, 2026 •

edited

Loading

`lib/run-script.js`

`lib/index.js`