feat(cloudflare): Add allow list to enableRpcTracePropagation

Author: JPeer264

dev-packages/cloudflare-integration-tests/suites/tracing/propagation/worker-workerentrypoint-rpc/index-sub-worker.ts

@@ -67,4 +67,12 @@ export const NoPropagationEntrypoint = Sentry.withSentry(
   MySubWorkerEntrypointBase,
 );
 
+// Deliberately not wrapped with Sentry: nothing strips a trailing RPC metadata argument here, so
+// this is what a caller corrupts if it propagates to a receiver it has no guarantees about.
+export class UninstrumentedEntrypoint extends WorkerEntrypoint<Env> {
+  get(key: string): { argumentCount: number; key: string } {
+    return { argumentCount: arguments.length, key };
+  }
+}
+
 export default BindingEntrypoint;

dev-packages/cloudflare-integration-tests/suites/tracing/propagation/worker-workerentrypoint-rpc/index.ts

@@ -11,6 +11,9 @@ interface Env {
   SUB_WORKER_NO_PROPAGATION: Fetcher & {
     get(key: string): Promise<{ argumentCount: number; key: string }>;
   };
+  SUB_WORKER_UNINSTRUMENTED: Fetcher & {
+    get(key: string): Promise<{ argumentCount: number; key: string }>;
+  };
 }
 
 class LoopbackEntrypointBase extends WorkerEntrypoint<Env> {
@@ -29,7 +32,9 @@ export default Sentry.withSentry(
     dsn: env.SENTRY_DSN,
     traceLifecycle: 'static',
     tracesSampleRate: 1.0,
-    enableRpcTracePropagation: true,
+    // Allowlisted by binding name: `SUB_WORKER_UNINSTRUMENTED` is deliberately left out, since its
+    // receiver has no Sentry to strip a trailing metadata argument.
+    enableRpcTracePropagation: ['SUB_WORKER', 'SUB_WORKER_NO_PROPAGATION'],
   }),
   {
     async fetch(request, env, ctx) {
@@ -61,6 +66,10 @@ export default Sentry.withSentry(
         }
       }
 
+      if (url.pathname === '/call-uninstrumented-rpc') {
+        return Response.json(await env.SUB_WORKER_UNINSTRUMENTED.get('uninstrumented-key'));
+      }
+
       if (url.pathname === '/call-entrypoint-rpc-no-propagation') {
         const result = await env.SUB_WORKER_NO_PROPAGATION.get('no-prop-key');
         return Response.json(result);

dev-packages/cloudflare-integration-tests/suites/tracing/propagation/worker-workerentrypoint-rpc/test.ts

@@ -244,6 +244,23 @@ it('captures errors thrown by custom WorkerEntrypoint RPC methods', async ({ sig
   await runner.completed();
 });
 
+// Regression test for https://github.com/getsentry/sentry-javascript/issues/23233: a receiver that
+// is not instrumented never strips Sentry's trailing metadata argument, so a caller must only
+// propagate to bindings it was explicitly told about.
+it('does not change RPC method arguments for a binding left off the allowlist', async ({ signal }) => {
+  const runner = createRunner(__dirname)
+    .expect(envelope => {
+      const transactionEvent = envelope[1]?.[0]?.[1] as Event;
+      expect(transactionEvent.transaction).toBe('GET /call-uninstrumented-rpc');
+    })
+    .start(signal);
+
+  const response = await runner.makeRequest<{ argumentCount: number; key: string }>('get', '/call-uninstrumented-rpc');
+  expect(response).toEqual({ argumentCount: 1, key: 'uninstrumented-key' });
+
+  await runner.completed();
+});
+
 it('does not inject RPC trace metadata into receiver calls when enableRpcTracePropagation is disabled', async ({
   signal,
 }) => {

dev-packages/cloudflare-integration-tests/suites/tracing/propagation/worker-workerentrypoint-rpc/wrangler.jsonc

@@ -14,5 +14,10 @@
       "service": "cloudflare-worker-workerentrypoint-rpc-sub",
       "entrypoint": "NoPropagationEntrypoint",
     },
+    {
+      "binding": "SUB_WORKER_UNINSTRUMENTED",
+      "service": "cloudflare-worker-workerentrypoint-rpc-sub",
+      "entrypoint": "UninstrumentedEntrypoint",
+    },
   ],
 }

packages/cloudflare/src/client.ts

@@ -200,6 +200,16 @@ interface BaseCloudflareOptions {
    * - Create spans for each RPC method invocation
    * - Capture errors thrown by RPC methods
    *
+   * Accepts:
+   * - `false` (default) - never propagate.
+   * - `true` - propagate on every Durable Object and Service Binding.
+   * - `Array<string | RegExp>` - propagate on only the bindings whose names match the given strings or regular expressions.
+   *
+   *
+   * Prefer the array form when you call bindings whose receiver may not run Sentry. RPC calls carry
+   * trace context as a trailing argument, and only a Sentry-instrumented receiver strips it again —
+   * anywhere else it arrives as a real argument and changes the method's signature.
+   *
    * **Important:** This option should be enabled on **both sides** for full trace propagation.
    *
    * @default false
@@ -229,8 +239,19 @@ interface BaseCloudflareOptions {
    *   MyEntrypointBase,
    * );
    * ```
+   * @example
+   * ```ts
+   * // Only propagate to `env.ORDERS` and every `env.SVC_*` binding
+   * export default Sentry.withSentry(
+   *   (env) => ({
+   *     dsn: env.SENTRY_DSN,
+   *     enableRpcTracePropagation: ['ORDERS', /^SVC_/],
+   *   }),
+   *   handler,
+   * );
+   * ```
    */
-  enableRpcTracePropagation?: boolean;
+  enableRpcTracePropagation?: boolean | Array<string | RegExp>;
 
   /**
    * Table names that should stay instrumented even though they match the reserved `cf_` prefix used

packages/cloudflare/src/instrumentations/worker/instrumentEnv.ts

@@ -12,6 +12,7 @@ import {
 } from '../../utils/isBinding';
 import { instrumentD1 } from './instrumentD1';
 import { appendRpcMeta } from '../../utils/rpcMeta';
+import { createRpcPropagationResolver } from '../../utils/rpcPropagation';
 import { instrumentDurableObjectNamespace, STUB_NON_RPC_METHODS } from '../instrumentDurableObjectNamespace';
 import { instrumentFetcher } from './instrumentFetcher';
 import { instrumentQueueProducer } from './instrumentQueueProducer';
@@ -44,6 +45,8 @@ export function instrumentEnv<Env extends Record<string, unknown>>(env: Env, opt
     return env;
   }
 
+  const shouldPropagateRpcTrace = createRpcPropagationResolver(options);
+
   return new Proxy(env, {
     get(target, prop, receiver) {
       const item = Reflect.get(target, prop, receiver);
@@ -91,7 +94,7 @@ export function instrumentEnv<Env extends Record<string, unknown>>(env: Env, opt
         return instrumented;
       }
 
-      if (!options?.enableRpcTracePropagation) {
+      if (!shouldPropagateRpcTrace(String(prop))) {
         return item;
       }
 

packages/cloudflare/src/utils/rpcPropagation.ts

@@ -0,0 +1,25 @@
+import { stringMatchesSomePattern } from '@sentry/core';
+import type { CloudflareOptions } from '../client';
+
+const PROPAGATE_TO_NONE = () => false;
+const PROPAGATE_TO_ALL = () => true;
+
+/**
+ * Builds the per-binding predicate that decides whether a binding takes part in RPC trace
+ * propagation.
+ */
+export function createRpcPropagationResolver(options: CloudflareOptions | undefined): (bindingName: string) => boolean {
+  const value: CloudflareOptions['enableRpcTracePropagation'] | undefined = options?.enableRpcTracePropagation;
+
+  if (value === true) {
+    return PROPAGATE_TO_ALL;
+  }
+
+  if (!Array.isArray(value) || !value.length) {
+    return PROPAGATE_TO_NONE;
+  }
+
+  // Strings must match a binding name exactly, without this, an entry of `DB` would also enable
+  // propagation for a binding named `MY_DB`. Regular expressions still give pattern matching.
+  return (bindingName: string) => stringMatchesSomePattern(bindingName, value, true);
+}

packages/cloudflare/test/instrumentations/instrumentEnv.test.ts

@@ -89,6 +89,34 @@ describe('instrumentEnv', () => {
     expect(instrumentDurableObjectNamespace).not.toHaveBeenCalled();
   });
 
+  it('instruments only the DurableObjectNamespace bindings named in the allowlist', () => {
+    const allowed = { idFromName: vi.fn(), idFromString: vi.fn(), get: vi.fn(), newUniqueId: vi.fn() };
+    const denied = { idFromName: vi.fn(), idFromString: vi.fn(), get: vi.fn(), newUniqueId: vi.fn() };
+    const env = { COUNTER: allowed, SESSIONS: denied };
+    const instrumented = instrumentEnv(env, { enableRpcTracePropagation: ['COUNTER'] });
+
+    expect((instrumented.COUNTER as any).__instrumented).toBe(true);
+    expect(instrumented.SESSIONS).toBe(denied);
+    expect(instrumentDurableObjectNamespace).toHaveBeenCalledTimes(1);
+    expect(instrumentDurableObjectNamespace).toHaveBeenCalledWith(allowed);
+  });
+
+  it('matches allowlisted binding names exactly rather than as substrings', () => {
+    const doNamespace = { idFromName: vi.fn(), idFromString: vi.fn(), get: vi.fn(), newUniqueId: vi.fn() };
+    const env = { MY_COUNTER: doNamespace };
+    const instrumented = instrumentEnv(env, { enableRpcTracePropagation: ['COUNTER'] });
+
+    expect(instrumented.MY_COUNTER).toBe(doNamespace);
+  });
+
+  it('supports regular expressions in the allowlist', () => {
+    const doNamespace = { idFromName: vi.fn(), idFromString: vi.fn(), get: vi.fn(), newUniqueId: vi.fn() };
+    const env = { SVC_ORDERS: doNamespace };
+    const instrumented = instrumentEnv(env, { enableRpcTracePropagation: [/^SVC_/] });
+
+    expect((instrumented.SVC_ORDERS as any).__instrumented).toBe(true);
+  });
+
   it('detects and instruments DurableObjectNamespace bindings when enableRpcTracePropagation is enabled', () => {
     const doNamespace = {
       idFromName: vi.fn(),
@@ -486,5 +514,44 @@ describe('instrumentEnv', () => {
 
       expect(rpcMethod).toHaveBeenCalledWith('arg1');
     });
+
+    // A receiver without Sentry never strips the trailing metadata argument, so a caller has to be
+    // able to limit propagation to the bindings it knows are instrumented.
+    // See https://github.com/getsentry/sentry-javascript/issues/23233.
+    it('injects meta only into JSRPC calls on allowlisted bindings', () => {
+      vi.spyOn(SentryCore, 'getTraceData').mockReturnValue({
+        'sentry-trace': '12345678901234567890123456789012-1234567890123456-1',
+        baggage: 'sentry-environment=production',
+      });
+
+      const allowedMethod = vi.fn();
+      const deniedMethod = vi.fn();
+      const createJsrpcBinding = (rpcMethod: ReturnType<typeof vi.fn>) =>
+        new Proxy(
+          { fetch: vi.fn(), myRpcMethod: rpcMethod },
+          {
+            get(target, prop) {
+              if (prop in target) {
+                return Reflect.get(target, prop);
+              }
+              return () => {};
+            },
+          },
+        );
+
+      const env = { ORDERS: createJsrpcBinding(allowedMethod), EXTERNAL: createJsrpcBinding(deniedMethod) };
+      const instrumented = instrumentEnv(env, { enableRpcTracePropagation: ['ORDERS'] });
+
+      instrumented.ORDERS.myRpcMethod('first');
+      instrumented.EXTERNAL.myRpcMethod('first');
+
+      expect(allowedMethod).toHaveBeenCalledWith('first', {
+        __sentry_rpc_meta__: {
+          'sentry-trace': '12345678901234567890123456789012-1234567890123456-1',
+          baggage: 'sentry-environment=production',
+        },
+      });
+      expect(deniedMethod).toHaveBeenCalledWith('first');
+    });
   });
 });

packages/cloudflare/test/utils/rpcPropagation.test.ts

@@ -0,0 +1,61 @@
+import { describe, expect, it } from 'vitest';
+import { createRpcPropagationResolver } from '../../src/utils/rpcPropagation';
+
+describe('createRpcPropagationResolver', () => {
+  it('propagates to nothing when no options are available', () => {
+    const shouldPropagate = createRpcPropagationResolver(undefined);
+
+    expect(shouldPropagate('MY_DO')).toBe(false);
+  });
+
+  it('propagates to nothing when the option is unset', () => {
+    const shouldPropagate = createRpcPropagationResolver({ enableRpcTracePropagation: undefined });
+
+    expect(shouldPropagate('MY_DO')).toBe(false);
+    expect(shouldPropagate('EXTERNAL')).toBe(false);
+  });
+
+  it('propagates to nothing when the option is `false`', () => {
+    const shouldPropagate = createRpcPropagationResolver({ enableRpcTracePropagation: false });
+
+    expect(shouldPropagate('MY_DO')).toBe(false);
+  });
+
+  it('propagates to every binding when the option is `true`', () => {
+    const shouldPropagate = createRpcPropagationResolver({ enableRpcTracePropagation: true });
+
+    expect(shouldPropagate('MY_DO')).toBe(true);
+    expect(shouldPropagate('EXTERNAL')).toBe(true);
+  });
+
+  it('propagates only to allowlisted binding names', () => {
+    const shouldPropagate = createRpcPropagationResolver({ enableRpcTracePropagation: ['MY_DO', 'EXTERNAL'] });
+
+    expect(shouldPropagate('MY_DO')).toBe(true);
+    expect(shouldPropagate('EXTERNAL')).toBe(true);
+    expect(shouldPropagate('OTHER')).toBe(false);
+  });
+
+  it('propagates to nothing for an empty allowlist', () => {
+    const shouldPropagate = createRpcPropagationResolver({ enableRpcTracePropagation: [] });
+
+    expect(shouldPropagate('MY_DO')).toBe(false);
+  });
+
+  it('matches binding names exactly, never as a substring', () => {
+    const shouldPropagate = createRpcPropagationResolver({ enableRpcTracePropagation: ['DB'] });
+
+    expect(shouldPropagate('DB')).toBe(true);
+    expect(shouldPropagate('MY_DB')).toBe(false);
+    expect(shouldPropagate('DB_REPLICA')).toBe(false);
+  });
+
+  it('supports regular expressions for pattern matching', () => {
+    const shouldPropagate = createRpcPropagationResolver({ enableRpcTracePropagation: [/^SVC_/] });
+
+    expect(shouldPropagate('SVC_ORDERS')).toBe(true);
+    expect(shouldPropagate('SVC_USERS')).toBe(true);
+    expect(shouldPropagate('ORDERS')).toBe(false);
+    expect(shouldPropagate('PREFIXED_SVC_ORDERS')).toBe(false);
+  });
+});