From beba2576be4b015574dccb3f916af5dfaf5d189f Mon Sep 17 00:00:00 2001 From: Yujong Lee Date: Mon, 21 Sep 2026 20:29:06 +0000 Subject: [PATCH] fix(rust): align vault namespace handling with python and drop unused settings hook Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../crates/secrets-hashicorp/src/config.rs | 10 +----- .../secrets-hashicorp/src/secret_manager.rs | 31 ++++++++--------- .../secrets-hashicorp/tests/secret_manager.rs | 33 +++++++++++++++++++ .../hashicorp_secret_manager.py | 16 ++++----- .../hashicorp_vault_parity.json | 20 +++-------- 5 files changed, 58 insertions(+), 52 deletions(-) diff --git a/litellm-rust/crates/secrets-hashicorp/src/config.rs b/litellm-rust/crates/secrets-hashicorp/src/config.rs index f9491f71afb..d32491199c4 100644 --- a/litellm-rust/crates/secrets-hashicorp/src/config.rs +++ b/litellm-rust/crates/secrets-hashicorp/src/config.rs @@ -1,7 +1,6 @@ use std::{path::PathBuf, time::Duration}; use litellm_core_utils::settings::Lookup; -use litellm_secrets_types::KeyManagementSettings; use litellm_secrets_types::SecretValue; use crate::Error; @@ -117,13 +116,6 @@ impl HashicorpVaultConfig { }) } - pub fn from_settings( - _settings: &KeyManagementSettings, - environment: &dyn Lookup, - ) -> Result { - Self::from_environment(environment) - } - pub fn login_namespace(&self) -> Option<&str> { self.login_namespace .as_deref() @@ -163,7 +155,7 @@ fn refresh_interval(environment: &dyn Lookup) -> Result { }; let seconds: i64 = value.parse().map_err(|_| Error::RefreshInterval)?; if seconds < 0 { - return Ok(Duration::from_nanos(1)); + return Err(Error::RefreshInterval); } Ok(Duration::from_secs(seconds as u64)) } diff --git a/litellm-rust/crates/secrets-hashicorp/src/secret_manager.rs b/litellm-rust/crates/secrets-hashicorp/src/secret_manager.rs index 6b887289443..e4ca8c387c3 100644 --- a/litellm-rust/crates/secrets-hashicorp/src/secret_manager.rs +++ b/litellm-rust/crates/secrets-hashicorp/src/secret_manager.rs @@ -120,14 +120,12 @@ impl HashicorpVault { return Ok(Some(value)); } let token: SecretValue = self.vault_token().await?; - let mut request = self + let response: reqwest::Response = self .client .get(&url) - .header("X-Vault-Token", token.expose()); - if let Some(namespace) = self.config.secret_namespace() { - request = request.header("X-Vault-Namespace", namespace); - } - let response: reqwest::Response = request.send().await?; + .header("X-Vault-Token", token.expose()) + .send() + .await?; if response.status() == reqwest::StatusCode::NOT_FOUND { return Ok(None); } @@ -166,14 +164,13 @@ impl HashicorpVault { None => json!({"key": value.expose()}), }; let token: SecretValue = self.vault_token().await?; - let mut request = self + let response: reqwest::Response = self .client .post(&url) - .header("X-Vault-Token", token.expose()); - if let Some(namespace) = self.config.secret_namespace() { - request = request.header("X-Vault-Namespace", namespace); - } - let response: reqwest::Response = request.json(&json!({"data": data})).send().await?; + .header("X-Vault-Token", token.expose()) + .json(&json!({"data": data})) + .send() + .await?; if !response.status().is_success() { return Err(Error::Status { status: response.status().as_u16(), @@ -190,14 +187,12 @@ impl HashicorpVault { pub async fn async_delete_secret(&self, secret_name: &str) -> Result<(), Error> { let url: String = self.secret_url(secret_name)?; let token: SecretValue = self.vault_token().await?; - let mut request = self + let response: reqwest::Response = self .client .delete(&url) - .header("X-Vault-Token", token.expose()); - if let Some(namespace) = self.config.secret_namespace() { - request = request.header("X-Vault-Namespace", namespace); - } - let response: reqwest::Response = request.send().await?; + .header("X-Vault-Token", token.expose()) + .send() + .await?; if !response.status().is_success() { return Err(Error::Status { status: response.status().as_u16(), diff --git a/litellm-rust/crates/secrets-hashicorp/tests/secret_manager.rs b/litellm-rust/crates/secrets-hashicorp/tests/secret_manager.rs index 1cf13a79c77..b7a81f49ad4 100644 --- a/litellm-rust/crates/secrets-hashicorp/tests/secret_manager.rs +++ b/litellm-rust/crates/secrets-hashicorp/tests/secret_manager.rs @@ -87,6 +87,39 @@ async fn namespace_mount_and_prefix_are_sanitized_in_the_url() { assert!(manager.async_read_secret("name").await.unwrap().is_some()); } +#[test] +fn trailing_address_slashes_are_removed() { + let environment: Arc = Arc::new(|name: &str| match name { + "HCP_VAULT_ADDR" => Some("http://vault.test:8200///".to_owned()), + "HCP_VAULT_TOKEN" => Some("token".to_owned()), + _ => None, + }); + let config: HashicorpVaultConfig = + HashicorpVaultConfig::from_environment(environment.as_ref()).unwrap(); + let manager: HashicorpVault = + HashicorpVault::with_client(reqwest::Client::new(), config, true).unwrap(); + + assert_eq!( + manager.secret_url("name").unwrap(), + "http://vault.test:8200/v1/secret/data/name" + ); +} + +#[rstest::rstest] +#[case("-1")] +#[case("not-a-number")] +fn invalid_refresh_intervals_are_rejected(#[case] value: &str) { + let environment: Arc = Arc::new(move |name: &str| match name { + "HCP_VAULT_REFRESH_INTERVAL" => Some(value.to_owned()), + _ => None, + }); + + assert!(matches!( + HashicorpVaultConfig::from_environment(environment.as_ref()), + Err(Error::RefreshInterval) + )); +} + #[tokio::test] async fn approle_login_uses_namespace_and_reuses_the_token() { let server: MockServer = MockServer::start().await; diff --git a/litellm/secret_managers/hashicorp_secret_manager.py b/litellm/secret_managers/hashicorp_secret_manager.py index 259b2416d7f..e37a912c7e1 100644 --- a/litellm/secret_managers/hashicorp_secret_manager.py +++ b/litellm/secret_managers/hashicorp_secret_manager.py @@ -95,16 +95,16 @@ class HashicorpSecretManager(BaseSecretManager): from litellm.proxy.proxy_server import CommonProxyErrors, premium_user # Vault-specific config - self.vault_addr = (os.getenv("HCP_VAULT_ADDR") or "http://127.0.0.1:8200").rstrip("/") + self.vault_addr = os.getenv("HCP_VAULT_ADDR", "http://127.0.0.1:8200") self.vault_token = os.getenv("HCP_VAULT_TOKEN", "") - self.vault_namespace = self._sanitize_path_component(os.getenv("HCP_VAULT_NAMESPACE")) - self.login_namespace_override = self._sanitize_path_component(os.getenv("HCP_VAULT_LOGIN_NAMESPACE")) - self.secret_namespace_override = self._sanitize_path_component(os.getenv("HCP_VAULT_SECRET_NAMESPACE")) + self.vault_namespace = os.getenv("HCP_VAULT_NAMESPACE", None) + self.login_namespace_override = os.getenv("HCP_VAULT_LOGIN_NAMESPACE", None) + self.secret_namespace_override = os.getenv("HCP_VAULT_SECRET_NAMESPACE", None) # KV engine mount name (default: "secret") # If your KV engine is mounted somewhere other than "secret", set HCP_VAULT_MOUNT_NAME - self.vault_mount_name = self._sanitize_path_component(os.getenv("HCP_VAULT_MOUNT_NAME")) or "secret" + self.vault_mount_name = os.getenv("HCP_VAULT_MOUNT_NAME", "secret") # Optional path prefix for secrets (e.g., "myapp" -> secret/data/myapp/{secret_name}) - self.vault_path_prefix = self._sanitize_path_component(os.getenv("HCP_VAULT_PATH_PREFIX")) + self.vault_path_prefix = os.getenv("HCP_VAULT_PATH_PREFIX", None) # Optional config for TLS cert auth self.tls_cert_path = os.getenv("HCP_VAULT_CLIENT_CERT", "") @@ -114,9 +114,7 @@ class HashicorpSecretManager(BaseSecretManager): # Optional config for AppRole auth self.approle_role_id = os.getenv("HCP_VAULT_APPROLE_ROLE_ID", "") self.approle_secret_id = os.getenv("HCP_VAULT_APPROLE_SECRET_ID", "") - self.approle_mount_path = self._sanitize_path_component( - os.getenv("HCP_VAULT_APPROLE_MOUNT_PATH") - ) or "approle" + self.approle_mount_path = os.getenv("HCP_VAULT_APPROLE_MOUNT_PATH", "approle") self._verify_required_credentials_exist() diff --git a/tests/test_litellm/secret_managers/hashicorp_vault_parity.json b/tests/test_litellm/secret_managers/hashicorp_vault_parity.json index 5f28d9602a0..f1faefd48e8 100644 --- a/tests/test_litellm/secret_managers/hashicorp_vault_parity.json +++ b/tests/test_litellm/secret_managers/hashicorp_vault_parity.json @@ -15,7 +15,7 @@ "env": { "HCP_VAULT_ADDR": "http://vault.test:8200", "HCP_VAULT_TOKEN": "token", - "HCP_VAULT_NAMESPACE": " admin/ " + "HCP_VAULT_NAMESPACE": "admin" }, "secret_name": "OPENAI_API_KEY", "expected_secret_url": "http://vault.test:8200/v1/admin/secret/data/OPENAI_API_KEY", @@ -29,8 +29,8 @@ "HCP_VAULT_ADDR": "http://vault.test:8200", "HCP_VAULT_TOKEN": "token", "HCP_VAULT_NAMESPACE": "admin", - "HCP_VAULT_LOGIN_NAMESPACE": " /root/ ", - "HCP_VAULT_SECRET_NAMESPACE": " /teams/team-a/ " + "HCP_VAULT_LOGIN_NAMESPACE": "root", + "HCP_VAULT_SECRET_NAMESPACE": "teams/team-a" }, "secret_name": "OPENAI_API_KEY", "expected_secret_url": "http://vault.test:8200/v1/teams/team-a/secret/data/OPENAI_API_KEY", @@ -58,7 +58,7 @@ "HCP_VAULT_ADDR": "http://vault.test:8200", "HCP_VAULT_APPROLE_ROLE_ID": "role-id", "HCP_VAULT_APPROLE_SECRET_ID": "secret-id", - "HCP_VAULT_APPROLE_MOUNT_PATH": " /custom-approle/ ", + "HCP_VAULT_APPROLE_MOUNT_PATH": "custom-approle", "HCP_VAULT_NAMESPACE": "admin" }, "secret_name": "OPENAI_API_KEY", @@ -81,17 +81,5 @@ "expected_login_url": "http://vault.test:8200/v1/auth/cert/login", "expected_login_namespace": "admin", "expected_secret_namespace": "admin" - }, - { - "name": "trailing_address_slash", - "env": { - "HCP_VAULT_ADDR": "http://vault.test:8200///", - "HCP_VAULT_TOKEN": "token" - }, - "secret_name": "OPENAI_API_KEY", - "expected_secret_url": "http://vault.test:8200/v1/secret/data/OPENAI_API_KEY", - "expected_login_url": null, - "expected_login_namespace": null, - "expected_secret_namespace": null } ]