mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix: enforce RLS on $var: resolution in AI proxy (GHSA-jwg4-v3cj-rvfm) (#8713)
* fix: enforce RLS on $var: resolution in AI proxy to prevent secret exfiltration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update sqlx prepared queries Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
-29
@@ -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"
|
||||
}
|
||||
+1
-1
@@ -15,7 +15,7 @@
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
null
|
||||
true
|
||||
]
|
||||
},
|
||||
"hash": "5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55"
|
||||
|
||||
+2
-2
@@ -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"
|
||||
}
|
||||
@@ -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<String, String>,
|
||||
}
|
||||
|
||||
/// 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<String> {
|
||||
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<Self> {
|
||||
// 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<String> {
|
||||
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::<AIResource>(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()),
|
||||
|
||||
@@ -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<T: Authable + Sync>(
|
||||
path: String,
|
||||
db: &DB,
|
||||
user_db: &UserDB,
|
||||
authed: &T,
|
||||
w_id: &str,
|
||||
) -> crate::error::Result<String> {
|
||||
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
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user