fix(runtime-paths): make Windows runtime dir resolution NTFS-aware (closes #637)#738
Merged
pandas-studio merged 1 commit intomainfrom May 3, 2026
Merged
Conversation
memtomem
approved these changes
May 3, 2026
Owner
memtomem
left a comment
There was a problem hiding this comment.
Fix is correct and well-scoped — the idempotency regression guard pins the exact failure mode #637 reported, and the trust-posture trade-off (drop synthesized-mode-bit checks, keep symlink + non-directory rejection) is documented in the module docstring so a future contributor can't silently re-enable a check that has no NTFS semantics. Approving.
Optional follow-ups (non-blocking):
- Multi-line test docstrings in
TestWindowsRuntimeDirviolate the repo's "one short line max" convention from CLAUDE.md — class docstring already covers the why. - Dead-code nit:
hasattr(os, "geteuid")insideif os.name != "nt":(_runtime_paths.py:136) can drop. requires_symlinkswill auto-skip on default Windows runners, leaving the only remaining safety gate effectively untested in CI most of the time. Worth a mock-based unit test to exercise the symlink rejection platform-independently.
3 tasks
…loses #637) Pre-fix, ``ensure_runtime_dir`` would silently break on Windows from the second invocation onward: NTFS synthesizes POSIX mode bits as ``0o666`` / ``0o777``, so ``stat.S_IMODE(st.st_mode) & 0o077`` was always non-zero and the validator raised ``PermissionError("unsafe permissions")`` against a directory it had just created. The XDG gate had a parallel issue — ``_is_safe_dir`` would reject any ``XDG_RUNTIME_DIR`` candidate on Windows for the same mode-bit reason, so the silent fall-through to the tempdir form happened by accident rather than by design. This PR makes the Windows path explicit: - ``runtime_dir()`` skips the ``XDG_RUNTIME_DIR`` branch entirely on Windows. The variable is a Linux/systemd convention and honoring it on Windows would require validating the base without POSIX mode bits — half-baked. ``%LOCALAPPDATA%\Temp\memtomem-0`` is the single resolved path on every Windows runtime. - ``ensure_runtime_dir()`` keeps the symlink and non-directory checks on every platform but skips the mode-bit + owner-uid gates on Windows. ``%LOCALAPPDATA%\Temp\`` is already per-user, so the cross-user threat that motivates the POSIX gates does not apply. Proper NTFS owner-SID validation needs ``pywin32`` / ``ctypes`` ``GetSecurityInfo`` calls and is intentionally out of scope. - Module docstring documents the asymmetric trust posture so a future contributor doesn't re-enable a check that has no NTFS semantics. Tests: - The three skipif-on-Windows classes (``TestRuntimeDir``, ``TestEnsureRuntimeDir``, ``TestServerPidPath``) switch their guard from ``not hasattr(os, "geteuid")`` to ``os.name == "nt"``. Same effect today, but the new wording is parallel to the production guard and stops being a coincidence. - New ``TestWindowsRuntimeDir`` class pins the NTFS-equivalent contract: tempdir-only resolution with ``uid=0`` suffix, XDG ignored even when set, idempotent ``ensure_runtime_dir`` (regression guard for the mode-bit failure), and the symlink + non-directory rejections that survive the NTFS relaxation. Symlink test uses the existing ``requires_symlinks`` marker so it auto-skips when the Windows runner doesn't have Developer Mode / admin. POSIX behaviour is untouched — ``ruff`` + full ``test_runtime_paths.py`` + downstream consumers (``test_uninstall_cmd``, ``test_web_routes_context``, ``test_context_skills``) all green on macOS. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
69c74f6 to
51224a3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ensure_runtime_dir()was broken on Windows from the second invocation onward: NTFS synthesizes POSIX mode bits as0o666/0o777, sostat.S_IMODE(st.st_mode) & 0o077was always non-zero and the validator raisedPermissionError("unsafe permissions")against a directory it had just created.runtime_dir()skips theXDG_RUNTIME_DIRbranch on Windows (it's a Linux/systemd convention, and honoring it without the POSIX mode-bit gate is half-baked);ensure_runtime_dir()keeps the symlink + non-directory rejections on every platform but skips the mode-bit + owner-uid gates on Windows (%LOCALAPPDATA%\Temp\is already per-user, and proper NTFS owner-SID checks viaGetSecurityInfoare out of scope).skipif(not hasattr(os, "geteuid"))guards on the POSIX-only test classes withskipif(os.name == "nt")— same effect today but parallel to the production guard.TestWindowsRuntimeDircovering the NTFS-equivalent contract: tempdir-only resolution withuid=0suffix, XDG ignored when set, idempotentensure_runtime_dir(regression guard for the mode-bit failure), and the symlink + non-directory rejections that survive the NTFS relaxation. Symlink test uses the existingrequires_symlinksmarker so it auto-skips when the Windows runner doesn't have Developer Mode/admin.What changed
packages/memtomem/src/memtomem/_runtime_paths.pyos.name != "nt"; mode-bit + owner-uid gates inensure_runtime_dirgated onos.name != "nt"; module docstring rewritten to call out the asymmetric posturepackages/memtomem/tests/test_runtime_paths.pyTestWindowsRuntimeDirclass (7 tests)POSIX behaviour is untouched.
_is_safe_diris now POSIX-only by contract — its docstring says so.Why these gates can be relaxed on Windows
tempfile.gettempdir()on Windows resolves to%LOCALAPPDATA%\Temp\— already a per-user directory. The cross-user threat that motivates0o700on shared/tmpdoes not apply.0o666/0o777) and do not reflect the real ACL — checking& 0o077is theatre and false-positives against any directory we ourselves just created.pywin32/ctypescalls intoGetSecurityInfo. Heavyweight optional dependency for marginal value when the temp base is already per-user — explicitly out of scope.Test plan
uv run pytest packages/memtomem/tests/test_runtime_paths.py -v— 18 passed, 7 skipped on macOSuv run pytest packages/memtomem/tests/test_uninstall_cmd.py packages/memtomem/tests/test_runtime_paths.py— 46 passed, 7 skipped (downstreamruntime_dir()consumer)uv run pytest packages/memtomem/tests/test_web_routes_context.py packages/memtomem/tests/test_context_skills.py— 86 passed (other consumers)uv run ruff check+ruff format --check— cleanuv run mypy packages/memtomem/src/memtomem/_runtime_paths.py— no issuesTestWindowsRuntimeDirclass — currently informational, should pass in this PR's runReferences
Path.home()on Windows (uses USERPROFILE) #644), fix(config): support Windows backslash paths in categorize_memory_dir (#316) #716 (Windows: wizard fails to auto-detect Claude/Codex memory dirs (backslash path regex) #316 backslash regex), test: skipif POSIX-only assertions on Windows (mode bits, fcntl, signal) — Windows CI PR 2/N #713 (POSIX skipifs), test: normalize path separators in 7 cross-platform assertions (#643) #711 (path separator normalization)🤖 Generated with Claude Code