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

Post npm install audit is bad #1512

Open
soryy708 opened this issue Oct 23, 2019 · 17 comments
Open

Post npm install audit is bad #1512

soryy708 opened this issue Oct 23, 2019 · 17 comments

Comments

@soryy708
Copy link
Contributor

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
Copy link
Member

ljharb commented Oct 23, 2019

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

@soryy708
Copy link
Contributor Author

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

@ljharb
Copy link
Member

ljharb commented Oct 23, 2019

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
Copy link
Member

ljharb commented Oct 23, 2019

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
Copy link
Contributor Author

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
Copy link
Member

ljharb commented Oct 23, 2019

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

@soryy708
Copy link
Contributor Author

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

@soryy708
Copy link
Contributor Author

soryy708 commented Oct 23, 2019

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
Copy link
Member

ljharb commented Oct 23, 2019

Ah, yes, aud doesn't handle file: deps. Filed ljharb/aud#2 for that.

@ljharb
Copy link
Member

ljharb commented Oct 23, 2019

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

@soryy708
Copy link
Contributor Author

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

@ljharb
Copy link
Member

ljharb commented Oct 23, 2019

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

@soryy708
Copy link
Contributor Author

What node & npm versions is this built with?

@ljharb
Copy link
Member

ljharb commented Oct 23, 2019

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

@soryy708
Copy link
Contributor Author

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
Copy link
Member

ljharb commented Oct 23, 2019

aud does that, yes

@ljharb
Copy link
Member

ljharb commented Jul 31, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants