Merge pull request #1232 from QueryaHub/feat/1231-mcp-rule-ids

feat(mcp): every refused query records the guard rule that refused it (#1231)

Author: ZhuchkaTriplesix

lib/core/mcp/mcp_query_service.dart

@@ -13,10 +13,13 @@ import 'package:querya_desktop/features/workspace/sql_execution_delegate.dart';
 /// A failure the MCP client should see as a tool error (the model reads the
 /// message and can fix its call).
 class McpToolException implements Exception {
-  const McpToolException(this.message);
+  const McpToolException(this.message, {this.rule});
 
   final String message;
 
+  /// The guard rule that refused the call, when a rule did.
+  final String? rule;
+
   @override
   String toString() => message;
 }
@@ -218,8 +221,10 @@ class McpQueryService {
   }
 
   void _guard(String sql, SqlDialect dialect) {
-    final reason = McpSqlGuard.check(sql, dialect);
-    if (reason != null) throw McpToolException(reason);
+    final refusal = McpSqlGuard.refusal(sql, dialect);
+    if (refusal != null) {
+      throw McpToolException(refusal.message, rule: refusal.rule);
+    }
   }
 
   Future<ErdSchema> _loadSchema(

lib/core/mcp/mcp_server_controller.dart

@@ -139,6 +139,7 @@ class McpServerController {
       rowCount: r.rowCount,
       durationMs: r.duration.inMilliseconds,
       error: r.error,
+      refusalRule: r.rule,
     ));
   }
 

lib/core/mcp/mcp_sql_guard.dart

@@ -2,6 +2,15 @@ import 'package:querya_desktop/core/database/destructive_sql_detector.dart';
 import 'package:querya_desktop/core/database/sql_mutation_classifier.dart';
 import 'package:querya_desktop/core/database/table_mutation_engine.dart';
 
+/// Why [McpSqlGuard] refused a query: a stable rule id for the audit log and
+/// the message the model reads.
+class McpSqlRefusal {
+  const McpSqlRefusal(this.rule, this.message);
+
+  final String rule;
+  final String message;
+}
+
 /// Decides whether SQL sent by an MCP client may run.
 ///
 /// First of three read-only layers (the others are the read-only database
@@ -55,23 +64,29 @@ abstract final class McpSqlGuard {
   static final _pragma =
       RegExp(r'^PRAGMA\s+(?:[A-Z_]+\.)?([A-Z_]+)\s*(\(\s*[A-Z_0-9]*\s*\))?$');
 
-  /// Returns `null` when [sql] may run, or the reason it may not.
-  static String? check(String sql, SqlDialect dialect) {
+  /// Returns `null` when [sql] may run, or the reason it may not, as the
+  /// message the model sees.
+  static String? check(String sql, SqlDialect dialect) =>
+      refusal(sql, dialect)?.message;
+
+  /// Returns `null` when [sql] may run, or why it may not: the rule that
+  /// refused it (stable, for the audit log) and the message for the model.
+  static McpSqlRefusal? refusal(String sql, SqlDialect dialect) {
     final statements = DestructiveSqlDetector.splitStatements(sql)
         .where((s) => DestructiveSqlDetector.stripCommentsAndStrings(s)
             .trim()
             .isNotEmpty)
         .toList();
-    if (statements.isEmpty) return 'The query is empty.';
+    if (statements.isEmpty) return const McpSqlRefusal('empty_query', 'The query is empty.');
     if (statements.length > 1) {
-      return 'Only one statement per call is allowed; send them separately.';
+      return const McpSqlRefusal('single_statement', 'Only one statement per call is allowed; send them separately.');
     }
     final statement = statements.single;
     // MySQL / MariaDB execute the body of `/*! ... */` and `/*M! ... */`
     // comments, which the comment stripper below would hide from the checks.
     if (dialect == SqlDialect.mysql &&
         RegExp(r'/\*M?!').hasMatch(statement)) {
-      return 'MySQL executable comments (/*! ... */) are not allowed.';
+      return const McpSqlRefusal('mysql_executable_comment', 'MySQL executable comments (/*! ... */) are not allowed.');
     }
     final upper = DestructiveSqlDetector.stripCommentsAndStrings(statement)
         .trim()
@@ -82,25 +97,25 @@ abstract final class McpSqlGuard {
     if (first == 'PRAGMA' && dialect == SqlDialect.sqlite) {
       final m = _pragma.firstMatch(upper.replaceAll(RegExp(r'\s+'), ' '));
       if (m != null && _readPragmas.contains(m.group(1))) return null;
-      return 'Only schema pragmas are allowed (${_readPragmas.map((p) => p.toLowerCase()).join(', ')}).';
+      return McpSqlRefusal('sqlite_pragma', 'Only schema pragmas are allowed (${_readPragmas.map((p) => p.toLowerCase()).join(', ')}).');
     }
     if (first == null || !_readStarts.contains(first)) {
-      return 'Only read-only queries are allowed (SELECT, WITH, EXPLAIN, SHOW, DESCRIBE).';
+      return const McpSqlRefusal('read_only_only', 'Only read-only queries are allowed (SELECT, WITH, EXPLAIN, SHOW, DESCRIBE).');
     }
     if (_forbiddenFunctions.hasMatch(upper)) {
-      return 'This query calls a server function that is not allowed over MCP.';
+      return const McpSqlRefusal('function_not_allowed', 'This query calls a server function that is not allowed over MCP.');
     }
     if (first == 'EXPLAIN') {
       if (_explainForbidden.hasMatch(upper)) {
-        return 'EXPLAIN is allowed only for read-only statements and without ANALYZE.';
+        return const McpSqlRefusal('explain_analyze_or_write', 'EXPLAIN is allowed only for read-only statements and without ANALYZE.');
       }
       return null;
     }
     if (isMutatingSqlStatement(statement)) {
-      return 'Data-modifying statements are not allowed over MCP.';
+      return const McpSqlRefusal('data_modifying', 'Data-modifying statements are not allowed over MCP.');
     }
     if (_selectSideEffects.hasMatch(upper)) {
-      return 'SELECT ... INTO and row locks (FOR UPDATE / FOR SHARE) are not allowed.';
+      return const McpSqlRefusal('select_into_or_lock', 'SELECT ... INTO and row locks (FOR UPDATE / FOR SHARE) are not allowed.');
     }
     return null;
   }

lib/core/mcp/querya_mcp_server.dart

@@ -16,6 +16,7 @@ class McpCallRecord {
     this.sql,
     this.rowCount,
     this.error,
+    this.rule,
   });
 
   final DateTime at;
@@ -26,6 +27,9 @@ class McpCallRecord {
   final String? sql;
   final int? rowCount;
   final String? error;
+
+  /// The guard rule that refused the call, when a rule did.
+  final String? rule;
 }
 
 /// MCP session for one client: read-only tools and a `schema://` resource on
@@ -210,24 +214,24 @@ base class QueryaMcpServer extends MCPServer with ToolsSupport, ResourcesSupport
       }
       try {
         final (text, rows, sql) = await body(args);
-        _report(tool, started, sw.elapsed, connectionId, sql, rows, null);
+        _report(tool, started, sw.elapsed, connectionId, sql, rows, null, null);
         return CallToolResult(content: [TextContent(text: text)]);
       } on McpToolException catch (e) {
         _report(tool, started, sw.elapsed, connectionId,
-            rawSql, null, e.message);
+            rawSql, null, e.message, e.rule);
         return CallToolResult(
             isError: true, content: [TextContent(text: e.message)]);
       } catch (e) {
         final message = McpRedaction.redact('Internal error: $e');
-        _report(tool, started, sw.elapsed, connectionId, rawSql, null, message);
+        _report(tool, started, sw.elapsed, connectionId, rawSql, null, message, null);
         return CallToolResult(
             isError: true, content: [TextContent(text: message)]);
       }
     };
   }
 
   void _report(String tool, DateTime at, Duration d, int? connectionId,
-      String? sql, int? rows, String? error) {
+      String? sql, int? rows, String? error, String? rule) {
     onCall?.call(McpCallRecord(
       at: at,
       client: _clientName,
@@ -237,6 +241,7 @@ base class QueryaMcpServer extends MCPServer with ToolsSupport, ResourcesSupport
       sql: sql,
       rowCount: rows,
       error: error,
+      rule: rule,
     ));
   }
 

lib/core/storage/local_db.dart

@@ -11,7 +11,7 @@ import 'package:querya_desktop/core/storage/connection_secrets_store.dart';
 import 'package:sqflite_common_ffi/sqflite_ffi.dart';
 
 const _dbName = 'querya.db';
-const _dbVersion = 11;
+const _dbVersion = 12;
 
 /// `app_settings` key under which each profile database's random id is
 /// stored (see [LocalDb._ensureProfileId] and issue #986).
@@ -221,7 +221,8 @@ class LocalDb {
         sql_text TEXT,
         row_count INTEGER,
         duration_ms INTEGER NOT NULL,
-        error TEXT
+        error TEXT,
+        refusal_rule TEXT
       )
     ''');
   }
@@ -340,6 +341,10 @@ class LocalDb {
     if (oldVersion < 11) {
       await _createMcpActivityTable(db);
     }
+    if (oldVersion >= 11 && oldVersion < 12) {
+      // The rule that refused a call, next to its error text.
+      await db.execute('ALTER TABLE mcp_activity ADD COLUMN refusal_rule TEXT');
+    }
   }
 
   Future<String?> getAppSetting(String key) async {
@@ -835,6 +840,7 @@ class McpActivityEntry {
     this.rowCount,
     required this.durationMs,
     this.error,
+    this.refusalRule,
   });
 
   final int? id;
@@ -850,6 +856,9 @@ class McpActivityEntry {
   final int durationMs;
   final String? error;
 
+  /// The guard rule that refused the call (`read_only_only`, ...), when one did.
+  final String? refusalRule;
+
   Map<String, Object?> toMap() => {
         'id': id,
         'recorded_at': recordedAt,
@@ -861,6 +870,7 @@ class McpActivityEntry {
         'row_count': rowCount,
         'duration_ms': durationMs,
         'error': error,
+        'refusal_rule': refusalRule,
       };
 
   static McpActivityEntry fromMap(Map<String, Object?> m) => McpActivityEntry(
@@ -874,6 +884,7 @@ class McpActivityEntry {
         rowCount: _sqliteInt(m['row_count']),
         durationMs: _sqliteInt(m['duration_ms']) ?? 0,
         error: m['error'] as String?,
+        refusalRule: m['refusal_rule'] as String?,
       );
 }
 

test/core/mcp/mcp_client_config_and_activity_test.dart

@@ -93,4 +93,28 @@ void main() {
       expect(await LocalDb.instance.listMcpActivity(), isEmpty);
     });
   });
+
+  test('an activity entry keeps the rule that refused its call (#1231)', () {
+    const entry = McpActivityEntry(
+      recordedAt: '2026-10-09T10:00:00Z',
+      client: 'test',
+      tool: 'run_query',
+      durationMs: 3,
+      error: 'Data-modifying statements are not allowed over MCP.',
+      refusalRule: 'data_modifying',
+    );
+    final back = McpActivityEntry.fromMap(entry.toMap());
+    expect(back.refusalRule, 'data_modifying');
+    expect(back.error, entry.error);
+    expect(
+      McpActivityEntry.fromMap({
+        'recorded_at': '2026-10-09T10:00:00Z',
+        'client': 'test',
+        'tool': 'run_query',
+        'duration_ms': 3,
+      }).refusalRule,
+      isNull,
+      reason: 'rows written before the column existed read as no rule',
+    );
+  });
 }

test/core/mcp/mcp_sql_guard_test.dart

@@ -96,4 +96,39 @@ void main() {
     refused('PRAGMA journal_mode', SqlDialect.sqlite);
     refused('PRAGMA table_info(users)', SqlDialect.postgres);
   });
+
+  group('refusal rule ids (#1231)', () {
+    String? ruleOf(String sql, SqlDialect d) =>
+        McpSqlGuard.refusal(sql, d)?.rule;
+
+    test('every refusal names the rule that caught it', () {
+      expect(ruleOf('', SqlDialect.postgres), 'empty_query');
+      expect(ruleOf('SELECT 1; SELECT 2', SqlDialect.postgres),
+          'single_statement');
+      expect(ruleOf('INSERT INTO t VALUES (1)', SqlDialect.sqlite),
+          'read_only_only');
+      expect(ruleOf('PRAGMA writable_schema = 1', SqlDialect.sqlite),
+          'sqlite_pragma');
+      expect(ruleOf('/*! DROP TABLE t */ SELECT 1', SqlDialect.mysql),
+          'mysql_executable_comment');
+      expect(ruleOf('WITH x AS (SELECT 1) DELETE FROM t', SqlDialect.postgres),
+          'data_modifying');
+      expect(ruleOf('SELECT 1 FOR UPDATE', SqlDialect.postgres),
+          'select_into_or_lock');
+      expect(ruleOf('EXPLAIN ANALYZE SELECT 1', SqlDialect.postgres),
+          'explain_analyze_or_write');
+    });
+
+    test('a query that may run has no refusal and no rule', () {
+      expect(McpSqlGuard.refusal('SELECT 1', SqlDialect.postgres), isNull);
+      expect(McpSqlGuard.check('SELECT 1', SqlDialect.postgres), isNull);
+    });
+
+    test('the message the model reads is unchanged by the rule id', () {
+      expect(McpSqlGuard.check('', SqlDialect.postgres),
+          'The query is empty.');
+      expect(McpSqlGuard.refusal('', SqlDialect.postgres)!.message,
+          'The query is empty.');
+    });
+  });
 }