feat: Adjusts the formatting and content for the status header

__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> = {}): Change => ({
+  ...defaultChange,
+  ...overwrites
+})
+
+export {createTestChange}

__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('<h2>Dependency Review</h2>');
+});
+
+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');
+});

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.<string, Array.<Change>>}} 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<InvalidLicenseChangeTypes, Changes>
 export async function getInvalidLicenseChanges(
   changes: Change[],
   licenses: {
     allow?: string[]
     deny?: string[]
   }
-): Promise<Record<string, Changes>> {
+): Promise<InvalidLicenseChanges> {
   const {allow, deny} = licenses
 
   const groupedChanges = await groupChanges(changes)
   const licensedChanges: Changes = groupedChanges.licensed
 
-  const invalidLicenseChanges: Record<string, Changes> = {
+  const invalidLicenseChanges: InvalidLicenseChanges = {
     unlicensed: groupedChanges.unlicensed,
     unresolved: [],
     forbidden: []

src/main.ts

@@ -54,10 +54,7 @@ async function run(): Promise<void> {
       }
     )
 
-    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)

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<string, Changes> | 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 <strong>${severity}</strong>.`
     )
 
-  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<string, Changes>,
   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
+}