From 285e2e6afe08d0143f4509ffe41542bded91368c Mon Sep 17 00:00:00 2001 From: Edmund Hung Date: Wed, 14 May 2025 11:31:51 +0100 Subject: [PATCH 1/4] fix(miniflare): disable stripCfConnectingIp option by default --- packages/miniflare/src/plugins/core/index.ts | 4 +++- packages/miniflare/test/index.spec.ts | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/miniflare/src/plugins/core/index.ts b/packages/miniflare/src/plugins/core/index.ts index 8244c8b58371..057373df6fc0 100644 --- a/packages/miniflare/src/plugins/core/index.ts +++ b/packages/miniflare/src/plugins/core/index.ts @@ -164,7 +164,9 @@ const CoreOptionsSchemaInput = z.intersection( tails: z.array(ServiceDesignatorSchema).optional(), // Strip the CF-Connecting-IP header from outbound fetches - stripCfConnectingIp: z.boolean().default(true), + // There is an issue with the connect() API and the globalOutbound workerd setting that impacts TCP ingress + // We should default it to true once https://github.com/cloudflare/workerd/pull/4145 is resolved + stripCfConnectingIp: z.boolean().default(false), }) ); export const CoreOptionsSchema = CoreOptionsSchemaInput.transform((value) => { diff --git a/packages/miniflare/test/index.spec.ts b/packages/miniflare/test/index.spec.ts index e4c630e32f88..d783263bfeb5 100644 --- a/packages/miniflare/test/index.spec.ts +++ b/packages/miniflare/test/index.spec.ts @@ -3010,6 +3010,7 @@ test("Miniflare: strips CF-Connecting-IP", async (t) => { const client = new Miniflare({ script: `export default { fetch(request) { return fetch('${serverUrl.href}', {headers: {"CF-Connecting-IP":"fake-value"}}) } }`, modules: true, + stripCfConnectingIp: true, }); t.teardown(() => client.dispose()); t.teardown(() => server.dispose()); From 2165a75225d4d147ced85a6c78c88ef7b3e4edce Mon Sep 17 00:00:00 2001 From: Edmund Hung Date: Wed, 14 May 2025 16:23:49 +0100 Subject: [PATCH 2/4] add a regression test from the wrangler persepective --- .../api/startDevWorker/startWorker.test.ts | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 packages/wrangler/src/__tests__/api/startDevWorker/startWorker.test.ts diff --git a/packages/wrangler/src/__tests__/api/startDevWorker/startWorker.test.ts b/packages/wrangler/src/__tests__/api/startDevWorker/startWorker.test.ts new file mode 100644 index 000000000000..fc39edf11f20 --- /dev/null +++ b/packages/wrangler/src/__tests__/api/startDevWorker/startWorker.test.ts @@ -0,0 +1,51 @@ +import path from "node:path"; +import { Response } from "miniflare"; +import dedent from "ts-dedent"; +import { startWorker } from "../../../api/startDevWorker"; +import { runInTempDir } from "../../helpers/run-in-tmp"; +import { seed } from "../../helpers/seed"; + +describe("startWorker", () => { + runInTempDir(); + + // We do not inject the `CF-Connecting-IP` header on Windows at the moment. + // See https://github.com/cloudflare/workerd/issues/3310 + it.skipIf(process.platform === "win32")( + "strips the CF-Connecting-IP header from all outbound requests", + async (t) => { + t.onTestFinished(() => worker?.dispose()); + + await seed({ + "src/index.ts": dedent` + export default { + fetch(request) { + if (request.headers.has('CF-Connecting-IP')) { + return fetch(request); + } + + return new Response("No CF-Connecting-IP header"); + } + } + `, + }); + + const worker = await startWorker({ + name: "test-worker", + entrypoint: path.resolve("src/index.ts"), + dev: { + outboundService(request) { + return new Response( + request.headers.get("CF-Connecting-IP") ?? + "CF-Connecting-IP header stripped" + ); + }, + }, + }); + + const response = await worker.fetch("http://example.com"); + await expect(response.text()).resolves.toEqual( + "CF-Connecting-IP header stripped" + ); + } + ); +}); From 2649e3a7e3d340d6067feaf4c05b094dd3f18d3b Mon Sep 17 00:00:00 2001 From: Edmund Hung Date: Wed, 14 May 2025 19:08:48 +0100 Subject: [PATCH 3/4] inject monkey patched fetch to strip header --- .../api/startDevWorker/startWorker.test.ts | 31 ++++++++++++------- .../wrangler/src/deployment-bundle/bundle.ts | 24 ++++++++++++++ .../strip-cf-connecting-ip-header.js | 13 ++++++++ 3 files changed, 57 insertions(+), 11 deletions(-) create mode 100644 packages/wrangler/templates/strip-cf-connecting-ip-header.js diff --git a/packages/wrangler/src/__tests__/api/startDevWorker/startWorker.test.ts b/packages/wrangler/src/__tests__/api/startDevWorker/startWorker.test.ts index fc39edf11f20..88cc48988950 100644 --- a/packages/wrangler/src/__tests__/api/startDevWorker/startWorker.test.ts +++ b/packages/wrangler/src/__tests__/api/startDevWorker/startWorker.test.ts @@ -1,5 +1,5 @@ +import http from "node:http"; import path from "node:path"; -import { Response } from "miniflare"; import dedent from "ts-dedent"; import { startWorker } from "../../../api/startDevWorker"; import { runInTempDir } from "../../helpers/run-in-tmp"; @@ -13,7 +13,22 @@ describe("startWorker", () => { it.skipIf(process.platform === "win32")( "strips the CF-Connecting-IP header from all outbound requests", async (t) => { - t.onTestFinished(() => worker?.dispose()); + const server = http.createServer((req, res) => { + res.writeHead(200); + res.end( + req.headers["cf-connecting-ip"] ?? "CF-Connecting-IP header stripped" + ); + }); + + t.onTestFinished(() => { + server.close(); + }); + + const address = server.listen(0).address(); + + if (address === null || typeof address === "string") { + expect.fail("Failed to get server address"); + } await seed({ "src/index.ts": dedent` @@ -32,17 +47,11 @@ describe("startWorker", () => { const worker = await startWorker({ name: "test-worker", entrypoint: path.resolve("src/index.ts"), - dev: { - outboundService(request) { - return new Response( - request.headers.get("CF-Connecting-IP") ?? - "CF-Connecting-IP header stripped" - ); - }, - }, }); - const response = await worker.fetch("http://example.com"); + t.onTestFinished(() => worker.dispose()); + + const response = await worker.fetch(`http://127.0.0.1:${address.port}`); await expect(response.text()).resolves.toEqual( "CF-Connecting-IP header stripped" ); diff --git a/packages/wrangler/src/deployment-bundle/bundle.ts b/packages/wrangler/src/deployment-bundle/bundle.ts index 0d46640573a1..b99dc29bc736 100644 --- a/packages/wrangler/src/deployment-bundle/bundle.ts +++ b/packages/wrangler/src/deployment-bundle/bundle.ts @@ -262,6 +262,30 @@ export async function bundleWorker( inject.push(checkedFetchFileToInject); } + // We injected the `CF-Connecting-IP` header in the entry worker on Miniflare + // This was previously stripped within miniflare but this causes an issue with TCP ingress + // due to the global outbound setup. This is a workaround until a fix is in place in workerd. + if (targetConsumer === "dev" && local) { + const stripCfConnectingIpHeaderFileToInject = path.join( + tmpDir.path, + "strip-cf-connecting-ip-header.js" + ); + + if (!fs.existsSync(stripCfConnectingIpHeaderFileToInject)) { + fs.writeFileSync( + stripCfConnectingIpHeaderFileToInject, + fs.readFileSync( + path.resolve( + getBasePath(), + "templates/strip-cf-connecting-ip-header.js" + ) + ) + ); + } + + inject.push(stripCfConnectingIpHeaderFileToInject); + } + // When multiple workers are running we need some way to disambiguate logs between them. Inject a patched version of `globalThis.console` that prefixes logs with the worker name if (getFlag("MULTIWORKER")) { middlewareToLoad.push({ diff --git a/packages/wrangler/templates/strip-cf-connecting-ip-header.js b/packages/wrangler/templates/strip-cf-connecting-ip-header.js new file mode 100644 index 000000000000..a011710ccf80 --- /dev/null +++ b/packages/wrangler/templates/strip-cf-connecting-ip-header.js @@ -0,0 +1,13 @@ +function stripCfConnectingIPHeader(input, init) { + const request = new Request(input, init); + request.headers.delete("CF-Connecting-IP"); + return request; +} + +globalThis.fetch = new Proxy(globalThis.fetch, { + apply(target, thisArg, argArray) { + return Reflect.apply(target, thisArg, [ + stripCfConnectingIPHeader.apply(null, argArray), + ]); + }, +}); From 59790ee28ed8ccd29f0b0ad380d4f494652b9f78 Mon Sep 17 00:00:00 2001 From: Edmund Hung Date: Wed, 14 May 2025 19:52:18 +0100 Subject: [PATCH 4/4] add changeset --- .changeset/public-cameras-thank.md | 8 ++++++++ packages/wrangler/src/deployment-bundle/bundle.ts | 8 +++++--- 2 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 .changeset/public-cameras-thank.md diff --git a/.changeset/public-cameras-thank.md b/.changeset/public-cameras-thank.md new file mode 100644 index 000000000000..a84fd65192dd --- /dev/null +++ b/.changeset/public-cameras-thank.md @@ -0,0 +1,8 @@ +--- +"miniflare": patch +"wrangler": patch +--- + +fix: strip `CF-Connecting-IP` header within `fetch` + +In v4.15.0, Miniflare began stripping the `CF-Connecting-IP` header via a global outbound service, which led to a TCP connection regression due to a bug in Workerd. This PR patches the `fetch` API to strip the header during local `wrangler dev` sessions as a temporary workaround until the underlying issue is resolved. diff --git a/packages/wrangler/src/deployment-bundle/bundle.ts b/packages/wrangler/src/deployment-bundle/bundle.ts index b99dc29bc736..c33314ac2a5b 100644 --- a/packages/wrangler/src/deployment-bundle/bundle.ts +++ b/packages/wrangler/src/deployment-bundle/bundle.ts @@ -262,9 +262,11 @@ export async function bundleWorker( inject.push(checkedFetchFileToInject); } - // We injected the `CF-Connecting-IP` header in the entry worker on Miniflare - // This was previously stripped within miniflare but this causes an issue with TCP ingress - // due to the global outbound setup. This is a workaround until a fix is in place in workerd. + // We injected the `CF-Connecting-IP` header in the entry worker on Miniflare. + // It used to be stripped by Miniflare, but that caused TCP ingress failures + // because of the global outbound setup. This is a temporary workaround until + // a proper fix is landed in Workerd. + // See https://github.com/cloudflare/workers-sdk/issues/9238 for more details. if (targetConsumer === "dev" && local) { const stripCfConnectingIpHeaderFileToInject = path.join( tmpDir.path,