From 425e11f59271e4ee08d49167739211d64a123af7 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 18:11:37 -0400 Subject: [PATCH] Add `show-fixes-in-summary` to surface vulnerabilities resolved by a PR (#10) * Initial plan * Add show-fixes-in-summary feature to display fixed vulnerabilities in PR summary Agent-Logs-Url: https://github.com/forks-felickz/dependency-review-action/sessions/d4753d81-a434-4948-8f3d-48fd04a79bf9 Co-authored-by: felickz <1760475+felickz@users.noreply.github.com> * fix: gate hasIssues on enabled checks and document show-fixes-in-summary --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: felickz <1760475+felickz@users.noreply.github.com> --- README.md | 1 + __tests__/summary.test.ts | 181 +++++++++++++++++++++++++- action.yml | 3 + src/config.ts | 4 +- src/main.ts | 14 +- src/schemas.ts | 1 + src/summary.ts | 261 +++++++++++++++++++++++++++++++------- 7 files changed, 413 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 9c99cd135..5484e342d 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,7 @@ All configuration options are optional. | `show-openssf-scorecard` | When set to `true`, the action will output information about all the known OpenSSF Scorecard scores for the dependencies changed in this pull request. | `true`, `false` | `true` | | `warn-on-openssf-scorecard-level` | When `show-openssf-scorecard-levels` is set to `true`, this option lets you configure the threshold for when a score is considered too low and gets a :warning: warning in the CI. | Any positive integer | 3 | | `show-patched-versions`\* | When set to `true`, the vulnerability summary table will include an additional column showing the first patched version for each vulnerability. This requires additional API calls to fetch advisory data. | `true`, `false` | `false` | +| `show-fixes-in-summary` | When set to `true`, the summary will include a "🎉 Issues Fixed" section showing vulnerabilities resolved by the PR. | `true`, `false` | `false` | > [!NOTE] > diff --git a/__tests__/summary.test.ts b/__tests__/summary.test.ts index eec1f4b80..192c54fd9 100644 --- a/__tests__/summary.test.ts +++ b/__tests__/summary.test.ts @@ -48,7 +48,8 @@ const defaultConfig: ConfigurationOptions = { warn_only: false, warn_on_openssf_scorecard_level: 3, show_openssf_scorecard: false, - show_patched_versions: false + show_patched_versions: false, + show_fixes_in_summary: false } const changesWithEmptyManifests: Changes = [ @@ -876,3 +877,181 @@ test('addChangeVulnerabilitiesToSummary() - completes all tasks even with varyin expect(completedAdvisories.size).toBe(20) expect(mockOctokitRequest).toHaveBeenCalledTimes(20) }) + +test('addSummaryToSummary() - shows three-section layout when show_fixes_in_summary is true and fixes exist', () => { + const config = {...defaultConfig, show_fixes_in_summary: true} + const vulnerabilities = [createTestChange({name: 'lodash'})] + const fixedVulns = [ + createTestChange({ + name: 'some-package', + version: '1.0.0', + change_type: 'removed', + vulnerabilities: [ + { + severity: 'high', + advisory_ghsa_id: 'GHSA-1234-abcd', + advisory_summary: 'Insecure Temporary File', + advisory_url: 'https://github.com/advisories/GHSA-1234-abcd' + } + ] + }) + ] + + summary.addSummaryToSummary( + vulnerabilities, + emptyInvalidLicenseChanges, + emptyChanges, + emptyScorecard, + config, + fixedVulns + ) + + const text = core.summary.stringify() + expect(text).toContain('Action Needed') + expect(text).toContain('Issues Fixed') + expect(text).toContain('Checks Passed') + expect(text).not.toContain('The following issues were found:') +}) + +test('addSummaryToSummary() - shows fixed vuln details in Issues Fixed section', () => { + const config = {...defaultConfig, show_fixes_in_summary: true} + const fixedVulns = [ + createTestChange({ + name: 'other-package', + version: '2.0.0', + change_type: 'removed', + vulnerabilities: [ + { + severity: 'critical', + advisory_ghsa_id: 'GHSA-5678-efgh', + advisory_summary: 'CVE-2022-XXXX', + advisory_url: 'https://github.com/advisories/GHSA-5678-efgh' + } + ] + }) + ] + + summary.addSummaryToSummary( + emptyChanges, + emptyInvalidLicenseChanges, + emptyChanges, + emptyScorecard, + config, + fixedVulns + ) + + const text = core.summary.stringify() + expect(text).toContain('CVE-2022-XXXX') + expect(text).toContain('other-package@2.0.0') + expect(text).toContain('critical severity') +}) + +test('addSummaryToSummary() - does not show three-section layout when show_fixes_in_summary is false', () => { + const config = {...defaultConfig, show_fixes_in_summary: false} + const vulnerabilities = [createTestChange({name: 'lodash'})] + const fixedVulns = [ + createTestChange({ + name: 'some-package', + change_type: 'removed' + }) + ] + + summary.addSummaryToSummary( + vulnerabilities, + emptyInvalidLicenseChanges, + emptyChanges, + emptyScorecard, + config, + fixedVulns + ) + + const text = core.summary.stringify() + expect(text).toContain('The following issues were found:') + expect(text).not.toContain('Issues Fixed') + expect(text).not.toContain('Action Needed') +}) + +test('addSummaryToSummary() - does not show three-section layout when there are no fixed vulns', () => { + const config = {...defaultConfig, show_fixes_in_summary: true} + const vulnerabilities = [createTestChange({name: 'lodash'})] + + summary.addSummaryToSummary( + vulnerabilities, + emptyInvalidLicenseChanges, + emptyChanges, + emptyScorecard, + config, + [] // no fixed vulns + ) + + const text = core.summary.stringify() + expect(text).toContain('The following issues were found:') + expect(text).not.toContain('Issues Fixed') +}) + +test('addSummaryToSummary() - shows only Issues Fixed and Checks Passed when no action needed but fixes exist', () => { + const config = {...defaultConfig, show_fixes_in_summary: true} + const fixedVulns = [ + createTestChange({ + name: 'fixed-package', + version: '1.0.0', + change_type: 'removed', + vulnerabilities: [ + { + severity: 'moderate', + advisory_ghsa_id: 'GHSA-abcd-1234', + advisory_summary: 'Some advisory', + advisory_url: 'https://github.com/advisories/GHSA-abcd-1234' + } + ] + }) + ] + + summary.addSummaryToSummary( + emptyChanges, + emptyInvalidLicenseChanges, + emptyChanges, + emptyScorecard, + config, + fixedVulns + ) + + const text = core.summary.stringify() + expect(text).toContain('Issues Fixed') + expect(text).toContain('Checks Passed') + expect(text).not.toContain('Action Needed') + expect(text).not.toContain('The following issues were found:') +}) + +test('addFixedVulnerabilitiesToSummary() - does nothing when no fixed vulns', () => { + summary.addFixedVulnerabilitiesToSummary([]) + const text = core.summary.stringify() + expect(text).toEqual('') +}) + +test('addFixedVulnerabilitiesToSummary() - renders fixed vulnerabilities table', () => { + const fixedVulns = [ + createTestChange({ + name: 'some-package', + version: '1.0.0', + change_type: 'removed', + source_repository_url: 'https://github.com/some/package', + vulnerabilities: [ + { + severity: 'high', + advisory_ghsa_id: 'GHSA-1234-abcd', + advisory_summary: 'Insecure Temporary File', + advisory_url: 'https://github.com/advisories/GHSA-1234-abcd' + } + ] + }) + ] + + summary.addFixedVulnerabilitiesToSummary(fixedVulns) + + const text = core.summary.stringify() + expect(text).toContain('

Fixed Vulnerabilities

') + expect(text).toContain('some-package') + expect(text).toContain('Insecure Temporary File') + expect(text).toContain('high') +}) diff --git a/action.yml b/action.yml index 957168159..a8b5dea1c 100644 --- a/action.yml +++ b/action.yml @@ -79,6 +79,9 @@ inputs: show-patched-versions: description: When set to `true`, the vulnerability summary table will include a column showing the first patched version for each vulnerability. required: false + show-fixes-in-summary: + description: When set to `true`, the summary will include a "🎉 Issues Fixed" section showing vulnerabilities resolved by the PR. + required: false outputs: comment-content: description: Prepared dependency report comment diff --git a/src/config.ts b/src/config.ts index 9bba05083..9b33e691f 100644 --- a/src/config.ts +++ b/src/config.ts @@ -53,6 +53,7 @@ function readInlineConfig(): ConfigurationOptionsPartial { 'warn-on-openssf-scorecard-level' ) const show_patched_versions = getOptionalBoolean('show-patched-versions') + const show_fixes_in_summary = getOptionalBoolean('show-fixes-in-summary') validateLicenses('allow-licenses', allow_licenses) validateLicenses('deny-licenses', deny_licenses) @@ -76,7 +77,8 @@ function readInlineConfig(): ConfigurationOptionsPartial { warn_only, show_openssf_scorecard, warn_on_openssf_scorecard_level, - show_patched_versions + show_patched_versions, + show_fixes_in_summary } return Object.fromEntries( diff --git a/src/main.ts b/src/main.ts index a23fc00c6..76ba499ac 100644 --- a/src/main.ts +++ b/src/main.ts @@ -167,6 +167,14 @@ async function run(): Promise { filteredChanges ) + const fixedVulns: Changes = config.show_fixes_in_summary + ? filteredChanges.filter( + change => + change.change_type === 'removed' && + change.vulnerabilities.length > 0 + ) + : [] + const invalidLicenseChanges = await getInvalidLicenseChanges( filteredChanges, { @@ -197,7 +205,8 @@ async function run(): Promise { invalidLicenseChanges, deniedChanges, scorecard, - config + config, + fixedVulns ) if (snapshot_warnings) { @@ -237,6 +246,9 @@ async function run(): Promise { printScorecardBlock(scorecard, config) createScorecardWarnings(scorecard, config) } + if (config.show_fixes_in_summary) { + summary.addFixedVulnerabilitiesToSummary(fixedVulns) + } core.setOutput('dependency-changes', JSON.stringify(changes)) summary.addScannedFiles(changes) diff --git a/src/schemas.ts b/src/schemas.ts index 077377dc2..155519c40 100644 --- a/src/schemas.ts +++ b/src/schemas.ts @@ -116,6 +116,7 @@ export const ConfigurationOptionsSchema = z show_openssf_scorecard: z.boolean().optional().default(true), warn_on_openssf_scorecard_level: z.number().default(3), show_patched_versions: z.boolean().default(false), + show_fixes_in_summary: z.boolean().default(false), comment_summary_in_pr: z .union([ z.preprocess( diff --git a/src/summary.ts b/src/summary.ts index 892b38e87..544b0a244 100644 --- a/src/summary.ts +++ b/src/summary.ts @@ -130,7 +130,8 @@ export function addSummaryToSummary( invalidLicenseChanges: InvalidLicenseChanges, deniedChanges: Changes, scorecard: Scorecard, - config: ConfigurationOptions + config: ConfigurationOptions, + fixedVulns: Changes = [] ): string { if (config.deny_licenses && config.deny_licenses.length > 0) { addDenyListsDeprecationWarningToSummary() @@ -144,12 +145,15 @@ export function addSummaryToSummary( core.summary.addHeading('Dependency Review', 1) out.push('# Dependency Review') - if ( - vulnerableChanges.length === 0 && - licenseIssues === 0 && - deniedChanges.length === 0 && - scorecardWarnings === 0 - ) { + const hasIssues = + (config.vulnerability_check && vulnerableChanges.length > 0) || + (config.license_check && licenseIssues > 0) || + deniedChanges.length > 0 || + (config.show_openssf_scorecard && scorecardWarnings > 0) + + const showFixesSection = config.show_fixes_in_summary && fixedVulns.length > 0 + + if (!hasIssues && !showFixesSection) { const issueTypes = [ config.vulnerability_check ? 'vulnerabilities' : '', config.license_check ? 'license issues' : '', @@ -168,48 +172,90 @@ export function addSummaryToSummary( return out.join('\n') } - const foundIssuesHeader = 'The following issues were found:' - core.summary.addRaw(foundIssuesHeader) - out.push(foundIssuesHeader) - - const summaryList: string[] = [ - ...(config.vulnerability_check - ? [ - `${checkOrFailIcon(vulnerableChanges.length)} ${ - vulnerableChanges.length - } vulnerable package(s)` - ] - : []), - ...(config.license_check - ? [ - `${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${ - invalidLicenseChanges.forbidden.length - } package(s) with incompatible licenses`, - `${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${ - invalidLicenseChanges.unresolved.length - } package(s) with invalid SPDX license definitions`, - `${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${ - invalidLicenseChanges.unlicensed.length - } package(s) with unknown licenses.` - ] - : []), - ...(deniedChanges.length > 0 - ? [ - `${checkOrWarnIcon(deniedChanges.length)} ${ - deniedChanges.length - } package(s) denied.` - ] - : []), - ...(config.show_openssf_scorecard && scorecardWarnings > 0 - ? [ - `${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.` - ] - : []) - ] - - core.summary.addList(summaryList) - for (const line of summaryList) { - out.push(`* ${line}`) + if (showFixesSection) { + if (hasIssues) { + core.summary.addRaw(`${icons.cross} Action Needed`, true) + out.push(`${icons.cross} **Action Needed**`) + const actionList = buildActionNeededList( + vulnerableChanges, + invalidLicenseChanges, + deniedChanges, + scorecardWarnings, + config + ) + core.summary.addList(actionList) + for (const line of actionList) { + out.push(`* ${line}`) + } + } + + core.summary.addRaw(`🎉 Issues Fixed`, true) + out.push(`🎉 **Issues Fixed**`) + const fixedList = buildFixedVulnList(fixedVulns) + core.summary.addList(fixedList) + for (const line of fixedList) { + out.push(`* ${line}`) + } + + const passedList = buildChecksPassedList( + vulnerableChanges, + invalidLicenseChanges, + deniedChanges, + scorecardWarnings, + config + ) + if (passedList.length > 0) { + core.summary.addRaw(`${icons.check} Checks Passed`, true) + out.push(`${icons.check} **Checks Passed**`) + core.summary.addList(passedList) + for (const line of passedList) { + out.push(`* ${line}`) + } + } + } else { + const foundIssuesHeader = 'The following issues were found:' + core.summary.addRaw(foundIssuesHeader) + out.push(foundIssuesHeader) + + const summaryList: string[] = [ + ...(config.vulnerability_check + ? [ + `${checkOrFailIcon(vulnerableChanges.length)} ${ + vulnerableChanges.length + } vulnerable package(s)` + ] + : []), + ...(config.license_check + ? [ + `${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${ + invalidLicenseChanges.forbidden.length + } package(s) with incompatible licenses`, + `${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${ + invalidLicenseChanges.unresolved.length + } package(s) with invalid SPDX license definitions`, + `${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${ + invalidLicenseChanges.unlicensed.length + } package(s) with unknown licenses.` + ] + : []), + ...(deniedChanges.length > 0 + ? [ + `${checkOrWarnIcon(deniedChanges.length)} ${ + deniedChanges.length + } package(s) denied.` + ] + : []), + ...(config.show_openssf_scorecard && scorecardWarnings > 0 + ? [ + `${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.` + ] + : []) + ] + + core.summary.addList(summaryList) + for (const line of summaryList) { + out.push(`* ${line}`) + } } core.summary.addRaw('See the Details below.') @@ -227,6 +273,123 @@ function addDenyListsDeprecationWarningToSummary(): void { ) } +function buildActionNeededList( + vulnerableChanges: Changes, + invalidLicenseChanges: InvalidLicenseChanges, + deniedChanges: Changes, + scorecardWarnings: number, + config: ConfigurationOptions +): string[] { + const items: string[] = [] + if (config.vulnerability_check && vulnerableChanges.length > 0) { + items.push(`${vulnerableChanges.length} vulnerable package(s)`) + } + if (config.license_check) { + if (invalidLicenseChanges.forbidden.length > 0) { + items.push( + `${invalidLicenseChanges.forbidden.length} package(s) with incompatible licenses` + ) + } + if (invalidLicenseChanges.unresolved.length > 0) { + items.push( + `${invalidLicenseChanges.unresolved.length} package(s) with invalid SPDX license definitions` + ) + } + if (invalidLicenseChanges.unlicensed.length > 0) { + items.push( + `${invalidLicenseChanges.unlicensed.length} package(s) with unknown licenses` + ) + } + } + if (deniedChanges.length > 0) { + items.push(`${deniedChanges.length} package(s) denied`) + } + if (config.show_openssf_scorecard && scorecardWarnings > 0) { + items.push(`${scorecardWarnings} package(s) with OpenSSF Scorecard issues`) + } + return items +} + +function buildChecksPassedList( + vulnerableChanges: Changes, + invalidLicenseChanges: InvalidLicenseChanges, + deniedChanges: Changes, + scorecardWarnings: number, + config: ConfigurationOptions +): string[] { + const items: string[] = [] + if (config.vulnerability_check && vulnerableChanges.length === 0) { + items.push('No vulnerable packages') + } + if (config.license_check) { + if (invalidLicenseChanges.forbidden.length === 0) { + items.push('No incompatible licenses') + } + if (invalidLicenseChanges.unresolved.length === 0) { + items.push('No invalid SPDX license definitions') + } + if (invalidLicenseChanges.unlicensed.length === 0) { + items.push('No unknown licenses') + } + } + if (config.show_openssf_scorecard && scorecardWarnings === 0) { + items.push('No OpenSSF Scorecard issues') + } + // Only show denied check as passed if there's a deny list configured + if ( + (config.deny_packages.length > 0 || config.deny_groups.length > 0) && + deniedChanges.length === 0 + ) { + items.push('No denied packages') + } + return items +} + +function buildFixedVulnList(fixedVulns: Changes): string[] { + const items: string[] = [] + for (const change of fixedVulns) { + for (const vuln of change.vulnerabilities) { + items.push( + `Fixed: ${vuln.advisory_summary} in ${change.name}@${change.version} (${vuln.severity} severity)` + ) + } + } + return items +} + +export function addFixedVulnerabilitiesToSummary(fixedVulns: Changes): void { + if (fixedVulns.length === 0) { + return + } + + core.summary.addHeading('Fixed Vulnerabilities', 2) + + const manifests = getManifestsSet(fixedVulns) + + for (const manifest of manifests) { + const rows: SummaryTableRow[] = [] + for (const change of fixedVulns.filter(pkg => pkg.manifest === manifest)) { + for (const vuln of change.vulnerabilities) { + rows.push([ + renderUrl(change.source_repository_url, change.name), + change.version, + renderUrl(vuln.advisory_url, vuln.advisory_summary), + vuln.severity + ]) + } + } + core.summary.addHeading(`${manifest}`, 4).addTable([ + [ + {data: 'Name', header: true}, + {data: 'Version', header: true}, + {data: 'Vulnerability', header: true}, + {data: 'Severity', header: true} + ], + ...rows + ]) + } +} + function countScorecardWarnings( scorecard: Scorecard, config: ConfigurationOptions