gh-115911: Ignore PermissionError during import from cwd (#116131)

Ignore PermissionError when checking cwd during import

On macOS `getcwd(3)` can return EACCES if a path component isn't readable,
resulting in PermissionError. `PathFinder.find_spec()` now catches these and
ignores them - the same treatment as a missing/deleted cwd.

Introduces `test.support.os_helper.save_mode(path, ...)`, a context manager
that restores the mode of a path on exit.

This is allows finer control of exception handling and robust environment
restoration across platforms in `FinderTests.test_permission_error_cwd()`.

Co-authored-by: Jason R. Coombs <jaraco@jaraco.com>
Co-authored-by: Brett Cannon <brett@python.org>

diff --git a/Doc/reference/import.rst b/Doc/reference/import.rst
index ac363e8cfa00dc40956cbe92bd8fd6098a80b4c4..48fdd0f5d021c74f285354629ae138801de48a96 100644 (file)
--- a/Doc/reference/import.rst
+++ b/Doc/reference/import.rst
@@ -762,10 +762,10 @@ module.
 
 The current working directory -- denoted by an empty string -- is handled
 slightly differently from other entries on :data:`sys.path`. First, if the
-current working directory is found to not exist, no value is stored in
-:data:`sys.path_importer_cache`. Second, the value for the current working
-directory is looked up fresh for each module lookup. Third, the path used for
-:data:`sys.path_importer_cache` and returned by
+current working directory cannot be determined or is found not to exist, no
+value is stored in :data:`sys.path_importer_cache`. Second, the value for the
+current working directory is looked up fresh for each module lookup. Third,
+the path used for :data:`sys.path_importer_cache` and returned by
 :meth:`importlib.machinery.PathFinder.find_spec` will be the actual current
 working directory and not the empty string.
 

diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py
index 697f7c55218a8fcc71e1cbf22a6879a3a7adb150..8bcd741c446bd2745fceb591b8f5f227885a8dbe 100644 (file)
--- a/Lib/importlib/_bootstrap_external.py
+++ b/Lib/importlib/_bootstrap_external.py
@@ -1244,7 +1244,7 @@ class PathFinder:
         if path == '':
             try:
                 path = _os.getcwd()
-            except FileNotFoundError:
+            except (FileNotFoundError, PermissionError):
                 # Don't cache the failure as the cwd can easily change to
                 # a valid directory later on.
                 return None

diff --git a/Lib/test/support/os_helper.py b/Lib/test/support/os_helper.py
index 8071c248b9b67ef0ebf3f36ed5ed748e82f55708..15dcdc9b1fddfb754db583b772d59fc74154d715 100644 (file)
--- a/Lib/test/support/os_helper.py
+++ b/Lib/test/support/os_helper.py
@@ -294,6 +294,33 @@ def skip_unless_working_chmod(test):
     return test if ok else unittest.skip(msg)(test)
 
 
+@contextlib.contextmanager
+def save_mode(path, *, quiet=False):
+    """Context manager that restores the mode (permissions) of *path* on exit.
+
+    Arguments:
+
+      path: Path of the file to restore the mode of.
+
+      quiet: if False (the default), the context manager raises an exception
+        on error.  Otherwise, it issues only a warning and keeps the current
+        working directory the same.
+
+    """
+    saved_mode = os.stat(path)
+    try:
+        yield
+    finally:
+        try:
+            os.chmod(path, saved_mode.st_mode)
+        except OSError as exc:
+            if not quiet:
+                raise
+            warnings.warn(f'tests may fail, unable to restore the mode of '
+                          f'{path!r} to {saved_mode.st_mode}: {exc}',
+                          RuntimeWarning, stacklevel=3)
+
+
 # Check whether the current effective user has the capability to override
 # DAC (discretionary access control). Typically user root is able to
 # bypass file read, write, and execute permission checks. The capability

diff --git a/Lib/test/test_importlib/import_/test_path.py b/Lib/test/test_importlib/import_/test_path.py
index bcd5ad6e76005ac8c787542b0753c7df9ff82c55..51ff6115e1281eda5f46431981f3cb3666a0f9c0 100644 (file)
--- a/Lib/test/test_importlib/import_/test_path.py
+++ b/Lib/test/test_importlib/import_/test_path.py
@@ -1,3 +1,4 @@
+from test.support import os_helper
 from test.test_importlib import util
 
 importlib = util.import_importlib('importlib')
@@ -153,6 +154,28 @@ class FinderTests:
             # Do not want FileNotFoundError raised.
             self.assertIsNone(self.machinery.PathFinder.find_spec('whatever'))
 
+    @os_helper.skip_unless_working_chmod
+    def test_permission_error_cwd(self):
+        # gh-115911: Test that an unreadable CWD does not break imports, in
+        # particular during early stages of interpreter startup.
+        with (
+            os_helper.temp_dir() as new_dir,
+            os_helper.save_mode(new_dir),
+            os_helper.change_cwd(new_dir),
+            util.import_state(path=['']),
+        ):
+            # chmod() is done here (inside the 'with' block) because the order
+            # of teardown operations cannot be the reverse of setup order. See
+            # https://github.com/python/cpython/pull/116131#discussion_r1739649390
+            try:
+                os.chmod(new_dir, 0o000)
+            except OSError:
+                self.skipTest("platform does not allow "
+                              "changing mode of the cwd")
+
+            # Do not want PermissionError raised.
+            self.assertIsNone(self.machinery.PathFinder.find_spec('whatever'))
+
     def test_invalidate_caches_finders(self):
         # Finders with an invalidate_caches() method have it called.
         class FakeFinder:

diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-02-29-16-55-52.gh-issue-115911.Vnkue_.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-02-29-16-55-52.gh-issue-115911.Vnkue_.rst
new file mode 100644 (file)
index 0000000..717804b
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-02-29-16-55-52.gh-issue-115911.Vnkue_.rst
@@ -0,0 +1,3 @@
+If the current working directory cannot be determined due to permissions,
+then import will no longer raise :exc:`PermissionError`. Patch by Alex
+Willmer.