diff --git a/crates/libsy-llm-client/src/backend.rs b/crates/libsy-llm-client/src/backend.rs index a526712fc..e7736c324 100644 --- a/crates/libsy-llm-client/src/backend.rs +++ b/crates/libsy-llm-client/src/backend.rs @@ -6,7 +6,7 @@ use std::{collections::BTreeMap, fmt}; use reqwest::RequestBuilder; -use reqwest::header::HeaderValue; +use reqwest::header::{HeaderName, HeaderValue}; use serde_json::Value; use switchyard_protocol::{Metadata, WireFormat}; @@ -44,13 +44,15 @@ pub struct HttpBackendConfig { /// Base URL of the provider API (e.g. `https://api.openai.com/v1`). pub base_url: String, /// API key for the provider, loaded by the caller. `None` sends no configured auth. + /// Client construction rejects active values that cannot form the provider's auth header. pub api_key: Option, /// Whether this backend forwards the caller's provider credential instead. pub forward_auth: bool, /// Custom headers added to every outbound call to this backend. /// /// Provider-owned headers are rejected so a static value cannot replace - /// configured or forwarded auth. Header names are case-insensitive. + /// configured or forwarded auth. Names and values must be valid HTTP header bytes; + /// header names are case-insensitive. pub extra_headers: BTreeMap, /// Default top-level request fields, applied only when the request omits the key. pub extra_body: BTreeMap, @@ -86,8 +88,25 @@ pub enum Backend { } impl Backend { - // Checks custom headers before the client can send a request. - pub(crate) fn validate_extra_headers(&self, model_name: &str) -> Result<()> { + // Matches reqwest's header conversions before the client can send a request. + pub(crate) fn validate_configured_headers(&self, model_name: &str) -> Result<()> { + for (name, value) in &self.config().extra_headers { + if HeaderName::from_bytes(name.as_bytes()).is_err() { + return Err(LlmClientError::Configuration { + message: format!( + "model {model_name:?} extra_headers contains invalid HTTP header name {name:?}" + ), + }); + } + if HeaderValue::from_bytes(value.as_bytes()).is_err() { + return Err(LlmClientError::Configuration { + message: format!( + "model {model_name:?} has invalid HTTP header value for extra_headers entry {name:?}" + ), + }); + } + } + let invalid_name = self.config().extra_headers.keys().find(|name| match self { Backend::OpenAiChat(_) | Backend::OpenAiResponses(_) => { name.eq_ignore_ascii_case("authorization") @@ -110,6 +129,23 @@ impl Backend { ), }); } + + let Some(api_key) = self.configured_api_key() else { + return Ok(()); + }; + let valid_api_key = match self { + Backend::OpenAiChat(_) | Backend::OpenAiResponses(_) => { + HeaderValue::try_from(format!("Bearer {api_key}")).is_ok() + } + Backend::Anthropic(_) => HeaderValue::from_str(api_key).is_ok(), + }; + if !valid_api_key { + return Err(LlmClientError::Configuration { + message: format!( + "model {model_name:?} api_key cannot be encoded as an HTTP header" + ), + }); + } Ok(()) } @@ -131,6 +167,15 @@ impl Backend { } } + // Static credentials are unused when the caller's authorization is forwarded. + fn configured_api_key(&self) -> Option<&str> { + if self.is_forwarding_auth() { + None + } else { + self.config().api_key.as_deref() + } + } + /// The fully resolved upstream URL for this backend's endpoint. /// /// Tolerates base URLs that already include the provider path (or a bare @@ -150,11 +195,7 @@ impl Backend { /// `x-api-key: ` plus the required `anthropic-version` header. A backend /// with `forward_auth` uses the caller's provider credential instead. pub fn apply_auth(&self, mut builder: RequestBuilder) -> RequestBuilder { - let api_key = if self.is_forwarding_auth() { - None - } else { - self.config().api_key.as_deref() - }; + let api_key = self.configured_api_key(); match self { Backend::OpenAiChat(_) | Backend::OpenAiResponses(_) => { if let Some(api_key) = api_key { @@ -407,6 +448,75 @@ mod tests { ); } + // Header validation follows reqwest for both accepted and rejected bytes. + #[test] + fn validates_additional_header_bytes() { + let cases = [ + ("x-display-name", "café", None), + ( + "bad header", + "value", + Some("invalid HTTP header name \"bad header\""), + ), + ( + "x-test-header", + "bad\nvalue", + Some("invalid HTTP header value for extra_headers entry \"x-test-header\""), + ), + ]; + + for (name, value, expected) in cases { + let mut config = config("x"); + config + .extra_headers + .insert(name.to_string(), value.to_string()); + let result = Backend::OpenAiChat(config).validate_configured_headers("model"); + match expected { + Some(expected) => assert!( + result.is_err_and(|error| error.to_string().contains(expected)), + "expected {expected:?}" + ), + None => result.expect("encodable header must pass validation"), + } + } + } + + // Only static credentials that apply_auth would send are validated. + #[test] + fn configured_api_key_validation_matches_auth_application() { + const INVALID_KEY: &str = "canary\nsecret"; + let mut config = config("x"); + config.api_key = Some(INVALID_KEY.to_string()); + let builders: [fn(HttpBackendConfig) -> Backend; 2] = + [Backend::OpenAiChat, Backend::Anthropic]; + let client = reqwest::Client::new(); + + for build_backend in builders { + let error = build_backend(config.clone()) + .validate_configured_headers("model") + .expect_err("invalid API key must fail") + .to_string(); + assert!( + error.contains("api_key cannot be encoded as an HTTP header"), + "{error}" + ); + assert!(!error.contains(INVALID_KEY), "API key leaked in: {error}"); + + let mut forwarded = config.clone(); + forwarded.forward_auth = true; + let backend = build_backend(forwarded); + backend + .validate_configured_headers("model") + .expect("unused API key must not fail validation"); + let request = backend + .apply_auth(client.get("https://example.test")) + .build() + .expect("request"); + assert!(!request.headers().contains_key("authorization")); + assert!(!request.headers().contains_key("x-api-key")); + } + } + #[test] fn openai_detects_canonical_and_wrapped_overflow() { let backend = Backend::OpenAiChat(config("x")); diff --git a/crates/libsy-llm-client/src/client.rs b/crates/libsy-llm-client/src/client.rs index de7f2ca6c..42b2a2678 100644 --- a/crates/libsy-llm-client/src/client.rs +++ b/crates/libsy-llm-client/src/client.rs @@ -131,9 +131,9 @@ impl TranslatingLlmClient { for config in model_configs { config .default_backend - .validate_extra_headers(&config.model_name)?; + .validate_configured_headers(&config.model_name)?; for backend in config.other_backends.iter().flatten() { - backend.validate_extra_headers(&config.model_name)?; + backend.validate_configured_headers(&config.model_name)?; } } let build_client = |builder: reqwest::ClientBuilder| { diff --git a/crates/switchyard-server/tests/cli.rs b/crates/switchyard-server/tests/cli.rs index f72c45858..83123f578 100644 --- a/crates/switchyard-server/tests/cli.rs +++ b/crates/switchyard-server/tests/cli.rs @@ -8,18 +8,18 @@ use std::process::Command; type TestResult = Result>; -#[test] -fn dry_run_rejects_invalid_base_url() -> TestResult { +fn dry_run_error(client_config: &str, env: Option<(&str, &str)>) -> TestResult { let directory = tempfile::tempdir()?; let config = directory.path().join("routes.toml"); fs::write( &config, - r#" + format!( + r#" schema_version = 1 [llm_clients.invalid] format = "openai_chat" -base_url = "not a url" +{client_config} [targets.invalid] id = "upstream-model" @@ -29,17 +29,53 @@ llm_client = "invalid" id = "test-route" type = "passthrough" target = "invalid" -"#, +"# + ), )?; - - let output = Command::new(env!("CARGO_BIN_EXE_switchyard-server")) - .args(["--config", config.to_string_lossy().as_ref(), "--dry-run"]) - .output()?; + let mut command = Command::new(env!("CARGO_BIN_EXE_switchyard-server")); + command.args(["--config", config.to_string_lossy().as_ref(), "--dry-run"]); + if let Some((name, value)) = env { + command.env(name, value); + } + let output = command.output()?; assert!(!output.status.success()); - let stderr = String::from_utf8(output.stderr)?; + Ok(String::from_utf8(output.stderr)?) +} + +#[test] +fn dry_run_rejects_invalid_base_url() -> TestResult { + let stderr = dry_run_error("base_url = \"not a url\"", None)?; assert!( stderr.contains("base_url must be an absolute HTTP(S) URL"), "{stderr}" ); Ok(()) } + +// Dry-run rejects unsendable headers before startup without exposing credentials. +#[test] +fn dry_run_rejects_unsendable_configured_headers() -> TestResult { + const INVALID_KEY_ENV: &str = "SWITCHYARD_CLI_TEST_INVALID_HEADER_KEY"; + const INVALID_KEY: &str = "canary\nsecret"; + let cases = [ + ( + "base_url = \"https://example.test/v1\"\n\ + extra_headers = { \"bad header\" = \"value\" }" + .to_string(), + None, + "invalid HTTP header name \"bad header\"", + ), + ( + format!("base_url = \"https://example.test/v1\"\napi_key_env = \"{INVALID_KEY_ENV}\""), + Some((INVALID_KEY_ENV, INVALID_KEY)), + "api_key cannot be encoded as an HTTP header", + ), + ]; + + for (client_config, env, expected) in cases { + let stderr = dry_run_error(&client_config, env)?; + assert!(stderr.contains(expected), "{stderr}"); + assert!(!stderr.contains(INVALID_KEY), "API key leaked in: {stderr}"); + } + Ok(()) +}