Skip to content

fix(runtime-paths): make Windows runtime dir resolution NTFS-aware (closes #637)#738

Merged
pandas-studio merged 1 commit intomainfrom
fix/windows-runtime-paths-637
May 3, 2026
Merged

fix(runtime-paths): make Windows runtime dir resolution NTFS-aware (closes #637)#738
pandas-studio merged 1 commit intomainfrom
fix/windows-runtime-paths-637

Conversation

@pandas-studio
Copy link
Copy Markdown
Collaborator

Summary

  • Pre-fix, ensure_runtime_dir() was broken 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.
  • Make the Windows path explicit: runtime_dir() skips the XDG_RUNTIME_DIR branch 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 via GetSecurityInfo are 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.
  • Replace the three skipif(not hasattr(os, "geteuid")) guards on the POSIX-only test classes with skipif(os.name == "nt") — same effect today but parallel to the production guard.
  • Add TestWindowsRuntimeDir covering the NTFS-equivalent contract: tempdir-only resolution with uid=0 suffix, XDG ignored 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.

What changed

File Change
packages/memtomem/src/memtomem/_runtime_paths.py XDG branch gated on os.name != "nt"; mode-bit + owner-uid gates in ensure_runtime_dir gated on os.name != "nt"; module docstring rewritten to call out the asymmetric posture
packages/memtomem/tests/test_runtime_paths.py 3× skipif guard reworded; new TestWindowsRuntimeDir class (7 tests)

POSIX behaviour is untouched. _is_safe_dir is 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 motivates 0o700 on shared /tmp does not apply.
  • POSIX mode bits on NTFS are synthesized (0o666/0o777) and do not reflect the real ACL — checking & 0o077 is theatre and false-positives against any directory we ourselves just created.
  • Symlinks on Windows exist (require Developer Mode/admin to create) and are exploitable identically to POSIX symlinks once present, so the symlink rejection survives. Non-directory rejection survives for the same reason.
  • Proper NTFS owner-SID validation would need pywin32 / ctypes calls into GetSecurityInfo. 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 macOS
  • uv run pytest packages/memtomem/tests/test_uninstall_cmd.py packages/memtomem/tests/test_runtime_paths.py — 46 passed, 7 skipped (downstream runtime_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 — clean
  • uv run mypy packages/memtomem/src/memtomem/_runtime_paths.py — no issues
  • Windows CI matrix run will exercise the new TestWindowsRuntimeDir class — currently informational, should pass in this PR's run

References

🤖 Generated with Claude Code

Copy link
Copy Markdown
Owner

@memtomem memtomem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 TestWindowsRuntimeDir violate the repo's "one short line max" convention from CLAUDE.md — class docstring already covers the why.
  • Dead-code nit: hasattr(os, "geteuid") inside if os.name != "nt": (_runtime_paths.py:136) can drop.
  • requires_symlinks will 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.

…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]>
@memtomem memtomem force-pushed the fix/windows-runtime-paths-637 branch from 69c74f6 to 51224a3 Compare May 3, 2026 07:58
@pandas-studio pandas-studio merged commit 70e9749 into main May 3, 2026
9 checks passed
@pandas-studio pandas-studio deleted the fix/windows-runtime-paths-637 branch May 3, 2026 08:14
@github-actions github-actions Bot locked and limited conversation to collaborators May 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: replace POSIX file-mode XDG_RUNTIME_DIR tests with NTFS-aware equivalents

2 participants