From 0446d8daa242de9a7c8be2af8f6a67ea81e60fad Mon Sep 17 00:00:00 2001 From: Andrew Chen <48723787+chuenchen309@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:24:47 +0800 Subject: [PATCH] fix(presets): raise PresetValidationError, not raw ValueError, on malformed catalog URL `PresetCatalog._validate_catalog_url` called `urlparse(url).hostname` without guarding it. For a malformed authority such as an unterminated IPv6 bracket (`https://[::1`), `urlparse(...).hostname` raises `ValueError: Invalid IPv6 URL`, which escapes the method. Its docstring promises `PresetValidationError`, and its callers (`preset catalog add`, `preset catalog list` reading the `SPECKIT_PRESET_CATALOG_URL` env var / `.specify/preset-catalogs.yml`) only catch `PresetValidationError` -- so a malformed URL crashes the CLI with a traceback instead of a clean error message. The shared `CatalogStackBase` (#3435), `workflows` (#3484), `bundler` (#3433) and `IntegrationCatalog` copies already wrap this in `try/except ValueError`; the preset validator was the remaining un-updated twin. Mirror the shared implementation: wrap `urlparse` + `.hostname`, re-raise as `PresetValidationError("Catalog URL is malformed: ...")`, and read the local `hostname` in the host check. Add a regression test mirroring `IntegrationCatalog`'s `test_malformed_url_rejected_cleanly`; it is red before the fix (raw `ValueError`) and green after. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/presets/__init__.py | 10 +++++++--- tests/test_presets.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 98bba08fd4..97e15aa648 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -2074,8 +2074,12 @@ def _validate_catalog_url(self, url: str) -> None: """ from urllib.parse import urlparse - parsed = urlparse(url) - is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1") + try: + parsed = urlparse(url) + hostname = parsed.hostname + except ValueError: + raise PresetValidationError(f"Catalog URL is malformed: {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 ): @@ -2086,7 +2090,7 @@ def _validate_catalog_url(self, url: str) -> None: # Check hostname, not netloc: netloc is truthy for host-less URLs like # "https://:8080" or "https://user@", so the host guarantee this error # promises would not actually hold. hostname is None in those cases (#3209). - if not parsed.hostname: + if not hostname: raise PresetValidationError( "Catalog URL must be a valid URL with a host." ) diff --git a/tests/test_presets.py b/tests/test_presets.py index 797835a88c..aec335c916 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1634,6 +1634,18 @@ def test_validate_catalog_url_hostless_rejected(self, project_dir, url): with pytest.raises(PresetValidationError, match="valid URL with a host"): catalog._validate_catalog_url(url) + def test_validate_catalog_url_malformed_rejected(self, project_dir): + """A malformed URL raises PresetValidationError, not a raw ValueError. + + ``urlparse('https://[::1').hostname`` raises ``ValueError: Invalid IPv6 + URL`` (unterminated bracket). Without wrapping, that leaks past callers' + ``except PresetValidationError`` guards and crashes the CLI. Mirrors the + shared ``CatalogStackBase`` (#3435) and ``IntegrationCatalog`` behaviour. + """ + catalog = PresetCatalog(project_dir) + with pytest.raises(PresetValidationError, match="malformed"): + catalog._validate_catalog_url("https://[::1") + def test_env_var_catalog_url(self, project_dir, monkeypatch): """Test catalog URL from environment variable.""" monkeypatch.setenv("SPECKIT_PRESET_CATALOG_URL", "https://custom.example.com/catalog.json")