diff --git a/.changeset/curly-drinks-tell.md b/.changeset/curly-drinks-tell.md new file mode 100644 index 000000000000..4e48caf948e9 --- /dev/null +++ b/.changeset/curly-drinks-tell.md @@ -0,0 +1,12 @@ +--- +"miniflare": patch +"wrangler": patch +--- + +fix: ensure we do not rewrite external Origin headers in wrangler dev + +In https://github.com/cloudflare/workers-sdk/pull/4812 we tried to fix the Origin headers to match the Host header but were overzealous and rewrote Origin headers for external origins (outside of the proxy server's origin). + +This is now fixed, and moreover we rewrite any headers that refer to the proxy server on the request with the configured host and vice versa on the response. + +This should ensure that CORS is not broken in browsers when a different host is being simulated based on routes in the Wrangler configuration. diff --git a/fixtures/worker-app/src/index.js b/fixtures/worker-app/src/index.js index c28a563063a6..1ae68585eec7 100644 --- a/fixtures/worker-app/src/index.js +++ b/fixtures/worker-app/src/index.js @@ -15,9 +15,10 @@ export default { async fetch(request) { console.log("request log"); - const { pathname } = new URL(request.url); + const { pathname, origin } = new URL(request.url); if (pathname === "/random") return new Response(hexEncode(randomBytes(8))); if (pathname === "/error") throw new Error("Oops!"); + if (pathname === "/redirect") return Response.redirect(`${origin}/foo`); if (request.headers.get("X-Test-URL") !== null) { return new Response(request.url); } diff --git a/fixtures/worker-app/tests/index.test.ts b/fixtures/worker-app/tests/index.test.ts index d98c883b3fe8..3dbb3edd2866 100644 --- a/fixtures/worker-app/tests/index.test.ts +++ b/fixtures/worker-app/tests/index.test.ts @@ -91,7 +91,7 @@ describe("'wrangler dev' correctly renders pages", () => { expect(text).toBe(`https://prod.example.org//thing?a=1`); }); - it("updates the Host and Origin headers appropriately", async ({ + it("rewrites the Host and Origin headers appropriately", async ({ expect, }) => { const response = await fetch(`http://${ip}:${port}/test`, { @@ -104,7 +104,7 @@ describe("'wrangler dev' correctly renders pages", () => { expect(text).toContain(`ORIGIN:https://prod.example.org`); }); - it("does not update Origin header if one is not passed by the client", async ({ + it("does not rewrite Origin header if one is not passed by the client", async ({ expect, }) => { const response = await fetch(`http://${ip}:${port}/test`, {}); @@ -112,4 +112,31 @@ describe("'wrangler dev' correctly renders pages", () => { expect(text).toContain(`HOST:prod.example.org`); expect(text).toContain(`ORIGIN:null`); }); + + it("does not rewrite Origin header if it not the same origin as the proxy Worker", async ({ + expect, + }) => { + const response = await fetch(`http://${ip}:${port}/test`, { + headers: { Origin: `http://foo.com` }, + }); + const text = await response.text(); + console.log(text); + expect(text).toContain(`HOST:prod.example.org`); + expect(text).toContain(`ORIGIN:http://foo.com`); + }); + + it("rewrites response headers containing the emulated host", async ({ + expect, + }) => { + // This /redirect request will add a Location header that points to prod.example.com/foo + // But we should rewrite this back to that of the proxy. + const response = await fetch(`http://${ip}:${port}/redirect`, { + redirect: "manual", + }); + expect(response.status).toBe(302); + expect(await response.text()).toEqual(""); + expect(response.headers.get("Location")).toEqual( + `http://${ip}:${port}/foo` + ); + }); }); diff --git a/packages/edge-preview-authenticated-proxy/tests/index.test.ts b/packages/edge-preview-authenticated-proxy/tests/index.test.ts index 615d68dc3d7b..891faf060263 100644 --- a/packages/edge-preview-authenticated-proxy/tests/index.test.ts +++ b/packages/edge-preview-authenticated-proxy/tests/index.test.ts @@ -142,7 +142,7 @@ compatibility_date = "2023-01-01" expect( removeUUID(resp.headers.get("set-cookie") ?? "") ).toMatchInlineSnapshot( - '"token=00000000-0000-0000-0000-000000000000; Domain=preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None"' + '"token=00000000-0000-0000-0000-000000000000; Domain=random-data.preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None"' ); tokenId = (resp.headers.get("set-cookie") ?? "") .split(";")[0] @@ -152,7 +152,7 @@ compatibility_date = "2023-01-01" { method: "GET", headers: { - cookie: `token=${tokenId}; Domain=preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, + cookie: `token=${tokenId}; Domain=random-data.preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, }, } ); @@ -183,7 +183,7 @@ compatibility_date = "2023-01-01" expect( removeUUID(resp.headers.get("set-cookie") ?? "") ).toMatchInlineSnapshot( - '"token=00000000-0000-0000-0000-000000000000; Domain=preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None"' + '"token=00000000-0000-0000-0000-000000000000; Domain=random-data.preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None"' ); tokenId = (resp.headers.get("set-cookie") ?? "") .split(";")[0] @@ -218,7 +218,7 @@ compatibility_date = "2023-01-01" { method: "GET", headers: { - cookie: `token=${tokenId}; Domain=preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, + cookie: `token=${tokenId}; Domain=random-data.preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, }, } ); @@ -237,7 +237,7 @@ compatibility_date = "2023-01-01" { method: "GET", headers: { - cookie: `token=${tokenId}; Domain=preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, + cookie: `token=${tokenId}; Domain=random-data.preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, }, redirect: "manual", } @@ -255,7 +255,7 @@ compatibility_date = "2023-01-01" { method: "PUT", headers: { - cookie: `token=${tokenId}; Domain=preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, + cookie: `token=${tokenId}; Domain=random-data.preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, }, redirect: "manual", } @@ -270,7 +270,7 @@ compatibility_date = "2023-01-01" method: "PUT", headers: { "X-Custom-Header": "custom", - cookie: `token=${tokenId}; Domain=preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, + cookie: `token=${tokenId}; Domain=random-data.preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, }, redirect: "manual", } @@ -284,7 +284,7 @@ compatibility_date = "2023-01-01" { method: "PUT", headers: { - cookie: `token=${tokenId}; Domain=preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, + cookie: `token=${tokenId}; Domain=random-data.preview.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None`, }, redirect: "manual", } diff --git a/packages/miniflare/src/workers/core/entry.worker.ts b/packages/miniflare/src/workers/core/entry.worker.ts index fe0431ca06dc..f10c94f348ba 100644 --- a/packages/miniflare/src/workers/core/entry.worker.ts +++ b/packages/miniflare/src/workers/core/entry.worker.ts @@ -94,10 +94,6 @@ function getUserRequest( if (rewriteHeadersFromOriginalUrl) { request.headers.set("Host", url.host); - // Only rewrite Origin header if there is already one - if (request.headers.has("Origin")) { - request.headers.set("Origin", url.origin); - } } request.headers.delete(CoreHeaders.PROXY_SHARED_SECRET); diff --git a/packages/wrangler/src/api/startDevWorker/events.ts b/packages/wrangler/src/api/startDevWorker/events.ts index 00aa873cf39f..7e8cca48df70 100644 --- a/packages/wrangler/src/api/startDevWorker/events.ts +++ b/packages/wrangler/src/api/startDevWorker/events.ts @@ -145,7 +145,7 @@ export type ProxyData = { userWorkerUrl: UrlOriginParts; userWorkerInspectorUrl: UrlOriginAndPathnameParts; userWorkerInnerUrlOverrides: Partial; - headers: Record; + headers: Record; liveReload?: boolean; proxyLogsToController?: boolean; internalDurableObjects?: CfDurableObject[]; diff --git a/packages/wrangler/src/dev/remote.tsx b/packages/wrangler/src/dev/remote.tsx index 7cfa0dc0b7a0..9120810f263e 100644 --- a/packages/wrangler/src/dev/remote.tsx +++ b/packages/wrangler/src/dev/remote.tsx @@ -323,10 +323,13 @@ export function useWorker( port: workerPreviewToken.inspectorUrl.port.toString(), pathname: workerPreviewToken.inspectorUrl.pathname, }, - userWorkerInnerUrlOverrides: {}, // there is no analagous prop for this option because we did not permit overriding request.url in remote mode + userWorkerInnerUrlOverrides: { + hostname: props.host, + port: props.port.toString(), + }, headers: { "cf-workers-preview-token": workerPreviewToken.value, - Cookie: accessToken && `CF_Authorization=${accessToken}`, + ...(accessToken ? { Cookie: `CF_Authorization=${accessToken}` } : {}), }, liveReload: false, // liveReload currently disabled in remote-mode, but will be supported with startDevWorker proxyLogsToController: true, @@ -436,10 +439,13 @@ export async function startRemoteServer(props: RemoteProps) { port: previewToken.inspectorUrl.port.toString(), pathname: previewToken.inspectorUrl.pathname, }, - userWorkerInnerUrlOverrides: {}, // there is no analagous prop for this option because we did not permit overriding request.url in remote mode + userWorkerInnerUrlOverrides: { + hostname: props.host, + port: props.port.toString(), + }, headers: { "cf-workers-preview-token": previewToken.value, - Cookie: accessToken && `CF_Authorization=${accessToken}`, + ...(accessToken ? { Cookie: `CF_Authorization=${accessToken}` } : {}), }, liveReload: false, // liveReload currently disabled in remote-mode, but will be supported with startDevWorker proxyLogsToController: true, diff --git a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts index 1ab8baf59ddc..4b5fb2a4f798 100644 --- a/packages/wrangler/templates/startDevWorker/ProxyWorker.ts +++ b/packages/wrangler/templates/startDevWorker/ProxyWorker.ts @@ -113,10 +113,11 @@ export class ProxyWorker implements DurableObject { this.requestRetryQueue.delete(request); this.requestQueue.delete(request); - const userWorkerUrl = new URL(request.url); + const outerUrl = new URL(request.url); const headers = new Headers(request.headers); // override url parts for proxying + const userWorkerUrl = new URL(request.url); Object.assign(userWorkerUrl, proxyData.userWorkerUrl); // set request.url in the UserWorker @@ -125,6 +126,8 @@ export class ProxyWorker implements DurableObject { headers.set("MF-Original-URL", innerUrl.href); headers.set("MF-Disable-Pretty-Error", "true"); // disables the UserWorker miniflare instance from rendering the pretty error -- instead the ProxyWorker miniflare instance will intercept the json error response and render the pretty error page + rewriteUrlRelatedHeaders(headers, outerUrl, innerUrl); + // merge proxyData headers with the request headers for (const [key, value] of Object.entries(proxyData.headers ?? {})) { if (value === undefined) continue; @@ -140,6 +143,9 @@ export class ProxyWorker implements DurableObject { // explicitly NOT await-ing this promise, we are in a loop and want to process the whole queue quickly + synchronously void fetch(userWorkerUrl, new Request(request, { headers })) .then((res) => { + res = new Response(res.body, res); + rewriteUrlRelatedHeaders(res.headers, innerUrl, outerUrl); + if (isHtmlResponse(res)) { res = insertLiveReloadScript(request, res, this.env, proxyData); } @@ -290,3 +296,20 @@ function insertLiveReloadScript( return htmlRewriter.transform(response); } + +/** + * Rewrite references to URLs in request/response headers. + * + * This function is used to map the URLs in headers like Origin and Access-Control-Allow-Origin + * so that this proxy is transparent to the Client Browser and User Worker. + */ +function rewriteUrlRelatedHeaders(headers: Headers, from: URL, to: URL) { + headers.forEach((value, key) => { + if (typeof value === "string" && value.includes(from.host)) { + headers.set( + key, + value.replaceAll(from.origin, to.origin).replaceAll(from.host, to.host) + ); + } + }); +}