Merge pull request #851 from SaurabhScripts/codex/harden-markdown-import-symlinks

fix(context): reject Markdown import symlinks

CHANGELOG.md

@@ -402,6 +402,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Markdown import followed filesystem links even though Markdown export already refused to overwrite them** (#851, follow-up to #765, #786) by @SaurabhScripts
+  - `AgentMemory._read_markdown_path()` now rejects symlink files, broken symlinks, symlinked directories, Windows directory junctions, and other Windows reparse points supplied directly; linked entries discovered inside an otherwise valid directory are safely skipped, preserving the current directory-import contract
+  - `_read_markdown_file_content()` re-checks the file and parent directory immediately before and after opening, uses `O_NOFOLLOW` where available, and verifies the resulting descriptor is a regular file via `fstat`/`S_ISREG`, so link swaps are rejected rather than silently followed
+  - Junction detection uses `os.path.isjunction()` where available and falls back to the Windows reparse-point file attribute on older Python versions; export applies the same link check before replacing a Markdown file
+  - Documented the import restriction in `docs/reference/context.md`; added 11 tests to `tests/context/test_agent_memory_markdown.py` covering file/directory/broken-symlink rejection, simulated open races, mocked and real Windows junctions, and the reparse-point fallback
+  - Any additional review follow-up commits land in this same PR/entry rather than as a separate changelog item
+
 - **`PipelineWithProvenance` raised `ModuleNotFoundError` on import and `AttributeError` on `.run()`** (#858, closes #858) by @Karunasagar12
   - `from .pipeline import Pipeline` failed because `semantica/pipeline/pipeline.py` does not exist; corrected to `from .pipeline_builder import Pipeline`
   - `.run()` called `self._pipeline.run()` on the `Pipeline` dataclass, which has no such method; replaced with `self._engine.execute_pipeline(self._pipeline, ...)` delegating to `ExecutionEngine`

docs/reference/context.md

@@ -625,8 +625,10 @@ malformed or duplicate fields before changing memory, and re-importing unchanged
 files is idempotent. Memory-local `entities` and `relationships` are preserved as
 provenance but are not applied to `ContextGraph` by Markdown import. Use a dedicated
 export directory: matching files are overwritten, but unrelated or stale Markdown
-files are not deleted automatically. Export refuses to overwrite symbolic links and
-uses atomic file replacement. Timestamp offsets are preserved in Markdown and
+files are not deleted automatically. Export refuses to overwrite filesystem links and
+uses atomic file replacement; import also refuses symlinks, Windows directory
+junctions, and other Windows reparse points.
+Timestamp offsets are preserved in Markdown and
 normalized to UTC only for comparisons, so aware and local-naive records can be
 queried together safely. Vector-store writes are deferred until the in-memory import
 commits; adapter synchronization remains best-effort and logs failures.

semantica/context/_markdown_filesystem.py

@@ -0,0 +1,32 @@
+"""Filesystem safety helpers for human-editable Markdown persistence."""
+
+import os
+import stat
+from pathlib import Path
+from typing import Optional
+
+
+def is_filesystem_link(path: Path) -> bool:
+    """Return whether *path* is a symlink, junction, or Windows reparse point."""
+    if path.is_symlink():
+        return True
+
+    isjunction = getattr(os.path, "isjunction", None)
+    if isjunction is not None and isjunction(path):
+        return True
+
+    try:
+        attributes = getattr(os.lstat(path), "st_file_attributes", 0)
+    except (FileNotFoundError, NotADirectoryError):
+        return False
+
+    reparse_point = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0x400)
+    return bool(attributes & reparse_point)
+
+
+def find_filesystem_link(path: Path) -> Optional[Path]:
+    """Return the first linked component in *path*, including its ancestors."""
+    for candidate in (path, *path.parents):
+        if is_filesystem_link(candidate):
+            return candidate
+    return None

semantica/context/agent_memory.py

@@ -77,6 +77,7 @@ import yaml
 from ..utils.logging import get_logger
 from ..utils.progress_tracker import get_progress_tracker
 from ..utils.types import EntityDict, RelationshipDict
+from ._markdown_filesystem import find_filesystem_link
 
 
 class _UniqueKeySafeLoader(yaml.SafeLoader):
@@ -1742,9 +1743,10 @@ class AgentMemory:
     @staticmethod
     def _write_markdown_file(file_path: Path, document: str) -> None:
         """Atomically replace a Markdown file without following output symlinks."""
-        if file_path.is_symlink():
+        if find_filesystem_link(file_path) is not None:
             raise ValueError(
-                f"Refusing to overwrite Markdown symbolic link: {file_path}"
+                "Refusing to overwrite Markdown symbolic link or junction: "
+                f"{file_path}"
             )
 
         temporary_path = None
@@ -1867,7 +1869,8 @@ class AgentMemory:
             if "\n" not in data and "\r" not in data:
                 candidate = Path(data)
                 try:
-                    candidate_exists = candidate.exists()
+                    candidate_is_link = find_filesystem_link(candidate) is not None
+                    candidate_exists = candidate_is_link or candidate.exists()
                 except OSError as exc:
                     error_message = (
                         "Failed to inspect possible Markdown import "
@@ -1909,62 +1912,71 @@ class AgentMemory:
         return memories
 
     def _read_markdown_file_content(self, file_path: Path) -> str:
-        if file_path.is_symlink():
-            raise ValueError(f"Symlink Markdown import paths are rejected: {file_path}")
+        if find_filesystem_link(file_path) is not None:
+            raise ValueError(
+                "Symlink Markdown import paths are rejected; symbolic links and "
+                f"junctions are unsafe: {file_path}"
+            )
 
         flags = os.O_RDONLY
-        if hasattr(os, "O_NOFOLLOW"):
-            # On POSIX, O_NOFOLLOW makes os.open() fail with ELOOP if the
-            # final path component is a symlink, atomically closing the TOCTOU
-            # window between the is_symlink() check above and the open call.
-            # On Windows, O_NOFOLLOW is not available; the is_symlink() pre-check
-            # above is the only symlink defense and remains vulnerable to a narrow
-            # race.  The fstat()/S_ISREG guard below still rejects special files
-            # (FIFOs, devices) on both platforms.
-            flags |= os.O_NOFOLLOW
+        nofollow_flag = getattr(os, "O_NOFOLLOW", 0)
+        flags |= nofollow_flag
 
         try:
             fd = os.open(str(file_path), flags)
         except OSError as exc:
-            if exc.errno == getattr(errno, "ELOOP", None):
+            if (
+                (nofollow_flag and exc.errno == errno.ELOOP)
+                or find_filesystem_link(file_path) is not None
+            ):
                 raise ValueError(
-                    f"Symlink Markdown import paths are rejected: {file_path}"
+                    "Symlink Markdown import paths are rejected; symbolic links "
+                    f"and junctions are unsafe: {file_path}"
                 ) from exc
             raise
 
         try:
-            stat_res = os.fstat(fd)
-            if not stat.S_ISREG(stat_res.st_mode):
+            if find_filesystem_link(file_path) is not None:
+                raise ValueError(
+                    "Symlink Markdown import paths are rejected; symbolic links "
+                    f"and junctions are unsafe: {file_path}"
+                )
+            if not stat.S_ISREG(os.fstat(fd).st_mode):
                 raise ValueError(
                     f"Markdown import path is not a regular file: {file_path}"
                 )
-            with open(fd, "r", encoding="utf-8", closefd=True) as f:
-                return f.read()
-        except Exception:
-            try:
+            with os.fdopen(fd, mode="r", encoding="utf-8") as source:
+                fd = -1
+                return source.read()
+        finally:
+            if fd >= 0:
                 os.close(fd)
-            except OSError:
-                pass
-            raise
 
     def _read_markdown_path(self, path: Path) -> List[Tuple[str, str]]:
-        if path.is_symlink():
-            raise ValueError(f"Symlink Markdown import paths are rejected: {path}")
+        if find_filesystem_link(path) is not None:
+            raise ValueError(
+                "Symlink Markdown import paths are rejected; symbolic links and "
+                f"junctions are unsafe: {path}"
+            )
 
         if not path.exists():
             raise FileNotFoundError(f"Markdown import path does not exist: {path}")
 
         if path.is_dir():
-            file_paths = sorted(
-                (
-                    file_path
-                    for file_path in path.iterdir()
-                    if file_path.is_file()
-                    and not file_path.is_symlink()
-                    and file_path.suffix.lower() in self._MARKDOWN_EXTENSIONS
-                ),
-                key=lambda file_path: (file_path.name.casefold(), file_path.name),
-            )
+            file_paths = []
+            for file_path in path.iterdir():
+                if file_path.suffix.lower() not in self._MARKDOWN_EXTENSIONS:
+                    continue
+                if find_filesystem_link(file_path) is not None:
+                    continue
+                if file_path.is_file():
+                    file_paths.append(file_path)
+            if find_filesystem_link(path) is not None:
+                raise ValueError(
+                    "Symlink Markdown import paths are rejected; symbolic links "
+                    f"and junctions are unsafe: {path}"
+                )
+            file_paths.sort(key=lambda item: (item.name.casefold(), item.name))
         elif path.is_file():
             file_paths = [path]
         else:

tests/context/test_agent_memory_markdown.py

@@ -1,7 +1,11 @@
 import errno
+import os
+import stat
+import subprocess
 import sys
 from copy import deepcopy
 from datetime import datetime, timedelta, timezone
+from pathlib import Path
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -56,6 +60,25 @@ def markdown_document(frontmatter, body=""):
     return f"---\n{yaml_text}---\n\n{body}"
 
 
+def _require_symlink_support(tmp_path):
+    """Skip the test if this environment cannot create symbolic links.
+
+    Symlink creation can be unavailable even on POSIX (e.g. restricted
+    containers) and commonly requires elevated privilege or Developer Mode
+    on Windows. Probe actual capability instead of assuming based on
+    platform, so these tests still run wherever symlinks genuinely work.
+    """
+    probe_target = tmp_path / ".symlink_probe_target"
+    probe_link = tmp_path / ".symlink_probe_link"
+    probe_target.write_text("", encoding="utf-8")
+    try:
+        probe_link.symlink_to(probe_target)
+    except OSError as exc:
+        pytest.skip(f"environment cannot create symbolic links: {exc}")
+    probe_link.unlink()
+    probe_target.unlink()
+
+
 def required_frontmatter(memory_id="mem_test", **overrides):
     frontmatter = {
         "id": memory_id,
@@ -707,6 +730,151 @@ def test_markdown_string_path_inspection_errors_are_actionable():
     assert exc_info.value.__cause__ is original_error
 
 
+@pytest.mark.parametrize("use_string_path", [False, True])
+def test_markdown_import_rejects_symlinked_file(tmp_path, use_string_path):
+    _require_symlink_support(tmp_path)
+    outside = tmp_path / "outside.md"
+    outside.write_text(
+        markdown_document(required_frontmatter(), "Do not import"),
+        encoding="utf-8",
+    )
+    source = tmp_path / "memory.md"
+    source.symlink_to(outside)
+    payload = str(source) if use_string_path else source
+    memory = AgentMemory()
+
+    with pytest.raises(ValueError, match="symbolic link"):
+        memory.import_data(payload, format="markdown")
+
+    assert memory.count() == 0
+
+
+@pytest.mark.parametrize("use_string_path", [False, True])
+def test_markdown_import_rejects_broken_symlink(tmp_path, use_string_path):
+    _require_symlink_support(tmp_path)
+    source = tmp_path / "missing-memory.md"
+    source.symlink_to(tmp_path / "missing-target.md")
+    payload = str(source) if use_string_path else source
+    memory = AgentMemory()
+
+    with pytest.raises(ValueError, match="symbolic link"):
+        memory.import_data(payload, format="markdown")
+
+    assert memory.count() == 0
+
+
+@pytest.mark.parametrize("use_string_path", [False, True])
+def test_markdown_import_rejects_symlinked_directory(tmp_path, use_string_path):
+    _require_symlink_support(tmp_path)
+    outside = tmp_path / "outside"
+    outside.mkdir()
+    (outside / "memory.md").write_text(
+        markdown_document(required_frontmatter(), "Do not import"),
+        encoding="utf-8",
+    )
+    source = tmp_path / "memory-export"
+    source.symlink_to(outside, target_is_directory=True)
+    payload = str(source) if use_string_path else source
+    memory = AgentMemory()
+
+    with pytest.raises(ValueError, match="symbolic link"):
+        memory.import_data(payload, format="markdown")
+
+    assert memory.count() == 0
+
+
+def test_markdown_import_skips_symlinked_file_in_directory(tmp_path):
+    _require_symlink_support(tmp_path)
+    outside = tmp_path / "outside.md"
+    outside.write_text(
+        markdown_document(required_frontmatter(), "Do not import"),
+        encoding="utf-8",
+    )
+    source = tmp_path / "memory-export"
+    source.mkdir()
+    (source / "memory.md").symlink_to(outside)
+    memory = AgentMemory()
+
+    assert memory.import_data(source, format="markdown") == 0
+    assert memory.count() == 0
+
+
+@pytest.mark.skipif(not hasattr(os, "O_NOFOLLOW"), reason="requires O_NOFOLLOW")
+def test_markdown_import_does_not_follow_symlink_raced_before_open(tmp_path):
+    _require_symlink_support(tmp_path)
+    outside = tmp_path / "outside.md"
+    outside.write_text(
+        markdown_document(required_frontmatter(), "Do not import"),
+        encoding="utf-8",
+    )
+    source = tmp_path / "memory.md"
+    source.symlink_to(outside)
+
+    with patch.object(Path, "is_symlink", return_value=False):
+        with pytest.raises(ValueError, match="symbolic link"):
+            AgentMemory()._read_markdown_file_content(source)
+
+
+def test_markdown_import_rejects_windows_junction(tmp_path, monkeypatch):
+    source = tmp_path / "junction"
+    source.mkdir()
+    (source / "memory.md").write_text("not read", encoding="utf-8")
+
+    monkeypatch.setattr(
+        os.path,
+        "isjunction",
+        lambda candidate: Path(candidate) == source,
+        raising=False,
+    )
+
+    with pytest.raises(ValueError, match="junction"):
+        AgentMemory().import_data(source, format="markdown")
+
+
+def test_markdown_import_rejects_windows_reparse_point_fallback(tmp_path, monkeypatch):
+    source = tmp_path / "reparse-point"
+    source.mkdir()
+    real_lstat = os.lstat
+
+    class ReparseStat:
+        st_file_attributes = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0x400)
+
+    monkeypatch.delattr(os.path, "isjunction", raising=False)
+    monkeypatch.setattr(Path, "is_symlink", lambda self: False)
+    monkeypatch.setattr(
+        os,
+        "lstat",
+        lambda candidate: (
+            ReparseStat() if Path(candidate) == source else real_lstat(candidate)
+        ),
+    )
+
+    with pytest.raises(ValueError, match="junction"):
+        AgentMemory().import_data(source, format="markdown")
+
+
+@pytest.mark.skipif(os.name != "nt", reason="requires Windows junctions")
+def test_markdown_import_rejects_real_windows_junction(tmp_path):
+    outside = tmp_path / "outside"
+    outside.mkdir()
+    (outside / "memory.md").write_text("not read", encoding="utf-8")
+    source = tmp_path / "junction"
+    result = subprocess.run(
+        ["cmd.exe", "/c", "mklink", "/J", str(source), str(outside)],
+        capture_output=True,
+        text=True,
+        check=False,
+    )
+    if result.returncode != 0:
+        pytest.skip(f"could not create Windows junction: {result.stderr}")
+
+    try:
+        with pytest.raises(ValueError, match="junction"):
+            AgentMemory().import_data(source, format="markdown")
+    finally:
+        os.rmdir(source)
+
+
 def test_legacy_dict_import_behavior_is_unchanged():
     memory = AgentMemory()
     data = {
@@ -751,6 +919,7 @@ def test_markdown_import_file_open_security_rejects_symlink(tmp_path):
     with pytest.raises(ValueError, match="Symlink Markdown import paths are rejected"):
         memory._read_markdown_file_content(symlink_file)
 
+
 def test_markdown_import_public_api_rejects_symlink(tmp_path):
     """
     import_data(..., format="markdown") must propagate the symlink rejection