From 3779241e1d81cfc338b2cc1a7eaa8f601f746615 Mon Sep 17 00:00:00 2001 From: Gustavo Freze Date: Fri, 25 Sep 2026 20:12:28 -0300 Subject: [PATCH] fix: Keep the request and the user out of Sentry events. The SDK default read the request body, so a server error on a login route sent the email and the password to the project. --- README.md | 7 ++- src/Internal/Reporting/SentryPrivacy.php | 27 +++++++++++ src/Reporters/SentryReporter.php | 13 ++++-- tests/Unit/Reporters/SentryReporterTest.php | 51 +++++++++++++++++++++ 4 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 src/Internal/Reporting/SentryPrivacy.php diff --git a/README.md b/README.md index 4b3c07a..1e84758 100644 --- a/README.md +++ b/README.md @@ -491,8 +491,13 @@ $middleware = ErrorMiddleware::create() An empty release becomes no release, because a working tree is not a version. An empty DSN leaves the SDK disabled, which is what an environment with reporting switched off wants, so the same wiring serves every environment. +Nothing about the request reaches the project. The SDK is told never to read a body, and the request and the user it +gathers on its own are dropped before sending, so a password in a login body, an authorization header or a query string +never leaves the process. The method and the path an event needs travel in the `http` context described below. + The second takes a hub the application already holds, for an application that boots the SDK itself because it needs a -sample rate, a transport, or an integration this reporter does not configure. It touches no global state. +sample rate, a transport, or an integration this reporter does not configure. It touches no global state, and what +that hub sends, the request and the user included, is the application's to decide. ```php $middleware = ErrorMiddleware::create() diff --git a/src/Internal/Reporting/SentryPrivacy.php b/src/Internal/Reporting/SentryPrivacy.php new file mode 100644 index 0000000..ae73daf --- /dev/null +++ b/src/Internal/Reporting/SentryPrivacy.php @@ -0,0 +1,27 @@ +before_send hook of a reporter booted by this library. + * + *

The SDK gathers the request on its own, from the globals of the process: the URL with its query + * string, the headers, and the body a login or a sign up carries. It also carries whatever user the + * application set. None of that is an error, and all of it can be personal data, so both leave the + * event before it is sent. The method and the path an event needs travel in the http + * context, which {@see SentryScope} writes.

+ */ +final readonly class SentryPrivacy +{ + public static function strip(Event $event): Event + { + $event->setUser(user: null); + $event->setRequest(request: []); + + return $event; + } +} diff --git a/src/Reporters/SentryReporter.php b/src/Reporters/SentryReporter.php index 8859b99..4d18475 100644 --- a/src/Reporters/SentryReporter.php +++ b/src/Reporters/SentryReporter.php @@ -10,6 +10,7 @@ use Sentry\State\HubInterface; use Sentry\State\Scope; use TinyBlocks\Http\ErrorHandler\ErrorReporter; +use TinyBlocks\Http\ErrorHandler\Internal\Reporting\SentryPrivacy; use TinyBlocks\Http\ErrorHandler\Internal\Reporting\SentryScope; use TinyBlocks\Http\ErrorHandler\ReportedError; use TinyBlocks\Http\ErrorHandler\ReportingFilter; @@ -55,6 +56,10 @@ public static function from( *

An empty release becomes no release, because a working tree is not a version. An empty DSN * leaves the SDK disabled, which is what an environment with reporting switched off wants.

* + *

The request never reaches the project. The SDK reads no body, and the request and the user + * it gathers on its own are dropped before sending, so a password in a login body, an + * authorization header or a query string never leaves the process.

+ * * @param string $dsn The DSN of the Sentry project, empty to leave the SDK disabled. * @param string $release The version the deploy reports, as package@version. * @param string $environment The environment the deploy runs in. @@ -68,9 +73,11 @@ public static function initializedWith( ReportingFilter $filter = ReportingFilter::SERVER_ERRORS ): SentryReporter { $client = ClientBuilder::create(options: [ - 'dsn' => $dsn, - 'release' => $release === '' ? null : $release, - 'environment' => $environment + 'dsn' => $dsn, + 'release' => $release === '' ? null : $release, + 'environment' => $environment, + 'before_send' => SentryPrivacy::strip(...), + 'max_request_body_size' => 'never' ])->getClient(); return new SentryReporter(hub: SentrySdk::setCurrentHub(hub: new Hub(client: $client)), filter: $filter); diff --git a/tests/Unit/Reporters/SentryReporterTest.php b/tests/Unit/Reporters/SentryReporterTest.php index 98dc109..15e14ca 100644 --- a/tests/Unit/Reporters/SentryReporterTest.php +++ b/tests/Unit/Reporters/SentryReporterTest.php @@ -11,8 +11,10 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use RuntimeException; +use Sentry\Event; use Sentry\SentrySdk; use Sentry\Severity; +use Sentry\UserDataBag; use Test\TinyBlocks\Http\ErrorHandler\Unit\ContextualException; use Test\TinyBlocks\Http\ErrorHandler\Unit\PrioritizedException; use Test\TinyBlocks\Http\ErrorHandler\Unit\RecordingSentry; @@ -429,6 +431,55 @@ public function testInitializedWithThenTheCurrentHubCarriesTheDeploymentValues() self::assertSame('1', $options->getDsn()?->getProjectId()); } + public function testInitializedWithThenTheSdkNeverReadsTheRequestBody(): void + { + /** @Given the deployment values an application resolves from its environment */ + $dsn = 'https://examplePublicKey@o0.ingest.sentry.io/1'; + + /** @When a reporter is built from them */ + SentryReporter::initializedWith(dsn: $dsn, release: 'client-gateway@1.0.0', environment: 'development'); + + /** @Then the SDK is told never to read a body, so a password in one is never gathered */ + $options = SentrySdk::getCurrentHub()->getClient()?->getOptions(); + self::assertNotNull($options); + self::assertSame('never', $options->getMaxRequestBodySize()); + self::assertFalse($options->shouldSendDefaultPii()); + } + + public function testInitializedWithWhenTheSdkGathersTheRequestAndTheUserThenNeitherIsSent(): void + { + /** @Given a reporter built from the deployment values */ + SentryReporter::initializedWith( + dsn: 'https://examplePublicKey@o0.ingest.sentry.io/1', + release: 'client-gateway@1.0.0', + environment: 'development' + ); + + /** @And an event carrying what the SDK gathers on its own while a login fails */ + $user = UserDataBag::createFromUserIdentifier(id: '0199a1b2-7c3d-7e4f-8a5b-6c7d8e9f0a1b') + ->setEmail(email: 'ana@example.com'); + + $event = Event::createEvent() + ->setUser(user: $user) + ->setRequest(request: [ + 'url' => 'https://api.example.com/v1/sessions?phone=5511999999999', + 'data' => ['email' => 'ana@example.com', 'password' => 'correct horse'], + 'method' => 'POST', + 'headers' => ['Authorization' => ['Bearer token']], + 'query_string' => 'phone=5511999999999' + ]); + + /** @When the SDK runs the hook it calls before sending */ + $options = SentrySdk::getCurrentHub()->getClient()?->getOptions(); + self::assertNotNull($options); + $sent = ($options->getBeforeSendCallback())($event, null); + + /** @Then the event leaves without the request and without the user */ + self::assertNotNull($sent); + self::assertNull($sent->getUser()); + self::assertSame([], $sent->getRequest()); + } + public function testReportWhenTheConsumerReplacesTheRuleThenItsPriorityDecides(): void { /** @Given a request */