fix: Limit webhook-event-check to known events in `/app` response (#1092)

Because `GET /app` does not include many events (e.g. `fork`, `installation`, etc.), we don't want to compare some events to the results of `GET /app`.

In these cases, webhook-event-check will silently ignore verifying whether the GitHub App is subscribed to the event.
This commit is contained in:
Maxim Devoir 2019-12-25 01:37:06 -08:00 committed by Gregor Martynus
parent f368013729
commit 28a4d43185
3 changed files with 66 additions and 9 deletions

View File

@ -20,7 +20,8 @@ async function webhookEventCheck (app: Application, eventName: string) {
if (await isSubscribedToEvent(app, baseEventName)) {
return true
} else if (didFailRetrievingAppMeta === false) {
app.log.error(`Your app is attempting to listen to "${eventName}", but your GitHub App is not subscribed to the "${baseEventName}" event.`)
const userFriendlyBaseEventName = baseEventName.split('_').join(' ')
app.log.error(`Your app is attempting to listen to "${eventName}", but your GitHub App is not subscribed to the "${userFriendlyBaseEventName}" event.`)
}
return didFailRetrievingAppMeta ? undefined : false
}
@ -28,12 +29,63 @@ async function webhookEventCheck (app: Application, eventName: string) {
/**
* @param {string} baseEventName The base event name refers to the part before
* the first period mark (e.g. the `issues` part in `issues.opened`).
* @returns Returns `true` when the application is subscribed to a webhook
* event. Otherwise, returns `false`. Returns `undefined` if Probot failed to
* @returns Returns `false` when the application is not subscribed to a webhook
* event. Otherwise, returns `true`. Returns `undefined` if Probot failed to
* retrieve GitHub App metadata.
*
* **Note**: Probot will only check against a list of events known to be in the
* `GET /app` response. Therefore, only the `false` value should be considered
* truthy.
*/
async function isSubscribedToEvent (app: Application, baseEventName: string) {
if (baseEventName === '*') {
// A list of events known to be in the response of `GET /app`. This list can
// be retrieved by calling `GET /app` from an authenticated app that has
// maximum permissions and is subscribed to all available webhook events.
const knownBaseEvents = [
'check_run',
'check_suite',
'commit_comment',
'content_reference',
'create',
'delete',
'deployment',
'deployment_status',
'deploy_key',
'fork',
'gollum',
'issues',
'issue_comment',
'label',
'member',
'membership',
'milestone',
'organization',
'org_block',
'page_build',
'project',
'project_card',
'project_column',
'public',
'pull_request',
'pull_request_review',
'pull_request_review_comment',
'push',
'release',
'repository',
'repository_dispatch',
'star',
'status',
'team',
'team_add',
'watch'
]
// Because `GET /app` does not include all events - such as default events
// that all GitHub Apps are subscribed to (e.g.`installation`, `meta`, or
// `marketplace_purchase`) - we can only check `baseEventName` if it is known
// to be in the `GET /app` response.
const eventMayExistInAppResponse = knownBaseEvents.includes(baseEventName)
if (!eventMayExistInAppResponse) {
return true
}

View File

@ -20,7 +20,7 @@ exports[`webhook-event-check warn user when listening to unsubscribed event 1`]
[MockFunction] {
"calls": Array [
Array [
"Your app is attempting to listen to \\"an-unsubscribed-event.action\\", but your GitHub App is not subscribed to the \\"an-unsubscribed-event\\" event.",
"Your app is attempting to listen to \\"pull_request.opened\\", but your GitHub App is not subscribed to the \\"pull request\\" event.",
],
],
"results": Array [

View File

@ -3,6 +3,12 @@ import { Application } from '../src'
import eventCheck, { clearCache } from '../src/webhook-event-check'
import { newApp } from './apps/helper'
/**
* Returns a mocked request for `/meta` with the subscribed `events`.
*
* By default, the mocked payload indicates the a GitHub App is subscribed to
* the `issues` event.
*/
function mockAppMetaRequest (events: string[] = ['issues']) {
return { events }
}
@ -10,7 +16,7 @@ function mockAppMetaRequest (events: string[] = ['issues']) {
describe('webhook-event-check', () => {
let app: Application
let originalNodeEnv: string
const unsubscribedEventName = 'an-unsubscribed-event.action'
const unsubscribedEventName = 'label.created'
beforeAll(() => {
originalNodeEnv = process.env.NODE_ENV || 'test'
@ -39,7 +45,7 @@ describe('webhook-event-check', () => {
expect(spyOnLogError).toMatchSnapshot()
})
test('succeeds when event name is wildcard character', async () => {
test('returns undefined for that will never be in the payload of /meta', async () => {
nock('https://api.github.com')
.defaultReplyHeaders({ 'Content-Type': 'application/json' })
.get('/app').reply(200, mockAppMetaRequest([]))
@ -54,7 +60,7 @@ describe('webhook-event-check', () => {
.get('/app').reply(200, mockAppMetaRequest())
app = newApp()
const spyOnLogError = jest.spyOn(app.log, 'error')
expect(await eventCheck(app, unsubscribedEventName)).toStrictEqual(false)
expect(await eventCheck(app, 'pull_request.opened')).toStrictEqual(false)
expect(spyOnLogError).toHaveBeenCalledTimes(1)
expect(spyOnLogError).toMatchSnapshot()
})
@ -66,7 +72,6 @@ describe('webhook-event-check', () => {
app = newApp()
const spyOnLogWarn = jest.spyOn(app.log, 'warn')
expect(await eventCheck(app, unsubscribedEventName)).toBeUndefined()
expect(await eventCheck(app, `another-${unsubscribedEventName}`)).toBeUndefined()
expect(spyOnLogWarn).toHaveBeenCalledTimes(1)
expect(spyOnLogWarn).toMatchSnapshot()
})