mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(bundle): surface a clean BundlerError on a malformed bundle download URL (#3586)
`_download_manifest` and its `_require_https` helper parsed the catalog entry's `download_url` with an unguarded `urlparse(url)`. A malformed authority — e.g. an unclosed IPv6 bracket like `https://[::1` — makes `urlparse` (or `.hostname` on older Pythons) raise a raw `ValueError`. The three `bundle` CLI commands (`info`, `install`, `update`) only catch `BundlerError`, so that `ValueError` escaped as an uncaught traceback. Wrap both parse sites in the same `try/except ValueError -> BundlerError` guard already used by the sibling `_validate_remote_url` (and established by the merged catalog-URL fix #3576), so a bad `download_url` reports a clean, actionable error in every mode. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -765,7 +765,16 @@ def _download_manifest(resolved, *, offline: bool):
|
||||
f"Catalog entry '{resolved.entry.id}' has no download_url; cannot resolve "
|
||||
"its manifest."
|
||||
)
|
||||
parsed = urlparse(url)
|
||||
# A malformed authority (e.g. an unclosed IPv6 bracket ``https://[::1``)
|
||||
# makes urlparse raise ValueError. Surface it as the documented
|
||||
# BundlerError, like the sibling ``_validate_remote_url``, rather than
|
||||
# leaking a raw ValueError past the callers, which only catch BundlerError.
|
||||
try:
|
||||
parsed = urlparse(url)
|
||||
except ValueError:
|
||||
raise BundlerError(
|
||||
f"Catalog entry '{resolved.entry.id}' has a malformed download_url: {url}"
|
||||
) from None
|
||||
scheme = parsed.scheme.lower()
|
||||
|
||||
# ``file://`` URLs and bare filesystem paths (including Windows drive paths
|
||||
@@ -802,8 +811,17 @@ def _download_manifest(resolved, *, offline: bool):
|
||||
def _require_https(label: str, url: str) -> None:
|
||||
from urllib.parse import urlparse
|
||||
|
||||
parsed = urlparse(url)
|
||||
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
|
||||
# urlparse / hostname access raise ValueError on a malformed authority;
|
||||
# keep the documented BundlerError contract (older Pythons surface this via
|
||||
# the .hostname access below rather than at the urlparse call).
|
||||
try:
|
||||
parsed = urlparse(url)
|
||||
hostname = parsed.hostname
|
||||
except ValueError:
|
||||
raise BundlerError(
|
||||
f"Refusing to download {label}: URL is malformed: {url}"
|
||||
) from None
|
||||
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
|
||||
if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost):
|
||||
raise BundlerError(
|
||||
f"Refusing to download {label} over non-HTTPS URL: {url}"
|
||||
|
||||
42
tests/unit/test_bundle_download_url.py
Normal file
42
tests/unit/test_bundle_download_url.py
Normal file
@@ -0,0 +1,42 @@
|
||||
"""Unit tests for malformed download-URL handling in bundle manifest resolution."""
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from specify_cli.bundler import BundlerError
|
||||
from specify_cli.commands.bundle import _download_manifest, _require_https
|
||||
|
||||
_MALFORMED_URLS = [
|
||||
"https://[::1", # unclosed IPv6 bracket
|
||||
"https://[not-an-ip]/bundle.yml",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("url", _MALFORMED_URLS)
|
||||
def test_download_manifest_rejects_malformed_url_cleanly(url):
|
||||
"""A malformed download_url must raise BundlerError, not a raw ValueError.
|
||||
|
||||
``urlparse`` raises ``ValueError`` on a malformed authority (e.g. an
|
||||
unclosed IPv6 bracket). The bundle CLI commands only catch BundlerError, so
|
||||
a raw ValueError would escape as an uncaught traceback. Sibling of the
|
||||
guarded ``_validate_remote_url`` (adapters) and the merged #3576 fix.
|
||||
"""
|
||||
resolved = SimpleNamespace(
|
||||
entry=SimpleNamespace(id="mybundle", download_url=url)
|
||||
)
|
||||
with pytest.raises(BundlerError):
|
||||
_download_manifest(resolved, offline=True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("url", _MALFORMED_URLS)
|
||||
def test_require_https_rejects_malformed_url_cleanly(url):
|
||||
"""``_require_https`` must also surface BundlerError on a malformed authority.
|
||||
|
||||
On older Python versions the ValueError is raised at ``.hostname`` access
|
||||
rather than at ``urlparse``, so guarding both keeps the contract across the
|
||||
CI Python matrix.
|
||||
"""
|
||||
with pytest.raises(BundlerError):
|
||||
_require_https("bundle 'x'", url)
|
||||
Reference in New Issue
Block a user