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

Forms api - Mask all potential PII from error message #12471

Merged
merged 0 commits into from
Apr 25, 2023

Conversation

gmrabian
Copy link
Contributor

@gmrabian gmrabian commented Apr 24, 2023

Following up on a recent incident where a JSON::ParserError displayed some PII when parsing an invalid payload - this surrounds the entire forms_api functionality, catches any errors, and raises them with every word entered by the user scrubbed from the message.

Summary

  • Add rescue block to controller method
  • Within rescue block, parse all values from the input and remove them from the error message

Related issue(s)

Testing done

  • Tested locally by manually throwing errors with various tests and payloads
  • Successfully removed all user info and potential PII from error message.

Screenshots

Test payload example

BEFORE:
Screenshot 2023-04-24 at 3 31 27 PM

AFTER:
Screenshot 2023-04-24 at 3 32 25 PM

In the scrubbed example we still get the failed JSON blob's structure, and we can see some newlines, which were the cause of our issue. This is related to the issue we fixed that caused the PII dump.

What areas of the site does it impact?

All forms using the forms_api (currently only 26-4555)

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

Requested Feedback

Is there a better way you would suggest catching/raising this error? I noticed the Common::Exceptions::etc. error patterns but did not have success implementing them, and this is meant to catch any error type.

Copy link
Contributor

@ryan-mcneil ryan-mcneil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@ryan-mcneil ryan-mcneil self-assigned this Apr 25, 2023
@gmrabian gmrabian merged commit f640fb3 into master Apr 25, 2023
@gmrabian gmrabian deleted the forms_api/pii-mask branch April 25, 2023 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants