feat(readonly): add read-only mode for all database connections

## Background Add a read-only toggle for database connections that blocks all write operations (INSERT, UPDATE, DELETE, DROP, etc.) when enabled, allowing only read queries (SELECT, SHOW, EXPLAIN, etc.). Closes #889. ## Changes ### Data Model - Added `read_only: bool` field to ConnectionConfig (Rust struct, TS interfaces, and ConnectionConfigData deserialization layer) - Used `#[serde(default, skip_serializing_if = "is_false")]` for backward/forward compatibility with existing configs ### Frontend UI - Added "Read Only" checkbox in connection dialog - 7-language i18n support (en/es/it/pt-BR/zh-CN/zh-TW) ### SQL Classification (query_execution_sql.rs) - Added `is_write_sql()` with two-layer defense: - Layer 1: First-keyword check (must start with known read keyword) - Layer 2: Embedded dangerous keyword detection (catches CTE-wrapped writes like `WITH ... AS (DELETE FROM ...)`) - `FROM` keyword supported as DuckDB SELECT-less syntax indicator - Added `check_read_only()` returning descriptive error with connection name - Added 16 unit tests covering: pure reads/writes, CTE, case insensitivity, string literal masking, comment stripping, stored procedure calls, edge cases ### SQL Execution Guards (query.rs / transfer.rs) - Added `check_read_only_for_connection()` and `_multi()` helper functions with lazy name clone (only allocates when read_only is true) - 6 interception points: `do_execute`, MySQL batch, SQL Server batch, DuckDB batch, transaction execution, transfer execution ### Non-SQL Write Guards — Tauri Commands - Added `ensure_connection_writable()` helper (connection.rs) - Mongo: 6 write entry points (insert/update/delete, single + batch) - Redis: 15 write entry points (SET, DEL, HSET, HDEL, LPUSH, LSET, LREM, SADD, SREM, ZADD, ZREM, EXPIRE, FLUSHDB, delete_keys, execute_command) - execute_command uses RedisCommandSafety classification to allow safe read commands through raw command interface - etcd: 2 write entry points (put, delete) - sql_file: SQL file execution guarded - MCP Bridge: 4 write entry points (Insert/Update/Delete/SQL query) ### Non-SQL Write Guards — Web API - Local `ensure_writable()` helper in each route module - Redis: 12 write endpoints including classified execute_command - Mongo: 6 write endpoints - etcd: 2 write endpoints - sql_file, table_import, transfer: early rejection ### Test Updates - Updated ConnectionConfig construction in 7 test files with `read_only: false` initialization ## Defense-in-Depth - Layer 1: Command/Route-level early rejection (saves resources) - Layer 2: Core SQL classifier (`is_write_sql` — first keyword + embedded scan) - Layer 3: Core execution-time interception (do_execute/transfer/transaction)
2026-11-05 08:54:20 +00:00 · 2026-06-10 10:58:52 +08:00
1 parent 7798f2231e
commit a440415e86
@@ -415,6 +415,7 @@ mod tests {
            jdbc_driver_class: None,
            jdbc_driver_paths: Vec::new(),
            one_time: false,
+            read_only: false,
        }
    }

@@ -596,6 +596,7 @@ mod tests {
            jdbc_driver_class: None,
            jdbc_driver_paths: Vec::new(),
            one_time: false,
+            read_only: false,
        };
        scrub_connection_secrets(&mut config);
        assert!(config.password.is_empty());
@@ -1210,6 +1210,7 @@ mod tests {
            jdbc_driver_class: None,
            jdbc_driver_paths: Vec::new(),
            one_time: false,
+            read_only: false,
        }
    }

@@ -511,6 +511,7 @@ mod tests {
            jdbc_driver_class: None,
            jdbc_driver_paths: Vec::new(),
            one_time: false,
+            read_only: false,
        }
    }

@@ -71,6 +71,8 @@ pub struct ConnectionConfig {
    pub jdbc_driver_paths: Vec<String>,
    #[serde(default, skip_serializing_if = "is_false")]
    pub one_time: bool,
+    #[serde(default, skip_serializing_if = "is_false")]
+    pub read_only: bool,
 }

 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
@@ -341,6 +343,8 @@ struct ConnectionConfigData {
    pub jdbc_driver_paths: Vec<String>,
    #[serde(default)]
    pub one_time: bool,
+    #[serde(default)]
+    pub read_only: bool,
 }

 impl From<ConnectionConfigData> for ConnectionConfig {
@@ -383,6 +387,7 @@ impl From<ConnectionConfigData> for ConnectionConfig {
            jdbc_driver_class: data.jdbc_driver_class,
            jdbc_driver_paths: data.jdbc_driver_paths,
            one_time: data.one_time,
+            read_only: data.read_only,
        }
    }
 }
@@ -1349,6 +1354,7 @@ mod tests {
            jdbc_driver_class: None,
            jdbc_driver_paths: Vec::new(),
            one_time: false,
+            read_only: false,
        }
    }

@@ -15,6 +15,47 @@ pub const QUERY_TIMEOUT: Duration = Duration::from_secs(30);
 pub const MAX_ROWS: usize = 10000;
 pub const QUERY_CANCELED: &str = "Query canceled";

+/// Check read-only protection for a connection, blocking write SQL statements.
+/// Only clones the connection name when read-only mode is active, avoiding
+/// unnecessary allocations otherwise.
+/// Uses config_for_pool_key to correctly resolve configs when pool_key includes
+/// a database suffix (e.g., "prod:app" → config stored under "prod").
+pub async fn check_read_only_for_connection(state: &AppState, pool_key: &str, sql: &str) -> Result<(), String> {
+    let conn_name = {
+        let configs = state.configs.read().await;
+        crate::connection::config_for_pool_key(pool_key, &configs).filter(|c| c.read_only).map(|c| c.name.clone())
+    };
+    if let Some(name) = conn_name {
+        crate::query_execution_sql::check_read_only(sql, &name)?;
+    }
+    Ok(())
+}
+
+/// Check read-only protection for a connection across multiple SQL statements.
+pub async fn check_read_only_for_connection_multi(
+    state: &AppState,
+    pool_key: &str,
+    statements: &[impl AsRef<str>],
+) -> Result<(), String> {
+    let conn_name = {
+        let configs = state.configs.read().await;
+        crate::connection::config_for_pool_key(pool_key, &configs).filter(|c| c.read_only).map(|c| c.name.clone())
+    };
+    if let Some(name) = conn_name {
+        for sql in statements {
+            crate::query_execution_sql::check_read_only(sql.as_ref(), &name)?;
+        }
+    }
+    Ok(())
+}
+
+/// Check whether a connection has read-only mode enabled, returning the connection name if so.
+/// This uses connection_id directly (not pool_key), so it is safe to call at command entry points
+/// before any pool key is constructed.
+pub async fn connection_readonly_name(state: &AppState, connection_id: &str) -> Option<String> {
+    state.configs.read().await.get(connection_id).filter(|c| c.read_only).map(|c| c.name.clone())
+}
+
 async fn connection_is_mongodb(state: &AppState, connection_id: &str) -> bool {
    let configs = state.configs.read().await;
    configs.get(connection_id).is_some_and(|config| config.db_type == DatabaseType::MongoDb)
@@ -561,13 +602,18 @@ pub async fn do_execute(
    options: QueryExecutionOptions,
 ) -> Result<db::QueryResult, String> {
    let query_timeout = resolve_query_timeout(options.timeout_secs);
-    let duckdb_attached_names = state
-        .configs
-        .read()
-        .await
-        .get(pool_key)
-        .map(|config| config.attached_databases.iter().map(|database| database.name.clone()).collect::<Vec<_>>())
-        .unwrap_or_default();
+    let (duckdb_attached_names, conn_name_if_readonly) = {
+        let configs = state.configs.read().await;
+        let config = crate::connection::config_for_pool_key(pool_key, &configs);
+        let attached = config
+            .map(|c| c.attached_databases.iter().map(|db| db.name.clone()).collect::<Vec<_>>())
+            .unwrap_or_default();
+        let conn_name = config.filter(|c| c.read_only).map(|c| c.name.clone());
+        (attached, conn_name)
+    };
+    if let Some(name) = conn_name_if_readonly {
+        crate::query_execution_sql::check_read_only(sql, &name)?;
+    }
    let pool_db_type = connection_database_type_for_pool_key(state, pool_key).await;
    let connections = state.connections.read().await;
    let pool = connections.get(pool_key).ok_or("Connection not found")?;
@@ -955,6 +1001,8 @@ pub async fn execute_multi_core_with_options(
    }

    if let Some((pool, mode)) = mysql_pool {
+        // Read-only check for MySQL batch path
+        check_read_only_for_connection_multi(state, &pool_key, &statements).await?;
        let mysql_dialect = connection_mysql_query_dialect(state, connection_id).await;
        return execute_multi_mysql(&pool, mode, mysql_dialect, &statements, cancel_token, options).await;
    }
@@ -1045,6 +1093,9 @@ async fn execute_multi_sqlserver(
    options: QueryExecutionOptions,
 ) -> Result<Vec<db::QueryResult>, String> {
    let batches = split_sql_batches(sql);
+
+    // Read-only check for SQL Server batch path
+    check_read_only_for_connection_multi(state, pool_key, &batches).await?;
    let mut all_results = Vec::new();
    let max_rows = options.max_rows;

@@ -1194,6 +1245,9 @@ pub async fn execute_statements_in_transaction(
        state.get_or_create_pool(connection_id, Some(database)).await?
    };

+    // Read-only check: intercept all transaction paths before dispatching
+    check_read_only_for_connection_multi(state, &pool_key, statements).await?;
+
    let start = std::time::Instant::now();

    // Clone the pool handle within the lock, then drop it before any async work.
@@ -1749,6 +1803,7 @@ mod tests {
            jdbc_driver_class: None,
            jdbc_driver_paths: Vec::new(),
            one_time: false,
+            read_only: false,
        };

        let params = external_driver_query_params(
@@ -97,13 +97,113 @@ fn is_safe_explain_source(sql: &str) -> bool {
    })
 }

-fn contains_dangerous_sql_keyword(sql: &str) -> bool {
+pub fn contains_dangerous_sql_keyword(sql: &str) -> bool {
    let source = strip_sql_comments_and_literals(sql).to_lowercase();
    ["drop", "delete", "truncate", "alter", "update", "merge", "replace", "insert", "create"]
        .iter()
        .any(|keyword| contains_word(&source, keyword))
 }

+/// Keywords that start a read-only SQL statement.
+/// Note: FROM is a DuckDB-specific read keyword supporting SELECT-less FROM syntax
+/// (e.g. `FROM table SELECT *`). In other databases, a statement starting with FROM
+/// is invalid and would be rejected by the database itself, so allowing it poses no risk.
+///
+/// PRAGMA is intentionally NOT in this list because some PRAGMA statements modify
+/// database or session state (e.g. SQLite `PRAGMA journal_mode=WAL`). Instead,
+/// read-only PRAGMA forms are handled separately in `is_safe_read_pragma`.
+const READ_SQL_KEYWORDS: &[&str] = &["SELECT", "WITH", "SHOW", "DESCRIBE", "DESC", "EXPLAIN", "FROM"];
+
+/// PRAGMA names that are known to be safe read-only queries in SQLite/DuckDB.
+/// Only the function-call form `PRAGMA name(args)` matching these names is allowed.
+/// Any PRAGMA with assignment (`PRAGMA name = value`) or not in this list is blocked.
+const SAFE_READ_PRAGMA_NAMES: &[&str] = &[
+    "TABLE_INFO",
+    "TABLE_XINFO",
+    "INDEX_LIST",
+    "INDEX_INFO",
+    "FOREIGN_KEY_LIST",
+    "DATABASE_LIST",
+    "COMPILE_OPTIONS",
+    "DATA_VERSION",
+];
+
+/// Returns true if the SQL statement is a write operation (not a pure read).
+pub fn is_write_sql(sql: &str) -> bool {
+    // 1. Strip comments and string literals
+    let cleaned = strip_sql_comments_and_literals(sql);
+    let trimmed = cleaned.trim_start();
+    if trimmed.is_empty() {
+        return false;
+    }
+    let upper = trimmed.to_uppercase();
+
+    // 2. Check if first keyword is a read keyword
+    let starts_with_read = READ_SQL_KEYWORDS.iter().any(|kw| {
+        upper.starts_with(kw) && (upper.len() == kw.len() || !upper.as_bytes()[kw.len()].is_ascii_alphanumeric())
+    });
+
+    // 3. Special handling for PRAGMA: only allow safe read-only forms
+    if !starts_with_read && starts_with_keyword(&upper, "PRAGMA") {
+        return !is_safe_read_pragma(&upper);
+    }
+
+    // A statement is a write if it doesn't start with a read keyword,
+    // or if it contains embedded dangerous keywords (e.g. CTE-wrapped writes like WITH ... AS (DELETE FROM ...))
+    !starts_with_read || contains_dangerous_sql_keyword(sql)
+}
+
+/// Check if a PRAGMA statement is a safe read-only form.
+/// Allows: PRAGMA table_info(...), PRAGMA index_list(...), etc.
+/// Blocks: PRAGMA name = value, PRAGMA name(value), or unknown PRAGMA names.
+fn is_safe_read_pragma(upper_stripped: &str) -> bool {
+    // Skip "PRAGMA" keyword to get the rest
+    let rest = upper_stripped.strip_prefix("PRAGMA").unwrap_or("").trim_start();
+
+    if rest.is_empty() {
+        return false;
+    }
+
+    // Extract the pragma name (first word)
+    let name_end = rest.find(|c: char| !c.is_ascii_alphanumeric() && c != '_').unwrap_or(rest.len());
+    let pragma_name = &rest[..name_end];
+
+    // Check if it's in the safe list
+    if !SAFE_READ_PRAGMA_NAMES.iter().any(|&safe| pragma_name == safe) {
+        return false;
+    }
+
+    // Check the form after the name: must be function-call style "(...)" or end of statement
+    let after_name = rest[name_end..].trim_start();
+    if after_name.is_empty() {
+        // PRAGMA table_info (no args) — safe
+        return true;
+    }
+    if after_name.starts_with('(') {
+        // PRAGMA table_info(users) — safe read form
+        return true;
+    }
+    // PRAGMA table_info = something or other unsafe form — blocked
+    false
+}
+
+fn starts_with_keyword(upper: &str, keyword: &str) -> bool {
+    upper.starts_with(keyword)
+        && (upper.len() == keyword.len() || !upper.as_bytes()[keyword.len()].is_ascii_alphanumeric())
+}
+
+/// Check whether a SQL statement is allowed under read-only mode.
+/// Returns Err with a descriptive message if the statement is a write operation.
+pub fn check_read_only(sql: &str, connection_name: &str) -> Result<(), String> {
+    if is_write_sql(sql) {
+        return Err(format!(
+            "Read-only mode: connection '{}' has read-only protection enabled. Write operation (including stored procedure calls) blocked.",
+            connection_name
+        ));
+    }
+    Ok(())
+}
+
 fn contains_word(source: &str, word: &str) -> bool {
    let bytes = source.as_bytes();
    let word_bytes = word.as_bytes();
@@ -178,7 +278,7 @@ fn strip_sql_comments(sql: &str) -> String {
    output
 }

-fn strip_sql_comments_and_literals(sql: &str) -> String {
+pub fn strip_sql_comments_and_literals(sql: &str) -> String {
    let mut output = String::with_capacity(sql.len());
    let mut chars = sql.chars().peekable();
    let mut in_line_comment = false;
@@ -354,4 +454,195 @@ mod tests {
            None
        );
    }
+
+    #[test]
+    fn strip_sql_comments_and_literals_basic() {
+        assert_eq!(strip_sql_comments_and_literals("SELECT 1"), "SELECT 1");
+        assert_eq!(strip_sql_comments_and_literals("SELECT 'hello'"), "SELECT        ");
+        assert_eq!(strip_sql_comments_and_literals("SELECT \"hello\""), "SELECT        ");
+        assert_eq!(strip_sql_comments_and_literals("-- comment\nSELECT 1"), " SELECT 1");
+        assert_eq!(strip_sql_comments_and_literals("/* block */ SELECT 1"), "  SELECT 1");
+        assert_eq!(strip_sql_comments_and_literals("# comment\nSELECT 1"), " SELECT 1");
+    }
+
+    #[test]
+    fn strip_sql_comments_and_literals_nested() {
+        // String literals containing comments should be stripped
+        assert_eq!(strip_sql_comments_and_literals("SELECT '/* not a comment */'"), "SELECT                      ");
+        // Comments containing string delimiters should be stripped
+        assert_eq!(strip_sql_comments_and_literals("/* 'not a string' */ SELECT 1"), "  SELECT 1");
+    }
+
+    #[test]
+    fn contains_dangerous_sql_keyword_detects_writes() {
+        assert!(contains_dangerous_sql_keyword("DROP TABLE users"));
+        assert!(contains_dangerous_sql_keyword("DELETE FROM users"));
+        assert!(contains_dangerous_sql_keyword("TRUNCATE TABLE users"));
+        assert!(contains_dangerous_sql_keyword("ALTER TABLE users ADD COLUMN age INT"));
+        assert!(contains_dangerous_sql_keyword("UPDATE users SET name = 'x'"));
+        assert!(contains_dangerous_sql_keyword("MERGE INTO target USING source"));
+        assert!(contains_dangerous_sql_keyword("REPLACE INTO users VALUES (1)"));
+        assert!(contains_dangerous_sql_keyword("INSERT INTO users VALUES (1)"));
+        assert!(contains_dangerous_sql_keyword("CREATE TABLE users (id INT)"));
+    }
+
+    #[test]
+    fn contains_dangerous_sql_keyword_ignores_substrings() {
+        // "updateable" contains "update" as substring but is a different word
+        assert!(!contains_dangerous_sql_keyword("SELECT * FROM updateable_view"));
+        // "dropped" contains "drop" as substring
+        assert!(!contains_dangerous_sql_keyword("SELECT dropped FROM t"));
+        // "inserted" contains "insert"
+        assert!(!contains_dangerous_sql_keyword("SELECT inserted FROM t"));
+    }
+
+    #[test]
+    fn contains_dangerous_sql_keyword_ignores_in_string_literals() {
+        assert!(!contains_dangerous_sql_keyword("SELECT 'DROP TABLE users' FROM t"));
+        assert!(!contains_dangerous_sql_keyword("SELECT 'delete' FROM t"));
+        assert!(!contains_dangerous_sql_keyword("SELECT \"CREATE TABLE\" FROM t"));
+    }
+
+    #[test]
+    fn is_write_sql_detects_simple_writes() {
+        assert!(is_write_sql("INSERT INTO users VALUES (1)"));
+        assert!(is_write_sql("UPDATE users SET name = 'x'"));
+        assert!(is_write_sql("DELETE FROM users"));
+        assert!(is_write_sql("DROP TABLE users"));
+        assert!(is_write_sql("CREATE TABLE users (id INT)"));
+        assert!(is_write_sql("ALTER TABLE users ADD COLUMN age INT"));
+        assert!(is_write_sql("TRUNCATE TABLE users"));
+        assert!(is_write_sql(
+            "MERGE INTO target USING source ON t.id = s.id WHEN MATCHED THEN UPDATE SET t.name = s.name"
+        ));
+        assert!(is_write_sql("REPLACE INTO users VALUES (1)"));
+    }
+
+    #[test]
+    fn is_write_sql_allows_reads() {
+        assert!(!is_write_sql("SELECT * FROM users"));
+        assert!(!is_write_sql("SELECT id, name FROM users WHERE active = true"));
+        assert!(!is_write_sql("WITH cte AS (SELECT 1) SELECT * FROM cte"));
+        assert!(!is_write_sql("SHOW TABLES"));
+        assert!(!is_write_sql("DESCRIBE users"));
+        assert!(!is_write_sql("DESC users"));
+        assert!(!is_write_sql("EXPLAIN SELECT * FROM users"));
+        assert!(!is_write_sql("PRAGMA table_info(users)"));
+        assert!(!is_write_sql("FROM users SELECT *"));
+    }
+
+    #[test]
+    fn is_write_sql_ignores_leading_whitespace_and_comments() {
+        assert!(!is_write_sql("   /* comment */ SELECT * FROM users"));
+        assert!(!is_write_sql("-- comment\nSELECT * FROM users"));
+        assert!(is_write_sql("   /* comment */ INSERT INTO users VALUES (1)"));
+    }
+
+    #[test]
+    fn is_write_sql_cte_with_nested_write() {
+        // CTE starting with WITH but containing a write operation inside
+        assert!(is_write_sql("WITH deleted AS (DELETE FROM users RETURNING id) SELECT * FROM deleted"));
+        assert!(is_write_sql("WITH updated AS (UPDATE users SET name = 'x' RETURNING id) SELECT * FROM updated"));
+        assert!(is_write_sql("WITH inserted AS (INSERT INTO users VALUES (1) RETURNING id) SELECT * FROM inserted"));
+        // Pure read CTE should be allowed
+        assert!(!is_write_sql("WITH cte AS (SELECT * FROM users) SELECT * FROM cte"));
+    }
+
+    #[test]
+    fn is_write_sql_case_insensitive() {
+        assert!(!is_write_sql("select * from users"));
+        assert!(!is_write_sql("Select * From users"));
+        assert!(is_write_sql("insert into users values (1)"));
+        assert!(is_write_sql("Insert Into users Values (1)"));
+        assert!(is_write_sql("update users set name = 'x'"));
+    }
+
+    #[test]
+    fn is_write_sql_edge_cases() {
+        assert!(!is_write_sql("")); // empty string -> not a write
+        assert!(!is_write_sql("   ")); // whitespace only -> not a write
+        assert!(is_write_sql("COMMIT")); // not a recognized read keyword
+        assert!(is_write_sql("ROLLBACK")); // not a recognized read keyword
+        assert!(is_write_sql("BEGIN")); // not a recognized read keyword
+        assert!(is_write_sql("GRANT SELECT ON users TO admin"));
+        assert!(is_write_sql("REVOKE SELECT ON users FROM admin"));
+    }
+
+    #[test]
+    fn is_write_sql_blocks_stored_procedure_calls() {
+        // CALL and EXEC don't start with a read keyword, so they are treated as writes
+        assert!(is_write_sql("CALL my_procedure(1, 2)"));
+        assert!(is_write_sql("call my_procedure()"));
+        assert!(is_write_sql("EXEC sp_update_stats"));
+        assert!(is_write_sql("EXECUTE sp_rename 'old', 'new'"));
+        assert!(is_write_sql("execute my_func()"));
+    }
+
+    #[test]
+    fn is_write_sql_allows_safe_read_pragmas() {
+        // Read-only PRAGMA forms (function-call style with known safe names) are allowed
+        assert!(!is_write_sql("PRAGMA table_info(users)"));
+        assert!(!is_write_sql("PRAGMA table_xinfo(users)"));
+        assert!(!is_write_sql("PRAGMA index_list(users)"));
+        assert!(!is_write_sql("PRAGMA index_info(idx_name)"));
+        assert!(!is_write_sql("PRAGMA foreign_key_list(users)"));
+        assert!(!is_write_sql("PRAGMA database_list"));
+        assert!(!is_write_sql("PRAGMA compile_options"));
+        assert!(!is_write_sql("PRAGMA data_version"));
+        assert!(!is_write_sql("pragma table_info(users)"));
+    }
+
+    #[test]
+    fn is_write_sql_blocks_unsafe_pragmas() {
+        // Assignment forms are always blocked
+        assert!(is_write_sql("PRAGMA journal_mode = WAL"));
+        assert!(is_write_sql("PRAGMA synchronous = OFF"));
+        assert!(is_write_sql("PRAGMA foreign_keys = ON"));
+        assert!(is_write_sql("PRAGMA cache_size = -2000"));
+        assert!(is_write_sql("PRAGMA user_version = 123"));
+        // Unknown PRAGMA names are blocked
+        assert!(is_write_sql("PRAGMA writable_schema = ON"));
+        assert!(is_write_sql("PRAGMA locking_mode = EXCLUSIVE"));
+        assert!(is_write_sql("PRAGMA temp_store = MEMORY"));
+        assert!(is_write_sql("PRAGMA some_unknown_pragma"));
+    }
+
+    #[test]
+    fn is_write_sql_string_literal_hides_keywords() {
+        // The dangerous keyword is inside a string literal, so it should NOT be detected
+        assert!(!is_write_sql("SELECT 'DROP TABLE users' AS hint FROM t"));
+        assert!(!is_write_sql("SELECT * FROM t WHERE name = 'delete'"));
+    }
+
+    #[test]
+    fn check_read_only_success_and_error() {
+        assert_eq!(check_read_only("SELECT * FROM users", "prod-db"), Ok(()));
+        assert_eq!(check_read_only("WITH cte AS (SELECT 1) SELECT * FROM cte", "prod-db"), Ok(()));
+
+        let err = check_read_only("DELETE FROM users", "prod-db");
+        assert!(err.is_err());
+        assert_eq!(
+            err.unwrap_err(),
+            "Read-only mode: connection 'prod-db' has read-only protection enabled. Write operation (including stored procedure calls) blocked."
+        );
+
+        let err2 = check_read_only("UPDATE users SET name = 'x'", "reporting-db");
+        assert!(err2.is_err());
+        assert!(err2.unwrap_err().contains("reporting-db"));
+    }
+
+    #[test]
+    fn strip_sql_comments_basic() {
+        assert_eq!(strip_sql_comments("SELECT 1"), "SELECT 1");
+        assert_eq!(strip_sql_comments("-- comment\nSELECT 1"), " SELECT 1");
+        assert_eq!(strip_sql_comments("/* block */ SELECT 1"), "  SELECT 1");
+        assert_eq!(strip_sql_comments("# comment\nSELECT 1"), " SELECT 1");
+    }
+
+    #[test]
+    fn strip_sql_comments_preserves_strings() {
+        // strip_sql_comments does NOT handle string delimiters, so it strips
+        // comments even inside string literals
+        assert_eq!(strip_sql_comments("SELECT 'hello /* not a comment */'"), "SELECT 'hello  '");
+    }
 }
@@ -658,6 +658,7 @@ mod tests {
            jdbc_driver_class: None,
            jdbc_driver_paths: Vec::new(),
            one_time: false,
+            read_only: false,
        }
    }

@@ -1624,6 +1624,8 @@ pub async fn execute_on_pool_with_max_rows(
    sql: &str,
    max_rows: Option<usize>,
 ) -> Result<db::QueryResult, String> {
+    // Read-only check: block transfer operations in readonly mode
+    crate::query::check_read_only_for_connection(state, pool_key, sql).await?;
    let connections = state.connections.read().await;
    let pool = connections.get(pool_key).ok_or("Connection not found")?;

@@ -3155,6 +3157,7 @@ mod tests {
            jdbc_driver_class: None,
            jdbc_driver_paths: Vec::new(),
            one_time: false,
+            read_only: false,
        }
    }