Skip to content

Commit eae672f

Browse files
mnriemCopilot
andcommitted
fix(presets): fail closed on invalid catalog sources
Preserve unavailable-source fallback while surfacing malformed catalog JSON and payloads through lookup and CLI commands, so lower-priority installation cannot bypass discovery-only policy. Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 989b4e2 commit eae672f

4 files changed

Lines changed: 102 additions & 5 deletions

File tree

‎docs/reference/presets.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,9 @@ Duplicate JSON keys are rejected before parsing can discard a release record.
216216
Historical `requires.extensions` entries follow the preset manifest format:
217217
extension IDs or mappings with an `id`, optional version constraint, and
218218
optional boolean `required` flag.
219+
An invalid catalog payload fails resolution rather than allowing an entry
220+
from a lower-priority catalog to bypass its installation policy. Unreachable
221+
catalogs can still be skipped so other configured sources remain available.
219222

220223
```json
221224
{

‎src/specify_cli/presets/_catalog.py‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,29 @@
2323
from ._manifest import PresetError, PresetValidationError
2424

2525

26+
class PresetCatalogValidationError(PresetError):
27+
"""A catalog supplied invalid content rather than being unreachable."""
28+
29+
2630
def _decode_catalog_json(raw: str | bytes, url: str) -> Any:
2731
"""Reject duplicate keys before JSON parsing discards conflicting records."""
2832

2933
def unique_object(pairs: list[tuple[str, Any]]) -> dict[str, Any]:
3034
result: dict[str, Any] = {}
3135
for key, value in pairs:
3236
if key in result:
33-
raise PresetError(
37+
raise PresetCatalogValidationError(
3438
f"Invalid preset catalog format from {url}: duplicate JSON key '{key}'."
3539
)
3640
result[key] = value
3741
return result
3842

39-
return json.loads(raw, object_pairs_hook=unique_object)
43+
try:
44+
return json.loads(raw, object_pairs_hook=unique_object)
45+
except json.JSONDecodeError as exc:
46+
raise PresetCatalogValidationError(
47+
f"Invalid preset catalog format from {url}: invalid JSON ({exc})"
48+
) from exc
4049

4150

4251
@dataclass
@@ -190,17 +199,17 @@ def _validate_catalog_payload(self, catalog_data: Any, url: str) -> None:
190199
PresetError: If the payload's shape is invalid.
191200
"""
192201
if not isinstance(catalog_data, dict):
193-
raise PresetError(
202+
raise PresetCatalogValidationError(
194203
f"Invalid preset catalog format from {url}: "
195204
"expected a JSON object"
196205
)
197206
if (
198207
"schema_version" not in catalog_data
199208
or "presets" not in catalog_data
200209
):
201-
raise PresetError(f"Invalid preset catalog format from {url}")
210+
raise PresetCatalogValidationError(f"Invalid preset catalog format from {url}")
202211
if not isinstance(catalog_data.get("presets"), dict):
203-
raise PresetError(
212+
raise PresetCatalogValidationError(
204213
f"Invalid preset catalog format from {url}: "
205214
"'presets' must be a JSON object"
206215
)
@@ -545,6 +554,8 @@ def _get_merged_packs(self, force_refresh: bool = False) -> Dict[str, Dict[str,
545554
continue
546555
pack_data_with_catalog = {**pack_data, "_catalog_name": entry.name, "_install_allowed": entry.install_allowed}
547556
merged[pack_id] = pack_data_with_catalog
557+
except PresetCatalogValidationError:
558+
raise
548559
except PresetError:
549560
continue
550561

@@ -713,6 +724,8 @@ def search(
713724
"""
714725
try:
715726
packs = self._get_merged_packs()
727+
except PresetCatalogValidationError:
728+
raise
716729
except PresetError:
717730
return []
718731

@@ -771,6 +784,8 @@ def get_pack_info(
771784
"""
772785
try:
773786
packs = self._get_merged_packs()
787+
except PresetCatalogValidationError:
788+
raise
774789
except PresetError:
775790
return None
776791

‎src/specify_cli/presets/command_info.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from rich.markup import escape as _escape_markup
77

88
from .._console import console
9+
from ._catalog import PresetCatalogValidationError
910
from ._catalog_versions import available_versions
1011
from ._commands import preset_app
1112

@@ -86,6 +87,9 @@ def preset_info(
8687
catalog = PresetCatalog(project_root)
8788
try:
8889
pack_info = catalog.get_pack_info(preset_id)
90+
except PresetCatalogValidationError as exc:
91+
console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}")
92+
raise typer.Exit(1) from exc
8993
except PresetError:
9094
pack_info = None
9195

‎tests/specify_cli/presets/test_catalog_versions.py‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,81 @@ def fetch(source, _refresh):
318318
assert catalog.get_pack_info("sample", "1.0.0") is None
319319

320320

321+
@pytest.mark.parametrize(
322+
"bad_payload, error",
323+
[
324+
(_duplicate_release_json, "duplicate JSON key"),
325+
(lambda: b'{"schema_version": "1.0", "presets": []}', "Invalid preset catalog format"),
326+
(lambda: b'{"schema_version":', "invalid JSON"),
327+
],
328+
)
329+
def test_invalid_discovery_catalog_cannot_delegate_install(
330+
project_dir, bad_payload, error
331+
):
332+
high_url = "https://example.com/discovery.json"
333+
low_url = "https://example.com/trusted.json"
334+
sources = [
335+
PresetCatalogEntry(high_url, "discovery", 1, False),
336+
PresetCatalogEntry(low_url, "trusted", 2, True),
337+
]
338+
old_bytes = _archive()
339+
lower = json.dumps({
340+
"schema_version": "1.0",
341+
"presets": {"sample": _entry(old_bytes)},
342+
}).encode()
343+
opened: list[str] = []
344+
345+
def open_url(_self, url, **_kwargs):
346+
opened.append(url)
347+
data = {
348+
high_url: bad_payload(),
349+
low_url: lower,
350+
OLD_URL: old_bytes,
351+
}
352+
return _response(data[url], url)
353+
354+
with (
355+
patch.object(PresetCatalog, "get_active_catalogs", return_value=sources),
356+
patch.object(PresetCatalog, "_open_url", open_url),
357+
patch.object(Path, "cwd", return_value=project_dir),
358+
patch("specify_cli.get_speckit_version", return_value="1.0.0"),
359+
):
360+
with pytest.raises(PresetError, match=error):
361+
PresetCatalog(project_dir).get_pack_info("sample", "1.0.0")
362+
result = CliRunner().invoke(
363+
app, ["preset", "add", "sample", "--version", "1.0.0"]
364+
)
365+
info = CliRunner().invoke(app, ["preset", "info", "sample"])
366+
search = CliRunner().invoke(app, ["preset", "search", "sample"])
367+
assert result.exit_code == 1, result.output
368+
assert error in result.output
369+
assert info.exit_code == 1 and error in info.output
370+
assert search.exit_code == 1 and error in search.output
371+
assert OLD_URL not in opened
372+
assert PresetManager(project_dir).get_pack("sample") is None
373+
374+
375+
def test_unreachable_high_priority_catalog_still_uses_lower_source(project_dir):
376+
catalog = PresetCatalog(project_dir)
377+
sources = [
378+
PresetCatalogEntry("https://example.com/unavailable.json", "high", 1, False),
379+
PresetCatalogEntry("https://example.com/trusted.json", "low", 2, True),
380+
]
381+
382+
def fetch(source, _refresh):
383+
if source.name == "high":
384+
raise PresetError("Failed to fetch preset catalog: offline")
385+
return {"presets": {"sample": _entry()}}
386+
387+
with (
388+
patch.object(catalog, "get_active_catalogs", return_value=sources),
389+
patch.object(catalog, "_fetch_single_catalog", side_effect=fetch),
390+
):
391+
selected = catalog.get_pack_info("sample", "1.0.0")
392+
assert selected["_catalog_name"] == "low"
393+
assert selected["_install_allowed"] is True
394+
395+
321396
def test_discovery_only_winner_does_not_delegate_exact_release(project_dir):
322397
catalog = PresetCatalog(project_dir)
323398
sources = [

0 commit comments

Comments
 (0)