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

Handle incorrectly UTF-8 encoded query and cookie url #4339

Merged

Conversation

unoduetre
Copy link
Contributor

@unoduetre unoduetre commented Oct 25, 2024

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

Stop sentry alerting on the “invalid byte sequence in UTF-8 errors”.

As discussed with @hannako, we will use a a solution based on an external gem called rack-utf8_sanitizer instead of developing our own gem. This solution will then be used in all our apps by copy-pasting the configuration of this gem.

Why

When security scanning of our site takes place we are inundated with these errors in our slack channel. These errors are not serving any purpose since there is no issue to be solved. Therefore we should remove them to keep all our alerts actionable.

We often get exceptions in Sentry coming from a security checking bot that attempts some attacks by using strings with special characters. All of those "attacks" fail, but they produce exceptions which we would like to ignore, so they do not clutter Sentry. On the other hand we don’t want to ignore exceptions of the same class coming from other sources.

Trello card

@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 25, 2024 12:47 Inactive
@unoduetre unoduetre force-pushed the 2938-silently-handle-invalid-byte-sequence-in-utf-8-errors-l branch from 88b7b7d to 7e99f3b Compare October 25, 2024 12:50
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 25, 2024 12:50 Inactive
@unoduetre unoduetre force-pushed the 2938-silently-handle-invalid-byte-sequence-in-utf-8-errors-l branch from 7e99f3b to 64396a0 Compare October 25, 2024 13:12
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 25, 2024 13:12 Inactive
@unoduetre
Copy link
Contributor Author

unoduetre commented Oct 28, 2024

Closing this PR as an alternative approach will be used.

@unoduetre unoduetre closed this Oct 28, 2024
@unoduetre unoduetre reopened this Oct 28, 2024
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 28, 2024 13:07 Inactive
@unoduetre unoduetre force-pushed the 2938-silently-handle-invalid-byte-sequence-in-utf-8-errors-l branch from 64396a0 to 964003e Compare October 28, 2024 13:53
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 28, 2024 13:54 Inactive
@unoduetre unoduetre force-pushed the 2938-silently-handle-invalid-byte-sequence-in-utf-8-errors-l branch from 964003e to 517e30d Compare October 28, 2024 14:03
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 28, 2024 14:04 Inactive
@unoduetre unoduetre force-pushed the 2938-silently-handle-invalid-byte-sequence-in-utf-8-errors-l branch from 517e30d to 2884d19 Compare October 28, 2024 14:37
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 28, 2024 14:38 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 31, 2024 09:25 Inactive
Use a different exception when this situation is detected:
* Sanitiser::Strategy::SanitisingError
This error is not sent to Sentry.
See: alphagov/govuk_app_config#402
@unoduetre unoduetre force-pushed the 2938-silently-handle-invalid-byte-sequence-in-utf-8-errors-l branch from 2884d19 to 84bbc37 Compare October 31, 2024 10:46
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-4339 October 31, 2024 10:46 Inactive
@unoduetre unoduetre merged commit 3619ba2 into main Oct 31, 2024
12 checks passed
@unoduetre unoduetre deleted the 2938-silently-handle-invalid-byte-sequence-in-utf-8-errors-l branch October 31, 2024 10:52
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