From 8f830835823ac1ddfe06ad4a093edbffe9a23b46 Mon Sep 17 00:00:00 2001 From: Clement Pakkam Isaac Date: Fri, 4 Sep 2026 09:59:40 -0700 Subject: [PATCH 1/3] fix(llm-client): validate configured headers at startup Signed-off-by: Clement Pakkam Isaac --- crates/libsy-llm-client/src/backend.rs | 44 ++++++++++++-- crates/libsy-llm-client/src/client.rs | 4 +- crates/switchyard-runner/src/config.rs | 64 +++++++++++++++++++- crates/switchyard-server/tests/cli.rs | 81 ++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 7 deletions(-) diff --git a/crates/libsy-llm-client/src/backend.rs b/crates/libsy-llm-client/src/backend.rs index a526712fc..131dda5c3 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 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.config().api_key.as_deref() 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(()) } 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-runner/src/config.rs b/crates/switchyard-runner/src/config.rs index a4bb309aa..5d64ae318 100644 --- a/crates/switchyard-runner/src/config.rs +++ b/crates/switchyard-runner/src/config.rs @@ -1390,7 +1390,7 @@ target = "azure" let configured = VALID_CONFIG.replacen( "base_url = \"https://example.test/v1\"", "base_url = \"https://example.test/v1\"\n\ - extra_headers = { X-Inference-Priority = \"batch\" }", + extra_headers = { X-Inference-Priority = \"batch\", X-Display-Name = \"café\" }", 1, ); @@ -1398,6 +1398,33 @@ target = "azure" Ok(()) } + #[test] + fn rejects_additional_headers_that_http_cannot_encode() { + let cases = [ + ( + "extra_headers = { \"bad header\" = \"value\" }", + "invalid HTTP header name \"bad header\"", + ), + ( + "extra_headers = { \"x-test-header\" = \"bad\\nvalue\" }", + "invalid HTTP header value for extra_headers entry \"x-test-header\"", + ), + ]; + + for (header_config, expected) in cases { + let configured = VALID_CONFIG.replacen( + "base_url = \"https://example.test/v1\"", + &format!("base_url = \"https://example.test/v1\"\n{header_config}"), + 1, + ); + let error = error_message(&configured); + assert!( + error.contains(expected), + "expected {expected:?}, got: {error}" + ); + } + } + #[test] fn retry_budget_rejects_negative_values() { let invalid = VALID_CONFIG.replacen( @@ -1446,6 +1473,41 @@ target = "azure" assert!(message.contains("is empty")); } + #[test] + fn rejects_api_keys_that_cannot_form_auth_headers() { + const INVALID_KEY_ENV: &str = "SWITCHYARD_CONFIG_TEST_INVALID_HEADER_KEY"; + const INVALID_KEY: &str = "canary\nsecret"; + unsafe { + std::env::set_var(INVALID_KEY_ENV, INVALID_KEY); + } + let cases = [ + "base_url = \"https://example.test/v1\"", + "base_url = \"https://example.test\"", + ]; + let messages = cases.map(|base_url| { + let configured = VALID_CONFIG.replacen( + base_url, + &format!("{base_url}\napi_key_env = \"{INVALID_KEY_ENV}\""), + 1, + ); + error_message(&configured) + }); + unsafe { + std::env::remove_var(INVALID_KEY_ENV); + } + + for message in messages { + assert!( + message.contains("api_key cannot be encoded as an HTTP header"), + "{message}" + ); + assert!( + !message.contains(INVALID_KEY), + "API key leaked in: {message}" + ); + } + } + #[test] fn forward_auth_rejects_conflicting_credentials() { let competing_auth = VALID_CONFIG.replacen( diff --git a/crates/switchyard-server/tests/cli.rs b/crates/switchyard-server/tests/cli.rs index f72c45858..138399615 100644 --- a/crates/switchyard-server/tests/cli.rs +++ b/crates/switchyard-server/tests/cli.rs @@ -43,3 +43,84 @@ target = "invalid" ); Ok(()) } + +#[test] +fn dry_run_rejects_invalid_configured_header() -> TestResult { + let directory = tempfile::tempdir()?; + let config = directory.path().join("routes.toml"); + fs::write( + &config, + r#" +schema_version = 1 + +[llm_clients.invalid] +format = "openai_chat" +base_url = "https://example.test/v1" +extra_headers = { "bad header" = "value" } + +[targets.invalid] +id = "upstream-model" +llm_client = "invalid" + +[routes.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()?; + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr)?; + assert!( + stderr.contains("invalid HTTP header name \"bad header\""), + "{stderr}" + ); + Ok(()) +} + +#[test] +fn dry_run_rejects_api_key_that_cannot_form_auth_header() -> TestResult { + const INVALID_KEY_ENV: &str = "SWITCHYARD_CLI_TEST_INVALID_HEADER_KEY"; + const INVALID_KEY: &str = "canary\nsecret"; + + let directory = tempfile::tempdir()?; + let config = directory.path().join("routes.toml"); + fs::write( + &config, + format!( + r#" +schema_version = 1 + +[llm_clients.invalid] +format = "openai_chat" +base_url = "https://example.test/v1" +api_key_env = "{INVALID_KEY_ENV}" + +[targets.invalid] +id = "upstream-model" +llm_client = "invalid" + +[routes.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"]) + .env(INVALID_KEY_ENV, INVALID_KEY) + .output()?; + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr)?; + assert!( + stderr.contains("api_key cannot be encoded as an HTTP header"), + "{stderr}" + ); + assert!(!stderr.contains(INVALID_KEY), "API key leaked in: {stderr}"); + Ok(()) +} From 3f176e0da520ce46311dcbedc8e35d778ec55529 Mon Sep 17 00:00:00 2001 From: Clement Pakkam Isaac Date: Fri, 4 Sep 2026 10:29:53 -0700 Subject: [PATCH 2/3] test: document header validation intent Signed-off-by: Clement Pakkam Isaac --- crates/switchyard-runner/src/config.rs | 3 +++ crates/switchyard-server/tests/cli.rs | 2 ++ 2 files changed, 5 insertions(+) diff --git a/crates/switchyard-runner/src/config.rs b/crates/switchyard-runner/src/config.rs index 5d64ae318..672fdb341 100644 --- a/crates/switchyard-runner/src/config.rs +++ b/crates/switchyard-runner/src/config.rs @@ -1385,6 +1385,7 @@ target = "azure" } } + // Header validation preserves opaque value bytes that the HTTP client can send. #[test] fn accepts_additional_headers() -> RunnerResult<()> { let configured = VALID_CONFIG.replacen( @@ -1398,6 +1399,7 @@ target = "azure" Ok(()) } + // Malformed names and values fail during offline deployment construction. #[test] fn rejects_additional_headers_that_http_cannot_encode() { let cases = [ @@ -1473,6 +1475,7 @@ target = "azure" assert!(message.contains("is empty")); } + // Both provider auth forms must be encodable without exposing credentials in errors. #[test] fn rejects_api_keys_that_cannot_form_auth_headers() { const INVALID_KEY_ENV: &str = "SWITCHYARD_CONFIG_TEST_INVALID_HEADER_KEY"; diff --git a/crates/switchyard-server/tests/cli.rs b/crates/switchyard-server/tests/cli.rs index 138399615..ba4be8536 100644 --- a/crates/switchyard-server/tests/cli.rs +++ b/crates/switchyard-server/tests/cli.rs @@ -44,6 +44,7 @@ target = "invalid" Ok(()) } +// Dry-run rejects malformed static header names before binding or routing. #[test] fn dry_run_rejects_invalid_configured_header() -> TestResult { let directory = tempfile::tempdir()?; @@ -81,6 +82,7 @@ target = "invalid" Ok(()) } +// Dry-run rejects malformed credentials without echoing them to stderr. #[test] fn dry_run_rejects_api_key_that_cannot_form_auth_header() -> TestResult { const INVALID_KEY_ENV: &str = "SWITCHYARD_CLI_TEST_INVALID_HEADER_KEY"; From c1bd4018020ff3027c49b0555ee4d8bde135a911 Mon Sep 17 00:00:00 2001 From: Clement Pakkam Isaac Date: Fri, 4 Sep 2026 15:31:15 -0700 Subject: [PATCH 3/3] refactor(llm-client): streamline header validation Signed-off-by: Clement Pakkam Isaac --- crates/libsy-llm-client/src/backend.rs | 88 ++++++++++++++++-- crates/switchyard-runner/src/config.rs | 67 +------------- crates/switchyard-server/tests/cli.rs | 119 ++++++++----------------- 3 files changed, 118 insertions(+), 156 deletions(-) diff --git a/crates/libsy-llm-client/src/backend.rs b/crates/libsy-llm-client/src/backend.rs index 131dda5c3..e7736c324 100644 --- a/crates/libsy-llm-client/src/backend.rs +++ b/crates/libsy-llm-client/src/backend.rs @@ -44,7 +44,7 @@ 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 values that cannot form the provider's auth header. + /// 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, @@ -130,7 +130,7 @@ impl Backend { }); } - let Some(api_key) = self.config().api_key.as_deref() else { + let Some(api_key) = self.configured_api_key() else { return Ok(()); }; let valid_api_key = match self { @@ -167,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 @@ -186,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 { @@ -443,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/switchyard-runner/src/config.rs b/crates/switchyard-runner/src/config.rs index 672fdb341..a4bb309aa 100644 --- a/crates/switchyard-runner/src/config.rs +++ b/crates/switchyard-runner/src/config.rs @@ -1385,13 +1385,12 @@ target = "azure" } } - // Header validation preserves opaque value bytes that the HTTP client can send. #[test] fn accepts_additional_headers() -> RunnerResult<()> { let configured = VALID_CONFIG.replacen( "base_url = \"https://example.test/v1\"", "base_url = \"https://example.test/v1\"\n\ - extra_headers = { X-Inference-Priority = \"batch\", X-Display-Name = \"café\" }", + extra_headers = { X-Inference-Priority = \"batch\" }", 1, ); @@ -1399,34 +1398,6 @@ target = "azure" Ok(()) } - // Malformed names and values fail during offline deployment construction. - #[test] - fn rejects_additional_headers_that_http_cannot_encode() { - let cases = [ - ( - "extra_headers = { \"bad header\" = \"value\" }", - "invalid HTTP header name \"bad header\"", - ), - ( - "extra_headers = { \"x-test-header\" = \"bad\\nvalue\" }", - "invalid HTTP header value for extra_headers entry \"x-test-header\"", - ), - ]; - - for (header_config, expected) in cases { - let configured = VALID_CONFIG.replacen( - "base_url = \"https://example.test/v1\"", - &format!("base_url = \"https://example.test/v1\"\n{header_config}"), - 1, - ); - let error = error_message(&configured); - assert!( - error.contains(expected), - "expected {expected:?}, got: {error}" - ); - } - } - #[test] fn retry_budget_rejects_negative_values() { let invalid = VALID_CONFIG.replacen( @@ -1475,42 +1446,6 @@ target = "azure" assert!(message.contains("is empty")); } - // Both provider auth forms must be encodable without exposing credentials in errors. - #[test] - fn rejects_api_keys_that_cannot_form_auth_headers() { - const INVALID_KEY_ENV: &str = "SWITCHYARD_CONFIG_TEST_INVALID_HEADER_KEY"; - const INVALID_KEY: &str = "canary\nsecret"; - unsafe { - std::env::set_var(INVALID_KEY_ENV, INVALID_KEY); - } - let cases = [ - "base_url = \"https://example.test/v1\"", - "base_url = \"https://example.test\"", - ]; - let messages = cases.map(|base_url| { - let configured = VALID_CONFIG.replacen( - base_url, - &format!("{base_url}\napi_key_env = \"{INVALID_KEY_ENV}\""), - 1, - ); - error_message(&configured) - }); - unsafe { - std::env::remove_var(INVALID_KEY_ENV); - } - - for message in messages { - assert!( - message.contains("api_key cannot be encoded as an HTTP header"), - "{message}" - ); - assert!( - !message.contains(INVALID_KEY), - "API key leaked in: {message}" - ); - } - } - #[test] fn forward_auth_rejects_conflicting_credentials() { let competing_auth = VALID_CONFIG.replacen( diff --git a/crates/switchyard-server/tests/cli.rs b/crates/switchyard-server/tests/cli.rs index ba4be8536..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,100 +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)?; - assert!( - stderr.contains("base_url must be an absolute HTTP(S) URL"), - "{stderr}" - ); - Ok(()) + Ok(String::from_utf8(output.stderr)?) } -// Dry-run rejects malformed static header names before binding or routing. #[test] -fn dry_run_rejects_invalid_configured_header() -> TestResult { - let directory = tempfile::tempdir()?; - let config = directory.path().join("routes.toml"); - fs::write( - &config, - r#" -schema_version = 1 - -[llm_clients.invalid] -format = "openai_chat" -base_url = "https://example.test/v1" -extra_headers = { "bad header" = "value" } - -[targets.invalid] -id = "upstream-model" -llm_client = "invalid" - -[routes.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()?; - assert!(!output.status.success()); - let stderr = String::from_utf8(output.stderr)?; +fn dry_run_rejects_invalid_base_url() -> TestResult { + let stderr = dry_run_error("base_url = \"not a url\"", None)?; assert!( - stderr.contains("invalid HTTP header name \"bad header\""), + stderr.contains("base_url must be an absolute HTTP(S) URL"), "{stderr}" ); Ok(()) } -// Dry-run rejects malformed credentials without echoing them to stderr. +// Dry-run rejects unsendable headers before startup without exposing credentials. #[test] -fn dry_run_rejects_api_key_that_cannot_form_auth_header() -> TestResult { +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 directory = tempfile::tempdir()?; - let config = directory.path().join("routes.toml"); - fs::write( - &config, - format!( - r#" -schema_version = 1 - -[llm_clients.invalid] -format = "openai_chat" -base_url = "https://example.test/v1" -api_key_env = "{INVALID_KEY_ENV}" - -[targets.invalid] -id = "upstream-model" -llm_client = "invalid" - -[routes.invalid] -id = "test-route" -type = "passthrough" -target = "invalid" -"# + 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", + ), + ]; - let output = Command::new(env!("CARGO_BIN_EXE_switchyard-server")) - .args(["--config", config.to_string_lossy().as_ref(), "--dry-run"]) - .env(INVALID_KEY_ENV, INVALID_KEY) - .output()?; - assert!(!output.status.success()); - let stderr = String::from_utf8(output.stderr)?; - assert!( - stderr.contains("api_key cannot be encoded as an HTTP header"), - "{stderr}" - ); - assert!(!stderr.contains(INVALID_KEY), "API key leaked in: {stderr}"); + 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(()) }