diff --git a/src/output/html-reporter.ts b/src/output/html-reporter.ts index 33857e7a..d09cb4df 100644 --- a/src/output/html-reporter.ts +++ b/src/output/html-reporter.ts @@ -11,7 +11,7 @@ import { renderMaintenanceFindingsHtml } from "./maintenance-findings-html.js"; import type { Finding } from "../types.js"; import type { OverrideFinding } from "../overrides/types.js"; import type { MaintenanceFinding } from "../maintenance/types.js"; -import type { SuggestedFixCommandPlan } from "../remediation/fix-commands.js"; +import { UNVERIFIED_PARENT_UPGRADE_NOTE, type SuggestedFixCommandPlan } from "../remediation/fix-commands.js"; export type SerializedFinding = ReturnType; @@ -698,6 +698,9 @@ export function renderFixPlan(plan: SuggestedFixCommandPlan | null): string { const cooldownHtml = t.cooldownWarning ? ` ${escapeHtml(formatCooldownWarning(t.cooldownWarning))}` : ""; + const unverifiedHtml = t.confidence === "unverified" + ? ` ${escapeHtml(UNVERIFIED_PARENT_UPGRADE_NOTE)}` + : ""; const coverageHtml = t.coverage === "partial" ? ` Path-specific remediation. Run this command, then rescan; ${t.remainingPaths?.length ?? 0} other known ${pluralize(t.remainingPaths?.length ?? 0, "path")} may still need separate parent upgrades.` : ""; @@ -711,7 +714,7 @@ export function renderFixPlan(plan: SuggestedFixCommandPlan | null): string { return ` ${proofText}`; })() : ""; - return `
${versionHtml}${breakBadge}${statsHtml}${noteHtml}${cooldownHtml}${coverageHtml}${chainProofHtml}
`; + return `
${versionHtml}${breakBadge}${statsHtml}${noteHtml}${cooldownHtml}${unverifiedHtml}${coverageHtml}${chainProofHtml}
`; }).join("\n"); const targetsHtml = section.targets.length > 0 diff --git a/src/output/printers.ts b/src/output/printers.ts index 184b1137..fbd170d6 100644 --- a/src/output/printers.ts +++ b/src/output/printers.ts @@ -1,6 +1,6 @@ import type { Finding, ScanInput, SeverityLabel } from "../types.js"; import { chalk, stripAnsi } from "../utils/chalk.js"; -import { buildSuggestedFixCommandPlan, type SuggestedFixTarget } from "../remediation/fix-commands.js"; +import { buildSuggestedFixCommandPlan, UNVERIFIED_PARENT_UPGRADE_NOTE, type SuggestedFixTarget } from "../remediation/fix-commands.js"; import { isMajorVersionBump } from "../utils/version.js"; import { getPrimaryParent } from "../utils/finding.js"; import { @@ -84,6 +84,12 @@ function printCooldownWarning(target: SuggestedFixTarget): void { } } +function printUnverifiedParentUpgradeWarning(target: SuggestedFixTarget): void { + if (target.confidence === "unverified") { + console.log(chalk.yellow(` ${UNVERIFIED_PARENT_UPGRADE_NOTE}`)); + } +} + function printChainProofLines(targets: SuggestedFixTarget[]): void { for (const target of targets) { if (!target.chainProof || !target.chainSafeVersion) continue; @@ -144,7 +150,10 @@ export function printSuggestedFixCommands( printParentUpgradeTargetsTable(section.targets, sharedParentUpgradeTableWidths); } - for (const target of section.targets) printCooldownWarning(target); + for (const target of section.targets) { + printCooldownWarning(target); + printUnverifiedParentUpgradeWarning(target); + } console.log(renderCommandCallout(section.command, section.targets)); printChainProofLines(section.targets); } diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index dd464a67..a55013da 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -10,6 +10,13 @@ import { buildPnpmWorkspaceMap } from "../parsers/pnpm-lock.js"; import { buildYarnWorkspaceMap } from "../parsers/yarn-lock.js"; import { buildBunWorkspaceMap } from "../parsers/bun-lock.js"; import { readConfiguredCooldown, cooldownWarningFor, type CooldownWarning } from "./cooldown.js"; +import { isVerifiedUpgrade } from "./parent-upgrade.js"; + +// Rendered next to a parent-upgrade fix target whose recommendedParentUpgrade +// could not be verified end-to-end (parent-upgrade.ts confidence: "unverified"). +// Shared across terminal and HTML output so the wording stays in sync. +export const UNVERIFIED_PARENT_UPGRADE_NOTE = + "⚠ unverified - rescan after upgrading to confirm this resolves the finding"; export type SuggestedFixPackageManager = "npm" | "pnpm" | "yarn" | "bun"; @@ -25,6 +32,10 @@ export type SuggestedFixTarget = { severity: SeverityLabel; adjusted: boolean; adjustmentNote?: string | null; + // Set from RecommendedParentUpgrade.confidence when this target was built from + // finding.recommendedParentUpgrade. Undefined for direct fixes, within-range + // parent updates, and validated chain upgrades, which don't carry this distinction. + confidence?: "verified" | "unverified"; reason: string; command?: string; workspaces?: string[]; @@ -330,6 +341,7 @@ export function buildSuggestedFixCommandPlan( chainProof: chainForParentUpgrade?.chain, chainSafeVersion: chainForParentUpgrade?.safeVersion, chainVulnerablePackage: chainForParentUpgrade ? finding.pkg.name : undefined, + confidence: finding.recommendedParentUpgrade.confidence, }); continue; } @@ -460,7 +472,19 @@ export function buildSuggestedFixCommandPlan( totalFindingCount: findings.length, }; - plan.coveredFindingCount = findings.filter(f => findSuggestedCommandForFinding(plan, f) !== null).length; + // A finding only counts as covered when it has a matching fix target AND that + // target isn't an unverified parent upgrade - an unverified recommendation may + // not actually resolve the finding, so it must not inflate the "fix all" claim. + plan.coveredFindingCount = findings.filter(f => { + const target = findFixTargetForFinding(plan, f); + if (!target) return false; + // Coverage is a per-finding property, not a per-target one: two unrelated + // findings can merge into the same target (same package upgrade), so a + // finding's own recommendedParentUpgrade confidence - not the blended + // target confidence - decides whether it is genuinely covered. + if (f.recommendedParentUpgrade) return isVerifiedUpgrade(f.recommendedParentUpgrade.confidence); + return true; + }).length; if (options?.subfolder) { const prefix = `cd ${options.subfolder} && `; @@ -613,6 +637,11 @@ function upsertTarget( chainProof: existing.chainProof ?? next.chainProof, chainSafeVersion: existing.chainSafeVersion ?? next.chainSafeVersion, chainVulnerablePackage: existing.chainVulnerablePackage ?? next.chainVulnerablePackage, + // Conservative: if either contributing finding's recommendation is unverified, + // keep the merged target unverified rather than overstate coverage. + confidence: existing.confidence === "unverified" || next.confidence === "unverified" + ? "unverified" + : existing.confidence ?? next.confidence, }; if (looksLikeVersion(existing.targetVersion) && looksLikeVersion(next.targetVersion)) { diff --git a/src/remediation/parent-upgrade.ts b/src/remediation/parent-upgrade.ts index 5c9e7f3e..bee53ec3 100644 --- a/src/remediation/parent-upgrade.ts +++ b/src/remediation/parent-upgrade.ts @@ -2,6 +2,10 @@ import type { Finding, PackageRef, RecommendedParentUpgrade } from "../types.js" import { compareVersions, isPreReleaseVersion, looksLikeVersion } from "../utils/version.js"; import { fetchPackument } from "./npm-registry.js"; +export function isVerifiedUpgrade(confidence: RecommendedParentUpgrade["confidence"]): boolean { + return confidence === "verified"; +} + export async function resolveRecommendedParentUpgrade( finding: Finding, packages: PackageRef[], @@ -42,9 +46,9 @@ export async function resolveRecommendedParentUpgrade( }); } - // Best-effort fallback for deeper chains. + // Deeper chains: verify along the path before recommending. // Use the immediate child of the direct parent (not the immediate parent of the vulnerable - // package) — that's what the direct parent actually lists in its own dependencies. + // package) - that's what the direct parent actually lists in its own dependencies. const directParentIdx = viaPath.indexOf(directParentName); if (directParentIdx < 0) return null; const immediateChildName = viaPath[directParentIdx + 1] ?? immediateParentName; @@ -58,6 +62,8 @@ export async function resolveRecommendedParentUpgrade( viaPath.slice(0, directParentIdx + 2), ) ?? "", vulnerableName, + vulnerableFixedVersion: finding.validatedFirstFixedVersion ?? finding.firstFixedVersion, + pathAfterParent: viaPath.slice(directParentIdx + 1), viaPath, }); } @@ -136,6 +142,65 @@ function startsWithPath(path: string[], prefix: string[]): boolean { return true; } +type DepManifest = { + dependencies?: Record; + optionalDependencies?: Record; +}; + +/** + * Highest published, non-prerelease version of `name` that satisfies `range`, + * or null if none do (or the packument cannot be fetched). + */ +export async function resolveHighestSatisfying(name: string, range: string): Promise { + const packument = await fetchPackument(name); + const versions = Object.keys(packument?.versions ?? {}) + .filter(looksLikeVersion) + .filter(v => !isPreReleaseVersion(v)) + .filter(version => versionSatisfiesRange(version, range)) + .sort(compareVersions); + + return versions.at(-1) ?? null; +} + +/** + * Walk `pathNames` (in order, ending with `vulnerableName`) starting from + * `startManifest`, resolving each declared range to its highest published + * version to find what the vulnerable package would actually resolve to. + * + * - A dep dropped mid-path means the vulnerable package is no longer pulled + * via this path: `{ terminalVersion: null, resolvable: true }`. + * - A range that cannot be resolved to any published version means the walk + * cannot be verified: `{ terminalVersion: null, resolvable: false }`. + */ +export async function resolvePathTerminalVersion( + startManifest: DepManifest, + pathNames: string[], + vulnerableName: string, +): Promise<{ terminalVersion: string | null; resolvable: boolean }> { + let currentManifest: DepManifest = startManifest; + + for (const nextName of pathNames) { + const range = currentManifest.dependencies?.[nextName] ?? currentManifest.optionalDependencies?.[nextName]; + if (range === undefined) { + return { terminalVersion: null, resolvable: true }; + } + + const resolved = await resolveHighestSatisfying(nextName, range); + if (resolved === null) { + return { terminalVersion: null, resolvable: false }; + } + + if (nextName === vulnerableName) { + return { terminalVersion: resolved, resolvable: true }; + } + + const nextPackument = await fetchPackument(nextName); + currentManifest = nextPackument?.versions?.[resolved] ?? {}; + } + + return { terminalVersion: null, resolvable: true }; +} + type ExactDirectChildArgs = { directParentName: string; directParentVersion: string; @@ -178,7 +243,7 @@ async function findUpgradeForExactDirectChild( targetVersion: version, viaPath: args.viaPath, vulnerablePackage: args.vulnerableName, - confidence: "exact-direct-child", + confidence: "verified", reason: `${args.directParentName}@${version} no longer allows ${args.vulnerableName}@${args.vulnerableInstalledVersion}${args.vulnerableFixedVersion ? ` and allows ${args.vulnerableFixedVersion}+` : ""}`, }; } @@ -192,9 +257,16 @@ type ImmediateIntermediateArgs = { immediateChildName: string; immediateChildInstalledVersion: string; vulnerableName: string; + vulnerableFixedVersion: string | null; + pathAfterParent: string[]; viaPath: string[]; }; +// Bound how many candidate parent versions we resolve the full path for, to keep +// network cost reasonable. Packuments are process-cached, so shared path prefixes +// across candidates are cheap, but the terminal walk still fetches per link. +const MAX_CANDIDATE_PARENT_VERSIONS = 25; + async function findUpgradeForImmediateIntermediate( args: ImmediateIntermediateArgs, ): Promise { @@ -207,34 +279,78 @@ async function findUpgradeForImmediateIntermediate( .filter(looksLikeVersion) .filter(v => !isPreReleaseVersion(v)) .filter(version => compareVersions(version, args.directParentVersion) > 0) - .sort(compareVersions); + .sort(compareVersions) + .slice(0, MAX_CANDIDATE_PARENT_VERSIONS); + + // Three outcomes: + // 1. A candidate whose subtree resolves the vulnerable package to a fixed + // version (or drops it entirely) -> recommend it, verified. + // 2. Every candidate is resolvable but none reaches a fixed version -> null + // (honest: no parent upgrade currently resolves this). + // 3. Some candidate could not be verified (a range/packument we could not + // resolve) -> fall back to the old "no longer allows the installed child" + // heuristic, but label it unverified. + let sawUnverifiable = false; + let heuristicFallback: RecommendedParentUpgrade | null = null; for (const version of versions) { const manifest = packument?.versions?.[version]; - const depRange = - manifest?.dependencies?.[args.immediateChildName] ?? - manifest?.optionalDependencies?.[args.immediateChildName]; - - if (!depRange) continue; - - const stillAllowsInstalledChild = versionSatisfiesRange( - args.immediateChildInstalledVersion, - depRange, - ); + if (!manifest) continue; + + const walk = await resolvePathTerminalVersion(manifest, args.pathAfterParent, args.vulnerableName); + + if (walk.resolvable) { + const isFixed = + walk.terminalVersion === null || + (args.vulnerableFixedVersion != null && + compareVersions(walk.terminalVersion, args.vulnerableFixedVersion) >= 0); + if (isFixed) { + // "verified" means verified against declared dependency ranges along this + // path (each link resolved to its highest published satisfying version), + // not an install-level guarantee - npm dedup/hoisting can still land a + // lower version. This is the honest upper bound we can compute without + // reimplementing npm's resolver. + const reason = walk.terminalVersion === null + ? `${args.directParentName}@${version} no longer pulls ${args.vulnerableName} on this path` + : `${args.directParentName}@${version} resolves ${args.vulnerableName} to ${walk.terminalVersion}${args.vulnerableFixedVersion ? `, at or above the fix ${args.vulnerableFixedVersion}` : ""}`; + return { + package: args.directParentName, + currentVersion: args.directParentVersion, + targetVersion: version, + viaPath: args.viaPath, + vulnerablePackage: args.vulnerableName, + confidence: "verified", + reason, + }; + } + continue; + } - if (!stillAllowsInstalledChild) { - return { - package: args.directParentName, - currentVersion: args.directParentVersion, - targetVersion: version, - viaPath: args.viaPath, - vulnerablePackage: args.vulnerableName, - confidence: "best-effort", - reason: `${args.directParentName}@${version} no longer allows ${args.immediateChildName}@${args.immediateChildInstalledVersion} in the current path`, - }; + // Could not verify this candidate's subtree. Remember the first candidate that + // also satisfies the old heuristic (parent no longer allows the installed + // immediate child) as an explicitly-unverified fallback. + sawUnverifiable = true; + if (!heuristicFallback) { + const depRange = + manifest.dependencies?.[args.immediateChildName] ?? + manifest.optionalDependencies?.[args.immediateChildName]; + if (depRange && !versionSatisfiesRange(args.immediateChildInstalledVersion, depRange)) { + heuristicFallback = { + package: args.directParentName, + currentVersion: args.directParentVersion, + targetVersion: version, + viaPath: args.viaPath, + vulnerablePackage: args.vulnerableName, + confidence: "unverified", + reason: `${args.directParentName}@${version} may resolve ${args.vulnerableName}, but this could not be verified; rescan after upgrading to confirm`, + }; + } } } + if (sawUnverifiable && heuristicFallback) { + return heuristicFallback; + } return null; } diff --git a/src/scanner.ts b/src/scanner.ts index ece063ca..4f1f6052 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -521,7 +521,7 @@ export async function scanPackages( targetVersion: finding.recommendedNpmTransitiveRemediation.targetVersion ?? "", viaPath: finding.recommendedNpmTransitiveRemediation.viaPath, vulnerablePackage: finding.pkg.name, - confidence: "exact-direct-child", + confidence: "verified", reason: finding.recommendedNpmTransitiveRemediation.reason, }; continue; diff --git a/src/types.ts b/src/types.ts index efe06d78..f4d232a5 100644 --- a/src/types.ts +++ b/src/types.ts @@ -103,7 +103,7 @@ export type RecommendedParentUpgrade = { targetVersion: string; viaPath: string[]; vulnerablePackage: string; - confidence: "exact-direct-child" | "best-effort"; + confidence: "verified" | "unverified"; reason: string; }; diff --git a/tests/fixture-scan.test.ts b/tests/fixture-scan.test.ts index 5568956d..d5d07422 100644 --- a/tests/fixture-scan.test.ts +++ b/tests/fixture-scan.test.ts @@ -197,7 +197,7 @@ describe("fixture remediation scans", () => { currentVersion: "4.22.1", targetVersion: "4.22.2", vulnerablePackage: "qs", - confidence: "best-effort", + confidence: "unverified", }); finding.recommendedParentUpgrade = parentUpgrade ?? undefined; @@ -256,7 +256,7 @@ describe("fixture remediation scans", () => { targetVersion: "15.2.1", viaPath: ["project", "lint-staged", "string-width"], vulnerablePackage: "string-width", - confidence: "exact-direct-child", + confidence: "verified", reason: "Upgrade lint-staged to pick up a safe string-width release.", }, }); diff --git a/tests/html-reporter.test.ts b/tests/html-reporter.test.ts index 17339392..a628ade4 100644 --- a/tests/html-reporter.test.ts +++ b/tests/html-reporter.test.ts @@ -475,6 +475,86 @@ describe("renderHtmlReport", () => { expect(html).not.toContain("release cooldown"); }); + it("renders an escaped unverified marker for a parent-upgrade fix target that could not be verified", () => { + const plan: SuggestedFixCommandPlan = { + packageManager: "npm", + sourceLabel: "package-lock.json", + command: "npm install jest@30.4.2", + sections: [ + { + key: "urgent:high", + kind: "urgent", + severity: "high", + title: "High severity fix commands", + command: "npm install jest@30.4.2", + targets: [ + { + package: "jest", + currentVersion: "30.4.1", + targetVersion: "30.4.2", + kind: "parent-upgrade", + urgent: true, + severity: "high", + adjusted: false, + reason: "Parent upgrade for vulnerable brace-expansion@2.1.2", + confidence: "unverified", + }, + ], + }, + ], + targets: [], + skipped: [], + coveredFindingCount: 0, + totalFindingCount: 1, + }; + + const html = renderHtmlReport(buildReportData({ ...BASE_PARAMS, suggestedFixCommands: plan })); + + expect(html).toContain("unverified - rescan after upgrading to confirm this resolves the finding"); + // Must appear inside the reusable fix-target-note span, not a new class. + expect(html).toContain(`⚠ unverified - rescan after upgrading to confirm this resolves the finding`); + expect(html).toContain("should fix 0 of 1 findings."); + }); + + it("does not render an unverified marker for a verified parent-upgrade fix target", () => { + const plan: SuggestedFixCommandPlan = { + packageManager: "npm", + sourceLabel: "package-lock.json", + command: "npm install jest@30.4.2", + sections: [ + { + key: "urgent:high", + kind: "urgent", + severity: "high", + title: "High severity fix commands", + command: "npm install jest@30.4.2", + targets: [ + { + package: "jest", + currentVersion: "30.4.1", + targetVersion: "30.4.2", + kind: "parent-upgrade", + urgent: true, + severity: "high", + adjusted: false, + reason: "Parent upgrade for vulnerable brace-expansion@2.1.2", + confidence: "verified", + }, + ], + }, + ], + targets: [], + skipped: [], + coveredFindingCount: 1, + totalFindingCount: 1, + }; + + const html = renderHtmlReport(buildReportData({ ...BASE_PARAMS, suggestedFixCommands: plan })); + + expect(html).not.toContain("unverified"); + expect(html).toContain("Running all commands should fix all 1 findings."); + }); + it("renders the descriptive recommendation without a Copy button when no runnable command exists", () => { const finding = makeFinding({ relationship: "transitive", diff --git a/tests/maintenance/dm001-maintenance-risk.test.ts b/tests/maintenance/dm001-maintenance-risk.test.ts index 64eb48d7..8259db21 100644 --- a/tests/maintenance/dm001-maintenance-risk.test.ts +++ b/tests/maintenance/dm001-maintenance-risk.test.ts @@ -30,7 +30,7 @@ function makeFinding(overrides: Partial = {}): Finding { targetVersion: "5.0.0", viaPath: ["gray-matter", "js-yaml"], vulnerablePackage: "js-yaml", - confidence: "exact-direct-child", + confidence: "verified", reason: "Major upgrade required", }, ...overrides, @@ -105,7 +105,7 @@ describe("detectDM001", () => { targetVersion: "4.1.0", viaPath: ["gray-matter", "js-yaml"], vulnerablePackage: "js-yaml", - confidence: "exact-direct-child", + confidence: "verified", reason: "Minor upgrade", }, }); @@ -173,7 +173,7 @@ describe("detectDM001", () => { targetVersion: "2.0.0", viaPath: ["z-package", "js-yaml"], vulnerablePackage: "js-yaml", - confidence: "best-effort", + confidence: "unverified", reason: "Major upgrade", }, }); @@ -184,7 +184,7 @@ describe("detectDM001", () => { targetVersion: "2.0.0", viaPath: ["a-package", "js-yaml"], vulnerablePackage: "js-yaml", - confidence: "best-effort", + confidence: "unverified", reason: "Major upgrade", }, }); diff --git a/tests/output.test.ts b/tests/output.test.ts index 4f151753..affd93a0 100644 --- a/tests/output.test.ts +++ b/tests/output.test.ts @@ -63,7 +63,7 @@ function createFinding(overrides?: Partial): Finding { targetVersion: "1.1.0", viaPath: ["project", "app", "lodash"], vulnerablePackage: "lodash", - confidence: "exact-direct-child", + confidence: "verified", reason: "app@1.1.0 no longer allows lodash@4.17.20", }, recommendedNpmTransitiveRemediation: undefined, @@ -330,7 +330,7 @@ describe("output formatters", () => { targetVersion: "1.1.0", viaPath: ["project", "app", "lodash"], vulnerablePackage: "lodash", - confidence: "exact-direct-child", + confidence: "verified", reason: "app@1.1.0 no longer allows lodash@4.17.20", }, }), @@ -403,7 +403,7 @@ describe("output formatters", () => { targetVersion: "6.0.0", viaPath: ["project", "gulp-diff", "diff"], vulnerablePackage: "diff", - confidence: "exact-direct-child", + confidence: "verified", reason: "gulp-diff@6.0.0 no longer pulls vulnerable diff", }, }), @@ -961,7 +961,7 @@ describe("output formatters", () => { targetVersion: "30.4.0", viaPath: ["project", "jest", "js-yaml"], vulnerablePackage: "js-yaml", - confidence: "exact-direct-child", + confidence: "verified", reason: "jest@30.4.0 no longer pulls vulnerable js-yaml", }, recommendedNpmTransitiveRemediation: undefined, @@ -985,7 +985,7 @@ describe("output formatters", () => { targetVersion: "30.4.0", viaPath: ["project", "jest", "js-yaml"], vulnerablePackage: "js-yaml", - confidence: "exact-direct-child", + confidence: "verified", reason: "jest@30.4.0 no longer pulls vulnerable js-yaml", }, recommendedNpmTransitiveRemediation: undefined, @@ -1009,7 +1009,7 @@ describe("output formatters", () => { targetVersion: "30.4.0", viaPath: ["project", "jest", "js-yaml"], vulnerablePackage: "js-yaml", - confidence: "exact-direct-child", + confidence: "verified", reason: "jest@30.4.0 no longer pulls vulnerable js-yaml", }, recommendedNpmTransitiveRemediation: undefined, @@ -1033,7 +1033,7 @@ describe("output formatters", () => { targetVersion: "30.4.0", viaPath: ["project", "jest", "js-yaml"], vulnerablePackage: "js-yaml", - confidence: "exact-direct-child", + confidence: "verified", reason: "jest@30.4.0 no longer pulls vulnerable js-yaml", }, recommendedNpmTransitiveRemediation: undefined, @@ -1429,7 +1429,7 @@ describe("output printers", () => { targetVersion: "12.0.0-beta-4", viaPath: ["project", "mocha", "diff"], vulnerablePackage: "diff", - confidence: "exact-direct-child", + confidence: "verified", reason: "mocha@12.0.0-beta-4 no longer allows diff@7.0.0", }, }), @@ -1965,7 +1965,7 @@ describe("output printers", () => { targetVersion: "1.1.0", viaPath: ["project", "app", "lodash"], vulnerablePackage: "lodash", - confidence: "exact-direct-child", + confidence: "verified", reason: "app@1.1.0 no longer allows lodash@4.17.20", }, }), @@ -1995,7 +1995,7 @@ describe("output printers", () => { targetVersion: "1.1.0", viaPath: ["project", "app", "lodash"], vulnerablePackage: "lodash", - confidence: "exact-direct-child", + confidence: "verified", reason: "app@1.1.0 no longer allows lodash@4.17.20", }, }), @@ -2037,7 +2037,7 @@ describe("output printers", () => { targetVersion: "17.0.0", viaPath: ["project", "lint-staged", "picomatch"], vulnerablePackage: "picomatch", - confidence: "exact-direct-child", + confidence: "verified", reason: "lint-staged@17.0.0 no longer allows picomatch@4.0.3", }, }); @@ -2733,3 +2733,100 @@ describe("printSuggestedFixCommands - cooldown warning", () => { expect(output).not.toContain("release cooldown"); }); }); + +describe("printSuggestedFixCommands - unverified parent upgrade", () => { + it("prints an unverified marker and excludes it from the fix-all claim", () => { + const findings = [ + createFinding({ + pkg: { name: "minimist", version: "0.0.8", ecosystem: "npm", paths: [["project", "minimist"]] }, + relationship: "direct", + dependencyPaths: [["project", "minimist"]], + severity: "critical", + firstFixedVersion: "1.2.8", + recommendedParentUpgrade: undefined, + }), + createFinding({ + pkg: { name: "brace-expansion", version: "2.1.2", ecosystem: "npm", paths: [["project", "jest", "brace-expansion"]] }, + relationship: "transitive", + dependencyPaths: [["project", "jest", "brace-expansion"]], + severity: "high", + firstFixedVersion: "5.0.8", + recommendedParentUpgrade: { + package: "jest", + currentVersion: "30.4.1", + targetVersion: "30.4.2", + viaPath: ["project", "jest", "brace-expansion"], + vulnerablePackage: "brace-expansion", + confidence: "unverified", + reason: "jest@30.4.2 may resolve brace-expansion, but this could not be verified; rescan after upgrading to confirm", + }, + }), + ]; + + const lines = captureLogs(() => { + printSuggestedFixCommands(findings, createScanInputForSource("package-lock")); + }); + const output = lines.join("\n"); + + expect(output).toContain("unverified - rescan after upgrading to confirm this resolves the finding"); + expect(output).toContain("should fix 1 of 2 vulnerability findings."); + expect(output).not.toContain("should fix all"); + }); + + it("does not print an unverified marker for a verified parent upgrade", () => { + const findings = [ + createFinding({ + pkg: { name: "brace-expansion", version: "2.1.2", ecosystem: "npm", paths: [["project", "jest", "brace-expansion"]] }, + relationship: "transitive", + dependencyPaths: [["project", "jest", "brace-expansion"]], + severity: "high", + firstFixedVersion: "5.0.8", + recommendedParentUpgrade: { + package: "jest", + currentVersion: "30.4.1", + targetVersion: "30.4.2", + viaPath: ["project", "jest", "brace-expansion"], + vulnerablePackage: "brace-expansion", + confidence: "verified", + reason: "jest@30.4.2 resolves brace-expansion to 5.0.8, at or above the fix 5.0.8", + }, + }), + ]; + + const lines = captureLogs(() => { + printSuggestedFixCommands(findings, createScanInputForSource("package-lock")); + }); + const output = lines.join("\n"); + + expect(output).not.toContain("unverified"); + expect(output).toContain("should fix all 1 vulnerability finding."); + }); +}); + +describe("serializeFinding - recommendedParentUpgrade confidence", () => { + it("includes confidence: unverified in the serialized recommendedParentUpgrade", () => { + const finding = createFinding({ + relationship: "transitive", + recommendedParentUpgrade: { + package: "jest", + currentVersion: "30.4.1", + targetVersion: "30.4.2", + viaPath: ["project", "jest", "brace-expansion"], + vulnerablePackage: "brace-expansion", + confidence: "unverified", + reason: "jest@30.4.2 may resolve brace-expansion, but this could not be verified; rescan after upgrading to confirm", + }, + }); + + const serialized = serializeFinding(finding); + expect(serialized.recommendedParentUpgrade?.confidence).toBe("unverified"); + }); + + it("includes confidence: verified in the serialized recommendedParentUpgrade", () => { + const finding = createFinding(); + expect(finding.recommendedParentUpgrade?.confidence).toBe("verified"); + + const serialized = serializeFinding(finding); + expect(serialized.recommendedParentUpgrade?.confidence).toBe("verified"); + }); +}); diff --git a/tests/parent-upgrade.test.ts b/tests/parent-upgrade.test.ts index a0cdd6c5..2044784c 100644 --- a/tests/parent-upgrade.test.ts +++ b/tests/parent-upgrade.test.ts @@ -66,6 +66,14 @@ async function loadResolver() { return module.resolveRecommendedParentUpgrade; } +async function loadPathHelpers() { + const module = await import(`../src/remediation/parent-upgrade.js?test=${Date.now()}-${Math.random()}`); + return { + resolveHighestSatisfying: module.resolveHighestSatisfying, + resolvePathTerminalVersion: module.resolvePathTerminalVersion, + }; +} + describe("npm transitive graph helpers", () => { it("finds the highest safe child version that still satisfies the immediate parent range", () => { const graph = createNpmTransitiveGraph({ @@ -501,7 +509,7 @@ describe("resolveNpmTransitiveRemediation — 3-level within-range gap (#522)", // @aws-amplify/core declares js-cookie: ^3.0.5 which already covers 3.0.7. // The correct fix is npm update js-cookie (within-range lockfile refresh). // Bug: resolveNpmTransitiveRemediation bails out when directParentName !== immediateParentName, - // so it returns null and falls back to the wrong best-effort parent upgrade. + // so it returns null and falls back to the wrong unverified parent upgrade. beforeEach(() => { fetchMock.mockReset(); clearPackumentCache(); @@ -686,7 +694,7 @@ describe("resolveRecommendedParentUpgrade", () => { currentVersion: "1.0.0", targetVersion: "1.1.0", vulnerablePackage: "lodash", - confidence: "exact-direct-child", + confidence: "verified", }); expect(result?.reason).toContain("no longer allows lodash@4.17.20"); expect(result?.reason).toContain("allows 4.17.21+"); @@ -723,19 +731,26 @@ describe("resolveRecommendedParentUpgrade", () => { currentVersion: "1.0.0", targetVersion: "1.1.0", vulnerablePackage: "lodash", - confidence: "exact-direct-child", + confidence: "verified", }); expect(result?.reason).toContain("allows 4.18.0+"); }); - it("recommends a best-effort upgrade for deeper paths when the direct parent stops allowing the current intermediate version", async () => { + it("recommends an unverified upgrade for deeper paths that cannot be verified when the direct parent stops allowing the current intermediate version", async () => { const resolveRecommendedParentUpgrade = await loadResolver(); - mockPackument({ - versions: { - "1.0.0": { dependencies: { mid: "^2.0.0" } }, - "1.1.0": { dependencies: { mid: "^2.0.0" } }, - "2.0.0": { dependencies: { mid: "^3.0.0" } }, + // `mid` has no published versions to resolve against, so the terminal walk + // cannot verify whether the vulnerable package would be resolved -> the + // resolver falls back to the old "no longer allows the installed child" + // heuristic, but labels the result unverified. + mockPackumentsByPackage(fetchMock, { + app: { + versions: { + "1.0.0": { dependencies: { mid: "^2.0.0" } }, + "1.1.0": { dependencies: { mid: "^2.0.0" } }, + "2.0.0": { dependencies: { mid: "^3.0.0" } }, + }, }, + mid: { versions: {} }, }); const finding = createFinding({ @@ -755,9 +770,9 @@ describe("resolveRecommendedParentUpgrade", () => { currentVersion: "1.0.0", targetVersion: "2.0.0", vulnerablePackage: "lodash", - confidence: "best-effort", + confidence: "unverified", }); - expect(result?.reason).toContain("no longer allows mid@2.0.0"); + expect(result?.reason).toContain("could not be verified"); }); it("returns null when the immediate parent version is missing or invalid in deeper paths", async () => { @@ -928,7 +943,7 @@ describe("resolveRecommendedParentUpgrade", () => { // mid@3.0.0 (from the Map's last-write-wins), the app@2.0.0 entry requires // mid ^3.0.0 which satisfies 3.0.0, so it would not be selected. With the // correct version (2.0.0), app@2.0.0 requires mid ^3.0.0 which no longer - // allows mid@2.0.0, triggering the best-effort upgrade recommendation. + // allows mid@2.0.0, triggering the unverified upgrade recommendation. const result = await resolveRecommendedParentUpgrade(finding, packages, new Set(["app"])); expect(result).toMatchObject({ @@ -936,8 +951,275 @@ describe("resolveRecommendedParentUpgrade", () => { currentVersion: "1.0.0", targetVersion: "2.0.0", vulnerablePackage: "lodash", - confidence: "best-effort", + confidence: "unverified", + }); + expect(result?.reason).toContain("could not be verified"); + }); +}); + +describe("resolveHighestSatisfying", () => { + beforeEach(() => { + fetchMock.mockReset(); + clearPackumentCache(); + }); + + it("picks the highest published non-prerelease version satisfying the range", async () => { + const { resolveHighestSatisfying } = await loadPathHelpers(); + mockPackumentsByPackage(fetchMock, { + foo: { + versions: { + "1.0.0": {}, + "1.1.0": {}, + "2.0.0-beta.1": {}, + "1.2.0": {}, + }, + }, + }); + + const result = await resolveHighestSatisfying("foo", "^1.0.0"); + + expect(result).toBe("1.2.0"); + }); + + it("returns null when no published version satisfies the range", async () => { + const { resolveHighestSatisfying } = await loadPathHelpers(); + mockPackumentsByPackage(fetchMock, { + foo: { + versions: { + "1.0.0": {}, + "1.1.0": {}, + }, + }, + }); + + const result = await resolveHighestSatisfying("foo", "^2.0.0"); + + expect(result).toBeNull(); + }); +}); + +describe("resolvePathTerminalVersion", () => { + beforeEach(() => { + fetchMock.mockReset(); + clearPackumentCache(); + }); + + it("walks the path names to the terminal and returns the vulnerable package's resolved version", async () => { + const { resolvePathTerminalVersion } = await loadPathHelpers(); + mockPackumentsByPackage(fetchMock, { + mid: { + versions: { + "2.0.0": { dependencies: { lodash: "^4.17.20" } }, + "2.1.0": { dependencies: { lodash: "^4.17.21" } }, + }, + }, + lodash: { + versions: { + "4.17.20": {}, + "4.17.21": {}, + }, + }, + }); + + const result = await resolvePathTerminalVersion( + { dependencies: { mid: "^2.0.0" } }, + ["mid", "lodash"], + "lodash", + ); + + expect(result).toEqual({ terminalVersion: "4.17.21", resolvable: true }); + }); + + it("treats a dropped dependency mid-path as fixed for this path", async () => { + const { resolvePathTerminalVersion } = await loadPathHelpers(); + mockPackumentsByPackage(fetchMock, { + mid: { + versions: { + "2.0.0": {}, + "2.1.0": { dependencies: {} }, + }, + }, + }); + + const result = await resolvePathTerminalVersion( + { dependencies: { mid: "^2.0.0" } }, + ["mid", "lodash"], + "lodash", + ); + + expect(result).toEqual({ terminalVersion: null, resolvable: true }); + }); + + it("returns unresolvable when a range mid-path has no satisfying published version", async () => { + const { resolvePathTerminalVersion } = await loadPathHelpers(); + mockPackumentsByPackage(fetchMock, { + mid: { + versions: { + "2.0.0": {}, + "2.1.0": {}, + }, + }, }); - expect(result?.reason).toContain("no longer allows mid@2.0.0"); + + const result = await resolvePathTerminalVersion( + { dependencies: { mid: "^99.0.0" } }, + ["mid", "lodash"], + "lodash", + ); + + expect(result).toEqual({ terminalVersion: null, resolvable: false }); + }); +}); + +describe("resolveRecommendedParentUpgrade - verified deep-chain outcomes (#896)", () => { + beforeEach(() => { + fetchMock.mockReset(); + clearPackumentCache(); + }); + + function deepFinding(): Finding { + return createFinding({ + firstFixedVersion: "4.17.21", + dependencyPaths: [["project", "app", "mid", "lodash"]], + pkg: { + name: "lodash", + version: "4.17.20", + ecosystem: "npm", + paths: [["project", "app", "mid", "lodash"]], + }, + }); + } + + it("recommends a verified upgrade when the path resolves the vulnerable package to a fixed version", async () => { + const resolveRecommendedParentUpgrade = await loadResolver(); + mockPackumentsByPackage(fetchMock, { + app: { + versions: { + "1.0.0": { dependencies: { mid: "^2.0.0" } }, + "1.1.0": { dependencies: { mid: "^2.1.0" } }, + }, + }, + mid: { + versions: { + "2.0.0": { dependencies: { lodash: "4.17.20" } }, + "2.1.0": { dependencies: { lodash: "^4.17.21" } }, + }, + }, + lodash: { versions: { "4.17.20": {}, "4.17.21": {} } }, + }); + + const result = await resolveRecommendedParentUpgrade(deepFinding(), createPackages()); + + expect(result).toMatchObject({ + package: "app", + targetVersion: "1.1.0", + vulnerablePackage: "lodash", + confidence: "verified", + }); + expect(result?.reason).toContain("4.17.21"); + }); + + it("returns null when no parent version resolves the vulnerable package to a fixed version", async () => { + const resolveRecommendedParentUpgrade = await loadResolver(); + mockPackumentsByPackage(fetchMock, { + app: { + versions: { + "1.0.0": { dependencies: { mid: "^2.0.0" } }, + "1.1.0": { dependencies: { mid: "^2.0.0" } }, + }, + }, + mid: { versions: { "2.0.0": { dependencies: { lodash: "4.17.20" } } } }, + lodash: { versions: { "4.17.20": {}, "4.17.21": {} } }, + }); + + const result = await resolveRecommendedParentUpgrade(deepFinding(), createPackages()); + + expect(result).toBeNull(); + }); + + it("recommends a verified upgrade when a parent version drops the vulnerable package from the path", async () => { + const resolveRecommendedParentUpgrade = await loadResolver(); + mockPackumentsByPackage(fetchMock, { + app: { + versions: { + "1.0.0": { dependencies: { mid: "^2.0.0" } }, + "1.1.0": { dependencies: { mid: "^2.1.0" } }, + }, + }, + mid: { + versions: { + "2.0.0": { dependencies: { lodash: "4.17.20" } }, + "2.1.0": { dependencies: {} }, + }, + }, + lodash: { versions: { "4.17.20": {}, "4.17.21": {} } }, + }); + + const result = await resolveRecommendedParentUpgrade(deepFinding(), createPackages()); + + expect(result).toMatchObject({ + package: "app", + targetVersion: "1.1.0", + confidence: "verified", + }); + expect(result?.reason).toContain("no longer pulls"); + }); + + it("terminates and returns null within the candidate cap when no version fixes the finding", async () => { + const resolveRecommendedParentUpgrade = await loadResolver(); + const appVersions: Record = {}; + for (let minor = 0; minor <= 30; minor++) { + appVersions[`1.${minor}.0`] = { dependencies: { mid: "^2.0.0" } }; + } + mockPackumentsByPackage(fetchMock, { + app: { versions: appVersions }, + mid: { versions: { "2.0.0": { dependencies: { lodash: "4.17.20" } } } }, + lodash: { versions: { "4.17.20": {}, "4.17.21": {} } }, + }); + + const result = await resolveRecommendedParentUpgrade(deepFinding(), createPackages()); + + expect(result).toBeNull(); + }); + + it("does not recommend a jest upgrade for brace-expansion when every jest still pulls the vulnerable version (#896 regression)", async () => { + const resolveRecommendedParentUpgrade = await loadResolver(); + // Every jest version above current still resolves the path + // jest -> @jest/core -> minimatch -> brace-expansion@2.x (below the 5.0.8 fix). + // The old resolver recommended jest@30.4.2 and claimed it "should fix" the + // finding; the verified resolver must return null instead. + mockPackumentsByPackage(fetchMock, { + jest: { + versions: { + "30.4.1": { dependencies: { "@jest/core": "^30.4.1" } }, + "30.4.2": { dependencies: { "@jest/core": "^30.4.1" } }, + }, + }, + "@jest/core": { versions: { "30.4.1": { dependencies: { minimatch: "^9.0.9" } } } }, + minimatch: { versions: { "9.0.9": { dependencies: { "brace-expansion": "^2.0.1" } } } }, + "brace-expansion": { versions: { "2.1.2": {}, "5.0.8": {} } }, + }); + + const finding = createFinding({ + firstFixedVersion: "5.0.8", + dependencyPaths: [["project", "jest", "@jest/core", "minimatch", "brace-expansion"]], + pkg: { + name: "brace-expansion", + version: "2.1.2", + ecosystem: "npm", + paths: [["project", "jest", "@jest/core", "minimatch", "brace-expansion"]], + }, + }); + + const packages: PackageRef[] = [ + { name: "jest", version: "30.4.1", ecosystem: "npm", paths: [["project", "jest"]] }, + { name: "@jest/core", version: "30.4.1", ecosystem: "npm", paths: [["project", "jest", "@jest/core"]] }, + { name: "minimatch", version: "9.0.9", ecosystem: "npm", paths: [["project", "jest", "@jest/core", "minimatch"]] }, + { name: "brace-expansion", version: "2.1.2", ecosystem: "npm", paths: [["project", "jest", "@jest/core", "minimatch", "brace-expansion"]] }, + ]; + + const result = await resolveRecommendedParentUpgrade(finding, packages, new Set(["jest"])); + + expect(result).toBeNull(); }); }); diff --git a/tests/remediation/fix-commands-unverified-parent-upgrade.test.ts b/tests/remediation/fix-commands-unverified-parent-upgrade.test.ts new file mode 100644 index 00000000..bc0cc2a5 --- /dev/null +++ b/tests/remediation/fix-commands-unverified-parent-upgrade.test.ts @@ -0,0 +1,110 @@ +import { buildSuggestedFixCommandPlan, findFixTargetForFinding } from "../../src/remediation/fix-commands.js"; +import type { Finding, ScanInput } from "../../src/types.js"; + +function scanInput(): ScanInput { + return { + mode: "resolved-lockfile", + source: "package-lock", + filePath: "/tmp/package-lock.json", + packages: [], + notes: [], + warnings: [], + skippedDependencies: [], + }; +} + +function transitiveFindingWithParentUpgrade(confidence: "verified" | "unverified"): Finding { + return { + pkg: { name: "brace-expansion", version: "2.1.2", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-mh99-v99m-4gvg" }], + severity: "high", + cveAliases: ["CVE-2026-9999"], + dependencyPaths: [["project", "jest", "brace-expansion"]], + relationship: "transitive", + firstFixedVersion: "5.0.8", + recommendedParentUpgrade: { + package: "jest", + currentVersion: "30.4.1", + targetVersion: "30.4.2", + viaPath: ["project", "jest", "brace-expansion"], + vulnerablePackage: "brace-expansion", + confidence, + reason: confidence === "verified" + ? "jest@30.4.2 resolves brace-expansion to 5.0.8, at or above the fix 5.0.8" + : "jest@30.4.2 may resolve brace-expansion, but this could not be verified; rescan after upgrading to confirm", + }, + }; +} + +describe("buildSuggestedFixCommandPlan - unverified parent upgrade coverage", () => { + it("does not count a finding whose only fix is an unverified parent upgrade as covered", () => { + const finding = transitiveFindingWithParentUpgrade("unverified"); + const plan = buildSuggestedFixCommandPlan([finding], scanInput()); + + expect(plan).not.toBeNull(); + expect(plan!.totalFindingCount).toBe(1); + expect(plan!.coveredFindingCount).toBe(0); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.confidence).toBe("unverified"); + }); + + it("counts a finding fixed by a verified parent upgrade as covered (no regression)", () => { + const finding = transitiveFindingWithParentUpgrade("verified"); + const plan = buildSuggestedFixCommandPlan([finding], scanInput()); + + expect(plan).not.toBeNull(); + expect(plan!.totalFindingCount).toBe(1); + expect(plan!.coveredFindingCount).toBe(1); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.confidence).toBe("verified"); + }); + + it("still counts a direct fix as covered when it has no recommendedParentUpgrade at all", () => { + const finding: Finding = { + pkg: { name: "minimist", version: "0.0.8", ecosystem: "npm" }, + vulnerabilities: [{ id: "CVE-2020-7598" }], + severity: "critical", + cveAliases: ["CVE-2020-7598"], + dependencyPaths: [["project", "minimist"]], + relationship: "direct", + firstFixedVersion: "1.2.8", + recommendedParentUpgrade: undefined, + }; + + const plan = buildSuggestedFixCommandPlan([finding], scanInput()); + + expect(plan).not.toBeNull(); + expect(plan!.coveredFindingCount).toBe(1); + expect(plan!.totalFindingCount).toBe(1); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.confidence).toBeUndefined(); + }); + + it("still counts a direct fix as covered when it shares a target package with an unrelated unverified parent upgrade (#896 merge regression)", () => { + // A direct jest CVE fixed by jest 30.4.1 -> 30.4.2, plus an unrelated + // transitive brace-expansion finding whose UNVERIFIED parent upgrade + // recommends the same jest 30.4.2. They merge into one target; the direct + // fix must keep its coverage credit (coverage is per-finding, not per-target). + const directJest: Finding = { + pkg: { name: "jest", version: "30.4.1", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-jest-0001" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "jest"]], + relationship: "direct", + firstFixedVersion: "30.4.2", + recommendedParentUpgrade: undefined, + }; + const unverifiedTransitive = transitiveFindingWithParentUpgrade("unverified"); + + const plan = buildSuggestedFixCommandPlan([directJest, unverifiedTransitive], scanInput()); + + expect(plan).not.toBeNull(); + expect(plan!.totalFindingCount).toBe(2); + // The direct jest fix is covered; only the unverified transitive one is not. + expect(plan!.coveredFindingCount).toBe(1); + }); +});