From bbed6f340aa805712b64b05418048d552ffabcfb Mon Sep 17 00:00:00 2001 From: Eli Reisman Date: Wed, 5 Jun 2024 22:57:06 -0700 Subject: [PATCH] update licenses pkg and tests --- __tests__/licenses.test.ts | 59 +++++++++++++++---------------- src/licenses.ts | 19 ++++------ src/spdx.ts | 14 ++++++++ types/spdx-license-satisfies.d.ts | 12 +++++++ 4 files changed, 62 insertions(+), 42 deletions(-) diff --git a/__tests__/licenses.test.ts b/__tests__/licenses.test.ts index 1bb21c2..bcb86f0 100644 --- a/__tests__/licenses.test.ts +++ b/__tests__/licenses.test.ts @@ -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', @@ -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', source_repository_url: 'github.com/some-repo', scope: 'runtime', vulnerabilities: [ @@ -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) }) @@ -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([]) }) @@ -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([]) }) @@ -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) @@ -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( @@ -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'] } @@ -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) diff --git a/src/licenses.ts b/src/licenses.ts index 8c88899..1c7f954 100644 --- a/src/licenses.ts +++ b/src/licenses.ts @@ -1,8 +1,7 @@ -import spdxSatisfies from 'spdx-satisfies' import {Change, Changes} from './schemas' import {octokitClient} from './utils' import {parsePURL} from './purl' -import {isValid} from './spdx' +import * as spdx from './spdx' /** * Loops through a list of changes, filtering and returning the @@ -48,7 +47,7 @@ export async function getInvalidLicenseChanges( const changeAsPackageURL = parsePURL(encodeURI(change.package_url)) - // We want to find if the licenseExclussion list contains the PackageURL of the Change + // We want to find if the licenseExclusion list contains the PackageURL of the Change // If it does, we want to filter it out and therefore return false // If it doesn't, we want to keep it and therefore return true if ( @@ -88,15 +87,11 @@ export async function getInvalidLicenseChanges( } else if (validityCache.get(license) === undefined) { try { if (allow !== undefined) { - const found = allow.find(spdxExpression => - spdxSatisfies(license, spdxExpression) - ) - validityCache.set(license, found !== undefined) + const found = spdx.satisfiesAny(license, allow) + validityCache.set(license, found) } else if (deny !== undefined) { - const found = deny.find(spdxExpression => - spdxSatisfies(license, spdxExpression) - ) - validityCache.set(license, found === undefined) + const found = spdx.satisfiesAny(license, deny) + validityCache.set(license, !found) } } catch (err) { invalidLicenseChanges.unresolved.push(change) @@ -166,7 +161,7 @@ const setGHLicenses = async (changes: Change[]): Promise => { // Currently Dependency Graph licenses are truncated to 255 characters // This possibly makes them invalid spdx ids const truncatedDGLicense = (license: string): boolean => - license.length === 255 && !isValid(license) + license.length === 255 && !spdx.isValid(license) async function groupChanges( changes: Changes diff --git a/src/spdx.ts b/src/spdx.ts index 0ba6d7f..f280998 100644 --- a/src/spdx.ts +++ b/src/spdx.ts @@ -8,6 +8,20 @@ export function satisfies( return spdx.satisfies(candidateExpr, constraintExpr) } +export function satisfiesAny( + candidateExpr: string, + licenses: string[] +): boolean { + return spdx.satisfiesAny(candidateExpr, licenses) +} + +export function satisfiesAll( + candidateExpr: string, + licenses: string[] +): boolean { + return spdx.satisfiesAll(candidateExpr, licenses) +} + // can be a single license or an SPDX expression export function isValid(spdxExpr: string): boolean { try { diff --git a/types/spdx-license-satisfies.d.ts b/types/spdx-license-satisfies.d.ts index 9de8a9a..23b0271 100644 --- a/types/spdx-license-satisfies.d.ts +++ b/types/spdx-license-satisfies.d.ts @@ -3,4 +3,16 @@ declare module '@onebeyond/spdx-license-satisfies' { candidateExpr: string, constraintExpr: string ): boolean + + export function satisfiesAny( + candidateExpr: string, + licenses: string[] + ): boolean + + export function satisfiesAll( + candidateExpr: string, + licenses: string[] + ): boolean + + export function isValid(candidateExpr: string): boolean }