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
8 changes: 7 additions & 1 deletion src/specify_cli/authentication/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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:
Expand Down
6 changes: 5 additions & 1 deletion src/specify_cli/extensions/_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
12 changes: 10 additions & 2 deletions src/specify_cli/presets/_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
mnriem marked this conversation as resolved.

def _is_allowed_download_url(parsed_url):
host = parsed_url.hostname
Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion src/specify_cli/workflows/_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
16 changes: 16 additions & 0 deletions tests/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions tests/test_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions tests/test_presets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)."""
Expand Down
Loading