[npm] fix failure to attempt parent update if unfixed transitive update is available #5799
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mctofu
changed the title
bdragon
approved these changes
Sep 28, 2022
bin/dry-run.rb
Outdated
| updated_deps.none? { |d| security_fix?(d) } | ||
| puts " (updated version is still vulnerable 🚨)" | ||
| log_conflicting_dependencies(checker.conflicting_dependencies) | ||
| next |
There was a problem hiding this comment.
I think L743-746 could use one more space of indentation.
There was a problem hiding this comment.
thanks! fixed with rubocop in 73b3e51
| "author": "", | ||
| "license": "ISC", | ||
| "dependencies": { | ||
| "@dependabot-fixtures/npm-parent-dependency-with-more-versions": "^1.0.0" |
The updater will abort before proceeding with the file update if the proposed update does not fix the vulnerability. It also triggers the conflicting dependencies check. See https://github.com/dependabot/dependabot-core/blob/8ab4d78efe7cf9a75bef76dd883d7ee3fffffb40/updater/lib/dependabot/updater.rb#L274-L282
A newer but still vulnerable version was allowed to return here which would cause the job to eventually fail because it proposed updating to a still vulnerable version. By filtering vulnerable versions here we trigger the updater to consider updating the parent to allow the transtive dep to be updated to a fixed version.
mctofu
force-pushed
the
mctofu/fix-npm-transitive-wrong-version
branch
from
73b3e51 to
65adb4a
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an issue that prevents Dependabot from trying to update a parent dependency when updating a vulnerable transitive dependency.
Ex: A vulnerability is reported in dependency B for all versions
< 2.0.0We start with:
To successfully fix this vulnerability we need to update A:
However, if Dependabot found there was a version B was allowed to update to it would propose that instead:
The end result would be the job fails as not possible because it fails the vuln fix check at
dependabot-core/updater/lib/dependabot/updater.rb
Lines 274 to 282 in 8ab4d78
This fix adds detection that the update of B to
1.0.1is still vulnerable and rejects it so we'll keep looking and find the solution that updates A. I've also tweaked the dry-run script to more closely match the updater.