diff --git a/.secrets.baseline b/.secrets.baseline index 5e153ce6..1d6cae90 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": [ diff --git a/CHANGELOG.md b/CHANGELOG.md index 543d2ea5..464df8ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 calling `nextval()`, could not be written to an `s3://` output or, in `fluid_build.contract_tests`, to Parquet (`Cannot create the view ...: it reads rows held in this process`). Each now behaves as on 1.5. +- **`fluid test` and `fluid contract-validation` print the contract's text as written.** + `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, 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. ## [0.21.0] — 2026-10-19 diff --git a/fluid_build/_console.py b/fluid_build/_console.py index 3532dfec..9618e788 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,34 @@ _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, 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. + + 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: + """``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``, 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) def _redact_str(value: str) -> str: @@ -162,6 +191,18 @@ 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`. ``markup`` and ``emoji`` + are always off, whatever ``kwargs`` says. + """ + kwargs.update(markup=False, emoji=False) + cprint(_show_controls(_redact_str(str(text))), **kwargs) + + def info(msg: str) -> None: """Informational message (cyan prefix).""" cprint(f"[cyan]ℹ[/cyan] {msg}") @@ -178,13 +219,22 @@ def warning(msg: str) -> None: def error(msg: str) -> None: - """Error message (red ❌) – written to *stderr*.""" - # Funnel through the typed ``_redact_str`` sanitiser so CodeQL - # recognises the sink path is sanitised on every branch. - safe_msg = _redact_str(str(msg)) + """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`). + """ + # 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(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..f65db246 100644 --- a/fluid_build/cli/__init__.py +++ b/fluid_build/cli/__init__.py @@ -511,9 +511,16 @@ 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. 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(f"[red]❌ Provider error: {e}[/red]") + context.console.print(literal_text(_redact_str(f"❌ Provider error: {e}"), "red")) cli.logger.log_safe( "error", "Provider error", @@ -532,7 +539,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(_redact_str(f"❌ Unexpected error: {e}"), "red")) cli.logger.log_safe( "error", "CLI unhandled exception", @@ -548,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()) + 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 8ad5ba93..43cbde10 100644 --- a/fluid_build/cli/console.py +++ b/fluid_build/cli/console.py @@ -39,11 +39,13 @@ console, cprint, cprint_json, + cprint_literal, detail, error, heading, hint, info, + literal_text, success, warning, ) @@ -57,11 +59,13 @@ "console", "cprint", "cprint_json", + "cprint_literal", "detail", "error", "heading", "hint", "info", + "literal_text", "success", "warning", ] diff --git a/fluid_build/cli/contract_validation.py b/fluid_build/cli/contract_validation.py index 5bd0deaa..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, 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 @@ -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,9 @@ 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; 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 80c61bb3..204690fd 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, cprint_literal, 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,9 @@ 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; as markup, the console's Rich-less + # fallback strips an ``exposes[orders]``-style ``[word]`` as a style tag. + cprint_literal(text) # ====================================================================== @@ -980,7 +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])}") + 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") @@ -1143,8 +1137,11 @@ 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 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}") + 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}") + cprint_literal(f" soda stderr: {redacted}") 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..1e8b5eec 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 # --------------------------------------------------------------------------- @@ -379,3 +415,298 @@ 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): + # ``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} 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): + _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_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]``.""" + + 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 + + +# --------------------------------------------------------------------------- +# 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") + report.add_issue("error", "schema", "location.path: C:\\work\\", "C:\\work\\") + + 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 + # 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): + """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_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 + from fluid_build.cli.test import _emit_soda_text + + # ``: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} on fe80::a:b:c\x1b[2J", + 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} 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 3db0f7cf..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 9538a4e2..29060b05 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, @@ -77,6 +78,47 @@ 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\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 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") + 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") captured = capsys.readouterr() @@ -92,3 +134,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_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: 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)