Consider updating dependency versions

[jmrossy]

Hi, thanks for the useful plugin!

I noticed the versions of some of this package's dependencies are very old. For example, the read-pkg-up version is from 4 years ago: https://github.com/benmosher/eslint-plugin-import/blob/master/package.json#L114

This can lead to insecure transitive dependencies being brought in for users of this plugin.

For example: eslint-plugin-import > read-pkg-up > read-pkg > normalize-package-data > hosted-git-info@2, which recently had a security alert.

[ljharb]

We support eslint 2, and all the node versions it supports, so I’m pretty sure everything is as updated as it could possibly be.

[jmrossy]

We support eslint 2, and all the node versions it supports, so I’m pretty sure everything is as updated as it could possibly be.

Ah I didn't realize compat for old eslint versions went so far back. That helps explain it, thanks! Would it make sense to to a major version split and drop support for very old eslint versions at some point? Of course you've probably already thought about that, I'm just curious.

[ljharb]

There's not much of a benefit to doing that. Most CVEs are false positives anyways, especially in the eslint ecosystem.

For the one you referenced, see #2047.

[chyzwar]

pkg-dir@^2.0.0:
  version "2.0.0"
  resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-2.0.0.tgz#f6d5d1109e19d63edf428e0bd57e12777615334b"
  integrity sha1-9tXREJ4Z1j7fQo4L1X4Sd3YVM0s=
  dependencies:
    find-up "^2.1.0"

pkg-dir@^3.0.0:
  version "3.0.0"
  resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-3.0.0.tgz#2749020f239ed990881b1f71210d51eb6523bea3"
  integrity sha512-/E57AYkoeQ25qkxMj5PBOVgF8Kiu/h7cYS30Z5+R7WaiCCBfLq58ZI/dSeaEKb9WVJV5n/03QwrN3IeWIFllvw==
  dependencies:
    find-up "^3.0.0"

pkg-dir@^4.1.0, pkg-dir@^4.2.0:
  version "4.2.0"
  resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-4.2.0.tgz#f099133df7ede422e81d1d8448270eeb3e4261f3"
  integrity sha512-HRDzbaKjC+AOWVXxAU/x54COGeIv9eb+6CkDSQoNTt4XyWoIJvuPsXizxu/Fr23EiekbtZwmh1IcIG/l/a10GQ==
  dependencies:
    find-up "^4.0.0"

pkg-dir@^5.0.0:
  version "5.0.0"
  resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-5.0.0.tgz#a02d6aebe6ba133a928f74aec20bafdfe6b8e760"
  integrity sha512-NPE8TDbzl/3YQYY7CSS228s3g2ollTFnc+Qi3tqmqJp9Vg2ovUpixcJEo2HJScN2Ez+kEaal6y70c0ehqJBJeA==
  dependencies:
    find-up "^5.0.0"

There's not much of a benefit to doing that. Most CVEs are false positives anyways, especially in the eslint ecosystem.

There are multiple issues with this approach.

1. Increases node_modules size by installing multiple version of same dependencies. There is 10milion of download weekly of eslint-plugin-import pull countless old packages.
2. Big companies have integrated security scanners into CI/CD systems. It is not fun to explain to your superiors why your project is "yellow"
3. It actually slows down ESLint process. There is time needed to require all these additional packages.

• https://npm.anvaka.com/#/view/2d/eslint-plugin-import 74 nodes
• https://npm.anvaka.com/#/view/2d/eslint 113 nodes

[ljharb]

node_modules size is irrelevant; disk space is infinite and free.

Yes, i understand that the fact that 99% of CVEs are false positives makes security scanners inconvenient. The solution isn’t to make one-off changes in specific deps, it’s to fix the security industry, or fix your metrics for what’s acceptable.

I would be very shocked if cold start require time mattered to eslint performance significantly. Either way, updating deps would have zero impact on this cost.

[chyzwar]

node_modules size is irrelevant; disk space is infinite and free.

It is not true. https://paulmillr.com/posts/chokidar-3-save-32tb-of-traffic/

Eslint alone:

$ du -sh ./node_modules/
18M     ./node_modules/

Eslint and eslint-plugin-import

du -sh ./node_modules/
28M     ./node_modules/

10MB is a non-trivial amount.

I would be very shocked if cold start require time mattered to eslint performance significantly. Either way, updating deps would have zero impact on this cost.

eslint is used as IDE/Editor plugin, pre commit hook, cli in CI/CD, webpack loader etc.

On Project with 3 source files, without any imports
Without eslint-plugin-import

yarn run v1.22.10
$ eslint src
Done in 0.40s.

real    0m0,608s
user    0m0,538s
sys     0m0,069s

With eslint-plugin-import

$ time yarn eslint
yarn run v1.22.10
$ eslint src
Done in 0.50s.

real    0m0,702s
user    0m0,639s
sys     0m0,103s

Just loading this plugin would add 100ms to startup time of eslint. Let's estimate that this plugin is loaded 1milion times daily.
This ((0.1 * 1000000 * 365) / 3600) / 24 would account 422 days globally being lost due this plugin.

Yes, i understand that the fact that 99% of CVEs are false positives makes security scanners inconvenient. The solution isn’t to make one-off changes in specific deps, it’s to fix the security industry, or fix your metrics for what’s acceptable.

You do not understand. In big companies, scanners are mandated and often every exception need to be approved by a human.
Scanners are net positive because they force you to update dependencies and properly maintain your application.

We are not an asking to micro optimize this plugin for require times. Just try to follow best practices and update dependencies on regular basis via automated dependabot/renovate.

[ljharb]

I do understand; i work in big companies. The only solution is to fix such naive mandating, and persuade your security teams to recognize that the cost of false positives is much higher than the cost of false negatives.

We do follow best practices - all our deps are as updated as they can be.

If you’d like to ensure that those deps follow best practices and backport security fixes to older major lines, then we’d be able to update to those.

[YodaDaCoda]

@ljharb I have to question why support for ESLint v2 is so important. ESLint v3 was released in 2016 which dropped support for node < 4. The current v7 release of ESLint supports node =v10 and >=v12. Node versions <=v11 are end-of-life, so even the current release of ESLint supports an end-of-life version of node, which should be plenty of historical support. You're essentially supporting a version of node that was marked end-of-life over four and a half years ago.

[ljharb]

EOL status and date is irrelevant.

[mccraveiro]

@ljharb that's it? No future updates just to keep the support for EOL versions of ESLint? As pointed above, projects like this need to be kept up to date for a lot of reasons. Your decision is making me rethink the usage of this package on our project.

[ljharb]

@mccraveiro Nobody said "no future updates".

The problem is that these specific dependencies decided to be hostile to their consumers, and unnecessarily drop support of older node versions. The future update will be removing these dependencies and replacing them with friendlier ones.

Most CVEs are false positives; literally every single one of them this issue refers to 100% does not apply to this plugin's usage of the packages. If you insist on blindly treating every CVE as a commandment not to use the package, then you'll rapidly find yourself running out of ecosystem packages to use.

[mccraveiro]

The future update will be removing these dependencies

That's a fine solution for this issue. However you closed this issue without a resolution or an explanation about this roadmap. That's much more reassuring and friendly. Thanks for the update!

[ljharb]

@mccraveiro Fair enough :-) and np, happy to elaborate whenever needed.

This particular category of problem is exceedingly noisy and one of the primary causes of maintainer burnout, so my temper's a bit short with it. See https://overreacted.io/npm-audit-broken-by-design/