Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit f199659

Browse filesBrowse files
committed
Allowing dependencies works with no licenses
When using the `allow-dependencies-licenses` option, the packages listed there should be allowed even if they have no license. This wasn't working because the filtering for allowed dependencies was done specifically on the list of packages that had licenses, leaving a separate list (unfiltered) for packages with no licenses. With this change, we filter out any changes for packages that have been allowed _before_ we retrieve licenses. Fixes #889
1 parent 38ecb5b commit f199659
Copy full SHA for f199659

File tree

2 files changed

+60
-30
lines changed
Filter options

2 files changed

+60
-30
lines changed

‎__tests__/licenses.test.ts

Copy file name to clipboardExpand all lines: __tests__/licenses.test.ts
+25Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,20 @@ const complexLicenseChange: Change = {
100100
]
101101
}
102102

103+
const unlicensedChange: Change = {
104+
change_type: 'added',
105+
manifest: '.github/workflows/ci.yml',
106+
ecosystem: 'actions',
107+
name: 'foo-org/actions-repo/.github/workflows/some-action.yml',
108+
version: '1.1.1',
109+
package_url:
110+
'pkg:githubactions/foo-org/actions-repo/.github/workflows/some-action.yml@1.1.1',
111+
license: null,
112+
source_repository_url: 'github.com/some-repo',
113+
scope: 'development',
114+
vulnerabilities: []
115+
}
116+
103117
jest.mock('@actions/core')
104118

105119
const mockOctokit = {
@@ -313,4 +327,15 @@ describe('GH License API fallback', () => {
313327
expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
314328
expect(unlicensed.length).toEqual(0)
315329
})
330+
331+
test('it does not call licenses API if the package is excluded', async () => {
332+
const {unlicensed} = await getInvalidLicenseChanges([unlicensedChange], {
333+
licenseExclusions: [
334+
'pkg:githubactions/foo-org/actions-repo/.github/workflows/some-action.yml'
335+
]
336+
})
337+
338+
expect(mockOctokit.rest.licenses.getForRepo).not.toHaveBeenCalled()
339+
expect(unlicensed.length).toEqual(0)
340+
})
316341
})

‎src/licenses.ts

Copy file name to clipboardExpand all lines: src/licenses.ts
+35-30Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {Change, Changes} from './schemas'
22
import {octokitClient} from './utils'
3-
import {parsePURL} from './purl'
3+
import {parsePURL, PackageURL} from './purl'
44
import * as spdx from './spdx'
55

66
/**
@@ -36,34 +36,8 @@ export async function getInvalidLicenseChanges(
3636
}
3737
)
3838

39-
const groupedChanges = await groupChanges(changes)
39+
const groupedChanges = await groupChanges(changes, licenseExclusions)
4040

41-
// Takes the changes from the groupedChanges object and filters out the ones that are part of the exclusions list
42-
// It does by creating a new PackageURL object from the change and comparing it to the exclusions list
43-
groupedChanges.licensed = groupedChanges.licensed.filter(change => {
44-
if (change.package_url.length === 0) {
45-
return true
46-
}
47-
48-
const changeAsPackageURL = parsePURL(encodeURI(change.package_url))
49-
50-
// We want to find if the licenseExclusion list contains the PackageURL of the Change
51-
// If it does, we want to filter it out and therefore return false
52-
// If it doesn't, we want to keep it and therefore return true
53-
if (
54-
licenseExclusions !== null &&
55-
licenseExclusions !== undefined &&
56-
licenseExclusions.findIndex(
57-
exclusion =>
58-
exclusion.type === changeAsPackageURL.type &&
59-
exclusion.name === changeAsPackageURL.name
60-
) !== -1
61-
) {
62-
return false
63-
} else {
64-
return true
65-
}
66-
})
6741
const licensedChanges: Changes = groupedChanges.licensed
6842

6943
const invalidLicenseChanges: InvalidLicenseChanges = {
@@ -172,16 +146,47 @@ const truncatedDGLicense = (license: string): boolean =>
172146
license.length === 255 && !spdx.isValid(license)
173147

174148
async function groupChanges(
175-
changes: Changes
149+
changes: Changes,
150+
licenseExclusions: PackageURL[] | null = null
176151
): Promise<Record<string, Changes>> {
177152
const result: Record<string, Changes> = {
178153
licensed: [],
179154
unlicensed: []
180155
}
181156

157+
let candidateChanges = changes
158+
159+
// If a package is excluded from license checking, we don't bother trying to
160+
// fetch the license for it and we leave it off of the `licensed` and
161+
// `unlicensed` lists.
162+
if (licenseExclusions !== null && licenseExclusions !== undefined) {
163+
candidateChanges = candidateChanges.filter(change => {
164+
if (change.package_url.length === 0) {
165+
return true
166+
}
167+
168+
const changeAsPackageURL = parsePURL(encodeURI(change.package_url))
169+
170+
// We want to find if the licenseExclusion list contains the PackageURL of the Change
171+
// If it does, we want to filter it out and therefore return false
172+
// If it doesn't, we want to keep it and therefore return true
173+
if (
174+
licenseExclusions.findIndex(
175+
exclusion =>
176+
exclusion.type === changeAsPackageURL.type &&
177+
exclusion.name === changeAsPackageURL.name
178+
) !== -1
179+
) {
180+
return false
181+
} else {
182+
return true
183+
}
184+
})
185+
}
186+
182187
const ghChanges = []
183188

184-
for (const change of changes) {
189+
for (const change of candidateChanges) {
185190
if (change.change_type === 'removed') {
186191
continue
187192
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.