diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 05aa35f7fb..fec109bd15 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -2631,8 +2631,20 @@ def download_extension( # Validate download URL requires HTTPS (prevent man-in-the-middle attacks) from urllib.parse import urlparse - parsed = urlparse(download_url) - is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1") + # A malformed authority (e.g. an unterminated IPv6 bracket + # "https://[::1") makes urlparse / hostname access raise ValueError. + # The download_url comes from catalog payload data, so surface a clean + # ExtensionError rather than leaking a raw ValueError past the command + # handler (which only catches ExtensionError). Mirrors catalogs (#3435) + # and workflows/catalog.py (#3484). + try: + parsed = urlparse(download_url) + hostname = parsed.hostname + except ValueError: + raise ExtensionError( + f"Extension download URL is malformed: {download_url}" + ) from None + is_localhost = hostname in ("localhost", "127.0.0.1", "::1") if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost): raise ExtensionError( f"Extension download URL must use HTTPS: {download_url}" diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 98bba08fd4..a5943de5ad 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -2713,8 +2713,20 @@ def download_pack( from urllib.parse import urlparse - parsed = urlparse(download_url) - is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1") + # A malformed authority (e.g. an unterminated IPv6 bracket + # "https://[::1") makes urlparse / hostname access raise ValueError. + # The download_url comes from catalog payload data, so surface a clean + # PresetError rather than leaking a raw ValueError past the command + # handler (which only catches PresetError). Mirrors catalogs (#3435) + # and workflows/catalog.py (#3484). + try: + parsed = urlparse(download_url) + hostname = parsed.hostname + except ValueError: + raise PresetError( + f"Preset download URL is malformed: {download_url}" + ) from None + is_localhost = hostname in ("localhost", "127.0.0.1", "::1") if parsed.scheme != "https" and not ( parsed.scheme == "http" and is_localhost ): diff --git a/tests/test_extensions.py b/tests/test_extensions.py index d2f4af8264..7faf3eaec0 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -4195,6 +4195,26 @@ def test_download_extension_rejects_sha256_mismatch(self, temp_dir): with pytest.raises(ExtensionError, match="[Ii]ntegrity"): catalog.download_extension("test-ext", target_dir=temp_dir) + def test_download_extension_malformed_url_raises_extension_error(self, temp_dir): + """A catalog ``download_url`` with a malformed authority (e.g. an + unterminated IPv6 bracket) surfaces a clean ``ExtensionError`` rather + than leaking a raw ``ValueError`` from ``urlparse``/``.hostname`` past + the command handler (which only catches ``ExtensionError``). + """ + from unittest.mock import patch + + catalog = self._make_catalog(temp_dir) + for bad_url in ("https://[::1", "https://[not-an-ip]/x"): + ext_info = { + "id": "test-ext", + "name": "Test Extension", + "version": "1.0.0", + "download_url": bad_url, + } + with patch.object(catalog, "get_extension_info", return_value=ext_info): + with pytest.raises(ExtensionError, match="malformed"): + catalog.download_extension("test-ext", target_dir=temp_dir) + def test_download_extension_without_sha256_still_succeeds(self, temp_dir): """Entries without ``sha256`` keep working (backwards compatible).""" from unittest.mock import patch diff --git a/tests/test_presets.py b/tests/test_presets.py index 797835a88c..656304e22e 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2291,6 +2291,28 @@ def test_download_pack_rejects_sha256_mismatch(self, project_dir): with pytest.raises(PresetError, match="[Ii]ntegrity"): catalog.download_pack("test-pack", target_dir=project_dir) + def test_download_pack_malformed_url_raises_preset_error(self, project_dir): + """A catalog ``download_url`` with a malformed authority (e.g. an + unterminated IPv6 bracket) surfaces a clean ``PresetError`` rather than + leaking a raw ``ValueError`` from ``urlparse``/``.hostname`` past the + command handler (which only catches ``PresetError``). Mirrors the + extensions coverage. + """ + from unittest.mock import patch + + catalog = PresetCatalog(project_dir) + for bad_url in ("https://[::1", "https://[not-an-ip]/x"): + pack_info = { + "id": "test-pack", + "name": "Test Pack", + "version": "1.0.0", + "download_url": bad_url, + "_install_allowed": True, + } + with patch.object(catalog, "get_pack_info", return_value=pack_info): + with pytest.raises(PresetError, match="malformed"): + catalog.download_pack("test-pack", target_dir=project_dir) + def test_download_pack_without_sha256_skips_verification(self, project_dir): """A catalog entry with no ``sha256`` keeps working: verification is opt-in, so the backwards-compatible path (``pack_info.get("sha256")``