mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(api): safely join api_url + path so misconfigured base can't corrupt routes (#1650)
This commit is contained in:
@@ -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<Mutex<()>> = 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!(
|
||||
|
||||
@@ -314,7 +314,7 @@ impl ComposioClient {
|
||||
error: Option<String>,
|
||||
}
|
||||
|
||||
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.
|
||||
|
||||
@@ -78,7 +78,7 @@ impl IntegrationClient {
|
||||
path: &str,
|
||||
body: &serde_json::Value,
|
||||
) -> anyhow::Result<T> {
|
||||
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<T: serde::de::DeserializeOwned>(&self, path: &str) -> anyhow::Result<T> {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user