aminghadersohi commented on code in PR #37411:
URL: https://github.com/apache/superset/pull/37411#discussion_r2757729474
##########
superset/sql/execution/executor.py:
##########
@@ -684,6 +696,34 @@ def _check_disallowed_functions(self, script: SQLScript)
-> set[str] | None:
return found if found else None
+ def _check_disallowed_tables(self, script: SQLScript) -> set[str] | None:
+ """
+ Check for disallowed SQL tables/views.
+
+ :param script: Parsed SQL script
+ :returns: Set of disallowed tables found, or None if none found
+ """
+ disallowed_config = app.config.get("DISALLOWED_SQL_TABLES", {})
+ engine_name = self.database.db_engine_spec.engine
+
+ # Get disallowed tables for this engine
+ engine_disallowed = disallowed_config.get(engine_name, set())
+ if not engine_disallowed:
+ return None
+
+ # Use AST-based table detection
+ if script.check_tables_present(engine_disallowed):
+ # Return the specific tables that were found
+ found = set()
+ for statement in script.statements:
+ present = {table.table.lower() for table in statement.tables}
+ for table in engine_disallowed:
+ if table.lower() in present:
+ found.add(table)
+ return found if found else None
Review Comment:
Valid catch — if `check_tables_present` returns `True` but the secondary
name-mapping loop yields an empty set (due to AST representation differences),
the query would be silently allowed. Fixed by falling back to the full
configured blocklist when this happens. The `sql_lab.py` path already had this
fallback via `found_tables or disallowed_tables`.
--
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]
