This is an automated email from the ASF dual-hosted git repository. beto pushed a commit to branch url-redirect in repository https://gitbox.apache.org/repos/asf/superset.git
commit 073f965cb50b688edd5bf0fe50c35af4380dce7f Author: Beto Dealmeida <[email protected]> AuthorDate: Wed Dec 17 14:55:54 2025 -0500 Fixes --- superset/utils/link_redirect.py | 19 +++++++++++++++---- superset/views/redirect.py | 9 +++++++-- tests/integration_tests/security_tests.py | 1 + tests/unit_tests/utils/test_link_redirect.py | 1 + 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/superset/utils/link_redirect.py b/superset/utils/link_redirect.py index f2d078d5e3..f6b6ce7959 100644 --- a/superset/utils/link_redirect.py +++ b/superset/utils/link_redirect.py @@ -67,18 +67,21 @@ def _get_redirect_url(original_url: str, base_url: str) -> str: return f"{base}/redirect/?url={quote(original_url, safe='')}" -def _process_link_element(link: Any, base_host: str, base_url: str) -> None: +def _process_link_element(link: Any, base_host: str, base_url: str) -> bool: """Process a single link element for external URL redirection.""" original_url = link["href"].strip() if not original_url or "/redirect?" in original_url: - return + return False + + modified = False # Handle protocol-relative URLs (e.g., //evil.com/foo) if original_url.startswith("//"): # Convert to https for processing original_url = "https:" + original_url link["href"] = original_url + modified = True # Parse the URL parsed_url = urlparse(original_url) @@ -91,6 +94,7 @@ def _process_link_element(link: Any, base_host: str, base_url: str) -> None: if link_host and link_host.lower() != base_host.lower(): redirect_url = _get_redirect_url(original_url, base_url) link["href"] = redirect_url + modified = True # Optionally add a visual indicator if current_app.config.get("ALERT_REPORTS_EXTERNAL_LINK_INDICATOR", True): @@ -98,8 +102,11 @@ def _process_link_element(link: Any, base_host: str, base_url: str) -> None: existing_class = link.get("class", []) if isinstance(existing_class, str): existing_class = existing_class.split() - existing_class.append("external-link") + if "external-link" not in existing_class: + existing_class.append("external-link") + modified = True link["class"] = existing_class + return modified def process_html_links(html_content: str, base_url: Optional[str] = None) -> str: @@ -135,8 +142,12 @@ def process_html_links(html_content: str, base_url: Optional[str] = None) -> str return html_content # Find all anchor tags with href attribute and process them + modified = False for link in soup.find_all("a", href=True): - _process_link_element(link, base_host, resolved_base_url) + modified |= _process_link_element(link, base_host, resolved_base_url) + + if not modified: + return html_content # Return the modified HTML return str(soup) diff --git a/superset/views/redirect.py b/superset/views/redirect.py index e9df40a8d4..42162a9619 100644 --- a/superset/views/redirect.py +++ b/superset/views/redirect.py @@ -16,6 +16,7 @@ # under the License. import logging +from typing import Any from urllib.parse import unquote, urlparse from flask import abort, redirect, request @@ -29,7 +30,7 @@ from superset.views.base import BaseSupersetView logger = logging.getLogger(__name__) -DANGEROUS_SCHEMES = ("javascript:", "data:", "vbscript:", "file:") +DANGEROUS_SCHEMES: tuple[str, ...] = ("javascript:", "data:", "vbscript:", "file:") def _normalize_url(url: str) -> str: @@ -71,7 +72,11 @@ def _is_url_in_trusted_cookie(target_url: str) -> bool: """Check if the URL is in the trusted URLs cookie.""" try: trusted_urls_cookie = request.cookies.get("superset_trusted_urls", "[]") - trusted_urls = json.loads(trusted_urls_cookie) + trusted_urls_raw: Any = json.loads(trusted_urls_cookie) + if not isinstance(trusted_urls_raw, list): + return False + + trusted_urls = [url for url in trusted_urls_raw if isinstance(url, str)] normalized_target = _normalize_url(target_url) return any( diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 96b2cb3e20..6fdc1704b4 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1562,6 +1562,7 @@ class TestRolePermission(SupersetTestCase): ["SupersetAuthView", "logout"], ["SupersetRegisterUserView", "register"], ["SupersetRegisterUserView", "activation"], + ["RedirectView", "redirect_warning"], ] unsecured_views = [] for view_class in appbuilder.baseviews: diff --git a/tests/unit_tests/utils/test_link_redirect.py b/tests/unit_tests/utils/test_link_redirect.py index 224e90eb39..48636e1cf0 100644 --- a/tests/unit_tests/utils/test_link_redirect.py +++ b/tests/unit_tests/utils/test_link_redirect.py @@ -293,6 +293,7 @@ def test_is_safe_redirect_url_no_base_configured(app): def test_process_html_links_invalid_base_url(app): """Test behavior with invalid base URL""" app.config["WEBDRIVER_BASEURL"] = "not-a-valid-url" + app.config["WEBDRIVER_BASEURL_USER_FRIENDLY"] = "not-a-valid-url" with app.app_context(): html = '<a href="https://external.com";>External</a>'
