diff --git a/src/specify_cli/workflows/catalog.py b/src/specify_cli/workflows/catalog.py index 57cc502834..329aaf8192 100644 --- a/src/specify_cli/workflows/catalog.py +++ b/src/specify_cli/workflows/catalog.py @@ -364,13 +364,24 @@ def _load_catalog_config( if not url: continue self._validate_catalog_url(url) + raw_priority = item.get("priority", idx + 1) + # bool is an int subclass: int(True) == 1 would silently accept a + # ``priority: true`` as priority 1. Reject it explicitly, mirroring + # the base CatalogStackBase loader. + if isinstance(raw_priority, bool): + raise WorkflowValidationError( + f"Invalid priority for catalog " + f"'{item.get('name', idx + 1)}': " + f"expected integer, got {raw_priority!r}" + ) try: - priority = int(item.get("priority", idx + 1)) - except (TypeError, ValueError): + priority = int(raw_priority) + except (TypeError, ValueError, OverflowError): + # OverflowError: int(float("inf")) — a ``priority: .inf``. raise WorkflowValidationError( f"Invalid priority for catalog " f"'{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): @@ -685,7 +696,9 @@ def add_catalog(self, url: str, name: str | None = None) -> None: def _coerce_priority(value: Any) -> int: try: return int(value) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): + # OverflowError: int(float("inf")) — treat an uncoercible + # existing priority as 0 rather than crashing 'catalog add'. return 0 max_priority = max( @@ -1007,13 +1020,23 @@ def _load_catalog_config( if not url: continue self._validate_catalog_url(url) + raw_priority = item.get("priority", idx + 1) + # bool is an int subclass: reject ``priority: true`` explicitly rather + # than silently coercing it to 1 (mirrors CatalogStackBase). + if isinstance(raw_priority, bool): + raise StepValidationError( + f"Invalid priority for catalog " + f"'{item.get('name', idx + 1)}': " + f"expected integer, got {raw_priority!r}" + ) try: - priority = int(item.get("priority", idx + 1)) - except (TypeError, ValueError): + priority = int(raw_priority) + except (TypeError, ValueError, OverflowError): + # OverflowError: int(float("inf")) — a ``priority: .inf``. raise StepValidationError( f"Invalid priority for catalog " f"'{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): @@ -1314,7 +1337,9 @@ def add_catalog(self, url: str, name: str | None = None) -> None: def _coerce_priority(value: Any) -> int: try: return int(value) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): + # OverflowError: int(float("inf")) — treat an uncoercible + # existing priority as 0 rather than crashing 'catalog add'. return 0 max_priority = max( diff --git a/tests/test_workflows.py b/tests/test_workflows.py index eab4bef540..a65aea8d17 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -5775,6 +5775,26 @@ def test_project_level_config(self, project_dir): assert len(entries) == 1 assert entries[0].name == "custom" + @pytest.mark.parametrize("bad_priority", [True, False, float("inf")]) + def test_config_priority_bool_or_inf_rejected(self, project_dir, bad_priority): + """`priority: true` must not be silently coerced to 1, and `priority: .inf` + must not crash with an uncaught OverflowError — both raise a clean + validation error (parity with the base CatalogStackBase loader).""" + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError + + config_path = project_dir / ".specify" / "workflow-catalogs.yml" + config_path.write_text(yaml.dump({ + "catalogs": [{ + "name": "bad", + "url": "https://example.com/wf-catalog.json", + "priority": bad_priority, + "install_allowed": True, + }] + })) + catalog = WorkflowCatalog(project_dir) + with pytest.raises(WorkflowValidationError, match="Invalid priority|expected integer"): + catalog.get_active_catalogs() + def test_validate_url_http_rejected(self, project_dir): from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError @@ -5878,6 +5898,31 @@ def test_add_catalog(self, project_dir): assert len(data["catalogs"]) == 1 assert data["catalogs"][0]["url"] == "https://example.com/new-catalog.json" + def test_add_catalog_with_existing_inf_priority(self, project_dir): + """add_catalog() derives the new priority from existing ones via + _coerce_priority; an existing `priority: .inf` must not crash it + (int(float('inf')) is an OverflowError) — it is treated as 0 and the add + succeeds.""" + from specify_cli.workflows.catalog import WorkflowCatalog + + config_path = project_dir / ".specify" / "workflow-catalogs.yml" + config_path.parent.mkdir(parents=True, exist_ok=True) + config_path.write_text(yaml.dump({ + "catalogs": [{ + "name": "existing", + "url": "https://a.example.com/c.json", + "priority": float("inf"), + "install_allowed": True, + }] + })) + + catalog = WorkflowCatalog(project_dir) + catalog.add_catalog("https://b.example.com/c.json", "new") + + data = yaml.safe_load(config_path.read_text()) + new = next(c for c in data["catalogs"] if c["url"] == "https://b.example.com/c.json") + assert new["priority"] == 1 # max(inf coerced to 0) + 1 + def test_add_catalog_duplicate_rejected(self, project_dir): from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError @@ -6298,6 +6343,25 @@ def test_project_level_config(self, project_dir): assert len(entries) == 1 assert entries[0].name == "custom" + @pytest.mark.parametrize("bad_priority", [True, False, float("inf")]) + def test_config_priority_bool_or_inf_rejected(self, project_dir, bad_priority): + """`priority: true`/`.inf` in a step-catalog config raise a clean + validation error instead of coercing to 1 / crashing with OverflowError.""" + from specify_cli.workflows.catalog import StepCatalog, StepValidationError + + config_path = project_dir / ".specify" / "step-catalogs.yml" + config_path.write_text(yaml.dump({ + "catalogs": [{ + "name": "bad", + "url": "https://example.com/step-catalog.json", + "priority": bad_priority, + "install_allowed": True, + }] + })) + catalog = StepCatalog(project_dir) + with pytest.raises(StepValidationError, match="Invalid priority|expected integer"): + catalog.get_active_catalogs() + def test_validate_url_http_rejected(self, project_dir): from specify_cli.workflows.catalog import StepCatalog, StepValidationError @@ -6393,6 +6457,30 @@ def test_add_catalog(self, project_dir): assert len(data["catalogs"]) == 1 assert data["catalogs"][0]["url"] == "https://example.com/new-steps.json" + def test_add_catalog_with_existing_inf_priority(self, project_dir): + """Step-catalog add_catalog() must not crash when an existing entry has a + `priority: .inf` (int(float('inf')) is an OverflowError) — _coerce_priority + treats it as 0 and the add succeeds.""" + from specify_cli.workflows.catalog import StepCatalog + + config_path = project_dir / ".specify" / "step-catalogs.yml" + config_path.parent.mkdir(parents=True, exist_ok=True) + config_path.write_text(yaml.dump({ + "catalogs": [{ + "name": "existing", + "url": "https://a.example.com/s.json", + "priority": float("inf"), + "install_allowed": True, + }] + })) + + catalog = StepCatalog(project_dir) + catalog.add_catalog("https://b.example.com/s.json", "new") + + data = yaml.safe_load(config_path.read_text()) + new = next(c for c in data["catalogs"] if c["url"] == "https://b.example.com/s.json") + assert new["priority"] == 1 # max(inf coerced to 0) + 1 + def test_add_catalog_empty_yaml_file(self, project_dir): """An empty YAML config file should be treated as empty, not corrupted.""" from specify_cli.workflows.catalog import StepCatalog