Skip to content

Add read-only mode and query execution capabilities#20

Merged
danielmeppiel merged 26 commits intodanielmeppiel:mainfrom
kevingatera:feat/implement-query-execution-capabilities
Aug 22, 2025
Merged

Add read-only mode and query execution capabilities#20
danielmeppiel merged 26 commits intodanielmeppiel:mainfrom
kevingatera:feat/implement-query-execution-capabilities

Conversation

@kevingatera
Copy link
Contributor

This PR introduces read-only mode as a default security feature to prevent accidental write operations, adds the run_sql_query tool for executing SELECT statements, and implements a wrap_untrusted function to mitigate prompt injection risks. Changes update the DatabaseContext and DatabaseConnector classes to enforce read-only checks.

Please see: README documentation for configuration and usage.

This resolves #19 and is also a more flexible implementation of #17

Copy link
Owner

@danielmeppiel danielmeppiel left a comment

Choose a reason for hiding this comment

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

Great base, let's iterate - I've left my comments

@danielmeppiel danielmeppiel requested a review from Copilot July 21, 2025 15:39

This comment was marked as outdated.

@danielmeppiel danielmeppiel requested a review from Copilot July 22, 2025 06:47

This comment was marked as outdated.

danielmeppiel and others added 7 commits July 22, 2025 13:58
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This adds is_write_operation
- Add semicolon detection to prevent statement chaining attacks
- Implement regex pattern matching for DML/DDL keywords in comments/subqueries
- Block sophisticated SQL injection attempts that bypass basic prefix checks
- Maintain backward compatibility for legitimate SELECT/CTE queries
Go back to using
SYS.ODCIVARCHAR2LIST for better performance with large table lists.
@kevingatera
Copy link
Contributor Author

@danielmeppiel The PR is now ready. All concerns were addressed.

@danielmeppiel danielmeppiel requested a review from Copilot July 24, 2025 08:31

This comment was marked as outdated.

Copy link
Owner

@danielmeppiel danielmeppiel left a comment

Choose a reason for hiding this comment

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

Thanks for your latest changes @kevingatera !
Let's security-harden this with sqlparse library and we will be ready to merge

@kevingatera
Copy link
Contributor Author

@danielmeppiel The requested changes have been made. But, there are two items of note:

  1. I ran uv lock and it included the new sqlparse but I think in the newer versions of uv it appends an upload time that resulted in a larger than expected line changes. From the cursory glance I made nothing important appears to have been disturbed other than bumping the revision, adding an unavoidable upload time per package, and adding sqlparse.
  2. The _assert_query_executable method only validates SQL when in read-only mode. When read-only mode is disabled, it doesn't check for stacked statements at all. I chose to keep it that way for non-read mode so the user/llm will have freedom of arbitrary commands.

@danielmeppiel danielmeppiel requested a review from Copilot July 30, 2025 20:36

This comment was marked as outdated.

@kevingatera
Copy link
Contributor Author

Fixed and ran tests for it. Ready.

@kevingatera
Copy link
Contributor Author

@danielmeppiel is there anything else you'd like me to add/modify?

@danielmeppiel
Copy link
Owner

Sorry for the delay, currently on a summer break, but will have a look at merging this ASAP

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 introduces comprehensive read-only mode and SQL query execution capabilities to enhance database security and functionality. The default read-only mode prevents accidental write operations while still allowing full read access, and the new query execution tool enables users to run SELECT statements directly through the MCP server.

  • Adds read-only mode as a default security feature with environment variable configuration
  • Implements run_sql_query tool for executing SELECT statements with formatted output
  • Introduces wrap_untrusted function to mitigate prompt injection risks in returned data

Reviewed Changes

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

Show a summary per file
File Description
pyproject.toml Adds sqlparse dependency for SQL statement parsing
main.py Implements wrap_untrusted function, adds run_sql_query tool, and enables read-only mode configuration
db_context/schema/formatter.py Adds format_sql_query_result function for creating markdown tables from query results
db_context/database.py Refactors DatabaseConnector with read-only checks, SQL parsing, and query execution capabilities
db_context/init.py Updates DatabaseContext to support read-only mode and expose run_sql_query method
README.md Documents read-only mode configuration and usage instructions
Comments suppressed due to low confidence (1)

db_context/database.py:1

  • The _close_connection method implementation was removed but the method is still being called throughout the code. This will cause AttributeError exceptions when the method is invoked.
import sys

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@danielmeppiel danielmeppiel merged commit c4f85ef into danielmeppiel:main Aug 22, 2025
6 checks passed
@danielmeppiel danielmeppiel mentioned this pull request Aug 22, 2025
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.

Full support for executing ad-hoc SQL queries is required

2 participants