diff --git a/src/specify_cli/extensions/_commands.py b/src/specify_cli/extensions/_commands.py index 7ae4f8e9f0..5f2bd35b6a 100644 --- a/src/specify_cli/extensions/_commands.py +++ b/src/specify_cli/extensions/_commands.py @@ -790,10 +790,24 @@ def extension_search( # Stats stats = [] - if ext.get('downloads') is not None: - stats.append(f"Downloads: {ext['downloads']:,}") - if ext.get('stars') is not None: - stats.append(f"Stars: {ext['stars']}") + downloads = ext.get('downloads') + if downloads is not None: + # Catalog fields are untrusted; a non-numeric ``downloads`` + # (e.g. the JSON string "1500") would crash the ``:,`` format + # with "Cannot specify ',' with 's'". Only group-format numbers, + # and escape the fallback: the joined stats are rendered as Rich + # markup, so a value like "[/red]foo" would raise MarkupError + # (matching how every other catalog field here is escaped). + stats.append( + f"Downloads: {downloads:,}" + if isinstance(downloads, (int, float)) + else f"Downloads: {_escape_markup(str(downloads))}" + ) + stars = ext.get('stars') + if stars is not None: + # Same untrusted-value/Rich-markup hazard as `downloads` above, + # in the same joined string. + stats.append(f"Stars: {_escape_markup(str(stars))}") if stats: console.print(f" [dim]{' | '.join(stats)}[/dim]") @@ -971,10 +985,24 @@ def _print_extension_info(ext_info: dict, manager): # Statistics stats = [] - if ext_info.get('downloads') is not None: - stats.append(f"Downloads: {ext_info['downloads']:,}") - if ext_info.get('stars') is not None: - stats.append(f"Stars: {ext_info['stars']}") + downloads = ext_info.get('downloads') + if downloads is not None: + # Catalog fields are untrusted; a non-numeric ``downloads`` (e.g. the + # JSON string "1500") would crash the ``:,`` format with "Cannot + # specify ',' with 's'". Only group-format numbers, and escape the + # fallback: the joined stats are rendered as Rich markup, so a value + # like "[/red]foo" would raise MarkupError (matching how every other + # catalog field here is escaped). + stats.append( + f"Downloads: {downloads:,}" + if isinstance(downloads, (int, float)) + else f"Downloads: {_escape_markup(str(downloads))}" + ) + stars = ext_info.get('stars') + if stars is not None: + # Same untrusted-value/Rich-markup hazard as `downloads` above, in the + # same joined string. + stats.append(f"Stars: {_escape_markup(str(stars))}") if stats: console.print(f"[bold]Statistics:[/bold] {' | '.join(stats)}") console.print() diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 61826aad5b..8236a0f640 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -4034,6 +4034,91 @@ def test_search_all_extensions(self, temp_dir): results = catalog.search() assert len(results) == 2 + @pytest.mark.parametrize( + "downloads", + [ + "1500", # plain string: crashed the ``:,`` format + "[/red]foo", # unbalanced closing tag: raises MarkupError unescaped + "[bold]x[/bold]", # balanced tags: would silently restyle the output + ], + ) + def test_info_renders_non_numeric_downloads(self, downloads): + """A non-numeric ``downloads`` from an untrusted catalog must not crash the + info renderer — neither with 'Cannot specify ',' with 's'' (the ``:,`` + format) nor with a Rich MarkupError (the joined stats are markup).""" + from unittest.mock import MagicMock + from specify_cli.extensions._commands import _print_extension_info + + manager = MagicMock() + manager.registry.is_installed.return_value = False + ext_info = { + "name": "Jira", "id": "jira", "version": "1.0.0", + "description": "desc", "downloads": downloads, # from catalog JSON + } + # Must not raise ValueError or rich.errors.MarkupError. + _print_extension_info(ext_info, manager) + + def test_info_renders_markup_bearing_stars(self): + """``stars`` sits in the same joined stats string as ``downloads`` and is + equally catalog-controlled, so it must be escaped too.""" + from unittest.mock import MagicMock + from specify_cli.extensions._commands import _print_extension_info + + manager = MagicMock() + manager.registry.is_installed.return_value = False + ext_info = { + "name": "Jira", "id": "jira", "version": "1.0.0", + "description": "desc", "stars": "[/red]x", + } + _print_extension_info(ext_info, manager) # must not raise MarkupError + + @pytest.mark.parametrize("downloads", ["1500", "[/red]foo"]) + def test_search_survives_non_numeric_downloads(self, temp_dir, downloads): + """`specify extension search` must not abort when a catalog entry's + ``downloads`` is a non-numeric string — not with a raw ValueError from the + ``:,`` format, nor with a Rich MarkupError from unescaped markup.""" + import yaml as yaml_module + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + project_dir = temp_dir / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + config_path = project_dir / ".specify" / "extension-catalogs.yml" + with open(config_path, "w") as f: + yaml_module.dump( + {"catalogs": [{ + "name": "test-catalog", + "url": ExtensionCatalog.DEFAULT_CATALOG_URL, + "priority": 1, "install_allowed": True, + }]}, f, + ) + + catalog = ExtensionCatalog(project_dir) + catalog_data = { + "schema_version": "1.0", + "extensions": {"jira": { + "name": "Jira", "id": "jira", "version": "1.0.0", + "description": "Jira integration", "author": "x", + "tags": ["jira"], "verified": True, + "downloads": downloads, # non-numeric, straight from catalog JSON + }}, + } + catalog.cache_dir.mkdir(parents=True, exist_ok=True) + catalog.cache_file.write_text(json.dumps(catalog_data)) + catalog.cache_metadata_file.write_text(json.dumps({ + "cached_at": datetime.now(timezone.utc).isoformat(), + "catalog_url": "http://test.com", + })) + + runner = CliRunner() + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "search"], catch_exceptions=True) + assert result.exit_code == 0, result.output + # Rendered literally (escaped), not interpreted as markup or dropped. + assert f"Downloads: {downloads}" in result.output + def test_search_by_query(self, temp_dir): """Test searching by query text.""" import yaml as yaml_module