Draft: feat(scrub): secrets embedded in strings + full-tail list redaction

Summary

Successor to !26 (closed), carrying the half that needs real scrutiny. Stacked on !27 (merged) — target that branch, and retarget to main once !27 (merged) merges. The diff shown here is only this branch's two commits.

The key denylist redacts a field whose name looks sensitive. It does nothing when the secret is inside a string — which is how secrets usually reach an error tracker, because nobody chose to send them. An object gets stringified into an exception message or a stack-frame local and carries its constructor arguments along:

vars: { "creds": "Credentials(username='alice', password='hunter2')" }

This adds two value patterns for that. Server-side scrubbing stays a backstop, not the primary control — the right answer is still not to send the data. This is for operators who don't control the SDKs sending to them.

Rust-only, deliberately. apps/event_ingest/pii_scrubber.py is left alone as the Python ingest path is retired. Rust-only behavior uses a separate assert_event_scrub helper so the parity suite keeps meaning exactly "both engines agree" rather than silently absorbing the divergence.

Commits

1. String-headed list tail — moved here from !27 (merged) after review found two judgment calls in it. See below.
2. URL credentials — scheme://user:password@host, password only. ~+7%.
3. Inline assignments — quoted key='value' in free text, plus the corrected benchmark fixture. ~+83%.

Both commits are the fixed form. !26 (closed) had a fourth commit repairing two redaction bugs found by independent review; those repairs are folded into the commits that introduced them, so what you read here is the corrected code rather than a bug followed by a patch. Both bugs are described in the commit messages and covered by regression tests — they are the reason this half is separated, and they're summarised below.

The list-tail commit, and why it's here rather than in !27 (merged)

The [[key, value], …] fast path only handled element [1]. A longer entry kept its tail neither key-redacted nor pattern-scrubbed — the continue skipped the generic walk, so element [2] onward was never visited by any mechanism. extra, contexts and errors are Any-typed, so a client-sent three-element list reaches persistence. That part is an unambiguous bug.

The fix is not unambiguous, and review found two reasons why:

It over-redacts structured rows. The justification — "for a header pair every element past the key is a value" — holds for request.headers and request.query_string, where the pair shape is contractual. It does not hold for extra, contexts, breadcrumbs.*.data or frame vars, which routinely carry list-of-lists that are rows:

{"extra":{"timings":[["auth",12,34]]}}
  before: [["auth","[Filtered]",34]]
  after:  [["auth","[Filtered]","[Filtered]"]]

auth is a default key token, so a row whose first column happens to name one loses the rest of the row. The delta over the old behavior is narrow — element [1] was already redacted — but scrubbing precedes persistence, so it's unrecoverable.

It converts O(1) work per row into O(n), and that's a memory amplifier. Measured on {"extra":[["password",0,0,…×15M]]} — inside the 30 MB unzipped cap, a few KB gzipped:

	time
before	2.4 µs
after	800 ms
honest "must walk it" baseline (flat array)	68 ms

~730 ms of that is pure waste: 15M individual placeholder allocations to produce one constant repeated. Output grows 6.5× with the default placeholder, 17.5× with a 32-char operator one, and in-heap it's worse since each inline Value::Number becomes a separately-allocated Value::String. Total work stays linear in payload size — not superlinear, not a ReDoS — but it turns a previously skipped region into an amplifier.

pair.truncate(1) plus a single placeholder push bounds it at O(1) and redacts strictly more, at the cost of collapsing the row's arity. Not applied, because it changes output shape and that's a reviewer's call. Decision 4 below.

What earlier review found

The URL pattern escaped the URL and destroyed data. The password group excluded only @, whitespace and /, so a host:port URL with no path followed anywhere later by an @ consumed everything between and replaced it:

{"url":"http://host:8080","contact":"a@b.com"}  →  {"url":"http://host:[Filtered]@b.com"}

Serialized JSON and logfmt are the common carriers, so this was the normal case. Scrubbing precedes persistence, so the loss was unrecoverable, and it was a regression — on main those strings pass through untouched. The original tests missed it because every URL in them had a path and the / terminated the match. url_pattern_does_not_escape_into_surrounding_text now covers it, with every case deliberately lacking a / after the port.

A benign quoted wrapper hid every secret inside it. replace_all resumes past the whole match, and a quoted value body runs to its closing quote, so a non-sensitive key swallowed any nested assignment:

Error executing query: "UPDATE users SET password='hunter2'"   → unredacted
{'body': "password='hunter2'"}                                  → unredacted

Both are exactly the shapes this mechanism exists for. Non-sensitive bodies are now rescanned, depth-capped.

What it catches, and what it doesn't

Requiring the value to be quoted is the false-positive defense: reprs, dict dumps and embedded JSON quote their strings; prose does not.

Input	Result
Credentials(username='alice', password='hunter2')	password='[Filtered]', username kept
{'password': 'hunter2', 'host': 'pg'}	redacted
{"api_key": "sk-live-123", "page": "2"}	redacted
"UPDATE users SET password='hunter2'" (nested in a benign value)	redacted
postgres://app_user:p4ssw0rd@db:5432/app	app_user:[Filtered]@db
redis://:s3cret@cache:6379/0 (empty user)	redacted
password='he\'s in' (escaped quote)	redacted whole
auth=failed, session expired, retry: 3	untouched
{"url":"http://host:8080","contact":"a@b.com"}	untouched
Credentials('alice', 'hunter2') — positional	miss, no key to match on
password=hunter2 — unquoted	miss by design → aggressive_key_match
postgres://u:aB3/xY9@h — / in password	miss, admitting / re-opens the escape
https://<token>:x-oauth-basic@ — secret in user position	miss, constant password redacted instead

Every miss has a test, so they stay documented rather than becoming folklore.

Performance

The fixture carries repr-shaped frame locals and an embedded-JSON extra value, and the arms are flag-isolated so each pattern's marginal cost is reproducible from the repo:

key denylist only            58.1 us/event
+ cards/PEM (pre-series)     67.9
+ url creds                  72.5   (+7%)
+ inline assign (default)   124.1   (+83%)
+ emails                    129.3
aggressive key match        227.6

So: URL credentials are nearly free. Inline assignments roughly double the scrub. Absolute figures move a lot between machine states on this laptop — trust the ratios within a run, not the numbers.

cd crates/gt-ingest && cargo test --release scrub::tests::bench -- --ignored --nocapture

Open decisions

1. Should scrub_inline_assignments default on? It currently does. Against it: the +83%, plus it is the one mechanism that can remove text an operator wanted — source lines in context_line are also key = "value" shaped, and redacting inside exception.value changes the derived issue title and therefore the grouping hash. For it: it is the only thing here that fixes the repr-shaped leak, and it only fires when someone has already set enabled: true. My recommendation is to default it off; flipping is one word plus test-config churn.

2. The depth bound fails open. Past MAX_SCRUB_DEPTH (48) the subtree is persisted unscrubbed; serde_json accepts to 128, so 49–128 is reachable and extra is dict[str, Any]. For a redaction control, "too deep to inspect" arguably should mean "redact". That is data loss on deep-but-benign payloads and a divergence from long-standing behavior, so I have not changed it.

4. The list-tail redaction, two coupled sub-decisions. (a) Take truncate(1) + single push to bound the amplifier at O(1)? It also redacts more. (b) Restrict whole-tail redaction to the request.* sections where the pair shape is contractual, keeping index-1-only for extra/contexts? That preserves structured rows but needs section-awareness threaded through scrub_list, which it doesn't currently have.

3. CPU amplification. A 5 MB payload of a:'' costs ~159 ms, and gzips ~510:1, so a ~10 KB POST buys that. Not superlinear — verified 2.00×±0.05 per doubling, so no ReDoS — but scrub_event runs inline with no yield point on a runtime worker shared across tenants. Mitigation touches pipeline.rs, outside this change.

Known-good follow-ups, measured but not included

• is_sensitive_key is ~67% of total scrub time and allocates 3.7× per key; 82% of real keys are plain lowercase ASCII needing no normalization. A borrow-only fast path measured −32% on that function.
• The four guards are four separate passes over the same bytes and are ~76% of scrub_string's cost; one fused pass measured −60%.
• Implementing regex::Replacer::replace_append instead of closures returning String removes an allocation per non-sensitive match.

Together these would likely absorb a good part of the inline-assignment cost, which is relevant to decision 1.

Still outstanding

• No UI. Project.scrub_config is editable via Django admin as raw JSON and nothing else.
• No org-level tier, unlike scrub_ip_addresses.
• Backend GLITCHTIP_RUST_INGEST still defaults to False, so none of this is live until that flips.

---

AI disclosure: Claude Opus 5 (Claude Code) authored the patterns and tests, performed the split from !26 (closed), and drafted this description. Three independent fresh-context AI review passes found the two redaction bugs described above. Reviewed by @bufke before publishing.