Merge pull request #5 from QuLogic/safer-repo-update

Verify git checkout path is in site directory

Author: QuLogic

test_webhook.py

@@ -65,8 +65,9 @@ async def test_update_repo(tmp_path_factory):
     assert dest_commit == src_commit
 
 
-async def test_github_webhook_errors(aiohttp_client, monkeypatch):
+async def test_github_webhook_errors(aiohttp_client, monkeypatch, tmp_path):
     """Test invalid inputs to webhook."""
+    monkeypatch.setenv('SITE_DIR', str(tmp_path))
     client = await aiohttp_client(create_app())
 
     # Only /gh/<repo-name> exists.
@@ -121,6 +122,14 @@ async def test_github_webhook_errors(aiohttp_client, monkeypatch):
     assert resp.status == 400
     assert 'incorrect repository' in await resp.text()
 
+    # Fields provided, but invalid.
+    resp = await client.post(
+        '/gh/non-existent-repo', headers=valid_headers,
+        data='{"sender": {"login": "QuLogic"}, "repository":'
+             ' {"name": "..", "owner": {"login": "matplotlib"}}}')
+    assert resp.status == 400
+    assert 'incorrect repository' in await resp.text()
+
     # Problem on our side.
     resp = await client.post(
         '/gh/non-existent',
@@ -134,12 +143,12 @@ async def test_github_webhook_errors(aiohttp_client, monkeypatch):
 
 async def test_github_webhook_valid(aiohttp_client, monkeypatch, tmp_path):
     """Test valid input to webhook."""
+    monkeypatch.setenv('SITE_DIR', str(tmp_path))
     client = await aiohttp_client(create_app())
 
     # Do no actual work, since that's tested above.
     monkeypatch.setattr(webhook, 'verify_signature',
                         mock.Mock(verify_signature, return_value=True))
-    monkeypatch.setenv('SITE_DIR', str(tmp_path))
     ur_mock = mock.Mock(update_repo, return_value=None)
     monkeypatch.setattr(webhook, 'update_repo', ur_mock)
 

webhook.py

@@ -156,7 +156,11 @@ async def github_webhook(request: web.Request):
                  delivery, ref, expected_branch)
         return web.Response(status=200)
 
-    checkout = Path(os.environ.get('SITE_DIR', 'sites'), repository)
+    checkout = (request.app['site_dir'] / repository).resolve()
+    if not checkout.is_relative_to(request.app['site_dir']):
+        raise web.HTTPBadRequest(
+            reason=(f'{delivery}: Checkout for {organization}/{repository} '
+                    'does not exist'))
     if not (checkout / '.git').is_dir():
         raise web.HTTPInternalServerError(
             reason=(f'{delivery}: Checkout for {organization}/{repository} '
@@ -171,16 +175,18 @@ async def github_webhook(request: web.Request):
 
 def create_app():
     """Create the aiohttp app and setup routes."""
+    site_dir = Path(os.environ.get('SITE_DIR', 'sites')).resolve()
+    assert site_dir.is_dir()
+
     app = web.Application()
+    app['site_dir'] = site_dir
     app.add_routes([
         web.post('/gh/{repo}', github_webhook),
     ])
     return app
 
 
 if __name__ == '__main__':
-    assert Path(os.environ.get('SITE_DIR', 'sites')).is_dir()
-
     if len(sys.argv) > 1:
         from urllib.parse import urlparse
         url = sys.argv[1]