diff --git a/.changeset/orange-emus-mate.md b/.changeset/orange-emus-mate.md new file mode 100644 index 000000000000..0825abd68b0d --- /dev/null +++ b/.changeset/orange-emus-mate.md @@ -0,0 +1,14 @@ +--- +"wrangler": patch +--- + +fix: ensure that the Pages dev proxy server does not change the Host header + +Previously, when configuring `wrangler pages dev` to use a proxy to a 3rd party dev server, +the proxy would replace the Host header, resulting in problems at the dev server if it was +checking for cross-site scripting attacks. + +Now the proxy server passes through the Host header unaltered making it invisible to the +3rd party dev server. + +Fixes #4799 diff --git a/fixtures/pages-proxy-app/package.json b/fixtures/pages-proxy-app/package.json new file mode 100644 index 000000000000..cd989ceb1eb6 --- /dev/null +++ b/fixtures/pages-proxy-app/package.json @@ -0,0 +1,25 @@ +{ + "name": "pages-proxy-app", + "version": "0.1.2", + "private": true, + "sideEffects": false, + "main": "server/index.js", + "scripts": { + "build": "esbuild --bundle --platform=node server/index.ts --outfile=dist/index.js", + "check:type": "tsc", + "dev": "npx wrangler pages dev --compatibility-date=2024-01-17 --port 8790 --proxy 8791 -- pnpm run server", + "server": "node dist/index.js", + "test": "vitest run", + "test:watch": "vitest", + "type:tests": "tsc -p ./tests/tsconfig.json" + }, + "devDependencies": { + "@cloudflare/workers-tsconfig": "workspace:*", + "miniflare": "workspace:*", + "undici": "^5.28.2", + "wrangler": "workspace:*" + }, + "engines": { + "node": ">=14" + } +} diff --git a/fixtures/pages-proxy-app/server/index.ts b/fixtures/pages-proxy-app/server/index.ts new file mode 100644 index 000000000000..e6efbc2ab5fa --- /dev/null +++ b/fixtures/pages-proxy-app/server/index.ts @@ -0,0 +1,10 @@ +import { createServer } from "http"; + +const server = createServer(); + +server.on("request", (req, res) => { + res.write("Host:" + req.headers.host); + res.end(); +}); + +server.listen(8791); diff --git a/fixtures/pages-proxy-app/tests/index.test.ts b/fixtures/pages-proxy-app/tests/index.test.ts new file mode 100644 index 000000000000..0657f893652f --- /dev/null +++ b/fixtures/pages-proxy-app/tests/index.test.ts @@ -0,0 +1,36 @@ +import { fork } from "node:child_process"; +import { resolve } from "node:path"; +import { fetch } from "undici"; +import { afterAll, beforeAll, describe, it } from "vitest"; +import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived"; +import type { ChildProcess } from "node:child_process"; + +describe("pages-proxy-app", async () => { + let ip: string, port: number, stop: (() => Promise) | undefined; + let devServer: ChildProcess; + + beforeAll(async () => { + devServer = fork(resolve(__dirname, "../dist/index.js"), { + stdio: "ignore", + }); + + debugger; + ({ ip, port, stop } = await runWranglerPagesDev( + resolve(__dirname, ".."), + undefined, + ["--port=0", "--inspector-port=0", "--proxy=8791"] + )); + }); + + afterAll(async () => { + await stop?.(); + devServer.kill(); + }); + + it("receives the correct Host header", async ({ expect }) => { + debugger; + const response = await fetch(`http://${ip}:${port}/`); + const text = await response.text(); + expect(text).toContain(`Host:${ip}:${port}`); + }); +}); diff --git a/fixtures/pages-proxy-app/tests/tsconfig.json b/fixtures/pages-proxy-app/tests/tsconfig.json new file mode 100644 index 000000000000..d2ce7f144694 --- /dev/null +++ b/fixtures/pages-proxy-app/tests/tsconfig.json @@ -0,0 +1,7 @@ +{ + "extends": "@cloudflare/workers-tsconfig/tsconfig.json", + "compilerOptions": { + "types": ["node"] + }, + "include": ["**/*.ts", "../../../node-types.d.ts"] +} diff --git a/fixtures/pages-proxy-app/tsconfig.json b/fixtures/pages-proxy-app/tsconfig.json new file mode 100644 index 000000000000..902b5311b2a8 --- /dev/null +++ b/fixtures/pages-proxy-app/tsconfig.json @@ -0,0 +1,13 @@ +{ + "include": ["server"], + "compilerOptions": { + "target": "ES2020", + "module": "CommonJS", + "lib": ["ES2020"], + "types": ["node"], + "moduleResolution": "node", + "esModuleInterop": true, + "noEmit": true, + "skipLibCheck": true + } +} diff --git a/fixtures/pages-proxy-app/turbo.json b/fixtures/pages-proxy-app/turbo.json new file mode 100644 index 000000000000..1a6c74c9def8 --- /dev/null +++ b/fixtures/pages-proxy-app/turbo.json @@ -0,0 +1,9 @@ +{ + "$schema": "http://turbo.build/schema.json", + "extends": ["//"], + "pipeline": { + "build": { + "outputs": ["dist/**"] + } + } +} diff --git a/fixtures/pages-proxy-app/vitest.config.ts b/fixtures/pages-proxy-app/vitest.config.ts new file mode 100644 index 000000000000..846cddc41995 --- /dev/null +++ b/fixtures/pages-proxy-app/vitest.config.ts @@ -0,0 +1,9 @@ +import { defineProject, mergeConfig } from "vitest/config"; +import configShared from "../../vitest.shared"; + +export default mergeConfig( + configShared, + defineProject({ + test: {}, + }) +); diff --git a/fixtures/shared/src/run-wrangler-long-lived.ts b/fixtures/shared/src/run-wrangler-long-lived.ts index 0eab03135312..9d63214a8e65 100644 --- a/fixtures/shared/src/run-wrangler-long-lived.ts +++ b/fixtures/shared/src/run-wrangler-long-lived.ts @@ -16,10 +16,14 @@ export const wranglerEntryPath = path.resolve( */ export async function runWranglerPagesDev( cwd: string, - publicPath: string, + publicPath: string | undefined, options: string[] ) { - return runLongLivedWrangler(["pages", "dev", publicPath, ...options], cwd); + if (publicPath) { + return runLongLivedWrangler(["pages", "dev", publicPath, ...options], cwd); + } else { + return runLongLivedWrangler(["pages", "dev", ...options], cwd); + } } /** diff --git a/packages/wrangler/src/miniflare-cli/assets.ts b/packages/wrangler/src/miniflare-cli/assets.ts index 1da646944012..3731736f6471 100644 --- a/packages/wrangler/src/miniflare-cli/assets.ts +++ b/packages/wrangler/src/miniflare-cli/assets.ts @@ -1,3 +1,4 @@ +import assert from "node:assert"; import { existsSync, lstatSync, readFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { createMetadataObject } from "@cloudflare/pages-shared/metadata-generator/createMetadataObject"; @@ -6,6 +7,7 @@ import { parseRedirects } from "@cloudflare/pages-shared/metadata-generator/pars import { watch } from "chokidar"; import { getType } from "mime"; import { fetch, Request, Response } from "miniflare"; +import { Dispatcher, getGlobalDispatcher } from "undici"; import { hashFile } from "../pages/hash"; import type { Logger } from "../logger"; import type { Metadata } from "@cloudflare/pages-shared/asset-server/metadata"; @@ -15,6 +17,7 @@ import type { } from "@cloudflare/pages-shared/metadata-generator/types"; import type { Request as WorkersRequest } from "@cloudflare/workers-types/experimental"; import type { RequestInit } from "miniflare"; +import type { IncomingHttpHeaders } from "undici/types/header"; export interface Options { log: Logger; @@ -38,7 +41,9 @@ export default async function generateASSETSBinding(options: Options) { proxyRequest.headers.delete("Sec-WebSocket-Accept"); proxyRequest.headers.delete("Sec-WebSocket-Key"); } - return await fetch(proxyRequest); + return await fetch(proxyRequest, { + dispatcher: new ProxyDispatcher(miniflareRequest.headers.get("Host")), + }); } catch (thrown) { options.log.error(new Error(`Could not proxy request: ${thrown}`)); @@ -63,6 +68,64 @@ export default async function generateASSETSBinding(options: Options) { }; } +/** + * An Undici custom Dispatcher that is used for the fetch requests + * of the Pages dev server proxy. + * + * Notably, the ProxyDispatcher reinstates the Host header that was removed by the + * Undici `fetch` function call. Undici removes the Host header as a security precaution, + * but this is not relevant for an internal Proxy. + * + * The ProxyDispatcher will delegate through to the current global `Dispatcher`, + * ensuring that the request is routed correctly in case a developer has changed the + * global Dispatcher by a call to `setGlobalDispatcher()`. + */ +class ProxyDispatcher extends Dispatcher { + private dispatcher: Dispatcher = getGlobalDispatcher(); + + constructor(private host: string | null) { + super(); + } + + dispatch( + options: Dispatcher.DispatchOptions, + handler: Dispatcher.DispatchHandlers + ): boolean { + if (this.host !== null) { + ProxyDispatcher.reinstateHostHeader(options.headers, this.host); + } + return this.dispatcher.dispatch(options, handler); + } + + close() { + return this.dispatcher.close(); + } + + destroy() { + return this.dispatcher.destroy(); + } + + /** + * Ensure that the request contains a Host header, which would have been deleted + * by the `fetch()` function before calling `dispatcher.dispatch()`. + */ + private static reinstateHostHeader( + headers: string[] | IncomingHttpHeaders | null | undefined, + host: string + ) { + assert(headers, "Expected all proxy requests to contain headers."); + assert( + !Array.isArray(headers), + "Expected proxy request headers to be a hash object" + ); + assert( + Object.keys(headers).every((h) => h.toLowerCase() !== "host"), + "Expected Host header to have been deleted." + ); + headers["Host"] = host; + } +} + async function generateAssetsFetch( directory: string, log: Logger diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6bd9610d0b07..440cd1e66a07 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -393,6 +393,21 @@ importers: specifier: workspace:* version: link:../../packages/wrangler + fixtures/pages-proxy-app: + devDependencies: + '@cloudflare/workers-tsconfig': + specifier: workspace:* + version: link:../../packages/workers-tsconfig + miniflare: + specifier: workspace:* + version: link:../../packages/miniflare + undici: + specifier: ^5.28.2 + version: 5.28.2 + wrangler: + specifier: workspace:* + version: link:../../packages/wrangler + fixtures/pages-simple-assets: devDependencies: '@cloudflare/workers-tsconfig':