From e130df1c7ab1e42244d27ecfeded6b386ce88bb5 Mon Sep 17 00:00:00 2001 From: Levi Wilkerson Date: Thu, 15 Oct 2026 01:47:30 -0400 Subject: [PATCH] fix: keep the HERE key and queries out of Sentry spans The HERE proxy called the global fetch, which Sentry wraps on the first request, so every /geocode and /autocomplete span and breadcrumb recorded the upstream URL: the API key and what the user typed. Capture fetch at module load, as route planning, APNs, FCM, and analytics already do. --- src/index.test.ts | 18 ++++++++++-------- src/proxy.test.ts | 41 +++++++++++++++++++++++++++++++---------- src/proxy.ts | 9 ++++++++- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index 71be219..19643a2 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -4,6 +4,14 @@ import { RATE_LIMIT_PERIOD_SECONDS, rateLimitKey } from './rateLimit' vi.mock('cloudflare:workers', () => ({ DurableObject: class {} })) +// The HERE proxy captures `fetch` when its module loads, so its stub has to be +// in place before the import below. +const here = vi.hoisted(() => { + const state: { fetch?: typeof fetch } = {} + globalThis.fetch = ((...args: Parameters) => state.fetch!(...args)) as typeof fetch + return state +}) + import worker from './index' const NOW = Date.UTC(2026, 9, 8, 12, 0, 0, 250) @@ -84,10 +92,7 @@ describe('error envelope', () => { describe('per-IP rate limits', () => { it('refuses with 429 rate_limited and Retry-After from the limiter window', async () => { const env = makeEnv() - vi.stubGlobal( - 'fetch', - vi.fn(async () => Response.json({ items: [] })) - ) + here.fetch = vi.fn(async () => Response.json({ items: [] })) expect((await call(env, '/autocomplete?q=Main')).status).toBe(200) const refused = await call(env, '/geocode?q=Main') @@ -106,10 +111,7 @@ describe('per-IP rate limits', () => { it('counts each route family in its own bucket for the same IP', async () => { const limiter = singleUseLimiter() const env = makeEnv(limiter) - vi.stubGlobal( - 'fetch', - vi.fn(async () => Response.json({ items: [] })) - ) + here.fetch = vi.fn(async () => Response.json({ items: [] })) expect((await call(env, '/geocode?q=Main')).status).toBe(200) expect((await call(env, '/geocode?q=Main')).status).toBe(429) diff --git a/src/proxy.test.ts b/src/proxy.test.ts index ad9838d..88f8fa4 100644 --- a/src/proxy.test.ts +++ b/src/proxy.test.ts @@ -3,6 +3,18 @@ import type { Environment } from './types' vi.mock('cloudflare:workers', () => ({ DurableObject: class {} })) +// The proxy captures `fetch` when its module loads, before Sentry wraps the +// global, so HERE's stub has to be in place before the import below. +const here = vi.hoisted(() => { + const state: { fetch?: typeof fetch } = {} + globalThis.fetch = ((...args: Parameters) => state.fetch!(...args)) as typeof fetch + return state +}) + +const stubHere = (mock: unknown) => { + here.fetch = mock as typeof fetch +} + import worker from './index' const ENV = { @@ -22,9 +34,22 @@ async function geocode() { afterEach(() => vi.unstubAllGlobals()) describe('HERE API proxy', () => { + it('bypasses the global fetch that Sentry instruments', async () => { + const hereMock = vi.fn(async () => Response.json({ items: [] })) + stubHere(hereMock) + const instrumented = vi.fn(async () => Response.json({})) + vi.stubGlobal('fetch', instrumented) + + const response = await geocode() + + expect(response.status).toBe(200) + expect(hereMock).toHaveBeenCalledOnce() + expect(instrumented).not.toHaveBeenCalled() + }) + it('passes JSON responses through with the upstream status', async () => { const fetchMock = vi.fn(async () => Response.json({ items: [] }, { status: 200 })) - vi.stubGlobal('fetch', fetchMock) + stubHere(fetchMock) const response = await geocode() @@ -35,7 +60,7 @@ describe('HERE API proxy', () => { }) it('returns 502 when HERE returns a plain-text Cloudflare error page', async () => { - vi.stubGlobal('fetch', vi.fn(async () => new Response('error code: 520\n', { status: 520 }))) + stubHere(vi.fn(async () => new Response('error code: 520\n', { status: 520 }))) const response = await geocode() @@ -48,8 +73,7 @@ describe('HERE API proxy', () => { }) it('keeps HERE’s error body and adds the envelope and Retry-After on 429', async () => { - vi.stubGlobal( - 'fetch', + stubHere( vi.fn(async () => Response.json( { status: 429, title: 'Too Many Requests' }, @@ -73,8 +97,7 @@ describe('HERE API proxy', () => { }) it('defaults Retry-After when HERE is unavailable without one', async () => { - vi.stubGlobal( - 'fetch', + stubHere( vi.fn(async () => Response.json({ title: 'Unavailable' }, { status: 503 })) ) @@ -89,8 +112,7 @@ describe('HERE API proxy', () => { }) it('passes a HERE 400 through with a bad_request code and no Retry-After', async () => { - vi.stubGlobal( - 'fetch', + stubHere( vi.fn(async () => Response.json({ title: "Illegal input for parameter 'q'" }, { status: 400 }) ) @@ -109,8 +131,7 @@ describe('HERE API proxy', () => { }) it('returns 502 upstream_error when HERE can’t be reached', async () => { - vi.stubGlobal( - 'fetch', + stubHere( vi.fn(async () => { throw new TypeError('Network connection lost.') }) diff --git a/src/proxy.ts b/src/proxy.ts index 3682e81..883b112 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -70,8 +70,15 @@ function buildHereApiUrl(baseUrl: string, params: URLSearchParams): string { return `${baseUrl}?${params.toString()}`; } +/** + * Captured at module evaluation, before Sentry's fetch integration wraps + * `globalThis.fetch`. The wrapped fetch records each request URL as a span and + * breadcrumb, and HERE's URL carries both the API key and what the user typed. + */ +const uninstrumentedFetch: typeof fetch = globalThis.fetch.bind(globalThis); + async function fetchFromHereApi(url: string): Promise { - return fetch(url); + return uninstrumentedFetch(url); } /** Returns the parsed body, or `undefined` when it is not valid JSON. */