iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
615 stars 210 forks source link

transitively pinned minimatch 3.0.4 dependency is causing rush audit to fail. #4524

Closed MichaelBelousov closed 2 years ago

MichaelBelousov commented 2 years ago

Describe the bug

Our transitive dependencies now include minimatch@3.0.4 which has a ReDos vulnerability https://github.com/advisories/GHSA-f8q6-p94x-37v3

To Reproduce

run rush audit in master. You'll fail on this: https://github.com/advisories/GHSA-f8q6-p94x-37v3

Expected behavior

No failure

Additional context

we could use pnpm.overrides but we'd have to upgrade our rush version and downstream consumers of our build tools will still have to manually fix.

MichaelBelousov commented 2 years ago

the transitive dep pinning this recursive-readdir has gone 5 years without a publish and apparently had this minimatch vulnerability listed as an issue since march: https://github.com/jergason/recursive-readdir/issues

so I'm not sure how long or if they will provide an update. @aruniverse you expressed being concerned that our build-tool consumers would need any override we ourselves add to our package manager.

https://github.com/jergason/recursive-readdir/pull/85

does not seem to be any contributor response yet to this issue.

MichaelBelousov commented 2 years ago

also as a note, this is a create-react-app transitive dependency because the dependency path goes through react-dev-tools

aruniverse commented 2 years ago

i didn't realize this was also a transitive dep coming from cra... thats going to be a pain. for those that arent using cra, and using our build tools theyll still run into this issue.

@mattbjordan @evelynpreslar-bentley would be nice to refactor out the need for the recursive-readdir from itwin/build-tools if possible

mattbjordan commented 2 years ago

@aruniverse How much of a hassle is it to fork or copy the recursive-readdir and upgrade the minimatch version ourselves? I suppose we want to keep this as a package for ease-of-use - so does that mean that this is the biggest hassle?

aruniverse commented 2 years ago

I do not want to fork and continuously maintain them. the one fork we have now is already a pain