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

Validate $ref correctly #23

Open tobywf opened 5 years ago

tobywf commented 5 years ago

Issue

See also https://github.com/aws-cloudformation/aws-cloudformation-rpdk/issues/66

TL;DR: If we validate the schema as data, and our meta schema allows $ref, it doesn't work, as the "data" i.e. the schema is not further processed:

@Test
public void improperValidation() {
    final JSONObject definition = new JSONObject()
            .put(TYPE_NAME_KEY, EXAMPLE_TYPE_NAME)
            .put(DESCRIPTION_KEY, EXAMPLE_DESCRIPTION)
            .put(PROPERTIES_KEY, new JSONObject()
                .put("propertyA", new JSONObject()
                    .put("$ref", "#/garbage")))
            .put(PRIMARY_IDENTIFIER_KEY, Arrays.asList(EXAMPLE_PRIMARY_IDENTIFIER));

    validator.validateResourceDefinition(definition);
}

this passed, but the $ref is obviously garbage. The reason it "passes" is because we allow $refs:

https://github.com/aws-cloudformation/aws-cloudformation-resource-schema/blob/6a1ef45d612d7c90da1d77e71a9427a64cdd5237/src/main/resources/schema/provider.definition.schema.v1.json#L85-L87

however, all JSON schema mandates is:

"$ref": {
    "type": "string",
    "format": "uri-reference"
},

Solutions?

The obvious solution is to resolve all $refs before validation, and then it'd work. In Python, I'd use jsonref, or disallow circular references and simply inline all references, and disallow $ref.

It may be possible to use org.everit.json.schema.loader.SchemaLoader in some capacity. Instead of validating against draft-7 (the usual meta-schema), it may be possible to use our meta-schema instead. Since SchemaLoader handles $refs semantically, that ought to work. (I had planned to do something like this in the RPDK, where the library/dynamic nature makes it fairly straightforward to do.)

If in future we want to restrict $refs to the local or any hosted schemas, putting in our own code could be beneficial here in the long run.