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

Update dependencies to fix security warnings #679

Merged
merged 6 commits into from
Apr 8, 2020
Merged

Update dependencies to fix security warnings #679

merged 6 commits into from
Apr 8, 2020

Conversation

rfrm
Copy link
Contributor

@rfrm rfrm commented Apr 6, 2020 •
edited
Loading

After running npm audit fix there are still 8 vulnerabilities that have to be reviewed manually. The issue is caused by the optimist and lab libraries dependencies. The fix was to:

  1. Replace optimist with yargs.
  2. Update lab from v14.3.4 to v18.1.2

This closes #224

@rfrm rfrm force-pushed the replace-optimist-with-yargs branch 2 times, most recently from 5fd42b8 to c0579b4 Compare April 6, 2020 05:07
rfrm added 2 commits April 6, 2020 00:09
@rfrm
refactor: replace optimist with yargs
881b62a 
Signed-off-by: Robinson Rodriguez <rfrodriguez1992@gmail.com>
@rfrm
chore: upgrade lab to v15
a7b6acc 
Signed-off-by: Robinson Rodriguez <rfrodriguez1992@gmail.com>
@rfrm rfrm force-pushed the replace-optimist-with-yargs branch from c0579b4 to a7b6acc Compare April 6, 2020 05:10
@rfrm rfrm mentioned this pull request Apr 6, 2020
Closed
@rfrm
Copy link
Contributor Author

rfrm commented Apr 6, 2020

Fixes https://github.com/pagerinc/api-scheduling/issues/341

rfrm added 3 commits April 6, 2020 00:44
@rfrm
chore: update lab to v16
31b4373 
Signed-off-by: Robinson Rodriguez <rfrodriguez1992@gmail.com>
@rfrm
chore: update lab to v18
4a01ee7 
Signed-off-by: Robinson Rodriguez <rfrodriguez1992@gmail.com>
@rfrm
chore: ignore coverage.html
7ac28a4 
Signed-off-by: Robinson Rodriguez <rfrodriguez1992@gmail.com>
@aorinevo aorinevo mentioned this pull request Apr 6, 2020
Closed
@aorinevo
Copy link

aorinevo commented Apr 6, 2020

This PR would also close #674

@wzrdtales
Copy link
Member

It is passing the tests, that is a good sign, the other PR made them fail. Since you made some changes to the tests itself due to the update of lab, I will review those first.

wzrdtales
wzrdtales requested changes Apr 6, 2020
View reviewed changes
Copy link
Member

@wzrdtales wzrdtales left a comment

Choose a reason for hiding this comment

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

Looks very good, thank you so much for your contribution! Especially for taking time and care for upgrading the lab version and migrating the test over to async/await. A very welcome addition.

Please see my comments for some minor adjustments.

test/migrator_test.js Show resolved Hide resolved
test/integration/create_test.js Outdated Show resolved Hide resolved
@rfrm
chore: use Bluebird promises in test
bebfb2b 
Signed-off-by: Robinson Rodriguez <rfrodriguez1992@gmail.com>
@rfrm rfrm requested a review from wzrdtales April 6, 2020 21:23
@wzrdtales wzrdtales merged commit 94222a8 into db-migrate:master Apr 8, 2020
@BorntraegerMarc
Copy link
Contributor

@wzrdtales should a new version be released so we can update & fix our security issues?

@wzrdtales
Copy link
Member

It needs a backport to v0.11.x first. I am thinking if I can go ahead maybe with the current master tree already and am looking into what has to be done for that.

@wzrdtales
Copy link
Member

@BorntraegerMarc especially looking which breaking changes that means, if it is not feasible I will backport.

wzrdtales added a commit that referenced this pull request Apr 14, 2020
@wzrdtales
fix(vuln): backport #679
8b5beac 
Signed-off-by: Tobias Gurtzick <magic@wizardtales.com>
@wzrdtales
Copy link
Member

decided to backport now, I don't want to rush anything here. there is a new 0.11.7 out right now, let me know should there be any troubles.

@wzrdtales
Copy link
Member

to be exact, v1.0.0 or the master branch as of now is deprecating older node versions and drops support for them entirely. The backport shouldn't change anything about the support level of supported versions for 0.11.x. As of now on top of my head I don't saw another breaking change other than modernizing the code base to a newer standard. everything else (as always) I managed to broadly keep backwards compatible. However, I will go sure to check this one more time.

@BorntraegerMarc
Copy link
Contributor

@wzrdtales awesome 🎉 I updated this lib in our project & our CI passes. LGTM 🙂

@BorntraegerMarc
Copy link
Contributor

BorntraegerMarc commented Apr 15, 2020 •
edited
Loading

hehe looks like CI doesn't catch all the problems @wzrdtales you forgot to remove a debug statement in this commit: 8b5beac#diff-e4e7fada050c1319260e0dda7821fc5aR10

happens to the best of us 😉

Will you remove it & create a new version? Or should I provide a PR?

@wzrdtales
Copy link
Member

yah, removed that one on the master already, didn't realized it slipped through into v0.11.x too, i will fix that now

@wzrdtales
Copy link
Member

I will put rg console.log into my workflow for releasing^^ ce77a28#diff-b4e530c41bbf801b4ef6f4840a3b01aaL127 that one also slipped through

@wzrdtales
Copy link
Member

0.11.9 is out though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aorinevo aorinevo aorinevo left review comments

@rshah1207 rshah1207 rshah1207 approved these changes

@DiegoRBaquero DiegoRBaquero DiegoRBaquero approved these changes

@wzrdtales wzrdtales wzrdtales approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace optimist
6 participants
@rfrm @aorinevo @wzrdtales @BorntraegerMarc @DiegoRBaquero @rshah1207