From 180ef098fe169bf5e6342345cf05a9aa49835822 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 14 Oct 2026 16:34:17 +0700 Subject: [PATCH 1/7] fix(linux): withhold a credential carrier called in a join or WHERE --- .../core/src/sql_syntax/credential_rules.rs | 73 +++++++++++++++++-- .../core/src/sql_syntax/history_recording.rs | 30 ++++++++ linux/crates/drivers/mssql/src/dialect.rs | 2 + linux/crates/drivers/mysql/src/dialect.rs | 2 + linux/crates/drivers/postgres/src/dialect.rs | 8 ++ 5 files changed, 109 insertions(+), 6 deletions(-) diff --git a/linux/crates/core/src/sql_syntax/credential_rules.rs b/linux/crates/core/src/sql_syntax/credential_rules.rs index d48164e7a2..2d23876692 100644 --- a/linux/crates/core/src/sql_syntax/credential_rules.rs +++ b/linux/crates/core/src/sql_syntax/credential_rules.rs @@ -25,7 +25,24 @@ pub struct CredentialRules { /// name followed by a parenthesis is a table and its column list, not a /// call: `CREATE TABLE s3 (id int)`. A carrier the engine reserves, such /// as T-SQL's OPENROWSET, cannot name a table, so there it is a call. -const NAME_POSITIONS: [&str; 6] = ["TABLE", "INTO", "VIEW", "REFERENCES", "EXISTS", "ON"]; +/// `EXISTS` and `ON` also start conditions, so `names_an_object` takes +/// them only in `TABLE IF NOT EXISTS name` and `CREATE INDEX ... ON name`. +const NAME_POSITIONS: [&str; 4] = ["TABLE", "INTO", "VIEW", "REFERENCES"]; + +/// The words a CREATE takes before INDEX or STATISTICS, in any engine. +const INDEX_MODIFIERS: [&str; 11] = [ + "UNIQUE", + "CLUSTERED", + "NONCLUSTERED", + "COLUMNSTORE", + "PRIMARY", + "SELECTIVE", + "XML", + "SPATIAL", + "FULLTEXT", + "OR", + "REPLACE", +]; impl CredentialRules { pub(crate) fn matches(&self, grammar: SqlGrammar, tokens: &[StatementToken<'_>]) -> bool { @@ -135,21 +152,65 @@ fn qualified_name_start(tokens: &[StatementToken<'_>], at: usize) -> usize { } } +/// Whether the name starting at `start` is where the grammar puts a +/// table. Anything else is read as a call, since a call there that was +/// taken for a name would put its arguments in the history. fn names_an_object(tokens: &[StatementToken<'_>], start: usize) -> bool { - let Some(before) = start.checked_sub(1).and_then(|index| tokens.get(index)) else { + let word_before = |back: usize| start.checked_sub(back).and_then(|index| tokens.get(index)); + let Some(before) = word_before(1) else { return false; }; + if before.is_word("ON") { + return on_names_an_indexed_table(tokens, start - 1); + } + if before.is_word("EXISTS") { + // `IF [NOT] EXISTS name (` declares a table or a view; a bare + // EXISTS is a condition. + let mut back = 2; + if word_before(back).is_some_and(|token| token.is_word("NOT")) { + back += 1; + } + return word_before(back).is_some_and(|token| token.is_word("IF")) + && word_before(back + 1).is_some_and(|token| token.is_word("TABLE") || token.is_word("VIEW")); + } if !NAME_POSITIONS.iter().any(|word| before.is_word(word)) { return false; } // ClickHouse describes a table function's result: DESCRIBE TABLE s3(...). - let described = start - .checked_sub(2) - .and_then(|index| tokens.get(index)) - .is_some_and(|token| token.is_word("DESCRIBE") || token.is_word("DESC")); + let described = word_before(2).is_some_and(|token| token.is_word("DESCRIBE") || token.is_word("DESC")); !(before.is_word("TABLE") && described) } +/// `CREATE [UNIQUE ...] INDEX|STATISTICS [name] ON table (columns)`, the +/// one place a table name and a parenthesis follow ON. A JOIN's ON, or +/// any other, starts a condition. +fn on_names_an_indexed_table(tokens: &[StatementToken<'_>], on: usize) -> bool { + let mut at = on; + while at > 0 { + at -= 1; + let Some(token) = tokens.get(at) else { + return false; + }; + if token.is_word("CREATE") { + let mut next = at + 1; + while tokens + .get(next) + .is_some_and(|token| INDEX_MODIFIERS.iter().any(|word| token.is_word(word))) + { + next += 1; + } + return next < on + && tokens + .get(next) + .is_some_and(|token| token.is_word("INDEX") || token.is_word("STATISTICS")); + } + if token.name().is_none() && !token.is_punct('.') { + return false; + } + } + false +} + /// `EXEC name`, or `EXEC @status = name`. fn follows_exec(tokens: &[StatementToken<'_>], start: usize) -> bool { let token_before = |back: usize| start.checked_sub(back).and_then(|index| tokens.get(index)); diff --git a/linux/crates/core/src/sql_syntax/history_recording.rs b/linux/crates/core/src/sql_syntax/history_recording.rs index 3f362fa662..ef0f369e16 100644 --- a/linux/crates/core/src/sql_syntax/history_recording.rs +++ b/linux/crates/core/src/sql_syntax/history_recording.rs @@ -277,9 +277,39 @@ mod tests { "CREATE VIEW s3 (id) AS SELECT 1", "SELECT s3.id FROM s3", "SELECT 1 AS s3", + "CREATE INDEX i ON s3 (id) TYPE minmax", + "CREATE INDEX IF NOT EXISTS i ON db.s3 (id) TYPE minmax", ] { assert_eq!(classify(SqlGrammar::ClickHouse, sql), HistoryRecording::Record, "{sql}"); } + for (grammar, sql) in [ + (SqlGrammar::PostgreSql, "CREATE UNIQUE INDEX CONCURRENTLY i ON s3 (id)"), + (SqlGrammar::PostgreSql, "CREATE INDEX ON s3 (id)"), + (SqlGrammar::MySql, "CREATE UNIQUE INDEX i USING BTREE ON s3 (id)"), + (SqlGrammar::MsSql, "CREATE UNIQUE NONCLUSTERED INDEX i ON dbo.s3 (id)"), + (SqlGrammar::MsSql, "CREATE STATISTICS st ON s3 (id)"), + (SqlGrammar::Sqlite, "CREATE INDEX IF NOT EXISTS main.i ON s3 (id)"), + (SqlGrammar::Sqlite, "CREATE TABLE t (id int REFERENCES s3 (id))"), + ] { + assert_eq!(classify(grammar, sql), HistoryRecording::Record, "{grammar:?}: {sql}"); + } + } + + #[test] + fn a_carrier_in_a_condition_is_a_call() { + for grammar in SqlGrammar::ALL { + for sql in [ + "SELECT u.id FROM users u JOIN logins l ON dblink_connect('password=x') = l.v", + "SELECT a.id FROM a JOIN b ON a.id = b.id AND dblink_connect('password=x') = 'OK'", + "CREATE VIEW v AS SELECT a.id FROM a JOIN b ON dblink_connect('password=x') = b.v", + "SELECT 1 WHERE EXISTS dblink_connect('password=x')", + "SELECT 1 WHERE NOT EXISTS dblink_connect('password=x')", + "CREATE TABLE t AS SELECT 1 FROM a WHERE EXISTS dblink_connect('password=x')", + "SELECT * FROM a JOIN b ON s3('url', 'key', 'secret') = b.v", + ] { + assert_eq!(classify(grammar, sql), WITHHELD, "{grammar:?}: {sql}"); + } + } } #[test] diff --git a/linux/crates/drivers/mssql/src/dialect.rs b/linux/crates/drivers/mssql/src/dialect.rs index 2f4d4c9546..fa51698434 100644 --- a/linux/crates/drivers/mssql/src/dialect.rs +++ b/linux/crates/drivers/mssql/src/dialect.rs @@ -692,6 +692,7 @@ mod tests { "SELECT ENCRYPTBYPASSPHRASE('passphrase', 'data')", "SELECT DECRYPTBYPASSPHRASE('passphrase', secret) FROM t", "EXEC sp_addlinkedserver @server = 's', @srvproduct = '', @provider = 'MSOLEDBSQL', @provstr = 'Server=h;Uid=u;Pwd=p;'", + "SELECT u.id FROM users u JOIN logins l ON DECRYPTBYPASSPHRASE('secret', l.hash) = u.token", ] { assert_eq!( dialect().history_recording(sql), @@ -711,6 +712,7 @@ mod tests { "ALTER TABLE users ADD password varchar(100)", "INSERT INTO [OPENROWSET] (c) VALUES (1)", "SELECT 1\nPRINT sp_addlogin", + "CREATE NONCLUSTERED INDEX ix ON dbo.sp_password (id)", ] { assert_eq!(dialect().history_recording(sql), HistoryRecording::Record, "{sql}"); } diff --git a/linux/crates/drivers/mysql/src/dialect.rs b/linux/crates/drivers/mysql/src/dialect.rs index cef0619386..7059b6bbc3 100644 --- a/linux/crates/drivers/mysql/src/dialect.rs +++ b/linux/crates/drivers/mysql/src/dialect.rs @@ -653,6 +653,7 @@ mod tests { "SELECT OLD_PASSWORD('x')", "START GROUP_REPLICATION USER='u', PASSWORD='p'", "CREATE TABLE t (id int) /*!50100 ENGINE=FEDERATED CONNECTION='mysql://u:p@h/d/t' */", + "SELECT u.id FROM users u JOIN logins l ON PASSWORD('secret') = l.hash", ] { assert_eq!( dialect(MySqlFlavour::MySql).history_recording(sql), @@ -673,6 +674,7 @@ mod tests { "INSERT INTO users (name, password) VALUES ('a', 'b')", "SELECT start, password FROM sessions", "START TRANSACTION", + "CREATE INDEX by_user USING BTREE ON password (user_id)", ] { assert_eq!( dialect(MySqlFlavour::MySql).history_recording(sql), diff --git a/linux/crates/drivers/postgres/src/dialect.rs b/linux/crates/drivers/postgres/src/dialect.rs index 0a766d0e9c..ee49f0682e 100644 --- a/linux/crates/drivers/postgres/src/dialect.rs +++ b/linux/crates/drivers/postgres/src/dialect.rs @@ -626,6 +626,8 @@ mod tests { "SELECT pgp_sym_encrypt('data', 'passphrase')", "SELECT pgp_sym_decrypt(secret, 'passphrase') FROM t", "UPDATE users SET pw = crypt('secret', gen_salt('bf'))", + "SELECT u.id FROM users u JOIN credentials c ON crypt('secret', c.hash) = c.hash", + "SELECT id FROM users WHERE EXISTS crypt('secret', hash)", ] { assert_eq!( dialect().history_recording(sql), @@ -641,5 +643,11 @@ mod tests { tablepro_test_support::history_cases::unrecorded_plain_statements(&dialect()), Vec::<&str>::new() ); + for sql in [ + "CREATE INDEX ON crypt (hash)", + "CREATE TABLE IF NOT EXISTS crypt (hash text)", + ] { + assert_eq!(dialect().history_recording(sql), HistoryRecording::Record, "{sql}"); + } } } From 6913279ae0ee0818a865051e3abe9630f3db25cb Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 14 Oct 2026 16:34:17 +0700 Subject: [PATCH 2/7] fix(linux-sqlite): compare foreign key violations by the key they break --- linux/crates/core/src/foreign_key_check.rs | 13 ++ linux/crates/drivers/sqlite/src/catalog.rs | 3 +- .../drivers/sqlite/src/ddl/rebuild_plan.rs | 3 +- .../sqlite/src/foreign_key_verification.rs | 147 +++++++++++++++--- .../crates/drivers/sqlite/src/write_batch.rs | 40 ++++- .../drivers/sqlite/tests/integration/ddl.rs | 80 ++++++++++ 6 files changed, 261 insertions(+), 25 deletions(-) diff --git a/linux/crates/core/src/foreign_key_check.rs b/linux/crates/core/src/foreign_key_check.rs index 06b106f526..af443535b9 100644 --- a/linux/crates/core/src/foreign_key_check.rs +++ b/linux/crates/core/src/foreign_key_check.rs @@ -1,3 +1,4 @@ +use crate::ddl::ColumnRename; use crate::meta::TableRef; /// One table whose foreign keys a suspended batch verifies. @@ -17,6 +18,10 @@ pub struct ForeignKeyCheck { /// known by a different rowid on each side, so the violations in it /// compare by count. pub rowids_renumbered: bool, + /// The table's columns the batch renames. A key on a renamed column + /// is the same constraint on both sides, so a violation it already + /// had is not the batch's. + pub renamed_columns: Vec, } impl ForeignKeyCheck { @@ -25,6 +30,7 @@ impl ForeignKeyCheck { before, after, rowids_renumbered: false, + renamed_columns: Vec::new(), } } @@ -39,4 +45,11 @@ impl ForeignKeyCheck { ..self } } + + pub fn with_renamed_columns(self, renamed_columns: Vec) -> Self { + Self { + renamed_columns, + ..self + } + } } diff --git a/linux/crates/drivers/sqlite/src/catalog.rs b/linux/crates/drivers/sqlite/src/catalog.rs index 52d19e4c29..2b1cfbcff4 100644 --- a/linux/crates/drivers/sqlite/src/catalog.rs +++ b/linux/crates/drivers/sqlite/src/catalog.rs @@ -44,7 +44,8 @@ pub(crate) const INDEX_XINFO_SQL: &str = pub(crate) const FOREIGN_KEY_LIST_SQL: &str = "SELECT id, \"table\", \"from\", \"to\", on_update, on_delete \ FROM pragma_foreign_key_list(?1, ?2) ORDER BY id, seq"; -pub(crate) const FOREIGN_KEY_CHECK_SQL: &str = "SELECT \"table\", rowid, parent FROM pragma_foreign_key_check(?1, ?2)"; +pub(crate) const FOREIGN_KEY_CHECK_SQL: &str = + "SELECT \"table\", rowid, parent, fkid FROM pragma_foreign_key_check(?1, ?2)"; /// `sqlite_schema` of one schema, which has to be named in the FROM. pub(crate) fn objects_sql(schema_sql: &str) -> String { diff --git a/linux/crates/drivers/sqlite/src/ddl/rebuild_plan.rs b/linux/crates/drivers/sqlite/src/ddl/rebuild_plan.rs index 8b6f498435..3cdc78734d 100644 --- a/linux/crates/drivers/sqlite/src/ddl/rebuild_plan.rs +++ b/linux/crates/drivers/sqlite/src/ddl/rebuild_plan.rs @@ -355,7 +355,8 @@ impl Rebuild<'_> { /// table. fn checks(&self, target: &TableRef) -> Vec { let table = &self.original.table; - let rebuilt = ForeignKeyCheck::new(Some(table.clone()), target.clone()); + let rebuilt = ForeignKeyCheck::new(Some(table.clone()), target.clone()) + .with_renamed_columns(self.changes.renamed_columns.clone()); let mut checks = vec![match self.rowids_renumbered() { true => rebuilt.renumbered(), false => rebuilt, diff --git a/linux/crates/drivers/sqlite/src/foreign_key_verification.rs b/linux/crates/drivers/sqlite/src/foreign_key_verification.rs index ab665ef6a1..b8033dde7e 100644 --- a/linux/crates/drivers/sqlite/src/foreign_key_verification.rs +++ b/linux/crates/drivers/sqlite/src/foreign_key_verification.rs @@ -9,6 +9,7 @@ use std::collections::HashMap; +use tablepro_core::ddl::ColumnRename; use tablepro_core::{DriverError, ServerCode, ServerDiagnostics}; use crate::errors::{SQLITE_CONSTRAINT, SQLITE_CONSTRAINT_FOREIGNKEY}; @@ -21,6 +22,18 @@ pub(crate) struct Violation { /// name the row by, and for one whose rows the batch renumbers. pub rowid: Option, pub parent: String, + pub key: ViolatedKey, +} + +/// The foreign key a row breaks, by its columns rather than its number: +/// a rebuild numbers the keys again, and can trade one key for another +/// to the same parent. +#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)] +pub(crate) struct ViolatedKey { + pub columns: Vec, + /// Empty for one that names no columns and so points at the parent's + /// primary key. + pub parent_columns: Vec, } /// Violations in one child table against one parent, that the batch @@ -32,25 +45,65 @@ pub(crate) struct IntroducedViolations { pub count: usize, } -/// Name `baseline` rows by the names the tables have after the batch, -/// so a table the batch renamed compares with itself. -pub(crate) fn renamed(baseline: Vec, renames: &HashMap) -> Vec { - let rename = |name: String| renames.get(&name.to_lowercase()).cloned().unwrap_or(name); - baseline - .into_iter() - .map(|violation| Violation { - table: rename(violation.table), - rowid: violation.rowid, - parent: rename(violation.parent), - }) - .collect() +/// The names a batch gives tables and their columns, keyed by the +/// table's name before it, folded. +#[derive(Debug, Default)] +pub(crate) struct Renames { + tables: HashMap, + columns: HashMap>, +} + +impl Renames { + pub(crate) fn add(&mut self, before: &str, after: &str, columns: &[ColumnRename]) { + let before = before.to_lowercase(); + self.tables.insert(before.clone(), after.to_owned()); + if !columns.is_empty() { + let renamed = columns + .iter() + .map(|rename| (rename.from.to_lowercase(), rename.to.clone())) + .collect(); + self.columns.insert(before, renamed); + } + } + + /// Name `baseline` rows by the names their tables and key columns + /// have after the batch, so a renamed table or key compares with + /// itself. + pub(crate) fn apply(&self, baseline: Vec) -> Vec { + baseline + .into_iter() + .map(|violation| Violation { + key: ViolatedKey { + columns: self.column_names(&violation.table, violation.key.columns), + parent_columns: self.column_names(&violation.parent, violation.key.parent_columns), + }, + table: self.table_name(violation.table), + rowid: violation.rowid, + parent: self.table_name(violation.parent), + }) + .collect() + } + + fn table_name(&self, name: String) -> String { + self.tables.get(&name.to_lowercase()).cloned().unwrap_or(name) + } + + fn column_names(&self, table: &str, columns: Vec) -> Vec { + let Some(renamed) = self.columns.get(&table.to_lowercase()) else { + return columns; + }; + columns + .into_iter() + .map(|column| renamed.get(&column.to_lowercase()).cloned().unwrap_or(column)) + .collect() + } } /// The rows of `after` that `before` does not account for, compared as /// multisets. Rows without a rowid are all alike, so for them it is the -/// count per child and parent that compares. +/// count per child, parent and key that compares. pub(crate) fn introduced(before: &[Violation], after: &[Violation]) -> Vec { - let mut remaining: HashMap<(String, Option, String), usize> = HashMap::new(); + let mut remaining: HashMap = HashMap::new(); for violation in before { *remaining.entry(key(violation)).or_default() += 1; } @@ -79,12 +132,18 @@ pub(crate) fn introduced(before: &[Violation], after: &[Violation]) -> Vec (String, Option, String) { +type ViolationKey = (String, Option, String, Vec, Vec); + +/// SQLite names tables and columns case-insensitively, and so does the +/// comparison. +fn key(violation: &Violation) -> ViolationKey { + let folded = |names: &[String]| names.iter().map(|name| name.to_lowercase()).collect(); ( violation.table.to_lowercase(), violation.rowid, violation.parent.to_lowercase(), + folded(&violation.key.columns), + folded(&violation.key.parent_columns), ) } @@ -114,27 +173,49 @@ mod tests { use super::*; fn violation(table: &str, rowid: Option, parent: &str) -> Violation { + keyed(table, rowid, parent, &["parent"], &["id"]) + } + + fn keyed(table: &str, rowid: Option, parent: &str, columns: &[&str], parent_columns: &[&str]) -> Violation { + let owned = |names: &[&str]| names.iter().map(|name| (*name).to_owned()).collect(); Violation { table: table.to_owned(), rowid, parent: parent.to_owned(), + key: ViolatedKey { + columns: owned(columns), + parent_columns: owned(parent_columns), + }, } } + fn renames(before: &str, after: &str, columns: &[(&str, &str)]) -> Renames { + let mut renames = Renames::default(); + let columns: Vec = columns + .iter() + .map(|(from, to)| ColumnRename { + from: (*from).to_owned(), + to: (*to).to_owned(), + }) + .collect(); + renames.add(before, after, &columns); + renames + } + #[test] fn introduced_violations_ignore_baseline_rows_across_rename() { let before = vec![ violation("orders_old", Some(7), "people"), violation("lines", Some(1), "orders_old"), ]; - let renames = HashMap::from([("orders_old".to_owned(), "orders".to_owned())]); + let renames = renames("orders_old", "orders", &[]); let after = vec![ violation("orders", Some(7), "people"), violation("lines", Some(1), "orders"), violation("lines", Some(2), "orders"), ]; - let introduced = introduced(&renamed(before, &renames), &after); + let introduced = introduced(&renames.apply(before), &after); assert_eq!( introduced, @@ -146,6 +227,36 @@ mod tests { ); } + #[test] + fn a_row_breaking_another_key_to_the_same_parent_is_introduced() { + let before = vec![keyed("children", Some(1), "parents", &["a"], &["id"])]; + let after = vec![keyed("children", Some(1), "parents", &["b"], &["id"])]; + + assert_eq!( + introduced(&before, &after), + [IntroducedViolations { + table: "children".to_owned(), + parent: "parents".to_owned(), + count: 1 + }] + ); + } + + #[test] + fn a_renamed_key_column_keeps_its_violation_on_either_side() { + let before = vec![ + keyed("children", Some(1), "parents", &["parent"], &["id"]), + keyed("grandchildren", Some(4), "children", &["child"], &["Parent"]), + ]; + let renames = renames("children", "children", &[("parent", "parent_id")]); + let after = vec![ + keyed("children", Some(1), "parents", &["parent_id"], &["id"]), + keyed("grandchildren", Some(4), "children", &["child"], &["parent_id"]), + ]; + + assert!(introduced(&renames.apply(before), &after).is_empty()); + } + #[test] fn without_rowid_violations_compare_by_count() { let before = vec![violation("tags", None, "posts"), violation("tags", None, "posts")]; diff --git a/linux/crates/drivers/sqlite/src/write_batch.rs b/linux/crates/drivers/sqlite/src/write_batch.rs index 83165a5542..14e872ec2d 100644 --- a/linux/crates/drivers/sqlite/src/write_batch.rs +++ b/linux/crates/drivers/sqlite/src/write_batch.rs @@ -19,10 +19,10 @@ use tablepro_core::{ }; use crate::autocommit::is_autocommit; -use crate::catalog::{FOREIGN_KEY_CHECK_SQL, MAIN_SCHEMA}; +use crate::catalog::{FOREIGN_KEY_CHECK_SQL, FOREIGN_KEY_LIST_SQL, MAIN_SCHEMA}; use crate::encode::bind_all; use crate::errors::{SQLITE_IOERR, is_interrupt, map_error, primary_of}; -use crate::foreign_key_verification::{Violation, introduced, renamed, violation_error}; +use crate::foreign_key_verification::{Renames, ViolatedKey, Violation, introduced, violation_error}; use crate::interrupt::InterruptFlag; use crate::result_stream::{count, reports_changes}; use crate::sqlite_pooled::SqlitePooled; @@ -300,15 +300,15 @@ fn stop_cause(reason: StopReason) -> DriverError { /// The violations already there, named as they will be after the batch. async fn baseline(connection: &mut SqlxConnection, checks: &[ForeignKeyCheck]) -> Result, DriverError> { let mut found = Vec::new(); - let mut renames = HashMap::new(); + let mut renames = Renames::default(); for check in checks { let Some(before) = &check.before else { continue; }; found.extend(violations(connection, before, check.rowids_renumbered).await?); - renames.insert(before.name.to_lowercase(), check.after.name.clone()); + renames.add(&before.name, &check.after.name, &check.renamed_columns); } - Ok(renamed(found, &renames)) + Ok(renames.apply(found)) } async fn violations_after( @@ -341,14 +341,44 @@ async fn violations( .fetch_all(&mut *connection) .await .map_err(map_error)?; + if rows.is_empty() { + return Ok(Vec::new()); + } + let keys = foreign_keys(connection, table, schema).await?; rows.iter() .map(|row| { let rowid: Option = row.try_get("rowid").map_err(map_error)?; + let fkid: i64 = row.try_get("fkid").map_err(map_error)?; Ok(Violation { table: row.try_get("table").map_err(map_error)?, rowid: rowid.filter(|_| !rowids_renumbered), parent: row.try_get("parent").map_err(map_error)?, + key: keys.get(&fkid).cloned().unwrap_or_default(), }) }) .collect() } + +/// The table's foreign keys by the id `foreign_key_check` names them by. +async fn foreign_keys( + connection: &mut SqlxConnection, + table: &TableRef, + schema: &str, +) -> Result, DriverError> { + let rows = sqlx::query(FOREIGN_KEY_LIST_SQL) + .bind(table.name.as_str()) + .bind(schema) + .fetch_all(&mut *connection) + .await + .map_err(map_error)?; + let mut keys: HashMap = HashMap::new(); + for row in rows { + let id: i64 = row.try_get("id").map_err(map_error)?; + let from: String = row.try_get("from").map_err(map_error)?; + let to: Option = row.try_get("to").map_err(map_error)?; + let key = keys.entry(id).or_default(); + key.columns.push(from); + key.parent_columns.extend(to); + } + Ok(keys) +} diff --git a/linux/crates/drivers/sqlite/tests/integration/ddl.rs b/linux/crates/drivers/sqlite/tests/integration/ddl.rs index 183ccde5cc..240314fa55 100644 --- a/linux/crates/drivers/sqlite/tests/integration/ddl.rs +++ b/linux/crates/drivers/sqlite/tests/integration/ddl.rs @@ -461,6 +461,86 @@ async fn adding_violating_foreign_key_rolls_back_with_diagnostics() { connection.close().await.unwrap(); } +/// A row that already broke one key still makes the save fail when the +/// save swaps that key for another one the row breaks. +#[tokio::test] +async fn replacing_a_violated_key_with_another_the_row_breaks_rolls_back() { + let file = TestFile::new().await; + let connection = file.open().await; + apply( + connection.as_ref(), + &[ + "CREATE TABLE parents (id INTEGER PRIMARY KEY)", + "CREATE TABLE children (id INTEGER PRIMARY KEY, a INTEGER, b INTEGER, FOREIGN KEY (a) REFERENCES \ + parents (id))", + "INSERT INTO parents VALUES (1)", + ], + ) + .await + .unwrap(); + unchecked(connection.as_ref(), &["INSERT INTO children VALUES (1, 99, 98)"]).await; + let before = read(connection.as_ref(), &named("children")).await; + + let outcome = save(connection.as_ref(), &named("children"), |draft| { + draft.foreign_keys = vec![ForeignKeyInfo { + name: "children_b".to_owned(), + columns: vec!["b".to_owned()], + ref_schema: None, + ref_table: "parents".to_owned(), + ref_columns: vec!["id".to_owned()], + on_delete: None, + on_update: None, + }]; + }) + .await; + + let Err(DriverError::RolledBack { source, .. }) = &outcome else { + panic!("a key the save added was let through broken: {outcome:?}"); + }; + assert!(source.to_string().contains("1 rows of children"), "{source}"); + assert_eq!(read(connection.as_ref(), &named("children")).await, before); + connection.close().await.unwrap(); +} + +/// A key column renamed in a rebuild is still the key it was, so the +/// rows that broke it before are the file's, on either side of the key. +#[tokio::test] +async fn a_renamed_key_column_keeps_the_violations_it_had() { + let file = TestFile::new().await; + let connection = file.open().await; + apply( + connection.as_ref(), + &[ + "CREATE TABLE parents (id INTEGER PRIMARY KEY, label TEXT)", + "CREATE TABLE children (id INTEGER PRIMARY KEY, parent INTEGER REFERENCES parents (id), note TEXT)", + "INSERT INTO parents VALUES (1, 'one')", + ], + ) + .await + .unwrap(); + unchecked(connection.as_ref(), &["INSERT INTO children VALUES (1, 99, 'x')"]).await; + + let children = save(connection.as_ref(), &named("children"), |draft| { + column(draft, "parent").name = "parent_id".to_owned(); + retype(draft, "note", "BLOB"); + }) + .await; + let parents = save(connection.as_ref(), &named("parents"), |draft| { + column(draft, "id").name = "key".to_owned(); + retype(draft, "label", "BLOB"); + }) + .await; + + assert!(children.is_ok(), "{children:?}"); + assert!(parents.is_ok(), "{parents:?}"); + let keys = read(connection.as_ref(), &named("children")).await.foreign_keys; + assert_eq!(keys[0].columns, ["parent_id"]); + assert_eq!(keys[0].ref_columns, ["key"]); + let orphans = query(connection.as_ref(), "SELECT count(*) FROM pragma_foreign_key_check").await; + assert_eq!(orphans.rows[0][0], Value::Int(1)); + connection.close().await.unwrap(); +} + /// Two columns trade names while one of them changes type, so the /// renames after the rebuild step aside through a temporary name. #[tokio::test] From 02179aa8bfed2b396787289f6e12e2c9cbbd2cf7 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 14 Oct 2026 16:39:17 +0700 Subject: [PATCH 3/7] fix(linux-grid): keep a cell menu on the rows it was opened for --- linux/crates/app/src/ui/grid/context_menu.rs | 167 +++++++++++++++++-- linux/crates/app/src/ui/grid/grid_menus.rs | 37 ++-- linux/crates/app/src/ui/grid/menu_rows.rs | 3 +- 3 files changed, 175 insertions(+), 32 deletions(-) diff --git a/linux/crates/app/src/ui/grid/context_menu.rs b/linux/crates/app/src/ui/grid/context_menu.rs index 7a404b7753..6ebe217508 100644 --- a/linux/crates/app/src/ui/grid/context_menu.rs +++ b/linux/crates/app/src/ui/grid/context_menu.rs @@ -1,26 +1,66 @@ +use std::cell::RefCell; +use std::collections::HashSet; + use gtk4::prelude::*; use gtk4::{self as gtk, glib}; -use super::GridCell; use super::grid_menus::GridMenus; use super::menu_rows::menu_rows_of; use super::selection::select_row_for_menu; +use super::{GridCell, RowObject}; -/// The cell a menu was opened on. The cell holds the menu through its -/// gesture, so the menu holds the cell weakly, and asks it for its row -/// only when an item is chosen. +/// What a menu was opened on, fixed as it opens. A page that lands while +/// the menu is open rebinds the cell to another row, so an item chosen +/// then acts on the rows the menu was opened for, or on nothing once +/// they have left the grid. Everything is held weakly: the cell holds +/// the menu through its gesture, and a row the grid let go of is gone. #[derive(Default)] pub(super) struct CellContext { + opened_on: RefCell>, +} + +struct OpenedOn { cell: glib::WeakRef, + row: glib::WeakRef, + rows: Vec>, +} + +/// The cell, the row it showed and the rows a chosen item acts on. +pub(super) struct MenuTarget { + pub(super) cell: GridCell, + pub(super) row: RowObject, + pub(super) rows: Vec, } impl CellContext { - fn set(&self, cell: &GridCell) { - self.cell.set(Some(cell)); + fn set(&self, cell: &GridCell, rows: &[RowObject]) { + let opened_on = cell.row().map(|row| OpenedOn { + cell: cell.downgrade(), + row: row.downgrade(), + rows: rows.iter().map(|row| row.downgrade()).collect(), + }); + self.opened_on.replace(opened_on); } - pub(super) fn cell(&self) -> Option { - self.cell.upgrade() + /// The target while the grid still shows every row of it. + pub(super) fn target(&self, column_view: >k::ColumnView) -> Option { + let opened_on = self.opened_on.borrow(); + let opened_on = opened_on.as_ref()?; + let target = MenuTarget { + cell: opened_on.cell.upgrade()?, + row: opened_on.row.upgrade()?, + rows: opened_on + .rows + .iter() + .map(|row| row.upgrade()) + .collect::>>()?, + }; + let model = column_view.model()?; + let shown: HashSet = (0..model.n_items()) + .filter_map(|position| model.item(position).and_downcast::()) + .collect(); + let all_shown = shown.contains(&target.row) && target.rows.iter().all(|row| shown.contains(row)); + all_shown.then_some(target) } } @@ -28,11 +68,12 @@ impl CellContext { /// The cell's lock follows the row bound to it, so the menu is picked /// when it opens. pub(super) fn prepare_menu(menus: &GridMenus, cell: &GridCell, column_view: >k::ColumnView) -> gtk::PopoverMenu { - menus.context.set(cell); if let Some(position) = cell.position() { select_row_for_menu(column_view, position); } - menus.actions.arm(cell, &menu_rows_of(column_view, cell)); + let rows = menu_rows_of(column_view, cell); + menus.context.set(cell, &rows); + menus.actions.arm(cell, &rows); match cell.lock() { None => menus.editable_popover.clone(), Some(_) => menus.readonly_popover.clone(), @@ -64,3 +105,109 @@ pub(super) fn attach_cell_gesture(cell: &GridCell, column_view: &glib::WeakRef()) + .find(|gesture| gesture.button() == 3) + .expect("a cell answers a right-click"); + gesture.emit_by_name::<()>("pressed", &[&1i32, &1.0f64, &1.0f64]); + } + + fn choose(grid: &Grid, actions: &[&str]) { + for action in actions { + grid.view + .activate_action(action, None) + .unwrap_or_else(|_| panic!("the menu has no {action}")); + } + } + + /// Each row-changing message the menu sent, by the rows it names. + fn acted_on(grid: &Grid) -> Vec<(&'static str, Vec>)> { + let keys = |rows: &[RowObject]| rows.iter().map(|row| row.key().cloned()).collect(); + grid.take_messages() + .into_iter() + .filter_map(|msg| match msg { + GridMsg::SetCellValue { row, .. } => Some(("set", vec![row.key().cloned()])), + GridMsg::DeleteRows { rows } => Some(("delete", keys(&rows))), + GridMsg::DuplicateRows { rows } => Some(("duplicate", keys(&rows))), + GridMsg::CopyRowsAsInsert { rows } => Some(("insert", keys(&rows))), + _ => None, + }) + .collect() + } + + fn page(ids: std::ops::Range) -> Vec { + let rows: Vec> = ids + .map(|id| vec![Value::Int(id), Value::Text(format!("row {id}"))]) + .collect(); + persisted(&rows) + } + + #[gtk4::test] + fn a_page_that_lands_under_an_open_menu_leaves_its_items_acting_on_nothing() { + let grid = two_text_rows(); + let cell = grid.cell_of(2, 1); + open_menu(&cell); + + grid.store.splice(0, grid.store.n_items(), &page(3..5)); + drain_main_context(); + choose( + &grid, + &[ + "cell.set-null", + "cell.delete-row", + "cell.duplicate-row", + "cell.copy-row-insert", + ], + ); + + assert_eq!(acted_on(&grid), Vec::new(), "the menu acted on a row of the new page"); + } + + /// The list keeps the widgets of the first row for its focus, so the + /// menu opens on a row further down, which scrolling hands on. + #[gtk4::test] + fn a_cell_scrolled_onto_another_row_leaves_the_menu_on_its_own() { + let grid = two_text_rows(); + grid.store.splice(grid.store.n_items(), 0, &page(3..300)); + let cell = drawn_cell(&grid, 6, 1); + open_menu(&cell); + + grid.view + .scroll_to(grid.store.n_items() - 1, None, gtk::ListScrollFlags::NONE, None); + wait_until(WAIT, || cell.row().is_some_and(|row| row.key() != Some(&key(6)))) + .expect("the scroll did not rebind the clicked cell"); + choose(&grid, &["cell.set-null", "cell.delete-row"]); + + assert_eq!( + acted_on(&grid), + [("set", vec![Some(key(6))]), ("delete", vec![Some(key(6))])] + ); + } + + fn drawn_cell(grid: &Grid, id: i64, column: usize) -> GridCell { + let mut drawn = None; + wait_until(WAIT, || { + drawn = grid + .cells() + .into_iter() + .find(|cell| cell.column() == column && cell.row().is_some_and(|row| row.key() == Some(&key(id)))); + drawn.is_some() + }) + .expect("the row's cell is drawn"); + drawn.expect("a drawn cell") + } +} diff --git a/linux/crates/app/src/ui/grid/grid_menus.rs b/linux/crates/app/src/ui/grid/grid_menus.rs index f0417c3f86..a0d2ea7a5c 100644 --- a/linux/crates/app/src/ui/grid/grid_menus.rs +++ b/linux/crates/app/src/ui/grid/grid_menus.rs @@ -8,8 +8,7 @@ use tablepro_core::column::ResultColumn; use super::cell_edit::begin_edit; use super::cell_editor_kind::CellEditorKind; use super::cell_preset::cell_presets; -use super::context_menu::CellContext; -use super::menu_rows::menu_rows_of; +use super::context_menu::{CellContext, MenuTarget}; use super::row_json_dialog::show_row_json_dialog; use super::selection::{export_snapshot, snapshot_result}; use super::{ @@ -198,15 +197,19 @@ pub(super) fn install_grid_context_menus(init: GridMenuInit<'_>) -> GridMenus { let group = gio::SimpleActionGroup::new(); - // The row is the one the cell shows when the item is chosen, which - // is still the clicked one after rows above it came or went. macro_rules! cell_action { - ($name:literal, |$cell:ident, $row:ident| $body:expr) => {{ + ($name:literal, |$cell:ident, $row:ident, $rows:ident| $body:expr) => {{ let ctx = context.clone(); + let view = view.clone(); gio::ActionEntry::builder($name) .activate(move |_, _, _| { - let Some($cell) = ctx.cell() else { return }; - let Some($row) = $cell.row() else { return }; + let Some(cv) = view.upgrade() else { return }; + let Some(target) = ctx.target(&cv) else { return }; + let MenuTarget { + cell: $cell, + row: $row, + rows: $rows, + } = target; let $cell = &$cell; $body; }) @@ -222,17 +225,8 @@ pub(super) fn install_grid_context_menus(init: GridMenuInit<'_>) -> GridMenus { )), } } - // The rows each item acts on are read when it is chosen, from the - // row the cell shows then. macro_rules! rows_action { - ($name:literal, |$cell:ident, $rows:ident| $body:expr) => {{ - let view = view.clone(); - cell_action!($name, |$cell, _row| { - let Some(cv) = view.upgrade() else { return }; - let $rows = menu_rows_of(&cv, $cell); - $body; - }) - }}; + ($name:literal, |$cell:ident, $rows:ident| $body:expr) => {{ cell_action!($name, |$cell, _row, $rows| $body) }}; } macro_rules! copy_action { ($name:literal, |$rows:ident| $message:expr) => {{ @@ -247,11 +241,14 @@ pub(super) fn install_grid_context_menus(init: GridMenuInit<'_>) -> GridMenus { macro_rules! send_action { ($name:literal, |$cell:ident, $row:ident| $msg:expr) => {{ let s = sink.clone(); - cell_action!($name, |$cell, $row| s($msg)) + cell_action!($name, |$cell, $row, _rows| s($msg)) }}; } - let edit_action = cell_action!("edit", |cell, _row| begin_edit(cell)); + // Editing opens on the cell, which must still show the row. + let edit_action = cell_action!("edit", |cell, row, _rows| if cell.row().as_ref() == Some(&row) { + begin_edit(cell) + }); // Plain Copy reads the cell's value, never the widget's text: the // widget carries the display form, which is the `` sentinel, // the `(auto)` placeholder, `` for a blob and a value cut @@ -353,7 +350,7 @@ pub(super) fn install_grid_context_menus(init: GridMenuInit<'_>) -> GridMenus { let cols = columns.clone(); let view = view.clone(); let tab = tab.clone(); - cell_action!("show-row-json", |_cell, row| { + cell_action!("show-row-json", |_cell, row, _rows| { let Some(cv) = view.upgrade() else { return }; let text = tablepro_core::export::row_to_json(&cols, &tab.snapshot(&row).to_values()); show_row_json_dialog(&cv, text); diff --git a/linux/crates/app/src/ui/grid/menu_rows.rs b/linux/crates/app/src/ui/grid/menu_rows.rs index 6fc794b514..8175e8bd3b 100644 --- a/linux/crates/app/src/ui/grid/menu_rows.rs +++ b/linux/crates/app/src/ui/grid/menu_rows.rs @@ -19,8 +19,7 @@ pub fn menu_rows(selection: >k::MultiSelection, clicked: &RowObject, clicked_p .collect() } -/// The rows a menu opened on `cell` acts on, from the row the cell -/// shows when the item is chosen. +/// The rows a menu opened on `cell` acts on, read as it opens. pub(super) fn menu_rows_of(column_view: >k::ColumnView, cell: &GridCell) -> Vec { let Some(row) = cell.row() else { return Vec::new(); From 17706b52ae4f774be99aa26a1890014041c01a8c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 14 Oct 2026 16:39:17 +0700 Subject: [PATCH 4/7] fix(linux): restore sessions for a launch during the quit's last write --- linux/crates/app/src/application/imp.rs | 11 +++++ .../app/src/application/quit_sequence.rs | 15 ++++-- linux/crates/app/src/application/tests.rs | 46 +++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/linux/crates/app/src/application/imp.rs b/linux/crates/app/src/application/imp.rs index c3461360bf..f7c311add8 100644 --- a/linux/crates/app/src/application/imp.rs +++ b/linux/crates/app/src/application/imp.rs @@ -39,6 +39,12 @@ pub(super) enum RestoreState { /// The record is being read, and the windows open once it is. Loading, Done, + /// An approved quit is writing the record. A launch meanwhile asks + /// for it to be restored once written: read earlier, it would race + /// the write, and the quit then empties the list it restored into. + Quitting { + relaunched: bool, + }, } #[glib::object_subclass] @@ -89,6 +95,7 @@ impl ApplicationImpl for TableProApplication { match self.restore.get() { RestoreState::NotAttempted => app.restore_session(), RestoreState::Loading => {} + RestoreState::Quitting { .. } => self.restore.set(RestoreState::Quitting { relaunched: true }), RestoreState::Done => { app.open_window(WindowLaunch::Picker); } @@ -108,6 +115,10 @@ impl ApplicationImpl for TableProApplication { self.obj().restore_session(); } RestoreState::Loading => self.pending_files.borrow_mut().extend_from_slice(files), + RestoreState::Quitting { .. } => { + self.pending_files.borrow_mut().extend_from_slice(files); + self.restore.set(RestoreState::Quitting { relaunched: true }); + } } } diff --git a/linux/crates/app/src/application/quit_sequence.rs b/linux/crates/app/src/application/quit_sequence.rs index 9d9fe83f38..ba94cb2a9a 100644 --- a/linux/crates/app/src/application/quit_sequence.rs +++ b/linux/crates/app/src/application/quit_sequence.rs @@ -23,7 +23,8 @@ use crate::workspace::{CloseDecision, CloseReason}; /// /// An approved quit can leave the process running with no window while /// a session closes, and a second launch then activates it. That -/// activate restores the record as a new process would. +/// activate restores the record as a new process would, after the +/// record is written if it lands while the quit writes it. pub struct QuitSequence; impl QuitSequence { @@ -49,11 +50,19 @@ impl QuitSequence { if let Some(services) = &services { match decision { CloseDecision::Approved => { + let restore = &app.imp().restore; + let restorable = restore.get() == RestoreState::Done; + if restorable { + restore.set(RestoreState::Quitting { relaunched: false }); + } services.open_sessions().flush().await; services.open_sessions().end_quit(); - let restore = &app.imp().restore; - if restore.get() == RestoreState::Done { + if restorable { + let relaunched = restore.get() == RestoreState::Quitting { relaunched: true }; restore.set(RestoreState::NotAttempted); + if relaunched { + app.restore_session(); + } } } CloseDecision::Cancelled => services.open_sessions().thaw(|key| { diff --git a/linux/crates/app/src/application/tests.rs b/linux/crates/app/src/application/tests.rs index 518ae82df7..9d98d38b95 100644 --- a/linux/crates/app/src/application/tests.rs +++ b/linux/crates/app/src/application/tests.rs @@ -480,6 +480,52 @@ fn an_activate_after_an_approved_quit_restores_its_record() { drop(closing); } +/// A launch that lands while an approved quit writes the record waits +/// for the write, then reopens what it wrote, once. +#[gtk4::test] +fn an_activate_during_the_quit_flush_restores_the_record_once_written() { + let test = TestApp::started("QuitFlushActivate"); + let (first, second) = (test.database("first.db"), test.database("second.db")); + test.record(&[first.clone(), second.clone()]); + test.app.activate(); + test.wait_for_sessions(&[first.clone(), second.clone()]); + let services = test.services(); + let closing = services.hold().acquire(); + let decision = Rc::new(std::cell::RefCell::new(None)); + let (app, slot) = (test.app.clone(), decision.clone()); + glib::spawn_future_local(async move { + slot.replace(Some(QuitSequence::run(app).await)); + }); + + let mut activated_during_flush = false; + wait_until(WAIT, || { + let flushing = + decision.borrow().is_none() && test.app.windows().is_empty() && test.app.imp().quit_asking.get().is_none(); + if flushing && !activated_during_flush { + test.app.activate(); + test.app.activate(); + activated_during_flush = true; + } + decision.borrow().is_some() + }) + .expect("the quit to finish"); + + assert!( + activated_during_flush, + "the quit ended before a launch could land in its flush" + ); + assert_eq!(*decision.borrow(), Some(CloseDecision::Approved)); + test.wait_for_sessions(&[first.clone(), second.clone()]); + let _ = wait_until(Duration::from_millis(300), || test.app.windows().len() > 2); + assert_eq!( + test.app.windows().len(), + 2, + "a picker opened beside the restored windows" + ); + assert_eq!(services.open_sessions().keys(), [first, second]); + drop(closing); +} + #[gtk4::test] fn a_cancelled_quit_forgets_a_restore_that_failed_during_it() { let test = TestApp::started("QuitFailedRestore"); From ea7922cc27913adf1270feb2685e27507999832b Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 14 Oct 2026 16:52:09 +0700 Subject: [PATCH 5/7] fix(linux-ssh): replace a changed host key only for the session's hosts --- .../crates/app/src/ui/session_window/tests.rs | 17 +- .../app/src/ui/workspace/health_banner.rs | 151 ++++++++++++-- .../session/src/health/health_monitor.rs | 15 +- linux/crates/session/src/health/mod.rs | 2 + .../session/src/health/reconnect_plan.rs | 12 +- .../session/src/health/reviewed_host_key.rs | 7 + .../session/src/health/trusted_host_key.rs | 63 ++++-- linux/crates/session/src/lib.rs | 2 +- linux/crates/session/src/session/session.rs | 33 ++- linux/crates/ssh/src/askpass_bridge.rs | 11 +- linux/crates/ssh/src/known_hosts.rs | 188 +++++++++++++++++- linux/crates/ssh/src/lib.rs | 2 +- linux/crates/ssh/src/stderr_classify.rs | 14 +- 13 files changed, 454 insertions(+), 63 deletions(-) create mode 100644 linux/crates/session/src/health/reviewed_host_key.rs diff --git a/linux/crates/app/src/ui/session_window/tests.rs b/linux/crates/app/src/ui/session_window/tests.rs index 6850172c0f..ad4270658c 100644 --- a/linux/crates/app/src/ui/session_window/tests.rs +++ b/linux/crates/app/src/ui/session_window/tests.rs @@ -46,6 +46,9 @@ pub(crate) struct Script { /// The saved connection logs in to a server with a password, rather /// than opening a file. pub network: bool, + /// The saved connection reaches its server through this SSH tunnel, + /// logging in to the database as the system user. + pub tunnel: Option, /// The connection list on disk cannot be read, so it holds nothing. pub unreadable_list: bool, } @@ -93,12 +96,18 @@ impl TestWindow { credentials, ssh, network, + tunnel, unreadable_list, } = script; - let saved = if network { - network_connection() - } else { - saved_connection(&dir.path().join("fake.db")) + let network = network || tunnel.is_some(); + let saved = match tunnel { + Some(tunnel) => SavedConnection { + ssh: Some(tunnel), + auth_mode: AuthMode::Kerberos, + ..network_connection() + }, + None if network => network_connection(), + None => saved_connection(&dir.path().join("fake.db")), }; if unreadable_list { if let Some(parent) = list_file.parent() { diff --git a/linux/crates/app/src/ui/workspace/health_banner.rs b/linux/crates/app/src/ui/workspace/health_banner.rs index 61c5eb2325..d419413f3b 100644 --- a/linux/crates/app/src/ui/workspace/health_banner.rs +++ b/linux/crates/app/src/ui/workspace/health_banner.rs @@ -2,7 +2,8 @@ use gtk4::prelude::*; use tablepro_core::{DriverError, SshFailure, TransportError}; -use tablepro_session::{FailureCause, MonitorSignal, SessionHealth}; +use tablepro_session::{FailureCause, MonitorSignal, ReviewedHostKey, SessionHealth}; +use tablepro_ssh::ForgetHostKeyError; use super::{WorkspaceInput, WorkspaceModel}; use crate::runtime::{DeliverInput, spawn_and_deliver}; @@ -114,7 +115,10 @@ impl WorkspaceModel { }; match failure.fingerprint.is_empty() { true => session.signal(MonitorSignal::RetryNow), - false => session.retry_trusting_host_key(failure.fingerprint.clone()), + false => session.retry_trusting_host_key(ReviewedHostKey { + host: failure.host.clone(), + fingerprint: failure.fingerprint.clone(), + }), } } @@ -125,8 +129,9 @@ impl WorkspaceModel { } /// ssh never accepts a changed key, so the old one leaves known_hosts - /// first, with the ssh-keygen command ssh itself suggests. Returns - /// whether ssh-keygen was started. + /// first, with the ssh-keygen command ssh itself suggests, once the + /// session's own tunnel confirms the host and the file. Returns + /// whether that was started. fn forget_old_host_key(&self, failure: HostKeyFailure) -> bool { let HostKeyFailureKind::Changed { known_hosts, .. } = &failure.kind else { return false; @@ -135,11 +140,20 @@ impl WorkspaceModel { tracing::warn!("no SSH services to remove the old host key with"); return false; }; + let Some(config) = self + .context + .active + .get() + .and_then(|session| session.ssh_config().cloned()) + else { + tracing::warn!("the session has no SSH tunnel to check the changed host key against"); + return false; + }; let (host, known_hosts) = (failure.host.clone(), known_hosts.clone()); let input = self.input.clone(); drop(spawn_and_deliver( &self.tasks, - async move { tablepro_ssh::forget_known_host(&ssh, &host, &known_hosts).await }, + async move { tablepro_ssh::forget_changed_host_key(&ssh, &config, &host, &known_hosts).await }, move |forgotten| { let forgotten = match forgotten { Ok(Ok(())) => Ok(()), @@ -153,10 +167,13 @@ impl WorkspaceModel { } } -fn forget_failure_detail(failure: &SshFailure) -> String { - match failure { - SshFailure::ConfigRejected { detail } => detail.clone(), - other => crate::ui::error_text::driver_message(&DriverError::Transport(TransportError::Ssh(other.clone()))), +fn forget_failure_detail(error: &ForgetHostKeyError) -> String { + match error { + ForgetHostKeyError::NotThisConnection { .. } => crate::i18n::gettext("it is not a key this connection uses"), + ForgetHostKeyError::Ssh(SshFailure::ConfigRejected { detail }) => detail.clone(), + ForgetHostKeyError::Ssh(other) => { + crate::ui::error_text::driver_message(&DriverError::Transport(TransportError::Ssh(other.clone()))) + } } } @@ -232,6 +249,7 @@ mod tests { use tablepro_session::ConnectError; use tablepro_session::fakes::FakeEstablish; use tablepro_ssh::{SshRuntimeCell, SshServices}; + use tablepro_storage::{SavedSshAuth, SavedSshConfig}; use tablepro_test_support::{ConnectionScript, FakeReply}; use super::*; @@ -324,6 +342,15 @@ mod tests { /// A window whose connection dropped and whose first reconnect /// parked it; `reconnects` answer that one and the ones after. fn parked(name: &str, reconnects: Vec, ssh: Option) -> TestWindow { + parked_with(name, reconnects, ssh, None) + } + + fn parked_with( + name: &str, + reconnects: Vec, + ssh: Option, + tunnel: Option, + ) -> TestWindow { let mut establish = vec![FakeEstablish::Connect(healthy())]; establish.extend(reconnects); let test = TestWindow::with_script( @@ -331,6 +358,7 @@ mod tests { Script { establish, ssh, + tunnel, ..Script::default() }, ); @@ -489,9 +517,13 @@ mod tests { } fn changed_key(known_hosts: &Path) -> FakeEstablish { + changed_key_of("[bastion]:2222", known_hosts) + } + + fn changed_key_of(host: &str, known_hosts: &Path) -> FakeEstablish { FakeEstablish::Fail(ConnectError::Driver(DriverError::Transport(TransportError::Ssh( SshFailure::HostKeyChanged { - host: "[bastion]:2222".to_owned(), + host: host.to_owned(), algorithm: "ED25519".to_owned(), sha256_fingerprint: FINGERPRINT.to_owned(), known_hosts: known_hosts.to_owned(), @@ -500,15 +532,55 @@ mod tests { )))) } + /// The tunnel the parked sessions go through. The server is on this + /// machine, so no reachability check runs. + fn tunnel() -> SavedSshConfig { + SavedSshConfig { + host: "127.0.0.1".to_owned(), + port: Some(2222), + username: None, + jump_hosts: Vec::new(), + auth: SavedSshAuth::Agent, + } + } + + /// SSH services whose `ssh -G` resolves the tunnel's host to + /// `[bastion]:2222`, looked up in `known_hosts`, the way an + /// ssh_config with `HostName bastion` would. + fn resolving_ssh(dir: &Path, known_hosts: &Path) -> SshServices { + use std::os::unix::fs::PermissionsExt; + + let program = dir.join("ssh"); + let resolved = format!( + "hostname bastion\\nport 2222\\nhostkeyalias none\\nuserknownhostsfile {} {}\\n", + known_hosts.display(), + dir.join("known_hosts2").display() + ); + std::fs::write(&program, format!("#!/bin/sh\nprintf '{resolved}'\n")).unwrap(); + std::fs::set_permissions(&program, std::fs::Permissions::from_mode(0o755)).unwrap(); + SshServices { + ssh_program: program, + ..ssh_services(dir) + } + } + + fn has_toast(test: &TestWindow, text: &str) -> bool { + descendants(&test.workspace()) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .any(|label| label.label().contains(text)) + } + #[gtk4::test] fn replace_removes_the_old_key_and_retries_trusting_the_new_one() { let dir = tempfile::tempdir().unwrap(); let known_hosts = dir.path().join("known_hosts"); std::fs::write(&known_hosts, format!("[bastion]:2222 {KEY}\nother {KEY}\n")).unwrap(); - let test = parked( + let test = parked_with( "ReviewReplace", vec![changed_key(&known_hosts), asks("[bastion]:2222", FINGERPRINT)], - Some(ssh_services(dir.path())), + Some(resolving_ssh(dir.path(), &known_hosts)), + Some(tunnel()), ); let banner = wait_for_button(&test, "Review Key…"); assert!(banner.title().contains("does not match"), "{}", banner.title()); @@ -537,12 +609,13 @@ mod tests { std::fs::set_permissions(&slow_keygen, std::fs::Permissions::from_mode(0o755)).unwrap(); let ssh = SshServices { ssh_keygen_program: slow_keygen, - ..ssh_services(dir.path()) + ..resolving_ssh(dir.path(), &known_hosts) }; - let test = parked( + let test = parked_with( "ReviewReplaceBusy", vec![changed_key(&known_hosts), asks("[bastion]:2222", FINGERPRINT)], Some(ssh), + Some(tunnel()), ); wait_for_button(&test, "Review Key…"); review_and_answer(&test, "replace"); @@ -559,4 +632,54 @@ mod tests { ); wait_until(WAIT, || health(&test) == SessionHealth::Online).expect("the session back online"); } + + /// ssh's stderr carries the server's login banner too, so a changed + /// key it reports for a host or a file this connection does not use + /// stays where it is. + #[gtk4::test] + fn replace_leaves_a_key_this_connection_does_not_use() { + for (name, host, file) in [ + ("ReviewForgedHost", "other", "known_hosts"), + ("ReviewForgedFile", "[bastion]:2222", "elsewhere"), + ] { + let dir = tempfile::tempdir().unwrap(); + let known_hosts = dir.path().join("known_hosts"); + let reported = dir.path().join(file); + let keys = format!("[bastion]:2222 {KEY}\nother {KEY}\n"); + for path in [&known_hosts, &dir.path().join("elsewhere")] { + std::fs::write(path, &keys).unwrap(); + } + let test = parked_with( + name, + vec![changed_key_of(host, &reported)], + Some(resolving_ssh(dir.path(), &known_hosts)), + Some(tunnel()), + ); + wait_for_button(&test, "Review Key…"); + + review_and_answer(&test, "replace"); + + wait_until(WAIT, || has_toast(&test, "it is not a key this connection uses")) + .unwrap_or_else(|_| panic!("{name}: the forged report was not refused")); + assert_eq!(std::fs::read_to_string(&reported).unwrap(), keys, "{name}"); + assert_eq!(test.connector.calls().len(), 2, "{name}: a refused replace retried"); + } + } + + #[gtk4::test] + fn trust_answers_only_for_the_host_it_was_reviewed_for() { + let test = parked( + "ReviewTrustHost", + vec![asks("bastion", FINGERPRINT), asks("jump", FINGERPRINT)], + None, + ); + wait_for_button(&test, "Review Key…"); + + review_and_answer(&test, "trust"); + + wait_until(WAIT, || alert(&test).is_some()).expect("ssh's question about another host to reach the user"); + if let Some(question) = alert(&test) { + question.force_close(); + } + } } diff --git a/linux/crates/session/src/health/health_monitor.rs b/linux/crates/session/src/health/health_monitor.rs index 0146f92cfa..4ecf3a4510 100644 --- a/linux/crates/session/src/health/health_monitor.rs +++ b/linux/crates/session/src/health/health_monitor.rs @@ -9,7 +9,7 @@ use tokio_util::sync::CancellationToken; use super::{ FailureCause, HealthPolicy, LinkGeneration, MonitorAction, MonitorMailbox, MonitorSignal, PendingSignals, - ProbeOutcome, ReachabilityTarget, ReconnectPlan, ReconnectResult, SessionHealth, + ProbeOutcome, ReachabilityTarget, ReconnectPlan, ReconnectResult, ReviewedHostKey, SessionHealth, }; use crate::connect::{ConnectError, PendingSession}; use crate::link::{ConnectionLease, ConnectionLink}; @@ -33,7 +33,7 @@ pub(crate) struct MonitorContext { /// The error that parked the session, for the user to review. pub last_rejection: Arc>>>, /// A host key the user reviewed and trusted for their next retry. - pub trusted_host_key: Arc>>, + pub trusted_host_key: Arc>>, } enum Wakeup { @@ -565,6 +565,13 @@ mod tests { assert_eq!(*started.session.health().borrow(), SessionHealth::Online); } + fn reviewed(fingerprint: &str) -> ReviewedHostKey { + ReviewedHostKey { + host: "bastion".to_owned(), + fingerprint: fingerprint.to_owned(), + } + } + fn ask_host_key(fingerprint: &str) -> FakeEstablish { FakeEstablish::AskHostKey { host: "bastion".to_owned(), @@ -618,7 +625,7 @@ mod tests { settle().await; assert_eq!(*started.session.health().borrow(), parked_on_unknown_host_key()); - started.session.retry_trusting_host_key("SHA256:new".to_owned()); + started.session.retry_trusting_host_key(reviewed("SHA256:new")); settle().await; assert_eq!(*started.session.health().borrow(), SessionHealth::Online); @@ -638,7 +645,7 @@ mod tests { started.session.signal(lost()); settle().await; - started.session.retry_trusting_host_key("SHA256:new".to_owned()); + started.session.retry_trusting_host_key(reviewed("SHA256:new")); settle().await; assert_eq!(prompter.requests().len(), 1, "a key nobody reviewed was trusted"); diff --git a/linux/crates/session/src/health/mod.rs b/linux/crates/session/src/health/mod.rs index 9188d17f74..32ceed8c24 100644 --- a/linux/crates/session/src/health/mod.rs +++ b/linux/crates/session/src/health/mod.rs @@ -16,6 +16,7 @@ mod reachability; mod reachability_target; mod reconnect_plan; mod reconnect_result; +mod reviewed_host_key; mod session_health; mod trusted_host_key; @@ -32,5 +33,6 @@ pub use reachability::Reachability; pub use reachability_target::ReachabilityTarget; pub use reconnect_plan::ReconnectPlan; pub use reconnect_result::ReconnectResult; +pub use reviewed_host_key::ReviewedHostKey; pub use session_health::SessionHealth; pub(crate) use trusted_host_key::TrustedHostKey; diff --git a/linux/crates/session/src/health/reconnect_plan.rs b/linux/crates/session/src/health/reconnect_plan.rs index f618205739..fcc62a84c7 100644 --- a/linux/crates/session/src/health/reconnect_plan.rs +++ b/linux/crates/session/src/health/reconnect_plan.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use tablepro_core::credentials::{CredentialInteraction, CredentialPrompter}; -use super::TrustedHostKey; +use super::{ReviewedHostKey, TrustedHostKey}; use crate::connect::{ConnectRequest, Connector, CredentialSource}; /// How the monitor reconnects: the same request, with its secrets read @@ -18,12 +18,12 @@ pub struct ReconnectPlan { } impl ReconnectPlan { - /// `trusted` is a host key fingerprint the user reviewed before - /// asking for this attempt, answered for them when ssh shows it. - pub(crate) fn interaction(&self, user_asked: bool, trusted: Option) -> CredentialInteraction { + /// `trusted` is a host key the user reviewed before asking for this + /// attempt, answered for them when ssh shows it for that host. + pub(crate) fn interaction(&self, user_asked: bool, trusted: Option) -> CredentialInteraction { match (user_asked, trusted, &self.prompter) { - (true, Some(fingerprint), prompter) => { - CredentialInteraction::Attended(Arc::new(TrustedHostKey::new(fingerprint, prompter.clone()))) + (true, Some(reviewed), prompter) => { + CredentialInteraction::Attended(Arc::new(TrustedHostKey::new(reviewed, prompter.clone()))) } (true, None, Some(prompter)) => CredentialInteraction::Attended(prompter.clone()), _ => CredentialInteraction::Unattended, diff --git a/linux/crates/session/src/health/reviewed_host_key.rs b/linux/crates/session/src/health/reviewed_host_key.rs new file mode 100644 index 0000000000..3212dd2258 --- /dev/null +++ b/linux/crates/session/src/health/reviewed_host_key.rs @@ -0,0 +1,7 @@ +/// A host key the user reviewed from a parked session and chose to +/// trust: the host ssh named and the key it showed for it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ReviewedHostKey { + pub host: String, + pub fingerprint: String, +} diff --git a/linux/crates/session/src/health/trusted_host_key.rs b/linux/crates/session/src/health/trusted_host_key.rs index 3a79e4eb04..a6c8e09e45 100644 --- a/linux/crates/session/src/health/trusted_host_key.rs +++ b/linux/crates/session/src/health/trusted_host_key.rs @@ -5,33 +5,40 @@ use futures::FutureExt; use futures::future::BoxFuture; use tablepro_core::credentials::{CredentialPrompt, CredentialPrompter, PromptPurpose, PromptReply}; +use super::ReviewedHostKey; + /// The user's answer to ssh's host key question, given before ssh asks. /// -/// The user reviewed this fingerprint from a parked session and chose to -/// trust it, so the retry they started should not ask them again. Only -/// that key is answered, and only once: ssh showing any other key, or -/// asking anything else, reaches the user as on any connect. ssh still -/// makes the decision and writes known_hosts itself. +/// The user reviewed this key from a parked session and chose to trust +/// it, so the retry they started should not ask them again. Only that +/// key for that host is answered, and only once: ssh showing any other +/// key, the same key for another host, or asking anything else, reaches +/// the user as on any connect. ssh still makes the decision and writes +/// known_hosts itself. pub(crate) struct TrustedHostKey { - fingerprint: String, + reviewed: ReviewedHostKey, prompter: Option>, answered: AtomicBool, } impl TrustedHostKey { - pub(crate) fn new(fingerprint: String, prompter: Option>) -> Self { + pub(crate) fn new(reviewed: ReviewedHostKey, prompter: Option>) -> Self { Self { - fingerprint, + reviewed, prompter, answered: AtomicBool::new(false), } } fn answers(&self, request: &CredentialPrompt) -> bool { - let PromptPurpose::SshHostKeyConfirmation { fingerprint, .. } = &request.purpose else { + let PromptPurpose::SshHostKeyConfirmation { host, fingerprint, .. } = &request.purpose else { return false; }; - !fingerprint.is_empty() && *fingerprint == self.fingerprint && !self.answered.swap(true, Ordering::AcqRel) + let reviewed = &self.reviewed; + !fingerprint.is_empty() + && *fingerprint == reviewed.fingerprint + && host.eq_ignore_ascii_case(&reviewed.host) + && !self.answered.swap(true, Ordering::AcqRel) } } @@ -58,9 +65,13 @@ mod tests { use super::*; fn host_key(fingerprint: &str) -> CredentialPrompt { + host_key_of("bastion", fingerprint) + } + + fn host_key_of(host: &str, fingerprint: &str) -> CredentialPrompt { CredentialPrompt { purpose: PromptPurpose::SshHostKeyConfirmation { - host: "bastion".to_owned(), + host: host.to_owned(), algorithm: "ED25519".to_owned(), fingerprint: fingerprint.to_owned(), }, @@ -77,7 +88,7 @@ mod tests { #[tokio::test] async fn the_reviewed_key_is_answered_once_and_anything_else_asks() { let user = Arc::new(FakePrompter::new([])); - let trusted = TrustedHostKey::new("SHA256:reviewed".to_owned(), Some(user.clone())); + let trusted = TrustedHostKey::new(reviewed("bastion", "SHA256:reviewed"), Some(user.clone())); let other = trusted.prompt(host_key("SHA256:other")).await; let reviewed = trusted.prompt(host_key("SHA256:reviewed")).await; @@ -91,8 +102,34 @@ mod tests { #[tokio::test] async fn an_empty_fingerprint_is_never_answered() { - let trusted = TrustedHostKey::new(String::new(), None); + let trusted = TrustedHostKey::new(reviewed("bastion", ""), None); assert!(!accepted(&trusted.prompt(host_key("")).await)); } + + #[tokio::test] + async fn a_key_trusted_for_one_host_does_not_answer_for_another() { + let user = Arc::new(FakePrompter::new([])); + let trusted = TrustedHostKey::new(reviewed("[db.example.com]:2222", "SHA256:reviewed"), Some(user.clone())); + + let other = trusted.prompt(host_key_of("jump.example.com", "SHA256:reviewed")).await; + let reviewed_host = trusted + .prompt(host_key_of("[DB.example.com]:2222", "SHA256:reviewed")) + .await; + + assert!(!accepted(&other), "a key trusted for one host was trusted for another"); + assert!(accepted(&reviewed_host)); + assert_eq!( + user.requests().len(), + 1, + "the other host's question did not reach the user" + ); + } + + fn reviewed(host: &str, fingerprint: &str) -> ReviewedHostKey { + ReviewedHostKey { + host: host.to_owned(), + fingerprint: fingerprint.to_owned(), + } + } } diff --git a/linux/crates/session/src/lib.rs b/linux/crates/session/src/lib.rs index d2e27f8ba1..67fcb3f2e1 100644 --- a/linux/crates/session/src/lib.rs +++ b/linux/crates/session/src/lib.rs @@ -23,7 +23,7 @@ pub use connect::{ ConnectionMetadata, Connector, CredentialSource, DriverConnector, NoStoredCredentials, PendingSave, PendingSession, PersistReport, RefreshedRequest, RequestTarget, persist_pending_save, resolve_saved, }; -pub use health::{FailureCause, LinkGeneration, MonitorSignal, ReachabilityTarget, SessionHealth}; +pub use health::{FailureCause, LinkGeneration, MonitorSignal, ReachabilityTarget, ReviewedHostKey, SessionHealth}; pub use link::{CloseReport, ConnectionLease, EstablishedConnection}; pub use operation::{DeadlinePolicy, DefaultDeadlines, OperationError, OperationKind, OperationRunner}; pub use registry::{ClaimError, RegistryEvent, SessionClaim, SessionKey, SessionRegistry}; diff --git a/linux/crates/session/src/session/session.rs b/linux/crates/session/src/session/session.rs index d6dfed2244..1f3cec1bde 100644 --- a/linux/crates/session/src/session/session.rs +++ b/linux/crates/session/src/session/session.rs @@ -11,10 +11,10 @@ use tokio_util::task::AbortOnDropHandle; use uuid::Uuid; use super::{ObjectListSnapshot, SchemaCatalog, SessionDeps}; -use crate::connect::{ConnectError, ConnectionMetadata, PendingSession}; +use crate::connect::{ConnectError, ConnectionMetadata, PendingSession, RequestTarget}; use crate::health::{ - LinkGeneration, MonitorContext, MonitorMailbox, MonitorSignal, ReachabilityTarget, ReconnectPlan, SessionHealth, - run_monitor, + LinkGeneration, MonitorContext, MonitorMailbox, MonitorSignal, ReachabilityTarget, ReconnectPlan, ReviewedHostKey, + SessionHealth, run_monitor, }; use crate::link::{CloseReport, ConnectionLease, ConnectionLink}; use crate::operation::{OperationError, OperationKind, OperationRunner}; @@ -31,7 +31,10 @@ pub struct Session { workspace_settings: WorkspaceSettings, reachability: Arc>, last_rejection: Arc>>>, - trusted_host_key: Arc>>, + trusted_host_key: Arc>>, + /// The tunnel the connection goes through, which a changed host key + /// is checked against before its old key is removed. + ssh: Option, runner: OperationRunner, mailbox: Arc, health: watch::Receiver, @@ -94,12 +97,17 @@ impl Session { trusted_host_key: trusted_host_key.clone(), }; let monitor = deps.tasks.spawn_owned(run_monitor(context)); + let ssh = match &parts.request.target { + RequestTarget::Network { ssh, .. } => ssh.clone(), + RequestTarget::File(_) => None, + }; Arc::new(Session { metadata: parts.request.metadata, workspace_settings: parts.request.workspace_settings, reachability, last_rejection, trusted_host_key, + ssh, runner, mailbox, health, @@ -209,15 +217,20 @@ impl Session { .clone() } - /// Retry, answering ssh's question about the host key `fingerprint` - /// for the user, who reviewed it. A different key, or any other - /// question, still reaches them; ssh records the key in known_hosts - /// as it does when the user answers. - pub fn retry_trusting_host_key(&self, fingerprint: String) { - *self.trusted_host_key.lock().unwrap_or_else(PoisonError::into_inner) = Some(fingerprint); + /// Retry, answering ssh's question about the `reviewed` host key + /// for the user, who reviewed it. A different key or host, or any + /// other question, still reaches them; ssh records the key in + /// known_hosts as it does when the user answers. + pub fn retry_trusting_host_key(&self, reviewed: ReviewedHostKey) { + *self.trusted_host_key.lock().unwrap_or_else(PoisonError::into_inner) = Some(reviewed); self.mailbox.post(MonitorSignal::RetryNow); } + /// The SSH tunnel the connection was made through, if any. + pub fn ssh_config(&self) -> Option<&tablepro_ssh::SshConfig> { + self.ssh.as_ref() + } + pub fn runner(&self) -> &OperationRunner { &self.runner } diff --git a/linux/crates/ssh/src/askpass_bridge.rs b/linux/crates/ssh/src/askpass_bridge.rs index 3955c6b6e4..72905534dc 100644 --- a/linux/crates/ssh/src/askpass_bridge.rs +++ b/linux/crates/ssh/src/askpass_bridge.rs @@ -138,10 +138,13 @@ impl AskpassBridge { fn record_declined(&mut self, prompt: &AskpassPrompt) { if let AskpassPrompt::HostKeyConfirmation { - algorithm, fingerprint, .. + host, + algorithm, + fingerprint, } = prompt { self.declined_host_key = Some(DeclinedHostKey { + host: host.clone(), algorithm: algorithm.clone(), fingerprint: fingerprint.clone(), }); @@ -236,8 +239,10 @@ mod tests { assert_eq!(exchange(&mut bridge, "", "deploy@bastion's password: ").await, [CANCEL]); assert_eq!(exchange(&mut bridge, "", HOST_KEY_PROMPT).await, [CANCEL]); assert_eq!( - bridge.declined_host_key().map(|key| key.fingerprint.as_str()), - Some("SHA256:abc") + bridge + .declined_host_key() + .map(|key| (key.host.as_str(), key.fingerprint.as_str())), + Some(("bastion", "SHA256:abc")) ); } diff --git a/linux/crates/ssh/src/known_hosts.rs b/linux/crates/ssh/src/known_hosts.rs index 96e4f44a77..0e5fe83a2d 100644 --- a/linux/crates/ssh/src/known_hosts.rs +++ b/linux/crates/ssh/src/known_hosts.rs @@ -1,13 +1,25 @@ use std::ffi::OsString; use std::io; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Stdio; use tablepro_core::SshFailure; use tokio::process::Command; +use crate::route::{lookup, trace_route}; use crate::{SshConfig, SshServices}; +/// Why a key ssh reported as changed was left in known_hosts. +#[derive(Debug, thiserror::Error)] +pub enum ForgetHostKeyError { + /// `ssh -G` names no host of this connection by `host` that reads + /// `known_hosts`, so the report did not come from ssh's own check. + #[error("{host} in {} is not a host key this connection uses", .known_hosts.display())] + NotThisConnection { host: String, known_hosts: PathBuf }, + #[error(transparent)] + Ssh(#[from] SshFailure), +} + pub async fn forget_host_key(services: &SshServices, config: &SshConfig, known_hosts: &Path) -> Result<(), SshFailure> { let mut args: Vec = vec!["-G".into()]; if let Some(config_file) = &services.config_file { @@ -32,10 +44,49 @@ pub async fn forget_host_key(services: &SshServices, config: &SshConfig, known_h forget_known_host(services, &lookup, known_hosts).await } +/// Remove the old key of a host whose key ssh reported as changed. +/// +/// The report is read from ssh's stderr, where a server's login banner +/// is printed too, so `host` and `known_hosts` are taken only when +/// `ssh -G` resolves the connection's destination or one of its jump +/// hosts to `host`, and lists `known_hosts` among the UserKnownHostsFile +/// entries for it. +pub async fn forget_changed_host_key( + services: &SshServices, + config: &SshConfig, + host: &str, + known_hosts: &Path, +) -> Result<(), ForgetHostKeyError> { + let program = &services.ssh_program; + let config_file = services.config_file.as_deref(); + let resolved = lookup(program, config_file, &config.destination, &config.jump_hosts).await; + let route = trace_route(program, config_file, resolved.as_deref()).await; + let mut hosts: Vec = resolved.into_iter().collect(); + for hop in &route.hops { + hosts.extend(lookup(program, config_file, &hop.destination, &[]).await); + } + let ours = hosts.iter().find_map(|resolved| { + let name = known_hosts_lookup(resolved)?; + let reads = user_known_hosts_files(resolved).iter().any(|file| file == known_hosts); + (reads && name.eq_ignore_ascii_case(host)).then_some(name) + }); + let Some(name) = ours else { + return Err(ForgetHostKeyError::NotThisConnection { + host: host.to_owned(), + known_hosts: known_hosts.to_owned(), + }); + }; + Ok(forget_known_host(services, &name, known_hosts).await?) +} + /// Remove every key `known_hosts` holds for `lookup`, the name ssh looks /// the host up by: `host`, `[host]:port` or a HostKeyAlias, as ssh /// prints it when a key changes. -pub async fn forget_known_host(services: &SshServices, lookup: &str, known_hosts: &Path) -> Result<(), SshFailure> { +pub(crate) async fn forget_known_host( + services: &SshServices, + lookup: &str, + known_hosts: &Path, +) -> Result<(), SshFailure> { if lookup.trim().is_empty() { return Err(SshFailure::InvalidDestination { detail: "no host to remove from known_hosts".to_owned(), @@ -75,6 +126,18 @@ pub(crate) fn known_hosts_lookup(ssh_g_output: &str) -> Option { } } +/// The UserKnownHostsFile entries `ssh -G` printed, `~` and `%` tokens +/// already expanded. ssh prints them unquoted, so a path with a space +/// in it splits and matches nothing. +fn user_known_hosts_files(ssh_g_output: &str) -> Vec { + ssh_g_output + .lines() + .filter_map(|line| line.strip_prefix("userknownhostsfile ")) + .flat_map(str::split_whitespace) + .map(PathBuf::from) + .collect() +} + async fn run(program: &Path, args: &[OsString]) -> Result { let output = Command::new(program) .args(args) @@ -154,6 +217,127 @@ mod tests { assert!(left.contains(&format!("db.example.com {KEY}")), "{left}"); } + /// An `ssh` whose `-G` prints what `hosts` gives for the host named + /// last, the way an ssh_config would resolve it, and fails for any + /// other. + fn fake_ssh(dir: &Path, hosts: &[(&str, String)]) -> PathBuf { + use std::os::unix::fs::PermissionsExt; + + let cases: String = hosts + .iter() + .map(|(host, resolved)| format!(" {host}) printf '{resolved}' ;;\n")) + .collect(); + let program = dir.join("ssh"); + std::fs::write( + &program, + format!("#!/bin/sh\nfor last; do :; done\ncase \"$last\" in\n{cases} *) exit 255 ;;\nesac\n"), + ) + .unwrap(); + std::fs::set_permissions(&program, std::fs::Permissions::from_mode(0o755)).unwrap(); + program + } + + fn tunnel(host: &str) -> SshConfig { + SshConfig { + destination: crate::SshDestination::new(host, None, None).unwrap(), + jump_hosts: Vec::new(), + auth: crate::SshAuth::Agent, + } + } + + /// A banner can print the lines of a changed key for any host and + /// file, so only a host and file `ssh -G` gives for the connection + /// lose a key. + #[tokio::test] + async fn a_changed_key_is_removed_only_for_this_connections_hosts_and_files() { + let dir = tempfile::tempdir().unwrap(); + let known_hosts = dir.path().join("known_hosts"); + let elsewhere = dir.path().join("elsewhere"); + let keys = format!("[db.example.com]:2222 {KEY}\nother.example.com {KEY}\n"); + for path in [&known_hosts, &elsewhere] { + std::fs::write(path, &keys).unwrap(); + } + let resolved = format!( + "hostname db.example.com\\nport 2222\\nuserknownhostsfile {} {}/known_hosts2\\n", + known_hosts.display(), + dir.path().display() + ); + let services = SshServices { + ssh_program: fake_ssh(dir.path(), &[("db", resolved)]), + ..services(dir.path()) + }; + + let other_host = forget_changed_host_key(&services, &tunnel("db"), "other.example.com", &known_hosts).await; + let other_file = forget_changed_host_key(&services, &tunnel("db"), "[db.example.com]:2222", &elsewhere).await; + + assert!( + matches!(other_host, Err(ForgetHostKeyError::NotThisConnection { .. })), + "{other_host:?}" + ); + assert!( + matches!(other_file, Err(ForgetHostKeyError::NotThisConnection { .. })), + "{other_file:?}" + ); + assert_eq!(std::fs::read_to_string(&known_hosts).unwrap(), keys); + assert_eq!(std::fs::read_to_string(&elsewhere).unwrap(), keys); + + forget_changed_host_key(&services, &tunnel("db"), "[db.example.com]:2222", &known_hosts) + .await + .unwrap(); + + let left = std::fs::read_to_string(&known_hosts).unwrap(); + assert!(!left.contains("[db.example.com]:2222"), "{left}"); + assert!(left.contains("other.example.com"), "{left}"); + } + + /// A jump host the configuration adds is one of the connection's + /// hosts, by the name and file its own `ssh -G` gives. + #[tokio::test] + async fn a_jump_hosts_changed_key_is_removed_from_its_own_file() { + let dir = tempfile::tempdir().unwrap(); + let known_hosts = dir.path().join("known_hosts"); + std::fs::write(&known_hosts, format!("bastion.example.com {KEY}\n10.0.0.5 {KEY}\n")).unwrap(); + let file = known_hosts.display(); + let services = SshServices { + ssh_program: fake_ssh( + dir.path(), + &[ + ( + "db", + format!("hostname 10.0.0.5\\nport 22\\nproxyjump bastion\\nuserknownhostsfile {file}\\n"), + ), + ( + "bastion", + format!("hostname bastion.example.com\\nport 22\\nuserknownhostsfile {file}\\n"), + ), + ], + ), + ..services(dir.path()) + }; + + forget_changed_host_key(&services, &tunnel("db"), "bastion.example.com", &known_hosts) + .await + .unwrap(); + + let left = std::fs::read_to_string(&known_hosts).unwrap(); + assert!(!left.contains("bastion.example.com"), "{left}"); + assert!(left.contains("10.0.0.5"), "{left}"); + } + + #[test] + fn user_known_hosts_files_from_ssh_g_output() { + let output = "hostname h\nuserknownhostsfile /home/u/.ssh/known_hosts /home/u/.ssh/known_hosts2\n"; + + assert_eq!( + user_known_hosts_files(output), + [ + PathBuf::from("/home/u/.ssh/known_hosts"), + PathBuf::from("/home/u/.ssh/known_hosts2") + ] + ); + assert!(user_known_hosts_files("hostname h\n").is_empty()); + } + #[tokio::test] async fn forget_known_host_refuses_an_empty_name() { let dir = tempfile::tempdir().unwrap(); diff --git a/linux/crates/ssh/src/lib.rs b/linux/crates/ssh/src/lib.rs index 74f6f5d834..fdc475a3d5 100644 --- a/linux/crates/ssh/src/lib.rs +++ b/linux/crates/ssh/src/lib.rs @@ -22,7 +22,7 @@ mod sweep; pub use config::{SshAuth, SshConfig}; pub use destination::SshDestination; pub use forward::SshForwardTransport; -pub use known_hosts::{forget_host_key, forget_known_host}; +pub use known_hosts::{ForgetHostKeyError, forget_changed_host_key, forget_host_key}; pub use prompt::AskpassPrompt; pub use runtime::SshRuntime; pub use runtime_cell::SshRuntimeCell; diff --git a/linux/crates/ssh/src/stderr_classify.rs b/linux/crates/ssh/src/stderr_classify.rs index 1fe6183493..8633916158 100644 --- a/linux/crates/ssh/src/stderr_classify.rs +++ b/linux/crates/ssh/src/stderr_classify.rs @@ -6,6 +6,9 @@ const PROTOCOL_DETAIL_LIMIT: usize = 4096; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct DeclinedHostKey { + /// The host ssh asked about, which behind a jump host is not always + /// the destination. + pub host: String, pub algorithm: String, pub fingerprint: String, } @@ -60,10 +63,10 @@ fn host_key_failure(stderr: &str, context: &ClassifyContext<'_>) -> Option Date: Wed, 14 Oct 2026 16:52:10 +0700 Subject: [PATCH 6/7] fix(linux): open a file only as a saved file connection's database --- .../crates/app/src/application/open_files.rs | 3 +- .../app/src/services/file_resolution.rs | 66 ++++++++++++++++--- linux/crates/app/src/ui/session_window/mod.rs | 3 +- .../session/src/registry/session_key.rs | 56 ++++++++++++---- 4 files changed, 102 insertions(+), 26 deletions(-) diff --git a/linux/crates/app/src/application/open_files.rs b/linux/crates/app/src/application/open_files.rs index 783f7b3901..4996de17cf 100644 --- a/linux/crates/app/src/application/open_files.rs +++ b/linux/crates/app/src/application/open_files.rs @@ -26,9 +26,10 @@ impl TableProApplication { }; let (path, uri) = (file.path(), file.uri().to_string()); let connections = services.storage.connections().clone(); + let drivers = services.drivers.clone(); let resolving = services .tasks - .spawn_blocking_task(move || resolve_file_blocking(path, uri, &connections)); + .spawn_blocking_task(move || resolve_file_blocking(path, uri, &connections, &drivers)); // An instance D-Bus started for this file would otherwise exit // before its window opens. let hold = self.hold(); diff --git a/linux/crates/app/src/services/file_resolution.rs b/linux/crates/app/src/services/file_resolution.rs index 27cf46f999..2a170600be 100644 --- a/linux/crates/app/src/services/file_resolution.rs +++ b/linux/crates/app/src/services/file_resolution.rs @@ -3,6 +3,7 @@ use std::path::{Path, PathBuf}; +use tablepro_core::DriverRegistry; use tablepro_session::SessionKey; use tablepro_session::registry::FileIdentity; use tablepro_storage::{ConnectionStore, SavedConnection}; @@ -51,7 +52,12 @@ pub fn resolve_file_identity_blocking(path: Option<&Path>, uri: &str) -> Result< /// Resolve the file, then find whether a saved connection opens the /// same one. -pub fn resolve_file_blocking(path: Option, uri: String, connections: &ConnectionStore) -> ResolvedFile { +pub fn resolve_file_blocking( + path: Option, + uri: String, + connections: &ConnectionStore, + drivers: &DriverRegistry, +) -> ResolvedFile { let target = resolve_file_identity_blocking(path.as_deref(), &uri).map(|identity| { let saved = match connections.load_blocking() { Ok(saved) => saved.to_vec(), @@ -60,7 +66,12 @@ pub fn resolve_file_blocking(path: Option, uri: String, connections: &C Vec::new() } }; - match SessionKey::for_file(&identity, &saved) { + let is_file_driver = |id: &str| { + drivers + .get(id) + .is_some_and(|driver| driver.capabilities().is_file_based()) + }; + match SessionKey::for_file(&identity, &saved, is_file_driver) { SessionKey::Saved(id) => match saved.into_iter().find(|connection| connection.id == id) { Some(connection) => FileTarget::Saved(Box::new(connection)), None => FileTarget::File(identity), @@ -73,6 +84,8 @@ pub fn resolve_file_blocking(path: Option, uri: String, connections: &C #[cfg(test)] mod tests { + use tablepro_test_support::{ConnectionScript, FakeDriver}; + use super::*; #[test] @@ -89,11 +102,24 @@ mod tests { )); } - #[test] - fn a_saved_file_opens_its_saved_connection() { + fn drivers(capabilities: &'static tablepro_core::DriverCapabilities) -> DriverRegistry { + let mut drivers = DriverRegistry::new(); + drivers.register(std::sync::Arc::new(FakeDriver::connecting( + capabilities, + ConnectionScript::default(), + ))); + drivers + } + + /// `database` saved as a connection of the fake driver, and what + /// opening the file then resolves to. + fn resolve_saved( + database: &str, + capabilities: &'static tablepro_core::DriverCapabilities, + ) -> (uuid::Uuid, ResolvedFile) { let root = tempfile::tempdir().expect("tempdir"); - let database = root.path().join("app.db"); - std::fs::write(&database, b"").expect("seed"); + let path = std::fs::canonicalize(root.path()).expect("canonical").join(database); + std::fs::write(&path, b"").expect("seed"); let store = ConnectionStore::new(&tablepro_storage::StoragePaths::under( root.path(), "tablepro-test", @@ -102,10 +128,10 @@ mod tests { let saved = SavedConnection { id: uuid::Uuid::new_v4(), name: "local".to_owned(), - driver_id: "sqlite".to_owned(), + driver_id: "fake".to_owned(), host: String::new(), port: 0, - database: database.display().to_string(), + database: path.display().to_string(), username: String::new(), tls: tablepro_core::TlsConfig::disabled(), read_only: false, @@ -116,12 +142,32 @@ mod tests { group: None, }; store.upsert_blocking(saved.clone()).expect("saved"); + let uri = format!("file://{}", path.display()); + + ( + saved.id, + resolve_file_blocking(Some(path), uri, &store, &drivers(capabilities)), + ) + } - let resolved = resolve_file_blocking(Some(database.clone()), format!("file://{}", database.display()), &store); + #[test] + fn a_saved_file_opens_its_saved_connection() { + let (id, resolved) = resolve_saved("app.db", &FakeDriver::FILE_CAPABILITIES); match resolved.target { - Ok(FileTarget::Saved(connection)) => assert_eq!(connection.id, saved.id), + Ok(FileTarget::Saved(connection)) => assert_eq!(connection.id, id), other => panic!("the saved connection was not found: {other:?}"), } } + + #[test] + fn a_server_whose_database_names_the_file_is_not_opened() { + let (_, resolved) = resolve_saved("shop", &FakeDriver::NETWORK_CAPABILITIES); + + assert!( + matches!(resolved.target, Ok(FileTarget::File(_))), + "{:?}", + resolved.target + ); + } } diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index 85c62707a7..d6faea6c30 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -585,10 +585,11 @@ impl SessionWindow { let uri = file.uri().to_string(); let resolving_uri = uri.clone(); let connections = state.services.storage.connections().clone(); + let drivers = state.services.drivers.clone(); let resolving = state .services .tasks - .spawn_blocking_task(move || resolve_file_blocking(path, resolving_uri, &connections)); + .spawn_blocking_task(move || resolve_file_blocking(path, resolving_uri, &connections, &drivers)); let input = self.input(); glib::spawn_future_local(async move { let resolved = resolving.await.unwrap_or_else(|failure| ResolvedFile { diff --git a/linux/crates/session/src/registry/session_key.rs b/linux/crates/session/src/registry/session_key.rs index 53120e1fdc..bd2ec26a06 100644 --- a/linux/crates/session/src/registry/session_key.rs +++ b/linux/crates/session/src/registry/session_key.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use tablepro_storage::SavedConnection; use uuid::Uuid; @@ -17,11 +19,14 @@ pub enum SessionKey { } impl SessionKey { - pub fn for_file(identity: &FileIdentity, saved: &[SavedConnection]) -> Self { + /// Only a saved connection whose driver `is_file_driver` says opens + /// a file can be the file: any other would log in to its server with + /// its stored credentials. + pub fn for_file(identity: &FileIdentity, saved: &[SavedConnection], is_file_driver: impl Fn(&str) -> bool) -> Self { if let Some(canonical) = identity.canonical_path.as_deref() { - let matched = saved - .iter() - .find(|connection| resolves_to(&connection.database, canonical)); + let matched = saved.iter().find(|connection| { + is_file_driver(&connection.driver_id) && resolves_to(&connection.database, canonical) + }); if let Some(connection) = matched { return Self::Saved(connection.id); } @@ -31,14 +36,17 @@ impl SessionKey { } /// A saved entry names its file however the user typed it, so both -/// sides are resolved before they are compared. -fn resolves_to(database: &str, canonical: &std::path::Path) -> bool { - if database.is_empty() { +/// sides are resolved before they are compared. A relative name would +/// resolve against the directory TablePro was started in, so only an +/// absolute one names a file. +fn resolves_to(database: &str, canonical: &Path) -> bool { + let database = Path::new(database); + if !database.is_absolute() { return false; } match std::fs::canonicalize(database) { Ok(resolved) => resolved == canonical, - Err(_) => std::path::Path::new(database) == canonical, + Err(_) => database == canonical, } } @@ -46,6 +54,10 @@ fn resolves_to(database: &str, canonical: &std::path::Path) -> bool { mod tests { use super::*; + fn file_driver(id: &str) -> bool { + id == "sqlite" + } + fn saved_file(database: &str) -> SavedConnection { SavedConnection { id: Uuid::new_v4(), @@ -80,7 +92,7 @@ mod tests { format!("file://{}", database.display()), ); - let key = SessionKey::for_file(&identity, &saved); + let key = SessionKey::for_file(&identity, &saved, file_driver); assert_eq!(key, SessionKey::Saved(saved[0].id)); } @@ -93,7 +105,7 @@ mod tests { let uri = format!("file://{}", database.display()); let identity = FileIdentity::new(Some(database.clone()), uri.clone()); - let key = SessionKey::for_file(&identity, &[]); + let key = SessionKey::for_file(&identity, &[], file_driver); assert_eq!(key, SessionKey::File(uri)); } @@ -102,7 +114,7 @@ mod tests { fn a_file_that_no_longer_resolves_still_gets_a_key() { let identity = FileIdentity::new(None, "file:///gone/app.db".to_owned()); - let key = SessionKey::for_file(&identity, &[saved_file("/gone/app.db")]); + let key = SessionKey::for_file(&identity, &[saved_file("/gone/app.db")], file_driver); assert_eq!( key, @@ -111,16 +123,32 @@ mod tests { ); } + /// `tablepro shop` must not log in to a saved server whose database + /// is named like the file, with that server's stored password. #[test] fn a_network_connection_never_matches_a_file() { let root = tempfile::tempdir().expect("tempdir"); - let database = root.path().join("app.db"); + let database = std::fs::canonicalize(root.path()).expect("canonical").join("shop"); std::fs::write(&database, b"").expect("seed"); - let mut network = saved_file("postgres"); + let mut network = saved_file(&database.to_string_lossy()); network.driver_id = "postgres".to_owned(); let identity = FileIdentity::new(Some(database.clone()), format!("file://{}", database.display())); - let key = SessionKey::for_file(&identity, &[network]); + let key = SessionKey::for_file(&identity, &[network], file_driver); + + assert!(matches!(key, SessionKey::File(_)), "{key:?}"); + } + + /// A relative name resolves against whatever directory TablePro was + /// started in, here the crate's own, so it never names the file. + #[test] + fn a_relative_database_never_matches_a_file() { + let manifest = std::fs::canonicalize(concat!(env!("CARGO_MANIFEST_DIR"), "/Cargo.toml")).expect("canonical"); + let identity = FileIdentity::new(Some(manifest.clone()), format!("file://{}", manifest.display())); + let mut network = saved_file("Cargo.toml"); + network.driver_id = "postgres".to_owned(); + + let key = SessionKey::for_file(&identity, &[saved_file("Cargo.toml"), network], file_driver); assert!(matches!(key, SessionKey::File(_)), "{key:?}"); } From 7b962d59af57b009612cc5bb660c4d86209082ad Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 14 Oct 2026 16:52:10 +0700 Subject: [PATCH 7/7] fix(linux): withhold encryption calls and CLONE from query history --- .../crates/drivers/clickhouse/src/dialect.rs | 13 ++++++++++++ linux/crates/drivers/mysql/src/dialect.rs | 20 ++++++++++++++++++- linux/crates/drivers/postgres/src/dialect.rs | 13 ++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/linux/crates/drivers/clickhouse/src/dialect.rs b/linux/crates/drivers/clickhouse/src/dialect.rs index 4b944bc215..2b77d439b8 100644 --- a/linux/crates/drivers/clickhouse/src/dialect.rs +++ b/linux/crates/drivers/clickhouse/src/dialect.rs @@ -399,6 +399,11 @@ const CREDENTIAL_RULES: CredentialRules = CredentialRules { "url", "odbc", "jdbc", + "encrypt", + "decrypt", + "tryDecrypt", + "aes_encrypt_mysql", + "aes_decrypt_mysql", ], }; @@ -607,6 +612,12 @@ mod tests { "CREATE TABLE t (id Int32) ENGINE = URL('http://u:p@h/f.csv', CSV)", "SELECT * FROM odbc('DSN=d;UID=u;PWD=p', 't')", "SELECT * FROM jdbc('jdbc:mysql://h/?user=u&password=p', 's', 't')", + "SELECT encrypt('aes-256-gcm', 'text', 'key', 'iv')", + "SELECT decrypt('aes-256-gcm', secret, 'key', 'iv') FROM t", + "SELECT tryDecrypt('aes-256-gcm', secret, 'key', 'iv') FROM t", + "SELECT aes_encrypt_mysql('aes-256-ecb', 'text', 'key')", + "SELECT aes_decrypt_mysql('aes-256-ecb', secret, 'key') FROM t", + "SELECT a.id FROM a JOIN b ON decrypt('aes-256-gcm', b.v, 'key', 'iv') = a.v", ] { assert_eq!( dialect().history_recording(sql), @@ -626,6 +637,8 @@ mod tests { "SELECT url, domain(url) FROM visits", "CREATE TABLE url (id Int32) ENGINE = Memory", "BACKUP TABLE t TO Disk('backups', 'b.zip')", + "SELECT encrypt, decrypt FROM settings", + "CREATE TABLE encrypt (id Int32) ENGINE = Memory", ] { assert_eq!(dialect().history_recording(sql), HistoryRecording::Record, "{sql}"); } diff --git a/linux/crates/drivers/mysql/src/dialect.rs b/linux/crates/drivers/mysql/src/dialect.rs index 7059b6bbc3..26751e1189 100644 --- a/linux/crates/drivers/mysql/src/dialect.rs +++ b/linux/crates/drivers/mysql/src/dialect.rs @@ -389,8 +389,18 @@ const CREDENTIAL_RULES: CredentialRules = CredentialRules { (&["START", "GROUP_REPLICATION"], &["PASSWORD"]), (&["CREATE"], &["CONNECTION"]), (&["ALTER"], &["CONNECTION"]), + (&["CLONE"], &["IDENTIFIED"]), + ], + carriers: &[ + "PASSWORD", + "OLD_PASSWORD", + "AES_ENCRYPT", + "AES_DECRYPT", + "DES_ENCRYPT", + "DES_DECRYPT", + "ENCODE", + "DECODE", ], - carriers: &["PASSWORD", "OLD_PASSWORD"], }; #[cfg(test)] @@ -654,6 +664,11 @@ mod tests { "START GROUP_REPLICATION USER='u', PASSWORD='p'", "CREATE TABLE t (id int) /*!50100 ENGINE=FEDERATED CONNECTION='mysql://u:p@h/d/t' */", "SELECT u.id FROM users u JOIN logins l ON PASSWORD('secret') = l.hash", + "CLONE INSTANCE FROM 'u'@'h':3306 IDENTIFIED BY 'pw'", + "SELECT AES_ENCRYPT('text', 'key')", + "SELECT AES_DECRYPT(secret, UNHEX(SHA2('passphrase', 512))) FROM t", + "SELECT DES_ENCRYPT('text', 'key'), DES_DECRYPT(secret, 'key') FROM t", + "SELECT ENCODE('text', 'pass'), DECODE(secret, 'pass') FROM t", ] { assert_eq!( dialect(MySqlFlavour::MySql).history_recording(sql), @@ -675,6 +690,9 @@ mod tests { "SELECT start, password FROM sessions", "START TRANSACTION", "CREATE INDEX by_user USING BTREE ON password (user_id)", + "SELECT aes_encrypt, decode FROM settings", + "CREATE TABLE aes_decrypt (id INT)", + "CLONE LOCAL DATA DIRECTORY = '/var/lib/clone'", ] { assert_eq!( dialect(MySqlFlavour::MySql).history_recording(sql), diff --git a/linux/crates/drivers/postgres/src/dialect.rs b/linux/crates/drivers/postgres/src/dialect.rs index ee49f0682e..4c3de9db06 100644 --- a/linux/crates/drivers/postgres/src/dialect.rs +++ b/linux/crates/drivers/postgres/src/dialect.rs @@ -372,6 +372,11 @@ const CREDENTIAL_RULES: CredentialRules = CredentialRules { "pgp_sym_decrypt_bytea", "pgp_pub_decrypt", "pgp_pub_decrypt_bytea", + "encrypt", + "decrypt", + "encrypt_iv", + "decrypt_iv", + "hmac", ], }; @@ -628,6 +633,11 @@ mod tests { "UPDATE users SET pw = crypt('secret', gen_salt('bf'))", "SELECT u.id FROM users u JOIN credentials c ON crypt('secret', c.hash) = c.hash", "SELECT id FROM users WHERE EXISTS crypt('secret', hash)", + "SELECT encrypt('data', 'key', 'aes')", + "SELECT decrypt(secret, 'key', 'aes') FROM t", + "SELECT encrypt_iv('data', 'key', 'iv', 'aes')", + "SELECT decrypt_iv(secret, 'key', 'iv', 'aes') FROM t", + "SELECT hmac('data', 'key', 'sha256')", ] { assert_eq!( dialect().history_recording(sql), @@ -646,6 +656,9 @@ mod tests { for sql in [ "CREATE INDEX ON crypt (hash)", "CREATE TABLE IF NOT EXISTS crypt (hash text)", + "SELECT encrypt, decrypt, hmac FROM settings", + "CREATE TABLE hmac (id int)", + "INSERT INTO encrypt_iv (id) VALUES (1)", ] { assert_eq!(dialect().history_recording(sql), HistoryRecording::Record, "{sql}"); }