import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.55k stars 1.57k forks source link

Post `npm install` audit is bad #1512

Open soryy708 opened 5 years ago

soryy708 commented 5 years ago

I forked this repository and ran npm install. After installing, a quick audit was performed and said:

found 54 vulnerabilities (3 low, 49 high, 2 critical)

There's no lockfile (package.lock.json) so I can't run npm audit nor npm audit fix.

ljharb commented 5 years ago

That’s fine, most CVEs are false positives anyways.

soryy708 commented 5 years ago

You're betting on chance, without in-depth analysis?

ljharb commented 5 years ago

No, I’d be happy to take a look at it, which is why the issue is still open :-)

however, this is an eslint plugin. Issues in dev deps are largely irrelevant (what does npm audit --production say?), and this project won’t ever be run in production and is configured by the user, so anything that’s like “catastrophic regex backtracking” or “prototype pollution” is simply not applicable to the whole project.

ljharb commented 5 years ago

Certainly we could also add a posttest script that runs npx aud (packages should never have a lockfile), once we figure out which warnings are real, if any.

soryy708 commented 5 years ago

npm audit --production says exactly the same as without the --production flag: "Neither npm-shrinkwrap.json nor package-lock.json found: Cannot audit a project without a lockfile"

npm install did succeed to do an audit, though.

Adding a posttest script sounds like a good idea.

ljharb commented 5 years ago

You can run npx aud --production as well, without a lockfile.

soryy708 commented 5 years ago

Note that same should be done for resolvers/node/ and resolvers/webpack/.

soryy708 commented 5 years ago

Actually npx aud --production fails because:

Could not install from "tests\files\order-redirect-scoped" as it does not contain a package.json file.

But that's false, because it does have a package.json file.

ljharb commented 5 years ago

Ah, yes, aud doesn't handle file: deps. Filed https://github.com/ljharb/aud/issues/2 for that.

ljharb commented 5 years ago

Looks like npm itself can't handle our file: dev deps ¯\_(ツ)_/¯

soryy708 commented 5 years ago

Why? It's in the docs: https://docs.npmjs.com/files/package.json#local-paths

ljharb commented 5 years ago

Presumably because it has a bug. npm install --package-lock --package-lock-only && npm audit --production complains about a malformed lockfile.

soryy708 commented 5 years ago

What node & npm versions is this built with?

ljharb commented 5 years ago

In this case, node 13.0.1 and npm 6.12.0, the latest possible of both.

soryy708 commented 5 years ago

I just tried this on an Ubuntu VM with Node v8.10.0 and npm v3.5.2 and got the exact same error, plus it said "npm is v3.5.2; we need ^6; installing npm in a temp dir..."

ljharb commented 5 years ago

aud does that, yes

ljharb commented 3 months ago

npm v10.2+ no longer requires a lockfile, so there's no need to use aud or create a lockfile.