From c722552a436a83ccdc18cd6bfcf6d08d11473adf Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:24:56 +0200 Subject: [PATCH 1/5] fix(console): error lines print exception and contract text literally console_error wrapped its message in [red]...[/red], so Rich parsed the message as markup: a lowercase [word] in it vanished, and a [/] raised MarkupError. The CLI's last-resort handler reports through console_error, so that raise escaped main() as a traceback. main()'s generic handler had the same flaw one step earlier: "[red]Unexpected error: {e}[/red]" raised on the same text and handed it to the fallback. - New _console.literal_text(): a rich Text, never markup, with control characters spelled \xNN. ESC opens an escape sequence and the C1 range holds the one-byte CSI and OSC; printed raw, a message could clear the screen or plant a link. Rich's highlighter broke such sequences up on a top-level str by accident; a Text is not highlighted. - _console.error prints that way, still redacted. None of its 130 call sites passes intended markup (AST scan). - validate.py drops the rich.markup.escape it applied to six messages to survive the old wrapper; kept, it would print a stray backslash. - main(): the Provider error and Unexpected error lines use literal_text, and the --debug traceback prints with markup=False. Text rather than escape: escape doubles a trailing backslash (C:\work\ prints as C:\work\\) when no tag follows it. --- fluid_build/_console.py | 37 ++++++++++- fluid_build/cli/__init__.py | 12 +++- fluid_build/cli/console.py | 2 + fluid_build/cli/validate.py | 32 +++------ tests/cli/test_cli_output_keeps_brackets.py | 73 +++++++++++++++++++++ tests/test_console.py | 22 +++++++ 6 files changed, 150 insertions(+), 28 deletions(-) diff --git a/fluid_build/_console.py b/fluid_build/_console.py index 3532dfec..83f23581 100644 --- a/fluid_build/_console.py +++ b/fluid_build/_console.py @@ -39,6 +39,7 @@ try: from rich.console import Console as RichConsole + from rich.text import Text as RichText RICH_AVAILABLE = True except ImportError: # pragma: no cover @@ -58,6 +59,28 @@ _SECRET_OUTPUT_PATTERN = re.compile( r"(?i)\b(password|passphrase|token|secret|api[_ -]?key)\b(\s*[:=]\s*)([^\s,;]+)" ) +# The control characters (Unicode Cc) a terminal acts on: ESC opens an escape +# sequence, and the C1 range holds the one-byte CSI and OSC. Printed raw, a contract +# value or an exception message could clear the screen, retitle the window or show a +# link that opens another URL. Tab and line feed stay. Carriage return is left to +# Rich, which drops it, so CRLF text reads normally. +_CONTROL_CHARS = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f-\x9f]") + + +def _show_controls(text: str) -> str: + """``text`` with each control character spelled ``\\xNN`` instead of sent to the terminal.""" + return _CONTROL_CHARS.sub(lambda m: f"\\x{ord(m.group()):02x}", text) + + +def literal_text(text: Any, style: str = "") -> RichText: + """``text`` as a Rich ``Text``: printed as written, control characters spelled out. + + For data a renderer quotes back, such as a contract value or an exception message. + Rich parses a ``str`` as markup, so a lowercase ``[word]`` vanishes and ``[/]`` + raises ``MarkupError``. ``rich.markup.escape`` is no substitute: it doubles a + trailing backslash when no tag follows it. Rich must be installed. + """ + return RichText(_show_controls(str(text)), style=style) def _redact_str(value: str) -> str: @@ -178,13 +201,21 @@ def warning(msg: str) -> None: def error(msg: str) -> None: - """Error message (red ❌) – written to *stderr*.""" + """Error message (red ❌) – written to *stderr*. + + ``msg`` is printed literally, never parsed as Rich markup. An error + quotes data: a contract id, a schema ``pattern``, an exception message. + Wrapped in ``[red]...[/red]``, a lowercase ``[word]`` in it vanished and + a ``[/]`` raised ``MarkupError``, which escaped the CLI's last-resort + handler as a traceback because that handler reports through this + function. Control characters are spelled out (see :func:`literal_text`). + """ # Funnel through the typed ``_redact_str`` sanitiser so CodeQL # recognises the sink path is sanitised on every branch. - safe_msg = _redact_str(str(msg)) + safe_msg = _redact_str(_show_controls(str(msg))) if RICH_AVAILABLE: _err_console = RichConsole(stderr=True) - _err_console.print(f"[red]❌ {safe_msg}[/red]") + _err_console.print(RichText(f"❌ {safe_msg}", style="red")) else: print(f"❌ {safe_msg}", file=sys.stderr) diff --git a/fluid_build/cli/__init__.py b/fluid_build/cli/__init__.py index 0ad945ac..9528abc2 100644 --- a/fluid_build/cli/__init__.py +++ b/fluid_build/cli/__init__.py @@ -511,9 +511,15 @@ def main(argv: Optional[List[str]] = None) -> int: raise except Exception as e: # noqa: BLE001 + # The exception text is data, not markup, as in the CLIError branch + # above. Parsed as markup, an error quoting ``[/]`` raised + # ``MarkupError`` from this handler, and the outer fallback then + # died on the same text with a traceback. + from fluid_build._console import literal_text + # Structured catch for provider errors (before generic Exception) if ProviderError is not None and isinstance(e, ProviderError): - context.console.print(f"[red]❌ Provider error: {e}[/red]") + context.console.print(literal_text(f"❌ Provider error: {e}", "red")) cli.logger.log_safe( "error", "Provider error", @@ -532,7 +538,7 @@ def main(argv: Optional[List[str]] = None) -> int: return 1 # Handle unexpected errors with enhanced context - context.console.print(f"[red]❌ Unexpected error: {e}[/red]") + context.console.print(literal_text(f"❌ Unexpected error: {e}", "red")) cli.logger.log_safe( "error", "CLI unhandled exception", @@ -548,7 +554,7 @@ def main(argv: Optional[List[str]] = None) -> int: import traceback context.console.print("\n[red]Debug traceback:[/red]") - context.console.print(traceback.format_exc()) + context.console.print(traceback.format_exc(), markup=False) # Provide helpful guidance context.console.print("\n[yellow]💡 This appears to be an unexpected error.[/yellow]") diff --git a/fluid_build/cli/console.py b/fluid_build/cli/console.py index 8ad5ba93..94a0e3c1 100644 --- a/fluid_build/cli/console.py +++ b/fluid_build/cli/console.py @@ -44,6 +44,7 @@ heading, hint, info, + literal_text, success, warning, ) @@ -62,6 +63,7 @@ "heading", "hint", "info", + "literal_text", "success", "warning", ] diff --git a/fluid_build/cli/validate.py b/fluid_build/cli/validate.py index 2fc56268..8e906a5f 100644 --- a/fluid_build/cli/validate.py +++ b/fluid_build/cli/validate.py @@ -389,47 +389,35 @@ def run(args, logger: logging.Logger) -> int: duration = time.time() - start_time log_operation_failure(logger, "validate_contract", error=e.event, duration=duration) - # ``console_error`` wraps its text in ``[red]...[/red]`` markup, so every - # value echoed into it is escaped first: a path or message holding a - # lowercase ``[word]`` would otherwise lose it (see ``_cprint_literal``). - from rich.markup import escape - - # Handle specific CLI errors with user-friendly messages + # Handle specific CLI errors with user-friendly messages. ``console_error`` + # prints its text literally, so no value needs escaping first. if e.event == "version_below_minimum": if not args.quiet: console_error( - escape( - f"Contract version {e.context.get('version')} does not meet minimum requirement {e.context.get('constraint')}" - ) + f"Contract version {e.context.get('version')} does not meet minimum requirement {e.context.get('constraint')}" ) elif e.event == "version_above_maximum": if not args.quiet: console_error( - escape( - f"Contract version {e.context.get('version')} exceeds maximum allowed {e.context.get('constraint')}" - ) + f"Contract version {e.context.get('version')} exceeds maximum allowed {e.context.get('constraint')}" ) elif e.event == "contract_file_not_found": if not args.quiet: - console_error(escape(f"Contract file not found: {e.context.get('path')}")) + console_error(f"Contract file not found: {e.context.get('path')}") elif e.event == "contract_version_unsupported": if not args.quiet: console_error( - escape( - str( - e.context.get( - "message", - f"Unsupported contract version: {e.context.get('version')}", - ) - ) + e.context.get( + "message", + f"Unsupported contract version: {e.context.get('version')}", ) ) elif e.event == "contract_required": if not args.quiet: - console_error(escape(f"{e.context.get('message', 'Contract file is required')}")) + console_error(f"{e.context.get('message', 'Contract file is required')}") else: if not args.quiet: - console_error(escape(f"Validation error: {e.event}")) + console_error(f"Validation error: {e.event}") if e.context: for key, value in e.context.items(): _cprint_literal(f" {key}: {value}") diff --git a/tests/cli/test_cli_output_keeps_brackets.py b/tests/cli/test_cli_output_keeps_brackets.py index 6e787ea7..4bbd1326 100644 --- a/tests/cli/test_cli_output_keeps_brackets.py +++ b/tests/cli/test_cli_output_keeps_brackets.py @@ -379,3 +379,76 @@ def test_fluid_cli_error_format_for_user_prints_brackets_literally(): assert "exposes[orders] names [bold]nothing[/bold]" in out assert "data-product-forge[gcp]" in out assert "C:\\\\work\\\\'" in out or "C:\\work\\" in out + + +# --------------------------------------------------------------------------- +# main(): the generic handlers, which report an exception's own text +# --------------------------------------------------------------------------- + +# ``[/]`` closes whatever tag is open, so inside a ``[red]...[/red]`` wrapper (or +# a report's own ``[bold]`` header) it left the real closing tag unmatched, and +# Rich raised MarkupError. A JSON Schema pattern lost its character classes. +MESSAGE = f"[/] {MARKUP_LIKE} ^[a-z0-9]+$" +# Escape sequences a terminal acts on: clear the screen, a link that opens another +# URL, and the one-byte CSI. Rich never writes any of them for its own styles. +ESCAPES = "\x1b[2J\x1b]8;;http://evil.example/\x1b\\click\x1b]8;;\x1b\\ \x9b31m" + + +def _reaches_the_terminal(stream: str) -> bool: + return any(seq in stream for seq in ("\x1b[2J", "\x1b]8;;", "\x9b")) + + +def test_unexpected_error_prints_its_message_verbatim(monkeypatch, capsys): + """``[red]{e}[/red]`` raised MarkupError on ``[/]``; the fallback then died on it too.""" + _raise_from_command(monkeypatch, ValueError(MESSAGE)) + + rc = cli_mod.main(["validate", "unused.fluid.yaml"]) + + assert rc == 2 + assert f"❌ Unexpected error: {MESSAGE}" in _unwrapped(capsys.readouterr().out) + + +def test_debug_traceback_prints_the_message_verbatim(monkeypatch, capsys): + _raise_from_command(monkeypatch, ValueError(MESSAGE)) + + rc = cli_mod.main(["--debug", "validate", "unused.fluid.yaml"]) + + assert rc == 2 + assert f"ValueError: {MESSAGE}" in _unwrapped(capsys.readouterr().out) + + +def test_provider_error_prints_its_message_verbatim(monkeypatch, capsys): + _raise_from_command(monkeypatch, cli_mod.ProviderError(MESSAGE)) + + rc = cli_mod.main(["validate", "unused.fluid.yaml"]) + + assert rc == 1 + assert f"❌ Provider error: {MESSAGE}" in _unwrapped(capsys.readouterr().out) + + +def test_last_resort_handler_prints_its_message_verbatim(monkeypatch, capsys): + """The outer fallback reports through ``console_error``, which wrapped it in ``[red]``.""" + + def _fail(self): + raise RuntimeError(MESSAGE) + + monkeypatch.setattr(cli_mod.ProductionCLI, "setup_production_environment", _fail) + + rc = cli_mod.main(["validate", "unused.fluid.yaml"]) + + assert rc == 2 + assert f"❌ CLI initialization failed: {MESSAGE}" in _unwrapped(capsys.readouterr().err) + + +def test_unexpected_error_spells_out_control_characters(monkeypatch, capsys): + """A highlighted ``str`` broke these sequences up by accident; a ``Text`` is not highlighted.""" + monkeypatch.setenv("FORCE_COLOR", "1") # a terminal + monkeypatch.setenv("COLUMNS", "400") + _raise_from_command(monkeypatch, ValueError(ESCAPES)) + + rc = cli_mod.main(["validate", "unused.fluid.yaml"]) + + out = capsys.readouterr().out + assert rc == 2 + assert not _reaches_the_terminal(out) + assert "Unexpected error: \\x1b[2J\\x1b]8;;http://evil.example/" in out diff --git a/tests/test_console.py b/tests/test_console.py index 9538a4e2..37dd6ad8 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -77,6 +77,28 @@ def test_error(self, capsys): captured = capsys.readouterr() assert "something broke" in captured.err + def test_error_prints_markup_shaped_text_literally(self, capsys): + # The ``[red]`` wrapper made a ``[/]`` raise MarkupError and ate ``[orders]``. + message = "[/] exposes[orders] ^[a-z0-9]+$ [bold]x[/bold] C:\\work\\" + error(message) + assert f"❌ {message}\n" in capsys.readouterr().err + + def test_error_still_redacts_secrets(self, capsys): + error("connect failed [/] password=hunter2") + err = capsys.readouterr().err + assert "password=" in err + assert "hunter2" not in err + + def test_error_spells_out_terminal_control_characters(self, capsys, monkeypatch): + # A terminal: Rich writes its own colour codes, but none from the message. + monkeypatch.setenv("FORCE_COLOR", "1") + error("A\x1b[2J\x1b]8;;http://evil.example/\x1b\\B \x9b31m\r\nnext line") + err = capsys.readouterr().err + assert "\x1b[2J" not in err and "\x1b]8;;" not in err and "\x9b" not in err + assert "A\\x1b[2J\\x1b]8;;http://evil.example/\\x1b\\B \\x9b31m" in err + # A CRLF line break still breaks the line; Rich drops the CR. + assert "\r" not in err and "next line" in err + def test_heading(self, capsys): heading("My Section") captured = capsys.readouterr() From b29baf0967e774009ceabcd5a5583f49b2830d37 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:26:30 +0200 Subject: [PATCH 2/5] fix(cli): fluid test and contract-validation print contract text literally Both reports quote the contract back, and Rich parsed it as markup. A JSON Schema pattern lost its character classes (^[a-z0-9][a-z0-9-]*[a-z0-9]$ printed as ^*$), exposes[orders] lost its id, and an id or expose id holding [/] raised MarkupError, so both commands died with a traceback. Only the overlay row of fluid test and the issue table of contract-validation were escaped (#734). - fluid test: the header panel and every table row go through literal_text; the _detail() escape helper is gone. - contract-validation: the summary panel, each issue's message and path, and a severity outside the styled three go through literal_text. - The Rich-less plain reports, and the soda engine's failed check names and stderr, print with markup=False (the plain fallback strips [word]). JSON, JUnit and the plain report file are unchanged. --- CHANGELOG.md | 12 ++ fluid_build/cli/contract_validation.py | 38 +++-- fluid_build/cli/test.py | 70 ++++---- tests/cli/test_cli_output_keeps_brackets.py | 174 ++++++++++++++++++++ tests/test_cli_test_ext.py | 6 +- 5 files changed, 248 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5643c62..e711f83f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 here`, or the rule's own `description`, instead of quoting the whole object and the subschema it must not match. +### Fixed + +- **`fluid test` and `fluid contract-validation` print the contract's text as written.** + Both read their report as Rich markup, so a JSON Schema pattern such as + `^[a-z0-9][a-z0-9-]*[a-z0-9]$` printed as `^*$`, `exposes[orders]` lost its id, and an + id or expose id holding `[/]` crashed the command with a `MarkupError` traceback. The + `❌` error lines every command prints, and the handler for an unexpected error, show + their text as written too, so an exception that quotes `[/]` no longer ends in a second + traceback. In that text, and in both reports, a control character prints as `\x1b` + and the like, so an escape sequence in a contract or an error message can no longer + clear the terminal or plant a link. JSON and JUnit output are unchanged. + ## [0.21.0] — 2026-10-19 ### Upgrade notes diff --git a/fluid_build/cli/contract_validation.py b/fluid_build/cli/contract_validation.py index 5bd0deaa..a640ebcf 100644 --- a/fluid_build/cli/contract_validation.py +++ b/fluid_build/cli/contract_validation.py @@ -39,7 +39,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, List, Optional -from fluid_build.cli.console import cprint, cprint_json, success +from fluid_build.cli.console import cprint, cprint_json, literal_text, success from fluid_build.cli.console import error as console_error # Rich imports for enhanced output @@ -1616,25 +1616,31 @@ def output_text_report(report: ValidationReport, output_file: Optional[str] = No def output_rich_report(report: ValidationReport, output_file: Optional[str] = None) -> None: - """Output validation report using Rich formatting.""" + """Output validation report using Rich formatting. + + The contract's own values (its id and version, every issue message and + path, a rule's severity) go in through ``literal_text``, never as markup: + an id holding ``[/]`` raised ``MarkupError``, and a JSON Schema + ``pattern`` lost its ``[a-z0-9]`` classes. + """ console = Console() # Summary panel status_color = "green" if report.is_valid() else "red" status_icon = "✅" if report.is_valid() else "❌" - summary_text = f"""[bold]{status_icon} Contract Validation: {report.contract_id}[/bold] -Version: {report.contract_version} -Validated: {report.exposes_validated} exposed, {report.consumes_validated} consumed -Results: {report.checks_passed} passed, {report.checks_failed} failed -Duration: {report.duration:.2f}s""" + summary_text = Text.assemble( + literal_text(f"{status_icon} Contract Validation: {report.contract_id}", "bold"), + literal_text(f"\nVersion: {report.contract_version}"), + f"\nValidated: {report.exposes_validated} exposed, {report.consumes_validated} consumed", + f"\nResults: {report.checks_passed} passed, {report.checks_failed} failed", + f"\nDuration: {report.duration:.2f}s", + ) console.print(Panel(summary_text, title="Validation Summary", border_style=status_color)) # Issues table if report.issues: - from rich.markup import escape - table = Table(title="Validation Issues", show_header=True) table.add_column("Severity", style="bold") table.add_column("Category") @@ -1646,12 +1652,13 @@ def output_rich_report(report: ValidationReport, output_file: Optional[str] = No "error": "[red]ERROR[/red]", "warning": "[yellow]WARNING[/yellow]", "info": "[blue]INFO[/blue]", - }.get(issue.severity, issue.severity.upper()) + }.get(issue.severity) or literal_text(issue.severity.upper()) - # Messages quote contract text (an overlay warning names the - # contract's ids), which Rich would parse as markup. table.add_row( - severity_style, issue.category, escape(issue.message), escape(issue.path or "") + severity_style, + issue.category, + literal_text(issue.message), + literal_text(issue.path or ""), ) console.print(table) @@ -1693,7 +1700,10 @@ def output_plain_report(report: ValidationReport, output_file: Optional[str] = N f.write(report_text) success(f"Report saved to: {output_file}") else: - cprint(report_text) + # The report quotes contract text; without ``markup=False`` the + # console's Rich-less fallback strips an ``exposes[orders]``-style + # ``[word]`` as if it were a style tag. + cprint(report_text, markup=False) def output_json_report(report: ValidationReport, output_file: Optional[str] = None) -> None: diff --git a/fluid_build/cli/test.py b/fluid_build/cli/test.py index 80c61bb3..7e1b940e 100644 --- a/fluid_build/cli/test.py +++ b/fluid_build/cli/test.py @@ -44,9 +44,9 @@ import sys import xml.etree.ElementTree as ET from pathlib import Path -from typing import Dict, Iterable, List, Optional +from typing import Dict, List, Optional -from fluid_build.cli.console import cprint, success, warning +from fluid_build.cli.console import cprint, literal_text, success, warning from fluid_build.cli.console import error as console_error from ._common import CLIError @@ -55,7 +55,6 @@ # Rich imports (optional) try: from rich.console import Console - from rich.markup import escape from rich.panel import Panel from rich.table import Table from rich.text import Text @@ -64,11 +63,6 @@ except ImportError: RICH_AVAILABLE = False - def escape(text: str) -> str: # type: ignore[misc] - """No-op stand-in when rich is absent — nothing parses markup then.""" - return text - - from fluid_build.severity import is_error as _is_error_severity from fluid_build.severity import is_warning as _is_warning_severity @@ -327,21 +321,16 @@ def run(args: argparse.Namespace, logger: logging.Logger) -> int: # ====================================================================== -def _detail(messages: Iterable[str]) -> str: - """Join issue messages into a Rich table cell, escaped as literal text. - - Validator messages quote the contract schema verbatim, so they routinely - carry ``[...]`` runs — a JSON Schema ``pattern`` (``[a-z0-9]``), a contract - path (``exposes[0]``), an enum list. Rich parses those as style markup and - swallows them, so ``^[a-z0-9][a-z0-9-]*[a-z0-9]$`` reached the operator as - the unactionable ``^*$``. ``escape`` keeps the cell showing exactly what - the validator said. - """ - return escape("; ".join(messages)) - - def _output_rich(report, output_file: Optional[str] = None) -> None: - """Render a Data-Contract-CLI-style test results table.""" + """Render a Data-Contract-CLI-style test results table. + + Everything the contract supplies (its id, version and provider, every + issue message) goes in through ``literal_text``, never as markup. + Messages quote the contract verbatim: a JSON Schema ``pattern`` + (``^[a-z0-9][a-z0-9-]*[a-z0-9]$`` reached the operator as ``^*$``), an + ``exposes[orders]`` path, an id. An id holding ``[/]`` raised + ``MarkupError`` and the command died with a traceback. + """ if not RICH_AVAILABLE: _output_plain(report, output_file) return @@ -353,15 +342,17 @@ def _output_rich(report, output_file: Optional[str] = None) -> None: icon = "\u2705" if passed else "\u274c" border = "green" if passed else "red" - header_lines = [ - f"[bold]{icon} Data Contract Test: {report.contract_id}[/bold]", - f"Version {report.contract_version} | Provider: {_detect_provider_label(report)}", - "Duration: {dur:.2f}s | {dt}".format( + header = Text.assemble( + literal_text(f"{icon} Data Contract Test: {report.contract_id}", "bold"), + literal_text( + f"\nVersion {report.contract_version} | Provider: {_detect_provider_label(report)}" + ), + "\nDuration: {dur:.2f}s | {dt}".format( dur=report.duration, dt=report.validation_time.strftime("%Y-%m-%d %H:%M:%S"), ), - ] - console.print(Panel("\n".join(header_lines), border_style=border, title="fluid test")) + ) + console.print(Panel(header, border_style=border, title="fluid test")) # ── check-by-check table ── rows = _build_check_rows(report) @@ -373,7 +364,7 @@ def _output_rich(report, output_file: Optional[str] = None) -> None: table.add_column("Details", overflow="fold") for idx, (status, name, detail) in enumerate(rows, 1): - table.add_row(str(idx), _ROW_ICON[status], name, detail) + table.add_row(str(idx), _ROW_ICON[status], name, literal_text(detail)) console.print(table) @@ -420,7 +411,8 @@ def _build_check_rows(report) -> List[tuple]: Split out of the renderer so each row's outcome is a value that can be counted (and unit-tested) rather than a decision made inline - while printing. + while printing. ``details`` is plain text, not markup; the renderer + keeps it literal. """ issues = report.issues data_checked = getattr(report, "data_checks_performed", True) @@ -443,7 +435,7 @@ def _warns(*categories: str) -> List: # entry it does not name): --strict fails on it, so it must show. overlay_issues = [i for i in issues if i.category == "overlay"] if overlay_issues: - rows.append((_WARN, "Environment overlay", _detail(e.message for e in overlay_issues))) + rows.append((_WARN, "Environment overlay", "; ".join(e.message for e in overlay_issues))) # Provider connectivity # The connection probe runs in provider detection, so it happens @@ -613,7 +605,10 @@ def _output_plain(report, output_file: Optional[str] = None) -> None: f.write(text) cprint(f"Report saved to: {output_file}") else: - cprint(text) + # The report quotes contract text; without ``markup=False`` the + # console's Rich-less fallback strips an ``exposes[orders]``-style + # ``[word]`` as if it were a style tag. + cprint(text, markup=False) # ====================================================================== @@ -980,7 +975,10 @@ def _run_soda_engine( if result.raw_stderr: from fluid_build.observability.secret_redactor import redact_secret_text - cprint(f" soda stderr: {redact_secret_text(result.raw_stderr.strip()[:500])}") + cprint( + f" soda stderr: {redact_secret_text(result.raw_stderr.strip()[:500])}", + markup=False, + ) # Render in the requested format (re-use the existing --output flag). output_format = getattr(args, "output", "text") @@ -1143,8 +1141,10 @@ def _emit_soda_text(result, datasource: str, rendering=None) -> None: cprint(counts) if result.failed_check_names: cprint(" failed checks:") + # A check name quotes the contract's rule (a ``valid regex`` reads + # ``[a-z]``), and Soda's stderr quotes the SodaCL. Both are data. for name in result.failed_check_names: - cprint(f" - {name}") + cprint(f" - {name}", markup=False) if not result.ok and result.raw_stderr: # Soda's stderr can include credential-bearing connection # strings (DSNs with embedded passwords, BigQuery service- @@ -1153,4 +1153,4 @@ def _emit_soda_text(result, datasource: str, rendering=None) -> None: from fluid_build.observability.secret_redactor import redact_secret_text redacted = redact_secret_text(result.raw_stderr.strip()[:500]) - cprint(f" soda stderr: {redacted}") + cprint(f" soda stderr: {redacted}", markup=False) diff --git a/tests/cli/test_cli_output_keeps_brackets.py b/tests/cli/test_cli_output_keeps_brackets.py index 4bbd1326..3b1fcdfc 100644 --- a/tests/cli/test_cli_output_keeps_brackets.py +++ b/tests/cli/test_cli_output_keeps_brackets.py @@ -452,3 +452,177 @@ def test_unexpected_error_spells_out_control_characters(monkeypatch, capsys): assert rc == 2 assert not _reaches_the_terminal(out) assert "Unexpected error: \\x1b[2J\\x1b]8;;http://evil.example/" in out + + +# --------------------------------------------------------------------------- +# fluid test / fluid contract-validation: the report quotes the contract back +# --------------------------------------------------------------------------- + +MARKUP_ID = "orders.[/]v1" +# Every issue category a ``fluid test`` table row lists the messages of. +ROW_CATEGORIES = ( + "schema", + "overlay", + "connection", + "binding", + "missing_resource", + "missing_field", + "empty_table", + "quality", + "metadata", + "drift", +) + + +def _contract_quoting_markup() -> Dict[str, Any]: + """A local product the schema rejects; each error quotes the value back.""" + return { + "fluidVersion": "0.7.5", + "kind": "DataProduct", + "id": MARKUP_ID, + "name": "Orders", + "domain": "sales", + "tags": ["Not A Tag"], + "metadata": {"owner": {"team": "data-platform"}}, + "exposes": [ + { + "exposeId": "orders[/]daily", + "kind": "table", + "binding": { + "platform": "local", + "format": "parquet", + "location": {"path": "out/orders.parquet"}, + }, + "contract": {"schema": [{"name": "order_id", "type": "integer"}]}, + } + ], + } + + +def _report_quoting_markup(): + from datetime import datetime + + from fluid_build.cli.contract_validation import ValidationReport + + report = ValidationReport( + "c.yaml", MARKUP_ID, "1.0.[/]", datetime(2026, 1, 1), 0.0, provider_name="lake[/]house" + ) + for category in ROW_CATEGORIES: + report.add_issue("error", category, f"{category}: {MESSAGE}", f"exposes[{category}]") + return report + + +@pytest.mark.parametrize( + "command, title", + [("test", "Data Contract Test"), ("contract-validation", "Contract Validation")], +) +def test_report_prints_a_markup_shaped_contract_verbatim( + tmp_path, monkeypatch, capsys, command, title +): + """End to end: the id's ``[/]`` crashed both commands with a MarkupError traceback.""" + monkeypatch.setenv("COLUMNS", "400") # one table line per message + path = _write(tmp_path, _contract_quoting_markup()) + + rc = cli_mod.main([command, path, "--no-data", "--no-cache"]) + + out = capsys.readouterr().out + assert rc == 1 # the schema errors + assert f"{title}: {MARKUP_ID}" in out + assert "'orders[/]daily' does not match" in out + assert "'Not A Tag' does not match '^[a-z0-9]" in out + + +def test_fluid_test_table_prints_every_row_and_the_header_verbatim(monkeypatch, capsys): + from fluid_build.cli.test import _output_rich + + monkeypatch.setenv("COLUMNS", "400") + report = _report_quoting_markup() + report.add_issue("error", "schema", "location.path: C:\\work\\") + + _output_rich(report) + + out = capsys.readouterr().out + assert f"Data Contract Test: {MARKUP_ID}" in out + assert "Version 1.0.[/] | Provider: lake[/]house" in out + for category in ROW_CATEGORIES: + assert f"{category}: {MESSAGE}" in out + # rich.markup.escape would have doubled the trailing backslash. + assert "location.path: C:\\work\\ " in out + + +def test_contract_validation_table_prints_messages_and_paths_verbatim(monkeypatch, capsys): + from fluid_build.cli.contract_validation import output_rich_report + + monkeypatch.setenv("COLUMNS", "400") + report = _report_quoting_markup() + # A rule's severity comes from the contract; one outside the styled set prints as given. + report.add_issue("[/]fatal", "quality", "rule failed", "exposes[orders].dq") + + output_rich_report(report) + + out = capsys.readouterr().out + assert f"Contract Validation: {MARKUP_ID}" in out + assert "Version: 1.0.[/]" in out + for category in ROW_CATEGORIES: + assert f"{category}: {MESSAGE}" in out + assert f"exposes[{category}]" in out + assert "[/]FATAL" in out + + +def test_reports_spell_out_control_characters(monkeypatch, capsys): + """On a terminal, a contract value must not clear it or plant a link.""" + from datetime import datetime + + from fluid_build.cli.contract_validation import ValidationReport, output_rich_report + from fluid_build.cli.test import _output_rich + + monkeypatch.setenv("FORCE_COLOR", "1") + monkeypatch.setenv("COLUMNS", "400") + report = ValidationReport( + "c.yaml", f"orders{ESCAPES}", f"1{ESCAPES}", datetime(2026, 1, 1), 0.0 + ) + report.add_issue("error", "schema", f"bad {ESCAPES}", f"exposes[{ESCAPES}]") + + _output_rich(report) + output_rich_report(report) + + out = capsys.readouterr().out + assert not _reaches_the_terminal(out) + assert "Data Contract Test: orders\\x1b[2J" in out + assert "Contract Validation: orders\\x1b[2J" in out + assert "bad \\x1b[2J\\x1b]8;;http://evil.example/" in out + + +def test_plain_reports_keep_brackets_when_rich_is_absent(monkeypatch, capsys): + """Without Rich, ``cprint`` strips every ``[word]`` unless told the text is not markup.""" + from fluid_build.cli.contract_validation import output_plain_report + from fluid_build.cli.test import _output_plain + + monkeypatch.setattr("fluid_build._console.console", None) + report = _report_quoting_markup() + + _output_plain(report) + output_plain_report(report) + + assert capsys.readouterr().out.count(MESSAGE) == 2 * len(ROW_CATEGORIES) + + +def test_soda_text_prints_check_names_and_stderr_verbatim(capsys): + """A check name quotes the contract's rule; Soda's stderr quotes the SodaCL.""" + from fluid_build.build_runners.soda.runner import SodaResult + from fluid_build.cli.test import _emit_soda_text + + name = "invalid_count(code) = 0 [/] ^[a-z]+$" + result = SodaResult( + return_code=1, + raw_stdout="", + raw_stderr=f"check failed: {name}", + checks_failed=1, + failed_check_names=[name], + ) + + _emit_soda_text(result, "warehouse") + + out = _unwrapped(capsys.readouterr().out) + assert f"- {name}" in out + assert f"soda stderr: check failed: {name}" in out diff --git a/tests/test_cli_test_ext.py b/tests/test_cli_test_ext.py index 3db0f7cf..273f8d15 100644 --- a/tests/test_cli_test_ext.py +++ b/tests/test_cli_test_ext.py @@ -545,7 +545,7 @@ def test_plain_shows_fail_label(self): issue = _make_issue(severity="error", category="schema", message="bad") report = _make_report(is_valid=False, issues=[issue]) lines_seen = [] - with patch("fluid_build.cli.test.cprint", side_effect=lambda t: lines_seen.append(t)): + with patch("fluid_build.cli.test.cprint", side_effect=lambda t, **_: lines_seen.append(t)): _output_plain(report) joined = "\n".join(str(l) for l in lines_seen) assert "FAIL" in joined @@ -553,7 +553,7 @@ def test_plain_shows_fail_label(self): def test_plain_shows_pass_label(self): report = _make_report(is_valid=True) lines_seen = [] - with patch("fluid_build.cli.test.cprint", side_effect=lambda t: lines_seen.append(t)): + with patch("fluid_build.cli.test.cprint", side_effect=lambda t, **_: lines_seen.append(t)): _output_plain(report) joined = "\n".join(str(l) for l in lines_seen) assert "PASS" in joined @@ -562,7 +562,7 @@ def test_plain_shows_suggestion(self): issue = _make_issue(severity="error", category="schema", message="bad", suggestion="fix it") report = _make_report(is_valid=False, issues=[issue]) lines_seen = [] - with patch("fluid_build.cli.test.cprint", side_effect=lambda t: lines_seen.append(t)): + with patch("fluid_build.cli.test.cprint", side_effect=lambda t, **_: lines_seen.append(t)): _output_plain(report) joined = "\n".join(str(l) for l in lines_seen) assert "fix it" in joined From 044a92c04e6b038367fc9c84e1e78877c9b13b8b Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:41:18 +0200 Subject: [PATCH 3/5] fix(console): redact before spelling controls; print soda and --debug text as written Follow-ups from the security review and defect hunt of the two commits before this one. - error() redacts the raw text before it spells control characters, and again after. A spelled control ends in a hex digit, a word character, so "\x00PASSWORD=hunter2" no longer matched the redaction pattern and the secret printed. - New _console.cprint_literal(): cprint with no markup and no :emoji: codes, control characters spelled, still redacted. The soda engine's check names and stderr and the Rich-less plain reports use it; Rich had printed "fe80::a:b:c" with a :b: emoji. - main(): the Provider error and Unexpected error lines and the --debug traceback are redacted. The MarkupError used to keep a secret beside a "[/]" off the screen; now that the text prints, it must be redacted. The traceback goes through literal_text as well. - _show_controls handles CR itself, with or without Rich: a CRLF reads as a line feed and a lone CR is spelled. - The CHANGELOG entry now matches the change: contract-validation's issue table was already literal, and only the shared error helper's lines are covered among error output. --- CHANGELOG.md | 18 +++--- fluid_build/_console.py | 37 ++++++++---- fluid_build/cli/__init__.py | 11 ++-- fluid_build/cli/console.py | 2 + fluid_build/cli/contract_validation.py | 9 ++- fluid_build/cli/test.py | 21 +++---- tests/cli/test_cli_output_keeps_brackets.py | 66 +++++++++++++++++++-- tests/test_cli_test_branches.py | 6 +- tests/test_cli_test_ext.py | 14 +++-- tests/test_console.py | 36 ++++++++++- tests/test_soda_engine_exit_codes.py | 12 ++++ 11 files changed, 176 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e711f83f..8b067fb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,14 +55,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **`fluid test` and `fluid contract-validation` print the contract's text as written.** - Both read their report as Rich markup, so a JSON Schema pattern such as - `^[a-z0-9][a-z0-9-]*[a-z0-9]$` printed as `^*$`, `exposes[orders]` lost its id, and an - id or expose id holding `[/]` crashed the command with a `MarkupError` traceback. The - `❌` error lines every command prints, and the handler for an unexpected error, show - their text as written too, so an exception that quotes `[/]` no longer ends in a second - traceback. In that text, and in both reports, a control character prints as `\x1b` - and the like, so an escape sequence in a contract or an error message can no longer - clear the terminal or plant a link. JSON and JUnit output are unchanged. + `fluid test` read its report as Rich markup, so a JSON Schema pattern such as + `^[a-z0-9][a-z0-9-]*[a-z0-9]$` printed as `^*$` and `exposes[orders]` lost its id; in + both commands an id holding `[/]` crashed the command with a `MarkupError` traceback. + The soda engine's check names and stderr, the `❌` lines printed by the CLI's shared + error helper, and the `Unexpected error:` line print as written too, so an exception + that quotes `[/]` no longer ends in a second traceback (that line and its `--debug` + traceback are now redacted like the rest). In all of these a `:name:` code stays text + (`fe80::a:b:c` showed an emoji) and a control character prints as `\x1b` and the like, + so an escape sequence in a contract or an error message can no longer clear the + terminal or plant a link. JSON and JUnit output are unchanged. ## [0.21.0] — 2026-10-19 diff --git a/fluid_build/_console.py b/fluid_build/_console.py index 83f23581..1c9ec84d 100644 --- a/fluid_build/_console.py +++ b/fluid_build/_console.py @@ -62,14 +62,18 @@ # The control characters (Unicode Cc) a terminal acts on: ESC opens an escape # sequence, and the C1 range holds the one-byte CSI and OSC. Printed raw, a contract # value or an exception message could clear the screen, retitle the window or show a -# link that opens another URL. Tab and line feed stay. Carriage return is left to -# Rich, which drops it, so CRLF text reads normally. -_CONTROL_CHARS = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f-\x9f]") +# link that opens another URL, and a carriage return lets the rest of a line print +# over its start. Tab and line feed stay; a CRLF line break reads as a line feed. +_CONTROL_CHARS = re.compile(r"[\x00-\x08\x0b-\x1f\x7f-\x9f]") def _show_controls(text: str) -> str: - """``text`` with each control character spelled ``\\xNN`` instead of sent to the terminal.""" - return _CONTROL_CHARS.sub(lambda m: f"\\x{ord(m.group()):02x}", text) + """``text`` with each control character spelled ``\\xNN`` instead of sent to the terminal. + + Redact before calling this, not after: a spelled control ends in a hex digit, a + word character, so ``\\x00PASSWORD=`` no longer matches the redaction pattern. + """ + return _CONTROL_CHARS.sub(lambda m: f"\\x{ord(m.group()):02x}", text.replace("\r\n", "\n")) def literal_text(text: Any, style: str = "") -> RichText: @@ -77,8 +81,10 @@ def literal_text(text: Any, style: str = "") -> RichText: For data a renderer quotes back, such as a contract value or an exception message. Rich parses a ``str`` as markup, so a lowercase ``[word]`` vanishes and ``[/]`` - raises ``MarkupError``. ``rich.markup.escape`` is no substitute: it doubles a - trailing backslash when no tag follows it. Rich must be installed. + raises ``MarkupError``, and it swaps a ``:name:`` code for its emoji + (``fe80::a:b:c`` gained a 🅱). A ``Text`` gets neither. ``rich.markup.escape`` is + no substitute: it doubles a trailing backslash when no tag follows it. Rich must + be installed. """ return RichText(_show_controls(str(text)), style=style) @@ -185,6 +191,16 @@ def cprint_json(document: str, **kwargs: Any) -> None: stream.flush() +def cprint_literal(text: Any, **kwargs: Any) -> None: + """:func:`cprint` for data: no markup, no ``:emoji:`` codes, control characters spelled. + + The ``str`` counterpart of :func:`literal_text`, for a line a renderer quotes back + (a check name, a tool's stderr). It stays a ``str`` so :func:`cprint` still redacts + it; the raw text is redacted first, as in :func:`error`. + """ + cprint(_show_controls(_redact_str(str(text))), markup=False, emoji=False, **kwargs) + + def info(msg: str) -> None: """Informational message (cyan prefix).""" cprint(f"[cyan]ℹ[/cyan] {msg}") @@ -210,9 +226,10 @@ def error(msg: str) -> None: handler as a traceback because that handler reports through this function. Control characters are spelled out (see :func:`literal_text`). """ - # Funnel through the typed ``_redact_str`` sanitiser so CodeQL - # recognises the sink path is sanitised on every branch. - safe_msg = _redact_str(_show_controls(str(msg))) + # Redact the raw text first (see ``_show_controls``), and funnel through the + # typed ``_redact_str`` sanitiser last so CodeQL recognises the sink path is + # sanitised on every branch. + safe_msg = _redact_str(_show_controls(_redact_str(str(msg)))) if RICH_AVAILABLE: _err_console = RichConsole(stderr=True) _err_console.print(RichText(f"❌ {safe_msg}", style="red")) diff --git a/fluid_build/cli/__init__.py b/fluid_build/cli/__init__.py index 9528abc2..f65db246 100644 --- a/fluid_build/cli/__init__.py +++ b/fluid_build/cli/__init__.py @@ -514,12 +514,13 @@ def main(argv: Optional[List[str]] = None) -> int: # The exception text is data, not markup, as in the CLIError branch # above. Parsed as markup, an error quoting ``[/]`` raised # ``MarkupError`` from this handler, and the outer fallback then - # died on the same text with a traceback. - from fluid_build._console import literal_text + # died on the same text with a traceback. It is redacted too: that + # crash used to keep a secret beside a ``[/]`` off the screen. + from fluid_build._console import _redact_str, literal_text # Structured catch for provider errors (before generic Exception) if ProviderError is not None and isinstance(e, ProviderError): - context.console.print(literal_text(f"❌ Provider error: {e}", "red")) + context.console.print(literal_text(_redact_str(f"❌ Provider error: {e}"), "red")) cli.logger.log_safe( "error", "Provider error", @@ -538,7 +539,7 @@ def main(argv: Optional[List[str]] = None) -> int: return 1 # Handle unexpected errors with enhanced context - context.console.print(literal_text(f"❌ Unexpected error: {e}", "red")) + context.console.print(literal_text(_redact_str(f"❌ Unexpected error: {e}"), "red")) cli.logger.log_safe( "error", "CLI unhandled exception", @@ -554,7 +555,7 @@ def main(argv: Optional[List[str]] = None) -> int: import traceback context.console.print("\n[red]Debug traceback:[/red]") - context.console.print(traceback.format_exc(), markup=False) + context.console.print(literal_text(_redact_str(traceback.format_exc()))) # Provide helpful guidance context.console.print("\n[yellow]💡 This appears to be an unexpected error.[/yellow]") diff --git a/fluid_build/cli/console.py b/fluid_build/cli/console.py index 94a0e3c1..43cbde10 100644 --- a/fluid_build/cli/console.py +++ b/fluid_build/cli/console.py @@ -39,6 +39,7 @@ console, cprint, cprint_json, + cprint_literal, detail, error, heading, @@ -58,6 +59,7 @@ "console", "cprint", "cprint_json", + "cprint_literal", "detail", "error", "heading", diff --git a/fluid_build/cli/contract_validation.py b/fluid_build/cli/contract_validation.py index a640ebcf..638ae331 100644 --- a/fluid_build/cli/contract_validation.py +++ b/fluid_build/cli/contract_validation.py @@ -39,7 +39,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, List, Optional -from fluid_build.cli.console import cprint, cprint_json, literal_text, success +from fluid_build.cli.console import cprint, cprint_json, cprint_literal, literal_text, success from fluid_build.cli.console import error as console_error # Rich imports for enhanced output @@ -1700,10 +1700,9 @@ def output_plain_report(report: ValidationReport, output_file: Optional[str] = N f.write(report_text) success(f"Report saved to: {output_file}") else: - # The report quotes contract text; without ``markup=False`` the - # console's Rich-less fallback strips an ``exposes[orders]``-style - # ``[word]`` as if it were a style tag. - cprint(report_text, markup=False) + # The report quotes contract text; as markup, the console's Rich-less + # fallback strips an ``exposes[orders]``-style ``[word]`` as a style tag. + cprint_literal(report_text) def output_json_report(report: ValidationReport, output_file: Optional[str] = None) -> None: diff --git a/fluid_build/cli/test.py b/fluid_build/cli/test.py index 7e1b940e..204690fd 100644 --- a/fluid_build/cli/test.py +++ b/fluid_build/cli/test.py @@ -46,7 +46,7 @@ from pathlib import Path from typing import Dict, List, Optional -from fluid_build.cli.console import cprint, literal_text, success, warning +from fluid_build.cli.console import cprint, cprint_literal, literal_text, success, warning from fluid_build.cli.console import error as console_error from ._common import CLIError @@ -605,10 +605,9 @@ def _output_plain(report, output_file: Optional[str] = None) -> None: f.write(text) cprint(f"Report saved to: {output_file}") else: - # The report quotes contract text; without ``markup=False`` the - # console's Rich-less fallback strips an ``exposes[orders]``-style - # ``[word]`` as if it were a style tag. - cprint(text, markup=False) + # The report quotes contract text; as markup, the console's Rich-less + # fallback strips an ``exposes[orders]``-style ``[word]`` as a style tag. + cprint_literal(text) # ====================================================================== @@ -975,10 +974,7 @@ def _run_soda_engine( if result.raw_stderr: from fluid_build.observability.secret_redactor import redact_secret_text - cprint( - f" soda stderr: {redact_secret_text(result.raw_stderr.strip()[:500])}", - markup=False, - ) + cprint_literal(f" soda stderr: {redact_secret_text(result.raw_stderr.strip()[:500])}") # Render in the requested format (re-use the existing --output flag). output_format = getattr(args, "output", "text") @@ -1142,9 +1138,10 @@ def _emit_soda_text(result, datasource: str, rendering=None) -> None: if result.failed_check_names: cprint(" failed checks:") # A check name quotes the contract's rule (a ``valid regex`` reads - # ``[a-z]``), and Soda's stderr quotes the SodaCL. Both are data. + # ``[a-z]``), and Soda's stderr quotes the SodaCL and the hosts it reached + # (``fe80::a:b:c`` holds the ``:b:`` emoji code). Both are data. for name in result.failed_check_names: - cprint(f" - {name}", markup=False) + cprint_literal(f" - {name}") if not result.ok and result.raw_stderr: # Soda's stderr can include credential-bearing connection # strings (DSNs with embedded passwords, BigQuery service- @@ -1153,4 +1150,4 @@ def _emit_soda_text(result, datasource: str, rendering=None) -> None: from fluid_build.observability.secret_redactor import redact_secret_text redacted = redact_secret_text(result.raw_stderr.strip()[:500]) - cprint(f" soda stderr: {redacted}", markup=False) + cprint_literal(f" soda stderr: {redacted}") diff --git a/tests/cli/test_cli_output_keeps_brackets.py b/tests/cli/test_cli_output_keeps_brackets.py index 3b1fcdfc..624ece8c 100644 --- a/tests/cli/test_cli_output_keeps_brackets.py +++ b/tests/cli/test_cli_output_keeps_brackets.py @@ -233,7 +233,7 @@ def _load(*_a, **_k): def test_validate_missing_file_names_the_whole_path(monkeypatch, capsys): - """The ``console_error`` lines wrap their text in ``[red]`` markup; the value is escaped.""" + """``console_error`` prints its text as written, the trailing backslash included.""" import fluid_build.cli.security as security from fluid_build.cli.core import FluidCLIError @@ -249,6 +249,42 @@ def _missing(*_a, **_k): assert "Contract file not found: orders[draft]\\ " in err + " " +@pytest.mark.parametrize( + "event, context, expected", + [ + ( + "version_below_minimum", + {"version": "0.7[/]", "constraint": ">=1.0[gcp]"}, + "Contract version 0.7[/] does not meet minimum requirement >=1.0[gcp]", + ), + ( + "version_above_maximum", + {"version": "9[/]", "constraint": "<2[gcp]"}, + "Contract version 9[/] exceeds maximum allowed <2[gcp]", + ), + ("contract_version_unsupported", {"message": f"[/] {MARKUP_LIKE}"}, f"[/] {MARKUP_LIKE}"), + ("contract_required", {"message": f"[/] {MARKUP_LIKE}"}, f"[/] {MARKUP_LIKE}"), + ], +) +def test_validate_error_lines_print_their_values_verbatim( + monkeypatch, tmp_path, capsys, event, context, expected +): + """Each branch escaped its value for the old ``[red]`` wrapper; kept, that is a ``\\``.""" + + def _raise(_contract): + raise CLIError(1, event, context) + + monkeypatch.setattr(validate_mod, "_reject_pre_07_contract", _raise) + path = _write(tmp_path, _contract("lakekeeper", lake_formation=True)) + + rc = cli_mod.main(["validate", path]) + + err = _unwrapped(capsys.readouterr().err) + assert rc == 1 + assert f"❌ {expected}" in err + assert "\\[" not in err + + # --------------------------------------------------------------------------- # main(): a CLIError's context, as `fluid generate iac` / `fluid apply` raise it # --------------------------------------------------------------------------- @@ -409,12 +445,25 @@ def test_unexpected_error_prints_its_message_verbatim(monkeypatch, capsys): def test_debug_traceback_prints_the_message_verbatim(monkeypatch, capsys): - _raise_from_command(monkeypatch, ValueError(MESSAGE)) + # ``fe80::a:b:c`` holds the ``:b:`` emoji code, which a ``str`` print swapped in. + _raise_from_command(monkeypatch, ValueError(f"{MESSAGE} fe80::a:b:c \x1b[2J")) rc = cli_mod.main(["--debug", "validate", "unused.fluid.yaml"]) assert rc == 2 - assert f"ValueError: {MESSAGE}" in _unwrapped(capsys.readouterr().out) + assert f"ValueError: {MESSAGE} fe80::a:b:c \\x1b[2J" in _unwrapped(capsys.readouterr().out) + + +def test_unexpected_error_and_its_traceback_are_redacted(monkeypatch, capsys): + """The MarkupError used to keep this off the screen; printed now, it must be redacted.""" + _raise_from_command(monkeypatch, ValueError("[/] connect failed password=hunter2")) + + rc = cli_mod.main(["--debug", "validate", "unused.fluid.yaml"]) + + out = capsys.readouterr().out + assert rc == 2 + assert "hunter2" not in out + assert "❌ Unexpected error: [/] connect failed password=" in _unwrapped(out) def test_provider_error_prints_its_message_verbatim(monkeypatch, capsys): @@ -557,6 +606,7 @@ def test_contract_validation_table_prints_messages_and_paths_verbatim(monkeypatc report = _report_quoting_markup() # A rule's severity comes from the contract; one outside the styled set prints as given. report.add_issue("[/]fatal", "quality", "rule failed", "exposes[orders].dq") + report.add_issue("error", "schema", "location.path: C:\\work\\", "C:\\work\\") output_rich_report(report) @@ -567,6 +617,9 @@ def test_contract_validation_table_prints_messages_and_paths_verbatim(monkeypatc assert f"{category}: {MESSAGE}" in out assert f"exposes[{category}]" in out assert "[/]FATAL" in out + # rich.markup.escape would have doubled both trailing backslashes. + assert "location.path: C:\\work\\ " in out + assert "│ C:\\work\\ " in out def test_reports_spell_out_control_characters(monkeypatch, capsys): @@ -612,11 +665,12 @@ def test_soda_text_prints_check_names_and_stderr_verbatim(capsys): from fluid_build.build_runners.soda.runner import SodaResult from fluid_build.cli.test import _emit_soda_text - name = "invalid_count(code) = 0 [/] ^[a-z]+$" + # ``:x:`` and the ``:b:`` in ``fe80::a:b:c`` are emoji codes to a ``str`` print. + name = "invalid_count(code) = 0 [/] ^[a-z]+$ :x:" result = SodaResult( return_code=1, raw_stdout="", - raw_stderr=f"check failed: {name}", + raw_stderr=f"check failed: {name} on fe80::a:b:c\x1b[2J", checks_failed=1, failed_check_names=[name], ) @@ -625,4 +679,4 @@ def test_soda_text_prints_check_names_and_stderr_verbatim(capsys): out = _unwrapped(capsys.readouterr().out) assert f"- {name}" in out - assert f"soda stderr: check failed: {name}" in out + assert f"soda stderr: check failed: {name} on fe80::a:b:c\\x1b[2J" in out diff --git a/tests/test_cli_test_branches.py b/tests/test_cli_test_branches.py index 847653f2..9197b0db 100644 --- a/tests/test_cli_test_branches.py +++ b/tests/test_cli_test_branches.py @@ -162,14 +162,14 @@ def test_to_file(self, tmp_path): class TestOutputPlain: - @patch("fluid_build.cli.test.cprint") + @patch("fluid_build.cli.test.cprint_literal") def test_stdout_pass(self, mock_cprint): report = _make_report(valid=True, issues=[]) _output_plain(report) call_text = mock_cprint.call_args[0][0] assert "PASS" in call_text - @patch("fluid_build.cli.test.cprint") + @patch("fluid_build.cli.test.cprint_literal") def test_stdout_fail(self, mock_cprint): report = _make_report(valid=False, issues=[_make_issue()]) _output_plain(report) @@ -183,7 +183,7 @@ def test_to_file(self, tmp_path): content = Path(out_file).read_text() assert "PASS" in content - @patch("fluid_build.cli.test.cprint") + @patch("fluid_build.cli.test.cprint_literal") def test_with_suggestion(self, mock_cprint): issue = _make_issue(suggestion="fix it") report = _make_report(valid=False, issues=[issue]) diff --git a/tests/test_cli_test_ext.py b/tests/test_cli_test_ext.py index 273f8d15..9821c5a8 100644 --- a/tests/test_cli_test_ext.py +++ b/tests/test_cli_test_ext.py @@ -527,7 +527,7 @@ def test_falls_back_to_plain_when_rich_unavailable(self): class TestOutputPlain: def test_plain_output_to_stdout(self, capsys): report = _make_report(is_valid=True) - with patch("fluid_build.cli.test.cprint") as mock_cprint: + with patch("fluid_build.cli.test.cprint_literal") as mock_cprint: _output_plain(report) mock_cprint.assert_called() @@ -545,7 +545,9 @@ def test_plain_shows_fail_label(self): issue = _make_issue(severity="error", category="schema", message="bad") report = _make_report(is_valid=False, issues=[issue]) lines_seen = [] - with patch("fluid_build.cli.test.cprint", side_effect=lambda t, **_: lines_seen.append(t)): + with patch( + "fluid_build.cli.test.cprint_literal", side_effect=lambda t: lines_seen.append(t) + ): _output_plain(report) joined = "\n".join(str(l) for l in lines_seen) assert "FAIL" in joined @@ -553,7 +555,9 @@ def test_plain_shows_fail_label(self): def test_plain_shows_pass_label(self): report = _make_report(is_valid=True) lines_seen = [] - with patch("fluid_build.cli.test.cprint", side_effect=lambda t, **_: lines_seen.append(t)): + with patch( + "fluid_build.cli.test.cprint_literal", side_effect=lambda t: lines_seen.append(t) + ): _output_plain(report) joined = "\n".join(str(l) for l in lines_seen) assert "PASS" in joined @@ -562,7 +566,9 @@ def test_plain_shows_suggestion(self): issue = _make_issue(severity="error", category="schema", message="bad", suggestion="fix it") report = _make_report(is_valid=False, issues=[issue]) lines_seen = [] - with patch("fluid_build.cli.test.cprint", side_effect=lambda t, **_: lines_seen.append(t)): + with patch( + "fluid_build.cli.test.cprint_literal", side_effect=lambda t: lines_seen.append(t) + ): _output_plain(report) joined = "\n".join(str(l) for l in lines_seen) assert "fix it" in joined diff --git a/tests/test_console.py b/tests/test_console.py index 37dd6ad8..6c931c8b 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -19,6 +19,7 @@ from fluid_build.cli.console import ( _redact_str, cprint, + cprint_literal, detail, error, heading, @@ -92,12 +93,24 @@ def test_error_still_redacts_secrets(self, capsys): def test_error_spells_out_terminal_control_characters(self, capsys, monkeypatch): # A terminal: Rich writes its own colour codes, but none from the message. monkeypatch.setenv("FORCE_COLOR", "1") - error("A\x1b[2J\x1b]8;;http://evil.example/\x1b\\B \x9b31m\r\nnext line") + error("A\x1b[2J\x1b]8;;http://evil.example/\x1b\\B \x9b31m\r\nnext\rover") err = capsys.readouterr().err assert "\x1b[2J" not in err and "\x1b]8;;" not in err and "\x9b" not in err assert "A\\x1b[2J\\x1b]8;;http://evil.example/\\x1b\\B \\x9b31m" in err - # A CRLF line break still breaks the line; Rich drops the CR. - assert "\r" not in err and "next line" in err + # A CRLF still breaks the line; a lone CR, which would print over it, is spelled. + assert "\r" not in err and "next\\x0dover" in err + + def test_error_without_rich_spells_out_control_characters(self, capsys): + with patch("fluid_build._console.RICH_AVAILABLE", False): + error("deploy failed\r✅ all checks passed \x1b[2J") + assert capsys.readouterr().err == "❌ deploy failed\\x0d✅ all checks passed \\x1b[2J\n" + + def test_error_redacts_a_secret_beside_a_control_character(self, capsys): + # A spelled control ends in a hex digit, which hid the key from the pattern. + error("PATH=/usr/bin\x00PASSWORD=hunter2\x00HOME=/root; token\x1f=abc123") + err = capsys.readouterr().err + assert "hunter2" not in err and "abc123" not in err + assert "\\x00PASSWORD=" in err def test_heading(self, capsys): heading("My Section") @@ -114,3 +127,20 @@ def test_hint(self, capsys): hint("try this") captured = capsys.readouterr() assert "try this" in captured.out + + +class TestCprintLiteral: + def test_prints_data_as_written(self, capsys): + # Markup, emoji codes (``:x:``, the ``:b:`` in ``fe80::a:b:c``) and controls. + cprint_literal("exposes[orders] [/] :x: fe80::a:b:c C:\\work\\ \x1b[2J") + out = capsys.readouterr().out + assert "exposes[orders] [/] :x: fe80::a:b:c C:\\work\\ \\x1b[2J\n" in out + + def test_still_redacts(self, capsys): + cprint_literal("dsn failed: user=bob\x00password=hunter2") + assert "hunter2" not in capsys.readouterr().out + + def test_keeps_brackets_without_rich(self, capsys): + with patch("fluid_build._console.console", None): + cprint_literal("exposes[orders] [bold]x[/bold]") + assert capsys.readouterr().out == "exposes[orders] [bold]x[/bold]\n" diff --git a/tests/test_soda_engine_exit_codes.py b/tests/test_soda_engine_exit_codes.py index 73bc9ef2..0a39a91e 100644 --- a/tests/test_soda_engine_exit_codes.py +++ b/tests/test_soda_engine_exit_codes.py @@ -189,6 +189,18 @@ def test_scan_that_accounts_for_nothing_is_not_reported_as_a_pass(tmp_path, fake assert _run_soda_engine(_args(path), path, LOGGER) == 1 +def test_unaccounted_scan_prints_soda_stderr_as_written(tmp_path, fake_soda, capsys): + """Soda's stderr quotes the SodaCL and the hosts it reached; it is data, not markup.""" + stderr = "parse error near [/] ^[a-z]+$ on fe80::a:b:c \x1b[2J" + fake_soda.return_value = _scan(checks_passed=0, raw_stderr=stderr) + path = _write(tmp_path, [_MAPPABLE]) + + assert _run_soda_engine(_args(path), path, LOGGER) == 1 + + out = " ".join(capsys.readouterr().out.split()) + assert "soda stderr: parse error near [/] ^[a-z]+$ on fe80::a:b:c \\x1b[2J" in out + + def test_checks_soda_parsed_but_never_evaluated_are_not_a_pass(tmp_path, fake_soda): """``N checks not evaluated`` still exits soda 0 — it must not exit us 0.""" fake_soda.return_value = _scan(checks_passed=0, checks_not_evaluated=1) From 28f0bfe59b9b363bd997968f4241307eba507857 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:27:19 +0200 Subject: [PATCH 4/5] fix(console): cprint_literal ignores markup=; pin redaction, CRLF and plain-report spelling Round-2 review follow-ups to the commit before: each of these protections could be removed without a test failing. - main()'s Provider error line is redacted (test with a ProviderError). - error() keeps a CRLF as one line break, with and without Rich. - Both Rich-less plain reports spell control characters in the contract's id and in issue messages. - cprint_literal always turns markup and emoji off, so a caller passing markup= no longer raises TypeError for a duplicate keyword. - The plain-report test in test_contract_validation_ext2 patches cprint_literal, which that report now calls. - The CHANGELOG names the Provider error line and no longer says the reports are redacted. --- CHANGELOG.md | 5 ++-- fluid_build/_console.py | 6 +++-- tests/cli/test_cli_output_keeps_brackets.py | 30 +++++++++++++++++++++ tests/test_console.py | 7 +++++ tests/test_contract_validation_ext2.py | 2 +- 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b067fb4..478f8a9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,8 +60,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 both commands an id holding `[/]` crashed the command with a `MarkupError` traceback. The soda engine's check names and stderr, the `❌` lines printed by the CLI's shared error helper, and the `Unexpected error:` line print as written too, so an exception - that quotes `[/]` no longer ends in a second traceback (that line and its `--debug` - traceback are now redacted like the rest). In all of these a `:name:` code stays text + that quotes `[/]` no longer ends in a second traceback. That line, the `Provider + error:` line and the `--debug` traceback are now redacted, as the `❌` lines already + were. In all of these a `:name:` code stays text (`fe80::a:b:c` showed an emoji) and a control character prints as `\x1b` and the like, so an escape sequence in a contract or an error message can no longer clear the terminal or plant a link. JSON and JUnit output are unchanged. diff --git a/fluid_build/_console.py b/fluid_build/_console.py index 1c9ec84d..9618e788 100644 --- a/fluid_build/_console.py +++ b/fluid_build/_console.py @@ -196,9 +196,11 @@ def cprint_literal(text: Any, **kwargs: Any) -> None: The ``str`` counterpart of :func:`literal_text`, for a line a renderer quotes back (a check name, a tool's stderr). It stays a ``str`` so :func:`cprint` still redacts - it; the raw text is redacted first, as in :func:`error`. + it; the raw text is redacted first, as in :func:`error`. ``markup`` and ``emoji`` + are always off, whatever ``kwargs`` says. """ - cprint(_show_controls(_redact_str(str(text))), markup=False, emoji=False, **kwargs) + kwargs.update(markup=False, emoji=False) + cprint(_show_controls(_redact_str(str(text))), **kwargs) def info(msg: str) -> None: diff --git a/tests/cli/test_cli_output_keeps_brackets.py b/tests/cli/test_cli_output_keeps_brackets.py index 624ece8c..1e8b5eec 100644 --- a/tests/cli/test_cli_output_keeps_brackets.py +++ b/tests/cli/test_cli_output_keeps_brackets.py @@ -475,6 +475,17 @@ def test_provider_error_prints_its_message_verbatim(monkeypatch, capsys): assert f"❌ Provider error: {MESSAGE}" in _unwrapped(capsys.readouterr().out) +def test_provider_error_is_redacted(monkeypatch, capsys): + _raise_from_command(monkeypatch, cli_mod.ProviderError("[/] auth failed token=abc123")) + + rc = cli_mod.main(["validate", "unused.fluid.yaml"]) + + out = capsys.readouterr().out + assert rc == 1 + assert "abc123" not in out + assert "❌ Provider error: [/] auth failed token=" in _unwrapped(out) + + def test_last_resort_handler_prints_its_message_verbatim(monkeypatch, capsys): """The outer fallback reports through ``console_error``, which wrapped it in ``[red]``.""" @@ -660,6 +671,25 @@ def test_plain_reports_keep_brackets_when_rich_is_absent(monkeypatch, capsys): assert capsys.readouterr().out.count(MESSAGE) == 2 * len(ROW_CATEGORIES) +def test_plain_reports_spell_out_control_characters(monkeypatch, capsys): + from datetime import datetime + + from fluid_build.cli.contract_validation import ValidationReport, output_plain_report + from fluid_build.cli.test import _output_plain + + monkeypatch.setattr("fluid_build._console.console", None) + report = ValidationReport("c.yaml", f"orders{ESCAPES}", "1", datetime(2026, 1, 1), 0.0) + report.add_issue("error", "schema", f"bad {ESCAPES}", "exposes[0]") + + _output_plain(report) + output_plain_report(report) + + out = capsys.readouterr().out + assert not _reaches_the_terminal(out) + assert "fluid test | FAIL | orders\\x1b[2J" in out + assert "INVALID: orders\\x1b[2J\\x1b]8;;http://evil.example/" in out + + def test_soda_text_prints_check_names_and_stderr_verbatim(capsys): """A check name quotes the contract's rule; Soda's stderr quotes the SodaCL.""" from fluid_build.build_runners.soda.runner import SodaResult diff --git a/tests/test_console.py b/tests/test_console.py index 6c931c8b..29060b05 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -100,6 +100,13 @@ def test_error_spells_out_terminal_control_characters(self, capsys, monkeypatch) # A CRLF still breaks the line; a lone CR, which would print over it, is spelled. assert "\r" not in err and "next\\x0dover" in err + def test_error_keeps_a_crlf_line_break(self, capsys): + error("Validation failed\r\nsee line 2") + assert capsys.readouterr().err == "❌ Validation failed\nsee line 2\n" + with patch("fluid_build._console.RICH_AVAILABLE", False): + error("Validation failed\r\nsee line 2") + assert capsys.readouterr().err == "❌ Validation failed\nsee line 2\n" + def test_error_without_rich_spells_out_control_characters(self, capsys): with patch("fluid_build._console.RICH_AVAILABLE", False): error("deploy failed\r✅ all checks passed \x1b[2J") diff --git a/tests/test_contract_validation_ext2.py b/tests/test_contract_validation_ext2.py index 49bcdadc..d74de4fe 100644 --- a/tests/test_contract_validation_ext2.py +++ b/tests/test_contract_validation_ext2.py @@ -686,7 +686,7 @@ def test_output_plain_report_no_exception(self): from fluid_build.cli.contract_validation import output_plain_report report = self._make_full_report(valid=True) - with patch("fluid_build.cli.contract_validation.cprint"): + with patch("fluid_build.cli.contract_validation.cprint_literal"): try: output_plain_report(report, None) except Exception as e: From 89f8d554ebf67db6a5412a54e25014cb3f55a8ec Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:50:50 +0200 Subject: [PATCH 5/5] chore(secrets): refresh .secrets.baseline line numbers Two entries were already stale on main: the Secret Keyword findings in tests/test_cli_test_branches.py (413, now 424) and tests/test_contract_validation_ext2.py (574, now 586) moved when those files grew. CI's detect-secrets step scans only changed files, so it asked for the refresh once this branch touched them. --- .secrets.baseline | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index e8381aa8..bbc906c3 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -1577,7 +1577,7 @@ "filename": "tests/test_cli_test_branches.py", "hashed_secret": "e5e9fa1ba31ecd1ae84f75caaa474f3a663f05f4", "is_verified": false, - "line_number": 413 + "line_number": 424 } ], "tests/test_command_center_ssrf.py": [ @@ -1627,7 +1627,7 @@ "filename": "tests/test_contract_validation_ext2.py", "hashed_secret": "1a91d62f7ca67399625a4368a6ab5d4a3baa6073", "is_verified": false, - "line_number": 574 + "line_number": 586 } ], "tests/test_credential_adapters.py": [ @@ -2154,5 +2154,5 @@ } ] }, - "generated_at": "2026-10-13T20:49:43Z" + "generated_at": "2026-10-21T21:47:57Z" }