[wrangler] fix: change wrangler (pages) dev to listen on `localhost…

…` by default (#4532) * fix: change `wrangler (pages) dev` to listen on `localhost` by default * fix: disable `MiniflareDurableObject` endpoints by default * fix: prevent unauthorised access to proxy server Require `MF-Op-Secret` header to match `MINIFLARE_PROXY_SECRET` data binding before request processed.
cloudflare · Nov 30, 2023 · 311ffbd · 311ffbd
1 parent 310281a
commit 311ffbd
Show file tree

Hide file tree

Showing 20 changed files with 108 additions and 11 deletions.
diff --git a/.changeset/shiny-poets-smell.md b/.changeset/shiny-poets-smell.md
@@ -0,0 +1,7 @@
+---
+"wrangler": minor
+---
+
+fix: change `wrangler (pages) dev` to listen on `localhost` by default
+
+Previously, Wrangler listened on all interfaces (`*`) by default. This change switches `wrangler (pages) dev` to just listen on local interfaces. Whilst this is technically a breaking change, we've decided the security benefits outweigh the potential disruption caused. If you need to access your dev server from another device on your network, you can use `wrangler (pages) dev --ip *` to restore the previous behaviour.
diff --git a/packages/miniflare/src/plugins/cache/index.ts b/packages/miniflare/src/plugins/cache/index.ts
@@ -13,6 +13,7 @@ import {
 	PersistenceSchema,
 	Plugin,
 	SERVICE_LOOPBACK,
+	getControlEndpointBindings,
 	getPersistPath,
 } from "../shared";
 
@@ -128,6 +129,7 @@ export const CACHE_PLUGIN: Plugin<
 							name: SharedBindings.MAYBE_SERVICE_LOOPBACK,
 							service: { name: SERVICE_LOOPBACK },
 						},
+						...getControlEndpointBindings(),
 					],
 				},
 			};

diff --git a/packages/miniflare/src/plugins/core/index.ts b/packages/miniflare/src/plugins/core/index.ts
@@ -52,6 +52,7 @@ import {
 	convertModuleDefinition,
 	withSourceURL,
 } from "./modules";
+import { PROXY_SECRET } from "./proxy";
 import { ServiceDesignatorSchema } from "./services";
 
 // `workerd`'s `trustBrowserCas` should probably be named `trustSystemCas`.
@@ -637,6 +638,10 @@ export function getGlobalServices({
 			name: CoreBindings.DURABLE_OBJECT_NAMESPACE_PROXY,
 			durableObjectNamespace: { className: "ProxyServer" },
 		},
+		{
+			name: CoreBindings.DATA_PROXY_SECRET,
+			data: PROXY_SECRET,
+		},
 		// Add `proxyBindings` here, they'll be added to the `ProxyServer` `env`
 		...proxyBindings,
 	];

diff --git a/packages/miniflare/src/plugins/core/proxy/client.ts b/packages/miniflare/src/plugins/core/proxy/client.ts
@@ -1,5 +1,6 @@
 /* eslint-disable @typescript-eslint/ban-types */
 import assert from "assert";
+import crypto from "crypto";
 import { ReadableStream, TransformStream } from "stream/web";
 import util from "util";
 import type { ServiceWorkerGlobalScope } from "@cloudflare/workers-types/experimental";
@@ -63,6 +64,9 @@ const revivers: ReducersRevivers = {
 	// `Native` reviver depends on `ProxyStubHandler` methods
 };
 
+export const PROXY_SECRET = crypto.randomBytes(16);
+const PROXY_SECRET_HEX = PROXY_SECRET.toString("hex");
+
 // Exported public API of the proxy system
 export class ProxyClient {
 	#bridge: ProxyClientBridge;
@@ -168,6 +172,7 @@ class ProxyClientBridge {
 			await this.dispatchFetch(this.url, {
 				method: "DELETE",
 				headers: {
+					[CoreHeaders.OP_SECRET]: PROXY_SECRET_HEX,
 					[CoreHeaders.OP]: ProxyOps.FREE,
 					[CoreHeaders.OP_TARGET]: addresses.join(","),
 				},
@@ -234,6 +239,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 				const resPromise = this.bridge.dispatchFetch(this.bridge.url, {
 					method: "POST",
 					headers: {
+						[CoreHeaders.OP_SECRET]: PROXY_SECRET_HEX,
 						[CoreHeaders.OP]: ProxyOps.GET, // GET without key just gets target
 						[CoreHeaders.OP_TARGET]: stringify(target, reducers),
 					},
@@ -367,6 +373,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 		const syncRes = this.bridge.sync.fetch(this.bridge.url, {
 			method: "POST",
 			headers: {
+				[CoreHeaders.OP_SECRET]: PROXY_SECRET_HEX,
 				[CoreHeaders.OP]: ProxyOps.GET,
 				[CoreHeaders.OP_TARGET]: this.#stringifiedTarget,
 				[CoreHeaders.OP_KEY]: key,
@@ -416,6 +423,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 		const syncRes = this.bridge.sync.fetch(this.bridge.url, {
 			method: "POST",
 			headers: {
+				[CoreHeaders.OP_SECRET]: PROXY_SECRET_HEX,
 				[CoreHeaders.OP]: ProxyOps.GET_OWN_DESCRIPTOR,
 				[CoreHeaders.OP_KEY]: key,
 				[CoreHeaders.OP_TARGET]: this.#stringifiedTarget,
@@ -440,6 +448,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 		const syncRes = this.bridge.sync.fetch(this.bridge.url, {
 			method: "POST",
 			headers: {
+				[CoreHeaders.OP_SECRET]: PROXY_SECRET_HEX,
 				[CoreHeaders.OP]: ProxyOps.GET_OWN_KEYS,
 				[CoreHeaders.OP_TARGET]: this.#stringifiedTarget,
 			},
@@ -526,6 +535,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 		const syncRes = this.bridge.sync.fetch(this.bridge.url, {
 			method: "POST",
 			headers: {
+				[CoreHeaders.OP_SECRET]: PROXY_SECRET_HEX,
 				[CoreHeaders.OP]: ProxyOps.CALL,
 				[CoreHeaders.OP_TARGET]: this.#stringifiedTarget,
 				[CoreHeaders.OP_KEY]: key,
@@ -548,6 +558,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 			resPromise = this.bridge.dispatchFetch(this.bridge.url, {
 				method: "POST",
 				headers: {
+					[CoreHeaders.OP_SECRET]: PROXY_SECRET_HEX,
 					[CoreHeaders.OP]: ProxyOps.CALL,
 					[CoreHeaders.OP_TARGET]: this.#stringifiedTarget,
 					[CoreHeaders.OP_KEY]: key,
@@ -563,6 +574,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 			resPromise = this.bridge.dispatchFetch(this.bridge.url, {
 				method: "POST",
 				headers: {
+					[CoreHeaders.OP_SECRET]: PROXY_SECRET_HEX,
 					[CoreHeaders.OP]: ProxyOps.CALL,
 					[CoreHeaders.OP_TARGET]: this.#stringifiedTarget,
 					[CoreHeaders.OP_KEY]: key,
@@ -581,6 +593,7 @@ class ProxyStubHandler<T extends object> implements ProxyHandler<T> {
 		const request = new Request(...args);
 		// If adding new headers here, remember to `delete()` them in `ProxyServer`
 		// before calling `fetch()`.
+		request.headers.set(CoreHeaders.OP_SECRET, PROXY_SECRET_HEX);
 		request.headers.set(CoreHeaders.OP, ProxyOps.CALL);
 		request.headers.set(CoreHeaders.OP_TARGET, this.#stringifiedTarget);
 		request.headers.set(CoreHeaders.OP_KEY, "fetch");

diff --git a/packages/miniflare/src/plugins/d1/index.ts b/packages/miniflare/src/plugins/d1/index.ts
@@ -11,6 +11,7 @@ import {
 	PersistenceSchema,
 	Plugin,
 	SERVICE_LOOPBACK,
+	getControlEndpointBindings,
 	getPersistPath,
 	kProxyNodeBinding,
 	migrateDatabase,
@@ -117,6 +118,7 @@ export const D1_PLUGIN: Plugin<
 							name: SharedBindings.MAYBE_SERVICE_LOOPBACK,
 							service: { name: SERVICE_LOOPBACK },
 						},
+						...getControlEndpointBindings(),
 					],
 				},
 			};

diff --git a/packages/miniflare/src/plugins/kv/index.ts b/packages/miniflare/src/plugins/kv/index.ts
@@ -11,6 +11,7 @@ import {
 	PersistenceSchema,
 	Plugin,
 	SERVICE_LOOPBACK,
+	getControlEndpointBindings,
 	getPersistPath,
 	kProxyNodeBinding,
 	migrateDatabase,
@@ -123,6 +124,7 @@ export const KV_PLUGIN: Plugin<
 							name: SharedBindings.MAYBE_SERVICE_LOOPBACK,
 							service: { name: SERVICE_LOOPBACK },
 						},
+						...getControlEndpointBindings(),
 					],
 				},
 			};

diff --git a/packages/miniflare/src/plugins/queues/index.ts b/packages/miniflare/src/plugins/queues/index.ts
@@ -15,6 +15,7 @@ import { getUserServiceName } from "../core";
 import {
 	Plugin,
 	SERVICE_LOOPBACK,
+	getControlEndpointBindings,
 	kProxyNodeBinding,
 	namespaceEntries,
 	namespaceKeys,
@@ -87,6 +88,7 @@ export const QUEUES_PLUGIN: Plugin<typeof QueuesOptionsSchema> = {
 						name: SharedBindings.MAYBE_SERVICE_LOOPBACK,
 						service: { name: SERVICE_LOOPBACK },
 					},
+					...getControlEndpointBindings(),
 					{
 						name: SharedBindings.DURABLE_OBJECT_NAMESPACE_OBJECT,
 						durableObjectNamespace: {

diff --git a/packages/miniflare/src/plugins/r2/index.ts b/packages/miniflare/src/plugins/r2/index.ts
@@ -11,6 +11,7 @@ import {
 	PersistenceSchema,
 	Plugin,
 	SERVICE_LOOPBACK,
+	getControlEndpointBindings,
 	getPersistPath,
 	kProxyNodeBinding,
 	migrateDatabase,
@@ -97,6 +98,7 @@ export const R2_PLUGIN: Plugin<
 							name: SharedBindings.MAYBE_SERVICE_LOOPBACK,
 							service: { name: SERVICE_LOOPBACK },
 						},
+						...getControlEndpointBindings(),
 					],
 				},
 			};

diff --git a/packages/miniflare/src/plugins/shared/constants.ts b/packages/miniflare/src/plugins/shared/constants.ts
@@ -26,6 +26,20 @@ export const WORKER_BINDING_SERVICE_LOOPBACK: Worker_Binding = {
 	service: { name: SERVICE_LOOPBACK },
 };
 
+const WORKER_BINDING_ENABLE_CONTROL_ENDPOINT: Worker_Binding = {
+	name: SharedBindings.MAYBE_JSON_ENABLE_CONTROL_ENDPOINTS,
+	json: "true",
+};
+let enableControlEndpoints = false;
+export function getControlEndpointBindings(): Worker_Binding[] {
+	if (enableControlEndpoints) return [WORKER_BINDING_ENABLE_CONTROL_ENDPOINT];
+	else return [];
+}
+/** @internal */
+export function _enableControlEndpoints() {
+	enableControlEndpoints = true;
+}
+
 export function objectEntryWorker(
 	durableObjectNamespace: Worker_Binding_DurableObjectNamespaceDesignator,
 	namespace: string

diff --git a/packages/miniflare/src/workers/core/constants.ts b/packages/miniflare/src/workers/core/constants.ts
@@ -6,6 +6,7 @@ export const CoreHeaders = {
 	ROUTE_OVERRIDE: "MF-Route-Override",
 
 	// API Proxy
+	OP_SECRET: "MF-Op-Secret",
 	OP: "MF-Op",
 	OP_TARGET: "MF-Op-Target",
 	OP_KEY: "MF-Op-Key",
@@ -25,6 +26,7 @@ export const CoreBindings = {
 	JSON_LOG_LEVEL: "MINIFLARE_LOG_LEVEL",
 	DATA_LIVE_RELOAD_SCRIPT: "MINIFLARE_LIVE_RELOAD_SCRIPT",
 	DURABLE_OBJECT_NAMESPACE_PROXY: "MINIFLARE_PROXY",
+	DATA_PROXY_SECRET: "MINIFLARE_PROXY_SECRET",
 } as const;
 
 export const ProxyOps = {

diff --git a/packages/miniflare/src/workers/core/proxy.worker.ts b/packages/miniflare/src/workers/core/proxy.worker.ts
@@ -1,7 +1,9 @@
 import assert from "node:assert";
+import { Buffer } from "node:buffer";
 import { parse } from "devalue";
 import { readPrefix, reduceError } from "miniflare:shared";
 import {
+	CoreBindings,
 	CoreHeaders,
 	ProxyAddresses,
 	ProxyOps,
@@ -59,6 +61,10 @@ function getType(value: unknown) {
 	return Object.prototype.toString.call(value).slice(8, -1); // `[object <type>]`
 }
 
+type Env = Record<string, unknown> & {
+	[CoreBindings.DATA_PROXY_SECRET]: ArrayBuffer;
+};
+
 // TODO(someday): extract `ProxyServer` into component that could be used by
 //  other (user) Durable Objects
 export class ProxyServer implements DurableObject {
@@ -105,7 +111,10 @@ export class ProxyServer implements DurableObject {
 	};
 	nativeReviver: ReducersRevivers = { Native: this.revivers.Native };
 
-	constructor(_state: DurableObjectState, env: Record<string, unknown>) {
+	constructor(
+		_state: DurableObjectState,
+		readonly env: Env
+	) {
 		this.heap.set(ProxyAddresses.GLOBAL, globalThis);
 		this.heap.set(ProxyAddresses.ENV, env);
 	}
@@ -123,6 +132,15 @@ export class ProxyServer implements DurableObject {
 	}
 
 	async #fetch(request: Request) {
+		// Validate secret header to prevent unauthorised access to proxy
+		const secretHex = request.headers.get(CoreHeaders.OP_SECRET);
+		if (secretHex == null) return new Response(null, { status: 401 });
+		const expectedSecret = this.env[CoreBindings.DATA_PROXY_SECRET];
+		const secretBuffer = Buffer.from(secretHex, "hex");
+		if (!crypto.subtle.timingSafeEqual(secretBuffer, expectedSecret)) {
+			return new Response(null, { status: 401 });
+		}
+
 		const opHeader = request.headers.get(CoreHeaders.OP);
 		const targetHeader = request.headers.get(CoreHeaders.OP_TARGET);
 		const keyHeader = request.headers.get(CoreHeaders.OP_KEY);
@@ -188,6 +206,7 @@ export class ProxyServer implements DurableObject {
 				const url = new URL(originalUrl ?? request.url);
 				// Create a new request to allow header mutation and use original URL
 				request = new Request(url, request);
+				request.headers.delete(CoreHeaders.OP_SECRET);
 				request.headers.delete(CoreHeaders.OP);
 				request.headers.delete(CoreHeaders.OP_TARGET);
 				request.headers.delete(CoreHeaders.OP_KEY);

diff --git a/packages/miniflare/src/workers/shared/constants.ts b/packages/miniflare/src/workers/shared/constants.ts
@@ -7,6 +7,7 @@ export const SharedBindings = {
 	DURABLE_OBJECT_NAMESPACE_OBJECT: "MINIFLARE_OBJECT",
 	MAYBE_SERVICE_BLOBS: "MINIFLARE_BLOBS",
 	MAYBE_SERVICE_LOOPBACK: "MINIFLARE_LOOPBACK",
+	MAYBE_JSON_ENABLE_CONTROL_ENDPOINTS: "MINIFLARE_ENABLE_CONTROL_ENDPOINTS",
 } as const;
 
 export enum LogLevel {

diff --git a/packages/miniflare/src/workers/shared/object.worker.ts b/packages/miniflare/src/workers/shared/object.worker.ts
@@ -18,6 +18,10 @@ export interface MiniflareDurableObjectEnv {
 	// NOTE: this binding is optional so simulators can run standalone, without
 	// a Node.js loopback server. In this case, logging is a no-op.
 	[SharedBindings.MAYBE_SERVICE_LOOPBACK]?: Fetcher;
+	// If set to `true`, Miniflare enables additional endpoints in Durable Objects
+	// for testing. Note these endpoints allow anyone with access to the Miniflare
+	// dev server to run arbitrary SQL queries and read arbitrary blobs.
+	[SharedBindings.MAYBE_JSON_ENABLE_CONTROL_ENDPOINTS]?: boolean;
 }
 
 export interface MiniflareDurableObjectCfControlOp {
@@ -119,8 +123,10 @@ export abstract class MiniflareDurableObject<
 	async fetch(req: Request<unknown, MiniflareDurableObjectCf>) {
 		// Allow control of object internals by specifying operations in the `cf`
 		// object. Used by tests to update fake time, and access internal storage.
-		const controlOp = req?.cf?.miniflare?.controlOp;
-		if (controlOp !== undefined) return this.#handleControlOp(controlOp);
+		if (this.env[SharedBindings.MAYBE_JSON_ENABLE_CONTROL_ENDPOINTS] === true) {
+			const controlOp = req?.cf?.miniflare?.controlOp;
+			if (controlOp !== undefined) return this.#handleControlOp(controlOp);
+		}
 
 		// Each regular request to a `MiniflareDurableObject` includes the object
 		// ID's name, so we can create the `BlobStore`. Note, we could just use the

diff --git a/packages/miniflare/test/plugins/core/proxy/client.spec.ts b/packages/miniflare/test/plugins/core/proxy/client.spec.ts
@@ -13,6 +13,7 @@ import {
 	ReplaceWorkersTypes,
 	Response,
 	WebSocketPair,
+	fetch,
 } from "miniflare";
 
 // This file tests API proxy edge cases. Cache, D1, Durable Object and R2 tests
@@ -265,3 +266,13 @@ test("ProxyClient: can `JSON.stringify()` proxies", async (t) => {
 		version: object.version,
 	});
 });
+
+test("ProxyServer: prevents unauthorised access", async (t) => {
+	const mf = new Miniflare({ script: nullScript });
+	t.teardown(() => mf.dispose());
+
+	const url = await mf.ready;
+	const res = await fetch(url, { headers: { "MF-Op": "GET" } });
+	t.is(res.status, 401);
+	await res.arrayBuffer(); // (drain)
+});
diff --git a/packages/miniflare/test/setup.mjs b/packages/miniflare/test/setup.mjs
@@ -1,5 +1,8 @@
 import Module from "node:module";
-import { _initialiseInstanceRegistry } from "../dist/src/index.js";
+import {
+	_initialiseInstanceRegistry,
+	_enableControlEndpoints,
+} from "../dist/src/index.js";
 import { fileURLToPath } from "node:url";
 import path from "node:path";
 
@@ -20,6 +23,8 @@ const registry = _initialiseInstanceRegistry();
 const bigSeparator = "=".repeat(80);
 const separator = "-".repeat(80);
 
+_enableControlEndpoints();
+
 // `process.on("exit")` is more like `worker_thread.on(`exit`)` here. It will
 // be called once AVA's finished running tests and `after` hooks. Note we can't
 // use an `after` hook here, as that would run before `miniflareTest`'s

diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts
@@ -30,7 +30,7 @@ describe("normalizeAndValidateConfig()", () => {
 			constellation: [],
 			hyperdrive: [],
 			dev: {
-				ip: "*",
+				ip: "localhost",
 				local_protocol: "http",
 				port: undefined, // the default of 8787 is set at runtime
 				upstream_protocol: "https",