fix: verify parent upgrades actually resolve the vulnerable package before recommending them#899
Merged
Merged
Conversation
…commending them Rewrite findUpgradeForImmediateIntermediate to three honest outcomes: walk the vulnerable path through a candidate parent's declared dependency ranges and only recommend (verified) when the terminal resolves to a fixed version or is dropped; return null when no candidate resolves it; fall back to an explicitly unverified heuristic only when the path cannot be resolved. Fixes the false parent-upgrade recommendation from issue #896. Refs #896
…fidence contamination Two unrelated findings that recommend upgrading the same package merge into one SuggestedFixTarget, so the blended target confidence must not decide coverage. A direct (or verified) fix sharing a target package with an unrelated unverified parent upgrade was wrongly dropped from coveredFindingCount. Coverage is now evaluated from each finding's own recommendedParentUpgrade confidence. Refs #896
…r brace-expansion Adds a jest -> @jest/core -> minimatch -> brace-expansion regression where every jest version still resolves brace-expansion below the fix, asserting the verified resolver returns null instead of the old false jest@30.4.2 recommendation. Refs #896
…ped-dep reason wording
Address final-review minor notes: add a comment that a 'verified' parent upgrade
is verified against declared ranges (not npm install-level dedup/hoisting), and
render a cleaner reason for the dropped-dependency case ('no longer pulls X on
this path') instead of the grammatically odd 'resolves X to not present, at or
above the fix'. Also fixes a stale 'best-effort' test comment.
Refs #896
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The remediation resolver recommended a parent-dependency upgrade that did not actually resolve the vulnerable transitive package to a fixed version, and overstated it ("Running all commands above should fix all N findings"). Concrete case: it recommended
jest@30.4.2for abrace-expansionfinding even though jest@30.4.2 still pulls the vulnerable brace-expansion viaglob -> minimatch.The deep-chain resolver now verifies along the dependency path: for each candidate parent version it walks the vulnerable path, resolving each declared range to its highest published version, and only recommends the upgrade (confidence
verified) when the terminal package resolves to a fixed version or is dropped from the path. When no candidate resolves it, the resolver returns nothing instead of a false recommendation; when the path cannot be resolved, it falls back to an explicitlyunverifiedsuggestion. Unverified parent upgrades are marked in the terminal and HTML output and no longer count toward the "should fix all findings" claim.Verified against the repo's own brace-expansion finding: the CLI no longer prints a false
jest@30.4.2fix, showing an honest advisory-hint-only result instead.Closes #896