Handle GitHub token revocation timeouts

src/github_app.py

@@ -4,12 +4,16 @@
 from __future__ import annotations
 
 import contextlib
+import logging
 import time
 from typing import Generator
 
 import jwt
 import requests
 
+GITHUB_API_TIMEOUT = (3.05, 10)
+logger = logging.getLogger(__name__)
+
 
 class GithubAppToken:
     def __init__(self, private_key, app_id) -> None:
@@ -22,17 +26,25 @@ def get_token(self, installation_id: int) -> Generator[str, None, None]:
         req = requests.post(
             url=f"https://api.github.com/app/installations/{installation_id}/access_tokens",
             headers=self.headers,
+            timeout=GITHUB_API_TIMEOUT,
         )
         req.raise_for_status()
         resp = req.json()
         try:
             # This token expires in an hour
             yield resp["token"]
         finally:
-            requests.delete(
-                "https://api.github.com/installation/token",
-                headers={"Authorization": f"token {resp['token']}"},
-            )
+            try:
+                requests.delete(
+                    "https://api.github.com/installation/token",
+                    headers={"Authorization": f"token {resp['token']}"},
+                    timeout=GITHUB_API_TIMEOUT,
+                ).raise_for_status()
+            except requests.RequestException:
+                logger.warning(
+                    "Failed to revoke GitHub installation token.",
+                    exc_info=True,
+                )
 
     def get_jwt_token(self, private_key, app_id):
         payload = {

src/github_sdk.py

@@ -6,11 +6,14 @@
 import logging
 import uuid
 from datetime import datetime
+from urllib.parse import urlparse
 
 import requests
 from sentry_sdk.envelope import Envelope
 from sentry_sdk.utils import format_timestamp
 
+HTTP_TIMEOUT = (3.05, 10)
+
 
 class GithubSentryError(Exception):
     pass
@@ -29,6 +32,7 @@
 class GithubClient:
     # This transform GH jobs conclusion keywords to Sentry performance status
     github_status_trace_status = {"success": "ok", "failure": "internal_error"}
+    allowed_github_hosts = {"api.github.com", "github.com"}
 
     def __init__(self, token, dsn, dry_run=False) -> None:
         self.token = token
@@ -39,10 +43,18 @@
             # '{BASE_URI}/api/{PROJECT_ID}/{ENDPOINT}/'
             self.sentry_project_url = f"{base_uri}/api/{project_id}/envelope/"
 
+    def _validate_github_url(self, url):
+        parsed = urlparse(url)
+        if parsed.scheme != "https":
+            raise GithubSentryError(f"Blocked non-HTTPS URL: {url}")
+        if not parsed.hostname or parsed.hostname not in self.allowed_github_hosts:
+            raise GithubSentryError(f"Blocked non-GitHub URL host: {url}")
+
     def _fetch_github(self, url):
+        self._validate_github_url(url)
         headers = {"Authorization": f"token {self.token}"}
 
-        req = requests.get(url, headers=headers)
+        req = requests.get(url, headers=headers, timeout=HTTP_TIMEOUT)
         req.raise_for_status()
         return req
 
@@ -132,6 +144,7 @@
             self.sentry_project_url,
             data=body.getvalue(),
             headers=headers,
+            timeout=HTTP_TIMEOUT,
         )
         req.raise_for_status()
         return req

src/sentry_config.py

@@ -3,6 +3,7 @@
 import base64
 import logging
 import os
+import re
 from configparser import ConfigParser
 from functools import lru_cache
 
@@ -15,6 +16,8 @@
 SENTRY_CONFIG_API_URL = (
     "https://api.github.com/repos/{owner}/.sentry/contents/sentry_config.ini"
 )
+GITHUB_API_TIMEOUT = (3.05, 10)
+GITHUB_OWNER_RE = re.compile(r"^(?!-)[A-Za-z0-9-]{1,39}(?<!-)$")
 
 
 def fetch_dsn_for_github_org(org: str, token: str) -> str:
@@ -24,10 +27,14 @@ def fetch_dsn_for_github_org(org: str, token: str) -> str:
         "Authorization": f"token {token}",
     }
     try:
-        api_url = SENTRY_CONFIG_API_URL.replace("{owner}", org)
+        if not isinstance(org, str) or not GITHUB_OWNER_RE.fullmatch(org):
+            raise ValueError("Invalid GitHub organization/owner name.")
+
+        safe_org = requests.utils.quote(org, safe="")
+        api_url = SENTRY_CONFIG_API_URL.replace("{owner}", safe_org)
 
         # - Get meta about sentry_config.ini file
-        resp = requests.get(api_url, headers=headers)
+        resp = requests.get(api_url, headers=headers, timeout=GITHUB_API_TIMEOUT)
         resp.raise_for_status()
         meta = resp.json()
 

tests/test_github_app.py

@@ -0,0 +1,91 @@
+from __future__ import annotations
+
+import logging
+
+import requests
+
+from src.github_app import GITHUB_API_TIMEOUT
+from src.github_app import GithubAppToken
+
+
+class FakeResponse:
+    def __init__(self, payload=None, error=None):
+        self.payload = payload or {}
+        self.error = error
+
+    def json(self):
+        return self.payload
+
+    def raise_for_status(self):
+        if self.error:
+            raise self.error
+
+
+def test_get_token_revocation_timeout_is_best_effort(monkeypatch, caplog):
+    delete_calls = []
+
+    def fake_post(url, headers, timeout):
+        return FakeResponse({"token": "installation-token"})
+
+    def fake_delete(url, headers, timeout):
+        delete_calls.append(
+            {
+                "url": url,
+                "headers": headers,
+                "timeout": timeout,
+            }
+        )
+        raise requests.ConnectTimeout()
+
+    monkeypatch.setattr("src.github_app.requests.post", fake_post)
+    monkeypatch.setattr("src.github_app.requests.delete", fake_delete)
+
+    token_client = GithubAppToken.__new__(GithubAppToken)
+    token_client.headers = {"Authorization": "Bearer jwt-token"}
+
+    caplog.set_level(logging.WARNING)
+    with token_client.get_token(123) as token:
+        assert token == "installation-token"
+
+    assert delete_calls == [
+        {
+            "url": "https://api.github.com/installation/token",
+            "headers": {"Authorization": "token installation-token"},
+            "timeout": GITHUB_API_TIMEOUT,
+        }
+    ]
+    assert "Failed to revoke GitHub installation token." in caplog.text
+
+
+def test_get_token_requests_access_token_with_timeout(monkeypatch):
+    post_calls = []
+
+    def fake_post(url, headers, timeout):
+        post_calls.append(
+            {
+                "url": url,
+                "headers": headers,
+                "timeout": timeout,
+            }
+        )
+        return FakeResponse({"token": "installation-token"})
+
+    monkeypatch.setattr("src.github_app.requests.post", fake_post)
+    monkeypatch.setattr(
+        "src.github_app.requests.delete",
+        lambda url, headers, timeout: FakeResponse(),
+    )
+
+    token_client = GithubAppToken.__new__(GithubAppToken)
+    token_client.headers = {"Authorization": "Bearer jwt-token"}
+
+    with token_client.get_token(123) as token:
+        assert token == "installation-token"
+
+    assert post_calls == [
+        {
+            "url": "https://api.github.com/app/installations/123/access_tokens",
+            "headers": {"Authorization": "Bearer jwt-token"},
+            "timeout": GITHUB_API_TIMEOUT,
+        }
+    ]