Skip to content

Commit 849f198

Browse files
brkalowclaudejacekradko
authored
fix(backend): harden FAPI proxy resilience and spec compliance (#8163)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Jacek <jacek@clerk.dev>
1 parent 1261d8e commit 849f198

File tree

3 files changed

+148
-13
lines changed

3 files changed

+148
-13
lines changed

.changeset/tough-ghosts-ask.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/backend': patch
3+
---
4+
5+
Improve the built-in Clerk Frontend API proxy, adding support for abort signals and addressing a number of small edge cases.

packages/backend/src/__tests__/proxy.test.ts

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,106 @@ describe('proxy', () => {
541541
expect(response.headers.get('Content-Type')).toBe('application/javascript');
542542
});
543543

544+
it('forwards DELETE request with body', async () => {
545+
const mockResponse = new Response(JSON.stringify({ deleted: true }), {
546+
status: 200,
547+
headers: { 'Content-Type': 'application/json' },
548+
});
549+
mockFetch.mockResolvedValue(mockResponse);
550+
551+
const requestBody = JSON.stringify({ id: '123' });
552+
const request = new Request('https://example.com/__clerk/v1/resource', {
553+
method: 'DELETE',
554+
headers: {
555+
'Content-Type': 'application/json',
556+
},
557+
body: requestBody,
558+
});
559+
560+
const response = await clerkFrontendApiProxy(request, {
561+
publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k',
562+
secretKey: 'sk_test_xxx',
563+
});
564+
565+
expect(mockFetch).toHaveBeenCalledTimes(1);
566+
const [, options] = mockFetch.mock.calls[0];
567+
568+
expect(options.method).toBe('DELETE');
569+
expect(options.body).not.toBeNull();
570+
expect(options.duplex).toBe('half');
571+
572+
expect(response.status).toBe(200);
573+
});
574+
575+
it('propagates abort signal to upstream fetch', async () => {
576+
const mockResponse = new Response(JSON.stringify({}), { status: 200 });
577+
mockFetch.mockResolvedValue(mockResponse);
578+
579+
const controller = new AbortController();
580+
const request = new Request('https://example.com/__clerk/v1/client', {
581+
signal: controller.signal,
582+
});
583+
584+
await clerkFrontendApiProxy(request, {
585+
publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k',
586+
secretKey: 'sk_test_xxx',
587+
});
588+
589+
const [, options] = mockFetch.mock.calls[0];
590+
expect(options.signal).toBe(request.signal);
591+
});
592+
593+
it('includes Cache-Control: no-store on error responses', async () => {
594+
const request = new Request('https://example.com/__clerk/v1/client');
595+
596+
// Missing publishableKey triggers an error response
597+
const response = await clerkFrontendApiProxy(request, {
598+
secretKey: 'sk_test_xxx',
599+
});
600+
601+
expect(response.status).toBe(500);
602+
expect(response.headers.get('Cache-Control')).toBe('no-store');
603+
});
604+
605+
it('includes Cache-Control: no-store on 502 error responses', async () => {
606+
mockFetch.mockRejectedValue(new Error('Network error'));
607+
608+
const request = new Request('https://example.com/__clerk/v1/client');
609+
610+
const response = await clerkFrontendApiProxy(request, {
611+
publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k',
612+
secretKey: 'sk_test_xxx',
613+
});
614+
615+
expect(response.status).toBe(502);
616+
expect(response.headers.get('Cache-Control')).toBe('no-store');
617+
});
618+
619+
it('strips dynamic hop-by-hop headers listed in the Connection header from requests', async () => {
620+
const mockResponse = new Response(JSON.stringify({}), { status: 200 });
621+
mockFetch.mockResolvedValue(mockResponse);
622+
623+
const request = new Request('https://example.com/__clerk/v1/client', {
624+
headers: {
625+
Connection: 'keep-alive, X-Custom-Hop',
626+
'X-Custom-Hop': 'some-value',
627+
'User-Agent': 'Test',
628+
},
629+
});
630+
631+
await clerkFrontendApiProxy(request, {
632+
publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k',
633+
secretKey: 'sk_test_xxx',
634+
});
635+
636+
const [, options] = mockFetch.mock.calls[0];
637+
// Connection and X-Custom-Hop should both be stripped
638+
expect(options.headers.has('Connection')).toBe(false);
639+
expect(options.headers.has('X-Custom-Hop')).toBe(false);
640+
// Non-hop-by-hop headers should be preserved
641+
expect(options.headers.get('User-Agent')).toBe('Test');
642+
});
643+
544644
it('preserves multiple Set-Cookie headers from FAPI response', async () => {
545645
const headers = new Headers();
546646
headers.append('Set-Cookie', '__client=abc123; Path=/; HttpOnly; Secure');

packages/backend/src/proxy.ts

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export interface ProxyError {
4343
}
4444

4545
// Hop-by-hop headers that should not be forwarded
46-
const HOP_BY_HOP_HEADERS = [
46+
const HOP_BY_HOP_HEADERS = new Set([
4747
'connection',
4848
'keep-alive',
4949
'proxy-authenticate',
@@ -52,14 +52,32 @@ const HOP_BY_HOP_HEADERS = [
5252
'trailer',
5353
'transfer-encoding',
5454
'upgrade',
55-
];
55+
]);
56+
57+
/**
58+
* Parses the Connection header to extract dynamically-nominated hop-by-hop
59+
* header names (RFC 7230 Section 6.1). These headers are specific to the
60+
* current connection and must not be forwarded by proxies.
61+
*/
62+
function getDynamicHopByHopHeaders(headers: Headers): Set<string> {
63+
const connectionValue = headers.get('connection');
64+
if (!connectionValue) {
65+
return new Set();
66+
}
67+
return new Set(
68+
connectionValue
69+
.split(',')
70+
.map(h => h.trim().toLowerCase())
71+
.filter(h => h.length > 0),
72+
);
73+
}
5674

5775
// Headers to strip from proxied responses. fetch() auto-decompresses
5876
// response bodies, so Content-Encoding no longer describes the body
5977
// and Content-Length reflects the compressed size. We request identity
6078
// encoding upstream to avoid the double compression pass, but strip
6179
// these defensively since servers may ignore Accept-Encoding: identity.
62-
const RESPONSE_HEADERS_TO_STRIP = ['content-encoding', 'content-length'];
80+
const RESPONSE_HEADERS_TO_STRIP = new Set(['content-encoding', 'content-length']);
6381

6482
/**
6583
* Derives the Frontend API URL from a publishable key.
@@ -114,6 +132,7 @@ function createErrorResponse(code: ProxyErrorCode, message: string, status: numb
114132
status,
115133
headers: {
116134
'Content-Type': 'application/json',
135+
'Cache-Control': 'no-store',
117136
},
118137
});
119138
}
@@ -230,9 +249,12 @@ export async function clerkFrontendApiProxy(request: Request, options?: Frontend
230249
// Build headers for the proxied request
231250
const headers = new Headers();
232251

233-
// Copy original headers, excluding hop-by-hop headers
252+
// Copy original headers, excluding hop-by-hop headers and any
253+
// dynamically-nominated hop-by-hop headers listed in the Connection header (RFC 7230 Section 6.1).
254+
const dynamicHopByHop = getDynamicHopByHopHeaders(request.headers);
234255
request.headers.forEach((value, key) => {
235-
if (!HOP_BY_HOP_HEADERS.includes(key.toLowerCase())) {
256+
const lower = key.toLowerCase();
257+
if (!HOP_BY_HOP_HEADERS.has(lower) && !dynamicHopByHop.has(lower)) {
236258
headers.set(key, value);
237259
}
238260
});
@@ -270,31 +292,39 @@ export async function clerkFrontendApiProxy(request: Request, options?: Frontend
270292
headers.set('X-Forwarded-For', clientIp);
271293
}
272294

273-
// Determine if request has a body
274-
const hasBody = ['POST', 'PUT', 'PATCH'].includes(request.method);
295+
// Determine if request has a body (handles DELETE-with-body and any other method)
296+
const hasBody = request.body !== null;
275297

276298
try {
277299
// Make the proxied request
300+
// TODO: Consider adding AbortSignal.timeout(30_000) via AbortSignal.any()
278301
const fetchOptions: RequestInit = {
279302
method: request.method,
280303
headers,
281304
redirect: 'manual',
282-
// @ts-expect-error - duplex is required for streaming bodies but not in all TS definitions
283-
duplex: hasBody ? 'half' : undefined,
305+
signal: request.signal,
284306
};
285307

286-
// Only include body for methods that support it
287-
if (hasBody && request.body) {
308+
// Only set duplex when body is present (required for streaming bodies)
309+
if (hasBody) {
310+
// @ts-expect-error - duplex is required for streaming bodies, but not present on the RequestInit type from undici
311+
fetchOptions.duplex = 'half';
288312
fetchOptions.body = request.body;
289313
}
290314

291315
const response = await fetch(targetUrl.toString(), fetchOptions);
292316

293-
// Build response headers, excluding hop-by-hop and encoding headers
317+
// Build response headers, excluding hop-by-hop and encoding headers.
318+
// Also strip dynamically-nominated hop-by-hop headers from the response Connection header.
319+
const responseDynamicHopByHop = getDynamicHopByHopHeaders(response.headers);
294320
const responseHeaders = new Headers();
295321
response.headers.forEach((value, key) => {
296322
const lower = key.toLowerCase();
297-
if (!HOP_BY_HOP_HEADERS.includes(lower) && !RESPONSE_HEADERS_TO_STRIP.includes(lower)) {
323+
if (
324+
!HOP_BY_HOP_HEADERS.has(lower) &&
325+
!RESPONSE_HEADERS_TO_STRIP.has(lower) &&
326+
!responseDynamicHopByHop.has(lower)
327+
) {
298328
if (lower === 'set-cookie') {
299329
responseHeaders.append(key, value);
300330
} else {

0 commit comments

Comments
 (0)