From 0dee2faf11c65d33dfe59506626cf5791e548168 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Thu, 21 May 2026 18:21:13 +0500 Subject: [PATCH] fix(catalogs): reject boolean priority in extension and preset catalog readers (#2589) `bool` is a subclass of `int` in Python, so `int(True)` silently returns `1`. The extension- and preset-catalog config readers coerced priority with a bare `int(item.get("priority", idx + 1))`, which meant a YAML config like: catalogs: - name: mine url: https://example.com/catalog.json priority: yes # parses to True was silently accepted as a valid priority of 1, quietly reordering the catalog stack instead of raising the same `Invalid priority` error a typo of `priority: not-a-number` already raises. The sibling integration-catalog reader in `src/specify_cli/catalogs.py` already guards this case (see `catalogs.py:137`). This change mirrors that pattern in `extensions.py` and `presets.py` so the three catalog validators stay consistent, and adds regression tests for both readers matching the existing `test_load_catalog_config_rejects_boolean_priority` template in `tests/integrations/test_integration_catalog.py`. --- src/specify_cli/presets.py | 16 ++++++++++++++-- tests/test_presets.py | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index fd9d4745f..c6e75ae79 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1903,12 +1903,24 @@ class PresetCatalog: if not url: continue self._validate_catalog_url(url) + raw_priority = item.get("priority", idx + 1) + # Reject bools explicitly: ``bool`` is a subclass of ``int`` so + # ``int(True)`` silently returns 1, which would let a YAML + # ``priority: true`` slip through as a valid priority of 1. The + # sibling integration-catalog reader in ``catalogs.py`` already + # guards this; mirror the check here so the three catalog + # validators stay consistent. + if isinstance(raw_priority, bool): + raise PresetValidationError( + f"Invalid priority for catalog '{item.get('name', idx + 1)}': " + f"expected integer, got {raw_priority!r}" + ) try: - priority = int(item.get("priority", idx + 1)) + priority = int(raw_priority) except (TypeError, ValueError): raise PresetValidationError( f"Invalid priority for catalog '{item.get('name', idx + 1)}': " - f"expected integer, got {item.get('priority')!r}" + f"expected integer, got {raw_priority!r}" ) raw_install = item.get("install_allowed", False) if isinstance(raw_install, str): diff --git a/tests/test_presets.py b/tests/test_presets.py index 8fa700fa7..f1c0e95f4 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1830,6 +1830,31 @@ class TestPresetCatalogMultiCatalog: with pytest.raises(PresetValidationError, match="Invalid priority"): catalog._load_catalog_config(config_path) + def test_load_catalog_config_rejects_boolean_priority(self, project_dir): + """A YAML ``priority: true`` is a typo, not a request for priority 1. + + ``bool`` is a subclass of ``int`` in Python, so ``int(True)`` silently + returns ``1``. Without an explicit guard a malformed config like + ``priority: yes`` would be accepted as a valid priority of 1 and + silently change catalog ordering. The sibling integration-catalog + reader rejects this case (see ``catalogs.py``); the preset catalog + reader must stay consistent. + """ + config_path = project_dir / ".specify" / "preset-catalogs.yml" + config_path.write_text(yaml.dump({ + "catalogs": [ + { + "name": "bool-priority", + "url": "https://example.com/catalog.json", + "priority": True, + } + ] + })) + + catalog = PresetCatalog(project_dir) + with pytest.raises(PresetValidationError, match="Invalid priority|expected integer"): + catalog._load_catalog_config(config_path) + def test_load_catalog_config_install_allowed_string(self, project_dir): """Test that install_allowed accepts string values.""" config_path = project_dir / ".specify" / "preset-catalogs.yml"