diff --git a/backend/windmill-worker/src/bigquery_executor.rs b/backend/windmill-worker/src/bigquery_executor.rs index 50a2d3811e..388f59300c 100644 --- a/backend/windmill-worker/src/bigquery_executor.rs +++ b/backend/windmill-worker/src/bigquery_executor.rs @@ -438,7 +438,7 @@ pub async fn do_bigquery( &sig, &bigquery_args, &reserved_variables, - SqlStringEscaping::Backslash, + SqlStringEscaping::BigQuery, )?; let queries = parse_sql_blocks(query, false); diff --git a/backend/windmill-worker/src/duckdb_executor.rs b/backend/windmill-worker/src/duckdb_executor.rs index 56b7e4101f..c672f4ac53 100644 --- a/backend/windmill-worker/src/duckdb_executor.rs +++ b/backend/windmill-worker/src/duckdb_executor.rs @@ -1396,7 +1396,7 @@ pub async fn do_duckdb( &sig, &job_args, &reserved_variables, - SqlStringEscaping::Standard, + SqlStringEscaping::QuoteAndBackslash, )?; let mut query = transform_s3_uris(query).await?; @@ -1414,7 +1414,7 @@ pub async fn do_duckdb( &sig, &job_args, &reserved_variables, - SqlStringEscaping::Standard, + SqlStringEscaping::QuoteAndBackslash, )?; Some(transform_s3_uris(&interp).await?) } diff --git a/backend/windmill-worker/src/mssql_executor.rs b/backend/windmill-worker/src/mssql_executor.rs index b5acbc8c9b..36c785b8d6 100644 --- a/backend/windmill-worker/src/mssql_executor.rs +++ b/backend/windmill-worker/src/mssql_executor.rs @@ -307,7 +307,7 @@ pub async fn do_mssql( &sig, &mssql_args, &reserved_variables, - SqlStringEscaping::Standard, + SqlStringEscaping::Quote, )?; let mut prepared_query = Query::new(query.to_owned()); diff --git a/backend/windmill-worker/src/mysql_executor.rs b/backend/windmill-worker/src/mysql_executor.rs index eb6d3180df..e8a69917cd 100644 --- a/backend/windmill-worker/src/mysql_executor.rs +++ b/backend/windmill-worker/src/mysql_executor.rs @@ -31,9 +31,7 @@ use crate::{ s3_stream_and_upload_with_logs, OccupancyMetrics, S3ModeWorkerData, }, handle_child::run_future_with_polling_update_job_poller, - sanitized_sql_params::{ - has_contextual_variables, sanitize_and_interpolate_unsafe_sql_args, SqlStringEscaping, - }, + sanitized_sql_params::{sanitize_and_interpolate_unsafe_sql_args, SqlStringEscaping}, }; #[derive(Deserialize)] @@ -163,26 +161,6 @@ fn do_mysql_inner<'a>( Ok(result_f.boxed()) } -/// Under `NO_BACKSLASH_ESCAPES` a backslash-escaped quote closes the literal, so quotes are -/// doubled instead when the session has it or when the script may turn it on itself. -async fn mysql_string_escaping( - conn: &mut mysql_async::Conn, - query: &str, -) -> windmill_common::error::Result { - let sql_mode: Option = conn - .query_first("SELECT @@SESSION.sql_mode") - .await - .map_err(to_anyhow)?; - let no_backslash_escapes = |s: &str| s.to_ascii_uppercase().contains("NO_BACKSLASH_ESCAPES"); - Ok( - if sql_mode.as_deref().is_some_and(no_backslash_escapes) || no_backslash_escapes(query) { - SqlStringEscaping::DoubledQuotes - } else { - SqlStringEscaping::Backslash - }, - ) -} - pub async fn do_mysql( job: &MiniPulledJob, client: &AuthedClient, @@ -284,21 +262,12 @@ pub async fn do_mysql( let reserved_variables = get_reserved_variables(job, &client.token, conn, parent_runnable_path).await?; - let pool = mysql_async::Pool::new(opts); - let mut mysql_conn = pool.get_conn().await.map_err(to_anyhow)?; - - let escaping = if has_contextual_variables(query) { - mysql_string_escaping(&mut mysql_conn, query).await? - } else { - SqlStringEscaping::Backslash - }; - let (query, args_to_skip) = &sanitize_and_interpolate_unsafe_sql_args( query, &sig, &job_args, &reserved_variables, - escaping, + SqlStringEscaping::MySql, )?; let using_named_params = RE_ARG_MYSQL_NAMED.captures_iter(query).count() > 0; @@ -372,6 +341,8 @@ pub async fn do_mysql( } } + let pool = mysql_async::Pool::new(opts); + let mysql_conn = pool.get_conn().await.map_err(to_anyhow)?; let conn_a = Arc::new(Mutex::new(mysql_conn)); let queries = parse_sql_blocks(query, false); diff --git a/backend/windmill-worker/src/oracledb_executor.rs b/backend/windmill-worker/src/oracledb_executor.rs index 787386d9e6..21ff78afc8 100644 --- a/backend/windmill-worker/src/oracledb_executor.rs +++ b/backend/windmill-worker/src/oracledb_executor.rs @@ -410,7 +410,7 @@ pub async fn do_oracledb( &sig, &job_args, &reserved_variables, - SqlStringEscaping::Standard, + SqlStringEscaping::Quote, )?; let (_, errors) = get_statement_values(sig.clone(), &job_args, &args_to_skip); diff --git a/backend/windmill-worker/src/pg_executor.rs b/backend/windmill-worker/src/pg_executor.rs index 89e2290b18..fa7ebc01e3 100644 --- a/backend/windmill-worker/src/pg_executor.rs +++ b/backend/windmill-worker/src/pg_executor.rs @@ -903,7 +903,7 @@ pub async fn do_postgresql( &sig.args, &pg_args, &reserved_variables, - SqlStringEscaping::Standard, + SqlStringEscaping::QuoteAndBackslash, )?; let queries = parse_sql_blocks(query, true); diff --git a/backend/windmill-worker/src/sanitized_sql_params.rs b/backend/windmill-worker/src/sanitized_sql_params.rs index 405140725c..5f88f8d7e4 100644 --- a/backend/windmill-worker/src/sanitized_sql_params.rs +++ b/backend/windmill-worker/src/sanitized_sql_params.rs @@ -33,47 +33,53 @@ fn sanitize_identifier(arg: &Arg, input: &str) -> Result<(), error::Error> { } } -/// How a dialect escapes characters inside a string literal. Contextual variables are -/// substituted as raw text, and `WM_END_USER_EMAIL` carries an app end user's email into a -/// query that runs with the author's credentials: an email may contain `'` (and `\` or `"` -/// in a quoted local part), so every value is escaped for any string literal it lands in. +/// How a dialect escapes a value substituted into a quote-delimited string literal. +/// Contextual variables are substituted as raw text, and `WM_END_USER_EMAIL` carries an app +/// end user's email into a query that runs with the author's credentials: an email may +/// contain `'` (and `\` or `"` in a quoted local part). +/// +/// The escaping must hold whatever literal form and session mode the query uses, so where a +/// dialect can process backslash escapes in some literal (PostgreSQL/DuckDB `E'...'`, MySQL +/// outside `NO_BACKSLASH_ESCAPES`), `\` is doubled too: otherwise `\'` in the value becomes +/// `\''`, whose second quote closes the literal. The price is a doubled `\` (or, in MySQL, a +/// doubled quote of the other style than the literal) in the rare value that contains one. #[derive(Clone, Copy)] pub enum SqlStringEscaping { - /// `'` → `''`; `\` is ordinary and `"` quotes identifiers (PostgreSQL, MSSQL, Oracle, - /// DuckDB). - Standard, - /// `\` → `\\`, `'` → `\'` and `"` → `\"`, exact in both `'...'` and `"..."` literals - /// (MySQL, Snowflake, BigQuery; BigQuery does not accept `''`). + /// `'` → `''`; `\` is never an escape (MSSQL, Oracle). #[cfg_attr( - not(any(feature = "mysql", feature = "snowflake", feature = "bigquery")), + not(any(all(feature = "enterprise", feature = "mssql"), feature = "oracledb")), allow(dead_code) )] - Backslash, - /// `'` → `''` and `"` → `""`, for MySQL under `NO_BACKSLASH_ESCAPES`, where `\` is - /// ordinary and `\'` would close the literal. A quote of the other style than the - /// surrounding literal comes out doubled. + Quote, + /// `\` → `\\`, `'` → `''`; `"` quotes identifiers (PostgreSQL, DuckDB, Snowflake). + QuoteAndBackslash, + /// `\` → `\\`, `'` → `''`, `"` → `""`: MySQL also accepts `"..."` literals, and only + /// doubling stays confined under `NO_BACKSLASH_ESCAPES`. #[cfg_attr(not(feature = "mysql"), allow(dead_code))] - DoubledQuotes, + MySql, + /// `\` → `\\`, `'` → `\'`, `"` → `\"`: backslash escapes are always on, both quote + /// styles delimit strings, and `''` is not accepted. + #[cfg_attr(not(feature = "bigquery"), allow(dead_code))] + BigQuery, } impl SqlStringEscaping { fn escape(self, value: &str) -> String { match self { - SqlStringEscaping::Standard => value.replace('\'', "''"), - SqlStringEscaping::Backslash => value + SqlStringEscaping::Quote => value.replace('\'', "''"), + SqlStringEscaping::QuoteAndBackslash => value.replace('\\', "\\\\").replace('\'', "''"), + SqlStringEscaping::MySql => value + .replace('\\', "\\\\") + .replace('\'', "''") + .replace('"', "\"\""), + SqlStringEscaping::BigQuery => value .replace('\\', "\\\\") .replace('\'', "\\'") .replace('"', "\\\""), - SqlStringEscaping::DoubledQuotes => value.replace('\'', "''").replace('"', "\"\""), } } } -#[cfg(feature = "mysql")] -pub fn has_contextual_variables(code: &str) -> bool { - RE_SQL_CONTEXTUAL_VAR.is_match(code) -} - fn replace_contextual_variables( code: &mut String, contextual_variables: &HashMap, @@ -190,21 +196,27 @@ mod tests { let backslash = r#""x\'/**/OR/**/1=1#"@e.com"#; assert_eq!( - interpolate(code, quote, SqlStringEscaping::Standard), + interpolate(code, quote, SqlStringEscaping::Quote), r"SELECT 1 WHERE email = 'x''/**/OR/**/''1''=''1''--@e.com'" ); + // `\` doubled too, so `\'` cannot become `\''` in an `E'...'` literal assert_eq!( - interpolate(code, backslash, SqlStringEscaping::Backslash), + interpolate(code, backslash, SqlStringEscaping::QuoteAndBackslash), + r#"SELECT 1 WHERE email = '"x\\''/**/OR/**/1=1#"@e.com'"# + ); + assert_eq!( + interpolate(code, backslash, SqlStringEscaping::MySql), + r#"SELECT 1 WHERE email = '""x\\''/**/OR/**/1=1#""@e.com'"# + ); + assert_eq!( + interpolate(code, backslash, SqlStringEscaping::BigQuery), r#"SELECT 1 WHERE email = '\"x\\\'/**/OR/**/1=1#\"@e.com'"# ); - assert_eq!( - interpolate(code, backslash, SqlStringEscaping::DoubledQuotes), - r#"SELECT 1 WHERE email = '""x\''/**/OR/**/1=1#""@e.com'"# - ); for escaping in [ - SqlStringEscaping::Standard, - SqlStringEscaping::Backslash, - SqlStringEscaping::DoubledQuotes, + SqlStringEscaping::Quote, + SqlStringEscaping::QuoteAndBackslash, + SqlStringEscaping::MySql, + SqlStringEscaping::BigQuery, ] { assert_eq!( interpolate(code, "a.b+c@e.com", escaping), diff --git a/backend/windmill-worker/src/snowflake_executor.rs b/backend/windmill-worker/src/snowflake_executor.rs index 2aa203859b..947835b7ec 100644 --- a/backend/windmill-worker/src/snowflake_executor.rs +++ b/backend/windmill-worker/src/snowflake_executor.rs @@ -208,7 +208,7 @@ fn do_snowflake_inner<'a>( &sig, &job_args, reserved_variables, - SqlStringEscaping::Backslash, + SqlStringEscaping::QuoteAndBackslash, )?; body.insert("statement".to_string(), json!(query));