Skip to content

Commit

Permalink
Fixing a bug with GHSA filtering.
Browse files Browse the repository at this point in the history
Co-authored-by: Christine Nagadya <cnagadya@github.com>
  • Loading branch information
febuiles and cnagadya committed Oct 11, 2022
1 parent 1d9bfbb commit 2dd6c6a
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
18 changes: 12 additions & 6 deletions __tests__/filter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {Change, Changes} from '../src/schemas'
import {
filterChangesBySeverity,
filterChangesByScopes,
filterOutAllowedAdvisories
filterAllowedAdvisories
} from '../src/filter'

let npmChange: Change = {
Expand Down Expand Up @@ -90,28 +90,34 @@ test('it properly filters changes by scope', async () => {
expect(result).toEqual([npmChange, rubyChange])
})

test('it properly handles undefined advisory IDs', async () => {
const changes = [npmChange, rubyChange, noVulnNpmChange]
let result = filterAllowedAdvisories(undefined, changes)
expect(result).toEqual([npmChange, rubyChange, noVulnNpmChange])
})

test('it properly filters changes with allowed vulnerabilities', async () => {
const changes = [npmChange, rubyChange, noVulnNpmChange]

let result = filterOutAllowedAdvisories(['notrealGHSAID'], changes)
let result = filterAllowedAdvisories(['notrealGHSAID'], changes)
expect(result).toEqual([npmChange, rubyChange, noVulnNpmChange])

result = filterOutAllowedAdvisories(['first-random_string'], changes)
result = filterAllowedAdvisories(['first-random_string'], changes)
expect(result).toEqual([rubyChange, noVulnNpmChange])

result = filterOutAllowedAdvisories(
result = filterAllowedAdvisories(
['second-random_string', 'third-random_string'],
changes
)
expect(result).toEqual([npmChange, noVulnNpmChange])

result = filterOutAllowedAdvisories(
result = filterAllowedAdvisories(
['first-random_string', 'second-random_string', 'third-random_string'],
changes
)
expect(result).toEqual([noVulnNpmChange])

// if we have a change with multiple vulnerabilities but only one is allowed, we still should not filter out that change
result = filterOutAllowedAdvisories(['second-random_string'], changes)
result = filterAllowedAdvisories(['second-random_string'], changes)
expect(result).toEqual([npmChange, rubyChange, noVulnNpmChange])
})
7 changes: 5 additions & 2 deletions src/filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,19 @@ export function filterChangesByScopes(
}

/**
* Filter out changes that are allowed by the allow_ghsas config
* option. We want to remove these changes before we do any
* processing.
* @param ghsas - list of GHSA IDs to allow
* @param changes - list of changes to filter
* @returns a list of changes with the allowed GHSAs removed
*/
export function removeAllowedAdvisories(
export function filterAllowedAdvisories(
ghsas: string[] | undefined,
changes: Changes
): Changes {
if (ghsas === undefined) {
return []
return changes
}

const filteredChanges = changes.filter(change => {
Expand Down
4 changes: 2 additions & 2 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {readConfig} from '../src/config'
import {
filterChangesBySeverity,
filterChangesByScopes,
removeAllowedAdvisories
filterAllowedAdvisories
} from '../src/filter'
import {getDeniedLicenseChanges} from './licenses'
import * as summary from './summary'
Expand All @@ -30,7 +30,7 @@ async function run(): Promise<void> {

const minSeverity = config.fail_on_severity as Severity
const scopedChanges = filterChangesByScopes(config.fail_on_scopes, changes)
const filteredChanges = removeAllowedAdvisories(
const filteredChanges = filterAllowedAdvisories(
config.allow_ghsas,
scopedChanges
)
Expand Down

0 comments on commit 2dd6c6a

Please sign in to comment.