defenseunicorns / go-oscal

Repository for the generation of OSCAL data types
Apache License 2.0
15 stars 1 forks source link

Missing properties for non-required top-level keys #269

Closed brandtkeller closed 2 months ago

brandtkeller commented 3 months ago

Steps to reproduce

  1. Add an invalid field to testdata/validation/valid-component-definition.yaml
  2. Execute go-oscal validate -f testdata/validation/valid-component-definition.yaml -r output.yaml
  3. Review the errors

Expected result

Single error for the invalid field

Actual Result

valid: false
timeStamp: 2024-06-18T15:59:49.449638313Z
errors:
    - keywordLocation: /oneOf/0/required
      absoluteKeywordLocation: http://csrc.nist.gov/ns/oscal/1.0/1.0.4/oscal-complete-schema.json#/oneOf/0/required
      instanceLocation: ""
      error: 'missing properties: ''catalog'', missing properties: ''profile'''
    - keywordLocation: /oneOf/2/properties/component-definition/$ref/properties/components/items/$ref/additionalProperties
      absoluteKeywordLocation: http://csrc.nist.gov/ns/oscal/1.0/1.0.4/oscal-complete-schema.json#/definitions/oscal-complete-oscal-component-definition:defined-component/additionalProperties
      instanceLocation: /component-definition/components/0
      error: additionalProperties 'test' not allowed
    - keywordLocation: /oneOf/3/required
      absoluteKeywordLocation: http://csrc.nist.gov/ns/oscal/1.0/1.0.4/oscal-complete-schema.json#/oneOf/3/required
      instanceLocation: ""
      error: 'missing properties: ''system-security-plan'', missing properties: ''assessment-plan'', missing properties: ''assessment-results'', missing properties: ''plan-of-action-and-milestones'''
metadata:
    documentPath: testdata/validation/valid-component-definition.yaml
    documentType: component-definition
    documentVersion: 1.0.4
    schemaVersion: 1.0.4

Note the two entries for missing-properties being the non-existent models.

Severity/Priority

medium

Additional Context

May be a byproduct of our use of the complete schema - when no invalid fields are present, the linting passes without any errors.

mike-winberry commented 3 months ago

This looks like you have two separate errors, one top level, violating "oneof" behavior at top level and one with the additionalProperties, am I missing something?

nvm, hmm, yeah this is a weird one...

mike-winberry commented 3 months ago

I would have to dig deeper, but my understanding of what is happening right now is that because a component in components is malformed this means -> component-definition is malformed and since there are no other top level fields possible due to "oneOf" jsonschema is telling you to fix your component definition or try a new field, but it would be nice to reduce noise if there is a consistent pattern that wont cause lack of info in other edge cases, right now it still gives what is needed to fix the oscal, though it could be more clear, also errors are completely different in v6 so we may need to make a decision on how much to sink into v5 before just upgrading and seeing what the new jsonschema version offers in way of error clarity as it has had a major overhaul.

mike-winberry commented 2 months ago

from #274: Screenshot 2024-07-17 at 10 17 48 PM

{
  "valid": false,
  "timeStamp": "2024-07-17T22:16:32.221863-07:00",
  "errors": [
    {
      "keywordLocation": "/oneOf/2/properties/component-definition/$ref/properties/components/items/$ref/additionalProperties",
      "absoluteKeywordLocation": "https://github.com/defenseunicorns/go-oscal/tree/main/src/internal/schemas/oscal_complete_schema-1-0-4.json#/definitions/oscal-complete-oscal-component-definition:defined-component/additionalProperties",
      "instanceLocation": "/component-definition/components/2",
      "error": "\"additional properties 'test' not allowed\""
    }
  ],
  "metadata": {
    "documentPath": "testdata/validation/valid-component-definition.yaml",
    "documentType": "component-definition",
    "documentVersion": "1.0.4",
    "schemaVersion": "1.0.4"
  }
}