Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion apps/tauri/src-tauri/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,16 +484,22 @@ pub fn db_erd_schema(

/// Execute SQL and stream `QueryEvent` batches over `on_event`.
/// Failures return as the invoke `Err` only (no dual channel `Error` event).
///
/// Pass `confirmed: true` after the UI acknowledged a
/// [`ConnectionError::NeedsConfirmation`] for DROP / TRUNCATE / unconditional
/// DELETE|UPDATE.
#[tauri::command]
pub fn db_execute_query(
id: ConnectionId,
sql: String,
limit: Option<u32>,
confirmed: Option<bool>,
on_event: Channel<crate::connection::QueryEvent>,
registry: State<'_, ConnectionRegistry>,
) -> Result<(), ConnectionError> {
let limit = limit.map(|n| n as usize);
registry.execute_query(&id, &sql, limit, |ev| {
let confirmed = confirmed.unwrap_or(false);
registry.execute_query(&id, &sql, limit, confirmed, |ev| {
on_event
.send(ev)
.map_err(|e| ConnectionError::io(format!("query channel closed: {e}")))
Expand Down
19 changes: 19 additions & 0 deletions apps/tauri/src-tauri/src/connection/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ pub enum ConnectionError {
rule: super::sql_guard::SqlGuardRule,
message: String,
},
/// Dangerous DML/DDL needs an explicit UI confirm flag before execute.
NeedsConfirmation {
#[serde(rename = "danger")]
kind: super::sql_guard::DangerousSqlKind,
message: String,
},
}

impl ConnectionError {
Expand Down Expand Up @@ -49,6 +55,16 @@ impl ConnectionError {
message: message.into(),
}
}

pub fn needs_confirmation(
kind: super::sql_guard::DangerousSqlKind,
message: impl Into<String>,
) -> Self {
Self::NeedsConfirmation {
kind,
message: message.into(),
}
}
}

impl fmt::Display for ConnectionError {
Expand All @@ -68,6 +84,9 @@ impl fmt::Display for ConnectionError {
Self::SqlGuard { rule, message } => {
write!(f, "sql guard ({}): {message}", rule.as_str())
}
Self::NeedsConfirmation { kind, message } => {
write!(f, "needs confirmation ({}): {message}", kind.as_str())
}
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions apps/tauri/src-tauri/src/connection/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,17 +226,22 @@ impl ConnectionRegistry {
}

/// Driver-aware query stream (SQLite + Postgres + MySQL).
///
/// `confirmed` must be true for DROP / TRUNCATE / unconditional DELETE|UPDATE
/// (see `sql_guard::assert_query_allowed`); otherwise returns
/// [`ConnectionError::NeedsConfirmation`].
pub fn execute_query(
&self,
id: &ConnectionId,
sql: &str,
limit: Option<usize>,
confirmed: bool,
emit: impl FnMut(QueryEvent) -> ConnectionResult<()>,
) -> ConnectionResult<()> {
let driver = self
.driver_of(id)
.ok_or_else(|| ConnectionError::NotFound { id: id.clone() })?;
super::sql_guard::assert_single_statement(sql, driver)?;
super::sql_guard::assert_query_allowed(sql, driver, confirmed)?;
match driver {
DriverKind::Sqlite => self.execute_sqlite_query(id, sql, limit, emit),
DriverKind::Postgres => {
Expand Down Expand Up @@ -500,7 +505,7 @@ mod tests {
})
.unwrap();
let err = reg
.execute_query(&id, "SELECT 1; SELECT 2", None, |_| Ok(()))
.execute_query(&id, "SELECT 1; SELECT 2", None, false, |_| Ok(()))
.unwrap_err();
match err {
ConnectionError::SqlGuard { rule, .. } => {
Expand Down
203 changes: 179 additions & 24 deletions apps/tauri/src-tauri/src/connection/sql_guard.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
//! sqlparser-rs multi-statement guard (Flutter MCP / table-browser parity).
//! sqlparser-rs query guards (Flutter MCP / table-browser parity).
//!
//! Product rule for `db_execute_query`: exactly one statement per call.
//! Trailing semicolons and comments are fine; scripts with two+ statements
//! are refused so the UI (or a later splitter) must send them separately.
//! `#1885` will layer dangerous-DML confirmation on top of this gate.
//! Product rules for `db_execute_query`:
//! 1. Exactly one statement per call (trailing `;` / comments OK).
//! 2. Dangerous DML/DDL (DROP / TRUNCATE / unconditional DELETE|UPDATE)
//! requires an explicit `confirmed` flag from the UI (#1885).

use super::error::{ConnectionError, ConnectionResult};
use super::id::DriverKind;
use sqlparser::ast::{ObjectType, Statement};
use sqlparser::dialect::{GenericDialect, MySqlDialect, PostgreSqlDialect, SQLiteDialect};
use sqlparser::parser::Parser;

Expand Down Expand Up @@ -37,6 +38,35 @@ impl SqlGuardRule {
}
}

/// Why a statement needs an explicit UI confirmation before running.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum DangerousSqlKind {
Drop,
Truncate,
UnconditionalDelete,
UnconditionalUpdate,
}

impl serde::Serialize for DangerousSqlKind {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_str(self.as_str())
}
}

impl DangerousSqlKind {
pub fn as_str(self) -> &'static str {
match self {
Self::Drop => "drop",
Self::Truncate => "truncate",
Self::UnconditionalDelete => "unconditional_delete",
Self::UnconditionalUpdate => "unconditional_update",
}
}
}

const EMPTY_QUERY_MSG: &str = "The query is empty.";

fn empty_err() -> ConnectionError {
Expand All @@ -47,17 +77,32 @@ fn parse_err(e: impl std::fmt::Display) -> ConnectionError {
ConnectionError::sql_guard(SqlGuardRule::Parse, format!("SQL parse error: {e}"))
}

/// Parse `sql` and ensure it is exactly one statement for `driver`.
pub fn assert_single_statement(sql: &str, driver: DriverKind) -> ConnectionResult<()> {
/// Full gate for `db_execute_query`: single-statement + destructive confirm.
pub fn assert_query_allowed(
sql: &str,
driver: DriverKind,
confirmed: bool,
) -> ConnectionResult<()> {
let stmt = parse_single_statement(sql, driver)?;
if confirmed {
return Ok(());
}
if let Some((kind, message)) = classify_dangerous(&stmt) {
return Err(ConnectionError::needs_confirmation(kind, message));
}
Ok(())
}

fn parse_single_statement(sql: &str, driver: DriverKind) -> ConnectionResult<Statement> {
let trimmed = sql.trim();
if trimmed.is_empty() {
return Err(empty_err());
}

let count = statement_count(trimmed, driver)?;
match count {
let mut statements = parse_statements(trimmed, driver)?;
match statements.len() {
0 => Err(empty_err()),
1 => Ok(()),
1 => Ok(statements.pop().expect("len checked")),
n => Err(ConnectionError::sql_guard(
SqlGuardRule::MultiStatement,
format!(
Expand All @@ -67,42 +112,118 @@ pub fn assert_single_statement(sql: &str, driver: DriverKind) -> ConnectionResul
}
}

fn statement_count(sql: &str, driver: DriverKind) -> ConnectionResult<usize> {
fn parse_statements(sql: &str, driver: DriverKind) -> ConnectionResult<Vec<Statement>> {
let primary = match driver {
DriverKind::Postgres => Parser::parse_sql(&PostgreSqlDialect {}, sql),
DriverKind::Sqlite => Parser::parse_sql(&SQLiteDialect {}, sql),
DriverKind::Mysql => Parser::parse_sql(&MySqlDialect {}, sql),
};

let statements = match primary {
Ok(stmts) => stmts,
match primary {
Ok(stmts) => Ok(stmts),
Err(primary_err) if !matches!(driver, DriverKind::Mysql) => {
Parser::parse_sql(&GenericDialect {}, sql).map_err(|_| parse_err(primary_err))?
Parser::parse_sql(&GenericDialect {}, sql).map_err(|_| parse_err(primary_err))
}
Err(primary_err) => return Err(parse_err(primary_err)),
};
Err(primary_err) => Err(parse_err(primary_err)),
}
}

Ok(statements.len())
fn classify_dangerous(stmt: &Statement) -> Option<(DangerousSqlKind, String)> {
match stmt {
Statement::Drop { object_type, names, .. } => {
let target = names
.first()
.map(ToString::to_string)
.unwrap_or_else(|| "object".into());
let label = object_type_label(*object_type);
Some((
DangerousSqlKind::Drop,
format!("DROP {label} \"{target}\" requires confirmation."),
))
}
Statement::Truncate(truncate) => {
let target = truncate
.table_names
.first()
.map(|t| t.name.to_string())
.unwrap_or_else(|| "table".into());
Some((
DangerousSqlKind::Truncate,
format!("TRUNCATE \"{target}\" requires confirmation."),
))
}
Statement::Delete(delete) if delete.selection.is_none() => {
let target = delete_target_name(delete);
Some((
DangerousSqlKind::UnconditionalDelete,
format!(
"DELETE FROM \"{target}\" without WHERE requires confirmation."
),
))
}
Statement::Update(update) if update.selection.is_none() => {
let target = update.table.relation.to_string();
Some((
DangerousSqlKind::UnconditionalUpdate,
format!(
"UPDATE \"{target}\" without WHERE requires confirmation."
),
))
}
_ => None,
}
}

fn object_type_label(object_type: ObjectType) -> String {
object_type.to_string()
}

fn delete_target_name(delete: &sqlparser::ast::Delete) -> String {
if let Some(name) = delete.tables.first() {
return name.to_string();
}
match &delete.from {
sqlparser::ast::FromTable::WithFromKeyword(tables)
| sqlparser::ast::FromTable::WithoutKeyword(tables) => tables
.first()
.map(|t| t.relation.to_string())
.unwrap_or_else(|| "table".into()),
}
}

#[cfg(test)]
mod tests {
use super::*;

fn allow(sql: &str, driver: DriverKind) {
assert_single_statement(sql, driver).unwrap_or_else(|e| {
assert_query_allowed(sql, driver, false).unwrap_or_else(|e| {
panic!("expected allow for {sql:?} ({driver}): {e}");
});
}

fn allow_confirmed(sql: &str, driver: DriverKind) {
assert_query_allowed(sql, driver, true).unwrap_or_else(|e| {
panic!("expected allow with confirm for {sql:?} ({driver}): {e}");
});
}

fn block_rule(sql: &str, driver: DriverKind, rule: SqlGuardRule) {
let err = assert_single_statement(sql, driver).expect_err("expected block");
let err = assert_query_allowed(sql, driver, false).expect_err("expected block");
match err {
ConnectionError::SqlGuard { rule: r, .. } => assert_eq!(r, rule),
other => panic!("expected SqlGuard, got {other:?}"),
}
}

fn needs_confirm(sql: &str, driver: DriverKind, kind: DangerousSqlKind) {
let err = assert_query_allowed(sql, driver, false).expect_err("expected confirm");
match err {
ConnectionError::NeedsConfirmation { kind: k, .. } => assert_eq!(k, kind),
other => panic!("expected NeedsConfirmation, got {other:?}"),
}
allow_confirmed(sql, driver);
}

#[test]
fn allows_single_select() {
allow("SELECT 1", DriverKind::Sqlite);
Expand Down Expand Up @@ -137,13 +258,36 @@ mod tests {
}

#[test]
fn allows_single_dml_for_now() {
// Dangerous-DML confirmation is #1885; this gate only counts statements.
allow("DELETE FROM t", DriverKind::Postgres);
allow("UPDATE t SET a = 1", DriverKind::Sqlite);
fn allows_conditional_dml_without_confirm() {
allow("DELETE FROM t WHERE id = 1", DriverKind::Postgres);
allow("UPDATE t SET a = 1 WHERE id = 2", DriverKind::Sqlite);
allow("INSERT INTO t VALUES (1)", DriverKind::Mysql);
}

#[test]
fn requires_confirm_for_drop_truncate_unconditional() {
needs_confirm(
"DROP TABLE t",
DriverKind::Postgres,
DangerousSqlKind::Drop,
);
needs_confirm(
"TRUNCATE TABLE t",
DriverKind::Postgres,
DangerousSqlKind::Truncate,
);
needs_confirm(
"DELETE FROM t",
DriverKind::Sqlite,
DangerousSqlKind::UnconditionalDelete,
);
needs_confirm(
"UPDATE t SET a = 1",
DriverKind::Mysql,
DangerousSqlKind::UnconditionalUpdate,
);
}

#[test]
fn rejects_multi_statement_even_when_first_is_select() {
block_rule(
Expand Down Expand Up @@ -214,4 +358,15 @@ mod tests {
assert_eq!(json["kind"], "sql_guard");
assert_eq!(json["rule"], "single_statement");
}
}

#[test]
fn needs_confirmation_serde() {
let err = ConnectionError::needs_confirmation(
DangerousSqlKind::Drop,
"DROP TABLE \"t\" requires confirmation.",
);
let json = serde_json::to_value(&err).unwrap();
assert_eq!(json["kind"], "needs_confirmation");
assert_eq!(json["danger"], "drop");
}
}
Loading
Loading