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

MEMPALACE_PALACE_PATH (and legacy MEMPAL_PALACE_PATH) read from the
environment was returned as-is from Config.palace_path, while the
sibling --palace CLI path gets os.path.abspath() applied at
mcp_server.py:62. That inconsistency means env-var callers can end
up with literal '~' or unresolved '..' segments in the path, which
(a) breaks user intuition and (b) lets a caller who can set env vars
on the target user's session redirect palace storage to an
unexpected location.

Apply os.path.abspath(os.path.expanduser(...)) to the env-var branch
so both code paths converge on the same resolved absolute path.

Closes #1163
This commit is contained in:
Arnold Wender
2026-04-24 11:06:30 +02:00
parent 8ac98f038c
commit bcd07916a3
2 changed files with 40 additions and 1 deletions
+4 -1
View File
@@ -168,7 +168,10 @@ class MempalaceConfig:
"""Path to the memory palace data directory.""" """Path to the memory palace data directory."""
env_val = os.environ.get("MEMPALACE_PALACE_PATH") or os.environ.get("MEMPAL_PALACE_PATH") env_val = os.environ.get("MEMPALACE_PALACE_PATH") or os.environ.get("MEMPAL_PALACE_PATH")
if env_val: 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) return self._file_config.get("palace_path", DEFAULT_PALACE_PATH)
@property @property
+36
View File
@@ -27,6 +27,42 @@ def test_env_override():
del os.environ["MEMPALACE_PALACE_PATH"] del os.environ["MEMPALACE_PALACE_PATH"]
def test_env_path_expanduser():
os.environ["MEMPALACE_PALACE_PATH"] = "~/mempalace-test"
try:
cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
# Tilde must be expanded to match the --palace CLI code path.
assert "~" not in cfg.palace_path
assert cfg.palace_path.endswith("mempalace-test")
assert cfg.palace_path == os.path.expanduser("~/mempalace-test")
finally:
del os.environ["MEMPALACE_PALACE_PATH"]
def test_env_path_abspath_collapses_traversal():
os.environ["MEMPALACE_PALACE_PATH"] = "/tmp/palace/../mempalace-test"
try:
cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
# .. segments must be collapsed, not preserved literally.
assert ".." not in cfg.palace_path
assert cfg.palace_path == "/tmp/mempalace-test"
finally:
del os.environ["MEMPALACE_PALACE_PATH"]
def test_env_path_legacy_alias_normalized():
# Legacy MEMPAL_PALACE_PATH gets the same normalization treatment.
os.environ.pop("MEMPALACE_PALACE_PATH", None)
os.environ["MEMPAL_PALACE_PATH"] = "~/legacy-alias/../mempalace-test"
try:
cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
assert "~" not in cfg.palace_path
assert ".." not in cfg.palace_path
assert cfg.palace_path == os.path.expanduser("~/mempalace-test")
finally:
del os.environ["MEMPAL_PALACE_PATH"]
def test_init(): def test_init():
tmpdir = tempfile.mkdtemp() tmpdir = tempfile.mkdtemp()
cfg = MempalaceConfig(config_dir=tmpdir) cfg = MempalaceConfig(config_dir=tmpdir)