This is an automated email from the ASF dual-hosted git repository.
jli 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 6c359733e1d fix(frontend): preserve absolute and protocol-relative
URLs in ensureAppRoot (#38316)
6c359733e1d is described below
commit 6c359733e1d8d009f142802e0a179e6a59a4eb77
Author: Joe Li <[email protected]>
AuthorDate: Thu Mar 5 14:06:16 2026 -0800
fix(frontend): preserve absolute and protocol-relative URLs in
ensureAppRoot (#38316)
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
---
superset-frontend/src/features/home/Menu.test.tsx | 115 +++++++++++++++++++++-
superset-frontend/src/utils/pathUtils.test.ts | 71 +++++++++++++
superset-frontend/src/utils/pathUtils.ts | 37 +++++--
3 files changed, 213 insertions(+), 10 deletions(-)
diff --git a/superset-frontend/src/features/home/Menu.test.tsx
b/superset-frontend/src/features/home/Menu.test.tsx
index 4b7e744188e..2c07963c7b7 100644
--- a/superset-frontend/src/features/home/Menu.test.tsx
+++ b/superset-frontend/src/features/home/Menu.test.tsx
@@ -21,9 +21,15 @@ import fetchMock from 'fetch-mock';
import { render, screen, userEvent } from 'spec/helpers/testing-library';
import setupCodeOverrides from 'src/setup/setupCodeOverrides';
import { getExtensionsRegistry } from '@superset-ui/core';
+import * as CoreUI from '@apache-superset/core/ui';
import { Menu } from './Menu';
import * as getBootstrapData from 'src/utils/getBootstrapData';
+jest.mock('@apache-superset/core/ui', () => ({
+ ...jest.requireActual('@apache-superset/core/ui'),
+ useTheme: jest.fn(),
+}));
+
const dropdownItems = [
{
label: 'Data',
@@ -243,6 +249,8 @@ const staticAssetsPrefixMock = jest.spyOn(
getBootstrapData,
'staticAssetsPrefix',
);
+const applicationRootMock = jest.spyOn(getBootstrapData, 'applicationRoot');
+const useThemeMock = CoreUI.useTheme as jest.Mock;
fetchMock.get(
'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))',
@@ -252,8 +260,11 @@ fetchMock.get(
beforeEach(() => {
// setup a DOM element as a render target
useSelectorMock.mockClear();
- // By default use empty static assets prefix
+ // By default use empty static assets prefix and default app root
staticAssetsPrefixMock.mockReturnValue('');
+ applicationRootMock.mockReturnValue('');
+ // By default useTheme returns the real default theme (brandLogoUrl is falsy)
+ useThemeMock.mockReturnValue(CoreUI.supersetTheme);
});
test('should render', async () => {
@@ -675,3 +686,105 @@ test('should not render the brand text if not available',
async () => {
const brandText = screen.queryByText(text);
expect(brandText).not.toBeInTheDocument();
});
+
+test('brand logo href should not be prefixed with app root when brandLogoHref
is an absolute URL', async () => {
+ applicationRootMock.mockReturnValue('/superset');
+ useThemeMock.mockReturnValue({
+ ...CoreUI.supersetTheme,
+ brandLogoUrl: '/static/assets/images/custom-logo.png',
+ brandLogoHref: 'https://external.example.com',
+ });
+ useSelectorMock.mockReturnValue({ roles: user.roles });
+
+ render(<Menu {...mockedProps} />, {
+ useRedux: true,
+ useQueryParams: true,
+ useRouter: true,
+ useTheme: true,
+ });
+
+ const brandLink = await screen.findByRole('link', {
+ name: /apache superset/i,
+ });
+ expect(brandLink).toHaveAttribute('href', 'https://external.example.com');
+});
+
+test('brand logo href should not be prefixed with app root when brandLogoHref
is protocol-relative', async () => {
+ applicationRootMock.mockReturnValue('/superset');
+ useThemeMock.mockReturnValue({
+ ...CoreUI.supersetTheme,
+ brandLogoUrl: '/static/assets/images/custom-logo.png',
+ brandLogoHref: '//external.example.com',
+ });
+ useSelectorMock.mockReturnValue({ roles: user.roles });
+
+ render(<Menu {...mockedProps} />, {
+ useRedux: true,
+ useQueryParams: true,
+ useRouter: true,
+ useTheme: true,
+ });
+
+ const brandLink = await screen.findByRole('link', {
+ name: /apache superset/i,
+ });
+ expect(brandLink).toHaveAttribute('href', '//external.example.com');
+});
+
+test('brand path should be prefixed with app root in subdirectory deployment',
async () => {
+ applicationRootMock.mockReturnValue('/superset');
+ useSelectorMock.mockReturnValue({ roles: user.roles });
+
+ const propsWithSimplePath = {
+ ...mockedProps,
+ data: {
+ ...mockedProps.data,
+ brand: {
+ ...mockedProps.data.brand,
+ path: '/welcome/',
+ },
+ },
+ };
+
+ render(<Menu {...propsWithSimplePath} />, {
+ useRedux: true,
+ useQueryParams: true,
+ useRouter: true,
+ useTheme: true,
+ });
+
+ const brandLink = await screen.findByRole('link', {
+ name: new RegExp(propsWithSimplePath.data.brand.alt, 'i'),
+ });
+ expect(brandLink).toHaveAttribute('href', '/superset/welcome/');
+});
+
+test('brand link falls back to brand.path when theme brandLogoUrl is absent',
async () => {
+ // useThemeMock default returns supersetTheme with brandLogoUrl undefined
(falsy)
+ applicationRootMock.mockReturnValue('/superset');
+ useSelectorMock.mockReturnValue({ roles: user.roles });
+
+ const propsWithFallbackPath = {
+ ...mockedProps,
+ data: {
+ ...mockedProps.data,
+ brand: {
+ ...mockedProps.data.brand,
+ path: '/welcome/',
+ },
+ },
+ };
+
+ render(<Menu {...propsWithFallbackPath} />, {
+ useRedux: true,
+ useQueryParams: true,
+ useRouter: true,
+ useTheme: true,
+ });
+
+ const brandLink = await screen.findByRole('link', {
+ name: new RegExp(propsWithFallbackPath.data.brand.alt, 'i'),
+ });
+ // ensureAppRoot must have been applied: /welcome/ → /superset/welcome/
+ expect(brandLink).toHaveAttribute('href', '/superset/welcome/');
+});
diff --git a/superset-frontend/src/utils/pathUtils.test.ts
b/superset-frontend/src/utils/pathUtils.test.ts
index bf24053e671..fd546ff01d5 100644
--- a/superset-frontend/src/utils/pathUtils.test.ts
+++ b/superset-frontend/src/utils/pathUtils.test.ts
@@ -158,3 +158,74 @@ test('makeUrl should handle URLs with anchors', async ()
=> {
'/superset/dashboard/123#anchor',
);
});
+
+// Representative URLs used across the absolute-URL passthrough tests below.
+const HTTPS_URL = 'https://external.example.com';
+const HTTP_URL = 'http://external.example.com';
+const PROTOCOL_RELATIVE_URL = '//external.example.com';
+const FTP_URL = 'ftp://files.example.com/data';
+const MAILTO_URL = 'mailto:[email protected]';
+const TEL_URL = 'tel:+1234567890';
+
+// Sets up bootstrap data and returns a fresh pathUtils module instance.
+// Passing appRoot='' (default) simulates no subdirectory deployment.
+async function loadPathUtils(appRoot = '') {
+ const bootstrapData = { common: { application_root: appRoot } };
+ document.body.innerHTML = `<div id="app"
data-bootstrap='${JSON.stringify(bootstrapData)}'></div>`;
+ jest.resetModules();
+ await import('./getBootstrapData');
+ return import('./pathUtils');
+}
+
+test('ensureAppRoot should preserve absolute and protocol-relative URLs
unchanged with default root', async () => {
+ const { ensureAppRoot } = await loadPathUtils();
+
+ expect(ensureAppRoot(HTTPS_URL)).toBe(HTTPS_URL);
+ expect(ensureAppRoot(HTTP_URL)).toBe(HTTP_URL);
+ expect(ensureAppRoot(PROTOCOL_RELATIVE_URL)).toBe(PROTOCOL_RELATIVE_URL);
+});
+
+test('ensureAppRoot should preserve absolute URLs unchanged with custom
subdirectory', async () => {
+ const { ensureAppRoot } = await loadPathUtils('/superset/');
+
+ expect(ensureAppRoot(HTTPS_URL)).toBe(HTTPS_URL);
+ expect(ensureAppRoot(HTTP_URL)).toBe(HTTP_URL);
+ // Non-http absolute schemes: all safe schemes must pass through
+ expect(ensureAppRoot(FTP_URL)).toBe(FTP_URL);
+ expect(ensureAppRoot(MAILTO_URL)).toBe(MAILTO_URL);
+ expect(ensureAppRoot(TEL_URL)).toBe(TEL_URL);
+});
+
+test('ensureAppRoot should preserve protocol-relative URLs unchanged', async
() => {
+ const { ensureAppRoot } = await loadPathUtils('/superset/');
+
+ expect(ensureAppRoot(PROTOCOL_RELATIVE_URL)).toBe(PROTOCOL_RELATIVE_URL);
+});
+
+test('makeUrl should preserve absolute and protocol-relative URLs unchanged',
async () => {
+ const { makeUrl } = await loadPathUtils('/superset/');
+
+ expect(makeUrl(HTTPS_URL)).toBe(HTTPS_URL);
+ expect(makeUrl(PROTOCOL_RELATIVE_URL)).toBe(PROTOCOL_RELATIVE_URL);
+ // Non-http absolute scheme parity with ensureAppRoot
+ expect(makeUrl(FTP_URL)).toBe(FTP_URL);
+});
+
+test('ensureAppRoot should block javascript: and data: schemes (XSS
prevention)', async () => {
+ const { ensureAppRoot } = await loadPathUtils('/superset/');
+
+ // Dangerous schemes must NOT pass through — they get prefixed to neutralise
them.
+ // Build the literals via concatenation so the linter's no-script-url rule
+ // does not flag this intentional test input.
+ const jsUrl = `${'javascript'}:alert(1)`;
+ const dataUrl = `${'data'}:text/html,<h1>xss</h1>`;
+ expect(ensureAppRoot(jsUrl)).toBe(`/superset/${jsUrl}`);
+ expect(ensureAppRoot(dataUrl)).toBe(`/superset/${dataUrl}`);
+});
+
+test('ensureAppRoot should prefix unknown schemes instead of passing through',
async () => {
+ const { ensureAppRoot } = await loadPathUtils('/superset/');
+
+ // Unknown / custom schemes are treated as relative paths
+ expect(ensureAppRoot('foo:bar')).toBe('/superset/foo:bar');
+});
diff --git a/superset-frontend/src/utils/pathUtils.ts
b/superset-frontend/src/utils/pathUtils.ts
index 0e6cac429f1..870d8aa95ab 100644
--- a/superset-frontend/src/utils/pathUtils.ts
+++ b/superset-frontend/src/utils/pathUtils.ts
@@ -19,25 +19,44 @@
import { applicationRoot } from 'src/utils/getBootstrapData';
/**
- * Takes a string path to a resource and prefixes it with the application root
that is
- * defined in the application configuration. The application path is sanitized.
- * @param path A string path to a resource
+ * Matches safe URI schemes that should pass through without an application
root
+ * prefix. Only well-known schemes are allowed; unknown or dangerous schemes
+ * (e.g. javascript:, data:) are treated as relative paths and prefixed.
+ */
+const SAFE_ABSOLUTE_URL_RE = /^(https?|ftp|mailto|tel):/i;
+
+/**
+ * Takes a string path to a resource and prefixes it with the application root
+ * defined in the application configuration.
+ *
+ * Absolute URLs with safe schemes (e.g. https://..., ftp://..., mailto:...,
+ * tel:...) and protocol-relative URLs (e.g. //example.com) are returned
+ * unchanged — only relative paths receive the application root prefix.
+ * Potentially dangerous schemes such as javascript: and data: are not treated
+ * as absolute and will be prefixed.
+ *
+ * @param path A string path or URL to a resource
*/
export function ensureAppRoot(path: string): string {
+ if (SAFE_ABSOLUTE_URL_RE.test(path) || path.startsWith('//')) {
+ return path;
+ }
return `${applicationRoot()}${path.startsWith('/') ? path : `/${path}`}`;
}
/**
- * Creates a URL with the proper application root prefix for subdirectory
deployments.
- * Use this when constructing URLs for navigation, API calls, or file
downloads.
+ * Creates a URL suitable for navigation, API calls, or file downloads.
Relative
+ * paths are prefixed with the application root for subdirectory deployments.
+ * Absolute URLs (e.g. https://...) and protocol-relative URLs (e.g.
//example.com)
+ * are returned unchanged.
*
- * @param path - The path to convert to a full URL (e.g., '/sqllab',
'/api/v1/chart/123')
- * @returns The path prefixed with the application root (e.g.,
'/superset/sqllab')
+ * @param path - The path or URL to resolve (e.g., '/sqllab',
'https://example.com')
+ * @returns The resolved URL (e.g., '/superset/sqllab' or
'https://example.com')
*
* @example
* // In a subdirectory deployment at /superset
- * makeUrl('/sqllab?new=true') // returns '/superset/sqllab?new=true'
- * makeUrl('/api/v1/chart/export/123/') // returns
'/superset/api/v1/chart/export/123/'
+ * makeUrl('/sqllab?new=true') // returns '/superset/sqllab?new=true'
+ * makeUrl('https://external.example.com') // returns
'https://external.example.com'
*/
export function makeUrl(path: string): string {
return ensureAppRoot(path);
