From 966f3defa980315e75983d5a6339b8e90d0451cd Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Fri, 29 May 2026 07:21:35 -0500 Subject: [PATCH 1/2] fix(gateway): bound teams webhook body size, document deserialization safety The teams webhook parses the inbound Bot Framework Activity JSON *before* JWT auth, because validating the token requires serviceUrl/channelId from the body. That makes the parse reachable unauthenticated. - Add a 256 KB pre-auth body-size guard (returns 413), mirroring the feishu adapter's WEBHOOK_BODY_LIMIT pattern. Real activities are a few KB; this shrinks the unauthenticated parse attack surface from axum's 2 MB default. - Document why the serde_json::from_str into Activity is safe, in response to an OX untrusted-deserialization finding (false positive): Activity is a derive-only strict DTO with no custom Deserialize, no side-effectful Drop, and no enum dispatch; serde_json cannot instantiate arbitrary types, and the recommended "strict DTO + validate after" pattern is already in place (JWT, activity-type, tenant-allowlist). No functional change to valid traffic. Co-Authored-By: Claude Opus 4.8 (1M context) --- gateway/src/adapters/teams.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/gateway/src/adapters/teams.rs b/gateway/src/adapters/teams.rs index d7b5433e4..123c57d84 100644 --- a/gateway/src/adapters/teams.rs +++ b/gateway/src/adapters/teams.rs @@ -417,6 +417,12 @@ fn ensure_trailing_slash(url: &str) -> String { // --- Webhook handler --- +/// Max webhook body size: 256 KB. Real Teams activities are a few KB; the +/// activity is parsed *before* JWT auth (Bot Framework requires serviceUrl / +/// channelId from the body to validate the token), so this caps the +/// unauthenticated parse attack surface. Mirrors the feishu adapter's limit. +const WEBHOOK_BODY_LIMIT: usize = 256 * 1024; + pub async fn webhook( State(state): State>, headers: HeaderMap, @@ -427,6 +433,12 @@ pub async fn webhook( None => return StatusCode::NOT_FOUND, }; + // Defense-in-depth: bound the pre-auth body size (axum's default limit is 2 MB). + if body.len() > WEBHOOK_BODY_LIMIT { + warn!(size = body.len(), "teams webhook body too large"); + return StatusCode::PAYLOAD_TOO_LARGE; + } + // Extract auth header early (before parsing activity) let auth_header = match headers.get("authorization").and_then(|v| v.to_str().ok()) { Some(h) => h.to_string(), @@ -436,7 +448,16 @@ pub async fn webhook( } }; - // Parse activity first (needed for JWT serviceUrl + endorsements validation) + // Parse activity first (needed for JWT serviceUrl + endorsements validation). + // + // SECURITY NOTE (OX untrusted-deserialization finding — false positive): + // `Activity` is a strict, derive-only DTO (String / Option<_> / nested + // structs) with no custom Deserialize, no side-effectful Drop, and no enum + // variant dispatch. serde_json's data model cannot instantiate arbitrary + // types (unlike bincode/serde_yaml/rmp-serde), so object-injection / RCE + // does not apply. The recommended "strict DTO + validate after" pattern is + // already in place: JWT, activity-type, and tenant-allowlist checks below. + // DoS is bounded by serde_json's recursion limit (128) and the body cap above. let activity: Activity = match serde_json::from_str(&body) { Ok(a) => a, Err(e) => { From 5e6f3917187caa91e37211b13743f9d5a12786f5 Mon Sep 17 00:00:00 2001 From: shaun-agent Date: Sun, 31 May 2026 17:52:53 +0000 Subject: [PATCH 2/2] test(gateway): cover teams webhook body limit --- gateway/src/adapters/teams.rs | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/gateway/src/adapters/teams.rs b/gateway/src/adapters/teams.rs index 123c57d84..593c3fcf7 100644 --- a/gateway/src/adapters/teams.rs +++ b/gateway/src/adapters/teams.rs @@ -648,6 +648,25 @@ mod tests { } } + fn make_test_state() -> Arc { + let (event_tx, _rx) = tokio::sync::broadcast::channel(16); + + Arc::new(crate::AppState { + telegram_bot_token: None, + telegram_secret_token: None, + line_channel_secret: None, + line_access_token: None, + teams: Some(TeamsAdapter::new(make_config(vec![]))), + teams_service_urls: tokio::sync::Mutex::new(std::collections::HashMap::new()), + feishu: None, + google_chat: None, + wecom: None, + ws_token: None, + event_tx, + reply_token_cache: Arc::new(std::sync::Mutex::new(std::collections::HashMap::new())), + }) + } + fn make_activity_with_tenant(tenant_id: Option<&str>) -> Activity { Activity { activity_type: "message".into(), @@ -665,6 +684,32 @@ mod tests { } } + // --- webhook body limit --- + + #[tokio::test] + async fn webhook_rejects_oversized_body_before_auth() { + let status = webhook( + State(make_test_state()), + HeaderMap::new(), + "x".repeat(WEBHOOK_BODY_LIMIT + 1), + ) + .await; + + assert_eq!(status, StatusCode::PAYLOAD_TOO_LARGE); + } + + #[tokio::test] + async fn webhook_allows_body_at_limit_to_reach_auth() { + let status = webhook( + State(make_test_state()), + HeaderMap::new(), + "x".repeat(WEBHOOK_BODY_LIMIT), + ) + .await; + + assert_eq!(status, StatusCode::UNAUTHORIZED); + } + #[test] fn tenant_allowed_when_list_empty() { let adapter = TeamsAdapter::new(make_config(vec![]));