adding spdx-satisfies

.devcontainer/devcontainer.json

@@ -1,9 +0,0 @@
-{
-  "name": "Dependency Review Action",
-  "image": "mcr.microsoft.com/devcontainers/typescript-node:18",
-  "postCreateCommand": "npm install",
-  "remoteUser": "node",
-  "features": {
-    "ghcr.io/devcontainers/features/ruby:1": {}
-  }
-}

.github/dependency-review-config.yml

@@ -0,0 +1 @@
+fail-on-severity: low

.github/workflows/check-dist.yml

@@ -23,10 +23,10 @@ jobs:
     steps:
       - uses: actions/checkout@v3
 
-      - name: Set Node.js 18.x
+      - name: Set Node.js 16.x
         uses: actions/setup-node@v3
         with:
-          node-version: 18.x
+          node-version: 16.x
 
       - name: Install dependencies
         run: npm ci

.github/workflows/ci.yml

@@ -17,7 +17,7 @@ jobs:
       - uses: actions/checkout@v3
       - uses: actions/setup-node@v3
         with:
-          node-version: 18
+          node-version: 16
           cache: npm
       - name: Install dependencies
         run: npm ci --ignore-scripts
@@ -30,7 +30,7 @@ jobs:
       - uses: actions/checkout@v3
       - uses: actions/setup-node@v3
         with:
-          node-version: 18
+          node-version: 16
           cache: npm
       - name: Install dependencies
         run: npm ci --ignore-scripts

CONTRIBUTING.md

@@ -1,5 +1,4 @@
 # Contributing
-
 [fork]: https://github.com/actions/dependency-review-action/fork
 [pr]: https://github.com/actions/dependency-review-action/compare
 [code-of-conduct]: CODE_OF_CONDUCT.md
@@ -10,6 +9,7 @@ Contributions to this project are
 [released](https://help.github.com/articles/github-terms-of-service/#6-contributions-under-repository-license)
 to the public under the [project's open source license](LICENSE).
 
+
 Please note that this project is released with a [Contributor Code of
 Conduct][code-of-conduct]. By participating in this project you agree
 to abide by its terms.
@@ -20,6 +20,7 @@ This Action makes an authenticated query to the Dependency Graph Diff
 API endpoint (`GET /repos/{owner}/{repo}/dependency-graph/compare/{basehead}`)
 to find out the set of added and removed dependencies for each manifest.
 
+
 ### Bootstrapping the project
 
 ```
@@ -34,11 +35,10 @@ npm install
 npm run test
 ```
 
-_Note_: We don't have any useful tests yet, contributions are welcome!
+*Note*: We don't have any useful tests yet, contributions are welcome!
 
 ## Local Development
 
-It is recommended to have atleast [Node 18](https://nodejs.org/en/) installed.
 We have a script to scan a given PR for vulnerabilities, this will
 help you test your local changes. Make sure to [grab a Personal Access Token (PAT)](https://github.com/settings/tokens) before proceeding (you'll need `repo` permissions for private repos):
 
@@ -56,24 +56,16 @@ Like this:
 $ GITHUB_TOKEN=my-secret-token ./scripts/scan_pr https://github.com/actions/dependency-review-action/pull/3
 ```
 
-[Configuration options](README.md#configuration-options) can be set by
-passing an external YAML [configuration file](README.md#configuration-file) to the
-`scan_pr` script with the `-c`/`--config-file` option:
-
-```sh
-$ GITHUB_TOKEN=<token> ./scripts/scan_pr --config-file my_custom_config.yml <pr_url>
-```
-
 ## Submitting a pull request
 
 0. [Fork][fork] and clone the repository
-1. Configure and install the dependencies: `npm install`
-2. Make sure the tests pass on your machine: `npm run test`
-3. Create a new branch: `git checkout -b my-branch-name`
-4. Make your change, add tests, and make sure the tests still pass
-5. Make sure to build and package before pushing: `npm run build && npm run package`
-6. Push to your fork and [submit a pull request][pr]
-7. Pat your self on the back and wait for your pull request to be reviewed and merged.
+0. Configure and install the dependencies: `npm install`
+0. Make sure the tests pass on your machine: `npm run test`
+0. Create a new branch: `git checkout -b my-branch-name`
+0. Make your change, add tests, and make sure the tests still pass
+0. Make sure to build and package before pushing: `npm run build && npm run package`
+0. Push to your fork and [submit a pull request][pr]
+0. Pat your self on the back and wait for your pull request to be reviewed and merged.
 
 Here are a few things you can do that will increase the likelihood of your pull request being accepted:
 
@@ -85,21 +77,21 @@ Here are a few things you can do that will increase the likelihood of your pull
 
 1. Update the version number in [package.json](https://github.com/actions/dependency-review-action/blob/main/package.json).
 1. Go to [Draft a new
-   release](https://github.com/actions/dependency-review-action/releases/new)
-   in the Releases page.
-1. Make sure that the `Publish this Action to the GitHub Marketplace`
-   checkbox is enabled
+release](https://github.com/actions/dependency-review-action/releases/new)
+in the Releases page.
+2. Make sure that the `Publish this Action to the GitHub Marketplace`
+checkbox is enabled
 
 <img width="481" alt="Screenshot 2022-06-15 at 12 08 19" src="https://user-images.githubusercontent.com/2161/173822484-4b60d8b4-c674-4bff-b5ff-b0c4a3650ab7.png">
 
 3. Click "Choose a tag" and then "Create new tag", where the tag name
-   will be your version prefixed by a `v` (e.g. `v1.2.3`).
+will be your version prefixed by a `v` (e.g. `v1.2.3`).
 4. Use a version number for the release title (e.g. "1.2.3").
 
 <img width="700" alt="Screenshot 2022-06-15 at 12 08 36" src="https://user-images.githubusercontent.com/2161/173822548-33ab3432-d679-4dc1-adf8-b50fdaf47de3.png">
 
 5. Add your release notes. If this is a major version make sure to
-   include a small description of the biggest changes in the new version.
+include a small description of the biggest changes in the new version.
 6. Click "Publish Release".
 
 You now have a tag and release using the semver version you used
@@ -110,7 +102,6 @@ automatically getting all the
 minor/patch updates.
 
 To do this just checkout `main`, force-create a new annotated tag, and push it:
-
 ```
 git tag -fa v2 -m "Updating v2 to 2.3.4"
 git push origin v2 --force

README.md

@@ -71,20 +71,13 @@ or by inlining these options in your workflow file.
 
 ### config-file
 
-A string representing the path to a configuraton file. It can be a
-local file, or a file located in an external repository. You can use
-this syntax for external repositories: `OWNER/REPOSITORY/FILENAME@BRANCH`.
+A string representing the path to an external configuraton file. By
+default external configuration files are not used.
 
-If the configuration file is located in an external private repository,
-use the [external-repo-token](#external-repo-token) parameter of the
-action to specify a token that has read access to the repository.
+**Possible values**: A string representing the absolute path to the
+configuration file.
 
-**Possible values**: A string representing a path to a file located
-in the current repository, or in an external one.
-
-**Example**: `config-file: ./.github/dependency-review-config.yml # local file`.
-
-**Example**: `config-file: github/octorepo/dependency-review-config.yml@main # external repo`
+**Example**: `config-file: ./.github/dependency-review-config.yml`.
 
 ### fail-on-severity
 
@@ -113,20 +106,19 @@ fail-on-scopes:
 
 ### allow-licenses
 
-Only allow the licenses that comply with the expressions in this list. See "[Licenses](https://github.com/actions/dependency-review-action#licenses)".
+Only allow the licenses in this list. See "[Licenses](https://github.com/actions/dependency-review-action#licenses)".
 
-**Possible values**: A list of of [SPDX-compliant license identifiers](https://spdx.org/licenses/).
+**Possible values**: Any `spdx_id` value(s) from
+https://docs.github.com/en/rest/licenses.
 
-**Inline example**: `allow-licenses: BSD-3-Clause, LGPL-2.1 OR MIT OR BSD-3-Clause`
+**Inline example**: `allow-licenses: BSD-3-Clause, MIT`
 
 **YAML example**:
 
 ```yaml
 allow-licenses:
   - BSD-3-Clause
-  - LGPL-2.1
   - MIT
-  - BSD-3-Clause
 ```
 
 ### deny-licenses
@@ -134,23 +126,24 @@ allow-licenses:
 Add a custom list of licenses you want to block. See
 "[Licenses](https://github.com/actions/dependency-review-action#licenses)".
 
-**Possible values**: Any valid set of [SPDX licenses](https://spdx.org/licenses/).
+**Possible values**: Any `spdx_id` value(s) from
+https://docs.github.com/en/rest/licenses.
 
-**Inline example**: `deny-licenses: LGPL-2.0, GPL-2.0+ WITH Bison-exception-2.2`
+**Inline example**: `deny-licenses: LGPL-2.0, BSD-2-Clause`
 
 **YAML example**:
 
 ```yaml
 deny-licenses:
   - LGPL-2.0
-  - GPL-2.0+ WITH Bison-exception-2.2
+  - BSD-2-Clause
 ```
 
 ### allow-ghsas
 
-A list of GitHub Security Advisory IDs that can be skipped during detection.
+Add a custom list of GitHub Advisory IDs that can be skipped during detection. 
 
-**Possible values**: Any valid GHSAs from the [GitHub Advisory Database](https://github.com/advisories).
+**Possible values**: Any valid advisory GHSA ids.
 
 **Inline example**: `allow-ghsas: GHSA-abcd-1234-5679, GHSA-efgh-1234-5679`
 
@@ -162,20 +155,6 @@ allow-ghsas:
   - GHSA-efgh-1234-5679
 ```
 
-### license-check/vulnerability-check
-
-Disable the license checks or vulnerability checks performed by this Action.
-You can't disable both checks.
-
-**Possible values**: `true` or `false`
-
-**Example**:
-
-```yaml
-license-check: true
-vulnerability-check: false
-```
-
 ### base-ref/head-ref
 
 Provide custom git references for the git base/head when performing
@@ -192,19 +171,6 @@ base-ref: 8bb8a58d6a4028b6c2e314d5caaf273f57644896
 head-ref: 69af5638bf660cf218aad5709a4c100e42a2f37b
 ```
 
-### external-repo-token
-
-A token for fetching external configuration files if they live in
-an external private repository.
-
-Visit the [developer settings](https://github.com/settings/tokens) to
-create a new personal access token with `read` permissions for the
-repository that hosts the config file.
-
-**Possible values**: Any GitHub token with read access to the external repository.
-
-**Example**: `external-repo-token: ghp_123456789abcdef...`
-
 ### Configuration File
 
 You can use an external configuration file to specify the settings for
@@ -293,8 +259,8 @@ forbid a subset of licenses. These options are not supported on Enterprise Serve
 
 You can use the [Licenses
 API](https://docs.github.com/en/rest/licenses) to see the full list of
-supported licenses. Use [SPDX licenses](https://spdx.org/licenses/)
-to filter the licenses. A couple of examples:
+supported licenses. Use the `spdx_id` field for every license you want
+to filter. A couple of examples:
 
 ```yaml
 # only allow MIT-licensed dependents
@@ -309,7 +275,7 @@ to filter the licenses. A couple of examples:
 - name: Dependency Review
   uses: actions/dependency-review-action@v2
   with:
-    deny-licenses: Apache-1.1+
+    deny-licenses: Apache-1.1, Apache-2.0
 ```
 
 ### Considerations

__tests__/config.test.ts

@@ -1,7 +1,6 @@
 import {expect, test, beforeEach} from '@jest/globals'
-import {readConfig} from '../src/config'
+import {readConfig, readConfigFile} from '../src/config'
 import {getRefs} from '../src/git-refs'
-import * as Utils from '../src/utils'
 
 // GitHub Action inputs come in the form of environment variables
 // with an INPUT prefix (e.g. INPUT_FAIL-ON-SEVERITY)
@@ -18,8 +17,6 @@ function clearInputs() {
     'ALLOW-LICENSES',
     'DENY-LICENSES',
     'ALLOW-GHSAS',
-    'LICENSE-CHECK',
-    'VULNERABILITY-CHECK',
     'CONFIG-FILE',
     'BASE-REF',
     'HEAD-REF'
@@ -30,62 +27,48 @@ function clearInputs() {
   })
 }
 
-beforeAll(() => {
-  jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(true)
-})
-
 beforeEach(() => {
   clearInputs()
 })
 
 test('it defaults to low severity', async () => {
-  const config = await readConfig()
-  expect(config.fail_on_severity).toEqual('low')
+  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 config = await readConfig()
-  expect(config.fail_on_severity).toEqual('critical')
-  expect(config.allow_licenses).toEqual(['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 config = await readConfig()
+  const options = readConfig()
 
-  expect(config.allow_licenses).toEqual(undefined)
-  expect(config.deny_licenses).toEqual(undefined)
+  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')
 
-  await expect(readConfig()).rejects.toThrow(
-    'You cannot specify both allow-licenses and deny-licenses'
-  )
-})
-test('it raises an error if an empty allow list is specified', async () => {
-  setInput('config-file', './__tests__/fixtures/config-empty-allow-sample.yml')
-
-  await expect(readConfig()).rejects.toThrow(
-    'You should provide at least one license in allow-licenses'
-  )
+  expect(() => readConfig()).toThrow()
 })
 
 test('it raises an error when given an unknown severity', async () => {
   setInput('fail-on-severity', 'zombies')
-
-  await expect(readConfig()).rejects.toThrow(/received 'zombies'/)
+  expect(() => readConfig()).toThrow()
 })
 
 test('it uses the given refs when the event is not a pull request', async () => {
   setInput('base-ref', 'a-custom-base-ref')
   setInput('head-ref', 'a-custom-head-ref')
 
-  const refs = getRefs(await readConfig(), {
+  const refs = getRefs(readConfig(), {
     payload: {},
     eventName: 'workflow_dispatch'
   })
@@ -94,9 +77,9 @@ test('it uses the given refs when the event is not a pull request', async () =>
 })
 
 test('it raises an error when no refs are provided and the event is not a pull request', async () => {
-  const config = await readConfig()
+  const options = readConfig()
   expect(() =>
-    getRefs(config, {
+    getRefs(options, {
       payload: {},
       eventName: 'workflow_dispatch'
     })
@@ -104,150 +87,91 @@ test('it raises an error when no refs are provided and the event is not a pull r
 })
 
 test('it reads an external config file', async () => {
-  setInput('config-file', './__tests__/fixtures/config-allow-sample.yml')
-
-  const config = await readConfig()
-  expect(config.fail_on_severity).toEqual('critical')
-  expect(config.allow_licenses).toEqual(['BSD', 'GPL 2'])
+  let options = readConfigFile('./__tests__/fixtures/config-allow-sample.yml')
+  expect(options.fail_on_severity).toEqual('critical')
+  expect(options.allow_licenses).toEqual(['BSD', 'GPL 2'])
 })
 
 test('raises an error when the the config file was not found', async () => {
-  setInput('config-file', 'fixtures/i-dont-exist')
-  await expect(readConfig()).rejects.toThrow(/Unable to fetch config file/)
+  expect(() => readConfigFile('fixtures/i-dont-exist')).toThrow()
 })
 
 test('it parses options from both sources', async () => {
   setInput('config-file', './__tests__/fixtures/config-allow-sample.yml')
 
-  let config = await readConfig()
-  expect(config.fail_on_severity).toEqual('critical')
+  let options = readConfig()
+  expect(options.fail_on_severity).toEqual('critical')
 
   setInput('base-ref', 'a-custom-base-ref')
-  config = await readConfig()
-  expect(config.base_ref).toEqual('a-custom-base-ref')
+  options = readConfig()
+  expect(options.base_ref).toEqual('a-custom-base-ref')
 })
 
-test('in case of conflicts, the inline config is the source of truth', async () => {
-  setInput('fail-on-severity', 'low')
+test('in case of conflicts, the external config is the source of truth', async () => {
   setInput('config-file', './__tests__/fixtures/config-allow-sample.yml') // this will set fail-on-severity to 'critical'
 
-  const config = await readConfig()
-  expect(config.fail_on_severity).toEqual('low')
+  let options = readConfig()
+  expect(options.fail_on_severity).toEqual('critical')
+
+  // this should not overwite the previous value
+  setInput('fail-on-severity', 'low')
+  options = readConfig()
+  expect(options.fail_on_severity).toEqual('critical')
 })
 
 test('it uses the default values when loading external files', async () => {
   setInput('config-file', './__tests__/fixtures/no-licenses-config.yml')
-  let config = await readConfig()
-  expect(config.allow_licenses).toEqual(undefined)
-  expect(config.deny_licenses).toEqual(undefined)
+  let options = readConfig()
+  expect(options.allow_licenses).toEqual(undefined)
+  expect(options.deny_licenses).toEqual(undefined)
 
   setInput('config-file', './__tests__/fixtures/license-config-sample.yml')
-  config = await readConfig()
-  expect(config.fail_on_severity).toEqual('low')
+  options = readConfig()
+  expect(options.fail_on_severity).toEqual('low')
 })
 
 test('it accepts an external configuration filename', async () => {
   setInput('config-file', './__tests__/fixtures/no-licenses-config.yml')
-  const config = await readConfig()
-  expect(config.fail_on_severity).toEqual('critical')
+  const options = readConfig()
+  expect(options.fail_on_severity).toEqual('critical')
 })
 
 test('it raises an error when given an unknown severity in an external config file', async () => {
   setInput('config-file', './__tests__/fixtures/invalid-severity-config.yml')
-  await expect(readConfig()).rejects.toThrow()
+  expect(() => readConfig()).toThrow()
 })
 
 test('it defaults to runtime scope', async () => {
-  const config = await readConfig()
-  expect(config.fail_on_scopes).toEqual(['runtime'])
+  const options = readConfig()
+  expect(options.fail_on_scopes).toEqual(['runtime'])
 })
 
 test('it parses custom scopes preference', async () => {
   setInput('fail-on-scopes', 'runtime, development')
-  let config = await readConfig()
-  expect(config.fail_on_scopes).toEqual(['runtime', 'development'])
+  let options = readConfig()
+  expect(options.fail_on_scopes).toEqual(['runtime', 'development'])
 
   clearInputs()
   setInput('fail-on-scopes', 'development')
-  config = await readConfig()
-  expect(config.fail_on_scopes).toEqual(['development'])
+  options = readConfig()
+  expect(options.fail_on_scopes).toEqual(['development'])
 })
 
 test('it raises an error when given invalid scope', async () => {
   setInput('fail-on-scopes', 'runtime, zombies')
-  await expect(readConfig()).rejects.toThrow(/received 'zombies'/)
+  expect(() => readConfig()).toThrow()
 })
 
 test('it defaults to an empty GHSA allowlist', async () => {
-  const config = await readConfig()
-  expect(config.allow_ghsas).toEqual([])
+  const options = readConfig()
+  expect(options.allow_ghsas).toEqual(undefined)
 })
 
 test('it successfully parses GHSA allowlist', async () => {
   setInput('allow-ghsas', 'GHSA-abcd-1234-5679, GHSA-efgh-1234-5679')
-  const config = await readConfig()
-  expect(config.allow_ghsas).toEqual([
+  const options = readConfig()
+  expect(options.allow_ghsas).toEqual([
     'GHSA-abcd-1234-5679',
     'GHSA-efgh-1234-5679'
   ])
 })
-
-test('it defaults to checking licenses', async () => {
-  const config = await readConfig()
-  expect(config.license_check).toBe(true)
-})
-
-test('it parses the license-check input', async () => {
-  setInput('license-check', 'false')
-  let config = await readConfig()
-  expect(config.license_check).toEqual(false)
-
-  clearInputs()
-  setInput('license-check', 'true')
-  config = await readConfig()
-  expect(config.license_check).toEqual(true)
-})
-
-test('it defaults to checking vulnerabilities', async () => {
-  const config = await readConfig()
-  expect(config.vulnerability_check).toBe(true)
-})
-
-test('it parses the vulnerability-check input', async () => {
-  setInput('vulnerability-check', 'false')
-  let config = await readConfig()
-  expect(config.vulnerability_check).toEqual(false)
-
-  clearInputs()
-  setInput('vulnerability-check', 'true')
-  config = await readConfig()
-  expect(config.vulnerability_check).toEqual(true)
-})
-
-test('it is not possible to disable both checks', async () => {
-  setInput('license-check', 'false')
-  setInput('vulnerability-check', 'false')
-  await expect(readConfig()).rejects.toThrow(
-    /Can't disable both license-check and vulnerability-check/
-  )
-})
-
-describe('licenses that are not valid SPDX licenses', () => {
-  beforeAll(() => {
-    jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(false)
-  })
-
-  test('it raises an error for invalid licenses in allow-licenses', async () => {
-    setInput('allow-licenses', ' BSD, GPL 2')
-    await expect(readConfig()).rejects.toThrow(
-      'Invalid license(s) in allow-licenses: BSD, GPL 2'
-    )
-  })
-
-  test('it raises an error for invalid licenses in deny-licenses', async () => {
-    setInput('deny-licenses', ' BSD, GPL 2')
-    await expect(readConfig()).rejects.toThrow(
-      'Invalid license(s) in deny-licenses: BSD, GPL 2'
-    )
-  })
-})

__tests__/filter.test.ts

@@ -3,7 +3,7 @@ import {Change, Changes} from '../src/schemas'
 import {
   filterChangesBySeverity,
   filterChangesByScopes,
-  filterAllowedAdvisories
+  filterOutAllowedAdvisories
 } from '../src/filter'
 
 let npmChange: Change = {
@@ -90,34 +90,28 @@ test('it properly filters changes by scope', async () => {
   expect(result).toEqual([npmChange, rubyChange])
 })
 
-test('it properly handles undefined advisory IDs', async () => {
-  const changes = [npmChange, rubyChange, noVulnNpmChange]
-  let result = filterAllowedAdvisories(undefined, changes)
-  expect(result).toEqual([npmChange, rubyChange, noVulnNpmChange])
-})
-
 test('it properly filters changes with allowed vulnerabilities', async () => {
   const changes = [npmChange, rubyChange, noVulnNpmChange]
 
-  let result = filterAllowedAdvisories(['notrealGHSAID'], changes)
+  let result = filterOutAllowedAdvisories(['notrealGHSAID'], changes)
   expect(result).toEqual([npmChange, rubyChange, noVulnNpmChange])
 
-  result = filterAllowedAdvisories(['first-random_string'], changes)
+  result = filterOutAllowedAdvisories(['first-random_string'], changes)
   expect(result).toEqual([rubyChange, noVulnNpmChange])
 
-  result = filterAllowedAdvisories(
+  result = filterOutAllowedAdvisories(
     ['second-random_string', 'third-random_string'],
     changes
   )
   expect(result).toEqual([npmChange, noVulnNpmChange])
 
-  result = filterAllowedAdvisories(
+  result = filterOutAllowedAdvisories(
     ['first-random_string', 'second-random_string', 'third-random_string'],
     changes
   )
   expect(result).toEqual([noVulnNpmChange])
 
   // if we have a change with multiple vulnerabilities but only one is allowed, we still should not filter out that change
-  result = filterAllowedAdvisories(['second-random_string'], changes)
+  result = filterOutAllowedAdvisories(['second-random_string'], changes)
   expect(result).toEqual([npmChange, rubyChange, noVulnNpmChange])
 })

__tests__/fixtures/config-empty-allow-sample.yml

@@ -1,2 +0,0 @@
-fail_on_severity: critical
-allow_licenses: []

__tests__/licenses.test.ts

@@ -1,7 +1,6 @@
-import {expect, jest, test} from '@jest/globals'
+import {expect, test} from '@jest/globals'
 import {Change, Changes} from '../src/schemas'
-
-let getInvalidLicenseChanges: Function
+import {getDeniedLicenseChanges} from '../src/licenses'
 
 let npmChange: Change = {
   manifest: 'package.json',
@@ -49,142 +48,53 @@ let rubyChange: Change = {
   ]
 }
 
-jest.mock('@actions/core')
-
-const mockOctokit = {
-  rest: {
-    licenses: {
-      getForRepo: jest
-        .fn()
-        .mockReturnValue({data: {license: {spdx_id: 'AGPL'}}})
-    }
-  }
-}
-
-jest.mock('octokit', () => {
-  return {
-    Octokit: class {
-      constructor() {
-        return mockOctokit
-      }
-    }
-  }
-})
-
-beforeEach(async () => {
-  jest.resetModules()
-  jest.doMock('spdx-satisfies', () => {
-    // mock spdx-satisfies return value
-    // true for BSD, false for all others
-    return jest.fn((license: string, _: string): boolean => license === 'BSD')
-  })
-  ;({getInvalidLicenseChanges} = require('../src/licenses'))
-})
-
-test('it adds license outside the allow list to forbidden changes', async () => {
+test('it fails if a license outside the allow list is found', async () => {
   const changes: Changes = [npmChange, rubyChange]
-  const {forbidden} = await getInvalidLicenseChanges(changes, {
-    allow: ['BSD']
-  })
-  expect(forbidden[0]).toBe(npmChange)
-  expect(forbidden.length).toEqual(1)
+  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {allow: ['BSD']})
+  expect(invalidChanges[0]).toBe(npmChange)
 })
 
-test('it adds license inside the deny list to forbidden changes', async () => {
+test('it fails if a license inside the deny list is found', async () => {
   const changes: Changes = [npmChange, rubyChange]
-  const {forbidden} = await getInvalidLicenseChanges(changes, {
+  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(forbidden[0]).toBe(rubyChange)
-  expect(forbidden.length).toEqual(1)
+  expect(invalidChanges.length).toBe(2)
 })
 
-test('it does not add license outside the allow list to forbidden changes if it is in removed changes', async () => {
+test('it does not fail if a license outside the allow list is found in removed changes', async () => {
   const changes: Changes = [
     {...npmChange, change_type: 'removed'},
     {...rubyChange, change_type: 'removed'}
   ]
-  const {forbidden} = await getInvalidLicenseChanges(changes, {
-    allow: ['BSD']
-  })
-  expect(forbidden).toStrictEqual([])
+  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {allow: ['BSD']})
+  expect(invalidChanges).toStrictEqual([])
 })
 
-test('it does not add license inside the deny list to forbidden changes if it is in removed changes', async () => {
+test('it does not fail if a license inside the deny list is found in removed changes', async () => {
   const changes: Changes = [
     {...npmChange, change_type: 'removed'},
     {...rubyChange, change_type: 'removed'}
   ]
-  const {forbidden} = await getInvalidLicenseChanges(changes, {
-    deny: ['BSD']
-  })
-  expect(forbidden).toStrictEqual([])
+  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {deny: ['BSD']})
+  expect(invalidChanges).toStrictEqual([])
 })
 
-test('it adds license outside the allow list to forbidden changes if it is in both added and removed changes', async () => {
+test('it fails if a license outside the allow list is found in both of added and removed changes', async () => {
   const changes: Changes = [
     {...npmChange, change_type: 'removed'},
     npmChange,
     {...rubyChange, change_type: 'removed'}
   ]
-  const {forbidden} = await getInvalidLicenseChanges(changes, {
-    allow: ['BSD']
-  })
-  expect(forbidden).toStrictEqual([npmChange])
-})
-
-test('it adds all licenses to unresolved if it is unable to determine the validity', async () => {
-  jest.resetModules() // reset module set in before
-  jest.doMock('spdx-satisfies', () => {
-    return jest.fn((_first: string, _second: string) => {
-      throw new Error('Some Error')
-    })
-  })
-  ;({getInvalidLicenseChanges} = require('../src/licenses'))
-  const changes: Changes = [npmChange, rubyChange]
-  const invalidLicenses = await getInvalidLicenseChanges(changes, {
-    allow: ['BSD']
-  })
-  expect(invalidLicenses.forbidden.length).toEqual(0)
-  expect(invalidLicenses.unlicensed.length).toEqual(0)
-  expect(invalidLicenses.unresolved.length).toEqual(2)
-})
-
-describe('GH License API fallback', () => {
-  test('it calls licenses endpoint if atleast one of the changes has null license and valid source_repository_url', async () => {
-    const nullLicenseChange = {
-      ...npmChange,
-      license: null,
-      source_repository_url: 'http://github.com/some-owner/some-repo'
-    }
-    const {unlicensed} = await getInvalidLicenseChanges(
-      [nullLicenseChange, rubyChange],
-      {}
-    )
-
-    expect(mockOctokit.rest.licenses.getForRepo).toHaveBeenNthCalledWith(1, {
-      owner: 'some-owner',
-      repo: 'some-repo'
-    })
-    expect(unlicensed.length).toEqual(0)
-  })
-
-  test('it does not call licenses API endpoint for change with null license and invalid source_repository_url ', async () => {
-    const {unlicensed} = await getInvalidLicenseChanges(
-      [{...npmChange, license: null}],
-      {}
-    )
-    expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
-    expect(unlicensed.length).toEqual(1)
-  })
-
-  test('it does not call licenses API endpoint if licenses for all changes are present', async () => {
-    const {unlicensed} = await getInvalidLicenseChanges(
-      [npmChange, rubyChange],
-      {}
-    )
-
-    expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
-    expect(unlicensed.length).toEqual(0)
-  })
+  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {allow: ['BSD']})
+  expect(invalidChanges).toStrictEqual([npmChange])
 })

action.yml

@@ -21,7 +21,7 @@ inputs:
     description: The head git ref to be used for this check. Has a default value when the workflow event is `pull_request` or `pull_request_target`. Must be provided otherwise.
     required: false
   config-file:
-    description: A path to the configuration file for the action.
+    description: A filepath to the configuration file for the action.
     required: false
   allow-licenses:
     description: Comma-separated list of allowed licenses (e.g. "MIT, GPL 3.0, BSD 2 Clause")
@@ -32,10 +32,6 @@ inputs:
   allow-ghsas:
     description: Comma-separated list of allowed Github Advisory IDs (e.g. "GHSA-abcd-1234-5679, GHSA-efgh-1234-5679")
     required: false
-  external-repo-token:
-    description: A token for fetching external configuration file if it lives in another repository. It is required if the repository is private
-    required: false
-
 runs:
   using: 'node16'
   main: 'dist/index.js'

jest.config.js

@@ -1,9 +1,9 @@
 module.exports = {
   clearMocks: true,
-  moduleFileExtensions: ['js', 'json', 'ts'],
+  moduleFileExtensions: ['js', 'ts'],
   testMatch: ['**/*.test.ts'],
   transform: {
     '^.+\\.ts$': 'ts-jest'
   },
   verbose: true
-}
+}

package.json

@@ -1,6 +1,6 @@
 {
   "name": "dependency-review-action",
-  "version": "2.5.1",
+  "version": "2.4.0",
   "private": true,
   "description": "A GitHub Action for Dependency Review",
   "main": "lib/main.js",
@@ -27,29 +27,24 @@
   "dependencies": {
     "@actions/core": "^1.10.0",
     "@actions/github": "^5.1.1",
-    "@octokit/plugin-retry": "^4.0.3",
-    "@octokit/request-error": "^3.0.2",
-    "ansi-styles": "^6.2.1",
-    "got": "^12.5.2",
+    "@octokit/plugin-retry": "^3.0.9",
+    "@octokit/request-error": "^3.0.1",
+    "ansi-styles": "^6.1.1",
+    "got": "^12.5.1",
     "nodemon": "^2.0.20",
-    "octokit": "^2.0.10",
-    "spdx-expression-parse": "^3.0.1",
     "spdx-satisfies": "^5.0.1",
-    "yaml": "^2.1.3",
+    "yaml": "^2.1.2",
     "zod": "^3.19.1"
   },
   "devDependencies": {
-    "@types/jest": "^27.5.2",
-    "@types/node": "^16.18.3",
-    "@typescript-eslint/eslint-plugin": "^5.42.0",
-    "@typescript-eslint/parser": "^5.42.0",
-    "@types/spdx-expression-parse": "^3.0.2",
-    "@types/spdx-satisfies": "^0.1.0",
+    "@types/node": "^16.11.63",
+    "@typescript-eslint/eslint-plugin": "^5.38.1",
+    "@typescript-eslint/parser": "^5.38.1",
     "@vercel/ncc": "^0.34.0",
     "esbuild-register": "^3.3.3",
-    "eslint": "^8.26.0",
-    "eslint-plugin-github": "^4.4.0",
-    "eslint-plugin-jest": "^27.1.3",
+    "eslint": "^8.24.0",
+    "eslint-plugin-github": "^4.3.7",
+    "eslint-plugin-jest": "^27.0.4",
     "jest": "^27.5.1",
     "js-yaml": "^4.1.0",
     "nodemon": "^2.0.20",

scripts/scan_pr

@@ -3,52 +3,22 @@ require 'json'
 require 'tempfile'
 require 'open3'
 require 'bundler/inline'
-require 'optparse'
 
 gemfile do
   source 'https://rubygems.org'
   gem 'octokit'
 end
 
-config_file = nil
 github_token = ENV["GITHUB_TOKEN"]
-
 if !github_token || github_token.empty?
   puts "Please set the GITHUB_TOKEN environment variable"
   exit -1
 end
 
-op = OptionParser.new do |opts|
-  usage = <<EOF
-Run Dependency Review on a repository.
-
-\e[1mUsage:\e[22m
-  scripts/scan_pr [options] <pr_url>
-
-\e[1mExample:\e[22m
-  scripts/scan_pr https://github.com/actions/dependency-review-action/pull/294
-
-EOF
-
-  opts.banner = usage
-
-  opts.on('-c', '--config-file <FILE>', 'Use an external configuration file') do |cf|
-    config_file = cf
-  end
-
-  opts.on("-h", "--help", "Prints this help") do
-    puts opts
-    exit
-  end
-end
-
-op.parse!
-
-# make sure we have a NWO somewhere in the parameters
-arg = /(?<repo_nwo>[\w\-]+\/[\w\-]+)\/pull\/(?<pr_number>\d+)/.match(ARGV.join(" "))
+arg = /(?<repo_nwo>[\w\-]+\/[\w\-]+)\/pull\/(?<pr_number>\d+)/.match(ARGV[0])
 
 if arg.nil?
-  puts op
+  puts "Usage: script/scan_pr <pr_url>"
   exit -1
 end
 
@@ -63,8 +33,7 @@ event_file.write("{ \"pull_request\": #{pr.to_h.to_json}}")
 event_file.close
 
 action_inputs = {
-  "repo-token": github_token,
-  "config-file": config_file
+  "repo-token" => github_token
 }
 
 dev_cmd_env = {

src/config.ts

@@ -3,62 +3,12 @@ import path from 'path'
 import YAML from 'yaml'
 import * as core from '@actions/core'
 import * as z from 'zod'
-import {ConfigurationOptions, ConfigurationOptionsSchema} from './schemas'
-import {isSPDXValid, octokitClient} from './utils'
-
-type ConfigurationOptionsPartial = Partial<ConfigurationOptions>
-
-export async function readConfig(): Promise<ConfigurationOptions> {
-  const inlineConfig = readInlineConfig()
-
-  const configFile = getOptionalInput('config-file')
-  if (configFile !== undefined) {
-    const externalConfig = await readConfigFile(configFile)
-
-    return ConfigurationOptionsSchema.parse({
-      ...externalConfig,
-      ...inlineConfig
-    })
-  }
-
-  return ConfigurationOptionsSchema.parse(inlineConfig)
-}
-
-function readInlineConfig(): ConfigurationOptionsPartial {
-  const fail_on_severity = getOptionalInput('fail-on-severity')
-  const fail_on_scopes = parseList(getOptionalInput('fail-on-scopes'))
-  const allow_licenses = parseList(getOptionalInput('allow-licenses'))
-  const deny_licenses = parseList(getOptionalInput('deny-licenses'))
-  const allow_ghsas = parseList(getOptionalInput('allow-ghsas'))
-  const license_check = getOptionalBoolean('license-check')
-  const vulnerability_check = getOptionalBoolean('vulnerability-check')
-  const base_ref = getOptionalInput('base-ref')
-  const head_ref = getOptionalInput('head-ref')
-
-  validateLicenses('allow-licenses', allow_licenses)
-  validateLicenses('deny-licenses', deny_licenses)
-
-  const keys = {
-    fail_on_severity,
-    fail_on_scopes,
-    allow_licenses,
-    deny_licenses,
-    allow_ghsas,
-    license_check,
-    vulnerability_check,
-    base_ref,
-    head_ref
-  }
-
-  return Object.fromEntries(
-    Object.entries(keys).filter(([_, value]) => value !== undefined)
-  )
-}
-
-function getOptionalBoolean(name: string): boolean | undefined {
-  const value = core.getInput(name)
-  return value.length > 0 ? core.getBooleanInput(name) : undefined
-}
+import {
+  ConfigurationOptions,
+  ConfigurationOptionsSchema,
+  SeveritySchema,
+  SCOPES
+} from './schemas'
 
 function getOptionalInput(name: string): string | undefined {
   const value = core.getInput(name)
@@ -73,93 +23,70 @@ function parseList(list: string | undefined): string[] | undefined {
   }
 }
 
-function validateLicenses(
-  key: 'allow-licenses' | 'deny-licenses',
-  licenses: string[] | undefined
-): void {
-  if (licenses === undefined) {
-    return
-  }
-  const invalid_licenses = licenses.filter(license => !isSPDXValid(license))
-
-  if (invalid_licenses.length > 0) {
-    throw new Error(
-      `Invalid license(s) in ${key}: ${invalid_licenses.join(', ')}`
-    )
+export function readConfig(): ConfigurationOptions {
+  const externalConfig = getOptionalInput('config-file')
+  if (externalConfig !== undefined) {
+    const config = readConfigFile(externalConfig)
+    // the reasoning behind reading the inline config when an external
+    // config file is provided is that we still want to allow users to
+    // pass inline options in the presence of an external config file.
+    const inlineConfig = readInlineConfig()
+    // the external config takes precedence
+    return Object.assign({}, inlineConfig, config)
+  } else {
+    return readInlineConfig()
   }
 }
 
-async function readConfigFile(
-  filePath: string
-): Promise<ConfigurationOptionsPartial> {
-  // match a remote config (e.g. 'owner/repo/filepath@someref')
-  const format = new RegExp(
-    '(?<owner>[^/]+)/(?<repo>[^/]+)/(?<path>[^@]+)@(?<ref>.*)'
+export function readInlineConfig(): ConfigurationOptions {
+  const fail_on_severity = SeveritySchema.parse(
+    getOptionalInput('fail-on-severity')
   )
+  const fail_on_scopes = z
+    .array(z.enum(SCOPES))
+    .default(['runtime'])
+    .parse(parseList(getOptionalInput('fail-on-scopes')))
 
-  let data: string
-  const pieces = format.exec(filePath)
+  const allow_licenses = parseList(getOptionalInput('allow-licenses'))
+  const deny_licenses = parseList(getOptionalInput('deny-licenses'))
 
-  try {
-    if (pieces?.groups && pieces.length === 5) {
-      data = await getRemoteConfig({
-        owner: pieces.groups.owner,
-        repo: pieces.groups.repo,
-        path: pieces.groups.path,
-        ref: pieces.groups.ref
-      })
-    } else {
-      data = fs.readFileSync(path.resolve(filePath), 'utf-8')
-    }
-    return parseConfigFile(data)
-  } catch (error) {
-    core.debug(error as string)
-    throw new Error('Unable to fetch config file')
+  if (allow_licenses !== undefined && deny_licenses !== undefined) {
+    throw new Error("Can't specify both allow_licenses and deny_licenses")
+  }
+
+  const allow_ghsas = parseList(getOptionalInput('allow-ghsas'))
+
+  const base_ref = getOptionalInput('base-ref')
+  const head_ref = getOptionalInput('head-ref')
+
+  return {
+    fail_on_severity,
+    fail_on_scopes,
+    allow_licenses,
+    deny_licenses,
+    allow_ghsas,
+    base_ref,
+    head_ref
   }
 }
 
-function parseConfigFile(configData: string): ConfigurationOptionsPartial {
+export function readConfigFile(filePath: string): ConfigurationOptions {
+  let data
+
   try {
-    const data = YAML.parse(configData)
-    for (const key of Object.keys(data)) {
-      if (key === 'allow-licenses' || key === 'deny-licenses') {
-        validateLicenses(key, data[key])
-      }
-      // get rid of the ugly dashes from the actions conventions
-      if (key.includes('-')) {
-        data[key.replace(/-/g, '_')] = data[key]
-        delete data[key]
-      }
-    }
-    return data
-  } catch (error) {
+    data = fs.readFileSync(path.resolve(filePath), 'utf-8')
+  } catch (error: unknown) {
     throw error
   }
-}
+  data = YAML.parse(data)
 
-async function getRemoteConfig(configOpts: {
-  [key: string]: string
-}): Promise<string> {
-  try {
-    const {data} = await octokitClient(
-      'external-repo-token',
-      false
-    ).rest.repos.getContent({
-      mediaType: {
-        format: 'raw'
-      },
-      owner: configOpts.owner,
-      repo: configOpts.repo,
-      path: configOpts.path,
-      ref: configOpts.ref
-    })
-
-    // When using mediaType.format = 'raw', the response.data is a string
-    // but this is not reflected in the return type of getContent, so we're
-    // casting the return value to a string.
-    return z.string().parse(data as unknown)
-  } catch (error) {
-    core.debug(error as string)
-    throw new Error('Error fetching remote config file')
+  // get rid of the ugly dashes from the actions conventions
+  for (const key of Object.keys(data)) {
+    if (key.includes('-')) {
+      data[key.replace(/-/g, '_')] = data[key]
+      delete data[key]
+    }
   }
+  const values = ConfigurationOptionsSchema.parse(data)
+  return values
 }

src/filter.ts

@@ -51,20 +51,12 @@ export function filterChangesByScopes(
   return filteredChanges
 }
 
-/**
- * Filter out changes that are allowed by the allow_ghsas config
- * option. We want to remove these changes before we do any
- * processing.
- * @param ghsas - list of GHSA IDs to allow
- * @param changes - list of changes to filter
- * @returns a list of changes with the allowed GHSAs removed
- */
-export function filterAllowedAdvisories(
+export function filterOutAllowedAdvisories(
   ghsas: string[] | undefined,
   changes: Changes
 ): Changes {
   if (ghsas === undefined) {
-    return changes
+    return []
   }
 
   const filteredChanges = changes.filter(change => {

src/licenses.ts

@@ -1,6 +1,4 @@
-import spdxSatisfies from 'spdx-satisfies'
-import {Change, Changes} from './schemas'
-import {isSPDXValid, octokitClient} from './utils'
+import {Change} from './schemas'
 
 /**
  * Loops through a list of changes, filtering and returning the
@@ -12,163 +10,40 @@ import {isSPDXValid, octokitClient} from './utils'
  * 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 {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
+ * @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 async function getInvalidLicenseChanges(
+export function getDeniedLicenseChanges(
   changes: Change[],
   licenses: {
     allow?: string[]
     deny?: string[]
   }
-): Promise<Record<string, Changes>> {
+): [Change[], Change[]] {
   const {allow, deny} = licenses
 
-  const groupedChanges = await groupChanges(changes)
-  const licensedChanges: Changes = groupedChanges.licensed
-
-  const invalidLicenseChanges: Record<string, Changes> = {
-    unlicensed: groupedChanges.unlicensed,
-    unresolved: [],
-    forbidden: []
-  }
-
-  const validityCache = new Map<string, boolean>()
-
-  for (const change of licensedChanges) {
-    const license = change.license
-
-    // should never happen since licensedChanges always have licenses but license is nullable in changes schema
-    if (license === null) {
-      continue
-    }
-
-    if (license === 'NOASSERTION') {
-      invalidLicenseChanges.unlicensed.push(change)
-    } else if (validityCache.get(license) === undefined) {
-      try {
-        if (allow !== undefined) {
-          const found = allow.find(spdxExpression =>
-            spdxSatisfies(license, spdxExpression)
-          )
-          validityCache.set(license, found !== undefined)
-        } else if (deny !== undefined) {
-          const found = deny.find(spdxExpression =>
-            spdxSatisfies(license, spdxExpression)
-          )
-          validityCache.set(license, found === undefined)
-        }
-      } catch (err) {
-        invalidLicenseChanges.unresolved.push(change)
-      }
-    }
-
-    if (validityCache.get(license) === false) {
-      invalidLicenseChanges.forbidden.push(change)
-    }
-  }
-
-  return invalidLicenseChanges
-}
-
-const fetchGHLicense = async (
-  owner: string,
-  repo: string
-): Promise<string | null> => {
-  try {
-    const response = await octokitClient().rest.licenses.getForRepo({
-      owner,
-      repo
-    })
-    return response.data.license?.spdx_id ?? null
-  } catch (_) {
-    return null
-  }
-}
-
-const parseGitHubURL = (url: string): {owner: string; repo: string} | null => {
-  try {
-    const parsed = new URL(url)
-    if (parsed.host !== 'github.com') {
-      return null
-    }
-    const components = parsed.pathname.split('/')
-    if (components.length < 3) {
-      return null
-    }
-    return {owner: components[1], repo: components[2]}
-  } catch (_) {
-    return null
-  }
-}
-
-const setGHLicenses = async (changes: Change[]): Promise<Change[]> => {
-  const updatedChanges = changes.map(async change => {
-    if (change.license !== null || change.source_repository_url === null) {
-      return change
-    }
-
-    const githubUrl = parseGitHubURL(change.source_repository_url)
-
-    if (githubUrl === null) {
-      return change
-    }
-
-    return {
-      ...change,
-      license: await fetchGHLicense(githubUrl.owner, githubUrl.repo)
-    }
-  })
-
-  return Promise.all(updatedChanges)
-}
-// Currently Dependency Graph licenses are truncated to 255 characters
-// This possibly makes them invalid spdx ids
-const truncatedDGLicense = (license: string): boolean =>
-  license.length === 255 && !isSPDXValid(license)
-
-async function groupChanges(
-  changes: Changes
-): Promise<Record<string, Changes>> {
-  const result: Record<string, Changes> = {
-    licensed: [],
-    unlicensed: []
-  }
-
-  const ghChanges = []
+  const disallowed: Change[] = []
+  const unknown: Change[] = []
 
   for (const change of changes) {
     if (change.change_type === 'removed') {
       continue
     }
 
-    if (change.license === null) {
-      if (change.source_repository_url !== null) {
-        ghChanges.push(change)
-      } else {
-        result.unlicensed.push(change)
+    const license = change.license
+    if (license === null) {
+      unknown.push(change)
+      continue
+    }
+    if (allow !== undefined) {
+      if (!allow.includes(license)) {
+        disallowed.push(change)
       }
-    } else {
-      if (
-        truncatedDGLicense(change.license) &&
-        change.source_repository_url !== null
-      ) {
-        ghChanges.push(change)
-      } else {
-        result.licensed.push(change)
+    } else if (deny !== undefined) {
+      if (deny.includes(license)) {
+        disallowed.push(change)
       }
     }
   }
 
-  if (ghChanges.length > 0) {
-    const ghLicenses = await setGHLicenses(ghChanges)
-    for (const change of ghLicenses) {
-      if (change.license === null) {
-        result.unlicensed.push(change)
-      } else {
-        result.licensed.push(change)
-      }
-    }
-  }
-
-  return result
+  return [disallowed, unknown]
 }

src/main.ts

@@ -8,9 +8,9 @@ import {readConfig} from '../src/config'
 import {
   filterChangesBySeverity,
   filterChangesByScopes,
-  filterAllowedAdvisories
+  filterOutAllowedAdvisories
 } from '../src/filter'
-import {getInvalidLicenseChanges} from './licenses'
+import {getDeniedLicenseChanges} from './licenses'
 import * as summary from './summary'
 import {getRefs} from './git-refs'
 
@@ -18,7 +18,7 @@ import {groupDependenciesByManifest} from './utils'
 
 async function run(): Promise<void> {
   try {
-    const config = await readConfig()
+    const config = readConfig()
     const refs = getRefs(config, github.context)
 
     const changes = await dependencyGraph.compare({
@@ -28,9 +28,9 @@ async function run(): Promise<void> {
       headRef: refs.head
     })
 
-    const minSeverity = config.fail_on_severity
+    const minSeverity = config.fail_on_severity as Severity
     const scopedChanges = filterChangesByScopes(config.fail_on_scopes, changes)
-    const filteredChanges = filterAllowedAdvisories(
+    const filteredChanges = filterOutAllowedAdvisories(
       config.allow_ghsas,
       scopedChanges
     )
@@ -45,7 +45,7 @@ async function run(): Promise<void> {
         change.vulnerabilities.length > 0
     )
 
-    const invalidLicenseChanges = await getInvalidLicenseChanges(
+    const [licenseErrors, unknownLicenses] = getDeniedLicenseChanges(
       filteredChanges,
       {
         allow: config.allow_licenses,
@@ -53,21 +53,13 @@ async function run(): Promise<void> {
       }
     )
 
-    summary.addSummaryToSummary(
-      config.vulnerability_check ? addedChanges : null,
-      config.license_check ? invalidLicenseChanges : null
-    )
-
-    if (config.vulnerability_check) {
-      summary.addChangeVulnerabilitiesToSummary(addedChanges, minSeverity)
-      printVulnerabilitiesBlock(addedChanges, minSeverity)
-    }
-    if (config.license_check) {
-      summary.addLicensesToSummary(invalidLicenseChanges, config)
-      printLicensesBlock(invalidLicenseChanges)
-    }
-
+    summary.addSummaryToSummary(addedChanges, licenseErrors, unknownLicenses)
+    summary.addChangeVulnerabilitiesToSummary(addedChanges, minSeverity)
+    summary.addLicensesToSummary(licenseErrors, unknownLicenses, config)
     summary.addScannedDependencies(changes)
+
+    printVulnerabilitiesBlock(addedChanges, minSeverity)
+    printLicensesBlock(licenseErrors, unknownLicenses)
     printScannedDependencies(changes)
   } catch (error) {
     if (error instanceof RequestError && error.status === 404) {
@@ -91,7 +83,7 @@ async function run(): Promise<void> {
 }
 
 function printVulnerabilitiesBlock(
-  addedChanges: Changes,
+  addedChanges: Change[],
   minSeverity: Severity
 ): void {
   let failed = false
@@ -127,28 +119,24 @@ function printChangeVulnerabilities(change: Change): void {
 }
 
 function printLicensesBlock(
-  invalidLicenseChanges: Record<string, Changes>
+  licenseErrors: Change[],
+  unknownLicenses: Change[]
 ): void {
   core.group('Licenses', async () => {
-    if (invalidLicenseChanges.forbidden.length > 0) {
-      core.info('\nThe following dependencies have incompatible licenses:')
-      printLicensesError(invalidLicenseChanges.forbidden)
+    if (licenseErrors.length > 0) {
+      printLicensesError(licenseErrors)
       core.setFailed('Dependency review detected incompatible licenses.')
     }
-    if (invalidLicenseChanges.unresolved.length > 0) {
-      core.warning(
-        '\nThe validity of the licenses of the dependencies below could not be determined. Ensure that they are valid SPDX licenses:'
-      )
-      printLicensesError(invalidLicenseChanges.unresolved)
-      core.setFailed(
-        'Dependency review could not detect the validity of all licenses.'
-      )
-    }
-    printNullLicenses(invalidLicenseChanges.unlicensed)
+    printNullLicenses(unknownLicenses)
   })
 }
 
-function printLicensesError(changes: Changes): void {
+function printLicensesError(changes: Change[]): void {
+  if (changes.length === 0) {
+    return
+  }
+
+  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}`
@@ -156,12 +144,12 @@ function printLicensesError(changes: Changes): void {
   }
 }
 
-function printNullLicenses(changes: Changes): void {
+function printNullLicenses(changes: Change[]): void {
   if (changes.length === 0) {
     return
   }
 
-  core.info('\nWe could not detect a license for the following dependencies:')
+  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}`
@@ -204,7 +192,7 @@ function renderScannedDependency(change: Change): string {
     } as const
   )[changeType]
 
-  return `${styles.color[color].open}${icon} ${change.name}@${change.version}${styles.color[color].close}`
+  return `${styles.color[color].open}${icon} ${change.manifest}@${change.version}${styles.color[color].close}`
 }
 
 function printScannedDependencies(changes: Changes): void {

src/schemas.ts

@@ -38,38 +38,18 @@ export const ConfigurationOptionsSchema = z
   .object({
     fail_on_severity: SeveritySchema,
     fail_on_scopes: z.array(z.enum(SCOPES)).default(['runtime']),
-    allow_licenses: z.array(z.string()).optional(),
-    deny_licenses: z.array(z.string()).optional(),
+    allow_licenses: z.array(z.string()).default([]),
+    deny_licenses: z.array(z.string()).default([]),
     allow_ghsas: z.array(z.string()).default([]),
-    license_check: z.boolean().default(true),
-    vulnerability_check: z.boolean().default(true),
-    config_file: z.string().optional(),
-    base_ref: z.string().optional(),
-    head_ref: z.string().optional()
-  })
-  .superRefine((config, context) => {
-    if (config.allow_licenses && config.deny_licenses) {
-      context.addIssue({
-        code: z.ZodIssueCode.custom,
-        message: 'You cannot specify both allow-licenses and deny-licenses'
-      })
-    }
-    if (config.allow_licenses && config.allow_licenses.length < 1) {
-      context.addIssue({
-        code: z.ZodIssueCode.custom,
-        message: 'You should provide at least one license in allow-licenses'
-      })
-    }
-    if (
-      config.license_check === false &&
-      config.vulnerability_check === false
-    ) {
-      context.addIssue({
-        code: z.ZodIssueCode.custom,
-        message: "Can't disable both license-check and vulnerability-check"
-      })
-    }
+    config_file: z.string().optional().default('false'),
+    base_ref: z.string(),
+    head_ref: z.string()
   })
+  .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)
 

src/summary.ts

@@ -1,27 +1,18 @@
 import * as core from '@actions/core'
-import {ConfigurationOptions, Changes} from './schemas'
+import {ConfigurationOptions, Change, Changes} from './schemas'
 import {SummaryTableRow} from '@actions/core/lib/summary'
 import {groupDependenciesByManifest, getManifestsSet, renderUrl} from './utils'
 
 export function addSummaryToSummary(
-  addedPackages: Changes | null,
-  invalidLicenseChanges: Record<string, Changes> | null
+  addedPackages: Changes,
+  licenseErrors: Change[],
+  unknownLicenses: Change[]
 ): 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.`
-          ]
-        : [])
-    ])
+    .addRaw(
+      `We found ${addedPackages.length} vulnerable package(s), ${licenseErrors.length} package(s) with incompatible licenses, and ${unknownLicenses.length} package(s) with unknown licenses.`
+    )
 }
 
 export function addChangeVulnerabilitiesToSummary(
@@ -85,7 +76,8 @@ export function addChangeVulnerabilitiesToSummary(
 }
 
 export function addLicensesToSummary(
-  invalidLicenseChanges: Record<string, Changes>,
+  licenseErrors: Change[],
+  unknownLicenses: Change[],
   config: ConfigurationOptions
 ): void {
   core.summary.addHeading('Licenses')
@@ -101,59 +93,62 @@ export function addLicensesToSummary(
     )
   }
 
-  if (Object.values(invalidLicenseChanges).every(item => item.length === 0)) {
+  if (licenseErrors.length === 0 && unknownLicenses.length === 0) {
     core.summary.addQuote('No license violations detected.')
     return
   }
 
-  core.debug(
-    `found ${invalidLicenseChanges.unlicensed.length} unknown licenses`
-  )
-
-  core.debug(
-    `${invalidLicenseChanges.unresolved.length} licenses could not be validated`
-  )
-
-  printLicenseViolation(
-    'Incompatible Licenses',
-    invalidLicenseChanges.forbidden
-  )
-  printLicenseViolation('Unknown Licenses', invalidLicenseChanges.unlicensed)
-  printLicenseViolation(
-    'Invalid SPDX License Definitions',
-    invalidLicenseChanges.unresolved
-  )
-}
-function printLicenseViolation(heading: string, changes: Changes): void {
-  core.summary.addHeading(heading, 5).addSeparator()
-
-  if (changes.length > 0) {
+  if (licenseErrors.length > 0) {
     const rows: SummaryTableRow[] = []
-    const manifests = getManifestsSet(changes)
+    const manifests = getManifestsSet(licenseErrors)
+
+    core.summary.addHeading('Incompatible Licenses', 3).addSeparator()
 
     for (const manifest of manifests) {
       core.summary.addHeading(`<em>${manifest}</em>`, 4)
 
-      for (const change of changes.filter(pkg => pkg.manifest === manifest)) {
+      for (const change of licenseErrors.filter(
+        pkg => pkg.manifest === manifest
+      )) {
         rows.push([
           renderUrl(change.source_repository_url, change.name),
           change.version,
-          formatLicense(change.license)
+          change.license || ''
         ])
       }
-
       core.summary.addTable([['Package', 'Version', 'License'], ...rows])
     }
   } else {
-    core.summary.addQuote(`No ${heading.toLowerCase()} detected.`)
+    core.summary.addQuote('No license violations detected.')
   }
-}
 
-function formatLicense(license: string | null): string {
-  if (license === null || license === 'NOASSERTION') {
-    return 'Null'
+  core.debug(`found ${unknownLicenses.length} unknown licenses`)
+
+  if (unknownLicenses.length > 0) {
+    const rows: SummaryTableRow[] = []
+    const manifests = getManifestsSet(unknownLicenses)
+
+    core.debug(
+      `found ${manifests.entries.length} manifests for unknown licenses`
+    )
+
+    core.summary.addHeading('Unknown Licenses', 3).addSeparator()
+
+    for (const manifest of manifests) {
+      core.summary.addHeading(`<em>${manifest}</em>`, 4)
+
+      for (const change of unknownLicenses.filter(
+        pkg => pkg.manifest === manifest
+      )) {
+        rows.push([
+          renderUrl(change.source_repository_url, change.name),
+          change.version
+        ])
+      }
+
+      core.summary.addTable([['Package', 'Version'], ...rows])
+    }
   }
-  return license
 }
 
 export function addScannedDependencies(changes: Changes): void {
@@ -162,7 +157,7 @@ export function addScannedDependencies(changes: Changes): void {
 
   const summary = core.summary
     .addHeading('Scanned Dependencies')
-    .addHeading(`We scanned ${dependencies.size} manifest files:`, 5)
+    .addRaw(`We scanned ${dependencies.size} manifest files:`)
 
   for (const manifest of manifests) {
     const deps = dependencies.get(manifest)
@@ -170,7 +165,7 @@ export function addScannedDependencies(changes: Changes): void {
       const dependencyNames = deps.map(
         dependency => `<li>${dependency.name}@${dependency.version}</li>`
       )
-      summary.addDetails(manifest, `<ul>${dependencyNames.join('')}</ul>`)
+      summary.addRaw(`<h3>${manifest}</h3><ul>${dependencyNames.join('')}</ul>`)
     }
   }
 }

src/utils.ts

@@ -1,6 +1,3 @@
-import * as core from '@actions/core'
-import {Octokit} from 'octokit'
-import spdxParse from 'spdx-expression-parse'
 import {Changes} from './schemas'
 
 export function groupDependenciesByManifest(
@@ -31,25 +28,3 @@ export function renderUrl(url: string | null, text: string): string {
     return text
   }
 }
-
-export function isSPDXValid(license: string): boolean {
-  try {
-    spdxParse(license)
-    return true
-  } catch (_) {
-    return false
-  }
-}
-
-export function octokitClient(token = 'repo-token', required = true): Octokit {
-  const opts: Record<string, unknown> = {}
-
-  // auth is only added if token is present. For remote config files in public
-  // repos the token is optional, so it could be undefined.
-  const auth = core.getInput(token, {required})
-  if (auth !== undefined) {
-    opts['auth'] = auth
-  }
-
-  return new Octokit(opts)
-}