From 00dd31494d1b83bbdab976dc04dbbd89ebb71d37 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Tue, 2 Jun 2026 11:52:02 -0700 Subject: [PATCH] fix: sanitize URL sinks and trim sensitive log fields (#40546) Co-authored-by: Claude Code --- superset-frontend/package-lock.json | 7 +++ superset-frontend/package.json | 1 + .../packages/superset-ui-core/package.json | 1 + .../src/components/ListViewCard/index.tsx | 3 +- .../src/connection/SupersetClientClass.ts | 3 +- .../src/SqlLab/components/ResultSet/index.tsx | 3 +- .../components/SaveDatasetModal/index.tsx | 6 +-- .../DatasourceEditor/DatasourceEditor.tsx | 3 +- .../src/components/GenericLink/index.tsx | 4 +- .../ExploreChartHeader.test.tsx | 53 ++++++++++--------- .../DatabaseModal/SqlAlchemyForm.tsx | 5 +- .../src/utils/navigationUtils.ts | 11 ++-- superset/mcp_service/storage.py | 4 +- 13 files changed, 65 insertions(+), 39 deletions(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index e75f2d3b984..80443db377d 100644 --- 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", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 1f40e196cae..8b4c5bf3b84 100644 --- 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", diff --git a/superset-frontend/packages/superset-ui-core/package.json b/superset-frontend/packages/superset-ui-core/package.json index 8b552c2dbb6..bf53881dbb8 100644 --- 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", diff --git a/superset-frontend/packages/superset-ui-core/src/components/ListViewCard/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/ListViewCard/index.tsx index e9271d0de6a..eff2c41bc12 100644 --- 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 = ({ to, children }) => ( - {children} + {children} ); function ListViewCard({ diff --git a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts index b5ceb932c21..0018000ab4f 100644 --- 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 = { diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index 8b74c9d29ca..310b8af58e7 100644 --- 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'), diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx index 9037fd42996..a1aa08b277f 100644 --- 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 = { diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx index 58f26f2829c..8bc2b6cf8f3 100644 --- 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 ( css` diff --git a/superset-frontend/src/components/GenericLink/index.tsx b/superset-frontend/src/components/GenericLink/index.tsx index 4a0aba7e0d1..474b174bfd5 100644 --- 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 = ({ }: PropsWithoutRef> & RefAttributes) => { if (typeof to === 'string' && isUrlExternal(to)) { return ( - + {children} ); diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx index 9ad16fcf04b..432de3fc6a4 100644 --- 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 .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'); diff --git a/superset-frontend/src/features/databases/DatabaseModal/SqlAlchemyForm.tsx b/superset-frontend/src/features/databases/DatabaseModal/SqlAlchemyForm.tsx index 450a55c7d6c..627ad0022a2 100644 --- 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 = ({
{t('Refer to the')}{' '} diff --git a/superset-frontend/src/utils/navigationUtils.ts b/superset-frontend/src/utils/navigationUtils.ts index 11606aa7e3d..4efb172ed9c 100644 --- 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)); } }; diff --git a/superset/mcp_service/storage.py b/superset/mcp_service/storage.py index b4c06a2758a..542485b5e1f 100644 --- 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)