From c05dd2726b6dba86a75aefda157ffdd83c3c427b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 00:43:41 +0000 Subject: [PATCH] fix(fields): sanitize html and richtext values with rehype-sanitize Replace the hand-written pattern filter in the html/richtext display pipeline with the rehype-sanitize allow-list the markdown pipeline already uses. The stored value is parsed with DOMParser, converted to hast, sanitized against one shared schema and rendered as React elements, so the renderer no longer uses dangerouslySetInnerHTML. The markdown pipeline now reads the same schema. Adds structural tests for each class of stored markup and a byte-for-byte round-trip of a formatted document, and updates the rich-text field docs to describe the allow-list. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01Fw91y6wN8SAu43vfrsAqHg --- .changeset/rich-text-display-sanitizer.md | 7 + content/docs/fields/rich-text.mdx | 12 +- packages/fields/package.json | 2 + ...Renderers.childlessContainer-8580.test.tsx | 12 +- .../readonly-richtext-display-5498.test.tsx | 3 +- .../richtext-cell-renderer-5452.test.tsx | 3 +- .../fields/src/widgets/MarkdownContent.tsx | 6 +- packages/fields/src/widgets/RichTextField.tsx | 2 +- .../widgets/__tests__/sanitizedHtml.test.tsx | 204 ++++++++++++++++++ .../fields/src/widgets/richTextDisplay.tsx | 52 ++--- packages/fields/src/widgets/sanitizedHtml.tsx | 161 ++++++++++++++ pnpm-lock.yaml | 6 + 12 files changed, 422 insertions(+), 48 deletions(-) create mode 100644 .changeset/rich-text-display-sanitizer.md create mode 100644 packages/fields/src/widgets/__tests__/sanitizedHtml.test.tsx create mode 100644 packages/fields/src/widgets/sanitizedHtml.tsx diff --git a/.changeset/rich-text-display-sanitizer.md b/.changeset/rich-text-display-sanitizer.md new file mode 100644 index 0000000000..2cd49abac8 --- /dev/null +++ b/.changeset/rich-text-display-sanitizer.md @@ -0,0 +1,7 @@ +--- +'@object-ui/fields': patch +--- + +`html` and `richtext` values are now sanitized with `rehype-sanitize`, the sanitizer the `markdown` pipeline already used, instead of a hand-written pattern filter. + +A stored value is parsed with the browser's HTML parser, filtered against one allow-list of elements, attributes and URL protocols, and rendered as React elements. Both display pipelines share that allow-list. Headings, paragraphs, emphasis, lists, quotes, code, tables, links and images keep their formatting. Event-handler attributes, `style`, script-bearing URLs, embedded frames and objects, and SVG and MathML content are dropped. A link whose URL is refused now renders without an `href`, where it used to point at `#`. Stored `id` and `name` values are prefixed with `user-content-`. diff --git a/content/docs/fields/rich-text.mdx b/content/docs/fields/rich-text.mdx index 7dcb83e48c..95a90f7a20 100644 --- a/content/docs/fields/rich-text.mdx +++ b/content/docs/fields/rich-text.mdx @@ -94,10 +94,14 @@ metadata keys — they are runtime widget props. See [Field Widget Props](/docs/ - **Tables**: Markdown tables ### HTML -- Full HTML editing support -- Sanitization for security -- Style preservation -- Embedded media support +- Headings, paragraphs, emphasis, lists, quotes, code, tables, links and images +- Sanitized on display against an allow-list (`rehype-sanitize`, GitHub-style): + links keep `http`, `https`, `mailto` and relative URLs, and images keep `http`, + `https` and relative URLs +- Inline `style`, event-handler attributes, embedded frames, objects and SVG or + MathML content are not rendered; an element outside the allow-list shows its text + without its tag +- The `markdown` and `html` / `richtext` display pipelines share the same allow-list ## Cell Renderer diff --git a/packages/fields/package.json b/packages/fields/package.json index 6102134c98..ebb960550f 100644 --- a/packages/fields/package.json +++ b/packages/fields/package.json @@ -38,6 +38,8 @@ "@object-ui/react": "workspace:*", "@object-ui/types": "workspace:*", "@objectstack/spec": "^18.0.0-next.1", + "hast-util-to-jsx-runtime": "^2.3.6", + "hastscript": "^9.0.1", "lucide-react": "^1.43.0", "react-markdown": "^10.1.0", "rehype-sanitize": "^6.0.0", diff --git a/packages/fields/src/__tests__/cellRenderers.childlessContainer-8580.test.tsx b/packages/fields/src/__tests__/cellRenderers.childlessContainer-8580.test.tsx index d27550a519..bed04d66b2 100644 --- a/packages/fields/src/__tests__/cellRenderers.childlessContainer-8580.test.tsx +++ b/packages/fields/src/__tests__/cellRenderers.childlessContainer-8580.test.tsx @@ -51,7 +51,7 @@ * case in this file. What refuses it is the POPULATED block: real markdown * still renders its elements, real HTML still renders SANITISED — pinned to * the sanitiser's exact output bytes, so a repair that shortcut the populated - * branch past `sanitizeHtml` fails on bytes, not on a count — and real + * branch past the sanitizer fails on bytes, not on a count — and real * richtext still renders. Each was RUN against the caricature; the observed * failure sentences are recorded in the PR. * @@ -127,8 +127,8 @@ const HOSTILE_HTML = '' + 'go'; -/** What `sanitizeHtml` makes of {@link HOSTILE_HTML} — the bytes, not a count. */ -const HOSTILE_HTML_SANITISED = '

ok

go'; +/** What the sanitizer makes of {@link HOSTILE_HTML} — the bytes, not a count. */ +const HOSTILE_HTML_SANITISED = '

ok

go'; /** What a WYSIWYG editor actually emits, beyond the seed's two tags. */ const WYSIWYG_DOCUMENT = [ @@ -241,10 +241,10 @@ describe('objectui#8580 — a rich-content cell holding [] or {} says what it ho const prose = container.querySelector('div.prose'); expect(prose, 'html: a populated value must still open its prose container').not.toBeNull(); - // The bytes, not a count: this is what `sanitizeHtml` produces for the + // The bytes, not a count: this is what the sanitizer produces for the // hostile document, so a populated branch that stopped running through // it — or a sanitiser that changed — fails here on content. - expect(prose?.innerHTML, 'html: the populated branch must still run through sanitizeHtml').toBe(HOSTILE_HTML_SANITISED); + expect(prose?.innerHTML, 'html: the populated branch must still run through the sanitizer').toBe(HOSTILE_HTML_SANITISED); expect(prose?.querySelector('script'), 'html: script blocks must still be stripped').toBeNull(); expect(affordance(container), 'html: a populated value must NOT render the affordance').toBeNull(); }); @@ -254,7 +254,7 @@ describe('objectui#8580 — a rich-content cell holding [] or {} says what it ho const prose = container.querySelector('div.prose'); expect(prose, 'richtext: a populated value must still open its prose container').not.toBeNull(); - expect(prose?.innerHTML, 'richtext: the populated branch must still run through sanitizeHtml').toBe(HOSTILE_HTML_SANITISED); + expect(prose?.innerHTML, 'richtext: the populated branch must still run through the sanitizer').toBe(HOSTILE_HTML_SANITISED); expect(affordance(container), 'richtext: a populated value must NOT render the affordance').toBeNull(); }); diff --git a/packages/fields/src/__tests__/readonly-richtext-display-5498.test.tsx b/packages/fields/src/__tests__/readonly-richtext-display-5498.test.tsx index 5b3501c42f..fb8b52cdc8 100644 --- a/packages/fields/src/__tests__/readonly-richtext-display-5498.test.tsx +++ b/packages/fields/src/__tests__/readonly-richtext-display-5498.test.tsx @@ -230,7 +230,8 @@ describe('negative controls — nothing else became a markup renderer (objectui# expect(prose.querySelector('script')).toBeNull(); expect(prose.innerHTML).not.toContain('alert(1)'); expect(prose.innerHTML).not.toContain('onerror'); - expect(prose.querySelector('a')?.getAttribute('href')).not.toContain('javascript:'); + // A dropped `href` reads `null`; a rewritten one must not name the scheme. + expect(prose.querySelector('a')?.getAttribute('href') ?? '').not.toContain('javascript:'); // …and the benign content around it survived, so the assertion above is // not passing because the whole value was dropped. expect(prose.textContent).toContain('ok'); diff --git a/packages/fields/src/__tests__/richtext-cell-renderer-5452.test.tsx b/packages/fields/src/__tests__/richtext-cell-renderer-5452.test.tsx index ac7b9a4662..4ce942bd7a 100644 --- a/packages/fields/src/__tests__/richtext-cell-renderer-5452.test.tsx +++ b/packages/fields/src/__tests__/richtext-cell-renderer-5452.test.tsx @@ -125,7 +125,8 @@ describe('rich-content cell renderers (objectui#5452)', () => { expect(prose.querySelector('script')).toBeNull(); expect(prose.innerHTML).not.toContain('onerror'); - expect(prose.querySelector('a')?.getAttribute('href')).not.toContain('javascript:'); + // A dropped `href` reads `null`; a rewritten one must not name the scheme. + expect(prose.querySelector('a')?.getAttribute('href') ?? '').not.toContain('javascript:'); expect(prose.textContent).toContain('ok'); }); diff --git a/packages/fields/src/widgets/MarkdownContent.tsx b/packages/fields/src/widgets/MarkdownContent.tsx index 5ca6ac1639..b07eaf2f67 100644 --- a/packages/fields/src/widgets/MarkdownContent.tsx +++ b/packages/fields/src/widgets/MarkdownContent.tsx @@ -2,16 +2,18 @@ import React from 'react'; import ReactMarkdown from 'react-markdown'; import remarkGfm from 'remark-gfm'; import rehypeSanitize from 'rehype-sanitize'; +import { RICH_TEXT_SANITIZE_SCHEMA } from './sanitizedHtml.js'; /** * Heavy markdown renderer, split into its own module so it can be lazy-loaded * by the cell/detail renderers (react-markdown pulls in ~100-200 KB). GitHub - * Flavored Markdown + sanitization (defense against XSS in stored content). + * Flavored Markdown + sanitization (defense against XSS in stored content), + * against the same schema the `html` / `richtext` pipeline uses. */ export default function MarkdownContent({ value }: { value: string }) { return (
- + {value}
diff --git a/packages/fields/src/widgets/RichTextField.tsx b/packages/fields/src/widgets/RichTextField.tsx index 1167247d39..597d3cb8ee 100644 --- a/packages/fields/src/widgets/RichTextField.tsx +++ b/packages/fields/src/widgets/RichTextField.tsx @@ -261,7 +261,7 @@ function resolveRichTextFieldType(field: unknown): string { * It now renders through {@link richTextCellRenderer} — literally the same * components `getCellRenderer` resolves, one table shared by both (see * `./richTextDisplay.tsx`), so the html/richtext path carries that renderer's - * `sanitizeHtml` trust boundary rather than a second hand-rolled escape, and a + * sanitizing trust boundary rather than a second hand-rolled escape, and a * future change to either pipeline moves both surfaces at once. * * ## Host plumbing (objectui#4810) diff --git a/packages/fields/src/widgets/__tests__/sanitizedHtml.test.tsx b/packages/fields/src/widgets/__tests__/sanitizedHtml.test.tsx new file mode 100644 index 0000000000..72abbf19ad --- /dev/null +++ b/packages/fields/src/widgets/__tests__/sanitizedHtml.test.tsx @@ -0,0 +1,204 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The `html` / `richtext` display pipeline's trust boundary + * (`../sanitizedHtml.tsx`), read through the renderer every read surface uses. + * + * Two halves, and both are load-bearing: + * + * - HOSTILE: each class of stored markup that can carry script comes out + * inert. The assertions are STRUCTURAL — no `on*` attribute anywhere, no + * script-bearing URL in any attribute, none of the embedding / foreign + * content elements, no `style` — read off the rendered DOM, so they hold + * for any spelling of a payload within its class, not just the one + * written here. + * - FORMATTING: a representative rich-text document renders back byte for + * byte. A sanitizer that drops everything passes the first half; this + * half is what refuses it. + */ + +import React from 'react'; +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { HtmlCellRenderer, RICH_TEXT_CELL_RENDERERS } from '../richTextDisplay'; +import { RICH_TEXT_SANITIZE_SCHEMA } from '../sanitizedHtml'; + +afterEach(cleanup); + +function renderHtml(value: string): HTMLElement { + const { container } = render( + , + ); + const prose = container.querySelector('div.prose'); + expect(prose, 'a populated value opens its prose container').not.toBeNull(); + return prose!; +} + +/** Elements whose presence alone means the value was not neutralised. */ +const FORBIDDEN_ELEMENTS = + 'script, style, iframe, frame, object, embed, applet, svg, math, link, meta, base, form, template, noscript'; + +/** Attributes that take a URL, in either namespace. */ +const URL_ATTRIBUTES = ['href', 'src', 'action', 'formaction', 'data', 'xlink:href', 'srcset', 'cite', 'poster', 'background']; + +/** Everything a hostile value must NOT have left in the DOM. */ +function expectInert(prose: HTMLElement, label: string) { + expect(prose.querySelector(FORBIDDEN_ELEMENTS), `${label}: no embedding or foreign-content element`).toBeNull(); + for (const element of Array.from(prose.querySelectorAll('*'))) { + for (const attribute of Array.from(element.attributes)) { + const name = attribute.name.toLowerCase(); + expect(name.startsWith('on'), `${label}: <${element.localName}> keeps no ${name}`).toBe(false); + expect(name, `${label}: <${element.localName}> keeps no style`).not.toBe('style'); + expect(name, `${label}: <${element.localName}> keeps no srcdoc`).not.toBe('srcdoc'); + if (URL_ATTRIBUTES.includes(name)) { + // Normalised the way a URL parser reads it: control characters and + // whitespace removed, case folded. + const url = Array.from(attribute.value) + .filter((char) => char.charCodeAt(0) > 0x20) + .join('') + .toLowerCase(); + expect(url, `${label}: <${element.localName} ${name}> is not a script URL`).not.toMatch( + /^(javascript|vbscript|data):/, + ); + } + } + } +} + +const HOSTILE: ReadonlyArray = [ + // Slash-separated attributes: no whitespace before the handler name. + ['slash-separated img handler', '

a

'], + ['slash-separated svg handler', ''], + ['slash before a quoted handler', ''], + ['handler on an allowed element', '

text

'], + ['mixed-case handler', 'x'], + // Unquoted and otherwise disguised script URLs. + ['unquoted javascript href', 'x'], + ['mixed-case javascript href', 'x'], + ['leading-space javascript href', 'x'], + ['tab inside the scheme', 'x'], + ['vbscript href', 'x'], + ['data URL href', 'x'], + ['javascript img src', ''], + // Entity-encoded schemes. + ['decimal-entity scheme', 'x'], + ['hex-entity scheme', 'x'], + ['entity-encoded tab in the scheme', 'x'], + ['padded decimal entities', 'x'], + // SVG and MathML. + ['svg animate handler', ''], + ['svg link', 'x'], + ['svg set href', 'x'], + ['svg foreignObject', ''], + ['math link', 'x'], + ['math href', 'x'], + ['math style breakout', '

x

'], + ['style attribute with an expression', '

x

'], + // Script and its neighbours. + ['script block', '

ok

'], + ['script with a src', ''], + ['nested script tags', 'ipt>alert(1)ipt>'], + ['noscript breakout', '
', + '
StageAmount
Won1,000
Total
', + '

Logo

', + '
', + '
Term
Definition
', + '
More

Hidden detail.

', +].join(''); + +describe('html / richtext display: a formatted document keeps its formatting', () => { + it('round-trips byte for byte', () => { + expect(renderHtml(FORMATTED_DOCUMENT).innerHTML).toBe(FORMATTED_DOCUMENT); + }); + + it('keeps the safe part of a mixed value', () => { + const prose = renderHtml( + '

Hello world

badgood', + ); + expect(prose.innerHTML).toBe('

Hello world

badgood'); + }); + + it('keeps the text of an element it does not allow', () => { + expect(renderHtml('

underlined and coloured

').innerHTML).toBe( + '

underlined and coloured

', + ); + }); + + it('renders plain text that holds no markup as text', () => { + expect(renderHtml('5 < 6 & 7 > 3').textContent).toBe('5 < 6 & 7 > 3'); + }); +}); diff --git a/packages/fields/src/widgets/richTextDisplay.tsx b/packages/fields/src/widgets/richTextDisplay.tsx index 422a154354..4fe7b400bc 100644 --- a/packages/fields/src/widgets/richTextDisplay.tsx +++ b/packages/fields/src/widgets/richTextDisplay.tsx @@ -34,6 +34,7 @@ import React from 'react'; import { EmptyValue } from '@object-ui/components'; import { coerceToSafeValue } from '../coerceToSafeValue.js'; +import { renderSanitizedHtml } from './sanitizedHtml.js'; import type { CellRendererProps } from '../index.js'; const LazyMarkdownContent = React.lazy(() => import('./MarkdownContent.js')); @@ -100,23 +101,6 @@ export function MarkdownCellRenderer({ value }: CellRendererProps): React.ReactE ); } -/** - * Minimal HTML sanitizer for display: drops