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

[BUG] npm audit places false blame #506

Closed
coreyfarrell opened this issue Nov 20, 2019 · 3 comments
Closed

[BUG] npm audit places false blame #506

coreyfarrell opened this issue Nov 20, 2019 · 3 comments
Labels
Bug thing that needs fixing Needs Discussion is pending a discussion

Comments

@coreyfarrell
Copy link

What / Why

npm audit reports a list of vulnerable packages per package-lock.json. This results in spam to maintainers as the audit report places frequently false blame for installing the vulnerable version.

When

package-lock.json was created before a vulnerability fix was released it blocks update to an in-range vulnerability fix.

Where

  • n/a

How

Current Behavior

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Arbitrary Code Execution                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ handlebars                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ istanbul-reports                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ istanbul-reports > handlebars                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1316                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

Steps to Reproduce

You can clone https://github.com/istanbuljs/nyc and check-out the v14.1.1 tag which has an old package-lock.json, run npm audit

Expected Behavior

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Arbitrary Code Execution                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ handlebars                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Installed by  │ package-lock.json                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1316                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

This reflects the fact that the source of the vulnerability is an outdated package-lock.json which is blocking an in-range update. My view is that istanbul-reports should not be named by npm audit unless the vulnerability fix is out of range, for example if istanbul-reports depends on handlebars@^4 but fixing requires upgrade to handlebars@5.0.0). The other exception is bundled dependencies, if a vulnerable version is pulled in due to bundling then it is proper to name the package which did the bundling.

Who

References

@ljharb
Copy link
Contributor

ljharb commented Nov 20, 2019

This is a great catch; i don’t use lockfiles on published projects so haven’t run into it, but the range, not the locked version, is the only thing that should matter on a published project.

I don’t think npm can avoid having a way to differentiate apps from packages, tbh :-/

@ying-rao
Copy link

ying-rao commented Jan 3, 2020

I think I am seeing the same issue. npm audit is reporting vulnerability in an older version of package open. The more info link is https://snyk.io/vuln/npm:open:20180512 and it mentions it's only for version <6.0.0, while the installed version is 6.4.0.

@darcyclarke
Copy link
Contributor

npm v6 is no longer in active development; We will continue to push security releases to v6 at our team's discretion as-per our Support Policy.

If your bug is preproducible on v7, please re-file this issue using our new issue template.

If your issue was a feature request, please consider opening a new RRFC or RFC. If your issue was a question or other idea that was not CLI-specific, consider opening a discussion on our feedback repo

Closing: This is an automated message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Discussion is pending a discussion
Projects
None yet
Development

No branches or pull requests

5 participants