bugsnag / bugsnag-php

BugSnag error monitoring and crash reporting tool for PHP apps
https://docs.bugsnag.com/platforms/php
MIT License
554 stars 77 forks source link

Redact command-line options #659

Closed danepowell closed 1 year ago

danepowell commented 1 year ago

Description

I maintain a Symfony Console application. Some commands accept sensitive options such as --password=12345. We need to be able to filter these options out of BugSnag reports. I've seen the redactedKeys option but this mainly seems oriented to web requests, and I'm pretty sure it can't work on the console. Anyway, it can't filter out just one option value from the console command.

Describe the solution you'd like

A way to filter / redact console options.

Describe alternatives you've considered

Removing any sensitive console options from our application entirely, and scolding users for passing sensitive info via the command line. Sure, in an ideal world that would be true, but there's cases where it makes sense.

Additional context

matthewjhowells commented 1 year ago

Hi Dane,

Thanks for reaching out and apologies for the delayed response.

If sensitive information is being included in events reported to BugSnag, it should be possible to modify the event object to remove this data using a callback. For instance, if a password passed in a console command to your app is being captured in the stack trace of your events, you might use a callback to parse through the stack trace of each event, identifying frames which contain sensitive information and modifying the frame to remove it.

So we can best suggest how to redact this information, I would be interested to know where you are seeing these sensitive options in your BugSnag events? If you would like to share some example events with us, feel free to open a support ticket with BugSnag support providing a link to this GitHub issue.

danepowell commented 1 year ago

Thanks, I sent a screenshot to help clarify.

I saw the documentation on customizing the error report but I didn't see how it helps here. The problem is that the sensitive parameters are in the "location" field (at least I think that's what you'd call it for a web app). They are not in the stack trace or metadata. And I can't see how to modify the reported location.

matthewjhowells commented 1 year ago

Hi Dane,

Thanks for sending that screenshot to us. From the screenshot I can see that your --password values are being included in the context of your events. As mentioned in my last message, it should be possible to remove this using a callback, however, instead of modifying the stack trace, you can modify the events context. To do this, you will need to use the GetContext and SetContext methods. For instance:

public function boot()
{
    parent::boot();
    $this->container->get('bugsnag')->registerCallback(function ($report) {
        $context = $report->getContext();
        if (strpos($context, "--password") !== false) {
            $report->setContext(substr($context, 0, strpos($context, "--password")));
        }
    });
}

It’s worth noting that in Symfony, callbacks should be registered within the boot function of your src/Kernel.php file, as shown in our docs here.

Please let me know if you have any problems implementing this in your project.

danepowell commented 1 year ago

Awesome, thanks! That snippet worked on the first try; very impressive! :) Here's my implementation for anyone who's curious: https://github.com/acquia/cli/pull/1465

I guess I missed in the docs that Context held this information. In hindsight I should have caught this:

This is typically set to a filename or request path automatically, depending on the framework and application type.

You mentioned callbacks should be registered on boot. I assume that's to ensure that the callbacks run even on early errors during boot? Our app is a little weird since we don't use the Framework Bundle for performance reasons, and this makes it difficult to access the container in the way you might expect. But we'll take this into consideration.