From e7346d8bf25680997b0a93e2667d3e0b3dccb677 Mon Sep 17 00:00:00 2001 From: brkalow Date: Wed, 25 Mar 2026 11:57:10 -0500 Subject: [PATCH] fix(backend): harden proxy with abort signals, dynamic hop-by-hop stripping, and DELETE body support - Propagate client abort signal to upstream fetch to prevent zombie requests - Strip dynamic hop-by-hop headers listed in the Connection header (RFC 7230) - Support request bodies on DELETE (and any method), not just POST/PUT/PATCH - Add Cache-Control: no-store to error responses to prevent CDN caching - Only set duplex option when request has a body Co-Authored-By: Claude Opus 4.6 --- packages/backend/src/__tests__/proxy.test.ts | 100 +++++++++++++++++++ packages/backend/src/proxy.ts | 55 +++++++--- 2 files changed, 143 insertions(+), 12 deletions(-) diff --git a/packages/backend/src/__tests__/proxy.test.ts b/packages/backend/src/__tests__/proxy.test.ts index 672aaaf32b9..4d92f4cd32a 100644 --- a/packages/backend/src/__tests__/proxy.test.ts +++ b/packages/backend/src/__tests__/proxy.test.ts @@ -525,6 +525,106 @@ describe('proxy', () => { expect(response.headers.get('Content-Type')).toBe('application/javascript'); }); + it('forwards DELETE request with body', async () => { + const mockResponse = new Response(JSON.stringify({ deleted: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + mockFetch.mockResolvedValue(mockResponse); + + const requestBody = JSON.stringify({ id: '123' }); + const request = new Request('https://example.com/__clerk/v1/resource', { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + }, + body: requestBody, + }); + + const response = await clerkFrontendApiProxy(request, { + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + secretKey: 'sk_test_xxx', + }); + + expect(mockFetch).toHaveBeenCalledTimes(1); + const [, options] = mockFetch.mock.calls[0]; + + expect(options.method).toBe('DELETE'); + expect(options.body).not.toBeNull(); + expect(options.duplex).toBe('half'); + + expect(response.status).toBe(200); + }); + + it('propagates abort signal to upstream fetch', async () => { + const mockResponse = new Response(JSON.stringify({}), { status: 200 }); + mockFetch.mockResolvedValue(mockResponse); + + const controller = new AbortController(); + const request = new Request('https://example.com/__clerk/v1/client', { + signal: controller.signal, + }); + + await clerkFrontendApiProxy(request, { + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + secretKey: 'sk_test_xxx', + }); + + const [, options] = mockFetch.mock.calls[0]; + expect(options.signal).toBe(request.signal); + }); + + it('includes Cache-Control: no-store on error responses', async () => { + const request = new Request('https://example.com/__clerk/v1/client'); + + // Missing publishableKey triggers an error response + const response = await clerkFrontendApiProxy(request, { + secretKey: 'sk_test_xxx', + }); + + expect(response.status).toBe(500); + expect(response.headers.get('Cache-Control')).toBe('no-store'); + }); + + it('includes Cache-Control: no-store on 502 error responses', async () => { + mockFetch.mockRejectedValue(new Error('Network error')); + + const request = new Request('https://example.com/__clerk/v1/client'); + + const response = await clerkFrontendApiProxy(request, { + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + secretKey: 'sk_test_xxx', + }); + + expect(response.status).toBe(502); + expect(response.headers.get('Cache-Control')).toBe('no-store'); + }); + + it('strips dynamic hop-by-hop headers listed in the Connection header from requests', async () => { + const mockResponse = new Response(JSON.stringify({}), { status: 200 }); + mockFetch.mockResolvedValue(mockResponse); + + const request = new Request('https://example.com/__clerk/v1/client', { + headers: { + Connection: 'keep-alive, X-Custom-Hop', + 'X-Custom-Hop': 'some-value', + 'User-Agent': 'Test', + }, + }); + + await clerkFrontendApiProxy(request, { + publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k', + secretKey: 'sk_test_xxx', + }); + + const [, options] = mockFetch.mock.calls[0]; + // Connection and X-Custom-Hop should both be stripped + expect(options.headers.has('Connection')).toBe(false); + expect(options.headers.has('X-Custom-Hop')).toBe(false); + // Non-hop-by-hop headers should be preserved + expect(options.headers.get('User-Agent')).toBe('Test'); + }); + it('preserves relative Location headers', async () => { const mockResponse = new Response(null, { status: 302, diff --git a/packages/backend/src/proxy.ts b/packages/backend/src/proxy.ts index 7a47d2c3bf7..27bd8789fe1 100644 --- a/packages/backend/src/proxy.ts +++ b/packages/backend/src/proxy.ts @@ -43,7 +43,7 @@ export interface ProxyError { } // Hop-by-hop headers that should not be forwarded -const HOP_BY_HOP_HEADERS = [ +const HOP_BY_HOP_HEADERS = new Set([ 'connection', 'keep-alive', 'proxy-authenticate', @@ -52,7 +52,25 @@ const HOP_BY_HOP_HEADERS = [ 'trailer', 'transfer-encoding', 'upgrade', -]; +]); + +/** + * Parses the Connection header to extract dynamically-nominated hop-by-hop + * header names (RFC 7230 Section 6.1). These headers are specific to the + * current connection and must not be forwarded by proxies. + */ +function getDynamicHopByHopHeaders(headers: Headers): Set { + const connectionValue = headers.get('connection'); + if (!connectionValue) { + return new Set(); + } + return new Set( + connectionValue + .split(',') + .map(h => h.trim().toLowerCase()) + .filter(h => h.length > 0), + ); +} // Headers to strip from proxied responses. fetch() auto-decompresses // response bodies, so Content-Encoding no longer describes the body @@ -114,6 +132,7 @@ function createErrorResponse(code: ProxyErrorCode, message: string, status: numb status, headers: { 'Content-Type': 'application/json', + 'Cache-Control': 'no-store', }, }); } @@ -223,9 +242,12 @@ export async function clerkFrontendApiProxy(request: Request, options?: Frontend // Build headers for the proxied request const headers = new Headers(); - // Copy original headers, excluding hop-by-hop headers + // Copy original headers, excluding hop-by-hop headers and any + // dynamically-nominated hop-by-hop headers listed in the Connection header (RFC 7230 Section 6.1). + const dynamicHopByHop = getDynamicHopByHopHeaders(request.headers); request.headers.forEach((value, key) => { - if (!HOP_BY_HOP_HEADERS.includes(key.toLowerCase())) { + const lower = key.toLowerCase(); + if (!HOP_BY_HOP_HEADERS.has(lower) && !dynamicHopByHop.has(lower)) { headers.set(key, value); } }); @@ -264,30 +286,39 @@ export async function clerkFrontendApiProxy(request: Request, options?: Frontend headers.set('X-Forwarded-For', clientIp); } - // Determine if request has a body - const hasBody = ['POST', 'PUT', 'PATCH'].includes(request.method); + // Determine if request has a body (handles DELETE-with-body and any other method) + const hasBody = request.body !== null; try { // Make the proxied request + // TODO: Consider adding AbortSignal.timeout(30_000) via AbortSignal.any() + // once it's available across all target runtimes. const fetchOptions: RequestInit = { method: request.method, headers, - // @ts-expect-error - duplex is required for streaming bodies but not in all TS definitions - duplex: hasBody ? 'half' : undefined, + signal: request.signal, }; - // Only include body for methods that support it - if (hasBody && request.body) { + // Only set duplex when body is present (required for streaming bodies) + if (hasBody) { + // @ts-expect-error - duplex is required for streaming bodies but not in all TS definitions + fetchOptions.duplex = 'half'; fetchOptions.body = request.body; } const response = await fetch(targetUrl.toString(), fetchOptions); - // Build response headers, excluding hop-by-hop and encoding headers + // Build response headers, excluding hop-by-hop and encoding headers. + // Also strip dynamically-nominated hop-by-hop headers from the response Connection header. + const responseDynamicHopByHop = getDynamicHopByHopHeaders(response.headers); const responseHeaders = new Headers(); response.headers.forEach((value, key) => { const lower = key.toLowerCase(); - if (!HOP_BY_HOP_HEADERS.includes(lower) && !RESPONSE_HEADERS_TO_STRIP.includes(lower)) { + if ( + !HOP_BY_HOP_HEADERS.has(lower) && + !RESPONSE_HEADERS_TO_STRIP.includes(lower) && + !responseDynamicHopByHop.has(lower) + ) { responseHeaders.set(key, value); } });