Skip to content

Commit bbc5f17

Browse files
mnriemCopilot
andauthored
fix(extensions): apply GHES auth and resolve release assets for extension add --from (#3217)
* fix(extensions): apply GHES auth and resolve release assets for --from The 'specify extension add --from <url>' path fetched ZIPs via a bare open_url with no GitHub release-asset resolution and no Accept header, diverging from the catalog download path. Against GHES it received an HTML login page and failed obscurely with zipfile.BadZipFile. Route --from through ExtensionCatalog so configured GHES credentials apply and release-download URLs resolve via /api/v3, and reject non-ZIP content with a clear error pointing at auth.json. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(extensions): use zipfile.is_zipfile for --from content guard Replace the weak zip_data.startswith(b"PK") prefix check with zipfile.is_zipfile() on a BytesIO so any non-ZIP payload (not just those lacking the PK magic) is rejected with the friendly error before install_from_zip can raise BadZipFile. Addresses PR review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ac47178 commit bbc5f17

2 files changed

Lines changed: 126 additions & 6 deletions

File tree

src/specify_cli/extensions/_commands.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ def extension_add(
482482

483483
elif from_url:
484484
# Install from URL (ZIP file)
485+
import io
485486
import urllib.error
486487

487488
console.print(f"Downloading from {safe_url}...")
@@ -498,10 +499,33 @@ def extension_add(
498499
zip_path = Path(download_file.name)
499500

500501
try:
501-
from specify_cli.authentication.http import open_url as _open_url
502-
503-
with _open_url(from_url, timeout=60) as response:
502+
# Use the catalog's authenticated fetch so configured
503+
# credentials (incl. GitHub Enterprise Server) are applied
504+
# and GHES release-asset URLs resolve via /api/v3 — keeping
505+
# --from consistent with catalog-based installs.
506+
dl_catalog = ExtensionCatalog(project_root)
507+
download_url = from_url
508+
extra_headers = None
509+
resolved_url = dl_catalog._resolve_github_release_asset_api_url(download_url)
510+
if resolved_url:
511+
download_url = resolved_url
512+
extra_headers = {"Accept": "application/octet-stream"}
513+
514+
with dl_catalog._open_url(
515+
download_url, timeout=60, extra_headers=extra_headers
516+
) as response:
504517
zip_data = response.read()
518+
519+
if not zipfile.is_zipfile(io.BytesIO(zip_data)):
520+
console.print(
521+
f"[red]Error:[/red] {safe_url} did not return a ZIP archive "
522+
f"(got {len(zip_data)} bytes). This usually means the request "
523+
f"was not authenticated and a login/HTML page was returned. "
524+
f"Verify the URL is correct and that credentials for its host "
525+
f"are configured in ~/.specify/auth.json."
526+
)
527+
raise typer.Exit(1)
528+
505529
zip_path.write_bytes(zip_data)
506530

507531
# Install from downloaded ZIP

tests/test_extensions.py

Lines changed: 99 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@
4040
version_satisfies,
4141
)
4242

43+
# Minimal valid ZIP (empty end-of-central-directory record). Passes
44+
# zipfile.is_zipfile() so --from download tests exercise the content guard.
45+
_MINIMAL_ZIP_BYTES = b"PK\x05\x06" + b"\x00" * 18
46+
4347

4448
def can_create_symlink(tmp_path: Path) -> bool:
4549
"""Return True when the current platform/user can create file symlinks."""
@@ -5378,7 +5382,7 @@ def fake_install_from_zip(self_obj, zip_path, speckit_version, priority=10, forc
53785382
runner = CliRunner()
53795383
with patch.object(Path, "cwd", return_value=project_dir), \
53805384
patch("typer.confirm", return_value=True), \
5381-
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(b"zip-bytes")), \
5385+
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(_MINIMAL_ZIP_BYTES)), \
53825386
patch.object(ExtensionManager, "install_from_zip", fake_install_from_zip), \
53835387
patch.object(ExtensionRegistry, "get", return_value={}):
53845388
result = runner.invoke(
@@ -5446,6 +5450,98 @@ def test_add_from_url_escapes_download_exception_markup(self, tmp_path):
54465450
assert "https://example.com/[red]ext[/red].zip" in result.output
54475451
assert "bad [red]download[/red]" in result.output
54485452

5453+
def test_add_from_url_rejects_non_zip_login_page(self, tmp_path):
5454+
"""An HTML login page (unauthenticated fetch) must fail clearly, not BadZipFile."""
5455+
import io
5456+
from typer.testing import CliRunner
5457+
from unittest.mock import patch
5458+
from specify_cli import app
5459+
5460+
class FakeResponse(io.BytesIO):
5461+
def __enter__(self):
5462+
return self
5463+
5464+
def __exit__(self, exc_type, exc, tb):
5465+
return False
5466+
5467+
project_dir = tmp_path / "test-project"
5468+
project_dir.mkdir()
5469+
(project_dir / ".specify").mkdir()
5470+
5471+
runner = CliRunner()
5472+
with patch.object(Path, "cwd", return_value=project_dir), \
5473+
patch("typer.confirm", return_value=True), \
5474+
patch(
5475+
"specify_cli.authentication.http.open_url",
5476+
return_value=FakeResponse(b"<!DOCTYPE html><html>Sign in</html>"),
5477+
), \
5478+
patch.object(ExtensionManager, "install_from_zip") as install:
5479+
result = runner.invoke(
5480+
app,
5481+
["extension", "add", "my-ext", "--from", "https://raw.ghe.example/o/r/ext.zip"],
5482+
catch_exceptions=True,
5483+
)
5484+
5485+
assert result.exit_code == 1, result.output
5486+
assert "did not return a ZIP archive" in result.output
5487+
install.assert_not_called()
5488+
5489+
def test_add_from_url_resolves_ghes_release_asset(self, tmp_path):
5490+
"""A GHES release-download URL resolves to /api/v3 with octet-stream Accept."""
5491+
import io
5492+
from types import SimpleNamespace
5493+
from typer.testing import CliRunner
5494+
from unittest.mock import patch
5495+
from specify_cli import app
5496+
import json
5497+
5498+
class FakeResponse(io.BytesIO):
5499+
def __enter__(self):
5500+
return self
5501+
5502+
def __exit__(self, exc_type, exc, tb):
5503+
return False
5504+
5505+
project_dir = tmp_path / "test-project"
5506+
project_dir.mkdir()
5507+
(project_dir / ".specify").mkdir()
5508+
seen = {}
5509+
5510+
def fake_open_url(url, timeout=10, extra_headers=None, redirect_validator=None):
5511+
if "/releases/tags/" in url:
5512+
body = json.dumps({
5513+
"assets": [{
5514+
"name": "ext.zip",
5515+
"url": "https://ghes.example/api/v3/repos/org/repo/releases/assets/42",
5516+
}]
5517+
}).encode()
5518+
return FakeResponse(body)
5519+
seen["url"] = url
5520+
seen["headers"] = extra_headers
5521+
return FakeResponse(_MINIMAL_ZIP_BYTES)
5522+
5523+
def fake_install(self_obj, zip_path, speckit_version, priority=10, force=False):
5524+
return SimpleNamespace(
5525+
id="x", name="X", version="1.0.0", description="", warnings=[], commands=[], hooks=[]
5526+
)
5527+
5528+
runner = CliRunner()
5529+
with patch.object(Path, "cwd", return_value=project_dir), \
5530+
patch("typer.confirm", return_value=True), \
5531+
patch("specify_cli.authentication.http.github_provider_hosts", return_value=("ghes.example",)), \
5532+
patch("specify_cli.authentication.http.open_url", side_effect=fake_open_url), \
5533+
patch.object(ExtensionManager, "install_from_zip", fake_install):
5534+
result = runner.invoke(
5535+
app,
5536+
["extension", "add", "x", "--from",
5537+
"https://ghes.example/org/repo/releases/download/v1.0/ext.zip"],
5538+
catch_exceptions=True,
5539+
)
5540+
5541+
assert result.exit_code == 0, result.output
5542+
assert "/api/v3/repos/org/repo/releases/assets/" in seen["url"]
5543+
assert seen["headers"] == {"Accept": "application/octet-stream"}
5544+
54495545
@pytest.mark.parametrize(
54505546
("exc_type", "label"),
54515547
[
@@ -5523,7 +5619,7 @@ def fake_install_from_zip(self_obj, zip_path, speckit_version, priority=10, forc
55235619
runner = CliRunner()
55245620
with patch.object(Path, "cwd", return_value=project_dir), \
55255621
patch("typer.confirm", return_value=True), \
5526-
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(b"zip-bytes")), \
5622+
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(_MINIMAL_ZIP_BYTES)), \
55275623
patch.object(ExtensionManager, "install_from_zip", fake_install_from_zip):
55285624
result = runner.invoke(
55295625
app,
@@ -5532,7 +5628,7 @@ def fake_install_from_zip(self_obj, zip_path, speckit_version, priority=10, forc
55325628
)
55335629

55345630
assert result.exit_code == 0
5535-
assert installed["zip_bytes"] == b"zip-bytes"
5631+
assert installed["zip_bytes"] == _MINIMAL_ZIP_BYTES
55365632
assert installed["zip_path"].resolve().is_relative_to(downloads_dir.resolve())
55375633
assert installed["zip_path"].name.startswith("extension-url-download-")
55385634
assert not installed["zip_path"].exists()

0 commit comments

Comments
 (0)