[openapi-response-validator] valid body fails incorrectly against allOf schemas with additionalProperties: false

[rwalle61]

Hi,

Thanks for this great package!

@rolfisub and I found a bug where if you validate a response body against allOf multiple schemas where one of them has additionalProperties: false, then you get a failure (when you should get no failures).

I.e., this test should pass:

module.exports = {
  constructorArgs: {
    responses: {
      '2XX': {
        schema: {
          allOf: [
            {
              type: 'object',
              properties: {
                expectedProperty1: {
                  type: 'string'
                }
              }
            },
            {
              type: 'object',
              properties: {
                expectedProperty2: {
                  type: 'string'
                }
              },
              additionalProperties: false,
            },
          ]
        }
      }
    },

    definitions: null
  },

  inputStatusCode: 200,
  inputResponseBody: {
    expectedProperty1: 'foo',
    expectedProperty2: 'bar',
  },

  expectedValidationError: void 0
};

But instead it fails with this error:

{
  message: 'The response was not valid.',
  errors: [
    {
      path: 'response',
      errorCode: 'additionalProperties.openapi.responseValidation',
      message: 'should NOT have additional properties'
    }
  ]
}

This bug seems to occur only in this specific case. For example, schemas with additionalProperties: true seem to work correctly, as do schemas with allOf just one schema, with additionalProperties: false, e.g.:

allOf: [
            {
              type: 'object',
              properties: {
                expectedProperty1: {
                  type: 'string'
                }
              }
              additionalProperties: false,
            },
            // just one schema, with `additionalProperties: false`
          ]

// this passes and fails when it should

Looking at the code of openapi-response-validator, it seems that you outsource your validation to Ajv, which I guess is this one https://github.com/epoberezkin/ajv/issues?utf8=%E2%9C%93&q=additionalProperties. I see recent mention of unexpected errors for anyOf schemas with additionalProperties: false, but they were resolved 2 months ago, and I'm a bit out of my depth spotting the fix.

Before I raise an issue there, I wondered if you have any ideas/thoughts on this?

[jsdevel]

i don't. would appreciate if you could jump in further to the code and see if you can fix it.

[stevelove]

👋 Thanks for the tools you've built. Has there been any forward momentum on this? I just ran into it using chai-openapi-response-validator and thought I'd give this issue a bump.

[rwalle61]

@mvegter and I have just reviewed this (for chai-openapi-response-validator), and now think this is actually expected behaviour. See openapi-library/OpenAPIValidators#46 (comment) for explanation

If everyone agrees, we can probably close this

[stevelove]

Yes, I also came to the conclusion that this is expected behavior.

[Envek]

Hey, does anyone know how to restrict additional properties in inherited types (which is usually done with allOf and additionalProperties: false) at the schema level and make response pass through openapi-response-validator?
I'm well stuck how to workaround this limitation properly.

[mvegter]

Hey, does anyone know how to restrict additional properties in inherited types (which is usually done with allOf and additionalProperties: false) at the schema level and make response pass through openapi-response-validator?
I'm well stuck how to workaround this limitation properly.

Hi @Envek , we ran into the same issue. We solved this by having a source version of our specification from which we generated the actual fully resolved version. You can see it here: https://github.com/AliceO2Group/Bookkeeping/tree/master/spec , it includes the required source code (in JavaScript). Do note that the convert.js file does more than just resolving, it will also sort members.

[Envek]

Finally I solved it by using json-schema-merge-allof library to preprocess existing schema.

It works like a charm with code like this:

import yaml from "js-yaml"
import { readFileSync } from "fs"
import mergeAllOf from "json-schema-merge-allof"
import $RefParser from "@apidevtools/json-schema-ref-parser"

const apiSpec = <Record<string, any>>yaml.safeLoad(readFileSync("./apiSchema.yaml").toString())

async function preprocessOpenapiSpec(apiSpec: Record<string, any>): Promise<Record<string, any>> {
  const schema = await $RefParser.dereference(apiSpec)

  function mergeAllOfRecursively(candidate: Record<string, any>, parent: Record<string, any>, k: any) {
    if (typeof candidate !== "object") return

    if (candidate.allOf) {
      parent[k] = mergeAllOf(candidate, { ignoreAdditionalProperties: true, deep: true })
    } else {
      for (const [key, value] of Object.entries(candidate)) {
        mergeAllOfRecursively(value, candidate, key)
      }
    }
  }

  for (const [key, value] of Object.entries(schema)) {
    mergeAllOfRecursively(value, apiSpec, key)
  }

  return schema
}

const preprocessedApiSpec = await preprocessOpenapiSpec(apiSpec)

See mokkabonna/json-schema-merge-allof#26 for more details about why it is so verbose 😄