diff --git a/src/github_app.py b/src/github_app.py index c18b6a9..25e1a4e 100644 --- a/src/github_app.py +++ b/src/github_app.py @@ -4,12 +4,15 @@ from __future__ import annotations import contextlib +import logging import time from typing import Generator import jwt import requests +logger = logging.getLogger(__name__) + class GithubAppToken: def __init__(self, private_key, app_id) -> None: @@ -29,10 +32,15 @@ def get_token(self, installation_id: int) -> Generator[str, None, None]: # This token expires in an hour yield resp["token"] finally: - requests.delete( - "https://api.github.com/installation/token", - headers={"Authorization": f"token {resp['token']}"}, - ) + # Revoking the ephemeral installation token is best-effort cleanup. + # A transient network failure here should not fail webhook handling. + try: + requests.delete( + "https://api.github.com/installation/token", + headers={"Authorization": f"token {resp['token']}"}, + ) + except requests.RequestException: + logger.warning("Failed to revoke GitHub installation token.", exc_info=True) def get_jwt_token(self, private_key, app_id): payload = { diff --git a/tests/test_github_app.py b/tests/test_github_app.py new file mode 100644 index 0000000..d7368f2 --- /dev/null +++ b/tests/test_github_app.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +from unittest.mock import Mock +from unittest.mock import patch + +import requests + +from src.github_app import GithubAppToken + + +def _new_token_manager() -> GithubAppToken: + token_manager = GithubAppToken.__new__(GithubAppToken) + token_manager.headers = {"Authorization": "Bearer fake-app-jwt"} + return token_manager + + +@patch("src.github_app.logger.warning") +@patch("src.github_app.requests.delete") +@patch("src.github_app.requests.post") +def test_get_token_delete_error_is_best_effort( + mock_post, + mock_delete, + mock_warning, +): + mock_resp = Mock() + mock_resp.raise_for_status.return_value = None + mock_resp.json.return_value = {"token": "temporary-installation-token"} + mock_post.return_value = mock_resp + mock_delete.side_effect = requests.exceptions.SSLError("eof during handshake") + + token_manager = _new_token_manager() + with token_manager.get_token(12345) as token: + assert token == "temporary-installation-token" + + mock_delete.assert_called_once_with( + "https://api.github.com/installation/token", + headers={"Authorization": "token temporary-installation-token"}, + ) + mock_warning.assert_called_once() + + +@patch("src.github_app.logger.warning") +@patch("src.github_app.requests.delete") +@patch("src.github_app.requests.post") +def test_get_token_still_revokes_when_delete_succeeds( + mock_post, + mock_delete, + mock_warning, +): + mock_resp = Mock() + mock_resp.raise_for_status.return_value = None + mock_resp.json.return_value = {"token": "temporary-installation-token"} + mock_post.return_value = mock_resp + + token_manager = _new_token_manager() + with token_manager.get_token(12345) as token: + assert token == "temporary-installation-token" + + mock_delete.assert_called_once_with( + "https://api.github.com/installation/token", + headers={"Authorization": "token temporary-installation-token"}, + ) + mock_warning.assert_not_called()