villebro commented on a change in pull request #11704:
URL:
https://github.com/apache/incubator-superset/pull/11704#discussion_r523928392
##########
File path: superset/config.py
##########
@@ -677,6 +677,10 @@ class CeleryConfig: # pylint:
disable=too-few-public-methods
# language. This allows you to define custom logic to process macro template.
CUSTOM_TEMPLATE_PROCESSORS: Dict[str, Type[BaseTemplateProcessor]] = {}
+# Prevent access to classes/objects and proxy methods in the default Jinja
context,
+# unless explicitly overridden by JINJA_CONTEXT_ADDONS or
CUSTOM_TEMPLATE_PROCESSORS.
+SAFE_JINJA_PROCESSING: bool = True
Review comment:
Would it make sense to introduce a `TEMPLATE_PROCESSOR` parameter that
accepts `TemplateProcessorEnum` values, something like
```python
TemplateProcessorEnum(enum.Enum):
SafeJinja = 1
LegacyJinja = 2
Chevron = 3
Custom = 4
```
In this approach, `LegacyJinja` would include the old `datetime`, `random`
etc base context, and `SafeJinja` would have a more limited set.
##########
File path: superset/jinja_context.py
##########
@@ -186,6 +188,28 @@ def url_param(
return result
+def safe_proxy(func: Callable[..., Any], *args: Any, **kwargs: Any) -> Any:
+ none_type = type(None).__name__
+ allowed_types = [
+ none_type,
+ "bool",
+ "str",
+ "unicode",
+ "int",
+ "long",
+ "float",
+ "list",
+ "dict",
+ "tuple",
+ ]
+ return_value = func(*args, **kwargs)
+ value_type = type(return_value).__name__
+ if value_type not in allowed_types:
+ raise SupersetTemplateException("Unsafe template value")
Review comment:
Would be nice to get a more verbose error here, something like
```python
raise SupersetTemplateException(__("Unsafe return type for function
%(func)s: %(value_type)s", func=func.__name__, value_type=value_type)
```
##########
File path: superset/jinja_context.py
##########
@@ -186,6 +188,28 @@ def url_param(
return result
+def safe_proxy(func: Callable[..., Any], *args: Any, **kwargs: Any) -> Any:
Review comment:
I like this approach 👍
##########
File path: superset/jinja_context.py
##########
@@ -186,6 +188,28 @@ def url_param(
return result
+def safe_proxy(func: Callable[..., Any], *args: Any, **kwargs: Any) -> Any:
+ none_type = type(None).__name__
+ allowed_types = [
+ none_type,
+ "bool",
+ "str",
+ "unicode",
Review comment:
nit: I'm pretty sure `unicode` has been replaced by `str` in Python 3.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
