Merge branch 'main' into move-config-file

Bump prettier from 2.6.2 to 2.7.0

.vscode/launch.json

@@ -0,0 +1,13 @@
+{
+    "version": "0.1.0",
+    "configurations": [
+        {
+            "name": "Debug Jest Tests",
+            "type": "node",
+            "request": "launch",
+            "runtimeArgs": ["--inspect-brk", "${workspaceRoot}/node_modules/.bin/jest", "--runInBand", "--coverage", "false"],
+            "console": "integratedTerminal",
+            "internalConsoleOptions": "neverOpen"
+        }
+    ]
+}

README.md

@@ -25,9 +25,100 @@ jobs:
       - name: 'Checkout Repository'
         uses: actions/checkout@v3
       - name: 'Dependency Review'
-        uses: actions/dependency-review-action@v1
+        uses: actions/dependency-review-action@v2
 ```
 
+Please keep in mind that you need a GitHub Advanced Security license if you're running this action on private repos.
+
+## Configuration
+You can pass additional options to the Dependency Review
+Action using your workflow file. Here's an example workflow with
+all the possible configurations:
+
+```yaml
+name: 'Dependency Review'
+on: [pull_request]
+permissions:
+  contents: read
+jobs:
+  dependency-review:
+    runs-on: ubuntu-latest
+    steps:
+      - name: 'Checkout Repository'
+        uses: actions/checkout@v3
+      - name: Dependency Review
+        uses: actions/dependency-review-action@v2
+        with:
+          # Possible values: "critical", "high", "moderate", "low"
+          # fail-on-severity: critical
+          #
+          # You can only can only include one of these two options: `allow-licenses` and `deny-licences`
+          #
+          # Possible values: Any `spdx_id` value(s) from https://docs.github.com/en/rest/licenses
+          # allow-licenses: GPL-3.0, BSD-3-Clause, MIT
+          #
+          # Possible values: Any `spdx_id` value(s) from https://docs.github.com/en/rest/licenses
+          # deny-licenses: LGPL-2.0, BSD-2-Clause
+```
+
+### Vulnerability Severity
+
+By default the action will fail on any pull request that contains a
+vulnerable dependency, regardless of the severity level. You can override this behavior by
+using the `fail-on-severity` option, which will cause a failure on any pull requests that introduce vulnerabilities of the specified severity level or higher. The possible values are: `critical`, `high`, `moderate`, or `low`. The
+action defaults to `low`.
+
+This example will only fail on pull requests with `critical` and `high` vulnerabilities:
+
+```yaml
+- name: Dependency Review
+  uses: actions/dependency-review-action@v2
+  with:
+    fail-on-severity: high
+```
+
+### Licenses
+
+You can set the action to fail on pull requests based on the licenses of the dependencies
+they introduce. With `allow-licenses` you can define the list of licenses
+your repository will accept. Alternatively, you can use `deny-licenses` to only
+forbid a subset of licenses.
+
+You can use the [Licenses
+API](https://docs.github.com/en/rest/licenses) to see the full list of
+supported licenses. Use the `spdx_id` field for every license you want
+to filter. A couple of examples:
+
+```yaml
+# only allow MIT-licensed dependents
+- name: Dependency Review
+  uses: actions/dependency-review-action@v2
+  with:
+    allow-licenses: MIT
+```
+
+```yaml
+# Block Apache 1.1 and 2.0 licensed dependents
+- name: Dependency Review
+  uses: actions/dependency-review-action@v2
+  with:
+    deny-licenses: Apache-1.1, Apache-2.0
+```
+
+**Important**
+
+* The action will only accept one of the two parameters; an error will
+be raised if you provide both.
+* By default both parameters are empty (no license checking is
+performed).
+* We don't have license information for all of your dependents. If we
+can't detect the license for a dependency **we will inform you, but the
+action won't fail**.
+
+## Blocking pull requests
+
+The Dependency Review GitHub Action check will only block a pull request from being merged if the repository owner has required the check to pass before merging. For more information, see the [documentation on protected branches](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches#require-status-checks-before-merging).
+
 ## Getting help
 
 If you have bug reports, questions or suggestions please [create a new

__tests__/config.test.ts

@@ -0,0 +1,53 @@
+import {expect, test, beforeEach} from '@jest/globals'
+import {readConfig} from '../src/config'
+
+// GitHub Action inputs come in the form of environment variables
+// with an INPUT prefix (e.g. INPUT_FAIL-ON-SEVERITY)
+function setInput(input: string, value: string) {
+  process.env[`INPUT_${input.toUpperCase()}`] = value
+}
+
+// We want a clean ENV before each test. We use `delete`
+// since we want `undefined` values and not empty strings.
+function clearInputs() {
+  delete process.env['INPUT_FAIL-ON-SEVERITY']
+  delete process.env['INPUT_ALLOW-LICENSES']
+  delete process.env['INPUT_DENY-LICENSES']
+}
+
+beforeEach(() => {
+  clearInputs()
+})
+
+test('it defaults to low severity', async () => {
+  const options = readConfig()
+  expect(options.fail_on_severity).toEqual('low')
+})
+
+test('it reads custom configs', async () => {
+  setInput('fail-on-severity', 'critical')
+  setInput('allow-licenses', ' BSD, GPL 2')
+
+  const options = readConfig()
+  expect(options.fail_on_severity).toEqual('critical')
+  expect(options.allow_licenses).toEqual(['BSD', 'GPL 2'])
+})
+
+test('it defaults to empty allow/deny lists ', async () => {
+  const options = readConfig()
+
+  expect(options.allow_licenses).toEqual(undefined)
+  expect(options.deny_licenses).toEqual(undefined)
+})
+
+test('it raises an error if both an allow and denylist are specified', async () => {
+  setInput('allow-licenses', 'MIT')
+  setInput('deny-licenses', 'BSD')
+
+  expect(() => readConfig()).toThrow()
+})
+
+test('it raises an error when given an unknown severity', async () => {
+  setInput('fail-on-severity', 'zombies')
+  expect(() => readConfig()).toThrow()
+})

__tests__/filter.test.ts

@@ -0,0 +1,59 @@
+import {expect, test} from '@jest/globals'
+import {Change, Changes} from '../src/schemas'
+import {filterChangesBySeverity} from '../src/filter'
+
+let npmChange: Change = {
+  manifest: 'package.json',
+  change_type: 'added',
+  ecosystem: 'npm',
+  name: 'Reeuhq',
+  version: '1.0.2',
+  package_url: 'pkg:npm/[email protected]',
+  license: 'MIT',
+  source_repository_url: 'github.com/some-repo',
+  vulnerabilities: [
+    {
+      severity: 'critical',
+      advisory_ghsa_id: 'first-random_string',
+      advisory_summary: 'very dangerouns',
+      advisory_url: 'github.com/future-funk'
+    }
+  ]
+}
+
+let rubyChange: Change = {
+  change_type: 'added',
+  manifest: 'Gemfile.lock',
+  ecosystem: 'rubygems',
+  name: 'actionsomething',
+  version: '3.2.0',
+  package_url: 'pkg:gem/[email protected]',
+  license: 'BSD',
+  source_repository_url: 'github.com/some-repo',
+  vulnerabilities: [
+    {
+      severity: 'moderate',
+      advisory_ghsa_id: 'second-random_string',
+      advisory_summary: 'not so dangerouns',
+      advisory_url: 'github.com/future-funk'
+    },
+    {
+      severity: 'low',
+      advisory_ghsa_id: 'third-random_string',
+      advisory_summary: 'dont page me',
+      advisory_url: 'github.com/future-funk'
+    }
+  ]
+}
+
+test('it properly filters changes by severity', async () => {
+  const changes = [npmChange, rubyChange]
+  let result = filterChangesBySeverity('high', changes)
+  expect(result).toEqual([npmChange])
+
+  result = filterChangesBySeverity('low', changes)
+  expect(changes).toEqual([npmChange, rubyChange])
+
+  result = filterChangesBySeverity('critical', changes)
+  expect(changes).toEqual([npmChange, rubyChange])
+})

__tests__/fixtures/config-allow-sample.yml

@@ -0,0 +1,4 @@
+fail_on_severity: critical
+allow_licenses:
+  - "BSD"
+  - "GPL 2"

__tests__/fixtures/conflictive-config.yml

@@ -0,0 +1,2 @@
+allow_licenses: []
+deny_licenses: []

__tests__/fixtures/no-licenses-config.yml

@@ -0,0 +1 @@
+fail_on_severity: critical

__tests__/licenses.test.ts

@@ -0,0 +1,70 @@
+import {expect, test} from '@jest/globals'
+import {Change, Changes} from '../src/schemas'
+import {getDeniedLicenseChanges} from '../src/licenses'
+
+let npmChange: Change = {
+  manifest: 'package.json',
+  change_type: 'added',
+  ecosystem: 'npm',
+  name: 'Reeuhq',
+  version: '1.0.2',
+  package_url: 'pkg:npm/[email protected]',
+  license: 'MIT',
+  source_repository_url: 'github.com/some-repo',
+  vulnerabilities: [
+    {
+      severity: 'critical',
+      advisory_ghsa_id: 'first-random_string',
+      advisory_summary: 'very dangerouns',
+      advisory_url: 'github.com/future-funk'
+    }
+  ]
+}
+
+let rubyChange: Change = {
+  change_type: 'added',
+  manifest: 'Gemfile.lock',
+  ecosystem: 'rubygems',
+  name: 'actionsomething',
+  version: '3.2.0',
+  package_url: 'pkg:gem/[email protected]',
+  license: 'BSD',
+  source_repository_url: 'github.com/some-repo',
+  vulnerabilities: [
+    {
+      severity: 'moderate',
+      advisory_ghsa_id: 'second-random_string',
+      advisory_summary: 'not so dangerouns',
+      advisory_url: 'github.com/future-funk'
+    },
+    {
+      severity: 'low',
+      advisory_ghsa_id: 'third-random_string',
+      advisory_summary: 'dont page me',
+      advisory_url: 'github.com/future-funk'
+    }
+  ]
+}
+
+test('it fails if a license outside the allow list is found', async () => {
+  const changes: Changes = [npmChange, rubyChange]
+  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {allow: ['BSD']})
+  expect(invalidChanges[0]).toBe(npmChange)
+})
+
+test('it fails if a license inside the deny list is found', async () => {
+  const changes: Changes = [npmChange, rubyChange]
+  const [invalidChanges] = getDeniedLicenseChanges(changes, {deny: ['BSD']})
+  expect(invalidChanges[0]).toBe(rubyChange)
+})
+
+// This is more of a "here's a behavior that might be surprising" than an actual
+// thing we want in the system. Please remove this test after refactoring.
+test('it fails all license checks when allow is provided an empty array', async () => {
+  const changes: Changes = [npmChange, rubyChange]
+  let [invalidChanges, _] = getDeniedLicenseChanges(changes, {
+    allow: [],
+    deny: ['BSD']
+  })
+  expect(invalidChanges.length).toBe(2)
+})

__tests__/main.test.ts

@@ -1,5 +0,0 @@
-import {expect, test} from '@jest/globals'
-
-test('tests things', async () => {
-  expect(true).toEqual(true)
-})

action.yml

@@ -3,9 +3,19 @@ description: 'Prevent the introduction of dependencies with known vulnerabilitie
 author: 'GitHub'
 inputs:
   repo-token:
-    description: 'Token for the repository. Can be passed in using `{{ secrets.GITHUB_TOKEN }}`.'
+    description: Token for the repository. Can be passed in using `{{ secrets.GITHUB_TOKEN }}`.
     required: false
     default: ${{ github.token }}
+  fail-on-severity:
+    description: Don't block PRs below this severity. Possible values are `low`, `moderate`, `high`, `critical`.
+    required: false
+    default: 'low'
+  allow-licenses:
+    description: Comma-separated list of allowed licenses (e.g. "MIT, GPL 3.0, BSD 2 Clause")
+    required: false
+  deny-licenses:
+    description: Comma-separated list of forbidden licenses (e.g. "MIT, GPL 3.0, BSD 2 Clause")
+    required: false
 runs:
   using: 'node16'
   main: 'dist/index.js'

package.json

@@ -25,29 +25,30 @@
   "author": "GitHub",
   "license": "MIT",
   "dependencies": {
-    "@actions/core": "^1.8.0",
+    "@actions/core": "^1.8.2",
-    "@actions/github": "^5.0.1",
+    "@actions/github": "^5.0.3",
     "@octokit/plugin-retry": "^3.0.9",
     "@octokit/request-error": "^2.1.0",
     "ansi-styles": "^6.1.0",
-    "got": "^12.0.4",
+    "got": "^12.1.0",
     "nodemon": "^2.0.16",
-    "zod": "^3.15.1"
+    "yaml": "^2.1.1",
+    "zod": "^3.17.3"
   },
   "devDependencies": {
-    "@types/node": "^17.0.31",
+    "@types/node": "^17.0.43",
-    "@typescript-eslint/eslint-plugin": "^5.22.0",
+    "@typescript-eslint/eslint-plugin": "^5.28.0",
-    "@typescript-eslint/parser": "^5.22.0",
+    "@typescript-eslint/parser": "^5.28.0",
-    "@vercel/ncc": "^0.33.4",
+    "@vercel/ncc": "^0.34.0",
-    "esbuild-register": "^3.3.2",
+    "esbuild-register": "^3.3.3",
-    "eslint": "^8.15.0",
+    "eslint": "^8.17.0",
     "eslint-plugin-github": "^4.3.6",
-    "eslint-plugin-jest": "^26.1.5",
+    "eslint-plugin-jest": "^26.5.3",
     "jest": "^27.5.1",
     "js-yaml": "^4.1.0",
     "nodemon": "^2.0.16",
-    "prettier": "2.6.2",
+    "prettier": "2.7.0",
     "ts-jest": "^27.1.4",
-    "typescript": "^4.6.4"
+    "typescript": "^4.7.3"
   }
 }

src/config.ts

@@ -0,0 +1,27 @@
+import * as core from '@actions/core'
+import * as z from 'zod'
+import {ConfigurationOptions, SEVERITIES} from './schemas'
+
+function getOptionalInput(name: string): string | undefined {
+  const value = core.getInput(name)
+  return value.length > 0 ? value : undefined
+}
+
+export function readConfig(): ConfigurationOptions {
+  const fail_on_severity = z
+    .enum(SEVERITIES)
+    .default('low')
+    .parse(getOptionalInput('fail-on-severity'))
+  const allow_licenses = getOptionalInput('allow-licenses')
+  const deny_licenses = getOptionalInput('deny-licenses')
+
+  if (allow_licenses !== undefined && deny_licenses !== undefined) {
+    throw new Error("Can't specify both allow_licenses and deny_licenses")
+  }
+
+  return {
+    fail_on_severity,
+    allow_licenses: allow_licenses?.split(',').map(x => x.trim()),
+    deny_licenses: deny_licenses?.split(',').map(x => x.trim())
+  }
+}

src/filter.ts

@@ -0,0 +1,36 @@
+import {Changes} from './schemas'
+import {Severity, SEVERITIES} from './schemas'
+
+export function filterChangesBySeverity(
+  severity: Severity,
+  changes: Changes
+): Changes {
+  const severityIdx = SEVERITIES.indexOf(severity)
+  let filteredChanges = []
+  for (let change of changes) {
+    if (
+      change === undefined ||
+      change.vulnerabilities === undefined ||
+      change.vulnerabilities.length === 0
+    ) {
+      continue
+    }
+
+    let fChange = {
+      ...change,
+      vulnerabilities: change.vulnerabilities.filter(vuln => {
+        const vulnIdx = SEVERITIES.indexOf(vuln.severity)
+        if (vulnIdx <= severityIdx) {
+          return true
+        }
+      })
+    }
+    filteredChanges.push(fChange)
+  }
+
+  // don't want to deal with changes with no vulnerabilities
+  filteredChanges = filteredChanges.filter(
+    change => change.vulnerabilities.length > 0
+  )
+  return filteredChanges
+}

src/licenses.ts

@@ -0,0 +1,46 @@
+import {Change, ChangeSchema} from './schemas'
+
+/**
+ * Loops through a list of changes, filtering and returning the
+ * ones that don't conform to the licenses allow/deny lists.
+ *
+ * Keep in mind that we don't let users specify both an allow and a deny
+ * list in their config files, so this code works under the assumption that
+ * one of the two list parameters will be empty. If both lists are provided,
+ * we will ignore the deny list.
+ * @param {Change[]} changes The list of changes to filter.
+ * @param { { allow?: string[], deny?: string[]}} licenses An object with `allow`/`deny` keys, each containing a list of licenses.
+ * @returns {[Array<Change>, Array<Change]} A tuple where the first element is the list of denied changes and the second one is the list of changes with unknown licenses
+ */
+export function getDeniedLicenseChanges(
+  changes: Array<Change>,
+  licenses: {
+    allow?: Array<string>
+    deny?: Array<string>
+  }
+): [Array<Change>, Array<Change>] {
+  let {allow, deny} = licenses
+
+  let disallowed: Change[] = []
+  let unknown: Change[] = []
+
+  for (const change of changes) {
+    let license = change.license
+    // TODO: be loud about unknown licenses
+    if (license === null) {
+      unknown.push(change)
+      continue
+    }
+    if (allow !== undefined) {
+      if (!allow.includes(license)) {
+        disallowed.push(change)
+      }
+    } else if (deny !== undefined) {
+      if (deny.includes(license)) {
+        disallowed.push(change)
+      }
+    }
+  }
+
+  return [disallowed, unknown]
+}

src/main.ts

@@ -3,7 +3,10 @@ import * as dependencyGraph from './dependency-graph'
 import * as github from '@actions/github'
 import styles from 'ansi-styles'
 import {RequestError} from '@octokit/request-error'
-import {PullRequestSchema} from './schemas'
+import {Change, PullRequestSchema, Severity} from './schemas'
+import {readConfig} from '../src/config'
+import {filterChangesBySeverity} from '../src/filter'
+import {getDeniedLicenseChanges} from './licenses'
 
 async function run(): Promise<void> {
   try {
@@ -24,32 +27,48 @@ async function run(): Promise<void> {
       headRef: pull_request.head.sha
     })
 
+    let config = readConfig()
+    let minSeverity = config.fail_on_severity
     let failed = false
 
-    for (const change of changes) {
+    let licenses = {
+      allow: config.allow_licenses,
+      deny: config.deny_licenses
+    }
+
+    let filteredChanges = filterChangesBySeverity(
+      minSeverity as Severity,
+      changes
+    )
+
+    for (const change of filteredChanges) {
       if (
         change.change_type === 'added' &&
         change.vulnerabilities !== undefined &&
         change.vulnerabilities.length > 0
       ) {
-        for (const vuln of change.vulnerabilities) {
+        printChangeVulnerabilities(change)
-          core.info(
-            `${styles.bold.open}${change.manifest} » ${change.name}@${
-              change.version
-            }${styles.bold.close} – ${vuln.advisory_summary} ${renderSeverity(
-              vuln.severity
-            )}`
-          )
-          core.info(`  ↪ ${vuln.advisory_url}`)
-        }
         failed = true
       }
     }
 
+    let [licenseErrors, unknownLicenses] = getDeniedLicenseChanges(
+      changes,
+      licenses
+    )
+
+    if (licenseErrors.length > 0) {
+      printLicensesError(licenseErrors, licenses)
+      printNullLicenses(unknownLicenses)
+      core.setFailed('Dependency review detected incompatible licenses.')
+    }
+
     if (failed) {
-      throw new Error('Dependency review detected vulnerable packages.')
+      core.setFailed('Dependency review detected vulnerable packages.')
     } else {
-      core.info('Dependency review did not detect any vulnerable packages.')
+      core.info(
+        `Dependency review did not detect any vulnerable packages with severity level "${minSeverity}" or higher.`
+      )
     }
   } catch (error) {
     if (error instanceof RequestError && error.status === 404) {
@@ -60,12 +79,29 @@ async function run(): Promise<void> {
       core.setFailed(
         `Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled, see https://github.com/${github.context.repo.owner}/${github.context.repo.repo}/settings/security_analysis`
       )
-    } else if (error instanceof Error) {
+    } else {
-      core.setFailed(error.message)
+      if (error instanceof Error) {
+        core.setFailed(error.message)
+      } else {
+        core.setFailed('Unexpected fatal error')
+      }
     }
   }
 }
 
+function printChangeVulnerabilities(change: Change) {
+  for (const vuln of change.vulnerabilities) {
+    core.info(
+      `${styles.bold.open}${change.manifest} » ${change.name}@${
+        change.version
+      }${styles.bold.close} – ${vuln.advisory_summary} ${renderSeverity(
+        vuln.severity
+      )}`
+    )
+    core.info(`  ↪ ${vuln.advisory_url}`)
+  }
+}
+
 function renderSeverity(
   severity: 'critical' | 'high' | 'moderate' | 'low'
 ): string {
@@ -80,4 +116,34 @@ function renderSeverity(
   return `${styles.color[color].open}(${severity} severity)${styles.color[color].close}`
 }
 
+function printLicensesError(
+  changes: Array<Change>,
+  licenses: {
+    allow?: Array<string>
+    deny?: Array<string>
+  }
+): void {
+  if (changes.length == 0) {
+    return
+  }
+
+  let {allow = [], deny = []} = licenses
+
+  core.info('\nThe following dependencies have incompatible licenses:\n')
+  for (const change of changes) {
+    core.info(
+      `${styles.bold.open}${change.manifest} » ${change.name}@${change.version}${styles.bold.close} – License: ${styles.color.red.open}${change.license}${styles.color.red.close}`
+    )
+  }
+}
+
+function printNullLicenses(changes: Array<Change>): void {
+  core.info('\nWe could not detect a license for the following dependencies:\n')
+  for (const change of changes) {
+    core.info(
+      `${styles.bold.open}${change.manifest} » ${change.name}@${change.version}${styles.bold.close}`
+    )
+  }
+}
+
 run()

src/schemas.ts

@@ -1,6 +1,8 @@
 import * as z from 'zod'
 
-const ChangeSchema = z.object({
+export const SEVERITIES = ['critical', 'high', 'moderate', 'low'] as const
+
+export const ChangeSchema = z.object({
   change_type: z.enum(['added', 'removed']),
   manifest: z.string(),
   ecosystem: z.string(),
@@ -19,6 +21,7 @@ const ChangeSchema = z.object({
       })
     )
     .optional()
+    .default([])
 })
 
 export const PullRequestSchema = z.object({
@@ -27,6 +30,21 @@ export const PullRequestSchema = z.object({
   head: z.object({sha: z.string()})
 })
 
+export const ConfigurationOptionsSchema = z
+  .object({
+    fail_on_severity: z.enum(SEVERITIES).default('low'),
+    allow_licenses: z.array(z.string()).default([]),
+    deny_licenses: z.array(z.string()).default([])
+  })
+  .partial()
+  .refine(
+    obj => !(obj.allow_licenses && obj.deny_licenses),
+    "Your workflow file has both an allow_licenses list and deny_licenses list, but you can only set one or the other."
+  )
+
 export const ChangesSchema = z.array(ChangeSchema)
 
+export type Change = z.infer<typeof ChangeSchema>
 export type Changes = z.infer<typeof ChangesSchema>
+export type ConfigurationOptions = z.infer<typeof ConfigurationOptionsSchema>
+export type Severity = typeof SEVERITIES[number]