Fix(web): render search highlights as text nodes to prevent XSS by mday-io · Pull Request #6163 · SQLMesh/sqlmesh

mday-io · 2026-10-26T19:31:26Z

Description

SearchResult in web/client/src/library/components/search/SearchList.tsx rendered search results with dangerouslySetInnerHTML, using the output of highlightMatch. That function wrapped matches in  tags but never escaped the surrounding project text. A filename or model description containing markup (e.g. '<img src=x onerror=...>') could therefore execute script in the UI origin when someone searched for nearby text (CWE-79).

Changes:

highlightMatch (search/help.ts) now returns plain { text, isMatch } parts instead of an HTML string. An empty search returns the source unchanged; previously replaceAll('', ...) inserted  between every character.
A new internal HighlightedText component renders those parts as React text nodes, wrapping matches in  with the same classes as before.
Both dangerouslySetInnerHTML assignments in SearchResult are removed. No other dangerouslySetInnerHTML remains in web/client/src.

Highlighting stays case-sensitive, matching the previous behavior.

Test Plan

Added web/client/src/library/components/search/tests/SearchList.spec.tsx. It renders SearchList, types a search, and inspects the results, for both display branches (showIndex true and false):
- an <img onerror=...> payload in the project text is rendered as text, with only the match highlighted;
- the same payload as the search string is rendered as text;
- adjacent and trailing matches are all highlighted.
Against the pre-fix code, the four XSS tests fail because a real <img> element is created. The highlighting test passes on both versions, confirming parity.
NODE_ENV=testing npx vitest --watch=false in web/client: 11 files, 78 tests passed.
ESLint and Prettier are clean on the changed files; tsc reports no errors in the search directory.

Checklist

I have run make style and fixed any issues (ran ESLint and Prettier on the changed web files)
I have added tests for my changes (if applicable)
All existing tests pass (make fast-test) (ran the web client unit tests; no Python code changed)
My commits are signed off (git commit -s) per the DCO

Summary by CodeRabbit

Bug Fixes
- Search results now highlight matching text while displaying surrounding text as entered, including text containing HTML-like characters.
- Search result names and indexes no longer turn HTML-like text into page elements.
- Adjacent and trailing matches are highlighted consistently, and searches with no match display the original text without highlighting.

SearchResult inserted project text (file names, model descriptions) into the DOM via dangerouslySetInnerHTML after wrapping matches in tags, so markup in that text could execute in the UI origin. highlightMatch now returns plain text parts, and a HighlightedText component renders them as React text nodes with matches wrapped in elements. Both dangerouslySetInnerHTML assignments are removed. Signed-off-by: mday-io <mdaytn@gmail.com>

Signed-off-by: mday-io <mdaytn@gmail.com>

coderabbitai · 2026-10-26T19:31:43Z

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: f4e566cd-b1ca-4adf-9424-e0ed390abfcf

Commits

Reviewing files that changed from the base of the PR and between 34ddb9d and b662e5d.

Files selected for processing: 2

web/client/src/library/components/search/SearchList.tsx
web/client/src/library/components/search/tests/SearchList.spec.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

Search highlighting now returns matched and unmatched text parts. Search results render the parts as React elements instead of inserting highlighted HTML.

Changes: Search highlighting

Layer / File(s)	Summary
Highlight text segmentation `web/client/src/library/components/search/help.ts`	`highlightMatch` returns text parts with match status. Empty matches return the source as unmatched text, and empty parts are omitted.
React search result rendering `web/client/src/library/components/search/SearchList.tsx`, `web/client/src/library/components/search/tests/SearchList.spec.tsx`	`HighlightedText` renders matched text in bold and unmatched text as text. `SearchResult` uses it for the index and displayed name. Tests check markup-like input and multiple, adjacent, and trailing matches.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Minimal · up to b662e

The XSS-prone HTML rendering was removed and the changed search behavior is covered by focused tests.

Security architecture risk: Minimal · up to b662e

The change reduces script-injection exposure by rendering search content as text rather than HTML. The compared changes introduce no new service access, privileges, or persistent state, and no material security concern remains in the reviewed surface.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The directly affected security boundary is search-result rendering in the web client. The full comparison adds no backend, identity, infrastructure, deployment, or persistence changes; it does not establish exposure across particular tenants or projects.

Security Findings and Attack Paths

observed — The base allowed markup-bearing source or query text to reach dangerouslySetInnerHTML through highlightMatch. The head removes both such assignments and renders their inputs as text, closing that interpretation path rather than expanding its reachability.

Trust Boundaries and Controls

observed — The control is structural separation of content from markup: input supplies text values, while the renderer supplies fixed bold elements and fragments. Added tests assert that markup-like source and query strings do not create image elements in either display branch.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the primary change: preventing XSS by rendering search highlights as text nodes.
Description check		The description is complete and directly matches the changes. It explains the XSS issue, implementation, behavior, tests, validation results, and checklist status.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

Exercise both SearchList display branches instead of the HighlightedText helper, so the XSS tests fail on the pre-fix code for the intended reason (a real <img> element is created) and HighlightedText no longer needs to be exported only for tests. Drop the highlightMatch unit tests, which asserted the exact parts array rather than rendered output. Signed-off-by: mday-io <mdaytn@gmail.com>

cmgoffena13 · 2026-10-26T21:00:12Z

@@ -0,0 +1,66 @@
+import { describe, it, expect, vi } from 'vitest'


Need the # SPDX-License-Identifier: Apache-2.0 header on new files

cmgoffena13

I don't really know typescript / javascript, but I can rubber stamp this. Need the header for the new file, could be something to have the AI check for, I've noticed people keep forgetting

mday-io added 2 commits October 26, 2026 19:30

Test(web): cover edge cases in search highlightMatch

34ddb9d

Signed-off-by: mday-io <mdaytn@gmail.com>

cmgoffena13 reviewed Oct 26, 2026

View reviewed changes

Navigation Menu

Repository navigation

Fix(web): render search highlights as text nodes to prevent XSS - #6163

Fix(web): render search highlights as text nodes to prevent XSS#6163
mday-io wants to merge 3 commits into
SQLMesh:mainfrom
mday-io:mday/cool-sagan-iqtmgi

mday-io commented Oct 26, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

cmgoffena13 Oct 26, 2026

Uh oh!

cmgoffena13 left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,66 @@
		import { describe, it, expect, vi } from 'vitest'

Conversation

mday-io commented Oct 26, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Plan

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmgoffena13 Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

cmgoffena13 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mday-io commented Oct 26, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading