aws-cloudformation / cloudformation-resource-schema

The CloudFormation Resource Schema defines the shape and semantic for resources provisioned by CloudFormation. It is used by provider developers using the CloudFormation RPDK.
Apache License 2.0
93 stars 38 forks source link

Scrub messages #40

Closed aygold92 closed 4 years ago

aygold92 commented 4 years ago

Issue #, if available:

Description of changes: Ensure that during schema validation, no messages contain values.

The strategy is to use keywords and change the error message for the ones that emit values.

Certain keywords were whitelisted as safe, in case we start using a new keyword. All other keywords fail with message: <SchemaPointer>: failed validation constraint for keyword [<keyword>]

The following emit values:

Bumped the version because it has backwards incompatible changes (changed schemaLocation -> schemaPointer). Will need to fix the java plugin as well

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

aygold92 commented 4 years ago

build fails saying

 serializable class <anonymous com.amazonaws.cloudformation.resource.exceptions.ValidationException$1> has no definition of serialVersionUID 

but I didn't change that and the class defines the serialVersionUID...

PatMyron commented 3 years ago

This currently scrubs too much information. In addition to scrubbing the property value, it also removes the constraint itself, which is the most important context. Is it possible to only scrub writeOnlyProperties?

aygold92 commented 3 years ago

This currently scrubs too much information. In addition to scrubbing the property value, it also removes the constraint itself, which is the most important context. Is it possible to only scrub writeOnlyProperties?

What do you mean it removes the constraint? Is there a bug? As noted in description, the message should be <SchemaPointer>: failed validation constraint for keyword [<keyword>]

PatMyron commented 3 years ago

The actual pattern/enum/minimum/maximum/etc. is the most important context. Knowing which pattern you failed is far more useful than just knowing you failed some pattern that's been scrubbed, and we could reveal these without revealing property values:

**** is greater than the maximum of 100
**** is less than the minimum of 0
'****' is not one of ['a', 'b', 'c']
'****' does not match 'arn:.*'
aygold92 commented 3 years ago

What is the actual pattern/enum/minimum/maximum/etc.? That's the most important context. Knowing which pattern you failed is far more useful than just knowing you failed some pattern that's been scrubbed, and we could reveal these without revealing property values:

**** is greater than the maximum of 100
**** is less than the minimum of 0
'****' is not one of ['a', 'b', 'c']
'****' does not match 'arn:.*'

True, this could be a better error message, I would support improving it. That being said, for now they can always check the schema by calling DescribeType