Skip to content

fix(validation): reject a schema file that is valid JSON but not a schema - #542

Merged
jbonofre merged 1 commit into
apache:mainfrom
ayushtkn:fix/validation-reject-invalid-schema
Oct 9, 2026
Merged

jbonofre merged 1 commit into
apache:mainfrom
ayushtkn:fix/validation-reject-invalid-schema

Conversation

@ayushtkn

@ayushtkn ayushtkn commented Oct 8, 2026

Copy link
Copy Markdown
Member

Summary

Follow up from: #534 (review)

--schema pointing at a file whose top-level JSON value is neither an object nor a boolean gets past json.loads and then raises from inside
referencing, naming neither the schema nor the file it came from.

Currently, with --schema at a file holding []:

File ".../referencing/jsonschema.py", line 50, in _dollar_id
return contents.get("$id")
AttributeError: 'list' object has no attribute 'get'

main() now checks the parsed schema against the Draft 2020-12 metaschema and reports
the problem with the path to it, in the same " -> " form validate_schema already
uses for model diagnostics:

[]                                        Error: Invalid schema S: (root): [] is not of type 'object', 'boolean'
null                                      Error: Invalid schema S: (root): None is not of type 'object', 'boolean'
{"type":"notatype"}                       Error: Invalid schema S: type: 'notatype' is not valid under any of the given schemas
{"type":"object","required":"name"}       Error: Invalid schema S: required: 'name' is not of type 'array'
{"properties":{"name":{"minLength":"3"}}} Error: Invalid schema S: properties -> name -> minLength: '3' is not of type 'integer'

Two notes on scope:

  • check_schema also catches a schema that is an object but whose keywords are
    malformed. That is the same single call, and the extra case is a real misreport: on
    main, {"required": "name"} iterates the string and demands four properties
    'n', 'a', 'm', 'e' of the model, blaming the model for a mistake in the
    schema. A bare isinstance(schema, (dict, bool)) check would address only the
    literal traceback and leave that in place.
  • true and false are legal JSON Schemas and keep working — true accepts
    everything, false fails through the normal [Schema] path. A test pins both, so
    the check is not narrowed into rejecting non-objects.

Related Issues

Checklist

Specification

  • Spec changes are included in core-spec/ and follow the existing structure
  • Spec changes have been discussed on the mailing list or in a linked issue
  • Breaking changes to the spec are clearly called out in the summary

Ontology

  • Ontology changes in ontology/ are consistent with spec changes
  • New or modified terms are defined and documented

Converters

  • Converter logic in converters/ is updated to reflect spec or ontology changes
  • New converters include tests under the converter's test directory
  • If adding a new converter, .github/labeler.yml is updated with the new path

Validation

  • Validation rules in validation/ are updated if the spec changed
  • New validation cases are covered by tests

Documentation

  • docs/ is updated to reflect any user-facing changes
  • New features or behaviors are documented with examples where appropriate
  • CONTRIBUTING.md is updated if the contribution process changed

Examples

  • examples/ are added or updated for any new spec constructs or converter support

Tests

  • All existing tests pass (pytest / CI green)
  • New functionality is covered by tests

Compliance

  • ASF license headers are present on all new source files
  • No third-party dependencies are added without PMC/IPMC approval
  • If third-party source code is vendored/copied (not just declared as a dependency), NOTICE and/or LICENSE have been updated per ASF policy

…hema

--schema pointing at a JSON file whose top-level value is neither an object nor a
boolean got past json.loads and then raised AttributeError from inside
referencing -- "'list' object has no attribute 'get'" -- a traceback naming
neither the schema nor the file it came from.

main() now checks the parsed schema against the Draft 2020-12 metaschema and
reports the first problem with the path to it, in the same form as a model
diagnostic. That also covers an object whose keywords have the wrong shape: a
string "required" used to make the validator demand one property per character,
reporting four missing-property errors against the model for a mistake in the
schema.

true and false stay valid schemas and behave as before.
Copilot AI balanced review requested due to automatic review settings October 8, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation correctly addresses the failure modes and provides focused regression coverage.

0 open findings

What changed in this PR

Adds early JSON Schema validation so malformed schemas produce actionable CLI errors instead of tracebacks or misleading model diagnostics.

Changes:

  • Validates schemas against the Draft 2020-12 metaschema.
  • Reports invalid schema locations and preserves boolean-schema support.
  • Adds integration coverage for invalid roots, malformed keywords, and boolean schemas.
File Description
validation/​validate.py Adds schema validation and clear error reporting.
validation/​test_validate.py Tests invalid and boolean schema behavior.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jbonofre
jbonofre self-requested a review October 9, 2026 03:33

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

Great follow-up to #534. Using check_schema() neatly handles both non-schema JSON roots and malformed keywords with actionable CLI diagnostics, while keeping boolean schema intact.

Thanks @ayushtkn!

@jbonofre
jbonofre merged commit 9206a13 into apache:main Oct 9, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants