Repository navigation
fix(report): escape Markdown table cells - #140
Conversation
Task ids or gate values containing `|`, line breaks, or a leading `<` broke the per-task and baseline-gate Markdown tables piped into the job summary: an embedded pipe changed the parsed column count and a cell opening with raw HTML could be interpreted as an HTML element. Escape every interpolated Markdown cell with a private `_md_cell` helper and add regression tests for both tables.
defeac4 to
0412997
Compare
ssf0409
left a comment
ssf0409
left a comment
There was a problem hiding this comment.
Thanks for contribution, added one feedback!
| """ | ||
| text = text.replace("\r\n", "\n").replace("\r", "\n").replace("\n", " ") | ||
| text = text.replace("|", "\\|") | ||
| if text.startswith("<"): |
There was a problem hiding this comment.
instead of doing this check, can we use built in html.escape() to escape the html elements?
example
from html import escape
escape("task <img src=x> & result", quote=False)
# 'task <img src=x> & result'
There was a problem hiding this comment.
Good call, html.escape() is cleaner than the hand-rolled < check.
_md_cell now collapses newlines and escapes | first, then returns escape(text, quote=False).
Pipe escaping and newline collapsing stay because html.escape handles neither, and they are Markdown concerns rather than HTML ones.
This also widens the fix slightly: every &, < and > in a cell is escaped now, not just a leading <.
Tests and the changelog entry are updated to match, with an & case added.
Table cells now neutralise &, < and > through html.escape instead of only backslash-escaping a leading <, so a task id or gate value can neither open a raw HTML element nor inject an entity into the Markdown tables piped into $GITHUB_STEP_SUMMARY. Pipe escaping and newline collapsing stay because they are Markdown, not HTML, concerns.
Resolves the overlap with #140 (Markdown table cell escaping): the gate table keeps main's `_md_cell` escaping and this branch's Evidence column, the #134 test expects the eight-column row, and both changelog entries sit under [Unreleased]. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01N1GtYXaFXi7LpXcpRxBP7o
|
Thanks for merging, @ssf0409. Switching to Keeping the pipe and newline handling separate turned out to matter too, because those are Markdown problems rather than HTML ones. Appreciate you taking the time to review it. |
Summary
_md_cell()helper for Markdown table cells<[Unreleased]Validation
The added tests cover task IDs containing
|, raw HTML, and newlines, including both the per-task and baseline-gate tables.make verifyshould validate the full repository before merge.Closes #134