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

Dependency Management Part I #103

Closed
aorinevo opened this issue Apr 6, 2020 · 10 comments
Closed

Dependency Management Part I #103

aorinevo opened this issue Apr 6, 2020 · 10 comments

Comments

@aorinevo
Copy link
Collaborator

aorinevo commented Apr 6, 2020

Introduction

Shepherd has a number of advisory exceptions when installing off of v1.4.1 ranging from low to high.

(544 low, 14 moderate, 85 high)

Proposed Changes

Update dependencies to reduce vulnerabilites

Strategy

Phase I

  • Packages
    • simple-git
    • ora
    • @types/ora
    • @types/joi -> @types/hapi__joi
    • joi -> @hapi/joi
  • CI changes
    • drop support for Node 6; support Node 8, 10, and 12

Phase II

  • chalk
  • child-process-promise (already on latest)
  • js-yaml
  • lodash
  • log-symbols

Phase III

  • Packages
    • commander
    • fs-extra
    • fs-extra-promise
    • netrc
  • Audit
    • run npm audit fix

Notes

  • Breaking out remaining deps into a separate issue as those will require large changes.

Screenshots

Screen Shot 2020-04-05 at 8 39 45 PM

Screen Shot 2020-04-05 at 8 40 12 PM

Screen Shot 2020-04-05 at 8 40 20 PM

@parshap
Copy link
Contributor

parshap commented Apr 6, 2020

Thanks for the report! Do you want to send a PR to upgrade these dependencies?

@aorinevo
Copy link
Collaborator Author

aorinevo commented Apr 6, 2020

Absolutely, I’ve already started. Should have it ready for review in the next couple of days.

@aorinevo
Copy link
Collaborator Author

aorinevo commented Apr 8, 2020

Phase I (see #106):

  • update simple-git from v1.104.0 to v1.132.0 (see steveukx/git-js@v1.104.0...v1.132.0)
    • Github client (not sure why we aren't using octokit here)
  • update ora from v2.1.0 to v4.0.3 (see sindresorhus/ora@v2.1.0...v4.0.3)
  • update @types/ora from v1.3.4 to v3.2.0 (see https://www.npmjs.com/package/@types/ora)
    • Note that ora provides its own type definition
    • Should we remove this then? (@parshap)
  • update joi
    • installation via joi is deprecated as of v14.3.1 in favor of @hapi/joi
    • install @hapi/joi v
    • while a major version bump, joi is minimially used (see
    • replace @types/joi with @types/hapi__joi
    • updated code and unit test to reflect delta in dep code

Notes

  • tests pass
  • tested locally on a couple of code migrations
  • spread operator for objects is only supported for Node v8+
    /home/travis/build/NerdWalletOSS/shepherd/node_modules/@hapi/joi/lib/trace.js:203
            internals.debug(state, { type: source, ...value });
                                                   ^^^
    SyntaxError: Unexpected token ...

@aorinevo
Copy link
Collaborator Author

aorinevo commented Apr 8, 2020

Phase II (see #105):

Notes

  • tests pass
  • tested locally on a couple of code migrations

@aorinevo
Copy link
Collaborator Author

aorinevo commented Apr 8, 2020

Phase III (see #107):

  • update commander from v2.18.0 to v5.0.0
  • update fs-extra from v6.0.1 to v9.0.0
    -- update @types/fs-extra from v5.0.4 to v8.1.0
  • fs-extra-promise(already on latest)
    -- update @types/fs-extra-promise from v1.0.7 to v1.0.8
  • netrc (already on latest)
  • Run npm audit fix twice to resolve all but 206 low severity vulnerabilities
found 206 low severity vulnerabilities in 18033 scanned packages
  run `npm audit fix` to fix 144 of them.
  62 vulnerabilities require semver-major dependency updates.

Notes

@nerdwallet/shepherd@1.4.1 /Users/aorinevo/Repositories/nerdwallet/shepherd
└─┬ jest@23.6.0
  └─┬ jest-cli@23.6.0
    └─┬ istanbul-api@1.3.7
      └─┬ istanbul-reports@1.5.1
        └── handlebars@4.5.3

@aorinevo aorinevo changed the title Dependency Management Dependency Management Part I Apr 8, 2020
@aorinevo
Copy link
Collaborator Author

aorinevo commented Apr 8, 2020

@parshap, PR #107 is ready for review.

@aorinevo
Copy link
Collaborator Author

aorinevo commented Apr 8, 2020

Screenshots (after changes):
Screen Shot 2020-04-08 at 4 50 02 PM
Screen Shot 2020-04-08 at 4 50 47 PM

parshap pushed a commit that referenced this issue Apr 8, 2020
* chore: update deps phase 1 as described in issue #103.

* fix: linting errors.

* chore: update deps phase II as described in issue #103.

* break: remove support for Node 6, add node 12 to integration testing.

* chore: run npm audit fix
@aorinevo
Copy link
Collaborator Author

aorinevo commented Apr 8, 2020

This is is closed by way of #107

@aorinevo aorinevo closed this as completed Apr 8, 2020
@parshap
Copy link
Contributor

parshap commented Apr 9, 2020

@aorinevo are the other prs you have open to upgrade dependencies ready for review as well?

@aorinevo
Copy link
Collaborator Author

aorinevo commented Apr 9, 2020

I closed the other PRs as they were incremental steps towards PR #107 which closed Part I of the dependency management.

Next I’m going to tackle the jest dependencies but that will require some changes in the unit tests. Should be ready by EOD tomorrow.

Note that once jest update is complete, vulnerabilities will drop to zero :)

jugaltheshah pushed a commit to jugaltheshah/shepherd that referenced this issue Jan 21, 2021
* chore: update deps phase 1 as described in issue NerdWalletOSS#103.

* fix: linting errors.

* chore: update deps phase II as described in issue NerdWalletOSS#103.

* break: remove support for Node 6, add node 12 to integration testing.

* chore: run npm audit fix
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

No branches or pull requests

2 participants