fix(protocol,node): scrub secrets out of a crash frame's source context

[krassx]

Companion to bugsee/bugsee-background-worker#53. Both must land — see Why both sides below. Neither should be merged before the other is reviewed.

The defect

A crash frame can carry context — the source lines around the throwing line — and those lines ship to the wire verbatim. So a hardcoded credential on or near the throwing line is disclosed in plaintext, while the same value held in a local variable arrives as <redacted>, because captureLocalVariables scrubs by key name (packages/node/src/local-variables.ts:145). One value, two answers, decided by which feature happened to see it.

Why both sides

The worker replaces this window on a sourcemap remap, rebuilding it from the map's own sourcesContent, and since its PR #48 it invents one for a frame that arrived without any — default-on. Almost every JS frame is remapped, so scrubbing only the SDK would leave the common path unprotected. Verified against symbolfiles/sourcemap.py and crash/javascript.py rather than taken on trust.

The judgement call: what "scrub a source line" means, and what it costs

redactSourceLines in @bugsee/protocol. Two passes, both reusing definitions that already exist rather than adding one:

1. Sensitive assignment. A string, template or numeric literal whose key/identifier satisfies isSensitiveKey becomes <redacted> — const apiKey = "…", "password": "…", obj["token"] = "…", if (password === "…"), const cvv = 123, and the wrapped form where the literal sits on the line after its key. isSensitiveHeader is consulted too, because an object literal in a fetch call is the one place a header name appears as a key.
2. Credential shape. redactShapes — this package's existing pass (anchored JWT, AWS access key ids, Stripe sk_live_/whsec_, GitHub ghp_…), already applied to network values, error messages and stack text.

Credential-shape detection: YES, but only the provider-prefixed set already in the repo — and no entropy heuristic. This was the real trade-off, and it is deliberately not the maximal option:

• The published comparisons of secret scanners (TruffleHog, Gitleaks, and the academic evaluations of both) land on the same split: entropy thresholds and broad "long alphanumeric run" regexes have high recall and low precision; the precise rules are the ones anchored to a provider prefix.
• A crash window shows exactly the code that is full of hashes, digests, UUIDs, minified identifiers and asset fingerprints. Those are the false positives.
• A false positive here silently destroys the feature: a redacted line tells the reader nothing, and unlike a leak, nobody reports it. The extra recall is not free.
• Reusing redactShapes rather than writing new patterns also means the false-positive rate is already known — those patterns run on every network body and error message the SDK captures today.

The cost, stated plainly: a secret that is neither assigned to a sensitively-named identifier nor of a recognised provider shape still ships. const k = "9f3a…" with a bare name is not caught, and neither is a secret on a bare continuation line of a multi-line expression. That is the price of not redacting most of the ordinary code in every crash window — and "fail safe, over-redact" does not by itself settle it, because over-redaction is only cheap when it is rare.

Two smaller calls, both toward over-redacting:

• Numeric literals are redacted (const pin = 1234), which also catches maxTokens: 4096 — token is a substring of maxTokens. Readability cost, no safety cost.
• The substring rule is inherited, not narrowed. isSensitiveKey matches by substring, so const author = "Ada" is redacted (auth), as are passed, compass, bypass. Narrowing it here would create the second definition CLAUDE.md forbids; if it proves too noisy, the fix is to shape the shared list, which improves network capture at the same time. There is a test asserting this over-redaction on purpose, so nobody "fixes" it locally by accident.

Credit cards are not redacted: redactShapes is called without its opt-in Luhn flag, matching every other call site. A Luhn scan over source code hits version numbers, timestamps and ids.

Order of operations

Redaction runs before the maxLineLength clip and takes the whole window at once:

• Before the clip, because clipping first cuts a long line mid-literal — no closing quote for pass 1 to match, no complete shape for pass 2, and a usable prefix of the credential on the wire. Which means the input can be a whole minified bundle line, so the pattern is anchored and carries linearity tests.
• Whole-window, because a wrapped assignment puts the key on one line and the literal on the next; a per-line rule reads that continuation as a bare string with no key to judge it by. redactSourceLines returns exactly as many lines as it was given — a multi-line template literal keeps its newline count — so pre/line/post stay aligned. A window that shifted would point a frame at source it did not throw from.

Drift

The rule now exists twice, in TypeScript and Python. That is unavoidable; the two disagreeing silently is not.

• packages/protocol/source-line-redaction.vectors.json carries the denylists and 35 whole-window vectors. Both test suites run the vectors, so an algorithmic divergence turns one of the two CIs red, independently, with no cross-repo wiring.
• The denylists in the artifact are asserted equal to sensitive.ts here, and the worker reads them at import time instead of restating them — so there is no second copy of the ~86 names anywhere.
• What is left: re-vendoring the artifact into the worker. packages/node/src/source-context.test.ts checks that when the worker checkout is present (BUGSEE_WORKER_REPO, or the sibling directory) and skips loudly where it is not. Compared by content, not bytes — pnpm lint:fix reformats the JSON, and a guard that fires on whitespace gets suppressed.

Also in here

• A lookahead that could not fire is gone. (?!=|>) was written to keep token => "x" from reading as an assignment; the mutator loop disproved it — removing it changed no test outcome, because between the operator and the value the pattern allows only whitespace and > is neither whitespace nor the start of a literal. The arrow cases stay as the pins that say why it is safe to be absent.
• The locals e2e now hardcodes its secret at the throw site instead of referencing LOCALS_SECRET, so the existing "the secret never appears anywhere in the bundle" assertion covers the source window too, plus a guard that fails if someone turns it back into a reference. Verified non-vacuous: bypassing the redaction fails it on node, bun and deno.

Testing

• pnpm ci:local --e2e passes: lint, typecheck, cycles, per-package coverage gate, harness units, and all 197 e2e tests.
• @bugsee/protocol coverage: 100% lines/functions/statements, 98.46% branches.
• Every new or changed entity went through the mutator loop (21 mutations); all are caught, and the two that survived produced the two real changes above (the dead lookahead, and a weak arrow-function test).

🤖 Generated with Claude Code

[krassx]

Closing on a product decision: we are not doing line-level secret scrubbing in source context.

The three review rounds on the worker side made the case better than any argument could. Each found a real defect, and the last was a regression introduced by the previous round's fix. That is what approximating a TypeScript parser with a regular expression looks like — the gaps are unbounded, and the next ones are already visible (password?: string, Foo<T = string>, template literals, comments between operator and literal).

Checking what the field actually does settled it. Nobody scrubs source context:

Vendor	Scrubs frame variables	Scrubs source lines	Evidence
Sentry	yes — vars is pii = "true"	no — pre_context / context_line / post_context carry no PII annotation	relay-event-schema/src/protocol/stacktrace.rs
BugSnag	metadata / request / response only	no — the code snippet is not among the four redaction paths	core/lib/json-payload.js
Rollbar	by key name	no — code is not a scrub field	src/scrub.js
Firebase Crashlytics	—	n/a — no JS/web, no source context

Sentry scrubbing vars and deliberately leaving source alone is not an oversight; it is the same line we are now drawing. The two are different risk classes:

• Runtime data — network values, local variables — can hold an end user's password or a live token, data that never existed in the repo. We scrub that, and will keep scrubbing it.
• Source code is what the customer wrote and already uploaded to us. A hardcoded secret in it is one already committed to their repository.

The guarantee for anyone who does not want their source rendered is the off switch, source_context_disabled — appserver #29, worker #48, viewer #27 — which is what every vendor above offers as their answer.

Nothing here is lost: the branch stays, and it is the reference if we ever revisit.

One finding from this work does NOT belong to it and is being carried forward separately. isSensitiveKey misses hyphenated header spellings, so {"x-api-key": "…"}, {"api-key": "…"} and {"cookie": "…"} are not redacted in a JSON body today. Verified against the 59-entry SENSITIVE_KEY_SUBSTRINGS list: it carries api_key and apikey but not the hyphenated form, and cookie lives only in SENSITIVE_HEADERS, which matches exactly and covers the header path alone. That is runtime data in a place where we do promise scrubbing — a real defect, and in scope.

🤖 Generated with Claude Code