From e3b36fceadcbc16d53dc8a6afc88f8ccbba6664f Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Fri, 3 Jul 2026 20:08:38 +0200 Subject: [PATCH] fix(datatable-migrations): never resolve instance credentials into migration job args datatable_database_arg eagerly resolved instance data-table credentials (including the shared instance-wide Postgres password) and passed them as the migration job's plaintext `database` arg, landing in v2_job.args. Since the run route has no admin gate, a non-admin could run a migration and read args.database to recover the password, granting cross-workspace psql access to all instance data-table DBs. Pass a `datatable://` reference for both resource-backed and instance data tables instead; the pg executor already resolves it to real credentials server-side at run time, so nothing sensitive is ever stored in the job args. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/datatable_migrations.rs | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index 93f6674441..5494e08c86 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -33,9 +33,7 @@ use windmill_common::runnable_settings::{ConcurrencySettingsWithCustom, Debounci use windmill_common::scripts::ScriptLang; use windmill_common::users::username_to_permissioned_as; use windmill_common::worker::to_raw_value; -use windmill_common::workspaces::{ - get_datatable_resource_from_db_unchecked, DataTable, DataTableCatalogResourceType, -}; +use windmill_common::workspaces::get_datatable_resource_from_db_unchecked; use windmill_common::{PgDatabase, DB}; use windmill_git_sync::{handle_deployment_metadata, DeployedObject}; use windmill_queue::{push, PushArgs, PushIsolationLevel}; @@ -102,15 +100,19 @@ struct RunDatatableMigrationsQuery { only: Option, } -/// Build the `database` argument for a migration job. Resource-backed data -/// tables pass a `$res:` reference (resolved + redacted by the worker); instance -/// data tables have no resource, so the resolved credentials are passed. +/// Build the `database` argument for a migration job. Both resource-backed and +/// instance data tables pass a `datatable://` reference; the pg executor +/// resolves it to real credentials server-side at run time. It must never be +/// resolved here: the resolved instance credentials include a single +/// instance-wide Postgres password, and the job's `args` are readable by the — +/// possibly non-admin — user who ran the migration. async fn datatable_database_arg( db: &DB, w_id: &str, datatable_name: &str, ) -> Result> { - let config = sqlx::query_scalar!( + // Fail fast with a clear error if the data table doesn't exist. + sqlx::query_scalar!( "SELECT ws.datatable->'datatables'->$2 FROM workspace_settings ws WHERE ws.workspace_id = $1", w_id, datatable_name, @@ -118,18 +120,8 @@ async fn datatable_database_arg( .fetch_one(db) .await? .ok_or_else(|| Error::internal_err(format!("datatable {datatable_name} not found")))?; - let datatable: DataTable = serde_json::from_value(config)?; - match datatable.database.resource_type { - DataTableCatalogResourceType::Postgresql => Ok(to_raw_value(&format!( - "$res:{}", - datatable.database.resource_path - ))), - DataTableCatalogResourceType::Instance => { - let resolved = - get_datatable_resource_from_db_unchecked(db, w_id, datatable_name).await?; - Ok(to_raw_value(&resolved)) - } - } + + Ok(to_raw_value(&format!("datatable://{datatable_name}"))) } /// Run a migration's SQL as a normal Windmill `postgresql` job, permissioned as