alexbrazier / simple-update-notifier

Simple update notifier to check for npm updates for cli applications
MIT License
16 stars 9 forks source link

Bump semver version to avoid audit errors for users of the library #19

Closed dzek69 closed 1 year ago

dzek69 commented 1 year ago

Hello, semver is currently locked to a set of versions that are vulnerable to a DOS attack. Currently if somebody uses simple-update-notifier and has a rule that prevents them from releasing/deploying if there is any audit warning they are blocked.

This PR bumps semver version (tests are passing) and bumps jest version (because I could not run tests on current node version without the upgrade). There are still yarn audit warnings when developing this lib - but I'll leave that do you. The dependencies section is what matters to end-users.

Best regards, Jacek

alexbrazier commented 1 year ago

Hi Jacek, thanks for raising the PR. I remember when first setting this up I had explicitly reverted the version to 7.0.0 to support node 8 https://github.com/alexbrazier/simple-update-notifier/pull/6

Probably need to look into this issue a bit more to work out the best solution, but might need to do a major version bump to support this unless there is some other workaround

dzek69 commented 1 year ago

While I understand your concerns and I also try to support everything old as long as possible - I always prefer security.

Node 8 security support ended 3,5 years ago. Node 16 is the oldest supported LTS version, but it only has 2,5 months of security support anyway.

It's really time to move on, sacrificing security of everyone* to support few ones that decided to be insecure anway isn't the best approach. Especially when you seem to be the last one (of Node & semver) to support such old version :)

* - I know, your use of semver probably can't really affect anyone.

PS. All this is a little ironic when discussed on a page of a library that helps keeping stuff up to date :)

wellwelwel commented 1 year ago

As a complement, someone who decides to use Node 8 needs to make decisions because of it.

In a simpler way, update the semver version and release it as a major version of simple-update-notifier.

That could be documented at the CHANGELOG.md or in README.md like a suggestion for Node 8 users to install the version 1.1.0.


By otherwise, I think it's possible for an user who needs the Node 8 to do that manually:

I don't think a person using an unsupported version of NodeJS since 2019 is concerned about having the latest version of all the packages.

Forcing semver in version 7.0.0 #### YARN > package.json ```json "resolutions": { "**/semver": "~7.0.0" } ``` - Then ```sh yarn install ``` --- #### NPM > package.json ```json "resolutions": { "semver": "7.0.0" } ``` - Then ```sh npm i -D npm-force-resolutions npx npm-force-resolutions ``` --- Thus, the `simple-update-notifier` can keep security as a priority and anyone using an unsupported version of Node can change the `semver` support manually. It could be documented, for example.

Related:

wellwelwel commented 1 year ago

I tested the most common methods from semver 7.5.3, then I got "breaks" on Node 8 using:

But when I use any other method, every line works fine (including gt, the only method used in simple-update-notifier).

Since the simple-update-notifier doesn't uses any method that can breaks on Node 8, I think it don't needs a break change.

@alexbrazier, I would like your opinion about that.

alexbrazier commented 1 year ago

@wellwelwel thanks for looking into it! IIRC the updated version caused nodemon - the package this was originally created for to break with node 8. Unless we can be sure that it wont cause issues for nodemon and other packages I'd rather not make the decision on their behalf to deprecate node 8.

I'm currently leaning towards doing a major version bump, updating to the latest semver and dropping node 8. Also following this issue to see if nodemon wants to do a major version bump https://github.com/remy/nodemon/issues/2119. If this works for everyone then I can get this done fairly shortly.

alexbrazier commented 1 year ago

Fixed in version 2.0.0