Skip to content

Commit decd033

Browse files
authored
fix: use path-based lookup for direct parent when same package installed at multiple versions (#1051)
When the same package (e.g. eslint) is installed at multiple versions in the dependency tree, the packagesByName Map in resolveDirectParentContext kept only the last entry (last-write-wins). If the nested version appeared last, it became directParentVersion, causing the candidate filter to admit versions that are actually downgrades from the root installation (e.g. eslint@6.15.0 when eslint@9.36.0 is installed at the root). Fix: use findPackageAlongPath with the path prefix up to and including the direct parent position in viaPath, falling back to the existing Map lookup. Path-based iteration finds the specific installed instance on the finding's dependency path rather than an arbitrary same-name package. Regression test added for the eslint@9.36.0 / eslint@6.14.0 collision case.
1 parent a9072cb commit decd033

2 files changed

Lines changed: 68 additions & 1 deletion

File tree

src/remediation/parent-upgrade.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,15 @@ function resolveDirectParentContext(
110110
const directParentName = directParentCandidates[0];
111111
if (!directParentName) return null;
112112

113-
const directParent = findDirectDependency(packages, directParentName, directDependencyNames, packagesByName);
113+
// Path-based lookup disambiguates when the same package name is installed at
114+
// multiple versions (e.g. eslint@9.36.0 at root AND eslint@6.14.0 nested).
115+
// A name-keyed Map (last-write-wins) would return an arbitrary version, making
116+
// directParentVersion wrong and potentially producing downgrade suggestions.
117+
const directParentIdx = viaPath.indexOf(directParentName);
118+
const directParentPathPrefix = viaPath.slice(0, directParentIdx + 1);
119+
const directParent =
120+
findPackageAlongPath(packages, directParentName, directParentPathPrefix) ??
121+
findDirectDependency(packages, directParentName, directDependencyNames, packagesByName);
114122
if (!directParent) return null;
115123

116124
return { directParentName, immediateParentName, directParent };

tests/parent-upgrade.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,6 +1516,65 @@ describe("resolveRecommendedParentUpgrade", () => {
15161516
});
15171517
expect(result?.reason).toContain("could not be verified");
15181518
});
1519+
1520+
it("uses the root direct-dep version, not a nested same-name installation, as the currentVersion baseline when filtering upgrade candidates", async () => {
1521+
// Regression test for the packagesByName Map collision on the DIRECT parent itself.
1522+
//
1523+
// Graph: project -> eslint@9.36.0 -> eslint-utils@1.4.3 (vulnerable path)
1524+
// project -> some-tool -> eslint@6.14.0 (nested, unrelated path)
1525+
//
1526+
// packagesByName is built as new Map(packages.map(p => [p.name, p])).
1527+
// Because eslint appears twice and 6.14.0 is last in the array, the map
1528+
// holds eslint -> 6.14.0. findDirectDependency then returns eslint@6.14.0
1529+
// as the direct parent, so directParentVersion = "6.14.0". The candidate
1530+
// filter compareVersions(version, "6.14.0") > 0 admits eslint@6.15.0,
1531+
// producing a downgrade suggestion from the actually-installed 9.36.0.
1532+
//
1533+
// After the fix: path-based lookup must return eslint@9.36.0, so only
1534+
// candidates > 9.36.0 (i.e. eslint@10.0.0) are considered.
1535+
const resolveRecommendedParentUpgrade = await loadResolver();
1536+
1537+
mockPackumentsByPackage(fetchMock, {
1538+
eslint: {
1539+
versions: {
1540+
"6.14.0": { dependencies: { "eslint-utils": "^1.4.0" } },
1541+
"6.15.0": { dependencies: { "eslint-utils": "^1.4.4" } },
1542+
"9.36.0": { dependencies: { "eslint-utils": "^1.4.0" } },
1543+
"10.0.0": { dependencies: { "eslint-utils": "^1.4.4" } },
1544+
},
1545+
},
1546+
});
1547+
1548+
const packages: PackageRef[] = [
1549+
// Root eslint@9.36.0 - the actual direct dep on the finding's path.
1550+
{ name: "eslint", version: "9.36.0", ecosystem: "npm", paths: [["project", "eslint"]] },
1551+
{ name: "eslint-utils", version: "1.4.3", ecosystem: "npm", paths: [["project", "eslint", "eslint-utils"]] },
1552+
// Nested eslint@6.14.0 - appears LAST so it wins the packagesByName Map,
1553+
// causing findDirectDependency to return the wrong version when the bug is present.
1554+
{ name: "eslint", version: "6.14.0", ecosystem: "npm", paths: [["project", "some-tool", "eslint"]] },
1555+
];
1556+
1557+
const finding = createFinding({
1558+
pkg: { name: "eslint-utils", version: "1.4.3", ecosystem: "npm", paths: [["project", "eslint", "eslint-utils"]] },
1559+
dependencyPaths: [["project", "eslint", "eslint-utils"]],
1560+
firstFixedVersion: "1.4.4",
1561+
});
1562+
1563+
const result = await resolveRecommendedParentUpgrade(finding, packages, new Set(["eslint"]));
1564+
1565+
// Must reflect the root installation version, not the nested one.
1566+
expect(result?.currentVersion).toBe("9.36.0");
1567+
// eslint@6.15.0 is a downgrade from 9.36.0 and must never be suggested.
1568+
expect(result?.targetVersion).not.toBe("6.15.0");
1569+
// The only valid upgrade above 9.36.0 is 10.0.0.
1570+
expect(result).toMatchObject({
1571+
package: "eslint",
1572+
currentVersion: "9.36.0",
1573+
targetVersion: "10.0.0",
1574+
vulnerablePackage: "eslint-utils",
1575+
confidence: "verified",
1576+
});
1577+
});
15191578
});
15201579

15211580
describe("resolveHighestSatisfying", () => {

0 commit comments

Comments
 (0)