From 0692b97c8a3818549d7050ea3e057e9cbf1ddb44 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 21 May 2026 15:21:27 +0000 Subject: [PATCH] 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) * 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) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../tests/ai_routes.rs | 126 ++++++++++++++++++ backend/windmill-api/src/ai.rs | 45 ++++++- 2 files changed, 164 insertions(+), 7 deletions(-) diff --git a/backend/windmill-api-integration-tests/tests/ai_routes.rs b/backend/windmill-api-integration-tests/tests/ai_routes.rs index 72b284fcd1..cfd93caef5 100644 --- a/backend/windmill-api-integration-tests/tests/ai_routes.rs +++ b/backend/windmill-api-integration-tests/tests/ai_routes.rs @@ -10,6 +10,10 @@ fn authed(builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder { 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) { assert!( (200..300).contains(&status), @@ -106,3 +110,125 @@ async fn test_ai_proxy_endpoints(db: Pool) -> anyhow::Result<()> { 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) -> 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(()) +} diff --git a/backend/windmill-api/src/ai.rs b/backend/windmill-api/src/ai.rs index 05bf995700..e7bc089d7c 100644 --- a/backend/windmill-api/src/ai.rs +++ b/backend/windmill-api/src/ai.rs @@ -1,6 +1,7 @@ #[cfg(feature = "bedrock")] use crate::bedrock; use crate::db::{ApiAuthed, DB}; +use crate::utils::check_scopes; #[cfg(feature = "bedrock")] use axum::routing::get; @@ -669,6 +670,7 @@ async fn global_proxy( async fn proxy( authed: ApiAuthed, Extension(db): Extension, + Extension(user_db): Extension, Path((w_id, mut ai_path)): Path<(String, String)>, method: Method, headers: HeaderMap, @@ -689,6 +691,16 @@ async fn proxy( .get("X-Resource-Path") .map(|v| v.to_str().unwrap_or("").to_string()); 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 { Some(request_cache) if !request_cache.is_expired() && forced_resource_path.is_none() => { request_cache.config @@ -759,13 +771,32 @@ async fn proxy( ) }; - let resource = sqlx::query_scalar::<_, Option>>>( - "SELECT value FROM resource WHERE path = $1 AND workspace_id = $2", - ) - .bind(&resource_path) - .bind(&resource_workspace) - .fetch_optional(&db) - .await? + // For user-specified resources, fetch through an RLS-scoped + // connection so PostgreSQL row-level security enforces the same + // folder/group boundaries as the regular resource API. For the + // workspace/instance ai_config path, the resource_path was already + // validated by an admin/devops user when configuring the workspace, + // so the raw pool is used. + let resource = if is_user_specified_resource { + let mut tx = user_db.clone().begin(&authed).await?; + let res = sqlx::query_scalar::<_, Option>>>( + "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>>>( + "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::BadRequest(format!("Empty resource value for {}", resource_path)))?;