From 68d5958fe78b5e8ffee9cc5e6e00376d1735cd83 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 30 May 2026 19:37:05 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=84=20fix:=20Harden=20Configured=20Ric?= =?UTF-8?q?h=20Text=20Rendering=20(#13423)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- client/src/components/Banners/Banner.tsx | 30 ++-- .../components/Chat/Input/MCPConfigDialog.tsx | 32 ++-- .../components/MCP/CustomUserVarsSection.tsx | 42 ++--- .../MCPServerDialog/sections/TrustSection.tsx | 34 ++-- .../sections/__tests__/TrustSection.spec.tsx | 166 ++++++++++++++++++ client/src/utils/__tests__/configHtml.test.ts | 46 +++++ client/src/utils/configHtml.ts | 45 +++++ client/src/utils/index.ts | 1 + 8 files changed, 311 insertions(+), 85 deletions(-) create mode 100644 client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/__tests__/TrustSection.spec.tsx create mode 100644 client/src/utils/__tests__/configHtml.test.ts create mode 100644 client/src/utils/configHtml.ts diff --git a/client/src/components/Banners/Banner.tsx b/client/src/components/Banners/Banner.tsx index a1e9056c07..72c1891683 100644 --- a/client/src/components/Banners/Banner.tsx +++ b/client/src/components/Banners/Banner.tsx @@ -1,8 +1,12 @@ -import DOMPurify from 'dompurify'; import { XIcon } from 'lucide-react'; import { useRecoilState } from 'recoil'; import { Button, cn } from '@librechat/client'; import { useEffect, useMemo, useRef } from 'react'; +import { + CONFIG_HTML_TEXT_TAGS, + CONFIG_HTML_CLASS_ATTR, + createConfigHtmlSanitizer, +} from '~/utils/configHtml'; import { useGetBannerQuery } from '~/data-provider'; import store from '~/store'; @@ -10,25 +14,21 @@ export const Banner = ({ onHeightChange }: { onHeightChange?: (height: number) = const { data: banner } = useGetBannerQuery(); const [hideBannerHint, setHideBannerHint] = useRecoilState(store.hideBannerHint); const bannerRef = useRef(null); + const sanitize = useMemo( + () => + createConfigHtmlSanitizer({ + allowedTags: CONFIG_HTML_TEXT_TAGS, + allowedAttr: CONFIG_HTML_CLASS_ATTR, + }), + [], + ); const sanitizedMessage = useMemo(() => { if (!banner?.message) { return ''; } - const sanitizer = DOMPurify(); - sanitizer.addHook('afterSanitizeAttributes', (node) => { - if (node.tagName === 'A') { - node.setAttribute('target', '_blank'); - node.setAttribute('rel', 'noopener noreferrer'); - } - }); - return sanitizer.sanitize(banner.message, { - ALLOWED_TAGS: ['a', 'strong', 'b', 'em', 'i', 'br', 'code', 'span'], - ALLOWED_ATTR: ['href', 'class', 'target', 'rel'], - ALLOW_DATA_ATTR: false, - ALLOW_ARIA_ATTR: false, - }); - }, [banner?.message]); + return sanitize(banner.message); + }, [banner?.message, sanitize]); useEffect(() => { if (onHeightChange && bannerRef.current) { diff --git a/client/src/components/Chat/Input/MCPConfigDialog.tsx b/client/src/components/Chat/Input/MCPConfigDialog.tsx index a4cb03251e..206c29b692 100644 --- a/client/src/components/Chat/Input/MCPConfigDialog.tsx +++ b/client/src/components/Chat/Input/MCPConfigDialog.tsx @@ -1,8 +1,12 @@ -import DOMPurify from 'dompurify'; import React, { useEffect, useMemo } from 'react'; import { useForm, Controller } from 'react-hook-form'; import { Button, Input, Label, OGDialog, OGDialogTemplate } from '@librechat/client'; import type { ConfigFieldDetail } from '~/common'; +import { + CONFIG_HTML_BLOCK_TAGS, + CONFIG_HTML_CLASS_ATTR, + createConfigHtmlSanitizer, +} from '~/utils/configHtml'; import { useLocalize } from '~/hooks'; interface MCPConfigDialogProps { @@ -36,24 +40,14 @@ export default function MCPConfigDialog({ defaultValues: initialValues, }); - const sanitizer = useMemo(() => { - const instance = DOMPurify(); - instance.addHook('afterSanitizeAttributes', (node) => { - if (node.tagName === 'A') { - node.setAttribute('target', '_blank'); - node.setAttribute('rel', 'noopener noreferrer'); - } - }); - return instance; - }, []); - - const sanitize = (html: string) => - sanitizer.sanitize(html, { - ALLOWED_TAGS: ['a', 'strong', 'b', 'em', 'i', 'br', 'code', 'span', 'p'], - ALLOWED_ATTR: ['href', 'class', 'target', 'rel'], - ALLOW_DATA_ATTR: false, - ALLOW_ARIA_ATTR: false, - }); + const sanitize = useMemo( + () => + createConfigHtmlSanitizer({ + allowedTags: CONFIG_HTML_BLOCK_TAGS, + allowedAttr: CONFIG_HTML_CLASS_ATTR, + }), + [], + ); useEffect(() => { if (isOpen) { diff --git a/client/src/components/MCP/CustomUserVarsSection.tsx b/client/src/components/MCP/CustomUserVarsSection.tsx index a986a2786f..83dcb5f482 100644 --- a/client/src/components/MCP/CustomUserVarsSection.tsx +++ b/client/src/components/MCP/CustomUserVarsSection.tsx @@ -1,8 +1,12 @@ import React, { useMemo } from 'react'; -import DOMPurify from 'dompurify'; import { useForm, Controller } from 'react-hook-form'; import { Input, Label, Button } from '@librechat/client'; import { useMCPAuthValuesQuery } from '~/data-provider/Tools/queries'; +import { + CONFIG_HTML_INLINE_TAGS, + CONFIG_HTML_CLASS_ATTR, + createConfigHtmlSanitizer, +} from '~/utils/configHtml'; import { useLocalize } from '~/hooks'; export interface CustomUserVarConfig { @@ -29,34 +33,18 @@ interface AuthFieldProps { function AuthField({ name, config, hasValue, control, errors, autoFocus }: AuthFieldProps) { const localize = useLocalize(); const statusText = hasValue ? localize('com_ui_set') : localize('com_ui_unset'); - - const sanitizer = useMemo(() => { - const instance = DOMPurify(); - instance.addHook('afterSanitizeAttributes', (node) => { - if (node.tagName && node.tagName === 'A') { - node.setAttribute('target', '_blank'); - node.setAttribute('rel', 'noopener noreferrer'); - } - }); - return instance; - }, []); + const sanitize = useMemo( + () => + createConfigHtmlSanitizer({ + allowedTags: CONFIG_HTML_INLINE_TAGS, + allowedAttr: CONFIG_HTML_CLASS_ATTR, + }), + [], + ); const sanitizedDescription = useMemo(() => { - if (!config.description) { - return ''; - } - try { - return sanitizer.sanitize(config.description, { - ALLOWED_TAGS: ['a', 'strong', 'b', 'em', 'i', 'br', 'code'], - ALLOWED_ATTR: ['href', 'class', 'target', 'rel'], - ALLOW_DATA_ATTR: false, - ALLOW_ARIA_ATTR: false, - }); - } catch (error) { - console.error('Sanitization failed', error); - return config.description; - } - }, [config.description, sanitizer]); + return sanitize(config.description); + }, [config.description, sanitize]); return (
diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/TrustSection.tsx b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/TrustSection.tsx index 36d8d73a49..7ac9d3a523 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/TrustSection.tsx +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/TrustSection.tsx @@ -1,17 +1,25 @@ +import { useMemo } from 'react'; import { useFormContext, Controller } from 'react-hook-form'; import { Checkbox, Label } from '@librechat/client'; import { useLocalize, useLocalizedConfig } from '~/hooks'; import { useGetStartupConfig } from '~/data-provider'; +import { createConfigHtmlSanitizer } from '~/utils/configHtml'; import type { MCPServerFormData } from '../hooks/useMCPServerForm'; export default function TrustSection() { const localize = useLocalize(); const { data: startupConfig } = useGetStartupConfig(); const getLocalizedValue = useLocalizedConfig(); + const sanitize = useMemo(() => createConfigHtmlSanitizer(), []); const { control, formState: { errors }, } = useFormContext(); + const trustCheckbox = startupConfig?.interface?.mcpServers?.trustCheckbox; + const labelHTML = sanitize(getLocalizedValue(trustCheckbox?.label, localize('com_ui_trust_app'))); + const subLabelHTML = sanitize( + getLocalizedValue(trustCheckbox?.subLabel, localize('com_agents_mcp_trust_subtext')), + ); return (
@@ -37,35 +45,13 @@ export default function TrustSection() { />
diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/__tests__/TrustSection.spec.tsx b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/__tests__/TrustSection.spec.tsx new file mode 100644 index 0000000000..f573d72465 --- /dev/null +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/__tests__/TrustSection.spec.tsx @@ -0,0 +1,166 @@ +import { render, screen } from '@testing-library/react'; +import { FormProvider, useForm } from 'react-hook-form'; +import type { ChangeEvent, ReactNode } from 'react'; +import TrustSection from '../TrustSection'; +import type { MCPServerFormData } from '../../hooks/useMCPServerForm'; + +type LocalizedValue = string | Record; + +type StartupConfigMock = { + interface?: { + mcpServers?: { + trustCheckbox?: { + label?: LocalizedValue; + subLabel?: LocalizedValue; + }; + }; + }; +}; + +let mockStartupConfig: StartupConfigMock | undefined; + +jest.mock('~/data-provider', () => ({ + useGetStartupConfig: () => ({ data: mockStartupConfig }), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => { + const translations: Record = { + com_ui_trust_app: 'I trust this app', + com_agents_mcp_trust_subtext: 'Only continue if you trust this MCP server.', + com_ui_field_required: 'This field is required', + }; + return translations[key] ?? key; + }, + useLocalizedConfig: () => (value: LocalizedValue | undefined, fallback: string) => { + if (value === undefined) { + return fallback; + } + if (typeof value === 'string') { + return value; + } + return value.en ?? Object.values(value)[0] ?? fallback; + }, +})); + +jest.mock( + '@librechat/client', + () => { + const React = jest.requireActual('react'); + return { + Checkbox: ({ + checked, + onCheckedChange, + ...props + }: { + checked: boolean; + onCheckedChange: (checked: boolean) => void; + }) => + React.createElement('input', { + type: 'checkbox', + checked, + onChange: (event: ChangeEvent) => onCheckedChange(event.target.checked), + ...props, + }), + Label: ({ children, ...props }: { children: ReactNode }) => + React.createElement('label', props, children), + }; + }, + { virtual: true }, +); + +function createDefaultValues(): MCPServerFormData { + return { + title: '', + description: '', + icon: '', + url: '', + type: 'streamable-http', + auth: { + auth_type: 'none' as MCPServerFormData['auth']['auth_type'], + }, + trust: false, + }; +} + +function renderTrustSection() { + function Wrapper() { + const methods = useForm({ + defaultValues: createDefaultValues(), + }); + return ( + + + + ); + } + + return render(); +} + +describe('TrustSection', () => { + beforeEach(() => { + mockStartupConfig = undefined; + }); + + it('sanitizes script-capable trust checkbox label and sub-label HTML', () => { + mockStartupConfig = { + interface: { + mcpServers: { + trustCheckbox: { + label: + 'Trust OK', + subLabel: + 'Learn', + }, + }, + }, + }; + + const { container } = renderTrustSection(); + const label = container.querySelector('#trust-label'); + const description = container.querySelector('#trust-description'); + const link = screen.getByText('Learn').closest('a'); + + expect(label).not.toBeNull(); + expect(description).not.toBeNull(); + expect(label?.innerHTML).not.toMatch(/onerror/i); + expect(description?.innerHTML).not.toMatch(/onclick/i); + expect(container.querySelector('img')).not.toBeInTheDocument(); + expect(container.querySelector('script')).not.toBeInTheDocument(); + expect(container.querySelector('svg')).not.toBeInTheDocument(); + expect(link).not.toBeNull(); + expect(link).not.toHaveAttribute('href'); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', 'noopener noreferrer'); + }); + + it('preserves documented formatting while normalizing links', () => { + mockStartupConfig = { + interface: { + mcpServers: { + trustCheckbox: { + label: { en: 'I understand' }, + subLabel: + 'Read Learn more.
safe', + }, + }, + }, + }; + + const { container } = renderTrustSection(); + const emphasis = screen.getByText('understand'); + const strong = screen.getByText('Learn more.'); + const code = screen.getByText('safe'); + const link = strong.closest('a'); + + expect(emphasis.tagName).toBe('EM'); + expect(strong.tagName).toBe('STRONG'); + expect(code.tagName).toBe('CODE'); + expect(container.querySelector('#trust-description br')).toBeInTheDocument(); + expect(link).not.toBeNull(); + expect(link).toHaveAttribute('href', 'https://example.com/docs'); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', 'noopener noreferrer'); + }); +}); diff --git a/client/src/utils/__tests__/configHtml.test.ts b/client/src/utils/__tests__/configHtml.test.ts new file mode 100644 index 0000000000..66ee2969e1 --- /dev/null +++ b/client/src/utils/__tests__/configHtml.test.ts @@ -0,0 +1,46 @@ +import { + CONFIG_HTML_BLOCK_TAGS, + CONFIG_HTML_CLASS_ATTR, + CONFIG_HTML_INLINE_TAGS, + createConfigHtmlSanitizer, + sanitizeConfigHtml, +} from '../configHtml'; + +describe('configHtml', () => { + it('removes active attributes and unsupported elements', () => { + const sanitized = sanitizeConfigHtml( + 'Learn', + ); + + expect(sanitized).toBe( + 'Learn', + ); + }); + + it('keeps configured rich text tags and normalizes links', () => { + const sanitize = createConfigHtmlSanitizer({ + allowedTags: CONFIG_HTML_BLOCK_TAGS, + allowedAttr: CONFIG_HTML_CLASS_ATTR, + }); + const sanitized = sanitize( + '

Read more
safe

', + ); + + expect(sanitized).toBe( + '

Read more
safe

', + ); + }); + + it('keeps relative links but removes protocol-relative links', () => { + const sanitize = createConfigHtmlSanitizer({ + allowedTags: CONFIG_HTML_INLINE_TAGS, + }); + const sanitized = sanitize( + 'Docs Remote', + ); + + expect(sanitized).toBe( + 'Docs Remote', + ); + }); +}); diff --git a/client/src/utils/configHtml.ts b/client/src/utils/configHtml.ts new file mode 100644 index 0000000000..171b8ce629 --- /dev/null +++ b/client/src/utils/configHtml.ts @@ -0,0 +1,45 @@ +import DOMPurify from 'dompurify'; + +export const CONFIG_HTML_INLINE_TAGS = ['a', 'strong', 'b', 'em', 'i', 'br', 'code'] as const; +export const CONFIG_HTML_TEXT_TAGS = [...CONFIG_HTML_INLINE_TAGS, 'span'] as const; +export const CONFIG_HTML_BLOCK_TAGS = [...CONFIG_HTML_TEXT_TAGS, 'p'] as const; +export const CONFIG_HTML_LINK_ATTR = ['href', 'target', 'rel'] as const; +export const CONFIG_HTML_CLASS_ATTR = [...CONFIG_HTML_LINK_ATTR, 'class'] as const; + +const CONFIG_HTML_SAFE_URI = + /^(?:(?:https?|mailto|tel):|(?!(?:\s*[a-z][a-z0-9+.-]*:|\s*\/\/))[\s\S])/i; + +type ConfigHtmlSanitizerOptions = { + allowedTags?: readonly string[]; + allowedAttr?: readonly string[]; +}; + +export function createConfigHtmlSanitizer({ + allowedTags = CONFIG_HTML_INLINE_TAGS, + allowedAttr = CONFIG_HTML_LINK_ATTR, +}: ConfigHtmlSanitizerOptions = {}) { + const sanitizer = DOMPurify(); + sanitizer.addHook('afterSanitizeAttributes', (node) => { + if (node.tagName === 'A') { + node.setAttribute('target', '_blank'); + node.setAttribute('rel', 'noopener noreferrer'); + } + }); + + return (html?: string | null): string => { + if (!html) { + return ''; + } + return sanitizer.sanitize(html, { + ALLOWED_TAGS: [...allowedTags], + ALLOWED_ATTR: [...allowedAttr], + ALLOWED_URI_REGEXP: CONFIG_HTML_SAFE_URI, + ALLOW_DATA_ATTR: false, + ALLOW_ARIA_ATTR: false, + }); + }; +} + +export function sanitizeConfigHtml(html?: string | null, options?: ConfigHtmlSanitizerOptions) { + return createConfigHtmlSanitizer(options)(html); +} diff --git a/client/src/utils/index.ts b/client/src/utils/index.ts index 60d87a3a23..14d76e7b73 100644 --- a/client/src/utils/index.ts +++ b/client/src/utils/index.ts @@ -26,6 +26,7 @@ export * from './languages'; export * from './conversation'; export * from './endpoints'; export * from './resources'; +export * from './configHtml'; export * from './downloadFile'; export * from './scaleImage'; export * from './timestamps';