diff --git a/__tests__/fixtures/mock-change.ts b/__tests__/fixtures/mock-change.ts new file mode 100644 index 0000000..ebe7ff7 --- /dev/null +++ b/__tests__/fixtures/mock-change.ts @@ -0,0 +1,35 @@ +import {Change} from '../../src/schemas' + +const defaultChange: Change = { + change_type: 'added', + manifest: 'package.json', + ecosystem: 'npm', + name: 'lodash', + version: '4.17.20', + package_url: 'pkg:npm/lodash@4.17.20', + license: 'MIT', + source_repository_url: 'https://github.com/lodash/lodash', + scope: 'runtime', + vulnerabilities: [ + { + severity: 'high', + advisory_ghsa_id: 'GHSA-35jh-r3h4-6jhm', + advisory_summary: 'Command Injection in lodash', + advisory_url: 'https://github.com/advisories/GHSA-35jh-r3h4-6jhm' + }, + { + severity: 'moderate', + advisory_ghsa_id: 'GHSA-29mw-wpgm-hmr9', + advisory_summary: + 'Regular Expression Denial of Service (ReDoS) in lodash', + advisory_url: 'https://github.com/advisories/GHSA-29mw-wpgm-hmr9' + } + ] +} + +const createTestChange = (overwrites: Partial = {}): Change => ({ + ...defaultChange, + ...overwrites +}) + +export {createTestChange} diff --git a/__tests__/summary.test.ts b/__tests__/summary.test.ts new file mode 100644 index 0000000..08f2351 --- /dev/null +++ b/__tests__/summary.test.ts @@ -0,0 +1,110 @@ +import {expect, jest, test} from '@jest/globals' +import {Change, Changes, ConfigurationOptions} from '../src/schemas' +import * as summary from '../src/summary'; +import * as core from '@actions/core'; +import { createTestChange } from './fixtures/mock-change'; + + +afterEach(() => { + jest.clearAllMocks(); + core.summary.emptyBuffer(); +}); + +const emptyChanges: Changes = []; +const emptyInvalidLicenseChanges = { + forbidden: [], + unresolved: [], + unlicensed: [] +}; +const defaultConfig: ConfigurationOptions = { + vulnerability_check: true, + license_check: true, + fail_on_severity: 'high', + fail_on_scopes: ['runtime'], + allow_ghsas: [], + allow_licenses: [], + deny_licenses: [], + comment_summary_in_pr: true, +} + +test('prints headline as h2', () => { + summary.addSummaryToSummary(emptyChanges, emptyInvalidLicenseChanges, defaultConfig); + const text = core.summary.stringify(); + + expect(text).toContain('

Dependency Review

'); +}); + +test('only includes "No vulnerabilities or license issues found"-message if both are configured and nothing was found', () => { + summary.addSummaryToSummary(emptyChanges, emptyInvalidLicenseChanges, defaultConfig); + const text = core.summary.stringify(); + + expect(text).toContain('✅ No vulnerabilities or license issues found.'); +}); + +test('only includes "No vulnerabilities found"-message if "license_check" is set to false and nothing was found', () => { + const config = {...defaultConfig, license_check: false}; + summary.addSummaryToSummary(emptyChanges, emptyInvalidLicenseChanges, config); + const text = core.summary.stringify(); + + expect(text).toContain('✅ No vulnerabilities found.'); +}); + +test('only includes "No license issues found"-message if "vulnerability_check" is set to false and nothing was found', () => { + const config = {...defaultConfig, vulnerability_check: false}; + summary.addSummaryToSummary(emptyChanges, emptyInvalidLicenseChanges, config); + const text = core.summary.stringify(); + + expect(text).toContain('✅ No license issues found.'); +}); + +test('does not include status section if nothing was found', () => { + summary.addSummaryToSummary(emptyChanges, emptyInvalidLicenseChanges, defaultConfig); + const text = core.summary.stringify(); + + expect(text).not.toContain('The following issues were found:'); +}); + + +test('includes count and status icons for all findings', () => { + const vulnerabilities = [ + createTestChange({ name: 'lodash'}), + createTestChange({ name: 'underscore', package_url: 'test-url'}), + ]; + const licenseIssues = { + forbidden: [createTestChange()], + unresolved: [createTestChange(), createTestChange()], + unlicensed: [createTestChange(), createTestChange(), createTestChange()], + }; + + summary.addSummaryToSummary(vulnerabilities, licenseIssues, defaultConfig); + + const text = core.summary.stringify(); + expect(text).toContain('❌ 2 vulnerable package(s)'); + expect(text).toContain('❌ 2 package(s) with invalid SPDX license definitions'); + expect(text).toContain('❌ 1 package(s) with incompatible licenses'); + expect(text).toContain('⚠️ 3 package(s) with unknown licenses'); +}); + +test('uses checkmarks for license issues if only vulnerabilities were found', () => { + const vulnerabilities = [ createTestChange() ]; + + summary.addSummaryToSummary(vulnerabilities, emptyInvalidLicenseChanges, defaultConfig); + + const text = core.summary.stringify(); + expect(text).toContain('❌ 1 vulnerable package(s)'); + expect(text).toContain('✅ 0 package(s) with invalid SPDX license definitions'); + expect(text).toContain('✅ 0 package(s) with incompatible licenses'); + expect(text).toContain('✅ 0 package(s) with unknown licenses'); +}); + +test('uses checkmarks for vulnerabilities if only license issues were found.', () => { + const licenseIssues = { forbidden: [createTestChange()], unresolved: [], unlicensed: [] }; + + summary.addSummaryToSummary(emptyChanges, licenseIssues, defaultConfig); + + const text = core.summary.stringify(); + expect(text).toContain('✅ 0 vulnerable package(s)'); + expect(text).toContain('✅ 0 package(s) with invalid SPDX license definitions'); + expect(text).toContain('❌ 1 package(s) with incompatible licenses'); + expect(text).toContain('✅ 0 package(s) with unknown licenses'); +}); diff --git a/src/licenses.ts b/src/licenses.ts index abc2d06..76fc966 100644 --- a/src/licenses.ts +++ b/src/licenses.ts @@ -14,19 +14,21 @@ import {isSPDXValid, octokitClient} from './utils' * @param { { allow?: string[], deny?: string[]}} licenses An object with `allow`/`deny` keys, each containing a list of licenses. * @returns {Promise<{Object.>}} A promise to a Record Object. The keys are strings, unlicensed, unresolved and forbidden. The values are a list of changes */ +type InvalidLicenseChangeTypes = 'unlicensed' | 'unresolved' | 'forbidden' +export type InvalidLicenseChanges = Record export async function getInvalidLicenseChanges( changes: Change[], licenses: { allow?: string[] deny?: string[] } -): Promise> { +): Promise { const {allow, deny} = licenses const groupedChanges = await groupChanges(changes) const licensedChanges: Changes = groupedChanges.licensed - const invalidLicenseChanges: Record = { + const invalidLicenseChanges: InvalidLicenseChanges = { unlicensed: groupedChanges.unlicensed, unresolved: [], forbidden: [] diff --git a/src/main.ts b/src/main.ts index 9885998..67a2d8d 100644 --- a/src/main.ts +++ b/src/main.ts @@ -54,10 +54,7 @@ async function run(): Promise { } ) - summary.addSummaryToSummary( - config.vulnerability_check ? addedChanges : null, - config.license_check ? invalidLicenseChanges : null - ) + summary.addSummaryToSummary(addedChanges, invalidLicenseChanges, config) if (config.vulnerability_check) { summary.addChangeVulnerabilitiesToSummary(addedChanges, minSeverity) diff --git a/src/summary.ts b/src/summary.ts index 19844b9..75b5377 100644 --- a/src/summary.ts +++ b/src/summary.ts @@ -1,27 +1,83 @@ import * as core from '@actions/core' import {ConfigurationOptions, Changes} from './schemas' import {SummaryTableRow} from '@actions/core/lib/summary' +import {InvalidLicenseChanges} from './licenses' import {groupDependenciesByManifest, getManifestsSet, renderUrl} from './utils' -export function addSummaryToSummary( - addedPackages: Changes | null, - invalidLicenseChanges: Record | null +const icons = { + check: '✅', + cross: '❌', + warning: '⚠️' +} + +export function createSummary( + addedChanges: Changes, + invalidLicenseChanges: InvalidLicenseChanges, + config: ConfigurationOptions ): void { - core.summary - .addHeading('Dependency Review') - .addRaw('We found:') - .addList([ - ...(addedPackages - ? [`${addedPackages.length} vulnerable package(s)`] - : []), - ...(invalidLicenseChanges - ? [ - `${invalidLicenseChanges.unresolved.length} package(s) with invalid SPDX license definitions`, - `${invalidLicenseChanges.forbidden.length} package(s) with incompatible licenses`, - `${invalidLicenseChanges.unlicensed.length} package(s) with unknown licenses.` - ] - : []) - ]) + addSummaryToSummary( + config.vulnerability_check ? addedChanges : [], + config.license_check + ? invalidLicenseChanges + : {unresolved: [], forbidden: [], unlicensed: []}, + config + ) + + if (config.vulnerability_check && addedChanges.length > 0) { + addChangeVulnerabilitiesToSummary(addedChanges, config.fail_on_severity) + } + + if (config.license_check && invalidLicenseChanges.unresolved.length > 0) { + addLicensesToSummary(invalidLicenseChanges, config) + } +} + +export function addSummaryToSummary( + addedPackages: Changes, + invalidLicenseChanges: InvalidLicenseChanges, + config: ConfigurationOptions +): void { + core.summary.addHeading('Dependency Review', 2) + + if ( + addedPackages.length === 0 && + countLicenseIssues(invalidLicenseChanges) === 0 + ) { + if (!config.license_check) { + core.summary.addRaw(`${icons.check} No vulnerabilities found.`) + } else if (!config.vulnerability_check) { + core.summary.addRaw(`${icons.check} No license issues found.`) + } else { + core.summary.addRaw( + `${icons.check} No vulnerabilities or license issues found.` + ) + } + } else { + core.summary + .addRaw('The following issues were found:') + .addList([ + ...(config.vulnerability_check + ? [ + `${checkOrFail(addedPackages.length)} ${ + addedPackages.length + } vulnerable package(s)` + ] + : []), + ...(config.license_check + ? [ + `${checkOrFail(invalidLicenseChanges.unresolved.length)} ${ + invalidLicenseChanges.unresolved.length + } package(s) with invalid SPDX license definitions`, + `${checkOrFail(invalidLicenseChanges.forbidden.length)} ${ + invalidLicenseChanges.forbidden.length + } package(s) with incompatible licenses`, + `${checkOrWarn(invalidLicenseChanges.unlicensed.length)} ${ + invalidLicenseChanges.unlicensed.length + } package(s) with unknown licenses.` + ] + : []) + ]) + } } export function addChangeVulnerabilitiesToSummary( @@ -33,16 +89,11 @@ export function addChangeVulnerabilitiesToSummary( const manifests = getManifestsSet(addedPackages) core.summary - .addHeading('Vulnerabilities') + .addHeading('Vulnerabilities', 3) .addQuote( `Vulnerabilities were filtered by minimum severity ${severity}.` ) - if (addedPackages.length === 0) { - core.summary.addQuote('No vulnerabilities found in added packages.') - return - } - for (const manifest of manifests) { for (const change of addedPackages.filter( pkg => pkg.manifest === manifest @@ -88,7 +139,7 @@ export function addLicensesToSummary( invalidLicenseChanges: Record, config: ConfigurationOptions ): void { - core.summary.addHeading('Licenses') + core.summary.addHeading('License Issues', 3) if (config.allow_licenses && config.allow_licenses.length > 0) { core.summary.addQuote( @@ -161,7 +212,7 @@ export function addScannedDependencies(changes: Changes): void { const manifests = dependencies.keys() const summary = core.summary - .addHeading('Scanned Dependencies') + .addHeading('Scanned Dependencies', 3) .addHeading(`We scanned ${dependencies.size} manifest files:`, 5) for (const manifest of manifests) { @@ -174,3 +225,20 @@ export function addScannedDependencies(changes: Changes): void { } } } + +function countLicenseIssues( + invalidLicenseChanges: InvalidLicenseChanges +): number { + return Object.values(invalidLicenseChanges).reduce( + (acc, val) => acc + val.length, + 0 + ) +} + +function checkOrFail(count: number): string { + return count === 0 ? icons.check : icons.cross +} + +function checkOrWarn(count: number): string { + return count === 0 ? icons.check : icons.warning +}