From 0bc7f3dafc25dd062176773022d1beec07ec0c66 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:36:14 +0200 Subject: [PATCH 1/2] fix(bundler): wrap invalid YAML from a zipped bundle manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Installing a bundle from a .zip parsed speckit-bundle.yml with a bare yaml.safe_load() on the zip byte stream, so an invalid-YAML or non-UTF-8 manifest crashed 'specify bundle install' with a raw traceback instead of a BundlerError. The directory-install path already reports these as clean errors via the shared yamlio helpers, and the zip open/read errors themselves were wrapped by #3141 — only the parse step was left bare. Wrap safe_load in try/except yaml.YAMLError and raise BundlerError with the same 'Invalid YAML' wording as the yamlio contract. PyYAML reports undecodable bytes from a byte stream as ReaderError (a YAMLError subclass), so one clause covers both corruption modes. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/commands/bundle/__init__.py | 15 ++++++++- .../integration/test_bundler_local_install.py | 33 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/commands/bundle/__init__.py b/src/specify_cli/commands/bundle/__init__.py index 7476cb41b5..11c4b085ed 100644 --- a/src/specify_cli/commands/bundle/__init__.py +++ b/src/specify_cli/commands/bundle/__init__.py @@ -771,7 +771,20 @@ def _local_manifest_source(arg: str): error_type=BundlerError, label="bundle manifest", ) - data = _yaml.safe_load(io.BytesIO(raw)) + # The bounded-zip helpers above keep archive failures inside the + # BundlerError contract, but the manifest *parse* needs the same + # treatment: PyYAML raises YAMLError for malformed YAML, and its + # Reader wraps undecodable bytes in ReaderError (a YAMLError + # subclass) when fed a byte stream, so one clause covers both + # corruption modes. Mirrors yamlio.load_yaml's "Invalid YAML in ..." + # message so a manifest inside a .zip fails like the directory and + # bundle.yml sources do. + try: + data = _yaml.safe_load(io.BytesIO(raw)) + except _yaml.YAMLError as exc: + raise BundlerError( + f"Invalid YAML in bundle.yml inside '{candidate}': {exc}" + ) from exc return BundleManifest.from_dict(data) if candidate.name == "bundle.yml" or candidate.suffix in (".yml", ".yaml"): diff --git a/tests/integration/test_bundler_local_install.py b/tests/integration/test_bundler_local_install.py index 164de57006..d268db83ce 100644 --- a/tests/integration/test_bundler_local_install.py +++ b/tests/integration/test_bundler_local_install.py @@ -62,6 +62,39 @@ def test_local_source_rejects_unknown_file(tmp_path: Path): _local_manifest_source(str(weird)) +def test_local_source_zip_invalid_manifest_yaml_raises_bundler_error(tmp_path: Path): + """Malformed YAML inside a .zip's bundle.yml must raise BundlerError. + + The zip open and member read already degrade into BundlerError via the + shared bounded-zip helpers, but the subsequent ``yaml.safe_load`` did + not: an invalid manifest escaped as a raw ``yaml.YAMLError`` traceback, + while the same manifest in a directory or as a plain bundle.yml goes + through ``load_yaml``'s "Invalid YAML in ..." BundlerError contract. + """ + artifact = tmp_path / "demo.zip" + with zipfile.ZipFile(artifact, "w") as archive: + archive.writestr("bundle.yml", "bundle: [unclosed\n") + + with pytest.raises(BundlerError, match="Invalid YAML"): + _local_manifest_source(str(artifact)) + + +def test_local_source_zip_non_utf8_manifest_raises_bundler_error(tmp_path: Path): + """Undecodable bundle.yml bytes inside a .zip must raise BundlerError. + + PyYAML's Reader wraps invalid bytes from a byte stream in ReaderError — + a ``YAMLError`` subclass — so this corruption mode rides the same clause, + but it deserves its own coverage: it is the realistic on-disk failure + (e.g. a UTF-16 manifest produced by PowerShell's ``Out-File``). + """ + artifact = tmp_path / "demo.zip" + with zipfile.ZipFile(artifact, "w") as archive: + archive.writestr("bundle.yml", b"\xff\xfe bundle \xc3\x28\n") + + with pytest.raises(BundlerError, match="Invalid YAML"): + _local_manifest_source(str(artifact)) + + def test_install_bundled_extension_from_zip_offline(tmp_path: Path): """End-to-end: build → install (offline, local .zip) → list → remove.""" project = make_project(tmp_path / "proj") From e1898e517cebe4ed61297dc4ca5b3ac95f6e7775 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Mon, 3 Aug 2026 22:13:18 +0200 Subject: [PATCH 2/2] fix: decode the zipped manifest as UTF-8 before parsing Review follow-up: feeding PyYAML the byte stream let its Reader honour a UTF-16 BOM and accept a manifest yamlio.load_yaml rejects, so zip and directory sources diverged. Decode raw as UTF-8 (UnicodeError -> BundlerError 'Could not read ...') then parse, and cover a well-formed UTF-16 manifest in the regression tests. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/commands/bundle/__init__.py | 24 ++++++++++------- .../integration/test_bundler_local_install.py | 27 +++++++++++++++---- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/specify_cli/commands/bundle/__init__.py b/src/specify_cli/commands/bundle/__init__.py index 11c4b085ed..5d8f42be7c 100644 --- a/src/specify_cli/commands/bundle/__init__.py +++ b/src/specify_cli/commands/bundle/__init__.py @@ -752,8 +752,6 @@ def _local_manifest_source(arg: str): return BundleManifest.from_file(manifest_path) if candidate.suffix == ".zip": - import io - import yaml as _yaml from ..._download_security import open_zip_bounded, read_zip_member_limited @@ -772,15 +770,21 @@ def _local_manifest_source(arg: str): label="bundle manifest", ) # The bounded-zip helpers above keep archive failures inside the - # BundlerError contract, but the manifest *parse* needs the same - # treatment: PyYAML raises YAMLError for malformed YAML, and its - # Reader wraps undecodable bytes in ReaderError (a YAMLError - # subclass) when fed a byte stream, so one clause covers both - # corruption modes. Mirrors yamlio.load_yaml's "Invalid YAML in ..." - # message so a manifest inside a .zip fails like the directory and - # bundle.yml sources do. + # BundlerError contract, but the manifest bytes need the same + # treatment as yamlio.load_yaml: decode as UTF-8 explicitly — + # feeding PyYAML the byte stream would let its Reader auto-detect + # a UTF-16 BOM and accept a manifest the directory and bundle.yml + # sources reject — then parse, mirroring load_yaml's + # "Could not read ..." / "Invalid YAML in ..." messages so a + # manifest inside a .zip fails exactly like the other sources. + try: + text = raw.decode("utf-8") + except UnicodeError as exc: + raise BundlerError( + f"Could not read bundle.yml inside '{candidate}': {exc}" + ) from exc try: - data = _yaml.safe_load(io.BytesIO(raw)) + data = _yaml.safe_load(text) except _yaml.YAMLError as exc: raise BundlerError( f"Invalid YAML in bundle.yml inside '{candidate}': {exc}" diff --git a/tests/integration/test_bundler_local_install.py b/tests/integration/test_bundler_local_install.py index d268db83ce..7a8397354c 100644 --- a/tests/integration/test_bundler_local_install.py +++ b/tests/integration/test_bundler_local_install.py @@ -82,16 +82,33 @@ def test_local_source_zip_invalid_manifest_yaml_raises_bundler_error(tmp_path: P def test_local_source_zip_non_utf8_manifest_raises_bundler_error(tmp_path: Path): """Undecodable bundle.yml bytes inside a .zip must raise BundlerError. - PyYAML's Reader wraps invalid bytes from a byte stream in ReaderError — - a ``YAMLError`` subclass — so this corruption mode rides the same clause, - but it deserves its own coverage: it is the realistic on-disk failure - (e.g. a UTF-16 manifest produced by PowerShell's ``Out-File``). + The manifest bytes are decoded as UTF-8 explicitly, matching + ``yamlio.load_yaml``'s "Could not read ..." contract, instead of + escaping as a raw ``UnicodeDecodeError``/``ReaderError`` traceback. """ artifact = tmp_path / "demo.zip" with zipfile.ZipFile(artifact, "w") as archive: archive.writestr("bundle.yml", b"\xff\xfe bundle \xc3\x28\n") - with pytest.raises(BundlerError, match="Invalid YAML"): + with pytest.raises(BundlerError, match="Could not read"): + _local_manifest_source(str(artifact)) + + +def test_local_source_zip_utf16_manifest_rejected_like_directory(tmp_path: Path): + """A well-formed UTF-16 manifest must fail the same way in a .zip. + + ``yamlio.load_yaml`` decodes strictly as UTF-8, so a UTF-16 bundle.yml + (the realistic PowerShell ``Out-File`` output) is rejected when read + from a directory. Feeding the zip bytes straight to PyYAML would let + its Reader honour the UTF-16 BOM and *accept* the same manifest, + making zip and directory sources diverge. + """ + artifact = tmp_path / "demo.zip" + manifest_text = "bundle:\n id: demo-bundle\n version: 1.0.0\n" + with zipfile.ZipFile(artifact, "w") as archive: + archive.writestr("bundle.yml", manifest_text.encode("utf-16")) + + with pytest.raises(BundlerError, match="Could not read"): _local_manifest_source(str(artifact))