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

update package: update-notifier, fixes package vulnerability #387

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kfurmann
Copy link

Vulnerability:
High Prototype Pollution
Package dot-prop
Patched in >=5.1.1
update-notifier > configstore > dot-prop

@lucasmbrown
Copy link

I'm not sure who the maintainers are of this project -- @kfurmann @Splaktar @LinusU ? -- but would be great to merge this to fix the security vulnerability. Appreciate your help, if you're looking for more maintainers I'm happy to help!

@@ -262,15 +310,13 @@
"version": "1.1.0",
"resolved": "https://registry.npmjs.org/arr-flatten/-/arr-flatten-1.1.0.tgz",
"integrity": "sha512-L3hKV5R/p5o81R7O02IGnwpDmkp6E982XhtbuwSe3O4qOtMMMtodicASA1Cny2U+aCXcNpml+m4dPsvsJ3jatg==",
"dev": true,
"optional": true

Choose a reason for hiding this comment

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

Why were all of the optional fields removed? Which version of NPM did you run your install/update from?

@@ -89,7 +89,7 @@
"semver-diff": "^2.0.0",
"text-table": "^0.2.0",
"throat": "^2.0.2",
"update-notifier": "^2.1.0",

Choose a reason for hiding this comment

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

It looks like CI passed, but with this upgrading 2 major versions, there is some risk. Did you evaluate what changed in update-notifier between 2.1 and 4.1? No breaking changes that affect npm-check?

Copy link
Author

Choose a reason for hiding this comment

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

no I didn't

Copy link
Author

Choose a reason for hiding this comment

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

not really familiar with the project, just need the vulnerability gone

Copy link

@stevejay stevejay Aug 23, 2020

Choose a reason for hiding this comment

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

Just thought I'd chime in here (I'm not the submitter). I had a look at the release notes for the update-notifier package and, from my checking, I only see one breaking change of note: in v3.0.0 of update-notifier, support for node v6 was dropped, and they currently only support node v8 onwards. Maybe npm-check's Travis CI file needs to be updated to remove node versions 4 and 6 from it?

The other breaking changes are in v4.0.0 of update-notifier, but from my source code searches they do not affect npm-check:

  • "Remove the callback option". callback is a field in the options object that can be passed to updateNotifier. npm-check only uses the pkg option.
  • "Rename the boxenOpts option to boxenOptions". This is one of the options you can optionally pass to notifier.notify([options]). npm-check does not use any options with that notify call.
  • "Update boxen dependency". The release notes say that this may affect the boxenOptions values. npm-check does not use boxenOpts/boxenOptions.
  • "Disable update notifications when NODE_ENV is test". This affects testing scenarios only so should have no impact on npm-check usage.

Choose a reason for hiding this comment

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

@Splaktar -- just bumping this thread. Thanks!

@dkhunt27
Copy link
Contributor

Merged with #397

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.

5 participants