diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts b/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts index bbeee228783a..89bdd61f5491 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts @@ -9,7 +9,7 @@ declare const process: { env: Record }; Sentry.init({ dsn: process.env.E2E_TEST_DSN, - tunnel: 'http://localhost:3061/', + tunnel: process.env.E2E_TEST_TUNNEL ? true : 'http://localhost:3061/', tracesSampleRate: 1.0, }); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/assets.ts b/dev-packages/e2e-tests/test-applications/remix-v3/app/assets.ts index 345e6b2eaffd..4bfe6672caf4 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/assets.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/assets.ts @@ -15,6 +15,7 @@ export const assets = createAssetServer({ // into a browser module. The asset server substitutes these when it compiles. define: { 'process.env.E2E_TEST_DSN': JSON.stringify(process.env.E2E_TEST_DSN), + 'process.env.E2E_TEST_TUNNEL': JSON.stringify(process.env.E2E_TEST_TUNNEL ?? ''), }, }, }); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts b/dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts index 03724dd7ea27..2102a5768372 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/router.ts @@ -14,8 +14,21 @@ declare module 'remix/router' { } } +// The tunnel variant checks that the managed tunnel answers before an auth middleware like this one. +function authMiddleware(context: { request: Request }, next: () => Promise): Promise | Response { + if ( + process.env.E2E_TEST_TUNNEL && + context.request.method === 'POST' && + context.request.headers.get('x-auth') !== 'ok' + ) { + return new Response('blocked by user auth', { status: 401 }); + } + + return next(); +} + export const router = createRouter({ - middleware: [renderMiddleware], + middleware: [authMiddleware, renderMiddleware], }); router.map(routes, controller); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/package.json b/dev-packages/e2e-tests/test-applications/remix-v3/package.json index 11208a42ee98..abeb9f393e6f 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/package.json +++ b/dev-packages/e2e-tests/test-applications/remix-v3/package.json @@ -8,7 +8,9 @@ "test": "playwright test", "clean": "npx rimraf node_modules pnpm-lock.yaml", "test:build": "pnpm install", - "test:assert": "pnpm test" + "test:assert": "pnpm test", + "test:assert:tunnel": "E2E_TEST_TUNNEL=1 E2E_TEST_DSN=http://public@localhost:3061/1337 playwright test tunnel", + "test:assert:tunnel-plain-server": "E2E_TEST_PLAIN_SERVER=1 pnpm test:assert:tunnel" }, "dependencies": { "@sentry/remix": "file:../../packed/sentry-remix-packed.tgz", @@ -23,5 +25,19 @@ }, "volta": { "extends": "../../package.json" + }, + "sentryTest": { + "variants": [ + { + "build-command": "pnpm test:build", + "assert-command": "pnpm test:assert:tunnel", + "label": "remix-v3 (tunnel)" + }, + { + "build-command": "pnpm test:build", + "assert-command": "pnpm test:assert:tunnel-plain-server", + "label": "remix-v3 (tunnel, plain server)" + } + ] } } diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/server.ts b/dev-packages/e2e-tests/test-applications/remix-v3/server.ts index c9ac4f00d087..e661e5eb8808 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/server.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/server.ts @@ -1,27 +1,65 @@ import * as Sentry from '@sentry/remix/v3'; import * as http from 'node:http'; +import { Readable } from 'node:stream'; import { createRequestListener } from 'remix/node-fetch-server'; import { router } from './app/router.ts'; Sentry.init({ dsn: process.env.E2E_TEST_DSN, - tunnel: 'http://localhost:3061/', + tunnel: process.env.E2E_TEST_TUNNEL ? true : 'http://localhost:3061/', tracesSampleRate: 1.0, }); const port = process.env.PORT ? Number.parseInt(process.env.PORT, 10) : 3060; -const server = http.createServer( - createRequestListener(request => { - // Handled outside the router on purpose: the only hook that can see this is the listener's - // `onError`, which is the case the router middleware cannot cover. - if (new URL(request.url).pathname === '/plain-throw') { - throw new Error('Plain handler failed'); +// The plain server variant serves the router without `createRequestListener`, so only the router middleware can +// answer the tunnel there. +const plainServer = !!process.env.E2E_TEST_PLAIN_SERVER; + +function handle(request: Request): Response | Promise { + // Handled outside the router on purpose: the only hook that can see this is the listener's + // `onError`, which is the case the router middleware cannot cover. + if (new URL(request.url).pathname === '/plain-throw') { + throw new Error('Plain handler failed'); + } + // The tunnel variant checks that the managed tunnel answers before code the app runs in front of its router. With + // the plain server nothing runs ahead of this, so it would block the tunnel there. + if ( + process.env.E2E_TEST_TUNNEL && + !plainServer && + request.method === 'POST' && + request.headers.get('x-app-auth') !== 'ok' + ) { + return new Response('blocked by the app before its router', { status: 403 }); + } + return router.fetch(request); +} + +/** + * A bare `node:http` adapter for the fetch handler. + */ +function toNodeListener(handler: (request: Request) => Response | Promise): http.RequestListener { + return async (req, res) => { + const hasBody = req.method !== 'GET' && req.method !== 'HEAD'; + const request = new Request(`http://localhost:${port}${req.url}`, { + method: req.method, + headers: req.headers as HeadersInit, + body: hasBody ? (Readable.toWeb(req) as ReadableStream) : undefined, + duplex: 'half', + } as RequestInit); + + try { + const response = await handler(request); + res.writeHead(response.status, Object.fromEntries(response.headers)); + res.end(Buffer.from(await response.arrayBuffer())); + } catch { + res.writeHead(500).end(); } - return router.fetch(request); - }), -); + }; +} + +const server = http.createServer(plainServer ? toNodeListener(handle) : createRequestListener(handle)); server.listen(port, () => { console.log(`Server listening on http://localhost:${port}`); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/tests/tunnel.test.ts b/dev-packages/e2e-tests/test-applications/remix-v3/tests/tunnel.test.ts new file mode 100644 index 000000000000..373e0c73a8ce --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/remix-v3/tests/tunnel.test.ts @@ -0,0 +1,25 @@ +import { expect, test } from '@playwright/test'; +import { isServerSpanAt, watchForTunnelSpan } from '@sentry-internal/test-utils'; + +test.skip(!process.env.E2E_TEST_TUNNEL, 'Only runs in the tunnel variant'); + +const TUNNEL_PATH = /^\/[a-z0-9]{1,7}$/; + +test('answers the managed tunnel before the app handler, route matching and the app middleware', async ({ page }) => { + // The variant's handler rejects every other POST before its router, except with the plain server, where nothing ahead + // of the router could answer the tunnel. Its auth middleware rejects the POSTs that get through. + if (!process.env.E2E_TEST_PLAIN_SERVER) { + expect((await page.request.post('/anything', { data: 'x' })).status()).toBe(403); + } + expect((await page.request.post('/anything', { data: 'x', headers: { 'x-app-auth': 'ok' } })).status()).toBe(401); + + const raceTunnelSpan = watchForTunnelSpan('remix-v3', isServerSpanAt(TUNNEL_PATH)); + const tunnelResponsePromise = page.waitForResponse( + response => response.request().method() === 'POST' && TUNNEL_PATH.test(new URL(response.url()).pathname), + ); + + await page.goto('/'); + + expect((await tunnelResponsePromise).status()).toBe(200); + expect(await raceTunnelSpan(isServerSpanAt('/api/items/1'), () => page.request.get('/api/items/1'))).toBe('anchor'); +}); diff --git a/packages/remix/src/v3/server/instrument.ts b/packages/remix/src/v3/server/instrument.ts index f348c9ae01b0..b70fe91e9b4c 100644 --- a/packages/remix/src/v3/server/instrument.ts +++ b/packages/remix/src/v3/server/instrument.ts @@ -1,6 +1,7 @@ import * as diagnosticsChannel from 'node:diagnostics_channel'; import { createMultiMatcher } from 'remix/route-pattern/match'; import { consoleSandbox } from '@sentry/core'; +import { handleTunnelRequestIfMatched } from '@sentry/core/server'; import type { OrchestrionChannelContext } from '@sentry/server-utils'; import { remixV3Channels } from '@sentry/server-utils/orchestrion/config'; @@ -58,9 +59,9 @@ function subscribeToCreateRouter(): void { } /** - * Report errors from any fetch handler, router or not. The middleware only sees failures inside a - * router; this is the only hook that covers a plain handler. No abort guard is needed, because the - * listener drops aborted requests before calling `onError`. + * Report errors from any fetch handler, router or not, and answer managed tunnel requests before it runs. + * The middleware only sees failures inside a router; this is the only hook that covers a plain handler. + * No abort guard is needed, because the listener drops aborted requests before calling `onError`. */ function subscribeToCreateRequestListener(): void { diagnosticsChannel @@ -71,6 +72,7 @@ function subscribeToCreateRequestListener(): void { start(data) { try { injectOnError(ensureOptions(data.arguments, 1)); + wrapFetchHandler(data.arguments); } catch { // Ignored on purpose. } @@ -82,6 +84,21 @@ function subscribeToCreateRequestListener(): void { }); } +/** + * Answers managed tunnel requests before the app's fetch handler, so code the app runs in front of its router never + * sees them. The router middleware still answers them for apps served some other way. + */ +function wrapFetchHandler(args: unknown[]): void { + const handler = args[0]; + if (typeof handler !== 'function') { + return; + } + + args[0] = function sentryFetchHandler(this: unknown, request: Request, ...rest: unknown[]): unknown { + return handleTunnelRequestIfMatched(request) ?? handler.call(this, request, ...rest); + }; +} + function injectOnError(raw: Record | undefined): void { if (!raw || markInjected(raw)) { return; diff --git a/packages/remix/src/v3/server/middleware.ts b/packages/remix/src/v3/server/middleware.ts index f2ce59c48f01..b5803af44dcb 100644 --- a/packages/remix/src/v3/server/middleware.ts +++ b/packages/remix/src/v3/server/middleware.ts @@ -10,6 +10,7 @@ import { updateSpanName, winterCGRequestToRequestData, } from '@sentry/core'; +import { handleTunnelRequestIfMatched } from '@sentry/core/server'; import { formatRouteTiming } from '../routeTiming'; import type { MatcherLike, MiddlewareLike, NextFunctionLike, RequestContextLike } from '../types'; @@ -25,6 +26,11 @@ import { resolveRoutePattern } from './route'; */ export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike { return async function sentryMiddleware(context: RequestContextLike, next: NextFunctionLike): Promise { + const tunnelResponse = handleTunnelRequestIfMatched(context.request); + if (tunnelResponse) { + return tunnelResponse; + } + const isolationScope = getIsolationScope(); isolationScope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(context.request) }); diff --git a/packages/remix/test/v3/instrument.test.ts b/packages/remix/test/v3/instrument.test.ts index 1a564d1f1758..c0d984e763f7 100644 --- a/packages/remix/test/v3/instrument.test.ts +++ b/packages/remix/test/v3/instrument.test.ts @@ -13,6 +13,15 @@ vi.mock('@sentry/core', async importOriginal => ({ consoleSandbox: (callback: () => unknown) => callback(), })); +const TUNNEL_URL = 'http://localhost/fdv680'; +const tunnelResponse = new Response(null, { status: 200 }); + +// Answers only `TUNNEL_URL`, like a server with the managed tunnel at that path. +vi.mock('@sentry/core/server', () => ({ + handleTunnelRequestIfMatched: (request: Request) => + request.url === TUNNEL_URL ? Promise.resolve(tunnelResponse) : undefined, +})); + const { instrumentRemixV3 } = await import('../../src/v3/server/instrument'); const startChannel = channel(`tracing:${remixV3Channels.REMIX_V3_CREATE_ROUTER}:start`); @@ -135,3 +144,33 @@ describe('the createRequestListener error hook', () => { expect(args[1]).toEqual(expect.objectContaining({ onError: expect.any(Function) })); }); }); + +describe('the createRequestListener tunnel', () => { + beforeAll(() => { + instrumentRemixV3(); + }); + + it("answers a managed tunnel request without calling the app's handler", async () => { + const appHandler = vi.fn(() => new Response('app')); + const args: unknown[] = [appHandler]; + + publishCreateRequestListener(args); + + expect(await (args[0] as (request: Request) => unknown)(new Request(TUNNEL_URL, { method: 'POST' }))).toBe( + tunnelResponse, + ); + expect(appHandler).not.toHaveBeenCalled(); + }); + + it("hands every other request to the app's handler", () => { + const appResponse = new Response('app'); + const appHandler = vi.fn(() => appResponse); + const args: unknown[] = [appHandler]; + const request = new Request('http://localhost/page'); + + publishCreateRequestListener(args); + + expect((args[0] as (request: Request, client: unknown) => unknown)(request, 'client')).toBe(appResponse); + expect(appHandler).toHaveBeenCalledWith(request, 'client'); + }); +}); diff --git a/packages/remix/test/v3/middleware.test.ts b/packages/remix/test/v3/middleware.test.ts index 3bc7031300bf..c86d426793e9 100644 --- a/packages/remix/test/v3/middleware.test.ts +++ b/packages/remix/test/v3/middleware.test.ts @@ -1,14 +1,24 @@ import type * as SentryCore from '@sentry/core'; +import type * as SentryCoreServer from '@sentry/core/server'; import { beforeEach, describe, expect, it, vi } from 'vitest'; // Trace data as the SDK would produce it for the active request; empty when nothing is active. const getTraceData = vi.hoisted(() => vi.fn<() => { 'sentry-trace'?: string; baggage?: string }>(() => ({}))); +const handleTunnelRequestIfMatched = vi.hoisted(() => + vi.fn<(request: Request) => Promise | undefined>(() => undefined), +); + vi.mock('@sentry/core', async importOriginal => ({ ...(await importOriginal()), getTraceData, })); +vi.mock('@sentry/core/server', async importOriginal => ({ + ...(await importOriginal()), + handleTunnelRequestIfMatched, +})); + import { sentryRemixMiddleware } from '../../src/v3/server/middleware'; import type { MatcherLike, RequestContextLike } from '../../src/v3/types'; @@ -27,6 +37,20 @@ function contextFor(url: string, headers: Record = {}): RequestC describe('sentryRemixMiddleware', () => { beforeEach(() => { getTraceData.mockReturnValue({}); + handleTunnelRequestIfMatched.mockReturnValue(undefined); + }); + + it('answers tunnel requests before route matching and the rest of the chain', async () => { + const tunnelResponse = new Response('forwarded'); + handleTunnelRequestIfMatched.mockReturnValue(Promise.resolve(tunnelResponse)); + const next = vi.fn(); + const context = contextFor('http://x/abc123'); + + const response = await sentryRemixMiddleware(matcherFor('/'))(context, next); + + expect(response).toBe(tunnelResponse); + expect(handleTunnelRequestIfMatched).toHaveBeenCalledWith(context.request); + expect(next).not.toHaveBeenCalled(); }); it('propagates the trace to HTML responses through Server-Timing, ahead of the route', async () => {