diff --git a/src/specify_cli/bundler/models/records.py b/src/specify_cli/bundler/models/records.py index 2d0c8b73a0..748b23759a 100644 --- a/src/specify_cli/bundler/models/records.py +++ b/src/specify_cli/bundler/models/records.py @@ -13,7 +13,7 @@ from .. import BundlerError from ..lib.yamlio import dump_json, ensure_within, load_json -from .manifest import COMPONENT_KINDS, ComponentRef +from .manifest import COMPONENT_KINDS, ComponentRef, _text RECORDS_FILENAME = "bundle-records.json" RECORDS_SCHEMA_VERSION = "1.0" @@ -65,8 +65,14 @@ def from_dict(cls, data: Any) -> "InstalledBundleRecord": raise BundlerError( "Corrupt record: 'contributed_components' must be a list." ) - bundle_id = str(data.get("bundle_id", "")).strip() - version = str(data.get("version", "")).strip() + # ``.get(key, "")`` defaults only a *missing* key. A key that is + # present but null -- how a hand-edited or corrupt record spells an + # empty field -- yields ``None``, and ``str(None)`` is the non-empty + # literal ``"None"``, which sails past the required-field checks + # below. Reuse the manifest's ``_text`` so records and bundle.yml + # agree on what an explicit null means. + bundle_id = _text(data.get("bundle_id")) + version = _text(data.get("version")) if not bundle_id: raise BundlerError( "Corrupt records file: an installed-bundle record is missing " @@ -80,7 +86,7 @@ def from_dict(cls, data: Any) -> "InstalledBundleRecord": return cls( bundle_id=bundle_id, version=version, - installed_at=str(data.get("installed_at", "")).strip(), + installed_at=_text(data.get("installed_at")), contributed_components=tuple( _component_from_dict(c) for c in components_raw ), @@ -201,8 +207,8 @@ def _component_to_dict(ref: ComponentRef) -> dict[str, Any]: def _component_from_dict(data: Any) -> ComponentRef: if not isinstance(data, dict): raise BundlerError("Each contributed component must be a mapping.") - kind = str(data.get("kind", "")).strip() - cid = str(data.get("id", "")).strip() + kind = _text(data.get("kind")) + cid = _text(data.get("id")) if kind not in COMPONENT_KINDS: raise BundlerError( f"Corrupt records file: component 'kind' must be one of " diff --git a/tests/unit/test_bundler_records.py b/tests/unit/test_bundler_records.py index 8f6f0d6547..dc1da118a1 100644 --- a/tests/unit/test_bundler_records.py +++ b/tests/unit/test_bundler_records.py @@ -209,3 +209,73 @@ def test_load_records_accepts_forward_compatible_minor_schema(tmp_path: Path): payload = {"schema_version": "1.5", "bundles": []} records_path(tmp_path).write_text(json.dumps(payload), encoding="utf-8") assert load_records(tmp_path) == [] + + +@pytest.mark.parametrize( + "field,message", + [ + ("bundle_id", "missing its 'bundle_id'"), + ("version", "missing its 'version'"), + ], +) +def test_load_records_rejects_explicit_null_record_field( + tmp_path: Path, field: str, message: str +): + """An explicit JSON ``null`` is how a corrupt record spells an empty field. + + ``str(data.get(field, ""))`` defaults only a *missing* key, so a + present-but-null value became the literal text ``"None"`` — non-empty, so + it sailed past the required-field checks and the record was accepted as a + bundle actually named ``"None"``. Mirrors ``manifest._text``. + """ + (tmp_path / ".specify").mkdir() + record = {"bundle_id": "a", "version": "1.0.0", "contributed_components": []} + record[field] = None + payload = {"schema_version": "1.0", "bundles": [record]} + records_path(tmp_path).write_text(json.dumps(payload), encoding="utf-8") + + with pytest.raises(BundlerError, match=message): + load_records(tmp_path) + + +def test_load_records_rejects_explicit_null_component_id(tmp_path: Path): + """A null component id became ``"None"`` and entered the refcount. + + ``components_still_needed`` would then report a phantom + ``('presets', 'None')`` as protected. + """ + (tmp_path / ".specify").mkdir() + payload = { + "schema_version": "1.0", + "bundles": [ + { + "bundle_id": "a", + "version": "1.0.0", + "contributed_components": [{"kind": "presets", "id": None}], + } + ], + } + records_path(tmp_path).write_text(json.dumps(payload), encoding="utf-8") + + with pytest.raises(BundlerError, match="missing its 'id'"): + load_records(tmp_path) + + +def test_load_records_accepts_explicit_null_installed_at(tmp_path: Path): + """``installed_at`` is optional, so a null must become "" — not "None".""" + (tmp_path / ".specify").mkdir() + payload = { + "schema_version": "1.0", + "bundles": [ + { + "bundle_id": "a", + "version": "1.0.0", + "installed_at": None, + "contributed_components": [], + } + ], + } + records_path(tmp_path).write_text(json.dumps(payload), encoding="utf-8") + + records = load_records(tmp_path) + assert records[0].installed_at == ""