From 9e9f58b4659b9f079bdc6f86d604d7bf3b01d01a Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Sat, 25 Jul 2026 09:34:36 +0500 Subject: [PATCH] fix: escape Rich markup in catalog list output The `catalog list` subcommands for workflows, workflow steps, presets, and integrations printed user-editable catalog fields (name/url/ description from the `*-catalogs.yml` files) through `console.print` with Rich markup enabled. Any bracketed content such as a description `Does [stuff] nicely` was parsed as a style tag and silently swallowed, and a malformed tag could raise while rendering. Route each untrusted field through the module's already-imported `escape` helper, matching the pattern already used by `extension catalog list`. Adds regression tests for all four commands that inject bracketed name/url/description and assert the brackets survive verbatim in the output. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integrations/_query_commands.py | 9 ++-- src/specify_cli/presets/_commands.py | 6 +-- src/specify_cli/workflows/_commands.py | 12 ++--- .../integrations/test_integration_catalog.py | 34 ++++++++++++ tests/test_presets.py | 22 ++++++++ tests/test_workflows.py | 54 +++++++++++++++++++ 6 files changed, 124 insertions(+), 13 deletions(-) diff --git a/src/specify_cli/integrations/_query_commands.py b/src/specify_cli/integrations/_query_commands.py index bb47e6142e..a4fb8ee52d 100644 --- a/src/specify_cli/integrations/_query_commands.py +++ b/src/specify_cli/integrations/_query_commands.py @@ -489,13 +489,14 @@ def integration_catalog_list(): display_name = str(raw_name).strip() if raw_name is not None else "" if not display_name: display_name = f"catalog-{i + 1}" + safe_name = _rich_escape(display_name) if env_override or project_configs is None: - console.print(f" - [bold]{display_name}[/bold] — {install_status}") + console.print(f" - [bold]{safe_name}[/bold] — {install_status}") else: - console.print(f" [{i}] [bold]{display_name}[/bold] — {install_status}") - console.print(f" {cfg.get('url', '')}") + console.print(f" [{i}] [bold]{safe_name}[/bold] — {install_status}") + console.print(f" {_rich_escape(str(cfg.get('url', '')))}") if cfg.get("description"): - console.print(f" [dim]{cfg['description']}[/dim]") + console.print(f" [dim]{_rich_escape(str(cfg['description']))}[/dim]") console.print() diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index ba2b85b351..8c58e086a2 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -580,10 +580,10 @@ def preset_catalog_list(): if entry.install_allowed else "[yellow]discovery only[/yellow]" ) - console.print(f" [bold]{entry.name}[/bold] (priority {entry.priority})") + console.print(f" [bold]{_escape_markup(str(entry.name))}[/bold] (priority {entry.priority})") if entry.description: - console.print(f" {entry.description}") - console.print(f" URL: {entry.url}") + console.print(f" {_escape_markup(str(entry.description))}") + console.print(f" URL: {_escape_markup(str(entry.url))}") console.print(f" Install: {install_str}") console.print() diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index f088519934..f7b49f2270 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -2424,10 +2424,10 @@ def workflow_catalog_list(): console.print("\n[bold cyan]Workflow Catalog Sources:[/bold cyan]\n") for i, cfg in enumerate(configs): install_status = "[green]install allowed[/green]" if cfg["install_allowed"] else "[yellow]discovery only[/yellow]" - console.print(f" [{i}] [bold]{cfg['name']}[/bold] — {install_status}") - console.print(f" {cfg['url']}") + console.print(f" [{i}] [bold]{_escape_markup(str(cfg['name']))}[/bold] — {install_status}") + console.print(f" {_escape_markup(str(cfg['url']))}") if cfg.get("description"): - console.print(f" [dim]{cfg['description']}[/dim]") + console.print(f" [dim]{_escape_markup(str(cfg['description']))}[/dim]") console.print() @@ -3067,10 +3067,10 @@ def workflow_step_catalog_list(): if cfg["install_allowed"] else "[yellow]discovery only[/yellow]" ) - console.print(f" [{i}] [bold]{cfg['name']}[/bold] — {install_status}") - console.print(f" {cfg['url']}") + console.print(f" [{i}] [bold]{_escape_markup(str(cfg['name']))}[/bold] — {install_status}") + console.print(f" {_escape_markup(str(cfg['url']))}") if cfg.get("description"): - console.print(f" [dim]{cfg['description']}[/dim]") + console.print(f" [dim]{_escape_markup(str(cfg['description']))}[/dim]") console.print() diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index 2b6c76161c..2e9706e77d 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -633,6 +633,40 @@ def test_list_without_catalog_still_works(self, tmp_path): assert "copilot" in result.output assert "installed" in result.output + def test_catalog_list_escapes_rich_markup(self, tmp_path, monkeypatch): + """User-editable catalog name/url/description must not be parsed as Rich markup.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.integrations.catalog import IntegrationCatalog + runner = CliRunner() + project = self._init_project(tmp_path) + + configs = [ + { + "name": "Bracket [Catalog]", + "url": "https://example.com/[cat].json", + "description": "desc [with] brackets", + "install_allowed": True, + }, + ] + monkeypatch.setattr( + IntegrationCatalog, + "get_project_catalog_configs", + lambda self: [dict(c) for c in configs], + ) + + old = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "catalog", "list"]) + finally: + os.chdir(old) + + assert result.exit_code == 0, result.output + assert "Bracket [Catalog]" in result.output + assert "https://example.com/[cat].json" in result.output + assert "desc [with] brackets" in result.output + # --------------------------------------------------------------------------- # CLI: integration upgrade diff --git a/tests/test_presets.py b/tests/test_presets.py index 193d86b6ae..b10e10b2f0 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2637,6 +2637,28 @@ def test_default_active_catalogs(self, project_dir): assert active[1].priority == 2 assert active[1].install_allowed is False + def test_catalog_list_escapes_rich_markup(self, project_dir): + """User-editable catalog name/url/description must not be parsed as Rich markup.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + entry = PresetCatalogEntry( + url="https://example.com/[cat].json", + name="Bracket [Catalog]", + priority=1, + install_allowed=True, + description="desc [with] brackets", + ) + runner = CliRunner() + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(PresetCatalog, "get_active_catalogs", return_value=[entry]): + result = runner.invoke(app, ["preset", "catalog", "list"]) + assert result.exit_code == 0, result.output + assert "Bracket [Catalog]" in result.output + assert "https://example.com/[cat].json" in result.output + assert "desc [with] brackets" in result.output + def test_env_var_overrides_catalogs(self, project_dir, monkeypatch): """Test that SPECKIT_PRESET_CATALOG_URL env var overrides defaults.""" monkeypatch.setenv( diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 54cea4d770..af196e846f 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -10052,6 +10052,60 @@ def test_search_escapes_rich_markup_in_catalog_fields(self, project_dir, monkeyp assert "desc [with] brackets" in result.output assert "tag[1]" in result.output + def test_catalog_list_escapes_rich_markup(self, project_dir, monkeypatch): + """User-editable catalog name/url/description must not be parsed as Rich markup.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog + + monkeypatch.chdir(project_dir) + configs = [ + { + "name": "Bracket [Catalog]", + "url": "https://example.com/[cat].json", + "description": "desc [with] brackets", + "install_allowed": True, + }, + ] + monkeypatch.setattr( + WorkflowCatalog, + "get_catalog_configs", + lambda self: [dict(c) for c in configs], + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "catalog", "list"]) + assert result.exit_code == 0, result.output + assert "Bracket [Catalog]" in result.output + assert "https://example.com/[cat].json" in result.output + assert "desc [with] brackets" in result.output + + def test_step_catalog_list_escapes_rich_markup(self, project_dir, monkeypatch): + """User-editable step-catalog name/url/description must not be parsed as Rich markup.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import StepCatalog + + monkeypatch.chdir(project_dir) + configs = [ + { + "name": "Bracket [Step]", + "url": "https://example.com/[step].json", + "description": "step [with] brackets", + "install_allowed": True, + }, + ] + monkeypatch.setattr( + StepCatalog, + "get_catalog_configs", + lambda self: [dict(c) for c in configs], + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "step", "catalog", "list"]) + assert result.exit_code == 0, result.output + assert "Bracket [Step]" in result.output + assert "https://example.com/[step].json" in result.output + assert "step [with] brackets" in result.output + # -- update ---------------------------------------------------------- def test_update_no_workflows_installed(self, project_dir, monkeypatch):