-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(react-router): Replace module proxy with otel wrap #16373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; | ||
import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; | ||
import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation'; | ||
import { | ||
getActiveSpan, | ||
getRootSpan, | ||
|
@@ -37,75 +37,71 @@ export class ReactRouterInstrumentation extends InstrumentationBase<Instrumentat | |
COMPONENT, | ||
supportedVersions, | ||
(moduleExports: ReactRouterModuleExports) => { | ||
return this._createPatchedModuleProxy(moduleExports); | ||
if (isWrapped(moduleExports['createRequestHandler'])) { | ||
this._unwrap(moduleExports, 'createRequestHandler'); | ||
} | ||
this._wrap(moduleExports, 'createRequestHandler', this._patchCreateRequestHandler()); | ||
return moduleExports; | ||
}, | ||
(_moduleExports: unknown) => { | ||
// nothing to unwrap here | ||
return _moduleExports; | ||
(moduleExports: ReactRouterModuleExports) => { | ||
this._unwrap(moduleExports, 'createRequestHandler'); | ||
}, | ||
); | ||
|
||
return reactRouterServerModule; | ||
} | ||
|
||
/** | ||
* Creates a proxy around the React Router module exports that patches the createRequestHandler function. | ||
* This allows us to wrap the request handler to add performance monitoring for data loaders and actions. | ||
* Returns a patched version of the createRequestHandler function that adds Sentry performance monitoring. | ||
* This wraps the request handler to create spans for data loader and action requests. | ||
*/ | ||
private _createPatchedModuleProxy(moduleExports: ReactRouterModuleExports): ReactRouterModuleExports { | ||
return new Proxy(moduleExports, { | ||
get(target, prop, receiver) { | ||
if (prop === 'createRequestHandler') { | ||
const original = target[prop]; | ||
return function sentryWrappedCreateRequestHandler(this: unknown, ...args: unknown[]) { | ||
const originalRequestHandler = original.apply(this, args); | ||
|
||
return async function sentryWrappedRequestHandler(request: Request, initialContext?: unknown) { | ||
let url: URL; | ||
try { | ||
url = new URL(request.url); | ||
} catch (error) { | ||
return originalRequestHandler(request, initialContext); | ||
} | ||
private _patchCreateRequestHandler(): (original: typeof reactRouter.createRequestHandler) => any { | ||
return function sentryWrappedCreateRequestHandler(this: unknown, ...args: unknown[]) { | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore not sure why original isn't found here? | ||
const originalRequestHandler = (original as typeof reactRouter.createRequestHandler).apply(this, args); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Risk: Affected versions of react-router are vulnerable to Insufficient Verification of Data Authenticity. A vulnerability in React Router's Framework mode allows an attacker to spoof pre-rendered loader data by providing a crafted JSON payload via the X-React-Router-Prerender-Data header. This manipulation can poison cached responses and lead to unintended page modifications, including potential XSS attacks. Fix: Upgrade this library to at least version 7.5.2 at sentry-javascript/yarn.lock:24603. Reference(s): GHSA-cpj6-fhp6-mr6j, CVE-2025-43865 💬 To ignore this, reply with: |
||
return async function sentryWrappedRequestHandler(request: Request, initialContext?: unknown) { | ||
let url: URL; | ||
try { | ||
url = new URL(request.url); | ||
} catch (error) { | ||
return originalRequestHandler(request, initialContext); | ||
} | ||
|
||
// We currently just want to trace loaders and actions | ||
if (!isDataRequest(url.pathname)) { | ||
return originalRequestHandler(request, initialContext); | ||
} | ||
// We currently just want to trace loaders and actions | ||
if (!isDataRequest(url.pathname)) { | ||
return originalRequestHandler(request, initialContext); | ||
} | ||
|
||
const activeSpan = getActiveSpan(); | ||
const rootSpan = activeSpan && getRootSpan(activeSpan); | ||
const activeSpan = getActiveSpan(); | ||
const rootSpan = activeSpan && getRootSpan(activeSpan); | ||
|
||
if (!rootSpan) { | ||
DEBUG_BUILD && logger.debug('No active root span found, skipping tracing for data request'); | ||
return originalRequestHandler(request, initialContext); | ||
} | ||
if (!rootSpan) { | ||
DEBUG_BUILD && logger.debug('No active root span found, skipping tracing for data request'); | ||
return originalRequestHandler(request, initialContext); | ||
} | ||
|
||
// Set the source and overwrite attributes on the root span to ensure the transaction name | ||
// is derived from the raw URL pathname rather than any parameterized route that may be set later | ||
// TODO: try to set derived parameterized route from build here (args[0]) | ||
rootSpan.setAttributes({ | ||
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE]: `${request.method} ${url.pathname}`, | ||
}); | ||
// Set the source and overwrite attributes on the root span to ensure the transaction name | ||
// is derived from the raw URL pathname rather than any parameterized route that may be set later | ||
// TODO: try to set derived parameterized route from build here (args[0]) | ||
rootSpan.setAttributes({ | ||
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_OVERWRITE]: `${request.method} ${url.pathname}`, | ||
}); | ||
|
||
return startSpan( | ||
{ | ||
name: getSpanName(url.pathname, request.method), | ||
attributes: { | ||
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react-router', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: getOpName(url.pathname, request.method), | ||
}, | ||
}, | ||
() => { | ||
return originalRequestHandler(request, initialContext); | ||
}, | ||
); | ||
}; | ||
}; | ||
} | ||
return Reflect.get(target, prop, receiver); | ||
}, | ||
}); | ||
return startSpan( | ||
{ | ||
name: getSpanName(url.pathname, request.method), | ||
attributes: { | ||
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react-router', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: getOpName(url.pathname, request.method), | ||
}, | ||
}, | ||
() => { | ||
return originalRequestHandler(request, initialContext); | ||
}, | ||
); | ||
}; | ||
}; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Risk: Affected versions of react-router are vulnerable to Insufficient Verification of Data Authenticity. A vulnerability in React Router's Framework mode allows an attacker to spoof pre-rendered loader data by providing a crafted JSON payload via the X-React-Router-Prerender-Data header. This manipulation can poison cached responses and lead to unintended page modifications, including potential XSS attacks.
Fix: Upgrade this library to at least version 7.5.2 at sentry-javascript/yarn.lock:24603.
Reference(s): GHSA-cpj6-fhp6-mr6j, CVE-2025-43865
💬 To ignore this, reply with:
•
/fp <comment>
for false positive•
/ar <comment>
for acceptable risk•
/other <comment>
for all other reasonsAlternatively, triage in Semgrep AppSec Platform to ignore the finding created by ssc-beb7e482-8f90-9d54-a2cb-68c86458077b.