mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
This commit is contained in:
+80
-2
@@ -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::<Value>(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<String> {
|
||||
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"),
|
||||
],
|
||||
|
||||
+107
-2
@@ -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("<html>404</html>"), "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::<BackendApiError>().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
|
||||
|
||||
Reference in New Issue
Block a user