fix(merge-chunks): validate untrusted subagent chunk JSON before merging

`graphify merge-chunks` concatenates agent-written `.graphify_chunk_*.json`
files with only a JSON-decode guard, so an oversized payload or a crafted
node/edge id (e.g. `../../etc/passwd`) flowed straight into the merged graph.

Route each chunk through `load_validated_semantic_fragment`, which stats the
file size BEFORE reading it (a multi-GB chunk can't blow up memory), parses the
JSON, and validates the byte/count caps + the node/edge id charset that blocks
path traversal (#825). An invalid chunk is skipped with a warning (filter
semantics; never abort). Left OUT of build_from_json/load_graph_json on purpose:
those must keep loading valid pre-existing graphs.

Also relax two over-strict checks in the shared validator that would otherwise
silently drop whole legitimate chunks (a relaxation — never a new rejection):
- file_type is no longer gated: build coerces any value via _FILE_TYPE_SYNONYMS
  (unknown -> "concept", #840), so synonyms like "markdown"/"tool" the loader
  maps must not fail validation.
- the id charset now allows Unicode word chars (build's normalize_id preserves
  CJK/Cyrillic/accented-Latin ids); the explicit path-separator/".." check still
  blocks directory escape.
Corrects the stale module docstring (the validator serves the devin skill path
and merge-chunks, not skill-opencode/codex).

graphify/cli.py

@@ -3040,11 +3040,23 @@ def dispatch_command(cmd: str) -> None:
             chunk_files.extend(sorted(expanded) if expanded else [arg])
         merged: dict = {"nodes": [], "edges": [], "hyperedges": [], "input_tokens": 0, "output_tokens": 0}
         seen_ids: set[str] = set()
+        # These chunk files are untrusted subagent output. load_validated_...
+        # stats the file size BEFORE reading it (so a multi-GB chunk can't blow up
+        # memory), parses the JSON, and validates the security caps + the node/
+        # edge id charset that blocks path traversal (#825) — the same enforcement
+        # the skill merge path applies. A bad chunk is skipped with a warning
+        # (filter semantics; never abort). Deliberately NOT wired into
+        # build_from_json/load_graph_json, which must keep loading valid
+        # pre-existing graphs. file_type is left to build's coercion (#840).
+        from graphify.semantic_cleanup import load_validated_semantic_fragment
         for cf in chunk_files:
-            try:
-                chunk = json.loads(Path(cf).read_text(encoding="utf-8"))
-            except (json.JSONDecodeError, OSError) as exc:
-                print(f"[graphify merge-chunks] warning: skipping {cf}: {exc}", file=sys.stderr)
+            chunk, _chunk_errs = load_validated_semantic_fragment(Path(cf))
+            if _chunk_errs:
+                print(
+                    f"[graphify merge-chunks] warning: skipping invalid chunk {cf}: "
+                    f"{'; '.join(_chunk_errs[:3])}",
+                    file=sys.stderr,
+                )
                 continue
             for n in chunk.get("nodes", []):
                 if n.get("id") not in seen_ids:

graphify/semantic_cleanup.py

@@ -1,11 +1,13 @@
 # Semantic fragment sanitizer — converts sentence-like rationale nodes into
 # attributes on related nodes and removes invalid file_type values.
 #
-# Currently called from the skill merge scripts (skill-opencode.md,
-# skill-codex.md) so that rationale text never leaks into the knowledge
-# graph as standalone nodes. (Future: graphify.llm may wire this into
-# _parse_llm_json / _merge_into for non-skill code paths; not done in
-# this cycle.)
+# Called from the skill merge path (see skill-devin.md) and from the in-process
+# `graphify merge-chunks` command — both ingest untrusted agent-written chunk
+# JSON, and validate_semantic_fragment() rejects malformed/oversized payloads and
+# crafted node/edge IDs before they touch the graph. The primary build/load paths
+# (build_from_json, load_graph_json) deliberately do NOT run this: they must keep
+# loading valid pre-existing graphs whose AST node IDs predate the stricter
+# semantic-ID charset.
 from __future__ import annotations
 
 import json
@@ -31,7 +33,12 @@ MAX_SEMANTIC_FRAGMENT_HYPEREDGES = 10_000
 MAX_SEMANTIC_HYPEREDGE_NODES = 256
 MAX_SEMANTIC_ID_LENGTH = 256
 VALID_SEMANTIC_FILE_TYPES = frozenset({"code", "document", "paper", "image", "rationale", "concept"})
-_SEMANTIC_ID_RE = re.compile(r"^[A-Za-z0-9._:-]+$")
+# Unicode word characters are allowed: build's normalize_id preserves CJK /
+# Cyrillic / accented-Latin identifiers, so an ASCII-only gate would reject valid
+# ids that the loader accepts. The explicit path-separator / ".." check in
+# _validate_semantic_id still blocks directory escape (#825); "/", "\\", spaces,
+# "@", "#" etc. are not \w and remain rejected.
+_SEMANTIC_ID_RE = re.compile(r"^[\w.:-]+$")
 
 
 def validate_semantic_fragment(fragment: object) -> list[str]:
@@ -74,12 +81,12 @@ def validate_semantic_fragment(fragment: object) -> list[str]:
             errors.append(f"nodes[{i}] must be an object")
             continue
         _validate_semantic_id(errors, f"nodes[{i}].id", node.get("id"))
-        file_type = node.get("file_type")
-        if file_type is not None and file_type not in VALID_SEMANTIC_FILE_TYPES:
-            errors.append(
-                f"nodes[{i}].file_type {file_type!r} is not one of "
-                f"{sorted(VALID_SEMANTIC_FILE_TYPES)}"
-            )  # validate file_type before any sanitize path can run
+        # file_type is intentionally NOT rejected here. It carries no security
+        # risk (it can't exhaust memory or escape a directory), and
+        # build_from_json already coerces every value via _FILE_TYPE_SYNONYMS
+        # (unknown -> "concept", #840). Rejecting a whole chunk over a synonym
+        # like "markdown"/"tool"/"framework" that the loader would happily map is
+        # pure data loss, so leave file_type normalization to build.
 
     for i, edge in enumerate(edges):
         if not isinstance(edge, dict):

tests/test_merge_chunks_validation.py

@@ -0,0 +1,104 @@
+"""Tests that `graphify merge-chunks` validates untrusted subagent chunk JSON.
+
+merge-chunks concatenates agent-written `.graphify_chunk_*.json` files. Those are
+untrusted output, so each is run through `validate_semantic_fragment` (caps + the
+node/edge ID charset that blocks path-escape). An invalid chunk is skipped with a
+warning; valid chunks still merge.
+"""
+import json
+
+import graphify.__main__ as mainmod
+
+
+def _write(path, obj):
+    path.write_text(json.dumps(obj), encoding="utf-8")
+
+
+def _run_merge(monkeypatch, argv):
+    monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
+    monkeypatch.setattr(mainmod.sys, "argv", argv)
+    mainmod.main()
+
+
+def test_merge_chunks_skips_chunk_with_path_escape_id(tmp_path, monkeypatch, capsys):
+    good = tmp_path / ".graphify_chunk_0.json"
+    _write(good, {"nodes": [{"id": "pkg.mod.good", "label": "G"}], "edges": [], "hyperedges": []})
+    bad = tmp_path / ".graphify_chunk_1.json"
+    # A node id with a path separator would escape the chunk directory (#825).
+    _write(bad, {"nodes": [{"id": "../../etc/passwd", "label": "B"}], "edges": [], "hyperedges": []})
+    out = tmp_path / "merged.json"
+
+    _run_merge(monkeypatch, ["graphify", "merge-chunks", str(good), str(bad), "--out", str(out)])
+
+    merged = json.loads(out.read_text())
+    assert {n["id"] for n in merged["nodes"]} == {"pkg.mod.good"}
+    assert "skipping invalid chunk" in capsys.readouterr().err
+
+
+def test_merge_chunks_skips_malformed_shape(tmp_path, monkeypatch, capsys):
+    bad = tmp_path / ".graphify_chunk_0.json"
+    _write(bad, {"nodes": "not-a-list", "edges": []})
+    out = tmp_path / "merged.json"
+
+    _run_merge(monkeypatch, ["graphify", "merge-chunks", str(bad), "--out", str(out)])
+
+    merged = json.loads(out.read_text())
+    assert merged["nodes"] == []
+    assert "skipping invalid chunk" in capsys.readouterr().err
+
+
+def test_merge_chunks_accepts_synonym_file_type(tmp_path, monkeypatch):
+    # file_type synonyms (markdown/tool/framework/...) are coerced by build, not
+    # a validation failure — the chunk must merge, not be silently dropped (#840).
+    c = tmp_path / ".graphify_chunk_0.json"
+    _write(c, {"nodes": [{"id": "pkg.readme", "label": "Readme", "file_type": "markdown"},
+                         {"id": "pkg.tool", "label": "Tool", "file_type": "tool"}],
+               "edges": [], "hyperedges": []})
+    out = tmp_path / "merged.json"
+    _run_merge(monkeypatch, ["graphify", "merge-chunks", str(c), "--out", str(out)])
+    merged = json.loads(out.read_text())
+    assert {n["id"] for n in merged["nodes"]} == {"pkg.readme", "pkg.tool"}
+
+
+def test_merge_chunks_accepts_unicode_id(tmp_path, monkeypatch):
+    # build's normalize_id preserves Unicode identifiers; validation must not
+    # reject a chunk that uses them.
+    c = tmp_path / ".graphify_chunk_0.json"
+    _write(c, {"nodes": [{"id": "mod_处理数据", "label": "handler", "file_type": "code"}],
+               "edges": [], "hyperedges": []})
+    out = tmp_path / "merged.json"
+    _run_merge(monkeypatch, ["graphify", "merge-chunks", str(c), "--out", str(out)])
+    merged = json.loads(out.read_text())
+    assert {n["id"] for n in merged["nodes"]} == {"mod_处理数据"}
+
+
+def test_validate_semantic_fragment_accepts_synonyms_and_unicode():
+    from graphify.semantic_cleanup import validate_semantic_fragment
+    frag = {"nodes": [{"id": "mod_处理", "file_type": "markdown"},
+                      {"id": "a.b::C.d", "file_type": "tool"}],
+            "edges": [], "hyperedges": []}
+    assert validate_semantic_fragment(frag) == []
+
+
+def test_validate_semantic_fragment_still_blocks_path_escape():
+    from graphify.semantic_cleanup import validate_semantic_fragment
+    errs = validate_semantic_fragment({"nodes": [{"id": "../../etc/passwd"}],
+                                       "edges": [], "hyperedges": []})
+    assert errs
+
+
+def test_merge_chunks_merges_valid_chunks(tmp_path, monkeypatch):
+    c0 = tmp_path / ".graphify_chunk_0.json"
+    _write(c0, {"nodes": [{"id": "a", "label": "A"}], "edges": [], "hyperedges": [],
+               "input_tokens": 10, "output_tokens": 5})
+    c1 = tmp_path / ".graphify_chunk_1.json"
+    _write(c1, {"nodes": [{"id": "b", "label": "B"}], "edges": [], "hyperedges": [],
+               "input_tokens": 7, "output_tokens": 3})
+    out = tmp_path / "merged.json"
+
+    _run_merge(monkeypatch, ["graphify", "merge-chunks", str(c0), str(c1), "--out", str(out)])
+
+    merged = json.loads(out.read_text())
+    assert {n["id"] for n in merged["nodes"]} == {"a", "b"}
+    assert merged["input_tokens"] == 17
+    assert merged["output_tokens"] == 8

tests/test_semantic_cleanup.py

@@ -51,11 +51,15 @@ def test_validate_semantic_fragment_rejects_path_separator_in_id():
     assert any("nodes[0].id" in e for e in errors)
 
 
-def test_validate_semantic_fragment_rejects_invalid_file_type():
+def test_validate_semantic_fragment_accepts_unknown_file_type():
+    """An unknown/synonym file_type is NOT a validation failure: build_from_json
+    coerces any value via _FILE_TYPE_SYNONYMS (unknown -> "concept", #840), so
+    rejecting a whole chunk over it would be pure data loss. file_type carries no
+    security risk, so it is left to build's coercion rather than gated here."""
     fragment = _valid_fragment()
     fragment["nodes"][0]["file_type"] = "executable"
     errors = sc.validate_semantic_fragment(fragment)
-    assert any("file_type" in e for e in errors)
+    assert not any("file_type" in e for e in errors)
 
 
 def test_validate_semantic_fragment_accepts_rationale_file_type():