mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-09 16:05:42 +00:00
fix(ai): enforce RLS and scope check on user-supplied X-Resource-Path (#9276)
* fix(ai): enforce RLS and scope check on user-supplied X-Resource-Path
The AI proxy handler accepts an X-Resource-Path header to override the
configured workspace AI provider. When supplied, the handler loaded the
resource value from the resource table using the root DB pool with no
resources:read scope check, so any authenticated workspace user could
point X-Resource-Path at a restricted AI resource (e.g. one in a folder
they cannot read) and the proxy would use that resource's provider
credentials for the outbound AI request.
For user-supplied resource paths, now require resources:read:{path}
scope and fetch the resource through user_db.begin(&authed) so RLS
enforces the same folder/group boundary as the resource API. The RLS-
scoped $var: resolution stays in place as defense in depth. The
admin-configured workspace/instance ai_config path is unchanged.
Fixes WIN-1971
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(ai): regression test for X-Resource-Path RLS enforcement
Cover all four cases:
- non-admin pointing X-Resource-Path at a restricted resource is rejected
- non-admin pointing it at a resource they own still works
- admin can point it at any resource
- workspace-configured proxy flow (no X-Resource-Path) is unchanged
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
Diego Imbert
co-authored by
Claude Opus 4.7
parent
cbbff01fbe
commit
7836a4e733
@@ -10,6 +10,10 @@ fn authed(builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder {
|
|||||||
builder.header("Authorization", "Bearer SECRET_TOKEN")
|
builder.header("Authorization", "Bearer SECRET_TOKEN")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn authed_with(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuilder {
|
||||||
|
builder.header("Authorization", format!("Bearer {token}"))
|
||||||
|
}
|
||||||
|
|
||||||
fn assert_2xx(status: u16, body: &str, endpoint: &str) {
|
fn assert_2xx(status: u16, body: &str, endpoint: &str) {
|
||||||
assert!(
|
assert!(
|
||||||
(200..300).contains(&status),
|
(200..300).contains(&status),
|
||||||
@@ -106,3 +110,125 @@ async fn test_ai_proxy_endpoints(db: Pool<Postgres>) -> anyhow::Result<()> {
|
|||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Regression test for WIN-1971: the AI proxy's X-Resource-Path header must
|
||||||
|
/// honour resource RLS so that a low-privilege user cannot point the proxy
|
||||||
|
/// at a resource they are not allowed to read.
|
||||||
|
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||||
|
async fn test_ai_proxy_x_resource_path_enforces_rls(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||||
|
initialize_tracing().await;
|
||||||
|
std::env::set_var("ALLOW_PRIVATE_AI_BASE_URLS", "true");
|
||||||
|
let server = ApiServer::start(db.clone()).await?;
|
||||||
|
let port = server.addr.port();
|
||||||
|
|
||||||
|
let mock_port = start_mock_ai_api().await;
|
||||||
|
let mock_url = format!("http://127.0.0.1:{mock_port}/v1");
|
||||||
|
|
||||||
|
// Resource owned by test-user (admin). With default extra_perms {} the
|
||||||
|
// RLS `see_own` policy restricts SELECT to user `test-user`.
|
||||||
|
sqlx::query(
|
||||||
|
"INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by) \
|
||||||
|
VALUES ('test-workspace', 'u/test-user/restricted_openai', $1::jsonb, 'openai', '{}', 'test-user')",
|
||||||
|
)
|
||||||
|
.bind(json!({
|
||||||
|
"api_key": "sk-secret-restricted",
|
||||||
|
"base_url": mock_url,
|
||||||
|
}))
|
||||||
|
.execute(&db)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
// Sanity-check: normal resource API rejects test-user-3 (non-admin) for the restricted path.
|
||||||
|
let resp = authed_with(
|
||||||
|
client().get(format!(
|
||||||
|
"http://localhost:{port}/api/w/test-workspace/resources/get/u/test-user/restricted_openai"
|
||||||
|
)),
|
||||||
|
"SECRET_TOKEN_3",
|
||||||
|
)
|
||||||
|
.send()
|
||||||
|
.await?;
|
||||||
|
assert!(
|
||||||
|
resp.status().as_u16() >= 400,
|
||||||
|
"normal resource API should deny test-user-3 reading restricted resource, got {}",
|
||||||
|
resp.status()
|
||||||
|
);
|
||||||
|
|
||||||
|
// The vulnerability: as a non-admin user, point X-Resource-Path at the
|
||||||
|
// restricted resource. Must be rejected before the proxy fetches/uses it.
|
||||||
|
let resp = authed_with(
|
||||||
|
client()
|
||||||
|
.post(format!(
|
||||||
|
"http://localhost:{port}/api/w/test-workspace/ai/proxy/chat/completions"
|
||||||
|
))
|
||||||
|
.header("X-Provider", "openai")
|
||||||
|
.header("X-Resource-Path", "u/test-user/restricted_openai")
|
||||||
|
.json(&json!({
|
||||||
|
"model": "gpt-4",
|
||||||
|
"messages": [{"role": "user", "content": "hi"}]
|
||||||
|
})),
|
||||||
|
"SECRET_TOKEN_3",
|
||||||
|
)
|
||||||
|
.send()
|
||||||
|
.await?;
|
||||||
|
let status = resp.status().as_u16();
|
||||||
|
let body = resp.text().await?;
|
||||||
|
assert!(
|
||||||
|
status >= 400,
|
||||||
|
"non-admin user should be rejected when X-Resource-Path points at a resource they cannot read, got {status}: {body}",
|
||||||
|
);
|
||||||
|
|
||||||
|
// A resource the non-admin owns must still work through X-Resource-Path.
|
||||||
|
sqlx::query(
|
||||||
|
"INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by) \
|
||||||
|
VALUES ('test-workspace', 'u/test-user-3/own_openai', $1::jsonb, 'openai', '{}', 'test-user-3')",
|
||||||
|
)
|
||||||
|
.bind(json!({
|
||||||
|
"api_key": "sk-self",
|
||||||
|
"base_url": mock_url,
|
||||||
|
}))
|
||||||
|
.execute(&db)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
let resp = authed_with(
|
||||||
|
client()
|
||||||
|
.post(format!(
|
||||||
|
"http://localhost:{port}/api/w/test-workspace/ai/proxy/chat/completions"
|
||||||
|
))
|
||||||
|
.header("X-Provider", "openai")
|
||||||
|
.header("X-Resource-Path", "u/test-user-3/own_openai")
|
||||||
|
.json(&json!({
|
||||||
|
"model": "gpt-4",
|
||||||
|
"messages": [{"role": "user", "content": "hi"}]
|
||||||
|
})),
|
||||||
|
"SECRET_TOKEN_3",
|
||||||
|
)
|
||||||
|
.send()
|
||||||
|
.await?;
|
||||||
|
assert_2xx(
|
||||||
|
resp.status().as_u16(),
|
||||||
|
&resp.text().await?,
|
||||||
|
"non-admin with X-Resource-Path on owned resource",
|
||||||
|
);
|
||||||
|
|
||||||
|
// Admin must still be able to use X-Resource-Path on any resource.
|
||||||
|
let resp = authed(
|
||||||
|
client()
|
||||||
|
.post(format!(
|
||||||
|
"http://localhost:{port}/api/w/test-workspace/ai/proxy/chat/completions"
|
||||||
|
))
|
||||||
|
.header("X-Provider", "openai")
|
||||||
|
.header("X-Resource-Path", "u/test-user/restricted_openai")
|
||||||
|
.json(&json!({
|
||||||
|
"model": "gpt-4",
|
||||||
|
"messages": [{"role": "user", "content": "hi"}]
|
||||||
|
})),
|
||||||
|
)
|
||||||
|
.send()
|
||||||
|
.await?;
|
||||||
|
assert_2xx(
|
||||||
|
resp.status().as_u16(),
|
||||||
|
&resp.text().await?,
|
||||||
|
"admin with X-Resource-Path on restricted resource",
|
||||||
|
);
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
#[cfg(feature = "bedrock")]
|
#[cfg(feature = "bedrock")]
|
||||||
use crate::bedrock;
|
use crate::bedrock;
|
||||||
use crate::db::{ApiAuthed, DB};
|
use crate::db::{ApiAuthed, DB};
|
||||||
|
use crate::utils::check_scopes;
|
||||||
|
|
||||||
#[cfg(feature = "bedrock")]
|
#[cfg(feature = "bedrock")]
|
||||||
use axum::routing::get;
|
use axum::routing::get;
|
||||||
@@ -669,6 +670,7 @@ async fn global_proxy(
|
|||||||
async fn proxy(
|
async fn proxy(
|
||||||
authed: ApiAuthed,
|
authed: ApiAuthed,
|
||||||
Extension(db): Extension<DB>,
|
Extension(db): Extension<DB>,
|
||||||
|
Extension(user_db): Extension<UserDB>,
|
||||||
Path((w_id, mut ai_path)): Path<(String, String)>,
|
Path((w_id, mut ai_path)): Path<(String, String)>,
|
||||||
method: Method,
|
method: Method,
|
||||||
headers: HeaderMap,
|
headers: HeaderMap,
|
||||||
@@ -689,6 +691,16 @@ async fn proxy(
|
|||||||
.get("X-Resource-Path")
|
.get("X-Resource-Path")
|
||||||
.map(|v| v.to_str().unwrap_or("").to_string());
|
.map(|v| v.to_str().unwrap_or("").to_string());
|
||||||
let is_user_specified_resource = forced_resource_path.is_some();
|
let is_user_specified_resource = forced_resource_path.is_some();
|
||||||
|
|
||||||
|
// When the caller supplies X-Resource-Path, the resource is treated as if it
|
||||||
|
// were being read through the normal resource API: scope and RLS checks must
|
||||||
|
// apply so that a low-privilege user cannot point the proxy at a restricted
|
||||||
|
// AI resource (e.g. one in a folder they cannot read) to exfiltrate the
|
||||||
|
// resource's provider credentials or use them via the proxy.
|
||||||
|
if let Some(resource_path) = forced_resource_path.as_ref() {
|
||||||
|
check_scopes(&authed, || format!("resources:read:{}", resource_path))?;
|
||||||
|
}
|
||||||
|
|
||||||
let request_config = match workspace_cache {
|
let request_config = match workspace_cache {
|
||||||
Some(request_cache) if !request_cache.is_expired() && forced_resource_path.is_none() => {
|
Some(request_cache) if !request_cache.is_expired() && forced_resource_path.is_none() => {
|
||||||
request_cache.config
|
request_cache.config
|
||||||
@@ -759,13 +771,32 @@ async fn proxy(
|
|||||||
)
|
)
|
||||||
};
|
};
|
||||||
|
|
||||||
let resource = sqlx::query_scalar::<_, Option<sqlx::types::Json<Box<RawValue>>>>(
|
// For user-specified resources, fetch through an RLS-scoped
|
||||||
"SELECT value FROM resource WHERE path = $1 AND workspace_id = $2",
|
// connection so PostgreSQL row-level security enforces the same
|
||||||
)
|
// folder/group boundaries as the regular resource API. For the
|
||||||
.bind(&resource_path)
|
// workspace/instance ai_config path, the resource_path was already
|
||||||
.bind(&resource_workspace)
|
// validated by an admin/devops user when configuring the workspace,
|
||||||
.fetch_optional(&db)
|
// so the raw pool is used.
|
||||||
.await?
|
let resource = if is_user_specified_resource {
|
||||||
|
let mut tx = user_db.clone().begin(&authed).await?;
|
||||||
|
let res = sqlx::query_scalar::<_, Option<sqlx::types::Json<Box<RawValue>>>>(
|
||||||
|
"SELECT value FROM resource WHERE path = $1 AND workspace_id = $2",
|
||||||
|
)
|
||||||
|
.bind(&resource_path)
|
||||||
|
.bind(&resource_workspace)
|
||||||
|
.fetch_optional(&mut *tx)
|
||||||
|
.await?;
|
||||||
|
tx.commit().await?;
|
||||||
|
res
|
||||||
|
} else {
|
||||||
|
sqlx::query_scalar::<_, Option<sqlx::types::Json<Box<RawValue>>>>(
|
||||||
|
"SELECT value FROM resource WHERE path = $1 AND workspace_id = $2",
|
||||||
|
)
|
||||||
|
.bind(&resource_path)
|
||||||
|
.bind(&resource_workspace)
|
||||||
|
.fetch_optional(&db)
|
||||||
|
.await?
|
||||||
|
}
|
||||||
.ok_or_else(|| Error::NotFound(format!("Could not find the resource {}, update the resource path in the workspace settings", resource_path)))?
|
.ok_or_else(|| Error::NotFound(format!("Could not find the resource {}, update the resource path in the workspace settings", resource_path)))?
|
||||||
.ok_or_else(|| Error::BadRequest(format!("Empty resource value for {}", resource_path)))?;
|
.ok_or_else(|| Error::BadRequest(format!("Empty resource value for {}", resource_path)))?;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user