Skip to content

Fix update-notifier vulnerability #114

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

Conversation

alexandre-lelain
Copy link
Contributor

What this PR does

Upgrade update-notifier@^3.0.1 to update-notifier@^4.1.0.

This is a major version upgrade, but the code is not impacted by the breaking-changes listed in its changelog: https://github.com/yeoman/update-notifier/releases.

Why

This upgrade solves the following security issue from dot-prop package in versions < 5.1.1: GHSA-ff7x-qrg7-qggm.

Related issues

Solves #112.

@sy-records sy-records requested a review from anikethsaha August 5, 2020 13:14
@sy-records sy-records linked an issue Aug 5, 2020 that may be closed by this pull request
Copy link
Member

@anikethsaha anikethsaha left a comment

Choose a reason for hiding this comment

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

it looks like there are some breaking changes in the upstream/

Can you check that ?

@alexandre-lelain
Copy link
Contributor Author

Hello @anikethsaha ,

I'm not too familliar with this project so I'm afraid I do not see what you mean by "upstream" ?

@anikethsaha
Copy link
Member

I meant update-notifier. can you compare the release cause we need to check if that release is breaking ours or not

@alexandre-lelain
Copy link
Contributor Author

I see, thanks. Just like I wrote it in the PR's description, the breaking-changes of update-notifier@^4.0.0 listed in its release notes should not impact docsify-cli's codebase.

update-notifier is only used there: https://github.com/docsifyjs/docsify-cli/blob/master/lib/cli.js#L7, and the v4 doesn't seem to impact it, based on their changelog.

Also, the tests & build passed, and I tested the cli on another project. The cli's output was the same.

Copy link
Member

@anikethsaha anikethsaha left a comment

Choose a reason for hiding this comment

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

Thanks a lot

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.

Vulnerabilities from old version of dependencies
2 participants