Skip to content

Commit 05cb3cb

Browse files
Noor-ul-ain001claudeCopilotmnriemCopilot
authored
fix(presets): tolerate non-string and non-list catalog fields in preset search/info (#3769)
* fix(presets): tolerate non-string and non-list catalog fields in preset search/info `preset search` and `preset info` crashed with a raw traceback on catalog payloads that are valid YAML/JSON but not string-typed. Catalog files are user-editable, so these shapes reach the code unvalidated. `PresetCatalog.search` had three unguarded assumptions: - `--author` called `.lower()` on the raw value → `AttributeError: 'int' object has no attribute 'lower'` for `author: 789`. - the query searchable-text join passed raw `name`/`description` through → `TypeError: sequence item 0: expected str instance, int found`. - the `--tag` filter iterated `tags` without a list check, so a scalar `tags: 5` (truthy, not iterable) raised `TypeError: 'int' object is not iterable`. PR #3743 fixed only the non-string *elements* of `tags` here; a non-list `tags` and the `author`/`name`/`description` fields were still unguarded. The sibling catalogs already handle all of these — `extensions/__init__.py` and `integrations/catalog.py` coerce with `str(...)` and gate on `isinstance(raw_tags, list)`. This aligns presets with them. The same scalar-`tags` crash reached the four display sites in `presets/_commands.py`, so those now gate on `isinstance(tags, list)`, matching `integrations/_query_commands.py`. Note `PresetManifest.tags` returns `self.data.get("tags", [])` and manifest validation does not enforce list-ness, so a local `preset.yaml` with `tags: 5` validates successfully and then crashed `preset info` — hence the guard on the local-manifest branch too. While here, `preset search` printed tags unescaped, so a tag containing `[bold]` was silently swallowed as a Rich style tag; it now routes through `_escape_markup` like the `preset list` line directly above it. Regression tests in `TestPresetTagsNonString` drive the full CLI path via CliRunner. All five fail before the fix, each with the exact exception it targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * chore: regenerate security audit requirements (annotated-doc 0.0.5) The Security Audit workflow's "Check committed audit requirements are current" step regenerates requirements with `uv pip compile --upgrade`, which now resolves annotated-doc==0.0.5. Re-sync the committed snapshot so the check passes. No pyproject dependency changes; upgrade drift only. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3d0333b-4a36-4f69-9273-3ac0c3f46481 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3d0333b-4a36-4f69-9273-3ac0c3f46481
1 parent 2e44ed6 commit 05cb3cb

4 files changed

Lines changed: 158 additions & 27 deletions

File tree

‎.github/security-audit-requirements.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
annotated-doc==0.0.4 \
2-
--hash=sha256:571ac1dc6991c450b25a9c2d84a3705e2ae7a53467b5d111c24fa8baabbed320 \
3-
--hash=sha256:fbcda96e87e9c92ad167c2e53839e57503ecfda18804ea28102353485033faa4
1+
annotated-doc==0.0.5 \
2+
--hash=sha256:117bac03a25ede5df5440e855b32d556049ca169ead221505badf432fed4b101 \
3+
--hash=sha256:c7e58ce09192557605d8bbd92836d7e1d520ac9580096042c0bfd197efacf1bb
44
# via typer
55
click==8.4.2 \
66
--hash=sha256:9a6cea6e60b17ebe0a44c5cc636d94f09bd66142c1cd7d8b4cd731c4917a15f6 \

‎src/specify_cli/presets/__init__.py‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4513,23 +4513,34 @@ def search(
45134513
results = []
45144514

45154515
for pack_id, pack_data in packs.items():
4516-
if author and pack_data.get("author", "").lower() != author.lower():
4517-
continue
4516+
if author:
4517+
author_val = pack_data.get("author", "")
4518+
if not isinstance(author_val, str):
4519+
author_val = str(author_val) if author_val is not None else ""
4520+
if author_val.lower() != author.lower():
4521+
continue
45184522

4519-
if tag and tag.lower() not in [
4520-
str(t).lower() for t in pack_data.get("tags", [])
4521-
]:
4522-
continue
4523+
if tag:
4524+
raw_tags = pack_data.get("tags", [])
4525+
tags_list = raw_tags if isinstance(raw_tags, list) else []
4526+
if tag.lower() not in [
4527+
str(t).lower() for t in tags_list
4528+
]:
4529+
continue
45234530

45244531
if query:
45254532
query_lower = query.lower()
4533+
raw_tags = pack_data.get("tags", [])
4534+
tags_list = raw_tags if isinstance(raw_tags, list) else []
4535+
name_val = pack_data.get("name", "")
4536+
desc_val = pack_data.get("description", "")
45264537
searchable_text = " ".join(
45274538
[
4528-
pack_data.get("name", ""),
4529-
pack_data.get("description", ""),
4539+
str(name_val) if name_val is not None else "",
4540+
str(desc_val) if desc_val is not None else "",
45304541
pack_id,
45314542
]
4532-
+ [str(t) for t in pack_data.get("tags", [])]
4543+
+ [str(t) for t in tags_list]
45334544
).lower()
45344545

45354546
if query_lower not in searchable_text:

‎src/specify_cli/presets/_commands.py‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@ def preset_list():
6161
pri = pack.get('priority', 10)
6262
console.print(f" [bold]{pack['name']}[/bold] ({pack['id']}) v{pack['version']} — {status} — priority {pri}")
6363
console.print(f" {pack['description']}")
64-
if pack.get("tags"):
65-
tags_str = _escape_markup(", ".join(str(t) for t in pack["tags"]))
64+
tags = pack.get("tags", [])
65+
if isinstance(tags, list) and tags:
66+
tags_str = _escape_markup(", ".join(str(t) for t in tags))
6667
console.print(f" [dim]Tags: {tags_str}[/dim]")
6768
console.print(f" [dim]Templates: {pack['template_count']}[/dim]")
6869
console.print()
@@ -293,8 +294,9 @@ def preset_search(
293294
for pack in results:
294295
console.print(f" [bold]{pack.get('name', pack['id'])}[/bold] ({pack['id']}) v{pack.get('version', '?')}")
295296
console.print(f" {pack.get('description', '')}")
296-
if pack.get("tags"):
297-
tags_str = ", ".join(str(t) for t in pack["tags"])
297+
tags = pack.get("tags", [])
298+
if isinstance(tags, list) and tags:
299+
tags_str = _escape_markup(", ".join(str(t) for t in tags))
298300
console.print(f" [dim]Tags: {tags_str}[/dim]")
299301
console.print()
300302

@@ -384,8 +386,9 @@ def preset_info(
384386
console.print(f" Description: {local_pack.description}")
385387
if local_pack.author:
386388
console.print(f" Author: {local_pack.author}")
387-
if local_pack.tags:
388-
console.print(f" Tags: {', '.join(str(t) for t in local_pack.tags)}")
389+
local_tags = local_pack.tags
390+
if isinstance(local_tags, list) and local_tags:
391+
console.print(f" Tags: {', '.join(str(t) for t in local_tags)}")
389392
console.print(f" Templates: {len(local_pack.templates)}")
390393
for tmpl in local_pack.templates:
391394
console.print(f" - {tmpl['name']} ({tmpl['type']}): {tmpl.get('description', '')}")
@@ -420,8 +423,9 @@ def preset_info(
420423
console.print(f" Description: {pack_info.get('description', '')}")
421424
if pack_info.get("author"):
422425
console.print(f" Author: {pack_info['author']}")
423-
if pack_info.get("tags"):
424-
console.print(f" Tags: {', '.join(str(t) for t in pack_info['tags'])}")
426+
catalog_tags = pack_info.get("tags", [])
427+
if isinstance(catalog_tags, list) and catalog_tags:
428+
console.print(f" Tags: {', '.join(str(t) for t in catalog_tags)}")
425429
if pack_info.get("repository"):
426430
console.print(f" Repository: {pack_info['repository']}")
427431
if pack_info.get("license"):

‎tests/test_presets.py‎

Lines changed: 123 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11933,18 +11933,21 @@ class TestPresetTagsNonString:
1193311933
this with ``str(t) for t in ...`` — presets must match.
1193411934
"""
1193511935

11936-
def _seed_catalog(self, project_dir, tags):
11936+
def _seed_catalog(self, project_dir, tags, extra=None):
1193711937
catalog = PresetCatalog(project_dir)
1193811938
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
11939+
pack = {
11940+
"name": "Numeric Tags",
11941+
"description": "Preset with non-string tags",
11942+
"version": "1.0.0",
11943+
"tags": tags,
11944+
}
11945+
if extra:
11946+
pack.update(extra)
1193911947
catalog_data = {
1194011948
"schema_version": "1.0",
1194111949
"presets": {
11942-
"numeric-tags": {
11943-
"name": "Numeric Tags",
11944-
"description": "Preset with non-string tags",
11945-
"version": "1.0.0",
11946-
"tags": tags,
11947-
},
11950+
"numeric-tags": pack,
1194811951
},
1194911952
}
1195011953
catalog.cache_file.write_text(json.dumps(catalog_data))
@@ -11988,3 +11991,116 @@ def test_info_renders_non_string_tags(self, project_dir):
1198811991
assert result.exit_code == 0, result.output
1198911992
plain = strip_ansi(result.output)
1199011993
assert "Tags: 1, 2" in plain
11994+
11995+
def _default_only(self, catalog):
11996+
return [PresetCatalogEntry(
11997+
url=catalog.DEFAULT_CATALOG_URL, name="default", priority=1, install_allowed=True
11998+
)]
11999+
12000+
def test_search_by_author_tolerates_non_string_author(self, project_dir):
12001+
"""``--author`` must not crash on a numeric catalog ``author``.
12002+
12003+
``PresetCatalog.search`` called ``.lower()`` straight on the raw value,
12004+
raising ``AttributeError: 'int' object has no attribute 'lower'``. The
12005+
sibling extension/integration catalogs coerce with ``str(...)`` first.
12006+
"""
12007+
from typer.testing import CliRunner
12008+
from unittest.mock import patch
12009+
from specify_cli import app
12010+
12011+
catalog = self._seed_catalog(project_dir, ["ci"], extra={"author": 789})
12012+
12013+
with patch.object(Path, "cwd", return_value=project_dir), \
12014+
patch.object(PresetCatalog, "get_active_catalogs",
12015+
return_value=self._default_only(catalog)):
12016+
result = CliRunner().invoke(app, ["preset", "search", "--author", "789"])
12017+
12018+
assert result.exit_code == 0, result.output
12019+
assert "Numeric Tags" in strip_ansi(result.output)
12020+
12021+
def test_search_query_tolerates_non_string_name_and_description(self, project_dir):
12022+
"""A query search must not crash on numeric ``name``/``description``.
12023+
12024+
The searchable-text join passed the raw values through, raising
12025+
``TypeError: sequence item 0: expected str instance, int found``.
12026+
"""
12027+
from typer.testing import CliRunner
12028+
from unittest.mock import patch
12029+
from specify_cli import app
12030+
12031+
catalog = self._seed_catalog(
12032+
project_dir, ["ci"], extra={"name": 123, "description": 456}
12033+
)
12034+
12035+
with patch.object(Path, "cwd", return_value=project_dir), \
12036+
patch.object(PresetCatalog, "get_active_catalogs",
12037+
return_value=self._default_only(catalog)):
12038+
result = CliRunner().invoke(app, ["preset", "search", "123"])
12039+
12040+
assert result.exit_code == 0, result.output
12041+
assert "numeric-tags" in strip_ansi(result.output)
12042+
12043+
def test_search_tolerates_non_list_tags(self, project_dir):
12044+
"""A scalar ``tags:`` value must not crash the tag filter or display.
12045+
12046+
``tags: 5`` is truthy but not iterable, so both the ``--tag`` filter and
12047+
the result-display join raised ``TypeError: 'int' object is not
12048+
iterable``. Siblings guard with ``isinstance(raw_tags, list)``.
12049+
"""
12050+
from typer.testing import CliRunner
12051+
from unittest.mock import patch
12052+
from specify_cli import app
12053+
12054+
catalog = self._seed_catalog(project_dir, 5)
12055+
12056+
with patch.object(Path, "cwd", return_value=project_dir), \
12057+
patch.object(PresetCatalog, "get_active_catalogs",
12058+
return_value=self._default_only(catalog)):
12059+
filtered = CliRunner().invoke(app, ["preset", "search", "--tag", "ci"])
12060+
displayed = CliRunner().invoke(app, ["preset", "search", "Numeric"])
12061+
12062+
assert filtered.exit_code == 0, filtered.output
12063+
assert "No presets found" in strip_ansi(filtered.output)
12064+
12065+
assert displayed.exit_code == 0, displayed.output
12066+
plain = strip_ansi(displayed.output)
12067+
assert "Numeric Tags" in plain
12068+
assert "Tags:" not in plain
12069+
12070+
def test_info_tolerates_non_list_tags(self, project_dir):
12071+
"""``preset info`` must not crash rendering a scalar ``tags:`` value."""
12072+
from typer.testing import CliRunner
12073+
from unittest.mock import patch
12074+
from specify_cli import app
12075+
12076+
catalog = self._seed_catalog(project_dir, 5)
12077+
12078+
with patch.object(Path, "cwd", return_value=project_dir), \
12079+
patch.object(PresetCatalog, "get_active_catalogs",
12080+
return_value=self._default_only(catalog)):
12081+
result = CliRunner().invoke(app, ["preset", "info", "numeric-tags"])
12082+
12083+
assert result.exit_code == 0, result.output
12084+
plain = strip_ansi(result.output)
12085+
assert "numeric-tags" in plain
12086+
assert "Tags:" not in plain
12087+
12088+
def test_search_escapes_rich_markup_in_tags(self, project_dir):
12089+
"""Bracketed tag text must survive Rich markup parsing.
12090+
12091+
``preset search`` printed tags unescaped, so a tag like ``[bold]`` was
12092+
swallowed as a style tag. ``preset list`` already escaped this.
12093+
"""
12094+
from typer.testing import CliRunner
12095+
from unittest.mock import patch
12096+
from specify_cli import app
12097+
12098+
catalog = self._seed_catalog(project_dir, ["[bold]ci"])
12099+
12100+
with patch.object(Path, "cwd", return_value=project_dir), \
12101+
patch.object(PresetCatalog, "get_active_catalogs",
12102+
return_value=self._default_only(catalog)):
12103+
result = CliRunner().invoke(app, ["preset", "search", "Numeric"])
12104+
12105+
assert result.exit_code == 0, result.output
12106+
assert "[bold]ci" in strip_ansi(result.output)

0 commit comments

Comments
 (0)