GlitchTip Event items are scrubbed twice (before_send + transport), and redact() heap-allocates once per redaction pattern even when only one matches

[erskingardner]

Summary

Two redundant-work issues on the outbound GlitchTip path:

1. Every Sentry Event runs the full scrub pipeline twice — once in before_send = scrub_event, then again in ScrubbingTransport::send_envelope → scrub_envelope_item's EnvelopeItem::Event(_) arm.
2. redaction::redact calls replace_all(...).into_owned() for all patterns unconditionally on a dirty input, cloning into a fresh String even for patterns that did not match (where replace_all returned Cow::Borrowed), on top of the is_match pre-scan it already performed.

Location

• src/telemetry.rs:52 (before_send: Some(Arc::new(scrub_event))) vs src/telemetry.rs:249-254 (scrub_envelope_item's EnvelopeItem::Event arm re-running scrub_event_fields)
• src/redaction.rs:76-93 (redact), specifically the unconditional .into_owned() loop at lines 89-91

Details

Double scrub. before_send is applied by the Sentry client to all events (captured error!s and panics) before they become envelope items, and it fully runs scrub_event_fields. The transport's scrub_envelope then hands each EnvelopeItem::Event back through scrub_event_fields a second time. Per ScrubbingTransport's own doc comment (telemetry.rs:84-85), the transport exists as a "final outbound redaction guard for envelope item types that do not pass through before_send, such as performance transactions" — i.e. Transaction and Logs. The Event arm is redundant with before_send, so the whole regex pipeline (all patterns × every message / logentry / exception / breadcrumb / tag / context / extra field) runs twice for every first-party ERROR and every panic.

Redundant allocation. For a dirty input, redact (redaction.rs:85-91):

• has already scanned all patterns once in the is_match pre-check (lines 77-79), then
• loops output = pattern.replace_all(&output, *replacement).into_owned() over every pattern. replace_all returns Cow::Borrowed when a pattern doesn't match, and .into_owned() clones it into a new String regardless — so an input matching only one pattern still performs ~N heap allocations and a second full set of scans.

Why this is not a duplicate

The filed redaction issues (#320, #319, #294, #291, #280, #264, #257, #245, #241) are all about redaction coverage / ordering. None concern redundant double-invocation of the scrub pipeline or per-pattern allocation.

Recommendation

• Drop the EnvelopeItem::Event(_) scrub in scrub_envelope_item (rely on before_send), or document why defense-in-depth double-scrub is intentional. The transport still needs the Transaction/Logs arms.
• In redact, only reassign output when replace_all returns Cow::Owned (i.e. skip the clone when a pattern didn't match).

Impact

Cold path (only ERROR events + panics), so real-world cost is small — hence LOW. Worth cleaning up because the double-scrub also contradicts the transport's stated contract.