From de313cc03d48f86ec941388a47ad5fe967684f0f Mon Sep 17 00:00:00 2001 From: Louis-Philippe Bedard Date: Thu, 12 Jun 2025 13:03:25 -0400 Subject: [PATCH 1/4] fix: use wildcard for github domain validation --- github/Requester.py | 6 ++---- tests/Requester.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/github/Requester.py b/github/Requester.py index b3e5eb554c..b42d75d31c 100644 --- a/github/Requester.py +++ b/github/Requester.py @@ -1225,12 +1225,10 @@ def __makeAbsoluteUrl(self, url: str) -> str: url = f"{self.__prefix}{url}" else: o = urllib.parse.urlparse(url) - assert o.hostname in [ + assert ".".join(o.hostname.split(".")[-2:]) in [ self.__hostname, - "uploads.github.com", - "status.github.com", "github.com", - "objects.githubusercontent.com", + "githubusercontent.com", ], o.hostname assert o.path.startswith((self.__prefix, self.__graphql_prefix, "/api/", "/login/oauth")), o.path assert o.port == self.__port, o.port diff --git a/tests/Requester.py b/tests/Requester.py index a11ce1e2ba..11b34285c6 100644 --- a/tests/Requester.py +++ b/tests/Requester.py @@ -263,6 +263,35 @@ def testBaseUrlPrefixRedirection(self): "Following Github server redirection from /api/v3/repos/PyGithub/PyGithub to /repos/PyGithub/PyGithub" ) + def testMakeAbsoluteUrl(self): + class TestAuth(github.Auth.AppAuth): + pass + + # create a Requester with non-default arguments + auth = TestAuth(123, "key") + requester = github.Requester.Requester( + auth=auth, + base_url="https://base.url", + timeout=1, + user_agent="user agent", + per_page=123, + verify=False, + retry=3, + pool_size=5, + seconds_between_requests=1.2, + seconds_between_writes=3.4, + # v3: this should not be the default value, so if this has been changed in v3, + # change it here is well + lazy=True, + ) + + with self.assertRaises(AssertionError) as exc: + requester._Requester__makeAbsoluteUrl("https://github.com.malicious.com"), + self.assertEqual( + exc.exception.args, + "AssertionError: github.com.malicious.com" + ) + PrimaryRateLimitErrors = [ "API rate limit exceeded for x.x.x.x. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", ] From 7c1b7a2e5d74c3cbc143c4134b241365d10a4b48 Mon Sep 17 00:00:00 2001 From: Louis-Philippe Bedard Date: Thu, 12 Jun 2025 13:24:14 -0400 Subject: [PATCH 2/4] fix: adding more tests --- tests/Requester.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/Requester.py b/tests/Requester.py index 11b34285c6..1ad3b4cfc3 100644 --- a/tests/Requester.py +++ b/tests/Requester.py @@ -280,8 +280,6 @@ class TestAuth(github.Auth.AppAuth): pool_size=5, seconds_between_requests=1.2, seconds_between_writes=3.4, - # v3: this should not be the default value, so if this has been changed in v3, - # change it here is well lazy=True, ) @@ -292,6 +290,15 @@ class TestAuth(github.Auth.AppAuth): "AssertionError: github.com.malicious.com" ) + for url in [ + "github.com", + "uploads.github.com", + "status.github.com", + "objects.githubusercontent.com", + "release-assets.githubusercontent.com", + ]: + self.assertEqual(requester._Requester__makeAbsoluteUrl(f'https://{url}'), "") + PrimaryRateLimitErrors = [ "API rate limit exceeded for x.x.x.x. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", ] From 6d53e26d46d68f0137f548fca7449695ec4bd413 Mon Sep 17 00:00:00 2001 From: Louis-Philippe Bedard Date: Thu, 12 Jun 2025 14:28:28 -0400 Subject: [PATCH 3/4] fix: fmt --- tests/Requester.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/Requester.py b/tests/Requester.py index 1ad3b4cfc3..b1cf450630 100644 --- a/tests/Requester.py +++ b/tests/Requester.py @@ -285,19 +285,16 @@ class TestAuth(github.Auth.AppAuth): with self.assertRaises(AssertionError) as exc: requester._Requester__makeAbsoluteUrl("https://github.com.malicious.com"), - self.assertEqual( - exc.exception.args, - "AssertionError: github.com.malicious.com" - ) + self.assertEqual(exc.exception.args, "AssertionError: github.com.malicious.com") for url in [ - "github.com", - "uploads.github.com", - "status.github.com", - "objects.githubusercontent.com", - "release-assets.githubusercontent.com", + "github.com", + "uploads.github.com", + "status.github.com", + "objects.githubusercontent.com", + "release-assets.githubusercontent.com", ]: - self.assertEqual(requester._Requester__makeAbsoluteUrl(f'https://{url}'), "") + self.assertEqual(requester._Requester__makeAbsoluteUrl(f"https://{url}"), "") PrimaryRateLimitErrors = [ "API rate limit exceeded for x.x.x.x. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", From 3baf64582737ef2c647b6e922239ebe508240a0b Mon Sep 17 00:00:00 2001 From: Louis-Philippe Bedard Date: Thu, 12 Jun 2025 14:42:38 -0400 Subject: [PATCH 4/4] fix: Extract domain in its own method --- github/Requester.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/github/Requester.py b/github/Requester.py index b42d75d31c..28b1d7436c 100644 --- a/github/Requester.py +++ b/github/Requester.py @@ -1218,6 +1218,10 @@ def __recordRequestTime(self, verb: str) -> None: # Updates self.__last_requests with current timestamp for given verb self.__last_requests[verb] = datetime.now(timezone.utc).timestamp() + def __extractDomainFromHostname(self, hostname: str) -> str: + # Extracts the domain from a hostname + return ".".join(hostname.split(".")[-2:]) + def __makeAbsoluteUrl(self, url: str) -> str: # URLs generated locally will be relative to __base_url # URLs returned from the server will start with __base_url @@ -1225,7 +1229,7 @@ def __makeAbsoluteUrl(self, url: str) -> str: url = f"{self.__prefix}{url}" else: o = urllib.parse.urlparse(url) - assert ".".join(o.hostname.split(".")[-2:]) in [ + assert self.__extractDomainFromHostname(o.hostname) in [ self.__hostname, "github.com", "githubusercontent.com",