Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SPDX Expression Parsing #719

Merged
merged 9 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 8 additions & 16 deletions __tests__/config.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
import {expect, test, beforeEach} from '@jest/globals'
import {readConfig} from '../src/config'
import {getRefs} from '../src/git-refs'
import * as Utils from '../src/utils'
import * as spdx from '../src/spdx'
import {setInput, clearInputs} from './test-helpers'

beforeAll(() => {
jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(true)
})

beforeEach(() => {
clearInputs()
})
Expand All @@ -19,11 +15,11 @@ test('it defaults to low severity', async () => {

test('it reads custom configs', async () => {
setInput('fail-on-severity', 'critical')
setInput('allow-licenses', ' BSD, GPL 2')
setInput('allow-licenses', 'ISC, GPL-2.0')

const config = await readConfig()
expect(config.fail_on_severity).toEqual('critical')
expect(config.allow_licenses).toEqual(['BSD', 'GPL 2'])
expect(config.allow_licenses).toEqual(['ISC', 'GPL-2.0'])
})

test('it defaults to false for warn-only', async () => {
Expand All @@ -40,7 +36,7 @@ test('it defaults to empty allow/deny lists ', async () => {

test('it raises an error if both an allow and denylist are specified', async () => {
setInput('allow-licenses', 'MIT')
setInput('deny-licenses', 'BSD')
setInput('deny-licenses', 'BSD-3-Clause')

await expect(readConfig()).rejects.toThrow(
'You cannot specify both allow-licenses and deny-licenses'
Expand Down Expand Up @@ -204,21 +200,17 @@ test('it is not possible to disable both checks', async () => {
})

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')
setInput('allow-licenses', ' BSD-YOLO, GPL-2.0')
await expect(readConfig()).rejects.toThrow(
'Invalid license(s) in allow-licenses: BSD,GPL 2'
'Invalid license(s) in allow-licenses: BSD-YOLO'
)
})

test('it raises an error for invalid licenses in deny-licenses', async () => {
setInput('deny-licenses', ' BSD, GPL 2')
setInput('deny-licenses', ' GPL-2.0, BSD-YOLO, Apache-2.0, ToIll')
await expect(readConfig()).rejects.toThrow(
'Invalid license(s) in deny-licenses: BSD,GPL 2'
'Invalid license(s) in deny-licenses: BSD-YOLO, ToIll'
)
})
})
Expand Down
5 changes: 0 additions & 5 deletions __tests__/deny.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ jest.mock('octokit', () => {

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')
})

npmChange = createTestChange({ecosystem: 'npm'})
rubyChange = createTestChange({ecosystem: 'rubygems'})
Expand Down
8 changes: 2 additions & 6 deletions __tests__/external-config.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {expect, test, beforeEach} from '@jest/globals'
import {readConfig} from '../src/config'
import * as Utils from '../src/utils'
import * as spdx from '../src/spdx'
import {setInput, clearInputs} from './test-helpers'

const externalConfig = `fail_on_severity: 'high'
Expand All @@ -25,10 +25,6 @@ jest.mock('octokit', () => {
}
})

beforeAll(() => {
jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(true)
})

beforeEach(() => {
clearInputs()
})
Expand All @@ -38,7 +34,7 @@ test('it reads an external config file', async () => {

const config = await readConfig()
expect(config.fail_on_severity).toEqual('critical')
expect(config.allow_licenses).toEqual(['BSD', 'GPL 2'])
expect(config.allow_licenses).toEqual(['BSD-3-Clause', 'GPL-2.0'])
})

test('raises an error when the config file was not found', async () => {
Expand Down
4 changes: 2 additions & 2 deletions __tests__/fixtures/config-allow-sample.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
fail_on_severity: critical
allow_licenses:
- 'BSD'
- 'GPL 2'
- 'BSD-3-Clause'
- 'GPL-2.0'
59 changes: 29 additions & 30 deletions __tests__/licenses.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {expect, jest, test} from '@jest/globals'
import {Change, Changes} from '../src/schemas'

let getInvalidLicenseChanges: Function
import {getInvalidLicenseChanges} from '../src/licenses'

const npmChange: Change = {
manifest: 'package.json',
Expand Down Expand Up @@ -30,7 +29,7 @@ const rubyChange: Change = {
name: 'actionsomething',
version: '3.2.0',
package_url: 'pkg:gem/actionsomething@3.2.0',
license: 'BSD',
license: 'BSD-3-Clause',
elireisman marked this conversation as resolved.
Show resolved Hide resolved
source_repository_url: 'github.com/some-repo',
scope: 'runtime',
vulnerabilities: [
Expand Down Expand Up @@ -100,29 +99,32 @@ jest.mock('octokit', () => {

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')
})
// eslint-disable-next-line @typescript-eslint/no-require-imports
;({getInvalidLicenseChanges} = require('../src/licenses'))
})

test('it adds license outside the allow list to forbidden changes', async () => {
const changes: Changes = [npmChange, rubyChange]
const changes: Changes = [
npmChange, // MIT license
rubyChange // BSD license
]

const {forbidden} = await getInvalidLicenseChanges(changes, {
allow: ['BSD']
allow: ['BSD-3-Clause']
})

expect(forbidden[0]).toBe(npmChange)
expect(forbidden.length).toEqual(1)
})

test('it adds license inside the deny list to forbidden changes', async () => {
const changes: Changes = [npmChange, rubyChange]
const changes: Changes = [
npmChange, // MIT license
rubyChange // BSD license
]

const {forbidden} = await getInvalidLicenseChanges(changes, {
deny: ['BSD']
deny: ['BSD-3-Clause']
})

expect(forbidden[0]).toBe(rubyChange)
expect(forbidden.length).toEqual(1)
})
Expand All @@ -133,7 +135,7 @@ test('it does not add license outside the allow list to forbidden changes if it
{...rubyChange, change_type: 'removed'}
]
const {forbidden} = await getInvalidLicenseChanges(changes, {
allow: ['BSD']
allow: ['BSD-3-Clause']
})
expect(forbidden).toStrictEqual([])
})
Expand All @@ -144,7 +146,7 @@ test('it does not add license inside the deny list to forbidden changes if it is
{...rubyChange, change_type: 'removed'}
]
const {forbidden} = await getInvalidLicenseChanges(changes, {
deny: ['BSD']
deny: ['BSD-3-Clause']
})
expect(forbidden).toStrictEqual([])
})
Expand All @@ -156,23 +158,18 @@ test('it adds license outside the allow list to forbidden changes if it is in bo
{...rubyChange, change_type: 'removed'}
]
const {forbidden} = await getInvalidLicenseChanges(changes, {
allow: ['BSD']
allow: ['BSD-3-Clause']
})
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')
})
})
// eslint-disable-next-line @typescript-eslint/no-require-imports
;({getInvalidLicenseChanges} = require('../src/licenses'))
const changes: Changes = [npmChange, rubyChange]
const changes: Changes = [
{...npmChange, license: 'Foo'},
{...rubyChange, license: 'Bar'}
]
const invalidLicenses = await getInvalidLicenseChanges(changes, {
allow: ['BSD']
allow: ['Apache-2.0']
})
expect(invalidLicenses.forbidden.length).toEqual(0)
expect(invalidLicenses.unlicensed.length).toEqual(0)
Expand All @@ -182,7 +179,7 @@ test('it adds all licenses to unresolved if it is unable to determine the validi
test('it does not filter out changes that are on the exclusions list', async () => {
const changes: Changes = [pipChange, npmChange, rubyChange]
const licensesConfig = {
allow: ['BSD'],
allow: ['BSD-3-Clause'],
licenseExclusions: ['pkg:pypi/package-1@1.1.1', 'pkg:npm/reeuhq@1.0.2']
}
const invalidLicenses = await getInvalidLicenseChanges(
Expand All @@ -198,7 +195,7 @@ test('it does not fail when the packages dont have a valid PURL', async () => {

const changes: Changes = [emptyPurlChange, npmChange, rubyChange]
const licensesConfig = {
allow: ['BSD'],
allow: ['BSD-3-Clause'],
licenseExclusions: ['pkg:pypi/package-1@1.1.1', 'pkg:npm/reeuhq@1.0.2']
}

Expand All @@ -212,16 +209,18 @@ test('it does not fail when the packages dont have a valid PURL', async () => {
test('it does filters out changes if they are not on the exclusions list', async () => {
const changes: Changes = [pipChange, npmChange, rubyChange]
const licensesConfig = {
allow: ['BSD'],
allow: ['BSD-3-Clause'],
licenseExclusions: [
'pkg:pypi/notmypackage-1@1.1.1',
'pkg:npm/alsonot@1.0.2'
]
}

const invalidLicenses = await getInvalidLicenseChanges(
changes,
licensesConfig
)

expect(invalidLicenses.forbidden.length).toEqual(2)
expect(invalidLicenses.forbidden[0]).toBe(pipChange)
expect(invalidLicenses.forbidden[1]).toBe(npmChange)
Expand Down
Loading