Skip to content

Commit ac64ade

Browse files
mnriemCopilot
andcommitted
fix(authentication): validate direct GHE.com asset URLs
Reject ambiguous raw spellings and noncanonical paths before treating direct GHE.com URLs as release assets. Share the existing metadata URL spelling validation and cover empty delimiters, trailing slashes, encoded IDs, whitespace, semicolons, and malformed escapes. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 5899a2f commit ac64ade

2 files changed

Lines changed: 43 additions & 12 deletions

File tree

‎src/specify_cli/authentication/github_http.py‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,18 @@ def _has_valid_percent_escapes(value: str) -> bool:
4343
return True
4444

4545

46+
def _has_valid_asset_url_spelling(value: str) -> bool:
47+
"""Return whether an asset URL uses an unambiguous raw spelling."""
48+
return (
49+
not any(
50+
ord(character) <= 0x20 or ord(character) == 0x7F
51+
for character in value
52+
)
53+
and not any(delimiter in value for delimiter in ("?", "#", ";"))
54+
and _has_valid_percent_escapes(value)
55+
)
56+
57+
4658
def build_github_request(url: str) -> urllib.request.Request:
4759
"""Build a urllib Request, adding a GitHub auth header when available.
4860
@@ -180,10 +192,14 @@ def _is_asset_path(segments: list[str]) -> bool:
180192
and segments[3:5] == ["releases", "assets"]
181193
)
182194

183-
def _is_exact_asset_path(segments: list[str]) -> bool:
195+
def _is_exact_raw_asset_path(path: str) -> bool:
196+
segments = path.split("/")
184197
return (
185-
len(segments) == 6
186-
and _is_asset_path(segments)
198+
len(segments) == 7
199+
and segments[:2] == ["", "repos"]
200+
and bool(segments[2])
201+
and bool(segments[3])
202+
and segments[4:6] == ["releases", "assets"]
187203
and segments[-1].isascii()
188204
and segments[-1].isdigit()
189205
)
@@ -204,6 +220,7 @@ def _is_exact_asset_path(segments: list[str]) -> bool:
204220
ghe_com_web_hostname = _ghe_com_web_hostname(hostname)
205221
if (
206222
ghe_com_web_hostname is not None
223+
and _has_valid_asset_url_spelling(download_url)
207224
and parsed.scheme == "https"
208225
and parsed_port in (None, 443)
209226
and parsed.username is None
@@ -213,7 +230,7 @@ def _is_exact_asset_path(segments: list[str]) -> bool:
213230
and not parsed.params
214231
and _host_matches(hostname, github_hosts)
215232
and _host_matches(ghe_com_web_hostname, github_hosts)
216-
and _is_exact_asset_path(parts)
233+
and _is_exact_raw_asset_path(parsed.path)
217234
):
218235
return download_url
219236

@@ -259,14 +276,7 @@ def _is_expected_asset_url(asset_url: object) -> bool:
259276
# ``urlparse`` tolerates some raw spellings (for example whitespace)
260277
# even though the original metadata value is returned to the caller.
261278
# Reject those spellings before parsing rather than normalizing them.
262-
if (
263-
any(
264-
ord(character) <= 0x20 or ord(character) == 0x7F
265-
for character in asset_url
266-
)
267-
or any(delimiter in asset_url for delimiter in ("?", "#", ";"))
268-
or not _has_valid_percent_escapes(asset_url)
269-
):
279+
if not _has_valid_asset_url_spelling(asset_url):
270280
return False
271281
try:
272282
asset_parsed = urlparse(asset_url)

‎tests/specify_cli/authentication/test_github_http.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,27 @@ def test_passthrough_for_trusted_ghecom_api_asset_url(self):
354354
)
355355
assert result == url
356356

357+
@pytest.mark.parametrize(
358+
"url",
359+
[
360+
"https://api.msft.ghe.com/repos/org/repo/releases/assets/42?",
361+
"https://api.msft.ghe.com/repos/org/repo/releases/assets/42#",
362+
"https://api.msft.ghe.com/repos/org/repo/releases/assets/42/",
363+
"https://api.msft.ghe.com/repos/org/repo/releases/assets/%34%32",
364+
"https://api.msft.ghe.com/repos/org/repo/releases/assets/42 ",
365+
"https://api.msft.ghe.com/repos/org/repo/releases/assets/42;",
366+
"https://api.msft.ghe.com/repos/org/repo/releases/assets/%ZZ",
367+
],
368+
)
369+
def test_rejects_noncanonical_direct_ghecom_api_asset_url(self, url):
370+
"""Direct GHE.com asset URLs retain strict raw-spelling validation."""
371+
result = resolve_github_release_asset_api_url(
372+
url,
373+
lambda *a, **kw: None,
374+
github_hosts=("msft.ghe.com", "api.msft.ghe.com"),
375+
)
376+
assert result is None
377+
357378
def test_rejects_direct_ghecom_api_asset_url_without_trusted_pair(self):
358379
"""A direct GHE.com API URL needs both trusted tenant hostnames."""
359380
url = "https://api.msft.ghe.com/repos/org/repo/releases/assets/42"

0 commit comments

Comments
 (0)