fix: do not allow non-string values in bulk secret uploads (#5832)

Prior to Wrangler 3.4.0 we displayed an error if the user tried to upload a
JSON file that contained non-string secrets, since these are not supported
by the Cloudflare backend.

This change reintroduces that check to give the user a helpful error message
rather than a cryptic `workers.api.error.invalid_script_config` error code.

.changeset/real-waves-wave.md

@@ -0,0 +1,12 @@
+---
+"wrangler": patch
+---
+
+fix: do not allow non-string values in bulk secret uploads
+
+Prior to Wrangler 3.4.0 we displayed an error if the user tried to upload a
+JSON file that contained non-string secrets, since these are not supported
+by the Cloudflare backend.
+
+This change reintroduces that check to give the user a helpful error message
+rather than a cryptic `workers.api.error.invalid_script_config` error code.

packages/wrangler/src/__tests__/secret.test.ts

@@ -571,7 +571,7 @@ describe("wrangler secret", () => {
 			expect(std.err).toMatchInlineSnapshot(`""`);
 		});
 
-		it("should create secret:bulk", async () => {
+		it("should create secrets from JSON file", async () => {
 			writeFileSync(
 				"secret.json",
 				JSON.stringify({
@@ -614,6 +614,31 @@ describe("wrangler secret", () => {
 			expect(std.err).toMatchInlineSnapshot(`""`);
 		});
 
+		it("should fail if file is not valid JSON", async () => {
+			writeFileSync("secret.json", "bad file content");
+
+			await expect(
+				runWrangler("secret:bulk ./secret.json --name script-name")
+			).rejects.toThrowErrorMatchingInlineSnapshot(
+				`"The contents of \\"./secret.json\\" is not valid JSON: \\"ParseError: Unexpected token b\\""`
+			);
+		});
+
+		it("should fail if JSON file contains a record with non-string values", async () => {
+			writeFileSync(
+				"secret.json",
+				JSON.stringify({
+					"invalid-secret": 999,
+				})
+			);
+
+			await expect(
+				runWrangler("secret:bulk ./secret.json --name script-name")
+			).rejects.toThrowErrorMatchingInlineSnapshot(
+				`"The value for \\"invalid-secret\\" in \\"./secret.json\\" is not a \\"string\\" instead it is of type \\"number\\""`
+			);
+		});
+
 		it("should count success and network failure on secret:bulk", async () => {
 			writeFileSync(
 				"secret.json",

packages/wrangler/src/secret/index.ts

@@ -5,7 +5,7 @@ import { fetchResult } from "../cfetch";
 import { readConfig } from "../config";
 import { createWorkerUploadForm } from "../deployment-bundle/create-worker-upload-form";
 import { confirm, prompt } from "../dialogs";
-import { UserError } from "../errors";
+import { FatalError, UserError } from "../errors";
 import {
 	getLegacyScriptName,
 	isLegacyEnv,
@@ -380,10 +380,17 @@ export const secretBulkHandler = async (secretBulkArgs: SecretBulkArgs) => {
 	let content: Record<string, string>;
 	if (secretBulkArgs.json) {
 		const jsonFilePath = path.resolve(secretBulkArgs.json);
-		content = parseJSON<Record<string, string>>(
-			readFileSync(jsonFilePath),
-			jsonFilePath
-		);
+		try {
+			content = parseJSON<Record<string, string>>(
+				readFileSync(jsonFilePath),
+				jsonFilePath
+			);
+		} catch (e) {
+			throw new FatalError(
+				`The contents of "${secretBulkArgs.json}" is not valid JSON: "${e}"`
+			);
+		}
+		validateJSONFileSecrets(content, secretBulkArgs.json);
 	} else {
 		try {
 			const rl = readline.createInterface({ input: process.stdin });
@@ -487,3 +494,22 @@ export const secretBulkHandler = async (secretBulkArgs: SecretBulkArgs) => {
 		throw new Error(`🚨 ${upsertBindings.length} secrets failed to upload`);
 	}
 };
+
+function validateJSONFileSecrets(
+	content: unknown,
+	jsonFilePath: string
+): asserts content is Record<string, string> {
+	if (content === null || typeof content !== "object") {
+		throw new FatalError(
+			`The contents of "${jsonFilePath}" is not valid. It should be a JSON object of string values.`
+		);
+	}
+	const entries = Object.entries(content);
+	for (const [key, value] of entries) {
+		if (typeof value !== "string") {
+			throw new FatalError(
+				`The value for "${key}" in "${jsonFilePath}" is not a "string" instead it is of type "${typeof value}"`
+			);
+		}
+	}
+}