fix: enforce read-only mode at the database engine, not just the keyword classifier

src/connectors/__tests__/mariadb.integration.test.ts

@@ -474,4 +474,51 @@ describe('MariaDB Connector Integration Tests', () => {
       expect(result.rows.length).toBeGreaterThanOrEqual(3);
     });
   });
+
+  describe('Per-tool readonly engine backstop (options.readonly)', () => {
+    // The READ ONLY transaction reliably blocks DML. (DDL like DROP performs an
+    // implicit commit and escapes the transaction; stacked-DDL payloads such as
+    // `SELECT 1--1;DROP TABLE t` are instead rejected upstream by the read-only
+    // classifier — see the unit tests in allowed-keywords.test.ts.)
+    it('should block a stacked UPDATE hidden after -- via the READ ONLY transaction', async () => {
+      const connector = new MariaDBConnector();
+      try {
+        await connector.connect(mariadbTest.connectionString);
+
+        await expect(
+          connector.executeSQL("SELECT 1--1;UPDATE users SET name='hacked'", { readonly: true })
+        ).rejects.toThrow(/read only|read-only/i);
+
+        const check = await connector.executeSQL(
+          "SELECT COUNT(*) AS c FROM users WHERE name='hacked'",
+          {}
+        );
+        expect(Number(check.rows[0].c)).toBe(0);
+      } finally {
+        await connector.disconnect();
+      }
+    });
+
+    it('should block INSERT and keep the connection writable afterward', async () => {
+      const connector = new MariaDBConnector();
+      try {
+        await connector.connect(mariadbTest.connectionString);
+
+        await expect(
+          connector.executeSQL("INSERT INTO users (name, email) VALUES ('ro', 'ro@ro.com')", {
+            readonly: true,
+          })
+        ).rejects.toThrow(/read only|read-only/i);
+
+        const insert = await connector.executeSQL(
+          "INSERT INTO users (name, email) VALUES ('rw', 'rw@rw.com')",
+          {}
+        );
+        expect(insert.rowCount).toBe(1);
+        await connector.executeSQL("DELETE FROM users WHERE email = 'rw@rw.com'", {});
+      } finally {
+        await connector.disconnect();
+      }
+    });
+  });
 });

src/connectors/__tests__/mysql.integration.test.ts

@@ -459,4 +459,55 @@ describe('MySQL Connector Integration Tests', () => {
       }
     });
   });
+
+  describe('Per-tool readonly engine backstop (options.readonly)', () => {
+    // The READ ONLY transaction reliably blocks DML. (DDL like DROP performs an
+    // implicit commit and escapes the transaction; stacked-DDL payloads such as
+    // `SELECT 1--1;DROP TABLE t` are instead rejected upstream by the read-only
+    // classifier — see the unit tests in allowed-keywords.test.ts.)
+    it('should block a stacked UPDATE hidden after -- via the READ ONLY transaction', async () => {
+      const connector = new MySQLConnector();
+      try {
+        await connector.connect(mysqlTest.connectionString);
+
+        // multipleStatements is on, so MySQL sees this as SELECT then UPDATE. The
+        // READ ONLY transaction must reject the UPDATE (DML) at the engine.
+        await expect(
+          connector.executeSQL("SELECT 1--1;UPDATE users SET name='hacked'", { readonly: true })
+        ).rejects.toThrow(/read only|read-only/i);
+
+        // No row should have been renamed.
+        const check = await connector.executeSQL(
+          "SELECT COUNT(*) AS c FROM users WHERE name='hacked'",
+          {}
+        );
+        expect(Number(check.rows[0].c)).toBe(0);
+      } finally {
+        await connector.disconnect();
+      }
+    });
+
+    it('should block INSERT and keep the connection writable afterward', async () => {
+      const connector = new MySQLConnector();
+      try {
+        await connector.connect(mysqlTest.connectionString);
+
+        await expect(
+          connector.executeSQL("INSERT INTO users (name, email) VALUES ('ro', 'ro@ro.com')", {
+            readonly: true,
+          })
+        ).rejects.toThrow(/read only|read-only/i);
+
+        // Non-read-only call on the same pooled connection still works.
+        const insert = await connector.executeSQL(
+          "INSERT INTO users (name, email) VALUES ('rw', 'rw@rw.com')",
+          {}
+        );
+        expect(insert.rowCount).toBe(1);
+        await connector.executeSQL("DELETE FROM users WHERE email = 'rw@rw.com'", {});
+      } finally {
+        await connector.disconnect();
+      }
+    });
+  });
 });

src/connectors/__tests__/postgres.integration.test.ts

@@ -596,6 +596,69 @@ describe('PostgreSQL Connector Integration Tests', () => {
     });
   });
 
+  describe('Per-tool readonly engine backstop (options.readonly)', () => {
+    // These mirror the realistic config: the connection is writable (no
+    // connection-level readonly), and read-only is enforced per execution by
+    // running inside a READ ONLY transaction. This catches function-based writes
+    // that the leading-keyword classifier passes (e.g. SELECT setval()).
+    it('should block a sequence write via SELECT setval() when options.readonly is set', async () => {
+      const connector = new PostgresConnector();
+      try {
+        await connector.connect(postgresTest.connectionString);
+
+        // SELECT setval(...) passes the read-only keyword classifier but writes a
+        // sequence; the READ ONLY transaction must reject it at the engine.
+        await expect(
+          connector.executeSQL(
+            "SELECT setval(pg_get_serial_sequence('users','id'), 1, true)",
+            { readonly: true }
+          )
+        ).rejects.toThrow(/read-only transaction/i);
+      } finally {
+        await connector.disconnect();
+      }
+    });
+
+    it('should block INSERT when options.readonly is set', async () => {
+      const connector = new PostgresConnector();
+      try {
+        await connector.connect(postgresTest.connectionString);
+        await expect(
+          connector.executeSQL(
+            "INSERT INTO users (name, email) VALUES ('ro', 'ro@ro.com')",
+            { readonly: true }
+          )
+        ).rejects.toThrow(/read-only transaction/i);
+      } finally {
+        await connector.disconnect();
+      }
+    });
+
+    it('should keep the connection writable for non-read-only calls', async () => {
+      const connector = new PostgresConnector();
+      try {
+        await connector.connect(postgresTest.connectionString);
+
+        // A read-only call (rolled back) must not leave the session read-only.
+        await expect(
+          connector.executeSQL("INSERT INTO users (name, email) VALUES ('x', 'x@x.com')", {
+            readonly: true,
+          })
+        ).rejects.toThrow();
+
+        const insert = await connector.executeSQL(
+          "INSERT INTO users (name, email) VALUES ('rw', 'rw@rw.com') RETURNING id",
+          {}
+        );
+        expect(insert.rows).toHaveLength(1);
+
+        await connector.executeSQL("DELETE FROM users WHERE email = 'rw@rw.com'", {});
+      } finally {
+        await connector.disconnect();
+      }
+    });
+  });
+
   describe('Search Path Configuration Tests', () => {
     it('should use first schema in search_path as default for discovery', async () => {
       const connector = new PostgresConnector();

src/connectors/__tests__/sqlite.integration.test.ts

@@ -563,4 +563,67 @@ describe('SQLite Connector Integration Tests', () => {
       ).rejects.toThrow();
     });
   });
+
+  describe('Per-tool readonly engine backstop (options.readonly)', () => {
+    // The connection itself is writable (shared by read-only and writable tools);
+    // read-only enforcement is applied per execution via PRAGMA query_only.
+    it('should block a write-effecting PRAGMA when options.readonly is set', async () => {
+      // query_only makes the engine reject the header write; the classifier would
+      // also reject the assignment form, so this is defense in depth.
+      await expect(
+        sqliteTest.connector.executeSQL('PRAGMA user_version = 1337', { readonly: true })
+      ).rejects.toThrow(/readonly|query_only/i);
+    });
+
+    it('should block INSERT when options.readonly is set', async () => {
+      await expect(
+        sqliteTest.connector.executeSQL(
+          "INSERT INTO users (name, email) VALUES ('ro', 'ro@test.com')",
+          { readonly: true }
+        )
+      ).rejects.toThrow(/readonly/i);
+    });
+
+    it('should restore writability for non-read-only calls on the same connection', async () => {
+      // A read-only call toggles query_only ON; it must be turned back OFF after.
+      await expect(
+        sqliteTest.connector.executeSQL('PRAGMA user_version = 1', { readonly: true })
+      ).rejects.toThrow();
+
+      // Subsequent writable call must succeed.
+      const insert = await sqliteTest.connector.executeSQL(
+        "INSERT INTO users (name, email) VALUES ('rw', 'rw@test.com')",
+        {}
+      );
+      expect(insert.rowCount).toBe(1);
+
+      // Cleanup
+      await sqliteTest.connector.executeSQL("DELETE FROM users WHERE email = 'rw@test.com'", {});
+    });
+
+    it('should allow read-only PRAGMA and SELECT when options.readonly is set', async () => {
+      const select = await sqliteTest.connector.executeSQL('SELECT 1 AS one', { readonly: true });
+      expect(Number(select.rows[0].one)).toBe(1);
+
+      const pragma = await sqliteTest.connector.executeSQL('PRAGMA table_info(users)', { readonly: true });
+      expect(pragma.rows.length).toBeGreaterThan(0);
+    });
+
+    it('should not let an in-batch query_only toggle disable the backstop', async () => {
+      // Even if the classifier were skipped, the engine re-asserts query_only=ON
+      // before each statement, so a mid-batch toggle cannot enable the later INSERT.
+      await expect(
+        sqliteTest.connector.executeSQL(
+          "PRAGMA query_only = OFF; INSERT INTO users (name, email) VALUES ('bypass', 'bypass@test.com')",
+          { readonly: true }
+        )
+      ).rejects.toThrow(/readonly|query_only/i);
+
+      const check = await sqliteTest.connector.executeSQL(
+        "SELECT COUNT(*) AS c FROM users WHERE email = 'bypass@test.com'",
+        {}
+      );
+      expect(Number(check.rows[0].c)).toBe(0);
+    });
+  });
 });

src/connectors/manager.ts

@@ -242,10 +242,12 @@ export class ConnectorManager {
     if (source.query_timeout !== undefined && connector.id !== 'sqlite') {
       config.queryTimeoutSeconds = source.query_timeout;
     }
-    // Pass readonly flag for SDK-level enforcement (PostgreSQL, SQLite)
-    if (source.readonly !== undefined) {
-      config.readonly = source.readonly;
-    }
+    // Note: read-only enforcement is per-tool, not per-source. It is applied at
+    // execution time via ExecuteOptions.readonly. Some connectors also add an
+    // engine-level backstop in executeSQL (e.g. READ ONLY transactions or SQLite PRAGMA query_only),
+    // because a single source connection may be shared by both read-only and
+    // writable tools. ConnectorConfig.readonly (connection-level) remains supported
+    // for direct connector use but is intentionally not wired from source config.
     // Pass search_path for PostgreSQL
     if (source.search_path) {
       config.searchPath = source.search_path;

src/connectors/mariadb/index.ts

@@ -599,6 +599,18 @@ export class MariaDBConnector implements Connector {
     // This is critical for session-specific features like LAST_INSERT_ID()
     const conn = await this.pool.getConnection();
     try {
+      // Engine-level read-only backstop: run the batch inside a READ ONLY
+      // transaction so MariaDB rejects DML writes (INSERT/UPDATE/DELETE/REPLACE)
+      // that the keyword classifier missed (e.g. function-based writes). Note this
+      // does NOT stop DDL: statements like DROP/CREATE perform an implicit COMMIT
+      // that ends the read-only transaction first, so DDL escapes. Stacked-DDL
+      // payloads (e.g. `SELECT 1--1;DROP TABLE t`) are instead rejected upstream by
+      // the read-only classifier, which now splits `--`-hidden statements (see
+      // scanSingleLineCommentMySQL in sql-parser.ts).
+      if (options.readonly) {
+        await conn.query("START TRANSACTION READ ONLY");
+      }
+
       // Apply maxRows limit to SELECT queries if specified
       let processedSQL = sql;
       if (options.maxRows) {
@@ -634,8 +646,20 @@ export class MariaDBConnector implements Connector {
       // Parse results using shared utility that handles both single and multi-statement queries
       const rows = parseQueryResults(results);
       const rowCount = extractAffectedRows(results);
+
+      if (options.readonly) {
+        await conn.query("COMMIT");
+      }
       return { rows, rowCount };
     } catch (error) {
+      if (options.readonly) {
+        // Best-effort rollback so the connection returns to the pool clean.
+        try {
+          await conn.query("ROLLBACK");
+        } catch {
+          // ignore rollback failure; the original error is more useful
+        }
+      }
       console.error("Error executing query:", error);
       throw error;
     } finally {

src/connectors/mysql/index.ts

@@ -612,6 +612,18 @@ export class MySQLConnector implements Connector {
     // This is critical for session-specific features like LAST_INSERT_ID()
     const conn = await this.pool.getConnection();
     try {
+      // Engine-level read-only backstop: run the batch inside a READ ONLY
+      // transaction so MySQL rejects DML writes (INSERT/UPDATE/DELETE/REPLACE)
+      // that the keyword classifier missed (e.g. function-based writes). Note this
+      // does NOT stop DDL: statements like DROP/CREATE perform an implicit COMMIT
+      // that ends the read-only transaction first, so DDL escapes. Stacked-DDL
+      // payloads (e.g. `SELECT 1--1;DROP TABLE t`) are instead rejected upstream by
+      // the read-only classifier, which now splits `--`-hidden statements (see
+      // scanSingleLineCommentMySQL in sql-parser.ts).
+      if (options.readonly) {
+        await conn.query("START TRANSACTION READ ONLY");
+      }
+
       // Apply maxRows limit to SELECT queries if specified
       let processedSQL = sql;
       if (options.maxRows) {
@@ -651,8 +663,20 @@ export class MySQLConnector implements Connector {
       // Parse results using shared utility that handles both single and multi-statement queries
       const rows = parseQueryResults(firstResult);
       const rowCount = extractAffectedRows(firstResult);
+
+      if (options.readonly) {
+        await conn.query("COMMIT");
+      }
       return { rows, rowCount };
     } catch (error) {
+      if (options.readonly) {
+        // Best-effort rollback so the connection returns to the pool clean.
+        try {
+          await conn.query("ROLLBACK");
+        } catch {
+          // ignore rollback failure; the original error is more useful
+        }
+      }
       console.error("Error executing query:", error);
       throw error;
     } finally {