Merge pull request #1056 from actions/juxtin/fix-exclusion-match

Compare normalized purls to account for encoding quirks

__tests__/licenses.test.ts

@@ -253,6 +253,33 @@ test('it does not filter out changes that are on the exclusions list', async ()
   expect(invalidLicenses.forbidden.length).toEqual(0)
 })
 
+test('it excludes scoped npm packages when namespace separator is percent-encoded', async () => {
+  const scopedNpmChange: Change = {
+    manifest: 'package.json',
+    change_type: 'added',
+    ecosystem: 'npm',
+    name: '@lancedb/lancedb',
+    version: '0.14.3',
+    package_url: 'pkg:npm/%40lancedb/lancedb@0.14.3',
+    license: 'Apache-2.0',
+    source_repository_url: 'github.com/lancedb/lancedb',
+    scope: 'runtime',
+    vulnerabilities: []
+  }
+  const changes: Changes = [scopedNpmChange, rubyChange]
+  const licensesConfig = {
+    allow: ['BSD-3-Clause'],
+    // user provides %2F-encoded version
+    licenseExclusions: ['pkg:npm/%40lancedb%2Flancedb']
+  }
+  const invalidLicenses = await getInvalidLicenseChanges(
+    changes,
+    licensesConfig
+  )
+  // scoped package should be excluded, only rubyChange remains (allowed)
+  expect(invalidLicenses.forbidden.length).toEqual(0)
+})
+
 test('it does not fail when the packages dont have a valid PURL', async () => {
   const emptyPurlChange = pipChange
   emptyPurlChange.package_url = ''

__tests__/purl.test.ts

@@ -1,5 +1,5 @@
 import {expect, test} from '@jest/globals'
-import {parsePURL} from '../src/purl'
+import {parsePURL, purlsMatch} from '../src/purl'
 
 test('parsePURL returns an error if the purl does not start with "pkg:"', () => {
   const purl = 'not-a-purl'
@@ -184,3 +184,44 @@ test('parsePURL table test', () => {
     expect(result).toEqual(example.expected)
   }
 })
+
+test('purlsMatch matches identical PURLs', () => {
+  const a = parsePURL('pkg:npm/@scope/name@1.0.0')
+  const b = parsePURL('pkg:npm/@scope/name@2.0.0')
+  expect(purlsMatch(a, b)).toBe(true)
+})
+
+test('purlsMatch matches when namespace separator is percent-encoded', () => {
+  // %2F-encoded separator puts everything in name with no namespace
+  const encoded = parsePURL('pkg:npm/%40lancedb%2Flancedb')
+  // literal / splits into namespace + name
+  const literal = parsePURL('pkg:npm/%40lancedb/lancedb')
+  expect(purlsMatch(encoded, literal)).toBe(true)
+})
+
+test('purlsMatch matches scoped npm packages regardless of encoding', () => {
+  const a = parsePURL('pkg:npm/%40lancedb%2Flancedb')
+  const b = parsePURL('pkg:npm/@lancedb/lancedb')
+  const c = parsePURL('pkg:npm/%40lancedb/lancedb@0.14.3')
+  expect(purlsMatch(a, b)).toBe(true)
+  expect(purlsMatch(a, c)).toBe(true)
+  expect(purlsMatch(b, c)).toBe(true)
+})
+
+test('purlsMatch does not match different packages', () => {
+  const a = parsePURL('pkg:npm/@scope/foo')
+  const b = parsePURL('pkg:npm/@scope/bar')
+  expect(purlsMatch(a, b)).toBe(false)
+})
+
+test('purlsMatch does not match different types', () => {
+  const a = parsePURL('pkg:npm/@scope/name')
+  const b = parsePURL('pkg:pypi/@scope/name')
+  expect(purlsMatch(a, b)).toBe(false)
+})
+
+test('purlsMatch matches packages without namespaces', () => {
+  const a = parsePURL('pkg:npm/lodash@4.0.0')
+  const b = parsePURL('pkg:npm/lodash@5.0.0')
+  expect(purlsMatch(a, b)).toBe(true)
+})

src/licenses.ts

@@ -1,6 +1,6 @@
 import {Change, Changes} from './schemas'
 import {octokitClient} from './utils'
-import {parsePURL, PackageURL} from './purl'
+import {parsePURL, PackageURL, purlsMatch} from './purl'
 import * as spdx from './spdx'
 
 /**
@@ -180,11 +180,8 @@ async function groupChanges(
       // 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
+        licenseExclusions.findIndex(exclusion =>
+          purlsMatch(exclusion, changeAsPackageURL)
         ) !== -1
       ) {
         return false

src/purl.ts

@@ -70,3 +70,25 @@ export function parsePURL(purl: string): PackageURL {
   // we don't parse subpath or attributes, so we're done here
   return result
 }
+
+// Returns the full name of a package, combining namespace and name.
+// This normalizes PURLs where the namespace separator '/' may have been
+// percent-encoded as '%2F', causing it to be parsed as part of the name
+// rather than splitting namespace and name.
+function fullName(purl: PackageURL): string | null {
+  if (purl.namespace && purl.name) {
+    return `${purl.namespace}/${purl.name}`
+  }
+  return purl.name ?? purl.namespace
+}
+
+// Compare two PackageURLs for equality, ignoring version and normalizing
+// namespace/name splits. This handles the case where a PURL like
+// 'pkg:npm/%40scope%2Fname' is parsed as {namespace: null, name: '@scope/name'}
+// while 'pkg:npm/%40scope/name' is parsed as {namespace: '@scope', name: 'name'}.
+export function purlsMatch(a: PackageURL, b: PackageURL): boolean {
+  if (a.type !== b.type) {
+    return false
+  }
+  return fullName(a) === fullName(b)
+}