From f454cefe56f96b502f8b0c754b6dfe977851b2c3 Mon Sep 17 00:00:00 2001 From: goodboy Date: Fri, 14 Aug 2026 18:40:23 -0400 Subject: [PATCH] Enforce `get_rt_dir()` ownership on POSIX Linux previously accepted a pre-existing runtime bindspace without checking its owner or mode, even though Darwin enforced both. Share the POSIX directory guard so every managed root and subdir rejects non-directories and foreign UIDs before changing permissions. Normalize owner-controlled bindspaces to `0o700` and add Linux regressions for mode repair, foreign ownership, and non-directory paths. Review: PR #480 (goodboy) https://github.com/goodboy/tractor/pull/480 (this patch was generated in some part by `opencode` using `gpt-5.6-sol` (`openai`)) --- tests/ipc/test_each_tpt.py | 67 ++++++++++++++++++++- tractor/runtime/_state.py | 115 +++++++++++++++++++++---------------- 2 files changed, 130 insertions(+), 52 deletions(-) diff --git a/tests/ipc/test_each_tpt.py b/tests/ipc/test_each_tpt.py index b62e2341..512300a6 100644 --- a/tests/ipc/test_each_tpt.py +++ b/tests/ipc/test_each_tpt.py @@ -194,7 +194,10 @@ def test_rt_dir_rejects_non_directory( lambda appname: str(rt_file), ) - with pytest.raises(FileExistsError): + with pytest.raises( + PermissionError, + match='Unsafe POSIX', + ): _state.get_rt_dir() new_rt_dir: Path = tmp_path / 'new-runtime-dir' @@ -206,6 +209,68 @@ def test_rt_dir_rejects_non_directory( assert stat.S_IMODE(new_rt_dir.stat().st_mode) == 0o700 +def test_linux_rt_dir_secures_existing_path( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +): + ''' + Enforce owner-only access on an existing Linux runtime directory. + + Linux previously accepted any existing directory returned by + `platformdirs`, without checking ownership or correcting a + traversable mode. This test creates an owner-controlled `0o755` + directory and proves `get_rt_dir()` normalizes the managed + bindspace to `0o700` before returning it. + + ''' + rt_dir: Path = tmp_path / 'tractor' + rt_dir.mkdir(mode=0o755) + monkeypatch.setattr(sys, 'platform', 'linux') + monkeypatch.setattr( + 'platformdirs.user_runtime_dir', + lambda appname: str(rt_dir), + ) + + assert _state.get_rt_dir() == rt_dir + assert stat.S_IMODE(rt_dir.stat().st_mode) == 0o700 + + +def test_linux_rt_dir_rejects_foreign_owner( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +): + ''' + Reject an existing Linux runtime directory owned by another UID. + + A pre-created bindspace must never be made private with `chmod` + until ownership is verified. This test makes the current process + appear to have a different UID and proves `get_rt_dir()` rejects + the directory without changing its original mode. + + ''' + rt_dir: Path = tmp_path / 'tractor' + rt_dir.mkdir(mode=0o755) + original_mode: int = stat.S_IMODE(rt_dir.stat().st_mode) + monkeypatch.setattr(sys, 'platform', 'linux') + monkeypatch.setattr( + 'platformdirs.user_runtime_dir', + lambda appname: str(rt_dir), + ) + monkeypatch.setattr( + os, + 'getuid', + lambda: rt_dir.stat().st_uid + 1, + ) + + with pytest.raises( + PermissionError, + match='Unsafe POSIX', + ): + _state.get_rt_dir() + + assert stat.S_IMODE(rt_dir.stat().st_mode) == original_mode + + def test_macos_rt_dir_rejects_intermediate_symlink( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, diff --git a/tractor/runtime/_state.py b/tractor/runtime/_state.py index 38b64260..5caa92e3 100644 --- a/tractor/runtime/_state.py +++ b/tractor/runtime/_state.py @@ -323,6 +323,60 @@ def current_ipc_ctx( +def _ensure_owner_only_posix_dir( + path: Path, + *, + parents: bool = False, +) -> None: + ''' + Create or validate a UID-owned POSIX runtime directory. + + Pre-existing directories are accepted only when owned by the + current user. Their mode is normalized to `0o700` because runtime + directories hold IPC sockets and are private bindspaces. + + ''' + # TODO: https://github.com/goodboy/tractor/issues/494 + # Research having the actor-tree root process choose and create + # this bindspace, then propagate it to every subactor. On + # Linux, a private mount namespace could isolate it while letting + # spawned subactors inherit access; independently launched + # discovery clients would need an explicit join or fallback path. + # POSIX metadata alone records UID/GID ownership, so other systems + # still need explicit runtime metadata and lifecycle management. + try: + dir_stat: os.stat_result = path.lstat() + except FileNotFoundError: + try: + path.mkdir( + mode=0o700, + parents=parents, + ) + except FileExistsError: + pass + dir_stat = path.lstat() + + if ( + not stat.S_ISDIR(dir_stat.st_mode) + or + dir_stat.st_uid != os.getuid() + ): + platform_name: str = ( + 'Darwin' + if sys.platform == 'darwin' + else 'POSIX' + ) + raise PermissionError( + f'Unsafe {platform_name} runtime directory!\n' + f'path: {path}\n' + f'owner uid: {dir_stat.st_uid}\n' + f'mode: {stat.filemode(dir_stat.st_mode)}\n' + ) + + if stat.S_IMODE(dir_stat.st_mode) != 0o700: + path.chmod(0o700) + + def get_rt_dir( subdir: str|Path|None = None, appname: str = 'tractor', @@ -332,9 +386,9 @@ def get_rt_dir( userspace apps stick their IPC and cache related system util-files. - Linux uses `${XDG_RUNTIME_DIR}/tractor/`; Darwin uses a short, - owner-only `/tmp/tractor-` path; other platforms use the - lovely `platformdirs` lib. + Linux uses an owner-only `${XDG_RUNTIME_DIR}/tractor/`; Darwin + uses a short, owner-only `/tmp/tractor-` path; other + platforms use the lovely `platformdirs` lib. ''' # lazy-imported to keep it off the eager @@ -349,29 +403,6 @@ def get_rt_dir( _DARWIN_TMPDIR / f'{appname}-{os.getuid()}' ) - try: - rt_stat: os.stat_result = rt_root.lstat() - except FileNotFoundError: - try: - rt_root.mkdir(mode=0o700) - except FileExistsError: - pass - rt_stat = rt_root.lstat() - - if ( - not stat.S_ISDIR(rt_stat.st_mode) - or - rt_stat.st_uid != os.getuid() - ): - raise PermissionError( - f'Unsafe Darwin runtime directory!\n' - f'path: {rt_root}\n' - f'owner uid: {rt_stat.st_uid}\n' - f'mode: {stat.filemode(rt_stat.st_mode)}\n' - ) - if stat.S_IMODE(rt_stat.st_mode) != 0o700: - rt_root.chmod(0o700) - rt_dir: Path = rt_root else: rt_dir = Path( @@ -401,7 +432,7 @@ def get_rt_dir( f'{subdir!r}\n' ) - if rt_root is None: + if os.name != 'posix': if subdir_path is not None: rt_dir = rt_dir / subdir_path if not rt_dir.is_dir(): @@ -414,33 +445,15 @@ def get_rt_dir( ) return rt_dir + _ensure_owner_only_posix_dir( + rt_dir, + parents=(rt_root is None), + ) + if subdir_path is not None: for part in subdir_path.parts: rt_dir = rt_dir / part - try: - dir_stat: os.stat_result = rt_dir.lstat() - except FileNotFoundError: - try: - # Every Darwin component is private so no other - # user can replace descendants below `rt_root`. - rt_dir.mkdir(mode=0o700) - except FileExistsError: - pass - dir_stat = rt_dir.lstat() - - if ( - not stat.S_ISDIR(dir_stat.st_mode) - or - dir_stat.st_uid != os.getuid() - ): - raise PermissionError( - f'Unsafe Darwin runtime directory!\n' - f'path: {rt_dir}\n' - f'owner uid: {dir_stat.st_uid}\n' - f'mode: {stat.filemode(dir_stat.st_mode)}\n' - ) - if stat.S_IMODE(dir_stat.st_mode) != 0o700: - rt_dir.chmod(0o700) + _ensure_owner_only_posix_dir(rt_dir) return rt_dir