Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: show which additionalProperties are invalid #827

Closed
rbren opened this issue Jul 31, 2018 · 15 comments
Closed

Feature Request: show which additionalProperties are invalid #827

rbren opened this issue Jul 31, 2018 · 15 comments

Comments

@rbren
Copy link

rbren commented Jul 31, 2018

I'm happy to send over a PR if you can point me in the right direction.

I regularly get the error data should NOT have additionalProperties, but it's difficult to debug unless ajv reports what that property was, e.g. data should not have additional property "foo"

What version of Ajv are you using? Does the issue happen if you use the latest version?

6.5.2

Ajv options object
{useDefaults: true}

@gajus
Copy link
Contributor

gajus commented Jan 31, 2019

Doing this would create a precedent where validated data is used in error messages, creating a vulnerability (e.g., when ajv is used to validate API data/parameters and error messages are logged).

How could this be a security concern?

Assuming that the concern is that it will reveal the schema, then it is not the case. If you have provided an additional property that is not allowed, then the error will identify the user supplied property, not the system property name.

Assuming that the concern is that property name could include malicious content (e.g. attempted XSS injection), then a simple filter (e.g. /a-zA-Z_-\./) can be used. This would exclude some valid properties – then the validation message could say something along the lines of "1 property name excluded".

I run into this error a lot, and sometimes it is really unclear what property name the error is referring to (esp. when using oneOf schemas).

@epoberezkin
Copy link
Member

The logic here is that the validation somewhat replaces the need to sanitise data. Logging unsanitised data is the security concern. Sanitising the data before logging (that’s what you suggest) would have performance impact... (and you have to check the length first as the string can be large).

Arguably the same risks exist for format and pattern... keywords, but then it’s on the schema authors and library users to consider using maxLength (that’s validated first) and not using allErrors:true in public production APIs.

@epoberezkin
Copy link
Member

@gajus It all deserves a separate section in docs about security model and concerns...

E.g. schemas are considered as trusted as application code but it seems that some users generate them based on the external data... While schemas are validated against standard meta schemas, they are not sanitised in any way (which can be done by a different meta-schema).

Also there is no security assessment of schemas (and it also can be done with some other meta-schema - e.g. requiring that maxLength is used whenever format or pattern is used, that patternProperties is not used at all, etc.)

@epoberezkin
Copy link
Member

@ejoebstl
Copy link

Hello,

I wanted to ask you to consider to make this configurable.

We use ajv to validate the responses of 3rd party APIs with quite large responses. These APIs sometimes change, and we have regression tests for that in place.

However, the absence of the field name makes updating the JSON schemas a time consuming and cumbersome task.

I would argue that it is completely safe to log more verbose error descriptions in a test setting.

@epoberezkin
Copy link
Member

You can configure error messages and include the field name into them with Ajv-errors package - I’ve just updated it to support v7

@ejoebstl
Copy link

Thanks. Could you please expand on how to include the name of the invalid field?

I don't understand how the interpolation would work for field names.

@epoberezkin
Copy link
Member

epoberezkin commented Dec 10, 2020

You can use relative JSON pointer “0#” to refer to the name of the property currently being validated, so if you put it inside additionalProperties schema you can use it in error messages defined with errorMessage keyword inside templates - see the docs in ajv-errors.

If you want to refer to the property that is failed because of additionalProperties: false there isn’t a nice way to do it (no schema to put errorMessage inside of), but the workaround would be to replace it with:

{
  additionalProperties: {
    not: true,
    errorMessage: “extra property is ${0#}
  }
}

A bit hacky but it should work :)

Feel free to submit a proposal to add support for error message parameters in errorMessage templates to ajv-errors so the above (and other similar scenarios) can be written in a nicer way using some other syntax for interpolation of error parameters in addition to the available data interpolation.

@epoberezkin
Copy link
Member

Ditto unevaluatedProperties now available in v7

@epoberezkin
Copy link
Member

But if you think that “false” is a syntactic sugar for {not:true} which is a syntactic sugar for {not:{}} maybe it’s not that hacky, just not idiomatic. The point is that “false” is also a schema, it’s just there is no way to put something inside it without removing at least half of the sugar...

@epoberezkin
Copy link
Member

Also worth adding 0# to some example in docs there if there isn’t one

@ejoebstl
Copy link

That's great, thank you.

Created a PR which adds your example to the doc.

@epoberezkin
Copy link
Member

did it work :)?

@lmk07
Copy link

lmk07 commented Jan 13, 2021

hi @epoberezkin ,

how to throw error if particular property value is "NA" or "undefined"?

sample data: {
"a":"NA",
"b":"undefined",
"c":"some value"
}
Now i want to show error as "a" and "b" have inappropriate value. Also how to compare two date property? thnks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants