aws-cloudformation / aws-cloudformation/cloudformation-resource-schema

Validate $ref correctly

Open
#23 0 comments 0 reactions 0 assignees View on GitHub
bug help wanted schema
Dominant language
Java
Stars
96
Forks
38
PR merge metrics
No merged PRs in 30d

Description

# 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:

```java
@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 `$ref`s:

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:

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

# Solutions?

The obvious solution is to resolve all `$ref`s before validation, and then it'd work. In Python, I'd use [jsonref](https://pypi.org/project/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](https://github.com/everit-org/json-schema/blob/0d797d03bafccb7d6f050ba4bb20758f65d1eafb/core/src/main/java/org/everit/json/schema/loader/SchemaLoader.java) 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 `$ref`s semantically](https://github.com/everit-org/json-schema/blob/0d797d03bafccb7d6f050ba4bb20758f65d1eafb/core/src/main/java/org/everit/json/schema/loader/SchemaLoader.java#L401), 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 `$ref`s to the local or any hosted schemas, putting in our own code could be beneficial here in the long run.

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the example through validator.validateResourceDefinition and inspect provider.definition.schema.v1.json where $ref is allowed. Read the referenced SchemaLoader behavior and determine how the project’s meta-schema should be applied so a garbage local reference fails validation. Done means the example test is rejected while valid references continue to work.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend-api-design
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.