Merge pull request #933 from actions/dangoor/471-release

Bump version number for 4.7.1

__tests__/licenses.test.ts

@@ -100,6 +100,20 @@ const complexLicenseChange: Change = {
   ]
 }
 
+const unlicensedChange: Change = {
+  change_type: 'added',
+  manifest: '.github/workflows/ci.yml',
+  ecosystem: 'actions',
+  name: 'foo-org/actions-repo/.github/workflows/some-action.yml',
+  version: '1.1.1',
+  package_url:
+    'pkg:githubactions/foo-org/actions-repo/.github/workflows/some-action.yml@1.1.1',
+  license: null,
+  source_repository_url: 'github.com/some-repo',
+  scope: 'development',
+  vulnerabilities: []
+}
+
 jest.mock('@actions/core')
 
 const mockOctokit = {
@@ -276,6 +290,19 @@ test('it does filters out changes if they are not on the exclusions list', async
   expect(invalidLicenses.forbidden[1]).toBe(npmChange)
 })
 
+test('it does not fail if there is a license expression in the allow list', async () => {
+  const changes: Changes = [
+    {...npmChange, license: 'MIT AND Apache-2.0'},
+    {...rubyChange, license: 'BSD-3-Clause'}
+  ]
+
+  const {forbidden} = await getInvalidLicenseChanges(changes, {
+    allow: ['BSD-3-Clause', 'MIT AND Apache-2.0', 'MIT', 'Apache-2.0']
+  })
+
+  expect(forbidden.length).toEqual(0)
+})
+
 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 = {
@@ -313,4 +340,26 @@ describe('GH License API fallback', () => {
     expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
     expect(unlicensed.length).toEqual(0)
   })
+
+  test('it does not call licenses API if the package is excluded', async () => {
+    const {unlicensed} = await getInvalidLicenseChanges([unlicensedChange], {
+      licenseExclusions: [
+        'pkg:githubactions/foo-org/actions-repo/.github/workflows/some-action.yml'
+      ]
+    })
+
+    expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
+    expect(unlicensed.length).toEqual(0)
+  })
+
+  test('it checks namespaces when doing exclusions', async () => {
+    const {unlicensed} = await getInvalidLicenseChanges([unlicensedChange], {
+      licenseExclusions: [
+        'pkg:githubactions/bar-org/actions-repo/.github/workflows/some-action.yml'
+      ]
+    })
+
+    expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
+    expect(unlicensed.length).toEqual(1)
+  })
 })

dist/index.js

@@ -390,31 +390,18 @@ const spdx = __importStar(__nccwpck_require__(2593));
 function getInvalidLicenseChanges(changes, licenses) {
     return __awaiter(this, void 0, void 0, function* () {
         var _a;
-        const { allow, deny } = licenses;
+        const deny = licenses.deny;
+        let allow = licenses.allow;
+        // Filter out elements of the allow list that include AND
+        // or OR because the list should be simple license IDs and
+        // not expressions.
+        allow = allow === null || allow === void 0 ? void 0 : allow.filter(license => {
+            return !license.includes(' AND ') && !license.includes(' OR ');
+        });
         const licenseExclusions = (_a = licenses.licenseExclusions) === null || _a === void 0 ? void 0 : _a.map((pkgUrl) => {
             return (0, purl_1.parsePURL)(pkgUrl);
         });
-        const groupedChanges = yield groupChanges(changes);
-        // Takes the changes from the groupedChanges object and filters out the ones that are part of the exclusions list
-        // It does by creating a new PackageURL object from the change and comparing it to the exclusions list
-        groupedChanges.licensed = groupedChanges.licensed.filter(change => {
-            if (change.package_url.length === 0) {
-                return true;
-            }
-            const changeAsPackageURL = (0, purl_1.parsePURL)(encodeURI(change.package_url));
-            // 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 (licenseExclusions !== null &&
-                licenseExclusions !== undefined &&
-                licenseExclusions.findIndex(exclusion => exclusion.type === changeAsPackageURL.type &&
-                    exclusion.name === changeAsPackageURL.name) !== -1) {
-                return false;
-            }
-            else {
-                return true;
-            }
-        });
+        const groupedChanges = yield groupChanges(changes, licenseExclusions);
         const licensedChanges = groupedChanges.licensed;
         const invalidLicenseChanges = {
             unlicensed: groupedChanges.unlicensed,
@@ -509,14 +496,37 @@ const setGHLicenses = (changes) => __awaiter(void 0, void 0, void 0, function* (
 // Currently Dependency Graph licenses are truncated to 255 characters
 // This possibly makes them invalid spdx ids
 const truncatedDGLicense = (license) => license.length === 255 && !spdx.isValid(license);
-function groupChanges(changes) {
-    return __awaiter(this, void 0, void 0, function* () {
+function groupChanges(changes_1) {
+    return __awaiter(this, arguments, void 0, function* (changes, licenseExclusions = null) {
         const result = {
             licensed: [],
             unlicensed: []
         };
+        let candidateChanges = changes;
+        // If a package is excluded from license checking, we don't bother trying to
+        // fetch the license for it and we leave it off of the `licensed` and
+        // `unlicensed` lists.
+        if (licenseExclusions !== null && licenseExclusions !== undefined) {
+            candidateChanges = candidateChanges.filter(change => {
+                if (change.package_url.length === 0) {
+                    return true;
+                }
+                const changeAsPackageURL = (0, purl_1.parsePURL)(encodeURI(change.package_url));
+                // 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 (licenseExclusions.findIndex(exclusion => exclusion.type === changeAsPackageURL.type &&
+                    exclusion.namespace === changeAsPackageURL.namespace &&
+                    exclusion.name === changeAsPackageURL.name) !== -1) {
+                    return false;
+                }
+                else {
+                    return true;
+                }
+            });
+        }
         const ghChanges = [];
-        for (const change of changes) {
+        for (const change of candidateChanges) {
             if (change.change_type === 'removed') {
                 continue;
             }

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "dependency-review-action",
-  "version": "4.7.0",
+  "version": "4.7.1",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "dependency-review-action",
-      "version": "4.7.0",
+      "version": "4.7.1",
       "license": "MIT",
       "dependencies": {
         "@actions/core": "^1.10.1",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "dependency-review-action",
-  "version": "4.7.0",
+  "version": "4.7.1",
   "private": true,
   "description": "A GitHub Action for Dependency Review",
   "main": "lib/main.js",

src/licenses.ts

@@ -1,6 +1,6 @@
 import {Change, Changes} from './schemas'
 import {octokitClient} from './utils'
-import {parsePURL} from './purl'
+import {parsePURL, PackageURL} from './purl'
 import * as spdx from './spdx'
 
 /**
@@ -29,41 +29,24 @@ export async function getInvalidLicenseChanges(
     licenseExclusions?: string[]
   }
 ): Promise<InvalidLicenseChanges> {
-  const {allow, deny} = licenses
+  const deny = licenses.deny
+  let allow = licenses.allow
+
+  // Filter out elements of the allow list that include AND
+  // or OR because the list should be simple license IDs and
+  // not expressions.
+  allow = allow?.filter(license => {
+    return !license.includes(' AND ') && !license.includes(' OR ')
+  })
+
   const licenseExclusions = licenses.licenseExclusions?.map(
     (pkgUrl: string) => {
       return parsePURL(pkgUrl)
     }
   )
 
-  const groupedChanges = await groupChanges(changes)
+  const groupedChanges = await groupChanges(changes, licenseExclusions)
 
-  // Takes the changes from the groupedChanges object and filters out the ones that are part of the exclusions list
-  // It does by creating a new PackageURL object from the change and comparing it to the exclusions list
-  groupedChanges.licensed = groupedChanges.licensed.filter(change => {
-    if (change.package_url.length === 0) {
-      return true
-    }
-
-    const changeAsPackageURL = parsePURL(encodeURI(change.package_url))
-
-    // 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 (
-      licenseExclusions !== null &&
-      licenseExclusions !== undefined &&
-      licenseExclusions.findIndex(
-        exclusion =>
-          exclusion.type === changeAsPackageURL.type &&
-          exclusion.name === changeAsPackageURL.name
-      ) !== -1
-    ) {
-      return false
-    } else {
-      return true
-    }
-  })
   const licensedChanges: Changes = groupedChanges.licensed
 
   const invalidLicenseChanges: InvalidLicenseChanges = {
@@ -172,16 +155,48 @@ const truncatedDGLicense = (license: string): boolean =>
   license.length === 255 && !spdx.isValid(license)
 
 async function groupChanges(
-  changes: Changes
+  changes: Changes,
+  licenseExclusions: PackageURL[] | null = null
 ): Promise<Record<string, Changes>> {
   const result: Record<string, Changes> = {
     licensed: [],
     unlicensed: []
   }
 
+  let candidateChanges = changes
+
+  // If a package is excluded from license checking, we don't bother trying to
+  // fetch the license for it and we leave it off of the `licensed` and
+  // `unlicensed` lists.
+  if (licenseExclusions !== null && licenseExclusions !== undefined) {
+    candidateChanges = candidateChanges.filter(change => {
+      if (change.package_url.length === 0) {
+        return true
+      }
+
+      const changeAsPackageURL = parsePURL(encodeURI(change.package_url))
+
+      // 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 (
+        licenseExclusions.findIndex(
+          exclusion =>
+            exclusion.type === changeAsPackageURL.type &&
+            exclusion.namespace === changeAsPackageURL.namespace &&
+            exclusion.name === changeAsPackageURL.name
+        ) !== -1
+      ) {
+        return false
+      } else {
+        return true
+      }
+    })
+  }
+
   const ghChanges = []
 
-  for (const change of changes) {
+  for (const change of candidateChanges) {
     if (change.change_type === 'removed') {
       continue
     }