From 576df239d810553b1b4f79eb470e4d6e694cc76d Mon Sep 17 00:00:00 2001
From: carfeii <carfeii@users.noreply.github.com>
Date: Wed, 16 Sep 2026 22:49:39 +0800
Subject: [PATCH] Fix missing conversation ownership check on SQL driver
 endpoints

POST /sql-driver/{conversationId}/execute was gated only by
[Authorize] (any logged-in user, no role check) and performed no
check that the caller owns conversationId. The underlying
ConversationService.SetConversationId call has no ownership check
either, and silently creates a new conversation using the caller's
own user id if the given id doesn't already exist, so any
authenticated user could execute an arbitrary SQL statement against
any configured data source by supplying a conversationId of their
own choosing. The sibling endpoint /sql-driver/{conversationId}/result
had the identical gap.

Add the same admin-or-owner check ConversationController.GetConversation
already applies when reading a conversation's own dialog (IUserService.
IsAdminUser plus a ConversationFilter.UserId filter), to both
ExecuteSqlQuery and AddQueryExecutionResult.
---
 .../Controllers/SqlDriverController.cs        | 50 +++++++++++++++++--
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/src/Plugins/BotSharp.Plugin.SqlDriver/Controllers/SqlDriverController.cs b/src/Plugins/BotSharp.Plugin.SqlDriver/Controllers/SqlDriverController.cs
index 67700650d..8349430b2 100644
--- a/src/Plugins/BotSharp.Plugin.SqlDriver/Controllers/SqlDriverController.cs
+++ b/src/Plugins/BotSharp.Plugin.SqlDriver/Controllers/SqlDriverController.cs
@@ -1,4 +1,6 @@
 using BotSharp.Abstraction.Models;
+using BotSharp.Abstraction.Repositories.Filters;
+using BotSharp.Abstraction.Users;
 using BotSharp.Plugin.SqlDriver.Constants;
 using BotSharp.Plugin.SqlDriver.Controllers.ViewModels;
 using Microsoft.AspNetCore.Authorization;
@@ -11,16 +13,42 @@ namespace BotSharp.Plugin.SqlDriver.Controllers;
 public class SqlDriverController : ControllerBase
 {
     private readonly IServiceProvider _services;
+    private readonly IUserIdentity _user;
 
-    public SqlDriverController(IServiceProvider services)
+    public SqlDriverController(IServiceProvider services, IUserIdentity user)
     {
         _services = services;
+        _user = user;
     }
 
     [HttpPost]
     [Route("/sql-driver/{conversationId}/execute")]
     public async Task<IActionResult> ExecuteSqlQuery([FromRoute] string conversationId, [FromBody] SqlQueryRequest sqlQueryRequest)
     {
+        // [Authorize] only requires a logged-in session; it does not verify the
+        // caller owns conversationId. Without this check, any authenticated
+        // user could execute an arbitrary SQL statement against any configured
+        // data source by supplying any conversationId (including one they
+        // invent themselves), since SetConversationId performs no ownership
+        // check and silently creates the conversation if it doesn't exist.
+        // Mirror the same admin-or-owner check ConversationController.GetConversation
+        // already applies to reading a conversation's own dialog.
+        var userService = _services.GetRequiredService<IUserService>();
+        var conv = _services.GetRequiredService<IConversationService>();
+        var (isAdmin, currentUser) = await userService.IsAdminUser(_user.Id);
+        if (!isAdmin)
+        {
+            var existing = await conv.GetConversations(new ConversationFilter
+            {
+                Id = conversationId,
+                UserId = currentUser?.Id,
+            });
+            if (existing.Items?.FirstOrDefault() == null)
+            {
+                return Forbid();
+            }
+        }
+
         var match = Regex.Match(sqlQueryRequest.SqlStatement, @"```sql\s*([\s\S]*?)\s*```", RegexOptions.IgnoreCase);
         if (match.Success)
         {
@@ -28,13 +56,12 @@ public async Task<IActionResult> ExecuteSqlQuery([FromRoute] string conversation
         }
 
         var fn = _services.GetRequiredService<IRoutingService>();
-        var conv = _services.GetRequiredService<IConversationService>();
-        await conv.SetConversationId(conversationId, 
+        await conv.SetConversationId(conversationId,
             [
                 new MessageState(StateKeys.DBType, sqlQueryRequest.DbType),
                 new MessageState(StateKeys.DataSource, sqlQueryRequest.DataSource),
             ]);
-        
+
         var msg = new RoleDialogModel(AgentRole.User, sqlQueryRequest.SqlStatement)
         {
             CurrentAgentId = sqlQueryRequest.AgentId
@@ -63,7 +90,22 @@ await conv.SetConversationId(conversationId,
     [Route("/sql-driver/{conversationId}/result")]
     public async Task<IActionResult> AddQueryExecutionResult([FromRoute] string conversationId, [FromBody] SqlQueryExecutionResult sqlQueryResult)
     {
+        var userService = _services.GetRequiredService<IUserService>();
         var conv = _services.GetRequiredService<IConversationService>();
+        var (isAdmin, currentUser) = await userService.IsAdminUser(_user.Id);
+        if (!isAdmin)
+        {
+            var existing = await conv.GetConversations(new ConversationFilter
+            {
+                Id = conversationId,
+                UserId = currentUser?.Id,
+            });
+            if (existing.Items?.FirstOrDefault() == null)
+            {
+                return Forbid();
+            }
+        }
+
         await conv.SetConversationId(conversationId, []);
 
         var storage = _services.GetRequiredService<IConversationStorage>();