Skip to content

Commit ea93ade

Browse files
fix(workflows): raise catalog error, not raw ValueError, on a malformed catalog URL
The four catalog URL validators in `workflows/catalog.py` (`WorkflowCatalog`/`StepCatalog` `_validate_catalog_url`, and the nested fetch-path validators) accessed `urlparse(url).hostname` unguarded. A malformed authority — e.g. an unterminated IPv6 bracket `https://[::1` or a bracketed non-IP host `https://[not-an-ip]` — makes urlparse / hostname raise `ValueError`. Each validator's contract is to raise a domain error (`WorkflowValidationError` / `StepValidationError` / `WorkflowCatalogError` / `StepCatalogError`), and the command handlers catch only those. So `specify workflow catalog add "https://[::1"` surfaced an uncaught `ValueError` traceback instead of the clean `Error: Catalog URL is malformed` + exit 1 that a bad URL should give. The fetch-path validators also run on the post-redirect `resp.geturl()`, so a hostile redirect target could crash the fetch the same way. Guard each `urlparse`/`.hostname` access with `try/except ValueError -> domain error`, mirroring the fixes already applied to `specify_cli.catalogs` (#3435) and the bundler adapters (#3433). Also read `hostname` once and reuse it for the host check, matching those siblings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1be4299 commit ea93ade

2 files changed

Lines changed: 101 additions & 12 deletions

File tree

src/specify_cli/workflows/catalog.py

Lines changed: 58 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -157,16 +157,28 @@ def _validate_catalog_url(self, url: str) -> None:
157157
"""Validate that a catalog URL uses HTTPS (localhost HTTP allowed)."""
158158
from urllib.parse import urlparse
159159

160-
parsed = urlparse(url)
161-
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
160+
# A malformed authority (e.g. an unterminated IPv6 bracket
161+
# "https://[::1") makes urlparse / hostname access raise ValueError.
162+
# This validator's contract is to raise WorkflowValidationError for a
163+
# bad URL, so surface that rather than leaking a raw ValueError past the
164+
# command handler (which only catches WorkflowValidationError). Mirrors
165+
# specify_cli.catalogs (#3435).
166+
try:
167+
parsed = urlparse(url)
168+
hostname = parsed.hostname
169+
except ValueError:
170+
raise WorkflowValidationError(
171+
f"Catalog URL is malformed: {url}"
172+
) from None
173+
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
162174
if parsed.scheme != "https" and not (
163175
parsed.scheme == "http" and is_localhost
164176
):
165177
raise WorkflowValidationError(
166178
f"Catalog URL must use HTTPS (got {parsed.scheme}://). "
167179
"HTTP is only allowed for localhost."
168180
)
169-
if not parsed.hostname:
181+
if not hostname:
170182
raise WorkflowValidationError(
171183
"Catalog URL must be a valid URL with a host."
172184
)
@@ -332,15 +344,26 @@ def _fetch_single_catalog(
332344
from specify_cli.authentication.http import open_url as _open_url
333345

334346
def _validate_catalog_url(url: str) -> None:
335-
parsed = urlparse(url)
336-
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
347+
# A malformed authority (e.g. "https://[::1") makes urlparse /
348+
# hostname access raise ValueError; treat it as a refused fetch
349+
# rather than leaking a raw ValueError (this also validates the
350+
# post-redirect resp.geturl(), so a hostile redirect target cannot
351+
# crash the fetch either).
352+
try:
353+
parsed = urlparse(url)
354+
hostname = parsed.hostname
355+
except ValueError:
356+
raise WorkflowCatalogError(
357+
f"Refusing to fetch catalog from malformed URL: {url}"
358+
) from None
359+
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
337360
if parsed.scheme != "https" and not (
338361
parsed.scheme == "http" and is_localhost
339362
):
340363
raise WorkflowCatalogError(
341364
f"Refusing to fetch catalog from non-HTTPS URL: {url}"
342365
)
343-
if not parsed.hostname:
366+
if not hostname:
344367
raise WorkflowCatalogError(
345368
f"Refusing to fetch catalog from URL with no hostname: {url}"
346369
)
@@ -774,16 +797,28 @@ def _validate_catalog_url(self, url: str) -> None:
774797
"""Validate that a catalog URL uses HTTPS (localhost HTTP allowed)."""
775798
from urllib.parse import urlparse
776799

777-
parsed = urlparse(url)
778-
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
800+
# A malformed authority (e.g. an unterminated IPv6 bracket
801+
# "https://[::1") makes urlparse / hostname access raise ValueError.
802+
# This validator's contract is to raise StepValidationError for a bad
803+
# URL, so surface that rather than leaking a raw ValueError past the
804+
# command handler (which only catches StepValidationError). Mirrors
805+
# specify_cli.catalogs (#3435).
806+
try:
807+
parsed = urlparse(url)
808+
hostname = parsed.hostname
809+
except ValueError:
810+
raise StepValidationError(
811+
f"Catalog URL is malformed: {url}"
812+
) from None
813+
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
779814
if parsed.scheme != "https" and not (
780815
parsed.scheme == "http" and is_localhost
781816
):
782817
raise StepValidationError(
783818
f"Catalog URL must use HTTPS (got {parsed.scheme}://). "
784819
"HTTP is only allowed for localhost."
785820
)
786-
if not parsed.hostname:
821+
if not hostname:
787822
raise StepValidationError(
788823
"Catalog URL must be a valid URL with a host."
789824
)
@@ -949,15 +984,26 @@ def _fetch_single_catalog(
949984
from specify_cli.authentication.http import open_url as _open_url
950985

951986
def _validate_url(url: str) -> None:
952-
parsed = urlparse(url)
953-
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
987+
# A malformed authority (e.g. "https://[::1") makes urlparse /
988+
# hostname access raise ValueError; treat it as a refused fetch
989+
# rather than leaking a raw ValueError (this also validates the
990+
# post-redirect resp.geturl(), so a hostile redirect target cannot
991+
# crash the fetch either).
992+
try:
993+
parsed = urlparse(url)
994+
hostname = parsed.hostname
995+
except ValueError:
996+
raise StepCatalogError(
997+
f"Refusing to fetch catalog from malformed URL: {url}"
998+
) from None
999+
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
9541000
if parsed.scheme != "https" and not (
9551001
parsed.scheme == "http" and is_localhost
9561002
):
9571003
raise StepCatalogError(
9581004
f"Refusing to fetch catalog from non-HTTPS URL: {url}"
9591005
)
960-
if not parsed.hostname:
1006+
if not hostname:
9611007
raise StepCatalogError(
9621008
f"Refusing to fetch catalog from URL with no hostname: {url}"
9631009
)

tests/test_workflows.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4805,6 +4805,31 @@ def test_validate_url_localhost_http_allowed(self, project_dir):
48054805
# Should not raise
48064806
catalog._validate_catalog_url("http://localhost:8080/catalog.json")
48074807

4808+
@pytest.mark.parametrize(
4809+
"url",
4810+
[
4811+
"https://[::1", # unterminated IPv6 bracket
4812+
"https://[not-an-ip]/x", # bracketed non-IP host
4813+
],
4814+
)
4815+
def test_validate_url_malformed_raises_validation_error(self, project_dir, url):
4816+
"""A malformed authority must raise WorkflowValidationError, not leak a
4817+
raw ValueError.
4818+
4819+
``urlparse``/``.hostname`` raise ValueError on a malformed IPv6
4820+
authority. The command handler only catches WorkflowValidationError,
4821+
so a raw ValueError would surface as an uncaught traceback instead of a
4822+
clean 'Error:' message + exit 1. Mirrors specify_cli.catalogs (#3435).
4823+
"""
4824+
from specify_cli.workflows.catalog import (
4825+
WorkflowCatalog,
4826+
WorkflowValidationError,
4827+
)
4828+
4829+
catalog = WorkflowCatalog(project_dir)
4830+
with pytest.raises(WorkflowValidationError, match="malformed"):
4831+
catalog._validate_catalog_url(url)
4832+
48084833
def test_add_catalog(self, project_dir):
48094834
from specify_cli.workflows.catalog import WorkflowCatalog
48104835

@@ -5251,6 +5276,24 @@ def test_validate_url_localhost_http_allowed(self, project_dir):
52515276
# Should not raise
52525277
catalog._validate_catalog_url("http://localhost:8080/step-catalog.json")
52535278

5279+
@pytest.mark.parametrize(
5280+
"url",
5281+
[
5282+
"https://[::1", # unterminated IPv6 bracket
5283+
"https://[not-an-ip]/x", # bracketed non-IP host
5284+
],
5285+
)
5286+
def test_validate_url_malformed_raises_validation_error(self, project_dir, url):
5287+
"""A malformed authority must raise StepValidationError, not leak a raw
5288+
ValueError past the command handler (which only catches
5289+
StepValidationError). Mirrors specify_cli.catalogs (#3435).
5290+
"""
5291+
from specify_cli.workflows.catalog import StepCatalog, StepValidationError
5292+
5293+
catalog = StepCatalog(project_dir)
5294+
with pytest.raises(StepValidationError, match="malformed"):
5295+
catalog._validate_catalog_url(url)
5296+
52545297
def test_add_catalog(self, project_dir):
52555298
from specify_cli.workflows.catalog import StepCatalog
52565299

0 commit comments

Comments
 (0)