Laravel Exception Handler `dontFlash` property not honored

[bilfeldt]

Environment

How do you use Sentry? Laravel package

Which SDK and version? sentry/sentry-laravel:2.11.1

Steps to Reproduce

1. Add Sentry as an Laravel error reporting service in app/Exceptions/Handler.php@report

public function report(Throwable $exception)
{
    if (app()->bound('sentry') && $this->shouldReport($exception)) { // The Sentry id has been added to the layouts in `/resources/views/errors/`
        app('sentry')->captureException($exception);
    }

    parent::report($exception);
}

3. Add some parameter like test to the property $dontFlash of the file app/Exceptions/Handler.php

/**
 * A list of the inputs that are never flashed for validation exceptions.
 *
 * @var array
 */
protected $dontFlash = [
    'current_password',
    'password',
    'password_confirmation',
    'test', // <!------------------------- This is added
];

5. Add a dump in vendor/sentry/sentry/src/Transport/HttpTransport.php@L107 like so:

    /**
     * {@inheritdoc}
     */
    public function send(Event $event): PromiseInterface
    {
        $dsn = $this->options->getDsn();

        if (null === $dsn) {
            throw new \RuntimeException(sprintf('The DSN option must be set to use the "%s" transport.', self::class));
        }

        if (EventType::transaction() === $event->getType()) {
            $request = $this->requestFactory->createRequest('POST', $dsn->getEnvelopeApiEndpointUrl())
                ->withHeader('Content-Type', 'application/x-sentry-envelope')
                ->withBody($this->streamFactory->createStream($this->payloadSerializer->serialize($event)));
        } else {
            $request = $this->requestFactory->createRequest('POST', $dsn->getStoreApiEndpointUrl())
                ->withHeader('Content-Type', 'application/json')
                ->withBody($this->streamFactory->createStream($this->payloadSerializer->serialize($event)));
        }
dd($this->payloadSerializer->serialize($event)); // <! --------------------- This is added
        try {
            /** @var ResponseInterface $response */
            $response = $this->httpClient->sendAsyncRequest($request)->wait();
        } catch (\Throwable $exception) {
            $this->logger->error(
                sprintf('Failed to send the event to Sentry. Reason: "%s".', $exception->getMessage()),
                ['exception' => $exception, 'event' => $event]
            );

            return new RejectedPromise(new Response(ResponseStatus::failed(), $event));
        }
    ....

5. Now trigger an exception by going to a route like /dev/runtime-exception?test=foo-bar which simply throws a Runtime Exception:

Route::get('/dev/runtime-exception', fn () => throw new \RuntimeException('Something failed'));

Expected Result

The payload being transported to Sentry should not have the test parameter value "foo-bar" present in the payload being transported to Sentry but it does.

Actual Result

This is the payload being sent:

{
    "event_id": "fbd7e434598443ed9c762914587aaadb",
    "timestamp": 1646951964.943884,
    "platform": "php",
    "sdk": {
        "name": "sentry.php.laravel",
        "version": "2.11.1"
    },
    "logger": "php",
    "server_name": "Anderss-MacBook-Pro.local",
    "release": "868c7813",
    "environment": "local",
    "user": {
        "id": null,
        "username": null,
        "email": null,
        "ip_address": "127.0.0.1"
    },
    "contexts": {
        "os": {
            "name": "Darwin",
            "version": "20.6.0",
            "build": "Darwin Kernel Version 20.6.0: Wed Jun 23 00:26:31 PDT 2021; root:xnu-7195.141.2~5\/RELEASE_X86_64",
            "kernel_version": "DarwinAnderss-MacBook-Pro.local 20.6.0 Darwin Kernel Version 20.6.0: Wed Jun 23 00:26:31 PDT 2021; root:xnu-7195.141.2~5\/RELEASE_X86_64 x86_64"
        },
        "runtime": {
            "name": "php",
            "version": "8.0.16"
        }
    },
    "request": {
        "url": "https:\/\/example.test\/dev\/runtime-exception?test=ABC",
        "method": "GET",
        "query_string": "test=ABC",
        "env": {
            "REMOTE_ADDR": "127.0.0.1"
        },
....

Note that the parameter is present in the request.url and request.query_string. I would have expected those to be replaced by asterisks or something like that:

....
    "request": {
        "url": "https:\/\/example.test\/dev\/runtime-exception?test=***",
        "method": "GET",
        "query_string": "test=***",
        "env": {
            "REMOTE_ADDR": "127.0.0.1"
        },
...

[X-Coder264]

You've pasted yourself what is the purpose of the dontFlash property:

/**
 * A list of the inputs that are never flashed for validation exceptions.
 *
 * @var array
 */
protected $dontFlash

The only usage of it is in \Illuminate\Foundation\Exceptions\Handler::invalid :

    protected function invalid($request, ValidationException $exception)
    {
        return redirect($exception->redirectTo ?? url()->previous())
                    ->withInput(Arr::except($request->input(), $this->dontFlash))
                    ->withErrors($exception->errors(), $request->input('_error_bag', $exception->errorBag));
    }

And it does exactly what it says it would do.

What does session flashing for validation exceptions have to do with Sentry logging?

Saying that the dontFlash property is not honored does not make sense as Sentry logging and validation session flashing are two completely unrelated functionalities so IMO this issue should be closed.

[stayallive]

I agree that $dontFlash should not affect how Sentry handles and reports exceptions, it is a Laravel feature and doesn't translate to what Sentry does since "flashing" is a whole other operation than loggin data about the request.

You can read more about how to scrub data from Sentry reports here: https://docs.sentry.io/platforms/php/guides/laravel/data-management/sensitive-data/.

[bilfeldt]

I agree that $dontFlash should not affect how Sentry handles and reports exceptions, it is a Laravel feature and doesn't translate to what Sentry does since "flashing" is a whole other operation than loggin data about the request.

it seems like I misinterpreted the configuration option $dontFlash since indeed this config is solely regarding the flashing of validation exception data.

You can read more about how to scrub data from Sentry reports here: https://docs.sentry.io/platforms/php/guides/laravel/data-management/sensitive-data/.

It seems that the before_send hook is indeed capable of filtering out sensitive data from the request.

This is clearly not a but after all @stayallive, sorry about the misunderstanding. That being said would it not be beneficial for this dedicated Laravel Sentry package to provide a super easy and intuitive configuration option for filtering out sensitive data from the requests given that this is in fact already the recommended way of dealing with sensitive data according to the Sentry specs? That way every user would not have to write own filtering logic, but instead a simply array of fields could be provided:

Using before_send in the SDKs to scrub any data before it is sent is the recommended scrubbing approach, so sensitive data never leaves the local environment.