OSCAL 1.0.5 JSON schema uses allOf where anyOf is more appropriate

[laurelmay]

Describe the bug

There are many fields with well-known values but that also "may be locally defined". In OSCAL v1.0.5's JSON schema, these are represented as allOf between an enum and the string-with-format data type. allOf requires that the data match all given schemas, effectively limiting values to only the enum. anyOf may be more appropriate.

For example https://pages.nist.gov/OSCAL/reference/1.0.5/catalog/json-reference/#/catalog/metadata/parties/external-ids/scheme is shown in JSON schema as

  { "title" : "External Identifier Schema",
                  "description" : "Indicates the type of external identifier.",
                  "allOf" : 
                  [ 
                    { "$ref" : "#/definitions/URIDatatype" },

                    { "enum" : 
                      [ "http://orcid.org/" ] } ] },

I found the commit view useful to see the difference.

See: https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-validation-01#section-6.7

Who is the bug affecting

Consumers of the OSCAL JSON schema

What is affected by this bug

Tooling & API

How do we replicate this issue

Evaluate a value such as "https://example.com/" against the given field.

Expected behavior (i.e. solution)

The JSON schema should not reject valid data when the value "may be locally defined".

Other comments

For Draft 07, allOf (and friends) are defined the validation specification but have moved to Core in the latest spec.

For allOf:

An instance validates successfully against this keyword if it validates successfully against all schemas defined by this keyword's value.

For `anyOf:

An instance validates successfully against this keyword if it validates successfully against at least one schema defined by this keyword's value.

• Draft 07
• Draft 2020-12

Revisions

No response

[laurelmay]

Better reproduction:

git clone --recursive https://github.com/usnistgov/oscal-content
git clone --recurisve https://github.com/usnistgov/OSCAL

npm install -g ajv-cli ajv-formats

ajv validate --spec=draft7 -c ajv-formats -s OSCAL/json/schema/oscal_catalog_schema.json -d oscal-content/nist.gov/SP800-53/rev5/json/NIST_SP-800-53_rev5_catalog.json
ajv validate --spec=draft7 -c ajv-formats -s OSCAL/json/schema/oscal_catalog_schema.json -d oscal-content/nist.gov/SP800-53/rev4/json/NIST_SP-800-53_rev4_catalog.json

ajv validate --spec=draft7 -c ajv-formats -s OSCAL/json/schema/oscal_profile_schema.json -d oscal-content/nist.gov/SP800-53/rev4/json/NIST_SP-800-53_rev4_MODERATE-baseline_profile.json

All of those ajv invocations will fail because the fields don't match a value in an enum because of the use of allOf.

Very aggressively changing all occurrences of allOf (which probably isn't correct in every case but should be good-enough for the specific validation failure):

sed -i'' -e 's/allOf/anyOf/g' OSCAL/json/schema/oscal_catalog_schema.json

you can then perform the catalog validations and see them succeed.

[GaryGapinski]

I attempted to find out why there are two allOf possibilities, considering the first (a URI) would suffice. My JSON schema expertise is minimal. Your observation regarding allOf appears correct.

So I attempted to find the XML equivalent and found it is not equivalent. It is just a URI (i.e., the only possibility).

The reproduction faults with allOf seem a bit odd since the instance documents each have one party, each of which has no external identifier defined. The reported locus of the fault seems to be displaced from parties (seems to be metadata prop):

gapinski@flexion-mac-C02FCBVSMD6N usnistgov % ajv validate --spec=draft7 -c ajv-formats -s OSCAL/json/schema/oscal_catalog_schema.json -d oscal-content/nist.gov/SP800-53/rev5/json/NIST_SP-800-53_rev5_catalog.json
oscal-content/nist.gov/SP800-53/rev5/json/NIST_SP-800-53_rev5_catalog.json invalid
[
  {
    instancePath: '/catalog/metadata/props/0/name',
    schemaPath: '#/properties/name/allOf/1/enum',
    keyword: 'enum',
    params: { allowedValues: [Array] },
    message: 'must be equal to one of the allowed values'
  }
]
gapinski@flexion-mac-C02FCBVSMD6N usnistgov % 

which has an analogous allOf with two subordinate items.

[laurelmay]

I think the cause of this is one of the following:

• Fix handling of allowed-values enumeration in JSON Schemas metaschema#253
  • https://github.com/usnistgov/metaschema/blob/70085548fec75de999eebb480a0be7d7b1abdada/toolchains/xslt-M4/schema-gen/make-json-schema-metamap.xsl#L339-L348
• Metaschema / XSLT implementation alignment metaschema#197
  • https://github.com/usnistgov/metaschema/blob/70085548fec75de999eebb480a0be7d7b1abdada/toolchains/xslt-M4/schema-gen/make-json-schema-metamap.xsl#L196-L210

The latter feels more likely but I am not sure whether it can be fixed by naively changing to anyOf or if it requires more logic like the former.

[aj-stein-nist]

I think the cause of this is one of the following:
...
The latter feels more likely but I am not sure whether it can be fixed by naively changing to anyOf or if it requires more logic like the former.

Sorry we hadn't responded. Yesterday I looked into this but I am still uncertain as to why it is happening in the context of OSCAL. For the referenced usnistgov/metaschema#253 PR, we specifically tested the implementation change to drop allow-other="yes" enumerations since it is obvious that you do not want to force the issue there. I was the one who wrote the tests for this, so ...

I guess it is back to the drawing board since it did not work as intended when running the updated codebase against OSCAL. I am glad you figured all this out and reported it here and the relevant issue. Kudos to you, @kylelaker.

[aj-stein-nist]

Metaschema devs will need to look into the issue reported and determine where the bug is exactly and circle back on this, potentially in the next sprint or the one after.

[aj-stein-nist]

@JustKuzya, I know we discussed this earlier today during standup, but here is the PR feedback based on when you and I met with @nikitawootten-nist and paired.

usnistgov/metaschema#360 (review)

I am going to wait to hear back from @kylelaker to determine how we want to work together on the necessary unit tests to increase the coverage we were missing for this bit, and then the work is more or less done on this.

[aj-stein-nist]

@JustKuzya and @nikitawootten-nist, let me know if you want to talk about perf testing this afternoon let me know. Well at least everyone got the right idea, I was alluding to the Metaschema change.

[JustKuzya]

I have a local setup for testing. If you want to meet to establish additional criteria or add some timing script into pages, we can meet. DC From: A.J. Stein ***@***.***> Sent: Monday, June 12, 2023 12:47 PM To: usnistgov/OSCAL ***@***.***> Cc: Cousin, Dmitry A. (Fed) ***@***.***>; Mention ***@***.***> Subject: Re: [usnistgov/OSCAL] OSCAL 1.0.5 JSON schema uses `allOf` where `anyOf` is more appropriate (Issue #1773) @JustKuzya<https://github.com/JustKuzya> and @nikitawootten-nist<https://github.com/nikitawootten-nist>, let me know if you want to talk about perf testing this afternoon let me know. - Reply to this email directly, view it on GitHub<#1773 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACS6IVVORLCHNLWXP4IBWXLXK5BYDANCNFSM6AAAAAAXJ7DMJA>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>

[aj-stein-nist]

Marking blocked until Metaschema PR reviewed and merged.

[aj-stein-nist]

#380 has been merged. Separate of the PR there, I also tested the pushed up models and confirm many of the enumerations across 4/7 models had flag and field enumerations correctly updated by diffing them with Oxygen and ignoring whitespace. The only changes were correct updates from allOf to anyOf. I will close this out once I pull up the Metaschema module and that is merged into a pending release branch.

[aj-stein-nist]

Closing with 1.0.6. pre-release.