Skip to content

Conversation

@ramnov
Copy link
Contributor

@ramnov ramnov commented Aug 28, 2025

What does this PR do?

MySQL was incorrectly flagging proper query like "SELECT * from users;" as invalid. Updated logic to detect this properly. Added extra unit tests.

GitHub issue number?

[Link to the GitHub issue this PR addresses]

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
    • Spelling check passes: .\eng\common\spelling\Invoke-Cspell.ps1
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated README.md documentation
    • Updated command list in /docs/azmcp-commands.md
    • Updated test prompts in /docs/e2eTestPrompts.md
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
  • 👉 For Community (non-Azure team member) PRs:
    • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
    • Manual tests run: added comment /azp run azure - mcp to run Live Test Pipeline

Copilot AI review requested due to automatic review settings August 28, 2025 22:39
@ramnov ramnov requested review from a team and mattkohnms as code owners August 28, 2025 22:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes MySQL query validation logic that was incorrectly flagging valid queries containing semicolons as invalid. The fix updates regex patterns to properly detect dangerous SQL injection patterns and multiple statements while allowing legitimate single statements with trailing semicolons.

  • Updated regex patterns to distinguish between legitimate single statements with semicolons and actual multiple statements
  • Modified dangerous pattern detection to focus on semicolons followed by injection keywords rather than any semicolon
  • Added comprehensive test cases covering edge cases with semicolons and comments

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
MySqlService.cs Updated validation logic with improved regex patterns for detecting multiple statements and SQL injection attempts
MySqlServiceQueryValidationTests.cs Added test cases for valid queries with semicolons and additional test coverage for multiple statement detection

@ramnov ramnov changed the title Fix MySQL query validation logic Fix MySQL multiple statements validation logic Aug 28, 2025
@ramnov ramnov self-assigned this Aug 28, 2025
@ramnov ramnov enabled auto-merge (squash) September 4, 2025 18:28
@ramnov ramnov merged commit 16c2104 into microsoft:main Sep 4, 2025
23 checks passed
feiskyer pushed a commit to feiskyer/microsoft-mcp that referenced this pull request Sep 8, 2025
* Adds support for Azure Database for PostgreSQL

- Adds support for Azure Database for PostgreSQL - Flexible Server.
- Implements the following PostgreSQL commands:
    - Server
        - List
        - GetConfig
        - GetParam
    - Database
        - List
        - Query
    - Table
        - List
        - GetSchema
- Adds service and interface for PostgreSQL.
- Adds commands and arguments for PostgreSQL
- Registers the new commands in the CommandFactory.
- Registers the new service to start during service initialization.
- Updates the ArgumentDefinitions to include PostgreSQL commands.
- Updates the AzureMcp project file to include the new dependencies.

Notes:
- Inherits from SubscriptionCommand
- Adds resource group explicitly to options and arguments.

* Adds tests for Azure Database  for PostgreSQL

* Update README and azmcp docs with PostgreSQL capabilities.

* Rebase, addresses review comments & refactor base classes

- Update to work with the rebased code.
- Add PostgresJsonContext
- Remove explicit validations.
- Change the commandline arguments to give more context (pg => postgres)
- Simplify command line usage (get-schema => schema)
- Add JsonPropertyName
- Add more tests for validating missing parameters.
- Simplify logic in PG query & server config.
- Fix commmand names
- Use common BaseAzureService credential
- Refactor server and database to use common base classes.
- Update docs
- Run dotnet format.

* Remove the extra line
colbytimm pushed a commit to colbytimm/microsoft-mcp that referenced this pull request Sep 27, 2025
* Fix MySQL query validation logic

* remove duplicate unit test

* update logic

* dotnet format

* update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants