Add maintainer suggestions

Refactor to imprvoe readability, replaces regex in favor of startsWith
and make tests more clear by using .each.

app/assets/javascripts/lib/dompurify.js

-import { sanitize, addHook } from 'dompurify';
+import { sanitize as dompurifySanitize, addHook } from 'dompurify';
 import { getBaseURL, relativePathToAbsolute } from '~/lib/utils/url_utility';
 
 // Safely allow SVG <use> tags
@@ -7,27 +7,23 @@ const defaultConfig = {
   ADD_TAGS: ['use'],
 };
 
-const getIconUrlsRegex = () => {
-  const { gon } = window;
+// Only icons urls from `gon` are allowed
+const getAllowedIconUrls = (gon = window.gon) =>
+  [gon.sprite_file_icons, gon.sprite_icons].filter(Boolean);
 
-  // Only icons urls from `gon` are allowed
-  const allowed = [gon.sprite_file_icons, gon.sprite_icons]
-    .map(url => relativePathToAbsolute(url, getBaseURL()))
-    .filter(url => url);
+const isUrlAllowed = url => getAllowedIconUrls().some(allowedUrl => url.startsWith(allowedUrl));
 
-  if (allowed.length) {
-    return allowed.join('|');
+const isHrefSafe = url =>
+  isUrlAllowed(url) || isUrlAllowed(relativePathToAbsolute(url, getBaseURL()));
+
+const removeUnsafeHref = (node, attr) => {
+  if (!node.hasAttribute(attr)) {
+    return;
   }
-  return null; // No urls allowed
-};
 
-const removeUnsafeHref = (node, allowedRegex = null, attr = 'href') => {
-  if (node.hasAttribute(attr) && !node.getAttribute(attr).match(allowedRegex)) {
-    const url = relativePathToAbsolute(node.getAttribute(attr), getBaseURL());
-    if (!url.match(allowedRegex)) {
+  if (!isHrefSafe(node.getAttribute(attr))) {
     node.removeAttribute(attr);
   }
-  }
 };
 
 /**
@@ -38,35 +34,20 @@ const removeUnsafeHref = (node, allowedRegex = null, attr = 'href') => {
  *   <use href="/assets/icons-xxx.svg#icon_name"></use>
  * </svg>
  *
- * Note: In order to render icons, you should still allow <use>
- * when invoking `sanitize`, for example:
- *
- * ```
- * import { sanitize } from '~/lib/dompurify';
- *
- * sanitize(content, { ADD_TAGS: ['use'] });
- * ```
- *
  * @param {Object} node - Node to sanitize
  */
-const sanitizeSvgIcons = node => {
-  const allowed = getIconUrlsRegex();
-
-  removeUnsafeHref(node, allowed);
+const sanitizeSvgIcon = node => {
+  removeUnsafeHref(node, 'href');
 
-  // Note: `xlink:href` is deprecated
+  // Note: `xlink:href` is deprecated, but still in use
   // https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href
-  removeUnsafeHref(node, allowed, 'xlink:href');
+  removeUnsafeHref(node, 'xlink:href');
 };
 
 addHook('afterSanitizeAttributes', node => {
   if (node.tagName.toLowerCase() === 'use') {
-    sanitizeSvgIcons(node);
+    sanitizeSvgIcon(node);
   }
 });
 
-const defaultSanitize = (val, config = defaultConfig) => {
-  return sanitize(val, config);
-};
-
-export { defaultSanitize as sanitize };
+export const sanitize = (val, config = defaultConfig) => dompurifySanitize(val, config);

spec/frontend/lib/dompurify_spec.js

 import { sanitize } from '~/lib/dompurify';
 
 // GDK
-const localGon = {
+const rootGon = {
   sprite_file_icons: '/assets/icons-123a.svg',
   sprite_icons: '/assets/icons-456b.svg',
 };
@@ -12,24 +12,40 @@ const absoluteGon = {
   sprite_icons: `${window.location.protocol}//${window.location.hostname}/assets/icons-456b.svg`,
 };
 
+const expectedSanitized = '<svg><use></use></svg>';
+
+const safeUrls = {
+  root: Object.values(rootGon).map(url => `${url}#ellipsis_h`),
+  absolute: Object.values(absoluteGon).map(url => `${url}#ellipsis_h`),
+};
+
+const unsafeUrls = [
+  '/an/evil/url',
+  '../../../evil/url',
+  'https://evil.url/assets/icons-123a.svg',
+  'https://evil.url/assets/icons-456b.svg',
+  `https://evil.url/${rootGon.sprite_icons}`,
+  `https://evil.url/${rootGon.sprite_file_icons}`,
+  `https://evil.url/${absoluteGon.sprite_icons}`,
+  `https://evil.url/${absoluteGon.sprite_file_icons}`,
+];
+
 describe('~/lib/dompurify', () => {
   let originalGon;
 
-  describe('uses local configuration', () => {
+  it('uses local configuration when given', () => {
     // As dompurify uses a "Persistent Configuration", it might
     // ignore config, this check verifies we respect
     // https://github.com/cure53/DOMPurify#persistent-configuration
-    it('no allowed tags', () => {
-      expect(sanitize('<br/>', { ALLOWED_TAGS: [] })).toBe('');
+    expect(sanitize('<br>', { ALLOWED_TAGS: [] })).toBe('');
     expect(sanitize('<strong></strong>', { ALLOWED_TAGS: [] })).toBe('');
   });
-  });
 
   describe.each`
     type          | gon
-    ${'local'}    | ${localGon}
+    ${'root'}     | ${rootGon}
     ${'absolute'} | ${absoluteGon}
-  `('when gon contains $type icon urls', ({ gon }) => {
+  `('when gon contains $type icon urls', ({ type, gon }) => {
     beforeAll(() => {
       originalGon = window.gon;
       window.gon = gon;
@@ -39,67 +55,31 @@ describe('~/lib/dompurify', () => {
       window.gon = originalGon;
     });
 
-    it('sanitizes icons allowing safe xlink:href sprite_file_icons', () => {
-      const html = '<svg><use xlink:href="/assets/icons-123a.svg#ellipsis_h"></use></svg>';
-
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe(
-        '<svg><use xlink:href="/assets/icons-123a.svg#ellipsis_h"></use></svg>',
-      );
+    it('allows no href attrs', () => {
+      const htmlHref = `<svg><use></use></svg>`;
+      expect(sanitize(htmlHref)).toBe(htmlHref);
     });
 
-    it('sanitizes icons allowing safe href sprite_file_icons', () => {
-      const html = '<svg><use href="/assets/icons-123a.svg#ellipsis_h"></use></svg>';
+    it.each(safeUrls[type])('allows safe URL %s', url => {
+      const htmlHref = `<svg><use href="${url}"></use></svg>`;
+      expect(sanitize(htmlHref)).toBe(htmlHref);
 
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe(
-        '<svg><use href="/assets/icons-123a.svg#ellipsis_h"></use></svg>',
-      );
+      const htmlXlink = `<svg><use xlink:href="${url}"></use></svg>`;
+      expect(sanitize(htmlXlink)).toBe(htmlXlink);
     });
 
-    it('sanitizes icons allowing safe href sprite_icons', () => {
-      const html = '<svg><use href="/assets/icons-456b.svg#ellipsis_h"></use></svg>';
+    it.each(unsafeUrls)('sanitizes unsafe URL %s', url => {
+      const htmlHref = `<svg><use href="${url}"></use></svg>`;
+      const htmlXlink = `<svg><use xlink:href="${url}"></use></svg>`;
 
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe(
-        '<svg><use href="/assets/icons-456b.svg#ellipsis_h"></use></svg>',
-      );
-    });
-
-    it('sanitizes icons allowing safe xlink:href sprite_icons', () => {
-      const html = '<svg><use xlink:href="/assets/icons-456b.svg#ellipsis_h"></use></svg>';
-
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe(
-        '<svg><use xlink:href="/assets/icons-456b.svg#ellipsis_h"></use></svg>',
-      );
-    });
-
-    it('sanitizes icons disabling unsafe href paths', () => {
-      const html = '<svg><use href="/an/evil/url"></use></svg>';
-
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe('<svg><use></use></svg>');
-    });
-
-    it('sanitizes icons disabling unsafe xlink:href paths', () => {
-      const html = '<svg><use xlink:href="/an/evil/url"></use></svg>';
-
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe('<svg><use></use></svg>');
-    });
-
-    it('sanitizes icons disabling unsafe href hosts', () => {
-      const html = '<svg><use href="https://evil.url/assets/icons-123a.svg"></use></svg>';
-
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe('<svg><use></use></svg>');
-    });
-
-    it('sanitizes icons disabling unsafe xlink:href hosts', () => {
-      const html = '<svg><use xlink:href="https://evil.url/assets/icons-123a.svg"></use></svg>';
-
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe('<svg><use></use></svg>');
+      expect(sanitize(htmlHref)).toBe(expectedSanitized);
+      expect(sanitize(htmlXlink)).toBe(expectedSanitized);
     });
   });
 
   describe('when gon does not contain icon urls', () => {
     beforeAll(() => {
       originalGon = window.gon;
-
       window.gon = {};
     });
 
@@ -107,16 +87,12 @@ describe('~/lib/dompurify', () => {
       window.gon = originalGon;
     });
 
-    it('sanitizes icons disabling all xlink:href values', () => {
-      const html = '<svg><use xlink:href="/assets/icons-123a.svg#ellipsis_h"></use></svg>';
-
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe('<svg><use></use></svg>');
-    });
-
-    it('sanitizes icons disabling all href values', () => {
-      const html = '<svg><use href="/assets/icons-123a.svg#ellipsis_h"></use></svg>';
+    it.each([...safeUrls.root, ...safeUrls.absolute, ...unsafeUrls])('sanitizes URL %s', url => {
+      const htmlHref = `<svg><use href="${url}"></use></svg>`;
+      const htmlXlink = `<svg><use xlink:href="${url}"></use></svg>`;
 
-      expect(sanitize(html, { ADD_TAGS: ['use'] })).toBe('<svg><use></use></svg>');
+      expect(sanitize(htmlHref)).toBe(expectedSanitized);
+      expect(sanitize(htmlXlink)).toBe(expectedSanitized);
     });
   });
 });