diff --git a/src/github_app.py b/src/github_app.py index c18b6a9..a430548 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,19 @@ 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']}"}, - ) + # Token revocation is best-effort; cleanup failures should not hide + # the original exception from webhook processing. + try: + requests.delete( + "https://api.github.com/installation/token", + headers={"Authorization": f"token {resp['token']}"}, + timeout=5, + ) + except requests.RequestException: + logger.warning( + "Failed to revoke GitHub installation token during cleanup.", + 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..976c1fb --- /dev/null +++ b/tests/test_github_app.py @@ -0,0 +1,71 @@ +from __future__ import annotations + +from unittest.mock import Mock +from unittest.mock import patch + +import pytest +import requests + +from src.github_app import GithubAppToken + + +def _new_client(): + client = GithubAppToken.__new__(GithubAppToken) + client.headers = {"Authorization": "Bearer jwt-token"} + return client + + +def test_get_token_revokes_installation_token(): + client = _new_client() + token_response = Mock() + token_response.raise_for_status.return_value = None + token_response.json.return_value = {"token": "installation-token"} + + with patch("src.github_app.requests.post", return_value=token_response) as post_mock: + with patch("src.github_app.requests.delete") as delete_mock: + with client.get_token(12345) as token: + assert token == "installation-token" + + post_mock.assert_called_once_with( + url="https://api.github.com/app/installations/12345/access_tokens", + headers=client.headers, + ) + delete_mock.assert_called_once_with( + "https://api.github.com/installation/token", + headers={"Authorization": "token installation-token"}, + timeout=5, + ) + + +def test_get_token_cleanup_network_error_is_non_fatal(): + client = _new_client() + token_response = Mock() + token_response.raise_for_status.return_value = None + token_response.json.return_value = {"token": "installation-token"} + + with patch("src.github_app.requests.post", return_value=token_response): + with patch( + "src.github_app.requests.delete", + side_effect=requests.ConnectionError("cleanup failed"), + ): + with patch("src.github_app.logger.warning") as warning_mock: + with client.get_token(12345) as token: + assert token == "installation-token" + + warning_mock.assert_called_once() + + +def test_get_token_cleanup_error_does_not_mask_inner_exception(): + client = _new_client() + token_response = Mock() + token_response.raise_for_status.return_value = None + token_response.json.return_value = {"token": "installation-token"} + + with patch("src.github_app.requests.post", return_value=token_response): + with patch( + "src.github_app.requests.delete", + side_effect=requests.ConnectionError("cleanup failed"), + ): + with pytest.raises(ValueError, match="primary failure"): + with client.get_token(12345): + raise ValueError("primary failure")