diff --git a/src/output/formatters.ts b/src/output/formatters.ts index 1f38421..44b39a2 100644 --- a/src/output/formatters.ts +++ b/src/output/formatters.ts @@ -3,6 +3,7 @@ import { chalk } from "../utils/chalk.js"; import { severityOrder } from "../constants.js"; import { loadCache } from "../osv/cache.js"; import { inferSeverity } from "../osv/severity.js"; +import { getPrimaryParent } from "../utils/finding.js"; export function formatSeverityLabel(severity: string): string { const lower = severity.toLowerCase(); @@ -20,12 +21,6 @@ export function formatRelationshipLabel(value: string): string { return chalk.gray(value); } -export function getPrimaryParent(finding: Finding): string | null { - const firstPath = finding.dependencyPaths?.[0]; - if (!firstPath || firstPath.length < 3) return null; - return firstPath[1] ?? null; -} - export function getRecommendedAction(finding: Finding): string { if (finding.relationship === "direct" && finding.firstFixedVersion) { return `Upgrade ${finding.pkg.name} to ${finding.firstFixedVersion}+ in this project.`; @@ -40,13 +35,13 @@ export function getRecommendedAction(finding: Finding): string { const parent = getPrimaryParent(finding); if (parent && finding.firstFixedVersion) { - return `Review ${parent}; aim for a version that resolves ${finding.pkg.name} to ${finding.firstFixedVersion}+`; + return `Upgrade ${parent} — no safe version was identified automatically. Check for a release that resolves ${finding.pkg.name} to ${finding.firstFixedVersion}+.`; } if (parent) { return `Review ${parent}; it currently pulls in vulnerable ${finding.pkg.name}.`; } if (finding.firstFixedVersion) { - return `Upgrade the parent dependency chain so ${finding.pkg.name} resolves to ${finding.firstFixedVersion}+`; + return `No dependency path found for ${finding.pkg.name}. Inspect your lockfile to identify which package pulls it in.`; } return `Inspect the dependency path for ${finding.pkg.name} and choose the safest upgrade path.`; } @@ -89,8 +84,12 @@ export function summarizeNextAction(finding: Finding): string { if (finding.recommendedParentUpgrade) { return `Upgrade ${finding.recommendedParentUpgrade.package} ${finding.recommendedParentUpgrade.currentVersion} -> ${finding.recommendedParentUpgrade.targetVersion}.`; } + const parent = getPrimaryParent(finding); + if (parent && finding.firstFixedVersion) { + return `Upgrade ${parent} — no safe version identified. Find a release resolving ${finding.pkg.name} to ${finding.firstFixedVersion}+.`; + } if (finding.firstFixedVersion) { - return `Upgrade the parent dependency chain so it resolves ${finding.pkg.name} to ${finding.firstFixedVersion}+`; + return `No dependency path found for ${finding.pkg.name}. Check your lockfile for which package pulls it in.`; } return `Inspect the parent dependency chain for ${finding.pkg.name} and choose the safest available upgrade.`; } diff --git a/src/output/printers.ts b/src/output/printers.ts index d365108..17895f9 100644 --- a/src/output/printers.ts +++ b/src/output/printers.ts @@ -3,10 +3,10 @@ import { chalk, stripAnsi } from "../utils/chalk.js"; import { severityOrder } from "../constants.js"; import { buildSuggestedFixCommandPlan } from "../remediation/fix-commands.js"; import { isMajorVersionBump } from "../utils/version.js"; +import { getPrimaryParent } from "../utils/finding.js"; import { formatSeverityLabel, formatRelationshipLabel, - getPrimaryParent, getRecommendedAction, summarizeRisk, summarizeNextAction, @@ -168,20 +168,34 @@ export function printSuggestedFixCommands(findings: Finding[], scanInput: ScanIn for (const section of plan.sections) { console.log(""); console.log(colorFixSectionTitle(section.severity, section.title)); - const hasDirectTargets = section.targets.some(t => t.kind === "direct"); - if (section.kind === "direct" || section.kind === "direct-adjusted" || (section.kind === "urgent" && hasDirectTargets)) { + + if (section.kind === "direct" || section.kind === "direct-adjusted") { const validationSummary = summarizeAdjustedValidation(section.targets); const remainingNotes: string[] = []; - // Urgent sections compute their own widths so the Breaking? column - // is not truncated by widths derived from smaller non-urgent sections. - const tableWidths = section.kind === "urgent" ? undefined : sharedDirectTableWidths; - printDirectTargetsTable(section.targets, remainingNotes, validationSummary, tableWidths); + printDirectTargetsTable(section.targets, remainingNotes, validationSummary, sharedDirectTableWidths); for (const note of remainingNotes) { console.log(chalk.gray(` Note: ${note}`)); } + } else if (section.kind === "urgent") { + const directTargets = section.targets.filter(t => t.kind === "direct"); + const parentUpgradeTargets = section.targets.filter(t => t.kind === "parent-upgrade"); + if (directTargets.length > 0) { + const validationSummary = summarizeAdjustedValidation(directTargets); + const remainingNotes: string[] = []; + // Urgent sections compute their own widths so the Breaking? column + // is not truncated by widths derived from smaller non-urgent sections. + printDirectTargetsTable(directTargets, remainingNotes, validationSummary, undefined); + for (const note of remainingNotes) { + console.log(chalk.gray(` Note: ${note}`)); + } + } + if (parentUpgradeTargets.length > 0) { + printParentUpgradeTargetsTable(parentUpgradeTargets, undefined); + } } else if (shouldRenderParentUpgradeTable(section.targets)) { printParentUpgradeTargetsTable(section.targets, sharedParentUpgradeTableWidths); } + console.log(renderCommandCallout(section.command)); } } @@ -419,14 +433,26 @@ export function printCompactOutput(findings: Finding[], scanInput?: ScanInput) { ` ${chalk.gray(`Fix: upgrade ${finding.recommendedParentUpgrade.package} to ${finding.recommendedParentUpgrade.targetVersion}`)}`, ); } else if (finding.firstFixedVersion) { - const action = finding.relationship === "direct" - ? `upgrade to ${finding.firstFixedVersion}` - : `upgrade parent chain to resolve ${finding.firstFixedVersion}+`; + let action: string; + if (finding.relationship === "direct") { + action = `upgrade to ${finding.firstFixedVersion}`; + } else { + const parent = getPrimaryParent(finding); + action = parent + ? `Upgrade ${parent} — check for release resolving ${finding.pkg.name} to ${finding.firstFixedVersion}+` + : `No dependency path found — inspect lockfile to identify which package pulls in ${finding.pkg.name}`; + } console.log(` ${chalk.gray(`Fix: ${action}`)}`); } else { - const action = finding.relationship === "direct" - ? "review and upgrade directly" - : "upgrade parent chain to resolve"; + let action: string; + if (finding.relationship === "direct") { + action = "review and upgrade directly"; + } else { + const parent = getPrimaryParent(finding); + action = parent + ? `Upgrade ${parent} to resolve ${finding.pkg.name}` + : `No dependency path found — inspect lockfile to identify which package pulls in ${finding.pkg.name}`; + } console.log(` ${chalk.gray(`Fix: ${action}`)}`); } console.log(""); @@ -484,7 +510,7 @@ export function printCompactOutput(findings: Finding[], scanInput?: ScanInput) { if (parent) { console.log(`Upgrade ${chalk.whiteBright(parent)} to resolve ${topFinding.pkg.name}`); } else { - console.log(`Upgrade parent chain to resolve ${chalk.whiteBright(topFinding.pkg.name)}`); + console.log(`No dependency path found — inspect lockfile to identify which package pulls in ${chalk.whiteBright(topFinding.pkg.name)}`); } } if (plan) { @@ -740,7 +766,8 @@ function printParentUpgradeTargetsTable( }); if (rows.length === 0) return; - const widths = widthsOverride ?? computeTableWidths(headers, rows); + // Allow the Context column (index 4) up to 60 chars so reason text is not truncated. + const widths = widthsOverride ?? computeTableWidths(headers, rows, [40, 40, 40, 40, 60]); const line = (left: string, mid: string, right: string) => left + widths.map(w => "─".repeat(w + 2)).join(mid) + right; @@ -753,10 +780,11 @@ function printParentUpgradeTargetsTable( console.log(line("└", "┴", "┘")); } -function computeTableWidths(headers: string[], rows: string[][]): number[] { - return headers.map((header, index) => - Math.min(40, Math.max(header.length, ...rows.map(row => stripAnsi(String(row[index])).length))), - ); +function computeTableWidths(headers: string[], rows: string[][], columnMaxWidths?: number[]): number[] { + return headers.map((header, index) => { + const maxWidth = columnMaxWidths?.[index] ?? 40; + return Math.min(maxWidth, Math.max(header.length, ...rows.map(row => stripAnsi(String(row[index])).length))); + }); } function computeSharedDirectTableWidths( @@ -843,5 +871,5 @@ function computeSharedParentUpgradeTableWidths( } } - return computeTableWidths(headers, rows); + return computeTableWidths(headers, rows, [40, 40, 40, 40, 60]); } diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 9c05640..79c14f9 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -1,6 +1,7 @@ import type { Finding, ScanInput, SeverityLabel } from "../types.js"; import { severityOrder } from "../constants.js"; import { compareVersions, looksLikeVersion } from "../utils/version.js"; +import { getPrimaryParent } from "../utils/finding.js"; export type SuggestedFixPackageManager = "npm" | "pnpm" | "yarn" | "bun"; @@ -186,12 +187,26 @@ export function buildSuggestedFixCommandPlan( continue; } + const primaryParent = getPrimaryParent(finding); + if (finding.relationship === "transitive" && primaryParent) { + const fixClause = finding.firstFixedVersion + ? ` — check for a release that resolves ${finding.pkg.name} to ${finding.firstFixedVersion}+` + : ""; + skippedByKey.set(`${finding.relationship}:${finding.pkg.name}@${finding.pkg.version}`, { + package: finding.pkg.name, + version: finding.pkg.version, + relationship: finding.relationship, + reason: `${finding.pkg.name}@${finding.pkg.version} is pulled in by ${primaryParent}. No safe upgrade version for ${primaryParent} was identified automatically${fixClause}.`, + }); + continue; + } + skippedByKey.set(`${finding.relationship}:${finding.pkg.name}@${finding.pkg.version}`, { package: finding.pkg.name, version: finding.pkg.version, relationship: finding.relationship, reason: finding.relationship === "transitive" - ? "No specific parent upgrade target was found for this transitive issue." + ? `No dependency path available for ${finding.pkg.name}@${finding.pkg.version}. Inspect your lockfile to find which package pulls it in.` : "No confident automatic fix command could be generated for this issue.", }); } diff --git a/src/utils/finding.ts b/src/utils/finding.ts new file mode 100644 index 0000000..18d8562 --- /dev/null +++ b/src/utils/finding.ts @@ -0,0 +1,7 @@ +import type { Finding } from "../types.js"; + +export function getPrimaryParent(finding: Finding): string | null { + const firstPath = finding.dependencyPaths?.[0]; + if (!firstPath || firstPath.length < 3) return null; + return firstPath[1] ?? null; +} diff --git a/tests/output.test.ts b/tests/output.test.ts index 78cca69..5a307df 100644 --- a/tests/output.test.ts +++ b/tests/output.test.ts @@ -2,8 +2,8 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { jest } from "@jest/globals"; +import { getPrimaryParent } from "../src/utils/finding.js"; import { - getPrimaryParent, getRecommendedAction, logInfo, logWarn, @@ -107,6 +107,18 @@ function captureLogs(run: () => void): string[] { } describe("output formatters", () => { + it("getPrimaryParent returns null for paths shorter than 3 nodes", () => { + const shortPath = createFinding({ + dependencyPaths: [["project", "lodash"]], + }); + expect(getPrimaryParent(shortPath)).toBeNull(); + }); + + it("getPrimaryParent returns null for empty paths", () => { + const noPath = createFinding({ dependencyPaths: [] }); + expect(getPrimaryParent(noPath)).toBeNull(); + }); + it("derives the primary parent and recommendation text from findings", () => { const finding = createFinding(); @@ -434,6 +446,103 @@ describe("output formatters", () => { ]); }); + it("getRecommendedAction Tier 2: names the parent and is honest about no known safe version", () => { + const finding = createFinding({ + relationship: "transitive", + dependencyPaths: [["project", "app", "lodash"]], + recommendedParentUpgrade: undefined, + firstFixedVersion: "4.17.21", + }); + const action = getRecommendedAction(finding); + expect(action).toContain("Upgrade app"); + expect(action).toContain("no safe version was identified automatically"); + expect(action).toContain("4.17.21"); + }); + + it("getRecommendedAction Tier 3: honest message when no dependency path exists", () => { + const finding = createFinding({ + relationship: "transitive", + dependencyPaths: [], + recommendedParentUpgrade: undefined, + firstFixedVersion: "4.17.21", + }); + const action = getRecommendedAction(finding); + expect(action).toContain("No dependency path found for lodash"); + expect(action).toContain("Inspect your lockfile"); + expect(action).not.toContain("Upgrade the parent dependency chain"); + }); + + it("summarizeNextAction Tier 2: names the parent when path is available", () => { + const finding = createFinding({ + relationship: "transitive", + dependencyPaths: [["project", "app", "lodash"]], + recommendedParentUpgrade: undefined, + firstFixedVersion: "4.17.21", + }); + const summary = summarizeNextAction(finding); + expect(summary).toContain("Upgrade app"); + expect(summary).toContain("no safe version identified"); + expect(summary).toContain("4.17.21"); + }); + + it("summarizeNextAction Tier 3: honest message when no path exists", () => { + const finding = createFinding({ + relationship: "transitive", + dependencyPaths: [], + recommendedParentUpgrade: undefined, + firstFixedVersion: "4.17.21", + }); + const summary = summarizeNextAction(finding); + expect(summary).toContain("No dependency path found for lodash"); + expect(summary).not.toContain("Upgrade the parent dependency chain"); + }); + + it("skips Tier 2 transitive finding with parent-aware reason", () => { + const findings = [ + createFinding({ + pkg: { name: "picomatch", version: "2.2.1", ecosystem: "npm", paths: [["project", "lint-staged", "picomatch"]] }, + relationship: "transitive", + dependencyPaths: [["project", "lint-staged", "picomatch"]], + severity: "high", + firstFixedVersion: "2.3.1", + recommendedParentUpgrade: undefined, + }), + ]; + + const plan = buildSuggestedFixCommandPlan(findings, createScanInputForSource("package-lock")); + + expect(plan?.skipped).toEqual([ + expect.objectContaining({ + package: "picomatch", + reason: expect.stringContaining("lint-staged"), + }), + ]); + expect(plan?.skipped[0].reason).toContain("2.3.1"); + expect(plan?.skipped[0].reason).not.toBe("No specific parent upgrade target was found for this transitive issue."); + }); + + it("skips Tier 3 transitive finding with honest no-path reason", () => { + const findings = [ + createFinding({ + pkg: { name: "picomatch", version: "2.2.1", ecosystem: "npm", paths: [] }, + relationship: "transitive", + dependencyPaths: [], + severity: "high", + firstFixedVersion: "2.3.1", + recommendedParentUpgrade: undefined, + }), + ]; + + const plan = buildSuggestedFixCommandPlan(findings, createScanInputForSource("package-lock")); + + expect(plan?.skipped).toEqual([ + expect.objectContaining({ + package: "picomatch", + reason: expect.stringContaining("No dependency path available"), + }), + ]); + }); + it("moves unpublishable fixed-version hints out of runnable commands", () => { const findings = [ createFinding({ @@ -867,6 +976,76 @@ describe("output printers", () => { expect(emptyLines).toContain("✔ Scan complete. No known vulnerabilities found."); }); + it("renders Context column for parent-upgrade targets in urgent sections", () => { + 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: "lodash", version: "4.17.20", ecosystem: "npm", paths: [["project", "app", "lodash"]] }, + relationship: "transitive", + dependencyPaths: [["project", "app", "lodash"]], + severity: "critical", + firstFixedVersion: "4.17.21", + recommendedParentUpgrade: { + package: "app", + currentVersion: "1.0.0", + targetVersion: "1.1.0", + viaPath: ["project", "app", "lodash"], + vulnerablePackage: "lodash", + confidence: "exact-direct-child", + reason: "app@1.1.0 no longer allows lodash@4.17.20", + }, + }), + ]; + + const lines = captureLogs(() => { + printSuggestedFixCommands(findings, createScanInputForSource("package-lock")); + }); + const output = lines.join("\n"); + + expect(output).toContain("Context"); + expect(output).toContain("Parent upgrade for vulnerable lodash@4.17.20"); + expect(output).toContain("> npm install minimist@1.2.8 app@1.1.0"); + }); + + it("renders Context column for urgent sections containing only parent-upgrade targets", () => { + const findings = [ + createFinding({ + pkg: { name: "lodash", version: "4.17.20", ecosystem: "npm", paths: [["project", "app", "lodash"]] }, + relationship: "transitive", + dependencyPaths: [["project", "app", "lodash"]], + severity: "critical", + firstFixedVersion: "4.17.21", + recommendedParentUpgrade: { + package: "app", + currentVersion: "1.0.0", + targetVersion: "1.1.0", + viaPath: ["project", "app", "lodash"], + vulnerablePackage: "lodash", + confidence: "exact-direct-child", + reason: "app@1.1.0 no longer allows lodash@4.17.20", + }, + }), + ]; + + const lines = captureLogs(() => { + printSuggestedFixCommands(findings, createScanInputForSource("package-lock")); + }); + const output = lines.join("\n"); + + expect(output).toContain("Context"); + expect(output).toContain("Parent upgrade for vulnerable lodash@4.17.20"); + expect(output).not.toContain("Versions scanned"); + expect(output).not.toContain("Breaking?"); + expect(output).toContain("> npm install app@1.1.0"); + }); + it("prints compact validation summary when scanned-version metrics are available", () => { const lines = captureLogs(() => { printCompactOutput(