Security: npm audit reports security issues for rollup-plugin-node-builtins
erezrokah opened this issue · 7 comments
Describe the bug
Netlify CLI uses this plugin as a dependency through Netlify Build.
Running npm audit on the CLI repo reports the following:
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate │ Regular Expression Denial of Service │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ semver │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=4.3.2 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @netlify/build │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @netlify/build > @netlify/plugin-edge-handlers > │
│ │ rollup-plugin-node-builtins > browserify-fs > levelup > │
│ │ semver │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://npmjs.com/advisories/31 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate │ Memory Exposure │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ bl │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=0.9.5 <1.0.0 || >=1.0.1 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @netlify/build │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @netlify/build > @netlify/plugin-edge-handlers > │
│ │ rollup-plugin-node-builtins > browserify-fs > levelup > bl │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://npmjs.com/advisories/596 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High │ Remote Memory Exposure │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ bl │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=1.2.3 <2.0.0 || >=2.2.1 <3.0.0 || >=3.0.1 <4.0.0 || │
│ │ >=4.0.3 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @netlify/build │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @netlify/build > @netlify/plugin-edge-handlers > │
│ │ rollup-plugin-node-builtins > browserify-fs > levelup > bl │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://npmjs.com/advisories/1555
Reproduction
git clone git@github.com:netlify/cli.gitcd clinpm cinpm audit
Pull requests
Not sure how to fix this as the library is no longer maintained. Inlining it will probably remove the errors but won't fix the security issues (unless we fix those ourselves). We can also fork the repo, fix the issues and publish it as a new package.
i think it would be nice to inline this plugin into this repo, so we can make edge handler specific changes if we need to.
i also expect this plugin to be used less and less over the next months since webpack just dropped support for node builtins and people are generally pivoting towards supporting web usecases well.
We can also fork the repo, fix the issues and publish it as a new package.
I would be in favor of this solution, so we can keep the tests, benefit from the community, and also keep those two different pieces of logic in separate repositories.
rollup-plugin-node-builtins is providing a range of shims over Node.js core modules. There is some inherent complexity to doing this. Also, it is quite imperfect, and I suspect many Node.js users to be surprised that their Edge handlers do not work even though they thought Node.js was supported.
Is Node.js support in Edge handlers documented? If so, users might expect full support. If not, I am wondering whether we should support this additional logic if users are not aware of the feature.
I think this feature is still pretty neat, I am just wondering whether we should ponder the tradeoff between maintenance cost vs value for our users, as we plan to transition to maintaining rollup-plugin-node-builtins (either as a fork or inlined). That being said, we can probably just limit ourselves to fixing the security vulnerabilities, providing we're ok with turning a blind eye on the many bugs reported in this unmaintained repository.
Also tagging @erquhart since this additional project would be maintained by Netlify Dev.
Is Node.js support in Edge handlers documented? If so, users might expect full support. If not, I am wondering whether we should support this additional logic if users are not aware of the feature.
good point. we mostly say that it's v8, we don't mention nodejs anywhere.
the only point of this plugin is to make the experience more out-of-the-box if some dependencies depend on nodejs primitives but work fine with shims.
at the time i added this i wasn't sure if people would be able to install the shims themselves so that rollup would pick them up properly. if that's possible, we could drop this plugin.
This was reported in netlify/cli#1497
This was also reported in netlify/cli#1528 (comment)
https://github.com/ionic-team/rollup-plugin-node-polyfills is a more up-to-date alternative without any vulnerable dependencies right now.