From 812ff6ea5e1144e38d4ba00a48de7d35bd6178bd Mon Sep 17 00:00:00 2001 From: Alexander Petric Date: Fri, 23 Oct 2026 18:49:17 -0400 Subject: [PATCH] Merge commit from fork Co-authored-by: Claude Opus 5 --- backend/ee-repo-ref.txt | 2 +- backend/windmill-api-flows/src/flows.rs | 42 ++++- .../tests/operator_manage_rights.rs | 59 ------- backend/windmill-api-jobs/src/query.rs | 37 +++- backend/windmill-api-schedule/src/lib.rs | 7 +- backend/windmill-api-scripts/src/scripts.rs | 4 +- .../windmill-api-workspaces/src/workspaces.rs | 163 ++---------------- backend/windmill-api/src/apps.rs | 4 +- backend/windmill-api/src/mcp/utils.rs | 4 +- backend/windmill-api/src/raw_apps.rs | 4 +- backend/windmill-common/src/lib.rs | 64 ------- backend/windmill-store/src/resources.rs | 7 +- backend/windmill-store/src/variables.rs | 7 +- 13 files changed, 105 insertions(+), 299 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index d961a68842..1986b0b23f 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -b6e23d229966cf0019c0a1926a254fda658d8def +5dfdf811c862c689e42f03f485e02f9f1e7bc7ca diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index 4c0ecaf4c4..5d4268a93c 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -134,6 +134,17 @@ async fn list_search_flows( Ok(Json(rows)) } +/// `WHERE` condition matching one label, directly on the flow or inherited from its folder. +/// +/// The template has two `?`, so both are filled in a single `binds` pass. Chaining `bind` +/// would not work: `bind` replaces the first `?` of the string *as it is at that moment*, +/// so the second call would find the `?` that the first call inserted as part of the bound +/// value and substitute there, leaving the value partly outside its quotes. +fn label_condition(label: &str) -> String { + "(o.labels @> ARRAY[?] OR folder_labels(o.workspace_id, o.path) @> ARRAY[?])" + .binds(&[&label.trim()]) +} + async fn list_flows( authed: ApiAuthed, Extension(user_db): Extension, @@ -217,11 +228,7 @@ async fn list_flows( } if let Some(label) = &lq.label { for l in label.split(',') { - sqlb.and_where( - "(o.labels @> ARRAY[?] OR folder_labels(o.workspace_id, o.path) @> ARRAY[?])" - .bind(&l.trim()) - .bind(&l.trim()), - ); + sqlb.and_where(label_condition(l)); } } @@ -2284,6 +2291,31 @@ mod tests { const SECOND: Duration = Duration::from_secs(1); + #[test] + fn label_condition_fills_both_placeholders_in_one_pass() { + // Ordinary label: unchanged behaviour, the value appears quoted in both positions. + assert_eq!( + super::label_condition(" finance "), + "(o.labels @> ARRAY['finance'] OR folder_labels(o.workspace_id, o.path) @> ARRAY['finance'])" + ); + + // A `?` in the label must stay inside the quoted literal in both positions and must + // not leave a placeholder behind. With chained `bind`s this produced + // `ARRAY['a'a?b'b'] ... ARRAY[?]`, i.e. label text outside the quotes. + let cond = super::label_condition("a?b"); + assert_eq!( + cond, + "(o.labels @> ARRAY['a?b'] OR folder_labels(o.workspace_id, o.path) @> ARRAY['a?b'])" + ); + assert!(!cond.contains("ARRAY[?]")); + + // Single quotes keep being escaped by doubling. + assert_eq!( + super::label_condition("a'?b"), + "(o.labels @> ARRAY['a''?b'] OR folder_labels(o.workspace_id, o.path) @> ARRAY['a''?b'])" + ); + } + #[test] fn flowmodule_serde() { let fv = FlowValue { diff --git a/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs b/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs index dbc5f36e01..cfbeae7a69 100644 --- a/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs +++ b/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs @@ -406,62 +406,3 @@ async fn test_operators_draft_resources_and_variables(db: Pool) -> any Ok(()) } - -/// The two endpoints that make the API server run `pg_dump`/`psql` against a database a request -/// names are closed to a role that cannot run code. -/// -/// Both polarities on each route: a 403 alone would equally be a route that does not exist or one -/// refusing the source for some other reason, and the admin's 400 shows the gate is about the role -/// rather than the request. The source is deliberately unresolvable, so the gate is the only thing -/// that can answer 403 and no database is ever reached. -#[sqlx::test(migrations = "../migrations", fixtures("base", "permissions_test"))] -#[serial] -async fn test_dumping_a_database_needs_the_code_right(db: Pool) -> anyhow::Result<()> { - initialize_tracing().await; - windmill_common::roles::invalidate_all_roles_cache(); - let server = ApiServer::start(db.clone()).await?; - let port = server.addr.port(); - let api = format!("http://localhost:{port}/api/w/{WS}"); - - let cases = [ - ( - "workspaces/export_pg_schema", - json!({"source": "u/operator/not-a-source"}), - "export a database schema", - ), - ( - "workspaces/import_pg_database", - json!({"source": "u/operator/not-a-source", "target": "u/operator/not-a-source", - "fork_behavior": "schema_only"}), - "import a database", - ), - ]; - - for (route, body, action) in cases { - let resp = operator_client() - .post(format!("{api}/{route}")) - .json(&body) - .send() - .await?; - let status = resp.status(); - let text = resp.text().await?; - assert_eq!(status, 403, "{route}: {text}"); - assert!( - text.contains(&format!("Your role cannot {action} in this workspace")), - "{route} refused for another reason: {text}" - ); - - let resp = reqwest::Client::new() - .post(format!("{api}/{route}")) - .header("Authorization", "Bearer SECRET_TOKEN") - .json(&body) - .send() - .await?; - let status = resp.status(); - let text = resp.text().await?; - assert_eq!(status, 400, "{route}: {text}"); - assert!(text.contains("Invalid source format"), "{route}: {text}"); - } - - Ok(()) -} diff --git a/backend/windmill-api-jobs/src/query.rs b/backend/windmill-api-jobs/src/query.rs index 96d8ca3698..ebfc0c3725 100644 --- a/backend/windmill-api-jobs/src/query.rs +++ b/backend/windmill-api-jobs/src/query.rs @@ -268,9 +268,10 @@ pub fn filter_list_queue_query( if let Some(bf) = &lq.broad_filter { let pat = format!("%{}%", escape_ilike_pattern(bf)); + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(runnable_path ILIKE ? OR v2_job.tag ILIKE ? OR trigger ILIKE ? OR trigger_kind::text ILIKE ?)" - .bind(&pat).bind(&pat).bind(&pat).bind(&pat) + .binds(&[&pat]), ); } @@ -490,7 +491,7 @@ pub fn filter_list_completed_query( } if let Some(dt) = &lq.created_or_started_after { let ts = dt.to_rfc3339(); - sqlb.and_where("(created_at >= ? OR started_at >= ?)".bind(&ts).bind(&ts)); + sqlb.and_where("(created_at >= ? OR started_at >= ?)".binds(&[&ts])); } if let Some(dt) = &lq.created_before { @@ -591,10 +592,11 @@ pub fn filter_list_completed_query( if let Some(bf) = &lq.broad_filter { let pat = format!("%{}%", escape_ilike_pattern(bf)); + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(runnable_path ILIKE ? OR v2_job.tag ILIKE ? OR trigger ILIKE ? OR trigger_kind::text ILIKE ? \ OR EXISTS (SELECT 1 FROM unnest(v2_job.labels) lbl WHERE lbl ILIKE ?))" - .bind(&pat).bind(&pat).bind(&pat).bind(&pat).bind(&pat) + .binds(&[&pat]), ); } @@ -945,6 +947,20 @@ mod tests { assert!(sql.contains("'schedule'")); } + #[test] + fn test_queue_broad_filter_with_question_mark() { + let lq = ListQueueQuery { broad_filter: Some("a?b".to_string()), ..empty_queue_query() }; + let sqlb = filter_list_queue_query( + SqlBuilder::select_from("v2_job_queue").clone(), + &lq, + "ws", + false, + ); + let sql = build_sql(sqlb); + assert_eq!(sql.matches("ILIKE '%a?b%'").count(), 4); + assert!(!sql.contains("ILIKE ?")); + } + // --- Completed query tests --- #[test] @@ -1124,4 +1140,19 @@ mod tests { let sql = build_sql(sqlb); assert!(sql.contains("result @>")); } + + #[test] + fn test_completed_broad_filter_with_question_mark() { + let lq = + ListCompletedQuery { broad_filter: Some("a?b".to_string()), ..empty_completed_query() }; + let sqlb = filter_list_completed_query( + SqlBuilder::select_from("v2_job_completed").clone(), + &lq, + "ws", + false, + ); + let sql = build_sql(sqlb); + assert_eq!(sql.matches("ILIKE '%a?b%'").count(), 5); + assert!(!sql.contains("ILIKE ?")); + } } diff --git a/backend/windmill-api-schedule/src/lib.rs b/backend/windmill-api-schedule/src/lib.rs index 787742d18d..e2fa7da8fe 100644 --- a/backend/windmill-api-schedule/src/lib.rs +++ b/backend/windmill-api-schedule/src/lib.rs @@ -894,17 +894,18 @@ async fn list_schedule( } if let Some(broad_filter) = &lsq.broad_filter { let pat = format!("%{}%", escape_ilike_pattern(broad_filter)); + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(path ILIKE ? OR script_path ILIKE ? OR description ILIKE ? OR summary ILIKE ? OR schedule ILIKE ?)" - .bind(&pat).bind(&pat).bind(&pat).bind(&pat).bind(&pat) + .binds(&[&pat]), ); } if let Some(label) = &lsq.label { for l in label.split(',') { + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(labels @> ARRAY[?] OR folder_labels(workspace_id, path) @> ARRAY[?])" - .bind(&l.trim()) - .bind(&l.trim()), + .binds(&[&l.trim()]), ); } } diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index e6dcfe8155..4e8516b8ed 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -309,10 +309,10 @@ async fn list_scripts( } if let Some(label) = &lq.label { for l in label.split(',') { + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(o.labels @> ARRAY[?] OR folder_labels(o.workspace_id, o.path) @> ARRAY[?])" - .bind(&l.trim()) - .bind(&l.trim()), + .binds(&[&l.trim()]), ); } } diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 9a6efec148..0534f3e7ee 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3366,10 +3366,6 @@ pub(crate) async fn resolve_pg_source_checked_with_kind( let pg: PgDatabase = serde_json::from_value(db_resource) .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?; - // Checked here rather than at each caller: every source and target of a dump, restore or copy - // comes through this function, and `validate_dbname` only ever reaches a target. The check is - // deliberately the narrow one — see `validate_dbname_not_option_like`. - windmill_common::validate_dbname_not_option_like(&pg.dbname)?; Ok((pg, managed_kind)) } @@ -3459,20 +3455,21 @@ pub(crate) struct PgDumpOptions<'a> { pub(crate) no_acl: bool, } -/// A pg_dump invocation against `pg_db` writing to `dump_path`, carrying the connection settings -/// the CLI reads from the environment. The returned file, when there is one, holds the root -/// certificate the command points at and must outlive it. -fn pg_dump_command( +/// Run pg_dump against a PgDatabase, writing output to a temp file on disk. +/// Returns a DumpFile handle; the file is deleted when the handle is dropped. +pub(crate) async fn pg_dump_database( pg_db: &PgDatabase, opts: PgDumpOptions<'_>, - dump_path: &std::path::Path, -) -> Result<(tokio::process::Command, Option)> { +) -> Result { + let dump_file = DumpFile::new()?; + let host = &pg_db.host; let port = pg_db.port.unwrap_or(5432).to_string(); let user = pg_db.login_name(); + let dbname = &pg_db.dbname; let mut cmd = tokio::process::Command::new("pg_dump"); - cmd.arg("--format=plain").arg("--file").arg(dump_path); + cmd.arg("--format=plain").arg("--file").arg(&dump_file.path); if opts.schema_only { cmd.arg("--schema-only"); } @@ -3490,31 +3487,13 @@ fn pg_dump_command( .arg("--port") .arg(&port) .arg("--username") - .arg(user); - - // The name comes from a resource, so it never goes on the command line: as the positional - // argument a name starting with `-` is read as an option of pg_dump (`--file`, - // `--restrict-key`), and as `--dbname` a name holding `=` is expanded as a libpq connection - // string overriding the host, port and user set above. libpq takes the environment variable - // as a plain database name, neither parsed as an option nor expanded. - cmd.env("PGDATABASE", &pg_db.dbname); + .arg(user) + .arg(dbname); if let Some(ref password) = pg_db.password { cmd.env("PGPASSWORD", password); } - let root_cert = apply_pg_tls_env(&mut cmd, pg_db)?; - Ok((cmd, root_cert)) -} - -/// Run pg_dump against a PgDatabase, writing output to a temp file on disk. -/// Returns a DumpFile handle; the file is deleted when the handle is dropped. -pub(crate) async fn pg_dump_database( - pg_db: &PgDatabase, - opts: PgDumpOptions<'_>, -) -> Result { - let dump_file = DumpFile::new()?; - - let (mut cmd, _root_cert) = pg_dump_command(pg_db, opts, &dump_file.path)?; + let _root_cert = apply_pg_tls_env(&mut cmd, pg_db)?; let output = cmd .output() @@ -3640,14 +3619,12 @@ fn psql_command(pg_db: &PgDatabase) -> Result<(tokio::process::Command, Option PgDatabase { - PgDatabase { - host: "127.0.0.1".to_string(), - user: None, - password: None, - port: Some(1), - sslmode: None, - dbname: dbname.to_string(), - root_certificate_pem: None, - accept_invalid_certs: None, - use_iam_auth: None, - region: None, - options: None, - } - } - - fn pgdatabase_of(cmd: &std::process::Command) -> Option { - cmd.get_envs() - .find(|(k, _)| *k == "PGDATABASE") - .and_then(|(_, v)| v.map(|v| v.to_os_string())) - } - - #[test] - fn pg_dump_takes_the_dbname_from_the_environment() { - let dump_file = DumpFile::new().unwrap(); - let name = "--file=/tmp/written_by_the_caller"; - let (cmd, _root_cert) = - pg_dump_command(&pg_db(name), PgDumpOptions::default(), &dump_file.path).unwrap(); - let cmd = cmd.as_std(); - assert!( - cmd.get_args().all(|arg| arg != name), - "the dbname is still in pg_dump's argument vector" - ); - assert_eq!(pgdatabase_of(cmd).as_deref(), Some(name.as_ref())); - // The only `--file` pg_dump is given is still Windmill's own. - assert_eq!( - cmd.get_args() - .filter(|arg| arg.to_string_lossy().starts_with("--file")) - .count(), - 1 - ); - } - - #[test] - fn psql_takes_the_dbname_from_the_environment() { - let name = "host=elsewhere"; - let (cmd, _root_cert) = psql_command(&pg_db(name)).unwrap(); - let cmd = cmd.as_std(); - assert!( - cmd.get_args().all(|arg| arg != name), - "the dbname is still in psql's argument vector" - ); - assert_eq!(pgdatabase_of(cmd).as_deref(), Some(name.as_ref())); - } - - /// The same, observed on the real binary: an option-shaped name must reach the connection - /// attempt rather than pg_dump's option parser. Skipped where pg_dump is not installed — - /// which is why the assertion also requires that pg_dump ran at all (our `pg_dump failed:` - /// prefix) instead of only that the option message is absent. - #[tokio::test] - async fn pg_dump_does_not_read_an_option_shaped_dbname_as_an_option() { - if std::process::Command::new("pg_dump") - .arg("--version") - .output() - .is_err() - { - eprintln!("skipped: pg_dump is not installed"); - return; - } - let Err(err) = pg_dump_database(&pg_db("--no-such-option"), PgDumpOptions::default()).await - else { - panic!("pg_dump reached a server on 127.0.0.1:1"); - }; - let err = err.to_string(); - assert!( - err.contains("pg_dump failed:"), - "pg_dump did not run: {err}" - ); - assert!(!err.contains("unrecognized option"), "{err}"); - } -} - /// GUC names the server backing `pg_db` knows about. /// /// Asked through psql rather than a tokio-postgres connection so the lookup reaches @@ -4175,25 +4061,6 @@ fn ensure_copied_without_roles(governing: &GoverningDatatable) -> Result<()> { Ok(()) } -/// Gate for the endpoints that make the API server run `pg_dump`/`psql` against a database a -/// request names. -/// -/// Those two processes are children of the API server, outside any job sandbox and holding its -/// environment, and the dump they exchange is a `psql` script assembled from strings the source -/// server returns — so reaching them is close enough to running code on the server that the role -/// which may not run code at all must not reach them either. `code` is the capability the data -/// table migration endpoints next door already ask for (`datatable_migrations::ensure_not_operator`), -/// and not admin: the database manager offers an export and an import of a user's own Postgres -/// resource to every author, who can run the same `pg_dump` in a job anyway. -async fn ensure_can_dump_databases( - db: &DB, - w_id: &str, - authed: &ApiAuthed, - action: &str, -) -> Result<()> { - windmill_common::workspaces::check_role_can_code(db, w_id, authed, action).await -} - /// Import (pg_dump/pg_import) from source to target async fn import_pg_database( authed: ApiAuthed, @@ -4202,8 +4069,6 @@ async fn import_pg_database( Path(w_id): Path, Json(req): Json, ) -> Result { - ensure_can_dump_databases(&db, &w_id, &authed, "import a database").await?; - if req.fork_behavior == DataTableForkBehavior::KeepOriginal { return Ok("No action needed for KeepOriginal behavior".to_string()); } @@ -4320,8 +4185,6 @@ async fn export_pg_schema( Path(w_id): Path, Json(req): Json, ) -> Result { - ensure_can_dump_databases(&db, &w_id, &authed, "export a database schema").await?; - let pg = resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.source).await?; let dump_file = pg_dump_database( &pg, diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 773e407372..575306fe95 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -740,10 +740,10 @@ async fn list_apps( if let Some(label) = &lq.label { for l in label.split(',') { + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(app.labels @> ARRAY[?] OR folder_labels(app.workspace_id, app.path) @> ARRAY[?])" - .bind(&l.trim()) - .bind(&l.trim()), + .binds(&[&l.trim()]), ); } } diff --git a/backend/windmill-api/src/mcp/utils.rs b/backend/windmill-api/src/mcp/utils.rs index dcf84e1851..6e125eefc6 100644 --- a/backend/windmill-api/src/mcp/utils.rs +++ b/backend/windmill-api/src/mcp/utils.rs @@ -183,8 +183,8 @@ fn items_query( let mut sqlb = SqlBuilder::select_from(&format!("{} as o", item_type)); if scope_type == "favorites" { sqlb.join("favorite") - .on("favorite.favorite_kind = ? AND favorite.workspace_id = o.workspace_id AND favorite.path = o.path AND favorite.usr = ?".bind(&item_type) - .bind(&authed.username)); + .on("favorite.favorite_kind = ? AND favorite.workspace_id = o.workspace_id AND favorite.path = o.path AND favorite.usr = ?" + .binds(&[&item_type, &authed.username])); } sqlb.and_where("o.workspace_id = ?".bind(&workspace_id)) .and_where("o.archived = false"); diff --git a/backend/windmill-api/src/raw_apps.rs b/backend/windmill-api/src/raw_apps.rs index 35e4f693a4..ad53872ca0 100644 --- a/backend/windmill-api/src/raw_apps.rs +++ b/backend/windmill-api/src/raw_apps.rs @@ -98,10 +98,10 @@ async fn list_apps( if let Some(label) = &lq.label { for l in label.split(',') { + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(app.labels @> ARRAY[?] OR folder_labels(app.workspace_id, app.path) @> ARRAY[?])" - .bind(&l.trim()) - .bind(&l.trim()), + .binds(&[&l.trim()]), ); } } diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index 2d60008373..4e1524580e 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -843,50 +843,6 @@ mod validate_dbname_tests { } } -#[cfg(test)] -mod validate_dbname_not_option_like_tests { - use super::validate_dbname_not_option_like; - - /// A source is any database a user already has, so everything `validate_dbname` refuses but - /// Postgres accepts has to keep working. - #[test] - fn accepts_every_name_that_is_not_an_option() { - for name in [ - "mydb", - "My-Db_1", - "2024_archive", - "my.db", - "db with spaces", - "Grüße", - "host=elsewhere port=5433", - "", - "a-dash-inside--it", - ] { - assert!( - validate_dbname_not_option_like(name).is_ok(), - "refused the legitimate name {name:?}" - ); - } - } - - #[test] - fn rejects_a_name_that_an_argument_vector_would_read_as_an_option() { - for name in [ - "--file=/tmp/x", - "--restrict-key=k", - "-f", - "-", - " --file=/tmp/x", - "\t-f", - ] { - let err = validate_dbname_not_option_like(name) - .expect_err(&format!("accepted the option-like name {name:?}")) - .to_string(); - assert!(err.contains("cannot start with '-'"), "{err}"); - } - } -} - #[cfg(test)] mod pg_tls_tests { use super::PgDatabase; @@ -1639,26 +1595,6 @@ pub fn validate_dbname(dbname: &str) -> error::Result<()> { Ok(()) } -/// The one restriction on the name of a database Windmill only *reads from*: it may not look like -/// a command-line option. -/// -/// [`validate_dbname`] is the rule for a name Windmill creates or interpolates into SQL, and is -/// far too narrow here: a source is any Postgres resource a user already has, and an existing -/// database may legitimately be named `2024_archive`, `my.db`, `Grüße` or anything else Postgres -/// accepts as a quoted identifier. Refusing those would break working resources. A name opening -/// with `-` is the one shape that is never a database and always a mistake or an attempt to smuggle -/// an option into a `pg_dump`/`psql` argument vector, so it is refused wherever a source is -/// resolved — the commands themselves pass the name through `PGDATABASE` and never on the command -/// line, and this keeps a future caller that forgets from re-opening that door. -pub fn validate_dbname_not_option_like(dbname: &str) -> error::Result<()> { - if dbname.trim_start().starts_with('-') { - return Err(error::Error::BadRequest( - "Database name cannot start with '-'".to_string(), - )); - } - Ok(()) -} - /// Lock the instance databases among `names` until `tx` ends, in a stable order. Taken by every /// settings save naming an instance database, and by [`drop_unused_instance_database`]: a save /// cannot start using a database between that drop's check that nothing does and the drop. diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index 9af92361f6..541369f327 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -439,9 +439,10 @@ async fn list_resources( if let Some(broad_filter) = &lq.broad_filter { let pat = format!("%{}%", escape_ilike_pattern(broad_filter)); + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(resource.path ILIKE ? OR resource.description ILIKE ? OR resource_type ILIKE ? OR resource.value::text ILIKE ?)" - .bind(&pat).bind(&pat).bind(&pat).bind(&pat) + .binds(&[&pat]), ); } @@ -455,10 +456,10 @@ async fn list_resources( if let Some(label) = &lq.label { for l in label.split(',') { + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(resource.labels @> ARRAY[?] OR folder_labels(resource.workspace_id, resource.path) @> ARRAY[?])" - .bind(&l.trim()) - .bind(&l.trim()), + .binds(&[&l.trim()]), ); } } diff --git a/backend/windmill-store/src/variables.rs b/backend/windmill-store/src/variables.rs index 7d85fa59ce..e9aa3ab60d 100644 --- a/backend/windmill-store/src/variables.rs +++ b/backend/windmill-store/src/variables.rs @@ -200,18 +200,19 @@ async fn list_variables( if let Some(broad_filter) = &lq.broad_filter { let pat = format!("%{}%", escape_ilike_pattern(broad_filter)); + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(variable.path ILIKE ? OR variable.description ILIKE ? OR (is_secret = FALSE AND variable.value ILIKE ?))" - .bind(&pat).bind(&pat).bind(&pat) + .binds(&[&pat]), ); } if let Some(label) = &lq.label { for l in label.split(',') { + // `binds` fills every `?` in one pass; chained `bind`s would re-scan a `?` in the bound value sqlb.and_where( "(variable.labels @> ARRAY[?] OR folder_labels(variable.workspace_id, variable.path) @> ARRAY[?])" - .bind(&l.trim()) - .bind(&l.trim()), + .binds(&[&l.trim()]), ); } }