diff --git a/backend/.sqlx/query-2fcddda99dd0aacf5007ed459cb27caa754424e062427edf5ddcb95f9d96888e.json b/backend/.sqlx/query-2fcddda99dd0aacf5007ed459cb27caa754424e062427edf5ddcb95f9d96888e.json deleted file mode 100644 index 5a1fbca834..0000000000 --- a/backend/.sqlx/query-2fcddda99dd0aacf5007ed459cb27caa754424e062427edf5ddcb95f9d96888e.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT path, value FROM variable\n WHERE path LIKE ('u/' || $1 || '/%')\n AND workspace_id = $2\n AND is_secret = true\n AND value LIKE '$vault:%'", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "path", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "value", - "type_info": "Varchar" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - false, - false - ] - }, - "hash": "2fcddda99dd0aacf5007ed459cb27caa754424e062427edf5ddcb95f9d96888e" -} diff --git a/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json b/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json index 713ccb9dd3..36ddb8ab9f 100644 --- a/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json +++ b/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json @@ -15,7 +15,7 @@ ] }, "nullable": [ - null + true ] }, "hash": "5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55" diff --git a/backend/.sqlx/query-a5a03b9235b25bca359235f2e546197f02ca1cf898d7f2686419749b1cb0679e.json b/backend/.sqlx/query-61f3627cbd2a50b745c4e491ed8ee71e58db22b7f53202c5360a01c92091fc08.json similarity index 67% rename from backend/.sqlx/query-a5a03b9235b25bca359235f2e546197f02ca1cf898d7f2686419749b1cb0679e.json rename to backend/.sqlx/query-61f3627cbd2a50b745c4e491ed8ee71e58db22b7f53202c5360a01c92091fc08.json index 3bcb7d31de..f97d5e310a 100644 --- a/backend/.sqlx/query-a5a03b9235b25bca359235f2e546197f02ca1cf898d7f2686419749b1cb0679e.json +++ b/backend/.sqlx/query-61f3627cbd2a50b745c4e491ed8ee71e58db22b7f53202c5360a01c92091fc08.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT value, is_secret \n FROM variable \n WHERE path = $1 AND workspace_id = $2", + "query": "SELECT value, is_secret\n FROM variable\n WHERE path = $1 AND workspace_id = $2", "describe": { "columns": [ { @@ -25,5 +25,5 @@ false ] }, - "hash": "a5a03b9235b25bca359235f2e546197f02ca1cf898d7f2686419749b1cb0679e" + "hash": "61f3627cbd2a50b745c4e491ed8ee71e58db22b7f53202c5360a01c92091fc08" } diff --git a/backend/windmill-api/src/ai.rs b/backend/windmill-api/src/ai.rs index 16cfb0166c..0e1cdf11fa 100644 --- a/backend/windmill-api/src/ai.rs +++ b/backend/windmill-api/src/ai.rs @@ -20,9 +20,10 @@ use windmill_common::ai_cache::current_instance_ai_config_revision; use windmill_common::ai_providers::{ empty_string_as_none, AIPlatform, AIProvider, ProviderConfig, ProviderModel, }; +use windmill_common::db::UserDB; use windmill_common::error::{to_anyhow, Error, Result}; use windmill_common::utils::configure_client; -use windmill_common::variables::get_variable_or_self; +use windmill_common::variables::{get_variable_or_self, get_variable_or_self_as}; // AI timeout configuration constants const AI_TIMEOUT_MIN_SECS: u64 = 1; @@ -211,13 +212,35 @@ struct AIRequestConfig { pub custom_headers: HashMap, } +/// Resolve a `$var:` reference. When `user_db`/`authed` are provided the query +/// goes through an RLS-scoped connection so the caller can only read variables +/// they are authorised to access. Without auth context the raw pool is used +/// (appropriate for admin/system paths where the resource was already validated). +async fn resolve_var( + path: String, + db: &DB, + w_id: &str, + user_db: Option<&UserDB>, + authed: Option<&ApiAuthed>, +) -> Result { + match (user_db, authed) { + (Some(udb), Some(auth)) => Ok(get_variable_or_self_as(path, db, udb, auth, w_id).await?), + _ => Ok(get_variable_or_self(path, db, w_id).await?), + } +} + impl AIRequestConfig { pub async fn new( provider: &AIProvider, db: &DB, w_id: &str, resource: AIResource, + authed: Option<&ApiAuthed>, ) -> Result { + // When authed is provided, resolve $var: references through RLS so that + // users can only read variables they have permission to access. + let user_db = authed.map(|_| UserDB::new(db.clone())); + let ( api_key, access_token, @@ -244,28 +267,29 @@ impl AIRequestConfig { provider.get_base_url(resource.base_url, db).await? }; let api_key = if let Some(api_key) = resource.api_key { - Some(get_variable_or_self(api_key, db, w_id).await?) + Some(resolve_var(api_key, db, w_id, user_db.as_ref(), authed).await?) } else { None }; let organization_id = if let Some(organization_id) = resource.organization_id { - Some(get_variable_or_self(organization_id, db, w_id).await?) + Some(resolve_var(organization_id, db, w_id, user_db.as_ref(), authed).await?) } else { None }; let aws_access_key_id = if let Some(access_key_id) = resource.aws_access_key_id { - Some(get_variable_or_self(access_key_id, db, w_id).await?) + Some(resolve_var(access_key_id, db, w_id, user_db.as_ref(), authed).await?) + } else { + None + }; + let aws_secret_access_key = if let Some(secret_access_key) = + resource.aws_secret_access_key + { + Some(resolve_var(secret_access_key, db, w_id, user_db.as_ref(), authed).await?) } else { None }; - let aws_secret_access_key = - if let Some(secret_access_key) = resource.aws_secret_access_key { - Some(get_variable_or_self(secret_access_key, db, w_id).await?) - } else { - None - }; let aws_session_token = if let Some(session_token) = resource.aws_session_token { - Some(get_variable_or_self(session_token, db, w_id).await?) + Some(resolve_var(session_token, db, w_id, user_db.as_ref(), authed).await?) } else { None }; @@ -287,11 +311,13 @@ impl AIRequestConfig { } AIResource::OAuth(resource) => { let user = if let Some(user) = resource.user.clone() { - Some(get_variable_or_self(user, db, w_id).await?) + Some(resolve_var(user, db, w_id, user_db.as_ref(), authed).await?) } else { None }; - let token = Self::get_token_using_oauth(resource, db, w_id).await?; + let token = + Self::get_token_using_oauth(resource, db, w_id, user_db.as_ref(), authed) + .await?; let base_url = provider.get_base_url(None, db).await?; ( @@ -331,10 +357,13 @@ impl AIRequestConfig { mut resource: AIOAuthResource, db: &DB, w_id: &str, + user_db: Option<&UserDB>, + authed: Option<&ApiAuthed>, ) -> Result { - resource.client_id = get_variable_or_self(resource.client_id, db, w_id).await?; - resource.client_secret = get_variable_or_self(resource.client_secret, db, w_id).await?; - resource.token_url = get_variable_or_self(resource.token_url, db, w_id).await?; + resource.client_id = resolve_var(resource.client_id, db, w_id, user_db, authed).await?; + resource.client_secret = + resolve_var(resource.client_secret, db, w_id, user_db, authed).await?; + resource.token_url = resolve_var(resource.token_url, db, w_id, user_db, authed).await?; let mut params = HashMap::new(); params.insert("grant_type", "client_credentials"); params.insert("scope", "https://cognitiveservices.azure.com/.default"); @@ -778,6 +807,7 @@ async fn proxy( let forced_resource_path = headers .get("X-Resource-Path") .map(|v| v.to_str().unwrap_or("").to_string()); + let is_user_specified_resource = forced_resource_path.is_some(); let request_config = match workspace_cache { Some(request_cache) if !request_cache.is_expired() && forced_resource_path.is_none() => { request_cache.config @@ -861,8 +891,22 @@ async fn proxy( let resource = serde_json::from_str::(resource.0.get()) .map_err(|e| Error::BadRequest(e.to_string()))?; - let request_config = - AIRequestConfig::new(&provider, &db, &resource_workspace, resource).await?; + // Enforce RLS on $var: resolution when the resource path was + // user-specified (X-Resource-Path header) so users can only read + // variables they have permission to access. + let enforce_authed = if is_user_specified_resource { + Some(&authed) + } else { + None + }; + let request_config = AIRequestConfig::new( + &provider, + &db, + &resource_workspace, + resource, + enforce_authed, + ) + .await?; if save_to_cache { AI_REQUEST_CACHE.insert( (w_id.clone(), provider.clone()), diff --git a/backend/windmill-common/src/variables.rs b/backend/windmill-common/src/variables.rs index 5d395bf221..68c1ac36fd 100644 --- a/backend/windmill-common/src/variables.rs +++ b/backend/windmill-common/src/variables.rs @@ -6,6 +6,7 @@ * LICENSE-AGPL for a copy of the license. */ +use crate::db::{Authable, UserDB}; use crate::error::{self, Error}; use crate::scripts::ScriptHash; use crate::utils::WarnAfterExt; @@ -473,8 +474,8 @@ pub async fn get_variable_or_self( let path = path.strip_prefix("$var:").unwrap().to_string(); let record = sqlx::query!( - "SELECT value, is_secret - FROM variable + "SELECT value, is_secret + FROM variable WHERE path = $1 AND workspace_id = $2", &path, &w_id @@ -499,3 +500,48 @@ pub async fn get_variable_or_self( ))) } } + +/// Like `get_variable_or_self`, but uses an RLS-scoped connection to enforce +/// that the caller has read access to the referenced variable. +pub async fn get_variable_or_self_as( + path: String, + db: &DB, + user_db: &UserDB, + authed: &T, + w_id: &str, +) -> crate::error::Result { + if !path.starts_with("$var:") { + return Ok(path); + } + let var_path = path.strip_prefix("$var:").unwrap().to_string(); + + // Use an RLS-scoped transaction so the query respects row-level security + let mut tx = user_db.clone().begin(authed).await?; + let record = sqlx::query!( + "SELECT value, is_secret + FROM variable + WHERE path = $1 AND workspace_id = $2", + &var_path, + &w_id + ) + .fetch_optional(&mut *tx) + .await?; + tx.commit().await?; + + if let Some(record) = record { + let mut value = record.value; + if record.is_secret { + let mc = build_crypt(db, w_id).await?; + value = decrypt(&mc, value).map_err(|e| { + Error::internal_err(format!("Error decrypting variable {}: {}", var_path, e)) + })?; + } + + Ok(value) + } else { + Err(Error::NotFound(format!( + "Variable not found when resolving `$var:{}`", + var_path + ))) + } +}