Mask session keys in cache events to protect sensitive information

cursoragent · cursoragent · commit 95ddd2d3ca95 · 2025-06-17T09:01:55.000Z
diff --git a/src/Sentry/Laravel/Features/CacheIntegration.php b/src/Sentry/Laravel/Features/CacheIntegration.php
@@ -86,11 +86,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 +111,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 +133,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 +185,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 +197,12 @@ public function handleRedisCommands(RedisEvents\CommandExecuted $event): void
         ];
 
         if ($this->shouldSendDefaultPii()) {
-            $data['db.redis.parameters'] = $event->parameters;
+            // Replace session keys in parameters if present
+            $parameters = $event->parameters;
+            if (!empty($parameters[0]) && is_string($parameters[0])) {
+                $parameters[0] = $this->replaceSessionKey($parameters[0]);
+            }
+            $data['db.redis.parameters'] = $parameters;
         }
 
         if ($this->isTracingFeatureEnabled('redis_origin')) {
@@ -245,6 +258,73 @@ private function maybeHandleCacheEventAsEndOfSpan(Events\CacheEvent $event): boo
         return false;
     }
 
+    /**
+     * Check if a cache key is the current session key.
+     *
+     * @param string $key
+     *
+     * @return bool
+     */
+    private function isSessionKey(string $key): bool
+    {
+        // Check if the container has a bound request and session
+        if (!$this->container()->bound('request')) {
+            return false;
+        }
+
+        try {
+            $request = $this->container()->make('request');
+
+            // Check if the request has a session
+            if (!$request->hasSession()) {
+                return false;
+            }
+
+            $session = $request->session();
+
+            // Don't start the session if it hasn't been started yet
+            if (!$session->isStarted()) {
+                return false;
+            }
+
+            // Get the session ID and check if the cache key matches
+            $sessionId = $session->getId();
+
+            // Check if the key equals the session ID or contains it
+            // This handles cases where the cache key might be prefixed
+            return $key === $sessionId || Str::endsWith($key, $sessionId);
+        } catch (\Exception $e) {
+            // If anything goes wrong, we assume it's not a session key
+            return false;
+        }
+    }
+
+    /**
+     * Replace a session key with a placeholder.
+     *
+     * @param string $key
+     *
+     * @return string
+     */
+    private function replaceSessionKey(string $key): string
+    {
+        return $this->isSessionKey($key) ? '{sessionKey}' : $key;
+    }
+
+    /**
+     * Replace session keys in an array of keys with placeholders.
+     *
+     * @param array $keys
+     *
+     * @return array
+     */
+    private function replaceSessionKeys(array $keys): array
+    {
+        return array_map(function ($key) {
+            return $this->replaceSessionKey($key);
+        }, $keys);
+    }
+
     /**
      * Normalize the array of keys to a array of only strings.
      *
diff --git a/test/Sentry/Features/CacheIntegrationTest.php b/test/Sentry/Features/CacheIntegrationTest.php
@@ -51,6 +51,33 @@ public function testCacheBreadcrumbIsNotRecordedWhenDisabled(): void
         $this->assertEmpty($this->getCurrentSentryBreadcrumbs());
     }
 
+    public function testCacheBreadcrumbReplacesSessionKeyWithPlaceholder(): void
+    {
+        // Start a session
+        $this->app['request']->setLaravelSession($session = $this->app['session.store']);
+        $session->start();
+        $sessionId = $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 +174,60 @@ public function testCacheRemoveSpanIsRecorded(): void
         $this->assertEquals(['foo'], $span->getData()['cache.key']);
     }
 
+    public function testCacheSpanReplacesSessionKeyWithPlaceholder(): void
+    {
+        $this->markSkippedIfTracingEventsNotAvailable();
+
+        // Start a session
+        $this->app['request']->setLaravelSession($session = $this->app['session.store']);
+        $session->start();
+        $sessionId = $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
+        $this->app['request']->setLaravelSession($session = $this->app['session.store']);
+        $session->start();
+        $sessionId = $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, {sessionKey}', $span->getDescription());
+        $this->assertEquals(['{sessionKey}', 'regular-key', '{sessionKey}'], $span->getData()['cache.key']);
+    }
+
+    public function testCacheOperationDoesNotStartSessionPrematurely(): void
+    {
+        $this->markSkippedIfTracingEventsNotAvailable();
+
+        // Make sure session is not started
+        $this->assertFalse($this->app['session.store']->isStarted());
+
+        $span = $this->executeAndReturnMostRecentSpan(function () {
+            Cache::get('some-key');
+        });
+
+        // Session should still not be started
+        $this->assertFalse($this->app['session.store']->isStarted());
+        
+        // And the key should not be replaced
+        $this->assertEquals('some-key', $span->getDescription());
+    }
+
     private function markSkippedIfTracingEventsNotAvailable(): void
     {
         if (class_exists(RetrievingKey::class)) {
