Merge pull request #827 from ebickle/fix/comment-warn-only

fix: add summary comment on failure when warn-only: true

Author: Ahmed3lmallah

dist/index.js

@@ -17,13 +17,13 @@ const COMMENT_MARKER = '<!-- dependency-review-pr-comment-marker -->'
 
 export async function commentPr(
   commentContent: string,
-  config: ConfigurationOptions
+  config: ConfigurationOptions,
+  issueFound: boolean
 ): Promise<void> {
   if (
     !(
       config.comment_summary_in_pr === 'always' ||
-      (config.comment_summary_in_pr === 'on-failure' &&
-        process.exitCode === core.ExitCode.Failure)
+      (config.comment_summary_in_pr === 'on-failure' && issueFound)
     )
   ) {
     return

dist/index.js.map

@@ -35,12 +35,6 @@ export async function getDeniedChanges(
     }
   }
 
-  if (hasDeniedPackage) {
-    core.setFailed('Dependency review detected denied packages.')
-  } else {
-    core.info('Dependency review did not detect any denied packages')
-  }
-
   return changesDenied
 }
 

dist/licenses.txt

@@ -141,23 +141,29 @@ async function run(): Promise<void> {
       summary.addSnapshotWarnings(config, snapshot_warnings)
     }
 
+    let issueFound = false
+
     if (config.vulnerability_check) {
       core.setOutput('vulnerable-changes', JSON.stringify(vulnerableChanges))
       summary.addChangeVulnerabilitiesToSummary(vulnerableChanges, minSeverity)
-      printVulnerabilitiesBlock(vulnerableChanges, minSeverity, warnOnly)
+      issueFound ||= await printVulnerabilitiesBlock(
+        vulnerableChanges,
+        minSeverity,
+        warnOnly
+      )
     }
     if (config.license_check) {
       core.setOutput(
         'invalid-license-changes',
         JSON.stringify(invalidLicenseChanges)
       )
       summary.addLicensesToSummary(invalidLicenseChanges, config)
-      printLicensesBlock(invalidLicenseChanges, warnOnly)
+      issueFound ||= await printLicensesBlock(invalidLicenseChanges, warnOnly)
     }
     if (config.deny_packages || config.deny_groups) {
       core.setOutput('denied-changes', JSON.stringify(deniedChanges))
       summary.addDeniedToSummary(deniedChanges)
-      printDeniedDependencies(deniedChanges, config)
+      issueFound ||= await printDeniedDependencies(deniedChanges, config)
     }
     if (config.show_openssf_scorecard) {
       summary.addScorecardToSummary(scorecard, config)
@@ -182,7 +188,7 @@ async function run(): Promise<void> {
     }
 
     // update the PR comment if needed with the right-sized summary
-    await commentPr(rendered, config)
+    await commentPr(rendered, config, issueFound)
   } catch (error) {
     if (error instanceof RequestError && error.status === 404) {
       core.setFailed(
@@ -208,14 +214,12 @@ function printVulnerabilitiesBlock(
   addedChanges: Changes,
   minSeverity: Severity,
   warnOnly: boolean
-): void {
-  let vulFound = false
-  core.group('Vulnerabilities', async () => {
-    if (addedChanges.length > 0) {
-      for (const change of addedChanges) {
-        printChangeVulnerabilities(change)
-      }
-      vulFound = true
+): Promise<boolean> {
+  return core.group('Vulnerabilities', async () => {
+    let vulFound = false
+
+    for (const change of addedChanges) {
+      vulFound ||= printChangeVulnerabilities(change)
     }
 
     if (vulFound) {
@@ -230,10 +234,12 @@ function printVulnerabilitiesBlock(
         `Dependency review did not detect any vulnerable packages with severity level "${minSeverity}" or higher.`
       )
     }
+
+    return vulFound
   })
 }
 
-function printChangeVulnerabilities(change: Change): void {
+function printChangeVulnerabilities(change: Change): boolean {
   for (const vuln of change.vulnerabilities) {
     core.info(
       `${styles.bold.open}${change.manifest} » ${change.name}@${
@@ -244,14 +250,18 @@ function printChangeVulnerabilities(change: Change): void {
     )
     core.info(`  ↪ ${vuln.advisory_url}`)
   }
+  return change.vulnerabilities.length > 0
 }
 
 function printLicensesBlock(
   invalidLicenseChanges: Record<string, Changes>,
   warnOnly: boolean
-): void {
-  core.group('Licenses', async () => {
+): Promise<boolean> {
+  return core.group('Licenses', async () => {
+    let issueFound = false
+
     if (invalidLicenseChanges.forbidden.length > 0) {
+      issueFound = true
       core.info('\nThe following dependencies have incompatible licenses:')
       printLicensesError(invalidLicenseChanges.forbidden)
       const msg = 'Dependency review detected incompatible licenses.'
@@ -262,6 +272,7 @@ function printLicensesBlock(
       }
     }
     if (invalidLicenseChanges.unresolved.length > 0) {
+      issueFound = true
       core.warning(
         '\nThe validity of the licenses of the dependencies below could not be determined. Ensure that they are valid SPDX licenses:'
       )
@@ -271,6 +282,8 @@ function printLicensesBlock(
       )
     }
     printNullLicenses(invalidLicenseChanges.unlicensed)
+
+    return issueFound
   })
 }
 
@@ -373,8 +386,10 @@ function printScannedDependencies(changes: Changes): void {
 function printDeniedDependencies(
   changes: Changes,
   config: ConfigurationOptions
-): void {
-  core.group('Denied', async () => {
+): Promise<boolean> {  
+  return core.group('Denied', async () => {
+    let issueFound = false
+
     for (const denied of config.deny_packages) {
       core.info(`Config: ${denied}`)
     }
@@ -383,6 +398,15 @@ function printDeniedDependencies(
       core.info(`Change: ${change.name}@${change.version} is denied`)
       core.info(`Change: ${change.package_url} is denied`)
     }
+
+    if (changes.length > 0) {
+      issueFound = true
+      core.setFailed('Dependency review detected denied packages.')
+    } else {
+      core.info('Dependency review did not detect any denied packages')
+    }
+
+    return issueFound
   })
 }