ITADN

ontologies: ec edges path relies on unguarded pandas index promotion

#705Openrealmarcin 创建于 22 天前
R
realmarcincommented
Found in the adversarial review of #704. Pre-existing, HIGH risk, not introduced by that PR. ## The hazard The `ec` branch prepends a **7-column** canonical header over an **8-field** body. `pd.read_csv` in `_normalize_schema` sees 7 names for 8 fields and silently promotes the leading `id` field to the DataFrame index. The remaining 7 fields then align 1:1 with the canonical header **by coincidence** — that alignment is the only reason `knowledge_source` → `primary_knowledge_source` works at all. Nothing asserts it. Demonstrated with real pandas: | body shape | result | |---|---| | 8 fields / 7-col header (today) | correct — index absorbs `id` | | **9 fields / 7-col header** | **no error.** Two-level MultiIndex; every column shifts by two. `subject` becomes the predicate, `predicate` becomes the object, `primary_knowledge_source` becomes `knowledge_assertion`. **Exit 0, silently corrupt.** | | reordered (`subject` before `id`) | silently wrong: `subject` = `urn:uuid:...` | **One additional upstream edge column is enough to trigger it.** KGX or the obograph writer adding a field would do it. ## Why the #704 fix does not cover this #704's backstop filters rows whose `subject` cell is literally `"subject"`. Under a column shift the leaked header's `subject` cell reads `"predicate"`, so it would not match. And #704 removes the visible stray row that was the only symptom of the structural mismatch — so the next occurrence is silent. ## Suggested fix Read the edges file with explicit column handling rather than relying on positional coincidence: - `pd.read_csv(..., index_col=False)` so a surplus leading field errors instead of becoming an index, or - read the real header, assert the field count matches, and fail loudly on mismatch. An assertion that `len(df.columns) == len(self.edge_header)` after the ec branch would have caught every variant above. ## Related - #704 — the symptom fix and the producer repair. - The same coincidence explains why the leaked header's `subject` cell read `"subject"` at all, which is what made the original defect visible. Without it the bug would have been silent from the start.
0 条评论