diff --git a/src/specify_cli/extensions/_commands.py b/src/specify_cli/extensions/_commands.py index 3c4b940fdf..7ae4f8e9f0 100644 --- a/src/specify_cli/extensions/_commands.py +++ b/src/specify_cli/extensions/_commands.py @@ -428,10 +428,17 @@ def extension_add( try: parsed = urlparse(from_url) + # Read .hostname inside the try: parsing a malformed authority -- or + # accessing .hostname on one, e.g. an invalid bracketed IPv6 host like + # "https://[not-an-ip]/x.zip" -- can raise ValueError. Keeping both the + # parse and the .hostname read inside the guard surfaces a clean + # "Invalid URL" message instead of leaking a raw traceback past the + # CLI. Reuse the value below. + hostname = parsed.hostname 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") + is_localhost = hostname in ("localhost", "127.0.0.1", "::1") if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost): console.print("[red]Error:[/red] URL must use HTTPS for security.") diff --git a/tests/test_extensions.py b/tests/test_extensions.py index f26dc2bf2f..61826aad5b 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -6607,6 +6607,80 @@ def test_add_from_malformed_ipv6_url_exits_cleanly(self, tmp_path): plain = strip_ansi(result.output) assert "Invalid URL" in plain + def test_add_from_bracketed_non_ip_url_exits_cleanly(self, tmp_path): + """A bracketed-but-invalid IPv6 host must produce a clean error, not a + ValueError traceback. "https://[not-an-ip]/ext.zip" is a malformed + authority that raises ValueError during URL validation; the try/except + guard around parsing and the .hostname read must turn that into a clean + "Invalid URL" message. + """ + 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://[not-an-ip]/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_from_url_lazy_hostname_valueerror_exits_cleanly(self, tmp_path, monkeypatch): + """Synthetic defensive coverage: monkeypatch urlparse() to return an + object whose .hostname raises ValueError lazily. This does not reproduce + any specific CPython behavior -- it just exercises the case where the + ValueError surfaces on the .hostname read rather than at parse time, so a + raw ValueError would leak if .hostname were read outside the try/except. + """ + import urllib.parse + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + real_urlparse = urllib.parse.urlparse + + class _LazyHostnameRaiser: + def __init__(self, parsed): + self._parsed = parsed + + @property + def hostname(self): + raise ValueError("simulated lazy IPv6 hostname failure") + + def __getattr__(self, name): + return getattr(self._parsed, name) + + def _fake_urlparse(url, *args, **kwargs): + return _LazyHostnameRaiser(real_urlparse(url, *args, **kwargs)) + + monkeypatch.setattr(urllib.parse, "urlparse", _fake_urlparse) + + 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://example.com/ext.zip"], + catch_exceptions=True, + ) + + assert result.exit_code == 1 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Invalid URL" in strip_ansi(result.output) + 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 631dc8cda2..193d86b6ae 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -5605,6 +5605,58 @@ def test_preset_add_from_malformed_ipv6_url_exits_cleanly(self, project_dir): assert "Invalid URL" in output open_url.assert_not_called() + def test_preset_add_from_bracketed_non_ip_url_exits_cleanly(self, project_dir): + """A bracketed-but-invalid IPv6 host in --from must exit cleanly. + + "https://[not-an-ip]/preset.zip" is a malformed authority that raises + ValueError during URL validation; the try/except guard around parsing + and the .hostname read must turn that into a clean "Invalid URL" message. + """ + 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://[not-an-ip]/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_out_of_range_port_exits_cleanly(self, project_dir): + """An out-of-range port raises ValueError lazily on .port access. + + The up-front guard reads ``_parsed.port`` (urllib validates the port + range/syntax there) inside its try/except, so "https://example.com:99999/ + preset.zip" must produce a clean "Invalid URL" message rather than + leaking a raw ValueError traceback past the CLI. + """ + 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://example.com:99999/preset.zip"], + catch_exceptions=True, + ) + + assert result.exit_code == 1 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Invalid URL" in strip_ansi(result.output) + open_url.assert_not_called() + def test_preset_add_bracketed_host_download_url_exits_cleanly(self, project_dir): """A catalog download_url with a bracketed non-IP host must render cleanly.