diff --git a/.changeset/container-key-validation-crash.md b/.changeset/container-key-validation-crash.md new file mode 100644 index 00000000000..f47673ea83e --- /dev/null +++ b/.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. diff --git a/packages/workers-utils/src/config/validation.ts b/packages/workers-utils/src/config/validation.ts index 215d2df0fa0..586ca35f7c5 100644 --- a/packages/workers-utils/src/config/validation.ts +++ b/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 ( diff --git a/packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts b/packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts index 4e92ecaa680..e529b3693d4 100644 --- a/packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts +++ b/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 }) => {