From 1d37d2dfca8455945c455c773b92811d0b0fa8ec Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Wed, 13 May 2026 08:39:34 -0700 Subject: [PATCH] fix(api): safely join api_url + path so misconfigured base can't corrupt routes (#1650) --- src/api/config.rs | 101 +++++++++++++++++++++++++++ src/openhuman/composio/client.rs | 2 +- src/openhuman/integrations/client.rs | 4 +- 3 files changed, 104 insertions(+), 3 deletions(-) diff --git a/src/api/config.rs b/src/api/config.rs index ebc5c9260..06635adfd 100644 --- a/src/api/config.rs +++ b/src/api/config.rs @@ -32,6 +32,43 @@ pub fn normalize_api_base_url(url: &str) -> String { url.trim().trim_end_matches('/').to_string() } +/// Safely join an API base URL with a path. +/// +/// Behaviour: +/// - Empty `path` → normalized `base` (no trailing slash). +/// - `path` starting with `/` → replaces any path on `base` (RFC 3986 +/// absolute-path reference). This is the case that protects us from a +/// misconfigured `api_url` like `https://api.tinyhumans.ai/openai/v1/chat/completions` +/// silently corrupting every `/agent-integrations/...` call. +/// - If `base` fails to parse as a URL, falls back to slash-safe concat +/// so callers always get a usable string. +/// +/// Paths SHOULD start with `/`. Relative paths (no leading slash) are +/// resolved against the base path per RFC 3986, which means the base's +/// last path segment is dropped — almost never what you want for an API. +pub fn api_url(base: &str, path: &str) -> String { + let base_trimmed = base.trim(); + if path.is_empty() { + return normalize_api_base_url(base_trimmed); + } + match url::Url::parse(base_trimmed) { + Ok(parsed) => match parsed.join(path) { + Ok(joined) => joined.to_string().trim_end_matches('/').to_string(), + Err(_) => fallback_concat(base_trimmed, path), + }, + Err(_) => fallback_concat(base_trimmed, path), + } +} + +fn fallback_concat(base: &str, path: &str) -> String { + let base = base.trim_end_matches('/'); + if path.starts_with('/') { + format!("{base}{path}") + } else { + format!("{base}/{path}") + } +} + /// Resolve API base URL from the environment. /// /// Each key is checked independently so that an empty `BACKEND_URL` does not @@ -117,6 +154,70 @@ mod tests { // parallel test execution (std::env is process-global). static ENV_LOCK: OnceLock> = OnceLock::new(); + #[test] + fn api_url_empty_path_returns_normalized_base() { + assert_eq!( + api_url("https://api.tinyhumans.ai", ""), + "https://api.tinyhumans.ai" + ); + assert_eq!( + api_url("https://api.tinyhumans.ai/", ""), + "https://api.tinyhumans.ai" + ); + assert_eq!( + api_url(" https://api.tinyhumans.ai/ ", ""), + "https://api.tinyhumans.ai" + ); + } + + #[test] + fn api_url_absolute_path_replaces_base_path() { + // This is the regression: api_url misconfigured with a path baked in + // must not corrupt /agent-integrations/* calls. + assert_eq!( + api_url( + "https://api.tinyhumans.ai/openai/v1/chat/completions", + "/agent-integrations/composio/toolkits" + ), + "https://api.tinyhumans.ai/agent-integrations/composio/toolkits" + ); + } + + #[test] + fn api_url_clean_base_joins_cleanly() { + assert_eq!( + api_url( + "https://api.tinyhumans.ai", + "/agent-integrations/composio/toolkits" + ), + "https://api.tinyhumans.ai/agent-integrations/composio/toolkits" + ); + assert_eq!( + api_url( + "https://api.tinyhumans.ai/", + "/agent-integrations/composio/toolkits" + ), + "https://api.tinyhumans.ai/agent-integrations/composio/toolkits" + ); + } + + #[test] + fn api_url_preserves_query_string_on_path() { + assert_eq!( + api_url( + "https://api.tinyhumans.ai", + "/agent-integrations/composio/tools?toolkits=gmail" + ), + "https://api.tinyhumans.ai/agent-integrations/composio/tools?toolkits=gmail" + ); + } + + #[test] + fn api_url_unparseable_base_falls_back_to_concat() { + assert_eq!(api_url("not a url", "/x"), "not a url/x"); + assert_eq!(api_url("not a url/", "/x"), "not a url/x"); + } + #[test] fn staging_app_env_uses_staging_default_api() { assert_eq!( diff --git a/src/openhuman/composio/client.rs b/src/openhuman/composio/client.rs index d12b40613..6ebd45ca4 100644 --- a/src/openhuman/composio/client.rs +++ b/src/openhuman/composio/client.rs @@ -314,7 +314,7 @@ impl ComposioClient { error: Option, } - let url = format!("{}{}", self.inner.backend_url, path); + let url = crate::api::config::api_url(&self.inner.backend_url, path); tracing::debug!("[composio] DELETE {}", url); // Build a fresh lightweight reqwest client for this DELETE. diff --git a/src/openhuman/integrations/client.rs b/src/openhuman/integrations/client.rs index 289146ceb..0c354ac78 100644 --- a/src/openhuman/integrations/client.rs +++ b/src/openhuman/integrations/client.rs @@ -78,7 +78,7 @@ impl IntegrationClient { path: &str, body: &serde_json::Value, ) -> anyhow::Result { - let url = format!("{}{}", self.backend_url, path); + let url = crate::api::config::api_url(&self.backend_url, path); tracing::debug!("[integrations] POST {}", url); let resp = self @@ -154,7 +154,7 @@ impl IntegrationClient { /// GET from a backend endpoint and parse the response `data` field. pub async fn get(&self, path: &str) -> anyhow::Result { - let url = format!("{}{}", self.backend_url, path); + let url = crate::api::config::api_url(&self.backend_url, path); tracing::debug!("[integrations] GET {}", url); let resp = self