fix(browser): Ensure IP address is only inferred by Relay if sendDefaultPii is true (#17364)

This PR fixes a long-standing problem in the SDK where it would set
incorrect information about when Relay should (not) infer IP addresses
from sent envelope requests.

Previously, this was thought to be controlled by setting
`event.user.ip_address: '{{auto}}'`. However, after an incident in
Relay, it was determined that this is in fact not a reliably way to
control IP inference. Instead, SDKs should set
`event.sdk.settings.infer_ip: 'auto' | 'never'` (see closes
#16252).
Unfortunately, this wasn't implemented immediately but is now taken care of
in this PR.

Author: Lms24

CHANGELOG.md

@@ -4,6 +4,24 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
+## Unreleased
+
+### Important Changes
+
+- **fix(browser): Ensure IP address is only inferred by Relay if `sendDefaultPii` is `true`**
+
+This release includes a fix for a [behaviour change](https://docs.sentry.io/platforms/javascript/migration/v8-to-v9/#behavior-changes)
+that was originally introduced with v9 of the SDK: User IP Addresses should only be added to Sentry events automatically,
+if `sendDefaultPii` was set to `true`.
+
+However, the change in v9 required further internal adjustment, which should have been included in v10 of the SDK.
+Unfortunately, the change did not make it into the initial v10 version but is now applied with `10.4.0`.
+There is _no API_ breakage involved and hence it is safe to update.
+However, after updating the SDK, events (errors, traces, replays, etc.) sent from the browser, will only include
+user IP addresses, if you set `sendDefaultPii: true` in your `Sentry.init` options.
+
+We apologize for any inconvenience caused!
+
 ## 10.3.0
 
 - feat(core): MCP Server - Capture prompt results from prompt function calls (#17284)

MIGRATION.md

@@ -80,6 +80,18 @@ The removal entails **no breaking API changes**. However, in rare cases, you mig
 - If you set up Sentry Alerts that depend on FID, be aware that these could trigger once you upgrade the SDK, due to a lack of new values.
   To replace them, adjust your alerts (or dashbaords) to use INP.
 
+### Update: User IP Address collection gated by `sendDefaultPii`
+
+Version `10.4.0` introduced a change that should have ideally been introduced with `10.0.0` of the SDK.
+Originally destined for [version `9.0.0`](https://docs.sentry.io/platforms/javascript/migration/v8-to-v9/#behavior-changes), but having not the desired effect until v10,
+SDKs will now control IP address inference of user IP addresses depending on the value of the top level `sendDefaultPii` init option.
+
+- If `sendDefaultPii` is `true`, Sentry will infer the IP address of users' devices to events (errors, traces, replays, etc) in all browser-based SDKs.
+- If `sendDefaultPii` is `false` or not set, Sentry will not infer or collect IP address data.
+
+Given that this was already the advertised behaviour since v9, we classify the change [as a fix](https://github.com/getsentry/sentry-javascript/pull/17364),
+though we recognize the potential impact of it. We apologize for any inconvenience caused.
+
 ## No Version Support Timeline
 
 Version support timelines are stressful for everybody using the SDK, so we won't be defining one.

dev-packages/browser-integration-tests/suites/feedback/attachTo/test.ts

@@ -61,6 +61,9 @@ sentryTest('should capture feedback with custom button', async ({ getLocalTestUr
       version: expect.any(String),
       name: 'sentry.javascript.browser',
       packages: expect.anything(),
+      settings: {
+        infer_ip: 'never',
+      },
     },
     request: {
       url: `${TEST_HOST}/index.html`,

dev-packages/browser-integration-tests/suites/feedback/captureFeedback/test.ts

@@ -61,6 +61,9 @@ sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => {
       version: expect.any(String),
       name: 'sentry.javascript.browser',
       packages: expect.anything(),
+      settings: {
+        infer_ip: 'never',
+      },
     },
     request: {
       url: `${TEST_HOST}/index.html`,

dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/test.ts

@@ -95,6 +95,9 @@ sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestUrl
       version: expect.any(String),
       name: 'sentry.javascript.browser',
       packages: expect.anything(),
+      settings: {
+        infer_ip: 'never',
+      },
     },
     request: {
       url: `${TEST_HOST}/index.html`,

dev-packages/browser-integration-tests/suites/feedback/captureFeedbackCsp/test.ts

@@ -61,6 +61,9 @@ sentryTest('should capture feedback', async ({ getLocalTestUrl, page }) => {
       version: expect.any(String),
       name: 'sentry.javascript.browser',
       packages: expect.anything(),
+      settings: {
+        infer_ip: 'never',
+      },
     },
     request: {
       url: `${TEST_HOST}/index.html`,

dev-packages/browser-integration-tests/suites/manual-client/browser-context/test.ts

@@ -41,6 +41,9 @@ sentryTest('allows to setup a client manually & capture exceptions', async ({ ge
       name: 'sentry.javascript.browser',
       version: expect.any(String),
       packages: [{ name: expect.any(String), version: expect.any(String) }],
+      settings: {
+        infer_ip: 'never',
+      },
     },
     contexts: {
       trace: { trace_id: expect.stringMatching(/[a-f0-9]{32}/), span_id: expect.stringMatching(/[a-f0-9]{16}/) },

dev-packages/browser-integration-tests/suites/public-api/captureException/simpleError/test.ts

@@ -39,5 +39,8 @@ sentryTest('should capture correct SDK metadata', async ({ getLocalTestUrl, page
         version: SDK_VERSION,
       },
     ],
+    settings: {
+      infer_ip: 'never',
+    },
   });
 });

dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/errors/test.ts

@@ -1,10 +1,12 @@
 import { expect } from '@playwright/test';
-import type { Event } from '@sentry/core';
 import { sentryTest } from '../../../../utils/fixtures';
-import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
 
-sentryTest('should default user to {{auto}} on errors when sendDefaultPii: true', async ({ getLocalTestUrl, page }) => {
-  const url = await getLocalTestUrl({ testDir: __dirname });
-  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
-  expect(eventData.user?.ip_address).toBe('{{auto}}');
-});
+sentryTest(
+  'sets sdk.settings.infer_ip to "auto" on errors when sendDefaultPii: true',
+  async ({ getLocalTestUrl, page }) => {
+    const url = await getLocalTestUrl({ testDir: __dirname });
+    const eventData = await envelopeRequestParser(await waitForErrorRequestOnUrl(page, url));
+    expect(eventData.sdk?.settings?.infer_ip).toBe('auto');
+  },
+);

dev-packages/browser-integration-tests/suites/public-api/sendDefaultPii/performance/test.ts

@@ -7,7 +7,7 @@ import {
 } from '../../../../utils/helpers';
 
 sentryTest(
-  'should default user to {{auto}} on transactions when sendDefaultPii: true',
+  'sets user.ip_address to "auto" on transactions when sendDefaultPii: true',
   async ({ getLocalTestUrl, page }) => {
     if (shouldSkipTracingTest()) {
       sentryTest.skip();
@@ -16,6 +16,6 @@ sentryTest(
     const url = await getLocalTestUrl({ testDir: __dirname });
     const req = await waitForTransactionRequestOnUrl(page, url);
     const transaction = envelopeRequestParser(req);
-    expect(transaction.user?.ip_address).toBe('{{auto}}');
+    expect(transaction.sdk?.settings?.infer_ip).toBe('auto');
   },
 );