Skip to content

Commit dc411dc

Browse files
fix(extensions,presets): surface clean error on malformed download URL
`ExtensionCatalog.download_extension` and `PresetCatalog.download_pack` read `download_url` from catalog payload data and pass it to `urlparse(...).hostname` during the HTTPS validation. A malformed authority (e.g. an unterminated IPv6 bracket like `https://[::1`) makes urlparse/hostname raise a raw `ValueError`, which escapes past the command handlers — they only catch `ExtensionError` / `PresetError` — and surfaces as an uncaught traceback. Guard the parse in a try/except and re-raise as the domain error so the CLI reports a clean "download URL is malformed" message. Mirrors the same fix in catalogs (#3435) and workflows/catalog.py (#3484). Adds regression coverage for both catalogs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7bdf6c5 commit dc411dc

4 files changed

Lines changed: 70 additions & 4 deletions

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2631,8 +2631,20 @@ def download_extension(
26312631
# Validate download URL requires HTTPS (prevent man-in-the-middle attacks)
26322632
from urllib.parse import urlparse
26332633

2634-
parsed = urlparse(download_url)
2635-
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
2634+
# A malformed authority (e.g. an unterminated IPv6 bracket
2635+
# "https://[::1") makes urlparse / hostname access raise ValueError.
2636+
# The download_url comes from catalog payload data, so surface a clean
2637+
# ExtensionError rather than leaking a raw ValueError past the command
2638+
# handler (which only catches ExtensionError). Mirrors catalogs (#3435)
2639+
# and workflows/catalog.py (#3484).
2640+
try:
2641+
parsed = urlparse(download_url)
2642+
hostname = parsed.hostname
2643+
except ValueError:
2644+
raise ExtensionError(
2645+
f"Extension download URL is malformed: {download_url}"
2646+
) from None
2647+
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
26362648
if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost):
26372649
raise ExtensionError(
26382650
f"Extension download URL must use HTTPS: {download_url}"

src/specify_cli/presets/__init__.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2713,8 +2713,20 @@ def download_pack(
27132713

27142714
from urllib.parse import urlparse
27152715

2716-
parsed = urlparse(download_url)
2717-
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
2716+
# A malformed authority (e.g. an unterminated IPv6 bracket
2717+
# "https://[::1") makes urlparse / hostname access raise ValueError.
2718+
# The download_url comes from catalog payload data, so surface a clean
2719+
# PresetError rather than leaking a raw ValueError past the command
2720+
# handler (which only catches PresetError). Mirrors catalogs (#3435)
2721+
# and workflows/catalog.py (#3484).
2722+
try:
2723+
parsed = urlparse(download_url)
2724+
hostname = parsed.hostname
2725+
except ValueError:
2726+
raise PresetError(
2727+
f"Preset download URL is malformed: {download_url}"
2728+
) from None
2729+
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
27182730
if parsed.scheme != "https" and not (
27192731
parsed.scheme == "http" and is_localhost
27202732
):

tests/test_extensions.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4195,6 +4195,26 @@ def test_download_extension_rejects_sha256_mismatch(self, temp_dir):
41954195
with pytest.raises(ExtensionError, match="[Ii]ntegrity"):
41964196
catalog.download_extension("test-ext", target_dir=temp_dir)
41974197

4198+
def test_download_extension_malformed_url_raises_extension_error(self, temp_dir):
4199+
"""A catalog ``download_url`` with a malformed authority (e.g. an
4200+
unterminated IPv6 bracket) surfaces a clean ``ExtensionError`` rather
4201+
than leaking a raw ``ValueError`` from ``urlparse``/``.hostname`` past
4202+
the command handler (which only catches ``ExtensionError``).
4203+
"""
4204+
from unittest.mock import patch
4205+
4206+
catalog = self._make_catalog(temp_dir)
4207+
for bad_url in ("https://[::1", "https://[not-an-ip]/x"):
4208+
ext_info = {
4209+
"id": "test-ext",
4210+
"name": "Test Extension",
4211+
"version": "1.0.0",
4212+
"download_url": bad_url,
4213+
}
4214+
with patch.object(catalog, "get_extension_info", return_value=ext_info):
4215+
with pytest.raises(ExtensionError, match="malformed"):
4216+
catalog.download_extension("test-ext", target_dir=temp_dir)
4217+
41984218
def test_download_extension_without_sha256_still_succeeds(self, temp_dir):
41994219
"""Entries without ``sha256`` keep working (backwards compatible)."""
42004220
from unittest.mock import patch

tests/test_presets.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,6 +2291,28 @@ def test_download_pack_rejects_sha256_mismatch(self, project_dir):
22912291
with pytest.raises(PresetError, match="[Ii]ntegrity"):
22922292
catalog.download_pack("test-pack", target_dir=project_dir)
22932293

2294+
def test_download_pack_malformed_url_raises_preset_error(self, project_dir):
2295+
"""A catalog ``download_url`` with a malformed authority (e.g. an
2296+
unterminated IPv6 bracket) surfaces a clean ``PresetError`` rather than
2297+
leaking a raw ``ValueError`` from ``urlparse``/``.hostname`` past the
2298+
command handler (which only catches ``PresetError``). Mirrors the
2299+
extensions coverage.
2300+
"""
2301+
from unittest.mock import patch
2302+
2303+
catalog = PresetCatalog(project_dir)
2304+
for bad_url in ("https://[::1", "https://[not-an-ip]/x"):
2305+
pack_info = {
2306+
"id": "test-pack",
2307+
"name": "Test Pack",
2308+
"version": "1.0.0",
2309+
"download_url": bad_url,
2310+
"_install_allowed": True,
2311+
}
2312+
with patch.object(catalog, "get_pack_info", return_value=pack_info):
2313+
with pytest.raises(PresetError, match="malformed"):
2314+
catalog.download_pack("test-pack", target_dir=project_dir)
2315+
22942316
def test_download_pack_without_sha256_skips_verification(self, project_dir):
22952317
"""A catalog entry with no ``sha256`` keeps working: verification is
22962318
opt-in, so the backwards-compatible path (``pack_info.get("sha256")``

0 commit comments

Comments
 (0)