Skip to content

[WTF-2695]: Escape package name before using in spawn#186

Open
weirdwater wants to merge 3 commits into
masterfrom
fix/windows-audit
Open

[WTF-2695]: Escape package name before using in spawn#186
weirdwater wants to merge 3 commits into
masterfrom
fix/windows-audit

Conversation

@weirdwater

@weirdwater weirdwater commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

This PR contains

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Other (describe)

What is the purpose of this PR?

Preventing shell expansion using single quotes did not work on Windows as the cmd prompt version of npm show would use the quote marks in the api call.

The closest specification of npm package names comes from the docs where it says that it must be usable as part of a URL. We don't use encodeURIComponent() as that would escape @ and / as well, so encodeURI is enough.

https://docs.npmjs.com/cli/v10/configuring-npm/package-json#name

Relevant changes

  • Escape the input for spawn rather than just relying on preventing shell expansion.
  • During testing the audit command would get stuck in cycles present in the npm audit report. This logic was updated to avoid such cycles.
  • Catch errors thrown by npm when looking up available versions.

What should be covered while testing?

Audit command works on windows, mac, and linux.

Preventing shell expansion using single quotes did not work on Windows
as the cmd prompt version of `npm show` would use the quote marks in the
api call.

The closest specification of npm package names comes from the docs where
it says that it must be usable as part of a URL. We don't use
encodeURIComponent() as that would escape @ and / as well, so encodeURI
is enough.

https://docs.npmjs.com/cli/v10/configuring-npm/package-json#name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants