From dc0c709574a27070b63c100d8d87062d28be8018 Mon Sep 17 00:00:00 2001
From: Mathieu Pillard <mpillard@mozilla.com>
Date: Fri, 18 Apr 2025 11:43:20 +0200
Subject: [PATCH] Ensure <li>s are wrapped in a <ol> or <ul> after sanitizing
 HTML

This matters when displaying the versions page, where each version
is in a <li> already, and developers can have release notes
containing <li> of their own.
---
 src/amo/purify.js                  | 24 ++++++++++++-
 tests/unit/amo/utils/test_index.js | 55 ++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/src/amo/purify.js b/src/amo/purify.js
index 3d721d6bccb..55d3cf0f27a 100644
--- a/src/amo/purify.js
+++ b/src/amo/purify.js
@@ -2,4 +2,26 @@ import createDOMPurify from 'dompurify';
 
 import universalWindow from 'amo/window';
 
-export default createDOMPurify(universalWindow);
+const _purify = createDOMPurify(universalWindow);
+_purify.addHook('uponSanitizeElement', (node, data, config) => {
+  const _ALLOWED_TAGS = config.ALLOWED_TAGS || [];
+  if (
+    node.tagName &&
+    node.parentNode &&
+    node.tagName === 'LI' &&
+    !['MENU', 'OL', 'UL'].includes(node.parentNode.tagName) &&
+    _ALLOWED_TAGS.includes('ul') &&
+    _ALLOWED_TAGS.includes('li')
+  ) {
+    // If we find a <li> with no <ul>/<ol>/<menu> parent, create one for it.
+    // It's not ideal because this might create multiple independent lists for
+    // each item, but it's better than creating invalid HTML that would throw
+    // the virtual DOM off.
+    const oldParent = node.parentNode;
+    const newParent = node.ownerDocument.createElement('ul');
+    newParent.appendChild(node);
+    oldParent.appendChild(newParent);
+  }
+});
+
+export default _purify;
diff --git a/tests/unit/amo/utils/test_index.js b/tests/unit/amo/utils/test_index.js
index 53d02c67d62..455db0945fb 100644
--- a/tests/unit/amo/utils/test_index.js
+++ b/tests/unit/amo/utils/test_index.js
@@ -698,6 +698,61 @@ describe(__filename, () => {
         __html: '<a href="http://example.org">link</a>',
       });
     });
+
+    describe('custom `<li>` handling through hook', () => {
+      it('removes `<li>` if not allowed', () => {
+        const html = '<li>witness me!</li>';
+        expect(sanitizeHTML(html, [])).toEqual({
+          __html: 'witness me!',
+        });
+
+        expect(sanitizeHTML(html)).toEqual({
+          __html: 'witness me!',
+        });
+      });
+
+      it('does not wrap `<li>` inside a `<ul>` if not allowed', () => {
+        const html = '<li>witness me!</li>';
+        expect(sanitizeHTML(html, ['li'])).toEqual({
+          __html: '<li>witness me!</li>',
+        });
+      });
+
+      it('wraps `<li>` inside a `<ul>` if they dont already', () => {
+        const html = '<li>witness me!</li>';
+        expect(sanitizeHTML(html, ['li', 'ul'])).toEqual({
+          __html: '<ul><li>witness me!</li></ul>',
+        });
+      });
+
+      it('wraps `<li>` inside a `<ul>` if their parent was not allowed', () => {
+        const html = '<ol><li>witness me!</li></ol>';
+        expect(sanitizeHTML(html, ['li', 'ul'])).toEqual({
+          __html: '<ul><li>witness me!</li></ul>',
+        });
+      });
+
+      it('doesnt wrap `<li>` inside a `<ul>` if not necessary', () => {
+        const html = '<ul><li>witness me!</li></ul>';
+        expect(sanitizeHTML(html, ['li', 'ul'])).toEqual({
+          __html: '<ul><li>witness me!</li></ul>',
+        });
+      });
+
+      it('doesnt wrap `<li>` inside a `<ul>` if not necessary with ol', () => {
+        const html = '<ol><li>witness me!</li></ol>';
+        expect(sanitizeHTML(html, ['li', 'ol', 'ul'])).toEqual({
+          __html: '<ol><li>witness me!</li></ol>',
+        });
+      });
+
+      it('doesnt wrap `<li>` inside a `<ul>` if not necessary with menu', () => {
+        const html = '<menu><li>witness me!</li></menu>';
+        expect(sanitizeHTML(html, ['li', 'menu'])).toEqual({
+          __html: '<menu><li>witness me!</li></menu>',
+        });
+      });
+    });
   });
 
   describe('removeProtocolFromURL', () => {