Skip to content
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.

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

Ability to filter request input from being sent to Sentry #1724

Open
dwightwatson opened this issue Mar 28, 2024 · 4 comments
Open

Ability to filter request input from being sent to Sentry #1724

dwightwatson opened this issue Mar 28, 2024 · 4 comments
Assignees

Comments

@dwightwatson
Copy link

Problem Statement

I would like an easy way to configure this integration from sending certain request input to Sentry. Right now the documentation suggests we use the before_send configuration option, but this has two issues.

  1. It's a painful amount of code for something very simple. Plus, I couldn't find any ready to go documentation on how to implement it.
'before_send' => function (Event $event, ?EventHint $hint): ?Event {
    $request = $event->getRequest();

    if ($data = Arr::get($request, 'data')) {
        foreach (['password', 'password_confirmation'] as $field) {
            if (Arr::has($data, $field)) {
                $data[$field] = '[FILTERED]';
            }
        }

        $request['data'] = $data;

        $event->setRequest($request);
    }

    return $event;
},
  1. It isn't supported when using config:cache. The docs suggest that I can use the callable array syntax but that doesn't seem to work for `before_send.
Symfony\Component\OptionsResolver\Exception\InvalidOptionsException 

The option "before_send" with value array is expected to be of type "callable", but is of type "array".

Solution Brainstorm

Simplest solution would be config option that accepts an array of field names that would be removed or filtered before being sent to Sentry.

'filter_fields' => ['password', 'password_confirmation'],

I note that Flare has more advanced options, including middleware that makes it easy to filter input and headers.

CensorRequestBodyFields::class => [
    'censor_fields' => [
        'password',
        'password_confirmation',
    ],
],
CensorRequestHeaders::class => [
    'headers' => [
        'API-KEY',
        'Authorization',
        'Cookie',
        'Set-Cookie',
        'X-CSRF-TOKEN',
        'X-XSRF-TOKEN',
    ]
]
@stayallive
Copy link
Collaborator

stayallive commented Mar 30, 2024

Hey @dwightwatson, you mention 1 issue here and a feature request.

The docs suggest that I can use the callable array syntax but that doesn't seem to work for `before_send.

Are you sure you followed the example to the letter? This works in all my testing so far, maybe you can share what you tried? A common thing is that it's overlooked that it needs to be a static function.

Simplest solution would be config option that accepts an array of field names that would be removed or filtered before being sent to Sentry.

I agree that that would be more elegant, happy to explore that path a bit further to see what we can cook up to make this a little simpler in the future!

@dwightwatson
Copy link
Author

Hey @stayallive thanks for that.

You're right - I missed that it was a static method required. I had a regular public method and after fixing that it now works fine. That's my bad.

Would love to see this simplified further to remove the additional boilerplate. Plus, it seems like it would be beneficial for password and password_confirmation to be filtered by default on a fresh Laravel app.

@stayallive
Copy link
Collaborator

stayallive commented Apr 1, 2024

I'm at least glad that the static fixes the before_send! One solved.

I'm going to see if we can figure something out for not sending data to Sentry. I'm guessing you do know that these fields are filtered away server side (or on Relay) but that you don't even want to sent it?

I know we had some filtering option in past SDK versions so unsure if we would want to bring a version of that back.

@dwightwatson
Copy link
Author

Yes - I have already added these as filtered on Sentry's side. In one app I'm dealing with some sensitive personal information and it seems better to me that this doesn't leave my server at all.

@cleptric cleptric self-assigned this Apr 2, 2024
@cleptric cleptric transferred this issue from getsentry/sentry-laravel Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants