diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 99442e7033..08b0b7846d 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -3082,6 +3082,43 @@ def extension_add( manager = ExtensionManager(project_root) speckit_version = get_speckit_version() + # Prompt for URL-based installs BEFORE the spinner so the user can + # actually see and respond to the confirmation (the Rich status + # spinner overwrites the typer.confirm prompt line, making it appear + # as though the command is hung). + # Guard with ``not dev`` so that --dev + --from does not show a + # confusing confirmation for a URL that will be ignored. + if from_url and not dev: + from urllib.parse import urlparse + from rich.markup import escape as _escape_markup + + parsed = urlparse(from_url) + is_localhost = parsed.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.") + console.print("HTTP is only allowed for localhost URLs.") + raise typer.Exit(1) + + safe_url = _escape_markup(from_url) + + # Warn about untrusted sources — default-deny confirmation + console.print() + console.print(Panel( + f"[bold]You are installing an extension from an external URL that is not\n" + f"listed in any of your configured extension catalogs.[/bold]\n\n" + f"URL: {safe_url}\n\n" + f"Only install extensions from sources you trust.", + title="[bold yellow]⚠ Untrusted Source[/bold yellow]", + border_style="yellow", + padding=(1, 2), + )) + console.print() + confirm = typer.confirm("Continue with installation?", default=False) + if not confirm: + console.print("Cancelled") + raise typer.Exit(0) + try: with console.status(f"[cyan]Installing extension: {extension}[/cyan]"): if dev: @@ -3104,37 +3141,9 @@ def extension_add( elif from_url: # Install from URL (ZIP file) - import urllib.request import urllib.error - from urllib.parse import urlparse - - # Validate URL - parsed = urlparse(from_url) - is_localhost = parsed.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.") - console.print("HTTP is only allowed for localhost URLs.") - raise typer.Exit(1) - - # Warn about untrusted sources — default-deny confirmation - console.print() - console.print(Panel( - f"[bold]You are installing an extension from an external URL that is not\n" - f"listed in any of your configured extension catalogs.[/bold]\n\n" - f"URL: {from_url}\n\n" - f"Only install extensions from sources you trust.", - title="[bold yellow]⚠ Untrusted Source[/bold yellow]", - border_style="yellow", - padding=(1, 2), - )) - console.print() - confirm = typer.confirm("Continue with installation?", default=False) - if not confirm: - console.print("Cancelled") - raise typer.Exit(0) - console.print(f"Downloading from {from_url}...") + console.print(f"Downloading from {safe_url}...") # Download ZIP to temp location download_dir = project_root / ".specify" / "extensions" / ".cache" / "downloads" @@ -3151,7 +3160,7 @@ def extension_add( # Install from downloaded ZIP manifest = manager.install_from_zip(zip_path, speckit_version, priority=priority) except urllib.error.URLError as e: - console.print(f"[red]Error:[/red] Failed to download from {from_url}: {e}") + console.print(f"[red]Error:[/red] Failed to download from {safe_url}: {e}") raise typer.Exit(1) finally: # Clean up downloaded ZIP diff --git a/tests/test_extensions.py b/tests/test_extensions.py index b26a4b8f2a..76eb9d4983 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3807,6 +3807,67 @@ def test_add_bundled_extension_not_found_gives_clear_error(self, tmp_path): assert "bundled with spec-kit" in result.output assert "reinstall" in result.output.lower() + def test_add_from_url_prompts_before_spinner(self, tmp_path): + """Confirm prompt for --from must fire before the console.status spinner. + + Regression test for #2783: typer.confirm() inside console.status() + was overwritten by the Rich spinner, making the command appear hung. + """ + from typer.testing import CliRunner + from unittest.mock import patch, MagicMock + from specify_cli import app + + project_dir = tmp_path / "test-project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + + call_order: list[str] = [] + + original_status = MagicMock() + + def record_status(*args, **kwargs): + call_order.append("spinner") + return original_status + + runner = CliRunner() + with patch.object(Path, "cwd", return_value=project_dir), \ + patch("specify_cli.console.status", side_effect=record_status), \ + patch("typer.confirm", side_effect=lambda *a, **kw: (call_order.append("confirm"), False)[-1]): + result = runner.invoke( + app, + ["extension", "add", "my-ext", "--from", "https://example.com/ext.zip"], + catch_exceptions=True, + ) + + assert "confirm" in call_order, "confirm prompt was never called" + # The confirm must fire BEFORE the spinner is entered + if "spinner" in call_order: + assert call_order.index("confirm") < call_order.index("spinner"), \ + f"confirm must precede spinner, got: {call_order}" + assert result.exit_code == 0 # user declined → clean exit + + def test_add_from_url_cancel_exits_cleanly(self, tmp_path): + """Declining the --from confirmation should exit with code 0.""" + 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), \ + patch("typer.confirm", return_value=False): + result = runner.invoke( + app, + ["extension", "add", "my-ext", "--from", "https://example.com/ext.zip"], + catch_exceptions=True, + ) + + assert result.exit_code == 0 + assert "Cancelled" in result.output + class TestDownloadExtensionBundled: """Tests for download_extension handling of bundled extensions."""