FIX: missing ip settings from SDK package (#957)

* testing

* add infer_ip to sdkinfo processor

* changelog

* Apply suggestions from code review

* remove auto

Author: lucas-zimerman

CHANGELOG.md

@@ -8,9 +8,25 @@
 
 ## Unreleased
 
+### Important Changes
+
+- **fix(browser): Ensure IP address is only inferred by Relay if `sendDefaultPii` is `true`** ([#957](https://github.com/getsentry/sentry-capacitor/pull/957))
+
+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 JavaScript 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.
+To avoid making a major bump, the fix was patched on the current version and not by bumping to V10.
+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!
+
 ### Features
 
-- Support for Swift Package Manager  ([#938](https://github.com/getsentry/sentry-capacitor/pull/938))
+- Support for Swift Package Manager ([#938](https://github.com/getsentry/sentry-capacitor/pull/938))
 
 ### Dependencies
 
@@ -97,7 +113,7 @@ Sentry.init({
     enableLogs: true,
   },
 });
-````
+```
 
 You can also filter the logs being collected by adding beforeSendLogs into `_experiments`
 

src/integrations/sdkinfo.ts

@@ -1,18 +1,37 @@
-import type { Event, Integration, Package } from '@sentry/core';
+import type { Event, Integration, Package, SdkInfo } from '@sentry/core';
 import { logger } from '@sentry/core';
 import { SDK_NAME, SDK_VERSION } from '../version';
 import { NATIVE } from '../wrapper';
 
+// TODO: Remove this on JS V10.
+interface IpPatchedSdkInfo extends SdkInfo {
+  settings?: {
+    infer_ip?: 'auto' | 'never';
+  };
+}
+
 const INTEGRATION_NAME = 'SdkInfo';
 
 let NativeSdkPackage: Package | null = null;
+let DefaultPii: boolean | undefined = undefined;
 
 export const sdkInfoIntegration = (): Integration => {
   return {
     name: INTEGRATION_NAME,
     processEvent: processEvent,
+    setup(client) {
+      const options = client.getOptions();
+      DefaultPii = options.sendDefaultPii;
+      if (DefaultPii) {
+        client.on('beforeSendEvent', (event => {
+          if (event.user?.ip_address === '{{auto}}') {
+            delete event.user.ip_address;
+          }
+        }));
+      }
+    }
   };
-};
+}
 
 async function processEvent(event: Event): Promise<Event> {
   // The native SDK info package here is only used on iOS as `beforeSend` is not called on `captureEnvelope`.
@@ -29,18 +48,27 @@ async function processEvent(event: Event): Promise<Event> {
   }
 
   event.platform = event.platform || 'javascript';
-  event.sdk = event.sdk || {};
-  event.sdk.name = event.sdk.name || SDK_NAME;
-  event.sdk.version = event.sdk.version || SDK_VERSION;
-  event.sdk.packages = [
+  const sdk = (event.sdk || {}) as IpPatchedSdkInfo;
+  sdk.name = sdk.name || SDK_NAME;
+  sdk.version = sdk.version || SDK_VERSION;
+  sdk.packages = [
     // default packages are added by baseclient and should not be added here
-    ...(event.sdk.packages || []),
+    ...(sdk.packages || []),
     ...((NativeSdkPackage && [NativeSdkPackage]) || []),
     {
       name: 'npm:@sentry/capacitor',
       version: SDK_VERSION,
     },
   ];
 
+  // Patch missing infer_ip.
+  sdk.settings = {
+    infer_ip: DefaultPii ? 'auto' : 'never',
+    // purposefully allowing already passed settings to override the default
+    ...sdk.settings
+  };
+
+  event.sdk = sdk;
+
   return event;
 }

test/integrations/sdkinfo.test.ts

@@ -1,4 +1,4 @@
-import type { Event, EventHint, Package } from '@sentry/core';
+import type {  Client,Event,  EventHint,  Package  } from '@sentry/core';
 import { SDK_NAME, SDK_VERSION } from '../../src/';
 import { sdkInfoIntegration } from '../../src/integrations';
 import { NATIVE } from '../../src/wrapper';
@@ -30,6 +30,7 @@ jest.mock('../../src/wrapper', () => {
 
 describe('Sdk Info', () => {
   afterEach(() => {
+
     NATIVE.platform = 'ios';
   });
 
@@ -76,9 +77,89 @@ describe('Sdk Info', () => {
     expect(processedEvent?.sdk?.name).toEqual(SDK_NAME);
     expect(processedEvent?.sdk?.version).toEqual(SDK_VERSION);
   });
+  it('Add none setting when defaultIp is undefined', async () => {
+    mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(null);
+    const mockEvent: Event = {};
+    const processedEvent = await processEvent(mockEvent, {}, undefined);
+
+    expect(processedEvent?.sdk?.name).toEqual(SDK_NAME);
+    expect(processedEvent?.sdk?.version).toEqual(SDK_VERSION);
+    // @ts-expect-error injected type.
+    expect(processedEvent?.sdk?.settings?.infer_ip).toEqual('never');
+  });
+
+  it('Add none setting when defaultIp is false', async () => {
+    mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(null);
+    const mockEvent: Event = {};
+    const processedEvent = await processEvent(mockEvent, {}, false);
+
+    expect(processedEvent?.sdk?.name).toEqual(SDK_NAME);
+    expect(processedEvent?.sdk?.version).toEqual(SDK_VERSION);
+    // @ts-expect-error injected type.
+    expect(processedEvent?.sdk?.settings?.infer_ip).toEqual('never');
+  });
+
+  it('Add auto setting when defaultIp is true', async () => {
+    mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(null);
+    const mockEvent: Event = {};
+    const processedEvent = await processEvent(mockEvent, {}, true);
+
+    expect(processedEvent?.sdk?.name).toEqual(SDK_NAME);
+    expect(processedEvent?.sdk?.version).toEqual(SDK_VERSION);
+    // @ts-expect-error injected type.
+    expect(processedEvent?.sdk?.settings?.infer_ip).toEqual('auto');
+  });
+
+  it('removes ip_address if it is "{{auto}}"', () => {
+    const mockHandler = jest.fn();
+
+    const client = {
+      getOptions: () => ({ sendDefaultPii: true }),
+      on: (eventName: string, cb: (event: any) => void) => {
+        if (eventName === 'beforeSendEvent') {
+          mockHandler.mockImplementation(cb);
+        }
+      }
+    };
+
+    sdkInfoIntegration().setup!(client as any);
+
+    const testEvent = { user: { ip_address: '{{auto}}' } };
+    mockHandler(testEvent);
+
+    expect(testEvent.user.ip_address).toBeUndefined();
+  });
+
+  it('keeps ip_address if it is not "{{auto}}"', () => {
+    const mockHandler = jest.fn();
+
+    const client = {
+      getOptions: () => ({ sendDefaultPii: true }),
+      on: (eventName: string, cb: (event: any) => void) => {
+        if (eventName === 'beforeSendEvent') {
+          mockHandler.mockImplementation(cb);
+        }
+      }
+    };
+
+    sdkInfoIntegration().setup!(client as any);
+
+    const testEvent = { user: { ip_address: '1.2.3.4' } };
+    mockHandler(testEvent);
+
+    expect(testEvent.user.ip_address).toBe('1.2.3.4');
+  });
 });
 
-function processEvent(mockedEvent: Event, mockedHint: EventHint = {}): Event | null | PromiseLike<Event | null> {
+function processEvent(mockedEvent: Event, mockedHint: EventHint = {}, sendDefaultPii?: boolean): Event | null | PromiseLike<Event | null> {
   const integration = sdkInfoIntegration();
+  if (sendDefaultPii != null) {
+    const mockClient: jest.Mocked<Client> = {
+      getOptions: jest.fn().mockReturnValue({ sendDefaultPii: sendDefaultPii }),
+      on: jest.fn(),
+    } as any;
+    integration.setup!(mockClient);
+
+  }
   return integration.processEvent!(mockedEvent, mockedHint, {} as any);
 }