Add allowedTags to HTMLContentTransform request (#5347)

* Add allowedTags to HTMLContentTransform request * Add PR number * Add screenshot
microsoft · Nov 4, 2024 · b245b63 · b245b63
1 parent 06bf029
commit b245b63
Show file tree

Hide file tree

Showing 7 changed files with 173 additions and 89 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -66,10 +66,11 @@ Notes: web developers are advised to use [`~` (tilde range)](https://github.com/
 -  Added code viewer dialog with syntax highlighting, in PR [#5335](https://github.com/microsoft/BotFramework-WebChat/pull/5335), by [@OEvgeny](https://github.com/OEvgeny)
 -  Added copy button to code blocks, in PR [#5334](https://github.com/microsoft/BotFramework-WebChat/pull/5334), by [@compulim](https://github.com/compulim)
 -  Added copy button to view code dialog, in PR [#5336](https://github.com/microsoft/BotFramework-WebChat/pull/5336), by [@compulim](https://github.com/compulim)
--  Added HTML content transformer middleware, in PR [#5338](https://github.com/microsoft/BotFramework-WebChat/pull/5338), by [@compulim](https://github.com/compulim)
+-  Added HTML content transformer middleware, in PR [#5338](https://github.com/microsoft/BotFramework-WebChat/pull/5338) and [#5347](https://github.com/microsoft/BotFramework-WebChat/pull/5347), by [@compulim](https://github.com/compulim)
    -  HTML content transformer is used by `useRenderMarkdown` to transform the result from `renderMarkdown`
    -  HTML sanitizer is moved from `renderMarkdown` into HTML content transformer for better coverage
    -  Copy button is added to fenced code blocks (`<pre><code>`)
+   -  Configure HTML sanitizer via `request.allowedTags`
 
 ### Changed
 

diff --git a/__tests__/html2/markdown/customElement.html b/__tests__/html2/markdown/customElement.html
@@ -0,0 +1,58 @@
+<!doctype html>
+<html lang="en-US">
+  <head>
+    <link href="/assets/index.css" rel="stylesheet" type="text/css" />
+    <script crossorigin="anonymous" src="/test-harness.js"></script>
+    <script crossorigin="anonymous" src="/test-page-object.js"></script>
+    <script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
+  </head>
+
+  <body>
+    <main id="webchat"></main>
+    <script>
+      run(async function () {
+        const {
+          WebChat: { renderWebChat }
+        } = window; // Imports in UMD fashion.
+
+        customElements.define(
+          'my-world',
+          class extends HTMLElement {
+            connectedCallback() {
+              this.append('🌎');
+            }
+          }
+        );
+
+        const allowCustomElementsTransform = () => next => request =>
+          next({
+            ...request,
+            allowedTags: Object.freeze(
+              new Map(request.allowedTags).set('my-world', Object.freeze({ attributes: Object.freeze([]) }))
+            )
+          });
+
+        const { directLine, store } = testHelpers.createDirectLineEmulator();
+
+        renderWebChat(
+          { directLine, htmlContentTransformMiddleware: [allowCustomElementsTransform], store },
+          document.getElementById('webchat')
+        );
+
+        await pageConditions.uiConnected();
+
+        await directLine.emulateIncomingActivity({
+          // Custom elements must not be self-closing.
+          text: `Hello, <my-world></my-world>!`,
+          type: 'message'
+        });
+
+        await pageConditions.numActivitiesShown(1);
+
+        // THEN: Should show as "Hello, 🌎!".
+        expect(pageElements.activityContents()[0].textContent).toBe('Hello, 🌎!');
+        await host.snapshot('local');
+      });
+    </script>
+  </body>
+</html>
diff --git a/__tests__/html2/markdown/customElement.html.snap-1.png b/__tests__/html2/markdown/customElement.html.snap-1.png
diff --git a/packages/bundle/src/markdown/middleware/createCodeBlockCopyButtonMiddleware.ts b/packages/bundle/src/markdown/middleware/createCodeBlockCopyButtonMiddleware.ts
@@ -7,6 +7,16 @@ export default function createCodeBlockCopyButtonMiddleware(): HTMLContentTransf
     next(
       Object.freeze({
         ...request,
+        allowedTags: Object.freeze(
+          new Map(request.allowedTags).set(
+            request.codeBlockCopyButtonTagName,
+            Object.freeze({
+              attributes: Object.freeze(
+                new Set(['class', 'data-alt-copy', 'data-alt-copied', 'data-testid', 'data-value'])
+              )
+            })
+          )
+        ),
         documentFragment: codeBlockCopyButtonDocumentMod(request.documentFragment, {
           codeBlockCopyButtonAltCopied: request.codeBlockCopyButtonAltCopied,
           codeBlockCopyButtonAltCopy: request.codeBlockCopyButtonAltCopy,

diff --git a/packages/bundle/src/markdown/middleware/createSanitizeMiddleware.ts b/packages/bundle/src/markdown/middleware/createSanitizeMiddleware.ts
@@ -1,108 +1,32 @@
+import { type HTMLContentTransformMiddleware } from 'botframework-webchat-component';
 import {
   parseDocumentFragmentFromString,
   serializeDocumentFragmentIntoString
 } from 'botframework-webchat-component/internal';
 import sanitizeHTML from 'sanitize-html';
 
 const BASE_SANITIZE_HTML_OPTIONS = Object.freeze({
-  allowedAttributes: {
-    a: ['aria-label', 'class', 'href', 'name', 'rel', 'target'],
-    button: ['aria-label', 'class', 'type', 'value'],
-    img: ['alt', 'aria-label', 'class', 'src', 'title'],
-    pre: ['class'],
-    span: ['aria-label']
-  },
   allowedSchemes: ['data', 'http', 'https', 'ftp', 'mailto', 'sip', 'tel'],
-  allowedTags: [
-    'a',
-    'b',
-    'blockquote',
-    'br',
-    'button',
-    'caption',
-    'code',
-    'del',
-    'div',
-    'em',
-    'h1',
-    'h2',
-    'h3',
-    'h4',
-    'h5',
-    'h6',
-    'hr',
-    'i',
-    'img',
-    'ins',
-    'li',
-    'nl',
-    'ol',
-    'p',
-    'pre',
-    's',
-    'span',
-    'strike',
-    'strong',
-    'table',
-    'tbody',
-    'td',
-    'tfoot',
-    'th',
-    'thead',
-    'tr',
-    'ul',
-
-    // Followings are for MathML elements, from https://developer.mozilla.org/en-US/docs/Web/MathML.
-    'annotation-xml',
-    'annotation',
-    'math',
-    'merror',
-    'mfrac',
-    'mi',
-    'mmultiscripts',
-    'mn',
-    'mo',
-    'mover',
-    'mpadded',
-    'mphantom',
-    'mprescripts',
-    'mroot',
-    'mrow',
-    'ms',
-    'mspace',
-    'msqrt',
-    'mstyle',
-    'msub',
-    'msubsup',
-    'msup',
-    'mtable',
-    'mtd',
-    'mtext',
-    'mtr',
-    'munder',
-    'munderover',
-    'semantics'
-  ],
   // Bug of https://github.com/apostrophecms/sanitize-html/issues/633.
   // They should not remove `alt=""` even though it is empty.
   nonBooleanAttributes: []
 });
 
-export default function createSanitizeMiddleware() {
+export default function createSanitizeMiddleware(): HTMLContentTransformMiddleware {
   return () => () => request => {
-    const { codeBlockCopyButtonTagName, documentFragment } = request;
-    const sanitizeHTMLOptions = {
-      ...BASE_SANITIZE_HTML_OPTIONS,
-      allowedAttributes: {
-        ...BASE_SANITIZE_HTML_OPTIONS.allowedAttributes,
-        [codeBlockCopyButtonTagName]: ['class', 'data-alt-copy', 'data-alt-copied', 'data-testid', 'data-value']
-      },
-      allowedTags: [...BASE_SANITIZE_HTML_OPTIONS.allowedTags, codeBlockCopyButtonTagName]
-    };
+    const { documentFragment } = request;
 
     const htmlAfterBetterLink = serializeDocumentFragmentIntoString(documentFragment);
 
-    const htmlAfterSanitization = sanitizeHTML(htmlAfterBetterLink, sanitizeHTMLOptions);
+    const htmlAfterSanitization = sanitizeHTML(htmlAfterBetterLink, {
+      ...BASE_SANITIZE_HTML_OPTIONS,
+      allowedAttributes: Object.fromEntries(
+        Array.from(request.allowedTags.entries()).map(
+          ([tag, { attributes }]) => [tag, Array.from(attributes)] satisfies [string, string[]]
+        )
+      ) satisfies Record<string, string[]>,
+      allowedTags: Array.from(request.allowedTags.keys() satisfies Iterator<string>) satisfies string[]
+    });
 
     return parseDocumentFragmentFromString(htmlAfterSanitization);
   };

diff --git a/...es/component/src/providers/HTMLContentTransformCOR/private/HTMLContentTransformContext.ts b/...es/component/src/providers/HTMLContentTransformCOR/private/HTMLContentTransformContext.ts
@@ -1,6 +1,13 @@
 import { createContext } from 'react';
 
 export type HTMLContentTransformRequest = Readonly<{
+  allowedTags: ReadonlyMap<
+    string,
+    Readonly<{
+      // TODO: Ultimately, we could allowlist a cherry-picked instance of element, but not all elements sharing the same tag name.
+      attributes: ReadonlySet<string>;
+    }>
+  >;
   codeBlockCopyButtonAltCopied: string;
   codeBlockCopyButtonAltCopy: string;
   codeBlockCopyButtonClassName: string;

diff --git a/packages/component/src/providers/HTMLContentTransformCOR/useTransformHTMLContent.ts b/packages/component/src/providers/HTMLContentTransformCOR/useTransformHTMLContent.ts
@@ -7,6 +7,89 @@ import useHTMLContentTransformContext from './private/useHTMLContentTransformCon
 
 const { useLocalizer } = hooks;
 
+const DEFAULT_ALLOWED_TAGS: ReadonlyMap<string, Readonly<{ attributes: ReadonlySet<string> }>> = Object.freeze(
+  new Map(
+    (
+      [
+        ['a', ['aria-label', 'class', 'href', 'name', 'rel', 'target']],
+        ['b', []],
+        ['blockquote', []],
+        ['br', []],
+        ['button', ['aria-label', 'class', 'type', 'value']],
+        ['caption', []],
+        ['code', []],
+        ['del', []],
+        ['div', []],
+        ['em', []],
+        ['h1', []],
+        ['h2', []],
+        ['h3', []],
+        ['h4', []],
+        ['h5', []],
+        ['h6', []],
+        ['hr', []],
+        ['i', []],
+        ['img', ['alt', 'aria-label', 'class', 'src', 'title']],
+        ['ins', []],
+        ['li', []],
+        ['nl', []],
+        ['ol', []],
+        ['p', []],
+        ['pre', ['class']],
+        ['s', []],
+        ['span', ['aria-label']],
+        ['strike', []],
+        ['strong', []],
+        ['table', []],
+        ['tbody', []],
+        ['td', []],
+        ['tfoot', []],
+        ['th', []],
+        ['thead', []],
+        ['tr', []],
+        ['ul', []],
+
+        // Followings are for MathML elements, from https://developer.mozilla.org/en-US/docs/Web/MathML.
+        ['annotation-xml', []],
+        ['annotation', []],
+        ['math', []],
+        ['merror', []],
+        ['mfrac', []],
+        ['mi', []],
+        ['mmultiscripts', []],
+        ['mn', []],
+        ['mo', []],
+        ['mover', []],
+        ['mpadded', []],
+        ['mphantom', []],
+        ['mprescripts', []],
+        ['mroot', []],
+        ['mrow', []],
+        ['ms', []],
+        ['mspace', []],
+        ['msqrt', []],
+        ['mstyle', []],
+        ['msub', []],
+        ['msubsup', []],
+        ['msup', []],
+        ['mtable', []],
+        ['mtd', []],
+        ['mtext', []],
+        ['mtr', []],
+        ['munder', []],
+        ['munderover', []],
+        ['semantics', []]
+      ] satisfies [string, string[]][]
+    ).map(
+      ([tag, attributes]) =>
+        [tag, Object.freeze({ attributes: Object.freeze(new Set(attributes)) })] satisfies [
+          string,
+          Readonly<{ attributes: ReadonlySet<string> }>
+        ]
+    )
+  )
+);
+
 export default function useTransformHTMLContent(): (documentFragment: DocumentFragment) => DocumentFragment {
   const [{ codeBlockCopyButton: codeBlockCopyButtonClassName }] = useStyleSet();
   const [codeBlockCopyButtonTagName] = useCodeBlockCopyButtonTagName();
@@ -20,6 +103,7 @@ export default function useTransformHTMLContent(): (documentFragment: DocumentFr
   return useCallback(
     documentFragment =>
       transform({
+        allowedTags: DEFAULT_ALLOWED_TAGS,
         codeBlockCopyButtonAltCopied,
         codeBlockCopyButtonAltCopy,
         codeBlockCopyButtonClassName,