fix(export): a control character in a label must not abort the export (#2897)

graphify/export.py

@@ -502,6 +502,31 @@ def to_cypher(G: nx.Graph, output_path: str) -> None:
 generate_html = to_html
 
 
+# Characters XML 1.0 cannot carry: the C0 controls except tab, LF and CR.
+_XML_ILLEGAL_RE = re.compile("[\x00-\x08\x0b\x0c\x0e-\x1f]")
+
+
+def _strip_xml_illegal(s: str) -> str:
+    """Drop characters XML 1.0 cannot represent, leaving tab/LF/CR intact.
+
+    ``nx.write_graphml`` raises ``ValueError("All strings must be XML
+    compatible: Unicode or ASCII, no NULL bytes or control characters")`` on any
+    of them and aborts the whole export over a single label. Labels arrive
+    unfiltered from the corpus, so this is ordinary content rather than hostile
+    input: an ANSI escape in a markdown heading pasted from a terminal capture,
+    or the form feed some Python/Emacs sources use as a section separator
+    (#2897).
+    """
+    return _XML_ILLEGAL_RE.sub("", s)
+
+
+# C0 controls and DEL, folded to a space when building a filename stem. Windows
+# rejects them in a path outright with OSError EINVAL, so one of them in a label
+# aborted a whole Obsidian vault export; POSIX would accept the name but leave a
+# note nothing can comfortably open (#2897).
+_CONTROL_TO_SPACE_RE = re.compile("[\x00-\x1f\x7f]")
+
+
 def _cap_filename(s: str, limit: int = 200) -> str:
     """Cap a filename stem to ``limit`` UTF-8 bytes so it stays under the 255-byte
     filesystem limit even after the ``.md`` extension and dedup suffix are added
@@ -531,7 +556,11 @@ def _obsidian_safe_stem(label: str, limit: int = 200) -> str:
     cleaned = re.sub(
         r'[\\/*?:"<>|#^[\]]',
         "",
-        label.replace("\r\n", " ").replace("\r", " ").replace("\n", " "),
+        # CR/LF were already folded to spaces here; every other C0 control now
+        # goes the same way. They are not merely awkward in a filename — Windows
+        # rejects them outright, so a single one aborted the whole vault export
+        # rather than spoiling one note (#2897).
+        _CONTROL_TO_SPACE_RE.sub(" ", label),
     ).strip()
     cleaned = re.sub(r"\.(md|mdx|qmd|markdown)$", "", cleaned, flags=re.IGNORECASE)
     # Obsidian treats a leading-dot filename as a hidden file (#2205). Only
@@ -1127,12 +1156,26 @@ def to_graphml(
     def _graphml_safe(val):
         if val is None:
             return ""
-        if isinstance(val, bool) or isinstance(val, (int, float, str)):
+        if isinstance(val, bool) or isinstance(val, (int, float)):
             return val  # GraphML-native scalars pass through unchanged
+        if isinstance(val, str):
+            # Scalar, but still has to be XML-representable — see
+            # _strip_xml_illegal. This is the line that turns "one label carried
+            # an ANSI escape" from a lost export into a lost escape character.
+            return _strip_xml_illegal(val)
         try:
-            return json.dumps(val, default=str, sort_keys=True)
+            return _strip_xml_illegal(json.dumps(val, default=str, sort_keys=True))
         except (TypeError, ValueError):
-            return str(val)
+            return _strip_xml_illegal(str(val))
+
+    # Node IDs become the `id` attribute of every <node> and edge endpoint, so
+    # they must be XML-representable too. Normalised ids never carry a control
+    # character, but a caller can hand us a hand-built graph, and a crash here
+    # loses the export just as completely as one in the values.
+    _id_remap = {n: _strip_xml_illegal(n) for n in H.nodes if isinstance(n, str)}
+    _id_remap = {k: v for k, v in _id_remap.items() if k != v}
+    if _id_remap:
+        H = nx.relabel_nodes(H, _id_remap, copy=True)
 
     for key, val in list(H.graph.items()):
         H.graph[key] = _graphml_safe(val)

tests/test_export_control_characters.py

@@ -0,0 +1,147 @@
+"""A control character in a label must not abort an export.
+
+Labels reach the exporters unfiltered from the corpus. A markdown heading pasted
+from a terminal capture carries ANSI escapes (`\\x1b`), and the form feed some
+Python/Emacs sources use as a section separator is `\\x0c`. Both are ordinary
+content, and detection, extraction and the graph build all accept them happily.
+
+Two exporters then died on the whole graph:
+
+* `to_graphml` -> `ValueError: All strings must be XML compatible: Unicode or
+  ASCII, no NULL bytes or control characters` (XML 1.0 permits tab, LF and CR
+  and no other C0 control);
+* `to_obsidian` -> `OSError: [Errno 22] Invalid argument` on Windows, which
+  rejects control characters in a path outright, so one bad label cost the
+  entire vault rather than one note.
+
+`to_cypher` already stripped them and `to_html` already routes labels through
+`security.sanitize_label`, so the codebase knew the hazard — those two paths
+just did not. (#2897)
+"""
+import json
+import xml.etree.ElementTree as ET
+
+import pytest
+
+from graphify.build import build_from_json
+from graphify.export import (
+    _obsidian_safe_stem,
+    to_cypher,
+    to_graphml,
+    to_json,
+    to_obsidian,
+)
+
+# Legal in XML and in a filename — these must survive untouched.
+KEEP = ["\t", "\n", "\r"]
+# Rejected by XML 1.0, and by Windows in a path.
+BREAK = ["\x00", "\x07", "\x08", "\x0b", "\x0c", "\x1b", "\x1f"]
+
+
+def _graph(label):
+    return build_from_json({
+        "nodes": [
+            {"id": "a", "label": label, "file_type": "document", "source_file": "d.md"},
+            {"id": "b", "label": "plain", "file_type": "code", "source_file": "b.py"},
+        ],
+        "edges": [{"source": "a", "target": "b", "relation": "references",
+                   "confidence": "INFERRED", "confidence_score": 0.85,
+                   "source_file": "d.md"}],
+        "hyperedges": [],
+    })
+
+
+COMMUNITIES = {0: ["a", "b"]}
+
+
+# ---------------------------------------------------------------------------
+# GraphML
+# ---------------------------------------------------------------------------
+
+@pytest.mark.parametrize("ch", BREAK)
+def test_graphml_survives_a_control_character(tmp_path, ch):
+    out = tmp_path / "g.graphml"
+    to_graphml(_graph(f"Build {ch}log capture"), COMMUNITIES, str(out))
+    ET.fromstring(out.read_text(encoding="utf-8"))  # must be well-formed XML
+
+
+@pytest.mark.parametrize("ch", KEEP)
+def test_graphml_keeps_the_whitespace_xml_allows(tmp_path, ch):
+    """Tab, LF and CR are valid XML and carry meaning in a label; the fix must
+    not sweep them up with the rest."""
+    out = tmp_path / "g.graphml"
+    to_graphml(_graph("a" + ch + "b"), COMMUNITIES, str(out))
+    ET.fromstring(out.read_text(encoding="utf-8"))  # still well-formed
+    # A writer may normalise CR to LF, so only the two that round-trip
+    # literally are asserted to survive verbatim.
+    if ch != "\r":
+        assert ("a" + ch + "b") in out.read_text(encoding="utf-8")
+
+
+def test_graphml_survives_a_control_character_in_a_node_id(tmp_path):
+    """IDs become XML attributes too."""
+    G = build_from_json({
+        "nodes": [{"id": "we\x0bird", "label": "x", "file_type": "code",
+                   "source_file": "a.py"}],
+        "edges": [], "hyperedges": [],
+    })
+    out = tmp_path / "g.graphml"
+    to_graphml(G, {0: ["we\x0bird"]}, str(out))
+    ET.fromstring(out.read_text(encoding="utf-8"))
+
+
+def test_graphml_still_carries_the_readable_part_of_the_label(tmp_path):
+    out = tmp_path / "g.graphml"
+    to_graphml(_graph("Build \x1b[31mlog\x1b[0m capture"), COMMUNITIES, str(out))
+    text = out.read_text(encoding="utf-8")
+    assert "log" in text and "capture" in text
+    assert "\x1b" not in text
+
+
+# ---------------------------------------------------------------------------
+# Obsidian
+# ---------------------------------------------------------------------------
+
+@pytest.mark.parametrize("ch", BREAK)
+def test_obsidian_export_survives_a_control_character(tmp_path, ch):
+    count = to_obsidian(_graph(f"Release {ch} Notes"), COMMUNITIES,
+                        str(tmp_path / "vault"))
+    assert count >= 2
+    assert list((tmp_path / "vault").glob("*.md"))
+
+
+@pytest.mark.parametrize("ch", BREAK + KEEP)
+def test_no_stem_ever_contains_a_control_character(ch):
+    stem = _obsidian_safe_stem(f"Release {ch} Notes")
+    assert not any(ord(c) < 32 or ord(c) == 127 for c in stem), repr(stem)
+
+
+def test_stem_keeps_the_words_around_the_control_character():
+    assert _obsidian_safe_stem("Release \x0c Notes").startswith("Release")
+    assert "Notes" in _obsidian_safe_stem("Release \x0c Notes")
+
+
+def test_a_label_that_is_only_control_characters_still_yields_a_name():
+    stem = _obsidian_safe_stem("\x00\x0b\x1b")
+    assert stem and not any(ord(c) < 32 for c in stem)
+
+
+# ---------------------------------------------------------------------------
+# The exporters that already coped must keep coping
+# ---------------------------------------------------------------------------
+
+def test_cypher_and_json_are_unaffected(tmp_path):
+    G = _graph("Build \x1b[31mlog\x1b[0m capture")
+    to_cypher(G, str(tmp_path / "g.cypher"))
+    to_json(G, COMMUNITIES, str(tmp_path / "g.json"))
+    json.loads((tmp_path / "g.json").read_text(encoding="utf-8"))
+    assert "\x1b" not in (tmp_path / "g.cypher").read_text(encoding="utf-8")
+
+
+def test_a_clean_label_round_trips_unchanged(tmp_path):
+    """The fix must be invisible for ordinary labels."""
+    G = _graph("Perfectly Ordinary Heading")
+    out = tmp_path / "g.graphml"
+    to_graphml(G, COMMUNITIES, str(out))
+    assert "Perfectly Ordinary Heading" in out.read_text(encoding="utf-8")
+    assert _obsidian_safe_stem("Perfectly Ordinary Heading") == "Perfectly Ordinary Heading"