From 12360e9793c4f2677f74f4e87b9c4286c2fc2052 Mon Sep 17 00:00:00 2001 From: onenewcode Date: Wed, 5 Aug 2026 14:51:13 +0800 Subject: [PATCH] fix(mysql): allow SHOW TRIGGERS in agent --- apps/desktop/src/lib/sidebar/treeNodeClick.ts | 4 +- crates/dbx-core/src/agent_tools.rs | 52 +++++++++++++++--- crates/dbx-core/src/sql_risk.rs | 53 ++++++++++++++++++- packages/app-tests/treeNodeClick.test.ts | 8 +-- 4 files changed, 103 insertions(+), 14 deletions(-) diff --git a/apps/desktop/src/lib/sidebar/treeNodeClick.ts b/apps/desktop/src/lib/sidebar/treeNodeClick.ts index ab229a26b..952ed2364 100644 --- a/apps/desktop/src/lib/sidebar/treeNodeClick.ts +++ b/apps/desktop/src/lib/sidebar/treeNodeClick.ts @@ -66,8 +66,8 @@ export function treeNodeRowDoubleClickAction(type: TreeNodeType, canOpenObjectBr return "none"; } -export function sidebarSelectionCopyAction(event: ShortcutLikeEvent): SidebarSelectionCopyAction { - return matchesShortcut(event, "Mod+C") ? "copy-name" : "none"; +export function sidebarSelectionCopyAction(event: ShortcutLikeEvent, platform?: string): SidebarSelectionCopyAction { + return matchesShortcut(event, "Mod+C", platform) ? "copy-name" : "none"; } export function copyNameForTreeNode(node: TreeNode): string { diff --git a/crates/dbx-core/src/agent_tools.rs b/crates/dbx-core/src/agent_tools.rs index 7cadf4429..bae1fca34 100644 --- a/crates/dbx-core/src/agent_tools.rs +++ b/crates/dbx-core/src/agent_tools.rs @@ -1097,12 +1097,12 @@ for line in sys.stdin: } #[cfg(unix)] - fn dameng_test_connection() -> ConnectionConfig { + fn agent_test_connection(id: &str, name: &str, db_type: DatabaseType, database: &str) -> ConnectionConfig { ConnectionConfig { - id: "dameng-1".to_string(), - name: "Dameng".to_string(), + id: id.to_string(), + name: name.to_string(), note: String::new(), - db_type: DatabaseType::Dameng, + db_type, driver_profile: None, driver_label: None, url_params: None, @@ -1111,7 +1111,7 @@ for line in sys.stdin: port: 5236, username: "APP_USER".to_string(), password: String::new(), - database: Some("APPDB".to_string()), + database: Some(database.to_string()), visible_databases: None, visible_schemas: None, show_system_schemas: false, @@ -1293,7 +1293,7 @@ for line in sys.stdin: let (client, _script) = spawn_recording_agent(&record_path).await; let storage = Storage::open(&temp_dir.path().join("storage.db")).await.unwrap(); let state = Arc::new(AppState::new(storage)); - let connection = dameng_test_connection(); + let connection = agent_test_connection("dameng-1", "Dameng", DatabaseType::Dameng, "APPDB"); state.configs.write().await.insert(connection.id.clone(), connection); state.connections.write().await.insert("dameng-1:APPDB".to_string(), PoolKind::agent(client)); @@ -1349,6 +1349,46 @@ for line in sys.stdin: } } + #[cfg(unix)] + #[tokio::test] + async fn mysql_agent_allows_show_triggers_without_write_confirmation() { + let temp_dir = tempfile::tempdir().unwrap(); + let record_path = temp_dir.path().join("agent-requests.jsonl"); + let (client, _script) = spawn_recording_agent(&record_path).await; + let storage = Storage::open(&temp_dir.path().join("storage.db")).await.unwrap(); + let state = Arc::new(AppState::new(storage)); + let connection = agent_test_connection("mysql-1", "MySQL", DatabaseType::Mysql, "rs_main"); + state.configs.write().await.insert(connection.id.clone(), connection); + state.connections.write().await.insert("mysql-1:rs_main".to_string(), PoolKind::agent(client)); + + let sql = "SHOW TRIGGERS FROM `rs_main` LIKE 'trg_order_items_after_%';"; + let call = ToolCall { + id: "show-triggers".to_string(), + name: "execute_query".to_string(), + arguments: json!({ "sql": sql }), + provider_payload: None, + }; + let result = execute_tool( + &call, + &state, + "mysql-1", + "rs_main", + None, + &DatabaseType::Mysql, + AgentSqlPermissions::default(), + ) + .await; + + assert!(!result.is_error, "{}", result.content); + let request = std::fs::read_to_string(&record_path) + .unwrap() + .lines() + .map(|line| serde_json::from_str::(line).unwrap()) + .find(|request| request["method"] == "execute_query") + .unwrap(); + assert_eq!(request["params"]["sql"], sql); + } + #[test] fn build_browse_query_qdrant() { let q = build_browse_query(&DatabaseType::Qdrant, "articles", "", 10).unwrap(); diff --git a/crates/dbx-core/src/sql_risk.rs b/crates/dbx-core/src/sql_risk.rs index 0315a4b5a..375b80d90 100644 --- a/crates/dbx-core/src/sql_risk.rs +++ b/crates/dbx-core/src/sql_risk.rs @@ -95,6 +95,14 @@ fn classify_statement(stmt: &Statement, detect_select_into: bool) -> SqlRisk { | Statement::ShowStatus { .. } | Statement::ShowProcessList { .. } => SqlRisk::ReadOnly, + // sqlparser has no dedicated MySQL `SHOW TRIGGERS` node and uses its + // loose `ShowVariable` fallback, with TRIGGERS as the first identifier. + Statement::ShowVariable { variable } + if variable.first().is_some_and(|identifier| identifier.value.eq_ignore_ascii_case("triggers")) => + { + SqlRisk::ReadOnly + } + // Write operations Statement::Insert { .. } | Statement::Update { .. } | Statement::Delete { .. } | Statement::Merge { .. } => { SqlRisk::Write @@ -797,8 +805,15 @@ fn classify_sql_risk_with_database( let parser_dialect = resolve_dialect(normalized_dialect); let detect_select_into = database_type.is_none(); let has_locking_clause = sql_contains_top_level_locking_clause(sql, parser_dialect.as_ref()); - let has_dialect_specific_write = database_type - .is_some_and(|database_type| crate::query_execution_sql::has_dialect_specific_write(sql, database_type)); + let has_dialect_specific_write = match database_type { + Some(database_type) => crate::query_execution_sql::has_dialect_specific_write(sql, database_type), + // Preserve MySQL executable-comment and file-output detection even + // when callers provide a dialect string instead of a database type. + None if normalized_dialect == "mysql" => { + crate::query_execution_sql::has_dialect_specific_write(sql, DatabaseType::Mysql) + } + None => false, + }; match Parser::parse_sql(parser_dialect.as_ref(), sql) { Ok(stmts) if !stmts.is_empty() => { @@ -874,6 +889,40 @@ mod tests { assert_eq!(classify_sql_risk("EXPLAIN SELECT * FROM users", "postgres").unwrap(), SqlRisk::ReadOnly); } + #[test] + fn classify_mysql_show_triggers_as_read_only() { + for sql in [ + "SHOW TRIGGERS;", + "SHOW TRIGGERS FROM `rs_main` LIKE 'trg_order_items_after_%';", + "show triggers in `rs_main` where `Event` = 'INSERT';", + ] { + assert_eq!(classify_sql_risk(sql, "mysql").unwrap(), SqlRisk::ReadOnly, "expected read-only: {sql}"); + assert_eq!( + classify_sql_risk_for_database(sql, DatabaseType::Mysql).unwrap(), + SqlRisk::ReadOnly, + "expected read-only: {sql}" + ); + assert!(!is_dangerous_sql_for_database(sql, DatabaseType::Mysql), "expected safe SQL: {sql}"); + } + } + + #[test] + fn mysql_show_triggers_preserves_write_detection() { + for (sql, expected_risk) in [ + ("SHOW TRIGGERS; DELETE FROM order_items", SqlRisk::Write), + ("SHOW TRIGGERS; DROP TABLE order_items", SqlRisk::Ddl), + ("SHOW TRIGGERS; /*!50000 DELETE FROM order_items */", SqlRisk::Write), + ] { + assert_eq!(classify_sql_risk(sql, "mysql").unwrap(), expected_risk, "expected write detection: {sql}"); + assert_eq!( + classify_sql_risk_for_database(sql, DatabaseType::Mysql).unwrap(), + expected_risk, + "expected write detection: {sql}" + ); + assert!(is_dangerous_sql_for_database(sql, DatabaseType::Mysql), "expected dangerous SQL: {sql}"); + } + } + #[test] fn classify_cte_read() { assert_eq!( diff --git a/packages/app-tests/treeNodeClick.test.ts b/packages/app-tests/treeNodeClick.test.ts index 7ab016947..d72ea9503 100644 --- a/packages/app-tests/treeNodeClick.test.ts +++ b/packages/app-tests/treeNodeClick.test.ts @@ -122,13 +122,13 @@ test("double click does not open object browser for non-browsable rows", () => { }); test("double click navigation mode copies the selected sidebar row name", () => { - assert.equal(sidebarSelectionCopyAction({ key: "c", metaKey: true }), "copy-name"); - assert.equal(sidebarSelectionCopyAction({ key: "C", ctrlKey: true }), "copy-name"); + assert.equal(sidebarSelectionCopyAction({ key: "c", metaKey: true }, "MacIntel"), "copy-name"); + assert.equal(sidebarSelectionCopyAction({ key: "C", ctrlKey: true }, "Win32"), "copy-name"); }); test("single click navigation mode copies the selected sidebar row name", () => { - assert.equal(sidebarSelectionCopyAction({ key: "c", metaKey: true }), "copy-name"); - assert.equal(sidebarSelectionCopyAction({ key: "C", ctrlKey: true }), "copy-name"); + assert.equal(sidebarSelectionCopyAction({ key: "c", metaKey: true }, "MacIntel"), "copy-name"); + assert.equal(sidebarSelectionCopyAction({ key: "C", ctrlKey: true }, "Win32"), "copy-name"); }); test("copying table child group rows uses the parent table name", () => {