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

npm erroneously reports no longer vulnerable with multiple versions of dependency. #5741

Closed
jeffwidman opened this issue Sep 17, 2022 · 2 comments
Labels
F: security-updates 🔐 Issues specific to security updates L: javascript:npm npm packages via npm T: tech-debt ⚙️

Comments

@jeffwidman
Copy link
Member

jeffwidman commented Sep 17, 2022

An npm project is allowed to have multiple versions of the same dependency in the tree. When parsing the dependency we consolidate the different versions using the DependencySet. This retains the lowest version of the dependency seen with the expectation that the lowest version is the oldest and most likely to be vulnerable (or at least most noteworthy to call out in the update PR).

However, we've seen this can lead to confusing states when there is a vulnerability that affects some versions of the dependency but not the lowest version. In this case Dependabot incorrectly reports that no security update is needed.

Fixing this will be a bit of a lift because dependabot-core code has a number of places that assumes there will only be one version of a dependency in the tree.

Related:

@jeffwidman
Copy link
Member Author

Watching our internal metrics now that ☝️ are deployed indicates that the amount of errors attributed to this condition have fallen off pretty sharply so I think we can call this done.

A number of these jobs will still fail for various reasons further down the job pipeline, but they'll be reported as update_not_possible rather than "no longer vulnerable".

This is a step in the right direction though though as the "no longer vulnerable" error we were giving before was very confusing and misleading.

Many thanks to @bdragon and @mctofu for their hard work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: security-updates 🔐 Issues specific to security updates L: javascript:npm npm packages via npm T: tech-debt ⚙️
Projects
None yet
Development

No branches or pull requests

1 participant