isaacs/node-mkdirp

ship a fix for mkdirp@0.5.1 due minimist

juanpicado opened this issue ยท 20 comments

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 mkdirp@0.5.1 still use and it is pinned

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

I'm asking to ship a fix for mkdirp@0.5.x 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

@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 โค๏ธ

Edit: Use mkdirp@0.5.3 - 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"
  }

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.

@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!

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

Isaac is going to publish a fix soon ๐Ÿ™

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

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

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

Thanks ๐ŸŽ‰ ๐ŸŽ‰ ๐ŸŽ‰ ๐ŸŽ‰ ๐ŸŽ‰ ! I think this can be closed!

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?

@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.

@karlhorky and if we don't use Yarn?

@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).

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

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).

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- commented

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

@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.