Problem/Motivation

Some events reported by Rollbar include potentially sensitive information (e.g. access tokenes).

Rollbar provides a "scrub_fields" configuration option to allow these data fields to be scrubbed: See https://docs.rollbar.com/docs/php-configuration-reference

scrub_fields: An array of field names to scrub out of the entire payload excluding the top level access_token. Values will be replaced with asterisks. If overriding, make sure to list all fields you want to scrub, not just fields you want to add to the default. Param names are converted to lowercase before comparing against the scrub list.

Currently, the Rollbar module does not support configuration of scrub_fields.

It would be useful it were possible to configure scrub_fields from the Rollbar settings form.

Proposed resolution

Add a "scrub_fields" configuration field to `RollbarSettingsForm`.

CommentFileSizeAuthor
#7 allow-configuration-scrub-fields-3409446-7.patch1.74 KBLeo Pitt
#2 allow-configuration-scrub-fields-3409446-2.patch2.5 KBLeo Pitt
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Leo Pitt created an issue. See original summary.

Leo Pitt’s picture

Leo Pitt CreditAttribution: Leo Pitt commented
FileSize
allow-configuration-scrub-fields-3409446-2.patch2.5 KB

Patch attached:

  • Add a scrub_fields field to module settings form.
  • Pass scrub_fields to Rollbar init() method.
  • Minor refactoring init() method for efficiency.
Leo Pitt’s picture

Leo Pitt CreditAttribution: Leo Pitt commented
Status: Active » Needs review
Leo Pitt’s picture

Leo Pitt CreditAttribution: Leo Pitt at SystemSeed commented
intrafusion’s picture

intrafusion
Primary language English
Location Edinburgh, UK
CreditAttribution: intrafusion commented
Status: Needs review » Needs work

This is unnecessarily complicated, the following changes can be made:

$form['scrub_fields'] = [
  '#type' => 'textarea',
  '#title' => $this->t('Scrub fields'),
  '#default_value' => implode("\n", $config->get('scrub_fields')),
  '#description' => $this->t('Field names to scrub out of the entire payload. Enter one field name per line. Values will be replaced with asterisks.'),
];

->set('scrub_fields', preg_split("(\r\n?|\n)", $form_state->getValue('scrub_fields')))

// Populate basic configuration.
$configuration = [
  'access_token' => $token,
  'environment' => $environment,
  'scrub_fields' => $this->config->get('scrub_fields') ?? [],
];

Also a merge request is much easier for a maintainer to review and accept

Leo Pitt’s picture

Leo Pitt CreditAttribution: Leo Pitt at SystemSeed commented

Thanks, new patch incoming.

Leo Pitt’s picture

Leo Pitt CreditAttribution: Leo Pitt at SystemSeed commented
FileSize
allow-configuration-scrub-fields-3409446-7.patch1.74 KB

Updated patch attached.

Leo Pitt’s picture

Leo Pitt CreditAttribution: Leo Pitt at SystemSeed commented
Status: Needs work » Needs review

  • Leo Pitt authored 26aa5db1 on 2.1.x 
    Issue #3409446 by Leo Pitt: Allow configuration of "scrub_fields"
intrafusion’s picture

intrafusion
Primary language English
Location Edinburgh, UK
CreditAttribution: intrafusion commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.