Skip to content

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion packages/nextjs/rollup.npm.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default [
// prevent this internal nextjs code from ending up in our built package (this doesn't happen automatically because
// the name doesn't match an SDK dependency)
packageSpecificConfig: {
external: ['next/router', 'next/constants', 'next/headers', 'stacktrace-parser'],
external: ['next/router', 'next/constants', 'next/headers', 'next/server', 'stacktrace-parser'],

// Next.js and our users are more happy when our client code has the "use client" directive
plugins: [
Expand Down
18 changes: 18 additions & 0 deletions packages/nextjs/src/common/wrapMiddlewareWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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);
Comment on lines +39 to +40

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 and new URL(req.url) is quite explanatory

Suggested change
const url = new URL(req.url);
const isTunnelRequest = url.pathname.startsWith(tunnelRoute);
const isTunnelRequest = new URL(req.url).pathname.startsWith(tunnelRoute);

What do you reckon?


if (isTunnelRequest) {
return NextResponse.next() as ReturnType<H>;

Choose a reason for hiding this comment

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

High severity vulnerability may affect your project—review required:
Line 44 lists a dependency (next) with a known High severity vulnerability.

ℹ️ Why this matters

Affected 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.

References: GHSA, CVE

To resolve this comment:
Check if you use authorization to protect a page directly under the application's root directory (for example, https://example.com/foo) and you do NOT host your application on Vercel.

  • If you're affected, upgrade this dependency to at least version 14.2.15 at yarn.lock.
  • If you're not affected, comment /fp we don't use this [condition]
💬 Ignore this finding

To ignore this, reply with:

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

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];
Expand Down
6 changes: 3 additions & 3 deletions packages/nextjs/src/config/withSentryConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Choose a reason for hiding this comment

The 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 typeof userSentryOptions.tunnelRoute === 'boolean'

Suggested change
typeof userSentryOptions.tunnelRoute === 'boolean' && userSentryOptions.tunnelRoute === true
typeof userSentryOptions.tunnelRoute === 'boolean' && userSentryOptions.tunnelRoute

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;

Choose a reason for hiding this comment

The 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 undefined specifically?

setUpTunnelRewriteRules(incomingUserNextConfigObject, resolvedTunnelRoute);
}
}
Expand Down
Loading