feat(remix)!: Do not double gate action form data capture

MIGRATION.md

@@ -166,6 +166,29 @@ 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.
 
+#### Remix action form data
+
+`captureActionFormDataKeys` is an integration-level override, so it no longer requires
+`dataCollection.httpBodies` to also include `'incomingRequest'`:
+
+```js
+// v10 — both were required
+Sentry.init({
+  captureActionFormDataKeys: { username: true },
+  dataCollection: { httpBodies: ['incomingRequest'] },
+});
+
+// v11 — the option opts in on its own
+Sentry.init({
+  captureActionFormDataKeys: { username: true },
+});
+```
+
+If `captureActionFormDataKeys` is not set, all form fields are captured when
+`dataCollection.httpBodies` includes `'incomingRequest'` (the v11 default). Values whose field name
+looks sensitive (`password`, `token`, …) are replaced with `[Filtered]`, including explicitly
+allowlisted ones.
+
 ### Channel-based instrumentation is the default
 
 Affected SDKs: `@sentry/node` and all dependents.

dev-packages/e2e-tests/test-applications/create-remix-app-express/instrument.mjs

@@ -7,7 +7,6 @@ Sentry.init({
   environment: 'qa', // dynamic sampling bias to keep transactions
   dsn: process.env.E2E_TEST_DSN,
   tunnel: 'http://localhost:3031/', // proxy server
-  dataCollection: { httpBodies: ['incomingRequest'] }, // Testing the FormData
   captureActionFormDataKeys: {
     file: true,
     text: true,

packages/remix/src/server/errors.ts

@@ -11,7 +11,7 @@ import {
   winterCGRequestToRequestData,
 } from '@sentry/core';
 import { DEBUG_BUILD } from '../utils/debug-build';
-import type { RemixOptions } from '../utils/remixOptions';
+import { resolveFormDataCapture } from '../utils/formData';
 import { storeFormDataKeys } from '../utils/utils';
 import { extractData, isResponse } from '../utils/vendor/response';
 
@@ -93,14 +93,10 @@ export async function errorHandleDataFunction(
   return handleCallbackErrors(
     async () => {
       if (name === 'action' && span) {
-        const client = getClient();
-        const options = client?.getOptions() as RemixOptions | undefined;
-
-        if (
-          client?.getDataCollectionOptions().httpBodies.includes('incomingRequest') &&
-          options?.captureActionFormDataKeys
-        ) {
-          await storeFormDataKeys(args, span, options.captureActionFormDataKeys);
+        const formDataCapture = resolveFormDataCapture(getClient());
+
+        if (formDataCapture) {
+          await storeFormDataKeys(args, span, formDataCapture);
         }
       }
 

packages/remix/src/server/integrations/RemixIntegration.ts

@@ -1,21 +1,15 @@
 import type { IntegrationFn } from '@sentry/core';
 import { defineIntegration, getClient } from '@sentry/core';
 import { instrumentRemix } from './tracing-channel';
-import type { RemixOptions } from '../../utils/remixOptions';
+import { resolveFormDataCapture } from '../../utils/formData';
 
 const INTEGRATION_NAME = 'Remix' as const;
 
 const _remixIntegration = (() => {
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
-      const client = getClient();
-      const options = client?.getOptions() as RemixOptions | undefined;
-      const actionFormDataAttributes = client?.getDataCollectionOptions().httpBodies.includes('incomingRequest')
-        ? options?.captureActionFormDataKeys
-        : undefined;
-
-      instrumentRemix(actionFormDataAttributes);
+      instrumentRemix(resolveFormDataCapture(getClient()));
     },
   };
 }) satisfies IntegrationFn;

packages/remix/src/server/integrations/tracing-channel.ts

@@ -23,6 +23,8 @@ import {
   SENTRY_KIND,
 } from '@sentry/conventions/attributes';
 import { remixChannels } from '@sentry/server-utils/orchestrion';
+import type { FormDataCapture } from '../../utils/formData';
+import { applyFormDataAttributes } from '../../utils/formData';
 
 const ORIGIN = 'auto.http.remix';
 
@@ -191,7 +193,7 @@ function subscribeCallRouteLoader(): void {
   );
 }
 
-function subscribeCallRouteAction(actionFormDataAttributes: Record<string, string | boolean> | undefined): void {
+function subscribeCallRouteAction(formDataCapture: FormDataCapture | undefined): void {
   bindTracingChannelToSpan<ActionChannelContext>(
     diagnosticsChannel.tracingChannel(remixChannels.REMIX_CALL_ROUTE_ACTION),
     data => {
@@ -201,7 +203,7 @@ function subscribeCallRouteAction(actionFormDataAttributes: Record<string, strin
       // delay the action promise, so reading only after it settles would race the parent
       // `requestHandler` span flushing the transaction. Reading here means the promise is (virtually
       // always) already resolved by `asyncEnd`, so ending the span costs a single microtask.
-      if (actionFormDataAttributes && params.request) {
+      if (formDataCapture && params.request) {
         const formData = params.request.clone().formData();
         // Attach a handler so an unconsumed rejection (e.g. the action errored) isn't unhandled.
         formData.catch(() => undefined);
@@ -226,12 +228,12 @@ function subscribeCallRouteAction(actionFormDataAttributes: Record<string, strin
       // capture isn't configured, let the helper end the span normally.
       deferSpanEnd: ({ span, data, end }) => {
         const formData = data._sentryFormData;
-        if (!actionFormDataAttributes || !formData || 'error' in data) {
+        if (!formDataCapture || !formData || 'error' in data) {
           return false;
         }
 
         formData
-          .then(resolved => applyFormDataAttributes(span, resolved, actionFormDataAttributes))
+          .then(resolved => applyFormDataAttributes(span, resolved, formDataCapture, 'formData.'))
           // Silently continue on any error. Typically happens because the action body cannot be
           // processed into FormData, in which case we should just continue.
           .catch(() => undefined)
@@ -243,21 +245,7 @@ function subscribeCallRouteAction(actionFormDataAttributes: Record<string, strin
   );
 }
 
-function applyFormDataAttributes(
-  span: Span,
-  formData: FormData,
-  actionFormDataAttributes: Record<string, string | boolean>,
-): void {
-  formData.forEach((value, key) => {
-    const mapped = actionFormDataAttributes[key];
-    if (mapped && typeof value === 'string') {
-      const keyName = mapped === true ? key : mapped;
-      span.setAttribute(`formData.${keyName}`, value);
-    }
-  });
-}
-
-export function instrumentRemix(actionFormDataAttributes: Record<string, string | boolean> | undefined): void {
+export function instrumentRemix(formDataCapture: FormDataCapture | undefined): void {
   // `tracingChannel` is unavailable before Node 18.19, so do nothing in that case.
   if (!diagnosticsChannel.tracingChannel) {
     return;
@@ -267,8 +255,8 @@ export function instrumentRemix(actionFormDataAttributes: Record<string, string
     subscribeRequestHandler();
     subscribeMatchServerRoutes();
     subscribeCallRouteLoader();
-    // Always instrument actions; `actionFormDataAttributes` only gates the optional form-data
+    // Always instrument actions; `formDataCapture` only gates the optional form-data
     // attribute extraction, not whether ACTION spans are created.
-    subscribeCallRouteAction(actionFormDataAttributes);
+    subscribeCallRouteAction(formDataCapture);
   });
 }

packages/remix/src/utils/formData.ts

@@ -0,0 +1,64 @@
+import type { Client, Span } from '@sentry/core';
+import { _INTERNAL_filterKeyValueData } from '@sentry/core';
+import type { RemixOptions } from './remixOptions';
+
+/**
+ * Resolved form-data capture config. `keys` is `undefined` when every field should be captured.
+ */
+export interface FormDataCapture {
+  keys: Record<string, string | boolean> | undefined;
+}
+
+/**
+ * Resolves whether (and which) `action` form data fields to capture.
+ *
+ * `captureActionFormDataKeys` is an integration-level option, so it takes precedence over
+ * `dataCollection.httpBodies` rather than being gated by it. Without it, `httpBodies` decides,
+ * in which case there is no configured key list and all fields are captured.
+ */
+export function resolveFormDataCapture(client: Client | undefined): FormDataCapture | undefined {
+  if (!client) {
+    return undefined;
+  }
+
+  const keys = (client.getOptions() as RemixOptions).captureActionFormDataKeys;
+  if (keys) {
+    return { keys };
+  }
+
+  return client.getDataCollectionOptions().httpBodies.includes('incomingRequest') ? { keys: undefined } : undefined;
+}
+
+/**
+ * Sets form-data span attributes under `attributePrefix`, honoring the configured key list and
+ * renames. Values are run through the shared sensitive-key filter, so an allowlisted `password`
+ * still reports as `[Filtered]` rather than in the clear.
+ */
+export function applyFormDataAttributes(
+  span: Span,
+  formData: FormData,
+  { keys }: FormDataCapture,
+  attributePrefix: string,
+): void {
+  const collected: Record<string, string> = {};
+
+  formData.forEach((value, key) => {
+    const mapped = keys ? keys[key] : true;
+    if (!mapped) {
+      return;
+    }
+
+    // Renames apply to the reported attribute name, but filtering must run against the original
+    // field name — otherwise renaming `password` to `pw` would defeat the denylist.
+    const attributeName = typeof mapped === 'string' ? mapped : key;
+    // File uploads report the filename rather than the contents.
+    const reported = typeof value === 'string' ? value : value.name || '[non-string value]';
+    const filtered = _INTERNAL_filterKeyValueData({ [key]: reported }, true);
+
+    collected[attributeName] = filtered[key] as string;
+  });
+
+  for (const [key, value] of Object.entries(collected)) {
+    span.setAttribute(`${attributePrefix}${key}`, value);
+  }
+}

packages/remix/src/utils/remixOptions.ts

@@ -4,18 +4,21 @@ import type { BrowserOptions } from '@sentry/react';
 
 export type RemixOptions = (Options | BrowserOptions | NodeOptions) & {
   /**
-   * Controls which `action` form data fields are captured and attached to spans/errors.
+   * Controls which `action` form data fields are captured and attached to spans/errors, optionally
+   * renaming them (`{ username: 'user' }` reports `username` as `user`).
    *
-   * This option only takes effect when incoming request bodies are collected, i.e. when
-   * `dataCollection.httpBodies` includes `'incomingRequest'`:
+   * Setting this option is enough to opt into capturing the configured fields, and it takes
+   * precedence over `dataCollection.httpBodies`:
    *
    * ```js
    * Sentry.init({
    *   captureActionFormDataKeys: { username: true },
-   *   dataCollection: { httpBodies: ['incomingRequest'] },
    * });
    * ```
+   *
+   * When this option is not set, all form fields are captured if `dataCollection.httpBodies`
+   * includes `'incomingRequest'` (the default). Either way, values whose field name looks
+   * sensitive (`password`, `token`, …) are replaced with `[Filtered]`.
    */
   captureActionFormDataKeys?: Record<string, string | boolean>;
-  // TODO(v11): Remove the requirement to also set `dataCollection.httpBodies`. Setting `captureActionFormDataKeys` should be enough to opt into capturing the configured form values
 };

packages/remix/src/utils/utils.ts

@@ -3,6 +3,8 @@ import type { AgnosticRouteObject } from '@remix-run/router';
 import type { Span, TransactionSource } from '@sentry/core';
 import { debug } from '@sentry/core';
 import { DEBUG_BUILD } from './debug-build';
+import type { FormDataCapture } from './formData';
+import { applyFormDataAttributes } from './formData';
 import { matchServerRoutes } from './vendor/response';
 
 type ServerRouteManifest = ServerBuild['routes'];
@@ -13,7 +15,7 @@ type ServerRouteManifest = ServerBuild['routes'];
 export async function storeFormDataKeys(
   args: LoaderFunctionArgs | ActionFunctionArgs,
   span: Span,
-  formDataKeys?: Record<string, string | boolean>,
+  formDataCapture: FormDataCapture,
 ): Promise<void> {
   try {
     // We clone the request for Remix be able to read the FormData later.
@@ -24,20 +26,7 @@ export async function storeFormDataKeys(
     // https://remix.run/docs/en/main/utils/parse-multipart-form-data#unstable_parsemultipartformdata
     const formData = await clonedRequest.formData();
 
-    formData.forEach((value, key) => {
-      let attrKey = key;
-
-      if (formDataKeys?.[key]) {
-        if (typeof formDataKeys[key] === 'string') {
-          attrKey = formDataKeys[key];
-        }
-
-        span.setAttribute(
-          `remix.action_form_data.${attrKey}`,
-          typeof value === 'string' ? value : '[non-string value]',
-        );
-      }
-    });
+    applyFormDataAttributes(span, formData, formDataCapture, 'remix.action_form_data.');
   } catch (e) {
     DEBUG_BUILD && debug.warn('Failed to read FormData from request', e);
   }

packages/remix/test/server/errors.test.ts

@@ -9,7 +9,10 @@ vi.mock('../../src/utils/utils', () => ({
 import { storeFormDataKeys } from '../../src/utils/utils';
 import { errorHandleDataFunction } from '../../src/server/errors';
 
-function createMockClient(httpBodies: string[] = []): Client {
+function createMockClient(
+  captureActionFormDataKeys: Record<string, string | boolean> | undefined,
+  httpBodies: string[] = [],
+): Client {
   return {
     getDataCollectionOptions: () => ({
       userInfo: false,
@@ -23,9 +26,7 @@ function createMockClient(httpBodies: string[] = []): Client {
       stackFrameVariables: true,
       frameContextLines: 5,
     }),
-    getOptions: () => ({
-      captureActionFormDataKeys: { username: true },
-    }),
+    getOptions: () => ({ captureActionFormDataKeys }),
   } as unknown as Client;
 }
 
@@ -38,8 +39,34 @@ describe('errorHandleDataFunction', () => {
     vi.restoreAllMocks();
   });
 
-  it('captures form data when httpBodies includes incomingRequest', async () => {
-    vi.spyOn(core, 'getClient').mockReturnValue(createMockClient(['incomingRequest']));
+  it('captures the configured keys when captureActionFormDataKeys is set', async () => {
+    vi.spyOn(core, 'getClient').mockReturnValue(createMockClient({ username: true }, ['incomingRequest']));
+    vi.spyOn(core, 'handleCallbackErrors').mockImplementation(async fn => fn());
+
+    const mockSpan = { setAttribute: vi.fn() } as any;
+    const mockArgs = { request: new Request('http://localhost', { method: 'POST' }) } as any;
+    const origFn = vi.fn().mockResolvedValue(new Response());
+
+    await errorHandleDataFunction.call(null, origFn, 'action', mockArgs, mockSpan);
+
+    expect(storeFormDataKeys).toHaveBeenCalledWith(mockArgs, mockSpan, { keys: { username: true } });
+  });
+
+  it('captures the configured keys even when httpBodies excludes incomingRequest', async () => {
+    vi.spyOn(core, 'getClient').mockReturnValue(createMockClient({ username: true }, []));
+    vi.spyOn(core, 'handleCallbackErrors').mockImplementation(async fn => fn());
+
+    const mockSpan = { setAttribute: vi.fn() } as any;
+    const mockArgs = { request: new Request('http://localhost', { method: 'POST' }) } as any;
+    const origFn = vi.fn().mockResolvedValue(new Response());
+
+    await errorHandleDataFunction.call(null, origFn, 'action', mockArgs, mockSpan);
+
+    expect(storeFormDataKeys).toHaveBeenCalledWith(mockArgs, mockSpan, { keys: { username: true } });
+  });
+
+  it('captures all fields when only httpBodies opts in', async () => {
+    vi.spyOn(core, 'getClient').mockReturnValue(createMockClient(undefined, ['incomingRequest']));
     vi.spyOn(core, 'handleCallbackErrors').mockImplementation(async fn => fn());
 
     const mockSpan = { setAttribute: vi.fn() } as any;
@@ -48,11 +75,11 @@ describe('errorHandleDataFunction', () => {
 
     await errorHandleDataFunction.call(null, origFn, 'action', mockArgs, mockSpan);
 
-    expect(storeFormDataKeys).toHaveBeenCalledWith(mockArgs, mockSpan, { username: true });
+    expect(storeFormDataKeys).toHaveBeenCalledWith(mockArgs, mockSpan, { keys: undefined });
   });
 
-  it('does NOT capture form data when httpBodies is empty', async () => {
-    vi.spyOn(core, 'getClient').mockReturnValue(createMockClient([]));
+  it('does NOT capture form data when neither option opts in', async () => {
+    vi.spyOn(core, 'getClient').mockReturnValue(createMockClient(undefined, []));
     vi.spyOn(core, 'handleCallbackErrors').mockImplementation(async fn => fn());
 
     const mockSpan = { setAttribute: vi.fn() } as any;
@@ -65,7 +92,7 @@ describe('errorHandleDataFunction', () => {
   });
 
   it('does NOT capture form data for loader functions', async () => {
-    vi.spyOn(core, 'getClient').mockReturnValue(createMockClient(['incomingRequest']));
+    vi.spyOn(core, 'getClient').mockReturnValue(createMockClient({ username: true }, ['incomingRequest']));
     vi.spyOn(core, 'handleCallbackErrors').mockImplementation(async fn => fn());
 
     const mockSpan = { setAttribute: vi.fn() } as any;