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

security: update minimist and geojson-rewind to avoid CVE-2021-44906 #12442

Merged

Conversation

Spasfonx
Copy link

@Spasfonx Spasfonx commented Dec 6, 2022

Update dependencies on old versions of mapbox-gl-js (1.13.2) to avoid GHSA-xvch-5gv4-984h issue.

Edit: This fix is for old version of mapbox-gl-js (1.3.2)

@stepankuzmin stepankuzmin added the skip changelog Used for PRs that do not need a changelog entry label Dec 7, 2022
Copy link
Contributor

@stepankuzmin stepankuzmin left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @Spasfonx!

@stepankuzmin stepankuzmin changed the base branch from release-v1.13.2 to main December 7, 2022 15:45
@stepankuzmin stepankuzmin requested a review from a team as a code owner December 7, 2022 15:45
@stepankuzmin stepankuzmin changed the base branch from main to release-v1.13.2 December 7, 2022 15:46
@stepankuzmin
Copy link
Contributor

Sorry, missed that the PR was made to the release-v1.13.2 branch. What is your intent here? The minimist is only used as a dev dependency, so it wouldn't affect the library builds that were previously made.

@Spasfonx
Copy link
Author

Spasfonx commented Dec 7, 2022

Thanks for your review ! Watch out, this fix apply only for old versions (1.x.x), I pointed at release-v1.13.2 because it was the last branch associated with the version 1.3.2 but maybe you have better branch to merge on :)

@stepankuzmin stepankuzmin self-requested a review December 7, 2022 15:47
@Spasfonx
Copy link
Author

Spasfonx commented Dec 7, 2022

stepankuzmin we reply at the same time héhé. Indeed it was my intent, the main update here for security is through geojson-rewind (which update minimist in 1.2.6 too). Because of this package we have audit issue in main dependencies even if this is a dev package juste by importing mapbox-gl-js@1.3.2

@stepankuzmin
Copy link
Contributor

I see, thanks! Thanks for the contribution, but I'll close this PR since it doesn't affect the users. Sorry for the confusion.

@stepankuzmin stepankuzmin reopened this Dec 7, 2022
@stepankuzmin
Copy link
Contributor

Ah, so there is a security alert when you use the 1.x version? We should update it then.

@Spasfonx
Copy link
Author

Spasfonx commented Dec 7, 2022

Yes indeed, sorry, the body of the issue lack of context maybe, my bad ! Here my yarn audit --groups dependencies log with mapbox-gl: 1.13.2 in our package.json

image

@Spasfonx
Copy link
Author

Spasfonx commented Jan 4, 2023

Hello there, happy new year everyone ! 🎉

Some news on this update ?

@pascaloliv
Copy link

The changes looks good to me too. 👍 Well done @Spasfonx 🎉

@SnailBones
Copy link
Contributor

SnailBones commented Jan 9, 2023

Happy new year all and thanks for the PR @Spasfonx ! 🎉

Also looks good to me! Though, we may want to make a new branch for the release. We're aiming to get a release with the update out next week.

@Spasfonx
Copy link
Author

Oh great ! Thanks you very much guys 💪

@stepankuzmin stepankuzmin changed the base branch from release-v1.13.2 to release-v1.13.3 January 16, 2023 16:19
@stepankuzmin stepankuzmin merged commit acb644e into mapbox:release-v1.13.3 Jan 16, 2023
@stepankuzmin stepankuzmin mentioned this pull request Jan 16, 2023
@stepankuzmin
Copy link
Contributor

Thanks, everyone! This is now fixed in the v1.13.3 release.

@Spasfonx
Copy link
Author

Thank you guys, perfect ! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changelog Used for PRs that do not need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants