diff --git a/MIGRATION.md b/MIGRATION.md index e9e4910c6db3..9232ab2758ab 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -244,6 +244,12 @@ User IP address inference, which was previously gated on `sendDefaultPii`, is no `dataCollection.userInfo`. An explicit `requestDataIntegration({ include: { ip: true } })` overrides `dataCollection.userInfo: false` for data collected by that integration. +#### Astro client IP + +`trackClientIp` no longer defaults to `false`. When you leave it unset, `handleRequest` now follows +`dataCollection.userInfo`, which defaults to `true`, so Astro apps that set neither option start +reporting `user.ip_address`. Pass `trackClientIp: false` to keep the v10 behaviour. + #### Remix action form data `captureActionFormDataKeys` is an integration-level override, so it no longer requires diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index c75ffd6ed968..f63807df9d8c 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -53,7 +53,7 @@ type MiddlewareOptions = { * * Only set this to `true` if you're fine with collecting potentially personally identifiable information (PII). * - * @default false (recommended) + * @default `dataCollection.userInfo` (`true` unless disabled) */ trackClientIp?: boolean; }; @@ -78,10 +78,7 @@ type AstroLocalsWithSentry = Record & { }; export const handleRequest: (options?: MiddlewareOptions) => MiddlewareHandler = options => { - const handlerOptions = { - trackClientIp: false, - ...options, - }; + const handlerOptions = { ...options }; return async (ctx, next) => { // If no Sentry client exists, just bail @@ -209,7 +206,8 @@ async function instrumentRequestStartHttpServerSpan( normalizedRequest: winterCGRequestToRequestData(request), }); - if (options.trackClientIp) { + // The integration option wins when set; otherwise `dataCollection.userInfo` decides. + if (options.trackClientIp ?? client.getDataCollectionOptions().userInfo) { isolationScope.setUser({ ip_address: ctx.clientAddress }); } diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index 89fb8c9ad3fd..f61ae7a56ff6 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -46,6 +46,29 @@ describe('sentryMiddleware', () => { }); const setSDKProcessingMetadataMock = vi.fn(); + const DATA_COLLECTION_DEFAULTS = { + userInfo: false, + cookies: true, + httpHeaders: { request: true, response: true }, + httpBodies: [], + urlQueryParams: true, + graphQL: { document: true, variables: true }, + genAI: { inputs: true, outputs: true }, + databaseQueryData: true, + stackFrameVariables: true, + frameContextLines: 5, + }; + + function mockClientWith(dataCollection: Partial): void { + vi.spyOn(SentryNode, 'getClient').mockImplementation( + () => + ({ + getOptions: () => ({}), + getDataCollectionOptions: () => ({ ...DATA_COLLECTION_DEFAULTS, ...dataCollection }), + }) as unknown as Client, + ); + } + beforeEach(() => { vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation(() => { return { @@ -56,24 +79,7 @@ describe('sentryMiddleware', () => { } as any; }); vi.spyOn(SentryNode, 'getActiveSpan').mockImplementation(getSpanMock); - vi.spyOn(SentryNode, 'getClient').mockImplementation( - () => - ({ - getOptions: () => ({}), - getDataCollectionOptions: () => ({ - userInfo: false, - cookies: true, - httpHeaders: { request: true, response: true }, - httpBodies: [], - urlQueryParams: true, - graphQL: { document: true, variables: true }, - genAI: { inputs: true, outputs: true }, - databaseQueryData: true, - stackFrameVariables: true, - frameContextLines: 5, - }), - }) as unknown as Client, - ); + mockClientWith({ userInfo: false }); vi.spyOn(SentryNode, 'getTraceMetaTags').mockImplementation( () => ` @@ -308,6 +314,48 @@ describe('sentryMiddleware', () => { }); }); + it('attaches the client IP when `trackClientIp` is unset and `dataCollection.userInfo` is on', async () => { + mockClientWith({ userInfo: true }); + const middleware = handleRequest(); + const ctx = { + ...DYNAMIC_REQUEST_CONTEXT, + }; + + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, async () => { + expect(SentryCore.getIsolationScope().getScopeData().user?.ip_address).toBe('192.168.0.1'); + return nextResult; + }); + }); + + it('does not attach a client IP when `trackClientIp` is unset and `dataCollection.userInfo` is off', async () => { + mockClientWith({ userInfo: false }); + const middleware = handleRequest(); + const ctx = { + ...DYNAMIC_REQUEST_CONTEXT, + }; + + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, async () => { + expect(SentryCore.getIsolationScope().getScopeData().user?.ip_address).toBeUndefined(); + return nextResult; + }); + }); + + it('lets `trackClientIp=false` win over `dataCollection.userInfo`', async () => { + mockClientWith({ userInfo: true }); + const middleware = handleRequest({ trackClientIp: false }); + const ctx = { + ...DYNAMIC_REQUEST_CONTEXT, + }; + + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, async () => { + expect(SentryCore.getIsolationScope().getScopeData().user?.ip_address).toBeUndefined(); + return nextResult; + }); + }); + it("doesn't attach a client IP if `trackClientIp=true` when handling static page requests", async () => { const middleware = handleRequest({ trackClientIp: true });