diff --git a/src/api/rest.rs b/src/api/rest.rs index c3ae10d2f..0311dc559 100644 --- a/src/api/rest.rs +++ b/src/api/rest.rs @@ -96,6 +96,69 @@ fn parse_message_path(path: &str) -> Option<(&str, &str)> { const CLIENT_VERSION_HEADER_MAX_LEN: usize = 64; +/// Max bytes of the `body_shape` key-name list echoed into the `authed_json` +/// report. Bounded so a body with pathologically many keys can't bloat the +/// event; truncation is UTF-8-safe. +const BACKEND_API_BODY_SHAPE_MAX_BYTES: usize = 120; + +/// PII-safe classification of a non-2xx response body for telemetry. +/// +/// `report_error`'s message is written to the core/Tauri daily logs BEFORE any +/// Sentry `before_send` scrubbing, and that scrubber only catches a few +/// secret-shaped patterns — so the raw body must never be echoed (a non-2xx body +/// can carry emails / profile JSON / OAuth errors / nonstandard token fields). +/// We emit only the SHAPE: for a JSON object, the count of top-level keys plus +/// the sorted subset that look like schema field names; otherwise a coarse +/// label. Even key NAMES are response-controlled (a foreign backend could return +/// `{"jo@example.com": 1}`), so only keys matching a conservative ASCII-identifier +/// shape are echoed — everything else is counted as `redacted` and never logged. +/// The surviving names are enough to identify which backend/gateway produced a +/// response — the `TAURI-RUST-8C` case (a 91-byte body matching no route this +/// backend emits), where our canonical envelope is `{success,error,errorCode}` +/// and a foreign gateway/proxy is not. +fn backend_api_body_shape(body: &str) -> String { + let trimmed = body.trim(); + if trimmed.is_empty() { + return "empty".to_string(); + } + match serde_json::from_str::(trimmed) { + Ok(Value::Object(map)) => { + let total = map.len(); + let mut safe: Vec<&str> = map + .keys() + .map(String::as_str) + .filter(|k| is_schema_like_key(k)) + .collect(); + safe.sort_unstable(); + let redacted = total - safe.len(); + // `safe` keys are ASCII identifiers, so the join is ASCII and the + // truncation can only ever land on a byte boundary — but route it + // through the UTF-8-safe truncator regardless (defence-in-depth). + let keys = crate::openhuman::util::truncate_at_byte_boundary( + &safe.join(","), + BACKEND_API_BODY_SHAPE_MAX_BYTES, + ); + format!("object(keys={total},safe=[{keys}],redacted={redacted})") + } + Ok(Value::Array(_)) => "array".to_string(), + Ok(_) => "scalar".to_string(), + Err(_) => "non_json".to_string(), + } +} + +/// A JSON key safe to echo into telemetry: a short ASCII identifier (the shape +/// of a schema field name). Anything else — non-ASCII, punctuation like `@`, +/// whitespace, or overlong — is treated as response-controlled data and excluded +/// so `body_shape` can never leak an email/UUID/free-text used as a key. +fn is_schema_like_key(key: &str) -> bool { + const MAX_KEY_LEN: usize = 40; + !key.is_empty() + && key.len() <= MAX_KEY_LEN + && key + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'_' | b'-' | b'.')) +} + fn sanitize_client_version(raw: &str) -> Option { let sanitized: String = raw .trim() @@ -672,12 +735,26 @@ impl BackendOAuthClient { url.path(), ); } else { + // Enrich the report with the two fields triage needs to pin a + // non-2xx's origin: the outbound `host` and a PII-safe `body_shape` + // (top-level JSON key names only — never values; see + // `backend_api_body_shape`). `report_error` previously logged only + // `response_body_len`, leaving us blind when a client hits a + // non-canonical backend (custom BACKEND_URL / proxy / foreign + // host) — TAURI-RUST-8C: 12k `GET /teams/me/usage` 404s from one + // user whose 91-byte body matched no route this backend emits, + // un-diagnosable because neither host nor shape was captured. + // `host_str()` carries no scheme/path/query/token. Telemetry only + // — the error still propagates below (no suppression). + let host = url.host_str().unwrap_or(""); + let body_shape = backend_api_body_shape(&text); crate::core::observability::report_error( format!( - "{} {} failed ({status}); response_body_len={}", + "{} {} failed ({status}); response_body_len={}; body_shape={}", method.as_str(), url.path(), - text.len() + text.len(), + body_shape, ) .as_str(), "backend_api", @@ -685,6 +762,7 @@ impl BackendOAuthClient { &[ ("method", method.as_str()), ("path", url.path()), + ("host", host), ("status", status_str.as_str()), ("failure", "non_2xx"), ], diff --git a/src/api/rest_tests.rs b/src/api/rest_tests.rs index 6ac969998..376e4f3bb 100644 --- a/src/api/rest_tests.rs +++ b/src/api/rest_tests.rs @@ -1,6 +1,6 @@ use super::{ - flatten_authed_error, key_bytes_from_string, parse_message_path, sanitize_client_version, - BackendApiError, BackendOAuthClient, + backend_api_body_shape, flatten_authed_error, key_bytes_from_string, parse_message_path, + sanitize_client_version, BackendApiError, BackendOAuthClient, BACKEND_API_BODY_SHAPE_MAX_BYTES, }; use axum::extract::State; use axum::http::HeaderMap; @@ -415,6 +415,111 @@ async fn authed_json_surfaces_unauthorized_on_401() { assert_eq!(path, "/referral/stats"); } +#[test] +fn backend_api_body_shape_emits_safe_keys_not_values() { + // PII guard (Codex P1 on #4058): the body SHAPE must expose only schema-like + // top-level key NAMES and NEVER the values — a non-2xx body can carry emails / + // tokens / profile JSON that would otherwise leak to unscrubbed daily logs. + let body = r#"{"error":"not found","email":"jo@example.com","token":"sk-secret"}"#; + let shape = backend_api_body_shape(body); + assert_eq!(shape, "object(keys=3,safe=[email,error,token],redacted=0)"); + assert!(!shape.contains("jo@example.com"), "value leaked: {shape}"); + assert!(!shape.contains("sk-secret"), "value leaked: {shape}"); + assert!(!shape.contains("not found"), "value leaked: {shape}"); +} + +#[test] +fn backend_api_body_shape_redacts_pii_and_nonidentifier_keys() { + // CodeRabbit Major on #4058: key NAMES are response-controlled too. A foreign + // backend can put an email / free text / unicode in the KEY position; those + // must be counted as `redacted`, never echoed. + let body = r#"{"jo@example.com":1,"a b":2,"naïve":3,"error":4}"#; + let shape = backend_api_body_shape(body); + // Only the schema-like `error` survives; the other three are redacted. + assert_eq!(shape, "object(keys=4,safe=[error],redacted=3)"); + assert!(!shape.contains("jo@example.com"), "PII key leaked: {shape}"); + assert!(!shape.contains("naïve"), "non-ascii key leaked: {shape}"); + assert!(!shape.contains("a b"), "free-text key leaked: {shape}"); +} + +#[test] +fn backend_api_body_shape_classifies_non_object_bodies() { + assert_eq!(backend_api_body_shape(""), "empty"); + assert_eq!(backend_api_body_shape(" "), "empty"); + assert_eq!( + backend_api_body_shape("Cannot GET /teams/me/usage"), + "non_json" + ); + assert_eq!(backend_api_body_shape("404"), "non_json"); + assert_eq!(backend_api_body_shape("[1,2,3]"), "array"); + assert_eq!(backend_api_body_shape("42"), "scalar"); +} + +#[test] +fn backend_api_body_shape_bounds_long_safe_key_list() { + // The `safe=[…]` list is truncated at BACKEND_API_BODY_SHAPE_MAX_BYTES = 120. + // Surviving keys are ASCII identifiers (non-ASCII keys are redacted upstream), + // so build many ASCII keys to overflow the cap and assert the truncation + // CONTRACT: bounded, ellipsis-terminated, and not carrying the last key. + let mut obj = serde_json::Map::new(); + for i in 0..30 { + obj.insert(format!("field{i:02}"), json!(1)); // 30 × "fieldNN" (7 bytes) ≫ 120 + } + let body = serde_json::to_string(&Value::Object(obj)).unwrap(); + let shape = backend_api_body_shape(&body); + + let keys = shape + .strip_prefix("object(keys=30,safe=[") + .and_then(|s| s.strip_suffix("],redacted=0)")) + .unwrap_or_else(|| panic!("unexpected shape: {shape}")); + assert!( + keys.len() <= BACKEND_API_BODY_SHAPE_MAX_BYTES, + "safe list exceeds cap ({} > {BACKEND_API_BODY_SHAPE_MAX_BYTES}): {keys}", + keys.len() + ); + assert!(keys.ends_with('…'), "expected ellipsis-terminated: {keys}"); + assert!( + !keys.contains("field29"), + "last key should be truncated away: {keys}" + ); +} + +#[tokio::test] +async fn authed_json_reports_non_channel_404_still_propagates() { + // TAURI-RUST-8C: a GET 404 on a non-channel path (e.g. `/teams/me/usage`) + // falls through to `report_error` (not a typed/suppressed state) — it must + // still return an Err (no suppression) and not a typed `BackendApiError`. + let app = Router::new().route( + "/teams/me/usage", + get(|| async { + ( + axum::http::StatusCode::NOT_FOUND, + r#"{"message":"Not Found"}"#, + ) + }), + ); + let listener = 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(); + + let err = client + .authed_json("mock-jwt", Method::GET, "/teams/me/usage", None) + .await + .unwrap_err(); + assert!(err.downcast_ref::().is_none()); + let msg = format!("{err:#}"); + assert!(msg.contains("404"), "error should carry the status: {msg}"); + assert!( + msg.contains("/teams/me/usage"), + "error should carry the path: {msg}" + ); +} + #[test] fn flatten_authed_error_maps_unauthorized_to_session_expired_sentinel() { // #3297: the typed `Unauthorized` (expected session-lapse 401) must flatten