Repository navigation
fix(query-engine): stop stripping angle brackets from warehouse error SQL - #1212
Confidence 4/5 · No issues found
🟢 Confidence 4/5 · likely safe to merge
The new tag allowlist silently narrows what an error scrubber in shared warehouse code removes; regex behavior verified by running it.
quality 98/100 · 1 note · tests covered · risk medium
Narrows the HTML scrubber in cleanErrorMessage to a tag allowlist so echoed SQL with <=/>= survives, and drops the space in the HTML-page truncation check. The doctype and unquoted-attribute gap is fixed; the allowlist itself now misses most HTML tags.
HTML_TAGincleanErrorMessagereplaces the generic/<[^>]+>/gscrub- HTML-page truncation check no longer allows whitespace after
< - Tests added for UNION-branch SQL and for inline/doctype HTML
Findings
🔵 Note · F2 · HTML_TAG allowlist omits script, style, table and most other tags
correctness · packages/query-engine/src/execution/errors.ts:82
The alternation lists 18 tag names, so inline HTML that the page-start cut does not catch keeps its markup and its text: cleanErrorMessage("Bad gateway <table><tr><td>x</td></tr></table>") returns the tags unchanged, and <script>alert(1)</script> or <style>body{…}</style> survive whole, where the old /<[^>]+>/g removed them. Error messages still read as HTML for pages that do not begin with <html>/<!doctype>/<head>.
Extend the tag alternation with the remaining common tags (`script`, `style`, `table`, `tbody`, `thead`, `tr`, `td`, `th`, `ul`, `ol`, `li`, `img`, `link`, `form`, `input`, `iframe`, `svg`, `nav`, `footer`, `section`, `article`), keeping the attribute group that preserves tag-like SQL.
Fixed since the last review
- ✅
F1 ·HTML_TAGleaves<!DOCTYPE html>and unquoted-attribute tags in cleaned errors
What was checked
- Ran the new regex on
<!DOCTYPE html>,<!DOCTYPE html PUBLIC "-//W3C//DTD…">and<p class=x>: all stripped, so F1 is gone - Ran
a < 5 AND b > 3,x<b AND y>3and three>=/<=branches: none mangled - Leftover HTML reaches React as an escaped text node; no
dangerouslySetInnerHTMLpath renders error messages
ada991c · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
Annotations
Check notice on line 82 in packages/query-engine/src/execution/errors.ts
maple-review-bot / Maple / review
correctness: `HTML_TAG` allowlist omits `script`, `style`, `table` and most other tags
The alternation lists 18 tag names, so inline HTML that the page-start cut does not catch keeps its markup and its text: `cleanErrorMessage("Bad gateway <table><tr><td>x</td></tr></table>")` returns the tags unchanged, and `<script>alert(1)</script>` or `<style>body{…}</style>` survive whole, where the old `/<[^>]+>/g` removed them. Error messages still read as HTML for pages that do not begin with `<html>`/`<!doctype>`/`<head>`.