michael-s-molina commented on code in PR #36739:
URL: https://github.com/apache/superset/pull/36739#discussion_r2631307285


##########
superset/mcp_service/sql_lab/tool/execute_sql.py:
##########
@@ -52,36 +59,75 @@ async def execute_sql(request: ExecuteSqlRequest, ctx: 
Context) -> ExecuteSqlRes
     # Log SQL query details (truncated for security)
     sql_preview = request.sql[:100] + "..." if len(request.sql) > 100 else 
request.sql
     await ctx.debug(
-        "SQL query details: sql_preview=%r, sql_length=%s, has_parameters=%s"
+        "SQL query details: sql_preview=%r, sql_length=%s, 
has_template_params=%s"
         % (
             sql_preview,
             len(request.sql),
-            bool(request.parameters),
+            bool(request.template_params),
         )
     )
 
     logger.info("Executing SQL query on database ID: %s", request.database_id)
 
     try:
-        # Use the ExecuteSqlCore to handle all the logic
-        sql_tool = ExecuteSqlCore(use_command_mode=False, logger=logger)
-        result = sql_tool.run_tool(request)
+        # Import inside function to avoid initialization issues
+        from superset import db, security_manager
+        from superset.models.core import Database
+
+        # 1. Get database and check access
+        database = 
db.session.query(Database).filter_by(id=request.database_id).first()
+        if not database:
+            raise SupersetErrorException(
+                SupersetError(
+                    message=f"Database with ID {request.database_id} not 
found",
+                    error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR,
+                    level=ErrorLevel.ERROR,
+                )
+            )
+
+        if not security_manager.can_access_database(database):
+            raise SupersetSecurityException(
+                SupersetError(
+                    message=f"Access denied to database 
{database.database_name}",
+                    
error_type=SupersetErrorType.DATABASE_SECURITY_ACCESS_ERROR,
+                    level=ErrorLevel.ERROR,
+                )
+            )
+
+        # 2. Build QueryOptions
+        options = QueryOptions(

Review Comment:
   I'm assuming we want all queries from MCP tools to be audited so I kept the 
default behavior of adding entries to the `query` table. Let me know if there's 
any case where we don't want this.



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