ref(tracing): Check mutability state of incoming baggage headers

Author: Lms24

packages/nextjs/src/utils/instrumentServer.ts

@@ -13,7 +13,7 @@ import {
   fill,
   isString,
   logger,
-  parseBaggageString,
+  parseAndFreezeBaggageIfNecessary,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
 import * as domain from 'domain';
@@ -259,8 +259,8 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
             IS_DEBUG_BUILD && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
           }
 
-          const baggage =
-            nextReq.headers && isString(nextReq.headers.baggage) && parseBaggageString(nextReq.headers.baggage);
+          const rawBaggageString = nextReq.headers && isString(nextReq.headers.baggage) && nextReq.headers.baggage;
+          const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
 
           // pull off query string, if any
           const reqPath = stripUrlQueryAndFragment(nextReq.url);
@@ -273,9 +273,8 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
             {
               name: `${namePrefix}${reqPath}`,
               op: 'http.server',
-              metadata: { requestPath: reqPath },
+              metadata: { requestPath: reqPath, baggage },
               ...traceparentData,
-              ...(baggage && { metadata: { baggage: baggage } }),
             },
             // Extra context passed to the `tracesSampler` (Note: We're combining `nextReq` and `req` this way in order
             // to not break people's `tracesSampler` functions, even though the format of `nextReq` has changed (see

packages/nextjs/src/utils/withSentry.ts

@@ -6,6 +6,7 @@ import {
   isString,
   logger,
   objectify,
+  parseAndFreezeBaggageIfNecessary,
   parseBaggageString,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
@@ -55,7 +56,8 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
             IS_DEBUG_BUILD && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
           }
 
-          const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage);
+          const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage;
+          const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
 
           const url = `${req.url}`;
           // pull off query string, if any
@@ -75,7 +77,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
               name: `${reqMethod}${reqPath}`,
               op: 'http.server',
               ...traceparentData,
-              ...(baggage && { metadata: { baggage: baggage } }),
+              metadata: { baggage: baggage },
             },
             // extra context passed to the `tracesSampler`
             { request: req },

packages/node/src/handlers.ts

@@ -8,7 +8,7 @@ import {
   isString,
   logger,
   normalize,
-  parseBaggageString,
+  parseAndFreezeBaggageIfNecessary,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
 import * as cookie from 'cookie';
@@ -64,14 +64,16 @@ export function tracingHandler(): (
     // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision)
     const traceparentData =
       req.headers && isString(req.headers['sentry-trace']) && extractTraceparentData(req.headers['sentry-trace']);
-    const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage);
+    const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage;
+
+    const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
 
     const transaction = startTransaction(
       {
         name: extractExpressTransactionName(req, { path: true, method: true }),
         op: 'http.server',
         ...traceparentData,
-        ...(baggage && { metadata: { baggage: baggage } }),
+        metadata: { baggage: baggage },
       },
       // extra context passed to the tracesSampler
       { request: extractRequestData(req) },

packages/serverless/src/awslambda.ts

@@ -11,7 +11,7 @@ import {
 } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
 import { Integration } from '@sentry/types';
-import { extensionRelayDSN, isString, logger, parseBaggageString } from '@sentry/utils';
+import { extensionRelayDSN, isString, logger, parseAndFreezeBaggageIfNecessary } from '@sentry/utils';
 // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil
 // eslint-disable-next-line import/no-unresolved
 import { Context, Handler } from 'aws-lambda';
@@ -289,16 +289,15 @@ export function wrapHandler<TEvent, TResult>(
       traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace']);
     }
 
-    const baggage =
-      eventWithHeaders.headers &&
-      isString(eventWithHeaders.headers.baggage) &&
-      parseBaggageString(eventWithHeaders.headers.baggage);
+    const rawBaggageString =
+      eventWithHeaders.headers && isString(eventWithHeaders.headers.baggage) && eventWithHeaders.headers.baggege;
+    const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
 
     const transaction = startTransaction({
       name: context.functionName,
       op: 'awslambda.handler',
       ...traceparentData,
-      ...(baggage && { metadata: { baggage: baggage } }),
+      metadata: { baggage: baggage },
     });
 
     const hub = getCurrentHub();

packages/serverless/src/gcpfunction/http.ts

@@ -1,6 +1,6 @@
 import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
-import { isString, logger, parseBaggageString, stripUrlQueryAndFragment } from '@sentry/utils';
+import { isString, logger, parseAndFreezeBaggageIfNecessary, stripUrlQueryAndFragment } from '@sentry/utils';
 
 import { IS_DEBUG_BUILD } from '../flags';
 import { domainify, getActiveDomain, proxyFunction } from './../utils';
@@ -57,16 +57,16 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
       traceparentData = extractTraceparentData(reqWithHeaders.headers['sentry-trace']);
     }
 
-    const baggage =
-      reqWithHeaders.headers &&
-      isString(reqWithHeaders.headers.baggage) &&
-      parseBaggageString(reqWithHeaders.headers.baggage);
+    const rawBaggageString =
+      reqWithHeaders.headers && isString(reqWithHeaders.headers.baggage) && reqWithHeaders.headers.baggage;
+
+    const baggage = parseAndFreezeBaggageIfNecessary(rawBaggageString, traceparentData);
 
     const transaction = startTransaction({
       name: `${reqMethod} ${reqUrl}`,
       op: 'gcp.function.http',
       ...traceparentData,
-      ...(baggage && { metadata: { baggage: baggage } }),
+      metadata: { baggage: baggage },
     });
 
     // getCurrentHub() is expected to use current active domain as a carrier

packages/tracing/src/browser/browsertracing.ts

@@ -1,6 +1,6 @@
 import { Hub } from '@sentry/hub';
 import { EventProcessor, Integration, Transaction, TransactionContext } from '@sentry/types';
-import { getGlobalObject, logger, parseBaggageString } from '@sentry/utils';
+import { getGlobalObject, logger, parseAndFreezeBaggageIfNecessary, parseBaggageString } from '@sentry/utils';
 
 import { IS_DEBUG_BUILD } from '../flags';
 import { startIdleTransaction } from '../hubextensions';
@@ -255,7 +255,7 @@ export function extractTraceDataFromMetaTags(): Partial<TransactionContext> | un
   const baggageValue = getMetaContent('baggage');
 
   const sentrytraceData = sentrytraceValue ? extractTraceparentData(sentrytraceValue) : undefined;
-  const baggage = baggageValue ? parseBaggageString(baggageValue) : undefined;
+  const baggage = parseAndFreezeBaggageIfNecessary(baggageValue, sentrytraceValue);
 
   // TODO more extensive checks for baggage validity/emptyness?
   if (sentrytraceData || baggage) {

packages/types/src/baggage.ts

@@ -8,10 +8,12 @@ export type BaggageObj = Partial<Record<AllowedBaggageKeys, string> & Record<str
  * It is expected that users interact with baggage using the helpers methods:
  * `createBaggage`, `getBaggageValue`, and `setBaggageValue`.
  *
- * Internally, the baggage data structure is a tuple of length 2, separating baggage values
+ * Internally, the baggage data structure is a tuple of length 3, separating baggage values
  * based on if they are related to Sentry or not. If the baggage values are
  * set/used by sentry, they will be stored in an object to be easily accessed.
  * If they are not, they are kept as a string to be only accessed when serialized
  * at baggage propagation time.
+ * The third tuple member controls the mutability of the baggage. If it is `true`,
+ * the baggage can not be modified any longer (i.e. is immutable).
  */
-export type Baggage = [BaggageObj, string];
+export type Baggage = [BaggageObj, string, boolean];

packages/utils/src/baggage.ts

@@ -1,4 +1,4 @@
-import { Baggage, BaggageObj } from '@sentry/types';
+import { Baggage, BaggageObj, TraceparentData } from '@sentry/types';
 
 import { IS_DEBUG_BUILD } from './flags';
 import { logger } from './logger';
@@ -17,8 +17,8 @@ export const SENTRY_BAGGAGE_KEY_PREFIX_REGEX = /^sentry-/;
 export const MAX_BAGGAGE_STRING_LENGTH = 8192;
 
 /** Create an instance of Baggage */
-export function createBaggage(initItems: BaggageObj, baggageString: string = ''): Baggage {
-  return [{ ...initItems }, baggageString];
+export function createBaggage(initItems: BaggageObj, baggageString: string = '', frozen: boolean = false): Baggage {
+  return [{ ...initItems }, baggageString, frozen];
 }
 
 /** Get a value from baggage */
@@ -28,7 +28,9 @@ export function getBaggageValue(baggage: Baggage, key: keyof BaggageObj): Baggag
 
 /** Add a value to baggage */
 export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: BaggageObj[keyof BaggageObj]): void {
-  baggage[0][key] = value;
+  if (!isBaggageFrozen(baggage)) {
+    baggage[0][key] = value;
+  }
 }
 
 /** Check if the Sentry part of the passed baggage (i.e. the first element in the tuple) is empty */
@@ -55,6 +57,23 @@ export function getThirdPartyBaggage(baggage: Baggage): string {
   return baggage[1];
 }
 
+/**
+ * Checks if baggage is frozen (i.e. immutable)
+ * @param baggage
+ * @returns true if baggage is frozen, else false
+ */
+export function isBaggageFrozen(baggage: Baggage): boolean {
+  return baggage[2];
+}
+
+/**
+ * Freezes baggage (i.e. makes it immutable)
+ * @param baggage
+ */
+export function freezeBaggage(baggage: Baggage): void {
+  baggage[2] = true;
+}
+
 /** Serialize a baggage object */
 export function serializeBaggage(baggage: Baggage): string {
   return Object.keys(baggage[0]).reduce((prev, key: keyof BaggageObj) => {
@@ -71,7 +90,7 @@ export function serializeBaggage(baggage: Baggage): string {
   }, baggage[1]);
 }
 
-/** Parse a baggage header to a string */
+/** Parse a baggage header from a string and return a Baggage object */
 export function parseBaggageString(inputBaggageString: string): Baggage {
   return inputBaggageString.split(',').reduce(
     ([baggageObj, baggageString], curr) => {
@@ -84,12 +103,13 @@ export function parseBaggageString(inputBaggageString: string): Baggage {
             [baggageKey]: decodeURIComponent(val),
           },
           baggageString,
+          false,
         ];
       } else {
-        return [baggageObj, baggageString === '' ? curr : `${baggageString},${curr}`];
+        return [baggageObj, baggageString === '' ? curr : `${baggageString},${curr}`, false];
       }
     },
-    [{}, ''],
+    [{}, '', false],
   );
 }
 
@@ -121,3 +141,27 @@ export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggag
   );
   return serializeBaggage(finalBaggage);
 }
+
+/**
+ * Helper function that takes a raw baggage string (if available) and the processed sentry-trace header
+ * data (if available), parses the baggage string and creates a Baggage object
+ * If there is no baggage string, it will create an empty Baggage object.
+ * In a second step, this functions determines when the created Baggage object should be frozen to prevent mutation
+ * of the Sentry data.
+ *
+ * Extracted this logic to a function because it's duplicated in a lot of places.
+ *
+ * @param rawBaggageString
+ * @param traceparentData
+ */
+//              parseBaggagePlusMutability
+export function parseAndFreezeBaggageIfNecessary(
+  rawBaggageString: string | false | undefined | null,
+  traceparentData: TraceparentData | string | false | undefined | null,
+): Baggage {
+  const baggage = parseBaggageString(rawBaggageString || '');
+  if (!isSentryBaggageEmpty(baggage) || (traceparentData && isSentryBaggageEmpty(baggage))) {
+    freezeBaggage(baggage);
+  }
+  return baggage;
+}