Skip to content

Commit

Permalink
fix(pii): Fix scrubbing user paths in minidump debug modules (#4351)
Browse files Browse the repository at this point in the history
#2724 silently broke user path
scrubbing in minidumps. The regression was not automatically flagged
because minidump scrubbing [ignores regexes that cannot be compiled with
unicode
disabled](https://github.com/getsentry/relay/blob/63ca425f5eba144cd5b8976b4201cd61084dafb9/relay-pii/src/attachments.rs#L37-L41),
regardless of whether they are user-defined regexes or builtin static
ones.

This PR both fixes the regex and adds an automatic test to each PII
regex, so we can catch similar problems in the future.
  • Loading branch information
jjbayer authored Dec 6, 2024
1 parent 050aa2d commit 8d83992
Show file tree
Hide file tree
Showing 3 changed files with 237 additions and 172 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

- Accept incoming requests even if there was an error fetching their project config. ([#4140](https://github.com/getsentry/relay/pull/4140))
- Rate limit profiles when transaction was sampled. ([#4195](https://github.com/getsentry/relay/pull/4195))
- Fix scrubbing user paths in minidump debug module names. ([#4351](https://github.com/getsentry/relay/pull/4351))

**Features**:

Expand Down
10 changes: 9 additions & 1 deletion relay-pii/src/attachments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,18 @@ fn apply_regex_to_utf8_bytes(
.build()
{
Ok(x) => x,
Err(_) => {
Err(e) => {
// XXX: This is not going to fly long-term
// Idea: Disable unicode support for regexes entirely, that drastically increases the
// likelihood this conversion will never fail.

// If we see this error in production, it means we need to add more regex validation
// to `validate_pii_config` (which is called sentry-side).
relay_log::error!(
error = &e as &dyn std::error::Error,
pattern = regex.as_str(),
"Regex failed to compile in non-unicode mode",
);
return matches;
}
};
Expand Down
Loading

0 comments on commit 8d83992

Please sign in to comment.