Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions packages/astro/src/server/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -78,10 +78,7 @@ type AstroLocalsWithSentry = Record<string, unknown> & {
};

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
Expand Down Expand Up @@ -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 });
}

Expand Down
84 changes: 66 additions & 18 deletions packages/astro/test/server/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof DATA_COLLECTION_DEFAULTS>): void {
vi.spyOn(SentryNode, 'getClient').mockImplementation(
() =>
({
getOptions: () => ({}),
getDataCollectionOptions: () => ({ ...DATA_COLLECTION_DEFAULTS, ...dataCollection }),
}) as unknown as Client,
);
}

beforeEach(() => {
vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation(() => {
return {
Expand All @@ -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(
() => `
<meta name="sentry-trace" content="123">
Expand Down Expand Up @@ -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;
});
});
Comment thread
s1gr1d marked this conversation as resolved.

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 });

Expand Down
Loading