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

Fixed semver, fixed reporting #892

Merged
merged 2 commits into from
Dec 18, 2023
Merged

Fixed semver, fixed reporting #892

merged 2 commits into from
Dec 18, 2023

Conversation

alanisaac
Copy link
Contributor

@alanisaac alanisaac commented Dec 13, 2023

Related Issue:

Description of changes:

This PR does three things:

  • Cleans up one leftover usage of "v" as a prefix in semver versions in the dictionary.json schema
  • Removes the v? prefix from the semver regex to disallow it in future schemas
  • Changes the way the event schema is defined for events.

The last bullet is a bit more complex:

  • There are two ways to achieve schema extensions in JSON schemas that have constraints on them: additionalProperties: false along with redeclaring the inherited properties OR unevaluatedProperties: false.
  • See https://json-schema.org/understanding-json-schema/reference/object#extending for more details on these patterns
  • Originally I went with the unevaluatedProperties pattern, as it avoids redeclaring property names as in this PR, but I found that validation messages were wonky in the schema validation library I used.
  • When the unevaluatedProperties does not pass on any nested attribute of the inherited schema, it would flag the entire schema (rather than the attribute) as being wrong. e.g. in the semver issue I am fixing in this PR...
    • An error message with unevaluatedProperties would say dictionary.json has unexpected attribute attributes because a sub-sub-sub-sub.... schema of attributes did not pass
    • An error message with additionalProperties like in this PR will explicitly point to the field that is wrong:
    File at events/findings/security_finding.json does not pass metaschema validation. Error: 'v1.1.0' does not match '^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$' at JSON path: '[email protected]'
    

TL;DR: while the new pattern in this PR is more verbose and repeats information, it produces much better error messages when schemas fail on validation, which I think is more important.

mikeradka
mikeradka previously approved these changes Dec 13, 2023
metaschema/event.schema.json Outdated Show resolved Hide resolved
@alanisaac alanisaac added enhancement New feature or request non_breaking Non Breaking, backwards compatible changes labels Dec 14, 2023
Copy link
Contributor

@floydtree floydtree left a comment

Choose a reason for hiding this comment

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

awesome! thank you.

@floydtree floydtree merged commit 27ce35a into ocsf:main Dec 18, 2023
2 checks passed
@floydtree floydtree added the v1.1.0 Changes marked for v1.1.0 of OCSF label Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request non_breaking Non Breaking, backwards compatible changes v1.1.0 Changes marked for v1.1.0 of OCSF
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants