fix(cli): exit cleanly on malformed IPv6 URLs in extension/preset/workflow add (#3369)

* fix(cli): exit cleanly on malformed IPv6 URLs in extension/preset/workflow add

extension add --from, preset add --from, and workflow add <url> parsed
the user-supplied URL with a bare urlparse before their HTTPS/host
validation, so an unclosed IPv6 bracket escaped as a raw ValueError
traceback. Wrap each parse and emit the surrounding validation's clean
error style + typer.Exit(1) instead.

Fixes #3368

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): convert malformed redirect URLs to URLError in shared redirect handler

Parse the redirect target once in _StripAuthOnRedirect.redirect_request
before the validator and stdlib handler run, converting ValueError into
URLError which every download path already catches. Also escape from_url
in the preset install message so IPv6 brackets don't break Rich markup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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:

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):

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

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:

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

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

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

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)."""