diff --git a/certbot/certbot/compat/filesystem.py b/certbot/certbot/compat/filesystem.py index e44e1b32a..5a1c2c874 100644 --- a/certbot/certbot/compat/filesystem.py +++ b/certbot/certbot/compat/filesystem.py @@ -237,8 +237,19 @@ def makedirs(file_path, mode=0o777): will be applied if ``None`` """ if POSIX_MODE: - return os.makedirs(file_path, mode) + # Since Python 3.7, os.makedirs does not set the given mode to the intermediate directories + # that could be created in the process. To keep things safe and consistent on all + # Python versions, we set the umask accordingly to have all directories (intermediate and + # leaf) created with the given mode. + current_umask = os.umask(0) + try: + os.umask(current_umask | 0o777 ^ mode) + return os.makedirs(file_path, mode) + finally: + os.umask(current_umask) + # TODO: Windows does not support umask. A specific PR (#7967) is handling this, and will need + # to add appropriate umask call for the Windows part of the logic below. orig_mkdir_fn = os.mkdir try: # As we know that os.mkdir is called internally by os.makedirs, we will swap the function in diff --git a/certbot/tests/compat/filesystem_test.py b/certbot/tests/compat/filesystem_test.py index 3f6a5ec1d..b20ee0e54 100644 --- a/certbot/tests/compat/filesystem_test.py +++ b/certbot/tests/compat/filesystem_test.py @@ -1,6 +1,7 @@ """Tests for certbot.compat.filesystem""" import contextlib import errno +import stat import unittest try: @@ -280,6 +281,26 @@ class WindowsMkdirTests(test_util.TempDirTestCase): self.assertEqual(original_mkdir, std_os.mkdir) +# TODO: This test can be used both by Linux and Windows once on #7967 +@unittest.skipUnless(POSIX_MODE, reason='Needs umask to succeed, and Windows does not have it') +class LinuxMkdirTests(test_util.TempDirTestCase): + """Unit tests for Linux mkdir + makedirs functions in filesystem module""" + def test_makedirs_correct_permissions(self): + path = os.path.join(self.tempdir, 'dir') + subpath = os.path.join(path, 'subpath') + + previous_umask = os.umask(0o022) + + try: + filesystem.makedirs(subpath, 0o700) + + import os as std_os # pylint: disable=os-module-forbidden + assert stat.S_IMODE(std_os.stat(path).st_mode) == 0o700 + assert stat.S_IMODE(std_os.stat(subpath).st_mode) == 0o700 + finally: + os.umask(previous_umask) + + class CopyOwnershipAndModeTest(test_util.TempDirTestCase): """Tests about copy_ownership_and_apply_mode, copy_ownership_and_mode and has_same_ownership""" def setUp(self):