fix: sanitize URL sinks and trim sensitive log fields (#40546)

Co-authored-by: Claude Code <noreply@anthropic.com>
2026-07-27 09:02:29 +00:00 · 2026-06-02 11:52:02 -07:00
parent b97d3ef520
commit 00dd31494d
13 changed files with 65 additions and 39 deletions
--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
@@ -15,6 +15,7 @@
      ],
      "dependencies": {
        "@apache-superset/core": "file:packages/superset-core",
+        "@braintree/sanitize-url": "^7.1.2",
        "@deck.gl/aggregation-layers": "~9.2.5",
        "@deck.gl/core": "~9.2.5",
        "@deck.gl/extensions": "~9.2.5",
@@ -2679,6 +2680,12 @@
        "url": "https://github.com/sponsors/Borewit"
      }
    },
+    "node_modules/@braintree/sanitize-url": {
+      "version": "7.1.2",
+      "resolved": "https://registry.npmjs.org/@braintree/sanitize-url/-/sanitize-url-7.1.2.tgz",
+      "integrity": "sha512-jigsZK+sMF/cuiB7sERuo9V7N9jx+dhmHHnQyDSVdpZwVutaBu7WvNYqMDLSgFgfB30n452TP3vjDAvFC973mA==",
+      "license": "MIT"
+    },
    "node_modules/@bramus/specificity": {
      "version": "2.4.2",
      "resolved": "https://registry.npmjs.org/@bramus/specificity/-/specificity-2.4.2.tgz",
--- a/superset-frontend/package.json
+++ b/superset-frontend/package.json
@@ -98,6 +98,7 @@
  ],
  "dependencies": {
    "@apache-superset/core": "file:packages/superset-core",
+    "@braintree/sanitize-url": "^7.1.2",
    "@deck.gl/aggregation-layers": "~9.2.5",
    "@deck.gl/core": "~9.2.5",
    "@deck.gl/extensions": "~9.2.5",
--- a/superset-frontend/packages/superset-ui-core/package.json
+++ b/superset-frontend/packages/superset-ui-core/package.json
@@ -27,6 +27,7 @@
    "@ant-design/icons": "^6.2.3",
    "@apache-superset/core": "*",
    "@babel/runtime": "^7.29.7",
+    "@braintree/sanitize-url": "^7.1.2",
    "@types/json-bigint": "^1.0.4",
    "@visx/responsive": "^3.12.0",
    "ace-builds": "^1.44.0",
--- a/superset-frontend/packages/superset-ui-core/src/components/ListViewCard/index.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/components/ListViewCard/index.tsx
@@ -16,6 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import { FC } from 'react';
 import { styled, useTheme, css } from '@apache-superset/core/theme';
 import { Skeleton } from '../Skeleton';
@@ -140,7 +141,7 @@ const ThinSkeleton = styled(Skeleton)`
 const paragraphConfig = { rows: 1, width: 150 };

 const AnchorLink: FC<LinkProps> = ({ to, children }) => (
-  <a href={to}>{children}</a>
+  <a href={to !== undefined ? sanitizeUrl(to) : undefined}>{children}</a>
 );

 function ListViewCard({
--- a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts
+++ b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts
@@ -16,6 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import callApiAndParseWithTimeout from './callApi/callApiAndParseWithTimeout';
 import {
  ClientConfig,
@@ -123,7 +124,7 @@ export default class SupersetClientClass {
    if (endpoint) {
      await this.ensureAuth();
      const hiddenForm = document.createElement('form');
-      hiddenForm.action = this.getUrl({ endpoint });
+      hiddenForm.action = sanitizeUrl(this.getUrl({ endpoint }));
      hiddenForm.method = 'POST';
      hiddenForm.target = target;
      const payloadWithToken: Record<string, any> = {
--- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
+++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
@@ -16,6 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import {
  useCallback,
  useEffect,
@@ -378,7 +379,7 @@ const ResultSet = ({
              { rows: rowsCount.toLocaleString() },
            ),
            onConfirm: () => {
-              window.location.href = getExportCsvUrl(query.id);
+              window.location.href = sanitizeUrl(getExportCsvUrl(query.id));
            },
            confirmText: t('OK'),
            cancelText: t('Close'),
--- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
@@ -16,7 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import { useCallback, useState, FormEvent } from 'react';
 import { ModalTitleWithIcon } from 'src/components/ModalTitleWithIcon';
 import { Radio, RadioChangeEvent } from '@superset-ui/core/components/Radio';
@@ -245,9 +245,9 @@ export const SaveDatasetModal = ({

  const createWindow = (url: string) => {
    if (openWindow) {
-      window.open(url, '_blank', 'noreferrer');
+      window.open(sanitizeUrl(url), '_blank', 'noreferrer');
    } else {
-      window.location.href = url;
+      window.location.href = sanitizeUrl(url);
    }
  };
  const formDataWithDefaults = {
--- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
@@ -16,6 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import rison from 'rison';
 import { PureComponent, useCallback, type ReactNode } from 'react';
 import { connect, ConnectedProps } from 'react-redux';
@@ -1771,7 +1772,7 @@ class DatasourceEditor extends PureComponent<
  renderOpenInSqlLabLink(isError = false) {
    return (
      <a
-        href={this.getSQLLabUrl()}
+        href={sanitizeUrl(this.getSQLLabUrl())}
        target="_blank"
        rel="noopener noreferrer"
        css={theme => css`
--- a/superset-frontend/src/components/GenericLink/index.tsx
+++ b/superset-frontend/src/components/GenericLink/index.tsx
@@ -16,7 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import { PropsWithoutRef, RefAttributes } from 'react';
 import { Link, LinkProps } from 'react-router-dom';
 import { isUrlExternal, parseUrl } from 'src/utils/urlUtils';
@@ -31,7 +31,7 @@ export const GenericLink = <S,>({
 }: PropsWithoutRef<LinkProps<S>> & RefAttributes<HTMLAnchorElement>) => {
  if (typeof to === 'string' && isUrlExternal(to)) {
    return (
-      <a data-test="external-link" href={parseUrl(to)} {...rest}>
+      <a data-test="external-link" href={sanitizeUrl(parseUrl(to))} {...rest}>
        {children}
      </a>
    );
--- a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx
+++ b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx
@@ -29,10 +29,13 @@ import * as chartAction from 'src/components/Chart/chartAction';
 import * as saveModalActions from 'src/explore/actions/saveModalActions';
 import * as downloadAsImage from 'src/utils/downloadAsImage';
 import * as exploreUtils from 'src/explore/exploreUtils';
-import { FeatureFlag, VizType } from '@superset-ui/core';
+import {
+  FeatureFlag,
+  VizType,
+  getChartMetadataRegistry,
+} from '@superset-ui/core';
 import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt';
 import ExploreHeader, { ExploreChartHeaderProps } from '.';
-import { getChartMetadataRegistry } from '@superset-ui/core';
 import fs from 'fs';
 import path from 'path';

@@ -910,23 +913,28 @@ describe('Additional actions tests', () => {
    let spyDownloadAsImage: jest.SpyInstance;
    let spyExportChart: jest.SpyInstance;

-    let originalURL: typeof URL;
    let anchorClickSpy: jest.SpyInstance;
+    let createObjectURLSpy: jest.SpyInstance;
+    let revokeObjectURLSpy: jest.SpyInstance;

    beforeAll(() => {
-      originalURL = global.URL;
-
-      // Replace global.URL with a version that has the blob helpers
-      const mockedURL = {
-        ...originalURL,
-        createObjectURL: jest.fn(() => 'blob:mock-url'),
-        revokeObjectURL: jest.fn(),
-      } as unknown as typeof URL;
-
-      Object.defineProperty(global, 'URL', {
-        writable: true,
-        value: mockedURL,
-      });
+      // jsdom does not define URL.createObjectURL / URL.revokeObjectURL, so we
+      // stub them before spying so that jest.spyOn finds a real property to wrap.
+      if (!URL.createObjectURL) {
+        URL.createObjectURL = () => '';
+      }
+      if (!URL.revokeObjectURL) {
+        URL.revokeObjectURL = () => {};
+      }
+      // Spy on static URL methods rather than replacing the constructor so that
+      // code paths calling `new URL(...)` (e.g. @braintree/sanitize-url) keep
+      // working while tests can assert on blob URL creation/revocation.
+      createObjectURLSpy = jest
+        .spyOn(URL, 'createObjectURL')
+        .mockReturnValue('blob:mock-url');
+      revokeObjectURLSpy = jest
+        .spyOn(URL, 'revokeObjectURL')
+        .mockImplementation(() => {});

      // Avoid jsdom navigation side-effects on <a>.click()
      anchorClickSpy = jest
@@ -935,11 +943,8 @@ describe('Additional actions tests', () => {
    });

    afterAll(() => {
-      // restore URL
-      Object.defineProperty(global, 'URL', {
-        writable: true,
-        value: originalURL,
-      });
+      createObjectURLSpy.mockRestore();
+      revokeObjectURLSpy.mockRestore();
      anchorClickSpy.mockRestore();
    });

@@ -1073,7 +1078,7 @@ describe('Additional actions tests', () => {
      userEvent.click(await screen.findByText('Export to .CSV'));

      expect(spyExportChart.mock.calls.length).toBe(1);
-      const args = spyExportChart.mock.calls[0][0];
+      const [[args]] = spyExportChart.mock.calls;
      expect(args.resultType).toBe('results');
      expect(args.resultFormat).toBe('csv');

@@ -1124,7 +1129,7 @@ describe('Additional actions tests', () => {
      userEvent.click(await screen.findByText(/Export to (Excel|\.XLSX)/i));

      expect(spyExportChart.mock.calls.length).toBe(1);
-      const args = spyExportChart.mock.calls[0][0];
+      const [[args]] = spyExportChart.mock.calls;
      expect(args.resultType).toBe('results');
      expect(args.resultFormat).toBe('xlsx');
      getSpy.mockRestore();
@@ -1162,7 +1167,7 @@ describe('Additional actions tests', () => {
        expect(spyExportChart.mock.calls.length).toBe(1);
      });

-      const args = spyExportChart.mock.calls[0][0];
+      const [[args]] = spyExportChart.mock.calls;
      expect(args.resultType).toBe('results');
      expect(args.resultFormat).toBe('json');

--- a/superset-frontend/src/features/databases/DatabaseModal/SqlAlchemyForm.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/SqlAlchemyForm.tsx
@@ -16,6 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import { EventHandler, ChangeEvent, MouseEvent, ReactNode } from 'react';
 import { t } from '@apache-superset/core/translation';
 import { SupersetTheme } from '@apache-superset/core/theme';
@@ -87,7 +88,9 @@ const SqlAlchemyTab = ({
        <div className="helper">
          {t('Refer to the')}{' '}
          <a
-            href={fallbackDocsUrl || conf?.SQLALCHEMY_DOCS_URL || ''}
+            href={sanitizeUrl(
+              fallbackDocsUrl || conf?.SQLALCHEMY_DOCS_URL || '',
+            )}
            target="_blank"
            rel="noopener noreferrer"
          >
--- a/superset-frontend/src/utils/navigationUtils.ts
+++ b/superset-frontend/src/utils/navigationUtils.ts
@@ -16,6 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import { ensureAppRoot } from './pathUtils';

 export const navigateTo = (
@@ -23,11 +24,15 @@ export const navigateTo = (
  options?: { newWindow?: boolean; assign?: boolean },
 ) => {
  if (options?.newWindow) {
-    window.open(ensureAppRoot(url), '_blank', 'noopener noreferrer');
+    window.open(
+      sanitizeUrl(ensureAppRoot(url)),
+      '_blank',
+      'noopener noreferrer',
+    );
  } else if (options?.assign) {
-    window.location.assign(ensureAppRoot(url));
+    window.location.assign(sanitizeUrl(ensureAppRoot(url)));
  } else {
-    window.location.href = ensureAppRoot(url);
+    window.location.href = sanitizeUrl(ensureAppRoot(url));
  }
 };

--- a/superset/mcp_service/storage.py
+++ b/superset/mcp_service/storage.py
@@ -145,7 +145,7 @@ def _create_redis_store(
                ssl=True,
                ssl_cert_reqs="none",
            )
-            logger.info("Created async Redis client with SSL at %s", parsed.hostname)
+            logger.info("Created async Redis client with SSL")
        else:
            redis_client = Redis(
                host=parsed.hostname or "localhost",
@@ -155,7 +155,7 @@ def _create_redis_store(
                password=parsed.password,
                decode_responses=True,
            )
-            logger.info("Created async Redis client at %s", parsed.hostname)
+            logger.info("Created async Redis client")

        # Pass pre-configured client to RedisStore
        redis_store = RedisStore(client=redis_client)