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

Update node-pre-gyp #219

Closed
wants to merge 1 commit into from
Closed

Update node-pre-gyp #219

wants to merge 1 commit into from

Conversation

finnp
Copy link

@finnp finnp commented May 9, 2018

No description provided.

@bnoordhuis
Copy link
Contributor

Why is that needed?

@finnp
Copy link
Author

finnp commented May 9, 2018

There has been a security vulnerability with deep-extend, which is a nested dependency of node-pre-gyp. Not a big issue, but it shows up as a security warning. https://nodesecurity.io/advisories/612

@remy
Copy link

remy commented May 12, 2018

I'm keen to see this merged too - this change fixes a nested vuln (via the bundled deps), which is causing issues filed against upstream packages (in my case, I've already had two nodemon issues raised because of npm's new audit tool).

@ehmicky
Copy link

ehmicky commented May 13, 2018

This commit should probably keep the caret, i.e. ^0.10.0 not 0.10.0

@fearphage
Copy link

Why is that needed?

Is there some benefit to having outdated dependencies?

@@ -5,7 +5,7 @@
"main": "fsevents.js",
"dependencies": {
"nan": "^2.9.2",
"node-pre-gyp": "^0.9.0"
"node-pre-gyp": "0.10.0"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing ^

@bnoordhuis
Copy link
Contributor

There has been a security vulnerability with deep-extend, which is a nested dependency of node-pre-gyp.

That should have been fixed by the upgrade to node-pre-gyp@0.9.0, see discussion in #201. I don't see any upgrades to deep-extend between 0.9.0 and 0.10.0 so I doubt this fixes anything.

@finnp
Copy link
Author

finnp commented May 13, 2018

Versions of deep-extend before 0.5.1 are vulnerable to prototype pollution.
https://nodesecurity.io/advisories/612:

And the tree looks like this:


└─┬ fsevents@1.2.3
  └─┬ node-pre-gyp@0.9.1
    └─┬ rc@1.2.6
      └── deep-extend@0.4.2

However I just noticed that node-pre-gyp didn't update rc yet. So this won't help, depends on this to be merged first: mapbox/node-pre-gyp#379

Seems like I overlooked that.

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 this pull request may close these issues.

None yet

6 participants