Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions __tests__/functions/prechecks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,134 @@ for (const checks of ['all', 'required'] as const) {
}
}

for (const {description, checks, ignoredChecks, required, isRequired} of [
{
description: 'an ignored check',
checks: 'all',
ignoredChecks: ['excluded-check'],
required: false,
isRequired: true
},
{
description: 'a check outside an explicit list',
checks: ['required-check'],
ignoredChecks: [],
required: false,
isRequired: true
},
{
description: 'an optional check in required mode',
checks: 'required',
ignoredChecks: [],
required: true,
isRequired: false
}
] as const) {
test(`ignores ambiguous GitHub App identities for ${description}`, () => {
const healthyCheck = {
checkSuite: {app: {databaseId: 1}},
conclusion: 'SUCCESS',
databaseId: 12,
id: 'healthy',
isRequired: true,
name: 'required-check'
}
const excludedCheck = {
checkSuite: {app: null},
conclusion: 'FAILURE',
databaseId: 10,
id: 'old',
isRequired,
name: 'excluded-check'
}
const rerun = {
...excludedCheck,
conclusion: 'SUCCESS',
databaseId: 11,
id: 'new'
}

assert.deepStrictEqual(
filterChecks(
checks,
[healthyCheck, excludedCheck, rerun],
ignoredChecks,
required
),
{message: 'all checks passed', status: 'SUCCESS'}
)
})
}

for (const [description, checks] of [
['an explicit check list', ['required-check']],
['an empty check list', []]
] as const) {
test(`rejects ambiguous GitHub App identities selected by ${description}`, () => {
const check = {
checkSuite: {app: null},
conclusion: 'FAILURE',
databaseId: 10,
id: 'old',
isRequired: true,
name: 'required-check'
}

assert.throws(
() =>
filterChecks(
checks,
[check, {...check, conclusion: 'SUCCESS', databaseId: 11, id: 'new'}],
[],
false
),
{
message:
'A duplicate check result is missing its integration identity: check:null:required-check'
}
)
})
}

for (const [olderRequired, newerRequired] of [
[false, true],
[true, false]
] as const) {
test(`rejects ambiguous check identities when either duplicate is required (${String(olderRequired)}, ${String(newerRequired)})`, () => {
const check = {
checkSuite: {app: null},
conclusion: 'FAILURE',
databaseId: 10,
id: 'old',
isRequired: olderRequired,
name: 'required-check'
}

assert.throws(
() =>
filterChecks(
'required',
[
check,
{
...check,
conclusion: 'SUCCESS',
databaseId: 11,
id: 'new',
isRequired: newerRequired
}
],
[],
true
),
{
message:
'A duplicate check result is missing its integration identity: check:null:required-check'
}
)
})
}

test('rejects malformed required-check metadata', () => {
assert.throws(
() =>
Expand Down
15 changes: 12 additions & 3 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

17 changes: 14 additions & 3 deletions src/functions/prechecks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,15 @@ export function filterChecks(
required: boolean
): {message: string; status: 'FAILURE' | 'MISSING' | 'PENDING' | 'SUCCESS'} {
const healthyCheckStatuses = ['SUCCESS', 'SKIPPED', 'NEUTRAL']
checkResults = latestCheckResults(checkResults)
checkResults = latestCheckResults(checkResults, check => {
const name = checkName(check)
const included =
typeof checks === 'string' ||
checks.length === 0 ||
checks.some(checkName => checkName === name)
const ignored = ignoredChecks.some(ignoredCheck => ignoredCheck === name)
return included && !ignored && (!required || check.isRequired)
})

const checksDisplay = typeof checks === 'string' ? checks : checks.join(',')
core.debug(`filterChecks() - checks: ${checksDisplay}`)
Expand Down Expand Up @@ -974,7 +982,8 @@ function checkNodeId(check: RawCheckResult): string | undefined {
}

export function latestCheckResults(
checkResults: readonly RawCheckResult[]
checkResults: readonly RawCheckResult[],
participatesInPolicy: (check: RawCheckResult) => boolean
): readonly RawCheckResult[] {
const latest = new Map<
string,
Expand All @@ -1001,7 +1010,9 @@ export function latestCheckResults(

if (
currentEntry.checkRun &&
(currentEntry.integrationId === null || candidate.integrationId === null)
(currentEntry.integrationId === null ||
candidate.integrationId === null) &&
(participatesInPolicy(current) || participatesInPolicy(check))
) {
throw new Error(
`A duplicate check result is missing its integration identity: ${identity}`
Expand Down
65 changes: 64 additions & 1 deletion tools/acceptance/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2874,10 +2874,73 @@ const scenarios = [
assertOutput(context, result, 'commit_status', 'SUCCESS')
})
},
{
name: 'policy-excluded ambiguous GitHub App identities allow deployment',
run: async () => {
for (const {description, inputs, isRequired} of [
{
description: 'ignored check',
inputs: {ignored_checks: 'excluded-check'},
isRequired: true
},
{
description: 'check outside an explicit list',
inputs: {checks: 'required-check'},
isRequired: true
},
{
description: 'optional check in required mode',
inputs: {checks: 'required'},
isRequired: false
}
] as const) {
await withMockGitHub(
`ambiguous GitHub App identity is ignored for ${description}`,
async context => {
setTriggerComment(context.state, '.deploy')
context.state.rollupState = 'FAILURE'
context.state.rollupContexts = [
{
conclusion: 'SUCCESS',
databaseId: 12,
integrationId: 1,
isRequired: true,
name: 'required-check',
type: 'check-run'
},
{
conclusion: 'FAILURE',
databaseId: 10,
integrationId: null,
isRequired,
name: 'excluded-check',
type: 'check-run'
},
{
conclusion: 'SUCCESS',
databaseId: 11,
integrationId: null,
isRequired,
name: 'excluded-check',
type: 'check-run'
}
]

const result = await runMain(context, inputs)

assertExit(context, result, 0)
assertReason(context, result, 'deployment_ready')
assertOutput(context, result, 'commit_status', 'SUCCESS')
requireDeployment(context)
}
)
}
}
},
{
name: 'ambiguous GitHub App identity fails closed for all check modes',
run: async () => {
for (const checks of ['all', 'required'] as const) {
for (const checks of ['all', 'required', 'required-check'] as const) {
await withMockGitHub(
`ambiguous GitHub App identity fails closed for ${checks} checks`,
async context => {
Expand Down
Loading