From 43f7185fcfcc0269b4c61a5354722596692c9679 Mon Sep 17 00:00:00 2001 From: Elyas Mehtabuddin Date: Mon, 12 Oct 2026 14:47:49 -0700 Subject: [PATCH 1/3] fix(runner): keep credential bytes out of api_key_env errors Signed-off-by: Elyas Mehtabuddin --- crates/switchyard-runner/src/config.rs | 55 +++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/crates/switchyard-runner/src/config.rs b/crates/switchyard-runner/src/config.rs index e8918efe0d..a26c09c59c 100644 --- a/crates/switchyard-runner/src/config.rs +++ b/crates/switchyard-runner/src/config.rs @@ -846,9 +846,19 @@ fn read_api_key(client: &str, variable: &str) -> RunnerResult { "{client} api_key_env must not be empty" ))); } + if variable.contains(['\0', '=']) { + return Err(RunnerError::configuration( + "Invalid credential environment variable name.", + )); + } let api_key = std::env::var(variable).map_err(|error| { + // VarError can contain the raw credential, so errors report only its category. + let reason = match error { + std::env::VarError::NotPresent => "environment variable not found", + std::env::VarError::NotUnicode(_) => "environment variable is not valid Unicode", + }; RunnerError::configuration(format!( - "{client} could not read api_key_env {variable}: {error}" + "{client} could not read api_key_env {variable}: {reason}" )) })?; if api_key.trim().is_empty() { @@ -2045,6 +2055,15 @@ confidence_threshold = 0.5 #[test] fn api_key_environment_reference_is_validated() { + const RAW_KEY_ENV: &str = "SWITCHYARD_CONFIG_TEST_RAW_KEY"; + if std::env::var_os(RAW_KEY_ENV).is_some() { + let error = read_api_key("fixture", RAW_KEY_ENV) + .expect_err("invalid raw credential") + .to_string(); + assert!(error.contains("not valid Unicode")); + assert!(!error.contains("DUMMY_SECRET_DO_NOT_LOG")); + return; + } let missing = VALID_CONFIG.replacen( "base_url = \"https://example.test/v1\"", "base_url = \"https://example.test/v1\"\napi_key_env = \"SWITCHYARD_CONFIG_TEST_KEY_THAT_IS_NOT_SET\"", @@ -2067,6 +2086,40 @@ confidence_threshold = 0.5 std::env::remove_var(EMPTY_KEY_ENV); } assert!(message.contains("is empty")); + #[cfg(unix)] + let raw = { + use std::os::unix::ffi::OsStringExt; + let mut bytes = b"DUMMY_SECRET_DO_NOT_LOG".to_vec(); + bytes.push(0xff); + std::ffi::OsString::from_vec(bytes) + }; + #[cfg(windows)] + let raw = { + use std::os::windows::ffi::OsStringExt; + let mut words: Vec<_> = "DUMMY_SECRET_DO_NOT_LOG".encode_utf16().collect(); + words.push(0xd800); + std::ffi::OsString::from_wide(&words) + }; + #[cfg(any(unix, windows))] + { + let output = + std::process::Command::new(std::env::current_exe().expect("test executable")) + .args([ + "--exact", + "config::deployment_tests::api_key_environment_reference_is_validated", + "--nocapture", + ]) + .env(RAW_KEY_ENV, raw) + .output() + .expect("raw credential regression"); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(String::from_utf8_lossy(&output.stdout).contains("1 passed")); + } } #[test] From b8e0537acba066096bcfc30d6b75cdd1d50f5fd9 Mon Sep 17 00:00:00 2001 From: Elyas Mehtabuddin Date: Thu, 15 Oct 2026 09:22:34 -0700 Subject: [PATCH 2/3] test(runner): check api_key_env errors in process and cover names with '=' Signed-off-by: Elyas Mehtabuddin --- crates/switchyard-runner/src/config.rs | 59 ++++++++++++-------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/crates/switchyard-runner/src/config.rs b/crates/switchyard-runner/src/config.rs index a26c09c59c..64d93d3195 100644 --- a/crates/switchyard-runner/src/config.rs +++ b/crates/switchyard-runner/src/config.rs @@ -847,9 +847,9 @@ fn read_api_key(client: &str, variable: &str) -> RunnerResult { ))); } if variable.contains(['\0', '=']) { - return Err(RunnerError::configuration( - "Invalid credential environment variable name.", - )); + return Err(RunnerError::configuration(format!( + "{client} api_key_env must not contain '=' or NUL" + ))); } let api_key = std::env::var(variable).map_err(|error| { // VarError can contain the raw credential, so errors report only its category. @@ -2055,15 +2055,6 @@ confidence_threshold = 0.5 #[test] fn api_key_environment_reference_is_validated() { - const RAW_KEY_ENV: &str = "SWITCHYARD_CONFIG_TEST_RAW_KEY"; - if std::env::var_os(RAW_KEY_ENV).is_some() { - let error = read_api_key("fixture", RAW_KEY_ENV) - .expect_err("invalid raw credential") - .to_string(); - assert!(error.contains("not valid Unicode")); - assert!(!error.contains("DUMMY_SECRET_DO_NOT_LOG")); - return; - } let missing = VALID_CONFIG.replacen( "base_url = \"https://example.test/v1\"", "base_url = \"https://example.test/v1\"\napi_key_env = \"SWITCHYARD_CONFIG_TEST_KEY_THAT_IS_NOT_SET\"", @@ -2086,6 +2077,21 @@ confidence_threshold = 0.5 std::env::remove_var(EMPTY_KEY_ENV); } assert!(message.contains("is empty")); + + // macOS reads `FOO` for the name `FOO=x`, so the config would use another variable. + let with_equals = VALID_CONFIG.replacen( + "base_url = \"https://example.test/v1\"", + "base_url = \"https://example.test/v1\"\napi_key_env = \"SWITCHYARD_CONFIG_TEST_KEY=x\"", + 1, + ); + assert!(error_message(&with_equals).contains("api_key_env must not contain '=' or NUL")); + } + + // `VarError::NotUnicode` holds the variable's value, so printing that error prints the key. + #[cfg(any(unix, windows))] + #[test] + fn invalid_unicode_api_key_is_not_printed() { + const RAW_KEY_ENV: &str = "SWITCHYARD_CONFIG_TEST_RAW_KEY"; #[cfg(unix)] let raw = { use std::os::unix::ffi::OsStringExt; @@ -2100,26 +2106,17 @@ confidence_threshold = 0.5 words.push(0xd800); std::ffi::OsString::from_wide(&words) }; - #[cfg(any(unix, windows))] - { - let output = - std::process::Command::new(std::env::current_exe().expect("test executable")) - .args([ - "--exact", - "config::deployment_tests::api_key_environment_reference_is_validated", - "--nocapture", - ]) - .env(RAW_KEY_ENV, raw) - .output() - .expect("raw credential regression"); - assert!( - output.status.success(), - "{}\n{}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - ); - assert!(String::from_utf8_lossy(&output.stdout).contains("1 passed")); + unsafe { + // "unsafe" is for concurrent reads and writes, very rare + std::env::set_var(RAW_KEY_ENV, raw); + } + let result = read_api_key("llm client fixture", RAW_KEY_ENV); + unsafe { + std::env::remove_var(RAW_KEY_ENV); } + let error = result.expect_err("invalid Unicode credential").to_string(); + assert!(error.contains("not valid Unicode"), "{error}"); + assert!(!error.contains("DUMMY_SECRET_DO_NOT_LOG"), "{error}"); } #[test] From cba0b56643599e06dc6f91e6b0508254a7ae1b0d Mon Sep 17 00:00:00 2001 From: Elyas Mehtabuddin Date: Thu, 15 Oct 2026 09:24:34 -0700 Subject: [PATCH 3/3] test(runner): keep the variable name out of the api_key_env '=' error Signed-off-by: Elyas Mehtabuddin --- crates/switchyard-runner/src/config.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/switchyard-runner/src/config.rs b/crates/switchyard-runner/src/config.rs index 64d93d3195..ccb2e7c6eb 100644 --- a/crates/switchyard-runner/src/config.rs +++ b/crates/switchyard-runner/src/config.rs @@ -846,6 +846,7 @@ fn read_api_key(client: &str, variable: &str) -> RunnerResult { "{client} api_key_env must not be empty" ))); } + // A name with `=` is often a pasted `NAME=key` line, so this error omits the name. if variable.contains(['\0', '=']) { return Err(RunnerError::configuration(format!( "{client} api_key_env must not contain '=' or NUL" @@ -2078,13 +2079,16 @@ confidence_threshold = 0.5 } assert!(message.contains("is empty")); - // macOS reads `FOO` for the name `FOO=x`, so the config would use another variable. + // macOS reads `FOO` for the name `FOO=x`, so the config would use another variable. A + // name with `=` is often a pasted `NAME=key` line, so the error leaves it out. let with_equals = VALID_CONFIG.replacen( "base_url = \"https://example.test/v1\"", - "base_url = \"https://example.test/v1\"\napi_key_env = \"SWITCHYARD_CONFIG_TEST_KEY=x\"", + "base_url = \"https://example.test/v1\"\napi_key_env = \"NVIDIA_API_KEY=DUMMY_SECRET_DO_NOT_LOG\"", 1, ); - assert!(error_message(&with_equals).contains("api_key_env must not contain '=' or NUL")); + let message = error_message(&with_equals); + assert!(message.contains("llm client primary api_key_env must not contain '=' or NUL")); + assert!(!message.contains("DUMMY_SECRET_DO_NOT_LOG"), "{message}"); } // `VarError::NotUnicode` holds the variable's value, so printing that error prints the key.