diff --git a/src/specify_cli/workflows/catalog.py b/src/specify_cli/workflows/catalog.py index 57cc50283..329aaf819 100644 --- a/src/specify_cli/workflows/catalog.py +++ b/src/specify_cli/workflows/catalog.py @@ -364,13 +364,24 @@ class WorkflowCatalog: if not url: continue self._validate_catalog_url(url) - try: - priority = int(item.get("priority", idx + 1)) - except (TypeError, ValueError): + 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 {item.get('priority')!r}" + f"expected integer, got {raw_priority!r}" + ) + try: + 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 {raw_priority!r}" ) raw_install = item.get("install_allowed", False) if isinstance(raw_install, str): @@ -685,7 +696,9 @@ class WorkflowCatalog: 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 @@ class StepCatalog: if not url: continue self._validate_catalog_url(url) - try: - priority = int(item.get("priority", idx + 1)) - except (TypeError, ValueError): + 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 {item.get('priority')!r}" + f"expected integer, got {raw_priority!r}" + ) + try: + 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 {raw_priority!r}" ) raw_install = item.get("install_allowed", False) if isinstance(raw_install, str): @@ -1314,7 +1337,9 @@ class StepCatalog: 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 d1da0c145..b1d984153 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -5862,6 +5862,26 @@ class TestWorkflowCatalog: 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 @@ -5965,6 +5985,31 @@ class TestWorkflowCatalog: 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 @@ -6385,6 +6430,25 @@ class TestStepCatalog: 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 @@ -6480,6 +6544,30 @@ class TestStepCatalog: 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