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: solve dependency security issues #474

Merged
merged 2 commits into from
Nov 7, 2018
Merged

fix: solve dependency security issues #474

merged 2 commits into from
Nov 7, 2018

Conversation

byCedric
Copy link
Member

Description

This is a dependency security upgrade, complementary to #473.

Motivation and Context

I did a manual security audit, with npm audit, and found 2 issues. While one of them isn't included in any commitlint package, I think it's best to still upgrade because of the security level (critical). The other issue is a minor one, also related to old Lodash, through commitizen.

root package

This one is related to docsify-cli using the insecure open library, this has been patched and refactored by opn. This change is done here.

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Critical      │ Command Injection                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ open                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ docsify-cli [dev]                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ docsify-cli > open                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/663                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

@commitlint/prompt

There were 3 issues, 2 of them easily fixable by upgrading a package (commitizen). The first two are, like said before, lodash issues through the use of another dependency. I've checked the major patch of commitlint, the only big change is the dropped support for Node <6. Luckily, commitlint doesn't support lower than 6 either, so that should be compatible.

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ lodash                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ commitizen [dev]                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ commitizen > lodash                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/577                       │
└───────────────┴──────────────────────────────────────────────────────────────┘


┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ lodash                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ commitizen [dev]                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ commitizen > inquirer > lodash                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/577                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

The other issue is related to Vorpal (and lodash again), but there is no fix (yet). Some discussion is taking place here.

┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ lodash                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.17.5                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ vorpal                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ vorpal > inquirer > lodash                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/577                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

Usage examples

How Has This Been Tested?

It might be best to search for an automatic and continuous security check.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@marionebl marionebl changed the title Fix dependency security issues fix: solve dependency security issues Nov 7, 2018
@marionebl marionebl merged commit e1ea490 into conventional-changelog:master Nov 7, 2018
marionebl pushed a commit that referenced this pull request Nov 7, 2018
fix: solve dependency security issues
escapedcat pushed a commit that referenced this pull request Nov 16, 2018
* fix: use correct label for failing empty subjects, see #476

* chore: budge to husky api change

* docs: use https for all external links in readme

* Fix dependency security issues (#474)

fix: solve dependency security issues

* chore: add global watch task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants