From 97c946575176eac95282bf84a0e0de2a9ebb223a Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Wed, 5 Apr 2023 15:14:57 +0200 Subject: [PATCH 01/10] separate tests for external configs --- __tests__/config.test.ts | 70 +------------------- __tests__/external-config.test.ts | 104 ++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 67 deletions(-) create mode 100644 __tests__/external-config.test.ts diff --git a/__tests__/config.test.ts b/__tests__/config.test.ts index f41165d..6c892a8 100644 --- a/__tests__/config.test.ts +++ b/__tests__/config.test.ts @@ -1,6 +1,6 @@ -import {expect, test, beforeEach} from '@jest/globals' -import {readConfig} from '../src/config' -import {getRefs} from '../src/git-refs' +import { expect, test, beforeEach } from '@jest/globals' +import { readConfig } 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 @@ -105,60 +105,6 @@ test('it raises an error when no refs are provided and the event is not a pull r ).toThrow() }) -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']) -}) - -test('raises an error when the config file was not found', async () => { - setInput('config-file', 'fixtures/i-dont-exist') - await expect(readConfig()).rejects.toThrow(/Unable to fetch/) -}) - -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') - - setInput('base-ref', 'a-custom-base-ref') - config = await readConfig() - expect(config.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') - 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') -}) - -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) - - setInput('config-file', './__tests__/fixtures/license-config-sample.yml') - config = await readConfig() - expect(config.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') -}) - -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() -}) - test('it defaults to runtime scope', async () => { const config = await readConfig() expect(config.fail_on_scopes).toEqual(['runtime']) @@ -234,16 +180,6 @@ test('it is not possible to disable both checks', async () => { ) }) -test('it supports comma-separated lists', async () => { - setInput( - 'config-file', - './__tests__/fixtures/inline-license-config-sample.yml' - ) - const config = await readConfig() - - expect(config.allow_licenses).toEqual(['MIT', 'GPL-2.0-only']) -}) - describe('licenses that are not valid SPDX licenses', () => { beforeAll(() => { jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(false) diff --git a/__tests__/external-config.test.ts b/__tests__/external-config.test.ts new file mode 100644 index 0000000..02e7f47 --- /dev/null +++ b/__tests__/external-config.test.ts @@ -0,0 +1,104 @@ +import { expect, test, beforeEach } from '@jest/globals' +import { readConfig } from '../src/config' +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) +function setInput(input: string, value: string): void { + 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(): void { + const allowedOptions = [ + 'FAIL-ON-SEVERITY', + 'FAIL-ON-SCOPES', + 'ALLOW-LICENSES', + 'DENY-LICENSES', + 'ALLOW-GHSAS', + 'LICENSE-CHECK', + 'VULNERABILITY-CHECK', + 'CONFIG-FILE', + 'BASE-REF', + 'HEAD-REF', + 'COMMENT-SUMMARY-IN-PR' + ] + + // eslint-disable-next-line github/array-foreach + allowedOptions.forEach(option => { + delete process.env[`INPUT_${option.toUpperCase()}`] + }) +} + +beforeAll(() => { + jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(true) +}) + +beforeEach(() => { + clearInputs() +}) + +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']) +}) + +test('raises an error when the config file was not found', async () => { + setInput('config-file', 'fixtures/i-dont-exist') + await expect(readConfig()).rejects.toThrow(/Unable to fetch/) +}) + +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') + + setInput('base-ref', 'a-custom-base-ref') + config = await readConfig() + expect(config.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') + 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') +}) + +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) + + setInput('config-file', './__tests__/fixtures/license-config-sample.yml') + config = await readConfig() + expect(config.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') +}) + +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() +}) + +test('it supports comma-separated lists', async () => { + setInput( + 'config-file', + './__tests__/fixtures/inline-license-config-sample.yml' + ) + const config = await readConfig() + + expect(config.allow_licenses).toEqual(['MIT', 'GPL-2.0-only']) +}) From 0041d7fa413e237c664c76f578de797f567f2de7 Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 16:21:52 +0200 Subject: [PATCH 02/10] Add a failing test. --- __tests__/config.test.ts | 6 +++--- __tests__/external-config.test.ts | 24 ++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/__tests__/config.test.ts b/__tests__/config.test.ts index 6c892a8..0d2ed14 100644 --- a/__tests__/config.test.ts +++ b/__tests__/config.test.ts @@ -1,6 +1,6 @@ -import { expect, test, beforeEach } from '@jest/globals' -import { readConfig } from '../src/config' -import { getRefs } from '../src/git-refs' +import {expect, test, beforeEach} from '@jest/globals' +import {readConfig} 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 diff --git a/__tests__/external-config.test.ts b/__tests__/external-config.test.ts index 02e7f47..c4f3029 100644 --- a/__tests__/external-config.test.ts +++ b/__tests__/external-config.test.ts @@ -1,5 +1,5 @@ -import { expect, test, beforeEach } from '@jest/globals' -import { readConfig } from '../src/config' +import {expect, test, beforeEach} from '@jest/globals' +import {readConfig} from '../src/config' import * as Utils from '../src/utils' // GitHub Action inputs come in the form of environment variables @@ -102,3 +102,23 @@ test('it supports comma-separated lists', async () => { expect(config.allow_licenses).toEqual(['MIT', 'GPL-2.0-only']) }) + +test('it reads a config file hosted in another repo', async () => { + setInput( + 'config-file', + 'future-funk/anyone-cualkiera/external-config.yml@main' + ) + setInput('external-repo-token', 'gh_viptoken') + + setInput( + 'config-file', + 'future-funk/anyone-cualkiera/external-config.yml@main' + ) + + setInput('external-repo-token', 'gh_viptoken') + + const config = await Config.readConfig() + + expect(config.fail_on_severity).toEqual('critical') + expect(config.allow_licenses).toEqual(['BSD', 'GPL 2']) +}) From 153f274eb48355be8dae98f3f54c02bcd2dee498 Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 17:11:16 +0200 Subject: [PATCH 03/10] Mock octokit. --- __tests__/external-config.test.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/__tests__/external-config.test.ts b/__tests__/external-config.test.ts index c4f3029..a8b3839 100644 --- a/__tests__/external-config.test.ts +++ b/__tests__/external-config.test.ts @@ -31,6 +31,28 @@ function clearInputs(): void { }) } +const externalConfig = `fail_on_severity: 'high' +allow_licenses: ['GPL-2.0-only'] +` +const mockOctokit = { + rest: { + repos: { + getContent: jest.fn().mockReturnValue({data: externalConfig}) + } + } +} + +jest.mock('octokit', () => { + return { + // eslint-disable-next-line @typescript-eslint/no-extraneous-class + Octokit: class { + constructor() { + return mockOctokit + } + } + } +}) + beforeAll(() => { jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(true) }) From ff46a4b16e6aebf1291de032051ca65b69a92830 Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 17:11:29 +0200 Subject: [PATCH 04/10] Fixing failing test. --- __tests__/external-config.test.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/__tests__/external-config.test.ts b/__tests__/external-config.test.ts index a8b3839..9da90b3 100644 --- a/__tests__/external-config.test.ts +++ b/__tests__/external-config.test.ts @@ -132,15 +132,8 @@ test('it reads a config file hosted in another repo', async () => { ) setInput('external-repo-token', 'gh_viptoken') - setInput( - 'config-file', - 'future-funk/anyone-cualkiera/external-config.yml@main' - ) + const config = await readConfig() - setInput('external-repo-token', 'gh_viptoken') - - const config = await Config.readConfig() - - expect(config.fail_on_severity).toEqual('critical') - expect(config.allow_licenses).toEqual(['BSD', 'GPL 2']) + expect(config.fail_on_severity).toEqual('high') + expect(config.allow_licenses).toEqual(['GPL-2.0-only']) }) From 2c065db2961a5d0870cae42b7865f83f795f149c Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 17:32:42 +0200 Subject: [PATCH 05/10] Add a test-helpers file. --- __tests__/config.test.ts | 30 +----------------------------- __tests__/external-config.test.ts | 30 +----------------------------- __tests__/test-helpers.ts | 28 ++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 58 deletions(-) create mode 100644 __tests__/test-helpers.ts diff --git a/__tests__/config.test.ts b/__tests__/config.test.ts index 0d2ed14..47e4ef9 100644 --- a/__tests__/config.test.ts +++ b/__tests__/config.test.ts @@ -2,35 +2,7 @@ import {expect, test, beforeEach} from '@jest/globals' import {readConfig} 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) -function setInput(input: string, value: string): void { - 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(): void { - const allowedOptions = [ - 'FAIL-ON-SEVERITY', - 'FAIL-ON-SCOPES', - 'ALLOW-LICENSES', - 'DENY-LICENSES', - 'ALLOW-GHSAS', - 'LICENSE-CHECK', - 'VULNERABILITY-CHECK', - 'CONFIG-FILE', - 'BASE-REF', - 'HEAD-REF', - 'COMMENT-SUMMARY-IN-PR' - ] - - // eslint-disable-next-line github/array-foreach - allowedOptions.forEach(option => { - delete process.env[`INPUT_${option.toUpperCase()}`] - }) -} +import {setInput, clearInputs} from './test-helpers' beforeAll(() => { jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(true) diff --git a/__tests__/external-config.test.ts b/__tests__/external-config.test.ts index 9da90b3..a1a0e7c 100644 --- a/__tests__/external-config.test.ts +++ b/__tests__/external-config.test.ts @@ -1,35 +1,7 @@ import {expect, test, beforeEach} from '@jest/globals' import {readConfig} from '../src/config' 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) -function setInput(input: string, value: string): void { - 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(): void { - const allowedOptions = [ - 'FAIL-ON-SEVERITY', - 'FAIL-ON-SCOPES', - 'ALLOW-LICENSES', - 'DENY-LICENSES', - 'ALLOW-GHSAS', - 'LICENSE-CHECK', - 'VULNERABILITY-CHECK', - 'CONFIG-FILE', - 'BASE-REF', - 'HEAD-REF', - 'COMMENT-SUMMARY-IN-PR' - ] - - // eslint-disable-next-line github/array-foreach - allowedOptions.forEach(option => { - delete process.env[`INPUT_${option.toUpperCase()}`] - }) -} +import {setInput, clearInputs} from './test-helpers' const externalConfig = `fail_on_severity: 'high' allow_licenses: ['GPL-2.0-only'] diff --git a/__tests__/test-helpers.ts b/__tests__/test-helpers.ts new file mode 100644 index 0000000..e5b18e7 --- /dev/null +++ b/__tests__/test-helpers.ts @@ -0,0 +1,28 @@ +// GitHub Action inputs come in the form of environment variables +// with an INPUT prefix (e.g. INPUT_FAIL-ON-SEVERITY) +export function setInput(input: string, value: string): void { + 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. +export function clearInputs(): void { + const allowedOptions = [ + 'FAIL-ON-SEVERITY', + 'FAIL-ON-SCOPES', + 'ALLOW-LICENSES', + 'DENY-LICENSES', + 'ALLOW-GHSAS', + 'LICENSE-CHECK', + 'VULNERABILITY-CHECK', + 'CONFIG-FILE', + 'BASE-REF', + 'HEAD-REF', + 'COMMENT-SUMMARY-IN-PR' + ] + + // eslint-disable-next-line github/array-foreach + allowedOptions.forEach(option => { + delete process.env[`INPUT_${option.toUpperCase()}`] + }) +} From 3f6a17c81c603ec3ce7016a37cb5106eeda13612 Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 17:58:58 +0200 Subject: [PATCH 06/10] Update examples to use underscores instead of dashes. --- __tests__/fixtures/inline-license-config-sample.yml | 2 +- __tests__/fixtures/invalid-severity-config.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/__tests__/fixtures/inline-license-config-sample.yml b/__tests__/fixtures/inline-license-config-sample.yml index a5730dc..ce6dae2 100644 --- a/__tests__/fixtures/inline-license-config-sample.yml +++ b/__tests__/fixtures/inline-license-config-sample.yml @@ -1 +1 @@ -allow-licenses: MIT, GPL-2.0-only +allow_licenses: MIT, GPL-2.0-only diff --git a/__tests__/fixtures/invalid-severity-config.yml b/__tests__/fixtures/invalid-severity-config.yml index 9e4b6b6..3847bb1 100644 --- a/__tests__/fixtures/invalid-severity-config.yml +++ b/__tests__/fixtures/invalid-severity-config.yml @@ -1,3 +1,3 @@ -fail-on-severity: 'so many zombies' -deny-licenses: +fail_on_severity: 'so many zombies' +deny_licenses: - MIT From 50b918791f08b762d3d1f0d1476ac011da7e39e0 Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 17:59:34 +0200 Subject: [PATCH 07/10] Update README. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0541774..72e2719 100644 --- a/README.md +++ b/README.md @@ -131,8 +131,8 @@ Start by specifying that you will be using an external configuration file: And then create the file in the path you just specified: ```yaml -fail-on-severity: 'critical' -allow-licenses: +fail_on_severity: 'critical' +allow_licenses: - 'GPL-3.0' - 'BSD-3-Clause' - 'MIT' From cebb5b1214690945d15593a6f790865646304766 Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 21:33:24 +0200 Subject: [PATCH 08/10] Don't use underscore for inline configs. --- __tests__/fixtures/inline-license-config-sample.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/fixtures/inline-license-config-sample.yml b/__tests__/fixtures/inline-license-config-sample.yml index ce6dae2..fa81ab0 100644 --- a/__tests__/fixtures/inline-license-config-sample.yml +++ b/__tests__/fixtures/inline-license-config-sample.yml @@ -1 +1 @@ -allow_licenses: MIT, GPL-2.0-only +allow-licenses: "MIT, GPL-2.0-only" From 9885d0c74cca15bb0f8128ee7a4018cbac4d6c32 Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 21:33:35 +0200 Subject: [PATCH 09/10] Remove default values in action.yml --- action.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/action.yml b/action.yml index c7dad3e..86e4221 100644 --- a/action.yml +++ b/action.yml @@ -1,3 +1,5 @@ +# Avoid using default values for options here since they will +# end up overriding external configurations. name: 'Dependency Review' description: 'Prevent the introduction of dependencies with known vulnerabilities' author: 'GitHub' @@ -9,11 +11,9 @@ inputs: fail-on-severity: description: Don't block PRs below this severity. Possible values are `low`, `moderate`, `high`, `critical`. required: false - default: 'low' fail-on-scopes: description: Dependency scopes to block PRs on. Comma-separated list. Possible values are 'unknown', 'runtime', and 'development' (e.g. "runtime, development") required: false - default: 'runtime' base-ref: description: The base 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 From 654eb5ca1c29dea80ae435b7e64ac77ec50eb2da Mon Sep 17 00:00:00 2001 From: Federico Builes Date: Thu, 6 Apr 2023 21:42:26 +0200 Subject: [PATCH 10/10] Updating README.md --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 72e2719..c0b5f70 100644 --- a/README.md +++ b/README.md @@ -128,7 +128,8 @@ Start by specifying that you will be using an external configuration file: config-file: './.github/dependency-review-config.yml' ``` -And then create the file in the path you just specified: +And then create the file in the path you just specified. Please note +that the **option names in external files use underscores instead of dashes**: ```yaml fail_on_severity: 'critical'