Fix event parsing for ping

Although GitHub docs say 'action' is a common field, it's not _required_
for the ping event. We need to parse the event action from the request
headers instead.

Author: QuLogic

test_webhook.py

@@ -105,35 +105,35 @@ async def test_github_webhook_errors(aiohttp_client, monkeypatch):
     monkeypatch.setattr(webhook, 'verify_signature',
                         mock.Mock(verify_signature, return_value=True))
 
+    valid_headers = {
+        'X-GitHub-Delivery': 'foo',
+        'X-Hub-Signature-256': 'unused',
+        'X-GitHub-Event': 'ping',
+    }
+
     # Data should be JSON.
-    resp = await client.post(
-        '/gh/non-existent-repo',
-        headers={'X-GitHub-Delivery': 'foo', 'X-Hub-Signature-256': 'unused'},
-        data='}{')
+    resp = await client.post('/gh/non-existent-repo', headers=valid_headers,
+                             data='}{')
     assert resp.status == 400
     assert 'Invalid data input' in await resp.text()
 
     # Some data fields are required.
-    resp = await client.post(
-        '/gh/non-existent-repo',
-        headers={'X-GitHub-Delivery': 'foo', 'X-Hub-Signature-256': 'unused'},
-        data='{}')
+    resp = await client.post('/gh/non-existent-repo', headers=valid_headers,
+                             data='{}')
     assert resp.status == 400
     assert 'Missing required fields' in await resp.text()
 
     resp = await client.post(
-        '/gh/non-existent-repo',
-        headers={'X-GitHub-Delivery': 'foo', 'X-Hub-Signature-256': 'unused'},
-        data='{"action": "ping", "sender": "QuLogic", "organization": "foo",'
+        '/gh/non-existent-repo', headers=valid_headers,
+        data='{"sender": "QuLogic", "organization": "foo",'
              ' "repository": "foo"}')
     assert resp.status == 400
     assert 'incorrect organization' in await resp.text()
 
     resp = await client.post(
-        '/gh/non-existent-repo',
-        headers={'X-GitHub-Delivery': 'foo', 'X-Hub-Signature-256': 'unused'},
-        data='{"action": "ping", "sender": "QuLogic", '
-             '"organization": "matplotlib", "repository": "foo"}')
+        '/gh/non-existent-repo', headers=valid_headers,
+        data='{"sender": "QuLogic", "organization": "matplotlib",'
+             ' "repository": "foo"}')
     assert resp.status == 400
     assert 'incorrect repository' in await resp.text()
 
@@ -149,11 +149,16 @@ async def test_github_webhook_valid(aiohttp_client, monkeypatch):
     ur_mock = mock.Mock(update_repo, return_value=None)
     monkeypatch.setattr(webhook, 'update_repo', ur_mock)
 
+    valid_headers = {
+        'X-GitHub-Delivery': 'foo',
+        'X-Hub-Signature-256': 'unused',
+    }
+
     # Ping event just returns success.
     resp = await client.post(
         '/gh/non-existent-repo',
-        headers={'X-GitHub-Delivery': 'foo', 'X-Hub-Signature-256': 'unused'},
-        data='{"action": "ping", "sender": "QuLogic", "hook_id": "foo",'
+        headers={**valid_headers, 'X-GitHub-Event': 'ping'},
+        data='{"sender": "QuLogic", "hook_id": 1234,'
              ' "zen": "Beautiful is better than ugly.",'
              ' "organization": "matplotlib",'
              ' "repository": "non-existent-repo"}')
@@ -163,9 +168,8 @@ async def test_github_webhook_valid(aiohttp_client, monkeypatch):
     # Push event should run an update.
     resp = await client.post(
         '/gh/non-existent-repo',
-        headers={'X-GitHub-Delivery': 'foo', 'X-Hub-Signature-256': 'unused'},
-        data='{"action": "push", "sender": "QuLogic",'
-             ' "organization": "matplotlib",'
+        headers={**valid_headers, 'X-GitHub-Event': 'push'},
+        data='{"sender": "QuLogic", "organization": "matplotlib",'
              ' "repository": "non-existent-repo"}')
     assert resp.status == 200
     ur_mock.assert_called_once_with(

webhook.py

@@ -83,6 +83,10 @@ async def github_webhook(request: web.Request):
         signature = request.headers['X-Hub-Signature-256']
     except KeyError:
         raise web.HTTPBadRequest(reason='No signature given')
+    try:
+        event = request.headers['X-GitHub-Event']
+    except KeyError:
+        raise web.HTTPBadRequest(reason='No event given')
     repo = request.match_info.get('repo')
     log.info('%s: Received webhook for %s', delivery, repo)
 
@@ -96,26 +100,25 @@ async def github_webhook(request: web.Request):
 
     # https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#webhook-payload-object-common-properties
     try:
-        action = data['action']
         sender = data['sender']
         organization = data['organization']
         repository = data['repository']
     except KeyError:
         raise web.HTTPBadRequest(reason='Missing required fields')
     log.info('%s: Received %s event from %s on %s/%s',
-             delivery, action, sender, organization, repository)
+             delivery, event, sender, organization, repository)
 
     if organization != 'matplotlib':
         raise web.HTTPBadRequest(reason=f'{delivery}: incorrect organization')
     if repository != repo:
         raise web.HTTPBadRequest(reason=f'{delivery}: incorrect repository')
 
-    if action == 'ping':
+    if event == 'ping':
         log.info('%s: Ping %s: %s', delivery, data['hook_id'], data['zen'])
         return web.Response(status=200)
 
-    if action != 'push':
-        log.info('%s: Ignoring webhook for unused action %s', delivery, action)
+    if event != 'push':
+        log.info('%s: Ignoring webhook for unused event %s', delivery, event)
         return web.Response(status=200)
 
     checkout = Path(os.environ.get('SITE_DIR', 'sites'), repository)