Merge pull request #1166 from arnoldwender/fix/security-palace-path-env-normalize

fix(security): normalize MEMPALACE_PALACE_PATH env var with abspath+expanduser

mempalace/config.py

@@ -168,7 +168,10 @@ class MempalaceConfig:
         """Path to the memory palace data directory."""
         env_val = os.environ.get("MEMPALACE_PALACE_PATH") or os.environ.get("MEMPAL_PALACE_PATH")
         if env_val:
-            return env_val
+            # Normalize: expand ~ and collapse .. to match the CLI --palace
+            # code path (mcp_server.py:62) and prevent surprise redirection
+            # when the env var contains unresolved components.
+            return os.path.abspath(os.path.expanduser(env_val))
         return self._file_config.get("palace_path", DEFAULT_PALACE_PATH)
 
     @property

tests/test_config.py

@@ -21,12 +21,64 @@ def test_config_from_file():
 
 
 def test_env_override():
-    os.environ["MEMPALACE_PALACE_PATH"] = "/env/palace"
+    raw = "/env/palace"
+    os.environ["MEMPALACE_PALACE_PATH"] = raw
+    try:
         cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
-    assert cfg.palace_path == "/env/palace"
+        # palace_path normalizes with abspath + expanduser to match the
+        # --palace CLI code path. On Unix that's a no-op for "/env/palace";
+        # on Windows abspath prepends the current drive letter.
+        assert cfg.palace_path == os.path.abspath(os.path.expanduser(raw))
+    finally:
         del os.environ["MEMPALACE_PALACE_PATH"]
 
 
+def test_env_path_expanduser():
+    # Tilde must be expanded to match the --palace CLI code path. We don't
+    # assert "~" is absent from the final string because Windows 8.3 short
+    # paths (e.g. C:\Users\RUNNER~1\...) legitimately contain tildes — the
+    # equality check is authoritative.
+    raw = os.path.join("~", "mempalace-test")
+    os.environ["MEMPALACE_PALACE_PATH"] = raw
+    try:
+        cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
+        assert cfg.palace_path == os.path.abspath(os.path.expanduser(raw))
+        assert cfg.palace_path.endswith("mempalace-test")
+    finally:
+        del os.environ["MEMPALACE_PALACE_PATH"]
+
+
+def test_env_path_abspath_collapses_traversal():
+    # Build a raw path with a .. segment using the platform separator so
+    # the assertion is portable (Windows uses \, POSIX uses /).
+    raw = os.path.join(tempfile.gettempdir(), "palace", "..", "mempalace-test")
+    expected = os.path.abspath(os.path.expanduser(raw))
+    os.environ["MEMPALACE_PALACE_PATH"] = raw
+    try:
+        cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
+        # .. segments must be collapsed, not preserved literally.
+        assert ".." not in cfg.palace_path
+        assert cfg.palace_path == expected
+    finally:
+        del os.environ["MEMPALACE_PALACE_PATH"]
+
+
+def test_env_path_legacy_alias_normalized():
+    # Legacy MEMPAL_PALACE_PATH gets the same normalization treatment as
+    # MEMPALACE_PALACE_PATH. We don't assert "~" is absent from the final
+    # string because Windows 8.3 short paths (e.g. C:\Users\RUNNER~1\...)
+    # legitimately contain tildes — the equality check below is authoritative.
+    os.environ.pop("MEMPALACE_PALACE_PATH", None)
+    raw = os.path.join("~", "legacy-alias", "..", "mempalace-test")
+    os.environ["MEMPAL_PALACE_PATH"] = raw
+    try:
+        cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
+        assert ".." not in cfg.palace_path
+        assert cfg.palace_path == os.path.abspath(os.path.expanduser(raw))
+    finally:
+        del os.environ["MEMPAL_PALACE_PATH"]
+
+
 def test_init():
     tmpdir = tempfile.mkdtemp()
     cfg = MempalaceConfig(config_dir=tmpdir)

tests/test_config_extra.py

@@ -63,10 +63,14 @@ def test_save_people_map(tmp_path):
 def test_env_mempal_palace_path(tmp_path):
     """MEMPAL_PALACE_PATH (legacy) should also work."""
     os.environ.pop("MEMPALACE_PALACE_PATH", None)
-    os.environ["MEMPAL_PALACE_PATH"] = "/legacy/path"
+    raw = "/legacy/path"
+    os.environ["MEMPAL_PALACE_PATH"] = raw
     try:
         cfg = MempalaceConfig(config_dir=str(tmp_path))
-        assert cfg.palace_path == "/legacy/path"
+        # palace_path is normalized via abspath + expanduser — compare
+        # against the normalized form so the test is portable between
+        # POSIX (no-op) and Windows (prepends current drive letter).
+        assert cfg.palace_path == os.path.abspath(os.path.expanduser(raw))
     finally:
         del os.environ["MEMPAL_PALACE_PATH"]