[playground-preview-worker] fix: don't report invalid upload input

packages/playground-preview-worker/src/index.ts

@@ -133,6 +133,13 @@ class PreviewRequestForbidden extends HttpError {
 	}
 }
 
+export class BadUpload extends HttpError {
+	name = "BadUpload";
+	constructor(message = "Invalid upload") {
+		super(message, 400, false);
+	}
+}
+
 /**
  * Given a preview token, this endpoint allows for raw http calls to be inspected
  *

packages/playground-preview-worker/src/sentry.ts

@@ -1,5 +1,5 @@
 import { Toucan } from "toucan-js";
-import { z, ZodError } from "zod";
+import { ZodError } from "zod";
 import { HttpError, ZodSchemaError } from ".";
 
 export function handleException(e: unknown, sentry: Toucan): Response {

packages/playground-preview-worker/src/user.do.ts

@@ -1,5 +1,6 @@
 import assert from "node:assert";
 import { Buffer } from "node:buffer";
+import z from "zod";
 import { constructMiddleware } from "./inject-middleware";
 import {
 	doUpload,
@@ -8,7 +9,7 @@ import {
 	UploadResult,
 } from "./realish";
 import { handleException, setupSentry } from "./sentry";
-import { ServiceWorkerNotSupported, WorkerTimeout } from ".";
+import { BadUpload, ServiceWorkerNotSupported, WorkerTimeout } from ".";
 
 const encoder = new TextEncoder();
 
@@ -26,6 +27,15 @@ function switchRemote(url: URL, remote: string) {
 	return workerUrl;
 }
 
+const UploadedMetadata = z.object({
+	body_part: z.ostring(),
+	main_module: z.ostring(),
+	compatibility_date: z.ostring(),
+	compatibility_flags: z.array(z.string()).optional(),
+});
+
+type UploadedMetadata = z.infer<typeof UploadedMetadata>;
+
 /**
  * This Durable object coordinates operations for a specific user session. It's purpose is to
  * communicate with the Realish preview service on behalf of a user, without leaking more info
@@ -114,17 +124,9 @@ export class UserSession {
 		await this.state.storage.put("inspectorUrl", this.inspectorUrl);
 	}
 
-	async fetch(request: Request) {
+	async handleRequest(request: Request) {
 		const url = new URL(request.url);
-		// We need to construct a new Sentry instance here because throwing
-		// errors across a DO boundary will wipe stack information etc...
-		const sentry = setupSentry(
-			request,
-			undefined,
-			this.env.SENTRY_DSN,
-			this.env.SENTRY_ACCESS_CLIENT_ID,
-			this.env.SENTRY_ACCESS_CLIENT_SECRET
-		);
+
 		// This is an inspector request. Forward to the correct inspector URL
 		if (request.headers.get("Upgrade") && url.pathname === "/api/inspector") {
 			assert(this.inspectorUrl !== undefined);
@@ -158,17 +160,33 @@ export class UserSession {
 			}
 			return workerResponse;
 		}
+
 		const userSession = this.state.id.toString();
-		const worker = await request.formData();
 
-		const m = worker.get("metadata");
+		let worker: FormData;
+		try {
+			worker = await request.formData();
+		} catch (e) {
+			throw new BadUpload(`Expected valid form data`);
+		}
 
-		assert(m instanceof File);
+		const m = worker.get("metadata");
+		if (!(m instanceof File)) {
+			throw new BadUpload("Expected metadata file to be defined");
+		}
 
-		const uploadedMetadata = JSON.parse(await m.text());
+		let uploadedMetadata: UploadedMetadata;
+		try {
+			uploadedMetadata = UploadedMetadata.parse(JSON.parse(await m.text()));
+		} catch {
+			throw new BadUpload("Expected metadata file to be valid");
+		}
 
-		if ("body_part" in uploadedMetadata) {
-			return new ServiceWorkerNotSupported().toResponse();
+		if (
+			uploadedMetadata.body_part !== undefined ||
+			uploadedMetadata.main_module === undefined
+		) {
+			throw new ServiceWorkerNotSupported();
 		}
 
 		const today = new Date();
@@ -206,19 +224,33 @@ export class UserSession {
 			})
 		);
 
-		try {
-			await this.uploadWorker(this.workerName, worker);
+		await this.uploadWorker(this.workerName, worker);
 
-			assert(this.inspectorUrl !== undefined);
+		assert(this.inspectorUrl !== undefined);
+
+		return Response.json({
+			// Include a hash of the inspector URL so as to ensure the client will reconnect
+			// when the inspector URL has changed (because of an updated preview session)
+			inspector: `/api/inspector?user=${userSession}&h=${await hash(
+				this.inspectorUrl
+			)}`,
+			preview: userSession,
+		});
+	}
 
-			return Response.json({
-				// Include a hash of the inspector URL so as to ensure the client will reconnect
-				// when the inspector URL has changed (because of an updated preview session)
-				inspector: `/api/inspector?user=${userSession}&h=${await hash(
-					this.inspectorUrl
-				)}`,
-				preview: userSession,
-			});
+	async fetch(request: Request) {
+		// We need to construct a new Sentry instance here because throwing
+		// errors across a DO boundary will wipe stack information etc...
+		const sentry = setupSentry(
+			request,
+			undefined,
+			this.env.SENTRY_DSN,
+			this.env.SENTRY_ACCESS_CLIENT_ID,
+			this.env.SENTRY_ACCESS_CLIENT_SECRET
+		);
+
+		try {
+			return await this.handleRequest(request);
 		} catch (e) {
 			return handleException(e, sentry);
 		}

packages/playground-preview-worker/tests/index.test.ts

@@ -5,9 +5,9 @@ const REMOTE = "https://playground-testing.devprod.cloudflare.dev";
 const PREVIEW_REMOTE =
 	"https://random-data.playground-testing.devprod.cloudflare.dev";
 
-const TEST_WORKER_CONTENT_TYPE =
-	"multipart/form-data; boundary=----WebKitFormBoundaryqJEYLXuUiiZQHgvf";
-const TEST_WORKER = `------WebKitFormBoundaryqJEYLXuUiiZQHgvf
+const TEST_WORKER_BOUNDARY = "----WebKitFormBoundaryqJEYLXuUiiZQHgvf";
+const TEST_WORKER_CONTENT_TYPE = `multipart/form-data; boundary=${TEST_WORKER_BOUNDARY}`;
+const TEST_WORKER = `--${TEST_WORKER_BOUNDARY}
 Content-Disposition: form-data; name="index.js"; filename="index.js"
 Content-Type: application/javascript+module
 
@@ -39,12 +39,12 @@ export default {
 	}
 }
 
-------WebKitFormBoundaryqJEYLXuUiiZQHgvf
+--${TEST_WORKER_BOUNDARY}
 Content-Disposition: form-data; name="metadata"; filename="blob"
 Content-Type: application/json
 
 {"compatibility_date":"2023-05-04","main_module":"index.js"}
-------WebKitFormBoundaryqJEYLXuUiiZQHgvf--`;
+--${TEST_WORKER_BOUNDARY}--`;
 
 async function fetchUserToken() {
 	return fetch(REMOTE).then(
@@ -318,6 +318,104 @@ describe("Upload Worker", () => {
 			'"{\\"error\\":\\"UploadFailed\\",\\"message\\":\\"Valid token not provided\\",\\"data\\":{}}"'
 		);
 	});
+	it("should reject invalid form data", async () => {
+		const w = await fetch(`${REMOTE}/api/worker`, {
+			method: "POST",
+			headers: {
+				cookie: `user=${defaultUserToken}`,
+				"Content-Type": "text/plain",
+			},
+			body: "not a form",
+		});
+		expect(w.status).toBe(400);
+		expect(await w.text()).toMatchInlineSnapshot(
+			'"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected valid form data\\",\\"data\\":{}}"'
+		);
+	});
+	it("should reject missing metadata", async () => {
+		const w = await fetch(`${REMOTE}/api/worker`, {
+			method: "POST",
+			headers: {
+				cookie: `user=${defaultUserToken}`,
+				"Content-Type": "text/plain",
+			},
+			body: `--${TEST_WORKER_BOUNDARY}
+Content-Disposition: form-data; name="index.js"; filename="index.js"
+Content-Type: application/javascript+module
+
+export default {
+	fetch(request) { return new Response("body"); }
+}
+
+--${TEST_WORKER_BOUNDARY}--`,
+		});
+		expect(w.status).toBe(400);
+		expect(await w.text()).toMatchInlineSnapshot(
+			'"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected valid form data\\",\\"data\\":{}}"'
+		);
+	});
+	it("should reject invalid metadata json", async () => {
+		const w = await fetch(`${REMOTE}/api/worker`, {
+			method: "POST",
+			headers: {
+				cookie: `user=${defaultUserToken}`,
+				"Content-Type": TEST_WORKER_CONTENT_TYPE,
+			},
+			body: `--${TEST_WORKER_BOUNDARY}
+Content-Disposition: form-data; name="metadata"; filename="blob"
+Content-Type: application/json
+
+{"compatibility_date":"2023-05-04",
+--${TEST_WORKER_BOUNDARY}--`,
+		});
+		expect(w.status).toBe(400);
+		expect(await w.text()).toMatchInlineSnapshot(
+			'"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected metadata file to be valid\\",\\"data\\":{}}"'
+		);
+	});
+	it("should reject invalid metadata", async () => {
+		const w = await fetch(`${REMOTE}/api/worker`, {
+			method: "POST",
+			headers: {
+				cookie: `user=${defaultUserToken}`,
+				"Content-Type": TEST_WORKER_CONTENT_TYPE,
+			},
+			body: `--${TEST_WORKER_BOUNDARY}
+Content-Disposition: form-data; name="metadata"; filename="blob"
+Content-Type: application/json
+
+{"compatibility_date":42,"main_module":"index.js"}
+--${TEST_WORKER_BOUNDARY}--`,
+		});
+		expect(w.status).toBe(400);
+		expect(await w.text()).toMatchInlineSnapshot(
+			'"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected metadata file to be valid\\",\\"data\\":{}}"'
+		);
+	});
+	it("should reject service worker", async () => {
+		const w = await fetch(`${REMOTE}/api/worker`, {
+			method: "POST",
+			headers: {
+				cookie: `user=${defaultUserToken}`,
+				"Content-Type": TEST_WORKER_CONTENT_TYPE,
+			},
+			body: `--${TEST_WORKER_BOUNDARY}
+Content-Disposition: form-data; name="index.js"; filename="index.js"
+Content-Type: application/javascript
+
+addEventListener("fetch", (event) => event.respondWith(new Response("body")));
+--${TEST_WORKER_BOUNDARY}
+Content-Disposition: form-data; name="metadata"; filename="blob"
+Content-Type: application/json
+
+{"compatibility_date":"2023-05-04","body_part":"index.js"}
+--${TEST_WORKER_BOUNDARY}--`,
+		});
+		expect(w.status).toBe(400);
+		expect(await w.text()).toMatchInlineSnapshot(
+			'"{\\"error\\":\\"ServiceWorkerNotSupported\\",\\"message\\":\\"Service Workers are not supported in the Workers Playground\\",\\"data\\":{}}"'
+		);
+	});
 });
 
 describe("Raw HTTP preview", () => {