fix(bundler): address PR review — annotations, Windows paths, HTTPS, errors, reproducible builds

Resolves automated review feedback on #3070:

- validator: drop redundant string-quoting on ReferenceChecker's
  `str | None` return so the annotation evaluates as a real union under
  `from __future__ import annotations`.
- adapters: normalize Windows drive-letter paths (e.g. C:\...) to the
  local-file branch so offline file catalogs resolve on Windows.
- adapters: enforce HTTPS (HTTP only for localhost) and require a host on
  remote catalog URLs before any network call, mirroring
  specify_cli.catalogs URL validation (MITM/downgrade protection).
- adapters: pass `origin` to loads_json for local files and HTTP payloads
  so JSON parse errors name the real source instead of <string>.
- manifest: parse component `priority` defensively, raising an actionable
  BundlerError on non-integer values instead of a raw ValueError.
- packager: write zip members with a fixed timestamp + permissions so
  identical inputs yield byte-for-byte identical artifacts (genuinely
  reproducible builds), and strengthen the determinism test accordingly.

Adds regression tests for priority validation, plain-HTTP/host rejection,
and byte-level artifact reproducibility (111 bundler tests pass; ruff clean).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: mnriem

src/specify_cli/bundler/models/manifest.py

@@ -202,15 +202,30 @@ def _parse_refs(kind: str, raw: Any) -> list[ComponentRef]:
     for item in raw:
         if not isinstance(item, dict):
             raise BundlerError(f"Each provides.{kind} entry must be a mapping.")
-        priority = item.get("priority")
+        priority = _parse_priority(kind, item.get("priority"))
         refs.append(
             ComponentRef(
                 kind=kind,
                 id=str(item.get("id", "")).strip(),
                 version=(str(item["version"]).strip() if item.get("version") else None),
                 source=(str(item["source"]).strip() if item.get("source") else None),
-                priority=(int(priority) if priority is not None else None),
+                priority=priority,
                 strategy=(str(item["strategy"]).strip() if item.get("strategy") else None),
             )
         )
     return refs
+
+
+def _parse_priority(kind: str, raw: Any) -> int | None:
+    if raw is None:
+        return None
+    if isinstance(raw, bool) or not isinstance(raw, (int, str)):
+        raise BundlerError(
+            f"provides.{kind} priority must be an integer, got {raw!r}."
+        )
+    try:
+        return int(raw)
+    except (TypeError, ValueError):
+        raise BundlerError(
+            f"provides.{kind} priority must be an integer, got {raw!r}."
+        ) from None

src/specify_cli/bundler/services/adapters.py

@@ -10,6 +10,7 @@
 """
 from __future__ import annotations
 
+import re
 from pathlib import Path
 from urllib.parse import urlparse
 
@@ -36,6 +37,33 @@
 
 HTTP_TIMEOUT_SECONDS = 10
 
+# Windows absolute paths like ``C:\catalog.json`` parse with a single-letter
+# ``scheme`` under urlparse; treat them as local files rather than URLs.
+_WINDOWS_DRIVE_RE = re.compile(r"^[A-Za-z]:[\\/]")
+
+
+def _is_windows_drive_path(url: str) -> bool:
+    return bool(_WINDOWS_DRIVE_RE.match(url))
+
+
+def _validate_remote_url(source_id: str, url: str) -> None:
+    """Restrict remote catalogs to HTTPS (HTTP only for localhost) with a host.
+
+    Mirrors ``specify_cli.catalogs`` URL validation to avoid MITM/downgrade
+    issues before any network call.
+    """
+    parsed = urlparse(url)
+    is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
+    if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost):
+        raise BundlerError(
+            f"Catalog '{source_id}' URL must use HTTPS (got {parsed.scheme}://). "
+            "HTTP is only allowed for localhost."
+        )
+    if not parsed.netloc:
+        raise BundlerError(
+            f"Catalog '{source_id}' URL must be a valid URL with a host: {url}"
+        )
+
 
 def make_catalog_fetcher(*, allow_network: bool = True):
     """Return a fetcher callable suitable for :class:`CatalogStack`.
@@ -55,18 +83,19 @@ def fetch(source: CatalogSource) -> dict:
                 raise BundlerError(f"Unknown built-in catalog '{url}'.")
             return payload
 
-        if scheme in ("", "file"):
+        if scheme in ("", "file") or _is_windows_drive_path(url):
             path = Path(parsed.path if scheme == "file" else url)
             if not path.exists():
                 raise BundlerError(f"Catalog file not found: {path}")
-            return loads_json(path.read_text(encoding="utf-8"))
+            return loads_json(path.read_text(encoding="utf-8"), origin=str(path))
 
         if scheme in ("http", "https"):
             if not allow_network:
                 raise BundlerError(
                     f"Network access disabled; cannot fetch catalog '{source.id}' "
                     f"from {url}."
                 )
+            _validate_remote_url(source.id, url)
             return _http_get_json(url)
 
         raise BundlerError(f"Unsupported catalog URL scheme: {url}")
@@ -82,7 +111,7 @@ def _http_get_json(url: str) -> dict:
             raw = response.read().decode("utf-8")
     except Exception as exc:  # noqa: BLE001
         raise BundlerError(f"Failed to fetch catalog from {url}: {exc}") from exc
-    return loads_json(raw)
+    return loads_json(raw, origin=url)
 
 
 class DefaultPrimitiveInstaller:

src/specify_cli/bundler/services/packager.py

@@ -19,6 +19,9 @@
 # Files/dirs never included in an artifact.
 EXCLUDE_NAMES = {".git", "__pycache__", ".DS_Store"}
 
+# Fixed member timestamp (zip epoch) for reproducible, byte-stable artifacts.
+_FIXED_TIMESTAMP = (1980, 1, 1, 0, 0, 0)
+
 
 @dataclass
 class BuildResult:
@@ -53,7 +56,13 @@ def build_bundle(
         for file_path in files:
             # Confinement: every packaged file must live under bundle_dir.
             ensure_within(bundle_dir, file_path)
-            archive.write(file_path, file_path.relative_to(bundle_dir).as_posix())
+            arcname = file_path.relative_to(bundle_dir).as_posix()
+            # Fixed timestamp + permissions so identical inputs yield a
+            # byte-for-byte identical artifact (reproducible builds).
+            info = zipfile.ZipInfo(filename=arcname, date_time=_FIXED_TIMESTAMP)
+            info.compress_type = zipfile.ZIP_DEFLATED
+            info.external_attr = 0o644 << 16
+            archive.writestr(info, file_path.read_bytes())
 
     return BuildResult(artifact_path=artifact_path, file_count=len(files))
 

src/specify_cli/bundler/services/validator.py

@@ -14,7 +14,7 @@
 from ..models.manifest import BundleManifest, ComponentRef
 
 # A reference checker returns None when resolvable, or an error string.
-ReferenceChecker = Callable[[ComponentRef], "str | None"]
+ReferenceChecker = Callable[[ComponentRef], str | None]
 
 
 @dataclass

tests/contract/test_manifest_schema.py

@@ -5,6 +5,9 @@
 """
 from __future__ import annotations
 
+import pytest
+
+from specify_cli.bundler import BundlerError
 from specify_cli.bundler.models.manifest import BundleManifest
 from tests.bundler_helpers import valid_manifest_dict
 
@@ -51,6 +54,13 @@ def test_invalid_preset_strategy_is_rejected():
     assert any("strategy" in e for e in errors)
 
 
+def test_non_integer_priority_raises_actionable_error():
+    data = valid_manifest_dict()
+    data["provides"]["presets"][0]["priority"] = "high"
+    with pytest.raises(BundlerError, match="priority must be an integer"):
+        BundleManifest.from_dict(data)
+
+
 def test_non_step_components_must_be_pinned():
     data = valid_manifest_dict()
     data["provides"]["extensions"] = [{"id": "ext-unpinned"}]

tests/integration/test_bundler_offline.py

@@ -52,3 +52,18 @@ def test_missing_file_catalog_errors_offline(tmp_path: Path):
     stack = CatalogStack([_src("local", str(tmp_path / "nope.json"))], fetcher)
     with pytest.raises(BundlerError):
         stack.resolve("anything")
+
+
+def test_plain_http_remote_rejected_before_network():
+    # HTTPS is required for non-localhost catalogs; reject http:// up front.
+    fetcher = make_catalog_fetcher(allow_network=True)
+    stack = CatalogStack([_src("remote", "http://example.com/catalog.json")], fetcher)
+    with pytest.raises(BundlerError, match="must use HTTPS"):
+        stack.resolve("anything")
+
+
+def test_remote_url_without_host_rejected():
+    fetcher = make_catalog_fetcher(allow_network=True)
+    stack = CatalogStack([_src("remote", "https:///catalog.json")], fetcher)
+    with pytest.raises(BundlerError, match="valid URL with a host"):
+        stack.resolve("anything")

tests/unit/test_bundler_packager.py

@@ -61,5 +61,11 @@ def test_build_is_deterministic(tmp_path: Path):
     first = build_bundle(bundle, output_dir=tmp_path / "out1")
     second = build_bundle(bundle, output_dir=tmp_path / "out2")
     with zipfile.ZipFile(first.artifact_path) as a, zipfile.ZipFile(second.artifact_path) as b:
-        # Same files, same order (sorted) — stable, reproducible manifests.
+        # Same files, same order (sorted).
         assert a.namelist() == b.namelist()
+        # Fixed timestamps + permissions make each member byte-identical.
+        for left, right in zip(a.infolist(), b.infolist()):
+            assert left.date_time == right.date_time
+            assert left.external_attr == right.external_attr
+    # The whole artifact is byte-for-byte reproducible.
+    assert first.artifact_path.read_bytes() == second.artifact_path.read_bytes()