From ebf68d79708bb48c3db500848adfe07774920228 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Mon, 27 Jul 2026 15:23:49 +0200 Subject: [PATCH] fix(ai): support OpenAI/Anthropic-compatible gateways in workspace AI settings (#10356) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ai): stop breaking OpenAI-compatible gateways in workspace AI settings The workspace/instance AI proxy sent credentials in a shape that OpenAI-compatible and Anthropic-compatible gateways reject, while the same resource worked in an AI agent step: - `is_azure` treated *any* OpenAI base URL other than api.openai.com as Azure, so a gateway got the Azure `api-key` header instead of `Authorization: Bearer` and an `/openai/v1/`-rewritten path. Match on the host instead. - The Anthropic proxy sent both `authorization: Bearer` and `X-API-Key`. Gateways reject ambiguous credentials; send only the header the endpoint expects, matching `get_auth_headers` and the agent-step path. Co-Authored-By: Claude Opus 5 (1M context) * fix(ai): match Azure on the endpoint and let resource headers own auth Review follow-ups: - Azure OpenAI reached through a custom domain keeps its `/openai/deployments` path, which `openai_azure_base_path` documents; match on it so those instance-wide settings are not reclassified as plain OpenAI-compatible. - Cover the sovereign-cloud API Management suffixes and FQDNs with a trailing dot. - A resource that supplies its own `authorization`/`x-api-key` header now suppresses the built-in one. Outgoing headers are appended rather than replaced, so both credentials used to travel, which is exactly what gateways reject; this is the escape hatch for endpoints wanting bearer auth. Co-Authored-By: Claude Opus 5 (1M context) * fix(ai): give the agent step the same resource-header credential rule Review round follow-ups: - `ai_executor` appended the query builder's credential header alongside the resource's, so an AI agent step still sent two credentials where the proxy now sends one. Both paths share `resource_owns_credentials`/`CREDENTIAL_HEADERS`; non-credential headers such as `anthropic-version` are kept. - Cover the OpenAI-compatible proxy's suppression branch with a test. - Match the Azure deployments path case-insensitively, like the host. Co-Authored-By: Claude Opus 5 (1M context) * fix(ai): scope the credential override to the header the provider uses A resource header now replaces the built-in credential only when it is the same header the provider authenticates with, or an `authorization` one (which every endpoint reads as the credential). Matching any credential-shaped header let an OpenAI-compatible resource's `x-api-key` routing header suppress the bearer token. Google AI's `x-goog-api-key` joins the list so the override reaches that provider too. Co-Authored-By: Claude Opus 5 (1M context) * refactor(ai): share credential and trailing header assembly across AI paths The proxy and the AI agent step each assembled outbound headers themselves, so this fix had to be applied at three sites and the rules could drift apart silently. Two pieces move into `proxy`: - `credential_header` picks the credential to send, applying the resource override. `authorization` carries a bearer token and every other credential header carries the raw key, which holds for every provider. - `common_outbound_headers` yields Windmill's own headers then the resource's, the tail every outbound request shares. A resource resolves to an api key or an OAuth token, never both, so selecting one drops the branch that could emit two `authorization` headers. Co-Authored-By: Claude Opus 5 (1M context) * fix(ai): keep OAuth tokens on the bearer header An OAuth resource resolves to an access token, which every provider reads from `authorization` — Azure OpenAI accepts api keys in `api-key` but Entra ID tokens only as a bearer. Sending it in the provider's key header left Azure OpenAI and Foundry Claude OAuth resources unauthenticated. Also covers the credential-override narrowing: a credential-shaped header the provider does not authenticate with is an ordinary header and must not suppress the built-in credential. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- backend/windmill-ai/src/ai_providers.rs | 94 ++++++++- .../windmill-ai/src/providers/anthropic.rs | 88 ++++++--- backend/windmill-ai/src/proxy.rs | 186 ++++++++++++++++-- backend/windmill-worker/src/ai_executor.rs | 23 ++- 4 files changed, 327 insertions(+), 64 deletions(-) diff --git a/backend/windmill-ai/src/ai_providers.rs b/backend/windmill-ai/src/ai_providers.rs index 551ba3129b..216d10bab9 100644 --- a/backend/windmill-ai/src/ai_providers.rs +++ b/backend/windmill-ai/src/ai_providers.rs @@ -30,6 +30,52 @@ pub const OPENAI_BASE_URL: &str = "https://api.openai.com/v1"; pub const DEEPSEEK_BASE_URL: &str = "https://api.deepseek.com/v1"; pub const GOOGLE_AI_BASE_URL: &str = "https://generativelanguage.googleapis.com/v1beta"; +/// Hosts that serve the OpenAI API with Azure conventions: Azure OpenAI +/// (`*.openai.azure.com`), AI Foundry (`*.services.ai.azure.com`, +/// `*.cognitiveservices.azure.com`), their sovereign-cloud counterparts, and API +/// Management fronting either. +const AZURE_HOST_SUFFIXES: &[&str] = &[ + ".azure.com", + ".azure.us", + ".azure.cn", + ".azure-api.net", + ".azure-api.us", + ".azure-api.cn", +]; + +/// The deployment path Azure OpenAI serves under. It identifies an Azure endpoint +/// reached through a custom domain, which `AZURE_HOST_SUFFIXES` cannot catch — it +/// is the format `openai_azure_base_path` documents. +const AZURE_DEPLOYMENTS_PATH: &str = "/openai/deployments"; + +/// Whether an OpenAI-API base URL is served by Azure, which authenticates with the +/// `api-key` header and lays its routes out under `/openai/...`. +/// +/// An OpenAI-compatible endpoint on an Azure-owned domain (e.g. a self-hosted +/// server behind API Management) is misread as Azure; such a resource has to use +/// the `customai` provider. +fn is_azure_endpoint(base_url: &str) -> bool { + let authority = base_url + .split_once("://") + .map_or(base_url, |(_, rest)| rest) + .split(['/', '?', '#']) + .next() + .unwrap_or_default(); + let host_port = authority.rsplit_once('@').map_or(authority, |(_, h)| h); + let host = host_port + .rsplit_once(':') + .map_or(host_port, |(host, _)| host) + .trim_end_matches('.') + .to_ascii_lowercase(); + + AZURE_HOST_SUFFIXES + .iter() + .any(|suffix| host.ends_with(suffix)) + || base_url + .to_ascii_lowercase() + .contains(AZURE_DEPLOYMENTS_PATH) +} + /// Empty string signals BedrockClient::from_env() to use the region from AWS environment/config /// (e.g., AWS_REGION or AWS_DEFAULT_REGION env vars, or ~/.aws/config) pub const USE_ENV_REGION: &str = ""; @@ -107,7 +153,7 @@ impl AIProvider { OPENAI_AZURE_BASE_PATH.clone() }; - Ok(azure_base_path.unwrap_or("https://api.openai.com/v1".to_string())) + Ok(azure_base_path.unwrap_or_else(|| OPENAI_BASE_URL.to_string())) } AIProvider::DeepSeek => Ok(DEEPSEEK_BASE_URL.to_string()), AIProvider::GoogleAI => Ok(GOOGLE_AI_BASE_URL.to_string()), @@ -139,10 +185,17 @@ impl AIProvider { /// Check whether this provider/URL combination uses Azure conventions /// (the `api-key` auth header and Azure URL building). This covers Azure /// OpenAI, Azure AI Foundry, and the `OpenAI` provider pointed at an Azure - /// base path override. + /// endpoint through `openai_azure_base_path` or a resource base URL. + /// + /// A custom `OpenAI` base URL that is not an Azure endpoint is an + /// OpenAI-compatible one (gateway, proxy, self-hosted server) and keeps bearer + /// auth and the plain `/` layout. pub fn is_azure(&self, base_url: &str) -> bool { - (matches!(self, AIProvider::OpenAI) && base_url != OPENAI_BASE_URL) - || matches!(self, AIProvider::AzureOpenAI | AIProvider::AzureFoundry) + match self { + AIProvider::AzureOpenAI | AIProvider::AzureFoundry => true, + AIProvider::OpenAI => is_azure_endpoint(base_url), + _ => false, + } } /// Build an Azure-style OpenAI-compatible URL (Azure OpenAI / Azure AI Foundry) @@ -300,6 +353,39 @@ mod tests { ); } + /// An OpenAI resource with a custom base URL is an OpenAI-compatible endpoint + /// unless it is an Azure one. Azure treatment swaps bearer auth for the + /// `api-key` header and rewrites the path, so both directions must hold. + #[test] + fn openai_is_azure_only_for_azure_endpoints() { + for base_url in [ + // A gateway path ending in /openai must not read as Azure. + "https://gateway-eu.pydantic.dev/proxy/openai", + "https://openrouter.ai/api/v1", + "http://localhost:4000/v1", + OPENAI_BASE_URL, + ] { + assert!( + !AIProvider::OpenAI.is_azure(base_url), + "{base_url} must not be treated as Azure" + ); + } + + for base_url in [ + "https://example.openai.azure.com/openai/deployments/my-deployment", + "https://wm-test-ai.services.ai.azure.com", + "https://contoso.azure-api.cn/openai", + // Azure OpenAI behind a custom domain, the format + // `openai_azure_base_path` documents. + "https://openai.contoso.com/openai/deployments/gpt-4o", + ] { + assert!( + AIProvider::OpenAI.is_azure(base_url), + "{base_url} must be treated as Azure" + ); + } + } + #[test] fn azure_foundry_anthropic_url_from_root_and_legacy() { // Root URL. diff --git a/backend/windmill-ai/src/providers/anthropic.rs b/backend/windmill-ai/src/providers/anthropic.rs index a255f47a6d..52ca0085f9 100644 --- a/backend/windmill-ai/src/providers/anthropic.rs +++ b/backend/windmill-ai/src/providers/anthropic.rs @@ -2,14 +2,13 @@ use crate::{ ai_google::parse_data_url, ai_providers::{AIPlatform, AIProvider}, image_handler::prepare_messages_for_api, - proxy::{add_user_to_body, ProxyBuildArgs, ProxyRequest}, + proxy::{ + add_user_to_body, common_outbound_headers, credential_header, ProxyBuildArgs, ProxyRequest, + }, query_builder::{BuildRequestArgs, ParsedResponse, QueryBuilder, StreamEventSink}, sse::{AnthropicSSEParser, SSEParser}, types::*, - utils::{ - collect_system_prompt, extract_text_content, should_use_structured_output_tool, - AI_HTTP_HEADERS, - }, + utils::{collect_system_prompt, extract_text_content, should_use_structured_output_tool}, }; use async_trait::async_trait; use http::Method; @@ -544,31 +543,23 @@ impl AnthropicQueryBuilder { } } - if let Some(api_key) = credentials.api_key.as_ref() { - headers.push(("authorization".to_string(), format!("Bearer {}", api_key))); - if !is_vertex { - headers.push(("X-API-Key".to_string(), api_key.clone())); - } - } - - if let Some(access_token) = credentials.access_token.as_ref() { - headers.push(( - "authorization".to_string(), - format!("Bearer {}", access_token), - )); - } + // One credential header, matching `get_auth_headers`: Vertex takes an OAuth + // bearer token, every other Messages endpoint takes x-api-key. Endpoints in + // front of Anthropic reject requests carrying both. + headers.extend(credential_header( + credentials, + if is_vertex { + "authorization" + } else { + "x-api-key" + }, + )); if let Some(org_id) = credentials.organization_id.as_ref() { headers.push(("OpenAI-Organization".to_string(), org_id.clone())); } - for (header_name, header_value) in AI_HTTP_HEADERS.iter() { - headers.push((header_name.clone(), header_value.clone())); - } - - for (header_name, header_value) in &credentials.custom_headers { - headers.push((header_name.clone(), header_value.clone())); - } + headers.extend(common_outbound_headers(credentials)); Ok(ProxyRequest { method: args.method.clone(), url, headers, body }) } @@ -953,12 +944,11 @@ mod tests { assert_eq!(request.method, Method::POST); assert_eq!(request.url, "https://api.anthropic.com/v1/messages"); assert_eq!(request.body, body.to_vec()); - assert!(has_header( - &request.headers, - "authorization", - "Bearer api-key" - )); - assert!(has_header(&request.headers, "X-API-Key", "api-key")); + assert!(has_header(&request.headers, "x-api-key", "api-key")); + assert!(!request + .headers + .iter() + .any(|(header_name, _)| header_name.eq_ignore_ascii_case("authorization"))); assert!(has_header( &request.headers, "anthropic-version", @@ -1148,6 +1138,40 @@ mod tests { assert!(matches!(err, Error::BadRequest(message) if message.contains("Missing 'model'"))); } + /// Endpoints that authenticate with a bearer token configure it as a resource + /// header; the built-in x-api-key must then step aside, since outgoing headers + /// are appended and both credentials would travel. + #[test] + fn resource_header_replaces_the_built_in_credential() { + let mut credentials = credentials(AIPlatform::Standard); + credentials.custom_headers = HashMap::from([( + "Authorization".to_string(), + "Bearer gateway-token".to_string(), + )]); + let builder = AnthropicQueryBuilder::new(AIProvider::Anthropic, AIPlatform::Standard); + let method = Method::POST; + + let request = builder + .build_proxy_request(&ProxyBuildArgs { + method: &method, + path: "messages", + headers: &HeaderMap::new(), + body: br#"{"model":"claude-sonnet-4","messages":[]}"#, + credentials: &credentials, + }) + .unwrap(); + + assert!(has_header( + &request.headers, + "Authorization", + "Bearer gateway-token" + )); + assert!(!request + .headers + .iter() + .any(|(header_name, _)| header_name.eq_ignore_ascii_case("x-api-key"))); + } + #[test] fn builds_azure_foundry_anthropic_proxy_request() { // Foundry resource stored with a legacy /openai/v1 suffix; the Anthropic SDK @@ -1176,6 +1200,6 @@ mod tests { request.url, "https://wm-test-ai.services.ai.azure.com/anthropic/v1/messages" ); - assert!(has_header(&request.headers, "X-API-Key", "api-key")); + assert!(has_header(&request.headers, "x-api-key", "api-key")); } } diff --git a/backend/windmill-ai/src/proxy.rs b/backend/windmill-ai/src/proxy.rs index fe4a0229d4..99c6188d3c 100644 --- a/backend/windmill-ai/src/proxy.rs +++ b/backend/windmill-ai/src/proxy.rs @@ -82,6 +82,72 @@ pub fn supports_query_builder_proxy(provider: &AIProvider) -> bool { proxy_execution_mode(provider).uses_query_builder_proxy() } +/// The headers a provider can carry its credential in. Any other resource header +/// is passed through untouched, so a header one provider authenticates with stays +/// an ordinary header for the providers that do not. +pub const CREDENTIAL_HEADERS: [&str; 4] = + ["authorization", "x-api-key", "api-key", "x-goog-api-key"]; + +/// Whether a resource header takes over `built_in`, the credential header Windmill +/// would otherwise send. Outgoing headers are appended rather than replaced, so +/// keeping the built-in one alongside a resource-supplied credential would put two +/// on the wire, which endpoints reject. +/// +/// An `authorization` header always takes over, whatever `built_in` is: every +/// endpoint reads it as the credential, so it can never coexist with another one. +pub fn resource_replaces_credential(credentials: &ProviderCredentials, built_in: &str) -> bool { + credentials.custom_headers.keys().any(|header_name| { + header_name.eq_ignore_ascii_case(built_in) + || header_name.eq_ignore_ascii_case("authorization") + }) +} + +/// The credential header for an outbound request, or `None` when the resource +/// supplies its own. +/// +/// An api key goes in `built_in`, the header the provider authenticates keys +/// with. An OAuth token is always a bearer token instead, whatever header that +/// provider's keys use — Azure OpenAI reads keys from `api-key` but Entra ID +/// tokens only from `authorization`. `authorization` carries `Bearer ` +/// and every other credential header carries the raw secret, for every provider. +pub fn credential_header( + credentials: &ProviderCredentials, + built_in: &str, +) -> Option<(String, String)> { + // `resource_replaces_credential` also matches a resource `authorization` + // header, so this covers the bearer case whatever `built_in` is. + if resource_replaces_credential(credentials, built_in) { + return None; + } + let (name, secret) = match (&credentials.api_key, &credentials.access_token) { + (_, Some(access_token)) => ("authorization", access_token), + (Some(api_key), None) => (built_in, api_key), + (None, None) => return None, + }; + let value = if name.eq_ignore_ascii_case("authorization") { + format!("Bearer {}", secret) + } else { + secret.clone() + }; + Some((name.to_string(), value)) +} + +/// The headers every outbound AI request ends with: Windmill's own, then the +/// resource's, which come last so a resource can add to what the provider set. +pub fn common_outbound_headers( + credentials: &ProviderCredentials, +) -> impl Iterator + '_ { + AI_HTTP_HEADERS + .iter() + .map(|(name, value)| (name.clone(), value.clone())) + .chain( + credentials + .custom_headers + .iter() + .map(|(name, value)| (name.clone(), value.clone())), + ) +} + pub fn build_openai_compatible_proxy_request(args: &ProxyBuildArgs<'_>) -> Result { let credentials = args.credentials; let body = if let Some(user) = credentials.user.as_ref() { @@ -100,32 +166,16 @@ pub fn build_openai_compatible_proxy_request(args: &ProxyBuildArgs<'_>) -> Resul let mut headers = vec![("content-type".to_string(), "application/json".to_string())]; - if let Some(api_key) = credentials.api_key.as_ref() { - if is_azure { - headers.push(("api-key".to_string(), api_key.clone())); - } else { - headers.push(("authorization".to_string(), format!("Bearer {}", api_key))); - } - } - - if let Some(access_token) = credentials.access_token.as_ref() { - headers.push(( - "authorization".to_string(), - format!("Bearer {}", access_token), - )); - } + headers.extend(credential_header( + credentials, + if is_azure { "api-key" } else { "authorization" }, + )); if let Some(org_id) = credentials.organization_id.as_ref() { headers.push(("OpenAI-Organization".to_string(), org_id.clone())); } - for (header_name, header_value) in AI_HTTP_HEADERS.iter() { - headers.push((header_name.clone(), header_value.clone())); - } - - for (header_name, header_value) in &credentials.custom_headers { - headers.push((header_name.clone(), header_value.clone())); - } + headers.extend(common_outbound_headers(credentials)); Ok(ProxyRequest { method: args.method.clone(), url, headers, body }) } @@ -198,6 +248,100 @@ mod tests { .contains(&("OpenAI-Organization".to_string(), "org-id".to_string()))); } + /// A resource that carries its own credential owns authentication; outgoing + /// headers are appended, not replaced, so the built-in one must be dropped or + /// two credentials reach the endpoint. + #[test] + fn resource_header_replaces_the_built_in_credential() { + let mut credentials = credentials(AIProvider::CustomAI, "https://gateway.example/openai"); + credentials.custom_headers = HashMap::from([( + "Authorization".to_string(), + "Bearer gateway-token".to_string(), + )]); + let method = Method::POST; + + let request = build_openai_compatible_proxy_request(&ProxyBuildArgs { + method: &method, + path: "chat/completions", + headers: &HeaderMap::new(), + body: br#"{"model":"model","messages":[]}"#, + credentials: &credentials, + }) + .unwrap(); + + assert_eq!( + request + .headers + .iter() + .filter(|(header_name, _)| header_name.eq_ignore_ascii_case("authorization")) + .collect::>(), + vec![&( + "Authorization".to_string(), + "Bearer gateway-token".to_string() + )] + ); + } + + /// Only the header the provider authenticates keys with hands over. A + /// credential-shaped header a provider does not read is an ordinary header, + /// and suppressing the built-in credential on account of it strands the + /// request with no credential at all. + #[test] + fn unrelated_credential_header_keeps_the_built_in_one() { + let mut credentials = credentials(AIProvider::CustomAI, "https://gateway.example/openai"); + credentials.custom_headers = + HashMap::from([("x-api-key".to_string(), "routing-key".to_string())]); + let method = Method::POST; + + let request = build_openai_compatible_proxy_request(&ProxyBuildArgs { + method: &method, + path: "chat/completions", + headers: &HeaderMap::new(), + body: br#"{"model":"model","messages":[]}"#, + credentials: &credentials, + }) + .unwrap(); + + assert!(request + .headers + .contains(&("authorization".to_string(), "Bearer api-key".to_string()))); + assert!(request + .headers + .contains(&("x-api-key".to_string(), "routing-key".to_string()))); + } + + /// Azure reads api keys from `api-key` but Entra ID tokens only from + /// `authorization`, so an OAuth resource must stay on the bearer header + /// whatever the provider's key header is. + #[test] + fn oauth_token_is_sent_as_a_bearer_on_azure() { + let mut credentials = credentials( + AIProvider::AzureOpenAI, + "https://example.openai.azure.com/openai", + ); + credentials.api_key = None; + credentials.access_token = Some("oauth-token".to_string()); + let method = Method::POST; + + let request = build_openai_compatible_proxy_request(&ProxyBuildArgs { + method: &method, + path: "chat/completions", + headers: &HeaderMap::new(), + body: br#"{"model":"deployment","messages":[]}"#, + credentials: &credentials, + }) + .unwrap(); + + assert!(request.headers.contains(&( + "authorization".to_string(), + "Bearer oauth-token".to_string() + ))); + assert!(!request + .headers + .iter() + .any(|(header_name, _)| header_name.eq_ignore_ascii_case("api-key"))); + } + #[test] fn query_builder_proxy_support_includes_anthropic() { let cases = [ diff --git a/backend/windmill-worker/src/ai_executor.rs b/backend/windmill-worker/src/ai_executor.rs index 8605bca374..507cb0420b 100644 --- a/backend/windmill-worker/src/ai_executor.rs +++ b/backend/windmill-worker/src/ai_executor.rs @@ -24,9 +24,10 @@ use windmill_ai::{ ai_providers::AIProvider, image_handler::upload_image_to_s3, providers::create_query_builder, + proxy::{common_outbound_headers, resource_replaces_credential, CREDENTIAL_HEADERS}, query_builder::{BuildRequestArgs, ParsedResponse}, types::*, - utils::{pinned_ai_client_for, should_use_structured_output_tool, AI_HTTP_HEADERS}, + utils::{pinned_ai_client_for, should_use_structured_output_tool}, }; use windmill_common::{ cache, @@ -977,12 +978,24 @@ pub async fn run_agent( let endpoint = query_builder.get_endpoint(base_url, args.provider.get_model(), output_type); let auth_headers = query_builder.get_auth_headers(api_key, base_url, output_type); + // A resource carrying its own credential owns authentication; the + // built-in one is dropped so the two do not both reach the endpoint. + // Non-credential headers (`anthropic-version`) stay either way. + let auth_headers: Vec<_> = auth_headers + .into_iter() + .filter(|(header_name, _)| { + let carries_credential = CREDENTIAL_HEADERS + .iter() + .any(|name| header_name.eq_ignore_ascii_case(name)); + !carries_credential || !resource_replaces_credential(&credentials, header_name) + }) + .collect(); let timeout = resolve_job_timeout(conn, &job.workspace_id, job.id, job.timeout) .await .0; - let resource_headers = &credentials.custom_headers; + let trailing_headers = common_outbound_headers(&credentials).collect::>(); // `endpoint` derives from the user-controlled provider base_url, so pin // DNS to the SSRF-validated address: the connect must not rebind to an @@ -1000,11 +1013,7 @@ pub async fn run_agent( req = req.header(*header_name, header_value.clone()); } - for (header_name, header_value) in AI_HTTP_HEADERS.iter() { - req = req.header(header_name.as_str(), header_value.as_str()); - } - - for (header_name, header_value) in resource_headers { + for (header_name, header_value) in &trailing_headers { req = req.header(header_name.as_str(), header_value.as_str()); }