Skip to content

Commit

Permalink
fix: ensure that the Pages dev proxy server does not change the Host …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
petebacondarwin committed Feb 2, 2024
1 parent 3e7cd6e commit 031dd56
Show file tree
Hide file tree
Showing 11 changed files with 237 additions and 32 deletions.
14 changes: 14 additions & 0 deletions .changeset/orange-emus-mate.md
Original file line number Diff line number Diff line change
@@ -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
25 changes: 25 additions & 0 deletions fixtures/pages-proxy-app/package.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
10 changes: 10 additions & 0 deletions fixtures/pages-proxy-app/server/index.ts
Original file line number Diff line number Diff line change
@@ -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);
34 changes: 34 additions & 0 deletions fixtures/pages-proxy-app/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
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<unknown>) | undefined;
let devServer: ChildProcess;

beforeAll(async () => {
devServer = fork(resolve(__dirname, "../dist/index.js"), {
stdio: "ignore",
});

({ 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 }) => {
const response = await fetch(`http://${ip}:${port}/`);
const text = await response.text();
expect(text).toContain(`Host:${ip}:${port}`);
});
});
7 changes: 7 additions & 0 deletions fixtures/pages-proxy-app/tests/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "@cloudflare/workers-tsconfig/tsconfig.json",
"compilerOptions": {
"types": ["node"]
},
"include": ["**/*.ts", "../../../node-types.d.ts"]
}
13 changes: 13 additions & 0 deletions fixtures/pages-proxy-app/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"include": ["server"],
"compilerOptions": {
"target": "ES2020",
"module": "CommonJS",
"lib": ["ES2020"],
"types": ["node"],
"moduleResolution": "node",
"esModuleInterop": true,
"noEmit": true,
"skipLibCheck": true
}
}
9 changes: 9 additions & 0 deletions fixtures/pages-proxy-app/turbo.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"$schema": "http://turbo.build/schema.json",
"extends": ["//"],
"pipeline": {
"build": {
"outputs": ["dist/**"]
}
}
}
9 changes: 9 additions & 0 deletions fixtures/pages-proxy-app/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { defineProject, mergeConfig } from "vitest/config";
import configShared from "../../vitest.shared";

export default mergeConfig(
configShared,
defineProject({
test: {},
})
);
8 changes: 6 additions & 2 deletions fixtures/shared/src/run-wrangler-long-lived.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

/**
Expand Down
44 changes: 43 additions & 1 deletion packages/wrangler/src/miniflare-cli/assets.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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 { Agent } from "undici";
import { hashFile } from "../pages/hash";
import type { Logger } from "../logger";
import type { Metadata } from "@cloudflare/pages-shared/asset-server/metadata";
Expand All @@ -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 { Dispatcher } from "undici";

export interface Options {
log: Logger;
Expand All @@ -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}`));

Expand All @@ -63,6 +68,43 @@ 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, this dispatcher will reinstate the Host header that is
* removed by the `fetch` machinery. This is removed as a security
* precaution, which is not relevant for this Proxy.
*/
class ProxyDispatcher extends Agent {
constructor(private host: string | null) {
super();
}

dispatch(
options: Agent.DispatchOptions,
handler: Dispatcher.DispatchHandlers
): boolean {
if (this.host) {
const headers = options.headers;
assert(headers, "Expected all proxied requests to contain headers.");
if (Array.isArray(headers)) {
assert(
headers.every(
(h) => h !== "Host",
"Expected Host header to have been deleted."
)
);
headers.push("Host", this.host);
} else if (headers) {
assert(!headers["Host"], "Expected Host header to have been deleted.");
headers["Host"] = this.host;
}
}
return super.dispatch(options, handler);
}
}

async function generateAssetsFetch(
directory: string,
log: Logger
Expand Down
Loading

0 comments on commit 031dd56

Please sign in to comment.