-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(nextjs): Automatically skip middleware requests for tunnel route #16812
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 1 commit
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 |
---|---|---|
|
@@ -13,6 +13,7 @@ import { | |
winterCGRequestToRequestData, | ||
withIsolationScope, | ||
} from '@sentry/core'; | ||
import { NextResponse } from 'next/server'; | ||
import type { EdgeRouteHandler } from '../edge/types'; | ||
import { flushSafelyWithTimeout } from './utils/responseEnd'; | ||
|
||
|
@@ -27,6 +28,23 @@ export function wrapMiddlewareWithSentry<H extends EdgeRouteHandler>( | |
): (...params: Parameters<H>) => Promise<ReturnType<H>> { | ||
return new Proxy(middleware, { | ||
apply: async (wrappingTarget, thisArg, args: Parameters<H>) => { | ||
const tunnelRoute = | ||
'_sentryRewritesTunnelPath' in globalThis | ||
? (globalThis as Record<string, unknown>)._sentryRewritesTunnelPath | ||
: undefined; | ||
|
||
if (tunnelRoute && typeof tunnelRoute === 'string') { | ||
const req: unknown = args[0]; | ||
// Check if the current request matches the tunnel route | ||
if (req instanceof Request) { | ||
const url = new URL(req.url); | ||
const isTunnelRequest = url.pathname.startsWith(tunnelRoute); | ||
|
||
if (isTunnelRequest) { | ||
return NextResponse.next() as ReturnType<H>; | ||
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. High severity vulnerability may affect your project—review required: ℹ️ Why this mattersAffected versions of next are vulnerable to Improper Authorization. Improper authorization handling in Next.js applications enables attackers to bypass security controls for paths directly under the application's root directory, potentially exposing sensitive data or functionality. This issue affects versions prior to Next.js 14.2.15, where authorization logic based solely on pathname fails to account for certain direct page accesses. To resolve this comment:
💬 Ignore this findingTo ignore this, reply with:
You can view more details on this finding in the Semgrep AppSec Platform here. |
||
} | ||
} | ||
} | ||
// TODO: We still should add central isolation scope creation for when our build-time instrumentation does not work anymore with turbopack. | ||
return withIsolationScope(isolationScope => { | ||
const req: unknown = args[0]; | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -107,17 +107,17 @@ function getFinalConfigObject( | |||||
showedExportModeTunnelWarning = true; | ||||||
// eslint-disable-next-line no-console | ||||||
console.warn( | ||||||
'[@sentry/nextjs] The Sentry Next.js SDK `tunnelRoute` option will not work in combination with Next.js static exports. The `tunnelRoute` option uses serverside features that cannot be accessed in export mode. If you still want to tunnel Sentry events, set up your own tunnel: https://docs.sentry.io/platforms/javascript/troubleshooting/#using-the-tunnel-option', | ||||||
'[@sentry/nextjs] The Sentry Next.js SDK `tunnelRoute` option will not work in combination with Next.js static exports. The `tunnelRoute` option uses server-side features that cannot be accessed in export mode. If you still want to tunnel Sentry events, set up your own tunnel: https://docs.sentry.io/platforms/javascript/troubleshooting/#using-the-tunnel-option', | ||||||
); | ||||||
} | ||||||
} else { | ||||||
const resolvedTunnelRoute = | ||||||
typeof userSentryOptions.tunnelRoute === 'boolean' | ||||||
typeof userSentryOptions.tunnelRoute === 'boolean' && userSentryOptions.tunnelRoute === true | ||||||
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. We could simplify that, since we know it's a boolean once we pass
Suggested change
Perhaps a bit redundant otherwise |
||||||
? generateRandomTunnelRoute() | ||||||
: userSentryOptions.tunnelRoute; | ||||||
|
||||||
// Update the global options object to use the resolved value everywhere | ||||||
userSentryOptions.tunnelRoute = resolvedTunnelRoute; | ||||||
userSentryOptions.tunnelRoute = resolvedTunnelRoute || undefined; | ||||||
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. I like that! 👍 Is there a reason why we want the value to be |
||||||
setUpTunnelRewriteRules(incomingUserNextConfigObject, resolvedTunnelRoute); | ||||||
} | ||||||
} | ||||||
|
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.
I don't think we need a
url
constant since we only use it once andnew URL(req.url)
is quite explanatoryWhat do you reckon?