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.

Sign up for GitHub

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

Jump to bottom

Fix vulnerabilities found by npm audit #5013

Merged
merged 6 commits into from
Mar 24, 2020
Merged

Fix vulnerabilities found by npm audit #5013

merged 6 commits into from
Mar 24, 2020

Conversation

cezaraugusto
Copy link
Contributor

  1. npm run test-security should pass
  2. CI should be ✅

@cezaraugusto cezaraugusto added the dependencies Pull requests that update a dependency file label Mar 23, 2020
@cezaraugusto cezaraugusto added this to the 1.5.x - placeholder milestone Mar 23, 2020
@cezaraugusto cezaraugusto self-assigned this Mar 23, 2020
@bsclifton
Copy link
Member

bsclifton commented Mar 23, 2020 •
edited
Loading

Unfortunately, storybook build fails with this. It seems to be passing on master, although there is the security error. I ran into the same problem too when trying #4998 and brave/brave-browser#8788

With my solution, I was getting errors with npm run build (see https://bravesoftware.slack.com/archives/C7VLGSR55/p1584737609008300). Your version seems to work though! 😄

If we can fix the storybook error, I think we'll be good to merge 😄

@cezaraugusto cezaraugusto force-pushed the uuln branch 2 times, most recently from 52ce98b to ec45c38 Compare March 24, 2020 00:04
@cezaraugusto
Copy link
Contributor Author

Rebased, working locally. Waiting Travis

@bsclifton bsclifton mentioned this pull request Mar 24, 2020
Closed
32 tasks
@bsclifton
Copy link
Member

Fixed the pep8 lint issue (my bad there, from #4947) and updated sync DEPS entries to use latest vulnerability free version (https://github.com/brave/sync/commits/master). Together with brave/brave-browser#8788, this should fix everything! Building locally after rebasing

@bsclifton
Copy link
Member

bsclifton commented Mar 24, 2020 •
edited
Loading

Jenkins passes completely, travis-ci only fails on npm run build-storybook... but it works great locally. Going to look into this

@bsclifton
Copy link
Member

Reproduced locally on Ubuntu 18 - will look at fix for Storybook. The brave-browser fix seems to work though

cezaraugusto
cezaraugusto commented Mar 24, 2020
View reviewed changes
package.json Outdated Show resolved Hide resolved
This was referenced Mar 24, 2020
Merged
Closed
@bsclifton
Copy link
Member

CI passed completely; will do quick follow up for travis-ci (doesn't need a full CI run)

@bsclifton bsclifton added the CI/skip Do not run CI builds (except noplatform) label Mar 24, 2020
bsclifton
bsclifton approved these changes Mar 24, 2020
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

We're good to go here! Thanks for the huge assist, @cezaraugusto! This was a rough one

@bsclifton bsclifton merged commit 32b4719 into master Mar 24, 2020
@bsclifton bsclifton deleted the uuln branch March 24, 2020 21:44
This was referenced Mar 25, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsclifton bsclifton bsclifton approved these changes

@mbacchi mbacchi Awaiting requested review from mbacchi

Assignees

@cezaraugusto cezaraugusto

Labels
CI/skip Do not run CI builds (except noplatform) dependencies Pull requests that update a dependency file
Projects
None yet
Milestone
1.8.x - Release
Development

Successfully merging this pull request may close these issues.
2 participants
@cezaraugusto @bsclifton