fix: ensure we do not rewrite external Origin headers in wrangler dev (…

…#4950) fix: ensure we do not rewrite external Origin headers in wrangler dev In #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.
cloudflare · Feb 9, 2024 · 05360e4 · 05360e4
1 parent 96c8cfb
commit 05360e4
Show file tree

Hide file tree

Showing 8 changed files with 86 additions and 21 deletions.
diff --git a/.changeset/curly-drinks-tell.md 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
@@ -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
@@ -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,12 +104,39 @@ 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`, {});
 		const text = await response.text();
 		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
@@ -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
@@ -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
@@ -145,7 +145,7 @@ export type ProxyData = {
 	userWorkerUrl: UrlOriginParts;
 	userWorkerInspectorUrl: UrlOriginAndPathnameParts;
 	userWorkerInnerUrlOverrides: Partial<UrlOriginParts>;
-	headers: Record<string, string | undefined>;
+	headers: Record<string, string>;
 	liveReload?: boolean;
 	proxyLogsToController?: boolean;
 	internalDurableObjects?: CfDurableObject[];

diff --git 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
@@ -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)
+			);
+		}
+	});
+}