diff --git a/tests/tools/test_local_cwd_permission_fallback.py b/tests/tools/test_local_cwd_permission_fallback.py new file mode 100644 index 00000000000..f6d9bfc6c05 --- /dev/null +++ b/tests/tools/test_local_cwd_permission_fallback.py @@ -0,0 +1,83 @@ +"""Regression tests for inaccessible-cwd fallback (#65583). + +``/root`` leaking into a non-root gateway/cron process's terminal cwd used to +kill every command with ``PermissionError: [Errno 13] Permission denied: +'/root'`` — ``os.path.isdir('/root')`` is True for a non-root user (stat only +needs search permission on ``/``), so the old existence-only check in +``_resolve_safe_cwd`` happily returned a directory ``subprocess.Popen`` could +not enter. The fix checks X_OK and falls back to the nearest usable ancestor. +""" + +import os +import sys +import tempfile + +import pytest + +from tools.environments.local import LocalEnvironment, _cwd_usable, _resolve_safe_cwd + + +@pytest.fixture +def denied_dir(tmp_path): + """A directory that exists but cannot be entered (simulates /root).""" + d = tmp_path / "rootlike" + d.mkdir() + d.chmod(0) + yield d + d.chmod(0o755) # so pytest can clean up + + +needs_posix_perms = pytest.mark.skipif( + sys.platform == "win32" or os.geteuid() == 0, + reason="chmod-based access denial needs POSIX + non-root", +) + + +@needs_posix_perms +class TestInaccessibleCwdFallback: + def test_cwd_usable_rejects_unenterable_directory(self, denied_dir): + assert os.path.isdir(denied_dir) # the trap: stat succeeds + assert _cwd_usable(str(denied_dir)) is False + + def test_resolve_safe_cwd_falls_back_from_denied_dir(self, denied_dir, tmp_path): + resolved = _resolve_safe_cwd(str(denied_dir)) + assert resolved != str(denied_dir) + assert os.access(resolved, os.X_OK) + # Nearest usable ancestor is the tmp_path parent, not a random tempdir. + assert resolved == str(tmp_path) + + def test_resolve_safe_cwd_climbs_past_denied_ancestor(self, denied_dir, tmp_path): + missing_child = str(denied_dir / "sub" / "dir") + resolved = _resolve_safe_cwd(missing_child) + assert os.access(resolved, os.X_OK) + assert resolved == str(tmp_path) + + def test_local_environment_survives_denied_cwd(self, denied_dir): + """The #65583 shape: env constructed with an unenterable cwd must + still execute commands instead of raising PermissionError.""" + env = LocalEnvironment(cwd=str(denied_dir), timeout=30) + try: + handle = env._run_bash("echo ALIVE") + out = handle.stdout.read() if handle.stdout else b"" + rc = handle.wait(timeout=15) + assert rc == 0 + assert b"ALIVE" in (out if isinstance(out, bytes) else out.encode()) + finally: + try: + env.cleanup() + except Exception: + pass + + +class TestUsableCwdBehaviorUnchanged: + def test_existing_accessible_cwd_returned_verbatim(self, tmp_path): + assert _resolve_safe_cwd(str(tmp_path)) == str(tmp_path) + + def test_missing_cwd_still_climbs_to_existing_ancestor(self, tmp_path): + missing = str(tmp_path / "gone" / "deeper") + assert _resolve_safe_cwd(missing) == str(tmp_path) + + def test_hopeless_path_falls_back_to_tempdir(self): + # A path whose every component is missing outside any real tree. + resolved = _resolve_safe_cwd("") + assert resolved == tempfile.gettempdir() diff --git a/tools/environments/local.py b/tools/environments/local.py index 37e760519a4..8b4450c7201 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -136,11 +136,27 @@ def _quote_bash_path(path: str) -> str: return shlex.quote(_bash_safe_path(path)) +def _cwd_usable(path: str) -> bool: + """True when *path* is a directory this process can actually chdir into. + + ``os.path.isdir`` alone is not enough: stat() on ``/root`` succeeds for a + non-root user (only ``/`` needs search permission), but + ``subprocess.Popen(cwd='/root')`` then dies with ``PermissionError: + [Errno 13] Permission denied: '/root'``. Seen in the wild when a + root-launched CLI session leaks ``/root`` into shared state that a + non-root gateway/cron process later reads (#65583) — every cron job's + terminal/file tool then fails on every command, forever. Checking + X_OK up front lets the caller fall back instead. + """ + return os.path.isdir(path) and os.access(path, os.X_OK) + + def _resolve_safe_cwd(cwd: str) -> str: - """Return ``cwd`` if it exists as a directory, else the nearest existing - ancestor. Falls back to ``tempfile.gettempdir()`` only if walking up the - path can't find any existing directory (effectively never on a healthy - filesystem, but cheap belt-and-braces). + """Return ``cwd`` if it exists as a directory this process can enter, + else the nearest existing accessible ancestor. Falls back to + ``tempfile.gettempdir()`` only if walking up the path can't find any + usable directory (effectively never on a healthy filesystem, but cheap + belt-and-braces). On Windows, also normalizes Git Bash / MSYS-style POSIX paths (``/c/Users/x``) to native Windows form before the isdir check so a @@ -149,16 +165,27 @@ def _resolve_safe_cwd(cwd: str) -> str: Used by ``_run_bash`` to recover when the configured cwd is gone — most commonly because a previous tool call deleted its own working directory - (issue #17558). Without this guard, ``subprocess.Popen(..., cwd=...)`` - raises ``FileNotFoundError`` before bash starts, wedging every subsequent - terminal call until the gateway restarts. + (issue #17558) — or inaccessible to this user, e.g. ``/root`` leaking + from a root-launched CLI session into a non-root gateway's cron jobs + (issue #65583). Without this guard, ``subprocess.Popen(..., cwd=...)`` + raises ``FileNotFoundError``/``PermissionError`` before bash starts, + wedging every subsequent terminal call until the gateway restarts. """ cwd = _msys_to_windows_path(cwd) if _IS_WINDOWS else cwd - if cwd and os.path.isdir(cwd): + if cwd and _cwd_usable(cwd): return cwd + if cwd and os.path.isdir(cwd): + logger.warning( + "Configured terminal cwd %r exists but is not accessible to " + "this user (uid=%s) — falling back to the nearest usable " + "directory. If this is a gateway/cron process, check for " + "root-owned paths leaking into terminal.cwd / TERMINAL_CWD " + "(#65583).", + cwd, getattr(os, "getuid", lambda: "?")(), + ) parent = os.path.dirname(cwd) if cwd else "" while parent: - if os.path.isdir(parent): + if _cwd_usable(parent): return parent next_parent = os.path.dirname(parent) if next_parent == parent: