fix(bundle): reject file:// / local download_url — catalog URLs are HTTPS-only (#3344)

* fix(bundle): resolve file:// download_url via the file-URL helper

_download_manifest built the local path from raw parsed.path, which
keeps the leading slash of file:///C:/x (yielding a \C:\x path that
never exists on Windows) and skips percent-decoding (my%20bundles stays
encoded on every OS) — so a catalog entry whose download_url is the
canonical URI Python itself produces via Path.as_uri() always fails
with 'Bundle manifest not found'. Route the file scheme through the
existing bundler.services.adapters._file_url_to_path helper, which
already handles drive letters, UNC hosts, and percent-decoding for
catalog file:// URLs (make_catalog_fetcher). The bare-path branch is
unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(bundle): reject file:// / local download_url; catalog URLs are HTTPS-only

Per maintainer review (route B): file:// in a catalog download_url was
never intended — catalog URLs are HTTPS-only (http for localhost) across
the extensions/presets/workflows catalog systems, and disk installs go
through the positional path (specify bundle install <path>), handled by
_local_manifest_source before catalog resolution. Remove the
file:///bare-path branch from _download_manifest and route everything
through _download_remote_manifest (HTTPS-only via _require_https), with an
actionable error pointing at the positional install. Invert the file://
tests to assert rejection (+ a positional-path resolution test), and
migrate the three bundle-info contract tests off local download_urls onto
an HTTPS-only entry with a mocked manifest fetch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(bundle): validate HTTPS before the offline gate in _download_manifest

Per review: for a non-local download_url the offline check ran before any
URL validation, so an invalid/non-HTTPS scheme surfaced a misleading
'Network access disabled' error under --offline when the real problem is
the URL would be rejected even online. Call _require_https before the
offline gate so the correct error is reported in every mode.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(bundle): reword non-HTTPS download_url error to not mislabel scheme-less URLs

A scheme-less download_url (urlparse scheme == "") can be a bare
filesystem path OR a missing-scheme value like 'example.com/foo.zip',
not necessarily file://. Reword the reject error to state the real
HTTPS-only constraint and enumerate what is rejected (file://, local
path, scheme-less), instead of labeling every case 'local/file://'.

Behavior unchanged; the 'bundle install' actionable hint is preserved,
so the existing reject-path tests still pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Ali jawwad
2026-07-13 19:15:55 +05:00
committed by GitHub
parent e59da78677
commit ba1f13a8b1
3 changed files with 124 additions and 29 deletions

View File

@@ -746,11 +746,16 @@ def _resolve_manifest_path(path: Path | None) -> Path:
def _download_manifest(resolved, *, offline: bool):
"""Resolve a bundle's manifest from its catalog ``download_url``.
Local/``file://`` URLs always work offline and may point at a ``.zip``
artifact, a bundle directory, or a ``bundle.yml`` (handled by
:func:`_local_manifest_source`). Remote ``https://`` URLs are fetched with
the shared authenticated, redirect-validated HTTP client, and only when not
``--offline``.
Catalog ``download_url``s are HTTPS-only (``http`` allowed for localhost),
matching the extensions/presets/workflows catalog systems. Remote URLs are
fetched with the shared authenticated, redirect-validated HTTP client, and
only when not ``--offline``.
Local and ``file://`` sources are intentionally not resolved here: to
install a bundle from disk, pass the path positionally
(``specify bundle install ./path/to/bundle.yml`` — a bundle directory or a
``.zip`` artifact also works), which :func:`_local_manifest_source` handles
before catalog resolution and which never touches ``download_url``.
"""
from urllib.parse import urlparse
@@ -763,26 +768,35 @@ def _download_manifest(resolved, *, offline: bool):
parsed = urlparse(url)
scheme = parsed.scheme.lower()
# On Windows an absolute path like ``C:\bundle.yml`` parses with a
# single-letter ``scheme``; treat it as a local file, not a URL scheme.
# ``file://`` URLs and bare filesystem paths (including Windows drive paths
# like ``C:\bundle.yml``, which urlparse reads as a single-letter scheme)
# are not valid catalog download URLs. Catalog URLs are HTTPS-only across
# every catalog system; installing from disk is done by passing the path
# positionally, which never reaches URL resolution. Give an actionable
# error rather than accepting a scheme the rest of the codebase rejects.
if scheme in ("", "file") or re.match(r"^[A-Za-z]:[\\/]", url):
local = Path(parsed.path if scheme == "file" else url)
manifest = _local_manifest_source(str(local))
if manifest is None:
raise BundlerError(f"Bundle manifest not found: {local}")
return manifest
raise BundlerError(
f"Catalog entry '{resolved.entry.id}' has a non-HTTP(S) download_url "
f"({url}); catalog download URLs must be HTTPS (http for localhost) — "
"a file:// URL, a local filesystem path, or a scheme-less value "
"(e.g. 'example.com/bundle.zip') is not accepted. "
"To install a bundle from disk, pass the path directly: "
"'specify bundle install <path-to-bundle.yml | bundle-dir | .zip>'."
)
if scheme in ("http", "https"):
if offline:
raise BundlerError(
f"Network access disabled; cannot download bundle '{resolved.entry.id}' "
f"from {url}."
)
return _download_remote_manifest(resolved.entry.id, url)
# Validate the scheme/host *before* the offline gate so an invalid or
# non-HTTPS download_url reports the real problem in every mode, rather
# than a misleading "Network access disabled" under --offline.
# (_download_remote_manifest re-checks this, but only once network access
# is permitted.) HTTPS-only, http allowed for localhost.
_require_https(f"bundle '{resolved.entry.id}'", url)
raise BundlerError(
f"Unsupported download_url scheme for bundle '{resolved.entry.id}': {url}"
)
if offline:
raise BundlerError(
f"Network access disabled; cannot download bundle '{resolved.entry.id}' "
f"from {url}."
)
return _download_remote_manifest(resolved.entry.id, url)
def _require_https(label: str, url: str) -> None: