T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/ontology.py:111
- Finding
- Graph Mutations Bypass Pre-Commit Schema Validation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/ontology.py:111-128`, `scripts/ontology.py:144-159`, `scripts/ontology.py:178-191`, and `scripts/ontology.py:511-542` **Vulnerability Type**: Missing pre-commit validation permitting invalid or sensitive data to be persisted **Risk Level**: Medium ### Vulnerable Code ```python def create_entity(type_name: str, properties: dict, graph_path: str, entity_id: str = None) -> dict: """Create a new entity.""" entity_id = entity_id or generate_id(type_name) timestamp = datetime.now(timezone.utc).isoformat() entity = { "id": entity_id, "type": type_name, "properties": properties, "created": timestamp, "updated": timestamp } record = {"op": "create", "entity": entity, "timestamp": timestamp} append_op(graph_path, record) return entity ``` ```python def update_entity(entity_id: str, properties: dict, graph_path: str) -> dict | None: """Update entity properties.""" entities, _ = load_graph(graph_path) if entity_id not in entities: return None timestamp = datetime.now(timezone.utc).isoformat() record = {"op": "update", "id": entity_id, "properties": properties, "timestamp": timestamp} append_op(graph_path, record) entities[entity_id]["properties"].update(properties) entities[entity_id]["updated"] = timestamp return entities[entity_id] ``` ```python def create_relation(from_id: str, rel_type: str, to_id: str, properties: dict, graph_path: str): """Create a relation between entities.""" timestamp = datetime.now(timezone.utc).isoformat() record = { "op": "relate", "from": from_id, "rel": rel_type, "to": to_id, "properties": properties, "timestamp": timestamp } append_op(graph_path, record) return record ``` The command dispatcher invokes these mutation functions directly: ```python if args.command == ...[truncated 4508 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Validate every proposed mutation before writing** - Load the applicable schema inside each mutation workflow. - Apply the proposed operation to an in-memory copy of the current graph. - Run all relevant entity and relation constraints against the proposed state. - Append the operation only if validation succeeds. 2. **Reject forbidden sensitive properties at the mutation boundary** - For constrained types such as `Credential`, reject forbidden property names during both creation and update. - Consider recursively checking nested objects for sensitive fields. - Return a nonzero exit status without echoing sensitive values. 3. **Validate relations before creation** - Require both endpoint entities to exist. - Enforce `from_types` and `to_types`. - Enforce cardinality constraints. - Test whether adding an acyclic relation would introduce a cycle before committing it. 4. **Make writes atomic** - Validate first and then append under an appropriate file lock. - Prevent a concurrent writer from changing the graph between validation and commit. - Flush and synchronize critical writes where durability is required. 5. **Provide safe handling for existing invalid data** - Add a repair or quarantine command that can remove or redact invalid records. - Do not rely on append-only deletion to protect secrets already present in historical JSONL records, because deleted values remain in earlier log entries. - Document and implement secure graph compaction or secret-redaction procedures. 6. **Add regression tests** - Verify that credentials containing forbidden fields never reach disk. - Verify rejection of missing required properties and invalid enum values. - Verify rejection of dangling, type-invalid, cardinality-violating, and cyclic relations. - Verify that failed validation leaves the graph byte-for-byte unchanged. ]]>