Repository navigation
fix(browser): Apply urlQueryParams to event URLs, static span names and LCP URLs - #25262
Merged
JPeer264 merged 3 commits intoOct 12, 2026
Merged
Conversation
… and LCP URLs `dataCollection.urlQueryParams` only filtered `url.full` and `url.query`. The page URL in `event.request.url`, the static `browser.*` and `resource.*` span names, and the LCP URL attributes still sent sensitive values such as `?jwt=...` to Sentry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 68306da. Configure here.
|
|
||
| startAndEndSpan(span, responseStartTimestamp, responseEndTimestamp, { | ||
| name: spanStreamingEnabled ? BROWSER_NAVIGATION_TIMING_SPAN_NAMES[BROWSER_RESPONSE] : entry.name, | ||
| name: spanStreamingEnabled ? BROWSER_NAVIGATION_TIMING_SPAN_NAMES[BROWSER_RESPONSE] : url, |
There was a problem hiding this comment.
Query filter ignores owning client
Low Severity
This is a check on whether getClient() is necessary, not a hard rule break. filterCollectedUrl runs for navigation and resource span URLs and for static lcp.url without the tracing client. It then reads urlQueryParams from getClient(), which can be missing or a different client, so that client's dataCollection.urlQueryParams policy is not applied.
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 68306da. Configure here.
Member
Author
Member
There was a problem hiding this comment.
technically yes if the client changes between span start and completion i guess. but pre-existing (here and elsewhere), so not a blocker imo
Contributor
size-limit report 📦
|
…n names and LCP URLs
JPeer264
marked this pull request as ready for review
October 12, 2026 11:56
JPeer264
requested review from
logaretm,
msonnb,
nicohrubec and
s1gr1d
and removed request for
a team
October 12, 2026 11:56
s1gr1d
approved these changes
Oct 12, 2026
| return; | ||
| } | ||
| const op = NAVIGATION_TIMING_SPAN_OPS[event]; | ||
| const url = filterCollectedUrl(entry.name); |
Member
There was a problem hiding this comment.
nit: can we rename this to filteredUrl?
Member
Author
There was a problem hiding this comment.
yup good idea. I changed it
…n names and LCP URLs
JPeer264
added this pull request to stack #25291
October 12, 2026 13:36
JPeer264
added a commit
that referenced
this pull request
Oct 12, 2026
… URLs (#25289) follow up to #25262 This applies `dataCollection.urlQueryParams` to the URLs in the fetch, XHR and navigation breadcrumbs and to the `ui.element.url` attribute of `elementTimingIntegration`. These still sent sensitive query values such as `?jwt=...` to Sentry. ## Sentry events Before (11.6.0), `jwt=sensitive-jwt-value` in the breadcrumbs: https://sentry-sdks.sentry.io/issues/7787761489/events/2bfc6630501c4314bf84b884a4afb1e9/ After (this PR), `jwt=[Filtered]`: https://sentry-sdks.sentry.io/issues/7787761463/events/6ae067a147b84e6eb33553ae7ce217c7/ Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
47 tasks done
3 tasks done
JPeer264
pushed a commit
that referenced
this pull request
Oct 14, 2026
…ames and LCP URLs (#25297) Backport of: #25262 ## Differences to the original PR - **Source files moved.** v10 still uses the older `browser-utils` layout. The `performance/entries.ts` and `web-vitals/tracking.ts` changes went into `src/metrics/browserMetrics.ts`. The `web-vitals/spans.ts` change went into `src/metrics/webVitalSpans.ts`. The filtering is the same. - **Navigation spans:** v10's `_addPerformanceNavigationTiming` / `_addRequest` set no `url.full` attribute and have no span-streaming name branch. So only the span `name` is now filtered (`filterCollectedUrl(entry.name)`). - **`_sendLcpSpan` (`webVitalSpans.ts`):** on v10 this function takes no `client` parameter. `filterCollectedUrl(entry.url)` falls back to `getClient()`. - **Added beyond the original:** `src/metrics/lcp.ts` (`_sendStandaloneLcpSpan`) now filters its `lcp.url` attribute too. This standalone LCP span code exists only on v10 (it was removed on develop). Without this change it would still send the unfiltered LCP URL. This matches the original PR's goal of filtering `lcp.url`. - **`httpcontext.ts`:** v10 spreads `...reqData` into `event.request` instead of setting `url` explicitly. The backport adds `url: filterCollectedUrl(reqData.url, client)` after the spread and before `...event.request`, so a URL the user set still wins. `preprocessEvent` now takes the `client` argument. - **Unit tests adapted to v10 paths and shapes:** - The new `browserMetrics.test.ts` cases are in `test/browser/browserMetrics.test.ts`. They check `description`/`data`, not `name`/`attributes`. - The navigation case checks only span descriptions, because v10 navigation spans have no `url.full`. To share it with the new case, the navigation `entry` fixture moved to `describe` scope. - The `_sendLcpSpan` expectation update is in `test/metrics/webVitalSpans.test.ts`. No client mock is needed. - `test/metrics/lcp.test.ts` gets the same query-param expectation for the standalone LCP span. - The `httpcontext.test.ts` cases were added at the top level, because v10 has no `dataCollection` describe block. - **Integration tests:** - The develop-only `web-vitals-lcp-static` suite maps to v10's `web-vitals-lcp` (static is the default there). - The new `pageload-sensitive-query-params-static` test uses string ops (`'pageload'`, `'browser.request'`, `'resource.img'`). v10's `@sentry/conventions@0.16.0` doesn't export `BROWSER_REQUEST` / `PAGELOAD` / `RESOURCE_IMG` from `@sentry/conventions/op`. - The `httpContext` integration test imports `waitForErrorRequest`/`envelopeRequestParser`, which v10's version didn't import yet. ## Verification - `yarn format`, `yarn lint:fix` (0 warnings, 0 errors) and `yarn build:dev` all pass. - Vitest passes for: `browser-utils` (`browserMetrics`, `lcp`, `webVitalSpans`), `browser` (`browserTracingIntegration`) and `nextjs` (`pagesRouterInstrumentation`). - `browser` `httpcontext.test.ts` passed (5/5) only with a temporary local workaround. Under the sandbox's Node 22, the existing `globalThis.navigator = …` setup throws, and it does the same on unmodified v10. The repo pins Node 20. - Playwright browser-integration tests were not run locally. They are left to CI. <!-- junior-request-attribution:start --> via Junior system actor `junior`. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent-dispatch%3Adispatch_968746a4c2386e3695487fe7c0061e2b --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/agent-dispatch%3Adispatch_968746a4c2386e3695487fe7c0061e2b) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent-dispatch%3Adispatch_968746a4c2386e3695487fe7c0061e2b/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #25247
closes JS-3907
Description
This applies
dataCollection.urlQueryParamsto the URLs that the browser SDK still sent unfiltered: the page URL inevent.request.url, thebrowser.*andresource.*span names in the static trace lifecycle, and the LCP URL attributes (lcp.url,browser.web_vital.lcp.url).event.request.urlleaked on error events with every trace lifecycle, the span names only withtraceLifecycle: 'static'(the v10 default).Most of the added lines are tests.
Sentry traces
Before (11.6.0),
jwt=sensitive-jwt-valueinrequest.urland span names: https://sentry-sdks.sentry.io/explore/traces/trace/056ddfee720849c78e4e0b68f9da8c5b/?project=4510555608449024×tamp=1791796663After (this PR),
jwt=[Filtered]: https://sentry-sdks.sentry.io/explore/traces/trace/ea9fd2cc90de4343bf5bfb6eb0d923ad/?project=4510555608449024×tamp=1791796677