Copilot commented on code in PR #36317:
URL: https://github.com/apache/superset/pull/36317#discussion_r2578027023
##########
superset/themes/utils.py:
##########
@@ -120,3 +124,62 @@ def sanitize_theme_tokens(theme_config: Dict[str, Any]) ->
Dict[str, Any]:
sanitized_config["token"] = tokens
return sanitized_config
+
+
+def _validate_single_font_url(index: int, url: Any, allowed_domains:
list[str]) -> str:
+ """
+ Validate a single font URL
+ """
+ if not isinstance(url, str):
+ raise ValidationError(f"fontUrls[{index}] must be a string")
+
+ # Reuse existing sanitize_url (blocks javascript:, data:, etc.)
+ sanitized = sanitize_url(url.strip())
+ if not sanitized:
+ raise ValidationError(f"fontUrls[{index}] contains invalid URL")
+
+ # Enforce HTTPS and validate domain
+ try:
+ parsed = urlparse(sanitized)
+ if parsed.scheme != "https":
+ raise ValidationError(f"fontUrls[{index}] must use HTTPS")
+ if parsed.netloc.lower() not in [d.lower() for d in allowed_domains]:
+ raise ValidationError(
+ f"fontUrls[{index}] domain '{parsed.netloc}' not in allowed
list"
+ )
+ except ValidationError:
+ raise
+ except Exception as e:
+ raise ValidationError(f"fontUrls[{index}] is malformed: {e}") from e
+
+ return sanitized
+
+
+def validate_font_urls(font_urls: Any) -> list[str]:
+ """
+ Validate fontUrls array in theme token configuration
Review Comment:
The docstring for `validate_font_urls` is minimal and doesn't describe the
parameters, return value, or potential exceptions. Consider adding more
complete documentation:
```python
def validate_font_urls(font_urls: Any) -> list[str]:
"""
Validate fontUrls array in theme token configuration.
Performs security validation including:
- Type checking (must be array of strings)
- URL sanitization (blocks javascript:, data: schemes)
- HTTPS enforcement
- Domain whitelisting
- Maximum URL count limit
Args:
font_urls: Font URLs to validate (None, list, or other type)
Returns:
list[str]: List of validated and sanitized font URLs (empty list if
input is None)
Raises:
ValidationError: If validation fails for any reason
"""
```
```suggestion
Validate and sanitize the fontUrls array in theme token configuration.
Performs security validation including:
- Type checking (must be a list of strings)
- URL sanitization (blocks javascript:, data: schemes)
- HTTPS enforcement
- Domain whitelisting
- Maximum URL count limit
Args:
font_urls (Any): Font URLs to validate (None, list, or other type).
Returns:
list[str]: List of validated and sanitized font URLs (empty list if
input is None).
Raises:
ValidationError: If validation fails for any reason, such as:
- font_urls is not a list
- Any URL is not a string
- Any URL is invalid, not HTTPS, or not in the allowed domains
- The number of URLs exceeds the configured maximum
```
##########
superset/themes/utils.py:
##########
@@ -120,3 +124,62 @@ def sanitize_theme_tokens(theme_config: Dict[str, Any]) ->
Dict[str, Any]:
sanitized_config["token"] = tokens
return sanitized_config
+
+
+def _validate_single_font_url(index: int, url: Any, allowed_domains:
list[str]) -> str:
+ """
+ Validate a single font URL
Review Comment:
The docstring for `_validate_single_font_url` is minimal and doesn't
describe the parameters, return value, or potential exceptions. Consider adding
more complete documentation:
```python
def _validate_single_font_url(index: int, url: Any, allowed_domains:
list[str]) -> str:
"""
Validate a single font URL for security and domain whitelisting.
Args:
index: The position of the URL in the fontUrls array (for error
messages)
url: The URL to validate (expected to be a string)
allowed_domains: List of allowed domain names for font URLs
Returns:
str: The sanitized URL
Raises:
ValidationError: If URL is invalid, uses non-HTTPS scheme, or domain
not allowed
"""
```
```suggestion
Validate a single font URL for security and domain whitelisting.
Args:
index: The position of the URL in the fontUrls array (for error
messages)
url: The URL to validate (expected to be a string)
allowed_domains: List of allowed domain names for font URLs
Returns:
str: The sanitized URL
Raises:
ValidationError: If URL is invalid, uses non-HTTPS scheme, or domain
not allowed
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
