Replace session key in cache integration with placeholder

src/Sentry/Laravel/Features/CacheIntegration.php

@@ -4,6 +4,7 @@
 
 use Illuminate\Cache\Events;
 use Illuminate\Contracts\Events\Dispatcher;
+use Illuminate\Contracts\Session\Session;
 use Illuminate\Redis\Events as RedisEvents;
 use Illuminate\Redis\RedisManager;
 use Illuminate\Support\Str;
@@ -86,11 +87,13 @@ public function handleCacheEventsForBreadcrumbs(Events\CacheEvent $event): void
                 return;
         }
 
+        $displayKey = $this->replaceSessionKey($event->key);
+
         Integration::addBreadcrumb(new Breadcrumb(
             Breadcrumb::LEVEL_INFO,
             Breadcrumb::TYPE_DEFAULT,
             'cache',
-            "{$message}: {$event->key}",
+            "{$message}: {$displayKey}",
             $event->tags ? ['tags' => $event->tags] : []
         ));
     }
@@ -109,15 +112,17 @@ public function handleCacheEventsForTracing(Events\CacheEvent $event): void
                         : $event->keys
                 );
 
+                $displayKeys = $this->replaceSessionKeys($keys);
+
                 $this->pushSpan(
                     $parentSpan->startChild(
                         SpanContext::make()
                             ->setOp('cache.get')
                             ->setData([
-                                'cache.key' => $keys,
+                                'cache.key' => $displayKeys,
                             ])
                             ->setOrigin('auto.cache')
-                            ->setDescription(implode(', ', $keys))
+                            ->setDescription(implode(', ', $displayKeys))
                     )
                 );
             }
@@ -129,30 +134,34 @@ public function handleCacheEventsForTracing(Events\CacheEvent $event): void
                         : $event->keys
                 );
 
+                $displayKeys = $this->replaceSessionKeys($keys);
+
                 $this->pushSpan(
                     $parentSpan->startChild(
                         SpanContext::make()
                             ->setOp('cache.put')
                             ->setData([
-                                'cache.key' => $keys,
+                                'cache.key' => $displayKeys,
                                 'cache.ttl' => $event->seconds,
                             ])
                             ->setOrigin('auto.cache')
-                            ->setDescription(implode(', ', $keys))
+                            ->setDescription(implode(', ', $displayKeys))
                     )
                 );
             }
 
             if ($event instanceof Events\ForgettingKey) {
+                $displayKey = $this->replaceSessionKey($event->key);
+
                 $this->pushSpan(
                     $parentSpan->startChild(
                         SpanContext::make()
                             ->setOp('cache.remove')
                             ->setData([
-                                'cache.key' => [$event->key],
+                                'cache.key' => [$displayKey],
                             ])
                             ->setOrigin('auto.cache')
-                            ->setDescription($event->key)
+                            ->setDescription($displayKey)
                     )
                 );
             }
@@ -177,7 +186,7 @@ public function handleRedisCommands(RedisEvents\CommandExecuted $event): void
         // If the first parameter is a string and does not contain a newline we use it as the description since it's most likely a key
         // This is not a perfect solution but it's the best we can do without understanding the command that was executed
         if (!empty($event->parameters[0]) && is_string($event->parameters[0]) && !Str::contains($event->parameters[0], "\n")) {
-            $keyForDescription = $event->parameters[0];
+            $keyForDescription = $this->replaceSessionKey($event->parameters[0]);
         }
 
         $context->setDescription(rtrim(strtoupper($event->command) . ' ' . $keyForDescription));
@@ -189,7 +198,7 @@ public function handleRedisCommands(RedisEvents\CommandExecuted $event): void
         ];
 
         if ($this->shouldSendDefaultPii()) {
-            $data['db.redis.parameters'] = $event->parameters;
+            $data['db.redis.parameters'] = $this->replaceSessionKeys($event->parameters);
         }
 
         if ($this->isTracingFeatureEnabled('redis_origin')) {
@@ -213,7 +222,7 @@ private function maybeHandleCacheEventAsEndOfSpan(Events\CacheEvent $event): boo
 
             if ($finishedSpan !== null && count($finishedSpan->getData()['cache.key'] ?? []) === 1) {
                 $finishedSpan->setData([
-                    'cache.hit' => $event instanceof Events\CacheHit,
+                    'cache.hit' => $event instanceof Events\CacheHit
                 ]);
             }
 
@@ -228,7 +237,7 @@ private function maybeHandleCacheEventAsEndOfSpan(Events\CacheEvent $event): boo
 
             if ($finishedSpan !== null) {
                 $finishedSpan->setData([
-                    'cache.success' => $event instanceof Events\KeyWritten,
+                    'cache.success' => $event instanceof Events\KeyWritten
                 ]);
             }
 
@@ -245,6 +254,53 @@ private function maybeHandleCacheEventAsEndOfSpan(Events\CacheEvent $event): boo
         return false;
     }
 
+    /**
+     * Retrieve the current session key if available.
+     */
+    private function getSessionKey(): ?string
+    {
+        try {
+            /** @var Session $sessionStore */
+            $sessionStore = $this->container()->make('session.store');
+
+            // It is safe for us to get the session ID here without checking if the session is started
+            // because getting the session ID does not start the session. In addition we need the ID before
+            // the session is started because the cache will retrieve the session ID from the cache before the session
+            // is considered started. So if we wait for the session to be started, we will not be able to replace the
+            // session key in the cache operation that is being executed to retrieve the session data from the cache.
+            return $sessionStore->getId();
+        } catch (\Exception $e) {
+            // We can assume the session store is not available here so there is no session key to retrieve
+            // We capture a generic exception to avoid breaking the application because some code paths can
+            // result in an exception other than the expected `Illuminate\Contracts\Container\BindingResolutionException`
+            return null;
+        }
+    }
+
+    /**
+     * Replace a session key with a placeholder.
+     */
+    private function replaceSessionKey(string $value): string
+    {
+        return $value === $this->getSessionKey() ? '{sessionKey}' : $value;
+    }
+
+    /**
+     * Replace session keys in an array of keys with placeholders.
+     *
+     * @param string[] $values
+     *
+     * @return mixed[]
+     */
+    private function replaceSessionKeys(array $values): array
+    {
+        $sessionKey = $this->getSessionKey();
+
+        return array_map(static function ($value) use ($sessionKey) {
+            return is_string($value) && $value === $sessionKey ? '{sessionKey}' : $value;
+        }, $values);
+    }
+
     /**
      * Normalize the array of keys to a array of only strings.
      *

test/Sentry/Features/CacheIntegrationTest.php

@@ -9,6 +9,10 @@
 
 class CacheIntegrationTest extends TestCase
 {
+    protected $defaultSetupConfig = [
+        'session.driver' => 'array',
+    ];
+
     public function testCacheBreadcrumbForWriteAndHitIsRecorded(): void
     {
         Cache::put($key = 'foo', 'bar');
@@ -51,6 +55,32 @@ public function testCacheBreadcrumbIsNotRecordedWhenDisabled(): void
         $this->assertEmpty($this->getCurrentSentryBreadcrumbs());
     }
 
+    public function testCacheBreadcrumbReplacesSessionKeyWithPlaceholder(): void
+    {
+        // Start a session properly in the test environment
+        $this->startSession();
+        $sessionId = $this->app['session']->getId();
+
+        // Use the session ID as a cache key
+        Cache::put($sessionId, 'session-data');
+
+        $breadcrumb = $this->getLastSentryBreadcrumb();
+        $this->assertEquals('Written: {sessionKey}', $breadcrumb->getMessage());
+
+        Cache::get($sessionId);
+
+        $breadcrumb = $this->getLastSentryBreadcrumb();
+        $this->assertEquals('Read: {sessionKey}', $breadcrumb->getMessage());
+    }
+
+    public function testCacheBreadcrumbDoesNotReplaceNonSessionKeys(): void
+    {
+        Cache::put('regular-key', 'value');
+
+        $breadcrumb = $this->getLastSentryBreadcrumb();
+        $this->assertEquals('Written: regular-key', $breadcrumb->getMessage());
+    }
+
     public function testCacheGetSpanIsRecorded(): void
     {
         $this->markSkippedIfTracingEventsNotAvailable();
@@ -147,6 +177,56 @@ public function testCacheRemoveSpanIsRecorded(): void
         $this->assertEquals(['foo'], $span->getData()['cache.key']);
     }
 
+    public function testCacheSpanReplacesSessionKeyWithPlaceholder(): void
+    {
+        $this->markSkippedIfTracingEventsNotAvailable();
+
+        $this->startSession();
+        $sessionId = $this->app['session']->getId();
+
+        $span = $this->executeAndReturnMostRecentSpan(function () use ($sessionId) {
+            Cache::get($sessionId);
+        });
+
+        $this->assertEquals('cache.get', $span->getOp());
+        $this->assertEquals('{sessionKey}', $span->getDescription());
+        $this->assertEquals(['{sessionKey}'], $span->getData()['cache.key']);
+    }
+
+    public function testCacheSpanReplacesMultipleSessionKeysWithPlaceholder(): void
+    {
+        $this->markSkippedIfTracingEventsNotAvailable();
+
+        // Start a session properly in the test environment
+        $this->startSession();
+        $sessionId = $this->app['session']->getId();
+
+        $span = $this->executeAndReturnMostRecentSpan(function () use ($sessionId) {
+            Cache::get([$sessionId, 'regular-key', $sessionId . '_another']);
+        });
+
+        $this->assertEquals('cache.get', $span->getOp());
+        $this->assertEquals('{sessionKey}, regular-key, ' . $sessionId . '_another', $span->getDescription());
+        $this->assertEquals(['{sessionKey}', 'regular-key', $sessionId . '_another'], $span->getData()['cache.key']);
+    }
+
+    public function testCacheOperationDoesNotStartSessionPrematurely(): void
+    {
+        $this->markSkippedIfTracingEventsNotAvailable();
+
+        // Don't start a session to ensure it's not started
+
+        $span = $this->executeAndReturnMostRecentSpan(function () {
+            Cache::get('some-key');
+        });
+
+        // Check that session was not started
+        $this->assertFalse($this->app['session']->isStarted());
+
+        // And the key should not be replaced
+        $this->assertEquals('some-key', $span->getDescription());
+    }
+
     private function markSkippedIfTracingEventsNotAvailable(): void
     {
         if (class_exists(RetrievingKey::class)) {