mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
fix(bundle): wrap malformed YAML in a local .zip bundle manifest (#4013)
`_local_manifest_source` handles three local bundle sources. The directory
and `bundle.yml` branches both go through `BundleManifest.from_file` ->
`load_yaml`, which converts a parse failure into a `BundlerError`. The `.zip`
branch instead parses inline with a bare `_yaml.safe_load`.
`yaml.YAMLError` derives directly from `Exception` -- it is neither a
`ValueError` nor an `OSError` -- so it escapes `bundle_install`'s
`except BundlerError` and reaches the user as a raw
`yaml.parser.ParserError` traceback.
The remote counterpart of this same call, `_download_manifest`, already
guards it and even names `_yaml.YAMLError` explicitly. Only the local zip
path was missed, so the same corrupt manifest is reported cleanly when
fetched from a catalog but crashes when installed from disk.
Before, for the identical malformed bundle.yml:
specify bundle install ./bundle-dir -> Error: Invalid YAML in ... (exit 1)
specify bundle install ./bundle.yml -> Error: Invalid YAML in ... (exit 1)
specify bundle install ./bundle.zip -> ParserError traceback
Two regression tests: one pins the `BundlerError` contract on the zip
branch, and one drives all three local sources through the CLI to assert
they now fail alike.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
8865e57344
commit
920ed7546d
@@ -771,7 +771,17 @@ def _local_manifest_source(arg: str):
|
||||
error_type=BundlerError,
|
||||
label="bundle manifest",
|
||||
)
|
||||
data = _yaml.safe_load(io.BytesIO(raw))
|
||||
try:
|
||||
data = _yaml.safe_load(io.BytesIO(raw))
|
||||
except _yaml.YAMLError as exc:
|
||||
# The sibling directory/bundle.yml branches reach YAML through
|
||||
# load_yaml(), which turns a parse failure into a BundlerError. This
|
||||
# branch parses inline, so without this it raises a raw YAMLError --
|
||||
# neither a ValueError nor an OSError -- which escapes
|
||||
# bundle_install()'s `except BundlerError` as a traceback.
|
||||
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"):
|
||||
|
||||
@@ -186,6 +186,50 @@ def test_local_zip_uses_bounded_archive_open(tmp_path: Path):
|
||||
_local_manifest_source(str(artifact))
|
||||
|
||||
|
||||
def test_local_zip_wraps_malformed_manifest_yaml(tmp_path: Path):
|
||||
"""A malformed bundle.yml inside a .zip must raise BundlerError.
|
||||
|
||||
The zip branch parses YAML inline rather than through load_yaml(), so the
|
||||
raw yaml.YAMLError used to escape. It is neither a ValueError nor an
|
||||
OSError, so nothing upstream caught it.
|
||||
"""
|
||||
artifact = tmp_path / "bad-manifest.zip"
|
||||
with zipfile.ZipFile(artifact, "w") as archive:
|
||||
archive.writestr("bundle.yml", "bundle: [unclosed\n id: demo\n")
|
||||
|
||||
with pytest.raises(BundlerError, match="Invalid YAML"):
|
||||
_local_manifest_source(str(artifact))
|
||||
|
||||
|
||||
def test_malformed_manifest_yaml_fails_alike_for_every_local_source(tmp_path: Path):
|
||||
"""`bundle install` reports malformed YAML the same way for all 3 sources.
|
||||
|
||||
Directory and bundle.yml sources already exited 1 with an "Invalid YAML"
|
||||
message; the .zip source dumped a yaml.parser.ParserError traceback.
|
||||
"""
|
||||
bad_yaml = "bundle: [unclosed\n id: demo\n"
|
||||
|
||||
directory = tmp_path / "dir-src"
|
||||
directory.mkdir()
|
||||
(directory / "bundle.yml").write_text(bad_yaml, encoding="utf-8")
|
||||
|
||||
manifest_file = tmp_path / "standalone.yml"
|
||||
manifest_file.write_text(bad_yaml, encoding="utf-8")
|
||||
|
||||
artifact = tmp_path / "artifact.zip"
|
||||
with zipfile.ZipFile(artifact, "w") as archive:
|
||||
archive.writestr("bundle.yml", bad_yaml)
|
||||
|
||||
runner = CliRunner()
|
||||
for source in (directory, manifest_file, artifact):
|
||||
result = runner.invoke(app, ["bundle", "install", str(source)])
|
||||
assert result.exit_code == 1, f"{source.name}: {result.output}"
|
||||
assert result.exception is None or isinstance(
|
||||
result.exception, SystemExit
|
||||
), f"{source.name} leaked {type(result.exception).__name__}"
|
||||
assert "Invalid YAML" in result.output, f"{source.name}: {result.output}"
|
||||
|
||||
|
||||
def test_invalid_local_manifest_is_rejected_before_project_init(
|
||||
tmp_path: Path,
|
||||
monkeypatch,
|
||||
|
||||
Reference in New Issue
Block a user