diff --git a/src/specify_cli/authentication/http.py b/src/specify_cli/authentication/http.py index a2888bcce..782403cc2 100644 --- a/src/specify_cli/authentication/http.py +++ b/src/specify_cli/authentication/http.py @@ -73,6 +73,13 @@ class _StripAuthOnRedirect(urllib.request.HTTPRedirectHandler): 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 @@ class _StripAuthOnRedirect(urllib.request.HTTPRedirectHandler): 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 6821419b3..7f874407a 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 eabfe650d..4e2cb1bbf 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 preset_add( ) 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 006a0b27a..b1eecd619 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -631,7 +631,11 @@ def workflow_add( 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 a89303d3d..917c698db 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -845,6 +845,22 @@ class TestRedirectStripping: 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 9dae5fefd..3118ccdef 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -5441,6 +5441,29 @@ class TestExtensionAddCLI: 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 054018b7a..54cb9dd5b 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -4538,6 +4538,27 @@ class TestBundledPresetLocator: 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 c8e6b5bed..41522d3e8 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -5593,6 +5593,23 @@ class TestWorkflowRemoveGuard: 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)."""