sarbbottam / eslint-find-rules

Find built-in ESLint rules you don't have in your custom config
http://npm.im/eslint-find-rules
MIT License
201 stars 33 forks source link

chore: upgrade `yargs` to latest version #314

Closed sturdynut closed 2 years ago

sturdynut commented 4 years ago

This change should fix a security vulnerability (mem v^1.1.0) from the version of yargs used by this project. Here is the warning we are getting from our repo that uses eslint-find-rules:

image

Here's the chain of dependencies that lead to the insecure package.

I ran tests after upgrading and seemed ok, however there are quite a few breaking changes between yargs v8.0.1 and the current v14.2.0, so i've listed them below for your reference.

Breaking Changes

since v8.0.1

ljharb commented 4 years ago

supporting node 4 seems important while we support eslint versions that support node 4.

AdrieanKhisbe commented 4 years ago

Having also stumbled on the mem security altert, I was thinking about doing a Pull Request for a package refreshing of dependencies. But before I saw this PR by @sturdynut that does what I need.

I understand @ljharb, but wouldn't it possible to make a major version dropping at least the node 4 and 6 that are EOL for long.

I can give a hand if needed.

cc @sarbbottam

ljharb commented 4 years ago

Of course it’s possible - the question is, is it desirable, given that this isn’t a particularly problematic CVE?

Separately, EOL status is irrelevant; tools should support what people use.

AdrieanKhisbe commented 4 years ago

@ljharb I Totally agree your points Indeed it's noise, this mem CVE is for sure not a problematic vulnerability, even more in that context. We can live with that :)

However we can also fix it, and took the opportunity to bump many libraries of eslint-find-rules (5 out of 7 of prod dependencies can have a major update available)


About your concern about breaking support for existing user, I think it can be addressed: When a major is released, it won't break anything for existing user until they update, and They would just do it when then need, and if the meet the EOL condition for next major. The only long term impact on existing user I see, it's that it will deprive user locked on 4, 6 the eventual new feature that might be added in the feature. (unless a compatible backport is possible and made) In fact many development node libraries (and even eslintplugins more specifically) have a similar policy in practice: for instance ava, and eslint-plugin-unicorn have drop former 8 LTS on their latest major.


Tell me what your mind about this. =) _(hoping I have not been too verbose :speak_noevil:)

NextNebula commented 4 years ago

I think you should follow the supported node versions of eslint. If users need older node support, they can use a older version of this plugin. eslint 6 dropped support for node 6 (https://eslint.org/docs/user-guide/migrating-to-6.0.0) and eslint 7 dropped support for node 8 (https://eslint.org/blog/2020/05/eslint-v7.0.0-released).

Having support for very old node versions while eslint doesn't even support them is less important then all users of this plugin having 2 npm audit messages.

ljharb commented 3 years ago

@NextNebula we do, but we support down to eslint 3, so we support whatever nodes any of eslint 3-7 support.

npm audit messages are almost always false positives; they're unimportant.

ljharb commented 2 years ago

Closed by 1e940a9 / #337.