[BUG] Recently updated NPM packages show "Unknown License"

[jescalada]

Related #704

Describe the bug
Recently updated NPM packages show "Unknown License" despite the license being present on NPM, and also fetchable via GitHub API:

Dependency Review output

NPM package

How to verify

This can be verified using gh or curl as follows:

# package in #704
gh api /repos/nodesource/setup-nsolid/license --jq '.license.spdx_id'

# yauzl's source repo
gh api /repos/thejoshwolfe/yauzl/license --jq '.license.spdx_id'

curl -s -H "Accept: application/vnd.github+json" \
  https://api.github.com/repos/nodesource/setup-nsolid/license | jq -r '.license.spdx_id'

curl -s -H "Accept: application/vnd.github+json" \
  https://api.github.com/repos/thejoshwolfe/yauzl/license | jq -r '.license.spdx_id'

Both of these print "MIT".

Expected behavior
There should be a fallback for those cases where the license hasn't been indexed by GitHub but can still be obtained programmatically. Note that this bug doesn't prevent the Dependency Review check from passing, it just raises questions as to whether the packages in question actually are missing their licenses.

Action version
5.0.0 (a1d282b36b6f3519aa1f3fc636f609c47dddb294)

Additional context
Original execution: https://github.com/finos/git-proxy/actions/runs/27081857339/job/79929142767?pr=1564

[jescalada]

I ran this through an agent to find a solution for both issues, but I'm worried about these changes affecting execution when the license really is missing (not just un-indexed), or other edge cases being introduced.

Suggested solution (AI-generated, manually tweaked)

licenses.ts

import {Change, Changes} from './schemas'
import {octokitClient} from './utils'
import {parsePURL, PackageURL, purlsMatch} from './purl'
import * as spdx from './spdx'

/**
 * Loops through a list of changes, filtering and returning the
 * ones that don't conform to the licenses allow/deny lists.
 * It will also filter out the changes which are defined in the licenseExclusions list.
 *
 * Keep in mind that we don't let users specify both an allow and a deny
 * list in their config files, so this code works under the assumption that
 * one of the two list parameters will be empty. If both lists are provided,
 * we will ignore the deny list.
 * @param {Change[]} changes The list of changes to filter.
 * @param { { allow?: string[], deny?: string[], licenseExclusions?: string[]}} licenses An object with `allow`/`deny`/`licenseExclusions` keys, each containing a list of licenses.
 * @returns {Promise<{Object.<string, Array.<Change>>}} A promise to a Record Object. The keys are strings, unlicensed, unresolved and forbidden. The values are a list of changes
 */
export type InvalidLicenseChangeTypes =
  | 'unlicensed'
  | 'unresolved'
  | 'forbidden'
export type InvalidLicenseChanges = Record<InvalidLicenseChangeTypes, Changes>
export async function getInvalidLicenseChanges(
  changes: Change[],
  licenses: {
    allow?: string[]
    deny?: string[]
    licenseExclusions?: string[]
  }
): Promise<InvalidLicenseChanges> {
  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, licenseExclusions)

  const licensedChanges: Changes = groupedChanges.licensed

  const invalidLicenseChanges: InvalidLicenseChanges = {
    unlicensed: groupedChanges.unlicensed,
    unresolved: [],
    forbidden: []
  }

  const validityCache = new Map<string, boolean>()

  for (const change of licensedChanges) {
    const license = change.license

    // should never happen since licensedChanges always have licenses but license is nullable in changes schema
    if (license === null) {
      continue
    }

    if (license === 'NOASSERTION') {
      invalidLicenseChanges.unlicensed.push(change)
    } else if (validityCache.get(license) === undefined) {
      try {
        if (allow !== undefined) {
          if (spdx.isValid(license)) {
            const found = spdx.satisfies(license, allow)
            validityCache.set(license, found)
          } else {
            invalidLicenseChanges.unresolved.push(change)
          }
        } else if (deny !== undefined) {
          if (spdx.isValid(license)) {
            const found = spdx.satisfiesAny(license, deny)
            validityCache.set(license, !found)
          } else {
            invalidLicenseChanges.unresolved.push(change)
          }
        }
      } catch (err) {
        invalidLicenseChanges.unresolved.push(change)
      }
    }

    if (validityCache.get(license) === false) {
      invalidLicenseChanges.forbidden.push(change)
    }
  }

  return invalidLicenseChanges
}

const fetchGHLicense = async (
  owner: string,
  repo: string
): Promise<string | null> => {
  try {
    const response = await octokitClient().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 {
    // Dependency graph sometimes reports the repository without a scheme
    const normalizedUrl = /^https?:\/\//i.test(url) ? url : `https://${url}`
    const parsed = new URL(normalizedUrl)
    if (parsed.host !== 'github.com') {
      return null
    }
    const components = parsed.pathname.split('/')
    if (components.length < 3 || !components[1] || !components[2]) {
      return null
    }
    return {owner: components[1], repo: components[2]}
  } catch (_) {
    return null
  }
}

const getGitHubRepo = (
  change: Change
): {owner: string; repo: string} | null => {
  // Trust the source repo URL reported by dependency graph
  if (change.source_repository_url !== null) {
    const fromUrl = parseGitHubURL(change.source_repository_url)
    if (fromUrl !== null) {
      return fromUrl
    }
  }

  // otherwise fall back to package name
  if (change.ecosystem === 'actions') {
    const [owner, repo] = change.name.split('/')
    if (owner && repo) {
      return {owner, repo}
    }
  }

  return null
}

const DEPS_DEV_SYSTEMS: Record<string, string> = {
  npm: 'npm',
  pip: 'pypi',
  pypi: 'pypi',
  maven: 'maven',
  go: 'go',
  cargo: 'cargo',
  rust: 'cargo',
  nuget: 'nuget'
}

// Fetch package license from the deps.dev API as last resort
// when neither dependency graph nor GitHub licenses API works
const fetchDepsDevLicense = async (change: Change): Promise<string | null> => {
  const system = DEPS_DEV_SYSTEMS[change.ecosystem.toLowerCase()]
  if (system === undefined || !change.name || !change.version) {
    return null
  }

  try {
    const url = `https://api.deps.dev/v3/systems/${system}/packages/${encodeURIComponent(
      change.name
    )}/versions/${encodeURIComponent(change.version)}`
    const response = await fetch(url)
    if (!response.ok) {
      return null
    }
    const data = (await response.json()) as {licenses?: string[]}
    if (!data.licenses || data.licenses.length === 0) {
      return null
    }
    // packages with more than one entry are multi-licensed
    // so we AND them together to remain conservative
    return data.licenses.join(' AND ')
  } catch (_) {
    return null
  }
}

// Attempts to resolve a license for every change with unknown license
const resolveLicenses = async (changes: Change[]): Promise<Change[]> => {
  const updatedChanges = changes.map(async change => {
    if (change.license !== null) {
      return change
    }

    let license: string | null = null

    const githubRepo = getGitHubRepo(change)
    if (githubRepo !== null) {
      license = await fetchGHLicense(githubRepo.owner, githubRepo.repo)
    }

    if (license === null) {
      license = await fetchDepsDevLicense(change)
    }

    return {...change, license}
  })

  return Promise.all(updatedChanges)
}

// Currently Dependency Graph licenses are truncated to 255 characters
// This possibly makes them invalid spdx ids
const truncatedDGLicense = (license: string): boolean =>
  license.length === 255 && !spdx.isValid(license)

async function groupChanges(
  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(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 =>
          purlsMatch(exclusion, changeAsPackageURL)
        ) !== -1
      ) {
        return false
      } else {
        return true
      }
    })
  }

  const changesToResolve = []

  for (const change of candidateChanges) {
    if (change.change_type === 'removed') {
      continue
    }

    if (change.license === null) {
      // License unknown: attempt to resolve it via the GitHub licenses API
      // and/or deps.dev.
      changesToResolve.push(change)
    } else if (
      truncatedDGLicense(change.license) &&
      getGitHubRepo(change) !== null
    ) {
      changesToResolve.push(change)
    } else {
      result.licensed.push(change)
    }
  }

  if (changesToResolve.length > 0) {
    const resolvedChanges = await resolveLicenses(changesToResolve)
    for (const change of resolvedChanges) {
      if (change.license === null) {
        result.unlicensed.push(change)
      } else {
        result.licensed.push(change)
      }
    }
  }

  return result
}

[ahpook]

Thanks for the report - we are actually in process of improving this on the back end, so the internal license resolution code will effectively do what you're suggesting: consult the package registry API for license information. This should greatly reduce the frequency of these "unknown license" problems.

[jescalada]

@ahpook Good to know, and hope my bug report helped 🙂