Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Invalid semver in package history causes crash when installing a package #5017

Closed
2 tasks done
bjorn-stange-expel opened this issue Jun 14, 2022 · 12 comments · Fixed by npm/metavuln-calculator#43
Closed
2 tasks done
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release

Comments

@bjorn-stange-expel
Copy link

bjorn-stange-expel commented Jun 14, 2022

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Installing a package with an invalid semver in its version history causes npm to crash. I wasn't sure if this was a bug with https://github.com/npm/metavuln-calculator or here (or somewhere else), but I thought I'd open the issue here first, since the maintainers probably know best where this bug belongs.

Using --no-audit causes the installation to succeed, so what seems like is happening is that package audit happens on install, not just for the version being installed, but for all versions of the package. Since the yui package has an invalid semver in its version history, when the package audit step iterates over all versions, it causes a crash. This is my guess, and I'm opening an issue here to confirm that this is the case.

After running npm i yui@3.18.1 here's the error:

npm ERR! Invalid Version: 3.5.0pr2

And the relevant debug log:

258 verbose stack TypeError: Invalid Version: 3.5.0pr2
258 verbose stack     at new SemVer (/Users/bjornstange/.npm/lib/node_modules/npm/node_modules/semver/classes/semver.js:38:13)
258 verbose stack     at compareBuild (/Users/bjornstange/.npm/lib/node_modules/npm/node_modules/semver/functions/compare-build.js:3:20)
258 verbose stack     at /Users/bjornstange/.npm/lib/node_modules/npm/node_modules/semver/functions/sort.js:2:51
258 verbose stack     at Array.sort (<anonymous>)
258 verbose stack     at Object.sort (/Users/bjornstange/.npm/lib/node_modules/npm/node_modules/semver/functions/sort.js:2:36)
258 verbose stack     at Advisory.[calculateRange] (/Users/bjornstange/.npm/lib/node_modules/npm/node_modules/@npmcli/metavuln-calculator/lib/advisory.js:169:28)
258 verbose stack     at Advisory.load (/Users/bjornstange/.npm/lib/node_modules/npm/node_modules/@npmcli/metavuln-calculator/lib/advisory.js:157:28)
258 verbose stack     at Calculator.[calculate] (/Users/bjornstange/.npm/lib/node_modules/npm/node_modules/@npmcli/metavuln-calculator/lib/index.js:60:14)
258 verbose stack     at async Promise.all (index 0)
258 verbose stack     at async Map.[init] (/Users/bjornstange/.npm/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/audit-report.js:186:9)

Expected Behavior

npm i yui@3.18.1 should succeed and not crash npm.

Steps To Reproduce

  1. With node 16.15.1 and npm 8.12.1
  2. Run npm i yui@3.18.1
  3. See error/logs, specifically npm ERR! Invalid Version: 3.5.0pr2

Environment

  • npm: 8.11.0
  • Node.js: 16.15.1
  • OS Name: Mac OS 12.4 Monterey
  • System Model Name: MacBook Pro
  • npm config:
; "user" config from /Users/bjornstange/.npmrc

prefix = "/Users/bjornstange/.npm"

; node bin location = /Users/bjornstange/.n/bin/node
; node version = v16.15.1
; npm local prefix = /Users/bjornstange/expel/adhoc/npm_crash
; npm version = 8.12.1
; cwd = /Users/bjornstange/expel/adhoc/npm_crash
; HOME = /Users/bjornstange
; Run `npm config ls -l` to show all defaults.
@bjorn-stange-expel bjorn-stange-expel added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Jun 14, 2022
@tschoartschi
Copy link

@bjorn-stange-expel we have the exact same problem 🤔 did you find a workaround without adding --no-audit? Is it possible to pin yui to some version that is not affected by this problem?

The real problem for us is, that some sub-dependency of a sub-dependency (you know heaviest objects in the universe -> node_modules 😉) is pulling in yui and this breaks our whole build...

@bourdeaudhui
Copy link

Same for me starting with a package sub-dependency gulp-yuidoc@0.1.2 ending to yui through other sub-dependencies...

Log:

3307 verbose stack TypeError: Invalid Version: 3.5.0pr2
3307 verbose stack     at new SemVer (/opt/homebrew/Cellar/node@16/16.15.0/lib/node_modules/npm/node_modules/semver/classes/semver.js:38:13)
3307 verbose stack     at compareBuild (/opt/homebrew/Cellar/node@16/16.15.0/lib/node_modules/npm/node_modules/semver/functions/compare-build.js:3:20)
3307 verbose stack     at /opt/homebrew/Cellar/node@16/16.15.0/lib/node_modules/npm/node_modules/semver/functions/sort.js:2:51
3307 verbose stack     at Array.sort (<anonymous>)
3307 verbose stack     at Object.sort (/opt/homebrew/Cellar/node@16/16.15.0/lib/node_modules/npm/node_modules/semver/functions/sort.js:2:36)
3307 verbose stack     at Advisory.[calculateRange] (/opt/homebrew/Cellar/node@16/16.15.0/lib/node_modules/npm/node_modules/@npmcli/metavuln-calculator/lib/advisory.js:167:28)
3307 verbose stack     at Advisory.load (/opt/homebrew/Cellar/node@16/16.15.0/lib/node_modules/npm/node_modules/@npmcli/metavuln-calculator/lib/advisory.js:155:28)
3307 verbose stack     at Calculator.[calculate] (/opt/homebrew/Cellar/node@16/16.15.0/lib/node_modules/npm/node_modules/@npmcli/metavuln-calculator/lib/index.js:60:14)

node v16.15.0
npm v8.5.5

@bjorn-stange-expel
Copy link
Author

bjorn-stange-expel commented Jun 15, 2022

@bjorn-stange-expel we have the exact same problem 🤔 did you find a workaround without adding --no-audit? Is it possible to pin yui to some version that is not affected by this problem?

The real problem for us is, that some sub-dependency of a sub-dependency (you know heaviest objects in the universe -> node_modules 😉) is pulling in yui and this breaks our whole build...

That's how we're pulling in yui as well, dependency of a dependency of a dependency. We decided to just remove the parent package for now to fix the build. I don't believe it matters which version of yui you use, with the latest npm they will all fail to install, if my understanding of this bug is correct. That's also why I opened this issue, to confirm my understanding. I might very well be wrong about the nature of this bug.

@tschoartschi
Copy link

@bjorn-stange-expel I installed the package npm-why to understand where yui is coming from. Of course I needed to run npm i with --no-audit to get a package-lock.json but then it gives me the following output:

$ npm-why yui
  Who required yui:
  pdc > @storybook/ember-cli-storybook > ember-cli-addon-docs-yuidoc > yuidocjs > yui@3.18.1

At least I know now which package it is and I reported this problem to them as well. Maybe they can remove or switch the package somehow. Let's see what they think :-)

@bjorn-stange-expel
Copy link
Author

@bjorn-stange-expel I installed the package npm-why to understand where yui is coming from. Of course I needed to run npm i with --no-audit to get a package-lock.json but then it gives me the following output:

$ npm-why yui
  Who required yui:
  pdc > @storybook/ember-cli-storybook > ember-cli-addon-docs-yuidoc > yuidocjs > yui@3.18.1

At least I know now which package it is and I reported this problem to them as well. Maybe they can remove or switch the package somehow. Let's see what they think :-)

You can also run npm ls yui which shows you the dependency tree of that package, and it's built in!

@ljharb
Copy link
Contributor

ljharb commented Jun 17, 2022

npm explain yui is what you’re looking for.

@nlepage
Copy link

nlepage commented Jun 23, 2022

I opened a PR at @npmcli/metavuln-calculator which would be a first step to fix this: npm/metavuln-calculator#43

I applied the patch on my local npm install, and it works fine.

Hope this helps.

@bjorn-stange-expel
Copy link
Author

I opened a PR at @npmcli/metavuln-calculator which would be a first step to fix this: npm/metavuln-calculator#43

I applied the patch on my local npm install, and it works fine.

Hope this helps.

Wow thanks!

@AmeBoond
Copy link

I applied also the patch and It works like a charm !
Thanks @nlepage

@nlepage
Copy link

nlepage commented Jun 29, 2022

This is still not fixed, see #5102 which actually merges the patch in npm/cli.

@nlepage
Copy link

nlepage commented Jun 30, 2022

The fix has beenmerged and released in v8.13.2

@DeveloperDanjo
Copy link

Are there any plans to backport this to NPM 7 (where audit is also turned on by default)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants