Merge pull request #439 from actions/juxtin/snapshot-warnings

Show snapshot warnings in the summary

__tests__/summary.test.ts

@@ -27,6 +27,45 @@ const defaultConfig: ConfigurationOptions = {
   comment_summary_in_pr: true
 }
 
+const changesWithEmptyManifests: Changes = [
+  {
+    change_type: 'added',
+    manifest: '',
+    ecosystem: 'unknown',
+    name: 'castore',
+    version: '0.1.17',
+    package_url: 'pkg:hex/castore@0.1.17',
+    license: null,
+    source_repository_url: null,
+    scope: 'runtime',
+    vulnerabilities: []
+  },
+  {
+    change_type: 'added',
+    manifest: '',
+    ecosystem: 'unknown',
+    name: 'connection',
+    version: '1.1.0',
+    package_url: 'pkg:hex/connection@1.1.0',
+    license: null,
+    source_repository_url: null,
+    scope: 'runtime',
+    vulnerabilities: []
+  },
+  {
+    change_type: 'added',
+    manifest: 'python/dist-info/METADATA',
+    ecosystem: 'pip',
+    name: 'pygments',
+    version: '2.6.1',
+    package_url: 'pkg:pypi/pygments@2.6.1',
+    license: 'BSD-2-Clause',
+    source_repository_url: 'https://github.com/pygments/pygments',
+    scope: 'runtime',
+    vulnerabilities: []
+  }
+]
+
 test('prints headline as h1', () => {
   summary.addSummaryToSummary(
     emptyChanges,
@@ -65,6 +104,22 @@ test('only includes "No license issues found"-message if "vulnerability_check" i
   expect(text).toContain('✅ No license issues found.')
 })
 
+test('groups dependencies with empty manifest paths together', () => {
+  summary.addSummaryToSummary(
+    changesWithEmptyManifests,
+    emptyInvalidLicenseChanges,
+    defaultConfig
+  )
+  summary.addScannedDependencies(changesWithEmptyManifests)
+  const text = core.summary.stringify()
+
+  expect(text).toContain('<summary>Unnamed Manifest</summary>')
+  expect(text).toContain('castore')
+  expect(text).toContain('connection')
+  expect(text).toContain('<summary>python/dist-info/METADATA</summary>')
+  expect(text).toContain('pygments')
+})
+
 test('does not include status section if nothing was found', () => {
   summary.addSummaryToSummary(
     emptyChanges,

src/dependency-graph.ts

@@ -1,9 +1,14 @@
 import * as core from '@actions/core'
 import * as githubUtils from '@actions/github/lib/utils'
 import * as retry from '@octokit/plugin-retry'
-import {Changes, ChangesSchema} from './schemas'
+import {
+  ChangesSchema,
+  ComparisonResponse,
+  ComparisonResponseSchema
+} from './schemas'
 
 const retryingOctokit = githubUtils.GitHub.plugin(retry.retry)
+const SnapshotWarningsHeader = 'x-github-dependency-graph-snapshot-warnings'
 const octo = new retryingOctokit(
   githubUtils.getOctokitOptions(core.getInput('repo-token', {required: true}))
 )
@@ -18,14 +23,31 @@ export async function compare({
   repo: string
   baseRef: string
   headRef: string
-}): Promise<Changes> {
+}): Promise<ComparisonResponse> {
+  let snapshot_warnings = ''
   const changes = await octo.paginate(
-    'GET /repos/{owner}/{repo}/dependency-graph/compare/{basehead}',
     {
+      method: 'GET',
+      url: '/repos/{owner}/{repo}/dependency-graph/compare/{basehead}',
       owner,
       repo,
       basehead: `${baseRef}...${headRef}`
+    },
+    response => {
+      if (
+        response.headers[SnapshotWarningsHeader] &&
+        typeof response.headers[SnapshotWarningsHeader] === 'string'
+      ) {
+        snapshot_warnings = Buffer.from(
+          response.headers[SnapshotWarningsHeader],
+          'base64'
+        ).toString('utf-8')
+      }
+      return ChangesSchema.parse(response.data)
     }
   )
-  return ChangesSchema.parse(changes)
+  return ComparisonResponseSchema.parse({
+    changes,
+    snapshot_warnings
+  })
 }

src/main.ts

@@ -22,12 +22,14 @@ async function run(): Promise<void> {
     const config = await readConfig()
     const refs = getRefs(config, github.context)
 
-    const changes = await dependencyGraph.compare({
+    const comparison = await dependencyGraph.compare({
       owner: github.context.repo.owner,
       repo: github.context.repo.repo,
       baseRef: refs.base,
       headRef: refs.head
     })
+    const changes = comparison.changes
+    const snapshot_warnings = comparison.snapshot_warnings
 
     if (!changes) {
       core.info('No Dependency Changes found. Skipping Dependency Review.')
@@ -65,6 +67,10 @@ async function run(): Promise<void> {
       config
     )
 
+    if (snapshot_warnings) {
+      summary.addSnapshotWarnings(snapshot_warnings)
+    }
+
     if (config.vulnerability_check) {
       summary.addChangeVulnerabilitiesToSummary(vulnerableChanges, minSeverity)
       printVulnerabilitiesBlock(vulnerableChanges, minSeverity)

src/schemas.ts

@@ -73,9 +73,14 @@ export const ConfigurationOptionsSchema = z
   })
 
 export const ChangesSchema = z.array(ChangeSchema)
+export const ComparisonResponseSchema = z.object({
+  changes: z.array(ChangeSchema),
+  snapshot_warnings: z.string()
+})
 
 export type Change = z.infer<typeof ChangeSchema>
 export type Changes = z.infer<typeof ChangesSchema>
+export type ComparisonResponse = z.infer<typeof ComparisonResponseSchema>
 export type ConfigurationOptions = z.infer<typeof ConfigurationOptionsSchema>
 export type Severity = z.infer<typeof SeveritySchema>
 export type Scope = (typeof SCOPES)[number]

src/summary.ts

@@ -215,6 +215,23 @@ export function addScannedDependencies(changes: Changes): void {
   }
 }
 
+export function addSnapshotWarnings(warnings: string): void {
+  // For now, we want to ignore warnings that just complain
+  // about missing snapshots on the head SHA. This is a product
+  // decision to avoid presenting warnings to users who simply
+  // don't use snapshots.
+  const ignore_regex = new RegExp(/No.*snapshot.*found.*head.*/, 'i')
+  if (ignore_regex.test(warnings)) {
+    return
+  }
+
+  core.summary.addHeading('Snapshot Warnings', 2)
+  core.summary.addQuote(`${icons.warning}: ${warnings}`)
+  core.summary.addRaw(
+    'Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.'
+  )
+}
+
 function countLicenseIssues(
   invalidLicenseChanges: InvalidLicenseChanges
 ): number {

src/utils.ts

@@ -8,7 +8,9 @@ export function groupDependenciesByManifest(
 ): Map<string, Changes> {
   const dependencies: Map<string, Changes> = new Map()
   for (const change of changes) {
-    const manifestName = change.manifest
+    // If the manifest is null or empty, give it a name now to avoid
+    // breaking the HTML rendering later
+    const manifestName = change.manifest || 'Unnamed Manifest'
 
     if (dependencies.get(manifestName) === undefined) {
       dependencies.set(manifestName, [])