From 205bed78ea657b7406a46dc2bb4912452ff9e415 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Tue, 28 Jul 2026 10:43:56 +0500 Subject: [PATCH] fix(presets): tolerate non-string and non-list catalog fields in preset search/info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) Assisted-by: Claude Opus 5 (1M context) --- src/specify_cli/presets/__init__.py | 29 ++++-- src/specify_cli/presets/_commands.py | 20 +++-- tests/test_presets.py | 130 +++++++++++++++++++++++++-- 3 files changed, 155 insertions(+), 24 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 93ad8fb80d..e7bdd88cd7 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4500,23 +4500,34 @@ def search( results = [] for pack_id, pack_data in packs.items(): - if author and pack_data.get("author", "").lower() != author.lower(): - continue + if author: + author_val = pack_data.get("author", "") + if not isinstance(author_val, str): + author_val = str(author_val) if author_val is not None else "" + if author_val.lower() != author.lower(): + continue - if tag and tag.lower() not in [ - str(t).lower() for t in pack_data.get("tags", []) - ]: - continue + if tag: + raw_tags = pack_data.get("tags", []) + tags_list = raw_tags if isinstance(raw_tags, list) else [] + if tag.lower() not in [ + str(t).lower() for t in tags_list + ]: + continue if query: query_lower = query.lower() + raw_tags = pack_data.get("tags", []) + tags_list = raw_tags if isinstance(raw_tags, list) else [] + name_val = pack_data.get("name", "") + desc_val = pack_data.get("description", "") searchable_text = " ".join( [ - pack_data.get("name", ""), - pack_data.get("description", ""), + str(name_val) if name_val else "", + str(desc_val) if desc_val else "", pack_id, ] - + [str(t) for t in pack_data.get("tags", [])] + + [str(t) for t in tags_list] ).lower() if query_lower not in searchable_text: diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 154346209f..9aceed9c56 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -60,8 +60,9 @@ def preset_list(): pri = pack.get('priority', 10) console.print(f" [bold]{pack['name']}[/bold] ({pack['id']}) v{pack['version']} — {status} — priority {pri}") console.print(f" {pack['description']}") - if pack.get("tags"): - tags_str = _escape_markup(", ".join(str(t) for t in pack["tags"])) + tags = pack.get("tags", []) + if isinstance(tags, list) and tags: + tags_str = _escape_markup(", ".join(str(t) for t in tags)) console.print(f" [dim]Tags: {tags_str}[/dim]") console.print(f" [dim]Templates: {pack['template_count']}[/dim]") console.print() @@ -287,8 +288,9 @@ def preset_search( for pack in results: console.print(f" [bold]{pack.get('name', pack['id'])}[/bold] ({pack['id']}) v{pack.get('version', '?')}") console.print(f" {pack.get('description', '')}") - if pack.get("tags"): - tags_str = ", ".join(str(t) for t in pack["tags"]) + tags = pack.get("tags", []) + if isinstance(tags, list) and tags: + tags_str = _escape_markup(", ".join(str(t) for t in tags)) console.print(f" [dim]Tags: {tags_str}[/dim]") console.print() @@ -378,8 +380,9 @@ def preset_info( console.print(f" Description: {local_pack.description}") if local_pack.author: console.print(f" Author: {local_pack.author}") - if local_pack.tags: - console.print(f" Tags: {', '.join(str(t) for t in local_pack.tags)}") + local_tags = local_pack.tags + if isinstance(local_tags, list) and local_tags: + console.print(f" Tags: {', '.join(str(t) for t in local_tags)}") console.print(f" Templates: {len(local_pack.templates)}") for tmpl in local_pack.templates: console.print(f" - {tmpl['name']} ({tmpl['type']}): {tmpl.get('description', '')}") @@ -414,8 +417,9 @@ def preset_info( console.print(f" Description: {pack_info.get('description', '')}") if pack_info.get("author"): console.print(f" Author: {pack_info['author']}") - if pack_info.get("tags"): - console.print(f" Tags: {', '.join(str(t) for t in pack_info['tags'])}") + catalog_tags = pack_info.get("tags", []) + if isinstance(catalog_tags, list) and catalog_tags: + console.print(f" Tags: {', '.join(str(t) for t in catalog_tags)}") if pack_info.get("repository"): console.print(f" Repository: {pack_info['repository']}") if pack_info.get("license"): diff --git a/tests/test_presets.py b/tests/test_presets.py index adf6484583..fdf7c19f7c 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -11717,18 +11717,21 @@ class TestPresetTagsNonString: this with ``str(t) for t in ...`` — presets must match. """ - def _seed_catalog(self, project_dir, tags): + def _seed_catalog(self, project_dir, tags, extra=None): catalog = PresetCatalog(project_dir) catalog.cache_dir.mkdir(parents=True, exist_ok=True) + pack = { + "name": "Numeric Tags", + "description": "Preset with non-string tags", + "version": "1.0.0", + "tags": tags, + } + if extra: + pack.update(extra) catalog_data = { "schema_version": "1.0", "presets": { - "numeric-tags": { - "name": "Numeric Tags", - "description": "Preset with non-string tags", - "version": "1.0.0", - "tags": tags, - }, + "numeric-tags": pack, }, } catalog.cache_file.write_text(json.dumps(catalog_data)) @@ -11772,3 +11775,116 @@ def test_info_renders_non_string_tags(self, project_dir): assert result.exit_code == 0, result.output plain = strip_ansi(result.output) assert "Tags: 1, 2" in plain + + def _default_only(self, catalog): + return [PresetCatalogEntry( + url=catalog.DEFAULT_CATALOG_URL, name="default", priority=1, install_allowed=True + )] + + def test_search_by_author_tolerates_non_string_author(self, project_dir): + """``--author`` must not crash on a numeric catalog ``author``. + + ``PresetCatalog.search`` called ``.lower()`` straight on the raw value, + raising ``AttributeError: 'int' object has no attribute 'lower'``. The + sibling extension/integration catalogs coerce with ``str(...)`` first. + """ + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + catalog = self._seed_catalog(project_dir, ["ci"], extra={"author": 789}) + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(PresetCatalog, "get_active_catalogs", + return_value=self._default_only(catalog)): + result = CliRunner().invoke(app, ["preset", "search", "--author", "789"]) + + assert result.exit_code == 0, result.output + assert "Numeric Tags" in strip_ansi(result.output) + + def test_search_query_tolerates_non_string_name_and_description(self, project_dir): + """A query search must not crash on numeric ``name``/``description``. + + The searchable-text join passed the raw values through, raising + ``TypeError: sequence item 0: expected str instance, int found``. + """ + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + catalog = self._seed_catalog( + project_dir, ["ci"], extra={"name": 123, "description": 456} + ) + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(PresetCatalog, "get_active_catalogs", + return_value=self._default_only(catalog)): + result = CliRunner().invoke(app, ["preset", "search", "123"]) + + assert result.exit_code == 0, result.output + assert "numeric-tags" in strip_ansi(result.output) + + def test_search_tolerates_non_list_tags(self, project_dir): + """A scalar ``tags:`` value must not crash the tag filter or display. + + ``tags: 5`` is truthy but not iterable, so both the ``--tag`` filter and + the result-display join raised ``TypeError: 'int' object is not + iterable``. Siblings guard with ``isinstance(raw_tags, list)``. + """ + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + catalog = self._seed_catalog(project_dir, 5) + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(PresetCatalog, "get_active_catalogs", + return_value=self._default_only(catalog)): + filtered = CliRunner().invoke(app, ["preset", "search", "--tag", "ci"]) + displayed = CliRunner().invoke(app, ["preset", "search", "Numeric"]) + + assert filtered.exit_code == 0, filtered.output + assert "No presets found" in strip_ansi(filtered.output) + + assert displayed.exit_code == 0, displayed.output + plain = strip_ansi(displayed.output) + assert "Numeric Tags" in plain + assert "Tags:" not in plain + + def test_info_tolerates_non_list_tags(self, project_dir): + """``preset info`` must not crash rendering a scalar ``tags:`` value.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + catalog = self._seed_catalog(project_dir, 5) + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(PresetCatalog, "get_active_catalogs", + return_value=self._default_only(catalog)): + result = CliRunner().invoke(app, ["preset", "info", "numeric-tags"]) + + assert result.exit_code == 0, result.output + plain = strip_ansi(result.output) + assert "numeric-tags" in plain + assert "Tags:" not in plain + + def test_search_escapes_rich_markup_in_tags(self, project_dir): + """Bracketed tag text must survive Rich markup parsing. + + ``preset search`` printed tags unescaped, so a tag like ``[bold]`` was + swallowed as a style tag. ``preset list`` already escaped this. + """ + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + catalog = self._seed_catalog(project_dir, ["[bold]ci"]) + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(PresetCatalog, "get_active_catalogs", + return_value=self._default_only(catalog)): + result = CliRunner().invoke(app, ["preset", "search", "Numeric"]) + + assert result.exit_code == 0, result.output + assert "[bold]ci" in strip_ansi(result.output)