From b984bf436d93e02e1804998921f1430204257fac Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sun, 25 Oct 2026 13:54:58 +0300 Subject: [PATCH 1/2] fix(security): review of the SQL guard, the destructive detector and the update zip extractor --- .../database/destructive_sql_detector.dart | 112 ++++++++++-------- lib/core/mcp/mcp_sql_guard.dart | 75 +++++++++++- .../installers/update_install_utils.dart | 22 ++++ .../destructive_sql_detector_test.dart | 74 ++++++++++++ test/core/mcp/mcp_sql_guard_test.dart | 25 ++++ .../update_platform_installer_test.dart | 18 +++ 6 files changed, 274 insertions(+), 52 deletions(-) diff --git a/lib/core/database/destructive_sql_detector.dart b/lib/core/database/destructive_sql_detector.dart index 2b0da30d..1a9a5de4 100644 --- a/lib/core/database/destructive_sql_detector.dart +++ b/lib/core/database/destructive_sql_detector.dart @@ -180,7 +180,7 @@ abstract final class DestructiveSqlDetector { ); static final _dropTableRegex = RegExp( - _withId(r'^\s*DROP\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:(?:{id}\.)?{id})'), + _withId(r'^\s*DROP\s+(?:TEMPORARY\s+|FOREIGN\s+)?TABLE\s+(?:IF\s+EXISTS\s+)?(?:(?:{id}\.)?{id})'), caseSensitive: false, unicode: true, ); @@ -204,7 +204,7 @@ abstract final class DestructiveSqlDetector { ); static final _deleteRegex = RegExp( - _withId(r'^\s*DELETE\s+(?:(?:LOW_PRIORITY|QUICK|IGNORE)\s+)*(?:[^;]*?\s)?FROM\s+(?:(?:{id}\.)?{id})'), + _withId(r'^\s*DELETE\s+(?:[^;]*?\s)??FROM\s+(?:(?:{id}\.)?{id})'), caseSensitive: false, unicode: true, ); @@ -217,7 +217,7 @@ abstract final class DestructiveSqlDetector { static final _dropOtherRegex = RegExp( _withId( - r'^\s*DROP\s+(?:UNIQUE\s+)?(?:INDEX|TYPE|FUNCTION|PROCEDURE|ROLE|USER|TRIGGER|SEQUENCE|DOMAIN|EXTENSION|TABLESPACE|PUBLICATION|SUBSCRIPTION|AGGREGATE|POLICY|RULE)\s+(?:CONCURRENTLY\s+)?(?:IF\s+EXISTS\s+)?(?:(?:{id}\.)?{id})', + r'^\s*DROP\s+(?:UNIQUE\s+)?(?:INDEX|TYPE|FUNCTION|PROCEDURE|ROLE|USER|TRIGGER|EVENT\s+TRIGGER|SEQUENCE|DOMAIN|EXTENSION|TABLESPACE|PUBLICATION|SUBSCRIPTION|AGGREGATE|POLICY|RULE|SERVER|OWNED\s+BY)\s+(?:CONCURRENTLY\s+)?(?:IF\s+EXISTS\s+)?(?:(?:{id}\.)?{id})', ), caseSensitive: false, unicode: true, @@ -225,7 +225,7 @@ abstract final class DestructiveSqlDetector { static final _dropColumnRegex = RegExp( _withId( - r'^\s*ALTER\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:ONLY\s+)?(?:(?:{id}\.)?{id})[^;]*?\bDROP\s+COLUMN\b', + r'^\s*ALTER\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:ONLY\s+)?(?:(?:{id}\.)?{id})[^;]*?\bDROP\s+(?:COLUMN\b|(?!(?:CONSTRAINT|INDEX|KEY|PRIMARY|FOREIGN|CHECK|DEFAULT|NOT|IDENTITY|EXPRESSION|PARTITION|IF|TRIGGER|RULE|PERIOD)\b)[\p{L}"`_])', ), caseSensitive: false, unicode: true, @@ -240,31 +240,72 @@ abstract final class DestructiveSqlDetector { static final _withRegex = RegExp(r'^\s*WITH\s+(?:RECURSIVE\s+)?', caseSensitive: false); - static final _dmlStartRegex = - RegExp(r'^(?:DELETE|UPDATE)\b', caseSensitive: false); + /// The keyword that starts the main statement after a `WITH` list. + static final _mainStartRegex = RegExp( + r'^(?:SELECT|INSERT|UPDATE|DELETE|MERGE|VALUES|TABLE)\b', + caseSensitive: false, + ); - /// The statement that actually runs: past an `EXPLAIN ANALYZE` prefix and - /// the `WITH name AS (...)` list in front of a `DELETE` / `UPDATE`. - static String _effectiveStatement(String sanitized) { + /// Splits [sanitized] (after an `EXPLAIN ANALYZE` prefix) into the bodies of + /// its `WITH name AS (...)` list and the main statement that follows it. A + /// statement without `WITH` is its own main statement. + static ({List bodies, String main}) _splitWith(String sanitized) { var sql = sanitized; final explain = _explainAnalyzeRegex.firstMatch(sql); if (explain != null) sql = sql.substring(explain.end); final withMatch = _withRegex.firstMatch(sql); - if (withMatch == null) return sql; + if (withMatch == null) return (bodies: const [], main: sql); + final bodies = []; var depth = 0; + var bodyStart = 0; for (var i = withMatch.end; i < sql.length; i++) { final ch = sql[i]; if (ch == '(') { + if (depth == 0) bodyStart = i + 1; depth++; } else if (ch == ')') { if (depth > 0) depth--; + if (depth == 0) bodies.add(sql.substring(bodyStart, i)); } else if (depth == 0 && - (i == 0 || !RegExp(r'[\w$]').hasMatch(sql[i - 1])) && - _dmlStartRegex.hasMatch(sql.substring(i))) { - return sql.substring(i); + !RegExp(r'[\w$]').hasMatch(sql[i - 1]) && + _mainStartRegex.hasMatch(sql.substring(i))) { + return (bodies: bodies, main: sql.substring(i)); } } - return sql; + return (bodies: bodies, main: sql); + } + + /// An unconditional `DELETE` / `UPDATE` in [sql], or null. + static DestructiveSqlOperation? _unconditionalDml( + String sql, + String rawStatement, + ) { + String targetOf(RegExpMatch m) { + final schema = m.group(1); + final table = m.group(2) ?? 'table'; + return (schema != null && schema.isNotEmpty) ? '$schema.$table' : table; + } + + final deleteMatch = _deleteRegex.firstMatch(sql); + if (deleteMatch != null) { + if (_hasTopLevelWhere(sql)) return null; + return DestructiveSqlOperation( + type: DestructiveSqlType.unconditionalDelete, + targetName: targetOf(deleteMatch), + rawStatement: rawStatement, + ); + } + final updateMatch = _updateRegex.firstMatch(sql); + if (updateMatch != null && + _setRegex.hasMatch(sql) && + !_hasTopLevelWhere(sql)) { + return DestructiveSqlOperation( + type: DestructiveSqlType.unconditionalUpdate, + targetName: targetOf(updateMatch), + rawStatement: rawStatement, + ); + } + return null; } static final _whereRegex = RegExp(r'\bWHERE\b', caseSensitive: false); @@ -464,7 +505,8 @@ abstract final class DestructiveSqlDetector { backslashEscapes: backslashEscapes, ).trim(); if (sanitized.isEmpty) continue; - final effective = _effectiveStatement(sanitized); + final withParts = _splitWith(sanitized); + final effective = withParts.main; // 1. DROP DATABASE final dropDbMatch = _dropDatabaseRegex.firstMatch(sanitized); @@ -594,41 +636,11 @@ abstract final class DestructiveSqlDetector { continue; } - // 7. DELETE FROM table without WHERE - final deleteMatch = _deleteRegex.firstMatch(effective); - if (deleteMatch != null) { - if (!_hasTopLevelWhere(effective)) { - final schema = deleteMatch.group(1); - final table = deleteMatch.group(2) ?? 'table'; - final target = - (schema != null && schema.isNotEmpty) ? '$schema.$table' : table; - operations.add( - DestructiveSqlOperation( - type: DestructiveSqlType.unconditionalDelete, - targetName: target, - rawStatement: rawStmt.trim(), - ), - ); - } - continue; - } - - // 8. UPDATE table SET ... without WHERE - final updateMatch = _updateRegex.firstMatch(effective); - if (updateMatch != null && - _setRegex.hasMatch(effective) && - !_hasTopLevelWhere(effective)) { - final schema = updateMatch.group(1); - final table = updateMatch.group(2) ?? 'table'; - final target = - (schema != null && schema.isNotEmpty) ? '$schema.$table' : table; - operations.add( - DestructiveSqlOperation( - type: DestructiveSqlType.unconditionalUpdate, - targetName: target, - rawStatement: rawStmt.trim(), - ), - ); + // 7. DELETE / UPDATE without WHERE: the main statement, and a + // data-modifying CTE (`WITH d AS (DELETE FROM t RETURNING *) SELECT ...`). + for (final body in [effective, ...withParts.bodies]) { + final op = _unconditionalDml(body.trim(), rawStmt.trim()); + if (op != null) operations.add(op); } } diff --git a/lib/core/mcp/mcp_sql_guard.dart b/lib/core/mcp/mcp_sql_guard.dart index 17ce1c19..41de65dc 100644 --- a/lib/core/mcp/mcp_sql_guard.dart +++ b/lib/core/mcp/mcp_sql_guard.dart @@ -28,6 +28,65 @@ abstract final class McpSqlGuard { 'VALUES', }; + /// Syntax the comment/string stripper and the server read differently, so + /// text after it could hide a call from the denylist: a quoted identifier + /// that holds a comment or quote character, a PostgreSQL `U&"..."` + /// identifier (escapes spell any name), MySQL `#` comments and `--` without + /// a space after it (which MySQL reads as two minus signs). + /// True when a `"..."` or `` `...` `` identifier holds `--`, `'`, `#` or + /// `/*`. Reads left to right, skipping string literals, so a closing quote is + /// never taken for an opening one. + static bool _hasAmbiguousIdentifier(String sql, {required bool backslashEscapes}) { + var i = 0; + while (i < sql.length) { + final c = sql[i]; + if (c == "'") { + i++; + while (i < sql.length) { + if (backslashEscapes && sql[i] == r'\') { + i += 2; + } else if (sql[i] == "'") { + if (i + 1 < sql.length && sql[i + 1] == "'") { + i += 2; + } else { + break; + } + } else { + i++; + } + } + i++; + } else if (c == '"' || c == '`') { + final start = i + 1; + i = start; + while (i < sql.length) { + if (sql[i] == c) { + if (i + 1 < sql.length && sql[i + 1] == c) { + i += 2; + continue; + } + break; + } + i++; + } + final body = sql.substring(start, i < sql.length ? i : sql.length); + if (body.contains('--') || + body.contains("'") || + body.contains('#') || + body.contains('/*')) { + return true; + } + i++; + } else { + i++; + } + } + return false; + } + static final _unicodeEscapeIdentifier = + RegExp(r'\bU&\s*"', caseSensitive: false); + static final _mysqlDashDashNoSpace = RegExp(r'--(?!\s)'); + static final _firstWord = RegExp(r'^\(*\s*([A-Z_]+)'); /// `SELECT ... INTO` creates a table (Postgres) or writes a file (MySQL @@ -48,8 +107,9 @@ abstract final class McpSqlGuard { r'QUERY_TO_XML\w*|CURSOR_TO_XML|TABLE_TO_XML\w*|PG_(?:TRY_)?ADVISORY_\w*|PG_SLEEP\w*|' r'NEXTVAL|SETVAL|DBLINK\w*|SET_CONFIG|LOAD_FILE|LOAD_EXTENSION|WRITEFILE|' r'READFILE|FSDIR|SLEEP|BENCHMARK|GET_LOCK|RELEASE_LOCK|RELEASE_ALL_LOCKS|IS_FREE_LOCK|' - r'PG_STAT_RESET\w*|PG_SWITCH_WAL|PG_CREATE_RESTORE_POINT|PG_LOGICAL_EMIT_MESSAGE|' - r'PG_PROMOTE|PG_DROP_REPLICATION_SLOT)\s*\(', + r'PG_STAT_\w*RESET\w*|PG_SWITCH_WAL|PG_CREATE_RESTORE_POINT|PG_LOGICAL_\w*|PG_REPLICATION_\w*|' + r'PG_NOTIFY|PG_LS_\w*|PG_PROMOTE|PG_DROP_REPLICATION_SLOT|PG_CREATE_\w*_SLOT|PG_COPY_\w*_SLOT|' + r'PG_BACKUP_\w*|PG_START_BACKUP|PG_STOP_BACKUP)\s*\(', ); /// SQLite pragmas that only read the schema. @@ -99,6 +159,13 @@ abstract final class McpSqlGuard { RegExp(r'/\*M?!').hasMatch(statement)) { return const McpSqlRefusal('mysql_executable_comment', 'MySQL executable comments (/*! ... */) are not allowed.'); } + if (_hasAmbiguousIdentifier(statement, backslashEscapes: escapes) || + _unicodeEscapeIdentifier.hasMatch(statement) || + (dialect == SqlDialect.mysql && + _mysqlDashDashNoSpace.hasMatch(statement))) { + return const McpSqlRefusal('ambiguous_syntax', + 'Quoted identifiers with comment or quote characters, U&"..." identifiers and "--" without a following space are not allowed over MCP.'); + } final upper = DestructiveSqlDetector.stripCommentsAndStrings( statement, backslashEscapes: escapes, @@ -113,6 +180,10 @@ abstract final class McpSqlGuard { if (m != null && _readPragmas.contains(m.group(1))) return null; return McpSqlRefusal('sqlite_pragma', 'Only schema pragmas are allowed (${_readPragmas.map((p) => p.toLowerCase()).join(', ')}).'); } + if (dialect == SqlDialect.mysql && upper.contains('#')) { + return const McpSqlRefusal('ambiguous_syntax', + 'MySQL "#" comments are not allowed over MCP.'); + } if (first == null || !_readStarts.contains(first)) { return const McpSqlRefusal('read_only_only', 'Only read-only queries are allowed (SELECT, WITH, EXPLAIN, SHOW, DESCRIBE).'); } diff --git a/lib/core/updater/installers/update_install_utils.dart b/lib/core/updater/installers/update_install_utils.dart index 0eed0f0b..2be28701 100644 --- a/lib/core/updater/installers/update_install_utils.dart +++ b/lib/core/updater/installers/update_install_utils.dart @@ -41,6 +41,15 @@ Future extractZipSecurely({ ); } + // A link already on disk would carry this entry out of the root, e.g. + // `a -> .`, `a/b -> ..`, then a file under `b/`; the checks above only read + // the names. + if (await _hasLinkBelowRoot(root, p.dirname(targetPath))) { + throw AppUpdaterException( + 'Security violation: archive entry "$name" is below a symlink', + ); + } + if (entry.isSymbolicLink) { // The macOS bundle keeps its framework symlinks (`Versions/Current`); // flattened to text files they fail `codesign --verify`. @@ -71,6 +80,19 @@ Future extractZipSecurely({ } } +/// Whether any directory from [root] (exclusive) down to [directory] is a +/// symlink. +Future _hasLinkBelowRoot(String root, String directory) async { + final parts = p.split(p.relative(directory, from: root)); + var current = root; + for (final part in parts) { + if (part == '.' || part.isEmpty) continue; + current = p.join(current, part); + if (await FileSystemEntity.isLink(current)) return true; + } + return false; +} + /// Restores the permission bits of an extracted file (POSIX only), so helper /// binaries in an update stay executable. Future _applyUnixPermissions(File file, int permissions) async { diff --git a/test/core/database/destructive_sql_detector_test.dart b/test/core/database/destructive_sql_detector_test.dart index 8d0b08d2..345b5a3c 100644 --- a/test/core/database/destructive_sql_detector_test.dart +++ b/test/core/database/destructive_sql_detector_test.dart @@ -316,6 +316,80 @@ void main() { } }); + test('a data-modifying CTE is inspected (review of #1770)', () { + expect( + only('WITH d AS (DELETE FROM orders RETURNING *) SELECT * FROM d').type, + DestructiveSqlType.unconditionalDelete, + ); + expect( + only('WITH u AS (UPDATE t SET a = 1 RETURNING *) SELECT 1').type, + DestructiveSqlType.unconditionalUpdate, + ); + expect( + DestructiveSqlDetector.inspect( + 'WITH d AS (DELETE FROM orders WHERE id = 1 RETURNING *) SELECT 1') + .isDestructive, + isFalse, + ); + }); + + test('an upsert or a column called delete after WITH is not flagged', () { + for (final sql in [ + 'WITH src AS (SELECT 1 AS id, 2 AS a) INSERT INTO t SELECT * FROM src ' + 'ON CONFLICT (id) DO UPDATE SET a = excluded.a', + 'WITH a AS (SELECT 1) SELECT f.delete FROM flags f', + 'WITH a AS (SELECT 1) SELECT * FROM t FOR UPDATE', + ]) { + expect(DestructiveSqlDetector.inspect(sql).isDestructive, isFalse, + reason: sql); + } + }); + + test('the dialog names the table that is deleted from', () { + expect( + only('DELETE FROM orders o USING (SELECT 1 FROM t WHERE x) s') + .targetName, + 'orders', + ); + expect( + only('DELETE FROM orders RETURNING (SELECT max(id) FROM archive)') + .targetName, + 'orders', + ); + }); + + test('a long run of modifiers does not freeze the check', () { + final sw = Stopwatch()..start(); + DestructiveSqlDetector.inspect('DELETE ${'QUICK ' * 10000}x'); + DestructiveSqlDetector.inspect('DELETE ${'LOW_PRIORITY ' * 5000}'); + expect(sw.elapsed, lessThan(const Duration(seconds: 2))); + }); + + test('more DROP forms and ALTER TABLE ... DROP column', () { + for (final sql in [ + 'DROP FOREIGN TABLE ft', + 'DROP TEMPORARY TABLE t', + 'DROP EVENT TRIGGER e', + 'DROP SERVER s CASCADE', + 'DROP OWNED BY bob', + ]) { + expect(DestructiveSqlDetector.inspect(sql).isDestructive, isTrue, + reason: sql); + } + expect(only('ALTER TABLE users DROP email').type, + DestructiveSqlType.dropColumn); + for (final sql in [ + 'ALTER TABLE t DROP CONSTRAINT c', + 'ALTER TABLE t DROP PRIMARY KEY', + 'ALTER TABLE t DROP FOREIGN KEY fk', + 'ALTER TABLE t ALTER COLUMN c DROP DEFAULT', + 'ALTER TABLE t ALTER COLUMN c DROP NOT NULL', + ]) { + expect(DestructiveSqlDetector.inspect(sql).isDestructive, isFalse, + reason: sql); + } + }); + test('ALTER TABLE ... DROP COLUMN is a HIGH risk operation', () { final op = only('ALTER TABLE public.users DROP COLUMN email'); expect(op.type, DestructiveSqlType.dropColumn); diff --git a/test/core/mcp/mcp_sql_guard_test.dart b/test/core/mcp/mcp_sql_guard_test.dart index 74eeb527..6becc879 100644 --- a/test/core/mcp/mcp_sql_guard_test.dart +++ b/test/core/mcp/mcp_sql_guard_test.dart @@ -117,6 +117,31 @@ void main() { refused('SELECT `sleep`(5)', SqlDialect.mysql); }); + test('syntax the stripper and the server read differently is refused', () { + refused('''SELECT 1 AS "--", pg_sleep(100)'''); + refused('''SELECT 1 AS "'", pg_read_file('/etc/passwd') AS "'"'''); + refused(r'SELECT U&"pg_sl\0065ep"(5)'); + refused('SELECT 1 AS `--`, SLEEP(10)', SqlDialect.mysql); + refused('SELECT 1--1, SLEEP(30)', SqlDialect.mysql); + refused("SELECT 1 # '\n, SLEEP(10) # '", SqlDialect.mysql); + }); + + test('ordinary quoting is still allowed', () { + allowed('''SELECT "id" FROM users WHERE name = 'x' AND "age" > 3'''); + allowed('''SELECT "a", "b" FROM t WHERE c = 'it''s' '''); + allowed("SELECT '#fff', 'a--b' FROM t", SqlDialect.mysql); + allowed('SELECT 1 -- a note', SqlDialect.mysql); + }); + + test('more server-side functions are refused (review of #1769)', () { + refused("SELECT pg_notify('chan', 'x')"); + refused('SELECT * FROM pg_ls_logdir()'); + refused('SELECT pg_stat_statements_reset()'); + refused("SELECT pg_create_logical_replication_slot('s', 'test_decoding')"); + refused("SELECT pg_logical_slot_get_changes('s', NULL, NULL)"); + refused("SELECT pg_replication_slot_advance('s', '0/0')"); + }); + test('EXPLAIN of a SELECT that calls REPLACE() is allowed (#1565)', () { allowed("EXPLAIN SELECT REPLACE(name, 'a', 'b') FROM users"); refused("EXPLAIN REPLACE INTO users VALUES (1)", SqlDialect.mysql); diff --git a/test/core/updater/update_platform_installer_test.dart b/test/core/updater/update_platform_installer_test.dart index bd41ef0f..52452fdd 100644 --- a/test/core/updater/update_platform_installer_test.dart +++ b/test/core/updater/update_platform_installer_test.dart @@ -291,6 +291,24 @@ void main() { ); }); + test('rejects a file written below a chain of symlinks (review of #1765)', + () async { + final temp = await tempDir(); + final archive = Archive() + ..addFile(symlinkEntry('a', '.')) + ..addFile(symlinkEntry('a/b', '..')) + ..addFile(ArchiveFile('b/evil', 4, [1, 2, 3, 4])); + final zipFile = File(p.join(temp.path, 'chain.zip')); + await zipFile.writeAsBytes(asUnixZip(archive)); + final out = Directory(p.join(temp.path, 'out')); + + await expectLater( + extractZipSecurely(zipFile: zipFile, destinationDir: out), + throwsA(isA()), + ); + expect(File(p.join(temp.path, 'evil')).existsSync(), isFalse); + }); + test('rejects an absolute symlink target', () async { final temp = await tempDir(); final archive = Archive() From 54883ee3717876fa6c02987b0d58f7dd66ec9b6b Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sun, 25 Oct 2026 13:59:20 +0300 Subject: [PATCH 2/2] fix(mcp): the ambiguous-syntax checks ignore string literal bodies --- lib/core/mcp/mcp_sql_guard.dart | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/lib/core/mcp/mcp_sql_guard.dart b/lib/core/mcp/mcp_sql_guard.dart index 41de65dc..70b1fe14 100644 --- a/lib/core/mcp/mcp_sql_guard.dart +++ b/lib/core/mcp/mcp_sql_guard.dart @@ -33,14 +33,21 @@ abstract final class McpSqlGuard { /// that holds a comment or quote character, a PostgreSQL `U&"..."` /// identifier (escapes spell any name), MySQL `#` comments and `--` without /// a space after it (which MySQL reads as two minus signs). - /// True when a `"..."` or `` `...` `` identifier holds `--`, `'`, `#` or - /// `/*`. Reads left to right, skipping string literals, so a closing quote is - /// never taken for an opening one. - static bool _hasAmbiguousIdentifier(String sql, {required bool backslashEscapes}) { + /// Reads [sql] left to right, skipping string literals, and returns whether + /// a `"..."` or `` `...` `` identifier holds `--`, `'`, `#` or `/*`, and the + /// text with every string literal body removed. Reading in order means a + /// closing quote is never taken for an opening one. + static ({bool ambiguousIdentifier, String outsideStrings}) _scanQuoting( + String sql, { + required bool backslashEscapes, + }) { + final outside = StringBuffer(); + var ambiguous = false; var i = 0; while (i < sql.length) { final c = sql[i]; if (c == "'") { + outside.write("''"); i++; while (i < sql.length) { if (backslashEscapes && sql[i] == r'\') { @@ -69,20 +76,24 @@ abstract final class McpSqlGuard { } i++; } - final body = sql.substring(start, i < sql.length ? i : sql.length); + final end = i < sql.length ? i : sql.length; + final body = sql.substring(start, end); if (body.contains('--') || body.contains("'") || body.contains('#') || body.contains('/*')) { - return true; + ambiguous = true; } + outside.write(sql.substring(start - 1, i < sql.length ? i + 1 : end)); i++; } else { + outside.write(c); i++; } } - return false; + return (ambiguousIdentifier: ambiguous, outsideStrings: outside.toString()); } + static final _unicodeEscapeIdentifier = RegExp(r'\bU&\s*"', caseSensitive: false); static final _mysqlDashDashNoSpace = RegExp(r'--(?!\s)'); @@ -159,10 +170,11 @@ abstract final class McpSqlGuard { RegExp(r'/\*M?!').hasMatch(statement)) { return const McpSqlRefusal('mysql_executable_comment', 'MySQL executable comments (/*! ... */) are not allowed.'); } - if (_hasAmbiguousIdentifier(statement, backslashEscapes: escapes) || - _unicodeEscapeIdentifier.hasMatch(statement) || + final quoting = _scanQuoting(statement, backslashEscapes: escapes); + if (quoting.ambiguousIdentifier || + _unicodeEscapeIdentifier.hasMatch(quoting.outsideStrings) || (dialect == SqlDialect.mysql && - _mysqlDashDashNoSpace.hasMatch(statement))) { + _mysqlDashDashNoSpace.hasMatch(quoting.outsideStrings))) { return const McpSqlRefusal('ambiguous_syntax', 'Quoted identifiers with comment or quote characters, U&"..." identifiers and "--" without a following space are not allowed over MCP.'); }