Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions src/api/rest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
113 changes: 112 additions & 1 deletion src/api/rest_tests.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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");
}
119 changes: 119 additions & 0 deletions src/core/observability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading