Skip to content

Commit ab5e033

Browse files
fix(extension): reuse bounded URL archive download path
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
1 parent 9d42e26 commit ab5e033

2 files changed

Lines changed: 77 additions & 40 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ def _install_extension_archive(
768768
)
769769

770770
if not tarfile.is_tarfile(archive_path):
771-
raise ValidationError("Extension archive must be a ZIP, .tar.gz, or .tgz file")
771+
raise ValidationError("Extension archive must be a ZIP, .tar, .tar.gz, or .tgz file")
772772

773773
with tempfile.TemporaryDirectory() as tmpdir:
774774
temp_path = Path(tmpdir)
@@ -3695,45 +3695,14 @@ def extension_add(
36953695
manifest = manager.install_from_directory(source_path, speckit_version, priority=priority)
36963696

36973697
elif from_url:
3698-
# Install from URL (ZIP file)
3699-
import urllib.request
3700-
import urllib.error
3701-
from urllib.parse import urlparse
3702-
3703-
# Validate URL
3704-
parsed = urlparse(from_url)
3705-
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
3706-
3707-
if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost):
3708-
console.print("[red]Error:[/red] URL must use HTTPS for security.")
3709-
console.print("HTTP is only allowed for localhost URLs.")
3710-
raise typer.Exit(1)
3711-
3712-
# Warn about untrusted sources
3713-
display_url = _redact_url_for_display(from_url)
3714-
console.print("[yellow]Warning:[/yellow] Installing from external URL.")
3715-
console.print("Only install extensions from sources you trust.\n")
3716-
console.print(f"Downloading from {display_url}...")
3717-
3718-
# Download ZIP to temp location
3719-
download_dir = project_root / ".specify" / "extensions" / ".cache" / "downloads"
3720-
download_dir.mkdir(parents=True, exist_ok=True)
3721-
zip_path = download_dir / f"{extension}-url-download.zip"
3722-
3723-
try:
3724-
with urllib.request.urlopen(from_url, timeout=60) as response:
3725-
zip_data = response.read()
3726-
zip_path.write_bytes(zip_data)
3727-
3728-
# Install from downloaded ZIP
3729-
manifest = manager.install_from_zip(zip_path, speckit_version, priority=priority)
3730-
except urllib.error.URLError as e:
3731-
console.print(f"[red]Error:[/red] Failed to download from {display_url}: {e}")
3732-
raise typer.Exit(1)
3733-
finally:
3734-
# Clean up downloaded ZIP
3735-
if zip_path.exists():
3736-
zip_path.unlink()
3698+
# Install from URL using bounded chunked download + archive validation.
3699+
manifest = _download_and_install_extension_url(
3700+
manager,
3701+
project_root,
3702+
from_url,
3703+
speckit_version,
3704+
priority=priority,
3705+
)
37373706

37383707
else:
37393708
# Try bundled extensions first (shipped with spec-kit)

tests/integrations/test_cli.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,20 @@ def test_tar_extension_archive_rejects_special_members(self, tmp_path):
408408
with pytest.raises(ValidationError, match="Unsupported TAR member type"):
409409
_install_extension_archive(object(), archive_path, "0.0.0")
410410

411+
def test_extension_archive_error_message_lists_plain_tar(self, tmp_path):
412+
"""Unsupported extension archive message should include plain .tar."""
413+
from specify_cli import _install_extension_archive
414+
from specify_cli.extensions import ValidationError
415+
416+
archive_path = tmp_path / "not-an-archive.txt"
417+
archive_path.write_text("not an archive", encoding="utf-8")
418+
419+
with pytest.raises(
420+
ValidationError,
421+
match=r"ZIP, \.tar, \.tar\.gz, or \.tgz",
422+
):
423+
_install_extension_archive(object(), archive_path, "0.0.0")
424+
411425
def test_extension_url_downloads_in_bounded_chunks(self, tmp_path, monkeypatch):
412426
"""URL extension downloads stream to disk instead of reading all bytes."""
413427
import urllib.request
@@ -464,6 +478,60 @@ def fake_install(manager, archive_path, speckit_version, priority=10):
464478
specify_cli.DOWNLOAD_CHUNK_BYTES,
465479
]
466480

481+
def test_extension_add_from_url_uses_shared_bounded_download_helper(self, tmp_path, monkeypatch):
482+
"""extension add --from should reuse the bounded URL download helper."""
483+
from types import SimpleNamespace
484+
from typer.testing import CliRunner
485+
import specify_cli
486+
from specify_cli import app
487+
from specify_cli.extensions import ExtensionManager
488+
489+
project = tmp_path / "url-extension-add"
490+
project.mkdir()
491+
(project / ".specify").mkdir()
492+
493+
captured = {}
494+
495+
def fake_download_and_install(manager, project_path, source_url, speckit_version, priority=10):
496+
captured["manager"] = manager
497+
captured["project_path"] = project_path
498+
captured["source_url"] = source_url
499+
captured["speckit_version"] = speckit_version
500+
captured["priority"] = priority
501+
return SimpleNamespace(
502+
id="url-ext",
503+
name="URL Extension",
504+
version="1.0.0",
505+
description="Downloaded from URL",
506+
warnings=[],
507+
commands=[],
508+
)
509+
510+
monkeypatch.setattr(
511+
specify_cli,
512+
"_download_and_install_extension_url",
513+
fake_download_and_install,
514+
)
515+
516+
runner = CliRunner()
517+
old_cwd = os.getcwd()
518+
try:
519+
os.chdir(project)
520+
result = runner.invoke(
521+
app,
522+
["extension", "add", "url-ext", "--from", "https://example.com/url-ext.zip"],
523+
catch_exceptions=False,
524+
)
525+
finally:
526+
os.chdir(old_cwd)
527+
528+
assert result.exit_code == 0, result.output
529+
assert isinstance(captured["manager"], ExtensionManager)
530+
assert captured["project_path"] == project
531+
assert captured["source_url"] == "https://example.com/url-ext.zip"
532+
assert captured["speckit_version"] == specify_cli.get_speckit_version()
533+
assert captured["priority"] == 10
534+
467535

468536
class TestForceExistingDirectory:
469537
"""Tests for --force merging into an existing named directory."""

0 commit comments

Comments
 (0)