mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(observability/auth): handle JWT-required expiry path and normalize backend API base URLs (#1551)
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
co-authored by
Steven Enamakel
parent
84d86da70e
commit
386b0025ed
+48
-17
@@ -203,8 +203,22 @@ pub struct BackendOAuthClient {
|
||||
|
||||
impl BackendOAuthClient {
|
||||
/// Creates a new `BackendOAuthClient` with the given API base URL.
|
||||
///
|
||||
/// Any path, query, or fragment in `api_base` is stripped so that
|
||||
/// `Url::join` always resolves root-relative REST paths correctly.
|
||||
/// This guards against callers who pass a full LLM completions URL
|
||||
/// (e.g. `https://host/v1/chat/completions`) instead of just the origin:
|
||||
/// without stripping, `join("teams/me/usage")` would produce the wrong
|
||||
/// path `/v1/chat/teams/me/usage` via RFC 3986 relative resolution.
|
||||
pub fn new(api_base: &str) -> Result<Self> {
|
||||
let base = Url::parse(api_base.trim()).context("Invalid API base URL")?;
|
||||
let mut base = Url::parse(api_base.trim()).context("Invalid API base URL")?;
|
||||
anyhow::ensure!(
|
||||
matches!(base.scheme(), "http" | "https") && base.host_str().is_some(),
|
||||
"API base URL must be an absolute http(s) URL with host"
|
||||
);
|
||||
base.set_path("");
|
||||
base.set_query(None);
|
||||
base.set_fragment(None);
|
||||
let client = build_backend_reqwest_client()?;
|
||||
Ok(Self { client, base })
|
||||
}
|
||||
@@ -429,23 +443,40 @@ impl BackendOAuthClient {
|
||||
let status = response.status();
|
||||
let text = response.text().await.unwrap_or_default();
|
||||
if !status.is_success() {
|
||||
let status_str = status.as_u16().to_string();
|
||||
crate::core::observability::report_error(
|
||||
format!(
|
||||
"{} {} failed ({status}): {text}",
|
||||
let status_code = status.as_u16();
|
||||
let status_str = status_code.to_string();
|
||||
// 502/503/504 are transient infrastructure errors (proxy/CDN/backend
|
||||
// temporarily unavailable). They are not code bugs and callers already
|
||||
// implement retry/disable logic, so skip Sentry to avoid noise.
|
||||
let is_transient_infra = matches!(status_code, 502 | 503 | 504);
|
||||
if is_transient_infra {
|
||||
tracing::warn!(
|
||||
method = method.as_str(),
|
||||
path = url.path(),
|
||||
status = status_code,
|
||||
"[backend_api] transient {status} on {} {} — not reporting to Sentry",
|
||||
method.as_str(),
|
||||
url.path()
|
||||
)
|
||||
.as_str(),
|
||||
"backend_api",
|
||||
"authed_json",
|
||||
&[
|
||||
("method", method.as_str()),
|
||||
("path", url.path()),
|
||||
("status", status_str.as_str()),
|
||||
("failure", "non_2xx"),
|
||||
],
|
||||
);
|
||||
url.path(),
|
||||
);
|
||||
} else {
|
||||
crate::core::observability::report_error(
|
||||
format!(
|
||||
"{} {} failed ({status}); response_body_len={}",
|
||||
method.as_str(),
|
||||
url.path(),
|
||||
text.len()
|
||||
)
|
||||
.as_str(),
|
||||
"backend_api",
|
||||
"authed_json",
|
||||
&[
|
||||
("method", method.as_str()),
|
||||
("path", url.path()),
|
||||
("status", status_str.as_str()),
|
||||
("failure", "non_2xx"),
|
||||
],
|
||||
);
|
||||
}
|
||||
anyhow::bail!(
|
||||
"{} {} failed ({status}): {text}",
|
||||
method.as_str(),
|
||||
|
||||
@@ -215,6 +215,39 @@ async fn backend_client_sends_x_core_version_on_auth_requests() {
|
||||
);
|
||||
}
|
||||
|
||||
// Regression: OPENHUMAN-TAURI-8K / Sentry issue 7473650958.
|
||||
// When config.api_url is a full LLM completions URL (e.g. /v1/chat/completions),
|
||||
// Url::join used to produce wrong paths like /v1/chat/teams/me/usage instead of
|
||||
// /teams/me/usage — BackendOAuthClient::new must strip the path to prevent this.
|
||||
#[test]
|
||||
fn new_strips_path_from_completions_url() {
|
||||
let client = BackendOAuthClient::new("https://api.tinyhumans.ai/v1/chat/completions").unwrap();
|
||||
let url = client.url_for("/teams/me/usage").unwrap();
|
||||
assert_eq!(url.path(), "/teams/me/usage");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_strips_path_from_openai_style_url() {
|
||||
let client = BackendOAuthClient::new("https://api.openai.com/v1/chat/completions").unwrap();
|
||||
let url = client.url_for("/teams/me/usage").unwrap();
|
||||
assert_eq!(url.path(), "/teams/me/usage");
|
||||
assert_eq!(url.host_str(), Some("api.openai.com"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_works_with_bare_origin() {
|
||||
let client = BackendOAuthClient::new("https://api.tinyhumans.ai").unwrap();
|
||||
let url = client.url_for("/teams/me/usage").unwrap();
|
||||
assert_eq!(url.path(), "/teams/me/usage");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_works_with_trailing_slash() {
|
||||
let client = BackendOAuthClient::new("https://api.tinyhumans.ai/").unwrap();
|
||||
let url = client.url_for("/teams/me/usage").unwrap();
|
||||
assert_eq!(url.path(), "/teams/me/usage");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn backend_raw_client_inherits_x_core_version_default_header() {
|
||||
let (base_url, captured) = spawn_header_capture_server().await;
|
||||
|
||||
@@ -138,11 +138,17 @@ pub async fn invoke_method(state: AppState, method: &str, params: Value) -> Resu
|
||||
/// repeatedly reporting this as a hard error to Sentry. See #1465-ish: users
|
||||
/// stuck on the onboarding `SkillsStep` would spam `composio_list_connections`
|
||||
/// failures every 5 s without ever being bounced back to the login screen.
|
||||
///
|
||||
/// "session JWT required" covers the case where a prior 401 already cleared the
|
||||
/// token and the very next RPC call (e.g. `channels_telegram_login_start`) finds
|
||||
/// no JWT in the store. This is the same auth-boundary condition, just surfaced
|
||||
/// as a local guard rather than a backend response.
|
||||
fn is_session_expired_error(msg: &str) -> bool {
|
||||
let lower = msg.to_lowercase();
|
||||
(lower.contains("401") && lower.contains("unauthorized"))
|
||||
|| lower.contains("invalid token")
|
||||
|| lower.contains("no backend session token")
|
||||
|| lower.contains("session jwt required")
|
||||
|| msg.contains("SESSION_EXPIRED")
|
||||
}
|
||||
|
||||
|
||||
@@ -597,6 +597,24 @@ fn is_session_expired_error_matches_missing_backend_session_token() {
|
||||
assert!(is_session_expired_error("NO BACKEND SESSION TOKEN"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_session_expired_error_matches_session_jwt_required() {
|
||||
// Regression: Sentry issue 7472592145.
|
||||
// A prior 401 clears the stored JWT; the very next RPC call (e.g.
|
||||
// channels_telegram_login_start) finds no token and returns "session JWT
|
||||
// required; complete login first". This is the same auth-boundary condition
|
||||
// and must not be reported to Sentry.
|
||||
assert!(is_session_expired_error(
|
||||
"session JWT required; complete login first"
|
||||
));
|
||||
assert!(is_session_expired_error(
|
||||
"session JWT required; complete login and store_session first"
|
||||
));
|
||||
assert!(is_session_expired_error("session JWT required"));
|
||||
// Case-insensitive.
|
||||
assert!(is_session_expired_error("SESSION JWT REQUIRED"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn escape_html_escapes_all_special_chars() {
|
||||
let raw = r#"<script>alert("x&y'z")</script>"#;
|
||||
|
||||
Reference in New Issue
Block a user