github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-08-03 06:26:30 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(workflows): reject bool / .inf catalog priority in workflow & step catalog loaders (#3526)

Browse Source 
* 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>

* test(workflows): cover add_catalog() OverflowError fallback for existing priority: .inf

The workflow/step catalog priority guards added OverflowError to _coerce_priority
(the 'catalog add' fallback), but the tests only exercised get_active_catalogs().
Add tests that prewrite an existing 'priority: .inf' entry and call add_catalog()
for both WorkflowCatalog and StepCatalog, asserting the command succeeds and the
new entry gets a valid priority (inf coerced to 0, +1). Fails before: int(inf)
OverflowError crashed add_catalog.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ali jawwad
2026-07-18 01:10:41 +05:00
committed by GitHub
parent eb2252a1cb
commit 57cc518d63
2 changed files with 123 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
src/specify_cli/workflows/catalog.py
View File

@@ -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(

88
tests/test_workflows.py
View File

@@ -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