diff --git a/src/specify_cli/authentication/http.py b/src/specify_cli/authentication/http.py index a2888bcce2..782403cc2f 100644 --- a/src/specify_cli/authentication/http.py +++ b/src/specify_cli/authentication/http.py @@ -73,6 +73,13 @@ def __init__( self._redirect_validator = redirect_validator def redirect_request(self, req, fp, code, msg, headers, newurl): + try: + new_parsed = urlparse(newurl) + except ValueError as exc: + # Malformed redirect target (e.g. unterminated IPv6 bracket). + # Surface as URLError so callers' download error handling applies. + raise urllib.error.URLError(f"malformed redirect URL: {exc}") from exc + if self._redirect_validator is not None: self._redirect_validator(req.full_url, newurl) @@ -83,7 +90,6 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): new_req = super().redirect_request(req, fp, code, msg, headers, newurl) if new_req is not None: old_scheme = urlparse(req.full_url).scheme - new_parsed = urlparse(newurl) hostname = (new_parsed.hostname or "").lower() is_https_downgrade = old_scheme == "https" and new_parsed.scheme != "https" if _hostname_in_hosts(hostname, self._hosts) and not is_https_downgrade: diff --git a/src/specify_cli/extensions/_commands.py b/src/specify_cli/extensions/_commands.py index 6821419b30..7f874407ac 100644 --- a/src/specify_cli/extensions/_commands.py +++ b/src/specify_cli/extensions/_commands.py @@ -426,7 +426,11 @@ def extension_add( if from_url and not dev: from urllib.parse import urlparse - parsed = urlparse(from_url) + try: + parsed = urlparse(from_url) + except ValueError: + console.print(f"[red]Error:[/red] Invalid URL: {_escape_markup(from_url)}") + raise typer.Exit(1) is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1") if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost): diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index eabfe650dd..4e2cb1bbf5 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -104,7 +104,13 @@ def preset_add( from ipaddress import ip_address from urllib.parse import urlparse as _urlparse - _parsed = _urlparse(from_url) + try: + _parsed = _urlparse(from_url) + except ValueError: + from rich.markup import escape as _escape_markup + + console.print(f"[red]Error:[/red] Invalid URL: {_escape_markup(from_url)}") + raise typer.Exit(1) def _is_allowed_download_url(parsed_url): host = parsed_url.hostname @@ -135,7 +141,9 @@ def _validate_download_redirect(old_url, new_url): ) raise typer.Exit(1) - console.print(f"Installing preset from [cyan]{from_url}[/cyan]...") + from rich.markup import escape as _esc + + console.print(f"Installing preset from [cyan]{_esc(from_url)}[/cyan]...") import urllib.error import tempfile import shutil diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index 095e3e415e..11b7d55f0a 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -617,7 +617,11 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: from urllib.parse import urlparse from specify_cli.authentication.http import open_url as _open_url - parsed_src = urlparse(source) + try: + parsed_src = urlparse(source) + except ValueError: + console.print(f"[red]Error:[/red] Invalid URL: {_escape_markup(source)}") + raise typer.Exit(1) src_host = parsed_src.hostname or "" src_loopback = src_host == "localhost" if not src_loopback: diff --git a/tests/test_authentication.py b/tests/test_authentication.py index a89303d3d8..917c698db0 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -845,6 +845,22 @@ def test_multi_hop_redirect_within_hosts_preserves_auth(self): auth3 = req3.get_header("Authorization") or req3.unredirected_hdrs.get("Authorization") assert auth3 == "Bearer tok" + def test_malformed_redirect_url_raises_urlerror_not_valueerror(self): + """A redirect to a malformed URL (unterminated IPv6 bracket) surfaces + as URLError, which download paths already handle, rather than an + unhandled ValueError traceback.""" + import urllib.error + from specify_cli.authentication.http import _StripAuthOnRedirect + from urllib.request import Request + import io + + handler = _StripAuthOnRedirect(("github.com",)) + req = Request("https://github.com/org/repo") + + with pytest.raises(urllib.error.URLError): + handler.redirect_request(req, io.BytesIO(b""), 302, "Found", {}, + "https://[::1/asset") + # --------------------------------------------------------------------------- # _fetch_latest_release_tag delegation diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 2a4b2aa660..c67590d2ad 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -5396,6 +5396,29 @@ def record_status(*args, **kwargs): f"confirm must precede spinner, got: {call_order}" assert result.exit_code == 0 # user declined → clean exit + def test_add_from_malformed_ipv6_url_exits_cleanly(self, tmp_path): + """A malformed IPv6 URL must produce a clean error, not a ValueError traceback.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + project_dir = tmp_path / "test-project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + + runner = CliRunner() + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke( + app, + ["extension", "add", "my-ext", "--from", "https://[::1/ext.zip"], + catch_exceptions=True, + ) + + assert result.exit_code == 1 + assert result.exception is None or isinstance(result.exception, SystemExit) + plain = strip_ansi(result.output) + assert "Invalid URL" in plain + def test_add_status_escapes_extension_markup(self, tmp_path): """User-controlled extension names must not be parsed as Rich markup.""" from rich.markup import escape as escape_markup diff --git a/tests/test_presets.py b/tests/test_presets.py index 054018b7a0..54cb9dd5b3 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -4538,6 +4538,27 @@ def test_preset_add_from_url_rejects_hostless_https_url(self, project_dir): assert "got https://" not in output open_url.assert_not_called() + def test_preset_add_from_malformed_ipv6_url_exits_cleanly(self, project_dir): + """A malformed IPv6 URL must produce a clean error, not a ValueError traceback.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + with patch.object(Path, "cwd", return_value=project_dir), \ + patch("specify_cli.authentication.http.open_url") as open_url: + result = runner.invoke( + app, + ["preset", "add", "--from", "https://[::1/preset.zip"], + catch_exceptions=True, + ) + + assert result.exit_code == 1 + assert result.exception is None or isinstance(result.exception, SystemExit) + output = strip_ansi(result.output) + assert "Invalid URL" in output + open_url.assert_not_called() + def test_preset_add_from_url_redirect_error_describes_disallowed_url(self, project_dir, monkeypatch, capsys): """Redirect rejection message covers hostless HTTPS, not only non-HTTPS URLs.""" import typer diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2fdbf887b3..459112282c 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -5427,6 +5427,23 @@ def test_remove_refuses_non_directory_workflow_path(self, project_dir, monkeypat class TestWorkflowAddSymlinkGuard: + def test_add_malformed_ipv6_url_exits_cleanly(self, temp_dir, monkeypatch): + """A malformed IPv6 URL must produce a clean error, not a ValueError traceback.""" + from typer.testing import CliRunner + from specify_cli import app + + (temp_dir / ".specify").mkdir(exist_ok=True) + monkeypatch.chdir(temp_dir) + result = CliRunner().invoke( + app, + ["workflow", "add", "https://[::1/wf.yaml"], + catch_exceptions=True, + ) + + assert result.exit_code == 1 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Invalid URL" in result.output + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") def test_add_refuses_symlinked_specify(self, temp_dir, monkeypatch): """workflow add must refuse a symlinked .specify (writes could escape root)."""