Skip to content

Commit 566974e

Browse files
committed
Harden preset URL installs against unsafe redirects
Preset URL installs already rejected non-HTTPS source URLs, but the authenticated opener follows redirects. Validate the final response URL before writing the ZIP, stream the response to disk, and keep catalog config serialization on safe YAML output. Constraint: open_url follows redirects, so source URL validation alone does not constrain the downloaded target Rejected: Keep response.read() for simplicity | large preset downloads should not be buffered entirely in memory Confidence: high Scope-risk: narrow Directive: Keep preset URL policy aligned with workflow installer redirect validation Tested: uvx ruff check src/specify_cli/presets/_commands.py tests/test_presets.py Tested: uv run pytest tests/test_presets.py -q Not-tested: Real network redirect integration against a live HTTP server
1 parent 3beb632 commit 566974e

2 files changed

Lines changed: 106 additions & 7 deletions

File tree

src/specify_cli/presets/_commands.py

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,25 +101,43 @@ def preset_add(
101101

102102
elif from_url:
103103
# Validate URL scheme before downloading
104+
from ipaddress import ip_address
104105
from urllib.parse import urlparse as _urlparse
106+
105107
_parsed = _urlparse(from_url)
106-
_is_localhost = _parsed.hostname in ("localhost", "127.0.0.1", "::1")
107-
if _parsed.scheme != "https" and not (_parsed.scheme == "http" and _is_localhost):
108-
console.print(f"[red]Error:[/red] URL must use HTTPS (got {_parsed.scheme}://). HTTP is only allowed for localhost.")
108+
109+
def _is_allowed_download_url(parsed_url):
110+
host = parsed_url.hostname or ""
111+
is_loopback = host == "localhost"
112+
if not is_loopback:
113+
try:
114+
is_loopback = ip_address(host).is_loopback
115+
except ValueError:
116+
# Host is not an IP literal (e.g., a regular hostname); treat as non-loopback.
117+
pass
118+
return parsed_url.scheme == "https" or (parsed_url.scheme == "http" and is_loopback)
119+
120+
if not _is_allowed_download_url(_parsed):
121+
console.print(f"[red]Error:[/red] URL must use HTTPS (got {_parsed.scheme}://). HTTP is only allowed for localhost/loopback.")
109122
raise typer.Exit(1)
110123

111124
console.print(f"Installing preset from [cyan]{from_url}[/cyan]...")
112-
import urllib.request
113125
import urllib.error
114126
import tempfile
127+
import shutil
115128

116129
with tempfile.TemporaryDirectory() as tmpdir:
117130
zip_path = Path(tmpdir) / "preset.zip"
118131
try:
119132
from specify_cli.authentication.http import open_url as _open_url
120133

121134
with _open_url(from_url, timeout=60) as response:
122-
zip_path.write_bytes(response.read())
135+
final_url = response.geturl()
136+
if not _is_allowed_download_url(_urlparse(final_url)):
137+
console.print(f"[red]Error:[/red] Preset URL redirected to non-HTTPS URL: {final_url}")
138+
raise typer.Exit(1)
139+
with zip_path.open("wb") as output:
140+
shutil.copyfileobj(response, output)
123141
except urllib.error.URLError as e:
124142
console.print(f"[red]Error:[/red] Failed to download: {e}")
125143
raise typer.Exit(1)
@@ -606,7 +624,7 @@ def preset_catalog_add(
606624
})
607625

608626
config["catalogs"] = catalogs
609-
config_path.write_text(yaml.dump(config, default_flow_style=False, sort_keys=False, allow_unicode=True), encoding="utf-8")
627+
config_path.write_text(yaml.safe_dump(config, default_flow_style=False, sort_keys=False, allow_unicode=True), encoding="utf-8")
610628

611629
install_label = "install allowed" if install_allowed else "discovery only"
612630
console.print(f"\n[green]✓[/green] Added catalog '[bold]{name}[/bold]' ({install_label})")
@@ -648,7 +666,7 @@ def preset_catalog_remove(
648666
raise typer.Exit(1)
649667

650668
config["catalogs"] = catalogs
651-
config_path.write_text(yaml.dump(config, default_flow_style=False, sort_keys=False, allow_unicode=True), encoding="utf-8")
669+
config_path.write_text(yaml.safe_dump(config, default_flow_style=False, sort_keys=False, allow_unicode=True), encoding="utf-8")
652670

653671
console.print(f"[green]✓[/green] Removed catalog '{name}'")
654672
if not catalogs:

tests/test_presets.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@
1111
"""
1212

1313
import pytest
14+
import io
1415
import json
1516
import tempfile
1617
import shutil
1718
import warnings
1819
import zipfile
1920
from pathlib import Path
2021
from datetime import datetime, timezone
22+
from types import SimpleNamespace
2123

2224
import yaml
2325

@@ -3687,6 +3689,85 @@ def test_bundled_preset_add_via_cli(self, project_dir):
36873689
assert "Lean Workflow" in result.output
36883690
assert "installed" in result.output.lower()
36893691

3692+
def test_preset_add_from_url_rejects_insecure_redirect(self, project_dir, monkeypatch):
3693+
"""URL installs reject redirects from HTTPS to non-loopback HTTP."""
3694+
import typer
3695+
from specify_cli.presets._commands import preset_add
3696+
3697+
class FakeResponse(io.BytesIO):
3698+
def __enter__(self):
3699+
return self
3700+
3701+
def __exit__(self, exc_type, exc, tb):
3702+
return False
3703+
3704+
def geturl(self):
3705+
return "http://example.com/preset.zip"
3706+
3707+
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
3708+
monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.0")
3709+
monkeypatch.setattr("specify_cli.authentication.http.open_url", lambda url, timeout: FakeResponse(b"zip"))
3710+
3711+
installed = False
3712+
3713+
def fake_install_from_zip(self, zip_path, speckit_version, priority=10):
3714+
nonlocal installed
3715+
installed = True
3716+
3717+
monkeypatch.setattr(PresetManager, "install_from_zip", fake_install_from_zip)
3718+
3719+
with pytest.raises(typer.Exit) as exc_info:
3720+
preset_add(preset_id=None, from_url="https://example.com/preset.zip", dev=None, priority=10)
3721+
3722+
assert exc_info.value.exit_code == 1
3723+
assert installed is False
3724+
3725+
def test_preset_add_from_url_streams_download_to_zip(self, project_dir, monkeypatch):
3726+
"""URL installs stream response bytes to disk before installing the ZIP."""
3727+
from specify_cli.presets._commands import preset_add
3728+
3729+
class FakeResponse(io.BytesIO):
3730+
def __init__(self, data):
3731+
super().__init__(data)
3732+
self.read_sizes = []
3733+
3734+
def __enter__(self):
3735+
return self
3736+
3737+
def __exit__(self, exc_type, exc, tb):
3738+
return False
3739+
3740+
def geturl(self):
3741+
return "https://example.com/preset.zip"
3742+
3743+
def read(self, size=-1):
3744+
assert size not in (-1, None)
3745+
self.read_sizes.append(size)
3746+
return super().read(size)
3747+
3748+
response = FakeResponse(b"zip-bytes")
3749+
installed = {}
3750+
3751+
def fake_install_from_zip(self, zip_path, speckit_version, priority=10):
3752+
installed["zip_bytes"] = Path(zip_path).read_bytes()
3753+
installed["speckit_version"] = speckit_version
3754+
installed["priority"] = priority
3755+
return SimpleNamespace(name="Test Preset", version="1.0.0")
3756+
3757+
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
3758+
monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.6.0")
3759+
monkeypatch.setattr("specify_cli.authentication.http.open_url", lambda url, timeout: response)
3760+
monkeypatch.setattr(PresetManager, "install_from_zip", fake_install_from_zip)
3761+
3762+
preset_add(preset_id=None, from_url="https://example.com/preset.zip", dev=None, priority=7)
3763+
3764+
assert response.read_sizes
3765+
assert installed == {
3766+
"zip_bytes": b"zip-bytes",
3767+
"speckit_version": "0.6.0",
3768+
"priority": 7,
3769+
}
3770+
36903771
def test_bundled_preset_in_catalog(self):
36913772
"""Verify the lean preset is listed in catalog.json with bundled marker."""
36923773
catalog_path = Path(__file__).parent.parent / "presets" / "catalog.json"

0 commit comments

Comments
 (0)