Skip to content

fix(cli): fluid test and contract-validation print contract and error text literally - #738

Open
fas89 wants to merge 5 commits into
mainfrom
fix/rich-markup-escape
Open

fas89 wants to merge 5 commits into
mainfrom
fix/rich-markup-escape

Conversation

@fas89

@fas89 fas89 commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Description

fluid test and fluid contract-validation passed the contract's own text to Rich as
markup. In fluid test a JSON Schema pattern lost its character classes
(^[a-z0-9][a-z0-9-]*[a-z0-9]$ printed as ^*$) and exposes[orders] lost its id. In
both commands an id or expose id holding [/] raised MarkupError. That error then hit
main()'s generic handler, which wrapped it in [red]...[/red] and raised again, and
finally the last-resort console_error(...), which did the same. So both commands died
with a traceback. #734 had already made the "Environment overlay" row and the
contract-validation issue table literal; this PR covers the rest. It was found by the
defect scan on #734.

What changed:

  • Two helpers in fluid_build/_console.py, re-exported from cli/console.py.
    • literal_text() returns a rich Text, so the value is never parsed as markup or
      :emoji: codes, and it spells control characters as \xNN.
    • cprint_literal() is its str counterpart for cprint. It passes markup=False
      and emoji=False, spells control characters, and still redacts.
    • Tab and line feed are kept, and a CRLF reads as a line feed.
  • fluid test: the header panel and every results-table row go through
    literal_text. The _detail() helper (rich.markup.escape) is gone. The soda
    engine's failed check names and Soda's stderr go through cprint_literal.
  • fluid contract-validation: the summary panel, each issue's message and path, and
    a severity outside the three styled ones go through literal_text.
  • Both commands, without Rich: the plain reports go through cprint_literal. The
    console's plain fallback otherwise strips anything shaped like [word].
  • _console.error prints its message through the same spelling, redacting the raw
    text before and after. An AST scan of its 130 call sites found none that passes
    intended markup. fluid validate stops escaping six messages before calling it; if it
    kept escaping, they would now print a stray backslash.
  • main(): the Provider error and Unexpected error lines and the --debug
    traceback go through literal_text and are now redacted. Before, the MarkupError
    kept a secret sitting next to a [/] off the screen; now that the text prints, it has
    to be redacted.

Why Text, not rich.markup.escape, for a whole value: escape appends a backslash to
a trailing one so the value can sit before a tag (rich#2987, fixed in rich PR #3063). When
no tag follows, C:\work\ prints as C:\work\\. The CLIError branch of main() already
made the same choice.

Why control characters are spelled out: the security review of this branch found that
a top-level str used to pass through Rich's highlighter, which happened to break up
ESC[2J or an OSC-8 link by styling the [. A Text is not highlighted. Without the
spelling, the ❌ lines would have let a contract value or an exception message clear
the screen or plant a link. Report panels and table cells passed those sequences raw
before this change too; they are spelled now as well.

Visible changes besides the fix: the changed lines lose Rich's automatic highlighting
(numbers, URLs) and :emoji: codes. fe80::a:b:c used to print with a 🅱. The report
panels and tables render byte-identical ANSI for content without brackets or control
characters. JSON, JUnit and the plain --output-file report were already literal and are
unchanged.

Out of scope, listed for follow-ups:

  • the typed-error renderers (main()'s CLIError branch and FluidCLIError.format_for_user)
    do not spell control characters;
  • 34 console.print(f"[red]❌ ...") sites in the forge, ide, init and template modes still
    parse their data as markup;
  • operator-typed values (--output-file, the contract path, --datasource) echoed
    through success() or cprint still parse as markup.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would break existing functionality)
  • Documentation update
  • Refactor / code cleanup
  • New provider or provider enhancement

Documentation

  • Docs PR linked:
  • No docs needed — this change does not affect user-facing behaviour
    • Justification: terminal-output bug fix. The commands now print the text the docs
      already describe; the only sample, the fluid test panel in docs/cli/test.md, has
      no brackets and reads the same. No flag, exit code or output format changes.
  • Docs TODO

Checklist

  • I have read the Contributing Guide
  • My code follows the project's coding standards
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass (pytest)
  • I have run ruff check and black with no errors
  • New maintained Python files include license headers (python scripts/add_license_headers.py; examples/** exempt)

Testing

  • New regression tests in tests/cli/test_cli_output_keeps_brackets.py,
    tests/test_console.py and tests/test_soda_engine_exit_codes.py. They cover:

    • both commands end to end through main();
    • every fluid test row and the contract-validation table, trailing backslashes
      included;
    • the Rich-less plain reports and the soda text output and stderr lines;
    • each fluid validate error branch;
    • main()'s Provider, Unexpected, --debug and last-resort handlers, and their
      redaction;
    • console_error: literal, redacted next to a control character, and without Rich;
    • cprint_literal;
    • control characters on a terminal (FORCE_COLOR=1).

    Each test fails on main or when its fix is reverted, and passes here.

  • Affected test files: every file touching a changed module, 64 in all. Result: 2309
    passed. The 5 failures and 2 errors also happen on main. All of them are
    AttributeError: type object 'TokenType' has no attribute 'LLRR_ARROW', from sqlglot
    version skew in the local venv.

  • Gates:

    • ruff check fluid_build/ tests/ and the dead-code gate;
    • black==24.10.0 --check;
    • scripts/check_license_headers.py;
    • lint-imports (6 contracts kept);
    • scripts/mypy_strict.py;
    • tools/product_guardrail/check.py;
    • tests/perf serially: only wall-clock budgets fail, identically on main, at load
      average ~200. fluid --help loads 1020 modules before and after.
  • Live runs:

    • Contract with id: "tutorial.[/]hello", exposeId: "hello[/]message" and a tag that
      fails ^[a-z0-9][a-z0-9-]*[a-z0-9]$, through fluid test and
      fluid contract-validation with --no-data --no-cache.
      • Before: a traceback ending in rich.errors.MarkupError: closing tag '[/bold]' at position 47 doesn't match any open tag.
      • After: exit 1 on the schema errors, with every value shown as written.
      • --output json, --output junit and --output-file stay raw.
    • The same contract with an id carrying ESC[2J and an OSC-8 link, on a forced
      terminal: 0 raw sequences reach the output; the spelled form is shown.
    • fluid test --engine soda with an unmappable rule whose id holds [/],
      fe80::a:b:c and ESC[2J: printed as written, with no emoji and no traceback.
  • Reviews: a security review and an adversarial defect hunt of the branch. Their
    findings (redaction order, emoji and control characters on the soda and --debug
    lines, CHANGELOG accuracy, test gaps) are fixed in 044a92c, which a second round
    reviewed; 28f0bfe adds the tests that round asked for.

Screenshots / Output

╭──────────────────────────── fluid test ────────────────────────────╮
│ ❌ Data Contract Test: tutorial.[/]hello                            │
...
│ 1 │ ❌ │ Schema syntax │ exposes[0].exposeId: 'hello[/]message' does not match
...   tags[1]: 'Bad Tag!' does not match '^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$'

fas89 added 4 commits October 21, 2026 22:42
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.
…rally

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.
… 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:🅰️b:c" with a 🅱️ 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.
… 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.
@github-actions github-actions Bot added cli Changes to the CLI surface or implementation tests Test coverage or test infrastructure changes labels Oct 21, 2026
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.
@github-actions github-actions Bot added the security Security-related changes or reports label Oct 21, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Changes to the CLI surface or implementation security Security-related changes or reports tests Test coverage or test infrastructure changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant