From a30ea630e5c5cedcf10d1cd4066d3637cc731e71 Mon Sep 17 00:00:00 2001 From: centdix <40307056+centdix@users.noreply.github.com> Date: Thu, 29 Jan 2026 12:43:02 +0100 Subject: [PATCH] chore: handle empty strings in AI resource fields via serde deserializer (#7723) * fix: handle empty strings in AI resource fields via serde deserializer Add `empty_string_as_none` deserializer that converts empty strings to None during deserialization. Applied to base_url, api_key, region, and AWS credential fields in AIStandardResource and ProviderResource. This fixes the "relative URL without a base" error when creating Anthropic resources with empty base_url fields. Co-Authored-By: Claude Opus 4.5 * nit * nit * nit * cleaning * cleaning * cleaning * cleaning * fix: apply empty_string_as_none deserializer to api_key field Consistent with other fields in ProviderResource, empty strings are now deserialized as None for the api_key field. Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 --- backend/windmill-api/src/ai.rs | 29 ++++++++----- backend/windmill-common/src/ai_providers.rs | 41 +++++++++---------- .../src/ai/providers/bedrock.rs | 7 ++-- backend/windmill-worker/src/ai/types.rs | 16 +++++--- backend/windmill-worker/src/ai_executor.rs | 18 ++++---- 5 files changed, 60 insertions(+), 51 deletions(-) diff --git a/backend/windmill-api/src/ai.rs b/backend/windmill-api/src/ai.rs index 224167514d..1422e9e52d 100644 --- a/backend/windmill-api/src/ai.rs +++ b/backend/windmill-api/src/ai.rs @@ -16,7 +16,7 @@ use serde::{Deserialize, Serialize}; use serde_json::{json, value::RawValue}; use std::collections::HashMap; use windmill_audit::{audit_oss::audit_log, ActionKind}; -use windmill_common::ai_providers::{AIProvider, ProviderConfig, ProviderModel}; +use windmill_common::ai_providers::{empty_string_as_none, AIProvider, ProviderConfig, ProviderModel}; use windmill_common::error::{to_anyhow, Error, Result}; use windmill_common::utils::configure_client; use windmill_common::variables::get_variable_or_self; @@ -143,15 +143,17 @@ enum AnthropicPlatform { #[derive(Deserialize, Debug)] struct AIStandardResource { - #[serde(alias = "baseUrl")] + #[serde(alias = "baseUrl", default, deserialize_with = "empty_string_as_none")] base_url: Option, - #[serde(alias = "apiKey")] + #[serde(alias = "apiKey", default, deserialize_with = "empty_string_as_none")] api_key: Option, + #[serde(default, deserialize_with = "empty_string_as_none")] organization_id: Option, + #[serde(default, deserialize_with = "empty_string_as_none")] region: Option, - #[serde(alias = "awsAccessKeyId")] + #[serde(alias = "awsAccessKeyId", default, deserialize_with = "empty_string_as_none")] aws_access_key_id: Option, - #[serde(alias = "awsSecretAccessKey")] + #[serde(alias = "awsSecretAccessKey", default, deserialize_with = "empty_string_as_none")] aws_secret_access_key: Option, /// Platform for Anthropic API (standard or google_vertex_ai) #[serde(default)] @@ -207,9 +209,14 @@ impl AIRequestConfig { AIResource::Standard(resource) => { let region = resource.region.clone(); let platform = resource.platform.clone(); - let base_url = provider - .get_base_url(resource.base_url, resource.region, db) - .await?; + // Skip get_base_url for Bedrock - it uses SDK directly, not HTTP + let base_url = if matches!(provider, AIProvider::AWSBedrock) { + String::new() + } else { + 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?) } else { @@ -251,7 +258,7 @@ impl AIRequestConfig { None }; let token = Self::get_token_using_oauth(resource, db, w_id).await?; - let base_url = provider.get_base_url(None, None, db).await?; + let base_url = provider.get_base_url(None, db).await?; ( None, @@ -578,7 +585,7 @@ async fn global_proxy( return Err(Error::BadRequest("API key is required".to_string())); }; - let base_url = provider.get_base_url(None, None, &db).await?; + let base_url = provider.get_base_url(None, &db).await?; let url = format!("{}/{}", base_url, ai_path); @@ -745,7 +752,7 @@ async fn proxy( let region = request_config .region .as_deref() - .ok_or_else(|| Error::internal_err("AWS region must be set for Bedrock"))?; + .unwrap_or(windmill_common::ai_providers::USE_ENV_REGION); // Audit log before making the SDK request let mut tx = db.begin().await?; diff --git a/backend/windmill-common/src/ai_providers.rs b/backend/windmill-common/src/ai_providers.rs index b156002dc8..2e657fd0ac 100644 --- a/backend/windmill-common/src/ai_providers.rs +++ b/backend/windmill-common/src/ai_providers.rs @@ -4,7 +4,17 @@ use crate::db::DB; use crate::error::{Error, Result}; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; + +/// Deserializes an Option where empty strings become None. +/// Use with `#[serde(default, deserialize_with = "empty_string_as_none")]` +pub fn empty_string_as_none<'de, D>(deserializer: D) -> std::result::Result, D::Error> +where + D: Deserializer<'de>, +{ + let opt = Option::::deserialize(deserializer)?; + Ok(opt.filter(|s| !s.is_empty())) +} lazy_static::lazy_static! { static ref OPENAI_AZURE_BASE_PATH: Option = std::env::var("OPENAI_AZURE_BASE_PATH").ok(); @@ -13,6 +23,10 @@ lazy_static::lazy_static! { pub const OPENAI_BASE_URL: &str = "https://api.openai.com/v1"; pub const GOOGLE_AI_BASE_URL: &str = "https://generativelanguage.googleapis.com/v1beta"; +/// 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 = ""; + #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Hash, Clone)] #[serde(rename_all = "lowercase")] pub enum AIProvider { @@ -36,11 +50,9 @@ impl AIProvider { pub async fn get_base_url( &self, resource_base_url: Option, - region: Option, db: &DB, ) -> Result { - // If a base URL is provided in the resource, use it (ignore empty strings) - if let Some(base_url) = resource_base_url.filter(|s| !s.is_empty()) { + if let Some(base_url) = resource_base_url { return Ok(base_url); } @@ -78,23 +90,10 @@ impl AIProvider { format!("{:?} provider requires a base URL in the resource", p), )), AIProvider::AWSBedrock => { - #[cfg(feature = "bedrock")] - { - Ok(format!( - "https://bedrock-runtime.{}.amazonaws.com", - region - .filter(|s| !s.is_empty()) - .unwrap_or_else(|| "us-east-1".to_string()) - )) - } - #[cfg(not(feature = "bedrock"))] - { - let _ = region; - Err(Error::BadRequest( - "AWS Bedrock support is not enabled. Build with 'bedrock' feature." - .to_string(), - )) - } + // AWS Bedrock uses the SDK directly, not HTTP base URL + Err(Error::internal_err( + "AWS Bedrock uses SDK directly, not HTTP base URL".to_string(), + )) } } } diff --git a/backend/windmill-worker/src/ai/providers/bedrock.rs b/backend/windmill-worker/src/ai/providers/bedrock.rs index f4df4bc393..14021d9232 100644 --- a/backend/windmill-worker/src/ai/providers/bedrock.rs +++ b/backend/windmill-worker/src/ai/providers/bedrock.rs @@ -51,10 +51,9 @@ impl BedrockQueryBuilder { ) -> Result { let bedrock_client = if !api_key.is_empty() { BedrockClient::from_bearer_token(api_key.to_string(), region).await? - } else if let (Some(access_key_id), Some(secret_access_key)) = ( - aws_access_key_id.filter(|s| !s.is_empty()), - aws_secret_access_key.filter(|s| !s.is_empty()), - ) { + } else if let (Some(access_key_id), Some(secret_access_key)) = + (aws_access_key_id, aws_secret_access_key) + { BedrockClient::from_credentials( access_key_id.to_string(), secret_access_key.to_string(), diff --git a/backend/windmill-worker/src/ai/types.rs b/backend/windmill-worker/src/ai/types.rs index b1d14bd64f..cd6a707bdd 100644 --- a/backend/windmill-worker/src/ai/types.rs +++ b/backend/windmill-worker/src/ai/types.rs @@ -14,7 +14,11 @@ pub struct McpToolSource { pub resource_path: String, } use windmill_common::{ - ai_providers::AIProvider, db::DB, error::Error, flow_status::AgentAction, flows::FlowModule, + ai_providers::{empty_string_as_none, AIProvider}, + db::DB, + error::Error, + flow_status::AgentAction, + flows::FlowModule, s3_helpers::S3Object, }; use windmill_parser::Typ; @@ -162,17 +166,18 @@ pub enum AnthropicPlatform { #[derive(Deserialize, Debug)] pub struct ProviderResource { - #[serde(alias = "apiKey")] + #[serde(alias = "apiKey", default, deserialize_with = "empty_string_as_none")] pub api_key: Option, - #[serde(alias = "baseUrl")] + #[serde(alias = "baseUrl", default, deserialize_with = "empty_string_as_none")] pub base_url: Option, #[allow(dead_code)] + #[serde(default, deserialize_with = "empty_string_as_none")] pub region: Option, #[allow(dead_code)] - #[serde(alias = "awsAccessKeyId")] + #[serde(alias = "awsAccessKeyId", default, deserialize_with = "empty_string_as_none")] pub aws_access_key_id: Option, #[allow(dead_code)] - #[serde(alias = "awsSecretAccessKey")] + #[serde(alias = "awsSecretAccessKey", default, deserialize_with = "empty_string_as_none")] pub aws_secret_access_key: Option, /// Platform for Anthropic API (standard or google_vertex_ai) #[serde(default)] @@ -199,7 +204,6 @@ impl ProviderWithResource { self.kind .get_base_url( self.resource.base_url.clone(), - self.resource.region.clone(), db, ) .await diff --git a/backend/windmill-worker/src/ai_executor.rs b/backend/windmill-worker/src/ai_executor.rs index dbf7d3cb5c..01edf44079 100644 --- a/backend/windmill-worker/src/ai_executor.rs +++ b/backend/windmill-worker/src/ai_executor.rs @@ -21,7 +21,7 @@ use windmill_mcp::McpClient; #[cfg(not(feature = "mcp"))] use crate::ai::tools::McpClientStub as McpClient; use windmill_common::{ - ai_providers::AIProvider, + ai_providers::{AIProvider}, cache, client::AuthedClient, db::DB, @@ -410,9 +410,14 @@ pub async fn run_agent( has_websearch: bool, ) -> error::Result> { let output_type = args.output_type.as_ref().unwrap_or(&OutputType::Text); - let base_url = args.provider.get_base_url(db).await?; + // Skip get_base_url for Bedrock - it uses SDK directly, not HTTP + let base_url = if args.provider.kind == AIProvider::AWSBedrock { + String::new() + } else { + args.provider.get_base_url(db).await? + }; let api_key = args.provider.get_api_key().unwrap_or(""); - + // Create the query builder for the provider let query_builder = create_query_builder(&args.provider); @@ -660,12 +665,7 @@ pub async fn run_agent( let parsed = if args.provider.kind == AIProvider::AWSBedrock { #[cfg(feature = "bedrock")] { - let region = args.provider.get_region(); - let Some(region) = region else { - return Err(Error::internal_err( - "AWS Bedrock region is required".to_string(), - )); - }; + let region = args.provider.get_region().unwrap_or(windmill_common::ai_providers::USE_ENV_REGION); // Use Bedrock SDK via dedicated query builder crate::ai::providers::bedrock::BedrockQueryBuilder::default() .execute_request(