Repository navigation
Conversation
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.
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.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
fluid testandfluid contract-validationpassed the contract's own text to Rich asmarkup. In
fluid testa JSON Schema pattern lost its character classes(
^[a-z0-9][a-z0-9-]*[a-z0-9]$printed as^*$) andexposes[orders]lost its id. Inboth commands an id or expose id holding
[/]raisedMarkupError. That error then hitmain()'s generic handler, which wrapped it in[red]...[/red]and raised again, andfinally the last-resort
console_error(...), which did the same. So both commands diedwith 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:
fluid_build/_console.py, re-exported fromcli/console.py.literal_text()returns a richText, so the value is never parsed as markup or:emoji:codes, and it spells control characters as\xNN.cprint_literal()is itsstrcounterpart forcprint. It passesmarkup=Falseand
emoji=False, spells control characters, and still redacts.fluid test: the header panel and every results-table row go throughliteral_text. The_detail()helper (rich.markup.escape) is gone. The sodaengine's failed check names and Soda's stderr go through
cprint_literal.fluid contract-validation: the summary panel, each issue's message and path, anda severity outside the three styled ones go through
literal_text.cprint_literal. Theconsole's plain fallback otherwise strips anything shaped like
[word]._console.errorprints its message through the same spelling, redacting the rawtext before and after. An AST scan of its 130 call sites found none that passes
intended markup.
fluid validatestops escaping six messages before calling it; if itkept escaping, they would now print a stray backslash.
main(): theProvider errorandUnexpected errorlines and the--debugtraceback go through
literal_textand are now redacted. Before, theMarkupErrorkept a secret sitting next to a
[/]off the screen; now that the text prints, it hasto be redacted.
Why
Text, notrich.markup.escape, for a whole value:escapeappends a backslash toa trailing one so the value can sit before a tag (rich#2987, fixed in rich PR #3063). When
no tag follows,
C:\work\prints asC:\work\\. The CLIError branch ofmain()alreadymade the same choice.
Why control characters are spelled out: the security review of this branch found that
a top-level
strused to pass through Rich's highlighter, which happened to break upESC[2Jor an OSC-8 link by styling the[. ATextis not highlighted. Without thespelling, the
❌lines would have let a contract value or an exception message clearthe 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:cused to print with a 🅱. The reportpanels and tables render byte-identical ANSI for content without brackets or control
characters. JSON, JUnit and the plain
--output-filereport were already literal and areunchanged.
Out of scope, listed for follow-ups:
FluidCLIError.format_for_user)do not spell control characters;
console.print(f"[red]❌ ...")sites in the forge, ide, init and template modes stillparse their data as markup;
--output-file, the contract path,--datasource) echoedthrough
success()orcprintstill parse as markup.Type of Change
Documentation
already describe; the only sample, the
fluid testpanel indocs/cli/test.md, hasno brackets and reads the same. No flag, exit code or output format changes.
Checklist
pytest)ruff checkandblackwith no errorspython scripts/add_license_headers.py;examples/**exempt)Testing
New regression tests in
tests/cli/test_cli_output_keeps_brackets.py,tests/test_console.pyandtests/test_soda_engine_exit_codes.py. They cover:main();fluid testrow and the contract-validation table, trailing backslashesincluded;
fluid validateerror branch;main()'s Provider, Unexpected,--debugand last-resort handlers, and theirredaction;
console_error: literal, redacted next to a control character, and without Rich;cprint_literal;FORCE_COLOR=1).Each test fails on
mainor 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 areAttributeError: type object 'TokenType' has no attribute 'LLRR_ARROW', from sqlglotversion 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/perfserially: only wall-clock budgets fail, identically onmain, at loadaverage ~200.
fluid --helploads 1020 modules before and after.Live runs:
id: "tutorial.[/]hello",exposeId: "hello[/]message"and a tag thatfails
^[a-z0-9][a-z0-9-]*[a-z0-9]$, throughfluid testandfluid contract-validationwith--no-data --no-cache.rich.errors.MarkupError: closing tag '[/bold]' at position 47 doesn't match any open tag.--output json,--output junitand--output-filestay raw.ESC[2Jand an OSC-8 link, on a forcedterminal: 0 raw sequences reach the output; the spelled form is shown.
fluid test --engine sodawith an unmappable rule whose id holds[/],fe80::a:b:candESC[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
--debuglines, CHANGELOG accuracy, test gaps) are fixed in 044a92c, which a second round
reviewed; 28f0bfe adds the tests that round asked for.
Screenshots / Output