From 2795e51fc3745bef452d87329c9992574daed0fc Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sun, 25 Oct 2026 12:40:38 +0300 Subject: [PATCH] fix(sql): the destructive-query detector covers non-Latin names, MySQL DELETE forms, WITH ... DELETE, EXPLAIN ANALYZE, DROP COLUMN and other DROPs (#1770) --- .../database/destructive_sql_detector.dart | 135 ++++++++++++++++-- .../destructive_sql_detector_test.dart | 98 +++++++++++++ 2 files changed, 220 insertions(+), 13 deletions(-) diff --git a/lib/core/database/destructive_sql_detector.dart b/lib/core/database/destructive_sql_detector.dart index 415cb956..2b0da30d 100644 --- a/lib/core/database/destructive_sql_detector.dart +++ b/lib/core/database/destructive_sql_detector.dart @@ -9,6 +9,8 @@ enum DestructiveSqlType { dropTable, dropView, dropMaterializedView, + dropColumn, + dropOther, truncateTable, unconditionalDelete, unconditionalUpdate, @@ -31,6 +33,8 @@ enum DestructiveSqlType { DestructiveSqlType.dropTable => 'DROP TABLE', DestructiveSqlType.dropView => 'DROP VIEW', DestructiveSqlType.dropMaterializedView => 'DROP MATERIALIZED VIEW', + DestructiveSqlType.dropColumn => 'DROP COLUMN', + DestructiveSqlType.dropOther => 'DROP', DestructiveSqlType.truncateTable => 'TRUNCATE TABLE', DestructiveSqlType.unconditionalDelete => 'UNCONDITIONAL DELETE', DestructiveSqlType.unconditionalUpdate => 'UNCONDITIONAL UPDATE', @@ -67,6 +71,8 @@ enum DestructiveSqlType { DestructiveSqlType.redisFlushDb => 'CRITICAL', DestructiveSqlType.redisShutdown => 'CRITICAL', DestructiveSqlType.redisKeys => 'HIGH', + DestructiveSqlType.dropColumn => 'HIGH', + DestructiveSqlType.dropOther => 'MEDIUM', DestructiveSqlType.dropMaterializedView => 'MEDIUM', DestructiveSqlType.dropView => 'MEDIUM', }; @@ -94,6 +100,10 @@ class DestructiveSqlOperation { DestructiveSqlType.dropView => 'Drops view "$targetName".', DestructiveSqlType.dropMaterializedView => 'Drops materialized view "$targetName".', + DestructiveSqlType.dropColumn => + 'Permanently drops a column of "$targetName" and the data in it.', + DestructiveSqlType.dropOther => + 'Permanently drops "$targetName". Objects that depend on it may be lost too.', DestructiveSqlType.truncateTable => 'Quickly deletes all rows from table "$targetName" without transaction rollbacks in some engines.', DestructiveSqlType.unconditionalDelete => @@ -151,46 +161,112 @@ class DestructiveSqlInspectionResult { /// Heuristic analyzer and sanitizer for detecting destructive SQL queries /// before executing them in the SQL workspace. abstract final class DestructiveSqlDetector { + /// One identifier part, optionally quoted: letters and digits of any script + /// (`заказы`), `_` and `$`. + static const _id = r'["`]?([\p{L}\p{N}_$]+)["`]?'; + + static String _withId(String pattern) => pattern.replaceAll('{id}', _id); + static final _dropDatabaseRegex = RegExp( - r'^\s*DROP\s+DATABASE\s+(?:IF\s+EXISTS\s+)?(?:["`]?([a-zA-Z0-9_]+)["`]?)', + _withId(r'^\s*DROP\s+DATABASE\s+(?:IF\s+EXISTS\s+)?(?:{id})'), caseSensitive: false, + unicode: true, ); static final _dropSchemaRegex = RegExp( - r'^\s*DROP\s+SCHEMA\s+(?:IF\s+EXISTS\s+)?(?:["`]?([a-zA-Z0-9_]+)["`]?)', + _withId(r'^\s*DROP\s+SCHEMA\s+(?:IF\s+EXISTS\s+)?(?:{id})'), caseSensitive: false, + unicode: true, ); static final _dropTableRegex = RegExp( - r'^\s*DROP\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:(?:["`]?([a-zA-Z0-9_]+)["`]?\.)?["`]?([a-zA-Z0-9_]+)["`]?)', + _withId(r'^\s*DROP\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:(?:{id}\.)?{id})'), caseSensitive: false, + unicode: true, ); static final _dropMatViewRegex = RegExp( - r'^\s*DROP\s+MATERIALIZED\s+VIEW\s+(?:IF\s+EXISTS\s+)?(?:(?:["`]?([a-zA-Z0-9_]+)["`]?\.)?["`]?([a-zA-Z0-9_]+)["`]?)', + _withId(r'^\s*DROP\s+MATERIALIZED\s+VIEW\s+(?:IF\s+EXISTS\s+)?(?:(?:{id}\.)?{id})'), caseSensitive: false, + unicode: true, ); static final _dropViewRegex = RegExp( - r'^\s*DROP\s+VIEW\s+(?:IF\s+EXISTS\s+)?(?:(?:["`]?([a-zA-Z0-9_]+)["`]?\.)?["`]?([a-zA-Z0-9_]+)["`]?)', + _withId(r'^\s*DROP\s+VIEW\s+(?:IF\s+EXISTS\s+)?(?:(?:{id}\.)?{id})'), caseSensitive: false, + unicode: true, ); static final _truncateRegex = RegExp( - r'^\s*TRUNCATE\s+(?:TABLE\s+)?(?:(?:["`]?([a-zA-Z0-9_]+)["`]?\.)?["`]?([a-zA-Z0-9_]+)["`]?)', + _withId(r'^\s*TRUNCATE\s+(?:TABLE\s+)?(?:(?:{id}\.)?{id})'), caseSensitive: false, + unicode: true, ); static final _deleteRegex = RegExp( - r'^\s*DELETE\s+FROM\s+(?:(?:["`]?([a-zA-Z0-9_]+)["`]?\.)?["`]?([a-zA-Z0-9_]+)["`]?)', + _withId(r'^\s*DELETE\s+(?:(?:LOW_PRIORITY|QUICK|IGNORE)\s+)*(?:[^;]*?\s)?FROM\s+(?:(?:{id}\.)?{id})'), caseSensitive: false, + unicode: true, ); static final _updateRegex = RegExp( - r'^\s*UPDATE\s+(?:(?:LOW_PRIORITY|IGNORE|ONLY)\s+)*(?:(?:["`]?([a-zA-Z0-9_]+)["`]?\.)?["`]?([a-zA-Z0-9_]+)["`]?)', + _withId(r'^\s*UPDATE\s+(?:(?:LOW_PRIORITY|IGNORE|ONLY)\s+)*(?:(?:{id}\.)?{id})'), + caseSensitive: false, + unicode: true, + ); + + 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})', + ), caseSensitive: false, + unicode: true, ); + static final _dropColumnRegex = RegExp( + _withId( + r'^\s*ALTER\s+TABLE\s+(?:IF\s+EXISTS\s+)?(?:ONLY\s+)?(?:(?:{id}\.)?{id})[^;]*?\bDROP\s+COLUMN\b', + ), + caseSensitive: false, + unicode: true, + ); + + /// `EXPLAIN ANALYZE` runs the statement it explains. + static final _explainAnalyzeRegex = RegExp( + r'^\s*EXPLAIN\s+(?:\([^)]*\bANALY[SZ]E\b[^)]*\)|ANALY[SZ]E\b(?:\s+VERBOSE\b)?)\s*', + caseSensitive: false, + ); + + static final _withRegex = + RegExp(r'^\s*WITH\s+(?:RECURSIVE\s+)?', caseSensitive: false); + + static final _dmlStartRegex = + RegExp(r'^(?:DELETE|UPDATE)\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) { + 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; + var depth = 0; + for (var i = withMatch.end; i < sql.length; i++) { + final ch = sql[i]; + if (ch == '(') { + depth++; + } else if (ch == ')') { + if (depth > 0) depth--; + } else if (depth == 0 && + (i == 0 || !RegExp(r'[\w$]').hasMatch(sql[i - 1])) && + _dmlStartRegex.hasMatch(sql.substring(i))) { + return sql.substring(i); + } + } + return sql; + } + static final _whereRegex = RegExp(r'\bWHERE\b', caseSensitive: false); static final _setRegex = RegExp(r'\bSET\b', caseSensitive: false); @@ -388,6 +464,7 @@ abstract final class DestructiveSqlDetector { backslashEscapes: backslashEscapes, ).trim(); if (sanitized.isEmpty) continue; + final effective = _effectiveStatement(sanitized); // 1. DROP DATABASE final dropDbMatch = _dropDatabaseRegex.firstMatch(sanitized); @@ -468,6 +545,38 @@ abstract final class DestructiveSqlDetector { continue; } + // 5b. DROP INDEX / TYPE / FUNCTION / ROLE / USER ... + final dropOtherMatch = _dropOtherRegex.firstMatch(sanitized); + if (dropOtherMatch != null) { + final schema = dropOtherMatch.group(1); + final name = dropOtherMatch.group(2) ?? 'object'; + operations.add( + DestructiveSqlOperation( + type: DestructiveSqlType.dropOther, + targetName: + (schema != null && schema.isNotEmpty) ? '$schema.$name' : name, + rawStatement: rawStmt.trim(), + ), + ); + continue; + } + + // 5c. ALTER TABLE ... DROP COLUMN + final dropColumnMatch = _dropColumnRegex.firstMatch(sanitized); + if (dropColumnMatch != null) { + final schema = dropColumnMatch.group(1); + final table = dropColumnMatch.group(2) ?? 'table'; + operations.add( + DestructiveSqlOperation( + type: DestructiveSqlType.dropColumn, + targetName: + (schema != null && schema.isNotEmpty) ? '$schema.$table' : table, + rawStatement: rawStmt.trim(), + ), + ); + continue; + } + // 6. TRUNCATE final truncateMatch = _truncateRegex.firstMatch(sanitized); if (truncateMatch != null) { @@ -486,9 +595,9 @@ abstract final class DestructiveSqlDetector { } // 7. DELETE FROM table without WHERE - final deleteMatch = _deleteRegex.firstMatch(sanitized); + final deleteMatch = _deleteRegex.firstMatch(effective); if (deleteMatch != null) { - if (!_hasTopLevelWhere(sanitized)) { + if (!_hasTopLevelWhere(effective)) { final schema = deleteMatch.group(1); final table = deleteMatch.group(2) ?? 'table'; final target = @@ -505,10 +614,10 @@ abstract final class DestructiveSqlDetector { } // 8. UPDATE table SET ... without WHERE - final updateMatch = _updateRegex.firstMatch(sanitized); + final updateMatch = _updateRegex.firstMatch(effective); if (updateMatch != null && - _setRegex.hasMatch(sanitized) && - !_hasTopLevelWhere(sanitized)) { + _setRegex.hasMatch(effective) && + !_hasTopLevelWhere(effective)) { final schema = updateMatch.group(1); final table = updateMatch.group(2) ?? 'table'; final target = diff --git a/test/core/database/destructive_sql_detector_test.dart b/test/core/database/destructive_sql_detector_test.dart index aaf2cb49..8d0b08d2 100644 --- a/test/core/database/destructive_sql_detector_test.dart +++ b/test/core/database/destructive_sql_detector_test.dart @@ -244,4 +244,102 @@ void main() { ); }); }); + + group('DestructiveSqlDetector gaps (#1770)', () { + DestructiveSqlOperation only(String sql) { + final res = DestructiveSqlDetector.inspect(sql); + expect(res.operations, hasLength(1), reason: sql); + return res.operations.single; + } + + test('non-Latin identifiers are detected and named in full', () { + var op = only('DROP TABLE "заказы"'); + expect(op.type, DestructiveSqlType.dropTable); + expect(op.targetName, 'заказы'); + op = only('DROP DATABASE склад'); + expect(op.type, DestructiveSqlType.dropDatabase); + expect(op.targetName, 'склад'); + expect(only('TRUNCATE "таблица"').type, DestructiveSqlType.truncateTable); + op = only('DROP TABLE public.заказы'); + expect(op.targetName, 'public.заказы'); + expect(only('DELETE FROM заказы').type, + DestructiveSqlType.unconditionalDelete); + }); + + test('MySQL DELETE modifiers and the multi-table form', () { + for (final sql in [ + 'DELETE IGNORE FROM t', + 'DELETE LOW_PRIORITY FROM t', + 'DELETE QUICK IGNORE FROM t', + 'DELETE t1 FROM t1 JOIN t2 ON t1.id = t2.id', + ]) { + expect(only(sql).type, DestructiveSqlType.unconditionalDelete, + reason: sql); + } + expect( + DestructiveSqlDetector.inspect( + 'DELETE t1 FROM t1 JOIN t2 ON t1.id = t2.id WHERE t2.x = 1') + .isDestructive, + isFalse, + ); + }); + + test('WITH ... DELETE / UPDATE and EXPLAIN ANALYZE are looked through', () { + expect( + only('WITH x AS (SELECT id FROM u WHERE a = 1) DELETE FROM t').type, + DestructiveSqlType.unconditionalDelete, + ); + expect( + only('WITH RECURSIVE x(n) AS (SELECT 1), y AS (SELECT 2) UPDATE t SET a = 1') + .type, + DestructiveSqlType.unconditionalUpdate, + ); + expect( + only('EXPLAIN ANALYZE DELETE FROM t').type, + DestructiveSqlType.unconditionalDelete, + ); + expect( + only('EXPLAIN (ANALYZE, VERBOSE) DELETE FROM t').type, + DestructiveSqlType.unconditionalDelete, + ); + }); + + test('a WHERE or a plain EXPLAIN keeps these quiet', () { + for (final sql in [ + 'WITH x AS (SELECT 1 WHERE true) DELETE FROM t WHERE id IN (SELECT 1)', + 'EXPLAIN DELETE FROM t', + 'EXPLAIN ANALYZE DELETE FROM t WHERE id = 1', + 'WITH x AS (SELECT 1) SELECT * FROM x', + ]) { + 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); + expect(op.targetName, 'public.users'); + expect(op.type.riskLevel, 'HIGH'); + expect( + DestructiveSqlDetector.inspect( + 'ALTER TABLE users DROP CONSTRAINT users_pk') + .isDestructive, + isFalse, + ); + }); + + test('DROP INDEX / TYPE / FUNCTION / ROLE / USER are detected', () { + for (final sql in [ + 'DROP INDEX CONCURRENTLY IF EXISTS idx_users_email', + 'DROP TYPE mood', + 'DROP FUNCTION IF EXISTS public.f', + 'DROP ROLE app', + 'DROP USER app', + ]) { + expect(only(sql).type, DestructiveSqlType.dropOther, reason: sql); + } + expect(only('DROP FUNCTION IF EXISTS public.f').targetName, 'public.f'); + }); + }); }