Skip to content
Draft
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- Add `disableAutoUpload` option to Expo plugin to disable source map and debug symbol uploads ([#6195](https://github.com/getsentry/sentry-react-native/pull/6195))
- Expose `pauseAppHangTracking` and `resumeAppHangTracking` APIs on iOS ([#6192](https://github.com/getsentry/sentry-react-native/pull/6192))
- Better route and dynamic param extraction for Expo Router ([#6197](https://github.com/getsentry/sentry-react-native/pull/6197))
- Instrument Expo Router `push`, `replace`, `navigate`, `back`, and `dismiss` (in addition to `prefetch`) with breadcrumbs and spans, and tag the resulting idle navigation span with the initiating `navigation.method` ([#6221](https://github.com/getsentry/sentry-react-native/pull/6221))

### Fixes

Expand Down
11 changes: 6 additions & 5 deletions packages/core/etc/sentry-react-native.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,12 +272,13 @@ export interface ExpoRouter {
// (undocumented)
back?: () => void;
// (undocumented)
dismiss?: (count?: number) => void;
// (undocumented)
navigate?: (...args: unknown[]) => void;
// Warning: (ae-forgotten-export) The symbol "ExpoRouterHref" needs to be exported by the entry point index.d.ts
//
// (undocumented)
prefetch?: (href: string | {
pathname?: string;
params?: Record<string, unknown>;
}) => void | Promise<void>;
prefetch?: (href: ExpoRouterHref) => void | Promise<void>;
// (undocumented)
push?: (...args: unknown[]) => void;
// (undocumented)
Expand Down Expand Up @@ -761,7 +762,7 @@ export function wrapExpoRouter<T extends ExpoRouter>(router: T): T;
// src/js/feedback/integration.ts:21:5 - (ae-forgotten-export) The symbol "ScreenshotButtonProps" needs to be exported by the entry point index.d.ts
// src/js/feedback/integration.ts:23:5 - (ae-forgotten-export) The symbol "FeedbackFormTheme" needs to be exported by the entry point index.d.ts
// src/js/tracing/reactnativetracing.ts:94:3 - (ae-forgotten-export) The symbol "ReactNativeTracingState" needs to be exported by the entry point index.d.ts
// src/js/tracing/reactnavigation.ts:219:3 - (ae-forgotten-export) The symbol "RouteOverrideProvider" needs to be exported by the entry point index.d.ts
// src/js/tracing/reactnavigation.ts:220:3 - (ae-forgotten-export) The symbol "RouteOverrideProvider" needs to be exported by the entry point index.d.ts

// (No @packageDocumentation comment for this package)

Expand Down
185 changes: 152 additions & 33 deletions packages/core/src/js/tracing/expoRouter.ts
Original file line number Diff line number Diff line change
@@ -1,64 +1,103 @@
import { SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core';
import { addBreadcrumb, getClient, SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core';

import { SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH } from './origin';
import { SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH } from './origin';
import { setPendingExpoRouterNavigation } from './pendingExpoRouterNavigation';

type ExpoRouterHref = string | { pathname?: string; params?: Record<string, unknown> };

/**
* Type definition for Expo Router's router object
* Type definition for Expo Router's router object.
*/
export interface ExpoRouter {
prefetch?: (href: string | { pathname?: string; params?: Record<string, unknown> }) => void | Promise<void>;
// Other router methods can be added here if needed
prefetch?: (href: ExpoRouterHref) => void | Promise<void>;
push?: (...args: unknown[]) => void;
replace?: (...args: unknown[]) => void;
back?: () => void;
navigate?: (...args: unknown[]) => void;
back?: () => void;
dismiss?: (count?: number) => void;
}

type NavigationMethod = 'push' | 'replace' | 'navigate' | 'back' | 'dismiss';

interface ParsedHref {
href?: unknown;
routeName: string;
pathname?: string;
params?: Record<string, unknown>;
}

/**
* Wraps Expo Router. It currently only does one thing: extends prefetch() method
* to add automated performance monitoring.
* Wraps Expo Router methods to add automated performance monitoring and breadcrumbs.
*
* Currently wraps:
* - `prefetch` โ€” wraps the call in a `navigation.prefetch` span.
* - `push` / `replace` / `navigate` / `back` / `dismiss` โ€” adds a navigation
* breadcrumb, wraps the call in a short-lived span that mirrors prefetch's
* error/status handling, and tags the subsequent idle navigation transaction
* with the initiating `navigation.method` so the resulting span can be
* attributed back to the call site.
*
* This function instruments the `prefetch` method of an Expo Router instance
* to create performance spans that measure how long route prefetching takes.
* Safe to call repeatedly โ€” guarded by a single `__sentryWrapped` flag.
*
* @param router - The Expo Router instance from `useRouter()` hook
* @returns The same router instance with an instrumented prefetch method
* @returns The same router instance with instrumented methods
*/
export function wrapExpoRouter<T extends ExpoRouter>(router: T): T {
if (!router?.prefetch) {
if (!router) {
return router;
}

// Check if already wrapped to avoid double-wrapping
if ((router as T & { __sentryPrefetchWrapped?: boolean }).__sentryPrefetchWrapped) {
const wrappedRouter = router as T & { __sentryWrapped?: boolean };
if (wrappedRouter.__sentryWrapped) {
return router;
}

const originalPrefetch = router.prefetch.bind(router);
if (router.prefetch) {
wrapPrefetch(router);
}

router.prefetch = ((href: Parameters<NonNullable<ExpoRouter['prefetch']>>[0]) => {
// Extract route name from href for better span naming
let routeName = 'unknown';
if (typeof href === 'string') {
routeName = href;
} else if (href && typeof href === 'object' && 'pathname' in href && href.pathname) {
routeName = href.pathname;
}
if (router.push) {
router.push = wrapNavigationMethod(router, 'push', router.push.bind(router));
}
if (router.replace) {
router.replace = wrapNavigationMethod(router, 'replace', router.replace.bind(router));
}
if (router.navigate) {
router.navigate = wrapNavigationMethod(router, 'navigate', router.navigate.bind(router));
}
if (router.back) {
router.back = wrapNavigationMethod(router, 'back', router.back.bind(router)) as NonNullable<T['back']>;
}
if (router.dismiss) {
const originalDismiss = router.dismiss.bind(router) as (...args: unknown[]) => unknown;
router.dismiss = wrapNavigationMethod(router, 'dismiss', originalDismiss) as NonNullable<T['dismiss']>;
}

Check warning on line 74 in packages/core/src/js/tracing/expoRouter.ts

View check run for this annotation

@sentry/warden / warden: security-review

Navigation pathname leaks PII to Sentry regardless of `sendDefaultPii` setting

In `wrapNavigationMethod`, `parsed.pathname` is unconditionally included in breadcrumb messages, breadcrumb `data.pathname`, span names, and `route.name` attributes โ€” but for string hrefs (e.g. `router.push('/users/42')`), `parseHref` sets `pathname === href`, so the full resolved path is always sent to Sentry even when `sendDefaultPii` is `false`, defeating the guard that protects `href` and `params`.

Check warning on line 74 in packages/core/src/js/tracing/expoRouter.ts

View check run for this annotation

@sentry/warden / warden: code-review

[X3Y-GQX] Stale pending navigation set before `original.apply()` can corrupt next navigation's span attributes (additional location)

If `original.apply(router, args)` throws synchronously (e.g. invalid route, navigation not ready), `setPendingExpoRouterNavigation` has already stored the failed call's data. The `catch` block does not clear it, so the next successful navigation will have `consumePendingExpoRouterNavigation()` return this stale value and incorrectly tag that span with the failed method's `navigation.method` and route info.
Comment on lines +60 to +74
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Navigation pathname leaks PII to Sentry regardless of sendDefaultPii setting

In wrapNavigationMethod, parsed.pathname is unconditionally included in breadcrumb messages, breadcrumb data.pathname, span names, and route.name attributes โ€” but for string hrefs (e.g. router.push('/users/42')), parseHref sets pathname === href, so the full resolved path is always sent to Sentry even when sendDefaultPii is false, defeating the guard that protects href and params.

Evidence
  • parseHref(href: string) returns { href, routeName: href, pathname: href } โ€” all three fields hold the identical value (e.g. /users/42).
  • wrapNavigationMethod guards parsed.href and parsed.params behind isSendDefaultPiiEnabled(), but parsed.pathname (same value for string hrefs) is written to the breadcrumb message (to ${parsed.pathname}), data.pathname, span name, and route.name with no guard at all.
  • The code comment explicitly labels /users/42 as PII and states it should be gated, yet pathname โ€” which equals that value โ€” bypasses the gate entirely.
  • wrapPrefetch has the same issue: route.name is always set to routeName which equals the full string href.

Identified by Warden security-review ยท VL9-XWJ


wrappedRouter.__sentryWrapped = true;
return router;
}

function wrapPrefetch<T extends ExpoRouter>(router: T): void {
Comment thread
sentry-warden[bot] marked this conversation as resolved.
const originalPrefetch = router.prefetch!.bind(router);

router.prefetch = ((href: ExpoRouterHref) => {
const { routeName } = parseHref(href);

const span = startInactiveSpan({
op: 'navigation.prefetch',
name: `Prefetch ${routeName}`,
attributes: {
'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH,
'route.href': typeof href === 'string' ? href : JSON.stringify(href),
'route.name': routeName,

Check warning on line 91 in packages/core/src/js/tracing/expoRouter.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

`route.name` leaks concrete URL paths (PII) when href is a string

When a string href like `/users/123` is passed to `prefetch`, `push`, `replace`, or `navigate`, `parseHref` sets `routeName = href` (the full concrete URL). This value is then written to the `route.name` span attribute and the span name unconditionally, bypassing the `sendDefaultPii` guard that only protects `route.href` โ€” so user-identifying path segments are always sent to Sentry.
// `route.href` may contain dynamic segment values (e.g. `/users/123`)
// or stringified `params`, so it is gated behind `sendDefaultPii`.
...(isSendDefaultPiiEnabled() ? { 'route.href': serializeHref(href) } : undefined),
},
});

try {
const result = originalPrefetch(href);

// Handle both promise and synchronous returns
if (result && typeof result === 'object' && 'then' in result && typeof result.then === 'function') {
return result
.then(res => {
Expand All @@ -71,21 +110,101 @@
span?.end();
throw error;
});
} else {
// Synchronous completion
span?.setStatus({ code: SPAN_STATUS_OK });
span?.end();
return result;
}

span?.setStatus({ code: SPAN_STATUS_OK });
span?.end();
return result;
} catch (error) {
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
span?.end();
throw error;
}
}) as NonNullable<T['prefetch']>;
}

// Mark as wrapped to prevent double-wrapping
(router as T & { __sentryPrefetchWrapped?: boolean }).__sentryPrefetchWrapped = true;
function wrapNavigationMethod(
router: ExpoRouter,
method: NavigationMethod,
original: (...args: unknown[]) => unknown,
): (...args: unknown[]) => unknown {
return (...args: unknown[]) => {
const parsed = parseMethodArgs(method, args);
const sendPii = isSendDefaultPiiEnabled();

return router;
addBreadcrumb({
category: 'navigation',
type: 'navigation',
message: `Expo Router ${method}${parsed.pathname ? ` to ${parsed.pathname}` : ''}`,
data: {
method,
...(parsed.pathname ? { pathname: parsed.pathname } : undefined),
// `href` (raw URL form) and `params` may contain user identifiers or
// other PII (e.g. `/users/42`, `{ id: '42' }`). Mirror the behavior of
// `reactnavigation.ts` and only include them when `sendDefaultPii` is on.
...(sendPii && parsed.href !== undefined ? { href: serializeHref(parsed.href) } : undefined),
...(sendPii && parsed.params ? { params: parsed.params } : undefined),
},
});

setPendingExpoRouterNavigation({
method,
href: parsed.href,
pathname: parsed.pathname,
params: parsed.params,
});

const span = startInactiveSpan({
op: `navigation.${method}`,
name: `Navigation ${method}${parsed.pathname ? ` to ${parsed.pathname}` : ''}`,
attributes: {
'sentry.origin': SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION,
'navigation.method': method,
...(parsed.routeName ? { 'route.name': parsed.routeName } : undefined),
...(sendPii && parsed.href !== undefined ? { 'route.href': serializeHref(parsed.href) } : undefined),

Check warning on line 164 in packages/core/src/js/tracing/expoRouter.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

JSON.stringify in serializeHref can throw and abort navigation before the original method is called

When `sendDefaultPii` is enabled and `href.params` contains a non-serializable value (e.g. `BigInt`, `Symbol`, or a circular reference), `serializeHref` throws a `TypeError` from `JSON.stringify`. Both call sites (inside `addBreadcrumb` and `startInactiveSpan`) are executed *before* the `try { original.apply(router, args) }` block, so the exception propagates uncaught and the original navigation method is never invoked โ€” silently dropping the navigation.
Comment on lines +145 to +164
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSON.stringify in serializeHref can throw and abort navigation before the original method is called

When sendDefaultPii is enabled and href.params contains a non-serializable value (e.g. BigInt, Symbol, or a circular reference), serializeHref throws a TypeError from JSON.stringify. Both call sites (inside addBreadcrumb and startInactiveSpan) are executed before the try { original.apply(router, args) } block, so the exception propagates uncaught and the original navigation method is never invoked โ€” silently dropping the navigation.

Evidence
  • params in ExpoRouterHref and ParsedHref is typed Record<string, unknown>, allowing any value including non-serializable types.
  • serializeHref (line 204) calls JSON.stringify(href) with no try/catch; BigInt/Symbol/circular refs all throw TypeError.
  • Lines 145 and 164 call serializeHref inside addBreadcrumb and startInactiveSpan argument objects, both located before the try block at line 170 that wraps original.apply(router, args).
  • If either throws, execution exits the wrapper without ever reaching original.apply, silently killing the navigation.
  • No existing test covers a non-serializable param value with sendDefaultPii enabled.

Identified by Warden find-bugs ยท 9U9-FWQ

},
});

try {
const result = original.apply(router, args);
span?.setStatus({ code: SPAN_STATUS_OK });
span?.end();
return result;
} catch (error) {
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
span?.end();

Check warning on line 175 in packages/core/src/js/tracing/expoRouter.ts

View check run for this annotation

@sentry/warden / warden: code-review

Stale pending navigation set before `original.apply()` can corrupt next navigation's span attributes

If `original.apply(router, args)` throws synchronously (e.g. invalid route, navigation not ready), `setPendingExpoRouterNavigation` has already stored the failed call's data. The `catch` block does not clear it, so the next successful navigation will have `consumePendingExpoRouterNavigation()` return this stale value and incorrectly tag that span with the failed method's `navigation.method` and route info.
Comment on lines +150 to +175
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale pending navigation set before original.apply() can corrupt next navigation's span attributes

If original.apply(router, args) throws synchronously (e.g. invalid route, navigation not ready), setPendingExpoRouterNavigation has already stored the failed call's data. The catch block does not clear it, so the next successful navigation will have consumePendingExpoRouterNavigation() return this stale value and incorrectly tag that span with the failed method's navigation.method and route info.

Evidence
  • setPendingExpoRouterNavigation is called unconditionally at line 150, before the try block at line 168.
  • original.apply(router, args) at line 169 can throw synchronously (e.g. bad route arg).
  • The catch block at lines 173โ€“177 only records the span error and re-throws; it never calls clearPendingExpoRouterNavigation() or otherwise resets the pending value.
  • In reactnavigation.ts line 456, consumePendingExpoRouterNavigation() is called on the next navigation state change, where it would dequeue the stale value and wrongly attribute the new span to the failed navigation.
  • The error-path test in expoRouter.test.ts does not assert that the pending value is cleared after a throw, confirming this is unguarded.
Also found at 2 additional locations
  • packages/core/src/js/tracing/expoRouter.ts:59-74
  • packages/core/src/js/tracing/reactnavigation.ts:437-440

Identified by Warden code-review ยท X3Y-GQX

throw error;
}
};
}

function parseMethodArgs(method: NavigationMethod, args: unknown[]): ParsedHref {
if (method === 'back' || method === 'dismiss') {
return { routeName: method };
}
return parseHref(args[0] as ExpoRouterHref | undefined);
}

function parseHref(href: ExpoRouterHref | undefined): ParsedHref {
if (typeof href === 'string') {
return { href, routeName: href, pathname: href };
}
if (href && typeof href === 'object') {
const pathname = typeof href.pathname === 'string' ? href.pathname : undefined;
return {
href,
routeName: pathname ?? 'unknown',
pathname,
params: href.params,
};
}
return { routeName: 'unknown' };
}

function serializeHref(href: unknown): string {
return typeof href === 'string' ? href : JSON.stringify(href);
}

function isSendDefaultPiiEnabled(): boolean {
return getClient()?.getOptions()?.sendDefaultPii ?? false;
}
1 change: 1 addition & 0 deletions packages/core/src/js/tracing/origin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ export const SPAN_ORIGIN_AUTO_UI_TIME_TO_DISPLAY = 'auto.ui.time_to_display';
export const SPAN_ORIGIN_MANUAL_UI_TIME_TO_DISPLAY = 'manual.ui.time_to_display';

export const SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH = 'auto.expo_router.prefetch';
export const SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION = 'auto.expo_router.navigation';
export const SPAN_ORIGIN_AUTO_RESOURCE_EXPO_IMAGE = 'auto.resource.expo_image';
export const SPAN_ORIGIN_AUTO_RESOURCE_EXPO_ASSET = 'auto.resource.expo_asset';
39 changes: 39 additions & 0 deletions packages/core/src/js/tracing/pendingExpoRouterNavigation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* Cross-module hand-off between {@link wrapExpoRouter} and the
* {@link reactNavigationIntegration} idle navigation span.
*
* When an Expo Router method (push / replace / navigate / back / dismiss) is
* called, it stores the initiating method here. The next idle navigation span
* consumes (and clears) this value so the span can be attributed to the call
* site via the `navigation.method` attribute.
*/

export interface PendingExpoRouterNavigation {
/** The Expo Router method that initiated the navigation. */
method: 'push' | 'replace' | 'navigate' | 'back' | 'dismiss';
/** The target href (string or object), if any. */
href?: unknown;
/** Parsed pathname from the href, if any. */
pathname?: string;
/** Parsed params from the href, if any. */
params?: Record<string, unknown>;
}

let pending: PendingExpoRouterNavigation | undefined;

/** Stores the initiating Expo Router navigation call. Overwrites any previous pending value. */
export function setPendingExpoRouterNavigation(value: PendingExpoRouterNavigation): void {
pending = value;
}

/** Returns and clears the pending Expo Router navigation, if any. */
export function consumePendingExpoRouterNavigation(): PendingExpoRouterNavigation | undefined {
const value = pending;
pending = undefined;
return value;
}

/** Test helper โ€” clears the pending value without consuming it. */
export function clearPendingExpoRouterNavigation(): void {
pending = undefined;
}
6 changes: 6 additions & 0 deletions packages/core/src/js/tracing/reactnavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
markRootSpanForDiscard,
} from './onSpanEndUtils';
import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from './origin';
import { consumePendingExpoRouterNavigation } from './pendingExpoRouterNavigation';
import { getReactNativeTracingIntegration } from './reactnativetracing';
import { SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes';
import {
Expand Down Expand Up @@ -433,6 +434,11 @@
latestNavigationSpan = startGenericIdleNavigationSpan(finalSpanOptions, { ...idleSpanOptions, isAppRestart });
latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION);
latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, navigationActionType);

const pendingExpoRouter = consumePendingExpoRouterNavigation();
if (pendingExpoRouter && latestNavigationSpan) {
latestNavigationSpan.setAttribute('navigation.method', pendingExpoRouter.method);

Check warning on line 440 in packages/core/src/js/tracing/reactnavigation.ts

View check run for this annotation

@sentry/warden / warden: code-review

[X3Y-GQX] Stale pending navigation set before `original.apply()` can corrupt next navigation's span attributes (additional location)

If `original.apply(router, args)` throws synchronously (e.g. invalid route, navigation not ready), `setPendingExpoRouterNavigation` has already stored the failed call's data. The `catch` block does not clear it, so the next successful navigation will have `consumePendingExpoRouterNavigation()` return this stale value and incorrectly tag that span with the failed method's `navigation.method` and route info.
}
if (ignoreEmptyBackNavigationTransactions) {
ignoreEmptyBackNavigation(getClient(), latestNavigationSpan);
}
Expand Down
Loading
Loading