chore(tracing): Enforce strict trace continuation

CHANGELOG.md

@@ -30,6 +30,10 @@
 - Added [`Dsn::org_id`](https://docs.rs/sentry-types/latest/sentry_types/struct.Dsn.html#method.org_id), which parses the Sentry SaaS organization ID from DSN hosts such as `o123.ingest.sentry.io` ([#1202](https://github.com/getsentry/sentry-rust/pull/1202)).
 - Added [`ClientOptions::org_id`](https://docs.rs/sentry-core/latest/sentry_core/struct.ClientOptions.html#method.org_id) and [`ClientOptions::strict_trace_continuation`](https://docs.rs/sentry-core/latest/sentry_core/struct.ClientOptions.html#method.strict_trace_continuation) ([#1203](https://github.com/getsentry/sentry-rust/pull/1203)). These options control how traces are continued and can help prevent traces from third-party services, which happen to be instrumented with Sentry, from being continued.
 
+### Improvements
+
+- Improved trace continuation safety by rejecting incoming traces with incompatible `sentry-org_id` values when starting transactions, according to [strict trace continuation rules](https://develop.sentry.dev/sdk/foundations/trace-propagation/#continuing-traces) ([#1218](https://github.com/getsentry/sentry-rust/pull/1218)).
+
 ## 0.48.4
 
 ### New Features

sentry-actix/src/lib.rs

@@ -482,6 +482,8 @@ mod tests {
 
     use futures::future::join_all;
     use sentry::Level;
+    use sentry_core::protocol::{Context, EnvelopeItem, TraceContext};
+    use sentry_core::Envelope;
 
     use super::*;
 
@@ -707,6 +709,83 @@ mod tests {
         assert_eq!(events.len(), 1);
     }
 
+    fn trace_context_from_single_transaction(envelopes: &[Envelope]) -> TraceContext {
+        let [envelope] = envelopes else {
+            panic!("Expected exactly one envelope");
+        };
+
+        let mut items = envelope.items();
+        let Some(EnvelopeItem::Transaction(transaction)) = items.next() else {
+            panic!("Expected a transaction envelope item");
+        };
+        assert!(items.next().is_none(), "expected only one envelope item");
+
+        match transaction.contexts.get("trace") {
+            Some(Context::Trace(trace)) => *trace.clone(),
+            unexpected => panic!("expected trace context, got {unexpected:#?}"),
+        }
+    }
+
+    fn run_request_with_org_ids(incoming_org_id: &str, client_org_id: &str) -> Vec<Envelope> {
+        sentry::test::with_captured_envelopes_options(
+            || {
+                block_on(async {
+                    let app = init_service(
+                        App::new()
+                            .wrap(
+                                Sentry::builder()
+                                    .with_hub(Hub::current())
+                                    .start_transaction(true)
+                                    .finish(),
+                            )
+                            .service(web::resource("/test").to(HttpResponse::Ok)),
+                    )
+                    .await;
+
+                    let req = TestRequest::get()
+                        .uri("/test")
+                        .insert_header((
+                            "sentry-trace",
+                            "09e04486820349518ac7b5d2adbf6ba5-9cf635fa5b870b3a-1",
+                        ))
+                        .insert_header(("baggage", format!("sentry-org_id={incoming_org_id}")))
+                        .to_request();
+                    let res = call_service(&app, req).await;
+                    assert!(res.status().is_success());
+                })
+            },
+            sentry::ClientOptions::new()
+                .org_id(client_org_id.parse().unwrap())
+                .strict_trace_continuation(true)
+                .traces_sample_rate(1.0),
+        )
+    }
+
+    #[actix_web::test]
+    async fn test_transaction_continues_matching_org_id() {
+        let envelopes = run_request_with_org_ids("42", "42");
+        let trace = trace_context_from_single_transaction(&envelopes);
+        assert_eq!(
+            trace.trace_id.to_string(),
+            "09e04486820349518ac7b5d2adbf6ba5"
+        );
+        assert_eq!(
+            trace.parent_span_id.map(|span_id| span_id.to_string()),
+            Some("9cf635fa5b870b3a".to_owned())
+        );
+    }
+
+    #[actix_web::test]
+    async fn test_transaction_rejects_mismatched_org_id() {
+        let envelopes = run_request_with_org_ids("43", "42");
+        let trace = trace_context_from_single_transaction(&envelopes);
+        assert_ne!(
+            trace.trace_id.to_string(),
+            "09e04486820349518ac7b5d2adbf6ba5"
+        );
+        assert_eq!(trace.parent_span_id, None);
+    }
+
     /// Ensures transaction name can be overridden in handler scope.
     #[actix_web::test]
     async fn test_override_transaction_name() {

sentry-core/src/performance/headers.rs

@@ -3,6 +3,7 @@
 use std::error::Error;
 use std::fmt::{Display, Formatter, Result as FmtResult};
 
+#[cfg(feature = "client")]
 use sentry_types::protocol::v7::OrganizationId;
 
 use crate::protocol::{SpanId, TraceId};
@@ -11,6 +12,7 @@ use crate::protocol::{SpanId, TraceId};
 type Header<'h> = (&'h str, &'h str);
 
 /// The baggage key for the Sentry org ID.
+#[cfg(feature = "client")]
 const SENTRY_ORG_ID: &str = "sentry-org_id";
 
 /// The [trace propagation] context.
@@ -31,6 +33,7 @@ pub struct TracePropagationContext {
     pub(crate) trace_id: TraceId,
     pub(crate) span_id: SpanId,
     pub(super) sampled: Option<bool>,
+    #[cfg(feature = "client")]
     pub(super) org_id: Option<OrganizationId>,
 }
 
@@ -60,6 +63,7 @@ impl TracePropagationContext {
             trace_id,
             span_id,
             sampled: None,
+            #[cfg(feature = "client")]
             org_id: None,
         }
     }
@@ -76,7 +80,8 @@ impl TracePropagationContext {
             trace_id,
             span_id,
             sampled,
-            org_id: _,
+            #[cfg(feature = "client")]
+                org_id: _,
         } = self;
 
         let sampled_suffix = sampled
@@ -94,6 +99,7 @@ impl TracePropagationContext {
         I: IntoIterator<Item = Header<'a>>,
     {
         let mut context_result = Err(HeaderParseError::Missing);
+        #[cfg(feature = "client")]
         let mut baggage = SentryBaggage::default();
 
         for (header, value) in headers {
@@ -104,14 +110,20 @@ impl TracePropagationContext {
                     .map_or(context_result, Ok)
                     .map_err(|_| HeaderParseError::Invalid);
             } else if header.eq_ignore_ascii_case("baggage") {
+                #[cfg(feature = "client")]
                 baggage.update_from_header(value);
             }
         }
 
         let context = context_result?;
 
+        #[cfg(feature = "client")]
         let SentryBaggage { org_id } = baggage;
-        Ok(TracePropagationContext { org_id, ..context })
+        Ok(TracePropagationContext {
+            #[cfg(feature = "client")]
+            org_id,
+            ..context
+        })
     }
 
     /// Attempts to construct a [`TracePropagationContext`] from the given Sentry trace header.
@@ -133,6 +145,7 @@ impl TracePropagationContext {
             trace_id,
             span_id,
             sampled,
+            #[cfg(feature = "client")]
             org_id: None,
         })
     }
@@ -150,7 +163,8 @@ where
         trace_id,
         span_id,
         sampled,
-        org_id: _,
+        #[cfg(feature = "client")]
+            org_id: _,
     } = TracePropagationContext::try_from_headers(headers).ok()?;
 
     Some(SentryTrace {
@@ -192,6 +206,7 @@ impl From<SentryTrace> for TracePropagationContext {
             trace_id: trace.trace_id,
             span_id: trace.span_id,
             sampled: trace.sampled,
+            #[cfg(feature = "client")]
             org_id: None,
         }
     }
@@ -211,11 +226,13 @@ impl std::fmt::Display for SentryTrace {
 /// A struct containing known Sentry baggage values.
 ///
 /// For now, this only includes the `org_id`, but we can add more values as we support them.
+#[cfg(feature = "client")]
 #[derive(Debug, Default)]
 struct SentryBaggage {
     org_id: Option<OrganizationId>,
 }
 
+#[cfg(feature = "client")]
 impl SentryBaggage {
     /// Update `self` with the known Sentry baggage values in the provided [baggage header].
     ///
@@ -264,6 +281,7 @@ mod tests {
                 trace_id,
                 span_id: parent_trace_id,
                 sampled: Some(false),
+                #[cfg(feature = "client")]
                 org_id: None,
             }
         );
@@ -277,6 +295,7 @@ mod tests {
         assert_eq!(parsed, trace);
     }
 
+    #[cfg(feature = "client")]
     #[test]
     fn parses_baggage_org_id() {
         let trace = TracePropagationContext::try_from_headers([
@@ -291,6 +310,7 @@ mod tests {
         assert_eq!(trace.org_id, Some("123".parse().unwrap()));
     }
 
+    #[cfg(feature = "client")]
     #[test]
     fn parses_baggage_org_id_with_unrelated_fields() {
         let trace = TracePropagationContext::try_from_headers([
@@ -308,6 +328,7 @@ mod tests {
         assert_eq!(trace.org_id, Some("123".parse().unwrap()));
     }
 
+    #[cfg(feature = "client")]
     #[test]
     fn accepts_mixed_case_baggage_header_name() {
         let trace = TracePropagationContext::try_from_headers([
@@ -322,6 +343,7 @@ mod tests {
         assert_eq!(trace.org_id, Some("123".parse().unwrap()));
     }
 
+    #[cfg(feature = "client")]
     #[test]
     fn treats_malformed_baggage_org_id_as_absent() {
         let trace = TracePropagationContext::try_from_headers([

sentry-core/src/performance/mod.rs

@@ -6,7 +6,9 @@ use std::time::SystemTime;
 
 #[cfg(feature = "client")]
 use sentry_types::protocol::v7::client_report::Reason as ClientReportReason;
-use sentry_types::protocol::v7::{OrganizationId, SpanId};
+#[cfg(feature = "client")]
+use sentry_types::protocol::v7::OrganizationId;
+use sentry_types::protocol::v7::SpanId;
 
 #[cfg(feature = "client")]
 use crate::clientoptions::TracesSamplingStrategy;
@@ -116,10 +118,7 @@ pub struct TransactionContext {
     parent_span_id: Option<protocol::SpanId>,
     span_id: protocol::SpanId,
     sampled: Option<bool>,
-    #[cfg_attr(
-        not(test),
-        expect(dead_code, reason = "used by future strict trace continuation")
-    )]
+    #[cfg(feature = "client")]
     incoming_org_id: Option<OrganizationId>,
     custom: Option<CustomTransactionContext>,
 }
@@ -156,6 +155,7 @@ impl TransactionContext {
             parent_span_id: None,
             span_id: Default::default(),
             sampled: None,
+            #[cfg(feature = "client")]
             incoming_org_id: None,
             custom: None,
         }
@@ -203,6 +203,7 @@ impl TransactionContext {
                 parent_span_id: None,
                 span_id: Default::default(),
                 sampled: None,
+                #[cfg(feature = "client")]
                 incoming_org_id: None,
                 custom: None,
             })
@@ -234,6 +235,7 @@ impl TransactionContext {
             trace_id,
             span_id: context_span_id,
             sampled,
+            #[cfg(feature = "client")]
             org_id,
         } = context;
 
@@ -243,6 +245,7 @@ impl TransactionContext {
             trace_id,
             parent_span_id: Some(context_span_id),
             sampled,
+            #[cfg(feature = "client")]
             incoming_org_id: org_id,
             span_id: span_id.unwrap_or_default(),
             custom: None,
@@ -283,6 +286,7 @@ impl TransactionContext {
             parent_span_id: Some(parent_span_id),
             span_id: protocol::SpanId::default(),
             sampled,
+            #[cfg(feature = "client")]
             incoming_org_id: None,
             custom: None,
         }
@@ -369,6 +373,17 @@ impl TransactionContext {
             ctx: TransactionContext::new(name, op),
         }
     }
+
+    /// Clears incoming trace state so the transaction starts a new trace.
+    #[cfg(feature = "client")]
+    fn reject_incoming_trace(&mut self) {
+        (
+            self.trace_id,
+            self.parent_span_id,
+            self.sampled,
+            self.incoming_org_id,
+        ) = Default::default();
+    }
 }
 
 /// A transaction context builder created by [`TransactionContext::builder`].
@@ -654,6 +669,19 @@ fn transaction_sample_rate(
     }
 }
 
+#[cfg(feature = "client")]
+fn should_continue_trace(
+    incoming: Option<OrganizationId>,
+    sdk: Option<OrganizationId>,
+    strict: bool,
+) -> bool {
+    match (incoming, sdk) {
+        (Some(incoming), Some(sdk)) => incoming == sdk,
+        (Some(_), None) | (None, Some(_)) => !strict,
+        (None, None) => true,
+    }
+}
+
 /// Determine whether the new transaction should be sampled.
 #[cfg(feature = "client")]
 impl Client {
@@ -720,15 +748,29 @@ impl<'a> TransactionData<'a> {
 
 impl Transaction {
     #[cfg(feature = "client")]
-    fn new(client: Option<Arc<Client>>, ctx: TransactionContext) -> Self {
+    fn new(client: Option<Arc<Client>>, mut ctx: TransactionContext) -> Self {
         let ((sampled, sample_rate), transaction) = match client.as_ref() {
-            Some(client) => (
-                client.determine_sampling_decision(&ctx),
-                Some(protocol::Transaction {
-                    name: Some(ctx.name),
-                    ..Default::default()
-                }),
-            ),
+            Some(client) => {
+                let options = client.options();
+                let incoming_org_id = ctx.incoming_org_id;
+                let sdk_org_id = options.org_id.or_else(|| options.dsn.as_ref()?.org_id());
+
+                if !should_continue_trace(
+                    incoming_org_id,
+                    sdk_org_id,
+                    options.strict_trace_continuation,
+                ) {
+                    ctx.reject_incoming_trace();
+                }
+
+                (
+                    client.determine_sampling_decision(&ctx),
+                    Some(protocol::Transaction {
+                        name: Some(ctx.name),
+                        ..Default::default()
+                    }),
+                )
+            }
             None => (
                 (
                     ctx.sampled.unwrap_or(false),