From 936609a7cd4b228fe0a66a6fdcc0d50f4ddf1ad8 Mon Sep 17 00:00:00 2001 From: equationalapplications Date: Thu, 25 Jun 2026 19:13:29 -0400 Subject: [PATCH 1/2] fix(grounding): harden native sanitizer and address review nits Strip base tags and unsafe img src on native, clarify attribute allowlist logic, and stop re-attaching the web wheel listener on layout sync. Co-authored-by: Cursor --- src/components/GroundingHtml.web.tsx | 2 +- .../__tests__/sanitizeGroundingHtml.test.ts | 38 +++++++++++++++++ src/utils/groundingShadowContent.ts | 7 +++- src/utils/sanitizeGroundingHtml.ts | 42 ++++++++++++------- 4 files changed, 70 insertions(+), 19 deletions(-) diff --git a/src/components/GroundingHtml.web.tsx b/src/components/GroundingHtml.web.tsx index f5e08240..1cce651c 100644 --- a/src/components/GroundingHtml.web.tsx +++ b/src/components/GroundingHtml.web.tsx @@ -114,7 +114,7 @@ export function GroundingHtml({ html, style }: GroundingHtmlProps) { scrollEl.addEventListener('wheel', onWheel, { capture: true, passive: false }) return () => scrollEl.removeEventListener('wheel', onWheel, { capture: true }) - }, [contentWidth]) + }, []) const captureBubblePress = useCallback((event: SyntheticEvent) => { stopBubbleScrollCapture(event) diff --git a/src/utils/__tests__/sanitizeGroundingHtml.test.ts b/src/utils/__tests__/sanitizeGroundingHtml.test.ts index ec5a7e70..614264ad 100644 --- a/src/utils/__tests__/sanitizeGroundingHtml.test.ts +++ b/src/utils/__tests__/sanitizeGroundingHtml.test.ts @@ -29,6 +29,18 @@ describe('stripExecutableGroundingMarkup', () => { const html = 'ipt>alert(1)ipt>' expect(stripExecutableGroundingMarkup(html)).toBe('') }) + + it('removes embed-capable elements', () => { + const html = + '
ok
' + expect(stripExecutableGroundingMarkup(html)).toBe('
ok
') + }) + + it('removes resource-loading head tags', () => { + const html = + '
ok
' + expect(stripExecutableGroundingMarkup(html)).toBe('
ok
') + }) }) describe('sanitizeGroundingHtmlLinksRegex', () => { @@ -41,6 +53,18 @@ describe('sanitizeGroundingHtmlLinksRegex', () => { const html = 'good' expect(sanitizeGroundingHtmlLinksRegex(html)).toBe('good') }) + + it('removes unsafe img src values', () => { + const html = 'track' + expect(sanitizeGroundingHtmlLinksRegex(html)).toBe('track') + }) + + it('keeps safe http(s) img src values', () => { + const html = 'icon' + expect(sanitizeGroundingHtmlLinksRegex(html)).toBe( + 'icon', + ) + }) }) describe('sanitizeGroundingHtmlForNative', () => { @@ -49,4 +73,18 @@ describe('sanitizeGroundingHtmlForNative', () => { 'x' expect(sanitizeGroundingHtmlForNative(html)).toBe('x') }) + + it('removes embed-capable elements and external stylesheet links', () => { + const html = + '
Suggestion
' + expect(sanitizeGroundingHtmlForNative(html)).toBe('
Suggestion
') + }) + + it('strips base tags and unsafe img src while preserving safe markup', () => { + const html = + 'xicongo' + expect(sanitizeGroundingHtmlForNative(html)).toBe( + 'xicongo', + ) + }) }) diff --git a/src/utils/groundingShadowContent.ts b/src/utils/groundingShadowContent.ts index 7e9bcbb4..983bd659 100644 --- a/src/utils/groundingShadowContent.ts +++ b/src/utils/groundingShadowContent.ts @@ -73,8 +73,11 @@ const GROUNDING_URL_ATTRS = new Set(['href', 'src']) function isAllowedGroundingAttribute(tagName: string, attributeName: string): boolean { const lowerName = attributeName.toLowerCase() - if (lowerName.startsWith('on') || lowerName.startsWith('data-') || lowerName.startsWith('aria-')) { - return lowerName.startsWith('aria-') || lowerName.startsWith('data-') + if (lowerName.startsWith('on')) { + return false + } + if (lowerName.startsWith('aria-') || lowerName.startsWith('data-')) { + return true } if (GROUNDING_GLOBAL_ALLOWED_ATTRS.has(lowerName)) { return true diff --git a/src/utils/sanitizeGroundingHtml.ts b/src/utils/sanitizeGroundingHtml.ts index f5a41d79..62867b3f 100644 --- a/src/utils/sanitizeGroundingHtml.ts +++ b/src/utils/sanitizeGroundingHtml.ts @@ -4,7 +4,9 @@ const SCRIPT_TAG_RE = /]*>)<[^<]*)*<\/script[^>] const SCRIPT_SELF_CLOSING_RE = /]*\/?>/gi const INLINE_EVENT_HANDLER_RE = /\s+on[a-z]+\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi const ANCHOR_TAG_RE = /]*)>/gi +const IMG_TAG_RE = /]*)>/gi const HREF_ATTR_RE = /\bhref\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/i +const SRC_ATTR_RE = /\bsrc\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/i /** Repeat replace until stable — avoids CodeQL incomplete-sanitization bypasses. */ function replaceUntilStable(html: string, pattern: RegExp): string { @@ -27,26 +29,34 @@ export function stripExecutableGroundingMarkup(html: string): string { result, /<(?:iframe|object|embed|frame|frameset)\b[^>]*>(?:[\s\S]*?<\/(?:iframe|object|embed|frame|frameset)\s*>)?/gi, ) - result = replaceUntilStable(result, /<(?:link|meta)\b[^>]*>/gi) + result = replaceUntilStable(result, /<(?:link|meta|base)\b[^>]*>/gi) return result } -/** Strip unsafe hrefs from anchor tags (native path; web uses DOMParser). */ +function stripUnsafeUrlAttribute( + tagName: 'a' | 'img', + attrs: string, + urlAttrRe: RegExp, +): string { + const urlMatch = attrs.match(urlAttrRe) + if (!urlMatch) { + return `<${tagName}${attrs}>` + } + + const url = urlMatch[1] ?? urlMatch[2] ?? urlMatch[3] ?? '' + if (!isSafeHttpUrl(url)) { + const cleanedAttrs = attrs.replace(urlAttrRe, '').trim() + return cleanedAttrs ? `<${tagName} ${cleanedAttrs}>` : `<${tagName}>` + } + + return `<${tagName}${attrs}>` +} + +/** Strip unsafe hrefs and img src values (native path; web uses DOMParser). */ export function sanitizeGroundingHtmlLinksRegex(html: string): string { - return html.replace(ANCHOR_TAG_RE, (_, attrs: string) => { - const hrefMatch = attrs.match(HREF_ATTR_RE) - if (!hrefMatch) { - return `` - } - - const href = hrefMatch[1] ?? hrefMatch[2] ?? hrefMatch[3] ?? '' - if (!isSafeHttpUrl(href)) { - const cleanedAttrs = attrs.replace(HREF_ATTR_RE, '').trim() - return cleanedAttrs ? `` : '' - } - - return `` - }) + return html + .replace(ANCHOR_TAG_RE, (_, attrs: string) => stripUnsafeUrlAttribute('a', attrs, HREF_ATTR_RE)) + .replace(IMG_TAG_RE, (_, attrs: string) => stripUnsafeUrlAttribute('img', attrs, SRC_ATTR_RE)) } /** Defense-in-depth sanitization before rendering grounding HTML on native WebViews. */ From 4c0e20719e19f9e9c4a204d09de68c6c84680865 Mon Sep 17 00:00:00 2001 From: equationalapplications Date: Thu, 25 Jun 2026 19:23:06 -0400 Subject: [PATCH 2/2] fix(grounding): harden URL attribute regex and duplicate handling Anchor href/src patterns now require start-of-attrs or whitespace (avoiding data-href/data-src false positives), support xlink:href, and use last-wins semantics when duplicate URL attributes are present. Co-authored-by: Cursor --- .../__tests__/sanitizeGroundingHtml.test.ts | 37 ++++++++++++++++ src/utils/sanitizeGroundingHtml.ts | 43 +++++++++++++++---- 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/src/utils/__tests__/sanitizeGroundingHtml.test.ts b/src/utils/__tests__/sanitizeGroundingHtml.test.ts index 614264ad..a448c6e7 100644 --- a/src/utils/__tests__/sanitizeGroundingHtml.test.ts +++ b/src/utils/__tests__/sanitizeGroundingHtml.test.ts @@ -65,6 +65,43 @@ describe('sanitizeGroundingHtmlLinksRegex', () => { 'icon', ) }) + + it('does not match data-href or data-src attribute names', () => { + const html = + 'xy' + expect(sanitizeGroundingHtmlLinksRegex(html)).toBe( + 'xy', + ) + }) + + it('removes href when the last duplicate is unsafe', () => { + const html = 'bad' + expect(sanitizeGroundingHtmlLinksRegex(html)).toBe('bad') + }) + + it('keeps href when the last duplicate is safe', () => { + const html = 'good' + expect(sanitizeGroundingHtmlLinksRegex(html)).toBe( + 'good', + ) + }) + + it('removes src when the last duplicate is unsafe', () => { + const html = 'x' + expect(sanitizeGroundingHtmlLinksRegex(html)).toBe('x') + }) + + it('keeps src when the last duplicate is safe on self-closing img', () => { + const html = 'x' + expect(sanitizeGroundingHtmlLinksRegex(html)).toBe( + 'x', + ) + }) + + it('sanitizes xlink:href on anchor tags', () => { + const html = 'bad' + expect(sanitizeGroundingHtmlLinksRegex(html)).toBe('bad') + }) }) describe('sanitizeGroundingHtmlForNative', () => { diff --git a/src/utils/sanitizeGroundingHtml.ts b/src/utils/sanitizeGroundingHtml.ts index 62867b3f..ee351102 100644 --- a/src/utils/sanitizeGroundingHtml.ts +++ b/src/utils/sanitizeGroundingHtml.ts @@ -5,8 +5,9 @@ const SCRIPT_SELF_CLOSING_RE = /]*\/?>/gi const INLINE_EVENT_HANDLER_RE = /\s+on[a-z]+\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi const ANCHOR_TAG_RE = /]*)>/gi const IMG_TAG_RE = /]*)>/gi -const HREF_ATTR_RE = /\bhref\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/i -const SRC_ATTR_RE = /\bsrc\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/i +const HREF_ATTR_RE = + /(?:^|\s)(?:xlink:)?href\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/gi +const SRC_ATTR_RE = /(?:^|\s)src\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/gi /** Repeat replace until stable — avoids CodeQL incomplete-sanitization bypasses. */ function replaceUntilStable(html: string, pattern: RegExp): string { @@ -33,23 +34,49 @@ export function stripExecutableGroundingMarkup(html: string): string { return result } +function splitSelfClosing(attrs: string): { body: string; selfClosing: boolean } { + const trimmed = attrs.trim() + const selfClosingMatch = trimmed.match(/^(.*?)\s*\/\s*$/) + if (selfClosingMatch) { + return { body: selfClosingMatch[1].trim(), selfClosing: true } + } + return { body: trimmed, selfClosing: false } +} + +function formatTag(tagName: 'a' | 'img', attrs: string, selfClosing = false): string { + const trimmed = attrs.trim() + if (!trimmed) { + return selfClosing ? `<${tagName} />` : `<${tagName}>` + } + return selfClosing ? `<${tagName} ${trimmed} />` : `<${tagName} ${trimmed}>` +} + function stripUnsafeUrlAttribute( tagName: 'a' | 'img', attrs: string, urlAttrRe: RegExp, ): string { - const urlMatch = attrs.match(urlAttrRe) - if (!urlMatch) { + const matches = [...attrs.matchAll(urlAttrRe)] + if (matches.length === 0) { return `<${tagName}${attrs}>` } - const url = urlMatch[1] ?? urlMatch[2] ?? urlMatch[3] ?? '' + const lastMatch = matches[matches.length - 1] + const url = lastMatch[1] ?? lastMatch[2] ?? lastMatch[3] ?? '' + const strippedAttrs = attrs.replace(urlAttrRe, '').replace(/\s{2,}/g, ' ').trim() + const { body, selfClosing } = splitSelfClosing(strippedAttrs) + if (!isSafeHttpUrl(url)) { - const cleanedAttrs = attrs.replace(urlAttrRe, '').trim() - return cleanedAttrs ? `<${tagName} ${cleanedAttrs}>` : `<${tagName}>` + return formatTag(tagName, body, selfClosing) + } + + if (matches.length === 1) { + return `<${tagName}${attrs}>` } - return `<${tagName}${attrs}>` + const safeAttr = lastMatch[0].trim() + const combinedBody = body ? `${body} ${safeAttr}` : safeAttr + return formatTag(tagName, combinedBody, selfClosing) } /** Strip unsafe hrefs and img src values (native path; web uses DOMParser). */