fix(arborist): skip re-resolution when audit fix requires semver-major bump#9543
Open
Zelys-DFKH wants to merge 2 commits into
Open
fix(arborist): skip re-resolution when audit fix requires semver-major bump#9543Zelys-DFKH wants to merge 2 commits into
Zelys-DFKH wants to merge 2 commits into
Conversation
…r bump When npm audit fix encounters a vulnerable package whose only available fix requires a semver-major version bump, #problemEdges was unconditionally treating the edge as a "problem" and scheduling dep re-resolution. This caused the latest in-range version to be installed even though it remains vulnerable, producing a spurious minor/patch update with no security benefit. Add a guard: only push the edge as a problem if vuln.fixAvailable === true (a fix exists within the current semver range) or if --force is set. The continue is preserved regardless, so there is no fallthrough to other checks. Fixes npm#9344
Adds a regression test that exercises the false branch of the fixAvailable guard added in the preceding commit: when all in-range versions of a vulnerable package are still vulnerable and the only fix requires a semver-major bump, audit fix must leave the lockfile unchanged.
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.
What
npm audit fixwas treating every edge to a vulnerable package as a "problem edge" requiring dep re-resolution, regardless of whether a fix is available within the current semver range. When the only fix requires a semver-major version bump (fixAvailableis{ isSemVerMajor: true }) or no fix exists at all (fixAvailableisfalse), the re-resolution still ran and picked the latest in-range version, which is still vulnerable. A spurious minor or patch update landed in the lockfile without fixing anything.Why
The check in
#problemEdgesinbuild-ideal-tree.jswas unconditional: every vulnerable edge got flagged as a problem, with no consultation ofvuln.fixAvailableto decide whether re-resolving would help.How
Before pushing an edge as a problem, check
vuln.fixAvailable. Only treat it as a problem iffixAvailable === true(a fix exists within the current semver range) or if--forceis set (user explicitly wants out-of-range updates). Thecontinueis preserved regardless, so there is no fallthrough to other checks.A regression test covers the case: a dependency where all
2.xversions are vulnerable and the fix requires bumping to3.0.0.audit fix(without--force) leaves the lockfile unchanged.Fixes #9344
Credit to @36degrees for the detailed report.