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

Updated metaschema to improve error reporting #1027

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

alanisaac
Copy link
Contributor

@alanisaac alanisaac commented Apr 9, 2024

Related Issue:

Fixes #1021

Description of changes:

This PR updates the event and object metaschemas to address the issue described in #1021, where reporting from the metaschema validation does not point to the location of the error. The strategy used is shown here: https://json-schema.org/understanding-json-schema/reference/object#extending

Because additionalProperties only recognizes properties declared in the same subschema, it considers anything other than "street_address", "city", and "state" to be additional. Combining the schemas with allOf doesn't change that. A workaround you can use is to move additionalProperties to the extending schema and redeclare the properties from the extended schema.

@rmouritzen-splunk
Copy link
Contributor

rmouritzen-splunk commented Apr 9, 2024

This is the basically the prior approach, it seems? I believe this will incorrectly allow extra top-level attributes though the JSON Schema check.

@alanisaac
Copy link
Contributor Author

alanisaac commented Apr 9, 2024

It is not the prior approach, though it does use the same technique. I believe the prior approach allowed extra top-level attributes in objects because nothing in the definition of object.schema.json disallowed that to happen (there was no additionalAttributes: false or similar constraint). That constraint was omitted because event "inherited" so to speak from object, which would not have been possible with that constraint in place.

The difference here is that both event and object now / still inherit from the common event object schema, which defines the attributes, and are both able to set additionalAttributes: false.

Below are test cases I wrote up locally (I could contribute these to the validator unit tests at some point).

If there are other tests you'd like me to work through or if I'm missing something about the problem you ran into before let me know!

An event should not pass with additional top-level properties

{
    "description": "Bad apple: an invalid event.",
    "extends": "base_event",
    "caption": "Bad Apple",
    "name": "bad_apple",
    "category": "network",
    "uid": 999,
    "profiles": [
      "host"
    ],
    "attributes": {
      "file": {
        "description": "The email file attachment.",
        "group": "primary",
        "requirement": "required"
      }
    },
    "foo": "bar"
}

results in:

TESTING: JSON files match their metaschema definitions
   FATAL: File at events/network/bad_apple.json does not pass metaschema validation. Error: Additional properties are not allowed ('foo' was unexpected) at JSON path: '$'

An object should not pass with additional top-level properties

{
    "description": "Bad apple: an invalid object.",
    "extends": "object",
    "caption": "Bad Apple",
    "name": "bad_apple",
    "profiles": [
      "host"
    ],
    "attributes": {
      "file": {
        "description": "The email file attachment.",
        "group": "primary",
        "requirement": "required"
      }
    },
    "foo": "bar"
}

results in:

TESTING: JSON files match their metaschema definitions
   FATAL: File at objects/bad_apple.json does not pass metaschema validation. Error: Additional properties are not allowed ('foo' was unexpected) at JSON path: '$'

An attribute that combines required and optional should show an error message pointing to the specific attribute

{
    "description": "Bad apple: an invalid object.",
    "extends": "object",
    "caption": "Bad Apple",
    "name": "bad_apple",
    "profiles": [
      "host"
    ],
    "attributes": {
      "file": {
        "description": "The email file attachment.",
        "group": "primary",
        "requirement": "optional"
      }
    }
}

results in:

TESTING: JSON files match their metaschema definitions
   FATAL: File at objects/bad_apple.json does not pass metaschema validation. Error: 'optional' is not one of ['recommended', 'required'] at JSON path: '$.attributes.file.requirement'

@rmouritzen-splunk
Copy link
Contributor

I tried this locally, and it works great. I say we go with it.

Copy link
Contributor

@mikeradka mikeradka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Alan!

@alanisaac alanisaac merged commit 73d1180 into ocsf:main Apr 11, 2024
2 checks passed
@alanisaac alanisaac deleted the update-metaschema-error-reporting branch April 11, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metaschema: error reporting is difficult to understand
4 participants