fix: enforce size limit on individual files in build_bundle()

src/specify_cli/bundles/manifest.py

@@ -85,8 +85,8 @@ def components(self) -> list[ComponentRef]:
     # -- construction ---------------------------------------------------------
 
     @classmethod
-    def from_file(cls, path: Path) -> "BundleManifest":
-        data = load_yaml(path)
+    def from_file(cls, path: Path, *, max_bytes: int | None = None) -> "BundleManifest":
+        data = load_yaml(path, max_bytes=max_bytes)
         manifest = cls.from_dict(data)
         manifest.source_path = Path(path)
         return manifest

src/specify_cli/bundles/packager.py

@@ -9,6 +9,7 @@
 
 import os
 import re
+import tempfile
 import zipfile
 from dataclasses import dataclass
 from pathlib import Path
@@ -24,6 +25,11 @@
 # Fixed member timestamp (zip epoch) for reproducible, byte-stable artifacts.
 _FIXED_TIMESTAMP = (1980, 1, 1, 0, 0, 0)
 
+# Maximum size (in bytes) for any individual file added to the archive.
+# This prevents a single oversized asset from exhausting memory during
+# compression or transmission.
+MAX_ZIP_MEMBER_BYTES = 10 * 1024 * 1024  # 10 MiB
+
 
 @dataclass
 class BuildResult:
@@ -48,7 +54,10 @@ def build_bundle(
             "README.md describing it."
         )
 
-    manifest = BundleManifest.from_file(manifest_path)
+    # The member loop below bounds every packaged file, but the manifest is
+    # parsed (fully read into memory) before that loop ever runs, so bound the
+    # read here as well.
+    manifest = BundleManifest.from_file(manifest_path, max_bytes=MAX_ZIP_MEMBER_BYTES)
     report = validate_manifest(manifest)
     if not report.ok:
         raise BundlerError(
@@ -76,28 +85,80 @@ def build_bundle(
         rf"^{re.escape(manifest.bundle.id)}-"
         r"\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?\.zip$"
     )
+    # A leftover mkstemp staging file (<id>-<version>-<random>.tmp) from a
+    # previous killed build may sit inside out_dir — which defaults to the
+    # bundle source tree — and must never be re-packaged as a member.
+    staging_re = re.compile(
+        rf"^{re.escape(manifest.bundle.id)}-"
+        r"\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?"
+        r"-[0-9A-Za-z_]+\.tmp$"
+    )
     files = _collect_files(
-        bundle_dir, skip=artifact_path, skip_dir=skip_dir, artifact_re=artifact_re
+        bundle_dir,
+        skip=artifact_path,
+        skip_dir=skip_dir,
+        artifact_re=artifact_re,
+        staging_re=staging_re,
     )
-    with zipfile.ZipFile(artifact_path, "w", zipfile.ZIP_DEFLATED) as archive:
-        for file_path in files:
-            # Confinement: every packaged file must live under bundle_dir.
-            ensure_within(bundle_dir, file_path)
-            arcname = file_path.relative_to(bundle_dir).as_posix()
-            # Fixed timestamp 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
-            # Reproducible, normalized permissions: preserve executability so
-            # bundled scripts (e.g. extension hook scripts) stay runnable after
-            # extraction, but collapse to two canonical modes (0755 when any
-            # execute bit is set on the source, otherwise 0644) so identical
-            # inputs yield a byte-for-byte identical artifact.
-            with file_path.open("rb") as fh:
-                st = os.fstat(fh.fileno())
-                mode = 0o755 if st.st_mode & 0o111 else 0o644
-                info.external_attr = mode << 16
-                archive.writestr(info, fh.read())
+
+    # Build into a temporary sibling and atomically replace the final path only
+    # after every member passes validation.  This prevents a partial/corrupt
+    # archive from being left at the final output path if a member exceeds the
+    # size limit or another error occurs mid-build.
+    tmp_fd, tmp_path_str = tempfile.mkstemp(
+        suffix=".tmp", prefix=f"{manifest.bundle.id}-{manifest.bundle.version}-", dir=str(out_dir)
+    )
+    tmp_path = Path(tmp_path_str)
+    try:
+        with os.fdopen(tmp_fd, "wb") as tmp_fh, zipfile.ZipFile(tmp_fh, "w", zipfile.ZIP_DEFLATED) as archive:
+            for file_path in files:
+                # Confinement: every packaged file must live under bundle_dir.
+                ensure_within(bundle_dir, file_path)
+                arcname = file_path.relative_to(bundle_dir).as_posix()
+                # Fixed timestamp 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
+                # Reproducible, normalized permissions: preserve executability so
+                # bundled scripts (e.g. extension hook scripts) stay runnable after
+                # extraction, but collapse to two canonical modes (0755 when any
+                # execute bit is set on the source, otherwise 0644) so identical
+                # inputs yield a byte-for-byte identical artifact.
+                with file_path.open("rb") as fh:
+                    st = os.fstat(fh.fileno())
+                    mode = 0o755 if st.st_mode & 0o111 else 0o644
+                    info.external_attr = mode << 16
+                    # Fast metadata rejection: skip files whose size exceeds the
+                    # limit before touching the read path.  Then also bound the
+                    # actual read so a TOCTOU race (file appended after fstat)
+                    # cannot bypass the limit.
+                    if st.st_size > MAX_ZIP_MEMBER_BYTES:
+                        raise BundlerError(
+                            f"Bundle file {arcname} exceeds {MAX_ZIP_MEMBER_BYTES}-byte limit"
+                        )
+                    content = fh.read(MAX_ZIP_MEMBER_BYTES + 1)
+                    if len(content) > MAX_ZIP_MEMBER_BYTES:
+                        raise BundlerError(
+                            f"Bundle file {arcname} exceeds {MAX_ZIP_MEMBER_BYTES}-byte limit"
+                        )
+                    archive.writestr(info, content)
+
+        # All members written successfully — atomically replace the final path.
+        # mkstemp() creates the staging file 0600 (owner-only) and os.replace()
+        # preserves that mode, which would silently publish every rebuild as an
+        # unreadable-to-others archive (a direct ZipFile(path, "w") write used
+        # to produce 0666 & ~umask).  Set an intentional mode first: a rebuild
+        # keeps the existing artifact's mode so publishing pipelines that
+        # chmod'd it are not overridden; a fresh build gets 0644.
+        if artifact_path.exists():
+            os.chmod(tmp_path, artifact_path.stat().st_mode & 0o777)
+        else:
+            os.chmod(tmp_path, 0o644)
+        os.replace(tmp_path, artifact_path)
+    except BaseException:
+        # Clean up the temporary file on any failure (exception, interrupt, etc.)
+        tmp_path.unlink(missing_ok=True)
+        raise
 
     return BuildResult(artifact_path=artifact_path, file_count=len(files))
 
@@ -115,6 +176,7 @@ def _collect_files(
     skip: Path,
     skip_dir: Path | None = None,
     artifact_re: re.Pattern[str] | None = None,
+    staging_re: re.Pattern[str] | None = None,
 ) -> list[Path]:
     collected: list[Path] = []
     # followlinks=False so a symlinked directory is never descended into,
@@ -140,6 +202,9 @@ def _collect_files(
             if artifact_re is not None and artifact_re.match(name):
                 # A prior build artifact for this bundle — never re-package it.
                 continue
+            if staging_re is not None and staging_re.match(name):
+                # A leftover packager staging file — never re-package it.
+                continue
             if path.is_symlink():
                 # Skip symlinked files to avoid escaping the bundle directory.
                 continue

src/specify_cli/bundles/yamlio.py

@@ -38,26 +38,41 @@ def ensure_within(root: Path, candidate: Path) -> Path:
     return candidate_resolved
 
 
-def load_yaml(path: Path) -> Any:
+def load_yaml(path: Path, *, max_bytes: int | None = None) -> Any:
     """Parse a YAML file, returning ``{}`` only for an *empty* document.
 
-    A non-empty document is returned exactly as parsed — including a
+    A non-empty document is returned exactly as parsed � including a
     non-mapping such as ``[]``, ``false``, ``0``, ``''``, or an explicit null
-    (``null``/``~``) — so callers can validate the top-level shape (e.g. reject
+    (``null``/``~``) � so callers can validate the top-level shape (e.g. reject
     a non-mapping config) instead of having it silently coerced to an empty
     mapping.
 
     ``yaml.safe_load`` returns ``None`` for *both* an empty document and an
     explicit null scalar, so ``yaml.compose`` (which yields no node only for a
     truly empty document) is used to tell them apart: an empty document becomes
-    ``{}`` while an explicit ``null``/``~`` is returned as ``None`` for the
+    ``{}`` while an explicit ``null`` is returned as ``None`` for the
     caller to reject.
+
+    When ``max_bytes`` is given the file is read through a bounded read, so a
+    file that grows after the size check (or is simply huge) never reaches the
+    YAML parser.
     """
     path = Path(path)
     if not path.exists():
         raise BundlerError(f"File not found: {path}")
     try:
-        text = path.read_text(encoding="utf-8")
+        if max_bytes is None:
+            text = path.read_text(encoding="utf-8")
+        else:
+            with open(path, "rb") as fh:
+                raw = fh.read(max_bytes + 1)
+            if len(raw) > max_bytes:
+                raise BundlerError(
+                    f"File {path} exceeds {max_bytes}-byte limit"
+                )
+            text = raw.decode("utf-8")
+    except BundlerError:
+        raise
     except (OSError, UnicodeError) as exc:
         # A non-UTF-8 file raises UnicodeDecodeError, which is a ValueError --
         # NOT an OSError -- so it escaped this module's "IO failures degrade

tests/specify_cli/bundles/test_packager.py

@@ -2,14 +2,15 @@
 from __future__ import annotations
 
 import os
+import stat
 import zipfile
 from pathlib import Path
 
 import pytest
 import yaml
 
 from specify_cli.bundler import BundlerError
-from specify_cli.bundles.packager import build_bundle
+from specify_cli.bundles.packager import MAX_ZIP_MEMBER_BYTES, build_bundle
 from tests.specify_cli.bundles.helpers import valid_manifest_dict
 
 
@@ -234,3 +235,135 @@ def test_toctou_stat_read_consistency(tmp_path: Path):
     assert content == b"\x00\x01\x02\x03"
     assert modes["assets/data.bin"] == 0o644
     assert modes["README.md"] == 0o644
+
+
+def test_oversized_file_is_rejected(tmp_path: Path):
+    """A file exceeding MAX_ZIP_MEMBER_BYTES must be rejected."""
+    bundle = _make_bundle(tmp_path / "b")
+    large = bundle / "assets" / "large.bin"
+    large.parent.mkdir(parents=True, exist_ok=True)
+    large.write_bytes(b"\x00" * (MAX_ZIP_MEMBER_BYTES + 1))
+
+    with pytest.raises(BundlerError, match="exceeds"):
+        build_bundle(bundle, output_dir=tmp_path / "out")
+
+
+def test_boundary_size_file_is_accepted(tmp_path: Path):
+    """A file exactly at MAX_ZIP_MEMBER_BYTES must be accepted."""
+    bundle = _make_bundle(tmp_path / "b")
+    boundary = bundle / "assets" / "boundary.bin"
+    boundary.parent.mkdir(parents=True, exist_ok=True)
+    boundary.write_bytes(b"\x00" * MAX_ZIP_MEMBER_BYTES)
+
+    result = build_bundle(bundle, output_dir=tmp_path / "out")
+    with zipfile.ZipFile(result.artifact_path) as archive:
+        content = archive.read("assets/boundary.bin")
+    assert len(content) == MAX_ZIP_MEMBER_BYTES
+
+
+def test_oversized_file_does_not_corrupt_output(tmp_path: Path):
+    """When an oversized file is rejected, the output path must not contain
+    a partial/corrupt archive — it should either not exist or contain the
+    previous valid artifact (if any)."""
+    bundle = _make_bundle(tmp_path / "b", extra_files={"good.txt": "ok"})
+
+    # First build succeeds.
+    out_dir = tmp_path / "out"
+    first = build_bundle(bundle, output_dir=out_dir)
+    first_bytes = first.artifact_path.read_bytes()
+
+    # Add an oversized file.
+    large = bundle / "assets" / "large.bin"
+    large.parent.mkdir(parents=True, exist_ok=True)
+    large.write_bytes(b"\x00" * (MAX_ZIP_MEMBER_BYTES + 1))
+
+    # Second build fails — but the original artifact must remain intact.
+    with pytest.raises(BundlerError, match="exceeds"):
+        build_bundle(bundle, output_dir=out_dir)
+
+    assert first.artifact_path.exists()
+    assert first.artifact_path.read_bytes() == first_bytes
+
+
+def test_temp_file_cleaned_up_on_failure(tmp_path: Path):
+    """No .tmp files must remain in the output directory after a failed build."""
+    bundle = _make_bundle(tmp_path / "b")
+    large = bundle / "assets" / "large.bin"
+    large.parent.mkdir(parents=True, exist_ok=True)
+    large.write_bytes(b"\x00" * (MAX_ZIP_MEMBER_BYTES + 1))
+
+    out_dir = tmp_path / "out"
+    with pytest.raises(BundlerError):
+        build_bundle(bundle, output_dir=out_dir)
+
+    tmp_files = list(out_dir.glob("*.tmp"))
+    assert tmp_files == [], f"Leftover temp files: {tmp_files}"
+
+
+def test_leftover_staging_file_is_not_packaged(tmp_path: Path):
+    """A leftover mkstemp staging file from a prior killed build must never be
+    collected — even with the default out_dir (the bundle source tree)."""
+    bundle = _make_bundle(
+        tmp_path / "b",
+        extra_files={"demo-bundle-1.2.0-abcd1234.tmp": "partial"},
+    )
+    result = build_bundle(bundle)
+    with zipfile.ZipFile(result.artifact_path) as archive:
+        names = set(archive.namelist())
+    assert "demo-bundle-1.2.0-abcd1234.tmp" not in names
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits")
+def test_artifact_is_world_readable_after_build(tmp_path: Path):
+    """A fresh build must publish a 0644 artifact, not the 0600 mode that
+    mkstemp() creates the staging file with."""
+    bundle = _make_bundle(tmp_path / "b")
+    result = build_bundle(bundle, output_dir=tmp_path / "out")
+    assert stat.S_IMODE(result.artifact_path.stat().st_mode) == 0o644
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits")
+def test_rebuild_preserves_existing_artifact_mode(tmp_path: Path):
+    """A rebuild must keep a pre-existing artifact's mode instead of resetting
+    it (publishing pipelines may have chmod'd the artifact deliberately)."""
+    bundle = _make_bundle(tmp_path / "b")
+    out_dir = tmp_path / "out"
+    first = build_bundle(bundle, output_dir=out_dir)
+    os.chmod(first.artifact_path, 0o640)
+
+    second = build_bundle(bundle, output_dir=out_dir)
+    assert stat.S_IMODE(second.artifact_path.stat().st_mode) == 0o640
+
+
+def test_oversized_manifest_is_rejected_before_parse(tmp_path: Path):
+    """``bundle.yml`` is fully read by the manifest parser before the member
+    loop's size check runs, so that read must be bounded too.
+
+    The message must come from the manifest read (it carries the full path),
+    not from the later member check (which names only the archive name).
+    """
+    bundle = _make_bundle(tmp_path / "b")
+    manifest_path = bundle / "bundle.yml"
+    original = manifest_path.read_text(encoding="utf-8")
+    manifest_path.write_text(
+        original + "\n#" + "x" * MAX_ZIP_MEMBER_BYTES + "\n", encoding="utf-8"
+    )
+
+    with pytest.raises(BundlerError) as excinfo:
+        build_bundle(bundle, output_dir=tmp_path / "out")
+
+    message = str(excinfo.value)
+    assert "exceeds" in message
+    assert str(manifest_path) in message
+
+
+def test_manifest_within_limit_still_builds(tmp_path: Path):
+    """A manifest under the limit keeps the normal build path working."""
+    bundle = _make_bundle(tmp_path / "b")
+    out_dir = tmp_path / "out"
+
+    result = build_bundle(bundle, output_dir=out_dir)
+
+    assert result.artifact_path.exists()
+    with zipfile.ZipFile(result.artifact_path) as archive:
+        assert "bundle.yml" in archive.namelist()