From 65ec88b1edc6f1b9a395507d888431e5cc9533f2 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Fri, 31 Jul 2026 09:44:58 +0500 Subject: [PATCH] fix: escape Rich markup in `workflow resolve` output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `workflow resolve` printed two lines through `console.print`, which has Rich markup enabled, without escaping: 1. The layer tier was wrapped in literal brackets: `f" • [{layer.tier}] {layer.source} ..."`. Rich parsed `[base]` and `[project-overlay]` as style tags, so the tier label was swallowed on *every* invocation -- no untrusted input required. The column has never rendered. 2. Step attribution interpolated `composed.step_id` raw. Step IDs come from base-workflow / overlay YAML and are only validated against `:` (see `_parse_edit`), so brackets pass validation. A balanced `[stuff]` is silently swallowed; an unbalanced `[/red]` raises `rich.errors.MarkupError`, producing an uncaught traceback and exit 1 -- the workflow cannot be inspected at all. Route the interpolated fields through `rich.markup.escape` and escape the literal tier bracket as `\[`, matching the existing pattern in `workflow info`'s step graph and `workflow_list`'s `\[disabled]`. Only display is affected; the returned payload was already unescaped and is unchanged. Adds 3 regression tests, all of which fail without the fix: the tier label renders, and a step ID survives both the swallowing and the crashing markup cases. Co-Authored-By: Claude Opus 5 (1M context) Assisted-by: Claude Opus 5 (1M context) --- .../workflows/overlays/_commands.py | 14 +++- tests/workflows/test_overlay_commands.py | 80 +++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/workflows/overlays/_commands.py b/src/specify_cli/workflows/overlays/_commands.py index cec7d8534a..549f1ea151 100644 --- a/src/specify_cli/workflows/overlays/_commands.py +++ b/src/specify_cli/workflows/overlays/_commands.py @@ -7,6 +7,7 @@ import typer import yaml +from rich.markup import escape as _escape_markup from ..._console import console, err_console from ...extensions import normalize_priority @@ -412,14 +413,23 @@ def workflow_resolve(project_root: Path, workflow_id: str) -> dict[str, Any] | N priority = ( "n/a" if layer.tier == "base" else str(normalize_priority(layer.priority)) ) + # ``\[`` keeps the literal bracket: unescaped, Rich parses ``[base]`` / + # ``[project-overlay]`` as a style tag and swallows the tier label whole. console.print( - f" \u2022 [{layer.tier}] {layer.source} " + f" \u2022 \\[{_escape_markup(layer.tier)}] " + f"{_escape_markup(layer.source)} " f"(priority={priority})" ) console.print("Step attribution:") for composed in attribution: - console.print(f" \u2022 {composed.step_id}: {composed.source}") + # Step IDs come from base-workflow / overlay YAML, which only bans ``:`` + # \u2014 brackets pass validation, so they reach Rich as markup. A balanced + # ``[stuff]`` is swallowed; an unbalanced ``[/red]`` raises MarkupError. + console.print( + f" \u2022 {_escape_markup(composed.step_id)}: " + f"{_escape_markup(composed.source)}" + ) return { "workflow_id": workflow_id, diff --git a/tests/workflows/test_overlay_commands.py b/tests/workflows/test_overlay_commands.py index e068738005..8a344cacdf 100644 --- a/tests/workflows/test_overlay_commands.py +++ b/tests/workflows/test_overlay_commands.py @@ -509,6 +509,86 @@ def test_workflow_resolve(self, project_dir, monkeypatch): assert payload["layers"][-1]["tier"] == "base" assert payload["layers"][-1]["priority"] is None + def test_workflow_resolve_prints_tier_labels(self, project_dir, monkeypatch): + """Layer tiers render literally; an unescaped ``[base]`` is eaten as markup.""" + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + _write_workflow( + project_dir, + "wf", + { + "schema_version": "1.0", + "workflow": {"id": "wf", "name": "WF", "version": "1.0.0"}, + "steps": [{"id": "a", "type": "command", "command": "echo"}], + }, + ) + _write_overlay( + project_dir, + "wf", + "ov1", + { + "id": "ov1", + "extends": "wf", + "priority": 10, + "edits": [ + { + "operation": "insert_after", + "anchor": "a", + "step": {"id": "new", "type": "command", "command": "echo"}, + } + ], + }, + ) + + result = runner.invoke(app, ["workflow", "resolve", "wf"]) + assert result.exit_code == 0, result.output + assert "[base]" in result.output + assert "[project-overlay]" in result.output + + @pytest.mark.parametrize( + "step_id", + [ + # Balanced tag: silently swallowed, so the step vanishes from output. + "new[stuff]", + # Unbalanced closer: raises MarkupError -> traceback and exit 1. + "new[/red]", + ], + ) + def test_workflow_resolve_escapes_rich_markup_in_step_id( + self, project_dir, monkeypatch, step_id + ): + """Step IDs are unvalidated for brackets, so they must be escaped.""" + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + _write_workflow( + project_dir, + "wf", + { + "schema_version": "1.0", + "workflow": {"id": "wf", "name": "WF", "version": "1.0.0"}, + "steps": [{"id": "a", "type": "command", "command": "echo"}], + }, + ) + _write_overlay( + project_dir, + "wf", + "ov1", + { + "id": "ov1", + "extends": "wf", + "priority": 10, + "edits": [ + { + "operation": "insert_after", + "anchor": "a", + "step": {"id": step_id, "type": "command", "command": "echo"}, + } + ], + }, + ) + + result = runner.invoke(app, ["workflow", "resolve", "wf"]) + assert result.exit_code == 0, result.output + assert step_id in result.output + def test_workflow_resolve_equal_priority_layers_sort_by_source(self, project_dir, monkeypatch): """Equal-priority overlays are listed alphabetically by source.""" monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)