Added schema validation #10
Conversation
Summary of ChangesHello @dwnoble, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes a robust foundation for schema validation within the Data Commons schema tools. By introducing a new Knowledge Graph implementation and a dedicated Schema Validation Service, it enables strict, RDFS-based validation of both schema definitions and data instances. This ensures data integrity and consistency, paving the way for more reliable schema manipulation and conversion features. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a foundational Knowledge Graph and schema validation system, including core documentation (SCHEMA.md, README.md), Pydantic models for RDF/SHACL, and a comprehensive SchemaValidationService. A critical security vulnerability has been identified: the use of rdflib's JSON-LD parser on untrusted input without restricting remote context resolution poses a significant SSRF risk. Two instances where external data is parsed into RDF graphs require immediate attention by disabling remote context loading or using a restricted document loader. Beyond this, the review also suggests improvements in the correctness and clarity of the validation logic and addresses minor inconsistencies in the documentation.
|
|
||
| g = Graph() | ||
| data = json.dumps(jsonld_input) if isinstance(jsonld_input, dict) else jsonld_input | ||
| g.parse(data=data, format="json-ld") |
There was a problem hiding this comment.
Similar to the issue in knowledge_graph.py, the _load_graph method here is vulnerable to SSRF via JSON-LD remote context resolution. Since this service is used to validate both schema definitions and data packets, an attacker could exploit this by providing a malicious schema or data payload containing a remote @context URL.
It is recommended to use a secure document loader for the JSON-LD parser that disables or restricts external network requests.
packages/datacommons-schema/datacommons_schema/services/schema_validation_service.py
Show resolved
Hide resolved
| - *Note*: Validation often requires checking if a referenced Class exists. If we are adding a new Class *and* an instance of it simultaneously, the validator must verify them together. | ||
| 3. If valid, merge temporary graph into main `_graph`. | ||
|
|
||
| ### 3. `SchemaValidationService` (The Validator) |
There was a problem hiding this comment.
The section numbering in the "Component Design" part appears to have skipped number 2. It jumps from "1. KnowledgeGraph" to "3. SchemaValidationService". For clarity, it would be good to ensure the numbering is sequential.
| datacommons-schema mcf2jsonld input.mcf | ||
|
|
||
| # With custom namespace | ||
| datacommons mcf2jsonld input.mcf --namespace "schema:https://schema.org/" |
There was a problem hiding this comment.
There's an inconsistency in the command-line examples. Line 170 uses datacommons-schema mcf2jsonld, while this line uses datacommons mcf2jsonld. To avoid confusion, it would be best to make the command consistent across all examples. Based on the pyproject.toml script definition, datacommons-schema is likely the correct command.
| datacommons mcf2jsonld input.mcf --namespace "schema:https://schema.org/" | |
| datacommons-schema mcf2jsonld input.mcf --namespace "schema:https://schema.org/" |
| report = self.validate(nodes) | ||
| if not report.is_valid: | ||
| error_msgs = "\n".join([f"{e.subject}: {e.message}" for e in report.errors]) | ||
| raise ValueError(f"Cannot add invalid nodes:\n{error_msgs}") | ||
|
|
||
| # If valid, merge | ||
| temp_graph = self._load_graph(nodes) | ||
| self._graph += temp_graph |
There was a problem hiding this comment.
The add method currently parses the input nodes into a temporary graph twice: once within the self.validate(nodes) call and again with self._load_graph(nodes). This is inefficient, especially for large inputs.
A more efficient approach would be to parse the input once at the beginning of the add method and then pass the resulting Graph object through the validation and merging steps.
| def expand_uri(uri: str) -> str: | ||
| """Expand a URI using the context.""" | ||
| if not isinstance(uri, str): | ||
| return uri | ||
| for prefix, namespace in context.items(): | ||
| if uri.startswith(f"{prefix}:"): | ||
| return uri.replace(f"{prefix}:", namespace, 1) | ||
| return uri |
There was a problem hiding this comment.
The expand_uri function manually implements CURIE expansion. JSON-LD context processing can be very complex (e.g., term definitions with @id, @type, or scoped contexts), and a manual implementation is likely to be incomplete and may not handle all valid JSON-LD cases correctly.
Consider leveraging rdflib's more robust parsing capabilities, which handle context processing automatically. If this parser is intended to be a lightweight utility, it would be helpful to add a docstring noting that it only supports simple prefix-based CURIEs.
packages/datacommons-schema/datacommons_schema/services/schema_validation_service.py
Show resolved
Hide resolved
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
This pull request introduces an implementation of a Knowledge Graph (KG) and schema validation system for the Data Commons schema tools. It adds documentation, models for RDF, RDFS, and XSD primitives, a new
KnowledgeGraphclass for in-memory graph management and validation, and a JSON-LD parser.Core system design and documentation:
README.mdwith an overview of schema validation concepts, component design, usage examples, and updated CLI instructions for consistency.Knowledge Graph implementation:
KnowledgeGraphclass inknowledge_graph.py, providing methods to validate and add nodes using an in-memoryrdflib.Graph, with integrated schema validation via theSchemaValidationService.