From e19b1924bec9c1e40fd0aabe33f3dcbe89197879 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:09:58 +0200 Subject: [PATCH 1/2] fix(bundler): validate catalog URLs in `catalog add` (HTTPS-only, require host) add_source persisted remote catalog URLs without the HTTPS/host checks that specify_cli.catalogs (#3210) and the bundler adapters (#3333) enforce, and an unclosed IPv6 bracket escaped as a raw ValueError. Mirror the catalogs.py validation for http(s) schemes and wrap urlparse so malformed input raises BundlerError. Fixes #3366 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../bundler/commands_impl/catalog_config.py | 25 +++++++++++-- tests/unit/test_bundler_catalog_config.py | 36 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/bundler/commands_impl/catalog_config.py b/src/specify_cli/bundler/commands_impl/catalog_config.py index e0650ce4b5..591453bf27 100644 --- a/src/specify_cli/bundler/commands_impl/catalog_config.py +++ b/src/specify_cli/bundler/commands_impl/catalog_config.py @@ -95,7 +95,11 @@ def _is_local_path(url: str) -> bool: """True when *url* denotes a local filesystem path rather than a URL.""" if _WINDOWS_DRIVE_RE.match(url): return True - scheme = urlparse(url).scheme.lower() + try: + scheme = urlparse(url).scheme.lower() + except ValueError: + # Malformed URLs (e.g. an unclosed IPv6 bracket) are not local paths. + return False return scheme not in _REMOTE_SCHEMES @@ -137,7 +141,10 @@ def add_source( url = url.strip() if not url: raise BundlerError("A catalog url is required.") - parsed = urlparse(url) + try: + parsed = urlparse(url) + except ValueError as exc: + raise BundlerError(f"Invalid catalog url: '{url}'.") from exc if not (parsed.scheme or parsed.path): raise BundlerError(f"Invalid catalog url: '{url}'.") # Reject unsupported URL schemes (e.g. ssh://, ftp://) up front so they are @@ -148,6 +155,20 @@ def add_source( f"Unsupported catalog url scheme '{parsed.scheme}://' in '{url}'. " "Use http(s)://, file://, builtin://, or a local path." ) + if parsed.scheme.lower() in {"http", "https"}: + # Mirror specify_cli.catalogs / bundler adapters URL validation + # (#3209/#3210): HTTPS only (HTTP just for localhost), and check + # hostname, not netloc — netloc is truthy for host-less URLs like + # "https://:8080" or "https://user@". Validating here keeps junk out + # of catalogs.yaml instead of failing later at fetch time. + is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1") + if parsed.scheme.lower() != "https" and not is_localhost: + raise BundlerError( + f"Catalog url must use HTTPS (got {parsed.scheme}://). " + "HTTP is only allowed for localhost." + ) + if not parsed.hostname: + raise BundlerError(f"Catalog url must be a valid URL with a host: {url}") url = _canonicalize_url(url) install_policy = InstallPolicy.parse(policy) diff --git a/tests/unit/test_bundler_catalog_config.py b/tests/unit/test_bundler_catalog_config.py index e2505ec14d..ddfa81b9ed 100644 --- a/tests/unit/test_bundler_catalog_config.py +++ b/tests/unit/test_bundler_catalog_config.py @@ -222,3 +222,39 @@ def test_add_source_allows_local_path_with_colon(tmp_path: Path, monkeypatch): # A relative path containing ':' but no '://' is still a local path. source = cc.add_source(project, "weird:name.json", policy="install-allowed", priority=50) assert source.url.endswith("weird:name.json") or "weird" in source.url + + +def test_add_source_rejects_plain_http_for_non_localhost(tmp_path: Path): + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + with pytest.raises(BundlerError, match="HTTPS"): + cc.add_source(project, "http://example.com/catalog.json", policy="install-allowed", priority=50) + + +def test_add_source_allows_http_for_localhost(tmp_path: Path): + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + source = cc.add_source(project, "http://localhost:8080/c.json", policy="install-allowed", priority=50) + assert source.url == "http://localhost:8080/c.json" + + +def test_add_source_rejects_host_less_remote_urls(tmp_path: Path): + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + for url in ("https://:8080", "https://user@"): + with pytest.raises(BundlerError, match="host"): + cc.add_source(project, url, policy="install-allowed", priority=50) + + +def test_add_source_wraps_invalid_ipv6_as_bundler_error(tmp_path: Path): + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + with pytest.raises(BundlerError, match="Invalid catalog url"): + cc.add_source(project, "https://[::1/c.json", policy="install-allowed", priority=50) + + +def test_remove_source_does_not_crash_on_invalid_ipv6(tmp_path: Path): + project = tmp_path / "proj" + (project / ".specify").mkdir(parents=True) + with pytest.raises(BundlerError, match="No project-scoped catalog source"): + cc.remove_source(project, "https://[::1/c.json") From cd6ddc7179e747b01410c571910eb905c2f34c9a Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:17:28 +0200 Subject: [PATCH 2/2] docs: correct config filename and validation reference in comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../bundler/commands_impl/catalog_config.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/bundler/commands_impl/catalog_config.py b/src/specify_cli/bundler/commands_impl/catalog_config.py index 591453bf27..54839192c4 100644 --- a/src/specify_cli/bundler/commands_impl/catalog_config.py +++ b/src/specify_cli/bundler/commands_impl/catalog_config.py @@ -156,11 +156,11 @@ def add_source( "Use http(s)://, file://, builtin://, or a local path." ) if parsed.scheme.lower() in {"http", "https"}: - # Mirror specify_cli.catalogs / bundler adapters URL validation - # (#3209/#3210): HTTPS only (HTTP just for localhost), and check - # hostname, not netloc — netloc is truthy for host-less URLs like - # "https://:8080" or "https://user@". Validating here keeps junk out - # of catalogs.yaml instead of failing later at fetch time. + # Mirror specify_cli.catalogs._validate_catalog_url (#3209/#3210): + # HTTPS only (HTTP just for localhost), and check hostname, not + # netloc — netloc is truthy for host-less URLs like "https://:8080" + # or "https://user@". Validating here keeps junk out of + # bundle-catalogs.yml instead of failing later at fetch time. is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1") if parsed.scheme.lower() != "https" and not is_localhost: raise BundlerError(