Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/specify_cli/extensions/_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
74 changes: 74 additions & 0 deletions tests/test_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 52 additions & 0 deletions tests/test_presets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down