From 4a2435a7dc5076d48858bc14f23049f846c480f3 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Wed, 12 Aug 2026 15:00:15 -0700 Subject: [PATCH 1/2] fix(app): allow credentials on reverse-proxy CORS preflight After #7164, app-dev answers OPTIONS itself and omitted Access-Control-Allow-Credentials, so credentialed cross-origin fetches failed. When Origin is present, include Allow-Credentials on the preflight response. --- .changeset/cors-proxy-credentials.md | 5 +++++ .../app/src/cli/utilities/app/http-reverse-proxy.test.ts | 2 ++ packages/app/src/cli/utilities/app/http-reverse-proxy.ts | 4 +++- 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 .changeset/cors-proxy-credentials.md diff --git a/.changeset/cors-proxy-credentials.md b/.changeset/cors-proxy-credentials.md new file mode 100644 index 00000000000..7a69c2c506d --- /dev/null +++ b/.changeset/cors-proxy-credentials.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Include Access-Control-Allow-Credentials on app-dev reverse-proxy CORS preflights so credentialed cross-origin fetches succeed. diff --git a/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts b/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts index 7d26a2bf0c8..b6565fdc2c8 100644 --- a/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts +++ b/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts @@ -70,6 +70,7 @@ describe.sequential.each(each)('http-reverse-proxy for %s', (protocol) => { expect(response.headers.get('access-control-allow-methods')).toBe('GET') expect(response.headers.get('access-control-allow-headers')).toBe('Authorization') expect(response.headers.get('access-control-max-age')).toBe('86400') + expect(response.headers.get('access-control-allow-credentials')).toBe('true') }) test('responds to CORS preflight OPTIONS with defaults when no request headers', {retry: 2}, async ({setup}) => { @@ -81,6 +82,7 @@ describe.sequential.each(each)('http-reverse-proxy for %s', (protocol) => { expect(response.headers.get('access-control-allow-origin')).toBe('*') expect(response.headers.get('access-control-allow-methods')).toBe('GET, POST, PUT, DELETE, PATCH, OPTIONS') expect(response.headers.get('access-control-allow-headers')).toBe('Content-Type, Authorization') + expect(response.headers.get('access-control-allow-credentials')).toBeNull() }) test('closes the server when aborted', async ({setup}) => { diff --git a/packages/app/src/cli/utilities/app/http-reverse-proxy.ts b/packages/app/src/cli/utilities/app/http-reverse-proxy.ts index a0380b05995..bb9ec41c85a 100644 --- a/packages/app/src/cli/utilities/app/http-reverse-proxy.ts +++ b/packages/app/src/cli/utilities/app/http-reverse-proxy.ts @@ -74,13 +74,15 @@ function getProxyServerRequestListener( // The proxy does not forward OPTIONS reliably, so we respond here // using the headers requested by the client. if (req.method === 'OPTIONS') { + const origin = req.headers.origin res.writeHead(204, { - 'Access-Control-Allow-Origin': req.headers.origin ?? '*', + 'Access-Control-Allow-Origin': origin ?? '*', 'Access-Control-Allow-Methods': req.headers['access-control-request-method'] ?? 'GET, POST, PUT, DELETE, PATCH, OPTIONS', 'Access-Control-Allow-Headers': req.headers['access-control-request-headers'] ?? 'Content-Type, Authorization', 'Access-Control-Max-Age': '86400', + ...(origin ? {'Access-Control-Allow-Credentials': 'true'} : {}), }) return res.end() } From d4cb0313710b6f5c65cd1fde92e66ef730e2be68 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Fri, 11 Sep 2026 01:13:05 +0000 Subject: [PATCH 2/2] fix(app): delegate CORS preflight policy to target --- .changeset/cors-proxy-credentials.md | 2 +- .../utilities/app/http-reverse-proxy.test.ts | 72 ++++++++++++++----- .../cli/utilities/app/http-reverse-proxy.ts | 16 ----- 3 files changed, 57 insertions(+), 33 deletions(-) diff --git a/.changeset/cors-proxy-credentials.md b/.changeset/cors-proxy-credentials.md index 7a69c2c506d..33a62e48d3a 100644 --- a/.changeset/cors-proxy-credentials.md +++ b/.changeset/cors-proxy-credentials.md @@ -2,4 +2,4 @@ '@shopify/app': patch --- -Include Access-Control-Allow-Credentials on app-dev reverse-proxy CORS preflights so credentialed cross-origin fetches succeed. +Forward app-dev preflight requests so target servers can safely authorize credentialed cross-origin fetches. diff --git a/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts b/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts index b6565fdc2c8..417e272728a 100644 --- a/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts +++ b/packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts @@ -8,6 +8,7 @@ import https from 'https' import net from 'net' const each = ['http', 'https'] as const +const authorizedOrigin = 'https://extensions.shopifycdn.com' describe.sequential.each(each)('http-reverse-proxy for %s', (protocol) => { const test = getTestReverseProxy(protocol) @@ -55,34 +56,58 @@ describe.sequential.each(each)('http-reverse-proxy for %s', (protocol) => { }) }) - test('responds to CORS preflight OPTIONS with default headers', {retry: 2}, async ({setup}) => { + test('forwards an authorized credentialed CORS preflight to the target', {retry: 2}, async ({setup}) => { const response = await fetch(`${protocol}://localhost:${setup.proxyPort}/path1/test`, { method: 'OPTIONS', headers: { - Origin: 'https://extensions.shopifycdn.com', - 'Access-Control-Request-Method': 'GET', - 'Access-Control-Request-Headers': 'Authorization', + Origin: authorizedOrigin, + 'Access-Control-Request-Method': 'POST', + 'Access-Control-Request-Headers': 'Content-Type', }, agent, }) expect(response.status).toBe(204) - expect(response.headers.get('access-control-allow-origin')).toBe('https://extensions.shopifycdn.com') - expect(response.headers.get('access-control-allow-methods')).toBe('GET') - expect(response.headers.get('access-control-allow-headers')).toBe('Authorization') - expect(response.headers.get('access-control-max-age')).toBe('86400') + expect(response.headers.get('x-preflight-handled-by')).toBe('target') + expect(response.headers.get('access-control-allow-origin')).toBe(authorizedOrigin) + expect(response.headers.get('access-control-allow-methods')).toBe('POST') + expect(response.headers.get('access-control-allow-headers')).toBe('Content-Type') expect(response.headers.get('access-control-allow-credentials')).toBe('true') }) - test('responds to CORS preflight OPTIONS with defaults when no request headers', {retry: 2}, async ({setup}) => { - const response = await fetch(`${protocol}://localhost:${setup.proxyPort}/path1/test`, { - method: 'OPTIONS', - agent, - }) + test('does not authorize credentials for untrusted origins', {retry: 2}, async ({setup}) => { + const untrustedOrigins = [ + 'https://evil.example', + 'null', + 'not a valid origin', + 'http://extensions.shopifycdn.com', + 'https://extensions.shopifycdn.com:3000', + ] + + await Promise.all( + untrustedOrigins.map(async (origin) => { + const response = await fetch(`${protocol}://localhost:${setup.proxyPort}/path1/test`, { + method: 'OPTIONS', + headers: { + Origin: origin, + 'Access-Control-Request-Method': 'POST', + 'Access-Control-Request-Headers': 'Content-Type', + }, + agent, + }) + expect(response.status).toBe(204) + expect(response.headers.get('access-control-allow-origin')).toBeNull() + expect(response.headers.get('access-control-allow-credentials')).toBeNull() + expect(response.headers.get('x-preflight-handled-by')).toBe('target') + }), + ) + }) + + test('forwards OPTIONS without an Origin without authorizing credentials', {retry: 2}, async ({setup}) => { + const response = await fetch(`${protocol}://localhost:${setup.proxyPort}/path1/test`, {method: 'OPTIONS', agent}) expect(response.status).toBe(204) - expect(response.headers.get('access-control-allow-origin')).toBe('*') - expect(response.headers.get('access-control-allow-methods')).toBe('GET, POST, PUT, DELETE, PATCH, OPTIONS') - expect(response.headers.get('access-control-allow-headers')).toBe('Content-Type, Authorization') + expect(response.headers.get('access-control-allow-origin')).toBeNull() expect(response.headers.get('access-control-allow-credentials')).toBeNull() + expect(response.headers.get('x-preflight-handled-by')).toBe('target') }) test('closes the server when aborted', async ({setup}) => { @@ -115,6 +140,21 @@ function getTestReverseProxy(protocol: 'http' | 'https') { // eslint-disable-next-line no-empty-pattern setup: async ({}, use) => { const targetServer1 = http.createServer((req, res) => { + if (req.method === 'OPTIONS') { + const origin = req.headers.origin + res.writeHead(204, { + 'X-Preflight-Handled-By': 'target', + ...(origin === authorizedOrigin + ? { + 'Access-Control-Allow-Origin': origin, + 'Access-Control-Allow-Credentials': 'true', + 'Access-Control-Allow-Methods': req.headers['access-control-request-method'] ?? '', + 'Access-Control-Allow-Headers': req.headers['access-control-request-headers'] ?? '', + } + : {}), + }) + return res.end() + } res.writeHead(200, {'Content-Type': 'text/plain'}) res.end('Response from target server 1') }) diff --git a/packages/app/src/cli/utilities/app/http-reverse-proxy.ts b/packages/app/src/cli/utilities/app/http-reverse-proxy.ts index bb9ec41c85a..9304bab8e84 100644 --- a/packages/app/src/cli/utilities/app/http-reverse-proxy.ts +++ b/packages/app/src/cli/utilities/app/http-reverse-proxy.ts @@ -70,22 +70,6 @@ function getProxyServerRequestListener( return function (req, res) { const target = match(rules, req) if (target) { - // Handle CORS preflight requests directly - // The proxy does not forward OPTIONS reliably, so we respond here - // using the headers requested by the client. - if (req.method === 'OPTIONS') { - const origin = req.headers.origin - res.writeHead(204, { - 'Access-Control-Allow-Origin': origin ?? '*', - 'Access-Control-Allow-Methods': - req.headers['access-control-request-method'] ?? 'GET, POST, PUT, DELETE, PATCH, OPTIONS', - 'Access-Control-Allow-Headers': - req.headers['access-control-request-headers'] ?? 'Content-Type, Authorization', - 'Access-Control-Max-Age': '86400', - ...(origin ? {'Access-Control-Allow-Credentials': 'true'} : {}), - }) - return res.end() - } return proxy.web(req, res, {target}, (err) => { useConcurrentOutputContext({outputPrefix: 'proxy', stripAnsi: false}, () => { const lastError = isAggregateError(err) ? err.errors[err.errors.length - 1] : undefined