Skip to content

feat(pii): add sensitive data scrubber #2170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Litarnus wants to merge 1 commit into
base: data-collection-config
Choose a base branch
from
+356 −0
Open

feat(pii): add sensitive data scrubber #2170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 164 additions & 0 deletions src/DataCollection/SensitiveDataScrubber.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
<?php

declare(strict_types=1);

namespace Sentry\DataCollection;

/**
* @internal
*
* @phpstan-type KeyValueCollectionBehavior array{mode: 'off'|'denyList'|'allowList', terms: string[]}
*/
final class SensitiveDataScrubber
{
private const SENSITIVE_DATA_DENYLIST = [
'auth',
'token',
'secret',
'password',
'passwd',
'pwd',
'key',
'jwt',
'bearer',
'sso',
'saml',
'csrf',
'xsrf',
'credentials',
'session',
'sid',
'identity',
];

/**
* cookie headers that we always want to redact.
*/
private const SENSITIVE_HEADERS = [
'cookie',
'set-cookie',
];

/**
* @var string|null
*/
private static $sensitiveDataDenyListRegex;

/**
* This class contains only static methods and should not be instantiated.
*/
private function __construct()
{
}

/**
* @param array<array-key, string[]> $headers
*
* @phpstan-param KeyValueCollectionBehavior $behavior
*
* @return array<string, string[]>
*/
public static function scrubHeaders(array $headers, array $behavior): array
{
$scrubbed = [];

foreach ($headers as $name => $values) {
$name = (string) $name;

if (\in_array(strtolower($name), self::SENSITIVE_HEADERS, true) || self::shouldScrubValue($name, $behavior)) {
foreach ($values as $headerLine => $headerValue) {
$values[$headerLine] = '[Filtered]';
}
}

$scrubbed[$name] = $values;
}

return $scrubbed;
}

/**
* @param array<array-key, mixed> $data
*
* @phpstan-param KeyValueCollectionBehavior $behavior
*
* @return array<string, mixed>
*/
public static function scrubKeyValueData(array $data, array $behavior): array
{
$scrubbed = [];

/** @mago-ignore analysis:mixed-assignment */
foreach ($data as $key => $value) {
$key = (string) $key;
$scrubbed[$key] = self::shouldScrubValue($key, $behavior) ? '[Filtered]' : $value;
}

return $scrubbed;
}

/**
* @phpstan-param KeyValueCollectionBehavior $behavior
*/
public static function scrubQueryString(string $queryString, array $behavior): string
{
$parts = explode('&', $queryString);

foreach ($parts as $index => $part) {
$separatorPosition = strpos($part, '=');
$encodedKey = $separatorPosition === false ? $part : substr($part, 0, $separatorPosition);
$key = urldecode($encodedKey);

if (self::shouldScrubValue($key, $behavior)) {
$parts[$index] = $encodedKey . '=[Filtered]';
}
}

return implode('&', $parts);
}

/**
* @phpstan-param KeyValueCollectionBehavior $behavior
*/
private static function shouldScrubValue(string $key, array $behavior): bool
{
if (self::matchesMandatoryDenyList($key)) {
return true;
}

if ($behavior['mode'] === 'allowList') {
return !self::matchesAnyTerm($key, $behavior['terms'], false);
}

return $behavior['terms'] !== [] && self::matchesAnyTerm($key, $behavior['terms'], true);
}
Comment on lines +124 to +134

@sentry sentry Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The shouldScrubValue function doesn't handle mode = 'off'. It falls through to denyList logic, incorrectly scrubbing custom terms when the mode is "off".
Severity: MEDIUM

Suggested Fix

Add an explicit check for mode === 'off' at the beginning of the shouldScrubValue function. If the mode is 'off', the function should return false after checking the mandatory deny list, ensuring no custom terms are processed.

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: src/DataCollection/SensitiveDataScrubber.php#L123-L134

Potential issue: The `shouldScrubValue` function lacks explicit handling for when the
scrubbing `mode` is set to `'off'`. As a result, the logic falls through to the final
return statement on line 133, which applies `denyList` logic. If a caller sets `mode` to
`'off'` but also provides a non-empty `terms` array, those terms will be treated as a
deny list, causing data to be scrubbed. This contradicts the expected behavior of an
`'off'` mode, which should disable custom scrubbing. The mandatory deny list is still
applied, which may be intentional, but the application of custom terms is a bug. This is
not covered by any tests.

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

@cursor cursor Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unhandled off collection mode

Medium Severity

shouldScrubValue accepts KeyValueCollectionBehavior, which includes mode off, but never handles it. After the mandatory denylist check, off falls through to deny-list logic, so non-mandatory keys stay unredacted even when collection for that category is disabled.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3686aef. Configure here.


private static function matchesMandatoryDenyList(string $key): bool
{
if (self::$sensitiveDataDenyListRegex === null) {
self::$sensitiveDataDenyListRegex = '/' . implode('|', array_map(static function (string $term): string {
return preg_quote($term, '/');
}, self::SENSITIVE_DATA_DENYLIST)) . '/i';
}

return preg_match(self::$sensitiveDataDenyListRegex, $key) === 1;
}

/**
* @param string[] $terms
*/
private static function matchesAnyTerm(string $key, array $terms, bool $partial): bool
{
$key = strtolower($key);

foreach ($terms as $term) {
$term = strtolower($term);

if (($partial && strpos($key, $term) !== false) || (!$partial && $key === $term)) {
return true;
}
}

return false;
}
}
192 changes: 192 additions & 0 deletions tests/DataCollection/SensitiveDataScrubberTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
<?php

declare(strict_types=1);

namespace Sentry\Tests\DataCollection;

use PHPUnit\Framework\TestCase;
use Sentry\DataCollection\SensitiveDataScrubber;

final class SensitiveDataScrubberTest extends TestCase
{
public function testAlwaysScrubMandatoryValues(): void
{
$behavior = ['mode' => 'denyList', 'terms' => []];

$scrubbed = SensitiveDataScrubber::scrubKeyValueData([
'authorization' => 'secret',
'public' => 'visible',
], $behavior);

$this->assertSame([
'authorization' => '[Filtered]',
'public' => 'visible',
], $scrubbed);
}

public function testScrubCustomAndMandatory(): void
{
$behavior = ['mode' => 'denyList', 'terms' => ['custom']];

$scrubbed = SensitiveDataScrubber::scrubKeyValueData([
'authorization' => 'secret',
'custom-field' => 'private',
'public' => 'visible',
], $behavior);

$this->assertSame([
'authorization' => '[Filtered]',
'custom-field' => '[Filtered]',
'public' => 'visible',
], $scrubbed);
}

public function testScrubCaseInsensitiveKeys(): void
{
$behavior = ['mode' => 'denyList', 'terms' => []];

$scrubbed = SensitiveDataScrubber::scrubKeyValueData(['AUTHORIZATION' => 'secret'], $behavior);

$this->assertSame(['AUTHORIZATION' => '[Filtered]'], $scrubbed);
}

public function testAllowList(): void
{
$behavior = ['mode' => 'allowList', 'terms' => ['theme']];

$scrubbed = SensitiveDataScrubber::scrubKeyValueData([
'theme' => 'dark',
'tracking_id' => '12345',
], $behavior);

$this->assertSame([
'theme' => 'dark',
'tracking_id' => '[Filtered]',
], $scrubbed);
}

public function testScrubHeadersAppliesDenyList(): void
{
$behavior = ['mode' => 'denyList', 'terms' => []];

$scrubbed = SensitiveDataScrubber::scrubHeaders([
'Authorization' => ['secret'],
'X-Request-Id' => ['request-id'],
], $behavior);

$this->assertSame([
'Authorization' => ['[Filtered]'],
'X-Request-Id' => ['request-id'],
], $scrubbed);
}

public function testScrubHeadersScrubsEveryLineOfMatchingHeaders(): void
{
$behavior = ['mode' => 'denyList', 'terms' => []];

$scrubbed = SensitiveDataScrubber::scrubHeaders(['X-Api-Key' => ['first', 'second']], $behavior);

$this->assertSame(['X-Api-Key' => ['[Filtered]', '[Filtered]']], $scrubbed);
}

public function testScrubHeadersAlwaysScrubsCookieHeaders(): void
{
$behavior = ['mode' => 'denyList', 'terms' => []];

$scrubbed = SensitiveDataScrubber::scrubHeaders([
'Cookie' => ['session_id=secret; theme=dark'],
'Set-Cookie' => ['session_id=secret'],
'X-Request-Id' => ['request-id'],
], $behavior);

$this->assertSame([
'Cookie' => ['[Filtered]'],
'Set-Cookie' => ['[Filtered]'],
'X-Request-Id' => ['request-id'],
], $scrubbed);
}

public function testScrubHeadersAllowListCannotOverrideCookieHeaders(): void
{
$behavior = ['mode' => 'allowList', 'terms' => ['cookie', 'set-cookie']];

$scrubbed = SensitiveDataScrubber::scrubHeaders([
'Cookie' => ['session_id=secret'],
'Set-Cookie' => ['session_id=secret'],
], $behavior);

$this->assertSame([
'Cookie' => ['[Filtered]'],
'Set-Cookie' => ['[Filtered]'],
], $scrubbed);
}

public function testExtendedDenyTerms(): void
{
$defaultBehavior = ['mode' => 'denyList', 'terms' => []];
$extendedBehavior = ['mode' => 'denyList', 'terms' => ['forwarded', '-ip', 'remote-', 'via', '-user']];
$headers = [
'X-Forwarded-For' => ['203.0.113.7'],
'X-Real-IP' => ['203.0.113.7'],
];

$this->assertSame($headers, SensitiveDataScrubber::scrubHeaders($headers, $defaultBehavior));
$this->assertSame([
'X-Forwarded-For' => ['[Filtered]'],
'X-Real-IP' => ['[Filtered]'],
], SensitiveDataScrubber::scrubHeaders($headers, $extendedBehavior));
}

public function testScrubHeadersAllowListCannotOverrideMandatoryDenyList(): void
{
$behavior = ['mode' => 'allowList', 'terms' => ['authorization', 'x-request-id']];

$scrubbed = SensitiveDataScrubber::scrubHeaders([
'Authorization' => ['secret'],
'X-Request-Id' => ['request-id'],
'Host' => ['example.com'],
], $behavior);

$this->assertSame([
'Authorization' => ['[Filtered]'],
'X-Request-Id' => ['request-id'],
'Host' => ['[Filtered]'],
], $scrubbed);
}

public function testScrubQueryStringAppliesMandatoryDenyList(): void
{
$behavior = ['mode' => 'denyList', 'terms' => []];

$scrubbed = SensitiveDataScrubber::scrubQueryString('token=secret&page=1', $behavior);

$this->assertSame('token=[Filtered]&page=1', $scrubbed);
}

public function testScrubQueryStringAppliesCustomDenyListTerms(): void
{
$behavior = ['mode' => 'denyList', 'terms' => ['page']];

$scrubbed = SensitiveDataScrubber::scrubQueryString('token=secret&page=1&flag', $behavior);

$this->assertSame('token=[Filtered]&page=[Filtered]&flag', $scrubbed);
}

public function testScrubQueryStringDecodesKeysBeforeMatching(): void
{
$behavior = ['mode' => 'denyList', 'terms' => []];

$scrubbed = SensitiveDataScrubber::scrubQueryString('api%5Ftoken=secret&page=1', $behavior);

$this->assertSame('api%5Ftoken=[Filtered]&page=1', $scrubbed);
}

public function testCookieNameIsAllowedInQueryParams(): void
{
$behaviour = ['mode' => 'denyList', 'terms' => []];

$scrubbed = SensitiveDataScrubber::scrubQueryString('cookie=foo&set-cookie=bar', $behaviour);

$this->assertSame('cookie=foo&set-cookie=bar', $scrubbed);
}
}