Skip to content

Commit 21c9e75

Browse files
Fix(V6): missing ip settings from SDK package (#5092)
* apply patch to V6 * Update CHANGELOG.md * Update samples/react-native/src/App.tsx * lint --------- Co-authored-by: Antonis Lilis <[email protected]>
1 parent 98f632c commit 21c9e75

File tree

3 files changed

+137
-7
lines changed

3 files changed

+137
-7
lines changed

CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,24 @@
66
> make sure you follow our [migration guide](https://docs.sentry.io/platforms/react-native/migration/) first.
77
<!-- prettier-ignore-end -->
88
9+
## Unreleased
10+
11+
### Important Changes
12+
13+
- **fix(browser): Ensure IP address is only inferred by Relay if `sendDefaultPii` is `true`** ([#5092](https://github.com/getsentry/sentry-react-native/pull/5092))
14+
15+
This release includes a fix for a [behaviour change](https://docs.sentry.io/platforms/javascript/migration/v8-to-v9/#behavior-changes)
16+
that was originally introduced with v9 of the JavaScript SDK: User IP Addresses should only be added to Sentry events automatically,
17+
if `sendDefaultPii` was set to `true`.
18+
19+
However, the change in v9 required further internal adjustment, which should have been included in v10 of the SDK.
20+
To avoid making a major bump, the fix was patched on the current version and not by bumping to V10.
21+
There is _no API_ breakage involved and hence it is safe to update.
22+
However, after updating the SDK, events (errors, traces, replays, etc.) sent from the browser, will only include
23+
user IP addresses, if you set `sendDefaultPii: true` in your `Sentry.init` options.
24+
25+
We apologize for any inconvenience caused!
26+
927
## 6.20.0
1028

1129
### Features

packages/core/src/js/integrations/sdkinfo.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ import { isExpoGo, notWeb } from '../utils/environment';
55
import { SDK_NAME, SDK_PACKAGE_NAME, SDK_VERSION } from '../version';
66
import { NATIVE } from '../wrapper';
77

8+
// TODO: Remove this on JS V10.
9+
interface IpPatchedSdkInfo extends SdkInfoType {
10+
settings?: {
11+
infer_ip?: 'auto' | 'never';
12+
};
13+
}
14+
815
const INTEGRATION_NAME = 'SdkInfo';
916

1017
type DefaultSdkInfo = Pick<Required<SdkInfoType>, 'name' | 'packages' | 'version'>;
@@ -19,13 +26,25 @@ export const defaultSdkInfo: DefaultSdkInfo = {
1926
],
2027
version: SDK_VERSION,
2128
};
29+
let DefaultPii: boolean | undefined = undefined;
2230

2331
/** Default SdkInfo instrumentation */
2432
export const sdkInfoIntegration = (): Integration => {
2533
const fetchNativeSdkInfo = createCachedFetchNativeSdkInfo();
2634

2735
return {
2836
name: INTEGRATION_NAME,
37+
setup(client) {
38+
const options = client.getOptions();
39+
DefaultPii = options.sendDefaultPii;
40+
if (DefaultPii) {
41+
client.on('beforeSendEvent', event => {
42+
if (event.user?.ip_address === '{{auto}}') {
43+
delete event.user.ip_address;
44+
}
45+
});
46+
}
47+
},
2948
setupOnce: () => {
3049
// noop
3150
},
@@ -37,15 +56,24 @@ async function processEvent(event: Event, fetchNativeSdkInfo: () => Promise<Pack
3756
const nativeSdkPackage = await fetchNativeSdkInfo();
3857

3958
event.platform = event.platform || 'javascript';
40-
event.sdk = event.sdk || {};
41-
event.sdk.name = event.sdk.name || defaultSdkInfo.name;
42-
event.sdk.version = event.sdk.version || defaultSdkInfo.version;
43-
event.sdk.packages = [
59+
const sdk = (event.sdk || {}) as IpPatchedSdkInfo;
60+
sdk.name = sdk.name || defaultSdkInfo.name;
61+
sdk.version = sdk.version || defaultSdkInfo.version;
62+
sdk.packages = [
4463
// default packages are added by baseclient and should not be added here
45-
...(event.sdk.packages || []),
64+
...(sdk.packages || []),
4665
...((nativeSdkPackage && [nativeSdkPackage]) || []),
4766
];
4867

68+
// Patch missing infer_ip.
69+
sdk.settings = {
70+
infer_ip: DefaultPii ? 'auto' : 'never',
71+
// purposefully allowing already passed settings to override the default
72+
...sdk.settings,
73+
};
74+
75+
event.sdk = sdk;
76+
4977
return event;
5078
}
5179

packages/core/test/integrations/sdkinfo.test.ts

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Event, EventHint, Package } from '@sentry/core';
1+
import type { Client, Event, EventHint, Package } from '@sentry/core';
22

33
import { SDK_NAME, SDK_VERSION } from '../../src/js';
44
import { sdkInfoIntegration } from '../../src/js/integrations/sdkinfo';
@@ -86,9 +86,93 @@ describe('Sdk Info', () => {
8686
expect(processedEvent?.sdk?.name).toEqual(SDK_NAME);
8787
expect(processedEvent?.sdk?.version).toEqual(SDK_VERSION);
8888
});
89+
90+
it('Add none setting when defaultIp is undefined', async () => {
91+
mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(null);
92+
const mockEvent: Event = {};
93+
const processedEvent = await processEvent(mockEvent, {}, undefined);
94+
95+
expect(processedEvent?.sdk?.name).toEqual(SDK_NAME);
96+
expect(processedEvent?.sdk?.version).toEqual(SDK_VERSION);
97+
// @ts-expect-error injected type.
98+
expect(processedEvent?.sdk?.settings?.infer_ip).toEqual('never');
99+
});
100+
101+
it('Add none setting when defaultIp is false', async () => {
102+
mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(null);
103+
const mockEvent: Event = {};
104+
const processedEvent = await processEvent(mockEvent, {}, false);
105+
106+
expect(processedEvent?.sdk?.name).toEqual(SDK_NAME);
107+
expect(processedEvent?.sdk?.version).toEqual(SDK_VERSION);
108+
// @ts-expect-error injected type.
109+
expect(processedEvent?.sdk?.settings?.infer_ip).toEqual('never');
110+
});
111+
112+
it('Add auto setting when defaultIp is true', async () => {
113+
mockedFetchNativeSdkInfo = jest.fn().mockResolvedValue(null);
114+
const mockEvent: Event = {};
115+
const processedEvent = await processEvent(mockEvent, {}, true);
116+
117+
expect(processedEvent?.sdk?.name).toEqual(SDK_NAME);
118+
expect(processedEvent?.sdk?.version).toEqual(SDK_VERSION);
119+
// @ts-expect-error injected type.
120+
expect(processedEvent?.sdk?.settings?.infer_ip).toEqual('auto');
121+
});
122+
123+
it('removes ip_address if it is "{{auto}}"', () => {
124+
const mockHandler = jest.fn();
125+
126+
const client = {
127+
getOptions: () => ({ sendDefaultPii: true }),
128+
on: (eventName: string, cb: (event: any) => void) => {
129+
if (eventName === 'beforeSendEvent') {
130+
mockHandler.mockImplementation(cb);
131+
}
132+
},
133+
};
134+
135+
sdkInfoIntegration().setup!(client as any);
136+
137+
const testEvent = { user: { ip_address: '{{auto}}' } };
138+
mockHandler(testEvent);
139+
140+
expect(testEvent.user.ip_address).toBeUndefined();
141+
});
142+
143+
it('keeps ip_address if it is not "{{auto}}"', () => {
144+
const mockHandler = jest.fn();
145+
146+
const client = {
147+
getOptions: () => ({ sendDefaultPii: true }),
148+
on: (eventName: string, cb: (event: any) => void) => {
149+
if (eventName === 'beforeSendEvent') {
150+
mockHandler.mockImplementation(cb);
151+
}
152+
},
153+
};
154+
155+
sdkInfoIntegration().setup!(client as any);
156+
157+
const testEvent = { user: { ip_address: '1.2.3.4' } };
158+
mockHandler(testEvent);
159+
160+
expect(testEvent.user.ip_address).toBe('1.2.3.4');
161+
});
89162
});
90163

91-
function processEvent(mockedEvent: Event, mockedHint: EventHint = {}): Event | null | PromiseLike<Event | null> {
164+
function processEvent(
165+
mockedEvent: Event,
166+
mockedHint: EventHint = {},
167+
sendDefaultPii?: boolean,
168+
): Event | null | PromiseLike<Event | null> {
92169
const integration = sdkInfoIntegration();
170+
if (sendDefaultPii != null) {
171+
const mockClient: jest.Mocked<Client> = {
172+
getOptions: jest.fn().mockReturnValue({ sendDefaultPii: sendDefaultPii }),
173+
on: jest.fn(),
174+
} as any;
175+
integration.setup!(mockClient);
176+
}
93177
return integration.processEvent!(mockedEvent, mockedHint, {} as any);
94178
}

0 commit comments

Comments
 (0)