Conversation
- Add DmlOnlySqlDriver class to allow DML operations while blocking DDL - Support INSERT, UPDATE, DELETE, and UPSERT operations - Add comprehensive test suite with 46 unit tests - Update documentation and CLI help text - Provides middle-ground security between unrestricted and restricted modes
Addressed all critical issues from code review: 1. RawStmt validation: Now validates inner statement (stmt.stmt) instead of wrapper 2. Error messages: Removed generic wrapper, now returns specific detailed errors 3. Function validation: Use isinstance(node, FuncCall) before accessing funcname 4. Timeout handling: Changed from ValueError to TimeoutError for better error distinction 5. Type safety: Added proper DefElem and FuncCall imports and type checking - Mirror SafeSqlDriver implementation patterns exactly - Improve error message clarity (e.g., 'Statement type CreateStmt not allowed in DML_ONLY mode') - Update all 46 tests to match new error message patterns - All tests passing (53/53) - Code style checks passing (ruff)
Add critical safety feature to prevent accidental data loss: - UPDATE and DELETE statements now require WHERE clause - Prevents accidental modification/deletion of all rows - Added 2 new tests for WHERE clause validation - Updated test_update_with_case to include WHERE clause - Updated PR description to reflect this design decision All 55 tests passing (48 DML_ONLY + 7 access mode).
|
This is a good feature addressing a real gap - application agents that need to modify data but shouldn't touch schema. The implementation is well-done: good test coverage, thoughtful safety features like the WHERE clause requirement for UPDATE/DELETE, and reuse of the existing I wanted to think through a couple of things to make sure we get the design right. One concern is the namingThe name
Since this mode allows both DQL and DML while blocking DDL, calling it Some alternatives:
Background on the designWe originally went with only two modes and with names that do not reference the technical specifics because it is difficult to make precise guarantees about the behavior. For example, we can restrict DDL when entered directly in a query, but what if it happens in a stored procedure or in an extension? The README describes the current design as deliberately choosing "two extremes" of the convenience/safety spectrum - UNRESTRICTED for dev environments, RESTRICTED for production. We designed these modes around use cases. Next stepsI've opened #138 to discuss the broader access modes design. Let's continue the conversation there. |
Add DML_ONLY Access Mode
Overview
Adds a new
DML_ONLYaccess mode to postgres-mcp that allows data manipulation (INSERT, UPDATE, DELETE, SELECT) while blocking schema changes and other DDL operations. This provides a middle ground betweenUNRESTRICTED(allows everything) andRESTRICTED(read-only) modes.Motivation
Users often need to perform data modifications without having the ability to alter database schema. The existing access modes didn't support this use case:
UNRESTRICTED: Too permissive, allows DDL operationsRESTRICTED: Too restrictive, blocks all writes including DMLDML_ONLYmode enables safe data manipulation for use cases like:Implementation Details
New Components:
DmlOnlySqlDriverclass insrc/postgres_mcp/sql/dml_only_sql.pySafeSqlDriver.ALLOWED_FUNCTIONSand extendsALLOWED_NODE_TYPESAllowed Operations:
SELECT- Read queriesINSERT- Including UPSERT withON CONFLICTUPDATE- With all standard clauses (WHERE, RETURNING, etc.)DELETE- With all standard clausesEXPLAIN- Query planning (but notEXPLAIN ANALYZE)SHOW- View configuration variablesBlocked Operations:
CREATE/ALTER/DROP TABLECREATE/DROP INDEXCREATE/DROP VIEW/FUNCTION/SCHEMA/DATABASETRUNCATEVACUUMCREATE/DROP EXTENSIONSET(configuration changes)BEGIN/COMMIT/ROLLBACK(transaction control)EXPLAIN ANALYZE(can impact performance)Usage:
Testing
Design Decisions
RawStmt Validation: Validates inner statement (
stmt.stmt) to properly check the actual SQL command, not just the wrapper nodeFunction Allow-list: Reuses
SafeSqlDriver.ALLOWED_FUNCTIONSto maintain consistency with read-only mode's security modelError Messages: Provides specific, actionable error messages (e.g., "Statement type CreateStmt not allowed in DML_ONLY mode") rather than generic failures
Timeout Handling: Returns
TimeoutError(notValueError) for query timeouts, enabling callers to distinguish timeout from validation failuresDELETE/UPDATE without WHERE: Required for safety. UPDATE and DELETE statements must include a WHERE clause to prevent accidental modification or deletion of all rows. This is a critical safety feature that helps prevent data loss from mistaken queries
Files Changed
src/postgres_mcp/server.py- Add DML_ONLY to AccessMode enum, update driver selectionsrc/postgres_mcp/sql/dml_only_sql.py- New DmlOnlySqlDriver implementationsrc/postgres_mcp/sql/__init__.py- Export DmlOnlySqlDrivertests/unit/sql/test_dml_only_sql.py- New comprehensive test suitetests/unit/test_access_mode.py- Add DML_ONLY test casesREADME.md- Document new access mode and usageRelated Documentation
Implementation follows the plan outlined in
DML_ONLY_MODE_IMPLEMENTATION.mdBreaking Changes
None. This is a purely additive feature that doesn't modify existing behavior.