test(astro): Cover the trackClientIp fallback with userInfo enabled

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: s1gr1d

dev-packages/node-integration-tests/suites/express/tracing/test.ts

@@ -305,7 +305,8 @@ describe('express tracing', () => {
                   'user-agent': expect.stringContaining(''),
                   'content-type': 'text/plain',
                 },
-                data: 'some plain text',
+                // A plain-text body has no keys the denylist can match, so it is filtered wholesale.
+                data: '[Filtered]',
               },
             },
           })
@@ -330,7 +331,7 @@ describe('express tracing', () => {
                   'user-agent': expect.stringContaining(''),
                   'content-type': 'application/octet-stream',
                 },
-                data: 'some plain text in buffer',
+                data: '[Filtered]',
               },
             },
           })
@@ -355,8 +356,7 @@ describe('express tracing', () => {
                   'user-agent': expect.stringContaining(''),
                   'content-type': 'application/octet-stream',
                 },
-                // This is some non-ascii string representation
-                data: expect.any(String),
+                data: '[Filtered]',
               },
             },
           })

dev-packages/node-integration-tests/suites/express/without-tracing/test.ts

@@ -76,7 +76,8 @@ describe('express without tracing', () => {
                   'user-agent': expect.stringContaining(''),
                   'content-type': 'text/plain',
                 },
-                data: 'some plain text',
+                // A plain-text body has no keys the denylist can match, so it is filtered wholesale.
+                data: '[Filtered]',
               },
             },
           })
@@ -103,7 +104,7 @@ describe('express without tracing', () => {
                   'user-agent': expect.stringContaining(''),
                   'content-type': 'application/octet-stream',
                 },
-                data: 'some plain text in buffer',
+                data: '[Filtered]',
               },
             },
           })
@@ -128,8 +129,7 @@ describe('express without tracing', () => {
                   'user-agent': expect.stringContaining(''),
                   'content-type': 'application/octet-stream',
                 },
-                // This is some non-ascii string representation
-                data: expect.any(String),
+                data: '[Filtered]',
               },
             },
           })

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<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 {
@@ -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">
@@ -308,8 +314,22 @@ describe('sentryMiddleware', () => {
       });
     });
 
-    it('follows `dataCollection.userInfo` when `trackClientIp` is not set', async () => {
-      // The shared client mock resolves `userInfo` to `false`.
+    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,
@@ -322,7 +342,8 @@ describe('sentryMiddleware', () => {
       });
     });
 
-    it('does not attach a client IP if `trackClientIp=false`', async () => {
+    it('lets `trackClientIp=false` win over `dataCollection.userInfo`', async () => {
+      mockClientWith({ userInfo: true });
       const middleware = handleRequest({ trackClientIp: false });
       const ctx = {
         ...DYNAMIC_REQUEST_CONTEXT,

packages/browser/src/integrations/graphqlClient.ts

@@ -52,6 +52,16 @@ interface GraphQLOperation {
 
 const INTEGRATION_NAME = 'GraphQLClient' as const;
 
+// GraphQL literal argument values can carry user data, so they are replaced before the document is
+// attached — matching what the server-side GraphQL instrumentation does with the parsed AST.
+// Only Int/Float/String/BlockString literals are replaced; names, enums and booleans are structural.
+const GRAPHQL_LITERAL_RE = /"""[\s\S]*?"""|"(?:[^"\\\n]|\\.)*"|-?\b\d+(?:\.\d+)?(?:[eE][+-]?\d+)?\b/g;
+
+/** Replaces every literal argument value in a raw GraphQL document with a placeholder. */
+export function _redactGraphqlDocument(document: string): string {
+  return document.replace(GRAPHQL_LITERAL_RE, match => (match.startsWith('"') ? '"*"' : '*'));
+}
+
 const _graphqlClientIntegration = ((options: GraphQLClientOptions) => {
   return {
     name: INTEGRATION_NAME,
@@ -103,7 +113,7 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
 
         // Handle standard requests - capture the query document when enabled via dataCollection (default true)
         if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
-          span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
+          span.setAttribute(GRAPHQL_DOCUMENT, _redactGraphqlDocument(graphqlBody.query));
         }
 
         // Handle persisted operations - capture hash for debugging
@@ -140,7 +150,7 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient
           data['graphql.operation'] = operationInfo;
 
           if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
-            data[GRAPHQL_DOCUMENT] = graphqlBody.query;
+            data[GRAPHQL_DOCUMENT] = _redactGraphqlDocument(graphqlBody.query);
           }
 
           if (isPersistedRequest(graphqlBody)) {

packages/browser/test/integrations/graphqlClient.test.ts

@@ -10,12 +10,33 @@ import { URL_FULL } from '@sentry/conventions/attributes';
 import { describe, expect, test } from 'vitest';
 import {
   _getGraphQLOperation,
+  _redactGraphqlDocument,
   getGraphQLRequestPayload,
   getRequestPayloadXhrOrFetch,
   graphqlClientIntegration,
   parseGraphQLQuery,
 } from '../../src/integrations/graphqlClient';
 
+describe('_redactGraphqlDocument', () => {
+  test('replaces string and numeric literal arguments', () => {
+    expect(_redactGraphqlDocument('query { user(email: "jane@example.com", age: 42) { name } }')).toBe(
+      'query { user(email: "*", age: *) { name } }',
+    );
+  });
+
+  test('replaces block string literals', () => {
+    expect(_redactGraphqlDocument('mutation { post(body: """secret\nlines""") { id } }')).toBe(
+      'mutation { post(body: "*") { id } }',
+    );
+  });
+
+  test('leaves documents without literals untouched', () => {
+    const document = 'query Test($id: ID!) {\n  people {\n    name\n  }\n}';
+
+    expect(_redactGraphqlDocument(document)).toBe(document);
+  });
+});
+
 describe('GraphqlClient', () => {
   describe('parseGraphQLQuery', () => {
     const queryOne = `query Test {

packages/cloudflare/test/request.test.ts

@@ -343,7 +343,7 @@ describe('withSentry', () => {
           request: new Request('https://example.com', {
             method: 'POST',
             headers: { 'content-type': 'application/json' },
-            body: JSON.stringify({ key: 'value' }),
+            body: JSON.stringify({ colour: 'blue' }),
           }),
           context,
         },
@@ -353,7 +353,7 @@ describe('withSentry', () => {
         },
       );
 
-      expect(sentryEvent.sdkProcessingMetadata?.normalizedRequest?.data).toEqual(JSON.stringify({ key: 'value' }));
+      expect(sentryEvent.sdkProcessingMetadata?.normalizedRequest?.data).toEqual(JSON.stringify({ colour: 'blue' }));
     });
 
     test('does not capture cookies when dataCollection.cookies is disabled', async () => {

packages/core/src/integrations/http/patch-request-to-capture-body.ts

@@ -2,6 +2,7 @@ import type { Scope } from '../../scope';
 import { debug } from '../../utils/debug-logger';
 import { DEBUG_BUILD } from '../../debug-build';
 import type { HttpIncomingMessage } from './types';
+import { filterCollectedHttpBodyString } from '../../utils/data-collection/filterHttpBody';
 import { getMaxBodyByteLength, type MaxRequestBodySize } from '../../utils/request';
 
 /**
@@ -92,7 +93,9 @@ export function patchRequestToCaptureBody(
 
     req.on('end', () => {
       try {
-        const body = Buffer.concat(chunks).toString('utf-8');
+        // Filtered before truncation: truncating first would make a parseable body unparseable, and
+        // an unparseable body has to be dropped wholesale.
+        const body = filterCollectedHttpBodyString(Buffer.concat(chunks).toString('utf-8'));
         if (body) {
           // Using Buffer.byteLength here, because the body may contain characters that are not 1 byte long
           const bodyByteLength = Buffer.byteLength(body, 'utf-8');

packages/core/src/utils/data-collection/filterHttpBody.ts

@@ -0,0 +1,67 @@
+import { isPlainObject } from '../is';
+import { FILTERED_VALUE } from './filtering-snippets';
+import { shouldFilterDataKey } from './filterKeyValueData';
+import { filterQueryParams } from './filterQueryParams';
+
+/** `key=value&key2=value2` — the only non-JSON body shape with keys the denylist can match. */
+const FORM_BODY_RE = /^[^=&]+=[^&]*(?:&[^=&]+=[^&]*)*$/;
+
+/**
+ * Scrubs an HTTP body the SDK collected itself, before it is attached as `request.data` /
+ * `http.request.body.data`.
+ *
+ * A body that can be parsed into key-value pairs keeps its shape, with values for keys matching the
+ * sensitive denylist replaced. A body that cannot be parsed has no keys to match against, so the
+ * spec requires the whole value to be replaced rather than sent raw.
+ *
+ * Bodies a user attaches themselves never pass through here — `dataCollection` only gates data the
+ * SDK collects automatically.
+ */
+export function filterCollectedHttpBody(body: unknown): unknown {
+  if (body == null) {
+    return body;
+  }
+
+  if (typeof body === 'string') {
+    return filterCollectedHttpBodyString(body);
+  }
+
+  // Anything that is not a key-value structure (a `Buffer`, a stream, a number) has no keys to match
+  // against, so it is filtered wholesale.
+  return isPlainObject(body) || Array.isArray(body) ? filterBodyValue(body) : FILTERED_VALUE;
+}
+
+/**
+ * String-only variant of {@link filterCollectedHttpBody}, for body capture sites that filter before
+ * truncating: truncating first would make a parseable body unparseable, forcing it to be dropped.
+ */
+export function filterCollectedHttpBodyString(body: string): string {
+  try {
+    const json: unknown = JSON.parse(body);
+    // A bare JSON scalar (`"hi"`, `42`) has no keys to match against, so it counts as unparseable.
+    if (typeof json === 'object' && json !== null) {
+      return JSON.stringify(filterBodyValue(json));
+    }
+  } catch {
+    // Not JSON — fall through to the form-encoded attempt below.
+  }
+
+  // Reuses the query-param filter so the body's original encoding is preserved byte-for-byte.
+  return (FORM_BODY_RE.test(body) && filterQueryParams(body, true)) || FILTERED_VALUE;
+}
+
+function filterBodyValue(value: unknown): unknown {
+  if (Array.isArray(value)) {
+    return value.map(filterBodyValue);
+  }
+
+  if (!isPlainObject(value)) {
+    return value;
+  }
+
+  const result: Record<string, unknown> = {};
+  for (const [key, nested] of Object.entries(value)) {
+    result[key] = shouldFilterDataKey(key, true) ? FILTERED_VALUE : filterBodyValue(nested);
+  }
+  return result;
+}

packages/core/src/utils/request.ts

@@ -7,6 +7,7 @@ import type { RequestEventData } from '../types/request';
 import type { WebFetchHeaders, WebFetchRequest } from '../types/webfetchapi';
 import { debug } from './debug-logger';
 import { FILTERED_VALUE, SENSITIVE_COOKIE_NAME_SNIPPETS } from './data-collection/filtering-snippets';
+import { filterCollectedHttpBody, filterCollectedHttpBodyString } from './data-collection/filterHttpBody';
 import { filterKeyValueData } from './data-collection/filterKeyValueData';
 import { safeUnref } from './timer';
 import { getUrlQuery } from './url';
@@ -158,17 +159,21 @@ export async function captureBodyFromWinterCGRequest(
       safeUnref(setTimeout(() => resolve(null), 2000));
     });
 
-    const body = await Promise.race([bodyPromise, timeoutPromise]);
+    const rawBody = await Promise.race([bodyPromise, timeoutPromise]);
 
-    if (body === null) {
+    if (rawBody === null) {
       DEBUG_BUILD && debug.log('Timeout reading request body');
       return;
     }
 
-    if (!body) {
+    if (!rawBody) {
       return;
     }
 
+    // Filtered before truncation: truncating first would make a parseable body unparseable, and an
+    // unparseable body has to be dropped wholesale.
+    const body = filterCollectedHttpBodyString(rawBody);
+
     // Using TextEncoder to get byte length for UTF-8 strings
     const encoder = new TextEncoder();
     const bytes = encoder.encode(body);
@@ -227,7 +232,8 @@ export function httpRequestToRequestData(request: {
 
   // This is non-standard, but may be sometimes set
   // It may be overwritten later by our own body handling
-  const data = (request as PolymorphicRequest).body || undefined;
+  const body = (request as PolymorphicRequest).body || undefined;
+  const data = body === undefined ? undefined : filterCollectedHttpBody(body);
 
   // This is non-standard, but may be set on e.g. Next.js or Express requests
   const cookies = (request as PolymorphicRequest).cookies;