From 9337af55d93d4bd9e7b154fe8c66325946c1803e Mon Sep 17 00:00:00 2001 From: mday-io Date: Mon, 26 Oct 2026 17:10:37 +0000 Subject: [PATCH 1/3] Fix(web): render search highlights as text nodes to prevent XSS 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 --- .../library/components/search/SearchList.tsx | 47 +++++++++++++---- .../src/library/components/search/help.ts | 25 ++++++++-- .../search/tests/SearchList.spec.tsx | 50 +++++++++++++++++++ 3 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 web/client/src/library/components/search/tests/SearchList.spec.tsx diff --git a/web/client/src/library/components/search/SearchList.tsx b/web/client/src/library/components/search/SearchList.tsx index fae23f2303..1621543846 100644 --- a/web/client/src/library/components/search/SearchList.tsx +++ b/web/client/src/library/components/search/SearchList.tsx @@ -281,6 +281,31 @@ export default function SearchList< ) } +export function HighlightedText({ + source, + search, +}: { + source: string + search: string +}): JSX.Element { + return ( + <> + {highlightMatch(source, search).map(({ text, isMatch }, idx) => + isMatch ? ( + + {text} + + ) : ( + {text} + ), + )} + + ) +} + function SearchResult = Record>({ item, index, @@ -322,21 +347,23 @@ function SearchResult = Record>({ )} - + + + ) : ( + > + + )} {isNotNil(descriptionBy) && ( diff --git a/web/client/src/library/components/search/help.ts b/web/client/src/library/components/search/help.ts index 5292676475..157377b3a2 100644 --- a/web/client/src/library/components/search/help.ts +++ b/web/client/src/library/components/search/help.ts @@ -3,11 +3,26 @@ export { filterListBy, highlightMatch } export const EMPTY_STRING = '' const MATCHED_LENGTH_TO_DISPLAY = 40 -function highlightMatch(source: string, match: string): string { - return source.replaceAll( - match, - `${match}`, - ) +export interface HighlightPart { + text: string + isMatch: boolean +} + +// NOTE: Returns plain text parts so callers can render them as React text nodes +function highlightMatch(source: string, match: string): HighlightPart[] { + if (match === EMPTY_STRING) return [{ text: source, isMatch: false }] + + return source + .split(match) + .flatMap((text, idx) => + idx === 0 + ? [{ text, isMatch: false }] + : [ + { text: match, isMatch: true }, + { text, isMatch: false }, + ], + ) + .filter(part => part.text !== EMPTY_STRING) } // NOTE: This function is used to filter the list of models in the search bar diff --git a/web/client/src/library/components/search/tests/SearchList.spec.tsx b/web/client/src/library/components/search/tests/SearchList.spec.tsx new file mode 100644 index 0000000000..e3d94206bd --- /dev/null +++ b/web/client/src/library/components/search/tests/SearchList.spec.tsx @@ -0,0 +1,50 @@ +import { describe, it, expect } from 'vitest' +import { render } from '../../../../tests/utils' +import { HighlightedText } from '../SearchList' +import { highlightMatch } from '../help' + +describe('highlightMatch', () => { + it('should split source into matched and unmatched parts', () => { + expect(highlightMatch('abcab', 'ab')).toEqual([ + { text: 'ab', isMatch: true }, + { text: 'c', isMatch: false }, + { text: 'ab', isMatch: true }, + ]) + }) + + it('should return the whole source when there is no match', () => { + expect(highlightMatch('abc', '')).toEqual([{ text: 'abc', isMatch: false }]) + expect(highlightMatch('abc', 'x')).toEqual([ + { text: 'abc', isMatch: false }, + ]) + }) +}) + +describe('HighlightedText', () => { + it('should render project text as text, not HTML', () => { + const source = 'model' + const { container } = render( + , + ) + + expect(container.querySelector('img')).toBeNull() + expect(container.textContent).toBe(source) + expect(container.querySelector('b')?.textContent).toBe('model') + }) + + it('should not interpret markup in the search string', () => { + const search = '' + const { container } = render( + , + ) + + expect(container.querySelector('img')).toBeNull() + expect(container.querySelector('b')?.textContent).toBe(search) + }) +}) From 34ddb9d6e2a25cebebb73d4b076c6395cb7ee6d1 Mon Sep 17 00:00:00 2001 From: mday-io Date: Mon, 26 Oct 2026 17:29:11 +0000 Subject: [PATCH 2/3] Test(web): cover edge cases in search highlightMatch Signed-off-by: mday-io --- .../components/search/tests/SearchList.spec.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/web/client/src/library/components/search/tests/SearchList.spec.tsx b/web/client/src/library/components/search/tests/SearchList.spec.tsx index e3d94206bd..a8ecb08627 100644 --- a/web/client/src/library/components/search/tests/SearchList.spec.tsx +++ b/web/client/src/library/components/search/tests/SearchList.spec.tsx @@ -12,6 +12,21 @@ describe('highlightMatch', () => { ]) }) + it('should handle adjacent matches and matches at the edges', () => { + expect(highlightMatch('abab', 'ab')).toEqual([ + { text: 'ab', isMatch: true }, + { text: 'ab', isMatch: true }, + ]) + expect(highlightMatch('xab', 'ab')).toEqual([ + { text: 'x', isMatch: false }, + { text: 'ab', isMatch: true }, + ]) + }) + + it('should return no parts for an empty source', () => { + expect(highlightMatch('', 'ab')).toEqual([]) + }) + it('should return the whole source when there is no match', () => { expect(highlightMatch('abc', '')).toEqual([{ text: 'abc', isMatch: false }]) expect(highlightMatch('abc', 'x')).toEqual([ From b662e5d81547a96cddf00443636f646146ef5aa2 Mon Sep 17 00:00:00 2001 From: mday-io Date: Mon, 26 Oct 2026 19:34:40 +0000 Subject: [PATCH 3/3] Test(web): test search highlighting at the SearchList boundary 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 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 --- .../library/components/search/SearchList.tsx | 2 +- .../search/tests/SearchList.spec.tsx | 105 +++++++++--------- 2 files changed, 54 insertions(+), 53 deletions(-) diff --git a/web/client/src/library/components/search/SearchList.tsx b/web/client/src/library/components/search/SearchList.tsx index 1621543846..865270297f 100644 --- a/web/client/src/library/components/search/SearchList.tsx +++ b/web/client/src/library/components/search/SearchList.tsx @@ -281,7 +281,7 @@ export default function SearchList< ) } -export function HighlightedText({ +function HighlightedText({ source, search, }: { diff --git a/web/client/src/library/components/search/tests/SearchList.spec.tsx b/web/client/src/library/components/search/tests/SearchList.spec.tsx index a8ecb08627..9eda3db102 100644 --- a/web/client/src/library/components/search/tests/SearchList.spec.tsx +++ b/web/client/src/library/components/search/tests/SearchList.spec.tsx @@ -1,65 +1,66 @@ -import { describe, it, expect } from 'vitest' -import { render } from '../../../../tests/utils' -import { HighlightedText } from '../SearchList' -import { highlightMatch } from '../help' +import { describe, it, expect, vi } from 'vitest' +import { MemoryRouter } from 'react-router' +import { render, fireEvent } from '../../../../tests/utils' +import SearchList from '../SearchList' -describe('highlightMatch', () => { - it('should split source into matched and unmatched parts', () => { - expect(highlightMatch('abcab', 'ab')).toEqual([ - { text: 'ab', isMatch: true }, - { text: 'c', isMatch: false }, - { text: 'ab', isMatch: true }, - ]) - }) +// jsdom has no ResizeObserver, which TBKResizeObserver needs to mount +vi.stubGlobal( + 'ResizeObserver', + class { + observe(): void {} + unobserve(): void {} + disconnect(): void {} + }, +) - it('should handle adjacent matches and matches at the edges', () => { - expect(highlightMatch('abab', 'ab')).toEqual([ - { text: 'ab', isMatch: true }, - { text: 'ab', isMatch: true }, - ]) - expect(highlightMatch('xab', 'ab')).toEqual([ - { text: 'x', isMatch: false }, - { text: 'ab', isMatch: true }, - ]) - }) +const PAYLOAD = '' + +function renderSearch( + name: string, + search: string, + showIndex: boolean, +): HTMLElement { + const { container } = render( + + + list={[{ name }]} + searchBy="name" + displayBy="name" + showIndex={showIndex} + /> + , + ) - it('should return no parts for an empty source', () => { - expect(highlightMatch('', 'ab')).toEqual([]) + fireEvent.input(container.querySelector('input')!, { + target: { value: search }, }) - it('should return the whole source when there is no match', () => { - expect(highlightMatch('abc', '')).toEqual([{ text: 'abc', isMatch: false }]) - expect(highlightMatch('abc', 'x')).toEqual([ - { text: 'abc', isMatch: false }, - ]) + return container.querySelector('[role="menuitem"]')! +} + +describe.each([true, false])('SearchList with showIndex=%s', showIndex => { + it('should render markup in project text as text', () => { + const name = `${PAYLOAD}model` + const result = renderSearch(name, 'model', showIndex) + + expect(result.querySelector('img')).toBeNull() + expect(result.textContent).toContain(name) + expect(result.querySelector('b')?.textContent).toBe('model') }) -}) -describe('HighlightedText', () => { - it('should render project text as text, not HTML', () => { - const source = 'model' - const { container } = render( - , - ) + it('should render markup in the search string as text', () => { + const result = renderSearch(`a${PAYLOAD}b`, PAYLOAD, showIndex) - expect(container.querySelector('img')).toBeNull() - expect(container.textContent).toBe(source) - expect(container.querySelector('b')?.textContent).toBe('model') + expect(result.querySelector('img')).toBeNull() + expect(result.querySelector('b')?.textContent).toBe(PAYLOAD) }) - it('should not interpret markup in the search string', () => { - const search = '' - const { container } = render( - , - ) + it('should highlight every match, including adjacent and trailing ones', () => { + const result = renderSearch('ababxab', 'ab', showIndex) - expect(container.querySelector('img')).toBeNull() - expect(container.querySelector('b')?.textContent).toBe(search) + expect( + Array.from(result.querySelectorAll('b'), b => b.textContent), + ).toEqual(['ab', 'ab', 'ab']) + expect(result.textContent).toContain('ababxab') }) })