search

mapbox / mapbox-sdk-js

A JavaScript client to Mapbox services, supporting Node, browsers, and React Native
Other
719 stars 186 forks source link

Update `got` to 11.8.5 (CVE-2022-33987) #440

Closed amureki closed 2 years ago

amureki commented 2 years ago

Greetings, dear fellow maintainers,

Currently, there is a fixed CVE in the got package: https://security.snyk.io/vuln/SNYK-JS-GOT-2932019

It is moderate according to GitHub: https://github.com/advisories/GHSA-pfrx-2q88-qq97

And it is critical according to npm audit.

I am a user of Mapbox and using JS SDK (this one) in one of my projects and would like to get this fixed. Therefore, I opened up this PR:

This seems bloated, but I tried to separate all steps that were done in separate commits. I also checked release notes for https://github.com/sindresorhus/got/releases/tag/v11.0.0, it does not seem that things used here were affected, but it would be good for maintainers to confirm that.

And I am happy to reconsider things and try to simplify it if you see a better way.

Best, Rust

amureki commented 2 years ago

One test in the build in Travis CI seemed to be timed out, but I cannot retry it. This part is purely in maintainers hands. :)

herrbenesch commented 2 years ago

@amureki latest LTS is 16.x. You also might consider to pin the version to the latest LTS like done here: https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#specifying-nodejs-versions

But I don't know the implications of it for this project.

Small ping to anyone maintaining this project to accept the security patch here.

amureki commented 2 years ago

@amureki latest LTS is 16.x. You also might consider to pin the version to the latest LTS like done here: https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#specifying-nodejs-versions

But I don't know the implications of it for this project.

Small ping to anyone maintaining this project to accept the security patch here.

thanks for the suggestion, it totally makes sense to me (easier maintenance, plus it is still a safe option)! I added one commit to reflect this.

hlehmann commented 2 years ago

Any update on this?

codingjoe commented 2 years ago

@andrewharvey would you be so kind as to release this change? It otherwise we're all still stuck on a vulnerability.

andrewharvey commented 2 years ago

@codingjoe We can prepare a release as a PR which would help, but ultimately I think only Mapbox can publish to their npm.

codingjoe commented 2 years ago

should we write mapbox support about this?

amureki commented 2 years ago

should we write mapbox support about this?

Well, I was submitting a ticket about this issue on July 29th and got no answer.

I am writing another one about (this time it is about releasing the package to NPM), but not sure when their support team would be able to check it out.

@GrigoryDobrov kindly pinging you here as you did the very last release, and this is a security issue, so it requires some attention...

allanlewis commented 2 years ago

I see that a v0.13.5 tag has been created, are we now waiting for someone with the right permissions to publish to NPM?

chriswhong commented 2 years ago

It was published this morning https://www.npmjs.com/package/@mapbox/mapbox-sdk

amureki commented 2 years ago

@chriswhong thanks for making it to the final line!

And thank you everyone for the help and assistance! 🙌