From bcd07916a3b1f73bb11345e6abd9b3c5bf3bcb6a Mon Sep 17 00:00:00 2001 From: Arnold Wender Date: Fri, 24 Apr 2026 11:06:30 +0200 Subject: [PATCH 1/3] 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 --- mempalace/config.py | 5 ++++- tests/test_config.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/mempalace/config.py b/mempalace/config.py index a9bcc7f..616334e 100644 --- a/mempalace/config.py +++ b/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 diff --git a/tests/test_config.py b/tests/test_config.py index e6dffc3..f41b3df 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -27,6 +27,42 @@ def test_env_override(): 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(): tmpdir = tempfile.mkdtemp() cfg = MempalaceConfig(config_dir=tmpdir) From 02a88b0864510bd20847beb2e48d2940cfe5a8da Mon Sep 17 00:00:00 2001 From: Arnold Wender Date: Fri, 24 Apr 2026 11:13:51 +0200 Subject: [PATCH 2/3] test(config): make palace_path tests portable across POSIX and Windows The new abspath+expanduser normalization means /env/palace no longer round-trips literally on Windows (abspath prepends the current drive, producing D:\env\palace). Rewrite the env-var tests to compare against os.path.abspath(os.path.expanduser(raw)) instead of hardcoded Unix strings, and build raw paths with os.path.join so backslash-vs-slash differences don't leak into assertions. Covers test_env_override, the three new tests, and the legacy-alias test in test_config_extra. --- tests/test_config.py | 33 ++++++++++++++++++++++----------- tests/test_config_extra.py | 8 ++++++-- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index f41b3df..9200214 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -21,31 +21,41 @@ def test_config_from_file(): def test_env_override(): - os.environ["MEMPALACE_PALACE_PATH"] = "/env/palace" - cfg = MempalaceConfig(config_dir=tempfile.mkdtemp()) - assert cfg.palace_path == "/env/palace" - del os.environ["MEMPALACE_PALACE_PATH"] + raw = "/env/palace" + os.environ["MEMPALACE_PALACE_PATH"] = raw + try: + cfg = MempalaceConfig(config_dir=tempfile.mkdtemp()) + # 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(): - os.environ["MEMPALACE_PALACE_PATH"] = "~/mempalace-test" + raw = os.path.join("~", "mempalace-test") + os.environ["MEMPALACE_PALACE_PATH"] = raw 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 == os.path.abspath(os.path.expanduser(raw)) 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" + # 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 == "/tmp/mempalace-test" + assert cfg.palace_path == expected finally: del os.environ["MEMPALACE_PALACE_PATH"] @@ -53,12 +63,13 @@ def test_env_path_abspath_collapses_traversal(): 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" + 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 ".." not in cfg.palace_path - assert cfg.palace_path == os.path.expanduser("~/mempalace-test") + assert cfg.palace_path == os.path.abspath(os.path.expanduser(raw)) finally: del os.environ["MEMPAL_PALACE_PATH"] diff --git a/tests/test_config_extra.py b/tests/test_config_extra.py index d0d9b5d..f7418c6 100644 --- a/tests/test_config_extra.py +++ b/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"] From ae1c52e43bd196b9673da169ae31c50fc9fb212c Mon Sep 17 00:00:00 2001 From: Arnold Wender Date: Fri, 24 Apr 2026 11:20:30 +0200 Subject: [PATCH 3/3] test(config): drop tilde-absence assertion for Windows 8.3 compatibility Windows 8.3 short paths legitimately contain tildes (e.g. the CI runner's USERPROFILE resolves to C:\Users\RUNNER~1\...), so asserting "~" is absent from the expanded path fails on Windows even when expanduser worked correctly. The equality check against os.path.abspath(os.path.expanduser()) is authoritative; drop the redundant absence heuristic. --- tests/test_config.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 9200214..824f6a8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -34,11 +34,14 @@ def test_env_override(): 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()) - # Tilde must be expanded to match the --palace CLI code path. assert cfg.palace_path == os.path.abspath(os.path.expanduser(raw)) assert cfg.palace_path.endswith("mempalace-test") finally: @@ -61,13 +64,15 @@ def test_env_path_abspath_collapses_traversal(): def test_env_path_legacy_alias_normalized(): - # Legacy MEMPAL_PALACE_PATH gets the same normalization treatment. + # 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 ".." not in cfg.palace_path assert cfg.palace_path == os.path.abspath(os.path.expanduser(raw)) finally: