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

WIP: Dependency, Rails, and Ruby upgrade main branch #912

Draft
wants to merge 163 commits into
base: master
Choose a base branch
from

Conversation

esoterik
Copy link
Member

@esoterik esoterik commented Apr 8, 2024

Just opening this so we can track the diff a bit better between master and what's been merged into the upgrade branch.

When we merge this it'd also be a good opportunity to switch from master to main as the main branch!

esoterik and others added 30 commits November 1, 2024 14:38
Upgrade Ruby: 3.0 -> 3.3.5
Fix asset pipeline + image handling
Since we intend to send exception, trace, and breadcrumb info to
a third-party Sentry, we now need to scrub potentially sensitive info
before sending.

This commit attempts to remove where we've seen it, including:
* from where we were intentionally sending addtiional data with exceptions
* removing from breadcrumbs and traces via the Rails filter_parameters, which
uses regexes to filter data from appearing in logs
* dropping query params from the path in action-controller breadcrumbs, since
it was showing get params that could reveal address information.

More might be needed
Scrub potentially sensitive info before sending to Sentry
Sentry gems automatically apply filter_parameters as of 5.21 (was a
bug - getsentry/sentry-ruby#2438) so we
no longer need to do it ourselves.

This also correctly scrubs the query params from path in action controller
breadcrumbs, which was previously allowing sensitive data through.

Lastly, it more specifically targets some filtered items to prevent
unintended filtering of the congress message campaign ID.

Tested locally.
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.

5 participants