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

Fix hoek dependency vulnerability #17

Closed
wants to merge 2 commits into from
Closed

Conversation

EricHanLiu
Copy link
Contributor

@EricHanLiu EricHanLiu commented May 18, 2018

With suggestions from here, used

npm i hoek
npm uninstall hoek
npm update
npm install

When I navigate to the fix_vulnerability branch, I still see the vulnerability message for some reason. Perhaps it needs to be merged to master for the message to be removed?

There were a lot of changes to the static files (diva.js, diva-map.js, pixel.js, thousands of slight line changes) when I had to reinstall gulp and rebuild, and I'm guessing they had to do with
the gulp build and the updated dependency versions when webpacking the code. Pixel still works fine and it's likely just improvements to all the modules and stuff?

I've tried rebuilding everything using the script (pixel.sh) they provide to build when users first clone the repo, and the changes seem to persist through, all the version numbers remain updated after rebuilding using their script.

@EricHanLiu
Copy link
Contributor Author

EricHanLiu commented May 18, 2018

It also seems that hoek 2.16.3 (the version that threw the security vulnerability) still persists in package-lock.json as a dependency of other dependencies (hawk 3.1.3).

I'm looking at this thread which I believe says to update hawk to 7.0.7, but I can't really tell from that thread if that was the accepted fix, and it wasn't done automatically by npm update.

I'm not sure how to get hawk to update to the later version, and I'm kinda afraid to tamper with the dependencies they've specified for pixel_wrapper. Also maybe running the build script pixel.sh resets the hawk version?

Actually, in the unchanged package-lock.json the one instance of hawk uses version 3.1.3, but in this branch's updated package-lock.json, which has two instances of hawk, one instance has hawk 6.0.2 which requires hoek 4.x.x. So maybe the vulnerability issue was fixed?

@EricHanLiu
Copy link
Contributor Author

Actually, updating request (which requires hawk) to version 2.86.0 seems to have fixed the issue. hawk 3.1.3 required hoek 2.16.3 as a dependency, but updating request seems to have updated hawk as well to version 6.0.2, which depends on hoek 4.x.x which no longer has the issue.

Apparently hoek 4.2.1 also throws this vulnerability alert, but it was patched (backported) and it's rather a bug with GitHub and a hoek developer has already sent a message

@EricHanLiu
Copy link
Contributor Author

I made a new branch to test around with updating different dependencies, making a new pull request from that branch which has the updated request, hawk, and hoek. Should get rid of the vulnerability alert, if it doesn't it's specifically a GitHub issue according to the link in the last comment

@EricHanLiu
Copy link
Contributor Author

EricHanLiu commented May 18, 2018

New pull request from the appropriate branch.

Closing this one since it doesn't contain the actual fix, the other pull request will link here.

@EricHanLiu EricHanLiu closed this May 18, 2018
@EricHanLiu EricHanLiu deleted the fix_vulnerability branch May 18, 2018 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant