diff --git a/src/specify_cli/bundler/commands_impl/catalog_config.py b/src/specify_cli/bundler/commands_impl/catalog_config.py index e0650ce4b5..54839192c4 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._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( + 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")