fix(bundler): treat an explicit-null records field as missing, not "None" (#4136)

`InstalledBundleRecord.from_dict` and `_component_from_dict` read required
fields with `str(data.get(key, "")).strip()`. The `""` default covers only a
MISSING key. A key that is present but JSON `null` — how a hand-edited or
corrupt `.specify/bundle-records.json` spells an empty field — yields
`None`, and `str(None)` is the literal `"None"`. That text is non-empty, so
it sails past the required-field guards that exist to reject such a file:

  bundle_id=null -> ACCEPTED: bundle_id='None' version='1.0.0'
  version=null   -> ACCEPTED: bundle_id='a'    version='None'
  id=null        -> ACCEPTED: components=[('presets', 'None')]

The phantom component then feeds the collateral-protection refcount, so
`components_still_needed()` reports ('presets', 'None') as protected.

Reuse `manifest._text`, the sibling helper whose docstring describes this
exact trap and which fixed the same bug for bundle.yml in #3798. The
falsy-non-list half of this hardening already landed in this file as #3666;
the explicit-null half was never mirrored here.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: jawwad-ali

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 "

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 == ""