Skip to content

Commit fde1245

Browse files
authored
Fix security of GH api calls (#691)
* Fix security issues with GH api calls * validate url
1 parent 082c655 commit fde1245

File tree

2 files changed

+58
-5
lines changed

2 files changed

+58
-5
lines changed

metriq_gym/exporters/github_pr_exporter.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
from metriq_gym.exporters.base_exporter import BaseExporter
1414

1515
MAX_BRANCH_SUFFIX_ATTEMPTS = 1000
16+
GITHUB_API_NETLOC = "api.github.com"
17+
GITHUB_API_TIMEOUT_SECONDS = 30
1618

1719

1820
class GitHubPRExporter(BaseExporter):
@@ -232,6 +234,14 @@ def _headers(
232234
headers["Content-Type"] = content_type
233235
return headers
234236

237+
def _urlopen_github_api(self, req: urllib.request.Request):
238+
url = req.full_url
239+
parsed = urllib.parse.urlsplit(url)
240+
if parsed.scheme != "https" or parsed.netloc.lower() != GITHUB_API_NETLOC:
241+
raise ValueError(f"Refusing to open non-GitHub API URL: {url!r}")
242+
opener = urllib.request.build_opener()
243+
return opener.open(req, timeout=GITHUB_API_TIMEOUT_SECONDS)
244+
235245
def _run(self, cmd: list[str]) -> None:
236246
try:
237247
subprocess.run(cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -276,7 +286,7 @@ def _create_pull_request(
276286
method="POST",
277287
)
278288
try:
279-
with urllib.request.urlopen(req) as resp:
289+
with self._urlopen_github_api(req) as resp:
280290
resp_data = json.loads(resp.read().decode("utf-8"))
281291
except urllib.error.HTTPError as e:
282292
raise RuntimeError(f"GitHub PR creation failed: {e.read().decode('utf-8')}") from e
@@ -300,7 +310,8 @@ def _add_labels_to_pull_request(
300310
method="POST",
301311
)
302312
try:
303-
urllib.request.urlopen(req).read()
313+
with self._urlopen_github_api(req) as resp:
314+
resp.read()
304315
except urllib.error.HTTPError as e:
305316
raise RuntimeError(f"GitHub label add failed: {e.read().decode('utf-8')}") from e
306317

@@ -346,7 +357,7 @@ def _get_authenticated_login(self, token: str) -> str:
346357
method="GET",
347358
)
348359
try:
349-
with urllib.request.urlopen(req) as resp:
360+
with self._urlopen_github_api(req) as resp:
350361
data = json.loads(resp.read().decode("utf-8"))
351362
except urllib.error.HTTPError as e:
352363
raise RuntimeError(
@@ -370,7 +381,8 @@ def _ensure_fork(self, *, token: str, owner: str, repo: str, login: str) -> None
370381
method="POST",
371382
)
372383
try:
373-
urllib.request.urlopen(req).read()
384+
with self._urlopen_github_api(req) as resp:
385+
resp.read()
374386
except urllib.error.HTTPError as e:
375387
# If fork already exists or immediate accept, ignore certain errors
376388
if e.code not in (202, 201):
@@ -396,7 +408,7 @@ def _repo_exists(self, token: str, *, owner: str, repo: str) -> bool:
396408
method="GET",
397409
)
398410
try:
399-
with urllib.request.urlopen(req) as resp:
411+
with self._urlopen_github_api(req) as resp:
400412
return resp.getcode() == 200
401413
except urllib.error.HTTPError as e:
402414
if e.code == 404:
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import urllib.request
2+
3+
import pytest
4+
5+
from metriq_gym.benchmarks.benchmark import BenchmarkResult
6+
from metriq_gym.exporters.github_pr_exporter import (
7+
GITHUB_API_TIMEOUT_SECONDS,
8+
GitHubPRExporter,
9+
)
10+
11+
12+
def test_urlopen_github_api_passes_timeout(metriq_job, monkeypatch):
13+
exporter = GitHubPRExporter(metriq_job, BenchmarkResult())
14+
called = {}
15+
16+
class FakeOpener:
17+
def open(self, req, timeout=None):
18+
called["url"] = req.full_url
19+
called["timeout"] = timeout
20+
return object()
21+
22+
monkeypatch.setattr(urllib.request, "build_opener", lambda: FakeOpener())
23+
24+
req = urllib.request.Request("https://api.github.com/user", method="GET")
25+
exporter._urlopen_github_api(req)
26+
27+
assert called == {"url": "https://api.github.com/user", "timeout": GITHUB_API_TIMEOUT_SECONDS}
28+
29+
30+
def test_urlopen_github_api_rejects_non_https(metriq_job):
31+
exporter = GitHubPRExporter(metriq_job, BenchmarkResult())
32+
req = urllib.request.Request("http://api.github.com/user", method="GET")
33+
with pytest.raises(ValueError, match="Refusing to open non-GitHub API URL"):
34+
exporter._urlopen_github_api(req)
35+
36+
37+
def test_urlopen_github_api_rejects_unexpected_host(metriq_job):
38+
exporter = GitHubPRExporter(metriq_job, BenchmarkResult())
39+
req = urllib.request.Request("https://example.com/user", method="GET")
40+
with pytest.raises(ValueError, match="Refusing to open non-GitHub API URL"):
41+
exporter._urlopen_github_api(req)

0 commit comments

Comments
 (0)