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 to fix security issues #606

Merged
merged 1 commit into from
Jun 15, 2018
Merged

Update node-pre-gyp to fix security issues #606

merged 1 commit into from
Jun 15, 2018

Conversation

crutchcorn
Copy link
Contributor

Because node-pre-gyp v0.9.1 has a dependency that opens security issues, I updated the version to 0.10.0 in order to fix this. I've ran tests and they all passed perfectly fine. That being said, I had to bump the version of Node supported to 6 rather than 4 (as pre-gyp drops support for 4 due to EOL upcoming) and thusly updated the major version of the package to 3.0.0 rather than 2.0.2 or 2.1.0 because of how semver works.

Either way, not a massive change.

Fixes #604

@fluxsauce
Copy link

@crutchcorn broken on Node 6?

@crutchcorn
Copy link
Contributor Author

Well that's not ideal @fluxsauce . It seems that it's running Object.entities which was introduced in NodeJS 7 - not 6. I'll look into it because node-pre-gyp was not documented dropping Node 6, so that should be fixed.

@lahdekorpi
Copy link

lahdekorpi commented May 17, 2018

Any updates on this?
npm install on the latest version shows that a vulnerability is found.
And npm audit is currently showing a warning about this...

The PR shows that the major of this package is going to update to 3, so why not merge this with a Node 7 requirement. And then figure out a solution for the older major?

@simonmeusel
Copy link

@crutchcorn Why are the node 4 and node 5 builds working then? And why is only the MacOS build failing?

If it's only because of Object.entities, then the build with prior versions of node should also be failing.

https://travis-ci.org/kelektiv/node.bcrypt.js/builds/376661218

@recrsn
Copy link
Collaborator

recrsn commented Jun 3, 2018

@lahdekorpi It will not be possible to just drop a supported LTS release.

Although the issue is rated as moderate, it is rated as low for us, as we do not use the module in run-time. Even the module is not invoked while installing from npm.

recrsn
recrsn previously requested changes Jun 3, 2018
@@ -11,10 +11,10 @@
"crypto"
],
"main": "./bcrypt",
"version": "2.0.1",
"version": "3.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not make this increment. We will release with a CHANGELOG and the increment is done as the part of it.

@paulbjensen
Copy link

The tests on Travis are all passing, so if the review item to revert the version change gets done, will this item be good to merge?

@recrsn recrsn dismissed their stale review June 15, 2018 19:28

not required

@recrsn recrsn merged commit fd70560 into kelektiv:master Jun 15, 2018
@recrsn
Copy link
Collaborator

recrsn commented Jun 15, 2018

I'll cut a release on Sunday

@hfsd
Copy link

hfsd commented Jun 25, 2018

@agathver Please publish on npm.

@thom-nic
Copy link
Contributor

thom-nic commented Jul 4, 2018

@agathver sorry for the ping, any update on when this will publish on npm?

fast-facts pushed a commit to fast-facts/node.bcrypt.js that referenced this pull request Jun 17, 2022
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.

update node-pre-gyp after vulnerability is fixed
8 participants