Skip to content

fix(powershell): avoid evaluating CLI paths as source - #10058

Draft
martinrrm wants to merge 1 commit into
latestfrom
martinrrm/fix-powershell-prefix-injection
Draft

martinrrm wants to merge 1 commit into
latestfrom
martinrrm/fix-powershell-prefix-injection

Conversation

@martinrrm

Copy link
Copy Markdown
Contributor

Summary

  • Replace Invoke-Expression in bin/npm.ps1 and bin/npx.ps1 with a native process launch using System.Diagnostics.ProcessStartInfo and UseShellExecute = false. The selected Node executable and CLI path no longer become PowerShell source.
  • Retain raw argument extraction to avoid simply reverting the quoting, option-forwarding, and comma-separated argument fixes introduced in fix(powershell): use Invoke-Expression to pass args #8278. Leave pipeline and explicit -File branches unchanged.
  • Log and discard project-level prefix before loading project configuration. Previously, the configuration loader logged that this setting was prohibited but still made it effective.
  • Add a Windows shim fixture containing literal $() and single quotes, plus assertions that the wrappers do not use Invoke-Expression. Extend the config regression to verify that the project prefix is absent and cannot change the effective global prefix.

Refs github/npm#15799, finding CLI-CROW-CMD-002.

Why both changes are needed

Quoting a path inside an Invoke-Expression string does not prevent PowerShell subexpression evaluation. Removing that evaluation boundary addresses the wrapper sink.

Rejecting project prefix independently prevents a repository's .npmrc from redirecting CLI selection to repository-controlled JavaScript. This is not a substitute for removing the wrapper sink.

Validation

  • Passed git diff --check.
  • Passed JavaScript syntax checks for all three changed JavaScript files.
  • Focused root/Windows shim and @npmcli/config TAP tests are blocked locally: tap is unavailable.
  • Root and config workspace lint are blocked locally: eslint is unavailable. @npmcli/template-oss is also unavailable.
  • Native Windows and PowerShell are unavailable in this Linux worktree. The Windows regression and argument-forwarding behavior have not been runtime-validated.
  • No dependency installs, lockfile changes, generated files, or CI changes.

Before marking ready

  • Pass the Windows shim suite in Windows PowerShell and PowerShell 7.
  • Verify native-process argument semantics, stdin/stdout/stderr, exit codes, and Ctrl-C behavior.
  • Pass the owning root and config workspace lint/tests and applicable CI matrix.
  • After validation and merge, prepare backports for release/v11 and release/v10.

Draft intentionally: the implementation and regressions are available for discussion, but runtime verification is still outstanding.

Launch the CLI through a native process instead of evaluating executable and CLI paths as PowerShell source, and reject project-level prefix settings so repository config cannot redirect CLI selection.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@martinrrm
martinrrm marked this pull request as ready for review October 1, 2026 18:18
@martinrrm
martinrrm requested a review from a team as a code owner October 1, 2026 18:18
@martinrrm
martinrrm marked this pull request as draft October 1, 2026 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant