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
13 changes: 9 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1028,8 +1028,8 @@
node-version-file: 'dev-packages/e2e-tests/test-applications/${{ matrix.test-application }}/package.json'
- name: Set up Bun
if:
contains(fromJSON('["node-exports-test-app","nextjs-16-bun", "elysia-bun", "elysia-bun-static", "hono-4",
"bun-bytecode", "bun-mysql"]'), matrix.test-application)
matrix.runtime == 'bun' || contains(fromJSON('["node-exports-test-app","nextjs-16-bun", "elysia-bun",
"elysia-bun-static", "hono-4", "bun-bytecode", "bun-mysql"]'), matrix.test-application)
uses: oven-sh/setup-bun@v2
with:
bun-version: '1.3.14'
Expand All @@ -1040,7 +1040,7 @@
use-installer: true
token: ${{ secrets.GITHUB_TOKEN }}
- name: Set up Deno
if: matrix.test-application == 'deno' || matrix.test-application == 'hono-4'
if: matrix.runtime == 'deno' || matrix.test-application == 'deno' || matrix.test-application == 'hono-4'
uses: denoland/setup-deno@v2.0.5
with:
deno-version: ${{ matrix.deno-version || 'v2.8.3' }}
Expand Down Expand Up @@ -1166,8 +1166,13 @@
uses: actions/setup-node@v7
with:
node-version-file: 'dev-packages/e2e-tests/test-applications/${{ matrix.test-application }}/package.json'
- name: Set up Bun
if: matrix.runtime == 'bun'
uses: oven-sh/setup-bun@v2
with:
bun-version: '1.3.14'

Check warning on line 1173 in .github/workflows/build.yml

View check run for this annotation

@sentry/warden / warden: security-review

Mutable oven-sh/setup-bun@v2 runs in secrets-bearing optional E2E job

Pin oven-sh/setup-bun to a full commit SHA; a moved v2 tag could execute untrusted code with access to E2E_TEST_AUTH_TOKEN, CLOUDFLARE_API_TOKEN, and other job secrets.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Comment on lines +1169 to +1173

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mutable oven-sh/setup-bun@v2 in secrets-bearing E2E job

Pin oven-sh/setup-bun to a full commit SHA; a rewritten v2 tag could run attacker-controlled code in this job and read E2E/Cloudflare secrets.

Evidence
  • The new step uses third-party oven-sh/setup-bun@v2 (mutable tag), unlike the adjacent SHA-pinned pnpm/action-setup@0977fd9....
  • job_optional_e2e_tests injects secrets.E2E_TEST_AUTH_TOKEN, secrets.CLOUDFLARE_API_TOKEN, secrets.E2E_OPENROUTER_API_KEY, and related DSNs into the job env.
  • The job is intentionally non-fork (head.repo.full_name == github.repository), so those secrets are present when the action runs.
  • A compromised or retagged v2 would execute in that privileged job context and could exfiltrate those credentials.

Identified by Warden · security-review · LR9-NP8

- name: Set up Deno
if: matrix.test-application == 'deno'
if: matrix.runtime == 'deno' || matrix.test-application == 'deno'
uses: denoland/setup-deno@v2.0.5
with:
deno-version: ${{ matrix.deno-version || 'v2.8.3' }}
Expand Down
45 changes: 45 additions & 0 deletions dev-packages/e2e-tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,51 @@ Make sure to add a `test:build` and `test:assert` command to the new app's `pack
Sentry packages are automatically resolved to the local build via pnpm overrides injected at test time, so no manual
registry configuration is needed.

## Runtime variants (Bun, Deno, Cloudflare)

To test a framework on a runtime other than Node.js, add an `optionalVariants` entry to the existing test app instead of
creating a new app. `react-router-8-framework` is the reference setup.

- **`RUNTIME` env var**: `node` (default), `bun`, `deno` or `cloudflare`. `tests/constants.ts` exports it, so tests can
branch on it where the runtimes are expected to differ (for example `platform` or `sdk.name`).
- **Start commands**: `playwright.config.mjs` selects the start command from `RUNTIME`. Bun and Deno use the same build
as Node and only change the start command, for example
`bun --bun --preload ./instrument.mjs ./node_modules/@react-router/serve/bin.cjs ./build/server/index.js` and
`deno run -A --preload ./instrument.mjs ./node_modules/@react-router/serve/bin.cjs ./build/server/index.js`.
- **Cloudflare**: the app has the Cloudflare dependencies installed all the time. The Cloudflare build has its own
`vite.cloudflare.config.ts` with `@cloudflare/vite-plugin` and `sentryCloudflareVitePlugin` from
`@sentry/cloudflare/vite`, and the variant's `build-command` passes it with `--config`. Node, Bun and Deno share
`vite.config.ts`. The Worker entry lives in a separate file (for example `workers/app.ts`) and exports a plain
handler: the Sentry plugin wraps it with `withSentry` and reads the init options from `instrument.server.ts` next to
the entry. The start command runs `wrangler dev` on the build output. Code at module scope must not do I/O (for
example open a database connection), because workerd does not allow it.
- **Runtime-specific files**: name them `<name>.<runtime>.<ext>` (for example `entry.server.cloudflare.tsx`). When the
framework does not let you configure a server entry, add `runtimeEntryPlugin` from
`@sentry-internal/test-utils/vite` to the runtime's Vite config, for example
`runtimeEntryPlugin('app/entry.server.tsx', 'cloudflare')`.
- **Scripts**: put `RUNTIME` in a named script (`"test:assert:bun": "RUNTIME=bun pnpm test:assert"`), not in the
`assert-command`. `yarn test:run` prefixes the command with `volta run`, which cannot run a leading env assignment.
- **`runtime` matrix key**: set `"runtime": "bun"` or `"runtime": "deno"` on the variant. CI then installs that runtime
for the job, so a new variant needs no change to `.github/workflows/build.yml`. A variant can also pin the version,
for example `"deno-version": "v2.9.0"`.
- **Bun**: under `bun run` the SDK cannot inject diagnostics channels into packages that stay outside the build (for
example Express behind `react-router-serve`), so those produce no spans on Bun. Where a test depends on them, branch
on `RUNTIME` and say why in a comment.

```json
"sentryTest": {
"optionalVariants": [
{ "assert-command": "pnpm test:assert:bun", "runtime": "bun", "label": "my-app (bun)" },
{ "assert-command": "pnpm test:assert:deno", "runtime": "deno", "label": "my-app (deno)" },
{
"build-command": "pnpm test:build:cloudflare",
"assert-command": "pnpm test:assert:cloudflare",
"label": "my-app (cloudflare)"
}
]
}
```

## Troubleshooting

### Common Issues
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import * as Sentry from '@sentry/react-router/cloudflare';
import { isbot } from 'isbot';
import { renderToReadableStream } from 'react-dom/server';
import { type EntryContext, type HandleErrorFunction, ServerRouter } from 'react-router';

async function handleRequest(
request: Request,
responseStatusCode: number,
responseHeaders: Headers,
routerContext: EntryContext,
): Promise<Response> {
let shellRendered = false;
const userAgent = request.headers.get('user-agent');

const body = await renderToReadableStream(<ServerRouter context={routerContext} url={request.url} />, {
signal: request.signal,
onError(error: unknown) {
responseStatusCode = 500;
// Errors thrown after the shell has flushed can't change the status code, so surface them.
if (shellRendered) {
// eslint-disable-next-line no-console
console.error(error);
}
},
});
shellRendered = true;

if (userAgent && isbot(userAgent)) {
await body.allReady;
}

responseHeaders.set('Content-Type', 'text/html');

return new Response(Sentry.injectTraceMetaTags(body), {
headers: responseHeaders,
status: responseStatusCode,
});
}

export const handleError: HandleErrorFunction = Sentry.createSentryHandleError({ logErrors: true });

export default Sentry.wrapSentryHandleRequest(handleRequest);
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import Redis from 'ioredis';
import type { Route } from './+types/redis';

const redis = new Redis();
// workerd does not allow a socket connect at module scope, so the client is made in the loader.
let redis: Redis | undefined;

export async function loader() {
redis ??= new Redis();
const key = 'cache:greeting';
await redis.set(key, 'hello from react-router');
const value = await redis.get(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ async function getUser() {
};
}

const authMiddleware: Route.MiddlewareFunction = async ({ request, context }, next) => {
Sentry.startSpan({ name: 'authMiddleware', op: 'middleware.auth' }, async () => {
const authMiddleware: Route.MiddlewareFunction = async ({ context }, next) => {
return Sentry.startSpan({ name: 'authMiddleware', op: 'middleware.auth' }, async () => {
const user: User = await getUser();
context.set(userContext, user);
await next();
return next();
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,40 @@
"react-router": "^8.0.0",
"@react-router/node": "^8.0.0",
"@react-router/serve": "^8.0.0",
"@sentry/cloudflare": "file:../../packed/sentry-cloudflare-packed.tgz",
"@sentry/react-router": "file:../../packed/sentry-react-router-packed.tgz",
"ioredis": "^5.11.1",
"isbot": "^5.1.43"
},
"devDependencies": {
"@cloudflare/vite-plugin": "^1.35.0",
"@types/react": "19.2.17",
"@types/react-dom": "19.2.3",
"@types/node": "^22",
"@react-router/dev": "^8.0.0",
"@playwright/test": "~1.63.0",
"@sentry-internal/test-utils": "link:../../../test-utils",
"typescript": "^5.6.3",
"vite": "^7.3.2"
"vite": "^7.3.2",
"wrangler": "^4.120.0"
},
"scripts": {
"build": "react-router build",
"test:build-latest": "pnpm install && pnpm add react-router@latest && pnpm add @react-router/node@latest && pnpm add @react-router/serve@latest && pnpm build",
"dev": "NODE_OPTIONS='--import ./instrument.mjs' react-router dev",
"start": "NODE_ENV=production NODE_OPTIONS='--import ./instrument.mjs' react-router-serve ./build/server/index.js",
"start:bun": "NODE_ENV=production bun --bun --preload ./instrument.mjs ./node_modules/@react-router/serve/bin.cjs ./build/server/index.js",
"start:deno": "NODE_ENV=production deno run -A --preload ./instrument.mjs ./node_modules/@react-router/serve/bin.cjs ./build/server/index.js",
"start:cloudflare": "wrangler dev --config ./build/server/wrangler.json --port 3030",
"proxy": "node start-event-proxy.mjs",
"typecheck": "react-router typegen && tsc",
"clean": "npx rimraf node_modules pnpm-lock.yaml",
"test:build": "pnpm install && pnpm build",
"test:build:cloudflare": "pnpm install && react-router build --config vite.cloudflare.config.ts",
"test:assert": "pnpm test:ts && pnpm test:playwright",
"test:assert:bun": "RUNTIME=bun pnpm test:assert",
"test:assert:deno": "RUNTIME=deno pnpm test:assert",
"test:assert:cloudflare": "RUNTIME=cloudflare pnpm test:assert",
"test:ts": "pnpm typecheck",
"test:playwright": "playwright test"
},
Expand Down Expand Up @@ -64,6 +74,24 @@
"build-command": "pnpm test:build-latest",
"label": "react-router-8-framework (latest)"
}
],
"optionalVariants": [
{
"assert-command": "pnpm test:assert:bun",
"runtime": "bun",
"label": "react-router-8-framework (bun)"
},
{
"assert-command": "pnpm test:assert:deno",
"runtime": "deno",
"deno-version": "v2.9.0",
"label": "react-router-8-framework (deno)"
},
{
"build-command": "pnpm test:build:cloudflare",
"assert-command": "pnpm test:assert:cloudflare",
"label": "react-router-8-framework (cloudflare)"
}
]
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
import { getPlaywrightConfig } from '@sentry-internal/test-utils';
import { fileURLToPath } from 'url';

const RUNTIME = process.env.RUNTIME || 'node';

const startCommands = {
node: 'PORT=3030 pnpm start',
bun: 'PORT=3030 pnpm start:bun',
deno: 'PORT=3030 pnpm start:deno',
cloudflare: 'pnpm start:cloudflare',
};

const config = getPlaywrightConfig(
{
startCommand: `PORT=3030 pnpm start`,
startCommand: startCommands[RUNTIME],
port: 3030,
},
// Boot Redis before the tests run, outside the webServer startup-timeout window.
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
export type Runtime = 'node' | 'bun' | 'deno' | 'cloudflare';

export const RUNTIME = (process.env.RUNTIME || 'node') as Runtime;

export const APP_NAME = 'react-router-8-framework';
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, test } from '@playwright/test';
import { waitForError } from '@sentry-internal/test-utils';
import { APP_NAME } from '../constants';
import { APP_NAME, RUNTIME } from '../constants';

test.describe('server-side errors', () => {
test('captures error thrown in server loader', async ({ page }) => {
Expand All @@ -26,21 +26,23 @@ test.describe('server-side errors', () => {
},
],
},
// todo: should be 'GET /errors/server-loader'
transaction: 'GET /{*splat}',
// Express names the transaction on Node and Deno. Without an Express layer (Cloudflare, and Bun, where
// Express is not instrumented under `bun run`) it stays the request path.
// todo: should be 'GET /errors/server-loader' everywhere
transaction: RUNTIME === 'cloudflare' || RUNTIME === 'bun' ? 'GET /errors/server-loader' : 'GET /{*splat}',
request: {
url: expect.stringContaining('errors/server-loader'),
headers: expect.any(Object),
},
level: 'error',
platform: 'node',
platform: RUNTIME === 'cloudflare' ? 'javascript' : 'node',
environment: 'qa',
sdk: {
integrations: expect.any(Array<string>),
name: 'sentry.javascript.react-router',
name: RUNTIME === 'cloudflare' ? 'sentry.javascript.cloudflare' : 'sentry.javascript.react-router',
version: expect.any(String),
},
tags: { runtime: 'node' },
...(RUNTIME === 'cloudflare' ? {} : { tags: { runtime: 'node' } }),
contexts: {
trace: {
span_id: expect.any(String),
Expand Down Expand Up @@ -74,21 +76,23 @@ test.describe('server-side errors', () => {
},
],
},
// todo: should be 'POST /errors/server-action'
transaction: 'POST /{*splat}',
// Express names the transaction on Node and Deno. Without an Express layer (Cloudflare, and Bun, where
// Express is not instrumented under `bun run`) it stays the request path.
// todo: should be 'POST /errors/server-action' everywhere
transaction: RUNTIME === 'cloudflare' || RUNTIME === 'bun' ? 'POST /errors/server-action.data' : 'POST /{*splat}',
request: {
url: expect.stringContaining('errors/server-action'),
headers: expect.any(Object),
},
level: 'error',
platform: 'node',
platform: RUNTIME === 'cloudflare' ? 'javascript' : 'node',
environment: 'qa',
sdk: {
integrations: expect.any(Array<string>),
name: 'sentry.javascript.react-router',
name: RUNTIME === 'cloudflare' ? 'sentry.javascript.cloudflare' : 'sentry.javascript.react-router',
version: expect.any(String),
},
tags: { runtime: 'node' },
...(RUNTIME === 'cloudflare' ? {} : { tags: { runtime: 'node' } }),
contexts: {
trace: {
span_id: expect.any(String),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, test } from '@playwright/test';
import { getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils';
import { APP_NAME } from '../constants';
import { APP_NAME, RUNTIME } from '../constants';

test.describe('client - navigation performance', () => {
test('should create navigation span', async ({ page }) => {
Expand Down Expand Up @@ -88,10 +88,14 @@ test.describe('client - navigation performance', () => {
'sentry.op': { value: 'navigation', type: 'string' },
'sentry.origin': { value: 'auto.navigation.react_router', type: 'string' },
'url.template': { value: '/performance', type: 'string' },
// the initial pageload to `/performance` gets 301-redirected to a trailing slash by react-router-serve
'url.path': { value: '/performance/', type: 'string' },
// the initial pageload to `/performance` gets 301-redirected to a trailing slash by react-router-serve, workerd does not
'url.path': { value: RUNTIME === 'cloudflare' ? '/performance' : '/performance/', type: 'string' },
'url.full': {
value: expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/\?query=test$/),
value: expect.stringMatching(
RUNTIME === 'cloudflare'
? /^https?:\/\/localhost:\d+\/performance\?query=test$/
: /^https?:\/\/localhost:\d+\/performance\/\?query=test$/,
),
type: 'string',
},
});
Expand Down Expand Up @@ -166,9 +170,16 @@ test.describe('client - navigation performance', () => {
'sentry.op': { value: 'navigation', type: 'string' },
'sentry.origin': { value: 'auto.navigation.react_router', type: 'string' },
'url.template': { value: '/performance', type: 'string' },
// react-router-serve 301-redirects the bare index route to a trailing slash
'url.path': { value: '/performance/', type: 'string' },
'url.full': { value: expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/$/), type: 'string' },
// react-router-serve 301-redirects the bare index route to a trailing slash, workerd does not
'url.path': { value: RUNTIME === 'cloudflare' ? '/performance' : '/performance/', type: 'string' },
'url.full': {
value: expect.stringMatching(
RUNTIME === 'cloudflare'
? /^https?:\/\/localhost:\d+\/performance$/
: /^https?:\/\/localhost:\d+\/performance\/$/,
),
type: 'string',
},
});
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect, test } from '@playwright/test';
import { getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils';
import { APP_NAME } from '../constants';
import { APP_NAME, RUNTIME } from '../constants';

test.describe('client - pageload performance', () => {
test('should send pageload span', async ({ page }) => {
Expand Down Expand Up @@ -30,9 +30,16 @@ test.describe('client - pageload performance', () => {
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
'sentry.sdk.integrations': { value: expect.arrayContaining([expect.any(String)]), type: 'array' },
'url.template': { value: '/performance', type: 'string' },
// react-router-serve 301-redirects the bare index route to a trailing slash
'url.path': { value: '/performance/', type: 'string' },
'url.full': { value: expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/$/), type: 'string' },
// react-router-serve 301-redirects the bare index route to a trailing slash, workerd does not
'url.path': { value: RUNTIME === 'cloudflare' ? '/performance' : '/performance/', type: 'string' },
'url.full': {
value: expect.stringMatching(
RUNTIME === 'cloudflare'
? /^https?:\/\/localhost:\d+\/performance$/
: /^https?:\/\/localhost:\d+\/performance\/$/,
),
type: 'string',
},
});
});

Expand Down
Loading
Loading