From 9e77f3cd1a1e46b489cc9e7d3ef046f82fa891aa Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Sat, 16 May 2026 03:30:47 +0500 Subject: [PATCH] fix(catalogs): reject boolean priority in extension and preset catalog readers `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/extensions.py | 16 ++++++++++++++-- src/specify_cli/presets.py | 16 ++++++++++++++-- tests/test_extensions.py | 27 +++++++++++++++++++++++++++ tests/test_presets.py | 25 +++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index f657de06ce..dcb93e88b1 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1768,12 +1768,24 @@ def _load_catalog_config(self, config_path: Path) -> Optional[List[CatalogEntry] skipped_entries.append(idx) 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 ValidationError( + 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 ValidationError( 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/src/specify_cli/presets.py b/src/specify_cli/presets.py index fd9d4745f1..c6e75ae790 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1903,12 +1903,24 @@ def _load_catalog_config(self, config_path: Path) -> Optional[List[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_extensions.py b/tests/test_extensions.py index 1434ba309d..a20b15447a 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -2854,6 +2854,33 @@ def test_load_catalog_config_localhost_allowed(self, temp_dir): assert len(entries) == 1 assert entries[0].url == "http://localhost:8000/catalog.json" + def test_load_catalog_config_rejects_boolean_priority(self, temp_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 extension catalog + reader must stay consistent. + """ + project_dir = self._make_project(temp_dir) + self._write_catalog_config( + project_dir, + [ + { + "name": "bool-priority", + "url": "https://example.com/catalog.json", + "priority": True, + "install_allowed": True, + } + ], + ) + + catalog = ExtensionCatalog(project_dir) + with pytest.raises(ValidationError, match="Invalid priority|expected integer"): + catalog.get_active_catalogs() + # --- Merge conflict resolution --- def test_merge_conflict_higher_priority_wins(self, temp_dir): diff --git a/tests/test_presets.py b/tests/test_presets.py index 8fa700fa77..f1c0e95f4e 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1830,6 +1830,31 @@ def test_load_catalog_config_invalid_priority(self, project_dir): 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"