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 vulnerability #75

Merged
merged 6 commits into from
Aug 1, 2018
Merged

Fix vulnerability #75

merged 6 commits into from
Aug 1, 2018

Conversation

clairesunstudio
Copy link
Contributor

@clairesunstudio clairesunstudio commented Jul 16, 2018

screen shot 2018-07-16 at 3 34 39 pm
auto fix npm vulnerabilities
To test: run npm audit

before [patternlab]
screen shot 2018-07-16 at 3 45 46 pm

after [patternlab]
screen shot 2018-07-17 at 9 41 43 am

before [react]
screen shot 2018-07-17 at 12 17 07 pm

after [react]
screen shot 2018-07-17 at 12 17 31 pm

@rbayliss
Copy link
Member

Hmm, we've been using yarn to manage things on the Patternlab side. Without a new yarn.lock file, I don't think it will invalidate our caching in Circle to use the new packages. Do you want me to take care of that, or can you?

@clairesunstudio
Copy link
Contributor Author

@rbayliss yes please! feel free to take over!

@rbayliss
Copy link
Member

This actually isn't possible to fix at the moment - we're waiting on an upstream update in nodejs/node-gyp#1471. Once that's done, we can run yarn upgrade inside patternlab/styleguide to fix the issue. In the meantime, I'm getting rid of our package-lock.json, which is outdated anyway.

@mrossi113
Copy link
Contributor

@rbayliss do you want me to review this PR? So we are removing the package-lock.json in patternlab and updating the package.json/package.lock in react. Do we need to monitor for an upstream update in nodejs/node-gyp#1471 somewhere (maybe in the mayflower/issues?)

@rbayliss
Copy link
Member

@mrossi113 - Let's leave it for now. It's still in progress, since we haven't updated all of the vulnerable packages yet.

@mrjmd
Copy link
Contributor

mrjmd commented Jul 17, 2018

The nwb vulnerability looks legit, the update here looks good so far from the React component library side.

@rbayliss
Copy link
Member

@mrjmd - is it enough of an issue that we should merge what's been done so far and follow up to get the other vulnerability later on?

@rbayliss
Copy link
Member

rbayliss commented Aug 1, 2018

I just checked back in on this, and the hoek vulnerability still isn't fixed. It seems like we're in for a long wait here as upstream packages update things in response to the new npm audit stuff and github warnings. I think we might just want to merge this as long as this build comes back green. It won't fix all of our problems, but it will fix the majority, and we can follow up later on with another update.

On a side note, babelify needs an update on the Patternlab side. This turns out to be complicated, since babel's made a lot of progress since Mayflower got started. I'm gonna create a separate ticket just for that.

Copy link
Member

@rbayliss rbayliss left a comment

Choose a reason for hiding this comment

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

@clairesunstudio - This looks good to me. I'm assuming we can go ahead and merge it now? See my comments above - the hoek vulnerability still isn't fixed upstream, but I don't think we should wait on it any longer.

@rbayliss rbayliss merged commit 87a5d95 into develop Aug 1, 2018
@rbayliss rbayliss deleted the fix-vulnerability branch August 1, 2018 20:58
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.

4 participants