fix(remediation): stop parent-update targets merging into install targets

src/remediation/fix-commands.ts

@@ -123,7 +123,7 @@ export function buildSuggestedFixCommandPlan(
       return a.pkg.name.localeCompare(b.pkg.name);
     });
 
-  const targetsByPackage = new Map<string, SuggestedFixTarget>();
+  const targetsByKey = new Map<string, SuggestedFixTarget>();
   const skippedByKey = new Map<string, SuggestedFixSkip>();
 
   const orderedFindings = [
@@ -176,7 +176,7 @@ export function buildSuggestedFixCommandPlan(
         validatedFieldOk
       ) {
         const pkgWorkspaces = workspaceMap.get(finding.pkg.name)?.filter(w => w !== ".") ?? [];
-        upsertTarget(targetsByPackage, {
+        upsertTarget(targetsByKey, finding, {
           package: finding.pkg.name,
           currentVersion: finding.pkg.version,
           targetVersion: directTarget,
@@ -244,7 +244,7 @@ export function buildSuggestedFixCommandPlan(
       finding.relationship === "transitive" &&
       finding.recommendedNpmTransitiveRemediation?.kind === "update-parent-within-range"
     ) {
-      upsertTarget(targetsByPackage, {
+      upsertTarget(targetsByKey, finding, {
         package: finding.recommendedNpmTransitiveRemediation.package,
         currentVersion: finding.recommendedNpmTransitiveRemediation.currentVersion,
         targetVersion: finding.recommendedNpmTransitiveRemediation.targetChildVersion,
@@ -281,7 +281,7 @@ export function buildSuggestedFixCommandPlan(
         ? finding.chainResolution
         : null;
       const npmParentUpgradeWorkspaces = workspaceMap.get(finding.recommendedNpmTransitiveRemediation.package)?.filter(w => w !== ".") ?? [];
-      upsertTarget(targetsByPackage, {
+      upsertTarget(targetsByKey, finding, {
         package: finding.recommendedNpmTransitiveRemediation.package,
         currentVersion: finding.recommendedNpmTransitiveRemediation.currentVersion,
         targetVersion: finding.recommendedNpmTransitiveRemediation.targetVersion,
@@ -322,7 +322,7 @@ export function buildSuggestedFixCommandPlan(
         ? finding.chainResolution
         : null;
       const parentUpgradeWorkspaces = workspaceMap.get(finding.recommendedParentUpgrade.package)?.filter(w => w !== ".") ?? [];
-      upsertTarget(targetsByPackage, {
+      upsertTarget(targetsByKey, finding, {
         package: finding.recommendedParentUpgrade.package,
         currentVersion: finding.recommendedParentUpgrade.currentVersion,
         targetVersion: finding.recommendedParentUpgrade.targetVersion,
@@ -369,7 +369,7 @@ export function buildSuggestedFixCommandPlan(
         : packageManager === "yarn"
           ? `yarn upgrade ${finding.pkg.name}`
           : `bun update ${finding.pkg.name}`;
-      upsertTarget(targetsByPackage, {
+      upsertTarget(targetsByKey, finding, {
         package: finding.pkg.name,
         currentVersion: finding.pkg.version,
         targetVersion: finding.recommendedNpmTransitiveRemediation.targetChildVersion,
@@ -393,7 +393,7 @@ export function buildSuggestedFixCommandPlan(
         const isDev = devLookup.get(`${finding.chainResolution.directDep}@${finding.chainResolution.directDepCurrentVersion}`) ?? false;
         const coverage = calculatePathCoverage(finding.dependencyPaths, [finding.chainResolution.directDep]);
         const chainWorkspaces = workspaceMap.get(finding.chainResolution.directDep)?.filter(w => w !== ".") ?? [];
-        upsertTarget(targetsByPackage, {
+        upsertTarget(targetsByKey, finding, {
           package: finding.chainResolution.directDep,
           currentVersion: finding.chainResolution.directDepCurrentVersion,
           targetVersion: finding.chainResolution.targetVersion,
@@ -436,7 +436,7 @@ export function buildSuggestedFixCommandPlan(
     });
   }
 
-  const targets = [...targetsByPackage.values()].sort((a, b) => {
+  const targets = [...targetsByKey.values()].sort((a, b) => {
     const sevDelta = severityOrder[b.severity] - severityOrder[a.severity];
     if (sevDelta !== 0) return sevDelta;
 
@@ -634,13 +634,30 @@ function packageManagerSourceLabel(scanInput: ScanInput): string {
   return scanInput.source;
 }
 
+// Install targets (direct / parent-upgrade) all mean "install <package>@<targetVersion>",
+// so findings that land on the same package merge into one target and the higher
+// version wins. A parent-update target means something else: "refresh <vulnerable
+// child> within <package>'s declared range", and its targetVersion is the CHILD's
+// version, not <package>'s. Merging it with an install target for the same package
+// compares versions of two different packages, and the child's version can end up
+// pinned onto the parent (`npm install axios@4.0.6`, where 4.0.6 is form-data's
+// version - #1007). Keep parent-update targets in their own keyspace, per child, so
+// they only ever merge with a refresh of the same child through the same package.
+function targetKey(finding: Finding, target: SuggestedFixTarget): string {
+  return target.kind === "parent-update"
+    ? `parent-update:${target.package}>${finding.pkg.name}`
+    : `install:${target.package}`;
+}
+
 function upsertTarget(
-  targetsByPackage: Map<string, SuggestedFixTarget>,
+  targetsByKey: Map<string, SuggestedFixTarget>,
+  finding: Finding,
   next: SuggestedFixTarget,
 ): void {
-  const existing = targetsByPackage.get(next.package);
+  const key = targetKey(finding, next);
+  const existing = targetsByKey.get(key);
   if (!existing) {
-    targetsByPackage.set(next.package, next);
+    targetsByKey.set(key, next);
     return;
   }
 
@@ -687,7 +704,7 @@ function upsertTarget(
       merged.knownVulnerableVersions = next.knownVulnerableVersions ?? merged.knownVulnerableVersions ?? null;
       merged.fixVersionPublishedAt = next.fixVersionPublishedAt ?? merged.fixVersionPublishedAt ?? null;
     }
-    targetsByPackage.set(next.package, merged);
+    targetsByKey.set(key, merged);
     return;
   }
 
@@ -701,7 +718,7 @@ function upsertTarget(
     merged.fixVersionPublishedAt = next.fixVersionPublishedAt ?? merged.fixVersionPublishedAt ?? null;
   }
 
-  targetsByPackage.set(next.package, merged);
+  targetsByKey.set(key, merged);
 }
 
 function buildParentUpgradeReason(

tests/remediation/fix-commands-parent-update-merge.test.ts

@@ -0,0 +1,147 @@
+import {
+  buildSuggestedFixCommandPlan,
+  findFixTargetForFinding,
+  findSuggestedCommandForFinding,
+} from "../../src/remediation/fix-commands.js";
+import type { Finding, ScanInput } from "../../src/types.js";
+
+// Regression coverage for #1007: a transitive finding whose remediation is a
+// within-range refresh (parent-update) carries the CHILD's target version under
+// the PARENT's package name. Keyed by package name alone, that target merged
+// with the direct fix for the parent and the child's version won the "higher
+// version" comparison, producing `npm install axios@4.0.6` - a version of axios
+// that does not exist (4.0.6 is form-data's).
+
+function scanInput(): ScanInput {
+  return {
+    mode: "resolved-lockfile",
+    source: "package-lock",
+    filePath: "/tmp/package-lock.json",
+    packages: [
+      { name: "axios", version: "1.16.1", ecosystem: "npm" },
+      { name: "js-yaml", version: "4.1.1", ecosystem: "npm" },
+      { name: "form-data", version: "4.0.5", ecosystem: "npm" },
+      { name: "follow-redirects", version: "1.15.0", ecosystem: "npm" },
+    ],
+    notes: [],
+    warnings: [],
+    skippedDependencies: [],
+  };
+}
+
+function directAxios(): Finding {
+  return {
+    pkg: { name: "axios", version: "1.16.1", ecosystem: "npm" },
+    vulnerabilities: [{ id: "GHSA-axios-0001" }],
+    severity: "high",
+    cveAliases: [],
+    dependencyPaths: [["project", "axios"]],
+    relationship: "direct",
+    firstFixedVersion: "1.19.0",
+    validatedFirstFixedVersion: "1.19.0",
+  };
+}
+
+function directJsYaml(): Finding {
+  return {
+    pkg: { name: "js-yaml", version: "4.1.1", ecosystem: "npm" },
+    vulnerabilities: [{ id: "GHSA-js-yaml-0001" }],
+    severity: "high",
+    cveAliases: [],
+    dependencyPaths: [["project", "js-yaml"]],
+    relationship: "direct",
+    firstFixedVersion: "4.3.1",
+    validatedFirstFixedVersion: "4.3.1",
+  };
+}
+
+function withinRangeChildOfAxios(name: string, installed: string, target: string): Finding {
+  return {
+    pkg: { name, version: installed, ecosystem: "npm" },
+    vulnerabilities: [{ id: `GHSA-${name}-0001` }],
+    severity: "high",
+    cveAliases: [],
+    dependencyPaths: [["project", "axios", name]],
+    relationship: "transitive",
+    firstFixedVersion: target,
+    validatedFirstFixedVersion: target,
+    recommendedNpmTransitiveRemediation: {
+      kind: "update-parent-within-range",
+      package: "axios",
+      currentVersion: "1.16.1",
+      targetChildVersion: target,
+      viaPath: ["project", "axios", name],
+      reason: `axios@1.16.1 already allows ${name}@${target} within the current dependency range`,
+    },
+  };
+}
+
+describe("buildSuggestedFixCommandPlan - parent-update targets do not merge with install targets (#1007)", () => {
+  it("keeps the direct fix version for the parent when a child is refreshed within the parent's range", () => {
+    const axios = directAxios();
+    const jsYaml = directJsYaml();
+    const formData = withinRangeChildOfAxios("form-data", "4.0.5", "4.0.6");
+
+    const plan = buildSuggestedFixCommandPlan([axios, jsYaml, formData], scanInput());
+    expect(plan).not.toBeNull();
+
+    const axiosInstall = plan!.targets.find(t => t.package === "axios" && t.kind === "direct");
+    expect(axiosInstall).toMatchObject({ currentVersion: "1.16.1", targetVersion: "1.19.0" });
+
+    // The refresh is still there, as its own target, still describing form-data's version.
+    const formDataRefresh = plan!.targets.find(t => t.kind === "parent-update");
+    expect(formDataRefresh).toMatchObject({
+      package: "axios",
+      targetVersion: "4.0.6",
+      command: "npm update axios",
+    });
+
+    // The emitted command never pins axios to form-data's version.
+    expect(plan!.command).toContain("npm install axios@1.19.0 js-yaml@4.3.1");
+    expect(plan!.command).not.toContain("axios@4.0.6");
+    for (const section of plan!.sections) {
+      expect(section.command).not.toContain("axios@4.0.6");
+    }
+
+    // Every finding still resolves to its own target and its own command.
+    expect(findFixTargetForFinding(plan!, axios)?.targetVersion).toBe("1.19.0");
+    expect(findFixTargetForFinding(plan!, jsYaml)?.targetVersion).toBe("4.3.1");
+    expect(findFixTargetForFinding(plan!, formData)?.targetVersion).toBe("4.0.6");
+    expect(findSuggestedCommandForFinding(plan!, axios)).toBe("npm install axios@1.19.0");
+    expect(findSuggestedCommandForFinding(plan!, formData)).toBe("npm update axios");
+    expect(plan!.coveredFindingCount).toBe(3);
+  });
+
+  it("does not merge two within-range refreshes of different children through the same parent", () => {
+    const formData = withinRangeChildOfAxios("form-data", "4.0.5", "4.0.6");
+    const followRedirects = withinRangeChildOfAxios("follow-redirects", "1.15.0", "1.15.6");
+
+    const plan = buildSuggestedFixCommandPlan([formData, followRedirects], scanInput());
+    expect(plan).not.toBeNull();
+
+    // Two distinct child versions cannot be compared against each other, so
+    // each refresh keeps its own target rather than one child's version
+    // overwriting the other's.
+    const refreshes = plan!.targets.filter(t => t.kind === "parent-update");
+    expect(refreshes.map(t => t.targetVersion).sort()).toEqual(["1.15.6", "4.0.6"]);
+    expect(refreshes.every(t => t.package === "axios")).toBe(true);
+
+    // The same refresh command is only emitted once.
+    expect(plan!.command).toBe("npm update axios");
+
+    expect(findFixTargetForFinding(plan!, formData)?.targetVersion).toBe("4.0.6");
+    expect(findFixTargetForFinding(plan!, followRedirects)?.targetVersion).toBe("1.15.6");
+    expect(plan!.coveredFindingCount).toBe(2);
+  });
+
+  it("still merges repeated install targets for the same package to the highest version (no regression)", () => {
+    const lower: Finding = { ...directAxios(), firstFixedVersion: "1.17.0", validatedFirstFixedVersion: "1.17.0", vulnerabilities: [{ id: "GHSA-axios-0002" }] };
+    const higher = directAxios();
+
+    const plan = buildSuggestedFixCommandPlan([lower, higher], scanInput());
+    expect(plan).not.toBeNull();
+    expect(plan!.targets).toHaveLength(1);
+    expect(plan!.targets[0]).toMatchObject({ package: "axios", kind: "direct", targetVersion: "1.19.0" });
+    expect(plan!.command).toBe("npm install axios@1.19.0");
+  });
+});