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

resolve 2/3 security advisories #48

Merged
merged 7 commits into from
Apr 6, 2020

Conversation

sojeri
Copy link
Contributor

@sojeri sojeri commented Apr 5, 2020

This doesn't fix the kind-of issue but does fix the other two alerts attached to this repo. Kind of is a lot messier so I think we can hold out a few more days to see if big maintainers like storybook and FB will fix their deps. It cleans up yarn audit output a ton.

minimist

security advisory: Prototype pollution in minimist

came from gonzales-pe pkg via postcss-sass

I could have fixed by patching yarn.lock alone, but used the force upgrade method for explictness here. See my work in b8e6805, c0f0a82, and 791b1e2.

acorn

security advisory: Regular Expression Denial of Service in Acorn

TBH, came from bad yarn logic. This is a long-time bug in yarn which happens from time to time: our dependency tree had no reason to have both 6.4.0 and 6.4.1, but yarn isn't great at cleaning these dupes. So I manually patched yarn.lock in fac6225.

reviewer notes

opened this based on yarn audit alone, so please check to ensure I didn't break the build? :)

yarn audit output-- before my changes (394 vulnerabilities)

Screen Shot 2020-04-05 at 6 31 40 PM

yarn audit output-- after my changes (1 vulnerability)

this last vulnerability isn't from kind of. a closed issue on storybook claims this is already fixed, but upgrading to 5.3.18 did not resolve this. I am still bored so may open a PR to upgrade marksy & then can follow up to upgrade storybook if nobody else catches it.

Screen Shot 2020-04-05 at 6 32 50 PM

@sojeri sojeri added type: dependencies Maintaining 3rd-party dependencies type: security Pull requests that address a security vulnerability labels Apr 5, 2020
@suzubara
Copy link
Contributor

suzubara commented Apr 6, 2020

I don't feel comfortable committing manual fixes directly to the yarn.lock file -- IMO they will be too easily lost and it's not clear enough what versions we're requiring and why. I found this yarn feature that sounds like a good enough alternative: https://classic.yarnpkg.com/en/docs/selective-version-resolutions/

@sojeri
Copy link
Contributor Author

sojeri commented Apr 6, 2020

@suzubara TIL! I will add those to package.json 👍

@@ -11179,7 +11164,7 @@ stylelint@^13.2.1:
lodash "^4.17.15"
log-symbols "^3.0.0"
mathml-tag-names "^2.1.3"
meow "^6.0.1"
meow "^6.1.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@gidjin gidjin left a comment

Choose a reason for hiding this comment

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

Thanks for making these updates.

@gidjin gidjin merged commit 951c9f2 into develop Apr 6, 2020
@gidjin gidjin deleted the sojeri/resolve-security-advisories branch April 6, 2020 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: dependencies Maintaining 3rd-party dependencies type: security Pull requests that address a security vulnerability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants