feat(templating): Safer Jinja template processing

UPDATING.md

@@ -23,6 +23,8 @@ assists people when migrating to a new version.
 
 ## Next
 
+* [11704](https://github.com/apache/incubator-superset/pull/11704) Breaking change: Jinja templating for SQL queries has been updated, removing default modules such as `datetime` and `random` and enforcing static template values. To restore or extend functionality, use `JINJA_CONTEXT_ADDONS` and `CUSTOM_TEMPLATE_PROCESSORS` in `superset_config.py`.
+
 * [11575](https://github.com/apache/incubator-superset/pull/11575) The Row Level Security (RLS) config flag has been moved to a feature flag. To migrate, add `ROW_LEVEL_SECURITY: True` to the `FEATURE_FLAGS` dict in `superset_config.py`.
 
 * NOTICE: config flag ENABLE_REACT_CRUD_VIEWS has been set to `True` by default, set to `False` if
@@ -36,7 +38,7 @@ assists people when migrating to a new version.
   and requires more work. You can easily turn on the languages you want
   to expose in your environment in superset_config.py
 
-* [11172](https://github.com/apache/incubator-superset/pull/11172): Breaking change: SQL templating is turned off be default. To turn it on set `ENABLE_TEMPLATE_PROCESSING` to True on `DEFAULT_FEATURE_FLAGS`
+* [11172](https://github.com/apache/incubator-superset/pull/11172): Breaking change: SQL templating is turned off be default. To turn it on set `ENABLE_TEMPLATE_PROCESSING` to True in `FEATURE_FLAGS`
 
 * [11155](https://github.com/apache/incubator-superset/pull/11155): The `FAB_UPDATE_PERMS` config parameter is no longer required as the Superset application correctly informs FAB under which context permissions should be updated.
 

superset/__init__.py

@@ -26,7 +26,6 @@
     db,
     event_logger,
     feature_flag_manager,
-    jinja_context_manager,
     manifest_processor,
     results_backend_manager,
     security_manager,
@@ -44,7 +43,6 @@
 get_feature_flags = feature_flag_manager.get_feature_flags
 get_manifest_files = manifest_processor.get_manifest_files
 is_feature_enabled = feature_flag_manager.is_feature_enabled
-jinja_base_context = jinja_context_manager.base_context
 results_backend = LocalProxy(lambda: results_backend_manager.results_backend)
 results_backend_use_msgpack = LocalProxy(
     lambda: results_backend_manager.should_use_msgpack

superset/app.py

@@ -35,7 +35,6 @@
     csrf,
     db,
     feature_flag_manager,
-    jinja_context_manager,
     machine_auth_provider_factory,
     manifest_processor,
     migrate,
@@ -515,7 +514,6 @@ def init_app(self) -> None:
         self.configure_logging()
         self.configure_middlewares()
         self.configure_cache()
-        self.configure_jinja_context()
 
         with self.flask_app.app_context():  # type: ignore
             self.init_app_in_ctx()
@@ -573,9 +571,6 @@ def configure_url_map_converters(self) -> None:
         self.flask_app.url_map.converters["regex"] = RegexConverter
         self.flask_app.url_map.converters["object_type"] = ObjectTypeConverter
 
-    def configure_jinja_context(self) -> None:
-        jinja_context_manager.init_app(self.flask_app)
-
     def configure_middlewares(self) -> None:
         if self.config["ENABLE_CORS"]:
             from flask_cors import CORS

superset/config.py

@@ -667,14 +667,19 @@ class CeleryConfig:  # pylint: disable=too-few-public-methods
 # A dictionary of items that gets merged into the Jinja context for
 # SQL Lab. The existing context gets updated with this dictionary,
 # meaning values for existing keys get overwritten by the content of this
-# dictionary.
+# dictionary. Exposing functionality through JINJA_CONTEXT_ADDONS has security
+# implications as it opens a window for a user to execute untrusted code.
+# It's important to make sure that the objects exposed (as well as objects attached
+# to those objets) are harmless. We recommend only exposing simple/pure functions that
+# return native types.
 JINJA_CONTEXT_ADDONS: Dict[str, Callable[..., Any]] = {}
 
-# A dictionary of macro template processors that gets merged into global
+# A dictionary of macro template processors (by engine) that gets merged into global
 # template processors. The existing template processors get updated with this
 # dictionary, which means the existing keys get overwritten by the content of this
-# dictionary. The customized addons don't necessarily need to use jinjia templating
-# language. This allows you to define custom logic to process macro template.
+# dictionary. The customized addons don't necessarily need to use Jinja templating
+# language. This allows you to define custom logic to process templates on a per-engine
+# basis. Example value = `{"presto": CustomPrestoTemplateProcessor}`
 CUSTOM_TEMPLATE_PROCESSORS: Dict[str, Type[BaseTemplateProcessor]] = {}
 
 # Roles that are controlled by the API / Superset and should not be changes

superset/connectors/sqla/models.py

@@ -875,14 +875,14 @@ def get_sqla_query(  # pylint: disable=too-many-arguments,too-many-locals,too-ma
     ) -> SqlaQuery:
         """Querying any sqla table from this common interface"""
         template_kwargs = {
-            "from_dttm": from_dttm,
+            "from_dttm": from_dttm.isoformat() if from_dttm else None,
             "groupby": groupby,
             "metrics": metrics,
             "row_limit": row_limit,
             "row_offset": row_offset,
-            "to_dttm": to_dttm,
+            "to_dttm": to_dttm.isoformat() if to_dttm else None,
             "filter": filter,
-            "columns": {col.column_name: col for col in self.columns},
+            "columns": [col.column_name for col in self.columns],
         }
         is_sip_38 = is_feature_enabled("SIP_38_VIZ_REARCHITECTURE")
         template_kwargs.update(self.template_params_dict)

superset/extensions.py

@@ -16,15 +16,10 @@
 # under the License.
 import json
 import os
-import random
-import time
-import uuid
-from datetime import datetime, timedelta
-from typing import Any, Callable, Dict, List, Optional, Type, TYPE_CHECKING
+from typing import Any, Callable, Dict, List, Optional
 
 import celery
 from cachelib.base import BaseCache
-from dateutil.relativedelta import relativedelta
 from flask import Flask
 from flask_appbuilder import AppBuilder, SQLA
 from flask_migrate import Migrate
@@ -36,37 +31,6 @@
 from superset.utils.feature_flag_manager import FeatureFlagManager
 from superset.utils.machine_auth import MachineAuthProviderFactory
 
-if TYPE_CHECKING:
-    from superset.jinja_context import BaseTemplateProcessor
-
-
-class JinjaContextManager:
-    def __init__(self) -> None:
-        self._base_context = {
-            "datetime": datetime,
-            "random": random,
-            "relativedelta": relativedelta,
-            "time": time,
-            "timedelta": timedelta,
-            "uuid1": uuid.uuid1,
-            "uuid3": uuid.uuid3,
-            "uuid4": uuid.uuid4,
-            "uuid5": uuid.uuid5,
-        }
-        self._template_processors: Dict[str, Type["BaseTemplateProcessor"]] = {}
-
-    def init_app(self, app: Flask) -> None:
-        self._base_context.update(app.config["JINJA_CONTEXT_ADDONS"])
-        self._template_processors.update(app.config["CUSTOM_TEMPLATE_PROCESSORS"])
-
-    @property
-    def base_context(self) -> Dict[str, Any]:
-        return self._base_context
-
-    @property
-    def template_processors(self) -> Dict[str, Type["BaseTemplateProcessor"]]:
-        return self._template_processors
-
 
 class ResultsBackendManager:
     def __init__(self) -> None:
@@ -140,7 +104,6 @@ def get_manifest_files(self, bundle: str, asset_type: str) -> List[str]:
 _event_logger: Dict[str, Any] = {}
 event_logger = LocalProxy(lambda: _event_logger.get("event_logger"))
 feature_flag_manager = FeatureFlagManager()
-jinja_context_manager = JinjaContextManager()
 machine_auth_provider_factory = MachineAuthProviderFactory()
 manifest_processor = UIManifestProcessor(APP_DIR)
 migrate = Migrate()