mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(presets): raise PresetValidationError, not raw ValueError, on malformed catalog URL (#3576)
`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) <noreply@anthropic.com>
This commit is contained in:
@@ -2074,8 +2074,12 @@ class PresetCatalog:
|
||||
"""
|
||||
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 @@ class PresetCatalog:
|
||||
# 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."
|
||||
)
|
||||
|
||||
@@ -1634,6 +1634,18 @@ class TestPresetCatalog:
|
||||
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")
|
||||
|
||||
Reference in New Issue
Block a user