You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/24 23:16:13 UTC

(superset) branch master updated: fix(safe-markdown): do not mutate the shared sanitization schema (#41298)

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 4519a5c52d8 fix(safe-markdown): do not mutate the shared sanitization schema (#41298)
4519a5c52d8 is described below

commit 4519a5c52d8f4e1545723f10e7e1962c0b9b2ea6
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Wed Jun 24 16:16:06 2026 -0700

    fix(safe-markdown): do not mutate the shared sanitization schema (#41298)
    
    Co-authored-by: Amin Ghadersohi <am...@gmail.com>
    Co-authored-by: Claude Fable 5 <no...@anthropic.com>
---
 .../src/components/SafeMarkdown/SafeMarkdown.tsx   | 13 +++++++--
 .../test/components/SafeMarkdown.test.tsx          | 32 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/superset-frontend/packages/superset-ui-core/src/components/SafeMarkdown/SafeMarkdown.tsx b/superset-frontend/packages/superset-ui-core/src/components/SafeMarkdown/SafeMarkdown.tsx
index 5a21225a039..e990d2a329b 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/SafeMarkdown/SafeMarkdown.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/components/SafeMarkdown/SafeMarkdown.tsx
@@ -22,7 +22,7 @@ import rehypeSanitize, { defaultSchema } from 'rehype-sanitize';
 // remark-gfm v4+ requires react-markdown v9+, which requires React 18.
 // Currently pinned to v3.0.1 for compatibility with react-markdown v8 and React 17.
 import remarkGfm from 'remark-gfm';
-import { mergeWith } from 'lodash';
+import { cloneDeep, mergeWith } from 'lodash';
 import { FeatureFlag, isFeatureEnabled } from '../../utils';
 
 interface SafeMarkdownProps {
@@ -85,8 +85,15 @@ export function getOverrideHtmlSchema(
   originalSchema: typeof defaultSchema,
   htmlSchemaOverrides: SafeMarkdownProps['htmlSchemaOverrides'],
 ) {
-  return mergeWith(originalSchema, htmlSchemaOverrides, (objValue, srcValue) =>
-    Array.isArray(objValue) ? objValue.concat(srcValue) : undefined,
+  // Merge into a fresh clone: mergeWith mutates its first argument, and the
+  // array customizer concatenates, so merging into the shared defaultSchema
+  // import would progressively widen the sanitization allowlist for every
+  // SafeMarkdown instance app-wide.
+  return mergeWith(
+    cloneDeep(originalSchema),
+    htmlSchemaOverrides,
+    (objValue, srcValue) =>
+      Array.isArray(objValue) ? objValue.concat(srcValue) : undefined,
   );
 }
 
diff --git a/superset-frontend/packages/superset-ui-core/test/components/SafeMarkdown.test.tsx b/superset-frontend/packages/superset-ui-core/test/components/SafeMarkdown.test.tsx
index e736c32699d..d3209e6907c 100644
--- a/superset-frontend/packages/superset-ui-core/test/components/SafeMarkdown.test.tsx
+++ b/superset-frontend/packages/superset-ui-core/test/components/SafeMarkdown.test.tsx
@@ -17,6 +17,8 @@
  * under the License.
  */
 import { render } from '@testing-library/react';
+import { cloneDeep } from 'lodash';
+import { defaultSchema } from 'rehype-sanitize';
 import {
   getOverrideHtmlSchema,
   SafeMarkdown,
@@ -51,6 +53,36 @@ describe('getOverrideHtmlSchema', () => {
     expect(result.attributes).toEqual({ '*': ['size', 'src'], h1: ['style'] });
     expect(result.tagNames).toEqual(['h1', 'h2', 'h3', 'iframe']);
   });
+
+  test('should not mutate the original schema', () => {
+    const original = {
+      attributes: { '*': ['size'] },
+      tagNames: ['h1'],
+    };
+    getOverrideHtmlSchema(original, {
+      attributes: { '*': ['src'] },
+      tagNames: ['iframe'],
+    });
+    // The original passed in is left untouched.
+    expect(original.attributes).toEqual({ '*': ['size'] });
+    expect(original.tagNames).toEqual(['h1']);
+  });
+
+  test('should not mutate the shared defaultSchema import or accumulate across calls', () => {
+    const snapshot = cloneDeep(defaultSchema);
+    const overrides = { tagNames: ['iframe'] };
+
+    const first = getOverrideHtmlSchema(defaultSchema, overrides);
+    const second = getOverrideHtmlSchema(defaultSchema, overrides);
+
+    // The shared singleton is never modified...
+    expect(defaultSchema).toEqual(snapshot);
+    // ...and repeated calls do not accumulate the override (no growing arrays).
+    expect(first.tagNames).toEqual(second.tagNames);
+    expect(
+      (second.tagNames ?? []).filter(name => name === 'iframe'),
+    ).toHaveLength(1);
+  });
 });
 
 describe('transformLinkUri', () => {