Merge pull request #284 from actions/cn/license-api-fallback

Use GH Licenses API to retrieve null licenses

__tests__/licenses.test.ts

@@ -1,4 +1,4 @@
-import {expect, test} from '@jest/globals'
+import {expect, jest, test} from '@jest/globals'
 import {Change, Changes} from '../src/schemas'
 import {getDeniedLicenseChanges} from '../src/licenses'
 
@@ -48,15 +48,41 @@ let rubyChange: Change = {
   ]
 }
 
+jest.mock('@actions/core')
+
+const mockOctokit = {
+  rest: {
+    licenses: {
+      getForRepo: jest
+        .fn()
+        .mockReturnValue({data: {license: {spdx_id: 'AGPL'}}})
+    }
+  }
+}
+
+jest.mock('octokit', () => {
+  return {
+    Octokit: class {
+      constructor() {
+        return mockOctokit
+      }
+    }
+  }
+})
+
 test('it fails if a license outside the allow list is found', async () => {
   const changes: Changes = [npmChange, rubyChange]
-  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {allow: ['BSD']})
+  const [invalidChanges, _] = await getDeniedLicenseChanges(changes, {
+    allow: ['BSD']
+  })
   expect(invalidChanges[0]).toBe(npmChange)
 })
 
 test('it fails if a license inside the deny list is found', async () => {
   const changes: Changes = [npmChange, rubyChange]
-  const [invalidChanges] = getDeniedLicenseChanges(changes, {deny: ['BSD']})
+  const [invalidChanges] = await getDeniedLicenseChanges(changes, {
+    deny: ['BSD']
+  })
   expect(invalidChanges[0]).toBe(rubyChange)
 })
 
@@ -64,7 +90,7 @@ test('it fails if a license inside the deny list is found', async () => {
 // thing we want in the system. Please remove this test after refactoring.
 test('it fails all license checks when allow is provided an empty array', async () => {
   const changes: Changes = [npmChange, rubyChange]
-  let [invalidChanges, _] = getDeniedLicenseChanges(changes, {
+  let [invalidChanges, _] = await getDeniedLicenseChanges(changes, {
     allow: [],
     deny: ['BSD']
   })
@@ -76,7 +102,9 @@ test('it does not fail if a license outside the allow list is found in removed c
     {...npmChange, change_type: 'removed'},
     {...rubyChange, change_type: 'removed'}
   ]
-  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {allow: ['BSD']})
+  const [invalidChanges, _] = await getDeniedLicenseChanges(changes, {
+    allow: ['BSD']
+  })
   expect(invalidChanges).toStrictEqual([])
 })
 
@@ -85,7 +113,9 @@ test('it does not fail if a license inside the deny list is found in removed cha
     {...npmChange, change_type: 'removed'},
     {...rubyChange, change_type: 'removed'}
   ]
-  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {deny: ['BSD']})
+  const [invalidChanges, _] = await getDeniedLicenseChanges(changes, {
+    deny: ['BSD']
+  })
   expect(invalidChanges).toStrictEqual([])
 })
 
@@ -95,6 +125,47 @@ test('it fails if a license outside the allow list is found in both of added and
     npmChange,
     {...rubyChange, change_type: 'removed'}
   ]
-  const [invalidChanges, _] = getDeniedLicenseChanges(changes, {allow: ['BSD']})
+  const [invalidChanges, _] = await getDeniedLicenseChanges(changes, {
+    allow: ['BSD']
+  })
   expect(invalidChanges).toStrictEqual([npmChange])
 })
+
+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 = {
+      ...npmChange,
+      license: null,
+      source_repository_url: 'http://github.com/some-owner/some-repo'
+    }
+    const [_, unknownChanges] = await getDeniedLicenseChanges(
+      [nullLicenseChange, rubyChange],
+      {}
+    )
+
+    expect(mockOctokit.rest.licenses.getForRepo).toHaveBeenNthCalledWith(1, {
+      owner: 'some-owner',
+      repo: 'some-repo'
+    })
+    expect(unknownChanges.length).toEqual(0)
+  })
+
+  test('it does not call licenses API endpoint for change with null license and invalid source_repository_url ', async () => {
+    const [_, unknownChanges] = await getDeniedLicenseChanges(
+      [{...npmChange, license: null}],
+      {}
+    )
+    expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
+    expect(unknownChanges.length).toEqual(1)
+  })
+
+  test('it does not call licenses API endpoint if licenses for all changes are present', async () => {
+    const [_, unknownChanges] = await getDeniedLicenseChanges(
+      [npmChange, rubyChange],
+      {}
+    )
+
+    expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
+    expect(unknownChanges.length).toEqual(0)
+  })
+})

package.json

@@ -32,10 +32,12 @@
     "ansi-styles": "^6.2.1",
     "got": "^12.5.2",
     "nodemon": "^2.0.20",
+    "octokit": "^2.0.7",
     "yaml": "^2.1.3",
     "zod": "^3.19.1"
   },
   "devDependencies": {
+    "@types/jest": "^27.5.2",
     "@types/node": "^16.11.65",
     "@typescript-eslint/eslint-plugin": "^5.40.0",
     "@typescript-eslint/parser": "^5.40.0",

src/licenses.ts

@@ -1,3 +1,5 @@
+import * as core from '@actions/core'
+import {Octokit} from 'octokit'
 import {Change} from './schemas'
 
 /**
@@ -10,21 +12,27 @@ import {Change} from './schemas'
  * we will ignore the deny list.
  * @param {Change[]} changes The list of changes to filter.
  * @param { { allow?: string[], deny?: string[]}} licenses An object with `allow`/`deny` keys, each containing a list of licenses.
- * @returns {[Array<Change>, Array<Change]} A tuple where the first element is the list of denied changes and the second one is the list of changes with unknown licenses
+ * @returns {Promise<[Array.<Change>, Array.<Change>]>} A promise to a 2 element tuple. The first element is the list of denied changes and the second one is the list of changes with unknown licenses
  */
-export function getDeniedLicenseChanges(
+export async function getDeniedLicenseChanges(
   changes: Change[],
   licenses: {
     allow?: string[]
     deny?: string[]
   }
-): [Change[], Change[]] {
+): Promise<[Change[], Change[]]> {
   const {allow, deny} = licenses
 
   const disallowed: Change[] = []
   const unknown: Change[] = []
 
-  for (const change of changes) {
+  const consolidatedChanges = changes.some(
+    ({source_repository_url, license}) => !license && source_repository_url
+  )
+    ? await setGHLicenses(changes)
+    : changes
+
+  for (const change of consolidatedChanges) {
     if (change.change_type === 'removed') {
       continue
     }
@@ -47,3 +55,56 @@ export function getDeniedLicenseChanges(
 
   return [disallowed, unknown]
 }
+
+const fetchGHLicense = async (
+  owner: string,
+  repo: string
+): Promise<string | null> => {
+  const octokit = new Octokit({
+    auth: core.getInput('repo-token', {required: true})
+  })
+
+  try {
+    const response = await octokit.rest.licenses.getForRepo({owner, repo})
+    return response.data.license?.spdx_id ?? null
+  } catch (_) {
+    return null
+  }
+}
+
+const parseGitHubURL = (url: string): {owner: string; repo: string} | null => {
+  try {
+    const parsed = new URL(url)
+    if (parsed.host !== 'github.com') {
+      return null
+    }
+    const components = parsed.pathname.split('/')
+    if (components.length < 3) {
+      return null
+    }
+    return {owner: components[1], repo: components[2]}
+  } catch (_) {
+    return null
+  }
+}
+
+const setGHLicenses = async (changes: Change[]): Promise<Change[]> => {
+  const updatedChanges = changes.map(async change => {
+    if (change.license !== null || change.source_repository_url === null) {
+      return change
+    }
+
+    const githubUrl = parseGitHubURL(change.source_repository_url)
+
+    if (githubUrl === null) {
+      return change
+    }
+
+    return {
+      ...change,
+      license: await fetchGHLicense(githubUrl.owner, githubUrl.repo)
+    }
+  })
+
+  return Promise.all(updatedChanges)
+}

src/main.ts

@@ -45,7 +45,7 @@ async function run(): Promise<void> {
         change.vulnerabilities.length > 0
     )
 
-    const [licenseErrors, unknownLicenses] = getDeniedLicenseChanges(
+    const [licenseErrors, unknownLicenses] = await getDeniedLicenseChanges(
       filteredChanges,
       {
         allow: config.allow_licenses,