fix: sanitize page URLs recorded into telemetry

[mayberryzane]

Summary

ErrorMessage.url was set from window.location.href verbatim, so any secret sitting in the query string or fragment (OAuth access_token/id_token, magic links, password reset tokens) was uploaded to the backend with the error event.

sanitizeUrl already redacts sensitive query params and strips token-bearing fragments — that landed in #595 — but it was only ever wired into the network listener. The error path is separate code (client/index.tsx, not network-sanitizer.ts) and was never covered.

This adds sanitizedLocationHref() next to sanitizeUrl and routes every telemetry-bound page URL through it.

What changed

Error URLs

• Highlight._recordErrorMessage — client/index.tsx
• ObserveSDK error recording, both call sites — sdk/observe.ts
• ErrorListener — window.onerror and unhandled rejections
• the console.error capture in FirstLoadListeners

Span attributes and names

• url.full and event.url on user-interaction spans
• url.full on ld.track spans
• url.full, page_view.url, page_view.previous_url on the page-view span — this one fires on every SPA navigation, including OAuth callbacks
• the history-navigation span name built in _updateInteractionName

Metric group attributes across viewport, device, web-vital, performance, and network-performance gauges, in both SDK generations (client/index.tsx and sdk/observe.ts) plus H.recordMetric

Custom events and session properties

• Navigate / Reload custom events
• the Referrer custom event and referrer session property (document.referrer carries the previous page's query string)
• the jank listener's emitted newLocation

Deliberately unchanged

• Comparisons and change detection still read the raw window.location.href (LocationChangeInstrumentation._lastUrl, jankState.location, the pre/post URLs in _patchHistoryMethod). Sanitizing those would let redaction collapse two distinct URLs into one and silently drop legitimate page views. Only the recorded value is sanitized.
• window.location.pathname sites — no query or fragment to leak.
• SegmentIntegrationListener's initial callback(window.location.href). Its consumers branch on obj.type, so a bare string is dropped and never recorded.

How did you test this change?

• New error-listener.test.ts drives the real window.onerror handler with ?access_token=… and #id_token=… in the URL and asserts the secret is absent from the recorded ErrorMessage.url, plus a case confirming benign paths, params, and anchors survive.
• New sanitizedLocationHref cases alongside the existing sanitizeUrl suite.
• Verified the new tests are genuine regression coverage: stashed the fix, confirmed they fail on the unsanitized URL, restored, confirmed they pass.
• yarn turbo run lint enforce-size test --filter highlight.run — 25 files, 452 tests pass; bundle 169.83 kB brotli against the 256 kB limit.
• yarn format-check and yarn dedupe --check clean.

Are there any deployment considerations?

Patch-level. One behavior change worth flagging: metric group values and page-view URLs are now redacted, so aggregation keys shift for any URL that contained sensitive params. Metrics from such URLs will group under the redacted form rather than the raw one.

🤖 Generated with Claude Code

---

Note

Overview
Stops secrets in query strings and URL fragments (OAuth tokens, magic links, etc.) from being uploaded with errors, spans, metrics, and session replay custom events by routing telemetry-bound page URLs through existing sanitizeUrl logic via a new sanitizedLocationHref() helper.

Errors and listeners: ErrorMessage.url and console-captured errors now use sanitizedLocationHref() instead of raw window.location.href in the main client, Observe SDK, ErrorListener, and FirstLoadListeners.

Spans and analytics: User-interaction spans (url.full, event.url), page-view spans on SPA navigations (url.full, page_view.url, page_view.previous_url), ld.track spans, and history-driven navigation span names are sanitized before recording; raw URLs are still used only for equality / change detection so redaction does not drop navigations.

Metrics and replay: Metric group attributes (viewport, device, web vitals, performance, H.recordMetric) use the sanitized href. Navigate / Reload custom events, Referrer events and referrer session properties (sanitizeUrl(document.referrer)), and jank newLocation are redacted before recording.

Tests: New error-listener.test.ts and sanitizedLocationHref cases in the OTel instrumentation suite.

^{Reviewed by Cursor Bugbot for commit dc66458. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit dc66458. Configure here.}

[cursor]

History span name sanitization gap

High Severity

_patchHistoryMethod builds urlAfter as pathname + hash + search, then passes that string to sanitizeUrl. That order puts the query after the fragment, so the parser treats query params as part of the hash. Token-bearing fragments can survive when a benign query is present, and sensitive query keys like sig can miss redaction when a hash exists. Elsewhere this PR records sanitizedLocationHref() instead.

 

^{Triggered by learned rule: URLs emitted to telemetry must go through sanitizeUrl — covers fragments and OAuth query params}

^{Reviewed by Cursor Bugbot for commit dc66458. Configure here.}

[mayberryzane]

Confirmed and fixed in 09f73d4.

Reproduced both halves against the real URL parser:

composed string	parsed hash	parsed search	leak
/callback#access_token=SECRET?foo=bar	#access_token=SECRET?foo=bar	""	sanitizeFragment splits on the ? and only inspects foo=bar, so the token survives
/p#section?sig=SECRET	#section?sig=SECRET	""	search is empty so query redaction never runs, and sig is not in SENSITIVE_FRAGMENT_PARAMS

Fix is to compose pathname + search + hash so the string is a well-formed URL. Both url and urlAfter use the same order, so the raw comparison that gates the rename is unaffected.

Added user-interaction.test.ts, which drives the real patched history.pushState and asserts on the span name. Verified it is genuine regression coverage — with the old ordering restored, 3 of the 4 cases fail (/download#section?sig=SECRET, /users#section?page=2&sort=name, and the access_token case); with the fix they pass.

yarn turbo run lint enforce-size test --filter highlight.run: 26 files, 456 tests pass. format-check and tsc --noEmit clean.