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] Overrides not working for some transitive dependencies, cause ERR ELSPROBLEMS in npm ls #4889

Closed
2 tasks done
ed-gallagher opened this issue May 12, 2022 · 4 comments · Fixed by #4963
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

@ed-gallagher
Copy link

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

Using the following dependencies/overrides in package.json:

"dependencies": {
    "appium": "^1.22.3",
    "mkdirp": "0.5.5",
    "optimist": "0.6.1"
},
"overrides": {
    "minimist": ">=1.2.6"
}

All versions of minimist under appium fail to get overridden. The minimist versions under mkdirp@0.5.5 (which is also a transitive dependency of appium) and optimist@0.6.1 are correct.

Running npm ls minimist yields the following output:
Screen Shot 2022-05-12 at 10 36 05 AM

Expected Behavior

All instances of minimist should be overridden to the specified version.

Steps To Reproduce

  1. Use the above dependencies/overrides in package.json
  2. npm i
  3. npm ls minimist

Environment

  • npm: 8.10.0
  • Node.js: 14.19.1
  • OS Name: macOS Monterey 12.2.1 (also tested on Windows 10)
  • npm config:
; node bin location = /Users/egallagher/.nvm/versions/node/v14.19.1/bin/node
; node version = v14.19.1
; npm local prefix = /Users/egallagher/.npm/_logs
; npm version = 8.10.0
; cwd = /Users/egallagher/.npm/_logs
; HOME = /Users/egallagher
; Run `npm config ls -l` to show all defaults.
@ed-gallagher ed-gallagher 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 May 12, 2022
@github-matthieu-wipliez

Hey there, I also have the same issue. And I suspect that sooner or later thousands of developers looking to upgrade minimist (27M downloads/weeks of insecure < 1.2.6 versions) will attempt to use overrides to solve this (like I did). And fail miserably because overrides is not working properly for this relatively simple and yet crucial use case 🤷 @nlf @wraithgar @lukekarrys @ruyadorno or anyone else in the @npm cli team, can you please prioritize this?

@github-matthieu-wipliez

It appears that there are two other recently opened issues that look similar to this: #4942 and #4939. This issue is almost 3 weeks old, and points at a major bug in a feature that is essential to replace vulnerable dependency versions. Can you please take a look at it?

@nlf
Copy link
Contributor

nlf commented May 31, 2022

the failed overrides are because appium includes a shrinkwrap and overrides do not apply to shrinkwrapped or bundled dependencies at this time.

that said, npm ls should not be showing you that there are problems here. i'll have a fix up for that shortly.

@github-matthieu-wipliez
Copy link

the failed overrides are because appium includes a shrinkwrap and overrides do not apply to shrinkwrapped or bundled dependencies at this time.

I find this surprising given the documentation and the overrides RFC.

Looking at the Motivation section of the RFC, I see:

A security vulnerability is identified in a transitive dependency

Which is also what is mentioned in the npm doc. The author of said RFC seems to agree in Questions and bikeshedding:

my suspicion is that the user expectation is that they would be overridden.

Should I open a feature request for this?

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.

3 participants