mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(channels): suppress Telegram PATCH 404 reaching Sentry (TAURI-R7) (#2222)
This commit is contained in:
+36
-2
@@ -30,13 +30,25 @@ pub enum BackendApiError {
|
||||
}
|
||||
|
||||
/// Extract `(provider, message_id)` from a backend channel path of the
|
||||
/// shape `/channels/<provider>/messages/<id>`. Returns `None` for paths
|
||||
/// with a different segment count or non-`channels` first segment.
|
||||
/// shape `…/channels/<provider>/messages/<id>`. 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/<p>/messages/<id>
|
||||
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/<p>/messages/<id>
|
||||
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
|
||||
|
||||
+112
-1
@@ -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::<BackendApiError>().unwrap();
|
||||
let BackendApiError::MessageNotFound {
|
||||
provider,
|
||||
message_id,
|
||||
} = typed;
|
||||
assert_eq!(provider, "telegram");
|
||||
assert_eq!(message_id, "9999");
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user