From cf4a38ade817aa5e82e9551609a3fbfdc1a620ee Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 29 Feb 2024 17:07:31 +0000 Subject: [PATCH] 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 Co-authored-by: Brett Cannon --- Doc/reference/import.rst | 8 +++--- Lib/importlib/_bootstrap_external.py | 2 +- Lib/test/support/os_helper.py | 27 +++++++++++++++++++ Lib/test/test_importlib/import_/test_path.py | 23 ++++++++++++++++ ...-02-29-16-55-52.gh-issue-115911.Vnkue_.rst | 3 +++ 5 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-02-29-16-55-52.gh-issue-115911.Vnkue_.rst diff --git a/Doc/reference/import.rst b/Doc/reference/import.rst index ac363e8cfa00dc..48fdd0f5d021c7 100644 --- 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 1b76328429f63a..f19a96f309bb7d 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1234,7 +1234,7 @@ def _path_importer_cache(cls, path): 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 891405943b78c5..66546ca1de537c 100644 --- 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 89b52fbd1e1aff..a5621bbbd4a163 100644 --- 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 @@ def test_deleted_cwd(self): # 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 index 00000000000000..717804be95b18b --- /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.