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(, { + 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(, { + 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(, { + 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(, { + 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:user@example.com'; +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 = `
`; + 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,

xss

`; + 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);