fix(core): Only consider ingest endpoint requests when checking isSentryRequestUrl (#17393)

This PR makes a change to our `isSentryRequestUrl` utility which is used
in various parts of the SDKs.

The function checks both, the DSN as well as the `tunnel` option to
determine if a request URL is a URL to Sentry. I would argue, we should
only return `true` for requests to Sentry's ingest endpoint. For
example, if users make regular requests to the Sentry API from within
their app that uses a Sentry SDK, we should not special case that
request.

Therefore, this PR makes the check for the request URL more specific: 
- If `tunnel` is not provided, return `true` iff the the url includes
the host of the DSN AND if it includes the `sentry_key` query param.
This param is mandatory to be sent along, as it's equal to the [public
key of the DSN
](https://develop.sentry.dev/sdk/overview/#parsing-the-dsn).
- If `tunnel` is provided, the check was already specific enough because
the request URL has to match _exactly_ the configured tunnel URL.

While writing this, I realized there are still a bunch of edge cases
here that we probably also should fix but for now, let's keep things
atomic.

closes #17385
(^ very likely. We didn't repro this specifically but the
`httpClientIntegration` bails out exactly if it hits the
`isSentryRequestUrl` check)

Author: Lms24

packages/cloudflare/test/integrations/fetch.test.ts

@@ -101,8 +101,8 @@ describe('WinterCGFetch instrumentation', () => {
     expect(fetchInstrumentationHandlerCallback).toBeDefined();
 
     const startHandlerData: HandlerDataFetch = {
-      fetchData: { url: 'https://dsn.ingest.sentry.io/1337', method: 'POST' },
-      args: ['https://dsn.ingest.sentry.io/1337'],
+      fetchData: { url: 'https://dsn.ingest.sentry.io/1337?sentry_key=123', method: 'POST' },
+      args: ['https://dsn.ingest.sentry.io/1337?sentry_key=123'],
       startTimestamp: Date.now(),
     };
     fetchInstrumentationHandlerCallback(startHandlerData);

packages/core/src/utils/isSentryRequestUrl.ts

@@ -1,5 +1,6 @@
 import type { Client } from '../client';
 import type { DsnComponents } from '../types-hoist/dsn';
+import { isURLObjectRelative, parseStringToURLObject } from './url';
 
 /**
  * Checks whether given url points to Sentry server
@@ -21,7 +22,17 @@ function checkTunnel(url: string, tunnel: string | undefined): boolean {
 }
 
 function checkDsn(url: string, dsn: DsnComponents | undefined): boolean {
-  return dsn ? url.includes(dsn.host) : false;
+  // Requests to Sentry's ingest endpoint must have a `sentry_key` in the query string
+  // This is equivalent to the public_key which is required in the DSN
+  // see https://develop.sentry.dev/sdk/overview/#parsing-the-dsn
+  // Therefore, a request to the same host and with a `sentry_key` in the query string
+  // can be considered a request to the ingest endpoint.
+  const urlParts = parseStringToURLObject(url);
+  if (!urlParts || isURLObjectRelative(urlParts)) {
+    return false;
+  }
+
+  return dsn ? urlParts.host.includes(dsn.host) && /(^|&|\?)sentry_key=/.test(urlParts.search) : false;
 }
 
 function removeTrailingSlash(str: string): string {

packages/core/test/lib/utils/isSentryRequestUrl.test.ts

@@ -4,20 +4,46 @@ import type { Client } from '../../../src/client';
 
 describe('isSentryRequestUrl', () => {
   it.each([
-    ['', 'sentry-dsn.com', '', false],
-    ['http://sentry-dsn.com/my-url', 'sentry-dsn.com', '', true],
-    ['http://sentry-dsn.com', 'sentry-dsn.com', '', true],
-    ['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200', true],
-    ['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200/', true],
-    ['http://tunnel:4200/', 'sentry-dsn.com', 'http://tunnel:4200', true],
-    ['http://tunnel:4200/a', 'sentry-dsn.com', 'http://tunnel:4200', false],
-  ])('works with url=%s, dsn=%s, tunnel=%s', (url: string, dsn: string, tunnel: string, expected: boolean) => {
+    ['http://sentry-dsn.com/my-url?sentry_key=123', 'sentry-dsn.com', ''],
+
+    ['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200'],
+    ['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200/'],
+    ['http://tunnel:4200/', 'sentry-dsn.com', 'http://tunnel:4200'],
+    ['http://tunnel:4200/', 'another-dsn.com', 'http://tunnel:4200'],
+  ])('returns `true` for url=%s, dsn=%s, tunnel=%s', (url: string, dsn: string, tunnel: string) => {
+    const client = {
+      getOptions: () => ({ tunnel }),
+      getDsn: () => ({ host: dsn }),
+    } as unknown as Client;
+
+    expect(isSentryRequestUrl(url, client)).toBe(true);
+  });
+
+  it.each([
+    ['http://tunnel:4200/?sentry_key=123', 'another-dsn.com', ''],
+    ['http://sentry-dsn.com/my-url', 'sentry-dsn.com', ''],
+    ['http://sentry-dsn.com', 'sentry-dsn.com', ''],
+    ['http://sAntry-dsn.com/?sentry_key=123', 'sentry-dsn.com', ''],
+    ['http://sAntry-dsn.com/?sAntry_key=123', 'sAntry-dsn.com', ''],
+    ['/ingest', 'sentry-dsn.com', ''],
+    ['/ingest?sentry_key=123', 'sentry-dsn.com', ''],
+    ['/ingest', '', ''],
+    ['', '', ''],
+    ['', 'sentry-dsn.com', ''],
+
+    ['http://tunnel:4200/', 'another-dsn.com', 'http://tunnel:4200/sentry-tunnel'],
+    ['http://tunnel:4200/a', 'sentry-dsn.com', 'http://tunnel:4200'],
+    ['http://tunnel:4200/a', '', 'http://tunnel:4200/'],
+  ])('returns `false` for url=%s, dsn=%s, tunnel=%s', (url: string, dsn: string, tunnel: string) => {
     const client = {
       getOptions: () => ({ tunnel }),
       getDsn: () => ({ host: dsn }),
     } as unknown as Client;
 
-    // Works with client passed
-    expect(isSentryRequestUrl(url, client)).toBe(expected);
+    expect(isSentryRequestUrl(url, client)).toBe(false);
+  });
+
+  it('handles undefined client', () => {
+    expect(isSentryRequestUrl('http://sentry-dsn.com/my-url?sentry_key=123', undefined)).toBe(false);
   });
 });

packages/replay-internal/test/integration/shouldFilterRequest.test.ts

@@ -20,6 +20,6 @@ describe('Integration | shouldFilterRequest', () => {
   it('should filter requests for Sentry ingest URLs', async () => {
     const { replay } = await mockSdk();
 
-    expect(shouldFilterRequest(replay, 'https://03031aa.ingest.f00.f00/api/129312/')).toBe(true);
+    expect(shouldFilterRequest(replay, 'https://03031aa.ingest.f00.f00/api/129312/?sentry_key=123')).toBe(true);
   });
 });

packages/vercel-edge/test/wintercg-fetch.test.ts

@@ -102,8 +102,8 @@ describe('WinterCGFetch instrumentation', () => {
     expect(fetchInstrumentationHandlerCallback).toBeDefined();
 
     const startHandlerData: HandlerDataFetch = {
-      fetchData: { url: 'https://dsn.ingest.sentry.io/1337', method: 'POST' },
-      args: ['https://dsn.ingest.sentry.io/1337'],
+      fetchData: { url: 'https://dsn.ingest.sentry.io/1337?sentry_key=123', method: 'POST' },
+      args: ['https://dsn.ingest.sentry.io/1337?sentry_key=123'],
       startTimestamp: Date.now(),
     };
     fetchInstrumentationHandlerCallback(startHandlerData);