sha174n commented on code in PR #40567:
URL: https://github.com/apache/superset/pull/40567#discussion_r3348770010


##########
superset/models/helpers.py:
##########
@@ -1412,6 +1414,23 @@ def query(self, query_obj: QueryObjectDict) -> 
QueryResult:
         qry_start_dttm = datetime.now()
         query_str_ext = self.get_query_str_extended(query_obj)
         sql = query_str_ext.sql
+
+        # Mirror the DISALLOWED_SQL_* gate that sql_lab.execute_sql_statement
+        # enforces so both query surfaces honour the same denylist.
+        engine = self.db_engine_spec.engine
+        disallowed_functions = 
app.config["DISALLOWED_SQL_FUNCTIONS"].get(engine, set())
+        disallowed_tables = app.config["DISALLOWED_SQL_TABLES"].get(engine, 
set())
+        if disallowed_functions or disallowed_tables:
+            parsed_script = SQLScript(sql, engine=engine)
+            if disallowed_functions and parsed_script.check_functions_present(
+                disallowed_functions
+            ):
+                raise 
SupersetDisallowedSQLFunctionException(disallowed_functions)
+            if disallowed_tables and parsed_script.check_tables_present(
+                disallowed_tables
+            ):
+                raise SupersetDisallowedSQLTableException(disallowed_tables)

Review Comment:
   Fixed in d683683a87: the chart-data gate now reports only the tables 
actually present in the query (mirroring the adhoc-expression and SQL Lab 
paths), and the logic is extracted into a helper.



##########
superset/models/helpers.py:
##########
@@ -1459,6 +1459,23 @@ def query(self, query_obj: QueryObjectDict) -> 
QueryResult:
         qry_start_dttm = datetime.now()
         query_str_ext = self.get_query_str_extended(query_obj)
         sql = query_str_ext.sql
+
+        # Mirror the DISALLOWED_SQL_* gate that sql_lab.execute_sql_statement
+        # enforces so both query surfaces honour the same denylist.
+        engine = self.db_engine_spec.engine
+        disallowed_functions = 
app.config["DISALLOWED_SQL_FUNCTIONS"].get(engine, set())
+        disallowed_tables = app.config["DISALLOWED_SQL_TABLES"].get(engine, 
set())
+        if disallowed_functions or disallowed_tables:
+            parsed_script = SQLScript(sql, engine=engine)
+            if disallowed_functions and parsed_script.check_functions_present(
+                disallowed_functions
+            ):
+                raise 
SupersetDisallowedSQLFunctionException(disallowed_functions)
+            if disallowed_tables and parsed_script.check_tables_present(
+                disallowed_tables
+            ):
+                raise SupersetDisallowedSQLTableException(disallowed_tables)

Review Comment:
   Fixed in d683683a87: the chart-data gate now reports only the tables 
actually present in the query (mirroring the adhoc-expression and SQL Lab 
paths), and the logic is extracted into a helper.



##########
superset/models/helpers.py:
##########
@@ -1459,6 +1459,23 @@ def query(self, query_obj: QueryObjectDict) -> 
QueryResult:
         qry_start_dttm = datetime.now()
         query_str_ext = self.get_query_str_extended(query_obj)
         sql = query_str_ext.sql
+
+        # Mirror the DISALLOWED_SQL_* gate that sql_lab.execute_sql_statement
+        # enforces so both query surfaces honour the same denylist.
+        engine = self.db_engine_spec.engine
+        disallowed_functions = 
app.config["DISALLOWED_SQL_FUNCTIONS"].get(engine, set())
+        disallowed_tables = app.config["DISALLOWED_SQL_TABLES"].get(engine, 
set())
+        if disallowed_functions or disallowed_tables:
+            parsed_script = SQLScript(sql, engine=engine)
+            if disallowed_functions and parsed_script.check_functions_present(
+                disallowed_functions
+            ):
+                raise 
SupersetDisallowedSQLFunctionException(disallowed_functions)
+            if disallowed_tables and parsed_script.check_tables_present(
+                disallowed_tables
+            ):
+                raise SupersetDisallowedSQLTableException(disallowed_tables)

Review Comment:
   Fixed in d683683a87: the chart-data gate now reports only the tables 
actually present in the query (mirroring the adhoc-expression and SQL Lab 
paths), and the logic is extracted into a helper.



-- 
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]

Reply via email to