Skip to content

Commit 8a14af5

Browse files
mnriemCopilot
andcommitted
Validate deeply nested workflow and step catalogs
Classify JSON recursion during parsing or cache writes as malformed catalog data, recover from poisoned caches, and keep non-fetch step catalog errors blocking exact-ID lookup. Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 6995bd7 commit 8a14af5

4 files changed

Lines changed: 146 additions & 2 deletions

File tree

‎src/specify_cli/workflows/catalog/_domain.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,7 @@ def validate_payload(data: Any) -> dict[str, Any]:
544544
except (
545545
UnicodeDecodeError,
546546
json.JSONDecodeError,
547+
RecursionError,
547548
OSError,
548549
WorkflowCatalogValidationError,
549550
):
@@ -613,6 +614,7 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
613614
RedirectPolicyError,
614615
UnicodeError,
615616
json.JSONDecodeError,
617+
RecursionError,
616618
) as exc:
617619
raise WorkflowCatalogValidationError(
618620
f"Invalid workflow catalog from {entry.url}: {exc}"
@@ -627,6 +629,7 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
627629
except (
628630
json.JSONDecodeError,
629631
ValueError,
632+
RecursionError,
630633
OSError,
631634
WorkflowCatalogValidationError,
632635
):
@@ -647,6 +650,10 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
647650
json.dump({"url": entry.url, "fetched_at": time.time()}, f)
648651
except OSError:
649652
pass # Proceed without caching if disk write fails
653+
except RecursionError as exc:
654+
raise WorkflowCatalogValidationError(
655+
f"Invalid workflow catalog from {entry.url}: excessive nesting ({exc})"
656+
) from exc
650657

651658
return data
652659

‎src/specify_cli/workflows/step/catalog/_domain.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,7 @@ def validate_payload(data: Any) -> dict[str, Any]:
509509
except (
510510
UnicodeDecodeError,
511511
json.JSONDecodeError,
512+
RecursionError,
512513
OSError,
513514
StepCatalogValidationError,
514515
):
@@ -580,6 +581,7 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
580581
RedirectPolicyError,
581582
UnicodeError,
582583
json.JSONDecodeError,
584+
RecursionError,
583585
) as exc:
584586
raise StepCatalogValidationError(
585587
f"Invalid step catalog from {entry.url}: {exc}"
@@ -595,6 +597,7 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
595597
except (
596598
json.JSONDecodeError,
597599
ValueError,
600+
RecursionError,
598601
OSError,
599602
StepCatalogValidationError,
600603
):
@@ -615,6 +618,10 @@ def _validate_redirect(_old_url: str, new_url: str) -> None:
615618
json.dump({"url": entry.url, "fetched_at": time.time()}, f)
616619
except OSError:
617620
pass # Proceed without caching if disk write fails
621+
except RecursionError as exc:
622+
raise StepCatalogValidationError(
623+
f"Invalid step catalog from {entry.url}: excessive nesting ({exc})"
624+
) from exc
618625

619626
return data
620627

@@ -635,8 +642,8 @@ def _get_merged_steps(
635642
except _DuplicateCatalogField:
636643
raise
637644
except StepCatalogError as exc:
638-
if target_id is not None and isinstance(
639-
exc, StepCatalogValidationError
645+
if target_id is not None and not isinstance(
646+
exc, StepCatalogFetchError
640647
):
641648
raise
642649
if (

‎tests/specify_cli/bundles/test_references.py‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,108 @@ def open_url(url, **kwargs):
646646
assert warnings == []
647647

648648

649+
@pytest.mark.parametrize("kind", ["workflows", "steps"])
650+
@pytest.mark.parametrize("source", ["network", "fresh-cache", "stale-cache"])
651+
def test_deeply_nested_catalog_is_handled_as_malformed_data(
652+
tmp_path, monkeypatch, kind, source,
653+
):
654+
import io
655+
import json
656+
from urllib.error import URLError
657+
658+
from specify_cli.authentication import http
659+
from specify_cli.workflows.catalog import (
660+
StepCatalog,
661+
StepCatalogEntry,
662+
StepCatalogFetchError,
663+
StepCatalogValidationError,
664+
WorkflowCatalog,
665+
WorkflowCatalogEntry,
666+
WorkflowCatalogFetchError,
667+
WorkflowCatalogValidationError,
668+
)
669+
670+
catalog_type, entry_type, validation_error, fetch_error = {
671+
"workflows": (
672+
WorkflowCatalog, WorkflowCatalogEntry,
673+
WorkflowCatalogValidationError, WorkflowCatalogFetchError,
674+
),
675+
"steps": (
676+
StepCatalog, StepCatalogEntry,
677+
StepCatalogValidationError, StepCatalogFetchError,
678+
),
679+
}[kind]
680+
catalog = catalog_type(tmp_path)
681+
entry = entry_type("https://example.com/catalog.json", "trusted", 1, True)
682+
nested = b'{"nested":' + b"[" * 10000 + b"0" + b"]" * 10000 + b"}"
683+
valid = {"schema_version": "1.0", kind: {"requested": {"version": "1.0.0"}}}
684+
685+
class Response(io.BytesIO):
686+
def geturl(self):
687+
return entry.url
688+
689+
if source != "network":
690+
cache_file, _ = catalog._get_cache_paths(entry.url)
691+
cache_file.parent.mkdir(parents=True, exist_ok=True)
692+
cache_file.write_bytes(nested)
693+
monkeypatch.setattr(
694+
catalog, "_is_url_cache_valid", lambda _url: source == "fresh-cache"
695+
)
696+
697+
def open_url(*args, **kwargs):
698+
if source == "stale-cache":
699+
raise URLError("connection failed")
700+
payload = nested if source == "network" else json.dumps(valid).encode()
701+
return Response(payload)
702+
703+
monkeypatch.setattr(http, "open_url", open_url)
704+
if source == "network":
705+
with pytest.raises(validation_error, match="Invalid.*catalog"):
706+
catalog._fetch_single_catalog(entry, force_refresh=True)
707+
elif source == "stale-cache":
708+
with pytest.raises(fetch_error, match="Failed to fetch catalog"):
709+
catalog._fetch_single_catalog(entry)
710+
else:
711+
assert catalog._fetch_single_catalog(entry) == valid
712+
713+
714+
@pytest.mark.parametrize("kind", ["workflows", "steps"])
715+
def test_deep_catalog_does_not_escape_during_cache_write(
716+
tmp_path, monkeypatch, kind,
717+
):
718+
import io
719+
720+
from specify_cli.authentication import http
721+
from specify_cli.workflows.catalog import (
722+
StepCatalog,
723+
StepCatalogEntry,
724+
StepCatalogValidationError,
725+
WorkflowCatalog,
726+
WorkflowCatalogEntry,
727+
WorkflowCatalogValidationError,
728+
)
729+
730+
catalog_type, entry_type, error_type = {
731+
"workflows": (
732+
WorkflowCatalog, WorkflowCatalogEntry, WorkflowCatalogValidationError,
733+
),
734+
"steps": (StepCatalog, StepCatalogEntry, StepCatalogValidationError),
735+
}[kind]
736+
catalog = catalog_type(tmp_path)
737+
entry = entry_type("https://example.com/catalog.json", "trusted", 1, True)
738+
payload = b'{"nested":' + b"[" * 1200 + b"0" + b"]" * 1200 + b"}"
739+
740+
class Response(io.BytesIO):
741+
def geturl(self):
742+
return entry.url
743+
744+
monkeypatch.setattr(
745+
http, "open_url", lambda *args, **kwargs: Response(payload)
746+
)
747+
with pytest.raises(error_type, match="Invalid.*catalog"):
748+
catalog._fetch_single_catalog(entry, force_refresh=True)
749+
750+
649751
@pytest.mark.parametrize("kind", ["extensions", "workflows", "steps"])
650752
def test_targeted_lookup_stops_before_lower_priority_catalog(
651753
tmp_path, monkeypatch, kind,

‎tests/specify_cli/workflows/step/test_catalog_versions.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,34 @@ def test_list_catalog_rejects_duplicate_step_ids(project_dir, monkeypatch):
114114
catalog.get_step_info("deploy")
115115

116116

117+
@pytest.mark.parametrize("raised_during_fetch", [False, True])
118+
def test_targeted_lookup_rejects_duplicate_ids_before_lower_catalog(
119+
project_dir, monkeypatch, raised_during_fetch,
120+
):
121+
catalog = StepCatalog(project_dir)
122+
sources = [
123+
StepCatalogEntry("https://example.com/high.json", "high", 1, True),
124+
StepCatalogEntry("https://example.com/low.json", "low", 2, True),
125+
]
126+
monkeypatch.setattr(catalog, "get_active_catalogs", lambda: sources)
127+
128+
def fetch(source, force_refresh=False):
129+
if source.name == "high":
130+
if raised_during_fetch:
131+
raise StepCatalogError("Duplicate step ID 'deploy' in catalog 'high'.")
132+
return {"steps": [
133+
{"id": "deploy", "version": "1.0"},
134+
{"id": "deploy", "version": "2.0"},
135+
]}
136+
return {"steps": {"deploy": {"version": "3.0"}}}
137+
138+
monkeypatch.setattr(catalog, "_fetch_single_catalog", fetch)
139+
with pytest.raises(StepCatalogError, match="Duplicate step ID 'deploy'"):
140+
catalog.get_step_info("deploy")
141+
if raised_during_fetch:
142+
assert catalog.search(query="deploy")[0]["version"] == "3.0"
143+
144+
117145
@pytest.mark.parametrize("cached", [True, False])
118146
def test_duplicate_json_release_key_is_not_silently_overwritten(
119147
project_dir, monkeypatch, cached

0 commit comments

Comments
 (0)