fix(relay): Limit maximum number of otel logs deserialized from JSON by klochek · Pull Request #6273 · getsentry/relay

klochek · 2026-07-30T17:25:03Z

No description provided.

linear-code · 2026-07-30T17:25:07Z

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 2ce987e. Configure here.}

cursor · 2026-07-30T17:27:48Z

+fn parse_logs_data(format: OtelFormat, payload: &[u8], max_logs: usize) -> Result<LogsData, Error> {
    match format {
-        OtelFormat::Json => serde_json::from_slice(payload).map_err(|e| {
+        OtelFormat::Json => otel_json_deserializer::deserialize(payload, max_logs).map_err(|e| {


Limit error remapped to InvalidJson

Medium Severity

otel_json_deserializer::deserialize already returns logs::Error, including TooManyExpandedLogs, but parse_logs_data maps every failure to Invalid(InvalidJson). That drops the dedicated over-limit error and its InvalidLog outcome, so oversized expansions are misclassified as bad JSON.

^{Reviewed by Cursor Bugbot for commit 2ce987e. Configure here.}

cursor · 2026-07-30T17:27:48Z

+    type Value = v1::ResourceLogs;
+
+    fn deserialize<D: Deserializer<'de>>(self, d: D) -> Result<Self::Value, D::Error> {
+        self.0.spend()?;


Containers consume log expansion budget

Medium Severity

Meter::spend runs for each ResourceLogs and ScopeLogs as well as each LogRecord, but max_expanded_log_count is documented as the max logs produced by expansion. Multi-resource OTEL batches can burn most of the budget on containers and reject payloads still under the configured log count.

Additional Locations (1)

relay-server/src/processing/logs/integrations/otel_json_deserializer.rs#L182-L183

^{Reviewed by Cursor Bugbot for commit 2ce987e. Configure here.}

sentry · 2026-07-30T17:28:33Z

+    fn visit_seq<A: SeqAccess<'de>>(self, mut seq: A) -> Result<Self::Value, A::Error> {
+        let mut out = Vec::new();
+        while let Some(record) = seq.next_element::<v1::LogRecord>()? {
+            self.0.spend()?;
+            out.push(record);
+        }
+        Ok(out)
+    }


Bug: The spend() check in LogRecordsSeed::visit_seq happens after deserializing a log record with next_element(), causing one extra record to be deserialized beyond the limit.
_{Severity: LOW}

Suggested Fix

The budget check should occur before deserialization. This can be achieved by either wrapping v1::LogRecord deserialization in a new seed that checks the budget first, or by checking the budget before calling next_element(). A pattern similar to next_element_seed should be used.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: relay-server/src/processing/logs/integrations/otel_json_deserializer.rs#L232-L239 Potential issue: In `LogRecordsSeed::visit_seq`, the code calls `seq.next_element::<v1::LogRecord>()` to deserialize a log record before calling `self.0.spend()` to check if the budget has been exceeded. This results in one extra log record being fully deserialized into memory beyond the configured `max_logs` limit. Although this extra record is dropped and not included in the output, it violates the intended goal of limiting the deserialization process itself, leading to unnecessary memory allocation for one record.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

sentry · 2026-07-30T17:28:33Z

+    match res {
+        Ok(logs) => {
+            // Only call 'end' if we're on the Ok path (end expects EOF/trailing whitespace, will
+            // error otherwise.)
+            de.end()
+                .map_err(|_| logs::Error::Invalid(DiscardReason::InvalidJson))?;
+            Ok(logs)
+        }
+        Err(_) if budget.is_empty() => Err(logs::Error::TooManyExpandedLogs),
+        Err(_) => Err(logs::Error::Invalid(DiscardReason::InvalidJson)),
+    }


Bug: When max_logs is 0, any deserialization failure (like invalid JSON) incorrectly reports a TooManyExpandedLogs error because the budget is already empty.
_{Severity: LOW}

Suggested Fix

The error handling logic should be updated to distinguish between a budget that was exhausted during processing and one that started empty. One approach is to check if the budget was non-zero initially before attributing the error to TooManyExpandedLogs.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: relay-server/src/processing/logs/integrations/otel_json_deserializer.rs#L249-L259 Potential issue: The error handling logic in the `deserialize` function incorrectly attributes errors when the log limit is set to zero. If `max_logs` is configured to `0`, the `budget` starts empty. Consequently, any deserialization failure, including one caused by invalid JSON, will match the `Err(_) if budget.is_empty()` condition. This causes the function to incorrectly return a `TooManyExpandedLogs` error instead of an `InvalidJson` error, which can mislead operators about the root cause of the failure.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

sentry-warden · 2026-07-30T18:59:17Z

@@ -46,7 +48,9 @@ pub fn expand(

    let settings = match integration {


OTel protobuf log expansion ignores max_expanded_log_count limit

otel::expand receives max_expanded_log_count for both JSON and protobuf, but only applies it to JSON, allowing a protobuf payload with millions of minimal log records to bypass the limit and exhaust CPU.

Evidence

mod.rs:49 passes max_expanded_log_count to otel::expand(format, &payload, max_expanded_log_count, produce).

otel.rs:36 parse_logs_data(format, payload, max_logs) receives the limit as max_logs.

otel.rs:45 uses LogsData::decode(payload) for OtelFormat::Protobuf, silently discarding max_logs.

otel.rs:22-31 iterates every decoded record and calls produce for each, so a protobuf with millions of minimal records expands to millions of logs.

A crafted protobuf near the max envelope size (200 MiB) could contain millions of tiny LogRecord entries, each processed through relay_ourlogs::otel_to_sentry_log.

Also found at 6 additional locations

relay-config/src/config.rs:642-642

relay-config/src/config.rs:733

relay-server/src/processing/logs/integrations/otel.rs:45-50

relay-server/src/processing/logs/mod.rs:58

relay-server/src/processing/logs/process.rs:50-51

relay-server/src/processing/logs/mod.rs:161

_{Identified by Warden · wrdn-dos-review · U83-KKN}

sentry-warden · 2026-07-30T18:59:17Z

+    fn deserialize<D: Deserializer<'de>>(self, d: D) -> Result<Self::Value, D::Error> {
+        d.deserialize_seq(self)
+    }
+}


OTEL JSON deserializer delegates recursive AnyValue to unbounded serde without depth limit

The Meter count cap bounds the number of ResourceLogs, ScopeLogs, and LogRecord elements, but Resource, InstrumentationScope, and LogRecord are deserialized via opentelemetry_proto's derived serde impl, which recursively deserializes AnyValue with no depth bound. A single deeply nested element can cause a stack-overflow abort before the count cap is reached.

Evidence

relay-server/src/utils/rmp.rs:16-20 caps rmp_serde deserialization depth at 128 to prevent stack overflow, establishing a codebase precedent for bounding recursive deserialization.

LogRecordsSeed::visit_seq at line 223 calls seq.next_element::<v1::LogRecord>()?, delegating to the opentelemetry_proto derived Deserialize impl (compiled with with-serde).

opentelemetry_proto::tonic::common::v1::AnyValue is recursive through ArrayValue and KeyValueList/KeyValue; prost-generated serde provides no depth limit, so a JSON payload with ~10,000 nested kvlistValue levels (well under the 1 MiB max_log_size) can overflow the stack.

Similarly, Resource at line 138 and InstrumentationScope at line 180 are deserialized without any depth bound.

The Meter count cap at line 224 is enforced after each LogRecord is fully materialized, so the first attacker-crafted record is always deserialized regardless of budget.

Also found at 1 additional location

relay-server/src/processing/logs/integrations/otel.rs:38-43

_{Identified by Warden · wrdn-dos-review · A4V-3PA}

sentry-warden · 2026-07-30T18:59:17Z

        filter::feature_flag(ctx).reject(&logs)?;

-        let mut logs = process::expand(logs)?;
+        let mut logs = process::expand(logs, ctx.config.max_expanded_log_count())?;


NEL and Vercel log integrations expanded without count limit

The max_expanded_log_count parameter is forwarded to OTEL but not to NEL or Vercel, allowing unbounded JSON array deserialization from untrusted payloads.

Evidence

mod.rs:161 passes ctx.config.max_expanded_log_count() into process::expand.

integrations/mod.rs receives the limit but only forwards it to otel::expand.

nel::expand calls serde_json::from_slice::<Vec<_>>(payload) with no count cap.

vercel::expand calls serde_json::from_slice::<Vec<VercelLog>>(payload) for JSON and iterates all NdJson lines without applying any count limit.

_{Identified by Warden · wrdn-dos-review · UMT-LHG}

fix(relay): Limit maximum number of otel logs deserialized from JSON

2ce987e

klochek requested a review from a team as a code owner July 30, 2026 17:25

cursor Bot reviewed Jul 30, 2026

View reviewed changes

sentry Bot reviewed Jul 30, 2026

View reviewed changes

sentry-warden Bot reviewed Jul 30, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(relay): Limit maximum number of otel logs deserialized from JSON - #6273

fix(relay): Limit maximum number of otel logs deserialized from JSON#6273
klochek wants to merge 1 commit into
masterfrom
christopherklochek/ingest-1099-add-bounded-json-serializer

klochek commented Jul 30, 2026

Uh oh!

linear-code Bot commented Jul 30, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Jul 30, 2026

Uh oh!

cursor Bot Jul 30, 2026

Uh oh!

sentry Bot Jul 30, 2026

Uh oh!

sentry Bot Jul 30, 2026

Uh oh!

sentry-warden Bot Jul 30, 2026

Uh oh!

sentry-warden Bot Jul 30, 2026

Uh oh!

sentry-warden Bot Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -46,7 +48,9 @@ pub fn expand(

		let settings = match integration {

Uh oh!

Conversation

klochek commented Jul 30, 2026

Uh oh!

linear-code Bot commented Jul 30, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 30, 2026

Choose a reason for hiding this comment

Limit error remapped to InvalidJson

Uh oh!

cursor Bot Jul 30, 2026

Choose a reason for hiding this comment

Containers consume log expansion budget

Uh oh!

sentry Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

sentry-warden Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

sentry-warden Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

sentry-warden Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant