Skip to content

Commit ab203d3

Browse files
jawwad-aliclaude
andcommitted
fix(workflows): reject bool/.inf catalog priority in workflow & step catalog loaders
The WorkflowRegistry and StepRegistry catalog-config loaders coerced priority with int() inside except (TypeError, ValueError), missing two guards the base CatalogStackBase loader already has: - bool is an int subclass, so 'priority: true' was silently coerced to 1; - int(float('inf')) raises OverflowError (not caught), so 'priority: .inf' crashed with an uncaught traceback. Add the explicit bool check and OverflowError to both loaders, and add OverflowError to the two _coerce_priority helpers used by 'catalog add' (they return 0 on an uncoercible existing priority instead of crashing). Parametrized tests on both TestWorkflowCatalog and TestStepCatalog reject priority true/false/.inf (fail before: bool coerced to 1 / inf OverflowError). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent faeb956 commit ab203d3

2 files changed

Lines changed: 72 additions & 8 deletions

File tree

src/specify_cli/workflows/catalog.py

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -364,13 +364,24 @@ def _load_catalog_config(
364364
if not url:
365365
continue
366366
self._validate_catalog_url(url)
367+
raw_priority = item.get("priority", idx + 1)
368+
# bool is an int subclass: int(True) == 1 would silently accept a
369+
# ``priority: true`` as priority 1. Reject it explicitly, mirroring
370+
# the base CatalogStackBase loader.
371+
if isinstance(raw_priority, bool):
372+
raise WorkflowValidationError(
373+
f"Invalid priority for catalog "
374+
f"'{item.get('name', idx + 1)}': "
375+
f"expected integer, got {raw_priority!r}"
376+
)
367377
try:
368-
priority = int(item.get("priority", idx + 1))
369-
except (TypeError, ValueError):
378+
priority = int(raw_priority)
379+
except (TypeError, ValueError, OverflowError):
380+
# OverflowError: int(float("inf")) — a ``priority: .inf``.
370381
raise WorkflowValidationError(
371382
f"Invalid priority for catalog "
372383
f"'{item.get('name', idx + 1)}': "
373-
f"expected integer, got {item.get('priority')!r}"
384+
f"expected integer, got {raw_priority!r}"
374385
)
375386
raw_install = item.get("install_allowed", False)
376387
if isinstance(raw_install, str):
@@ -685,7 +696,9 @@ def add_catalog(self, url: str, name: str | None = None) -> None:
685696
def _coerce_priority(value: Any) -> int:
686697
try:
687698
return int(value)
688-
except (TypeError, ValueError):
699+
except (TypeError, ValueError, OverflowError):
700+
# OverflowError: int(float("inf")) — treat an uncoercible
701+
# existing priority as 0 rather than crashing 'catalog add'.
689702
return 0
690703

691704
max_priority = max(
@@ -1007,13 +1020,23 @@ def _load_catalog_config(
10071020
if not url:
10081021
continue
10091022
self._validate_catalog_url(url)
1023+
raw_priority = item.get("priority", idx + 1)
1024+
# bool is an int subclass: reject ``priority: true`` explicitly rather
1025+
# than silently coercing it to 1 (mirrors CatalogStackBase).
1026+
if isinstance(raw_priority, bool):
1027+
raise StepValidationError(
1028+
f"Invalid priority for catalog "
1029+
f"'{item.get('name', idx + 1)}': "
1030+
f"expected integer, got {raw_priority!r}"
1031+
)
10101032
try:
1011-
priority = int(item.get("priority", idx + 1))
1012-
except (TypeError, ValueError):
1033+
priority = int(raw_priority)
1034+
except (TypeError, ValueError, OverflowError):
1035+
# OverflowError: int(float("inf")) — a ``priority: .inf``.
10131036
raise StepValidationError(
10141037
f"Invalid priority for catalog "
10151038
f"'{item.get('name', idx + 1)}': "
1016-
f"expected integer, got {item.get('priority')!r}"
1039+
f"expected integer, got {raw_priority!r}"
10171040
)
10181041
raw_install = item.get("install_allowed", False)
10191042
if isinstance(raw_install, str):
@@ -1314,7 +1337,9 @@ def add_catalog(self, url: str, name: str | None = None) -> None:
13141337
def _coerce_priority(value: Any) -> int:
13151338
try:
13161339
return int(value)
1317-
except (TypeError, ValueError):
1340+
except (TypeError, ValueError, OverflowError):
1341+
# OverflowError: int(float("inf")) — treat an uncoercible
1342+
# existing priority as 0 rather than crashing 'catalog add'.
13181343
return 0
13191344

13201345
max_priority = max(

tests/test_workflows.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5232,6 +5232,26 @@ def test_project_level_config(self, project_dir):
52325232
assert len(entries) == 1
52335233
assert entries[0].name == "custom"
52345234

5235+
@pytest.mark.parametrize("bad_priority", [True, False, float("inf")])
5236+
def test_config_priority_bool_or_inf_rejected(self, project_dir, bad_priority):
5237+
"""`priority: true` must not be silently coerced to 1, and `priority: .inf`
5238+
must not crash with an uncaught OverflowError — both raise a clean
5239+
validation error (parity with the base CatalogStackBase loader)."""
5240+
from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError
5241+
5242+
config_path = project_dir / ".specify" / "workflow-catalogs.yml"
5243+
config_path.write_text(yaml.dump({
5244+
"catalogs": [{
5245+
"name": "bad",
5246+
"url": "https://example.com/wf-catalog.json",
5247+
"priority": bad_priority,
5248+
"install_allowed": True,
5249+
}]
5250+
}))
5251+
catalog = WorkflowCatalog(project_dir)
5252+
with pytest.raises(WorkflowValidationError, match="Invalid priority|expected integer"):
5253+
catalog.get_active_catalogs()
5254+
52355255
def test_validate_url_http_rejected(self, project_dir):
52365256
from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError
52375257

@@ -5755,6 +5775,25 @@ def test_project_level_config(self, project_dir):
57555775
assert len(entries) == 1
57565776
assert entries[0].name == "custom"
57575777

5778+
@pytest.mark.parametrize("bad_priority", [True, False, float("inf")])
5779+
def test_config_priority_bool_or_inf_rejected(self, project_dir, bad_priority):
5780+
"""`priority: true`/`.inf` in a step-catalog config raise a clean
5781+
validation error instead of coercing to 1 / crashing with OverflowError."""
5782+
from specify_cli.workflows.catalog import StepCatalog, StepValidationError
5783+
5784+
config_path = project_dir / ".specify" / "step-catalogs.yml"
5785+
config_path.write_text(yaml.dump({
5786+
"catalogs": [{
5787+
"name": "bad",
5788+
"url": "https://example.com/step-catalog.json",
5789+
"priority": bad_priority,
5790+
"install_allowed": True,
5791+
}]
5792+
}))
5793+
catalog = StepCatalog(project_dir)
5794+
with pytest.raises(StepValidationError, match="Invalid priority|expected integer"):
5795+
catalog.get_active_catalogs()
5796+
57585797
def test_validate_url_http_rejected(self, project_dir):
57595798
from specify_cli.workflows.catalog import StepCatalog, StepValidationError
57605799

0 commit comments

Comments
 (0)