From e8d4ae8fd0f49db6bb5f75250c76e67a5096cd2f Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 23 Sep 2026 16:03:24 +0200 Subject: [PATCH] fix(worker): pick MySQL literal escaping from the session sql_mode Backslash escapes are exact in both MySQL quote styles, but under NO_BACKSLASH_ESCAPES a backslash-escaped quote closes the literal. Read the session sql_mode (only when the script uses a contextual variable) and fall back to doubled quotes when it, or the script itself, turns that mode on. Snowflake and BigQuery share the backslash escaping. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../windmill-worker/src/bigquery_executor.rs | 2 +- backend/windmill-worker/src/mysql_executor.rs | 37 +++++++++++-- .../src/sanitized_sql_params.rs | 54 +++++++++---------- .../windmill-worker/src/snowflake_executor.rs | 2 +- 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/backend/windmill-worker/src/bigquery_executor.rs b/backend/windmill-worker/src/bigquery_executor.rs index 388f59300c..50a2d3811e 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::BigQuery, + SqlStringEscaping::Backslash, )?; let queries = parse_sql_blocks(query, false); diff --git a/backend/windmill-worker/src/mysql_executor.rs b/backend/windmill-worker/src/mysql_executor.rs index e8a69917cd..eb6d3180df 100644 --- a/backend/windmill-worker/src/mysql_executor.rs +++ b/backend/windmill-worker/src/mysql_executor.rs @@ -31,7 +31,9 @@ use crate::{ s3_stream_and_upload_with_logs, OccupancyMetrics, S3ModeWorkerData, }, handle_child::run_future_with_polling_update_job_poller, - sanitized_sql_params::{sanitize_and_interpolate_unsafe_sql_args, SqlStringEscaping}, + sanitized_sql_params::{ + has_contextual_variables, sanitize_and_interpolate_unsafe_sql_args, SqlStringEscaping, + }, }; #[derive(Deserialize)] @@ -161,6 +163,26 @@ 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, @@ -262,12 +284,21 @@ 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, - SqlStringEscaping::MySql, + escaping, )?; let using_named_params = RE_ARG_MYSQL_NAMED.captures_iter(query).count() > 0; @@ -341,8 +372,6 @@ 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/sanitized_sql_params.rs b/backend/windmill-worker/src/sanitized_sql_params.rs index 82e3c9fd7c..405140725c 100644 --- a/backend/windmill-worker/src/sanitized_sql_params.rs +++ b/backend/windmill-worker/src/sanitized_sql_params.rs @@ -42,37 +42,38 @@ pub enum SqlStringEscaping { /// `'` → `''`; `\` is ordinary and `"` quotes identifiers (PostgreSQL, MSSQL, Oracle, /// DuckDB). Standard, - /// `\` → `\\` and `'` → `''`; `"` quotes identifiers. - #[cfg_attr(not(feature = "snowflake"), allow(dead_code))] - Snowflake, - /// `\` → `\\`, `'` → `''` and `"` → `""`. Doubling rather than `\'`/`\"` because under - /// `NO_BACKSLASH_ESCAPES` a backslash escape would close the literal; the price is that - /// a `"` in a `'...'` literal (or `'` in `"..."`) comes out doubled. + /// `\` → `\\`, `'` → `\'` and `"` → `\"`, exact in both `'...'` and `"..."` literals + /// (MySQL, Snowflake, BigQuery; BigQuery does not accept `''`). + #[cfg_attr( + not(any(feature = "mysql", feature = "snowflake", feature = "bigquery")), + 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. #[cfg_attr(not(feature = "mysql"), allow(dead_code))] - MySql, - /// `\` → `\\`, `'` → `\'` and `"` → `\"`, valid in both quote styles; the dialect does - /// not accept `''`. - #[cfg_attr(not(feature = "bigquery"), allow(dead_code))] - BigQuery, + DoubledQuotes, } impl SqlStringEscaping { fn escape(self, value: &str) -> String { match self { SqlStringEscaping::Standard => value.replace('\'', "''"), - SqlStringEscaping::Snowflake => value.replace('\\', "\\\\").replace('\'', "''"), - SqlStringEscaping::MySql => value - .replace('\\', "\\\\") - .replace('\'', "''") - .replace('"', "\"\""), - SqlStringEscaping::BigQuery => value + SqlStringEscaping::Backslash => 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, @@ -193,22 +194,17 @@ mod tests { r"SELECT 1 WHERE email = 'x''/**/OR/**/''1''=''1''--@e.com'" ); assert_eq!( - interpolate(code, backslash, SqlStringEscaping::Snowflake), - 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), + interpolate(code, backslash, SqlStringEscaping::Backslash), 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::Snowflake, - SqlStringEscaping::MySql, - SqlStringEscaping::BigQuery, + SqlStringEscaping::Backslash, + SqlStringEscaping::DoubledQuotes, ] { 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 3d2a0b77b1..2aa203859b 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::Snowflake, + SqlStringEscaping::Backslash, )?; body.insert("statement".to_string(), json!(query));