migrate to ajv >= 7 (ajv 6 has vulnerability and reaches end of life 30/06/21)

asarver commented 3 years ago

The different open-api libraries are using v6 of AJV. There is an outstanding security vulnerability for version 6 (https://www.whitesourcesoftware.com/vulnerability-database/CVE-2020-15366) and while AJV says they'll fix security issues until June 30 2021, it's reaching end of life in 2 months.

It's not clear which JSON schema draft versions open-api supports (some places say draft 4, others don't specify a specific version), however AJV v6 supports draft 4, v7/v8 only support 6 and up.

I can put up PRs for some of the changes required, but I'll need more information on how this impacts the different libraries/documentation/etc.

main differences in new AJV version:

the . separator for property names has changed to /
array errors no longer have [0], it's just 0
errors changed should to must
dataPath changed to instancePath
for custom keywords, KeywordDefinition has changed to use DataValidationCtx that contains the parent schema data
id is no longer supported (json schema draft 4) and instead needs to be $id (json schema draft 7)
if using format in json schema, the library ajv-formats needs to be required now
missing references always throws an error now where before it was an error open api showed through its normal errors
positiveInteger has changed to nonNegativeInteger (which is great, the name is correct now)
unknownFormats is no longer supported, and formats to ignore have to be explicitly ignored

asarver commented 3 years ago

it looks like swagger-schema-official is using draft-04, but I can't find a repo anywhere to open up a PR. AJV can't be updated without moving off of draft-04

asarver commented 3 years ago

since that schema hasn't been updated in 6 years, my current plan is to copy it into the openapi-schema-validator project and remove the swagger dep

asarver commented 3 years ago

running into some issues with tests, but I'm hoping to have a PR opened tomorrow

asarver commented 3 years ago

since this is a full library upgrade, it would make sense to be a breaking change version increment, or a major at the very least. I'll also have to create a PR for each package, since they are dependent on each other, and will need the latest version to pull in ajv v8

asarver commented 3 years ago

just waiting for an open-api version bump, and I can close this issue out

asarver commented 3 years ago

@jsdevel will those PRs be a part of the v9 release, or will a version bump be needed?

jsdevel commented 3 years ago

probably v9 at this point, since it's a breaking change for some of the packages. had it all been in 1 PR it could've been 1 major release.

asarver commented 3 years ago

thanks! Yeah, I was concerned about putting it all in one PR since the contribution guidelines state one commit per PR, and the commits are done on a per package basis. It would be good to update the docs on guidance for breaking changes that affect multiple packages

jsdevel commented 3 years ago

@asarver can you pull down the latest and run npm t? i'm seeing some failed tests in express-openapi

asarver commented 3 years ago

yeah, looking

asarver commented 3 years ago

@jsdevel it looks like the issue is express-openapi uses ajv, I need to upgrade that package (PR incoming shortly)

should ajv be added as its own dependency to express-openapi? Right now it just gets included because @types/ajv is required

asarver commented 3 years ago

looks like the types are just imported but not used so it's not too big of a deal

kogosoftwarellc / open-api

migrate to ajv >= 7 (ajv 6 has vulnerability and reaches end of life 30/06/21) #727