Merge pull request #1405 from MemPalace/fix/1156-exporter-reject-symlinks

fix(exporter): refuse symlinks at export targets (#1156)

mempalace/exporter.py

@@ -11,6 +11,7 @@ Streams drawers in paginated batches so memory usage stays bounded
 regardless of palace size.
 """
 
+import errno
 import os
 import re
 from collections import defaultdict
@@ -26,6 +27,44 @@ def _safe_path_component(name: str) -> str:
     return name or "unknown"
 
 
+def _reject_symlink(path: str, label: str) -> None:
+    """Refuse to write into a path that is itself a symlink.
+
+    Defense-in-depth: a pre-placed symlink at the export target would
+    redirect writes to wherever it points (e.g., system directories).
+    Mirrors the miner's input-side caution.
+    """
+    if os.path.islink(path):
+        raise ValueError(
+            f"refusing to export: {label} is a symbolic link ({path!r}). "
+            f"Remove the symlink or choose a different output path."
+        )
+
+
+def _safe_open_for_write(path: str, mode: str, encoding: str = "utf-8"):
+    """Open a file for writing, refusing to follow a symlink at the target path.
+
+    On POSIX (O_NOFOLLOW available) the open itself fails with ELOOP if path is
+    a symlink — closing the TOCTOU window between an islink check and the open.
+    On platforms without O_NOFOLLOW (Windows), pre-checks ``os.path.islink``,
+    which is narrower than no check at all.
+    """
+    o_nofollow = getattr(os, "O_NOFOLLOW", 0)
+    if o_nofollow:
+        flags = os.O_WRONLY | os.O_CREAT | o_nofollow
+        flags |= os.O_APPEND if "a" in mode else os.O_TRUNC
+        try:
+            fd = os.open(path, flags, 0o600)
+        except OSError as e:
+            if e.errno == errno.ELOOP:
+                raise ValueError(f"refusing to write: {path!r} is a symbolic link.") from None
+            raise
+        return os.fdopen(fd, mode, encoding=encoding)
+    if os.path.islink(path):
+        raise ValueError(f"refusing to write: {path!r} is a symbolic link.")
+    return open(path, mode, encoding=encoding)
+
+
 def export_palace(palace_path: str, output_dir: str, format: str = "markdown") -> dict:
     """Export all palace drawers as markdown files organized by wing/room.
 
@@ -48,6 +87,7 @@ def export_palace(palace_path: str, output_dir: str, format: str = "markdown") -
         print("  Palace is empty — nothing to export.")
         return {"wings": 0, "rooms": 0, "drawers": 0}
 
+    _reject_symlink(output_dir, "output_dir")
     os.makedirs(output_dir, exist_ok=True)
     try:
         os.chmod(output_dir, 0o700)
@@ -89,6 +129,7 @@ def export_palace(palace_path: str, output_dir: str, format: str = "markdown") -
             safe_wing = _safe_path_component(wing)
             wing_dir = os.path.join(output_dir, safe_wing)
             if wing_dir not in created_wing_dirs:
+                _reject_symlink(wing_dir, f"wing directory {safe_wing!r}")
                 os.makedirs(wing_dir, exist_ok=True)
                 try:
                     os.chmod(wing_dir, 0o700)
@@ -102,7 +143,7 @@ def export_palace(palace_path: str, output_dir: str, format: str = "markdown") -
                 key = (wing, room)
                 is_new = key not in opened_rooms
 
-                with open(room_path, "a" if not is_new else "w", encoding="utf-8") as f:
+                with _safe_open_for_write(room_path, "a" if not is_new else "w") as f:
                     if is_new:
                         f.write(f"# {wing} / {room}\n\n")
                         opened_rooms.add(key)
@@ -152,7 +193,7 @@ def export_palace(palace_path: str, output_dir: str, format: str = "markdown") -
     index_lines.append("")
 
     index_path = os.path.join(output_dir, "index.md")
-    with open(index_path, "w", encoding="utf-8") as f:
+    with _safe_open_for_write(index_path, "w") as f:
         f.write("\n".join(index_lines))
 
     stats = {

tests/test_exporter.py

@@ -134,3 +134,104 @@ def test_export_empty_palace():
         assert stats == {"wings": 0, "rooms": 0, "drawers": 0}
     finally:
         shutil.rmtree(tmpdir, ignore_errors=True)
+
+
+def _try_symlink_or_skip(target: str, link: str):
+    """Create a symlink, skipping the test if the runtime forbids it.
+
+    Windows without Developer Mode/admin and some restricted CI sandboxes
+    refuse os.symlink with OSError or NotImplementedError. The exporter
+    hardening is meaningful only where symlinks can be created at all, so
+    skipping is preferable to a hard failure.
+    """
+    import pytest
+
+    try:
+        os.symlink(target, link)
+    except (OSError, NotImplementedError) as e:
+        pytest.skip(f"symlink creation not supported in this environment: {e}")
+
+
+def test_export_refuses_symlinked_output_dir():
+    """A symlink at the output path must not be followed (defense-in-depth)."""
+    import pytest
+
+    tmpdir = tempfile.mkdtemp()
+    try:
+        palace_path = _setup_palace(tmpdir)
+        decoy_target = os.path.join(tmpdir, "decoy_target")
+        os.makedirs(decoy_target)
+        output_dir = os.path.join(tmpdir, "export")
+        _try_symlink_or_skip(decoy_target, output_dir)
+
+        with pytest.raises(ValueError, match="symbolic link"):
+            export_palace(palace_path, output_dir)
+
+        # Decoy target must remain empty — nothing followed the symlink.
+        assert os.listdir(decoy_target) == []
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=True)
+
+
+def test_export_refuses_symlinked_wing_dir():
+    """A symlink pre-placed at a wing subdirectory must also be refused."""
+    import pytest
+
+    tmpdir = tempfile.mkdtemp()
+    try:
+        palace_path = _setup_palace(tmpdir)
+        decoy_target = os.path.join(tmpdir, "decoy_target")
+        os.makedirs(decoy_target)
+        output_dir = os.path.join(tmpdir, "export")
+        os.makedirs(output_dir)
+        _try_symlink_or_skip(decoy_target, os.path.join(output_dir, "alpha"))
+
+        with pytest.raises(ValueError, match="symbolic link"):
+            export_palace(palace_path, output_dir)
+
+        assert os.listdir(decoy_target) == []
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=True)
+
+
+def test_export_refuses_symlinked_room_file():
+    """A symlink pre-placed at a room file path must not be followed."""
+    import pytest
+
+    tmpdir = tempfile.mkdtemp()
+    try:
+        palace_path = _setup_palace(tmpdir)
+        decoy_target = os.path.join(tmpdir, "decoy_target.md")
+        Path(decoy_target).write_text("untouched\n", encoding="utf-8")
+        output_dir = os.path.join(tmpdir, "export")
+        os.makedirs(os.path.join(output_dir, "alpha"))
+        _try_symlink_or_skip(decoy_target, os.path.join(output_dir, "alpha", "backend.md"))
+
+        with pytest.raises(ValueError, match="symbolic link"):
+            export_palace(palace_path, output_dir)
+
+        # Decoy file must remain unchanged — open did not follow the symlink.
+        assert Path(decoy_target).read_text(encoding="utf-8") == "untouched\n"
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=True)
+
+
+def test_export_refuses_symlinked_index_file():
+    """A symlink pre-placed at output_dir/index.md must not be followed."""
+    import pytest
+
+    tmpdir = tempfile.mkdtemp()
+    try:
+        palace_path = _setup_palace(tmpdir)
+        decoy_target = os.path.join(tmpdir, "decoy_index.md")
+        Path(decoy_target).write_text("untouched\n", encoding="utf-8")
+        output_dir = os.path.join(tmpdir, "export")
+        os.makedirs(output_dir)
+        _try_symlink_or_skip(decoy_target, os.path.join(output_dir, "index.md"))
+
+        with pytest.raises(ValueError, match="symbolic link"):
+            export_palace(palace_path, output_dir)
+
+        assert Path(decoy_target).read_text(encoding="utf-8") == "untouched\n"
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=True)