Merge pull request #930 from actions/889-allow-no-license

Allowing dependencies works with no licenses

__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/[email protected]',
+  license: null,
+  source_repository_url: 'github.com/some-repo',
+  scope: 'development',
+  vulnerabilities: []
+}
+
 jest.mock('@actions/core')
 
 const mockOctokit = {
@@ -313,4 +327,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

@@ -394,27 +394,7 @@ function getInvalidLicenseChanges(changes, licenses) {
         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);
+        const groupedChanges = yield 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 = (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 licensedChanges = groupedChanges.licensed;
         const invalidLicenseChanges = {
             unlicensed: groupedChanges.unlicensed,
@@ -509,14 +489,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) {
+function groupChanges(changes_1) {
-    return __awaiter(this, void 0, void 0, function* () {
+    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;
             }

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'
 
 /**
@@ -36,34 +36,8 @@ export async function getInvalidLicenseChanges(
     }
   )
 
-  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 +146,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
     }