From 6c5c1e8e1e6c4efd70a41ce3db440a864515177b Mon Sep 17 00:00:00 2001 From: Michael Schmitt Date: Wed, 22 Apr 2026 19:44:42 +0900 Subject: [PATCH 1/5] feat: add colorized option for highstate output Colorize unified diffs in the highstate outputter: added lines are green, removed lines are red, hunk headers are cyan, and context lines are gray. `newfile:` and other change keys are rendered with the same indentation as the current functionality provides. --- changelog/68982.added.md | 6 + doc/ref/cli/_includes/output-options.rst | 15 +- doc/ref/configuration/master.rst | 10 + doc/ref/configuration/minion.rst | 10 + salt/output/highstate.py | 148 +++++++++++++- tests/pytests/unit/output/test_highstate.py | 213 ++++++++++++++++++++ 6 files changed, 397 insertions(+), 5 deletions(-) create mode 100644 changelog/68982.added.md diff --git a/changelog/68982.added.md b/changelog/68982.added.md new file mode 100644 index 000000000000..b6c4d421763b --- /dev/null +++ b/changelog/68982.added.md @@ -0,0 +1,6 @@ +Added ``_color`` modifier for ``state_output``: setting ``state_output`` to +``full_color``, ``terse_color``, ``mixed_color``, ``changes_color``, or +``filter_color`` enables colorized unified diff output in the highstate +outputter. Added lines are green, removed lines are red, hunk headers +(``@@``) are cyan, file headers (``---``) are red, and context lines are +gray. All other behavior is identical to the base mode without ``_color``. diff --git a/doc/ref/cli/_includes/output-options.rst b/doc/ref/cli/_includes/output-options.rst index a70c48ac153a..708623219f69 100644 --- a/doc/ref/cli/_includes/output-options.rst +++ b/doc/ref/cli/_includes/output-options.rst @@ -44,8 +44,19 @@ Output Options .. option:: --state-output=STATE_OUTPUT, --state_output=STATE_OUTPUT Override the configured state_output value for minion - output. One of 'full', 'terse', 'mixed', 'changes' or - 'filter'. Default: 'none'. + output. One of ``full``, ``terse``, ``mixed``, ``changes`` or + ``filter``. Default: ``none``. + + Each mode accepts two optional suffixes: + + * ``_id`` — use the state ID as the display name instead of the state's + ``name`` value (e.g. ``full_id``). + * ``_color`` — colorize unified diffs in the changes section: added lines + green, removed lines red, hunk headers (``@@``) cyan, file headers + (``---``) red, context lines gray (e.g. ``full_color``). + + The two suffixes can be combined in either order, e.g. ``full_id_color`` + or ``full_color_id``. .. option:: --state-verbose=STATE_VERBOSE, --state_verbose=STATE_VERBOSE diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index 3cb473072f0c..b1d4844c6db8 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -3255,10 +3255,20 @@ The state_output setting controls which results will be output full multi line: ``full_id``, ``mixed_id``, ``changes_id`` and ``terse_id`` are also allowed; when set, the state ID will be used as name in the output. +Any of the above modes can be suffixed with ``_color`` (e.g. ``full_color``, +``mixed_color``) to enable colorized unified diff output in the changes +section. Added lines are shown in green, removed lines in red, hunk headers +in cyan, and context lines in gray. All other output behavior is identical to +the mode without the ``_color`` suffix. + .. code-block:: yaml state_output: full +.. code-block:: yaml + + state_output: full_color + .. conf_master:: state_output_diff ``state_output_diff`` diff --git a/doc/ref/configuration/minion.rst b/doc/ref/configuration/minion.rst index a4a9354697c3..74da50a0c5d4 100644 --- a/doc/ref/configuration/minion.rst +++ b/doc/ref/configuration/minion.rst @@ -2408,10 +2408,20 @@ The state_output setting controls which results will be output full multi line: ``full_id``, ``mixed_id``, ``changes_id`` and ``terse_id`` are also allowed; when set, the state ID will be used as name in the output. +Any of the above modes can be suffixed with ``_color`` (e.g. ``full_color``, +``mixed_color``) to enable colorized unified diff output in the changes +section. Added lines are shown in green, removed lines in red, hunk headers +in cyan, and context lines in gray. All other output behavior is identical to +the mode without the ``_color`` suffix. + .. code-block:: yaml state_output: full +.. code-block:: yaml + + state_output: full_color + .. conf_minion:: state_output_diff ``state_output_diff`` diff --git a/salt/output/highstate.py b/salt/output/highstate.py index dc00885753fa..2ca5d23cfe07 100644 --- a/salt/output/highstate.py +++ b/salt/output/highstate.py @@ -44,13 +44,22 @@ These can be set as such from the command line, or in the Salt config as `state_output_exclude` or `state_output_terse`, respectively. - The output modes have one modifier: + The output modes have two modifiers that can be combined: ``full_id``, ``terse_id``, ``mixed_id``, ``changes_id`` and ``filter_id`` If ``_id`` is used, then the corresponding form will be used, but the value for ``name`` will be drawn from the state ID. This is useful for cases where the name value might be very long and hard to read. + ``full_color``, ``terse_color``, ``mixed_color``, ``changes_color`` and ``filter_color`` + If ``_color`` is used, unified diffs in the changes section will be + colorized: added lines in green, removed lines in red, hunk headers + (``@@``) in cyan, file headers (``---``) in red, and context lines in + gray. All other output behavior is identical to the base mode. + + The ``_id`` and ``_color`` modifiers can be combined, e.g. ``full_id_color`` + or ``full_color_id``. + state_tabular: If `state_output` uses the terse output, set this to `True` for an aligned output format. If you wish to use a custom format, this can be set to a @@ -479,6 +488,9 @@ def _format_host(host, data, indent_level=1): tcolor = colors["LIGHT_YELLOW"] state_output = __opts__.get("state_output", "full").lower() + # Strip the _color modifier before all mode comparisons so that + # e.g. "full_color" behaves identically to "full" for layout purposes. + state_output = state_output.replace("_color", "") comps = tname.split("_|-") if state_output.endswith("_id"): @@ -742,15 +754,138 @@ def _counts(label, count): return "\n".join(hstrs), nchanges > 0 +def _render_diff(diff_str, indent): + """ + Render a unified diff string with per-line ANSI colorization. + + Each line is colored according to its unified-diff role: + ``---`` / ``+++`` (file headers) → LIGHT_RED (bold) + ``@@`` (hunk header) → CYAN + ``+`` (added line) → GREEN + ``-`` (removed line) → RED + context lines (leading space) → GREEN (same as other change values) + + The ``indent`` argument (an integer) is prepended as spaces to every line, + matching the nesting depth used by the surrounding nested outputter output. + """ + prefix = " " * indent + + if __opts__.get("color") is False: + return "\n".join(prefix + line for line in diff_str.splitlines()) + + colors = salt.utils.color.get_colors(True, __opts__.get("color_theme")) + GREEN = str(colors["GREEN"]) + ENDC = str(colors["ENDC"]) + RED = str(colors["RED"]) + CYAN = str(colors["CYAN"]) + WHITE = str(colors["LIGHT_GRAY"]) + LIGHT_RED = str(colors["LIGHT_RED"]) + + result = [] + for line in diff_str.splitlines(): + if line.startswith("---"): + color = LIGHT_RED + elif line.startswith("+++"): + color = GREEN + elif line.startswith("@@"): + color = CYAN + elif line.startswith("+"): + color = GREEN + elif line.startswith("-"): + color = RED + else: + color = WHITE + result.append(f"{prefix}{color}{line}{ENDC}") + return "\n".join(result) + + +def _render_changes_dict(changes, indent): + """ + Render a changes dict as indented lines, mirroring nested outputter style. + + Does not go through the Salt loader, so nested_indent is guaranteed to + apply correctly regardless of Salt version. Returns a list of strings + (no trailing newline). + """ + colors = salt.utils.color.get_colors( + __opts__.get("color"), __opts__.get("color_theme") + ) + CYAN = str(colors["CYAN"]) + GREEN = str(colors["GREEN"]) + ENDC = str(colors["ENDC"]) + + val_indent = indent + 4 + pad = " " * indent + val_pad = " " * val_indent + lines = [] + # Top-level separator (mirrors what NestDisplay.display does for Mapping at indent>0) + lines.append(f"{pad}{CYAN}----------{ENDC}") + for key in sorted(changes): + lines.append(f"{pad}{CYAN}{key}:{ENDC}") + val = changes[key] + if isinstance(val, str): + lines.extend( + f"{val_pad}{GREEN}{line}{ENDC}" for line in val.splitlines() + ) + elif isinstance(val, dict): + lines.extend(_render_changes_dict(val, val_indent)) + else: + lines.append(f"{val_pad}{GREEN}{val}{ENDC}") + return lines + + def _nested_changes(changes): """ - Print the changes data using the nested outputter + Print the changes data using the nested outputter. """ ret = "\n" ret += salt.output.out_format(changes, "nested", __opts__, nested_indent=14) return ret +def _nested_changes_colorized(changes): + """ + Print the changes data with diff colorization (used when state_output + contains the ``_color`` modifier, e.g. ``full_color``). + + If the changes dict contains a ``diff`` key whose value is a string, that + diff is rendered with per-line color (added=green, removed=red, etc.). + All other values are rendered by ``_render_changes_dict`` which mirrors the + nested outputter layout without going through the Salt loader, ensuring + correct indentation on all Salt versions. + """ + diff_str = None + if isinstance(changes, dict) and isinstance(changes.get("diff"), str): + diff_str = changes.pop("diff") + + # key_indent=14: "----------" separator and key names sit at 14 spaces. + # val_indent=18: string values sit 4 spaces deeper. + key_indent = 14 + val_indent = key_indent + 4 + + colors = salt.utils.color.get_colors( + __opts__.get("color"), __opts__.get("color_theme") + ) + CYAN = str(colors["CYAN"]) + ENDC = str(colors["ENDC"]) + + ret = "\n" + if changes: + ret += "\n".join(_render_changes_dict(changes, key_indent)) + elif diff_str is not None: + # No other keys: emit the separator manually. + ret += f"{' ' * key_indent}{CYAN}----------{ENDC}" + + if diff_str is not None: + key_line = f"{' ' * key_indent}{CYAN}diff:{ENDC}" + rendered_diff = _render_diff(diff_str, val_indent) + ret += "\n" + key_line + "\n" + rendered_diff + # Restore the diff key so the caller's data structure is unchanged. + changes["diff"] = diff_str + + return ret + + def _format_changes(changes, orchestration=False): """ Format the changes dict based on what the data is @@ -758,7 +893,11 @@ def _format_changes(changes, orchestration=False): if not changes: return False, "" + colorize = "_color" in __opts__.get("state_output", "").lower() + if orchestration: + if colorize: + return True, _nested_changes_colorized(changes) return True, _nested_changes(changes) if not isinstance(changes, dict): @@ -774,7 +913,10 @@ def _format_changes(changes, orchestration=False): changed = changed or c else: changed = True - ctext = _nested_changes(changes) + if colorize: + ctext = _nested_changes_colorized(changes) + else: + ctext = _nested_changes(changes) return changed, ctext diff --git a/tests/pytests/unit/output/test_highstate.py b/tests/pytests/unit/output/test_highstate.py index 7661a17aa350..9189348f4615 100644 --- a/tests/pytests/unit/output/test_highstate.py +++ b/tests/pytests/unit/output/test_highstate.py @@ -917,3 +917,216 @@ def test_nested_output(): assert " Succeeded: 2 (changed=1)" in ret assert " Failed: 0" in ret assert " Total states run: 2" in ret + + +# --------------------------------------------------------------------------- +# Tests for diff colorization +# --------------------------------------------------------------------------- + +# ANSI color codes used by salt.utils.color when color=True +_GREEN = "\x1b[0;32m" +_RED = "\x1b[0;31m" +_CYAN = "\x1b[0;36m" +_WHITE = "\x1b[0;37m" +_LIGHT_RED = "\x1b[0;1;31m" +_ENDC = "\x1b[0;0m" + + +def _strip_ansi(text): + """Remove all ANSI escape sequences from *text*.""" + return re.sub(r"\x1b\[[0-9;]+m", "", text) + + +def _leading_color(line): + """Return the first ANSI escape code found on *line*, or '' if none.""" + m = re.search(r"(\x1b\[[0-9;]+m)", line) + return m.group(1) if m else "" + + +import re # noqa: E402 (re is already imported at top; harmless duplicate) + + +class TestRenderDiff: + """Tests for highstate._render_diff — the direct diff colorizer.""" + + @pytest.fixture(autouse=True) + def _setup(self, minion_opts): + minion_opts.update({"color": True, "color_theme": None}) + with patch.dict(highstate.__opts__, minion_opts): + yield + + def _render(self, diff_str, indent=18): + """Call _render_diff and return the output lines.""" + return highstate._render_diff(diff_str, indent).splitlines() + + # ------------------------------------------------------------------ + # Individual line-type tests + # ------------------------------------------------------------------ + def test_added_line_is_green(self): + out = self._render("+added content\n") + assert _leading_color(out[0]) == _GREEN + + def test_removed_line_is_red(self): + out = self._render("-removed content\n") + assert _leading_color(out[0]) == _RED + + def test_hunk_header_is_cyan(self): + out = self._render("@@ -1,3 +1,4 @@\n") + assert _leading_color(out[0]) == _CYAN + + def test_file_header_minus_is_light_red(self): + out = self._render("--- /etc/motd\n") + assert _leading_color(out[0]) == _LIGHT_RED + + def test_file_header_plus_is_green(self): + out = self._render("+++ /etc/motd\n") + assert _leading_color(out[0]) == _GREEN + + def test_context_line_is_white(self): + out = self._render(" a context line\n") + assert _leading_color(out[0]) == _WHITE + + # ------------------------------------------------------------------ + # Indentation + # ------------------------------------------------------------------ + def test_indent_is_applied(self): + out = self._render("+line\n", indent=18) + assert _strip_ansi(out[0]).startswith(" " * 18) + + def test_indent_zero(self): + out = self._render("+line\n", indent=0) + assert _strip_ansi(out[0]).startswith("+") + + # ------------------------------------------------------------------ + # Content preservation + # ------------------------------------------------------------------ + def test_content_is_preserved(self): + diff = ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,3 @@\n" + " context\n" + "-old line\n" + "+new line\n" + ) + out = self._render(diff, indent=18) + plain = [_strip_ansi(line).strip() for line in out] + assert plain == [ + "--- /etc/motd", + "+++ /etc/motd", + "@@ -1,2 +1,3 @@", + "context", + "-old line", + "+new line", + ] + + # ------------------------------------------------------------------ + # No-color mode: plain text, still indented + # ------------------------------------------------------------------ + def test_no_color_produces_plain_indented_text(self, minion_opts): + minion_opts.update({"color": False}) + with patch.dict(highstate.__opts__, minion_opts): + out = highstate._render_diff("+line\n-line\n", indent=18) + assert "\x1b" not in out + for line in out.splitlines(): + assert line.startswith(" " * 18) + + # ------------------------------------------------------------------ + # End-to-end: diff surfaced through the full highstate output pipeline + # ------------------------------------------------------------------ + def _run_diff_color_test(self, minion_opts): + state_data = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": { + "diff": ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + "+new line\n" + ) + }, + "comment": "File /etc/motd updated", + "duration": 10.0, + "name": "/etc/motd", + "result": True, + "start_time": "10:00:00.000000", + }, + } + } + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + + # Removed line must be wrapped in RED + assert _RED in rendered + # Added line must be wrapped in GREEN + assert _GREEN in rendered + # Plain text must still be present + assert "-old line" in _strip_ansi(rendered) + assert "+new line" in _strip_ansi(rendered) + # Diff lines must be indented (18 spaces for value inside nested_indent=14) + for line in rendered.splitlines(): + plain = _strip_ansi(line) + if "-old line" in plain or "+new line" in plain: + assert plain.startswith( + " " * 18 + ), f"Expected 18-space indent: {repr(plain)}" + + def test_diff_in_full_color_output(self, minion_opts): + """file.managed diff has red removed and green added lines with full_color mode.""" + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + self._run_diff_color_test(minion_opts) + + def test_diff_in_full_color_output_color_default(self, minion_opts): + """With color=None (default), full_color mode still colorizes diff lines.""" + minion_opts.update( + { + "color": None, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + self._run_diff_color_test(minion_opts) + + def test_diff_not_colorized_in_plain_full_output(self, minion_opts): + """With state_output=full (no _color), diff is not colorized.""" + state_data = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": {"diff": "-old line\n+new line\n"}, + "comment": "File /etc/motd updated", + "duration": 10.0, + "name": "/etc/motd", + "result": True, + "start_time": "10:00:00.000000", + }, + } + } + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + # The diff-specific RED should not appear without _color mode + assert _RED not in rendered From 8a915cc95821f86b6b9d827218bac60666088832 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 12 Jun 2026 20:38:33 -0700 Subject: [PATCH 2/5] Fix pre-commit failures in highstate colorizer - Black reformat the lines.extend() generator call in _render_changes_dict - Replace U+2192 arrows in _render_diff docstring with ASCII -> so the cp1252 docstring-encoding hook passes (these characters break salt-run -d and salt -d on Windows where stdout uses the locale-default encoding) --- salt/output/highstate.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/salt/output/highstate.py b/salt/output/highstate.py index 2ca5d23cfe07..d7a513519b19 100644 --- a/salt/output/highstate.py +++ b/salt/output/highstate.py @@ -759,11 +759,11 @@ def _render_diff(diff_str, indent): Render a unified diff string with per-line ANSI colorization. Each line is colored according to its unified-diff role: - ``---`` / ``+++`` (file headers) → LIGHT_RED (bold) - ``@@`` (hunk header) → CYAN - ``+`` (added line) → GREEN - ``-`` (removed line) → RED - context lines (leading space) → GREEN (same as other change values) + ``---`` / ``+++`` (file headers) -> LIGHT_RED (bold) + ``@@`` (hunk header) -> CYAN + ``+`` (added line) -> GREEN + ``-`` (removed line) -> RED + context lines (leading space) -> GREEN (same as other change values) The ``indent`` argument (an integer) is prepended as spaces to every line, matching the nesting depth used by the surrounding nested outputter output. @@ -824,9 +824,7 @@ def _render_changes_dict(changes, indent): lines.append(f"{pad}{CYAN}{key}:{ENDC}") val = changes[key] if isinstance(val, str): - lines.extend( - f"{val_pad}{GREEN}{line}{ENDC}" for line in val.splitlines() - ) + lines.extend(f"{val_pad}{GREEN}{line}{ENDC}" for line in val.splitlines()) elif isinstance(val, dict): lines.extend(_render_changes_dict(val, val_indent)) else: From 55921262ee3a8d10866125f6867a8fa4034de7f6 Mon Sep 17 00:00:00 2001 From: Michael Schmitt Date: Fri, 17 Jul 2026 13:18:09 +0900 Subject: [PATCH 3/5] fix: update highstate colorizer per PR feedback --- salt/output/highstate.py | 128 +++---- tests/pytests/unit/output/test_highstate.py | 361 ++++++++++---------- 2 files changed, 234 insertions(+), 255 deletions(-) diff --git a/salt/output/highstate.py b/salt/output/highstate.py index d7a513519b19..084c872b4337 100644 --- a/salt/output/highstate.py +++ b/salt/output/highstate.py @@ -454,7 +454,7 @@ def _format_host(host, data, indent_level=1): schanged = True nchanges += 1 else: - schanged, ctext = _format_changes(ret["changes"]) + schanged, ctext = _format_changes(ret["changes"], colors) # if compressed, the changes are keyed by name if schanged and compressed_count > 1: nchanges += len(ret["changes"].get("compressed changes", {})) or 1 @@ -754,7 +754,7 @@ def _counts(label, count): return "\n".join(hstrs), nchanges > 0 -def _render_diff(diff_str, indent): +def _render_diff(diff_str, indent, colors): """ Render a unified diff string with per-line ANSI colorization. @@ -763,17 +763,18 @@ def _render_diff(diff_str, indent): ``@@`` (hunk header) -> CYAN ``+`` (added line) -> GREEN ``-`` (removed line) -> RED - context lines (leading space) -> GREEN (same as other change values) + context lines (leading space) -> LIGHT_GRAY The ``indent`` argument (an integer) is prepended as spaces to every line, matching the nesting depth used by the surrounding nested outputter output. + The ``colors`` dict must be pre-built by the caller (e.g. from + ``salt.utils.color.get_colors``). """ prefix = " " * indent if __opts__.get("color") is False: return "\n".join(prefix + line for line in diff_str.splitlines()) - colors = salt.utils.color.get_colors(True, __opts__.get("color_theme")) GREEN = str(colors["GREEN"]) ENDC = str(colors["ENDC"]) RED = str(colors["RED"]) @@ -799,39 +800,6 @@ def _render_diff(diff_str, indent): return "\n".join(result) -def _render_changes_dict(changes, indent): - """ - Render a changes dict as indented lines, mirroring nested outputter style. - - Does not go through the Salt loader, so nested_indent is guaranteed to - apply correctly regardless of Salt version. Returns a list of strings - (no trailing newline). - """ - colors = salt.utils.color.get_colors( - __opts__.get("color"), __opts__.get("color_theme") - ) - CYAN = str(colors["CYAN"]) - GREEN = str(colors["GREEN"]) - ENDC = str(colors["ENDC"]) - - val_indent = indent + 4 - pad = " " * indent - val_pad = " " * val_indent - lines = [] - # Top-level separator (mirrors what NestDisplay.display does for Mapping at indent>0) - lines.append(f"{pad}{CYAN}----------{ENDC}") - for key in sorted(changes): - lines.append(f"{pad}{CYAN}{key}:{ENDC}") - val = changes[key] - if isinstance(val, str): - lines.extend(f"{val_pad}{GREEN}{line}{ENDC}" for line in val.splitlines()) - elif isinstance(val, dict): - lines.extend(_render_changes_dict(val, val_indent)) - else: - lines.append(f"{val_pad}{GREEN}{val}{ENDC}") - return lines - - def _nested_changes(changes): """ Print the changes data using the nested outputter. @@ -841,52 +809,68 @@ def _nested_changes(changes): return ret -def _nested_changes_colorized(changes): +def _nested_changes_colorized(changes, colors): """ Print the changes data with diff colorization (used when state_output contains the ``_color`` modifier, e.g. ``full_color``). - If the changes dict contains a ``diff`` key whose value is a string, that - diff is rendered with per-line color (added=green, removed=red, etc.). - All other values are rendered by ``_render_changes_dict`` which mirrors the - nested outputter layout without going through the Salt loader, ensuring - correct indentation on all Salt versions. + Replaces every ``diff`` string value at any nesting depth with a unique + sentinel, delegates all structural formatting to the native nested + outputter, then swaps each sentinel line back for a per-line colorized + diff. Note: ``colors`` must be pre-built by the caller. """ - diff_str = None - if isinstance(changes, dict) and isinstance(changes.get("diff"), str): - diff_str = changes.pop("diff") + sentinels = {} # sentinel string -> raw diff string + + def _replace_diffs(obj): + if isinstance(obj, dict): + return { + k: ( + _assign_sentinel(v) + if k == "diff" and isinstance(v, str) + else _replace_diffs(v) + ) + for k, v in obj.items() + } + if isinstance(obj, list): + return [_replace_diffs(i) for i in obj] + return obj - # key_indent=14: "----------" separator and key names sit at 14 spaces. - # val_indent=18: string values sit 4 spaces deeper. - key_indent = 14 - val_indent = key_indent + 4 + def _assign_sentinel(diff_str): + sentinel = f"__COLORDIFF_{len(sentinels)}__" + sentinels[sentinel] = diff_str + return sentinel - colors = salt.utils.color.get_colors( - __opts__.get("color"), __opts__.get("color_theme") + nested_output = salt.output.out_format( + _replace_diffs(changes), "nested", __opts__, nested_indent=14 ) - CYAN = str(colors["CYAN"]) - ENDC = str(colors["ENDC"]) - ret = "\n" - if changes: - ret += "\n".join(_render_changes_dict(changes, key_indent)) - elif diff_str is not None: - # No other keys: emit the separator manually. - ret += f"{' ' * key_indent}{CYAN}----------{ENDC}" - - if diff_str is not None: - key_line = f"{' ' * key_indent}{CYAN}diff:{ENDC}" - rendered_diff = _render_diff(diff_str, val_indent) - ret += "\n" + key_line + "\n" + rendered_diff - # Restore the diff key so the caller's data structure is unchanged. - changes["diff"] = diff_str + for sentinel, diff_str in sentinels.items(): + # The nested outputter renders each sentinel as a single indented line + # (possibly wrapped in ANSI codes). Capture the leading whitespace to + # determine the indent depth, then replace with colorized diff lines. + pattern = re.compile( + r"^( *)(?:\x1b\[[0-9;]*m)*" + re.escape(sentinel) + r"(?:\x1b\[[0-9;]*m)*$", + re.MULTILINE, + ) - return ret + def _make_replacer(ds): + def _replacer(m): + return _render_diff(ds, len(m.group(1)), colors) + + return _replacer + nested_output = pattern.sub(_make_replacer(diff_str), nested_output) -def _format_changes(changes, orchestration=False): + return "\n" + nested_output + + +def _format_changes(changes, colors, orchestration=False): """ - Format the changes dict based on what the data is + Format the changes dict based on what the data is. + + ``colors`` is the pre-built ANSI color mapping from the calling + ``_format_host`` invocation; passing it in avoids re-initializing the + color engine on every state block. """ if not changes: return False, "" @@ -895,7 +879,7 @@ def _format_changes(changes, orchestration=False): if orchestration: if colorize: - return True, _nested_changes_colorized(changes) + return True, _nested_changes_colorized(changes, colors) return True, _nested_changes(changes) if not isinstance(changes, dict): @@ -912,7 +896,7 @@ def _format_changes(changes, orchestration=False): else: changed = True if colorize: - ctext = _nested_changes_colorized(changes) + ctext = _nested_changes_colorized(changes, colors) else: ctext = _nested_changes(changes) return changed, ctext diff --git a/tests/pytests/unit/output/test_highstate.py b/tests/pytests/unit/output/test_highstate.py index 9189348f4615..471de6652e7f 100644 --- a/tests/pytests/unit/output/test_highstate.py +++ b/tests/pytests/unit/output/test_highstate.py @@ -937,196 +937,191 @@ def _strip_ansi(text): return re.sub(r"\x1b\[[0-9;]+m", "", text) -def _leading_color(line): - """Return the first ANSI escape code found on *line*, or '' if none.""" - m = re.search(r"(\x1b\[[0-9;]+m)", line) - return m.group(1) if m else "" - - -import re # noqa: E402 (re is already imported at top; harmless duplicate) - - -class TestRenderDiff: - """Tests for highstate._render_diff — the direct diff colorizer.""" - - @pytest.fixture(autouse=True) - def _setup(self, minion_opts): - minion_opts.update({"color": True, "color_theme": None}) - with patch.dict(highstate.__opts__, minion_opts): - yield - - def _render(self, diff_str, indent=18): - """Call _render_diff and return the output lines.""" - return highstate._render_diff(diff_str, indent).splitlines() - - # ------------------------------------------------------------------ - # Individual line-type tests - # ------------------------------------------------------------------ - def test_added_line_is_green(self): - out = self._render("+added content\n") - assert _leading_color(out[0]) == _GREEN - - def test_removed_line_is_red(self): - out = self._render("-removed content\n") - assert _leading_color(out[0]) == _RED - - def test_hunk_header_is_cyan(self): - out = self._render("@@ -1,3 +1,4 @@\n") - assert _leading_color(out[0]) == _CYAN - - def test_file_header_minus_is_light_red(self): - out = self._render("--- /etc/motd\n") - assert _leading_color(out[0]) == _LIGHT_RED - - def test_file_header_plus_is_green(self): - out = self._render("+++ /etc/motd\n") - assert _leading_color(out[0]) == _GREEN - - def test_context_line_is_white(self): - out = self._render(" a context line\n") - assert _leading_color(out[0]) == _WHITE - - # ------------------------------------------------------------------ - # Indentation - # ------------------------------------------------------------------ - def test_indent_is_applied(self): - out = self._render("+line\n", indent=18) - assert _strip_ansi(out[0]).startswith(" " * 18) - - def test_indent_zero(self): - out = self._render("+line\n", indent=0) - assert _strip_ansi(out[0]).startswith("+") - - # ------------------------------------------------------------------ - # Content preservation - # ------------------------------------------------------------------ - def test_content_is_preserved(self): - diff = ( - "--- /etc/motd\n" - "+++ /etc/motd\n" - "@@ -1,2 +1,3 @@\n" - " context\n" - "-old line\n" - "+new line\n" - ) - out = self._render(diff, indent=18) - plain = [_strip_ansi(line).strip() for line in out] - assert plain == [ - "--- /etc/motd", - "+++ /etc/motd", - "@@ -1,2 +1,3 @@", - "context", - "-old line", - "+new line", - ] - - # ------------------------------------------------------------------ - # No-color mode: plain text, still indented - # ------------------------------------------------------------------ - def test_no_color_produces_plain_indented_text(self, minion_opts): - minion_opts.update({"color": False}) - with patch.dict(highstate.__opts__, minion_opts): - out = highstate._render_diff("+line\n-line\n", indent=18) - assert "\x1b" not in out - for line in out.splitlines(): - assert line.startswith(" " * 18) - - # ------------------------------------------------------------------ - # End-to-end: diff surfaced through the full highstate output pipeline - # ------------------------------------------------------------------ - def _run_diff_color_test(self, minion_opts): - state_data = { - "minion": { - "file_|-/etc/motd_|-/etc/motd_|-managed": { - "__id__": "/etc/motd", - "__run_num__": 0, - "__sls__": "motd", - "changes": { +_MOTD_STATE = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": { + "diff": ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + "+new line\n" + ) + }, + "comment": "File /etc/motd updated", + "duration": 10.0, + "name": "/etc/motd", + "result": True, + "start_time": "10:00:00.000000", + }, + } +} + + +def _assert_diff_colorized(rendered): + assert _RED in rendered + assert _GREEN in rendered + assert "-old line" in _strip_ansi(rendered) + assert "+new line" in _strip_ansi(rendered) + for line in rendered.splitlines(): + plain = _strip_ansi(line) + if "-old line" in plain or "+new line" in plain: + assert plain.startswith( + " " * 18 + ), f"Expected 18-space indent: {repr(plain)}" + + +def test_diff_in_full_color_output(minion_opts): + """file.managed diff has red removed and green added lines with full_color mode.""" + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + _assert_diff_colorized(highstate.output(_MOTD_STATE)) + + +def test_diff_in_full_color_output_color_default(minion_opts): + """With color=None (default), full_color mode still colorizes diff lines.""" + minion_opts.update( + { + "color": None, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + _assert_diff_colorized(highstate.output(_MOTD_STATE)) + + +def test_diff_not_colorized_in_plain_full_output(minion_opts): + """With state_output=full (no _color), diff is not colorized.""" + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(_MOTD_STATE) + assert _RED not in rendered + + +@pytest.mark.parametrize("state_output", ["changes_color", "filter_color"]) +def test_diff_colorized_for_all_color_variants(minion_opts, state_output): + """Any XXX_color state_output variant colorizes the diff block.""" + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": state_output, + } + ) + with patch.dict(highstate.__opts__, minion_opts): + _assert_diff_colorized(highstate.output(_MOTD_STATE)) + + +def test_diff_colorized_in_mixed_color_on_failure(minion_opts): + """mixed_color only shows full output for failures; a failed state's diff is colorized.""" + state_data = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": { + "diff": ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + "+new line\n" + ) + }, + "comment": "File /etc/motd failed to update", + "duration": 10.0, + "name": "/etc/motd", + "result": False, + "start_time": "10:00:00.000000", + }, + } + } + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "mixed_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + assert _RED in rendered + assert "-old line" in _strip_ansi(rendered) + + +def test_nested_diff_colorized_in_file_recurse(minion_opts): + """file.recurse changes nest diff under a file-path key; diff must still be colorized.""" + state_data = { + "minion": { + "file_|-/etc/test_|-/etc/test_|-recurse": { + "__id__": "recurse_test", + "__run_num__": 0, + "__sls__": "test", + "changes": { + "/etc/test/file.txt": { "diff": ( - "--- /etc/motd\n" - "+++ /etc/motd\n" + "--- /etc/test/file.txt\n" + "+++ /etc/test/file.txt\n" "@@ -1,2 +1,2 @@\n" " unchanged\n" "-old line\n" "+new line\n" ) }, - "comment": "File /etc/motd updated", - "duration": 10.0, - "name": "/etc/motd", - "result": True, - "start_time": "10:00:00.000000", }, - } + "comment": "changes incoming", + "duration": 10.0, + "name": "/etc/test", + "result": None, + "start_time": "10:00:00.000000", + }, } - with patch.dict(highstate.__opts__, minion_opts): - rendered = highstate.output(state_data) - - # Removed line must be wrapped in RED - assert _RED in rendered - # Added line must be wrapped in GREEN - assert _GREEN in rendered - # Plain text must still be present - assert "-old line" in _strip_ansi(rendered) - assert "+new line" in _strip_ansi(rendered) - # Diff lines must be indented (18 spaces for value inside nested_indent=14) - for line in rendered.splitlines(): - plain = _strip_ansi(line) - if "-old line" in plain or "+new line" in plain: - assert plain.startswith( - " " * 18 - ), f"Expected 18-space indent: {repr(plain)}" - - def test_diff_in_full_color_output(self, minion_opts): - """file.managed diff has red removed and green added lines with full_color mode.""" - minion_opts.update( - { - "color": True, - "color_theme": None, - "state_verbose": True, - "state_output": "full_color", - } - ) - self._run_diff_color_test(minion_opts) - - def test_diff_in_full_color_output_color_default(self, minion_opts): - """With color=None (default), full_color mode still colorizes diff lines.""" - minion_opts.update( - { - "color": None, - "color_theme": None, - "state_verbose": True, - "state_output": "full_color", - } - ) - self._run_diff_color_test(minion_opts) - - def test_diff_not_colorized_in_plain_full_output(self, minion_opts): - """With state_output=full (no _color), diff is not colorized.""" - state_data = { - "minion": { - "file_|-/etc/motd_|-/etc/motd_|-managed": { - "__id__": "/etc/motd", - "__run_num__": 0, - "__sls__": "motd", - "changes": {"diff": "-old line\n+new line\n"}, - "comment": "File /etc/motd updated", - "duration": 10.0, - "name": "/etc/motd", - "result": True, - "start_time": "10:00:00.000000", - }, - } + } + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", } - minion_opts.update( - { - "color": True, - "color_theme": None, - "state_verbose": True, - "state_output": "full", - } - ) - with patch.dict(highstate.__opts__, minion_opts): - rendered = highstate.output(state_data) - # The diff-specific RED should not appear without _color mode - assert _RED not in rendered + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + assert _RED in rendered + assert _GREEN in rendered + assert "-old line" in _strip_ansi(rendered) + assert "+new line" in _strip_ansi(rendered) + + +def test_diff_not_colorized_without_color_modifier(minion_opts): + """Default state_output=full produces no diff colorization.""" + minion_opts.update({"color": True, "color_theme": None, "state_verbose": True}) + minion_opts.pop("state_output", None) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(_MOTD_STATE) + assert _RED not in rendered From 5c7599d151138be2a147529a5ff854f0516a0af9 Mon Sep 17 00:00:00 2001 From: Michael Schmitt Date: Mon, 27 Jul 2026 14:03:51 +0900 Subject: [PATCH 4/5] fix: single-pass sentinel substitution in highstate colorizer Replace the per-sentinel regex compile/scan loop in _nested_changes_colorized with a single combined pattern that matches every __COLORDIFF_N__ sentinel in one sweep, looking up each raw diff by its captured id. Avoids repeated full-string scans (and potential layout clobbering) when a state such as file.recurse produces multiple diffs, per PR review feedback. --- salt/output/highstate.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/salt/output/highstate.py b/salt/output/highstate.py index 084c872b4337..0ec032602b56 100644 --- a/salt/output/highstate.py +++ b/salt/output/highstate.py @@ -844,22 +844,24 @@ def _assign_sentinel(diff_str): _replace_diffs(changes), "nested", __opts__, nested_indent=14 ) - for sentinel, diff_str in sentinels.items(): - # The nested outputter renders each sentinel as a single indented line - # (possibly wrapped in ANSI codes). Capture the leading whitespace to - # determine the indent depth, then replace with colorized diff lines. - pattern = re.compile( - r"^( *)(?:\x1b\[[0-9;]*m)*" + re.escape(sentinel) + r"(?:\x1b\[[0-9;]*m)*$", - re.MULTILINE, - ) - - def _make_replacer(ds): - def _replacer(m): - return _render_diff(ds, len(m.group(1)), colors) + if not sentinels: + return "\n" + nested_output + + # The nested outputter renders each sentinel as a single indented line + # (possibly wrapped in ANSI codes). Match every sentinel in a single pass: + # group 1 = leading whitespace (the runtime indent depth) + # group 2 = the sentinel id, used to look up its raw diff string + combined_pattern = re.compile( + r"^( *)(?:\x1b\[[0-9;]*m)*(__COLORDIFF_\d+__)(?:\x1b\[[0-9;]*m)*$", + re.MULTILINE, + ) - return _replacer + def _replacer(match): + indent = len(match.group(1)) + diff_str = sentinels.get(match.group(2), "") + return _render_diff(diff_str, indent, colors) - nested_output = pattern.sub(_make_replacer(diff_str), nested_output) + nested_output = combined_pattern.sub(_replacer, nested_output) return "\n" + nested_output From b1acc19334b088c3f3c5541c78acfd69d141379b Mon Sep 17 00:00:00 2001 From: Michael Schmitt Date: Tue, 28 Jul 2026 12:50:53 +0900 Subject: [PATCH 5/5] refactor: tidy and harden highstate colordiff colorizer - Hoist the sentinel template and match pattern to module-level constants (_COLORDIFF_SENTINEL / _COLORDIFF_RE) instead of building the f-string in _assign_sentinel and recompiling the regex on every _nested_changes_colorized call, keeping the placeholder format defined in one place so producer and consumer can't drift apart. - Neutralize embedded terminal escape sequences in the (untrusted) diff content in _render_diff before applying the colorization escape codes, matching the nested outputter's strip_colors behavior; the colorized path had been bypassing that protection. - Render the +++ to-file header in bold green (LIGHT_GREEN) to mirror the bold red --- from-file header, and update the docstring, changelog, and CLI docs to match. - Drop the _color handling from the orchestration branch of _format_changes, which is never reached (orchestration=True is not passed anywhere). - Add a test for escape-sequence neutralization and asserts the file-header colors in the shared helper. - test_highstate.py used re.sub in _strip_ansi without importing re. - _format_host built its color mapping straight from __opts__["color"], so a value of None produced empty color codes and silently disabled diff colorization. Treat None as "auto -> on" to fix the check in _render_diff. --- changelog/68982.added.md | 5 +- doc/ref/cli/_includes/output-options.rst | 3 +- salt/output/highstate.py | 60 +++++++++++++------- tests/pytests/unit/output/test_highstate.py | 61 +++++++++++++++++++++ 4 files changed, 106 insertions(+), 23 deletions(-) diff --git a/changelog/68982.added.md b/changelog/68982.added.md index b6c4d421763b..465609059cce 100644 --- a/changelog/68982.added.md +++ b/changelog/68982.added.md @@ -2,5 +2,6 @@ Added ``_color`` modifier for ``state_output``: setting ``state_output`` to ``full_color``, ``terse_color``, ``mixed_color``, ``changes_color``, or ``filter_color`` enables colorized unified diff output in the highstate outputter. Added lines are green, removed lines are red, hunk headers -(``@@``) are cyan, file headers (``---``) are red, and context lines are -gray. All other behavior is identical to the base mode without ``_color``. +(``@@``) are cyan, file headers (``---`` / ``+++``) are bold red/green, and +context lines are gray. All other behavior is identical to the base mode +without ``_color``. diff --git a/doc/ref/cli/_includes/output-options.rst b/doc/ref/cli/_includes/output-options.rst index 708623219f69..3d3c0f1cd79a 100644 --- a/doc/ref/cli/_includes/output-options.rst +++ b/doc/ref/cli/_includes/output-options.rst @@ -53,7 +53,8 @@ Output Options ``name`` value (e.g. ``full_id``). * ``_color`` — colorize unified diffs in the changes section: added lines green, removed lines red, hunk headers (``@@``) cyan, file headers - (``---``) red, context lines gray (e.g. ``full_color``). + (``---`` / ``+++``) bold red/green, context lines gray (e.g. + ``full_color``). The two suffixes can be combined in either order, e.g. ``full_id_color`` or ``full_color_id``. diff --git a/salt/output/highstate.py b/salt/output/highstate.py index 0ec032602b56..69fb4ecc95df 100644 --- a/salt/output/highstate.py +++ b/salt/output/highstate.py @@ -54,8 +54,9 @@ ``full_color``, ``terse_color``, ``mixed_color``, ``changes_color`` and ``filter_color`` If ``_color`` is used, unified diffs in the changes section will be colorized: added lines in green, removed lines in red, hunk headers - (``@@``) in cyan, file headers (``---``) in red, and context lines in - gray. All other output behavior is identical to the base mode. + (``@@``) in cyan, file headers (``---`` / ``+++``) in bold red/green, and + context lines in gray. All other output behavior is identical to the base + mode. The ``_id`` and ``_color`` modifiers can be combined, e.g. ``full_id_color`` or ``full_color_id``. @@ -145,6 +146,16 @@ log = logging.getLogger(__name__) +# Placeholder injected in place of each diff string before the nested outputter +# runs, then swapped back for a colorized diff. ``{}`` is filled with a unique +# index; the regex below matches every such placeholder in a single pass, +# capturing the leading indent (group 1) and the sentinel id (group 2). +_COLORDIFF_SENTINEL = "__COLORDIFF_{}__" +_COLORDIFF_RE = re.compile( + r"^( *)(?:\x1b\[[0-9;]*m)*(__COLORDIFF_\d+__)(?:\x1b\[[0-9;]*m)*$", + re.MULTILINE, +) + def _compress_ids(data): """ @@ -349,9 +360,15 @@ def _format_host(host, data, indent_level=1): """ host = salt.utils.data.decode(host) - colors = salt.utils.color.get_colors( - __opts__.get("color"), __opts__.get("color_theme") - ) + # There is no ``color`` key in the config defaults; ``get_printout`` + # resolves it to True/False (default True) before the outputter normally + # runs. On a direct call that resolution is skipped, so an unset value can + # reach here as None. Fall back to Salt's default of True so it matches + # the ``color is False`` check used in ``_render_diff``. + color_opt = __opts__.get("color") + if color_opt is None: + color_opt = True + colors = salt.utils.color.get_colors(color_opt, __opts__.get("color_theme")) tabular = __opts__.get("state_tabular", False) rcounts = {} rdurations = [] @@ -759,7 +776,8 @@ def _render_diff(diff_str, indent, colors): Render a unified diff string with per-line ANSI colorization. Each line is colored according to its unified-diff role: - ``---`` / ``+++`` (file headers) -> LIGHT_RED (bold) + ``---`` (from-file header) -> LIGHT_RED (bold) + ``+++`` (to-file header) -> LIGHT_GREEN (bold) ``@@`` (hunk header) -> CYAN ``+`` (added line) -> GREEN ``-`` (removed line) -> RED @@ -768,10 +786,18 @@ def _render_diff(diff_str, indent, colors): The ``indent`` argument (an integer) is prepended as spaces to every line, matching the nesting depth used by the surrounding nested outputter output. The ``colors`` dict must be pre-built by the caller (e.g. from - ``salt.utils.color.get_colors``). + ``salt.utils.color.get_colors``). Embedded terminal escape sequences in + the (untrusted) diff content are neutralized when ``strip_colors`` is set, + matching the ``nested`` outputter. """ prefix = " " * indent + # Diff content is untrusted; neutralize any embedded escape sequences up + # front (ESC is not a line boundary, so this is equivalent to stripping + # each line) before we add the colorization escape codes. + if __opts__.get("strip_colors", True): + diff_str = salt.output.strip_esc_sequence(diff_str) + if __opts__.get("color") is False: return "\n".join(prefix + line for line in diff_str.splitlines()) @@ -781,13 +807,14 @@ def _render_diff(diff_str, indent, colors): CYAN = str(colors["CYAN"]) WHITE = str(colors["LIGHT_GRAY"]) LIGHT_RED = str(colors["LIGHT_RED"]) + LIGHT_GREEN = str(colors["LIGHT_GREEN"]) result = [] for line in diff_str.splitlines(): if line.startswith("---"): color = LIGHT_RED elif line.startswith("+++"): - color = GREEN + color = LIGHT_GREEN elif line.startswith("@@"): color = CYAN elif line.startswith("+"): @@ -836,7 +863,7 @@ def _replace_diffs(obj): return obj def _assign_sentinel(diff_str): - sentinel = f"__COLORDIFF_{len(sentinels)}__" + sentinel = _COLORDIFF_SENTINEL.format(len(sentinels)) sentinels[sentinel] = diff_str return sentinel @@ -848,20 +875,15 @@ def _assign_sentinel(diff_str): return "\n" + nested_output # The nested outputter renders each sentinel as a single indented line - # (possibly wrapped in ANSI codes). Match every sentinel in a single pass: - # group 1 = leading whitespace (the runtime indent depth) - # group 2 = the sentinel id, used to look up its raw diff string - combined_pattern = re.compile( - r"^( *)(?:\x1b\[[0-9;]*m)*(__COLORDIFF_\d+__)(?:\x1b\[[0-9;]*m)*$", - re.MULTILINE, - ) - + # (possibly wrapped in ANSI codes). ``_COLORDIFF_RE`` matches every + # sentinel in a single pass: group 1 is the runtime indent, group 2 the + # sentinel id used to look up its raw diff string. def _replacer(match): indent = len(match.group(1)) diff_str = sentinels.get(match.group(2), "") return _render_diff(diff_str, indent, colors) - nested_output = combined_pattern.sub(_replacer, nested_output) + nested_output = _COLORDIFF_RE.sub(_replacer, nested_output) return "\n" + nested_output @@ -880,8 +902,6 @@ def _format_changes(changes, colors, orchestration=False): colorize = "_color" in __opts__.get("state_output", "").lower() if orchestration: - if colorize: - return True, _nested_changes_colorized(changes, colors) return True, _nested_changes(changes) if not isinstance(changes, dict): diff --git a/tests/pytests/unit/output/test_highstate.py b/tests/pytests/unit/output/test_highstate.py index 471de6652e7f..0bd25ce716ac 100644 --- a/tests/pytests/unit/output/test_highstate.py +++ b/tests/pytests/unit/output/test_highstate.py @@ -1,5 +1,6 @@ import copy import logging +import re import pytest @@ -929,6 +930,7 @@ def test_nested_output(): _CYAN = "\x1b[0;36m" _WHITE = "\x1b[0;37m" _LIGHT_RED = "\x1b[0;1;31m" +_LIGHT_GREEN = "\x1b[0;1;32m" _ENDC = "\x1b[0;0m" @@ -974,6 +976,14 @@ def _assert_diff_colorized(rendered): assert plain.startswith( " " * 18 ), f"Expected 18-space indent: {repr(plain)}" + # Diff file headers use the bold variants: --- (from) red, +++ (to) + # green. A real header is the marker followed by a space + path + # ("--- /path"), which distinguishes it from the outputter's own + # "----------" separator lines. + if plain.lstrip().startswith("--- "): + assert _LIGHT_RED in line + if plain.lstrip().startswith("+++ "): + assert _LIGHT_GREEN in line def test_diff_in_full_color_output(minion_opts): @@ -1125,3 +1135,54 @@ def test_diff_not_colorized_without_color_modifier(minion_opts): with patch.dict(highstate.__opts__, minion_opts): rendered = highstate.output(_MOTD_STATE) assert _RED not in rendered + + +def test_colorized_diff_strips_embedded_escape_sequences(minion_opts): + """ + Diff content is untrusted; embedded terminal escape sequences must be + neutralized in the colorized path just as the nested outputter does for + the plain path (strip_colors defaults to True). + """ + # ESC ]0;injected_title BEL is an OSC that would set the terminal title if + # printed. + injected = "\x1b]0;injected_title\x07" + state_data = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": { + "diff": ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + f"+new line{injected}\n" + ) + }, + "comment": "File /etc/motd updated", + "duration": 10.0, + "name": "/etc/motd", + "result": True, + "start_time": "10:00:00.000000", + }, + } + } + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "strip_colors": True, + "state_output": "full_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + # The raw OSC injection must not survive into the output... + assert injected not in rendered + # ...but the visible text and colorization are preserved. + assert _GREEN in rendered + assert "+new line" in _strip_ansi(rendered)