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 #660

Closed
wants to merge 1 commit into from
Closed

Conversation

chrispotter
Copy link

Impact: minor
Type: chore

Issue

Better security update with upgraded next package. Tests wouldn't run locally so PR for CircleCI testing.

@chrispotter
Copy link
Author

Hmm, looks like upgrading next introduced errors. Back to the drawing board, suggestions welcome...

@focusaurus
Copy link
Contributor

If the security fixes aren't backported to next v7 then we can file a bug and have storefront devs handle upgrading next as broader maintenance work.

@HarisSpahija
Copy link
Collaborator

I would like to pick this up, could you maybe shed some more light on the errors you encountered? @chrispotter

@chrispotter
Copy link
Author

@HarisSpahija here are the vulns we've had reported through next.js. Snyk recommends that we update to [email protected]

Screen Shot 2020-03-03 at 7 19 00 AM

Screen Shot 2020-03-03 at 7 19 11 AM

Screen Shot 2020-03-03 at 7 19 20 AM

@HarisSpahija
Copy link
Collaborator

Best way seems to just upgrade the NextJS platform right? I am not sure if security fixes will ever be backported to v7 since we are on Next v9 atm. I have rarely seen them backporting fixes to more than 1 major patch.

Is it okay to take over this PR or create a new PR and update the NextJS and fix the resulting erros/bugs?

@chrispotter
Copy link
Author

@HarisSpahija feel free to make a new PR or take over this one!

@HarisSpahija
Copy link
Collaborator

I currently see only unit tests failing? Did the update only introduce bugs to the tests?

Also, some items can/should be removed with the upgrade of Next to V9. For example react-helmet is no longer needed and can be replaced with Head from 'next/head. I will create separate issues with the tag ESv2. They will be part of a bigger update that I am working on.

@willopez
Copy link
Member

willopez commented Apr 1, 2020

closing in favor of #664

@willopez willopez closed this Apr 1, 2020
@mpaktiti mpaktiti deleted the chrispotter-security-vul-2-10-2020 branch April 10, 2020 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants