Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse purls cautiously in getDeniedChanges #753

Merged
merged 7 commits into from Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions __tests__/config.test.ts
Expand Up @@ -54,6 +54,20 @@ test('it raises an error if an empty allow list is specified', async () => {
)
})

test('it raises an error when an invalid package-url is used for deny-packages', async () => {
setInput('deny-packages', 'not-a-purl')

await expect(readConfig()).rejects.toThrow(`Error parsing package-url`)
})

test('it raises an error when an argument to deny-groups is missing a namespace', async () => {
setInput('deny-groups', 'pkg:npm/my-fun-org')

await expect(readConfig()).rejects.toThrow(
`package-url must have a namespace`
)
})

test('it raises an error when given an unknown severity', async () => {
setInput('fail-on-severity', 'zombies')

Expand Down
19 changes: 19 additions & 0 deletions __tests__/deny.test.ts
Expand Up @@ -106,6 +106,25 @@ test('denies packages that match the deny group list exactly', async () => {
expect(deniedChanges[0]).toBe(changes[1])
})

test(`denies packages using the namespace from the name when there's no package_url`, async () => {
const changes: Changes = [
createTestChange({
package_url: 'pkg:npm/org.test.pass/pass-this@1.0.0',
ecosystem: 'npm'
}),
createTestChange({
name: 'org.test:deny-this',
package_url: '',
ecosystem: 'maven'
})
bteng22 marked this conversation as resolved.
Show resolved Hide resolved
]
const deniedGroups = createTestPURLs(['pkg:maven/org.test/'])
const deniedChanges = await getDeniedChanges(changes, [], deniedGroups)

expect(deniedChanges.length).toEqual(1)
expect(deniedChanges[0]).toBe(changes[1])
})

test('allows packages not defined in the deny packages and groups list', async () => {
const changes: Changes = [npmChange, pipChange]
const deniedPackages = createTestPURLs([
Expand Down
3 changes: 1 addition & 2 deletions __tests__/fixtures/create-test-change.ts
@@ -1,7 +1,6 @@
import {PackageURL} from 'packageurl-js'
import {Change} from '../../src/schemas'
import {createTestVulnerability} from './create-test-vulnerability'
import {parsePURL} from '../../src/utils'
import {PackageURL, parsePURL} from '../../src/purl'

const defaultNpmChange: Change = {
change_type: 'added',
Expand Down
162 changes: 162 additions & 0 deletions __tests__/purl.test.ts
@@ -0,0 +1,162 @@
import {expect, test} from '@jest/globals'
import {parsePURL} from '../src/purl'

test('parsePURL returns an error if the purl does not start with "pkg:"', () => {
const purl = 'not-a-purl'
const result = parsePURL(purl)
expect(result.error).toEqual('package-url must start with "pkg:"')
})

test('parsePURL returns an error if the purl does not contain a type', () => {
const purl = 'pkg:/'
const result = parsePURL(purl)
expect(result.error).toEqual('package-url must contain a type')
})

test('parsePURL returns an error if the purl does not contain a namespace or name', () => {
const purl = 'pkg:ecosystem/'
const result = parsePURL(purl)
expect(result.type).toEqual('ecosystem')
expect(result.error).toEqual('package-url must contain a namespace or name')
})

test('parsePURL returns a PURL with the correct values in the happy case', () => {
const purl = 'pkg:ecosystem/namespace/name@version'
const result = parsePURL(purl)
expect(result.type).toEqual('ecosystem')
expect(result.namespace).toEqual('namespace')
expect(result.name).toEqual('name')
expect(result.version).toEqual('version')
expect(result.original).toEqual(purl)
expect(result.error).toBeNull()
})

test('parsePURL table test', () => {
const examples = [
{
purl: 'pkg:npm/@n4m3SPACE/Name@^1.2.3',
expected: {
type: 'npm',
namespace: '@n4m3SPACE',
name: 'Name',
version: '^1.2.3',
original: 'pkg:npm/@n4m3SPACE/Name@^1.2.3',
error: null
}
},
{
purl: 'pkg:npm/%40ns%20foo/n%40me@1.%2f2.3',
expected: {
type: 'npm',
namespace: '@ns foo',
name: 'n@me',
version: '1./2.3',
bteng22 marked this conversation as resolved.
Show resolved Hide resolved
original: 'pkg:npm/%40ns%20foo/n%40me@1.%2f2.3',
error: null
}
},
{
purl: 'pkg:ecosystem/name@version',
expected: {
type: 'ecosystem',
namespace: null,
name: 'name',
version: 'version',
original: 'pkg:ecosystem/name@version',
error: null
}
},
{
purl: 'pkg:npm/namespace/',
expected: {
type: 'npm',
namespace: 'namespace',
name: null,
version: null,
original: 'pkg:npm/namespace/',
error: null
}
},
{
purl: 'pkg:ecosystem/name',
expected: {
type: 'ecosystem',
namespace: null,
name: 'name',
version: null,
original: 'pkg:ecosystem/name',
error: null
}
},
{
purl: 'pkg:/?',
expected: {
type: '',
namespace: null,
name: null,
version: null,
original: 'pkg:/?',
error: 'package-url must contain a type'
}
},
{
purl: 'pkg:ecosystem/#',
expected: {
type: 'ecosystem',
namespace: null,
name: null,
version: null,
original: 'pkg:ecosystem/#',
error: 'package-url must contain a namespace or name'
}
},
{
purl: 'pkg:ecosystem/name@version#subpath?attributes=123',
expected: {
type: 'ecosystem',
namespace: null,
name: 'name',
version: 'version',
original: 'pkg:ecosystem/name@version#subpath?attributes=123',
error: null
}
},
{
purl: 'pkg:ecosystem/name@version#subpath',
expected: {
type: 'ecosystem',
namespace: null,
name: 'name',
version: 'version',
original: 'pkg:ecosystem/name@version#subpath',
error: null
}
},
{
purl: 'pkg:ecosystem/namespace/name@version?attributes',
expected: {
type: 'ecosystem',
namespace: 'namespace',
name: 'name',
version: 'version',
original: 'pkg:ecosystem/namespace/name@version?attributes',
error: null
}
},
{
purl: 'pkg:ecosystem/name#subpath?attributes',
expected: {
type: 'ecosystem',
namespace: null,
name: 'name',
version: null,
original: 'pkg:ecosystem/name#subpath?attributes',
error: null
}
}
]
for (const example of examples) {
const result = parsePURL(example.purl)
expect(result).toEqual(example.expected)
}
})
4 changes: 3 additions & 1 deletion __tests__/test-helpers.ts
Expand Up @@ -19,7 +19,9 @@ export function clearInputs(): void {
'BASE-REF',
'HEAD-REF',
'COMMENT-SUMMARY-IN-PR',
'WARN-ONLY'
'WARN-ONLY',
'DENY-GROUPS',
'DENY-PACKAGES'
]

// eslint-disable-next-line github/array-foreach
Expand Down
2 changes: 1 addition & 1 deletion action.yml
Expand Up @@ -59,7 +59,7 @@ inputs:
description: A comma-separated list of package URLs to deny (e.g. "pkg:npm/express, pkg:pypi/pycrypto"). If version specified, only deny matching packages and version; else, deny all regardless of version.
required: false
deny-groups:
description: A comma-separated list of package URLs for group(s)/namespace(s) to deny (e.g. "pkg:npm/express, pkg:pypi/pycrypto")
description: A comma-separated list of package URLs for group(s)/namespace(s) to deny (e.g. "pkg:npm/express/, pkg:pypi/pycrypto/"). Please note that the group name must be followed by a `/`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

required: false
retry-on-snapshot-warnings:
description: Whether to retry on snapshot warnings
Expand Down