[eschutho]

User description

Summary

This PR fixes subdirectory deployments in Apache Superset by addressing two key issues:

1. App Icon Subdirectory Support

• Fixes APP_ICON path to include subdirectory prefix when APPLICATION_ROOT is not "/"
• Automatically prefixes /static/ paths with the configured subdirectory
• Preserves custom icon paths that don't start with /static/

2. Alert/Report Screenshot URL Generation

• Fixes alert and report screenshot generation in subdirectory deployments
• Ensures URLs generated by Celery tasks include the proper subdirectory prefix
• Prevents screenshot failures when Superset is deployed in subdirectories

Problem

When Superset is deployed in a subdirectory (e.g., https://example.com/superset/), two features don't work correctly:

1. App icon (APP_ICON) uses hardcoded /static/ paths that don't work in subdirectory deployments
2. Alert/report screenshots fail because URLs generated by Celery tasks don't include the subdirectory prefix

Solution

App Icon Fix

Added logic in superset/app.py that:

• Automatically prefixes APP_ICON paths that start with /static/
• Only applies the fix for subdirectory deployments (app_root != "/")
• Preserves custom icon paths

Example behavior:

• Default: APP_ICON = "/static/assets/images/logo.png" → unchanged
• Subdirectory: APP_ICON = "/static/assets/images/logo.png" → "/superset/static/assets/images/logo.png"

Alert/Report Screenshot Fix

Enhanced get_url_path() in superset/utils/urls.py to:

• Include APPLICATION_ROOT prefix when generating URLs outside request context
• Fix screenshot URL generation for both charts and dashboards in Celery tasks
• Prevent double-prefixing edge cases

Example behavior:

• Default: Dashboard URL → http://localhost:8088/dashboard/123/
• Subdirectory: Dashboard URL → http://localhost:8088/superset/dashboard/123/

Test Plan

• App icon tests: Verified path generation for various deployment configurations
• Screenshot URL tests: Verified URL generation includes subdirectory prefixes
• Middleware tests: Confirmed AppRootMiddleware functionality
• Edge case tests: Various subdirectory names, double-prefix prevention
• Regression tests: Default deployments remain unchanged
• Pre-commit validation: All linting, formatting, and type checking passes

Additional Context

Files Changed:

• superset/app.py - App icon subdirectory fix
• superset/utils/urls.py - Screenshot URL generation fix
• tests/integration_tests/test_subdirectory_deployments.py - Comprehensive test coverage

Test Coverage:

• App icon configuration for default and subdirectory deployments
• URL generation for charts and dashboards in various deployment scenarios
• AppRootMiddleware path handling and 404 responses
• Environment variable handling and configuration precedence

This change follows existing patterns in Superset for subdirectory deployment support and ensures alerts, reports, and UI elements work correctly in all deployment scenarios.

🤖 Generated with Claude Code

---

CodeAnt-AI Description

Fix subdirectory deployments for app icon, SQL Lab links, and background URLs

What Changed

• UI links and buttons that open, copy, or navigate to SQL Lab, saved queries, datasources, and exports now include the configured subdirectory prefix so opening SQL Lab, copying saved-query links, and CSV exports work when Superset is served under a subpath.
• Background URL generation used by headless tasks (e.g., alert/report screenshots) now includes APPLICATION_ROOT when running outside a request context, preventing invalid URLs from Celery tasks.
• The application icon and theme brand logo URLs/hrefs are prefixed with the subdirectory for deployments using default /static/ paths, ensuring the logo loads and the header link points to the correct root.
• Tests added for path utilities and WSGI middleware to verify subdirectory handling and prevent regressions.

Impact

✅ App icon loads in subdirectory deployments
✅ Fewer failed alert/report screenshots in subdirectory deployments
✅ SQL Lab navigation, saved-query links, and exports work under subpath

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Fixes #35027
Fixes #34773

[korbit-ai]

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Potential Double Slash in Swagger Redirect URL ▹ view	✅ Fix detected

Files scanned

​

File Path	Reviewed
superset/app.py	✅
superset/views/core.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

[dpgaspar]

@eschutho I approved your flask-appbuilder PR, it's probably better

[eschutho]

@eschutho I approved your flask-appbuilder PR, it's probably better

Thanks! I removed the subdirectory piece and added in alerts/reports urls here.

[korbit-ai]

I've completed my review and didn't find any issues.

Files scanned

​

File Path	Reviewed
superset/utils/urls.py	✅
superset/app.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

[codeant-ai-for-open-source]

CodeAnt AI is reviewing your PR.

[codeant-ai-for-open-source]

Suggestion: Security (reverse tabnabbing): opening a URL with window.open(url) without specifying a target and opener protection allows the opened page to access window.opener and potentially redirect the original page. Use window.open with target '_blank' and prevent access to window.opener by passing window features (e.g., 'noopener,noreferrer') so the newly opened page cannot manipulate the opener. [security]

Severity Level: Critical 🚨

Suggested change

	window.open(url);
	window.open(url, '_blank', 'noopener,noreferrer');

Why it matters? ⭐

The existing code calls window.open(url) which can expose the opener to the newly opened page in some browsers. The suggested change (adding '_blank' and 'noopener,noreferrer') prevents reverse tabnabbing by ensuring the opened window can't access or manipulate window.opener. This is a real security hardening (not just stylistic) and is safe and self-contained in this PR context.

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset-frontend/src/SqlLab/components/QueryTable/index.tsx
**Line:** 73:73
**Comment:**
	*Security: Security (reverse tabnabbing): opening a URL with window.open(url) without specifying a target and opener protection allows the opened page to access window.opener and potentially redirect the original page. Use window.open with target '_blank' and prevent access to window.opener by passing window features (e.g., 'noopener,noreferrer') so the newly opened page cannot manipulate the opener.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

[codeant-ai-for-open-source]

Suggestion: Path injection / malformed-URL risk: clientId is interpolated directly into the path passed to makeUrl. If clientId contains unexpected characters (slashes, spaces, query chars), the resulting URL can be malformed or allow unintentional path manipulation. Apply URL-encoding to clientId (encodeURIComponent) before building the URL. [security]

Severity Level: Critical 🚨

Suggested change

	makeUrl(`/api/v1/sqllab/export/${clientId}/`);
	makeUrl(`/api/v1/sqllab/export/${encodeURIComponent(clientId)}/`);

Why it matters? ⭐

URL-encoding the clientId is a small, useful hardening: if clientId ever contains characters like slashes, spaces or query chars it could produce a malformed path. The current code interpolates raw input into a path; encodeURIComponent(clientId) is safe and doesn't change expected behavior for normal IDs (UUIDs/numeric/nanoid) while preventing accidental path manipulation. This is a real, low-risk security/robustness improvement.

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset-frontend/src/SqlLab/components/ResultSet/index.tsx
**Line:** 318:318
**Comment:**
	*Security: Path injection / malformed-URL risk: `clientId` is interpolated directly into the path passed to `makeUrl`. If `clientId` contains unexpected characters (slashes, spaces, query chars), the resulting URL can be malformed or allow unintentional path manipulation. Apply URL-encoding to `clientId` (encodeURIComponent) before building the URL.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

[codeant-ai-for-open-source]

Suggestion: The URL constructed for opening SQL Lab includes datasource raw; if datasource contains characters that must be URL-encoded the resulting URL can be malformed. Encode datasource with encodeURIComponent before interpolating it into the query string passed to makeUrl(...). [possible bug]

Severity Level: Critical 🚨

Suggested change

	`/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`,
	`/sqllab?datasourceKey=${encodeURIComponent(datasource ?? '')}&sql=${encodeURIComponent(currentSQL)}`,

Why it matters? ⭐

This is a straightforward bug: interpolating datasource directly into a query string can break URLs if it contains characters like '&', '?', or '#'. Using encodeURIComponent(datasource ?? '') is the correct and minimal fix to avoid malformed URLs and potential parameter injection.

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset-frontend/src/explore/components/controls/ViewQuery.tsx
**Line:** 142:142
**Comment:**
	*Possible Bug: The URL constructed for opening SQL Lab includes `datasource` raw; if `datasource` contains characters that must be URL-encoded the resulting URL can be malformed. Encode `datasource` with `encodeURIComponent` before interpolating it into the query string passed to `makeUrl(...)`.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

[codeant-ai-for-open-source]

Suggestion: The create redirect '/chart/add' is a raw path and will not include the application root for subdirectory deployments; wrap it with makeUrl so the navigation URL is correct when the app is served from a subdirectory. [logic error]

Severity Level: Minor ⚠️

Suggested change

	[WelcomeTable.Charts]: '/chart/add',
	[WelcomeTable.Charts]: makeUrl('/chart/add'),

Why it matters? ⭐

Using a raw '/chart/add' path will break when the app is served from a subdirectory. Converting to makeUrl('/chart/add') ensures the application root is prepended and is consistent with how SavedQueries.create is handled.

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset-frontend/src/features/home/EmptyState.tsx
**Line:** 60:60
**Comment:**
	*Logic Error: The create redirect '/chart/add' is a raw path and will not include the application root for subdirectory deployments; wrap it with `makeUrl` so the navigation URL is correct when the app is served from a subdirectory.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

[codeant-ai-for-open-source]

Suggestion: The new import added makeUrl but if you revert the menu.url change (as suggested) makeUrl becomes unused. Keep imports minimal to avoid unused-import lint errors and confusion: only import ensureAppRoot here since other code in this file calls ensureAppRoot directly. [code quality]

Severity Level: Minor ⚠️

Suggested change

	import { ensureAppRoot, makeUrl } from 'src/utils/pathUtils';
	import { ensureAppRoot } from 'src/utils/pathUtils';

Why it matters? ⭐

This is a valid code-quality suggestion but conditional: as the file currently stands makeUrl is used (menu.url uses makeUrl). If you apply suggestion #1 (revert menu.url to the raw route), then makeUrl will become unused and should be removed to avoid lint errors and confusion. Removing the unused import is a small, correct cleanup.

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset-frontend/src/features/home/RightMenu.tsx
**Line:** 36:36
**Comment:**
	*Code Quality: The new import added `makeUrl` but if you revert the menu.url change (as suggested) `makeUrl` becomes unused. Keep imports minimal to avoid unused-import lint errors and confusion: only import `ensureAppRoot` here since other code in this file calls `ensureAppRoot` directly.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

[codeant-ai-for-open-source]

Suggestion: Using makeUrl to pre-prefix the SQL query URL stores an application-root-prefixed path into menu.url. Later code expects menu.url to be a raw route and calls ensureAppRoot(menu.url) when rendering external links (or passes it to isFrontendRoute). Because ensureAppRoot always prefixes the application root, this causes double-prefixing (e.g. '/superset/superset/sqllab...') or incorrect frontend-route detection in subdirectory deployments. Use the raw route here (e.g., '/sqllab?new=true') and let the existing rendering logic call ensureAppRoot where needed. [logic error]

Severity Level: Minor ⚠️

Suggested change

	icon: <Icons.SearchOutlined data-test={`menu-item-${t('SQL query')}`} />,
	url: '/sqllab?new=true',

Why it matters? ⭐

The suggestion is correct and fixes a real logic bug. makeUrl() is just a thin wrapper that calls ensureAppRoot(path) (see src/utils/pathUtils.ts where makeUrl returns ensureAppRoot(path)). In RightMenu.buildNewDropdownItems the code later calls ensureAppRoot(menu.url) when rendering non-frontend links (and also passes menu.url to isFrontendRoute). If menu.url is already prefixed via makeUrl, calling ensureAppRoot again double-prefixes the path (e.g. '/superset/sqllab' -> '/superset/superset/sqllab') and can also break frontend-route detection. I verified makeUrl usage and implementation (pathUtils.ts) and the RightMenu rendering sites that wrap menu.url with ensureAppRoot, so keeping the raw route ('/sqllab?new=true') here is the correct fix.

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset-frontend/src/features/home/RightMenu.tsx
**Line:** 205:205
**Comment:**
	*Logic Error: Using makeUrl to pre-prefix the SQL query URL stores an application-root-prefixed path into `menu.url`. Later code expects `menu.url` to be a raw route and calls `ensureAppRoot(menu.url)` when rendering external links (or passes it to `isFrontendRoute`). Because `ensureAppRoot` always prefixes the application root, this causes double-prefixing (e.g. '/superset/superset/sqllab...') or incorrect frontend-route detection in subdirectory deployments. Use the raw route here (e.g., '/sqllab?new=true') and let the existing rendering logic call `ensureAppRoot` where needed.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

[codeant-ai-for-open-source]

Nitpicks 🔍

🔒 No security issues identified

⚡ Recommended areas for review Reverse tabnabbing The code opens a new tab with window.open(makeUrl(...), '_blank') without using noopener/noreferrer or nulling window.opener. This can allow the opened page to access and manipulate the opener window (reverse tabnabbing). Consider mitigating by using noopener/noreferrer or setting opener to null after opening. Unsafe type assumptions The patch assumes app.config["THEME_DEFAULT"] / THEME_DARK are mappings and that theme.get("token", {}) returns a mapping with .get. If theme values are None or not dict-like, calling .get will raise. Validate types before using .get and before mutating nested structures. Double-prefix / double-slash risk The current concatenation can produce '//' or duplicate subdirectory prefixes when applicationRoot() ends with '/' or when the caller already passes a path that already includes the app root. This may lead to incorrect URLs or broken static asset references in subdirectory deployments. Prefix boundary check The code checks not url.startswith(app_root) to avoid double-prefixing, but startswith can yield false positives (e.g. app_root='/superset' and url='/superset2/...'), causing incorrect behavior. Validate prefix boundaries precisely (match exact segment or trailing slash) to avoid skipping needed prefixes or skipping when unrelated paths share the same initial substring. Full URLs / protocol-relative URLs ensureAppRoot / makeUrl always prefix the provided path. If a full URL (e.g. 'http://...', 'https://...', or protocol-relative '//...') or a data URI is passed, it should not be prefixed. Otherwise external links will be corrupted. Normalization of APPLICATION_ROOT app_root is used with rstrip("/") later but the code does not normalize leading/trailing slashes before checks. If APPLICATION_ROOT is configured with or without a leading/trailing slash (e.g., 'superset' vs '/superset/'), behavior may be inconsistent. Ensure app_root is normalized (leading slash present, no trailing slash) before checks/concatenation. Path normalization The code concatenates app_root and paths (e.g. app.config['APP_ICON'], theme token URLs) without normalizing app_root. If app_root contains or lacks leading/trailing slashes unexpectedly (e.g. "superset", "/superset/", "/superset"), this can produce double slashes or missing slashes in generated URLs. Normalize app_root (ensure leading slash, remove trailing slash) before using it for prefixing. Import side-effects The new top-level import of makeUrl brings in src/utils/pathUtils at module load time. That module may read bootstrap data from the DOM (or perform other side effects) during import which can cause test flakiness or runtime errors (SSR or widget initialization ordering). Consider lazy/dynamic importing or otherwise avoiding side-effectful module initialization at module scope.

[codeant-ai-for-open-source]

CodeAnt AI finished reviewing your PR.