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

ship a fix for [email protected] due minimist #7

Closed
juanpicado opened this issue Mar 12, 2020 · 20 comments
Closed

ship a fix for [email protected] due minimist #7

juanpicado opened this issue Mar 12, 2020 · 20 comments

Comments

@juanpicado
Copy link

juanpicado commented Mar 12, 2020

hi,

I've been using mkdirp for a while in the 0.x version. https://www.npmjs.com/package/mkdirp your library has a massive amount of downloads and still many dependencies use it.

Recently was announced a security vulnerability at minimist,

https://app.snyk.io/vuln/SNYK-JS-MINIMIST-559764

a dependency which [email protected] still use and it is pinned

https://github.com/isaacs/node-mkdirp/blob/d4eff0f06093aed4f387e88e9fc301cb76beedc7/package.json

I'm asking to ship a fix for [email protected] updating minimist. At Verdaccio also we use it, I had some breaking changes but was an easy fix, but we have several transitive dependencies that might not update anytime soon due to the breaking changes.

Screen Shot 2020-03-12 at 7 52 03 AM

@lirantal
Copy link

lirantal commented Mar 12, 2020

@isaacs if there's any way to help with that such as pushing out a PR or something that helps you keep up to date let me know please, happy to lend a hand ❤️

@karlhorky
Copy link

karlhorky commented Mar 15, 2020

Edit: Use [email protected] - the problem is fixed there.

For anyone looking for a fix for the interim, using Yarn Resolutions you can specify this version number, even though it doesn't match the semver range of mkdirp (basically like applying #8).

Add this to your package.json if you're using Yarn and run yarn to update your lockfile:

  "resolutions": {
    "**/mkdirp/minimist": "0.2.1"
  }

@drifterz28
Copy link

This is still an issue, there needs to be a 0.5.2 version as other project seem to depend on the 0.5 version of this package. Parcel v1 to be exact. version 1 of this package is a break in the API and would require a rewrite of a lot of code (beyond me) and this seems like a better solution.

@LinusU
Copy link

LinusU commented Mar 17, 2020

@isaacs this is a problem for Multer as well, since the 1.x release line is still supported and we cannot drop support for Node.js <10 without making that a breaking change, thus we cannot upgrade to mkdirp 1.x.

If it would be possible to release a new 0.5.x package that just bumps minimist that would be great. I'd be happy to do all the legwork on it if you want!

@gwynnarth
Copy link

Same applies to https://github.com/less/less.js/ which still locks "mkdirp": "^0.5.0", :(

@lirantal
Copy link

Isaac is going to publish a fix soon 🙏

@will-cbase
Copy link

@lirantal
Copy link

I think Isaac is still working on it since there's no 0.5.x releases at all on npmjs

@LinusU
Copy link

LinusU commented Mar 17, 2020

0.5.3 is released to Npm, but it's deprecated (as it probably should be 👍)

https://www.npmjs.com/package/mkdirp/v/0.5.3

It include an upgraded dependency on minimist 👏

I think that this issue can now be closed

@lirantal
Copy link

Ahh yes indeed! 0.5.3 is the latest version and the one that should be used.

@juanpicado
Copy link
Author

Thanks 🎉 🎉 🎉 🎉 🎉 ! I think this can be closed!

Den-dp added a commit to Den-dp/jimp that referenced this issue Mar 18, 2020
Fixing https://www.npmjs.com/advisories/1179 by relaxing mkdirp versions range to allow 0.5.2 or 0.5.3 which contains a fix as per isaacs/node-mkdirp#7 (comment)
Den-dp added a commit to Den-dp/jimp that referenced this issue Mar 18, 2020
Fixing https://www.npmjs.com/advisories/1179 by relaxing mkdirp versions range to allow 0.5.2 or 0.5.3 which contains a fix as per isaacs/node-mkdirp#7 (comment)
hipstersmoothie pushed a commit to jimp-dev/jimp that referenced this issue Mar 18, 2020
Fixing https://www.npmjs.com/advisories/1179 by relaxing mkdirp versions range to allow 0.5.2 or 0.5.3 which contains a fix as per isaacs/node-mkdirp#7 (comment)
@kbarnesweb
Copy link

Ahh yes indeed! 0.5.3 is the latest version and the one that should be used.

How should I go about adding this? In resolutions?

@karlhorky
Copy link

@kbarnesweb If you use Yarn, a resolution like this will do it:

  "resolutions": {
    "mkdirp": "^0.5.3"
  }

Once you have run yarn and your yarn.lock has been updated, you can remove the resolution again.

@jmz527
Copy link

jmz527 commented Mar 24, 2020

@karlhorky and if we don't use Yarn?

@brettz9
Copy link

brettz9 commented Mar 24, 2020

@jmz527 : You can use npm update --depth=9999 (setting to the full 9999 may time out, but I find setting to a depth of 10 usually catches the necessary updates). Alternatively, you can remove node_modules and package-lock.json and run a local npm install.

People should note that unless a project has updated to mkdirp 1.0, the 0.5.3 update is supposed to give a deprecated warning when getting the new patched version (the version is deprecated, but at least it fixes the vulnerability).

@karlhorky
Copy link

karlhorky commented Mar 24, 2020

Edit: There are critical shortcomings in npm-force-resolutions if you need multiple major versions of a package: handlebars-lang/handlebars.js#1661 (comment)

If you use npm, there is a package that makes this easier: npm-force-resolutions

npm will also at some point probably receive full Resolutions support: npm/rfcs#56

@karfau
Copy link

karfau commented Mar 24, 2020

For me running npm audit fix worked to resolve those issues(, which uses what @brettz9 suggested by detecting applying the required --depth and only updates those dependencies that are having an issue).

@karlhorky
Copy link

Nice, cool that npm audit fix knows which dependents to update to fix security issues further down in the dependency tree - didn't know that it could do that!

To compare, @dependabot can't do that trick yet: https://twitter.com/karlhorky/status/1239183753911701504

@jcw-
Copy link

jcw- commented Apr 1, 2020

For me, npm audit fix couldn't fix all of them, but this did the trick:

npm list mkdirp # see usages before
npm update mkdirp --depth=10 # update them
npm list mkdirp # see usages after

image

@jmz527
Copy link

jmz527 commented Apr 1, 2020

@brettz9 thanks for the suggestions! npm audit fix worked for me only after I had deleted and regenerated the package-lock.json file.

@karlhorky I did try out npm-force-resolutions, but yeah, it's shortcomings became clear when it broke within our CircleCI flow.

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 a pull request may close this issue.