Repository: getsentry/sentry-php (php)

Issue:
GuzzleTracingMiddleware can potentially leak private data in baggage header
### How do you use Sentry?

Self-hosted/on-premise

### Version

3cefa7b69f2ebb4e0042c4f39e5c18bf573a4cab

### Steps to Reproduce

I noticed that the GuzzleTracingMiddleware adds the baggage header, which contains various metadata such as the transaction name, to all outbound requests to external URLs.

I'm concerned that some data that isn't intended to be shared with third-parties could be sent via this avenue. For example, the transaction name might include the URL that is currently being accessed, which might include some secret or identifying tokens in the path or query string.

Whereas previously, only a relatively harmless sentry-trace identifier was being sent.

It seems like, by default, sending extra baggage data needs to be limited to certain acceptable external hosts?  Or at a minimum, usage of this middleware needs to be much better documented?

### Expected Result

No data is sent unintentionally to remote sites.

### Actual Result

A baggage HTTP header is sent to remote site, which might contain something like `sentry-trace_id=0123456789,sentry-sample_rate=1,sentry-transaction=GET%20http%3A%2F%2Fdev.example.com%2Fpassword%2Freset%3Ftoken%3Dsecret,sentry-public_key=0123456789,sentry-release=beta,sentry-environment=dev`

Pull request description:
feat: Add tracePropagationTargets option
To not break people's usage of the `GuzzleTracingMiddleware`, I only check for `tracePropagationTargets` when applying the `baggage` header.

Also nothing fancy here, a simple array with an allow-list of hosts.

Closes #1395 & #1361

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 942b3a8..d7bf096 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ## 3.9.0 (2022-10-04)
 
+- feat: Add tracePropagationTargets option (#1396)
 - feat: Expose a function to retrieve the URL of the CSP endpoint (#1378)
 - feat: Add support for Dynamic Sampling (#1360)
   - Add `segment` to `UserDataBag`
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
index 13a94bc..3b6e796 100644
--- a/phpstan-baseline.neon
+++ b/phpstan-baseline.neon
@@ -215,6 +215,11 @@ parameters:
 			count: 1
 			path: src/Options.php
 
+		-
+			message: "#^Method Sentry\\\\Options\\:\\:getTracePropagationTargets\\(\\) should return array\\<string\\> but returns mixed\\.$#"
+			count: 1
+			path: src/Options.php
+
 		-
 			message: "#^Method Sentry\\\\Options\\:\\:getTracesSampleRate\\(\\) should return float but returns mixed\\.$#"
 			count: 1
@@ -336,12 +341,12 @@ parameters:
 			path: src/UserDataBag.php
 
 		-
-			message: "#^Parameter \\#1 \\$username of method Sentry\\\\UserDataBag\\:\\:setUsername\\(\\) expects string\\|null, mixed given\\.$#"
+			message: "#^Parameter \\#1 \\$segment of method Sentry\\\\UserDataBag\\:\\:setSegment\\(\\) expects string\\|null, mixed given\\.$#"
 			count: 1
 			path: src/UserDataBag.php
 
 		-
-			message: "#^Parameter \\#1 \\$segment of method Sentry\\\\UserDataBag\\:\\:setSegment\\(\\) expects string\\|null, mixed given\\.$#"
+			message: "#^Parameter \\#1 \\$username of method Sentry\\\\UserDataBag\\:\\:setUsername\\(\\) expects string\\|null, mixed given\\.$#"
 			count: 1
 			path: src/UserDataBag.php
 
diff --git a/src/Options.php b/src/Options.php
index 8455e80..c506b88 100644
--- a/src/Options.php
+++ b/src/Options.php
@@ -396,6 +396,28 @@ public function setBeforeSendCallback(callable $callback): void
         $this->options = $this->resolver->resolve($options);
     }
 
+    /**
+     * Gets an allow list of trace propagation targets.
+     *
+     * @return string[]
+     */
+    public function getTracePropagationTargets(): array
+    {
+        return $this->options['trace_propagation_targets'];
+    }
+
+    /**
+     * Set an allow list of trace propagation targets.
+     *
+     * @param string[] $tracePropagationTargets Trace propagation targets
+     */
+    public function setTracePropagationTargets(array $tracePropagationTargets): void
+    {
+        $options = array_merge($this->options, ['trace_propagation_targets' => $tracePropagationTargets]);
+
+        $this->options = $this->resolver->resolve($options);
+    }
+
     /**
      * Gets a list of default tags for events.
      *
@@ -779,6 +801,7 @@ private function configureOptions(OptionsResolver $resolver): void
             'before_send' => static function (Event $event): Event {
                 return $event;
             },
+            'trace_propagation_targets' => [],
             'tags' => [],
             'error_types' => null,
             'max_breadcrumbs' => self::DEFAULT_MAX_BREADCRUMBS,
@@ -813,6 +836,7 @@ private function configureOptions(OptionsResolver $resolver): void
         $resolver->setAllowedTypes('dsn', ['null', 'string', 'bool', Dsn::class]);
         $resolver->setAllowedTypes('server_name', 'string');
         $resolver->setAllowedTypes('before_send', ['callable']);
+        $resolver->setAllowedTypes('trace_propagation_targets', 'string[]');
         $resolver->setAllowedTypes('tags', 'string[]');
         $resolver->setAllowedTypes('error_types', ['null', 'int']);
         $resolver->setAllowedTypes('max_breadcrumbs', 'int');
diff --git a/src/Tracing/GuzzleTracingMiddleware.php b/src/Tracing/GuzzleTracingMiddleware.php
index 2d074e5..d7c1c9a 100644
--- a/src/Tracing/GuzzleTracingMiddleware.php
+++ b/src/Tracing/GuzzleTracingMiddleware.php
@@ -22,6 +22,7 @@ public static function trace(?HubInterface $hub = null): Closure
         return static function (callable $handler) use ($hub): Closure {
             return static function (RequestInterface $request, array $options) use ($hub, $handler) {
                 $hub = $hub ?? SentrySdk::getCurrentHub();
+                $client = $hub->getClient();
                 $span = $hub->getSpan();
 
                 if (null === $span) {
@@ -35,8 +36,17 @@ public static function trace(?HubInterface $hub = null): Closure
                 $childSpan = $span->startChild($spanContext);
 
                 $request = $request
-                    ->withHeader('sentry-trace', $childSpan->toTraceparent())
-                    ->withHeader('baggage', $childSpan->toBaggage());
+                    ->withHeader('sentry-trace', $childSpan->toTraceparent());
+
+                // Check if the request destination is allow listed in the trace_propagation_targets option.
+                if (null !== $client) {
+                    $sdkOptions = $client->getOptions();
+
+                    if (\in_array($request->getUri()->getHost(), $sdkOptions->getTracePropagationTargets())) {
+                        $request = $request
+                            ->withHeader('baggage', $childSpan->toBaggage());
+                    }
+                }
 
                 $handlerPromiseCallback = static function ($responseOrException) use ($hub, $request, $childSpan) {
                     // We finish the span (which means setting the span end timestamp) first to ensure the measured time


Test patch:
diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php
index f918228..b1635da 100644
--- a/tests/OptionsTest.php
+++ b/tests/OptionsTest.php
@@ -248,6 +248,15 @@ static function (): void {},
             null,
         ];
 
+        yield [
+            'trace_propagation_targets',
+            ['www.example.com'],
+            'getTracePropagationTargets',
+            'setTracePropagationTargets',
+            null,
+            null,
+        ];
+
         yield [
             'before_breadcrumb',
             static function (): void {},
diff --git a/tests/Tracing/GuzzleTracingMiddlewareTest.php b/tests/Tracing/GuzzleTracingMiddlewareTest.php
index c5546b1..9a27de8 100644
--- a/tests/Tracing/GuzzleTracingMiddlewareTest.php
+++ b/tests/Tracing/GuzzleTracingMiddlewareTest.php
@@ -61,9 +61,14 @@ public function testTraceDoesNothingIfSpanIsNotSet(): void
     public function testTrace(Request $request, $expectedPromiseResult, array $expectedBreadcrumbData): void
     {
         $client = $this->createMock(ClientInterface::class);
-        $client->expects($this->exactly(3))
+        $client->expects($this->exactly(4))
             ->method('getOptions')
-            ->willReturn(new Options(['traces_sample_rate' => 1]));
+            ->willReturn(new Options([
+                'traces_sample_rate' => 1,
+                'trace_propagation_targets' => [
+                    'www.example.com',
+                ],
+            ]));
 
         $hub = new Hub($client);