Fix for node-notifier vulnerability #95
Conversation
| @@ -50,6 +50,7 @@ | |||
| }, | |||
| "dependencies": { | |||
| "@babel/preset-env": "^7.10.4", | |||
| "node-notifier": "^8.0.1", | |||
There was a problem hiding this comment.
Its this package, right? https://www.npmjs.com/package/node-notifier?activeTab=readme
What do we need this for?
There was a problem hiding this comment.
Hi @reubenae. I noticed dependency bot rising some security vulnerability in one of our dependencies: node-notifier in yarn.lock. I thought we are using it already and performed the suggested action to upgrade. Then noticed that it actually created a new dependency. Wanted to discuss this with you last whitespace day and opened the PR and forgot about it. May be a false alert from dependency bot? or is uuid causing this as it is among one of the dependencies in node-notifier section?
There was a problem hiding this comment.
Interesting... I see it uses uuid as a dependency, but uuid doesn't use it as a dependency.
It looks like this dev-dependency @jest/reporters uses it https://github.com/zwave-js/node-zwave-js/blob/31f21271c69a5db89b171494d8a06c8afba2d9a1/yarn.lock#L1385 and it looks like they have updated their dependency since https://github.com/facebook/jest/blob/master/packages/jest-reporters/package.json#L52 in version 27. If you track that dependency tree up it unsurprisingly looks like jest is the dependency responsible for bringing it along. We're currently on version 26, so I think upgrading jest to 27 should upgrade node-notifier to a secure version.
I don't think this is a security concern based on the original issue report in node-notifier mikaelbr/node-notifier#294 - but lets upgrade anyway this week.
Does that all make sense? Any errors in my thinking?
There was a problem hiding this comment.
Thanks @reubenae Amazing! That sounds right. I will close this PR and we can create another one to upgrade jest.
Title
Description of what's changing
...
What else might be impacted?
...
Which issue does this PR relate to?
...
Checklist
[ ] Tests are written and maintain or improve code coverage
[ ] I've tested this in my application using
yarn link(if applicable)[ ] You consent to and are confident this change can be released with no regression