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.

Sign up for GitHub

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

Jump to bottom

[BUG] npm audit backwards compatibility against third party tools #1875

Closed
doddi opened this issue Sep 29, 2020 · 3 comments
Closed

[BUG] npm audit backwards compatibility against third party tools #1875

doddi opened this issue Sep 29, 2020 · 3 comments
Assignees
@isaacs
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release
Milestone
OSS - Sprint 24

Comments

@doddi
Copy link

doddi commented Sep 29, 2020

Current Behavior:

When performing an npm audit using a third party tool that currently audits successfully when using npm cli v6, the response using npm v7 is 0 vulnerabilities.

The third party tool does not currently implement the new endpoint /-/npm/v1/security/advisories/bulk and as such the cli defaults back to using the quick audit implementation.

The quick audit response is converted to a bulk advisory but there is no vulnerable_versions entry and therefore no vulnerabilities are surfaced. Previously, vulnerable_versions was not a required entry to surface vulnerabilities.

Expected Behavior:

An npm audit using v6 and v7 should surface the same vulnerabilities or as a minimum fail safely and error if no vulnerable_versions entry is present an a response.

Steps To Reproduce:

Run an npm audit using v6/v7 cli and ensure in a quick audit response there is no vulnerable_versions entry in the response
v6: vulnerability information is surfaced
v7: no vulnerability information is surfaced

Environment:

  • OS: MacOS
  • Node: 14.2.0
  • npm: v7.0.0-beta.12 commit 9bc6966
@doddi doddi added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Sep 29, 2020
@darcyclarke darcyclarke added beta and removed Needs Triage needs review for next steps labels Oct 1, 2020
@darcyclarke
Copy link
Contributor

Thanks for catching this @doddi!

@darcyclarke darcyclarke added this to the OSS - Sprint 17 milestone Oct 6, 2020
@darcyclarke darcyclarke changed the title [BUG] Npm audit backwards compatibility against third party tools [BUG] npm audit backwards compatibility against third party tools Oct 6, 2020
@darcyclarke darcyclarke removed the beta label Oct 19, 2020
@darcyclarke darcyclarke removed this from the OSS - Sprint 19 milestone Nov 16, 2020
@isaacs
Copy link
Contributor

isaacs commented Feb 13, 2021

Hmm, so it seems like, if there's no vulnerable_versions field, then it should effectively be vulnerable_versions: "*"?

That would be quite easy to do.

isaacs added a commit to npm/metavuln-calculator that referenced this issue Feb 13, 2021
@isaacs
When an advisory lacks vulnerable_versions, use *
3df1d29 
Re: npm/cli#1875
@isaacs isaacs mentioned this issue Feb 13, 2021
Closed
isaacs added a commit to npm/metavuln-calculator that referenced this issue Feb 13, 2021
@isaacs
When an advisory lacks vulnerable_versions, use *
ab7a40f 
Re: npm/cli#1875
isaacs added a commit to npm/arborist that referenced this issue Feb 13, 2021
@isaacs
default audit severity=high, vulnerable_versions=*
9e88f87 
Fix: npm/cli#1875
Related: npm/metavuln-calculator#4
@isaacs isaacs mentioned this issue Feb 13, 2021
Closed
@darcyclarke darcyclarke added the Priority 2 secondary priority issue label Feb 13, 2021
@darcyclarke darcyclarke added this to the OSS - Sprint 24 milestone Feb 13, 2021
@darcyclarke
Copy link
Contributor

@isaacs this has definitely sat for a while, I'm going to make it a Priority 2 & pull it up into our Sprint so we at least can keep some eyes on it - maybe someone else can take it on & sync with you if you've got more context.

isaacs added a commit that referenced this issue Feb 18, 2021
@isaacs
@npmcli/[email protected]
8c36697 
* [#1875](#1875)
  [npm/arborist#230](npm/arborist#230) Set default
  advisory `severity`/`vulnerable_range` when missing from audit endpoint
  data ([@isaacs](https://github.com/isaacs))
* [npm/arborist#231](npm/arborist#231) skip
  optional deps with mismatched platform or engine
  ([@nlf](https://github.com/nlf))
* [#2251](#2251) Unpack shrinkwrapped deps
  not already unpacked ([@isaacs](https://github.com/isaacs),
  [@nlf](https://github.com/nlf))
* [#2714](#2714) Do not write package.json
  if nothing changed ([@isaacs](https://github.com/isaacs))
* [npm/rfcs#324](npm/rfcs#324) Prefer peer over
  prod dep, if both specified ([@isaacs](https://github.com/isaacs))
* [npm/arborist#236](npm/arborist#236) Fix
  additional peerOptional conflict cases
  ([@isaacs](https://github.com/isaacs))
@isaacs isaacs mentioned this issue Feb 18, 2021
Merged
@nlf nlf mentioned this issue Feb 22, 2021
Merged
@ruyadorno ruyadorno mentioned this issue Feb 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@isaacs isaacs

Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Milestone
OSS - Sprint 24
Development

Successfully merging a pull request may close this issue.

default audit severity=high, vulnerable_versions=* npm/arborist
3 participants
@isaacs @doddi @darcyclarke