diff --git a/servers/Azure.Mcp.Server/CHANGELOG.md b/servers/Azure.Mcp.Server/CHANGELOG.md index 7a81c2c95..507769e1f 100644 --- a/servers/Azure.Mcp.Server/CHANGELOG.md +++ b/servers/Azure.Mcp.Server/CHANGELOG.md @@ -6,6 +6,7 @@ The Azure MCP Server updates automatically by default whenever a new release com ### Features Added - add `azmcp sql server firewall-rule create` and `azmcp sql server firewall-rule delete` commands. [#121](https://github.com/microsoft/mcp/pull/121) +- Fixed a bug in MySQL query validation logic. [[#81](https://github.com/microsoft/mcp/pull/81)] ### Breaking Changes diff --git a/tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs b/tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs index 0c7f2799e..d4e317e1c 100644 --- a/tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs +++ b/tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs @@ -146,23 +146,15 @@ private static void ValidateQuerySafety(string query) throw new ArgumentException("Query cannot be empty after removing comments and whitespace.", nameof(query)); } - // Check for multiple SQL statements (semicolons that are not in comments) - var semicolonCount = cleanedQuery.Split(';').Length - 1; - if (semicolonCount > 1) - { - throw new InvalidOperationException("Multiple SQL statements are not allowed. Use only a single SELECT statement."); - } - - // Regex pattern to detect common SQL injection techniques (comments are already removed) - var dangerPattern = new Regex( - @"(;|\b(union\s+select|drop\s+table|insert\s+into|update\s+|delete\s+from|or\s+1=1)\b)", + // Regex pattern to detect multiple SQL statements (semicolon not at end) + var multipleStatementsPattern = new Regex( + @";\s*\w", RegexOptions.IgnoreCase | RegexOptions.Compiled ); - // Check for common SQL injection patterns using regex - if (dangerPattern.IsMatch(cleanedQuery)) + if (multipleStatementsPattern.IsMatch(cleanedQuery)) { - throw new InvalidOperationException("Query contains dangerous patterns that could indicate SQL injection attempts and is not allowed for security reasons."); + throw new InvalidOperationException("Multiple SQL statements are not allowed. Use only a single SELECT statement."); } // List of dangerous SQL keywords that should be blocked diff --git a/tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/Services/MySqlServiceQueryValidationTests.cs b/tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/Services/MySqlServiceQueryValidationTests.cs index 4e8569e49..e924d43c1 100644 --- a/tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/Services/MySqlServiceQueryValidationTests.cs +++ b/tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/Services/MySqlServiceQueryValidationTests.cs @@ -30,6 +30,8 @@ public MySqlServiceQueryValidationTests() [Theory] [InlineData("SELECT * FROM users LIMIT 100")] [InlineData("SELECT COUNT(*) FROM products LIMIT 1")] + [InlineData("SELECT COUNT(*) FROM products;")] + [InlineData("SELECT COUNT(*) FROM products; -- comment")] public void ValidateQuerySafety_WithSafeQueries_ShouldNotThrow(string query) { // Arrange @@ -129,6 +131,8 @@ public void ValidateQuerySafety_WithLongQuery_ShouldThrowInvalidOperationExcepti [Theory] [InlineData("SELECT * FROM users; DROP TABLE users")] [InlineData("SELECT * FROM users; SELECT * FROM products")] + [InlineData("SELECT * FROM users; SELECT * FROM products; --comment")] + [InlineData("SELECT * FROM Logs; union select password from Users")] public void ValidateQuerySafety_WithMultipleStatements_ShouldThrowInvalidOperationException(string query) { // Arrange @@ -139,7 +143,7 @@ public void ValidateQuerySafety_WithMultipleStatements_ShouldThrowInvalidOperati validateMethod.Invoke(null, new object[] { query })); Assert.IsType(exception.InnerException); - Assert.Contains("Query contains dangerous patterns that could indicate SQL injection attempts", exception.InnerException!.Message); + Assert.Contains("Multiple SQL statements are not allowed. Use only a single SELECT statement.", exception.InnerException!.Message); } [Theory]