fix: allow relative modules scriptPath outside of CWD (#4954)

.changeset/gentle-mayflies-shout.md

@@ -0,0 +1,17 @@
+---
+"miniflare": patch
+---
+
+fix: allow relative `scriptPath`/`modulesRoot`s to break out of current working directory
+
+Previously, Miniflare would resolve relative `scriptPath`s against `moduleRoot` multiple times resulting in incorrect paths and module names. This would lead to `can't use ".." to break out of starting directory` `workerd` errors. This change ensures Miniflare uses `scriptPath` as is, and only resolves it relative to `modulesRoot` when computing module names. Note this bug didn't affect service workers. This allows you to reference a modules `scriptPath` outside the working directory with something like:
+
+```js
+const mf = new Miniflare({
+	modules: true,
+	modulesRoot: "..",
+	scriptPath: "../worker.mjs",
+});
+```
+
+Fixes #4721

packages/miniflare/src/plugins/core/index.ts

@@ -721,8 +721,9 @@ function getWorkerScript(
 	workerIndex: number,
 	additionalModuleNames: string[]
 ): { serviceWorkerScript: string } | { modules: Worker_Module[] } {
-	const modulesRoot =
-		("modulesRoot" in options ? options.modulesRoot : undefined) ?? "";
+	const modulesRoot = path.resolve(
+		("modulesRoot" in options ? options.modulesRoot : undefined) ?? ""
+	);
 	if (Array.isArray(options.modules)) {
 		// If `modules` is a manually defined modules array, use that
 		return {

packages/miniflare/src/plugins/core/modules.ts

@@ -174,8 +174,6 @@ export class ModuleLocator {
 	}
 
 	visitEntrypoint(code: string, modulePath: string) {
-		modulePath = path.resolve(this.modulesRoot, modulePath);
-
 		// If we've already visited this path, return
 		if (this.#visitedPaths.has(modulePath)) return;
 		this.#visitedPaths.add(modulePath);

packages/miniflare/test/plugins/core/modules.spec.ts

@@ -1,8 +1,9 @@
 import assert from "assert";
+import fs from "fs/promises";
 import path from "path";
 import test from "ava";
 import { Miniflare, MiniflareCoreError, stripAnsi } from "miniflare";
-import { utf8Encode } from "../../test-shared";
+import { useCwd, useTmp, utf8Encode } from "../../test-shared";
 
 const ROOT = path.resolve(
 	__dirname,
@@ -278,6 +279,30 @@ You must manually define your modules when constructing Miniflare:
     at ${depPath}:2:8`
 	);
 });
+test.serial(
+	"Miniflare: collects modules outside of working directory",
+	async (t) => {
+		// https://github.com/cloudflare/workers-sdk/issues/4721
+		const tmp = await useTmp(t);
+		const child = path.join(tmp, "child");
+		await fs.mkdir(child);
+		await fs.writeFile(
+			path.join(tmp, "worker.mjs"),
+			'export default { fetch() { return new Response("body"); } }'
+		);
+		useCwd(t, child);
+
+		const mf = new Miniflare({
+			modules: true,
+			modulesRoot: "..",
+			scriptPath: "../worker.mjs",
+		});
+		t.teardown(() => mf.dispose());
+
+		const res = await mf.dispatchFetch("http://localhost");
+		t.is(await res.text(), "body");
+	}
+);
 test("Miniflare: suggests bundling on unknown module", async (t) => {
 	// Try with npm-package-like import
 	let mf = new Miniflare({

packages/miniflare/test/test-shared/storage.ts

@@ -32,6 +32,18 @@ export async function useTmp(t: ExecutionContext): Promise<string> {
 	return filePath;
 }
 
+// Must be called from a `.serial()` test
+export function useCwd(t: ExecutionContext, cwd: string) {
+	const originalCwd = process.cwd();
+	const originalPWD = process.env.PWD;
+	process.chdir(cwd);
+	process.env.PWD = cwd;
+	t.teardown(() => {
+		process.chdir(originalCwd);
+		process.env.PWD = originalPWD;
+	});
+}
+
 type ValidReadableStreamBYOBRequest = Omit<
 	ReadableStreamBYOBRequest,
 	"view"