getsentry / getsentry/sentry-javascript

browserSessionIntegration has no way to suppress session capture for consent-gated setups (GDPR)

Đang mở
#22,261 2 bình luận 0 reaction 1 người được giao Được @s1gr1d nhận Xem trên GitHub
Browser Feature Feature: Sessions javascript Package: browser
Ngôn ngữ chính
TypeScript
Star
8.7k
Fork
1.8k
Merge trung bình
1 ngày 17 giờ
Pull request đã merge (30 ngày)
515

Mô tả

## Problem Statement

`browserSessionIntegration`'s `setupOnce()` permanently wires two triggers once installed, with no way to later disable them (there's `client.addIntegration()` but no symmetric `removeIntegration`/teardown anywhere in the integration system):

* an isolation-scope listener that calls `captureSession()` whenever the scope's user id/ip changes
* (default `lifecycle: 'route'`) a history-instrumentation handler that calls `startSession()`/`captureSession()` on every navigation

Both bypass `beforeSend`/`beforeSendTransaction` — same root cause as getsentry/sentry-javascript#22220, just for session envelopes instead of Replay. For consent-gated setups (CookieYes/OneTrust/Usercentrics/custom CMPs) that keep `Sentry.init()` alive across consent grant/withdrawal and mute collection via `beforeSend`, `BrowserSession` can't be muted the same way.

Worse, it's self-triggering: consent-withdrawal code commonly does `getIsolationScope().clear()` to guarantee nothing written pre-consent leaks into a later event — but that exact mutation is what the scope listener watches for, so withdrawing consent immediately fires a fresh `captureSession()`.

Today the only mitigation is excluding `BrowserSession` from `integrations` at `Sentry.init()` entirely and permanently, losing Release Health even for users who did consent.

## Solution Brainstorm

Accept an `enabled` predicate, checked at all three capture sites:

```ts
const browserSessionIntegration = defineIntegration((options: { lifecycle?: 'route' | 'manual'; enabled?: () => boolean } = {}) => {
const lifecycle = options.lifecycle ?? 'route';
const isEnabled = () => options.enabled?.() ?? true;

return {
name: 'BrowserSession',
setupOnce() {
// ...
if (isEnabled()) { startSession({ ignoreDuration: true }); captureSession(); }

isolationScope.addScopeListener(scope => {
const maybeNewUser = scope.getUser();
if (isEnabled() && (previousUser?.id !== maybeNewUser?.id || previousUser?.ip_address !== maybeNewUser?.ip_address)) {
captureSession();
previousUser = maybeNewUser;
}
});

if (lifecycle === 'route') {
addHistoryInstrumentationHandler(({ from, to }) => {
if (isEnabled() && from !== to) { startSession({ ignoreDuration: true }); captureSession(); }
});
}
},
};
});
```

Default `enabled: () => true` — fully backward compatible. Lets consent-driven setups pass `enabled: () => consentGranted`, keep the integration permanently installed (correct init ordering for other instrumentation), and suppress session capture whenever consent isn't currently granted — including the withdrawal-triggered scope-clear case above.

## Additional Context

Same category as getsentry/sentry-javascript#22220, found while implementing the same consent-mode gating (`@sentry/react@10.53.1`). Happy to open a PR if this shape looks reasonable.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.