Sentry scrubber does not redact X-API-Key, so a 500 can ship a live brz_ credential

[ToddHebebrand]

Found while reviewing #2826. Not caused by that PR — pre-existing, and it affects every API-key-authenticated route, so filing separately.

What's wrong: apps/api/src/services/sentry.ts:24-30 scrubs exactly two request headers:

const headers = event?.request?.headers;
if (headers) {
  for (const k of Object.keys(headers)) {
    if (k.toLowerCase() === 'authorization' || k.toLowerCase() === 'cookie') headers[k] = '[redacted]';
  }
}

x-api-key is not in that list. Every API-key-authenticated surface passes its credential in that header — organization API keys (brz_...) and partner service principals (brz_sp_...). @sentry/node's HTTP integration attaches event.request.headers, and app.onError (apps/api/src/index.ts:1003-1020) calls captureException on any unhandled throw. So a single 500 on an API-key route can put a live, unexpired credential into Sentry, where it is retained and visible to anyone with project access.

The intent was already there. The extra scrubber immediately below (sentry.ts:31-40) does know about this credential shape:

if (SENSITIVE_KEYS.has(k.toLowerCase()) || (typeof v === 'string' && v.startsWith('brz_'))) {
  extra[k] = '[redacted]';
}

The brz_ prefix check was written for exactly this class of secret and was never extended to headers. This reads as an oversight in #1379 B3 rather than a deliberate scope decision.

Suggested fix: in scrubEvent, redact by value as well as by name — any header whose value matches /^brz_/, plus x-api-key and x-breeze-csrf by name. Redacting on the value catches future header names for free. scrubEvent is already exported for test, and sentry.test.ts has the harness, so this is a small change with a direct regression test.

Worth checking during the fix: whether any credential has already landed in the retained Sentry events. If so, those keys should be rotated rather than just scrubbed going forward.

Severity: not remotely exploitable on its own — it needs an unhandled 500 on an API-key route — but the payload is a working credential and the blast radius is whoever can read the Sentry project. I'd treat it as a should-fix rather than a drop-everything.

Staying open — no code change yet.

[bdunncompany]

I went to fix this and could not reproduce it. The code quoted here is no longer on main — but there is a live leak of exactly the credential you describe, on a different path. Details below, because the fix changes shape.

The quoted scrubber was replaced on 2026-07-27

sentry.ts:24-30 (the authorization/cookie header loop) and sentry.ts:31-40 (the extra scrubber with the brz_ prefix check) were both removed by #2843 (a50769487, "data classification, export preflight, and audit hardening (security wave 7)"):

$ git log --oneline -S "k.toLowerCase() === 'authorization'" -- apps/api/src/services/sentry.ts
a50769487 fix(security): data classification, export preflight, and audit hardening (security wave 7) (#2843)
82c8facc8 feat(observability): beforeSend PII scrub + uncaughtException handler (#1379 B3+B4) (#1824)

scrubEvent on d5653b482 no longer redacts headers by name, because it no longer keeps headers at all — it rebuilds the surface from an allowlist:

mutableEvent.request =
  typeof mutableEvent.request?.method === 'string'
    ? { method: mutableEvent.request.method }
    : undefined;
...
delete mutableEvent.extra;

So for the case as filed — an unhandled 500 on an api-key route, app.onError → captureException — the credential cannot ship. request is reduced to { method } and extra is deleted outright. Extending the header allowlist would be a no-op against dead code.

The real gap: beforeSend does not apply to transaction events

Sentry.init sets beforeSend only. In @sentry/core the two hooks are dispatched by event type (client.js:735-793):

if (isErrorEvent(processedEvent) && beforeSend) return beforeSend(processedEvent, hint);
if (isTransactionEvent(processedEvent)) { ... if (beforeSendTransaction) return beforeSendTransaction(processedEvent, hint); }

There is no beforeSendTransaction anywhere in the repo, so scrubEvent never runs on a transaction event.

Transaction events do carry the headers. requestDataIntegration() is a default node integration (@sentry/node-core getDefaultIntegrations), its processEvent runs on every event type, and extractNormalizedRequestData copies the header bag verbatim:

const headers = { ...normalizedRequest.headers };
if (include.headers) { requestData.headers = headers; ... }

The only names it removes are cookie (when cookies are excluded) and the ip-bearing headers. include.headers resolves to dataCollection.httpHeaders.request !== false, and with sendDefaultPii unset that value is an object ({ deny: PII_HEADER_SNIPPETS }), so it is truthy — headers are attached. The SENSITIVE_KEY_SNIPPETS deny list (which does contain key, and would have caught x-api-key) is only applied on the span-attribute path via httpHeadersToSpanAttributes, not to the transaction event's request.headers.

This is on by default for anyone who copies .env.example

.env.example:614:SENTRY_TRACES_SAMPLE_RATE=0.1
docker-compose.yml:184:  SENTRY_TRACES_SAMPLE_RATE: ${SENTRY_TRACES_SAMPLE_RATE:-0}

Compose defaults to 0, but .env.example ships 0.1. An operator who copies the example gets 10% transaction sampling, and every sampled request on an api-key route ships a live brz_ / brz_sp_ credential — no 500 required. That is strictly worse than the case filed: it needs no error at all, just traffic.

Fix shape

Not "add x-api-key to the header allowlist" — the allowlist already excludes everything. It is beforeSendTransaction.

The one thing to be careful about: scrubEvent cannot be reused verbatim for transactions. It does delete mutableEvent.contexts, and a transaction event without contexts.trace is invalid — that would silently disable tracing rather than secure it. The transaction scrubber needs to reduce request to { method } and drop extra the same way, while leaving contexts.trace and the span tree intact.

Happy to put that up as a small pr with a regression test alongside the existing scrubEvent harness in sentry.test.ts, if you want it — say the word and I will, or leave it to you if you would rather scope it with the rest of the wave.

Your "worth checking during the fix" point still stands and gets bigger: if any deployment has run with a nonzero SENTRY_TRACES_SAMPLE_RATE, the retained transaction events, not just the error events, are where a live key would be, and those keys want rotating.