[workers-utils] Stop container key validation from crashing on malformed input

.changeset/container-key-validation-crash.md

@@ -0,0 +1,9 @@
+---
+"@cloudflare/workers-utils": patch
+---
+
+Report malformed container SSH keys and a non-object `containers.configuration` as config errors instead of crashing
+
+Previously, a `containers` entry with a malformed `authorized_keys` or `trusted_user_ca_keys` entry (a missing or non-string `public_key`, or an entry that is not an object), or a `containers.configuration` set to `null`, made Wrangler exit with a stack trace and "If you think this is a bug, please open an issue" rather than pointing at the field.
+
+These configurations now produce an ordinary configuration error naming the offending field and array index, such as `containers.authorized_keys[0].public_key must be a string`. A `public_key` that is not an ED25519 key is also now reported with correct grammar.

packages/workers-utils/src/config/validation.ts

@@ -3375,6 +3375,49 @@ const validateBindingArray =
 		return isValid;
 	};
 
+/**
+ * Validate a list of SSH public key entries, used by both `containers.authorized_keys`
+ * and `containers.trusted_user_ca_keys`. Each check gates the next one, so a malformed
+ * entry is reported as a configuration error rather than dereferenced.
+ */
+function validateSshPublicKeys(
+	diagnostics: Diagnostics,
+	field: string,
+	value: unknown,
+	nameRequired: boolean
+): void {
+	if (!Array.isArray(value)) {
+		diagnostics.errors.push(`${field} must be an array`);
+		return;
+	}
+
+	for (const [index, key] of value.entries()) {
+		const fieldPath = `${field}[${index}]`;
+
+		if (typeof key !== "object" || key === null || Array.isArray(key)) {
+			diagnostics.errors.push(`${fieldPath} must be an object`);
+			continue;
+		}
+
+		const hasValidName = nameRequired
+			? isRequiredProperty(key, "name", "string")
+			: isOptionalProperty(key, "name", "string");
+		if (!hasValidName) {
+			diagnostics.errors.push(`${fieldPath}.name must be a string`);
+		}
+
+		if (
+			!isRequiredProperty<{ public_key: string }>(key, "public_key", "string")
+		) {
+			diagnostics.errors.push(`${fieldPath}.public_key must be a string`);
+		} else if (!key.public_key.toLowerCase().startsWith("ssh-ed25519")) {
+			diagnostics.errors.push(
+				`${fieldPath}.public_key is an unsupported key type. Please provide an ED25519 public key.`
+			);
+		}
+	}
+}
+
 function validateContainerApp(
 	envName: string,
 	topLevelName: string | undefined,
@@ -3449,14 +3492,13 @@ function validateContainerApp(
 				);
 				if (
 					typeof containerAppOptional.configuration !== "object" ||
+					containerAppOptional.configuration === null ||
 					Array.isArray(containerAppOptional.configuration)
 				) {
 					diagnostics.errors.push(
 						`"containers.configuration" should be an object`
 					);
-				}
-
-				if (
+				} else if (
 					containerAppOptional.instance_type &&
 					(containerAppOptional.configuration.disk !== undefined ||
 						containerAppOptional.configuration.vcpu !== undefined ||
@@ -3683,7 +3725,11 @@ function validateContainerApp(
 					"unsafe",
 				]
 			);
-			if ("configuration" in containerAppOptional) {
+			if (
+				typeof containerAppOptional.configuration === "object" &&
+				containerAppOptional.configuration !== null &&
+				!Array.isArray(containerAppOptional.configuration)
+			) {
 				validateAdditionalProperties(
 					diagnostics,
 					`${field}.configuration`,
@@ -3731,59 +3777,21 @@ function validateContainerApp(
 			}
 
 			if ("authorized_keys" in containerAppOptional) {
-				if (!Array.isArray(containerAppOptional.authorized_keys)) {
-					diagnostics.errors.push(`${field}.authorized_keys must be an array`);
-				} else {
-					for (const index in containerAppOptional.authorized_keys) {
-						const fieldPath = `${field}.authorized_keys[${index}]`;
-						const key = containerAppOptional.authorized_keys[index];
-
-						if (!isRequiredProperty(key, "name", "string")) {
-							diagnostics.errors.push(`${fieldPath}.name must be a string`);
-						}
-
-						if (!isRequiredProperty(key, "public_key", "string")) {
-							diagnostics.errors.push(
-								`${fieldPath}.public_key must be a string`
-							);
-						}
-
-						if (!key.public_key.toLowerCase().startsWith("ssh-ed25519")) {
-							diagnostics.errors.push(
-								`${fieldPath}.public_key is a unsupported key type. Please provide a ED25519 public key.`
-							);
-						}
-					}
-				}
+				validateSshPublicKeys(
+					diagnostics,
+					`${field}.authorized_keys`,
+					containerAppOptional.authorized_keys,
+					true
+				);
 			}
 
 			if ("trusted_user_ca_keys" in containerAppOptional) {
-				if (!Array.isArray(containerAppOptional.trusted_user_ca_keys)) {
-					diagnostics.errors.push(
-						`${field}.trusted_user_ca_keys must be an array`
-					);
-				} else {
-					for (const index in containerAppOptional.trusted_user_ca_keys) {
-						const fieldPath = `${field}.trusted_user_ca_keys[${index}]`;
-						const key = containerAppOptional.trusted_user_ca_keys[index];
-
-						if (!isOptionalProperty(key, "name", "string")) {
-							diagnostics.errors.push(`${fieldPath}.name must be a string`);
-						}
-
-						if (!isRequiredProperty(key, "public_key", "string")) {
-							diagnostics.errors.push(
-								`${fieldPath}.public_key must be a string`
-							);
-						}
-
-						if (!key.public_key.toLowerCase().startsWith("ssh-ed25519")) {
-							diagnostics.errors.push(
-								`${fieldPath}.public_key is a unsupported key type. Please provide a ED25519 public key.`
-							);
-						}
-					}
-				}
+				validateSshPublicKeys(
+					diagnostics,
+					`${field}.trusted_user_ca_keys`,
+					containerAppOptional.trusted_user_ca_keys,
+					false
+				);
 			}
 
 			if (

packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts

@@ -3665,6 +3665,55 @@ describe("normalizeAndValidateConfig()", () => {
 				`);
 			});
 
+			it("should error if containers.configuration is null", ({ expect }) => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						name: "test-worker",
+						containers: [
+							{
+								class_name: "test-class",
+								image: "registry.cloudflare.com/test:latest",
+								configuration: null,
+							},
+						],
+					} as unknown as RawConfig,
+					undefined,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					  - "containers.configuration" should be an object"
+				`);
+			});
+
+			it("should error if containers.configuration is null and instance_type is set", ({
+				expect,
+			}) => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						name: "test-worker",
+						containers: [
+							{
+								class_name: "test-class",
+								image: "registry.cloudflare.com/test:latest",
+								configuration: null,
+								instance_type: "lite",
+							},
+						],
+					} as unknown as RawConfig,
+					undefined,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					  - "containers.configuration" should be an object"
+				`);
+			});
+
 			it("should error if no containers name and no worker name are provided", ({
 				expect,
 			}) => {
@@ -3957,6 +4006,163 @@ describe("normalizeAndValidateConfig()", () => {
 				`);
 			});
 
+			it("should error if an authorized_keys entry has no public_key", ({
+				expect,
+			}) => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						name: "test-worker",
+						containers: [
+							{
+								class_name: "test-class",
+								image: "registry.cloudflare.com/test:latest",
+								authorized_keys: [{ name: "laptop" }],
+							},
+						],
+					} as unknown as RawConfig,
+					undefined,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					  - containers.authorized_keys[0].public_key must be a string"
+				`);
+			});
+
+			it("should error if an authorized_keys entry is not an object", ({
+				expect,
+			}) => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						name: "test-worker",
+						containers: [
+							{
+								class_name: "test-class",
+								image: "registry.cloudflare.com/test:latest",
+								authorized_keys: ["ssh-ed25519 AAAAC3NzaC1lZDI1NTE5"],
+							},
+						],
+					} as unknown as RawConfig,
+					undefined,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					  - containers.authorized_keys[0] must be an object"
+				`);
+			});
+
+			it("should error if an authorized_keys public_key is not a string", ({
+				expect,
+			}) => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						name: "test-worker",
+						containers: [
+							{
+								class_name: "test-class",
+								image: "registry.cloudflare.com/test:latest",
+								authorized_keys: [{ name: "laptop", public_key: 42 }],
+							},
+						],
+					} as unknown as RawConfig,
+					undefined,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					  - containers.authorized_keys[0].public_key must be a string"
+				`);
+			});
+
+			it("should error if a trusted_user_ca_keys entry has no public_key", ({
+				expect,
+			}) => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						name: "test-worker",
+						containers: [
+							{
+								class_name: "test-class",
+								image: "registry.cloudflare.com/test:latest",
+								trusted_user_ca_keys: [{ name: "ca" }],
+							},
+						],
+					} as unknown as RawConfig,
+					undefined,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					  - containers.trusted_user_ca_keys[0].public_key must be a string"
+				`);
+			});
+
+			it("should error if an authorized_keys public_key is not an ED25519 key", ({
+				expect,
+			}) => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						name: "test-worker",
+						containers: [
+							{
+								class_name: "test-class",
+								image: "registry.cloudflare.com/test:latest",
+								authorized_keys: [
+									{ name: "laptop", public_key: "ssh-rsa AAAAB3NzaC1yc2E" },
+								],
+							},
+						],
+					} as unknown as RawConfig,
+					undefined,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					  - containers.authorized_keys[0].public_key is an unsupported key type. Please provide an ED25519 public key."
+				`);
+			});
+
+			it("should accept valid authorized_keys and trusted_user_ca_keys", ({
+				expect,
+			}) => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						name: "test-worker",
+						containers: [
+							{
+								class_name: "test-class",
+								image: "registry.cloudflare.com/test:latest",
+								authorized_keys: [
+									{
+										name: "laptop",
+										public_key: "ssh-ed25519 AAAAC3NzaC1lZDI1",
+									},
+								],
+								trusted_user_ca_keys: [
+									{ public_key: "ssh-ed25519 AAAAC3NzaC1lZDI1" },
+								],
+							},
+						],
+					} as unknown as RawConfig,
+					undefined,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.hasErrors()).toBe(false);
+			});
+
 			it.for([{ value: 25 }, { value: [20, 50, 100] }])(
 				"should accept rollout_step_percentage set to $value",
 				(value, { expect }) => {