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:
Ruben Fiszel
2026-04-03 20:55:57 -04:00
committed by GitHub
parent f394e674f2
commit ff8e39c69b
5 changed files with 113 additions and 52 deletions
@@ -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"
}
@@ -15,7 +15,7 @@
]
},
"nullable": [
null
true
]
},
"hash": "5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55"
@@ -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"
}
+62 -18
View File
@@ -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()),
+48 -2
View File
@@ -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
)))
}
}