mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(bundler): reject falsy non-mapping requires/provides in manifest from_dict (#3661)
* fix(bundler): reject falsy non-mapping requires/provides in manifest from_dict
BundleManifest.from_dict used `data.get("requires") or {}` and
`data.get("provides") or {}`, so a FALSY non-mapping value ([], '', 0,
false) was coerced to {} BEFORE the isinstance guard — a malformed manifest
passed validation as one that requires/provides nothing. Only a truthy
non-mapping (e.g. "extensions") was rejected.
Handle None explicitly (default to {}) and reject every other non-mapping,
matching the sibling 'integration' guard added in #3629. Absent fields still
parse to the empty default.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(bundler): correct absent-optional-mapping regression assertion
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -111,8 +111,10 @@ class BundleManifest:
|
||||
license=str(bundle_raw.get("license", "")).strip(),
|
||||
)
|
||||
|
||||
requires_raw = data.get("requires") or {}
|
||||
if not isinstance(requires_raw, dict):
|
||||
requires_raw = data.get("requires")
|
||||
if requires_raw is None:
|
||||
requires_raw = {}
|
||||
elif not isinstance(requires_raw, dict):
|
||||
raise BundlerError("'requires' must be a mapping when present.")
|
||||
requires = Requires(
|
||||
speckit_version=str(requires_raw.get("speckit_version", "")).strip(),
|
||||
@@ -130,8 +132,10 @@ class BundleManifest:
|
||||
if isinstance(integration_raw, dict) and integration_raw.get("id"):
|
||||
integration = IntegrationRef(id=str(integration_raw["id"]).strip())
|
||||
|
||||
provides = data.get("provides") or {}
|
||||
if not isinstance(provides, dict):
|
||||
provides = data.get("provides")
|
||||
if provides is None:
|
||||
provides = {}
|
||||
elif not isinstance(provides, dict):
|
||||
raise BundlerError("'provides' must be a mapping when present.")
|
||||
|
||||
tags_raw = data.get("tags")
|
||||
|
||||
@@ -134,3 +134,34 @@ def test_string_integration_rejected_not_silently_dropped():
|
||||
data["integration"] = "copilot"
|
||||
with pytest.raises(BundlerError, match="'integration' must be a mapping when present"):
|
||||
BundleManifest.from_dict(data)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", [[], "", 0, False, "extensions"])
|
||||
def test_non_mapping_provides_rejected_including_falsy(bad):
|
||||
# `data.get("provides") or {}` coerced a FALSY non-mapping ([], '', 0, False)
|
||||
# to {} before the type check, so a malformed manifest passed validation as
|
||||
# a bundle that provides nothing. Only an absent/None value means "empty".
|
||||
data = valid_manifest_dict()
|
||||
data["provides"] = bad
|
||||
with pytest.raises(BundlerError, match="'provides' must be a mapping when present"):
|
||||
BundleManifest.from_dict(data)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", [[], "", 0, False, "speckit>=0.1"])
|
||||
def test_non_mapping_requires_rejected_including_falsy(bad):
|
||||
# Same falsy-coercion hole for `requires`.
|
||||
data = valid_manifest_dict()
|
||||
data["requires"] = bad
|
||||
with pytest.raises(BundlerError, match="'requires' must be a mapping when present"):
|
||||
BundleManifest.from_dict(data)
|
||||
|
||||
|
||||
def test_absent_provides_and_requires_do_not_raise_mapping_error():
|
||||
# Absent (None) optional mappings default to empty and must NOT trigger the
|
||||
# "must be a mapping when present" guard — that is reserved for present
|
||||
# non-mappings. (Structural completeness, e.g. requires.speckit_version, is
|
||||
# a separate concern checked by structural_errors().)
|
||||
data = valid_manifest_dict()
|
||||
data.pop("provides", None)
|
||||
data.pop("requires", None)
|
||||
BundleManifest.from_dict(data) # does not raise BundlerError
|
||||
|
||||
Reference in New Issue
Block a user