AmoghAtreya commented on code in PR #44968:
URL: https://github.com/apache/superset/pull/44968#discussion_r4179522205
##########
superset/mcp_service/sql_lab/tool/execute_sql.py:
##########
@@ -182,17 +183,16 @@ async def execute_sql(request: ExecuteSqlRequest, ctx:
Context) -> ExecuteSqlRes
# Authorize through the same entry point as the SQL Lab
# execution path (``superset/sqllab/validators.py``), so both
- # surfaces scope a query the same way: it covers database-level
- # access and, for a user without it, requires every table the
- # query references to resolve to a dataset they are granted.
+ # surfaces scope a query the same way, including
+ # ``SQLLAB_REQUIRE_DATASET_MATCH``.
try:
security_manager.raise_for_access(
database=database,
sql=request.sql,
catalog=request.catalog,
schema=request.schema_name,
template_params=template_params,
- force_dataset_match=True,
+ force_dataset_match=requires_dataset_match(),
Review Comment:
Fixed as well. The access check rendered the Jinja source once, and
`database.execute` rendered it again, so a template that does not always
produce the same SQL could run against a table the first check never saw. The
tool now renders once with `process_template`, authorizes that exact SQL, and
passes that same string to the destructive-SQL check and to `database.execute`
with `template_params=None`, so it is not rendered again. If the second check
fails, the query does not run.
--
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]
