diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 181b1eeef3..e8e43c59e1 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4304,19 +4304,32 @@ def _load_catalog_config(self, config_path: Path) -> Optional[List[PresetCatalog raise PresetValidationError( f"Failed to read catalog config {config_path}: {e}" ) + # Do NOT coerce with ``or {}`` here: that also turns a FALSY + # non-mapping top level (``[]``, ``false``, ``0``, ``''``) into ``{}`` + # and silently swallows it, while a TRUTHY non-mapping (``5``, a bare + # list) correctly raises below. Only an empty document/explicit + # ``null`` means "no document". if data is None: return None if not isinstance(data, dict): raise PresetValidationError( f"Invalid catalog config {config_path}: expected a mapping at root, got {type(data).__name__}" ) - catalogs_data = data.get("catalogs", []) - if not catalogs_data: + # Same asymmetry one nesting level down: the shape check has to run + # BEFORE the emptiness check, or a FALSY non-list ``catalogs`` value + # (``{}``, ``''``, ``0``, ``false``) is silently swallowed as "no + # catalogs" while a TRUTHY non-list (``catalogs: "not-a-list"``) + # correctly raises. An absent key or an explicit ``catalogs: null`` + # both keep their existing "nothing configured here" behavior. + catalogs_data = data.get("catalogs") + if catalogs_data is None: return None if not isinstance(catalogs_data, list): raise PresetValidationError( f"Invalid catalog config: 'catalogs' must be a list, got {type(catalogs_data).__name__}" ) + if not catalogs_data: + return None entries: List[PresetCatalogEntry] = [] for idx, item in enumerate(catalogs_data): if not isinstance(item, dict): diff --git a/tests/test_presets.py b/tests/test_presets.py index 749c96b78c..558da1bc4d 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3442,6 +3442,19 @@ def test_load_catalog_config_not_a_list(self, project_dir): with pytest.raises(PresetValidationError, match="must be a list"): catalog._load_catalog_config(config_path) + + @pytest.mark.parametrize("body", ["catalogs: {}\n", "catalogs: ''\n", "catalogs: 0\n", "catalogs: false\n"]) + def test_load_catalog_config_rejects_falsy_non_list_catalogs(self, project_dir, body): + """A FALSY non-list ``catalogs:`` value must raise, like a truthy one + (``catalogs: "not-a-list"``) already does. The shape check sat behind + the emptiness check, so these were silently swallowed as "no catalogs".""" + config_path = project_dir / ".specify" / "preset-catalogs.yml" + config_path.write_text(body, encoding="utf-8") + + catalog = PresetCatalog(project_dir) + with pytest.raises(PresetValidationError, match="must be a list"): + catalog._load_catalog_config(config_path) + def test_load_catalog_config_invalid_entry(self, project_dir): """Test that non-dict entry raises error.""" config_path = project_dir / ".specify" / "preset-catalogs.yml"