ajv-validator / ajv

The fastest JSON schema Validator. Supports JSON Schema draft-04/06/07/2019-09/2020-12 and JSON Type Definition (RFC8927)
https://ajv.js.org
MIT License
13.87k stars 877 forks source link

Adding a default additionalProperties: false #2453

Closed dblock closed 5 months ago

dblock commented 5 months ago

What version of Ajv you are you using?

8.13.0

What problem do you want to solve?

Coming from https://github.com/opensearch-project/opensearch-api-specification/issues/338.

We are authoring an OpenAPI specification for OpenSearch in https://github.com/opensearch-project/opensearch-api-specification. For example, the GET / API returns server info. To verify that the specification is correct we author test stories. For example this story makes a GET / request to a docker instance of OpenSearch, and we use AJV to verify that the response matches the schema. We basically verify the schema from actual server response data, instead of the other way around.

A large category of bugs in the schema is that some fields are missing. For example, the distribution filed in the response from GET / was added in https://github.com/opensearch-project/opensearch-api-specification/pull/336.

We'd like to have detected the missing field. Had the info schema had additionalProperties: false the schema validation would have failed with "data/version must NOT have additional properties". However adding that option means that our published schema cannot be future-proof for when new fields are added.

So far I am thinking of dynamically injecting additionalProperties: false but it feels messy.

What do you think is the correct solution to problem?

A way to generically implement the behavior of producing a validation error with any field that is returned in the data but is not referenced in the schema.

validate({ additionalProperties: false })

Some other approach?

Will you be able to implement it?

No promises, but sincerely appreciate everybody's work on this project! Million thanks.

jasoniangreen commented 5 months ago

To my knowledge the behaviour of things like that is decided by the JSON Schema spec and not something we would be open to changing, however I know it's possible to do some pretty amazing things with custom keywords. I can't be sure without delving much deeper into your use case, but maybe they can help you build some mechanism to achieve your goals.

See the docs on keywords - https://ajv.js.org/keywords.html

dblock commented 5 months ago

Thanks @jasoniangreen, I will look into this.

Possibly simpler, is there a way with AJV to extract the fields that are not referenced in the schema?

jasoniangreen commented 5 months ago

Thanks @jasoniangreen, I will look into this.

Possibly simpler, is there a way with AJV to extract the fields that are not referenced in the schema?

Not that I could see, but from what I remember about addKeyword, you will get access to all data and the schema so it should be possible to work it out and store the extra fields somewhere.

jasoniangreen commented 5 months ago

One more thought, I don't see what's messy about dynamically injecting additionalProperties: false. Sounds perfectly legit to have a wrapper functioning that can recursively add that prop and then you can maintain one schema for true validation and another for highlighting extra fields. It wouldn't be a lot of code at all.

Going to close as I think there is enough for OP to work with and I know for a fact that we won't want to change fundamental default behaviour or add new options for this.

dblock commented 5 months ago

@jasoniangreen Thank you so much!

dblock commented 5 months ago

Here's an implementation that adds ajv-errors and dynamically injects the following by walking the spec.

"additionalProperties": {
  "not": true,
  "errorMessage": "property is not defined in the spec"
}

https://github.com/opensearch-project/opensearch-api-specification/pull/342

Feel free to leave comments!