From 0a7f288ae4ffc8df4265ee19627bb263c6cca876 Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Thu, 23 Jul 2026 18:23:51 +0500 Subject: [PATCH] fix(bundler): reject falsy non-mapping requires/provides in manifest from_dict (#3661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * test(bundler): correct absent-optional-mapping regression assertion --------- Co-authored-by: Claude Opus 4.8 (1M context) --- src/specify_cli/bundler/models/manifest.py | 12 ++++++--- tests/contract/test_manifest_schema.py | 31 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/bundler/models/manifest.py b/src/specify_cli/bundler/models/manifest.py index e7969683c..6b971acd4 100644 --- a/src/specify_cli/bundler/models/manifest.py +++ b/src/specify_cli/bundler/models/manifest.py @@ -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") diff --git a/tests/contract/test_manifest_schema.py b/tests/contract/test_manifest_schema.py index 1c1c27986..8753e947b 100644 --- a/tests/contract/test_manifest_schema.py +++ b/tests/contract/test_manifest_schema.py @@ -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