rename to sql_select.fn

[hchen2020]

PR Type

Enhancement, Bug fix

---

Description

• Replace boolean FormattingResult with string ResultFormat parameter

  • Supports multiple output formats (markdown, csv)
  • Adds markdown table formatting implementation

• Add SQLite database support to SQL query execution

• Fix column name handling to replace # characters with underscores

• Rename template reference from sql_executor.fn to sql_select.fn

• Comment out debug logging statements in OpenAI chat provider

---

Diagram Walkthrough

flowchart LR
  A["ExecuteQueryArgs"] -->|ResultFormat string| B["FormatResultsToMarkdown"]
  A -->|ResultFormat string| C["FormatResultsToCsv"]
  D["SqlSelect"] -->|Add SQLite| E["RunQueryInSqlite"]
  F["Column Processing"] -->|Replace # char| G["Valid SQL columns"]
  H["SqlUtilityHook"] -->|Rename template| I["sql_select.fn"]

Loading

File Walkthrough

	Relevant files
Bug fix	2 files SqliteService.cs Handle hash character in column names                                        +2/-2      SqlUtilityHook.cs Rename sql_executor template to sql_select                              +1/-2
Miscellaneous	1 files ChatCompletionProvider.cs Comment out debug logging statements                                          +2/-2
Enhancement	6 files SqlDriverController.cs Update parameter name from FormattingResult to ResultFormat +1/-1      SqlQueryRequest.cs Change FormattingResult bool to ResultFormat string            +1/-1      ExecuteQueryFn.cs Implement markdown and CSV result formatting                          +81/-19  SqlDriverPlanningHook.cs Remove FormattingResult from ExecuteQueryArgs                        +1/-2      ExecuteQueryArgs.cs Replace FormattingResult with ResultFormat parameter          +2/-2      SqlSelect.cs Add SQLite database query execution support                            +11/-0
Configuration changes	1 files BotSharp.Plugin.SqlDriver.csproj Update template file references for sql_select                      +1/-2
Additional files	1 files util-db-sql_select.fn.liquid [link]

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	SQL injection Description: The new SQLite execution path (RunQueryInSqlite) executes the first SQL text without parameterization and without any validation or sanitization, increasing risk of SQL injection when user-controlled statements reach this path. SqlSelect.cs [80-85] Referred Code private IEnumerable<dynamic> RunQueryInSqlite(string connectionString, string[] sqlTexts) { var settings = _services.GetRequiredService<SqlDriverSetting>(); using var connection = new SqliteConnection(connectionString); return connection.Query(sqlTexts[0]); }
	Sensitive information exposure Description: Logging full SQL statements at info level (_logger.LogInformation("Executing SQL Statements: {SqlStatements}", ...)) can leak sensitive data such as credentials or PII contained in queries or literals, especially in production logs. ExecuteQueryFn.cs [34-46] Referred Code // Print all the SQL statements for debugging _logger.LogInformation("Executing SQL Statements: {SqlStatements}", string.Join("\r\n", args.SqlStatements)); IEnumerable<dynamic> results = []; try { results = dbType.ToLower() switch { "mysql" => RunQueryInMySql(args.SqlStatements), "sqlserver" or "mssql" => RunQueryInSqlServer(args.SqlStatements), "redshift" => RunQueryInRedshift(args.SqlStatements), "sqlite" => RunQueryInSqlite(dbConnectionString, args.SqlStatements), _ => throw new NotImplementedException($"Database type {dbType} is not supported.")
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit: SQL execution actions are performed and results returned without explicit audit logging of who executed what and the outcome. Referred Code // Print all the SQL statements for debugging _logger.LogInformation("Executing SQL Statements: {SqlStatements}", string.Join("\r\n", args.SqlStatements)); IEnumerable<dynamic> results = []; try { results = dbType.ToLower() switch { "mysql" => RunQueryInMySql(args.SqlStatements), "sqlserver" or "mssql" => RunQueryInSqlServer(args.SqlStatements), "redshift" => RunQueryInRedshift(args.SqlStatements), "sqlite" => RunQueryInSqlite(dbConnectionString, args.SqlStatements), _ => throw new NotImplementedException($"Database type {dbType} is not supported.") Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Generic errors: Errors are caught and surfaced with only exception messages without structured context (e.g., DB type, statement identifiers), which may hinder debugging and lacks explicit handling for empty results or unsupported formats. Referred Code catch (Exception ex) { _logger.LogError(ex, "Error occurred while executing SQL query."); message.Content = $"Error occurred while executing SQL query: {ex.Message}"; message.StopCompletion = true; return false; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error detail leak: The user-facing message returns the raw exception message in message.Content, which may expose internal details. Referred Code _logger.LogError(ex, "Error occurred while executing SQL query."); message.Content = $"Error occurred while executing SQL query: {ex.Message}"; message.StopCompletion = true; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Log SQL text: Logging full SQL statements at info level may expose sensitive data embedded in queries or parameters. Referred Code // Print all the SQL statements for debugging _logger.LogInformation("Executing SQL Statements: {SqlStatements}", string.Join("\r\n", args.SqlStatements)); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Injection risk: SQLite path executes raw sqlTexts[0] without parameterization or validation, which could allow SQL injection if inputs are external. Referred Code private IEnumerable<dynamic> RunQueryInSqlite(string connectionString, string[] sqlTexts) { var settings = _services.GetRequiredService<SqlDriverSetting>(); using var connection = new SqliteConnection(connectionString); return connection.Query(sqlTexts[0]); } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Prevent potential SQL injection vulnerability Fix a SQL injection vulnerability in the RunQueryInSqlite method by refactoring it to use parameterized queries, aligning it with the safer implementations of other query methods in the file. src/Plugins/BotSharp.Plugin.SqlDriver/UtilFunctions/SqlSelect.cs [80-85] -private IEnumerable<dynamic> RunQueryInSqlite(string connectionString, string[] sqlTexts) +private IEnumerable<dynamic> RunQueryInSqlite(string connectionString, SqlStatement args) { - var settings = _services.GetRequiredService<SqlDriverSetting>(); using var connection = new SqliteConnection(connectionString); - return connection.Query(sqlTexts[0]); + var dictionary = new Dictionary<string, object>(); + if (args.Parameters != null) + { + foreach (var p in args.Parameters) + { + dictionary["@" + p.Name] = p.Value; + } + } + return connection.Query(args.Statement, dictionary); } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant SQL injection vulnerability in the newly added RunQueryInSqlite method and provides a correct fix using parameterized queries, which is a critical security improvement.	High
High-level	Implement SQLite support consistently The PR inconsistently adds SQLite support to the SqlSelect function but omits it from the ExecuteQueryFn function. To ensure uniform database capabilities, SQLite support should also be implemented in ExecuteQueryFn. Examples: src/Plugins/BotSharp.Plugin.SqlDriver/Functions/ExecuteQueryFn.cs [40-44] results = dbType.ToLower() switch { "mysql" => RunQueryInMySql(args.SqlStatements), "sqlserver" or "mssql" => RunQueryInSqlServer(args.SqlStatements), "redshift" => RunQueryInRedshift(args.SqlStatements), src/Plugins/BotSharp.Plugin.SqlDriver/UtilFunctions/SqlSelect.cs [39] "sqlite" => RunQueryInSqlite(dbHook.GetConnectionString(message), [args.Statement]), Solution Walkthrough: Before: // In ExecuteQueryFn.cs public async Task<bool> Execute(RoleDialogModel message) { // ... var dbType = dbHook.GetDatabaseType(message); results = dbType.ToLower() switch { "mysql" => RunQueryInMySql(args.SqlStatements), "sqlserver" or "mssql" => RunQueryInSqlServer(args.SqlStatements), "redshift" => RunQueryInRedshift(args.SqlStatements), // "sqlite" case is missing _ => throw new NotImplementedException(...) }; // ... } After: // In ExecuteQueryFn.cs public async Task<bool> Execute(RoleDialogModel message) { // ... var dbType = dbHook.GetDatabaseType(message); results = dbType.ToLower() switch { "mysql" => RunQueryInMySql(args.SqlStatements), "sqlserver" or "mssql" => RunQueryInSqlServer(args.SqlStatements), "redshift" => RunQueryInRedshift(args.SqlStatements), "sqlite" => RunQueryInSqlite(args.SqlStatements), // Add this case _ => throw new NotImplementedException(...) }; // ... } private IEnumerable<dynamic> RunQueryInSqlite(string[] sqlTexts) { // Implementation for running query in SQLite } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies an incomplete feature implementation where SQLite support is added to SqlSelect but is missing in the parallel ExecuteQueryFn, leading to inconsistent behavior and a potential runtime error.	Medium
Possible issue	Properly escape all Markdown characters Improve the EscapeMarkdownField method by adding escaping for other special Markdown characters like backticks, asterisks, and underscores to prevent unintended formatting in the results table. src/Plugins/BotSharp.Plugin.SqlDriver/Functions/ExecuteQueryFn.cs [206-215] private string EscapeMarkdownField(string field) { if (string.IsNullOrEmpty(field)) { return string.Empty; } - // Escape pipe characters which are special in Markdown tables - return field.Replace("|", "\\|"); + // Escape characters that have special meaning in Markdown tables + return field.Replace("|", "\\|") + .Replace("`", "\\`") + .Replace("*", "\\*") + .Replace("_", "\\_"); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the new EscapeMarkdownField method is incomplete and could lead to broken table formatting. Expanding the escaping to include other special Markdown characters makes the output formatting more robust.	Medium
	Handle unsupported result formats gracefully Refactor the if-else if block for result formatting into a switch statement with a default case to gracefully handle unsupported formats by defaulting to "markdown". src/Plugins/BotSharp.Plugin.SqlDriver/Functions/ExecuteQueryFn.cs [99-107] -if (args.ResultFormat.ToLower() == "markdown") +switch (args.ResultFormat.ToLower()) { - message.Content = FormatResultsToMarkdown(results); -} -else if (args.ResultFormat.ToLower() == "csv") -{ - message.Content = FormatResultsToCsv(results); + case "csv": + message.Content = FormatResultsToCsv(results); + break; + case "markdown": + default: + message.Content = FormatResultsToMarkdown(results); + break; } message.StopCompletion = true; Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that an unsupported ResultFormat would lead to an empty response and proposes a more robust implementation using a switch with a default case, improving code quality and resilience.	Low
Learned best practice	Add null and input validation Validate that sqlQueryRequest and sqlQueryRequest.SqlStatement are not null or empty before using them, and return a 400 response if invalid. src/Plugins/BotSharp.Plugin.SqlDriver/Controllers/SqlDriverController.cs [24-28] +if (sqlQueryRequest == null || string.IsNullOrWhiteSpace(sqlQueryRequest.SqlStatement)) +{ + return BadRequest("SqlStatement is required."); +} var match = Regex.Match(sqlQueryRequest.SqlStatement, @"```sql\s*([\s\S]*?)\s*```", RegexOptions.IgnoreCase); if (match.Success) { sqlQueryRequest.SqlStatement = match.Groups[1].Value.Trim(); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Guard against nulls and invalid inputs before access to avoid NullReferenceExceptions.	Low
More