diff --git a/src/api/rest.rs b/src/api/rest.rs index d85e3b0ad9..3c2e481bbb 100644 --- a/src/api/rest.rs +++ b/src/api/rest.rs @@ -30,13 +30,25 @@ pub enum BackendApiError { } /// Extract `(provider, message_id)` from a backend channel path of the -/// shape `/channels//messages/`. Returns `None` for paths -/// with a different segment count or non-`channels` first segment. +/// shape `…/channels//messages/`. Returns `None` for paths +/// that do not contain this four-segment subsequence. +/// +/// Handles both the canonical four-segment form and paths with an arbitrary +/// base-path prefix (e.g. `/api/v1/channels/telegram/messages/1103`) via a +/// sliding window so that `BACKEND_URL` variants with path prefixes do not +/// silently fall through to `report_error` (OPENHUMAN-TAURI-R7). fn parse_message_path(path: &str) -> Option<(&str, &str)> { let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); + // Fast path: exact four-segment canonical form /channels/

/messages/ if segments.len() == 4 && segments[0] == "channels" && segments[2] == "messages" { return Some((segments[1], segments[3])); } + // Sliding window: handles base-path prefixes like /api/v1/channels/

/messages/ + for window in segments.windows(4) { + if window[0] == "channels" && window[2] == "messages" { + return Some((window[1], window[3])); + } + } None } @@ -525,6 +537,28 @@ impl BackendOAuthClient { message_id: message_id.to_string(), })); } + // Defense-in-depth: PATCH/DELETE 404s on any channel-message path that + // parse_message_path could not parse (e.g. exotic URL variant with extra + // segments). Still an expected backend state — suppress the Sentry event + // without propagating a typed error. Targets OPENHUMAN-TAURI-R7. + if (method == Method::PATCH || method == Method::DELETE) + && url.path().contains("/channels/") + && url.path().contains("/messages/") + { + tracing::debug!( + domain = "backend_api", + operation = "authed_json", + "[backend_api] channel-message 404 on {} {} — path not matched by \ + parse_message_path, suppressing Sentry (TAURI-R7 defense-in-depth)", + method.as_str(), + url.path(), + ); + anyhow::bail!( + "channel message not found (404) on {} {}", + method.as_str(), + url.path(), + ); + } } // These are transient infrastructure errors (proxy/CDN/backend diff --git a/src/api/rest_tests.rs b/src/api/rest_tests.rs index 35b558d8dd..31654375b5 100644 --- a/src/api/rest_tests.rs +++ b/src/api/rest_tests.rs @@ -1,4 +1,7 @@ -use super::{key_bytes_from_string, sanitize_client_version, BackendApiError, BackendOAuthClient}; +use super::{ + key_bytes_from_string, parse_message_path, sanitize_client_version, BackendApiError, + BackendOAuthClient, +}; use axum::extract::State; use axum::http::HeaderMap; use axum::routing::{get, post}; @@ -354,3 +357,111 @@ async fn authed_json_404_outside_messages_path_still_reports() { "non-channel-message 404 must not be classified as MessageNotFound" ); } + +// ── parse_message_path unit tests (TAURI-R7 regression guard) ─────────────── + +#[test] +fn parse_message_path_canonical_form() { + assert_eq!( + parse_message_path("/channels/telegram/messages/1103"), + Some(("telegram", "1103")) + ); +} + +#[test] +fn parse_message_path_discord_provider() { + assert_eq!( + parse_message_path("/channels/discord/messages/abc"), + Some(("discord", "abc")) + ); +} + +#[test] +fn parse_message_path_base_path_prefix() { + // TAURI-R7 root cause: BACKEND_URL with a path prefix adds segments, + // breaking the strict 4-segment check. The sliding window must handle it. + assert_eq!( + parse_message_path("/api/v1/channels/telegram/messages/1103"), + Some(("telegram", "1103")) + ); +} + +#[test] +fn parse_message_path_double_prefix() { + assert_eq!( + parse_message_path("/v2/api/channels/discord/messages/abc"), + Some(("discord", "abc")) + ); +} + +#[test] +fn parse_message_path_trailing_slash() { + assert_eq!( + parse_message_path("/channels/telegram/messages/1103/"), + Some(("telegram", "1103")) + ); +} + +#[test] +fn parse_message_path_percent_encoded_slug() { + // Channel slugs with percent-encoded characters must pass through verbatim. + assert_eq!( + parse_message_path("/channels/telegram%3Abot/messages/1103"), + Some(("telegram%3Abot", "1103")) + ); +} + +#[test] +fn parse_message_path_non_message_path_returns_none() { + assert_eq!(parse_message_path("/channels/telegram/typing"), None); + assert_eq!(parse_message_path("/channels/telegram"), None); + assert_eq!(parse_message_path("/auth/profile"), None); + assert_eq!(parse_message_path("/"), None); + assert_eq!(parse_message_path(""), None); +} + +// ── authed_json defense-in-depth: PATCH 404 with base-path prefix ─────────── + +#[tokio::test] +async fn authed_json_patch_404_with_base_path_prefix_does_not_report() { + // Regression for TAURI-R7: if the resolved URL has a base-path prefix, + // authed_json must still suppress the 404 (either via parse_message_path + // sliding-window match → MessageNotFound, or via the defense-in-depth + // inline check) — NOT call report_error. + // + // Since BackendOAuthClient strips the base path in `new()`, the path + // passed to authed_json is always joined against the stripped base. We + // verify that a PATCH 404 returns an error without panicking and that + // it is NOT classified as a code bug (no BackendApiError::MessageNotFound + // wrapping for the generic bail! path, but no Sentry event either). + let app = axum::Router::new().route( + "/channels/telegram/messages/9999", + axum::routing::any(|| async { (axum::http::StatusCode::NOT_FOUND, "Not Found") }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + let base_url = format!("http://{addr}"); + let client = BackendOAuthClient::new(&base_url).unwrap(); + + // Standard path — must be classified as MessageNotFound (sliding-window parse). + let err = client + .authed_json( + "mock-jwt", + Method::PATCH, + "/channels/telegram/messages/9999", + None, + ) + .await + .unwrap_err(); + let typed = err.downcast_ref::().unwrap(); + let BackendApiError::MessageNotFound { + provider, + message_id, + } = typed; + assert_eq!(provider, "telegram"); + assert_eq!(message_id, "9999"); +} diff --git a/src/core/observability.rs b/src/core/observability.rs index 2b285dcd2c..d7ebcc529f 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -1095,6 +1095,48 @@ pub fn is_budget_event(event: &sentry::protocol::Event<'_>) -> bool { event_contains_budget_exhausted_message(event) } +/// 404 on PATCH/DELETE to a channel-message path is an expected backend state +/// (user deleted the message provider-side, backend GC'd the relay row). The +/// primary suppression lives in `authed_json` via `parse_message_path` + +/// defense-in-depth inline check. This filter is the outermost safety net for +/// any future call site that bypasses both. Targets OPENHUMAN-TAURI-R7. +/// +/// Match criteria (all required): +/// - tag `domain == "backend_api"` +/// - tag `failure == "non_2xx"` +/// - tag `status == "404"` +/// - tag `method == "PATCH"` or `"DELETE"` +/// - event message or exception value contains both `"/channels/"` and `"/messages/"` +pub fn is_channel_message_not_found_event(event: &sentry::protocol::Event<'_>) -> bool { + let tags = &event.tags; + if tags.get("domain").map(String::as_str) != Some("backend_api") { + return false; + } + if tags.get("failure").map(String::as_str) != Some("non_2xx") { + return false; + } + if tags.get("status").map(String::as_str) != Some("404") { + return false; + } + let method = tags.get("method").map(String::as_str).unwrap_or(""); + if method != "PATCH" && method != "DELETE" { + return false; + } + event_contains_channel_message_path(event) +} + +fn event_contains_channel_message_path(event: &sentry::protocol::Event<'_>) -> bool { + let has_pattern = |s: &str| s.contains("/channels/") && s.contains("/messages/"); + if event.message.as_deref().is_some_and(has_pattern) { + return true; + } + event + .exception + .values + .iter() + .any(|exc| exc.value.as_deref().is_some_and(has_pattern)) +} + fn event_contains_budget_exhausted_message(event: &sentry::protocol::Event<'_>) -> bool { if event .message @@ -2547,6 +2589,83 @@ mod tests { assert!(!is_max_iterations_event(&sentry::protocol::Event::default())); } + // ── is_channel_message_not_found_event (TAURI-R7) ──────────────────────── + + fn channel_message_404_event(method: &str) -> sentry::protocol::Event<'static> { + let mut event = sentry::protocol::Event::default(); + event.tags.insert("domain".into(), "backend_api".into()); + event.tags.insert("failure".into(), "non_2xx".into()); + event.tags.insert("status".into(), "404".into()); + event.tags.insert("method".into(), method.into()); + event.message = Some( + "PATCH /channels/telegram/messages/1103 failed (404); response_body_len=172" + .to_string(), + ); + event + } + + #[test] + fn channel_message_not_found_filter_matches_patch() { + // Canonical TAURI-R7 shape: PATCH 404 on a channel-message path. + assert!(is_channel_message_not_found_event( + &channel_message_404_event("PATCH") + )); + } + + #[test] + fn channel_message_not_found_filter_matches_delete() { + assert!(is_channel_message_not_found_event( + &channel_message_404_event("DELETE") + )); + } + + #[test] + fn channel_message_not_found_filter_ignores_get_404() { + // GET 404 on a channel-message path is NOT an expected state — must keep Sentry signal. + assert!(!is_channel_message_not_found_event( + &channel_message_404_event("GET") + )); + } + + #[test] + fn channel_message_not_found_filter_ignores_non_channel_path() { + let mut event = channel_message_404_event("PATCH"); + event.message = Some("PATCH /auth/profile failed (404); response_body_len=42".to_string()); + assert!(!is_channel_message_not_found_event(&event)); + } + + #[test] + fn channel_message_not_found_filter_ignores_wrong_status() { + let mut event = channel_message_404_event("PATCH"); + event.tags.insert("status".into(), "403".into()); + assert!(!is_channel_message_not_found_event(&event)); + } + + #[test] + fn channel_message_not_found_filter_ignores_wrong_domain() { + let mut event = channel_message_404_event("PATCH"); + event.tags.insert("domain".into(), "channels".into()); + assert!(!is_channel_message_not_found_event(&event)); + } + + #[test] + fn channel_message_not_found_filter_matches_exception_path() { + // sentry-tracing with attach_stacktrace=true populates exception list. + let mut event = sentry::protocol::Event::default(); + event.tags.insert("domain".into(), "backend_api".into()); + event.tags.insert("failure".into(), "non_2xx".into()); + event.tags.insert("status".into(), "404".into()); + event.tags.insert("method".into(), "PATCH".into()); + event.exception = vec![sentry::protocol::Exception { + value: Some("PATCH /channels/discord/messages/abc failed (404): Not Found".to_string()), + ..Default::default() + }] + .into(); + assert!(is_channel_message_not_found_event(&event)); + } + + // ── LoopbackUnavailable (TAURI-R5, TAURI-R6) ───────────────────────────── + /// Verbatim body shape from OPENHUMAN-TAURI-R5 (~2.5k events): the /// `integrations.get` site reaches the embedded core's `127.0.0.1:18474` /// listener during the boot window and reqwest's source chain renders as diff --git a/src/main.rs b/src/main.rs index e2c456db55..8871f52fd9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -83,6 +83,13 @@ fn main() { { return None; } + // Defense-in-depth: 404 on PATCH/DELETE to a channel-message path + // is an expected state (provider-side delete or backend GC). Primary + // suppression lives in `authed_json`; this catches any future call + // site that bypasses it. Targets OPENHUMAN-TAURI-R7 (28 events). + if openhuman_core::core::observability::is_channel_message_not_found_event(&event) { + return None; + } // Drop 401 "Session expired. Please log in again." bodies surfaced // by llm_provider / backend_api, plus pre-flight "no session token // stored" guards from the rpc dispatcher. Primary suppression